All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] dt-bindings: mfd: Improve readability for TPS65217 interrupt sources
@ 2016-11-21 13:15 ` Milo Kim
  0 siblings, 0 replies; 19+ messages in thread
From: Milo Kim @ 2016-11-21 13:15 UTC (permalink / raw)
  To: bcousson, Tony Lindgren
  Cc: Lee Jones, linux-omap, devicetree, linux-arm-kernel,
	linux-kernel, Robert Nelson, Milo Kim

AC and USB interrupts are related with external power input.
PB interrupt means push button pressed or released event.
Use better human readable definitions.

Signed-off-by: Milo Kim <woogyom.kim@gmail.com>
---
 arch/arm/boot/dts/am335x-bone-common.dtsi | 4 ++--
 include/dt-bindings/mfd/tps65217.h        | 6 +++---
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/arm/boot/dts/am335x-bone-common.dtsi b/arch/arm/boot/dts/am335x-bone-common.dtsi
index dc561d5..1848d58 100644
--- a/arch/arm/boot/dts/am335x-bone-common.dtsi
+++ b/arch/arm/boot/dts/am335x-bone-common.dtsi
@@ -319,13 +319,13 @@
 	ti,pmic-shutdown-controller;
 
 	charger {
-		interrupts = <TPS65217_IRQ_AC>, <TPS65217_IRQ_USB>;
+		interrupts = <TPS65217_IRQ_AC_POWER>, <TPS65217_IRQ_USB_POWER>;
 		interrupts-names = "AC", "USB";
 		status = "okay";
 	};
 
 	pwrbutton {
-		interrupts = <TPS65217_IRQ_PB>;
+		interrupts = <TPS65217_IRQ_PUSHBUTTON>;
 		status = "okay";
 	};
 
diff --git a/include/dt-bindings/mfd/tps65217.h b/include/dt-bindings/mfd/tps65217.h
index cafb9e6..0293fdd 100644
--- a/include/dt-bindings/mfd/tps65217.h
+++ b/include/dt-bindings/mfd/tps65217.h
@@ -19,8 +19,8 @@
 #ifndef __DT_BINDINGS_TPS65217_H__
 #define __DT_BINDINGS_TPS65217_H__
 
-#define TPS65217_IRQ_USB	0
-#define TPS65217_IRQ_AC		1
-#define TPS65217_IRQ_PB		2
+#define TPS65217_IRQ_USB_POWER		0	/* USB power state change */
+#define TPS65217_IRQ_AC_POWER		1	/* AC power state change */
+#define TPS65217_IRQ_PUSHBUTTON		2	/* Push button state change */
 
 #endif
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH] dt-bindings: mfd: Improve readability for TPS65217 interrupt sources
@ 2016-11-21 13:15 ` Milo Kim
  0 siblings, 0 replies; 19+ messages in thread
From: Milo Kim @ 2016-11-21 13:15 UTC (permalink / raw)
  To: linux-arm-kernel

AC and USB interrupts are related with external power input.
PB interrupt means push button pressed or released event.
Use better human readable definitions.

Signed-off-by: Milo Kim <woogyom.kim@gmail.com>
---
 arch/arm/boot/dts/am335x-bone-common.dtsi | 4 ++--
 include/dt-bindings/mfd/tps65217.h        | 6 +++---
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/arm/boot/dts/am335x-bone-common.dtsi b/arch/arm/boot/dts/am335x-bone-common.dtsi
index dc561d5..1848d58 100644
--- a/arch/arm/boot/dts/am335x-bone-common.dtsi
+++ b/arch/arm/boot/dts/am335x-bone-common.dtsi
@@ -319,13 +319,13 @@
 	ti,pmic-shutdown-controller;
 
 	charger {
-		interrupts = <TPS65217_IRQ_AC>, <TPS65217_IRQ_USB>;
+		interrupts = <TPS65217_IRQ_AC_POWER>, <TPS65217_IRQ_USB_POWER>;
 		interrupts-names = "AC", "USB";
 		status = "okay";
 	};
 
 	pwrbutton {
-		interrupts = <TPS65217_IRQ_PB>;
+		interrupts = <TPS65217_IRQ_PUSHBUTTON>;
 		status = "okay";
 	};
 
diff --git a/include/dt-bindings/mfd/tps65217.h b/include/dt-bindings/mfd/tps65217.h
index cafb9e6..0293fdd 100644
--- a/include/dt-bindings/mfd/tps65217.h
+++ b/include/dt-bindings/mfd/tps65217.h
@@ -19,8 +19,8 @@
 #ifndef __DT_BINDINGS_TPS65217_H__
 #define __DT_BINDINGS_TPS65217_H__
 
