| Message ID | 1382550852-11508-2-git-send-email-sre@debian.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hello Sebastian, On 10/23/2013 07:54 PM, Sebastian Reichel wrote: > Add device tree support for twl4030 power button driver. > > Signed-off-by: Sebastian Reichel <sre@debian.org> > --- > .../devicetree/bindings/input/twl4030-pwrbutton.txt | 13 +++++++++++++ > drivers/input/misc/twl4030-pwrbutton.c | 16 ++++++++++++---- > 2 files changed, 25 insertions(+), 4 deletions(-) > create mode 100644 Documentation/devicetree/bindings/input/twl4030-pwrbutton.txt > > diff --git a/Documentation/devicetree/bindings/input/twl4030-pwrbutton.txt b/Documentation/devicetree/bindings/input/twl4030-pwrbutton.txt > new file mode 100644 > index 0000000..945ec74 > --- /dev/null > +++ b/Documentation/devicetree/bindings/input/twl4030-pwrbutton.txt > @@ -0,0 +1,13 @@ > +* TWL4030's pwrbutton device tree bindings > + > +Required SoC Specific Properties: > +- compatible: should be one of the following > + - "ti,twl4030-pwrbutton": For controllers compatible with twl4030 > +- interrupt: should be one of the following > + - <8>: For controllers compatible with twl4030 This is <8> for your particular case, but it will depend on your SoC, won't it? Moreover, this property will be most likely inherited from the root twl node, so I do not see the need to document it here. See: Documentation/devicetree/bindings/mfd/twl-familly.txt > + > +Example: > + twl_pwrbutton: pwrbutton { > + compatible = "ti,twl4030-pwrbutton"; > + interrupts = <8>; > + }; You are missing the root twl node here, no? > diff --git a/drivers/input/misc/twl4030-pwrbutton.c b/drivers/input/misc/twl4030-pwrbutton.c > index b9a05fd..a3a0fe3 100644 > --- a/drivers/input/misc/twl4030-pwrbutton.c > +++ b/drivers/input/misc/twl4030-pwrbutton.c Missing #include <linux/of.h> ? > @@ -52,7 +52,7 @@ static irqreturn_t powerbutton_irq(int irq, void *_pwr) > return IRQ_HANDLED; > } > > -static int __init twl4030_pwrbutton_probe(struct platform_device *pdev) > +static int twl4030_pwrbutton_probe(struct platform_device *pdev) > { > struct input_dev *pwr; > int irq = platform_get_irq(pdev, 0); > @@ -106,16 +106,24 @@ static int __exit twl4030_pwrbutton_remove(struct platform_device *pdev) > return 0; > } > > +#if IS_ENABLED(CONFIG_OF) > +static const struct of_device_id twl4030_pwrbutton_dt_match_table[] = { > + { .compatible = "ti,twl4030-pwrbutton" }, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, twl4030_pwrbutton_dt_match_table); > +#endif > + > static struct platform_driver twl4030_pwrbutton_driver = { > + .probe = twl4030_pwrbutton_probe, > .remove = __exit_p(twl4030_pwrbutton_remove), > .driver = { > .name = "twl4030_pwrbutton", > .owner = THIS_MODULE, > + .of_match_table = of_match_ptr(twl4030_pwrbutton_dt_match_table), > }, > }; > - > -module_platform_driver_probe(twl4030_pwrbutton_driver, > - twl4030_pwrbutton_probe); > +module_platform_driver(twl4030_pwrbutton_driver); > > MODULE_ALIAS("platform:twl4030_pwrbutton"); > MODULE_DESCRIPTION("Triton2 Power Button"); > Best regards, Florian -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Florian, On Thu, Oct 24, 2013 at 09:47:33AM +0200, Florian Vaussard wrote: > > +Required SoC Specific Properties: > > +- compatible: should be one of the following > > + - "ti,twl4030-pwrbutton": For controllers compatible with twl4030 > > +- interrupt: should be one of the following > > + - <8>: For controllers compatible with twl4030 > > This is <8> for your particular case, but it will depend on your > SoC, won't it? Moreover, this property will be most likely > inherited from the root twl node, so I do not see the need to > document it here. See: > > Documentation/devicetree/bindings/mfd/twl-familly.txt No. This is an internal twl4030 interrupt. TWL4030 functions itself as an interrupt controller. > > + > > +Example: > > + twl_pwrbutton: pwrbutton { > > + compatible = "ti,twl4030-pwrbutton"; > > + interrupts = <8>; > > + }; > > You are missing the root twl node here, no? So should I document it like this? twl4030 { compatible = "ti,twl4030"; pwrbutton { compatible = "ti,twl4030-pwrbutton"; interrupts = <8>; }; }; > > diff --git a/drivers/input/misc/twl4030-pwrbutton.c b/drivers/input/misc/twl4030-pwrbutton.c > > index b9a05fd..a3a0fe3 100644 > > --- a/drivers/input/misc/twl4030-pwrbutton.c > > +++ b/drivers/input/misc/twl4030-pwrbutton.c > > Missing #include <linux/of.h> ? It's included indirectly, but I should probably add a direct include. Thanks. > Best regards, Thanks for the comments. I will sent out an updated patchset later. -- Sebastian
Hello, On 10/24/2013 10:38 AM, Sebastian Reichel wrote: > Hi Florian, > > On Thu, Oct 24, 2013 at 09:47:33AM +0200, Florian Vaussard wrote: >>> +Required SoC Specific Properties: >>> +- compatible: should be one of the following >>> + - "ti,twl4030-pwrbutton": For controllers compatible with twl4030 >>> +- interrupt: should be one of the following >>> + - <8>: For controllers compatible with twl4030 >> >> This is <8> for your particular case, but it will depend on your >> SoC, won't it? Moreover, this property will be most likely >> inherited from the root twl node, so I do not see the need to >> document it here. See: >> >> Documentation/devicetree/bindings/mfd/twl-familly.txt > > No. This is an internal twl4030 interrupt. TWL4030 functions > itself as an interrupt controller. > So if it does not belong to the TWL parent, where is it used in your code? You should be parsing this property, so you can set up the IRQ properly. I am a bit confused here. If it is fixed, no need for a OF property. >>> + >>> +Example: >>> + twl_pwrbutton: pwrbutton { >>> + compatible = "ti,twl4030-pwrbutton"; >>> + interrupts = <8>; >>> + }; >> >> You are missing the root twl node here, no? > > So should I document it like this? > IMHO it is more clear for the user. > twl4030 { > compatible = "ti,twl4030"; > > pwrbutton { > compatible = "ti,twl4030-pwrbutton"; > interrupts = <8>; > }; > }; Nit, but existing documentations follow the "name@address" form for the root node, as the TWL is on an I2C bus. Either it is already defined, thus you should use "&twl4030" to reference it, or you create the TWL node and something like "twl4030@48" should be used. For an example, you can refer to existing bindings, like Documentation/devicetree/bindings/mfd/twl4030-audio.txt. Best regards, Florian -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/input/twl4030-pwrbutton.txt b/Documentation/devicetree/bindings/input/twl4030-pwrbutton.txt new file mode 100644 index 0000000..945ec74 --- /dev/null +++ b/Documentation/devicetree/bindings/input/twl4030-pwrbutton.txt @@ -0,0 +1,13 @@ +* TWL4030's pwrbutton device tree bindings + +Required SoC Specific Properties: +- compatible: should be one of the following + - "ti,twl4030-pwrbutton": For controllers compatible with twl4030 +- interrupt: should be one of the following + - <8>: For controllers compatible with twl4030 + +Example: + twl_pwrbutton: pwrbutton { + compatible = "ti,twl4030-pwrbutton"; + interrupts = <8>; + }; diff --git a/drivers/input/misc/twl4030-pwrbutton.c b/drivers/input/misc/twl4030-pwrbutton.c index b9a05fd..a3a0fe3 100644 --- a/drivers/input/misc/twl4030-pwrbutton.c +++ b/drivers/input/misc/twl4030-pwrbutton.c @@ -52,7 +52,7 @@ static irqreturn_t powerbutton_irq(int irq, void *_pwr) return IRQ_HANDLED; } -static int __init twl4030_pwrbutton_probe(struct platform_device *pdev) +static int twl4030_pwrbutton_probe(struct platform_device *pdev) { struct input_dev *pwr; int irq = platform_get_irq(pdev, 0); @@ -106,16 +106,24 @@ static int __exit twl4030_pwrbutton_remove(struct platform_device *pdev) return 0; } +#if IS_ENABLED(CONFIG_OF) +static const struct of_device_id twl4030_pwrbutton_dt_match_table[] = { + { .compatible = "ti,twl4030-pwrbutton" }, + {}, +}; +MODULE_DEVICE_TABLE(of, twl4030_pwrbutton_dt_match_table); +#endif + static struct platform_driver twl4030_pwrbutton_driver = { + .probe = twl4030_pwrbutton_probe, .remove = __exit_p(twl4030_pwrbutton_remove), .driver = { .name = "twl4030_pwrbutton", .owner = THIS_MODULE, + .of_match_table = of_match_ptr(twl4030_pwrbutton_dt_match_table), }, }; - -module_platform_driver_probe(twl4030_pwrbutton_driver, - twl4030_pwrbutton_probe); +module_platform_driver(twl4030_pwrbutton_driver); MODULE_ALIAS("platform:twl4030_pwrbutton"); MODULE_DESCRIPTION("Triton2 Power Button");
Add device tree support for twl4030 power button driver. Signed-off-by: Sebastian Reichel <sre@debian.org> --- .../devicetree/bindings/input/twl4030-pwrbutton.txt | 13 +++++++++++++ drivers/input/misc/twl4030-pwrbutton.c | 16 ++++++++++++---- 2 files changed, 25 insertions(+), 4 deletions(-) create mode 100644 Documentation/devicetree/bindings/input/twl4030-pwrbutton.txt