| Message ID | 20211028095640.12070-1-manuel.dipolt@robart.cc |
|---|---|
| State | Changes Requested |
| Delegated to: | Andre Przywara |
| Headers | show |
| Series | [v3] sunxi: h3: enable clock support for r_pio gpios | expand |
On Thu, Oct 28, 2021 at 3:27 PM Manuel Dipolt <manuel.dipolt@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 > > > Signed-off-by: Manuel Dipolt <manuel.dipolt@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. Jagan.
On Tue, 7 Dec 2021 12:01:56 +0530 Jagan Teki <jagan@amarulasolutions.com> wrote: Hi, > On Thu, Oct 28, 2021 at 3:27 PM Manuel Dipolt > <manuel.dipolt@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@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
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); /* Tell the uclass how many GPIOs we have */ if (plat) {
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 Signed-off-by: Manuel Dipolt <manuel.dipolt@robart.cc> --- drivers/gpio/sunxi_gpio.c | 8 ++++++++ 1 file changed, 8 insertions(+)