-#define TPS65217_IRQ_USB	0
-#define TPS65217_IRQ_AC		1
-#define TPS65217_IRQ_PB		2
+#define TPS65217_IRQ_USB_POWER		0	/* USB power state change */
+#define TPS65217_IRQ_AC_POWER		1	/* AC power state change */
+#define TPS65217_IRQ_PUSHBUTTON		2	/* Push button state change */
 
 #endif
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH] dt-bindings: mfd: Improve readability for TPS65217 interrupt sources
  2016-11-21 13:15 ` Milo Kim
@ 2016-11-22 15:57   ` Lee Jones
  -1 siblings, 0 replies; 19+ messages in thread
From: Lee Jones @ 2016-11-22 15:57 UTC (permalink / raw)
  To: Milo Kim
  Cc: bcousson, Tony Lindgren, linux-omap, devicetree,
	linux-arm-kernel, linux-kernel, Robert Nelson

On Mon, 21 Nov 2016, Milo Kim wrote:

> AC and USB interrupts are related with external power input.
> PB interrupt means push button pressed or released event.
> Use better human readable definitions.
> 
> Signed-off-by: Milo Kim <woogyom.kim@gmail.com>
> ---
>  arch/arm/boot/dts/am335x-bone-common.dtsi | 4 ++--
>  include/dt-bindings/mfd/tps65217.h        | 6 +++---
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/am335x-bone-common.dtsi b/arch/arm/boot/dts/am335x-bone-common.dtsi
> index dc561d5..1848d58 100644
> --- a/arch/arm/boot/dts/am335x-bone-common.dtsi
> +++ b/arch/arm/boot/dts/am335x-bone-common.dtsi
> @@ -319,13 +319,13 @@
>  	ti,pmic-shutdown-controller;
>  
>  	charger {
> -		interrupts = <TPS65217_IRQ_AC>, <TPS65217_IRQ_USB>;
> +		interrupts = <TPS65217_IRQ_AC_POWER>, <TPS65217_IRQ_USB_POWER>;
>  		interrupts-names = "AC", "USB";
>  		status = "okay";
>  	};
>  
>  	pwrbutton {
> -		interrupts = <TPS65217_IRQ_PB>;
> +		interrupts = <TPS65217_IRQ_PUSHBUTTON>;

Push button or power button?

>  		status = "okay";
>  	};
>  
> diff --git a/include/dt-bindings/mfd/tps65217.h b/include/dt-bindings/mfd/tps65217.h
> index cafb9e6..0293fdd 100644
> --- a/include/dt-bindings/mfd/tps65217.h
> +++ b/include/dt-bindings/mfd/tps65217.h
> @@ -19,8 +19,8 @@
>  #ifndef __DT_BINDINGS_TPS65217_H__
>  #define __DT_BINDINGS_TPS65217_H__
>  
> -#define TPS65217_IRQ_USB	0
> -#define TPS65217_IRQ_AC		1
> -#define TPS65217_IRQ_PB		2
> +#define TPS65217_IRQ_USB_POWER		0	/* USB power state change */
> +#define TPS65217_IRQ_AC_POWER		1	/* AC power state change */
> +#define TPS65217_IRQ_PUSHBUTTON		2	/* Push button state change */

This changes the ABI.

It will require a DT Ack.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH] dt-bindings: mfd: Improve readability for TPS65217 interrupt sources
@ 2016-11-22 15:57   ` Lee Jones
  0 siblings, 0 replies; 19+ messages in thread
From: Lee Jones @ 2016-11-22 15:57 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, 21 Nov 2016, Milo Kim wrote:

> AC and USB interrupts are related with external power input.
> PB interrupt means push button pressed or released event.
> Use better human readable definitions.
> 
> Signed-off-by: Milo Kim <woogyom.kim@gmail.com>
> ---
>  arch/arm/boot/dts/am335x-bone-common.dtsi | 4 ++--
>  include/dt-bindings/mfd/tps65217.h        | 6 +++---
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/am335x-bone-common.dtsi b/arch/arm/boot/dts/am335x-bone-common.dtsi
> index dc561d5..1848d58 100644
> --- a/arch/arm/boot/dts/am335x-bone-common.dtsi
> +++ b/arch/arm/boot/dts/am335x-bone-common.dtsi
> @@ -319,13 +319,13 @@
>  	ti,pmic-shutdown-controller;
>  
>  	charger {
> -		interrupts = <TPS65217_IRQ_AC>, <TPS65217_IRQ_USB>;
> +		interrupts = <TPS65217_IRQ_AC_POWER>, <TPS65217_IRQ_USB_POWER>;
>  		interrupts-names = "AC", "USB";
>  		status = "okay";
>  	};
>  
>  	pwrbutton {
> -		interrupts = <TPS65217_IRQ_PB>;
> +		interrupts = <TPS65217_IRQ_PUSHBUTTON>;

Push button or power button?

>  		status = "okay";
>  	};
>  
> diff --git a/include/dt-bindings/mfd/tps65217.h b/include/dt-bindings/mfd/tps65217.h
> index cafb9e6..0293fdd 100644
> --- a/include/dt-bindings/mfd/tps65217.h
> +++ b/include/dt-bindings/mfd/tps65217.h
> @@ -19,8 +19,8 @@
>  #ifndef __DT_BINDINGS_TPS65217_H__
>  #define __DT_BINDINGS_TPS65217_H__
>  
> -#define TPS65217_IRQ_USB	0
> -#define TPS65217_IRQ_AC		1
> -#define TPS65217_IRQ_PB		2
> +#define TPS65217_IRQ_USB_POWER		0	/* USB power state change */
> +#define TPS65217_IRQ_AC_POWER		1	/* AC power state change */
> +#define TPS65217_IRQ_PUSHBUTTON		2	/* Push button state change */

This changes the ABI.

It will require a DT Ack.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] dt-bindings: mfd: Improve readability for TPS65217 interrupt sources
@ 2016-11-22 16:00     ` Lee Jones
  0 siblings, 0 replies; 19+ messages in thread
