| Message ID | 1499043558-9336-5-git-send-email-andre.przywara@arm.com |
|---|---|
| State | Superseded |
| Delegated to: | Jagannadha Sutradharudu Teki |
| Headers | show |
Hi Andre, On 2 July 2017 at 18:59, Andre Przywara <andre.przywara@arm.com> wrote: > Instead of hard-coding GPIO pins used for a certain peripheral, we > should just use the pinctrl information from the DT. > The sun8i-emac driver has some simple implementation of that, so > let's just generalize this and copy the code to a more common > location. > On the way we add support for the new, generic pinctrl binding now > used by all Allwinner SoCs. > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > --- > arch/arm/include/asm/arch-sunxi/gpio.h | 3 ++ > arch/arm/mach-sunxi/pinmux.c | 79 ++++++++++++++++++++++++++++++++++ > 2 files changed, 82 insertions(+) It looks like this should be done with a pinctrl driver, shouldn't it? Regards, Simon
Hi Simon, On 07/07/17 04:58, Simon Glass wrote: > Hi Andre, > > On 2 July 2017 at 18:59, Andre Przywara <andre.przywara@arm.com> wrote: >> Instead of hard-coding GPIO pins used for a certain peripheral, we >> should just use the pinctrl information from the DT. >> The sun8i-emac driver has some simple implementation of that, so >> let's just generalize this and copy the code to a more common >> location. >> On the way we add support for the new, generic pinctrl binding now >> used by all Allwinner SoCs. >> >> Signed-off-by: Andre Przywara <andre.przywara@arm.com> >> --- >> arch/arm/include/asm/arch-sunxi/gpio.h | 3 ++ >> arch/arm/mach-sunxi/pinmux.c | 79 ++++++++++++++++++++++++++++++++++ >> 2 files changed, 82 insertions(+) > > It looks like this should be done with a pinctrl driver, shouldn't it? I knew you would say this ;-) So technically you are right, but it looks like this is quite some work. And as patch 5/6 demonstrates, this code here was actually already mostly in, so this is basically just moving and generalisation. The whole purpose of this series is to prepare for merging in the Linux 4.13-rc1 DTs and still have working Ethernet, so I wanted to keep the bar low on this. I haven't looked in detail what it would take to have a proper Allwinner pinctrl DM driver, but the binding Linux uses is not "self-contained", as we still need SoC specific knowledge (namely into which actual mux value the "emac" string, for instance, translates to; and this is per SoC and pin). So that means pulling *a lot of* tables into U-Boot, which I am not sure is worth at this point. We might get away with some short-cuts, basically keeping our hard coded mux values around or just defining those that we actually need. But in general I consider this work more long term, and it should not halt the more important goal of having the Linux and U-Boot DTs in sync. Cheers, Andre.
+Tom On 7 July 2017 at 04:30, Andre Przywara <andre.przywara@arm.com> wrote: > Hi Simon, > > On 07/07/17 04:58, Simon Glass wrote: >> Hi Andre, >> >> On 2 July 2017 at 18:59, Andre Przywara <andre.przywara@arm.com> wrote: >>> Instead of hard-coding GPIO pins used for a certain peripheral, we >>> should just use the pinctrl information from the DT. >>> The sun8i-emac driver has some simple implementation of that, so >>> let's just generalize this and copy the code to a more common >>> location. >>> On the way we add support for the new, generic pinctrl binding now >>> used by all Allwinner SoCs. >>> >>> Signed-off-by: Andre Przywara <andre.przywara@arm.com> >>> --- >>> arch/arm/include/asm/arch-sunxi/gpio.h | 3 ++ >>> arch/arm/mach-sunxi/pinmux.c | 79 ++++++++++++++++++++++++++++++++++ >>> 2 files changed, 82 insertions(+) >> >> It looks like this should be done with a pinctrl driver, shouldn't it? > > I knew you would say this ;-) > > So technically you are right, but it looks like this is quite some work. > And as patch 5/6 demonstrates, this code here was actually already > mostly in, so this is basically just moving and generalisation. > > The whole purpose of this series is to prepare for merging in the Linux > 4.13-rc1 DTs and still have working Ethernet, so I wanted to keep the > bar low on this. > > I haven't looked in detail what it would take to have a proper Allwinner > pinctrl DM driver, but the binding Linux uses is not "self-contained", > as we still need SoC specific knowledge (namely into which actual mux > value the "emac" string, for instance, translates to; and this is per > SoC and pin). So that means pulling *a lot of* tables into U-Boot, which > I am not sure is worth at this point. We might get away with some > short-cuts, basically keeping our hard coded mux values around or just > defining those that we actually need. > But in general I consider this work more long term, and it should not > halt the more important goal of having the Linux and U-Boot DTs in sync. With a due sense of foreboding and dread: Reviewed-by: Simon Glass <sjg@chromium.org> > > Cheers, > Andre.
> On 14 Jul 2017, at 15:50, Simon Glass <sjg@chromium.org> wrote: > > +Tom > > On 7 July 2017 at 04:30, Andre Przywara <andre.przywara@arm.com <mailto:andre.przywara@arm.com>> wrote: >> Hi Simon, >> >> On 07/07/17 04:58, Simon Glass wrote: >>> Hi Andre, >>> >>> On 2 July 2017 at 18:59, Andre Przywara <andre.przywara@arm.com> wrote: >>>> Instead of hard-coding GPIO pins used for a certain peripheral, we >>>> should just use the pinctrl information from the DT. >>>> The sun8i-emac driver has some simple implementation of that, so >>>> let's just generalize this and copy the code to a more common >>>> location. >>>> On the way we add support for the new, generic pinctrl binding now >>>> used by all Allwinner SoCs. >>>> >>>> Signed-off-by: Andre Przywara <andre.przywara@arm.com> >>>> --- >>>> arch/arm/include/asm/arch-sunxi/gpio.h | 3 ++ >>>> arch/arm/mach-sunxi/pinmux.c | 79 ++++++++++++++++++++++++++++++++++ >>>> 2 files changed, 82 insertions(+) >>> >>> It looks like this should be done with a pinctrl driver, shouldn't it? >> >> I knew you would say this ;-) >> >> So technically you are right, but it looks like this is quite some work. >> And as patch 5/6 demonstrates, this code here was actually already >> mostly in, so this is basically just moving and generalisation. >> >> The whole purpose of this series is to prepare for merging in the Linux >> 4.13-rc1 DTs and still have working Ethernet, so I wanted to keep the >> bar low on this. >> >> I haven't looked in detail what it would take to have a proper Allwinner >> pinctrl DM driver, but the binding Linux uses is not "self-contained", >> as we still need SoC specific knowledge (namely into which actual mux >> value the "emac" string, for instance, translates to; and this is per >> SoC and pin). So that means pulling *a lot of* tables into U-Boot, which >> I am not sure is worth at this point. We might get away with some >> short-cuts, basically keeping our hard coded mux values around or just >> defining those that we actually need. >> But in general I consider this work more long term, and it should not >> halt the more important goal of having the Linux and U-Boot DTs in sync. > > With a due sense of foreboding and dread: > > Reviewed-by: Simon Glass <sjg@chromium.org <mailto:sjg@chromium.org>> I had submitted a DM driver to do this a long time ago, which included these tables for the A64 (and had the necessary infrastructure to quickly add other boards). This was still tight enough, even to fit into an AArch64 SPL stage. So I don’t see the purpose of not doing a DM pinctrl for sunxi, as the tables can easily be shared between U-Boot and Linux—i.e. I had used the Linux tables verbatim and only added a few lines of wrapper for U-Boot. Regards, Philipp.
diff --git a/arch/arm/include/asm/arch-sunxi/gpio.h b/arch/arm/include/asm/arch-sunxi/gpio.h index 24f8520..b5a4b32 100644 --- a/arch/arm/include/asm/arch-sunxi/gpio.h +++ b/arch/arm/include/asm/arch-sunxi/gpio.h @@ -240,4 +240,7 @@ int axp_gpio_init(void); static inline int axp_gpio_init(void) { return 0; } #endif +int sunxi_gpio_setup_dt_pins(const void * volatile fdt_blob, int node, + int mux_sel); + #endif /* _SUNXI_GPIO_H */ diff --git a/arch/arm/mach-sunxi/pinmux.c b/arch/arm/mach-sunxi/pinmux.c index b026f78..ae36fe9 100644 --- a/arch/arm/mach-sunxi/pinmux.c +++ b/arch/arm/mach-sunxi/pinmux.c @@ -9,6 +9,9 @@ #include <common.h> #include <asm/io.h> #include <asm/arch/gpio.h> +#include <fdtdec.h> +#include <fdt_support.h> +#include <dt-bindings/pinctrl/sun4i-a10.h> void sunxi_gpio_set_cfgbank(struct sunxi_gpio *pio, int bank_offset, u32 val) { @@ -69,3 +72,79 @@ int sunxi_gpio_set_pull(u32 pin, u32 val) return 0; } + +static int sunxi_gpio_setup_single_node(const void * volatile fdt_blob, + int offset, int mux_sel) +{ + int drive, pull, pin, i; + const char *pin_name; + + drive = fdt_getprop_u32_default_node(fdt_blob, offset, 0, + "drive-strength", ~0); + if (drive != ~0) { + if (drive <= 10) + drive = SUN4I_PINCTRL_10_MA; + else if (drive <= 20) + drive = SUN4I_PINCTRL_20_MA; + else if (drive <= 30) + drive = SUN4I_PINCTRL_30_MA; + else + drive = SUN4I_PINCTRL_40_MA; + } else { + drive = fdt_getprop_u32_default_node(fdt_blob, offset, 0, + "allwinner,drive", ~0); + } + + if (fdt_get_property(fdt_blob, offset, "bias-pull-up", NULL)) + pull = SUN4I_PINCTRL_PULL_UP; + else if (fdt_get_property(fdt_blob, offset, "bias-disable", NULL)) + pull = SUN4I_PINCTRL_NO_PULL; + else if (fdt_get_property(fdt_blob, offset, "bias-pull-down", NULL)) + pull = SUN4I_PINCTRL_PULL_DOWN; + else + pull = fdt_getprop_u32_default_node(fdt_blob, offset, 0, + "allwinner,pull", ~0); + + for (i = 0; ; i++) { + pin_name = fdt_stringlist_get(fdt_blob, offset, + "allwinner,pins", i, NULL); + if (!pin_name) { + pin_name = fdt_stringlist_get(fdt_blob, offset, + "pins", i, NULL); + if (!pin_name) + break; + } + pin = sunxi_name_to_gpio(pin_name); + if (pin < 0) + continue; + + sunxi_gpio_set_cfgpin(pin, mux_sel); + if (drive != ~0) + sunxi_gpio_set_drv(pin, drive); + if (pull != ~0) + sunxi_gpio_set_pull(pin, pull); + } + + return i; +} + +int sunxi_gpio_setup_dt_pins(const void * volatile fdt_blob, int node, + int mux_sel) +{ + int offset, pins = 0, idx; + + for (idx = 0; ; idx++) { + offset = fdtdec_lookup_phandle_index(fdt_blob, node, + "pinctrl-0", idx); + if (offset < 0) { + if (idx == 0) + return offset; + + return pins; + } + + pins += sunxi_gpio_setup_single_node(fdt_blob, offset, mux_sel); + } + + return pins; +}
Instead of hard-coding GPIO pins used for a certain peripheral, we should just use the pinctrl information from the DT. The sun8i-emac driver has some simple implementation of that, so let's just generalize this and copy the code to a more common location. On the way we add support for the new, generic pinctrl binding now used by all Allwinner SoCs. Signed-off-by: Andre Przywara <andre.przywara@arm.com> --- arch/arm/include/asm/arch-sunxi/gpio.h | 3 ++ arch/arm/mach-sunxi/pinmux.c | 79 ++++++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+)