| Message ID | 1443105855-15738-1-git-send-email-peda@lysator.liu.se (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Peter, Thanks for the patch but you actually got beaten by Sylvain: http://lists.infradead.org/pipermail/linux-arm-kernel/2015-September/368426.html On 24/09/2015 at 16:44:15 +0200, Peter Rosin wrote : > From: Peter Rosin <peda@axentia.se> > > It seems pointless to pullup the tx line, but there is value in pulling > up the rx line. > > Cc: stable@vger.kernel.org > Signed-off-by: Peter Rosin <peda@axentia.se> > --- > arch/arm/boot/dts/sama5d3.dtsi | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/boot/dts/sama5d3.dtsi b/arch/arm/boot/dts/sama5d3.dtsi > index 9e2444b07bce..304a40c5552a 100644 > --- a/arch/arm/boot/dts/sama5d3.dtsi > +++ b/arch/arm/boot/dts/sama5d3.dtsi > @@ -545,8 +545,8 @@ > dbgu { > pinctrl_dbgu: dbgu-0 { > atmel,pins = > - <AT91_PIOB 30 AT91_PERIPH_A AT91_PINCTRL_NONE /* PB30 periph A */ > - AT91_PIOB 31 AT91_PERIPH_A AT91_PINCTRL_PULL_UP>; /* PB31 periph A with pullup */ > + <AT91_PIOB 30 AT91_PERIPH_A AT91_PINCTRL_PULL_UP /* PB30 periph A with pullup */ > + AT91_PIOB 31 AT91_PERIPH_A AT91_PINCTRL_NONE>; /* PB31 periph A */ > }; > }; > > -- > 1.7.10.4 >
On 2015-09-24 16:47, Alexandre Belloni wrote: > Hi Peter, > > Thanks for the patch but you actually got beaten by Sylvain: > http://lists.infradead.org/pipermail/linux-arm-kernel/2015-September/368426.html Ok, great! I just noticed that other ports have the same problem in sama5d3.dtsi. E.g. uart1 { pinctrl_uart1: uart1-0 { atmel,pins = <AT91_PIOA 30 AT91_PERIPH_B AT91_PINCTRL_NONE /* conflicts with TWD0, ISI_VSYNC */ AT91_PIOA 31 AT91_PERIPH_B AT91_PINCTRL_PULL_UP>; /* conflicts with TWCK0, ISI_HSYNC */ }; }; Given that the original bug I found spread all over the map, it seems like someone was confused when the pull-ups were originally added. Someone (else?) at Atmel needs to audit this so that pull-ups are added on the rx-pins instead of the tx-pins. Cheers, Peter > On 24/09/2015 at 16:44:15 +0200, Peter Rosin wrote : >> From: Peter Rosin <peda@axentia.se> >> >> It seems pointless to pullup the tx line, but there is value in pulling >> up the rx line. >> >> Cc: stable@vger.kernel.org >> Signed-off-by: Peter Rosin <peda@axentia.se> >> --- >> arch/arm/boot/dts/sama5d3.dtsi | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/arch/arm/boot/dts/sama5d3.dtsi b/arch/arm/boot/dts/sama5d3.dtsi >> index 9e2444b07bce..304a40c5552a 100644 >> --- a/arch/arm/boot/dts/sama5d3.dtsi >> +++ b/arch/arm/boot/dts/sama5d3.dtsi >> @@ -545,8 +545,8 @@ >> dbgu { >> pinctrl_dbgu: dbgu-0 { >> atmel,pins = >> - <AT91_PIOB 30 AT91_PERIPH_A AT91_PINCTRL_NONE /* PB30 periph A */ >> - AT91_PIOB 31 AT91_PERIPH_A AT91_PINCTRL_PULL_UP>; /* PB31 periph A with pullup */ >> + <AT91_PIOB 30 AT91_PERIPH_A AT91_PINCTRL_PULL_UP /* PB30 periph A with pullup */ >> + AT91_PIOB 31 AT91_PERIPH_A AT91_PINCTRL_NONE>; /* PB31 periph A */ >> }; >> }; >> >> -- >> 1.7.10.4 >> >
On 24/09/2015 at 17:22:54 +0200, Peter Rosin wrote : > I just noticed that other ports have the same problem in sama5d3.dtsi. E.g. > > uart1 { > pinctrl_uart1: uart1-0 { > atmel,pins = > <AT91_PIOA 30 AT91_PERIPH_B AT91_PINCTRL_NONE /* conflicts with TWD0, ISI_VSYNC */ > AT91_PIOA 31 AT91_PERIPH_B AT91_PINCTRL_PULL_UP>; /* conflicts with TWCK0, ISI_HSYNC */ > }; > }; > > Given that the original bug I found spread all over the map, it seems > like someone was confused when the pull-ups were originally added. > Someone (else?) at Atmel needs to audit this so that pull-ups are > added on the rx-pins instead of the tx-pins. > Sure, that is the plan! Thanks again for the report.
Hi again, I forgot about this, and it's been a year. But isn't it time to upstream those pull-up fixes that Sylvain provided? Cheers, Peter On 2015-09-24 16:47, Alexandre Belloni wrote: > Hi Peter, > > Thanks for the patch but you actually got beaten by Sylvain: > http://lists.infradead.org/pipermail/linux-arm-kernel/2015-September/368426.html > > On 24/09/2015 at 16:44:15 +0200, Peter Rosin wrote : >> From: Peter Rosin <peda@axentia.se> >> >> It seems pointless to pullup the tx line, but there is value in pulling >> up the rx line. >> >> Cc: stable@vger.kernel.org >> Signed-off-by: Peter Rosin <peda@axentia.se> >> --- >> arch/arm/boot/dts/sama5d3.dtsi | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/arch/arm/boot/dts/sama5d3.dtsi b/arch/arm/boot/dts/sama5d3.dtsi >> index 9e2444b07bce..304a40c5552a 100644 >> --- a/arch/arm/boot/dts/sama5d3.dtsi >> +++ b/arch/arm/boot/dts/sama5d3.dtsi >> @@ -545,8 +545,8 @@ >> dbgu { >> pinctrl_dbgu: dbgu-0 { >> atmel,pins = >> - <AT91_PIOB 30 AT91_PERIPH_A AT91_PINCTRL_NONE /* PB30 periph A */ >> - AT91_PIOB 31 AT91_PERIPH_A AT91_PINCTRL_PULL_UP>; /* PB31 periph A with pullup */ >> + <AT91_PIOB 30 AT91_PERIPH_A AT91_PINCTRL_PULL_UP /* PB30 periph A with pullup */ >> + AT91_PIOB 31 AT91_PERIPH_A AT91_PINCTRL_NONE>; /* PB31 periph A */ >> }; >> }; >> >> -- >> 1.7.10.4 >> >
Hi Peter, On Sun, Oct 16, 2016 at 12:57:09PM +0200, Peter Rosin wrote: > Hi again, > > I forgot about this, and it's been a year. But isn't it time to > upstream those pull-up fixes that Sylvain provided? Thank you for your reminder, actually I waited an answer on this nice subject: http://lists.infradead.org/pipermail/linux-arm-kernel/2015-September/368643.html But this is not relevant for this change anyway, if we should set a pull-(up|down) on all push-pull pads to reduce power consumption it should be enforced in the pinctrl driver, the dt must stay logical. I just sent a v2 with all useless comments removed as suggested by Alexandre and without the explanation about the removed extra power consumption when removing a pull-(up|down) on a push-pull output because it does even seem to be the contrary on Atmel SoCs. Sylvain
diff --git a/arch/arm/boot/dts/sama5d3.dtsi b/arch/arm/boot/dts/sama5d3.dtsi index 9e2444b07bce..304a40c5552a 100644 --- a/arch/arm/boot/dts/sama5d3.dtsi +++ b/arch/arm/boot/dts/sama5d3.dtsi @@ -545,8 +545,8 @@ dbgu { pinctrl_dbgu: dbgu-0 { atmel,pins = - <AT91_PIOB 30 AT91_PERIPH_A AT91_PINCTRL_NONE /* PB30 periph A */ - AT91_PIOB 31 AT91_PERIPH_A AT91_PINCTRL_PULL_UP>; /* PB31 periph A with pullup */ + <AT91_PIOB 30 AT91_PERIPH_A AT91_PINCTRL_PULL_UP /* PB30 periph A with pullup */ + AT91_PIOB 31 AT91_PERIPH_A AT91_PINCTRL_NONE>; /* PB31 periph A */ }; };