Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1526664 > unrolled thread
| Started by | Milo Kim <woogyom.kim@gmail.com> |
|---|---|
| First post | 2016-11-21 14:20 +0100 |
| Last post | 2016-11-23 14:10 +0100 |
| Articles | 7 — 3 participants |
Back to article view | Back to linux.kernel
[PATCH] dt-bindings: mfd: Improve readability for TPS65217 interrupt sources Milo Kim <woogyom.kim@gmail.com> - 2016-11-21 14:20 +0100
Re: [PATCH] dt-bindings: mfd: Improve readability for TPS65217 interrupt sources Lee Jones <lee.jones@linaro.org> - 2016-11-22 17:00 +0100
Re: [PATCH] dt-bindings: mfd: Improve readability for TPS65217 interrupt sources Arnd Bergmann <arnd@arndb.de> - 2016-11-22 22:10 +0100
Re: [PATCH] dt-bindings: mfd: Improve readability for TPS65217 interrupt sources Lee Jones <lee.jones@linaro.org> - 2016-11-22 17:00 +0100
Re: [PATCH] dt-bindings: mfd: Improve readability for TPS65217 interrupt sources Milo Kim <woogyom.kim@gmail.com> - 2016-11-23 12:40 +0100
Re: [PATCH] dt-bindings: mfd: Improve readability for TPS65217 interrupt sources Arnd Bergmann <arnd@arndb.de> - 2016-11-23 13:00 +0100
Re: [PATCH] dt-bindings: mfd: Improve readability for TPS65217 interrupt sources Milo Kim <woogyom.kim@gmail.com> - 2016-11-23 14:10 +0100
| From | Milo Kim <woogyom.kim@gmail.com> |
|---|---|
| Date | 2016-11-21 14:20 +0100 |
| Subject | [PATCH] dt-bindings: mfd: Improve readability for TPS65217 interrupt sources |
| Message-ID | <sFWFP-1DY-1@gated-at.bofh.it> |
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
[toc] | [next] | [standalone]
| From | Lee Jones <lee.jones@linaro.org> |
|---|---|
| Date | 2016-11-22 17:00 +0100 |
| Subject | Re: [PATCH] dt-bindings: mfd: Improve readability for TPS65217 interrupt sources |
| Message-ID | <sGlEe-BN-17@gated-at.bofh.it> |
| In reply to | #1526664 |
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
[toc] | [prev] | [next] | [standalone]
| From | Arnd Bergmann <arnd@arndb.de> |
|---|---|
| Date | 2016-11-22 22:10 +0100 |
| Message-ID | <sGque-45L-43@gated-at.bofh.it> |
| In reply to | #1527627 |
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
[toc] | [prev] | [next] | [standalone]
| From | Lee Jones <lee.jones@linaro.org> |
|---|---|
| Date | 2016-11-22 17:00 +0100 |
| Subject | Re: [PATCH] dt-bindings: mfd: Improve readability for TPS65217 interrupt sources |
| Message-ID | <sGlEe-BN-19@gated-at.bofh.it> |
| In reply to | #1526664 |
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
[toc] | [prev] | [next] | [standalone]
| From | Milo Kim <woogyom.kim@gmail.com> |
|---|---|
| Date | 2016-11-23 12:40 +0100 |
| Subject | Re: [PATCH] dt-bindings: mfd: Improve readability for TPS65217 interrupt sources |
| Message-ID | <sGE49-4fv-13@gated-at.bofh.it> |
| In reply to | #1527630 |
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
[toc] | [prev] | [next] | [standalone]
| From | Arnd Bergmann <arnd@arndb.de> |
|---|---|
| Date | 2016-11-23 13:00 +0100 |
| Message-ID | <sGEnv-4ne-13@gated-at.bofh.it> |
| In reply to | #1528337 |
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
[toc] | [prev] | [next] | [standalone]
| From | Milo Kim <woogyom.kim@gmail.com> |
|---|---|
| Date | 2016-11-23 14:10 +0100 |
| Subject | Re: [PATCH] dt-bindings: mfd: Improve readability for TPS65217 interrupt sources |
| Message-ID | <sGFtf-5jU-1@gated-at.bofh.it> |
| In reply to | #1528350 |
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
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web