[PATCH v3] sunxi: h3: enable clock support for r_pio gpios

[Andre Przywara]

On Tue, 7 Dec 2021 12:01:56 +0530
Jagan Teki <jagan at amarulasolutions.com> wrote:

Hi,

> On Thu, Oct 28, 2021 at 3:27 PM Manuel Dipolt
> <manuel.dipolt at robart.cc> wrote:
> >
> > This patch enables clock for the r_pio gpios for the h3
> > r_pio is required to access gpios from port L
> >
> > readout or setting of gpio PL10 (for example to control a led):
> >  
> > => gpio input PL10  
> > gpio: pin PL10 (gpio 298) value is 0  
> > => gpio set PL10  
> > gpio: pin PL10 (gpio 298) value is 1
> >
> > before:
> >  
> > => gpio input PL10  
> > gpio: pin PL10 (gpio 298) value is 0  
> > => gpio set PL10  
> > gpio: pin PL10 (gpio 298) value is 1
> >    Warning: value of pin is still 0

So indeed, the patch is fixing this issue. I think on the other SoCs
(A64, for instance) we enable the R_PIO clock already, probably in
sun8i_rsb.c, that's why not many people noticed.

But when applying this, I get multiple:
sunxi_set_gate: (CLK#54) unhandled
messages, plus one:
sunxi_set_gate: (CLK#9) unhandled

The first one is easy to fix (just adding the CLK_BUS_PIO gate
description to the H3 clock driver), but the latter one is more tricky:
This is the pll-periph clock from the r_ccu node, because the clock
driver just enables *all parent* clocks. Providing some dummy "enable"
handler for the PLL6 sounds at least dodgy.
I wonder if we actually need to enable *any* of the parent clocks:
certainly "hosc" does not need to be enabled, the RTC clocks have a
dummy enable, and the PLL_PERIPH is already enabled via the platform
setup, so we wouldn't miss out in our simple clock driver.

So what are people's opinion on this: Shall we drop the parent clock
enable call in the clock drivers altogether, or provide the
enable/disable bits for PLL_PERIPH?

> >
> >
> > Signed-off-by: Manuel Dipolt <manuel.dipolt at robart.cc>
> > ---
> >  drivers/gpio/sunxi_gpio.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/gpio/sunxi_gpio.c b/drivers/gpio/sunxi_gpio.c
> > index cdbc40d48f..0185eda70d 100644
> > --- a/drivers/gpio/sunxi_gpio.c
> > +++ b/drivers/gpio/sunxi_gpio.c
> > @@ -14,6 +14,7 @@
> >  #include <errno.h>
> >  #include <fdtdec.h>
> >  #include <malloc.h>
> > +#include <clk.h>
> >  #include <asm/io.h>
> >  #include <asm/gpio.h>
> >  #include <dm/device-internal.h>
> > @@ -245,6 +246,13 @@ static int gpio_sunxi_probe(struct udevice
> > *dev) {
> >         struct sunxi_gpio_plat *plat = dev_get_plat(dev);
> >         struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev);
> > +       struct clk gate_clk;
> > +       int ret;
> > +
> > +       ret = clk_get_by_name(dev, "apb", &gate_clk);
> > +
> > +       if (!ret)
> > +               clk_enable(&gate_clk);  
> 
> I'm wondering any of the sunxi driver or coding using this apb clock
> via CLK framework, if yes would you pointed it out.

Not sure what you are after here, exactly, but this "apb" clock is
CLK_BUS_PIO and CLK_APB0_PIO (for PIO and R_PIO, respectively).
We already describe the latter in the U-Boot clock driver, but nor the
former (see above).
Not enabling the PRCM PIO clock prevents the PortL GPIO controller from
working, hence the error message Manuel posted above.

Cheers,
Andre