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

[Paul Boddie]

On Sunday, 20 December 2020 21:26:48 CET H. Nikolaus Schaller wrote:
> 
> I have tried and now I have
> 
> [    0.183531] reg: 10010004: 08200200
> 
> instead of e.g.
> 
> [    0.197738] reg: 10010004: 08000200
> 
> And now I can see the VDD of the SD card jump to 3.3V for a short moment.
> But the mmc driver is still waiting for the SD card.
> 
> So our problem is the GPIO direction register. Maybe it is a general
> problem...
> 
> I have now tried to understand, and yes it is very different from OMAP.
> I had expected some drivers/gpio/ingenic or similar. But it uses
> pinctrl/pinctrl-ingenic.c
> 
> There we have a mix of pinmux and gpio control. But all things are in
> place like ingenic_gpio_direction_output(), ingenic_gpio_get(),
> ingenic_gpio_set().

One thing that occurs to me is that in the ingenic_pinmux_set_pin_fn function 
in pinctrl-ingenic.c, I shouldn't be changing the port direction:

    } else if (jzpc->info->version == ID_JZ4730) {
            ingenic_config_pin(jzpc, pin, JZ4730_GPIO_GPIER, false);
            ingenic_config_pin(jzpc, pin, JZ4730_GPIO_GPDIR, false);
            ingenic_config_pin_function(jzpc, pin, JZ4730_GPIO_GPAUR,
                                            JZ4730_GPIO_GPALR, func & 0x3);

Here, the JZ4730_GPIO_GPDIR bit clearing operation is largely irrelevant to 
the broader operation. And it occurs to me that even if something is setting 
the function to zero (GPIO), they might have set the direction before setting 
the function, even though that wouldn't make much sense.

So, by clearing the direction bit for the pin (making it an input), although 
this resets the default state of the pin, it might disrupt some configuration 
attempts which set the direction before the function. And a remedy for this 
might be to just remove that statement and see what happens.

The JZ4740 code only changes the function, trigger and select registers. The 
JZ4730 combines the function and select registers into the GPAUR and GPALR 
registers, distributing the pins across those registers instead of having one 
register (function) to indicate whether the pin uses an alternate function or 
not, and another register (select) to indicate which function. And the JZ4730 
equivalent to the trigger register is GPIER. So, the direction isn't changed 
in the JZ4740 code.

> What I can imagine is that in one of these functions our JZ4730 specific
> offset calculations are wrong.
> 
> Maybe some printk here and there will help to identify...

Probably, yes. I have reviewed the code a bit just now, and I can only see the 
above practice as being a potential problem, however. But mistakes are easy 
enough to make when filling in the functionality.

Paul