From: Lee Jones @ 2016-11-22 16:00 UTC (permalink / raw)
  To: Milo Kim
  Cc: bcousson, Tony Lindgren, linux-omap, devicetree,
	linux-arm-kernel, linux-kernel, Robert Nelson

On Tue, 22 Nov 2016, Lee Jones wrote:

> On Mon, 21 Nov 2016, Milo Kim wrote:
> 
> > AC and USB interrupts are related with external power input.
> > PB interrupt means push button pressed or released event.
> > Use better human readable definitions.
> > 
> > Signed-off-by: Milo Kim <woogyom.kim@gmail.com>
> > ---
> >  arch/arm/boot/dts/am335x-bone-common.dtsi | 4 ++--
> >  include/dt-bindings/mfd/tps65217.h        | 6 +++---
> >  2 files changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/am335x-bone-common.dtsi b/arch/arm/boot/dts/am335x-bone-common.dtsi
> > index dc561d5..1848d58 100644
> > --- a/arch/arm/boot/dts/am335x-bone-common.dtsi
> > +++ b/arch/arm/boot/dts/am335x-bone-common.dtsi
> > @@ -319,13 +319,13 @@
> >  	ti,pmic-shutdown-controller;
> >  
> >  	charger {
> > -		interrupts = <TPS65217_IRQ_AC>, <TPS65217_IRQ_USB>;
> > +		interrupts = <TPS65217_IRQ_AC_POWER>, <TPS65217_IRQ_USB_POWER>;
> >  		interrupts-names = "AC", "USB";
> >  		status = "okay";
> >  	};
> >  
> >  	pwrbutton {
> > -		interrupts = <TPS65217_IRQ_PB>;
> > +		interrupts = <TPS65217_IRQ_PUSHBUTTON>;
> 
> Push button or power button?
> 
> >  		status = "okay";
> >  	};
> >  
> > diff --git a/include/dt-bindings/mfd/tps65217.h b/include/dt-bindings/mfd/tps65217.h
> > index cafb9e6..0293fdd 100644
> > --- a/include/dt-bindings/mfd/tps65217.h
> > +++ b/include/dt-bindings/mfd/tps65217.h
> > @@ -19,8 +19,8 @@
> >  #ifndef __DT_BINDINGS_TPS65217_H__
> >  #define __DT_BINDINGS_TPS65217_H__
> >  
> > -#define TPS65217_IRQ_USB	0
> > -#define TPS65217_IRQ_AC		1
> > -#define TPS65217_IRQ_PB		2
> > +#define TPS65217_IRQ_USB_POWER		0	/* USB power state change */
> > +#define TPS65217_IRQ_AC_POWER		1	/* AC power state change */
> > +#define TPS65217_IRQ_PUSHBUTTON		2	/* Push button state change */
> 
> This changes the ABI.
> 
> It will require a DT Ack.

