diff mbox series

ARM: at91sam9x5/dt: enable internal pull-up for i2c-gpio lines

Message ID 20190729160022.22781-1-uwe@kleine-koenig.org (mailing list archive)
State New, archived
Headers show
Series ARM: at91sam9x5/dt: enable internal pull-up for i2c-gpio lines | expand

Commit Message

Uwe Kleine-König July 29, 2019, 4 p.m. UTC
This is what I need on my Arietta G25 to be able to just connect an i2c
device to the pin headers.
Also remove the comment that doesn't tell more than the pin declaration.

Signed-off-by: Uwe Kleine-König <uwe@kleine-koenig.org>
---
Hello,

not sure this is change suitable for the SoC dtsi. I'll leave it to the
at91 maintainers to decide.

Best regards
Uwe

 arch/arm/boot/dts/at91sam9x5.dtsi  | 12 ++++++------
 include/dt-bindings/pinctrl/at91.h |  2 ++
 2 files changed, 8 insertions(+), 6 deletions(-)

Comments

Ludovic Desroches July 31, 2019, 11:18 a.m. UTC | #1
On Mon, Jul 29, 2019 at 06:00:22PM +0200, Uwe Kleine-König wrote:
> External E-Mail
> 
> 
> This is what I need on my Arietta G25 to be able to just connect an i2c
> device to the pin headers.
> Also remove the comment that doesn't tell more than the pin declaration.
> 
> Signed-off-by: Uwe Kleine-König <uwe@kleine-koenig.org>
> ---
> Hello,
> 
> not sure this is change suitable for the SoC dtsi. I'll leave it to the
> at91 maintainers to decide.

Hello Uwe,

Usually we have pull-ups for those signals on our board. In this case,
it's useless to activate the internal pull-up. Even if I am not sure we
were consistent in our policy about what goes in the SoC dtsi and what
goes in the dts board, I would prefer to have this at the board level.

Regards

Ludovic

> 
> Best regards
> Uwe
> 
>  arch/arm/boot/dts/at91sam9x5.dtsi  | 12 ++++++------
>  include/dt-bindings/pinctrl/at91.h |  2 ++
>  2 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/at91sam9x5.dtsi b/arch/arm/boot/dts/at91sam9x5.dtsi
> index ef47c005ef03..5fc38626795e 100644
> --- a/arch/arm/boot/dts/at91sam9x5.dtsi
> +++ b/arch/arm/boot/dts/at91sam9x5.dtsi
> @@ -437,24 +437,24 @@
>  				i2c_gpio0 {
>  					pinctrl_i2c_gpio0: i2c_gpio0-0 {
>  						atmel,pins =
> -							<AT91_PIOA 30 AT91_PERIPH_GPIO AT91_PINCTRL_MULTI_DRIVE	/* PA30 gpio multidrive I2C0 data */
> -							 AT91_PIOA 31 AT91_PERIPH_GPIO AT91_PINCTRL_MULTI_DRIVE>;	/* PA31 gpio multidrive I2C0 clock */
> +							<AT91_PIOA 30 AT91_PERIPH_GPIO AT91_PINCTRL_MULTI_DRIVE_PU
> +							 AT91_PIOA 31 AT91_PERIPH_GPIO AT91_PINCTRL_MULTI_DRIVE_PU>;
>  					};
>  				};
>  
>  				i2c_gpio1 {
>  					pinctrl_i2c_gpio1: i2c_gpio1-0 {
>  						atmel,pins =
> -							<AT91_PIOC 0 AT91_PERIPH_GPIO AT91_PINCTRL_MULTI_DRIVE	/* PC0 gpio multidrive I2C1 data */
> -							 AT91_PIOC 1 AT91_PERIPH_GPIO AT91_PINCTRL_MULTI_DRIVE>;	/* PC1 gpio multidrive I2C1 clock */
> +							<AT91_PIOC 0 AT91_PERIPH_GPIO AT91_PINCTRL_MULTI_DRIVE_PU
> +							 AT91_PIOC 1 AT91_PERIPH_GPIO AT91_PINCTRL_MULTI_DRIVE_PU>;
>  					};
>  				};
>  
>  				i2c_gpio2 {
>  					pinctrl_i2c_gpio2: i2c_gpio2-0 {
>  						atmel,pins =
> -							<AT91_PIOB 4 AT91_PERIPH_GPIO AT91_PINCTRL_MULTI_DRIVE	/* PB4 gpio multidrive I2C2 data */
> -							 AT91_PIOB 5 AT91_PERIPH_GPIO AT91_PINCTRL_MULTI_DRIVE>;	/* PB5 gpio multidrive I2C2 clock */
> +							<AT91_PIOB 4 AT91_PERIPH_GPIO AT91_PINCTRL_MULTI_DRIVE_PU
> +							 AT91_PIOB 5 AT91_PERIPH_GPIO AT91_PINCTRL_MULTI_DRIVE_PU>;
>  					};
>  				};
>  
> diff --git a/include/dt-bindings/pinctrl/at91.h b/include/dt-bindings/pinctrl/at91.h
> index 3831f91fb3ba..c72d40f50acd 100644
> --- a/include/dt-bindings/pinctrl/at91.h
> +++ b/include/dt-bindings/pinctrl/at91.h
> @@ -20,6 +20,8 @@
>  #define AT91_PINCTRL_DEBOUNCE		(1 << 16)
>  #define AT91_PINCTRL_DEBOUNCE_VAL(x)	(x << 17)
>  
> +#define AT91_PINCTRL_MULTI_DRIVE_PU	(AT91_PINCTRL_MULTI_DRIVE | AT91_PINCTRL_PULL_UP)
> +
>  #define AT91_PINCTRL_PULL_UP_DEGLITCH	(AT91_PINCTRL_PULL_UP | AT91_PINCTRL_DEGLITCH)
>  
>  #define AT91_PINCTRL_DRIVE_STRENGTH_DEFAULT		(0x0 << 5)
> -- 
> 2.20.1
>
Uwe Kleine-König July 31, 2019, 10:14 p.m. UTC | #2
hello Ludovic,

