| Message ID | 1339590863-10564-9-git-send-email-richard.zhao@freescale.com |
|---|---|
| State | New |
| Headers | show |
On 06/13/2012 07:34 AM, Richard Zhao wrote: > Sometimes, boards have gpios that don't own by any driver or owner > by a generic driver that don't like hacks. Such gpios is normally > output and need setup once on boot. So I introduce the config-on-boot > gpios. > > Signed-off-by: Richard Zhao <richard.zhao@freescale.com> > Cc: Shawn Guo <shawn.guo@linaro.org> > Cc: Rob Herring <rob.herring@calxeda.com> > Cc: Grant Likely <grant.likely@secretlab.ca> > --- > .../devicetree/bindings/arm/config-on-boot.txt | 12 +++++++ > arch/arm/boot/dts/imx6q-sabrelite.dts | 7 ++++ > arch/arm/mach-imx/mach-imx6q.c | 35 ++++++++++++++++++++ > 3 files changed, 54 insertions(+) > create mode 100644 Documentation/devicetree/bindings/arm/config-on-boot.txt > > diff --git a/Documentation/devicetree/bindings/arm/config-on-boot.txt b/Documentation/devicetree/bindings/arm/config-on-boot.txt > new file mode 100644 > index 0000000..f98ed74 > --- /dev/null > +++ b/Documentation/devicetree/bindings/arm/config-on-boot.txt > @@ -0,0 +1,12 @@ > +* Configure on Boot > + > +Node name: config-on-boot > + It must be in root node. config-on-boot means to describe settings that needs > + to be set one time on boot but aren't owned by any driver, or the owned driver > + is too generic to handle such settings. For example, usb hub uses generic > + driver in usb core code, a on-board usb may need deassert reset pin. NAK. This is not a h/w description and should be solved within the kernel or bootloader. Either fix this in u-boot, the platform code, or make the generic driver support this in a generic way. Rob > + > +Optional properties: > +- output-gpios: Output gpio array that needs to set. > +- output-gpio-values: This property is required if output-gpios is set. > + The value is a array of 0 or 1. Total count eaquals the number of gpios. > diff --git a/arch/arm/boot/dts/imx6q-sabrelite.dts b/arch/arm/boot/dts/imx6q-sabrelite.dts > index e0ec929..1dd2261 100644 > --- a/arch/arm/boot/dts/imx6q-sabrelite.dts > +++ b/arch/arm/boot/dts/imx6q-sabrelite.dts > @@ -17,6 +17,13 @@ > model = "Freescale i.MX6 Quad SABRE Lite Board"; > compatible = "fsl,imx6q-sabrelite", "fsl,imx6q"; > > + config-on-boot { > + output-gpios = < > + &gpio3 22 0>; /* vbus reset */ > + output-gpio-values = < > + 1>; /* vbus reset */ > + }; > + > memory { > reg = <0x10000000 0x40000000>; > }; > diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c > index b47e98b..577cf19 100644 > --- a/arch/arm/mach-imx/mach-imx6q.c > +++ b/arch/arm/mach-imx/mach-imx6q.c > @@ -19,6 +19,7 @@ > #include <linux/irqdomain.h> > #include <linux/of.h> > #include <linux/of_address.h> > +#include <linux/of_gpio.h> > #include <linux/of_irq.h> > #include <linux/of_platform.h> > #include <linux/pinctrl/machine.h> > @@ -113,6 +114,38 @@ static void __init imx6q_sabrelite_init(void) > imx6q_sabrelite_cko1_setup(); > } > > +static void __init imx6q_config_on_boot(void) > +{ > + struct device_node *np; > + struct property *pp; > + int cnt, len, i; > + int gpio; > + > + np = of_find_node_by_path("/config-on-boot"); > + if (!np) > + return; > + cnt = of_gpio_named_count(np, "output-gpios"); > + pp = of_find_property(np, "output-gpio-values", &len); > + if (!pp || cnt != len / sizeof(u32)) { > + pr_err("Invalid config-on-boot gpios!\n"); > + of_node_put(np); > + return; > + } > + for (i = 0; i < cnt; i++) { > + gpio = of_get_named_gpio(np, "output-gpios", i); > + if (gpio_is_valid(gpio)) > + gpio_request_one(gpio, GPIOF_OUT_INIT_HIGH, > + "config-on-boot"); > + } > + > + of_node_put(np); > +} > + > +static void __init imx6q_post_populate(void) > +{ > + imx6q_config_on_boot(); > +} > + > static void __init imx6q_init_machine(void) > { > /* > @@ -126,6 +159,8 @@ static void __init imx6q_init_machine(void) > > of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL); > > + imx6q_post_populate(); > + > imx6q_pm_init(); > } >
Dear Rob Herring, > On 06/13/2012 07:34 AM, Richard Zhao wrote: > > Sometimes, boards have gpios that don't own by any driver or owner > > by a generic driver that don't like hacks. Such gpios is normally > > output and need setup once on boot. So I introduce the config-on-boot > > gpios. > > > > Signed-off-by: Richard Zhao <richard.zhao@freescale.com> > > Cc: Shawn Guo <shawn.guo@linaro.org> > > Cc: Rob Herring <rob.herring@calxeda.com> > > Cc: Grant Likely <grant.likely@secretlab.ca> > > --- > > > > .../devicetree/bindings/arm/config-on-boot.txt | 12 +++++++ > > arch/arm/boot/dts/imx6q-sabrelite.dts | 7 ++++ > > arch/arm/mach-imx/mach-imx6q.c | 35 > > ++++++++++++++++++++ 3 files changed, 54 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/arm/config-on-boot.txt > > > > diff --git a/Documentation/devicetree/bindings/arm/config-on-boot.txt > > b/Documentation/devicetree/bindings/arm/config-on-boot.txt new file mode > > 100644 > > index 0000000..f98ed74 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/arm/config-on-boot.txt > > @@ -0,0 +1,12 @@ > > +* Configure on Boot > > + > > +Node name: config-on-boot > > + It must be in root node. config-on-boot means to describe settings > > that needs + to be set one time on boot but aren't owned by any driver, > > or the owned driver + is too generic to handle such settings. For > > example, usb hub uses generic + driver in usb core code, a on-board usb > > may need deassert reset pin. > > NAK. This is not a h/w description and should be solved within the > kernel or bootloader. Either fix this in u-boot I don't want to be the bastard here, but this shouldn't go into uboot. > the platform code Yes, it should be here. But then, it's some usb reset, so maybe the PHY should somehow handle it? > , or > make the generic driver support this in a generic way. Well, this is platform specific stuff, so it shouldn't be in any way part of the driver. > > Rob > > > + > > +Optional properties: > > +- output-gpios: Output gpio array that needs to set. > > +- output-gpio-values: This property is required if output-gpios is set. > > + The value is a array of 0 or 1. Total count eaquals the number of > > gpios. diff --git a/arch/arm/boot/dts/imx6q-sabrelite.dts > > b/arch/arm/boot/dts/imx6q-sabrelite.dts index e0ec929..1dd2261 100644 > > --- a/arch/arm/boot/dts/imx6q-sabrelite.dts > > +++ b/arch/arm/boot/dts/imx6q-sabrelite.dts > > @@ -17,6 +17,13 @@ > > > > model = "Freescale i.MX6 Quad SABRE Lite Board"; > > compatible = "fsl,imx6q-sabrelite", "fsl,imx6q"; > > > > + config-on-boot { > > + output-gpios = < > > + &gpio3 22 0>; /* vbus reset */ > > + output-gpio-values = < > > + 1>; /* vbus reset */ > > + }; > > + > > > > memory { > > > > reg = <0x10000000 0x40000000>; > > > > }; > > > > diff --git a/arch/arm/mach-imx/mach-imx6q.c > > b/arch/arm/mach-imx/mach-imx6q.c index b47e98b..577cf19 100644 > > --- a/arch/arm/mach-imx/mach-imx6q.c > > +++ b/arch/arm/mach-imx/mach-imx6q.c > > @@ -19,6 +19,7 @@ > > > > #include <linux/irqdomain.h> > > #include <linux/of.h> > > #include <linux/of_address.h> > > > > +#include <linux/of_gpio.h> > > > > #include <linux/of_irq.h> > > #include <linux/of_platform.h> > > #include <linux/pinctrl/machine.h> > > > > @@ -113,6 +114,38 @@ static void __init imx6q_sabrelite_init(void) > > > > imx6q_sabrelite_cko1_setup(); > > > > } > > > > +static void __init imx6q_config_on_boot(void) > > +{ > > + struct device_node *np; > > + struct property *pp; > > + int cnt, len, i; > > + int gpio; > > + > > + np = of_find_node_by_path("/config-on-boot"); > > + if (!np) > > + return; > > + cnt = of_gpio_named_count(np, "output-gpios"); > > + pp = of_find_property(np, "output-gpio-values", &len); > > + if (!pp || cnt != len / sizeof(u32)) { > > + pr_err("Invalid config-on-boot gpios!\n"); > > + of_node_put(np); > > + return; > > + } > > + for (i = 0; i < cnt; i++) { > > + gpio = of_get_named_gpio(np, "output-gpios", i); > > + if (gpio_is_valid(gpio)) > > + gpio_request_one(gpio, GPIOF_OUT_INIT_HIGH, > > + "config-on-boot"); > > + } > > + > > + of_node_put(np); > > +} > > + > > +static void __init imx6q_post_populate(void) > > +{ > > + imx6q_config_on_boot(); > > +} > > + > > > > static void __init imx6q_init_machine(void) > > { > > > > /* > > > > @@ -126,6 +159,8 @@ static void __init imx6q_init_machine(void) > > > > of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL); > > > > + imx6q_post_populate(); > > + > > > > imx6q_pm_init(); > > > > } Best regards, Marek Vasut
On Wed, Jun 13, 2012 at 10:09:54AM -0500, Rob Herring wrote: > On 06/13/2012 07:34 AM, Richard Zhao wrote: > > Sometimes, boards have gpios that don't own by any driver or owner > > by a generic driver that don't like hacks. Such gpios is normally > > output and need setup once on boot. So I introduce the config-on-boot > > gpios. > > > > Signed-off-by: Richard Zhao <richard.zhao@freescale.com> > > Cc: Shawn Guo <shawn.guo@linaro.org> > > Cc: Rob Herring <rob.herring@calxeda.com> > > Cc: Grant Likely <grant.likely@secretlab.ca> > > --- > > .../devicetree/bindings/arm/config-on-boot.txt | 12 +++++++ > > arch/arm/boot/dts/imx6q-sabrelite.dts | 7 ++++ > > arch/arm/mach-imx/mach-imx6q.c | 35 ++++++++++++++++++++ > > 3 files changed, 54 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/arm/config-on-boot.txt > > > > diff --git a/Documentation/devicetree/bindings/arm/config-on-boot.txt b/Documentation/devicetree/bindings/arm/config-on-boot.txt > > new file mode 100644 > > index 0000000..f98ed74 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/arm/config-on-boot.txt > > @@ -0,0 +1,12 @@ > > +* Configure on Boot > > + > > +Node name: config-on-boot > > + It must be in root node. config-on-boot means to describe settings that needs > > + to be set one time on boot but aren't owned by any driver, or the owned driver > > + is too generic to handle such settings. For example, usb hub uses generic > > + driver in usb core code, a on-board usb may need deassert reset pin. > > NAK. This is not a h/w description It's misc things, but is hw description. > and should be solved within the > kernel or bootloader. Either fix this in u-boot, Kernel might be better not to depend on uboot. > the platform code, or How do I get gpio in platfrom code without dts description? > make the generic driver support this in a generic way. It's called just after populate devices. It's hard for generic driver to decide when it's called. Thanks Richard > > Rob > > > + > > +Optional properties: > > +- output-gpios: Output gpio array that needs to set. > > +- output-gpio-values: This property is required if output-gpios is set. > > + The value is a array of 0 or 1. Total count eaquals the number of gpios. > > diff --git a/arch/arm/boot/dts/imx6q-sabrelite.dts b/arch/arm/boot/dts/imx6q-sabrelite.dts > > index e0ec929..1dd2261 100644 > > --- a/arch/arm/boot/dts/imx6q-sabrelite.dts > > +++ b/arch/arm/boot/dts/imx6q-sabrelite.dts > > @@ -17,6 +17,13 @@ > > model = "Freescale i.MX6 Quad SABRE Lite Board"; > > compatible = "fsl,imx6q-sabrelite", "fsl,imx6q"; > > > > + config-on-boot { > > + output-gpios = < > > + &gpio3 22 0>; /* vbus reset */ > > + output-gpio-values = < > > + 1>; /* vbus reset */ > > + }; > > + > > memory { > > reg = <0x10000000 0x40000000>; > > }; > > diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c > > index b47e98b..577cf19 100644 > > --- a/arch/arm/mach-imx/mach-imx6q.c > > +++ b/arch/arm/mach-imx/mach-imx6q.c > > @@ -19,6 +19,7 @@ > > #include <linux/irqdomain.h> > > #include <linux/of.h> > > #include <linux/of_address.h> > > +#include <linux/of_gpio.h> > > #include <linux/of_irq.h> > > #include <linux/of_platform.h> > > #include <linux/pinctrl/machine.h> > > @@ -113,6 +114,38 @@ static void __init imx6q_sabrelite_init(void) > > imx6q_sabrelite_cko1_setup(); > > } > > > > +static void __init imx6q_config_on_boot(void) > > +{ > > + struct device_node *np; > > + struct property *pp; > > + int cnt, len, i; > > + int gpio; > > + > > + np = of_find_node_by_path("/config-on-boot"); > > + if (!np) > > + return; > > + cnt = of_gpio_named_count(np, "output-gpios"); > > + pp = of_find_property(np, "output-gpio-values", &len); > > + if (!pp || cnt != len / sizeof(u32)) { > > + pr_err("Invalid config-on-boot gpios!\n"); > > + of_node_put(np); > > + return; > > + } > > + for (i = 0; i < cnt; i++) { > > + gpio = of_get_named_gpio(np, "output-gpios", i); > > + if (gpio_is_valid(gpio)) > > + gpio_request_one(gpio, GPIOF_OUT_INIT_HIGH, > > + "config-on-boot"); > > + } > > + > > + of_node_put(np); > > +} > > + > > +static void __init imx6q_post_populate(void) > > +{ > > + imx6q_config_on_boot(); > > +} > > + > > static void __init imx6q_init_machine(void) > > { > > /* > > @@ -126,6 +159,8 @@ static void __init imx6q_init_machine(void) > > > > of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL); > > > > + imx6q_post_populate(); > > + > > imx6q_pm_init(); > > } > > >
Dear Richard Zhao, > On Wed, Jun 13, 2012 at 10:09:54AM -0500, Rob Herring wrote: > > On 06/13/2012 07:34 AM, Richard Zhao wrote: > > > Sometimes, boards have gpios that don't own by any driver or owner > > > by a generic driver that don't like hacks. Such gpios is normally > > > output and need setup once on boot. So I introduce the config-on-boot > > > gpios. > > > > > > Signed-off-by: Richard Zhao <richard.zhao@freescale.com> > > > Cc: Shawn Guo <shawn.guo@linaro.org> > > > Cc: Rob Herring <rob.herring@calxeda.com> > > > Cc: Grant Likely <grant.likely@secretlab.ca> > > > --- > > > > > > .../devicetree/bindings/arm/config-on-boot.txt | 12 +++++++ > > > arch/arm/boot/dts/imx6q-sabrelite.dts | 7 ++++ > > > arch/arm/mach-imx/mach-imx6q.c | 35 > > > ++++++++++++++++++++ 3 files changed, 54 insertions(+) > > > create mode 100644 > > > Documentation/devicetree/bindings/arm/config-on-boot.txt > > > > > > diff --git a/Documentation/devicetree/bindings/arm/config-on-boot.txt > > > b/Documentation/devicetree/bindings/arm/config-on-boot.txt new file > > > mode 100644 > > > index 0000000..f98ed74 > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/arm/config-on-boot.txt > > > @@ -0,0 +1,12 @@ > > > +* Configure on Boot > > > + > > > +Node name: config-on-boot > > > + It must be in root node. config-on-boot means to describe settings > > > that needs + to be set one time on boot but aren't owned by any > > > driver, or the owned driver + is too generic to handle such settings. > > > For example, usb hub uses generic + driver in usb core code, a > > > on-board usb may need deassert reset pin. > > > > NAK. This is not a h/w description > > It's misc things, but is hw description. > > > and should be solved within the > > kernel or bootloader. Either fix this in u-boot, > > Kernel might be better not to depend on uboot. And we don't want to polute uboot either, so this is a good move. > > the platform code, or > > How do I get gpio in platfrom code without dts description? Write a driver that does the GPIO setup in some generic way? Wasn't there something about this stuff going on? Shawn, you had some comments /wrt the M28 DTS. This might be what you meant? > > make the generic driver support this in a generic way. > > It's called just after populate devices. It's hard for generic driver to > decide when it's called. > > Thanks > Richard Best regards, Marek Vasut
On 06/13/2012 10:19 AM, Marek Vasut wrote: > Dear Rob Herring, > >> On 06/13/2012 07:34 AM, Richard Zhao wrote: >>> Sometimes, boards have gpios that don't own by any driver or owner >>> by a generic driver that don't like hacks. Such gpios is normally >>> output and need setup once on boot. So I introduce the config-on-boot >>> gpios. >>> >>> Signed-off-by: Richard Zhao <richard.zhao@freescale.com> >>> Cc: Shawn Guo <shawn.guo@linaro.org> >>> Cc: Rob Herring <rob.herring@calxeda.com> >>> Cc: Grant Likely <grant.likely@secretlab.ca> >>> --- >>> >>> .../devicetree/bindings/arm/config-on-boot.txt | 12 +++++++ >>> arch/arm/boot/dts/imx6q-sabrelite.dts | 7 ++++ >>> arch/arm/mach-imx/mach-imx6q.c | 35 >>> ++++++++++++++++++++ 3 files changed, 54 insertions(+) >>> create mode 100644 >>> Documentation/devicetree/bindings/arm/config-on-boot.txt >>> >>> diff --git a/Documentation/devicetree/bindings/arm/config-on-boot.txt >>> b/Documentation/devicetree/bindings/arm/config-on-boot.txt new file mode >>> 100644 >>> index 0000000..f98ed74 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/arm/config-on-boot.txt >>> @@ -0,0 +1,12 @@ >>> +* Configure on Boot >>> + >>> +Node name: config-on-boot >>> + It must be in root node. config-on-boot means to describe settings >>> that needs + to be set one time on boot but aren't owned by any driver, >>> or the owned driver + is too generic to handle such settings. For >>> example, usb hub uses generic + driver in usb core code, a on-board usb >>> may need deassert reset pin. >> >> NAK. This is not a h/w description and should be solved within the >> kernel or bootloader. Either fix this in u-boot > > I don't want to be the bastard here, but this shouldn't go into uboot. Just listing possible options. Obviously, no one wants the dirty stuff in their code... There would never be a need in u-boot to support boot from USB mass storage? In which case you would have to handle this reset line leaving it deasserted and linux would likely never know. (Yes, I know we can't rely on bootloader setup.) >> the platform code > > Yes, it should be here. But then, it's some usb reset, so maybe the PHY should > somehow handle it? > >> , or >> make the generic driver support this in a generic way. > > Well, this is platform specific stuff, so it shouldn't be in any way part of the > driver. An embedded hub with gpio controlled reset line sounds fairly generic to me. The only platform specific part is which gpio line which is nothing new to deal with. On the other hand, this is really a flaw in the h/w design as whether the hub is integrated on board or a separate hub should be transparent to s/w. So just keeping it contained in platform code may be the best option. Rob > >> >> Rob >> >>> + >>> +Optional properties: >>> +- output-gpios: Output gpio array that needs to set. >>> +- output-gpio-values: This property is required if output-gpios is set. >>> + The value is a array of 0 or 1. Total count eaquals the number of >>> gpios. diff --git a/arch/arm/boot/dts/imx6q-sabrelite.dts >>> b/arch/arm/boot/dts/imx6q-sabrelite.dts index e0ec929..1dd2261 100644 >>> --- a/arch/arm/boot/dts/imx6q-sabrelite.dts >>> +++ b/arch/arm/boot/dts/imx6q-sabrelite.dts >>> @@ -17,6 +17,13 @@ >>> >>> model = "Freescale i.MX6 Quad SABRE Lite Board"; >>> compatible = "fsl,imx6q-sabrelite", "fsl,imx6q"; >>> >>> + config-on-boot { >>> + output-gpios = < >>> + &gpio3 22 0>; /* vbus reset */ >>> + output-gpio-values = < >>> + 1>; /* vbus reset */ >>> + }; >>> + >>> >>> memory { >>> >>> reg = <0x10000000 0x40000000>; >>> >>> }; >>> >>> diff --git a/arch/arm/mach-imx/mach-imx6q.c >>> b/arch/arm/mach-imx/mach-imx6q.c index b47e98b..577cf19 100644 >>> --- a/arch/arm/mach-imx/mach-imx6q.c >>> +++ b/arch/arm/mach-imx/mach-imx6q.c >>> @@ -19,6 +19,7 @@ >>> >>> #include <linux/irqdomain.h> >>> #include <linux/of.h> >>> #include <linux/of_address.h> >>> >>> +#include <linux/of_gpio.h> >>> >>> #include <linux/of_irq.h> >>> #include <linux/of_platform.h> >>> #include <linux/pinctrl/machine.h> >>> >>> @@ -113,6 +114,38 @@ static void __init imx6q_sabrelite_init(void) >>> >>> imx6q_sabrelite_cko1_setup(); >>> >>> } >>> >>> +static void __init imx6q_config_on_boot(void) >>> +{ >>> + struct device_node *np; >>> + struct property *pp; >>> + int cnt, len, i; >>> + int gpio; >>> + >>> + np = of_find_node_by_path("/config-on-boot"); >>> + if (!np) >>> + return; >>> + cnt = of_gpio_named_count(np, "output-gpios"); >>> + pp = of_find_property(np, "output-gpio-values", &len); >>> + if (!pp || cnt != len / sizeof(u32)) { >>> + pr_err("Invalid config-on-boot gpios!\n"); >>> + of_node_put(np); >>> + return; >>> + } >>> + for (i = 0; i < cnt; i++) { >>> + gpio = of_get_named_gpio(np, "output-gpios", i); >>> + if (gpio_is_valid(gpio)) >>> + gpio_request_one(gpio, GPIOF_OUT_INIT_HIGH, >>> + "config-on-boot"); >>> + } >>> + >>> + of_node_put(np); >>> +} >>> + >>> +static void __init imx6q_post_populate(void) >>> +{ >>> + imx6q_config_on_boot(); >>> +} >>> + >>> >>> static void __init imx6q_init_machine(void) >>> { >>> >>> /* >>> >>> @@ -126,6 +159,8 @@ static void __init imx6q_init_machine(void) >>> >>> of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL); >>> >>> + imx6q_post_populate(); >>> + >>> >>> imx6q_pm_init(); >>> >>> } > > Best regards, > Marek Vasut
On 06/13/2012 10:28 AM, Richard Zhao wrote: > On Wed, Jun 13, 2012 at 10:09:54AM -0500, Rob Herring wrote: >> On 06/13/2012 07:34 AM, Richard Zhao wrote: >>> Sometimes, boards have gpios that don't own by any driver or owner >>> by a generic driver that don't like hacks. Such gpios is normally >>> output and need setup once on boot. So I introduce the config-on-boot >>> gpios. >>> >>> Signed-off-by: Richard Zhao <richard.zhao@freescale.com> >>> Cc: Shawn Guo <shawn.guo@linaro.org> >>> Cc: Rob Herring <rob.herring@calxeda.com> >>> Cc: Grant Likely <grant.likely@secretlab.ca> >>> --- >>> .../devicetree/bindings/arm/config-on-boot.txt | 12 +++++++ >>> arch/arm/boot/dts/imx6q-sabrelite.dts | 7 ++++ >>> arch/arm/mach-imx/mach-imx6q.c | 35 ++++++++++++++++++++ >>> 3 files changed, 54 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/arm/config-on-boot.txt >>> >>> diff --git a/Documentation/devicetree/bindings/arm/config-on-boot.txt b/Documentation/devicetree/bindings/arm/config-on-boot.txt >>> new file mode 100644 >>> index 0000000..f98ed74 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/arm/config-on-boot.txt >>> @@ -0,0 +1,12 @@ >>> +* Configure on Boot >>> + >>> +Node name: config-on-boot >>> + It must be in root node. config-on-boot means to describe settings that needs >>> + to be set one time on boot but aren't owned by any driver, or the owned driver >>> + is too generic to handle such settings. For example, usb hub uses generic >>> + driver in usb core code, a on-board usb may need deassert reset pin. >> >> NAK. This is not a h/w description > It's misc things, but is hw description. You are defining something based on whether linux has a driver or not. That should not matter to DT. That's backwards. >> and should be solved within the >> kernel or bootloader. Either fix this in u-boot, > Kernel might be better not to depend on uboot. >> the platform code, or > How do I get gpio in platfrom code without dts description? >> make the generic driver support this in a generic way. > It's called just after populate devices. It's hard for generic driver to > decide when it's called. You need to describe that you have a hub on the usb bus and add the gpio line to that node. Just like PCI is probe-able, you still need DT nodes sometimes for cases like this. A simpler approach would be to just add the gpio to the ehci controller node, but that's not exactly correct. Rob > > Thanks > Richard >> >> Rob >> >>> + >>> +Optional properties: >>> +- output-gpios: Output gpio array that needs to set. >>> +- output-gpio-values: This property is required if output-gpios is set. >>> + The value is a array of 0 or 1. Total count eaquals the number of gpios. >>> diff --git a/arch/arm/boot/dts/imx6q-sabrelite.dts b/arch/arm/boot/dts/imx6q-sabrelite.dts >>> index e0ec929..1dd2261 100644 >>> --- a/arch/arm/boot/dts/imx6q-sabrelite.dts >>> +++ b/arch/arm/boot/dts/imx6q-sabrelite.dts >>> @@ -17,6 +17,13 @@ >>> model = "Freescale i.MX6 Quad SABRE Lite Board"; >>> compatible = "fsl,imx6q-sabrelite", "fsl,imx6q"; >>> >>> + config-on-boot { >>> + output-gpios = < >>> + &gpio3 22 0>; /* vbus reset */ >>> + output-gpio-values = < >>> + 1>; /* vbus reset */ >>> + }; >>> + >>> memory { >>> reg = <0x10000000 0x40000000>; >>> }; >>> diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c >>> index b47e98b..577cf19 100644 >>> --- a/arch/arm/mach-imx/mach-imx6q.c >>> +++ b/arch/arm/mach-imx/mach-imx6q.c >>> @@ -19,6 +19,7 @@ >>> #include <linux/irqdomain.h> >>> #include <linux/of.h> >>> #include <linux/of_address.h> >>> +#include <linux/of_gpio.h> >>> #include <linux/of_irq.h> >>> #include <linux/of_platform.h> >>> #include <linux/pinctrl/machine.h> >>> @@ -113,6 +114,38 @@ static void __init imx6q_sabrelite_init(void) >>> imx6q_sabrelite_cko1_setup(); >>> } >>> >>> +static void __init imx6q_config_on_boot(void) >>> +{ >>> + struct device_node *np; >>> + struct property *pp; >>> + int cnt, len, i; >>> + int gpio; >>> + >>> + np = of_find_node_by_path("/config-on-boot"); >>> + if (!np) >>> + return; >>> + cnt = of_gpio_named_count(np, "output-gpios"); >>> + pp = of_find_property(np, "output-gpio-values", &len); >>> + if (!pp || cnt != len / sizeof(u32)) { >>> + pr_err("Invalid config-on-boot gpios!\n"); >>> + of_node_put(np); >>> + return; >>> + } >>> + for (i = 0; i < cnt; i++) { >>> + gpio = of_get_named_gpio(np, "output-gpios", i); >>> + if (gpio_is_valid(gpio)) >>> + gpio_request_one(gpio, GPIOF_OUT_INIT_HIGH, >>> + "config-on-boot"); >>> + } >>> + >>> + of_node_put(np); >>> +} >>> + >>> +static void __init imx6q_post_populate(void) >>> +{ >>> + imx6q_config_on_boot(); >>> +} >>> + >>> static void __init imx6q_init_machine(void) >>> { >>> /* >>> @@ -126,6 +159,8 @@ static void __init imx6q_init_machine(void) >>> >>> of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL); >>> >>> + imx6q_post_populate(); >>> + >>> imx6q_pm_init(); >>> } >>> >>
Dear Rob Herring, > On 06/13/2012 10:19 AM, Marek Vasut wrote: > > Dear Rob Herring, > > > >> On 06/13/2012 07:34 AM, Richard Zhao wrote: > >>> Sometimes, boards have gpios that don't own by any driver or owner > >>> by a generic driver that don't like hacks. Such gpios is normally > >>> output and need setup once on boot. So I introduce the config-on-boot > >>> gpios. > >>> > >>> Signed-off-by: Richard Zhao <richard.zhao@freescale.com> > >>> Cc: Shawn Guo <shawn.guo@linaro.org> > >>> Cc: Rob Herring <rob.herring@calxeda.com> > >>> Cc: Grant Likely <grant.likely@secretlab.ca> > >>> --- > >>> > >>> .../devicetree/bindings/arm/config-on-boot.txt | 12 +++++++ > >>> arch/arm/boot/dts/imx6q-sabrelite.dts | 7 ++++ > >>> arch/arm/mach-imx/mach-imx6q.c | 35 > >>> ++++++++++++++++++++ 3 files changed, 54 insertions(+) > >>> create mode 100644 > >>> Documentation/devicetree/bindings/arm/config-on-boot.txt > >>> > >>> diff --git a/Documentation/devicetree/bindings/arm/config-on-boot.txt > >>> b/Documentation/devicetree/bindings/arm/config-on-boot.txt new file > >>> mode 100644 > >>> index 0000000..f98ed74 > >>> --- /dev/null > >>> +++ b/Documentation/devicetree/bindings/arm/config-on-boot.txt > >>> @@ -0,0 +1,12 @@ > >>> +* Configure on Boot > >>> + > >>> +Node name: config-on-boot > >>> + It must be in root node. config-on-boot means to describe settings > >>> that needs + to be set one time on boot but aren't owned by any > >>> driver, or the owned driver + is too generic to handle such settings. > >>> For example, usb hub uses generic + driver in usb core code, a > >>> on-board usb may need deassert reset pin. > >> > >> NAK. This is not a h/w description and should be solved within the > >> kernel or bootloader. Either fix this in u-boot > > > > I don't want to be the bastard here, but this shouldn't go into uboot. > > Just listing possible options. Obviously, no one wants the dirty stuff > in their code... > > There would never be a need in u-boot to support boot from USB mass > storage? We already support that obviously ;-) > In which case you would have to handle this reset line leaving > it deasserted and linux would likely never know. (Yes, I know we can't > rely on bootloader setup.) But we defer the initialization of USB to a point where user really needs it. Therefore if you boot from ie. NAND, you won't have the USB inited anyway. > >> the platform code > > > > Yes, it should be here. But then, it's some usb reset, so maybe the PHY > > should somehow handle it? > > > >> , or > >> make the generic driver support this in a generic way. > > > > Well, this is platform specific stuff, so it shouldn't be in any way part > > of the driver. > > An embedded hub with gpio controlled reset line sounds fairly generic to > me. The only platform specific part is which gpio line which is nothing > new to deal with. And what about embedded hub that has two distinct power GPIOs? And we can go forever with this discussion. > On the other hand, this is really a flaw in the h/w design as whether > the hub is integrated on board or a separate hub should be transparent > to s/w. I don't think so. You might want to minimize the power consumption by disabling the hub power completely. > So just keeping it contained in platform code may be the best > option. Agreed > Rob > > >> Rob > >> > >>> + > >>> +Optional properties: > >>> +- output-gpios: Output gpio array that needs to set. > >>> +- output-gpio-values: This property is required if output-gpios is > >>> set. + The value is a array of 0 or 1. Total count eaquals the number > >>> of gpios. diff --git a/arch/arm/boot/dts/imx6q-sabrelite.dts > >>> b/arch/arm/boot/dts/imx6q-sabrelite.dts index e0ec929..1dd2261 100644 > >>> --- a/arch/arm/boot/dts/imx6q-sabrelite.dts > >>> +++ b/arch/arm/boot/dts/imx6q-sabrelite.dts > >>> @@ -17,6 +17,13 @@ > >>> > >>> model = "Freescale i.MX6 Quad SABRE Lite Board"; > >>> compatible = "fsl,imx6q-sabrelite", "fsl,imx6q"; > >>> > >>> + config-on-boot { > >>> + output-gpios = < > >>> + &gpio3 22 0>; /* vbus reset */ > >>> + output-gpio-values = < > >>> + 1>; /* vbus reset */ > >>> + }; > >>> + > >>> > >>> memory { > >>> > >>> reg = <0x10000000 0x40000000>; > >>> > >>> }; > >>> > >>> diff --git a/arch/arm/mach-imx/mach-imx6q.c > >>> b/arch/arm/mach-imx/mach-imx6q.c index b47e98b..577cf19 100644 > >>> --- a/arch/arm/mach-imx/mach-imx6q.c > >>> +++ b/arch/arm/mach-imx/mach-imx6q.c > >>> @@ -19,6 +19,7 @@ > >>> > >>> #include <linux/irqdomain.h> > >>> #include <linux/of.h> > >>> #include <linux/of_address.h> > >>> > >>> +#include <linux/of_gpio.h> > >>> > >>> #include <linux/of_irq.h> > >>> #include <linux/of_platform.h> > >>> #include <linux/pinctrl/machine.h> > >>> > >>> @@ -113,6 +114,38 @@ static void __init imx6q_sabrelite_init(void) > >>> > >>> imx6q_sabrelite_cko1_setup(); > >>> > >>> } > >>> > >>> +static void __init imx6q_config_on_boot(void) > >>> +{ > >>> + struct device_node *np; > >>> + struct property *pp; > >>> + int cnt, len, i; > >>> + int gpio; > >>> + > >>> + np = of_find_node_by_path("/config-on-boot"); > >>> + if (!np) > >>> + return; > >>> + cnt = of_gpio_named_count(np, "output-gpios"); > >>> + pp = of_find_property(np, "output-gpio-values", &len); > >>> + if (!pp || cnt != len / sizeof(u32)) { > >>> + pr_err("Invalid config-on-boot gpios!\n"); > >>> + of_node_put(np); > >>> + return; > >>> + } > >>> + for (i = 0; i < cnt; i++) { > >>> + gpio = of_get_named_gpio(np, "output-gpios", i); > >>> + if (gpio_is_valid(gpio)) > >>> + gpio_request_one(gpio, GPIOF_OUT_INIT_HIGH, > >>> + "config-on-boot"); > >>> + } > >>> + > >>> + of_node_put(np); > >>> +} > >>> + > >>> +static void __init imx6q_post_populate(void) > >>> +{ > >>> + imx6q_config_on_boot(); > >>> +} > >>> + > >>> > >>> static void __init imx6q_init_machine(void) > >>> { > >>> > >>> /* > >>> > >>> @@ -126,6 +159,8 @@ static void __init imx6q_init_machine(void) > >>> > >>> of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL); > >>> > >>> + imx6q_post_populate(); > >>> + > >>> > >>> imx6q_pm_init(); > >>> > >>> } > > > > Best regards, > > Marek Vasut
On Wed, Jun 13, 2012 at 11:50:35AM -0500, Rob Herring wrote: > On 06/13/2012 10:28 AM, Richard Zhao wrote: > > On Wed, Jun 13, 2012 at 10:09:54AM -0500, Rob Herring wrote: > >> On 06/13/2012 07:34 AM, Richard Zhao wrote: > >>> Sometimes, boards have gpios that don't own by any driver or owner > >>> by a generic driver that don't like hacks. Such gpios is normally > >>> output and need setup once on boot. So I introduce the config-on-boot > >>> gpios. > >>> > >>> Signed-off-by: Richard Zhao <richard.zhao@freescale.com> > >>> Cc: Shawn Guo <shawn.guo@linaro.org> > >>> Cc: Rob Herring <rob.herring@calxeda.com> > >>> Cc: Grant Likely <grant.likely@secretlab.ca> > >>> --- > >>> .../devicetree/bindings/arm/config-on-boot.txt | 12 +++++++ > >>> arch/arm/boot/dts/imx6q-sabrelite.dts | 7 ++++ > >>> arch/arm/mach-imx/mach-imx6q.c | 35 ++++++++++++++++++++ > >>> 3 files changed, 54 insertions(+) > >>> create mode 100644 Documentation/devicetree/bindings/arm/config-on-boot.txt > >>> > >>> diff --git a/Documentation/devicetree/bindings/arm/config-on-boot.txt b/Documentation/devicetree/bindings/arm/config-on-boot.txt > >>> new file mode 100644 > >>> index 0000000..f98ed74 > >>> --- /dev/null > >>> +++ b/Documentation/devicetree/bindings/arm/config-on-boot.txt > >>> @@ -0,0 +1,12 @@ > >>> +* Configure on Boot > >>> + > >>> +Node name: config-on-boot > >>> + It must be in root node. config-on-boot means to describe settings that needs > >>> + to be set one time on boot but aren't owned by any driver, or the owned driver > >>> + is too generic to handle such settings. For example, usb hub uses generic > >>> + driver in usb core code, a on-board usb may need deassert reset pin. > >> > >> NAK. This is not a h/w description > > It's misc things, but is hw description. > > You are defining something based on whether linux has a driver or not. > That should not matter to DT. That's backwards. I just define the board needs some misc settings. If you think config-on-boot is not good, we can name it like config-misc. > > >> and should be solved within the > >> kernel or bootloader. Either fix this in u-boot, > > Kernel might be better not to depend on uboot. > >> the platform code, or > > How do I get gpio in platfrom code without dts description? > >> make the generic driver support this in a generic way. > > It's called just after populate devices. It's hard for generic driver to > > decide when it's called. > > You need to describe that you have a hub on the usb bus and add the gpio > line to that node. Just like PCI is probe-able, you still need DT nodes > sometimes for cases like this. PCI has dev id and may add quirks. But for embedded, I don't know how to connect a of node to a hub device enumerated by usb core code. And it may also pollute the usb core code. > A simpler approach would be to just add > the gpio to the ehci controller node, but that's not exactly correct. That's exactly why this patch comes out. Thanks Richard > > Rob > > > > > > Thanks > > Richard > >> > >> Rob > >>
On Wed, Jun 13, 2012 at 06:00:38PM +0200, Marek Vasut wrote: > Write a driver that does the GPIO setup in some generic way? Wasn't there > something about this stuff going on? Shawn, you had some comments /wrt the M28 > DTS. This might be what you meant? > No. What Richard is looking for is a way to set/clear gpio, while what I was commenting on M28 DTS is the way to configure a pin into gpio mode. In this series, Richard chose to use "hog" feature to configure the pin into gpio mode, which means defining a pinctrl state for pin controller node itself, then at pinctrl subsystem init time, pinctrl core will configure these "hog" pins properly. The "hog" method makes some sense for Richard's case, where the pin has no clear device/driver owner, and when gpio_request call can not configure the requested pin into gpio mode automatically yet.
On Wed, Jun 13, 2012 at 9:34 AM, Richard Zhao <richard.zhao@freescale.com> wrote: > +static void __init imx6q_config_on_boot(void) > +{ > + struct device_node *np; > + struct property *pp; > + int cnt, len, i; > + int gpio; > + > + np = of_find_node_by_path("/config-on-boot"); > + if (!np) > + return; > + cnt = of_gpio_named_count(np, "output-gpios"); > + pp = of_find_property(np, "output-gpio-values", &len); > + if (!pp || cnt != len / sizeof(u32)) { > + pr_err("Invalid config-on-boot gpios!\n"); > + of_node_put(np); > + return; > + } > + for (i = 0; i < cnt; i++) { > + gpio = of_get_named_gpio(np, "output-gpios", i); > + if (gpio_is_valid(gpio)) > + gpio_request_one(gpio, GPIOF_OUT_INIT_HIGH, > + "config-on-boot"); > + } > + > + of_node_put(np); > +} Couldn't this function be made generic and put outside of mach-imx6q.c so that other platforms could use it? I need the same for mxs.
Fabio Estevam <festevam@gmail.com> wrote: >On Wed, Jun 13, 2012 at 9:34 AM, Richard Zhao ><richard.zhao@freescale.com> wrote: > >> +static void __init imx6q_config_on_boot(void) >> +{ >> + struct device_node *np; >> + struct property *pp; >> + int cnt, len, i; >> + int gpio; >> + >> + np = of_find_node_by_path("/config-on-boot"); >> + if (!np) >> + return; >> + cnt = of_gpio_named_count(np, "output-gpios"); >> + pp = of_find_property(np, "output-gpio-values", &len); >> + if (!pp || cnt != len / sizeof(u32)) { >> + pr_err("Invalid config-on-boot gpios!\n"); >> + of_node_put(np); >> + return; >> + } >> + for (i = 0; i < cnt; i++) { >> + gpio = of_get_named_gpio(np, "output-gpios", i); >> + if (gpio_is_valid(gpio)) >> + gpio_request_one(gpio, GPIOF_OUT_INIT_HIGH, >> + "config-on-boot"); >> + } >> + >> + of_node_put(np); >> +} > >Couldn't this function be made generic and put outside of mach-imx6q.c >so that other platforms could use it? > >I need the same for mxs. Yes, it is common. But it must be called after populate devices. Rob didn't agree the way yet.
On 06/13/2012 08:33 PM, Richard Zhao wrote: > On Wed, Jun 13, 2012 at 11:50:35AM -0500, Rob Herring wrote: >> On 06/13/2012 10:28 AM, Richard Zhao wrote: >>> On Wed, Jun 13, 2012 at 10:09:54AM -0500, Rob Herring wrote: >>>> On 06/13/2012 07:34 AM, Richard Zhao wrote: >>>>> Sometimes, boards have gpios that don't own by any driver or owner >>>>> by a generic driver that don't like hacks. Such gpios is normally >>>>> output and need setup once on boot. So I introduce the config-on-boot >>>>> gpios. >>>>> >>>>> Signed-off-by: Richard Zhao <richard.zhao@freescale.com> >>>>> Cc: Shawn Guo <shawn.guo@linaro.org> >>>>> Cc: Rob Herring <rob.herring@calxeda.com> >>>>> Cc: Grant Likely <grant.likely@secretlab.ca> >>>>> --- >>>>> .../devicetree/bindings/arm/config-on-boot.txt | 12 +++++++ >>>>> arch/arm/boot/dts/imx6q-sabrelite.dts | 7 ++++ >>>>> arch/arm/mach-imx/mach-imx6q.c | 35 ++++++++++++++++++++ >>>>> 3 files changed, 54 insertions(+) >>>>> create mode 100644 Documentation/devicetree/bindings/arm/config-on-boot.txt >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/arm/config-on-boot.txt b/Documentation/devicetree/bindings/arm/config-on-boot.txt >>>>> new file mode 100644 >>>>> index 0000000..f98ed74 >>>>> --- /dev/null >>>>> +++ b/Documentation/devicetree/bindings/arm/config-on-boot.txt >>>>> @@ -0,0 +1,12 @@ >>>>> +* Configure on Boot >>>>> + >>>>> +Node name: config-on-boot >>>>> + It must be in root node. config-on-boot means to describe settings that needs >>>>> + to be set one time on boot but aren't owned by any driver, or the owned driver >>>>> + is too generic to handle such settings. For example, usb hub uses generic >>>>> + driver in usb core code, a on-board usb may need deassert reset pin. >>>> >>>> NAK. This is not a h/w description >>> It's misc things, but is hw description. >> >> You are defining something based on whether linux has a driver or not. >> That should not matter to DT. That's backwards. > I just define the board needs some misc settings. If you think > config-on-boot is not good, we can name it like config-misc. Even worse. >> >>>> and should be solved within the >>>> kernel or bootloader. Either fix this in u-boot, >>> Kernel might be better not to depend on uboot. >>>> the platform code, or >>> How do I get gpio in platfrom code without dts description? >>>> make the generic driver support this in a generic way. >>> It's called just after populate devices. It's hard for generic driver to >>> decide when it's called. >> >> You need to describe that you have a hub on the usb bus and add the gpio >> line to that node. Just like PCI is probe-able, you still need DT nodes >> sometimes for cases like this. > PCI has dev id and may add quirks. But for embedded, I don't know how > to connect a of node to a hub device enumerated by usb core code. And > it may also pollute the usb core code. USB also uses device and manufacturer ids. One of the main reasons for putting pci devices into dts is to describe out of band signals just like this. Creating a binding and support code for full usb bus topology would be a lot of work which is why I propose a simpler approach below. >> A simpler approach would be to just add >> the gpio to the ehci controller node, but that's not exactly correct. > That's exactly why this patch comes out. I mean just something like "fsl,hub-reset-gpios" in the ehci device node. It's at least under a usb node. Whether the ehci driver handles this or you just have a separate piece of code to find this property and setup the gpio is up to you. Rob
On Wed, Jun 20, 2012 at 09:29:39AM -0500, Rob Herring wrote: > I mean just something like "fsl,hub-reset-gpios" in the ehci device > node. It's at least under a usb node. Whether the ehci driver handles > this or you just have a separate piece of code to find this property and > setup the gpio is up to you. > I ever had an argument on this. If we have "fsl,hub-reset-gpios" defined in ehci device node, it should simply be ehci driver who needs to find this property and set up the gpio. Defining the property in ehci device node while having platform code to find the property just seems wired to me. That said, if it's not ehci driver who will use the property, we should not define the property in in ehci device node.
On 06/20/2012 08:29 AM, Rob Herring wrote: > On 06/13/2012 08:33 PM, Richard Zhao wrote: >> On Wed, Jun 13, 2012 at 11:50:35AM -0500, Rob Herring wrote: >>> On 06/13/2012 10:28 AM, Richard Zhao wrote: >>>> On Wed, Jun 13, 2012 at 10:09:54AM -0500, Rob Herring wrote: >>>>> On 06/13/2012 07:34 AM, Richard Zhao wrote: >>>>>> Sometimes, boards have gpios that don't own by any driver or owner >>>>>> by a generic driver that don't like hacks. Such gpios is normally >>>>>> output and need setup once on boot. So I introduce the config-on-boot >>>>>> gpios. ... >>>>>> diff --git a/Documentation/devicetree/bindings/arm/config-on-boot.txt b/Documentation/devicetree/bindings/arm/config-on-boot.txt ... >>>>>> +Node name: config-on-boot >>>>>> + It must be in root node. config-on-boot means to describe settings that needs >>>>>> + to be set one time on boot but aren't owned by any driver, or the owned driver >>>>>> + is too generic to handle such settings. For example, usb hub uses generic >>>>>> + driver in usb core code, a on-board usb may need deassert reset pin. >>>>> >>>>> NAK. This is not a h/w description ... >>> You need to describe that you have a hub on the usb bus and add the gpio >>> line to that node. Just like PCI is probe-able, you still need DT nodes >>> sometimes for cases like this. >> >> PCI has dev id and may add quirks. But for embedded, I don't know how >> to connect a of node to a hub device enumerated by usb core code. And >> it may also pollute the usb core code. > > USB also uses device and manufacturer ids. One of the main reasons for > putting pci devices into dts is to describe out of band signals just > like this. > > Creating a binding and support code for full usb bus topology would be a > lot of work which is why I propose a simpler approach below. > >>> A simpler approach would be to just add >>> the gpio to the ehci controller node, but that's not exactly correct. >> That's exactly why this patch comes out. > > I mean just something like "fsl,hub-reset-gpios" in the ehci device > node. It's at least under a usb node. Whether the ehci driver handles > this or you just have a separate piece of code to find this property and > setup the gpio is up to you. I haven't been following the thread, but just noticed it. Very similar things (DT nodes for probed devices) are coming up quite a bit recently, for example: Re: Where to power on the wifi device before loading the driver. http://www.spinics.net/lists/arm-kernel/msg180368.html dt: rfkill-gpio: add bindings documentation http://www.spinics.net/lists/linux-tegra/msg03977.html I wonder if there's any kind of infra-structure or standardized bindings that would be useful here?
> I haven't been following the thread, but just noticed it. Very similar > things (DT nodes for probed devices) are coming up quite a bit recently, > for example: > > Re: Where to power on the wifi device before loading the driver. > http://www.spinics.net/lists/arm-kernel/msg180368.html > > dt: rfkill-gpio: add bindings documentation > http://www.spinics.net/lists/linux-tegra/msg03977.html > > I wonder if there's any kind of infra-structure or standardized bindings > that would be useful here? Hi Stephen, For your suggestion: sdhci@78000000 { compatible = "nvidia,tegra30-sdhci", "nvidia,tegra20-sdhci"; reg = <0x78000000 0x200>; interrupts = <0 14 0x04>; sdio-device { reset-gpios = <...>; enable-gpios = <...>; }; }; I have a question, if reset-gpio or enable-gpio is not set, how bus (sd or usb) know it is a sdio device as the wifi is not powered on at the time, it will not response host's command, the hub at usb bus is the same.
On 06/20/2012 10:05 AM, Shawn Guo wrote: > On Wed, Jun 20, 2012 at 09:29:39AM -0500, Rob Herring wrote: >> I mean just something like "fsl,hub-reset-gpios" in the ehci device >> node. It's at least under a usb node. Whether the ehci driver handles >> this or you just have a separate piece of code to find this property and >> setup the gpio is up to you. >> > I ever had an argument on this. If we have "fsl,hub-reset-gpios" > defined in ehci device node, it should simply be ehci driver who needs > to find this property and set up the gpio. Defining the property in > ehci device node while having platform code to find the property just > seems wired to me. That said, if it's not ehci driver who will use > the property, we should not define the property in in ehci device node. There's no requirement that 1 DT node be 1 driver. That's purely an OS decision and it's usually simpler that way. But if it it's just a one time thing at boot, then its fine for the platform code to handle it. Rob
On 06/20/2012 07:32 PM, Chen Peter-B29397 wrote: > >> I haven't been following the thread, but just noticed it. Very similar >> things (DT nodes for probed devices) are coming up quite a bit recently, >> for example: >> >> Re: Where to power on the wifi device before loading the driver. >> http://www.spinics.net/lists/arm-kernel/msg180368.html >> >> dt: rfkill-gpio: add bindings documentation >> http://www.spinics.net/lists/linux-tegra/msg03977.html >> >> I wonder if there's any kind of infra-structure or standardized bindings >> that would be useful here? > > Hi Stephen, > For your suggestion: > > sdhci@78000000 { > compatible = "nvidia,tegra30-sdhci", "nvidia,tegra20-sdhci"; > reg = <0x78000000 0x200>; > interrupts = <0 14 0x04>; > > sdio-device { > reset-gpios = <...>; > enable-gpios = <...>; > }; > }; > > I have a question, if reset-gpio or enable-gpio is not set, how bus (sd or usb) know > it is a sdio device as the wifi is not powered on at the time, it will not response host's > command, the hub at usb bus is the same. That's a good point. We need those properties to be part of the device that controls the port/slot/connector, not the thing that plugs into it. Having each such binding and driver implement this themselves might not be a good idea though. Perhaps we can standardize on a mechanism for this; something vaguely like rfkill, but for arbitrary types of devices connected to ports. For USB at least, where the power/reset/... GPIOs might affect just one port on a hub rather than the root USB host controller, the issue of representing these properties on a dynamically probed device still applies. Something like: ehci { /* The following node is dynamically detected, although the hub * IC is physically soldered onto the board */ hub { hub { /* also dynamic */ port@2 { child-supply = <®ulator>; reset-gpios = <&gpio 0 0>; }; }; }; }; But without knowing what might be connected to port@2, how do we know how to sequence the power supply and reset GPIOs according to its requirements? I guess this is part of the specification of the port itself though, not the attached device, since anything that is allowed to plug in must conform to whatever sequencing the port is designed to give. If the connected device is known, the port@2 node in DT can encode whatever sequencing requirements are associated with the known connected device.
> > That's a good point. We need those properties to be part of the device > that controls the port/slot/connector, not the thing that plugs into it. > Having each such binding and driver implement this themselves might not > be a good idea though. Perhaps we can standardize on a mechanism for > this; something vaguely like rfkill, but for arbitrary types of devices > connected to ports. > > For USB at least, where the power/reset/... GPIOs might affect just one > port on a hub rather than the root USB host controller, the issue of > representing these properties on a dynamically probed device still > applies. Something like: > > ehci { > /* The following node is dynamically detected, although the hub > * IC is physically soldered onto the board > */ > hub { > hub { /* also dynamic */ > port@2 { > child-supply = <®ulator>; > reset-gpios = <&gpio 0 0>; > }; > }; > }; > }; > > But without knowing what might be connected to port@2, how do we know > how to sequence the power supply and reset GPIOs according to its > requirements? I guess this is part of the specification of the port > itself though, not the attached device, since anything that is allowed > to plug in must conform to whatever sequencing the port is designed to > give. If the connected device is known, the port@2 node in DT can encode > whatever sequencing requirements are associated with the known connected > device. In fact, for specific usb/sdio interface device, there should be a specific interface class driver for it, the best place is at that class driver, like wifi or 3g driver, as it is chip/device specific. But the generic 3G and HUB devices are special, there is only one common driver for each of them (drivers/usb/core/hub.c & drivers/serial/option.c)
On 06/21/2012 06:03 PM, Chen Peter-B29397 wrote: > >> >> That's a good point. We need those properties to be part of the device >> that controls the port/slot/connector, not the thing that plugs into it. >> Having each such binding and driver implement this themselves might not >> be a good idea though. Perhaps we can standardize on a mechanism for >> this; something vaguely like rfkill, but for arbitrary types of devices >> connected to ports. >> >> For USB at least, where the power/reset/... GPIOs might affect just one >> port on a hub rather than the root USB host controller, the issue of >> representing these properties on a dynamically probed device still >> applies. Something like: >> >> ehci { >> /* The following node is dynamically detected, although the hub >> * IC is physically soldered onto the board >> */ >> hub { >> hub { /* also dynamic */ >> port@2 { >> child-supply = <®ulator>; >> reset-gpios = <&gpio 0 0>; >> }; >> }; >> }; >> }; >> >> But without knowing what might be connected to port@2, how do we know >> how to sequence the power supply and reset GPIOs according to its >> requirements? I guess this is part of the specification of the port >> itself though, not the attached device, since anything that is allowed >> to plug in must conform to whatever sequencing the port is designed to >> give. If the connected device is known, the port@2 node in DT can encode >> whatever sequencing requirements are associated with the known connected >> device. > > In fact, for specific usb/sdio interface device, there should be a specific interface > class driver for it, the best place is at that class driver, like wifi or 3g driver, > as it is chip/device specific. But like you said, there's no way to tell what's attached to a USB or SDIO port before probing it, at least if the port is at all user-accessible. The class driver for the host-side of the port (USB hub or SDIO controller) would be the one that needs to implement this, not the class driver for the thing that could be plugged in (WiFi or 3G device). > But the generic 3G and HUB devices are special, there is only one common driver > for each of them (drivers/usb/core/hub.c & drivers/serial/option.c)
> > But like you said, there's no way to tell what's attached to a USB or > SDIO port before probing it, at least if the port is at all > user-accessible. The class driver for the host-side of the port (USB hub > or SDIO controller) would be the one that needs to implement this, not > the class driver for the thing that could be plugged in (WiFi or 3G > device). > Agree, I did not consider the situation that the class driver is loaded as module and only be loaded when that kinds of device (wifi/3g) has found (through udev). Maybe we have to add misc control (power/reset) for devices at sd or usb controller driver's probe, and those gpio are belonged to controller's. > > But the generic 3G and HUB devices are special, there is only one > common driver > > for each of them (drivers/usb/core/hub.c & drivers/serial/option.c)
On 13.06.2012 14:34, Richard Zhao wrote: > Sometimes, boards have gpios that don't own by any driver or owner > by a generic driver that don't like hacks. Such gpios is normally > output and need setup once on boot. So I introduce the config-on-boot > gpios. > > Signed-off-by: Richard Zhao <richard.zhao@freescale.com> > Cc: Shawn Guo <shawn.guo@linaro.org> > Cc: Rob Herring <rob.herring@calxeda.com> > Cc: Grant Likely <grant.likely@secretlab.ca> > --- > .../devicetree/bindings/arm/config-on-boot.txt | 12 +++++++ > arch/arm/boot/dts/imx6q-sabrelite.dts | 7 ++++ > arch/arm/mach-imx/mach-imx6q.c | 35 ++++++++++++++++++++ > 3 files changed, 54 insertions(+) > create mode 100644 Documentation/devicetree/bindings/arm/config-on-boot.txt > > diff --git a/Documentation/devicetree/bindings/arm/config-on-boot.txt b/Documentation/devicetree/bindings/arm/config-on-boot.txt > new file mode 100644 > index 0000000..f98ed74 > --- /dev/null > +++ b/Documentation/devicetree/bindings/arm/config-on-boot.txt > @@ -0,0 +1,12 @@ > +* Configure on Boot > + > +Node name: config-on-boot > + It must be in root node. config-on-boot means to describe settings that needs > + to be set one time on boot but aren't owned by any driver, or the owned driver > + is too generic to handle such settings. For example, usb hub uses generic > + driver in usb core code, a on-board usb may need deassert reset pin. > + > +Optional properties: > +- output-gpios: Output gpio array that needs to set. > +- output-gpio-values: This property is required if output-gpios is set. > + The value is a array of 0 or 1. Total count eaquals the number of gpios. > diff --git a/arch/arm/boot/dts/imx6q-sabrelite.dts b/arch/arm/boot/dts/imx6q-sabrelite.dts > index e0ec929..1dd2261 100644 > --- a/arch/arm/boot/dts/imx6q-sabrelite.dts > +++ b/arch/arm/boot/dts/imx6q-sabrelite.dts > @@ -17,6 +17,13 @@ > model = "Freescale i.MX6 Quad SABRE Lite Board"; > compatible = "fsl,imx6q-sabrelite", "fsl,imx6q"; > > + config-on-boot { > + output-gpios = < > + &gpio3 22 0>; /* vbus reset */ > + output-gpio-values = < > + 1>; /* vbus reset */ > + }; > + > memory { > reg = <0x10000000 0x40000000>; > }; > diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c > index b47e98b..577cf19 100644 > --- a/arch/arm/mach-imx/mach-imx6q.c > +++ b/arch/arm/mach-imx/mach-imx6q.c > @@ -19,6 +19,7 @@ > #include <linux/irqdomain.h> > #include <linux/of.h> > #include <linux/of_address.h> > +#include <linux/of_gpio.h> > #include <linux/of_irq.h> > #include <linux/of_platform.h> > #include <linux/pinctrl/machine.h> > @@ -113,6 +114,38 @@ static void __init imx6q_sabrelite_init(void) > imx6q_sabrelite_cko1_setup(); > } > > +static void __init imx6q_config_on_boot(void) > +{ > + struct device_node *np; > + struct property *pp; > + int cnt, len, i; > + int gpio; > + > + np = of_find_node_by_path("/config-on-boot"); > + if (!np) > + return; > + cnt = of_gpio_named_count(np, "output-gpios"); > + pp = of_find_property(np, "output-gpio-values", &len); > + if (!pp || cnt != len / sizeof(u32)) { > + pr_err("Invalid config-on-boot gpios!\n"); > + of_node_put(np); > + return; > + } > + for (i = 0; i < cnt; i++) { > + gpio = of_get_named_gpio(np, "output-gpios", i); > + if (gpio_is_valid(gpio)) > + gpio_request_one(gpio, GPIOF_OUT_INIT_HIGH, > + "config-on-boot"); While trying to use this code: Is there a special reason why all pins are set to HIGH here? The contents of 'output-gpio-values' seem to be ignored? Best regards Dirk
On Tue, Jul 17, 2012 at 02:30:17PM +0200, Dirk Behme wrote: > On 13.06.2012 14:34, Richard Zhao wrote: > >Sometimes, boards have gpios that don't own by any driver or owner > >by a generic driver that don't like hacks. Such gpios is normally > >output and need setup once on boot. So I introduce the config-on-boot > >gpios. > > > >Signed-off-by: Richard Zhao <richard.zhao@freescale.com> > >Cc: Shawn Guo <shawn.guo@linaro.org> > >Cc: Rob Herring <rob.herring@calxeda.com> > >Cc: Grant Likely <grant.likely@secretlab.ca> > >--- > > .../devicetree/bindings/arm/config-on-boot.txt | 12 +++++++ > > arch/arm/boot/dts/imx6q-sabrelite.dts | 7 ++++ > > arch/arm/mach-imx/mach-imx6q.c | 35 ++++++++++++++++++++ > > 3 files changed, 54 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/arm/config-on-boot.txt > > > >diff --git a/Documentation/devicetree/bindings/arm/config-on-boot.txt b/Documentation/devicetree/bindings/arm/config-on-boot.txt > >new file mode 100644 > >index 0000000..f98ed74 > >--- /dev/null > >+++ b/Documentation/devicetree/bindings/arm/config-on-boot.txt > >@@ -0,0 +1,12 @@ > >+* Configure on Boot > >+ > >+Node name: config-on-boot > >+ It must be in root node. config-on-boot means to describe settings that needs > >+ to be set one time on boot but aren't owned by any driver, or the owned driver > >+ is too generic to handle such settings. For example, usb hub uses generic > >+ driver in usb core code, a on-board usb may need deassert reset pin. > >+ > >+Optional properties: > >+- output-gpios: Output gpio array that needs to set. > >+- output-gpio-values: This property is required if output-gpios is set. > >+ The value is a array of 0 or 1. Total count eaquals the number of gpios. > >diff --git a/arch/arm/boot/dts/imx6q-sabrelite.dts b/arch/arm/boot/dts/imx6q-sabrelite.dts > >index e0ec929..1dd2261 100644 > >--- a/arch/arm/boot/dts/imx6q-sabrelite.dts > >+++ b/arch/arm/boot/dts/imx6q-sabrelite.dts > >@@ -17,6 +17,13 @@ > > model = "Freescale i.MX6 Quad SABRE Lite Board"; > > compatible = "fsl,imx6q-sabrelite", "fsl,imx6q"; > >+ config-on-boot { > >+ output-gpios = < > >+ &gpio3 22 0>; /* vbus reset */ > >+ output-gpio-values = < > >+ 1>; /* vbus reset */ > >+ }; > >+ > > memory { > > reg = <0x10000000 0x40000000>; > > }; > >diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c > >index b47e98b..577cf19 100644 > >--- a/arch/arm/mach-imx/mach-imx6q.c > >+++ b/arch/arm/mach-imx/mach-imx6q.c > >@@ -19,6 +19,7 @@ > > #include <linux/irqdomain.h> > > #include <linux/of.h> > > #include <linux/of_address.h> > >+#include <linux/of_gpio.h> > > #include <linux/of_irq.h> > > #include <linux/of_platform.h> > > #include <linux/pinctrl/machine.h> > >@@ -113,6 +114,38 @@ static void __init imx6q_sabrelite_init(void) > > imx6q_sabrelite_cko1_setup(); > > } > >+static void __init imx6q_config_on_boot(void) > >+{ > >+ struct device_node *np; > >+ struct property *pp; > >+ int cnt, len, i; > >+ int gpio; > >+ > >+ np = of_find_node_by_path("/config-on-boot"); > >+ if (!np) > >+ return; > >+ cnt = of_gpio_named_count(np, "output-gpios"); > >+ pp = of_find_property(np, "output-gpio-values", &len); > >+ if (!pp || cnt != len / sizeof(u32)) { > >+ pr_err("Invalid config-on-boot gpios!\n"); > >+ of_node_put(np); > >+ return; > >+ } > >+ for (i = 0; i < cnt; i++) { > >+ gpio = of_get_named_gpio(np, "output-gpios", i); > >+ if (gpio_is_valid(gpio)) > >+ gpio_request_one(gpio, GPIOF_OUT_INIT_HIGH, > >+ "config-on-boot"); > > While trying to use this code: Is there a special reason why all > pins are set to HIGH here? The contents of 'output-gpio-values' seem > to be ignored? It may be bug here. The patch is rejected. imx6_sabrelite happens work by default value. So I won't follow it any more. Thanks Richard > > Best regards > > Dirk >
diff --git a/Documentation/devicetree/bindings/arm/config-on-boot.txt b/Documentation/devicetree/bindings/arm/config-on-boot.txt new file mode 100644 index 0000000..f98ed74 --- /dev/null +++ b/Documentation/devicetree/bindings/arm/config-on-boot.txt @@ -0,0 +1,12 @@ +* Configure on Boot + +Node name: config-on-boot + It must be in root node. config-on-boot means to describe settings that needs + to be set one time on boot but aren't owned by any driver, or the owned driver + is too generic to handle such settings. For example, usb hub uses generic + driver in usb core code, a on-board usb may need deassert reset pin. + +Optional properties: +- output-gpios: Output gpio array that needs to set. +- output-gpio-values: This property is required if output-gpios is set. + The value is a array of 0 or 1. Total count eaquals the number of gpios. diff --git a/arch/arm/boot/dts/imx6q-sabrelite.dts b/arch/arm/boot/dts/imx6q-sabrelite.dts index e0ec929..1dd2261 100644 --- a/arch/arm/boot/dts/imx6q-sabrelite.dts +++ b/arch/arm/boot/dts/imx6q-sabrelite.dts @@ -17,6 +17,13 @@ model = "Freescale i.MX6 Quad SABRE Lite Board"; compatible = "fsl,imx6q-sabrelite", "fsl,imx6q"; + config-on-boot { + output-gpios = < + &gpio3 22 0>; /* vbus reset */ + output-gpio-values = < + 1>; /* vbus reset */ + }; + memory { reg = <0x10000000 0x40000000>; }; diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c index b47e98b..577cf19 100644 --- a/arch/arm/mach-imx/mach-imx6q.c +++ b/arch/arm/mach-imx/mach-imx6q.c @@ -19,6 +19,7 @@ #include <linux/irqdomain.h> #include <linux/of.h> #include <linux/of_address.h> +#include <linux/of_gpio.h> #include <linux/of_irq.h> #include <linux/of_platform.h> #include <linux/pinctrl/machine.h> @@ -113,6 +114,38 @@ static void __init imx6q_sabrelite_init(void) imx6q_sabrelite_cko1_setup(); } +static void __init imx6q_config_on_boot(void) +{ + struct device_node *np; + struct property *pp; + int cnt, len, i; + int gpio; + + np = of_find_node_by_path("/config-on-boot"); + if (!np) + return; + cnt = of_gpio_named_count(np, "output-gpios"); + pp = of_find_property(np, "output-gpio-values", &len); + if (!pp || cnt != len / sizeof(u32)) { + pr_err("Invalid config-on-boot gpios!\n"); + of_node_put(np); + return; + } + for (i = 0; i < cnt; i++) { + gpio = of_get_named_gpio(np, "output-gpios", i); + if (gpio_is_valid(gpio)) + gpio_request_one(gpio, GPIOF_OUT_INIT_HIGH, + "config-on-boot"); + } + + of_node_put(np); +} + +static void __init imx6q_post_populate(void) +{ + imx6q_config_on_boot(); +} + static void __init imx6q_init_machine(void) { /* @@ -126,6 +159,8 @@ static void __init imx6q_init_machine(void) of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL); + imx6q_post_populate(); + imx6q_pm_init(); }
Sometimes, boards have gpios that don't own by any driver or owner by a generic driver that don't like hacks. Such gpios is normally output and need setup once on boot. So I introduce the config-on-boot gpios. Signed-off-by: Richard Zhao <richard.zhao@freescale.com> Cc: Shawn Guo <shawn.guo@linaro.org> Cc: Rob Herring <rob.herring@calxeda.com> Cc: Grant Likely <grant.likely@secretlab.ca> --- .../devicetree/bindings/arm/config-on-boot.txt | 12 +++++++ arch/arm/boot/dts/imx6q-sabrelite.dts | 7 ++++ arch/arm/mach-imx/mach-imx6q.c | 35 ++++++++++++++++++++ 3 files changed, 54 insertions(+) create mode 100644 Documentation/devicetree/bindings/arm/config-on-boot.txt