Tell a lie.  Sorry, I was getting false positives from my grep.  It
looks like you use the same scheme from within include/linux.  I
suggest that you probable don't want to do that.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] dt-bindings: mfd: Improve readability for TPS65217 interrupt sources
@ 2016-11-22 16:00     ` Lee Jones
  0 siblings, 0 replies; 19+ messages in thread
From: Lee Jones @ 2016-11-22 16:00 UTC (permalink / raw)
  To: Milo Kim
  Cc: bcousson-rdvid1DuHRBWk0Htik3J/w, Tony Lindgren,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Robert Nelson

On Tue, 22 Nov 2016, Lee Jones wrote:

> On Mon, 21 Nov 2016, Milo Kim wrote:
> 
> > AC and USB interrupts are related with external power input.
> > PB interrupt means push button pressed or released event.
> > Use better human readable definitions.
> > 
> > Signed-off-by: Milo Kim <woogyom.kim-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > ---
> >  arch/arm/boot/dts/am335x-bone-common.dtsi | 4 ++--
> >  include/dt-bindings/mfd/tps65217.h        | 6 +++---
> >  2 files changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/am335x-bone-common.dtsi b/arch/arm/boot/dts/am335x-bone-common.dtsi
> > index dc561d5..1848d58 100644
> > --- a/arch/arm/boot/dts/am335x-bone-common.dtsi
> > +++ b/arch/arm/boot/dts/am335x-bone-common.dtsi
> > @@ -319,13 +319,13 @@
> >  	ti,pmic-shutdown-controller;
> >  
> >  	charger {
> > -		interrupts = <TPS65217_IRQ_AC>, <TPS65217_IRQ_USB>;
> > +		interrupts = <TPS65217_IRQ_AC_POWER>, <TPS65217_IRQ_USB_POWER>;
> >  		interrupts-names = "AC", "USB";
> >  		status = "okay";
> >  	};
> >  
> >  	pwrbutton {
> > -		interrupts = <TPS65217_IRQ_PB>;
> > +		interrupts = <TPS65217_IRQ_PUSHBUTTON>;
> 
> Push button or power button?
> 
> >  		status = "okay";
> >  	};
> >  
> > diff --git a/include/dt-bindings/mfd/tps65217.h b/include/dt-bindings/mfd/tps65217.h
> > index cafb9e6..0293fdd 100644
> > --- a/include/dt-bindings/mfd/tps65217.h
> > +++ b/include/dt-bindings/mfd/tps65217.h
> > @@ -19,8 +19,8 @@
> >  #ifndef __DT_BINDINGS_TPS65217_H__
> >  #define __DT_BINDINGS_TPS65217_H__
> >  
> > -#define TPS65217_IRQ_USB	0
> > -#define TPS65217_IRQ_AC		1
> > -#define TPS65217_IRQ_PB		2
> > +#define TPS65217_IRQ_USB_POWER		0	/* USB power state change */
> > +#define TPS65217_IRQ_AC_POWER		1	/* AC power state change */
> > +#define TPS65217_IRQ_PUSHBUTTON		2	/* Push button state change */
> 
> This changes the ABI.
> 
> It will require a DT Ack.

Tell a lie.  Sorry, I was getting false positives from my grep.  It
looks like you use the same scheme from within include/linux.  I
suggest that you probable don't want to do that.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH] dt-bindings: mfd: Improve readability for TPS65217 interrupt sources
@ 2016-11-22 16:00     ` Lee Jones
  0 siblings, 0 replies; 19+ messages in thread
From: Lee Jones @ 2016-11-22 16:00 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, 22 Nov 2016, Lee Jones wrote:

> On Mon, 21 Nov 2016, Milo Kim wrote:
> 
> > AC and USB interrupts are related with external power input.
> > PB interrupt means push button pressed or released event.
> > Use better human readable definitions.
> > 
> > Signed-off-by: Milo Kim <woogyom.kim@gmail.com>
> > ---
> >  arch/arm/boot/dts/am335x-bone-common.dtsi | 4 ++--
> >  include/dt-bindings/mfd/tps65217.h        | 6 +++---
> >  2 files changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/am335x-bone-common.dtsi b/arch/arm/boot/dts/am335x-bone-common.dtsi
> > index dc561d5..1848d58 100644
> > --- a/arch/arm/boot/dts/am335x-bone-common.dtsi
> > +++ b/arch/arm/boot/dts/am335x-bone-common.dtsi
> > @@ -319,13 +319,13 @@
> >  	ti,pmic-shutdown-controller;
> >  
> >  	charger {
> > -		interrupts = <TPS65217_IRQ_AC>, <TPS65217_IRQ_USB>;
> > +		interrupts = <TPS65217_IRQ_AC_POWER>, <TPS65217_IRQ_USB_POWER>;
> >  		interrupts-names = "AC", "USB";
> >  		status = "okay";
> >  	};
> >  
> >  	pwrbutton {
> > -		interrupts = <TPS65217_IRQ_PB>;
> > +		interrupts = <TPS65217_IRQ_PUSHBUTTON>;
> 
> Push button or power button?
> 
> >  		status = "okay";
> >  	};
> >  
> > diff --git a/include/dt-bindings/mfd/tps65217.h b/include/dt-bindings/mfd/tps65217.h
> > index cafb9e6..0293fdd 100644
> > --- a/include/dt-bindings/mfd/tps65217.h
> > +++ b/include/dt-bindings/mfd/tps65217.h
> > @@ -19,8 +19,8 @@
> >  #ifndef __DT_BINDINGS_TPS65217_H__
> >  #define __DT_BINDINGS_TPS65217_H__
> >  
> > -#define TPS65217_IRQ_USB	0
> > -#define TPS65217_IRQ_AC		1
> > -#define TPS65217_IRQ_PB		2
> > +#define TPS65217_IRQ_USB_POWER		0	/* USB power state change */
> > +#define TPS65217_IRQ_AC_POWER		1	/* AC power state change */
> > +#define TPS65217_IRQ_PUSHBUTTON		2	/* Push button state change */
> 
> This changes the ABI.
> 
> It will require a DT Ack.