On Wed, Jul 31, 2019 at 01:18:28PM +0200, Ludovic Desroches wrote:
> On Mon, Jul 29, 2019 at 06:00:22PM +0200, Uwe Kleine-König wrote:
> > External E-Mail
> > 
> > 
> > This is what I need on my Arietta G25 to be able to just connect an i2c
> > device to the pin headers.
> > Also remove the comment that doesn't tell more than the pin declaration.
> > 
> > Signed-off-by: Uwe Kleine-König <uwe@kleine-koenig.org>
> > ---
> > Hello,
> > 
> > not sure this is change suitable for the SoC dtsi. I'll leave it to the
> > at91 maintainers to decide.
> 
> Usually we have pull-ups for those signals on our board. In this case,
> it's useless to activate the internal pull-up. Even if I am not sure we
> were consistent in our policy about what goes in the SoC dtsi and what
> goes in the dts board, I would prefer to have this at the board level.

OK.

The define I added in include/dt-bindings/pinctrl/at91.h would be nice
to have though to simplify overriding the SoC's default pinctrl. Would
it be OK to add this? (I kept the diff below.) Obviously there isn't a
mainline user (yet) because it's a custom modification of my Arietta
board that it has an i2c device (and it depends on the modification that
I need the internal pull up).

Best regards
Uwe

> > diff --git a/include/dt-bindings/pinctrl/at91.h b/include/dt-bindings/pinctrl/at91.h
> > index 3831f91fb3ba..c72d40f50acd 100644
> > --- a/include/dt-bindings/pinctrl/at91.h
> > +++ b/include/dt-bindings/pinctrl/at91.h
> > @@ -20,6 +20,8 @@
> >  #define AT91_PINCTRL_DEBOUNCE		(1 << 16)
> >  #define AT91_PINCTRL_DEBOUNCE_VAL(x)	(x << 17)
> >  
> > +#define AT91_PINCTRL_MULTI_DRIVE_PU	(AT91_PINCTRL_MULTI_DRIVE | AT91_PINCTRL_PULL_UP)
> > +
> >  #define AT91_PINCTRL_PULL_UP_DEGLITCH	(AT91_PINCTRL_PULL_UP | AT91_PINCTRL_DEGLITCH)
> >  
> >  #define AT91_PINCTRL_DRIVE_STRENGTH_DEFAULT		(0x0 << 5)
diff mbox series

