| Message ID | 1368636386-17138-1-git-send-email-dmurphy@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 11:46-20130515, Dan Murphy wrote: > The GPIO for LED D1 on the omap4-panda a1-a3 rev and the omap4-panda-es > are different. > > A1-A3 = gpio_wk7 > ES = gpio_110 > > There is no change to LED D2 > > Abstract away the pinmux and the LED definitions for the two boards into > the respective DTS files. > > Signed-off-by: Dan Murphy <dmurphy@ti.com> > --- nit: Giving patch history is a nice practise. > arch/arm/boot/dts/omap4-panda-common.dtsi | 16 +++++++++++- > arch/arm/boot/dts/omap4-panda-es.dts | 40 +++++++++++++++++++++++++++++ > 2 files changed, 55 insertions(+), 1 deletions(-) > > diff --git a/arch/arm/boot/dts/omap4-panda-common.dtsi b/arch/arm/boot/dts/omap4-panda-common.dtsi > index 03bd60d..2b516af 100644 > --- a/arch/arm/boot/dts/omap4-panda-common.dtsi > +++ b/arch/arm/boot/dts/omap4-panda-common.dtsi > @@ -16,7 +16,7 @@ > reg = <0x80000000 0x40000000>; /* 1 GB */ > }; > > - leds { > + leds: leds { > compatible = "gpio-leds"; > heartbeat { > label = "pandaboard::status1"; > @@ -137,6 +137,20 @@ I missed noticing this previously, Apologies on the same. Considering that drivers/leds/leds-gpio.c has ability to handle pinctrl, One better option might be to provide pinctrl phandle with leds - Couple of reasons why this might be good: a) one gets the following warning at boot: "leds-gpio leds.8: pins are not configured from the driver" b) you donot need to setup the pins by default at boot - it is not mandatory for the system functionality, instead we do it *if* the driver is enabled. Further, optionally, all you'd have to do in panda-es.dts is the following &led_wkgpio_pins { pinctrl-single,pins = < 0x1c 0x3 /* gpio_wk8 OUTPUT | MODE 3 */ >; } Similarly for gpios override for panda-es. > }; > }; > > +&omap4_pmx_wkup { > + pinctrl-names = "default"; > + pinctrl-0 = < > + &led_wkgpio_pins > + >; > + > + led_wkgpio_pins: pinmux_leds_wkpins { > + pinctrl-single,pins = < > + 0x1a 0x3 /* gpio_wk7 OUTPUT | MODE 3 */ > + 0x1c 0x3 /* gpio_wk8 OUTPUT | MODE 3 */ > + >; > + }; > +}; > + > &i2c1 { > pinctrl-names = "default"; > pinctrl-0 = <&i2c1_pins>; > diff --git a/arch/arm/boot/dts/omap4-panda-es.dts b/arch/arm/boot/dts/omap4-panda-es.dts > index f1d8c21..e6f696d 100644 > --- a/arch/arm/boot/dts/omap4-panda-es.dts > +++ b/arch/arm/boot/dts/omap4-panda-es.dts > @@ -34,3 +34,43 @@ > 0x5e 0x100 /* hdmi_sda.hdmi_sda INPUT | MODE 0 */ > >; > }; > + > +&leds { > + compatible = "gpio-leds"; > + heartbeat { > + label = "pandaboard::status1"; > + gpios = <&gpio4 14 0>; > + linux,default-trigger = "heartbeat"; > + }; > + mmc { > + label = "pandaboard::status2"; > + gpios = <&gpio1 8 0>; > + linux,default-trigger = "mmc0"; > + }; > +}; > + > +&omap4_pmx_core { > + pinctrl-names = "default"; > + pinctrl-0 = < > + &led_gpio_pins > + >; > + > + led_gpio_pins: gpio_led_pmx { > + pinctrl-single,pins = < > + 0xb6 0x3 /* gpio_110 OUTPUT | MODE 3 */ > + >; > + }; > +}; > + > +&omap4_pmx_wkup { > + pinctrl-names = "default"; > + pinctrl-0 = < > + &led_wkgpio_pins > + >; > + > + led_wkgpio_pins: pinmux_leds_wkpins { > + pinctrl-single,pins = < > + 0x1c 0x3 /* gpio_wk8 OUTPUT | MODE 3 */ > + >; > + }; > +}; > -- > 1.7.5.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-omap" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
On 05/15/2013 12:05 PM, Nishanth Menon wrote: > On 11:46-20130515, Dan Murphy wrote: >> The GPIO for LED D1 on the omap4-panda a1-a3 rev and the omap4-panda-es >> are different. >> >> A1-A3 = gpio_wk7 >> ES = gpio_110 >> >> There is no change to LED D2 >> >> Abstract away the pinmux and the LED definitions for the two boards into >> the respective DTS files. >> >> Signed-off-by: Dan Murphy <dmurphy@ti.com> >> --- > nit: Giving patch history is a nice practise. >> arch/arm/boot/dts/omap4-panda-common.dtsi | 16 +++++++++++- >> arch/arm/boot/dts/omap4-panda-es.dts | 40 +++++++++++++++++++++++++++++ >> 2 files changed, 55 insertions(+), 1 deletions(-) >> >> diff --git a/arch/arm/boot/dts/omap4-panda-common.dtsi b/arch/arm/boot/dts/omap4-panda-common.dtsi >> index 03bd60d..2b516af 100644 >> --- a/arch/arm/boot/dts/omap4-panda-common.dtsi >> +++ b/arch/arm/boot/dts/omap4-panda-common.dtsi >> @@ -16,7 +16,7 @@ >> reg = <0x80000000 0x40000000>; /* 1 GB */ >> }; >> >> - leds { >> + leds: leds { >> compatible = "gpio-leds"; >> heartbeat { >> label = "pandaboard::status1"; >> @@ -137,6 +137,20 @@ > I missed noticing this previously, Apologies on the same. > Considering that drivers/leds/leds-gpio.c has ability to handle pinctrl, > One better option might be to provide pinctrl phandle with leds - > Couple of reasons why this might be good: > a) one gets the following warning at boot: > "leds-gpio leds.8: pins are not configured from the driver" > b) you donot need to setup the pins by default at boot - it is not > mandatory for the system functionality, instead we do it *if* the driver > is enabled. > Further, optionally, all you'd have to do in panda-es.dts is the following > &led_wkgpio_pins { > pinctrl-single,pins = < > 0x1c 0x3 /* gpio_wk8 OUTPUT | MODE 3 */ > >; > } > Similarly for gpios override for panda-es. I am not sure you really want to do this. If I make the pinctrl part of the led structure then the only way the gpio_wk7 on a1-a3 to be configured is when the CONFIG_LEDS_GPIO flag is set. Do you really want that dependency? You did say it was a key fix At least this way the pins are configured regardless of that flag. >> }; >> }; >> >> +&omap4_pmx_wkup { >> + pinctrl-names = "default"; >> + pinctrl-0 = < >> + &led_wkgpio_pins >> + >; >> + >> + led_wkgpio_pins: pinmux_leds_wkpins { >> + pinctrl-single,pins = < >> + 0x1a 0x3 /* gpio_wk7 OUTPUT | MODE 3 */ >> + 0x1c 0x3 /* gpio_wk8 OUTPUT | MODE 3 */ >> + >; >> + }; >> +}; >> + >> &i2c1 { >> pinctrl-names = "default"; >> pinctrl-0 = <&i2c1_pins>; >> diff --git a/arch/arm/boot/dts/omap4-panda-es.dts b/arch/arm/boot/dts/omap4-panda-es.dts >> index f1d8c21..e6f696d 100644 >> --- a/arch/arm/boot/dts/omap4-panda-es.dts >> +++ b/arch/arm/boot/dts/omap4-panda-es.dts >> @@ -34,3 +34,43 @@ >> 0x5e 0x100 /* hdmi_sda.hdmi_sda INPUT | MODE 0 */ >> >; >> }; >> + >> +&leds { >> + compatible = "gpio-leds"; >> + heartbeat { >> + label = "pandaboard::status1"; >> + gpios = <&gpio4 14 0>; >> + linux,default-trigger = "heartbeat"; >> + }; >> + mmc { >> + label = "pandaboard::status2"; >> + gpios = <&gpio1 8 0>; >> + linux,default-trigger = "mmc0"; >> + }; >> +}; >> + >> +&omap4_pmx_core { >> + pinctrl-names = "default"; >> + pinctrl-0 = < >> + &led_gpio_pins >> + >; >> + >> + led_gpio_pins: gpio_led_pmx { >> + pinctrl-single,pins = < >> + 0xb6 0x3 /* gpio_110 OUTPUT | MODE 3 */ >> + >; >> + }; >> +}; >> + >> +&omap4_pmx_wkup { >> + pinctrl-names = "default"; >> + pinctrl-0 = < >> + &led_wkgpio_pins >> + >; >> + >> + led_wkgpio_pins: pinmux_leds_wkpins { >> + pinctrl-single,pins = < >> + 0x1c 0x3 /* gpio_wk8 OUTPUT | MODE 3 */ >> + >; >> + }; >> +}; >> -- >> 1.7.5.4 >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-omap" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, May 16, 2013 at 10:35 AM, Dan Murphy <dmurphy@ti.com> wrote: > I am not sure you really want to do this. > If I make the pinctrl part of the led structure then the only way the gpio_wk7 on a1-a3 to be configured is when > the CONFIG_LEDS_GPIO flag is set. > > Do you really want that dependency? You did say it was a key fix > At least this way the pins are configured regardless of that flag. That is better as the system will be left in the pinmux configuration handed over from bootloader. The point being, muxing up pins even when not needed(config switched off) has no real benefit - in this case albeit, the default mux was causing a bug. pinctrl IMHO should be considered as any other resource, if it is not mandatory for boot, and needed only for a device functionality when probed, it should done there only. just my 2 cents.
On 05/16/2013 01:18 PM, Nishanth Menon wrote: > On Thu, May 16, 2013 at 10:35 AM, Dan Murphy <dmurphy@ti.com> wrote: >> I am not sure you really want to do this. >> If I make the pinctrl part of the led structure then the only way the gpio_wk7 on a1-a3 to be configured is when >> the CONFIG_LEDS_GPIO flag is set. >> >> Do you really want that dependency? You did say it was a key fix >> At least this way the pins are configured regardless of that flag. > That is better as the system will be left in the pinmux configuration > handed over from bootloader. So you want to depend on a boot loader to configure pins correctly for the kernel? Hmmm seems risky to me. > The point being, muxing up pins even when not needed(config switched > off) has no real benefit - in this case albeit, the default mux was > causing a bug. > pinctrl IMHO should be considered as any other resource, if it is not > mandatory for boot, and needed only for a device functionality when > probed, it should done there only. > > just my 2 cents. > -- > To unsubscribe from this list: send the line "unsubscribe linux-omap" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, May 16, 2013 at 3:22 PM, Dan Murphy <dmurphy@ti.com> wrote: > On 05/16/2013 01:18 PM, Nishanth Menon wrote: >> On Thu, May 16, 2013 at 10:35 AM, Dan Murphy <dmurphy@ti.com> wrote: >>> I am not sure you really want to do this. >>> If I make the pinctrl part of the led structure then the only way the gpio_wk7 on a1-a3 to be configured is when >>> the CONFIG_LEDS_GPIO flag is set. >>> >>> Do you really want that dependency? You did say it was a key fix >>> At least this way the pins are configured regardless of that flag. >> That is better as the system will be left in the pinmux configuration >> handed over from bootloader. > So you want to depend on a boot loader to configure pins correctly for the kernel? > Hmmm seems risky to me. Not really! if it is a critical pinmux, pinmux defaults are great, else belongs definitely to device->pinctrl :) depending on bootloader pinmux implies NOT having pinmux even for device - NO. that is definitely not what I stated. Regards, NM
diff --git a/arch/arm/boot/dts/omap4-panda-common.dtsi b/arch/arm/boot/dts/omap4-panda-common.dtsi index 03bd60d..2b516af 100644 --- a/arch/arm/boot/dts/omap4-panda-common.dtsi +++ b/arch/arm/boot/dts/omap4-panda-common.dtsi @@ -16,7 +16,7 @@ reg = <0x80000000 0x40000000>; /* 1 GB */ }; - leds { + leds: leds { compatible = "gpio-leds"; heartbeat { label = "pandaboard::status1"; @@ -137,6 +137,20 @@ }; }; +&omap4_pmx_wkup { + pinctrl-names = "default"; + pinctrl-0 = < + &led_wkgpio_pins + >; + + led_wkgpio_pins: pinmux_leds_wkpins { + pinctrl-single,pins = < + 0x1a 0x3 /* gpio_wk7 OUTPUT | MODE 3 */ + 0x1c 0x3 /* gpio_wk8 OUTPUT | MODE 3 */ + >; + }; +}; + &i2c1 { pinctrl-names = "default"; pinctrl-0 = <&i2c1_pins>; diff --git a/arch/arm/boot/dts/omap4-panda-es.dts b/arch/arm/boot/dts/omap4-panda-es.dts index f1d8c21..e6f696d 100644 --- a/arch/arm/boot/dts/omap4-panda-es.dts +++ b/arch/arm/boot/dts/omap4-panda-es.dts @@ -34,3 +34,43 @@ 0x5e 0x100 /* hdmi_sda.hdmi_sda INPUT | MODE 0 */ >; }; + +&leds { + compatible = "gpio-leds"; + heartbeat { + label = "pandaboard::status1"; + gpios = <&gpio4 14 0>; + linux,default-trigger = "heartbeat"; + }; + mmc { + label = "pandaboard::status2"; + gpios = <&gpio1 8 0>; + linux,default-trigger = "mmc0"; + }; +}; + +&omap4_pmx_core { + pinctrl-names = "default"; + pinctrl-0 = < + &led_gpio_pins + >; + + led_gpio_pins: gpio_led_pmx { + pinctrl-single,pins = < + 0xb6 0x3 /* gpio_110 OUTPUT | MODE 3 */ + >; + }; +}; + +&omap4_pmx_wkup { + pinctrl-names = "default"; + pinctrl-0 = < + &led_wkgpio_pins + >; + + led_wkgpio_pins: pinmux_leds_wkpins { + pinctrl-single,pins = < + 0x1c 0x3 /* gpio_wk8 OUTPUT | MODE 3 */ + >; + }; +};
The GPIO for LED D1 on the omap4-panda a1-a3 rev and the omap4-panda-es are different. A1-A3 = gpio_wk7 ES = gpio_110 There is no change to LED D2 Abstract away the pinmux and the LED definitions for the two boards into the respective DTS files. Signed-off-by: Dan Murphy <dmurphy@ti.com> --- arch/arm/boot/dts/omap4-panda-common.dtsi | 16 +++++++++++- arch/arm/boot/dts/omap4-panda-es.dts | 40 +++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 1 deletions(-)