Tell a lie.  Sorry, I was getting false positives from my grep.  It
looks like you use the same scheme from within include/linux.  I
suggest that you probable don't want to do that.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] dt-bindings: mfd: Improve readability for TPS65217 interrupt sources
@ 2016-11-22 21:08       ` Arnd Bergmann
  0 siblings, 0 replies; 19+ messages in thread
From: Arnd Bergmann @ 2016-11-22 21:08 UTC (permalink / raw)
  To: Lee Jones
  Cc: Milo Kim, bcousson, Tony Lindgren, linux-omap, devicetree,
	linux-arm-kernel, linux-kernel, Robert Nelson

On Tuesday, November 22, 2016 4:00:13 PM CET Lee Jones wrote:
> > > diff --git a/include/dt-bindings/mfd/tps65217.h b/include/dt-bindings/mfd/tps65217.h
> > > index cafb9e6..0293fdd 100644
> > > --- a/include/dt-bindings/mfd/tps65217.h
> > > +++ b/include/dt-bindings/mfd/tps65217.h
> > > @@ -19,8 +19,8 @@
> > >  #ifndef __DT_BINDINGS_TPS65217_H__
> > >  #define __DT_BINDINGS_TPS65217_H__
> > >  
> > > -#define TPS65217_IRQ_USB   0
> > > -#define TPS65217_IRQ_AC            1
> > > -#define TPS65217_IRQ_PB            2
> > > +#define TPS65217_IRQ_USB_POWER             0       /* USB power state change */
> > > +#define TPS65217_IRQ_AC_POWER              1       /* AC power state change */
> > > +#define TPS65217_IRQ_PUSHBUTTON            2       /* Push button state change */
> > 
> > This changes the ABI.
> > 
> > It will require a DT Ack.
> 
> Tell a lie.  Sorry, I was getting false positives from my grep.  It
> looks like you use the same scheme from within include/linux.  I
> suggest that you probable don't want to do that.

Doing this change however would cause a bisection problem: you
can't rename just the constants in the header or just the driver
using those constants.

	Arnd

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] dt-bindings: mfd: Improve readability for TPS65217 interrupt sources
@ 2016-11-22 21:08       ` Arnd Bergmann
  0 siblings, 0 replies; 19+ messages in thread
From: Arnd Bergmann @ 2016-11-22 21:08 UTC (permalink / raw)
  To: Lee Jones
  Cc: Milo Kim, bcousson-rdvid1DuHRBWk0Htik3J/w, Tony Lindgren,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Robert Nelson

On Tuesday, November 22, 2016 4:00:13 PM CET Lee Jones wrote:
> > > diff --git a/include/dt-bindings/mfd/tps65217.h b/include/dt-bindings/mfd/tps65217.h
> > > index cafb9e6..0293fdd 100644
> > > --- a/include/dt-bindings/mfd/tps65217.h
> > > +++ b/include/dt-bindings/mfd/tps65217.h
> > > @@ -19,8 +19,8 @@
> > >  #ifndef __DT_BINDINGS_TPS65217_H__
> > >  #define __DT_BINDINGS_TPS65217_H__
> > >  
> > > -#define TPS65217_IRQ_USB   0
> > > -#define TPS65217_IRQ_AC            1
> > > -#define TPS65217_IRQ_PB            2
> > > +#define TPS65217_IRQ_USB_POWER             0       /* USB power state change */
> > > +#define TPS65217_IRQ_AC_POWER              1       /* AC power state change */
> > > +#define TPS65217_IRQ_PUSHBUTTON            2       /* Push button state change */
> > 
> > This changes the ABI.
> > 
> > It will require a DT Ack.
> 
> Tell a lie.  Sorry, I was getting false positives from my grep.  It
> looks like you use the same scheme from within include/linux.  I
> suggest that you probable don't want to do that.