Patch

diff --git a/arch/arm/boot/dts/at91sam9x5.dtsi b/arch/arm/boot/dts/at91sam9x5.dtsi
index ef47c005ef03..5fc38626795e 100644
--- a/arch/arm/boot/dts/at91sam9x5.dtsi
+++ b/arch/arm/boot/dts/at91sam9x5.dtsi
@@ -437,24 +437,24 @@ 
 				i2c_gpio0 {
 					pinctrl_i2c_gpio0: i2c_gpio0-0 {
 						atmel,pins =
-							<AT91_PIOA 30 AT91_PERIPH_GPIO AT91_PINCTRL_MULTI_DRIVE	/* PA30 gpio multidrive I2C0 data */
-							 AT91_PIOA 31 AT91_PERIPH_GPIO AT91_PINCTRL_MULTI_DRIVE>;	/* PA31 gpio multidrive I2C0 clock */
+							<AT91_PIOA 30 AT91_PERIPH_GPIO AT91_PINCTRL_MULTI_DRIVE_PU
+							 AT91_PIOA 31 AT91_PERIPH_GPIO AT91_PINCTRL_MULTI_DRIVE_PU>;
 					};
 				};
 
 				i2c_gpio1 {
 					pinctrl_i2c_gpio1: i2c_gpio1-0 {
 						atmel,pins =
-							<AT91_PIOC 0 AT91_PERIPH_GPIO AT91_PINCTRL_MULTI_DRIVE	/* PC0 gpio multidrive I2C1 data */
-							 AT91_PIOC 1 AT91_PERIPH_GPIO AT91_PINCTRL_MULTI_DRIVE>;	/* PC1 gpio multidrive I2C1 clock */
+							<AT91_PIOC 0 AT91_PERIPH_GPIO AT91_PINCTRL_MULTI_DRIVE_PU
+							 AT91_PIOC 1 AT91_PERIPH_GPIO AT91_PINCTRL_MULTI_DRIVE_PU>;
 					};
 				};
 
 				i2c_gpio2 {
 					pinctrl_i2c_gpio2: i2c_gpio2-0 {
 						atmel,pins =
-							<AT91_PIOB 4 AT91_PERIPH_GPIO AT91_PINCTRL_MULTI_DRIVE	/* PB4 gpio multidrive I2C2 data */
-							 AT91_PIOB 5 AT91_PERIPH_GPIO AT91_PINCTRL_MULTI_DRIVE>;	/* PB5 gpio multidrive I2C2 clock */
+							<AT91_PIOB 4 AT91_PERIPH_GPIO AT91_PINCTRL_MULTI_DRIVE_PU
+							 AT91_PIOB 5 AT91_PERIPH_GPIO AT91_PINCTRL_MULTI_DRIVE_PU>;
 					};
 				};
 
diff --git a/include/dt-bindings/pinctrl/at91.h b/include/dt-bindings/pinctrl/at91.h
index 3831f91fb3ba..c72d40f50acd 100644
--- a/include/dt-bindings/pinctrl/at91.h
+++ b/include/dt-bindings/pinctrl/at91.h
@@ -20,6 +20,8 @@ 
 #define AT91_PINCTRL_DEBOUNCE		(1 << 16)
 #define AT91_PINCTRL_DEBOUNCE_VAL(x)	(x << 17)
 
+#define AT91_PINCTRL_MULTI_DRIVE_PU	(AT91_PINCTRL_MULTI_DRIVE | AT91_PINCTRL_PULL_UP)
+
 #define AT91_PINCTRL_PULL_UP_DEGLITCH	(AT91_PINCTRL_PULL_UP | AT91_PINCTRL_DEGLITCH)
 
 #define AT91_PINCTRL_DRIVE_STRENGTH_DEFAULT		(0x0 << 5)