[Letux-kernel] [PATCH 00/20] A bunch of JZ4730 fixups for letux-kernel

[Paul Boddie]

Nikolaus,

On Monday, 21 December 2020 09:25:38 CET H. Nikolaus Schaller wrote:
> 
> Looking into the code of irq_set_type() there is something suspect:
> 
> static void ingenic_gpio_set_bit(struct ingenic_gpio_chip *jzgc,
> 		u8 reg, u8 offset, bool set)
> 
> ...
> 
> static void irq_set_type(struct ingenic_gpio_chip *jzgc,
> ...
> 	} else if (jzgc->jzpc->info->version == ID_JZ4730) {
> 		ingenic_gpio_set_bit(jzgc, offset, JZ4730_GPIO_GPDIR, false);
> 		ingenic_gpio_set_bits(jzgc, offset,
> 					JZ4730_GPIO_GPIDUR, JZ4730_GPIO_GPIDLR,
> 					(val2 ? 2 : 0) | (val1 ? 1 : 0));
> 		return;
> ...
> 	if (jzgc->jzpc->info->version >= ID_X1000) {
> 		ingenic_gpio_shadow_set_bit(jzgc, reg2, offset, val1);
> 		ingenic_gpio_shadow_set_bit(jzgc, reg1, offset, val2);
> 		ingenic_gpio_shadow_set_bit_load(jzgc);
> 	} else {
> 		ingenic_gpio_set_bit(jzgc, reg2, offset, val1);
> 		ingenic_gpio_set_bit(jzgc, reg1, offset, val2);
> 	}
> 
> So for the jz4730 we pass the register number(s) as 3rd argument,
> while for the other SoC we pass as 2nd. And offset is the other one.

Yes, this is definitely incorrect. Well done for noticing this! It just shows 
that such things are easy to miss when very tired (and when the same types are 
used for two different things, assuming that C would even care otherwise).

> What I don't understand is how this can wipe out the full register...
> 
> But I have tried to swap the offset behind the registers and the result is:
> 
> [    2.539027] Waiting for root device /dev/mmcblk1p2...
> [    2.537757] mmc_gpio_get_ro
> [    2.540796] ingenic_gpio_get: off=2
> [    2.544487] ingenic_gpio_read_reg reg=0
> [    2.530991] ingenic_gpio_get_value: offset=0x2
> [    2.538372] mmc0: new high speed SDHC card at address 59b4
> [    2.531623] mmcblk0: mmc0:59b4 USD   7.51 GiB (ro)
> [    2.547466]  mmcblk0: p1 p2
> [    3.337335] random: crng init done
> 
> Still stuck, but card and partitions are now detected! Yay!

Well done!

> It seems to report read-only (ro) but that is a different story (WP GPIO?).
> It should not stop running the init process.
> 
> Patch attached.

I think you could just apply this. The parameter order was simply wrong.

I also think that the direction reset in ingenic_pinmux_set_pin_fn probably 
shouldn't be there, either.

Sorry for all the inconvenience this has caused!

Paul