Doing this change however would cause a bisection problem: you
can't rename just the constants in the header or just the driver
using those constants.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH] dt-bindings: mfd: Improve readability for TPS65217 interrupt sources
@ 2016-11-22 21:08       ` Arnd Bergmann
  0 siblings, 0 replies; 19+ messages in thread
From: Arnd Bergmann @ 2016-11-22 21:08 UTC (permalink / raw)
  To: linux-arm-kernel

On Tuesday, November 22, 2016 4:00:13 PM CET Lee Jones wrote:
> > > diff --git a/include/dt-bindings/mfd/tps65217.h b/include/dt-bindings/mfd/tps65217.h
> > > index cafb9e6..0293fdd 100644
> > > --- a/include/dt-bindings/mfd/tps65217.h
> > > +++ b/include/dt-bindings/mfd/tps65217.h
> > > @@ -19,8 +19,8 @@
> > >  #ifndef __DT_BINDINGS_TPS65217_H__
> > >  #define __DT_BINDINGS_TPS65217_H__
> > >  
> > > -#define TPS65217_IRQ_USB   0
> > > -#define TPS65217_IRQ_AC            1
> > > -#define TPS65217_IRQ_PB            2
> > > +#define TPS65217_IRQ_USB_POWER             0       /* USB power state change */
> > > +#define TPS65217_IRQ_AC_POWER              1       /* AC power state change */
> > > +#define TPS65217_IRQ_PUSHBUTTON            2       /* Push button state change */
> > 
> > This changes the ABI.
> > 
> > It will require a DT Ack.
> 
> Tell a lie.  Sorry, I was getting false positives from my grep.  It
> looks like you use the same scheme from within include/linux.  I
> suggest that you probable don't want to do that.

Doing this change however would cause a bisection problem: you
can't rename just the constants in the header or just the driver
using those constants.

	Arnd

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] dt-bindings: mfd: Improve readability for TPS65217 interrupt sources
@ 2016-11-23 11:38     ` Milo Kim
  0 siblings, 0 replies; 19+ messages in thread
From: Milo Kim @ 2016-11-23 11:38 UTC (permalink / raw)
  To: Lee Jones
  Cc: bcousson, Tony Lindgren, linux-omap, devicetree,
	linux-arm-kernel, linux-kernel, Robert Nelson

On 11/23/2016 12:57 AM, Lee Jones wrote:
>>  	pwrbutton {
>> > -		interrupts = <TPS65217_IRQ_PB>;
>> > +		interrupts = <TPS65217_IRQ_PUSHBUTTON>;
> Push button or power button?
>

According to the datasheet, push button interrupt is correct.

	http://www.ti.com/lit/ds/symlink/tps65217.pdf

This is used for a power button input in Beaglebone boards. In other 
words, the power button is one of push button usages.

So, I'd like to keep general name for the interrupt.

Best regards,
Milo

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] dt-bindings: mfd: Improve readability for TPS65217 interrupt sources
@ 2016-11-23 11:38     ` Milo Kim
  0 siblings, 0 replies; 19+ messages in thread
From: Milo Kim @ 2016-11-23 11:38 UTC (permalink / raw)
  To: Lee Jones
  Cc: bcousson-rdvid1DuHRBWk0Htik3J/w, Tony Lindgren,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Robert Nelson

On 11/23/2016 12:57 AM, Lee Jones wrote:
>>  	pwrbutton {
>> > -		interrupts = <TPS65217_IRQ_PB>;
>> > +		interrupts = <TPS65217_IRQ_PUSHBUTTON>;
> Push button or power button?
>

According to the datasheet, push button interrupt is correct.

	http://www.ti.com/lit/ds/symlink/tps65217.pdf

This is used for a power button input in Beaglebone boards. In other 
words, the power button is one of push button usages.

So, I'd like to keep general name for the interrupt.

Best regards,
Milo
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH] dt-bindings: mfd: Improve readability for TPS65217 interrupt sources
@ 2016-11-23 11:38     ` Milo Kim
  0 siblings, 0 replies; 19+ messages in thread
From: Milo Kim @ 2016-11-23 11:38 UTC (permalink / raw)
  To: linux-arm-kernel

On 11/23/2016 12:57 AM, Lee Jones wrote:
>>  	pwrbutton {
>> > -		interrupts = <TPS65217_IRQ_PB>;
>> > +		interrupts = <TPS65217_IRQ_PUSHBUTTON>;
> Push button or power button?
>

According to the datasheet, push button interrupt is correct.

	http://www.ti.com/lit/ds/symlink/tps65217.pdf

This is used for a power button input in Beaglebone boards. In other 
words, the power button is one of push button usages.

So, I'd like to keep general name for the interrupt.

Best regards,
Milo

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] dt-bindings: mfd: Improve readability for TPS65217 interrupt sources
  2016-11-23 11:38     ` Milo Kim
  (?)
@ 2016-11-23 11:51       ` Arnd Bergmann
  -1 siblings, 0 replies; 19+ messages in thread
From: Arnd Bergmann @ 2016-11-23 11:51 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Milo Kim, Lee Jones, devicetree, Tony Lindgren, linux-kernel,
	bcousson, linux-omap, Robert Nelson

On Wednesday, November 23, 2016 8:38:00 PM CET Milo Kim wrote:
> On 11/23/2016 12:57 AM, Lee Jones wrote:
> >>      pwrbutton {
> >> > -          interrupts = <TPS65217_IRQ_PB>;
> >> > +          interrupts = <TPS65217_IRQ_PUSHBUTTON>;
> > Push button or power button?
> >
> 
> According to the datasheet, push button interrupt is correct.
> 
>         http://www.ti.com/lit/ds/symlink/tps65217.pdf
> 
> This is used for a power button input in Beaglebone boards. In other 
> words, the power button is one of push button usages.
> 
> So, I'd like to keep general name for the interrupt.

Ah, the numbers come from the data sheet. Please just remove the
header then, there is no need to keep them as an ABI, in particular
when the driver doesn't even include that header today.

I see you even have names for them in the node:

        charger {
-               interrupts = <TPS65217_IRQ_AC>, <TPS65217_IRQ_USB>;
+               interrupts = <TPS65217_IRQ_AC_POWER>, <TPS65217_IRQ_USB_POWER>;
                interrupts-names = "AC", "USB";
                status = "okay";

What matters here is the binding documentation in
Documentation/devicetree/bindings/power/supply/tps65217_charger.txt

Please fix that instead to mandate that the two interrupts are
required, what their functions are, and what the interrupt-names
(not interrupts-names) property must list.

	Arnd

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] dt-bindings: mfd: Improve readability for TPS65217 interrupt sources
@ 2016-11-23 11:51       ` Arnd Bergmann
  0 siblings, 0 replies; 19+ messages in thread
From: Arnd Bergmann @ 2016-11-23 11:51 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: devicetree, Tony Lindgren, Robert Nelson, linux-kernel, bcousson,
	Milo Kim, linux-omap, Lee Jones

On Wednesday, November 23, 2016 8:38:00 PM CET Milo Kim wrote:
> On 11/23/2016 12:57 AM, Lee Jones wrote:
> >>      pwrbutton {
> >> > -          interrupts = <TPS65217_IRQ_PB>;
> >> > +          interrupts = <TPS65217_IRQ_PUSHBUTTON>;
> > Push button or power button?
> >
> 
> According to the datasheet, push button interrupt is correct.
> 
>         http://www.ti.com/lit/ds/symlink/tps65217.pdf
> 
> This is used for a power button input in Beaglebone boards. In other 
> words, the power button is one of push button usages.
> 
> So, I'd like to keep general name for the interrupt.

Ah, the numbers come from the data sheet. Please just remove the
header then, there is no need to keep them as an ABI, in particular
when the driver doesn't even include that header today.

I see you even have names for them in the node:

        charger {
-               interrupts = <TPS65217_IRQ_AC>, <TPS65217_IRQ_USB>;
+               interrupts = <TPS65217_IRQ_AC_POWER>, <TPS65217_IRQ_USB_POWER>;
                interrupts-names = "AC", "USB";
                status = "okay";

What matters here is the binding documentation in
Documentation/devicetree/bindings/power/supply/tps65217_charger.txt

Please fix that instead to mandate that the two interrupts are
required, what their functions are, and what the interrupt-names
(not interrupts-names) property must list.

	Arnd

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH] dt-bindings: mfd: Improve readability for TPS65217 interrupt sources
@ 2016-11-23 11:51       ` Arnd Bergmann
  0 siblings, 0 replies; 19+ messages in thread
From: Arnd Bergmann @ 2016-11-23 11:51 UTC (permalink / raw)
  To: linux-arm-kernel

On Wednesday, November 23, 2016 8:38:00 PM CET Milo Kim wrote:
> On 11/23/2016 12:57 AM, Lee Jones wrote:
> >>      pwrbutton {
> >> > -          interrupts = <TPS65217_IRQ_PB>;
> >> > +          interrupts = <TPS65217_IRQ_PUSHBUTTON>;
> > Push button or power button?
> >
> 
> According to the datasheet, push button interrupt is correct.
> 
>         http://www.ti.com/lit/ds/symlink/tps65217.pdf
> 
> This is used for a power button input in Beaglebone boards. In other 
> words, the power button is one of push button usages.
> 
> So, I'd like to keep general name for the interrupt.

Ah, the numbers come from the data sheet. Please just remove the
header then, there is no need to keep them as an ABI, in particular
when the driver doesn't even include that header today.

I see you even have names for them in the node:

        charger {
-               interrupts = <TPS65217_IRQ_AC>, <TPS65217_IRQ_USB>;
+               interrupts = <TPS65217_IRQ_AC_POWER>, <TPS65217_IRQ_USB_POWER>;
                interrupts-names = "AC", "USB";
                status = "okay";

What matters here is the binding documentation in
Documentation/devicetree/bindings/power/supply/tps65217_charger.txt

Please fix that instead to mandate that the two interrupts are
required, what their functions are, and what the interrupt-names
(not interrupts-names) property must list.

	Arnd

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] dt-bindings: mfd: Improve readability for TPS65217 interrupt sources
  2016-11-23 11:51       ` Arnd Bergmann
  (?)
@ 2016-11-23 13:06         ` Milo Kim
  -1 siblings, 0 replies; 19+ messages in thread
From: Milo Kim @ 2016-11-23 13:06 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: linux-arm-kernel, Lee Jones, devicetree, Tony Lindgren,
	linux-kernel, bcousson, linux-omap, Robert Nelson

On 11/23/2016 08:51 PM, Arnd Bergmann wrote:
> Ah, the numbers come from the data sheet. Please just remove the
> header then, there is no need to keep them as an ABI, in particular
> when the driver doesn't even include that header today.

Got it.

> What matters here is the binding documentation in
> Documentation/devicetree/bindings/power/supply/tps65217_charger.txt

OK, in addition to this, the interrupt specifier needs to be added in 
Documentation/devicetree/bindings/input/tps65218-pwrbutton.txt.

> Please fix that instead to mandate that the two interrupts are
> required, what their functions are, and what the interrupt-names
> (not interrupts-names) property must list.

Oops, wrong interrupt name property - my bad.

Thanks for all your feedback.

Best regards,
Milo

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] dt-bindings: mfd: Improve readability for TPS65217 interrupt sources
@ 2016-11-23 13:06         ` Milo Kim
  0 siblings, 0 replies; 19+ messages in thread
From: Milo Kim @ 2016-11-23 13:06 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Lee Jones,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Tony Lindgren,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	bcousson-rdvid1DuHRBWk0Htik3J/w,
	linux-omap-u79uwXL29TY76Z2rM5mHXA, Robert Nelson

On 11/23/2016 08:51 PM, Arnd Bergmann wrote:
> Ah, the numbers come from the data sheet. Please just remove the
> header then, there is no need to keep them as an ABI, in particular
> when the driver doesn't even include that header today.

Got it.

> What matters here is the binding documentation in
> Documentation/devicetree/bindings/power/supply/tps65217_charger.txt

OK, in addition to this, the interrupt specifier needs to be added in 
Documentation/devicetree/bindings/input/tps65218-pwrbutton.txt.

> Please fix that instead to mandate that the two interrupts are
> required, what their functions are, and what the interrupt-names
> (not interrupts-names) property must list.

Oops, wrong interrupt name property - my bad.

Thanks for all your feedback.

Best regards,
Milo
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH] dt-bindings: mfd: Improve readability for TPS65217 interrupt sources
@ 2016-11-23 13:06         ` Milo Kim
  0 siblings, 0 replies; 19+ messages in thread
From: Milo Kim @ 2016-11-23 13:06 UTC (permalink / raw)
  To: linux-arm-kernel

On 11/23/2016 08:51 PM, Arnd Bergmann wrote:
> Ah, the numbers come from the data sheet. Please just remove the
> header then, there is no need to keep them as an ABI, in particular
> when the driver doesn't even include that header today.

Got it.

> What matters here is the binding documentation in
> Documentation/devicetree/bindings/power/supply/tps65217_charger.txt

OK, in addition to this, the interrupt specifier needs to be added in 
Documentation/devicetree/bindings/input/tps65218-pwrbutton.txt.

> Please fix that instead to mandate that the two interrupts are
> required, what their functions are, and what the interrupt-names
> (not interrupts-names) property must list.

Oops, wrong interrupt name property - my bad.

Thanks for all your feedback.

Best regards,
Milo

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2016-11-23 13:06 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-21 13:15 [PATCH] dt-bindings: mfd: Improve readability for TPS65217 interrupt sources Milo Kim
2016-11-21 13:15 ` Milo Kim
2016-11-22 15:57 ` Lee Jones
2016-11-22 15:57   ` Lee Jones
2016-11-22 16:00   ` Lee Jones
2016-11-22 16:00     ` Lee Jones
2016-11-22 16:00     ` Lee Jones
2016-11-22 21:08     ` Arnd Bergmann
2016-11-22 21:08       ` Arnd Bergmann
2016-11-22 21:08       ` Arnd Bergmann
2016-11-23 11:38   ` Milo Kim
2016-11-23 11:38     ` Milo Kim
2016-11-23 11:38     ` Milo Kim
2016-11-23 11:51     ` Arnd Bergmann
2016-11-23 11:51       ` Arnd Bergmann
2016-11-23 11:51       ` Arnd Bergmann
2016-11-23 13:06       ` Milo Kim
2016-11-23 13:06         ` Milo Kim
2016-11-23 13:06         ` Milo Kim

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.