[PATCH 4/4] ARM: dts: twl4030: Add power button support

Sebastian Reichel

unread,

Oct 22, 2013, 9:00:02 AM10/22/13

to

Enable support for the power button.

Signed-off-by: Sebastian Reichel <s...@debian.org>
---
arch/arm/boot/dts/twl4030.dtsi | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/twl4030.dtsi b/arch/arm/boot/dts/twl4030.dtsi
index 773c94c..19e3cd0 100644
--- a/arch/arm/boot/dts/twl4030.dtsi
+++ b/arch/arm/boot/dts/twl4030.dtsi
@@ -104,4 +104,9 @@
keypad,num-rows = <8>;
keypad,num-columns = <8>;
};
+
+ twl_pwrbutton: pwrbutton {
+ compatible = "ti,twl4030-pwrbutton";
+ interrupts = <8>;
+ };
};
--
1.8.4.rc3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Sebastian Reichel

unread,

Oct 22, 2013, 9:00:02 AM10/22/13

to

Use managed irq resource to simplify the driver.

Signed-off-by: Sebastian Reichel <s...@debian.org>
---

drivers/input/misc/twl4030-pwrbutton.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/input/misc/twl4030-pwrbutton.c b/drivers/input/misc/twl4030-pwrbutton.c
index 4e7a810..4ab9243 100644
--- a/drivers/input/misc/twl4030-pwrbutton.c
+++ b/drivers/input/misc/twl4030-pwrbutton.c
@@ -70,7 +70,7 @@ static int twl4030_pwrbutton_probe(struct platform_device *pdev)
pwr->phys = "twl4030_pwrbutton/input0";
pwr->dev.parent = &pdev->dev;

- err = request_threaded_irq(irq, NULL, powerbutton_irq,
+ err = devm_request_threaded_irq(&pwr->dev, irq, NULL, powerbutton_irq,
IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING,
"twl4030_pwrbutton", pwr);
if (err < 0) {
@@ -81,15 +81,13 @@ static int twl4030_pwrbutton_probe(struct platform_device *pdev)
err = input_register_device(pwr);
if (err) {
dev_err(&pdev->dev, "Can't register power button: %d\n", err);
- goto free_irq;
+ goto free_input_dev;
}

platform_set_drvdata(pdev, pwr);

return 0;

-free_irq:
- free_irq(irq, pwr);
free_input_dev:
input_free_device(pwr);
return err;
@@ -98,9 +96,7 @@ free_input_dev:
static int __exit twl4030_pwrbutton_remove(struct platform_device *pdev)
{
struct input_dev *pwr = platform_get_drvdata(pdev);
- int irq = platform_get_irq(pdev, 0);

- free_irq(irq, pwr);
input_unregister_device(pwr);

return 0;

Sebastian Reichel

unread,

Oct 22, 2013, 9:00:02 AM10/22/13

to

This patchset adds support for the Nokia N900
powerbutton.

Currently non-DT boot crashes for me very early,
so I was not able to check this patch using non-DT
boot.

It would be nice if somebody can check if the first
patch works using non-DT boot. It should be enough
to test it with any omap3 board using the twl4030
power button.

Sebastian Reichel (4):
Input: twl4030-pwrbutton - add device tree support
Input: twl4030-pwrbutton: use dev_err for errors
Input: twl4030-pwrbutton: simplify driver using devm_*
ARM: dts: twl4030: Add power button support

arch/arm/boot/dts/twl4030.dtsi | 5 +++++
drivers/input/misc/twl4030-pwrbutton.c | 33 ++++++++++++++++++++-------------
2 files changed, 25 insertions(+), 13 deletions(-)

Sebastian Reichel

unread,

Oct 22, 2013, 9:00:03 AM10/22/13

to

Use dev_err() to output errors instead of dev_dbg().

Signed-off-by: Sebastian Reichel <s...@debian.org>
---

drivers/input/misc/twl4030-pwrbutton.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/input/misc/twl4030-pwrbutton.c b/drivers/input/misc/twl4030-pwrbutton.c
index e450947..4e7a810 100644
--- a/drivers/input/misc/twl4030-pwrbutton.c
+++ b/drivers/input/misc/twl4030-pwrbutton.c
@@ -60,7 +60,7 @@ static int twl4030_pwrbutton_probe(struct platform_device *pdev)

pwr = input_allocate_device();
if (!pwr) {
- dev_dbg(&pdev->dev, "Can't allocate power button\n");
+ dev_err(&pdev->dev, "Can't allocate power button\n");
return -ENOMEM;
}

@@ -74,13 +74,13 @@ static int twl4030_pwrbutton_probe(struct platform_device *pdev)

IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING,
"twl4030_pwrbutton", pwr);
if (err < 0) {

- dev_dbg(&pdev->dev, "Can't get IRQ for pwrbutton: %d\n", err);
+ dev_err(&pdev->dev, "Can't get IRQ for pwrbutton: %d\n", err);
goto free_input_dev;

}

err = input_register_device(pwr);
if (err) {

- dev_dbg(&pdev->dev, "Can't register power button: %d\n", err);
+ dev_err(&pdev->dev, "Can't register power button: %d\n", err);
goto free_irq;

Sebastian Reichel

unread,

Oct 22, 2013, 9:00:03 AM10/22/13

to

Add device tree support for twl4030 power button driver.

Signed-off-by: Sebastian Reichel <s...@debian.org>
---

drivers/input/misc/twl4030-pwrbutton.c | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/input/misc/twl4030-pwrbutton.c b/drivers/input/misc/twl4030-pwrbutton.c
index b9a05fd..e450947 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,27 @@ 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);
+#define twl4030_pwrbutton_dt_match of_match_ptr(twl4030_pwrbutton_dt_match_table)
+#else
+#define twl4030_pwrbutton_dt_match NULL
+#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 = twl4030_pwrbutton_dt_match,
},
};
-
-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");

Felipe Balbi

unread,

Oct 22, 2013, 10:00:02 AM10/22/13

to

Hi,

you don't ned this trickery.. just always device the match table.

--
balbi

signature.asc

Sebastian Reichel

unread,

Oct 23, 2013, 11:10:01 AM10/23/13

to

Add device tree support for twl4030 power button driver.

Signed-off-by: Sebastian Reichel <s...@debian.org>
---

drivers/input/misc/twl4030-pwrbutton.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)

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),

Sebastian Reichel

unread,

Oct 23, 2013, 11:10:01 AM10/23/13

to

Use dev_err() to output errors instead of dev_dbg().

Signed-off-by: Sebastian Reichel <s...@debian.org>
---

drivers/input/misc/twl4030-pwrbutton.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/input/misc/twl4030-pwrbutton.c b/drivers/input/misc/twl4030-pwrbutton.c
index a3a0fe3..3efbb13 100644
--- a/drivers/input/misc/twl4030-pwrbutton.c
+++ b/drivers/input/misc/twl4030-pwrbutton.c

@@ -60,7 +60,7 @@ static int twl4030_pwrbutton_probe(struct platform_device *pdev)

pwr = input_allocate_device();
if (!pwr) {
- dev_dbg(&pdev->dev, "Can't allocate power button\n");
+ dev_err(&pdev->dev, "Can't allocate power button\n");
return -ENOMEM;
}

@@ -74,13 +74,13 @@ static int twl4030_pwrbutton_probe(struct platform_device *pdev)
IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING,
"twl4030_pwrbutton", pwr);
if (err < 0) {
- dev_dbg(&pdev->dev, "Can't get IRQ for pwrbutton: %d\n", err);
+ dev_err(&pdev->dev, "Can't get IRQ for pwrbutton: %d\n", err);
goto free_input_dev;
}

err = input_register_device(pwr);
if (err) {
- dev_dbg(&pdev->dev, "Can't register power button: %d\n", err);
+ dev_err(&pdev->dev, "Can't register power button: %d\n", err);
goto free_irq;
}

Sebastian Reichel

unread,

Oct 23, 2013, 11:10:02 AM10/23/13

to

Hi,

This is the second iteration of DT support for the TWL4030
power button.

Changes since v1 [0]:
* Simplify OF handling of twl4030_pwrbutton_dt_match_table
* Remove Patch 4 (queued by Benoit)

[0] https://lkml.org/lkml/2013/10/22/151

-- Sebastian

Sebastian Reichel (3):

Input: twl4030-pwrbutton - add device tree support
Input: twl4030-pwrbutton: use dev_err for errors
Input: twl4030-pwrbutton: simplify driver using devm_*

drivers/input/misc/twl4030-pwrbutton.c | 30 +++++++++++++++++-------------
1 file changed, 17 insertions(+), 13 deletions(-)

Sebastian Reichel

unread,

Oct 23, 2013, 11:10:02 AM10/23/13

to

Use managed irq resource to simplify the driver.

Signed-off-by: Sebastian Reichel <s...@debian.org>
---

drivers/input/misc/twl4030-pwrbutton.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/input/misc/twl4030-pwrbutton.c b/drivers/input/misc/twl4030-pwrbutton.c
index 3efbb13..49ca7b1 100644
--- a/drivers/input/misc/twl4030-pwrbutton.c
+++ b/drivers/input/misc/twl4030-pwrbutton.c

@@ -70,7 +70,7 @@ static int twl4030_pwrbutton_probe(struct platform_device *pdev)
pwr->phys = "twl4030_pwrbutton/input0";
pwr->dev.parent = &pdev->dev;

- err = request_threaded_irq(irq, NULL, powerbutton_irq,
+ err = devm_request_threaded_irq(&pwr->dev, irq, NULL, powerbutton_irq,

IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING,
"twl4030_pwrbutton", pwr);
if (err < 0) {

@@ -81,15 +81,13 @@ static int twl4030_pwrbutton_probe(struct platform_device *pdev)

err = input_register_device(pwr);
if (err) {

dev_err(&pdev->dev, "Can't register power button: %d\n", err);

- goto free_irq;
+ goto free_input_dev;
}

platform_set_drvdata(pdev, pwr);

return 0;

-free_irq:
- free_irq(irq, pwr);
free_input_dev:
input_free_device(pwr);
return err;
@@ -98,9 +96,7 @@ free_input_dev:

static int __exit twl4030_pwrbutton_remove(struct platform_device *pdev)

{
struct input_dev *pwr = platform_get_drvdata(pdev);
- int irq = platform_get_irq(pdev, 0);

- free_irq(irq, pwr);
input_unregister_device(pwr);

return 0;

Sebastian Reichel

unread,

Oct 23, 2013, 12:20:02 PM10/23/13

to

Hi,

On Wed, Oct 23, 2013 at 05:09:36PM +0100, Mark Rutland wrote:

> On Wed, Oct 23, 2013 at 04:01:20PM +0100, Sebastian Reichel wrote:
> > Add device tree support for twl4030 power button driver.
>

> This requires a binding document. As it is it's not possible to review.

Right. I will add it and sent a v3.

> >
> > Signed-off-by: Sebastian Reichel <s...@debian.org>
> > ---
> > drivers/input/misc/twl4030-pwrbutton.c | 16 ++++++++++++----
> > 1 file changed, 12 insertions(+), 4 deletions(-)
> >
> > 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" },
>

> There's no need to shorten this, "ti,twl4030-power-button" would be far easier
> to understand. Unless the datasheet refers to it as pwrbutton?

Yes it's abbreviated in the datasheet.

-- Sebastian

signature.asc

Mark Rutland

unread,

Oct 23, 2013, 12:20:02 PM10/23/13

to

On Wed, Oct 23, 2013 at 04:01:20PM +0100, Sebastian Reichel wrote:

> Add device tree support for twl4030 power button driver.

This requires a binding document. As it is it's not possible to review.

Mark.

>
> Signed-off-by: Sebastian Reichel <s...@debian.org>
> ---
> drivers/input/misc/twl4030-pwrbutton.c | 16 ++++++++++++----
> 1 file changed, 12 insertions(+), 4 deletions(-)
>
> 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" },

There's no need to shorten this, "ti,twl4030-power-button" would be far easier
to understand. Unless the datasheet refers to it as pwrbutton?

Thanks,
Mark.

Aaro Koskinen

unread,

Oct 23, 2013, 12:40:02 PM10/23/13

to

Hi,

On Wed, Oct 23, 2013 at 05:01:22PM +0200, Sebastian Reichel wrote:
> static int __exit twl4030_pwrbutton_remove(struct platform_device *pdev)
> {
> struct input_dev *pwr = platform_get_drvdata(pdev);
> - int irq = platform_get_irq(pdev, 0);
>
> - free_irq(irq, pwr);
> input_unregister_device(pwr);

You need convert the driver to use devm_input_allocate_device()
first. Otherwise driver will crash the kernel here if you get interrupt
after unregistering the device.

A.

Sebastian Reichel

unread,

Oct 23, 2013, 12:50:02 PM10/23/13

to

Hi,

This is the third iteration of DT support for the TWL4030
power button.

Changes since v2 [0]:
* add devicetree binding documentation
* use devm_input_allocate_device

[0] https://lkml.org/lkml/2013/10/23/323

-- Sebastian

Sebastian Reichel (3):
Input: twl4030-pwrbutton - add device tree support
Input: twl4030-pwrbutton: use dev_err for errors
Input: twl4030-pwrbutton: simplify driver using devm_*

.../bindings/input/twl4030-pwrbutton.txt | 13 ++++++++
drivers/input/misc/twl4030-pwrbutton.c | 38 +++++++++++-----------
2 files changed, 32 insertions(+), 19 deletions(-)
create mode 100644 Documentation/devicetree/bindings/input/twl4030-pwrbutton.txt

--
1.8.4.rc3

Sebastian Reichel

unread,

Oct 23, 2013, 12:50:02 PM10/23/13

to

Use managed irq resource to simplify the driver.

Signed-off-by: Sebastian Reichel <s...@debian.org>
---

drivers/input/misc/twl4030-pwrbutton.c | 16 ++++------------
1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/input/misc/twl4030-pwrbutton.c b/drivers/input/misc/twl4030-pwrbutton.c
index 3efbb13..91bb497 100644
--- a/drivers/input/misc/twl4030-pwrbutton.c
+++ b/drivers/input/misc/twl4030-pwrbutton.c
@@ -58,7 +58,7 @@ static int twl4030_pwrbutton_probe(struct platform_device *pdev)

int irq = platform_get_irq(pdev, 0);

int err;

- pwr = input_allocate_device();
+ pwr = devm_input_allocate_device(&pdev->dev);
if (!pwr) {

dev_err(&pdev->dev, "Can't allocate power button\n");
return -ENOMEM;

@@ -70,37 +70,29 @@ static int twl4030_pwrbutton_probe(struct platform_device *pdev)

pwr->phys = "twl4030_pwrbutton/input0";
pwr->dev.parent = &pdev->dev;

- err = request_threaded_irq(irq, NULL, powerbutton_irq,
+ err = devm_request_threaded_irq(&pwr->dev, irq, NULL, powerbutton_irq,
IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING,
"twl4030_pwrbutton", pwr);
if (err < 0) {

dev_err(&pdev->dev, "Can't get IRQ for pwrbutton: %d\n", err);
- goto free_input_dev;
+ return err;

}

err = input_register_device(pwr);
if (err) {
dev_err(&pdev->dev, "Can't register power button: %d\n", err);
- goto free_irq;

+ return err;
}

platform_set_drvdata(pdev, pwr);

return 0;
-
-free_irq:
- free_irq(irq, pwr);
-free_input_dev:
- input_free_device(pwr);
- return err;

}

static int __exit twl4030_pwrbutton_remove(struct platform_device *pdev)
{
struct input_dev *pwr = platform_get_drvdata(pdev);
- int irq = platform_get_irq(pdev, 0);

- free_irq(irq, pwr);
input_unregister_device(pwr);

return 0;

Sebastian Reichel

unread,

Oct 23, 2013, 12:50:02 PM10/23/13

to

Add device tree support for twl4030 power button driver.

Signed-off-by: Sebastian Reichel <s...@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
+
+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");

Sebastian Reichel

unread,

Oct 23, 2013, 12:50:02 PM10/23/13

to

Use dev_err() to output errors instead of dev_dbg().

Signed-off-by: Sebastian Reichel <s...@debian.org>
---

drivers/input/misc/twl4030-pwrbutton.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/input/misc/twl4030-pwrbutton.c b/drivers/input/misc/twl4030-pwrbutton.c
index a3a0fe3..3efbb13 100644
--- a/drivers/input/misc/twl4030-pwrbutton.c
+++ b/drivers/input/misc/twl4030-pwrbutton.c

@@ -60,7 +60,7 @@ static int twl4030_pwrbutton_probe(struct platform_device *pdev)

pwr = input_allocate_device();
if (!pwr) {

- dev_dbg(&pdev->dev, "Can't allocate power button\n");
+ dev_err(&pdev->dev, "Can't allocate power button\n");
return -ENOMEM;
}

@@ -74,13 +74,13 @@ static int twl4030_pwrbutton_probe(struct platform_device *pdev)

IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING,
"twl4030_pwrbutton", pwr);
if (err < 0) {

- dev_dbg(&pdev->dev, "Can't get IRQ for pwrbutton: %d\n", err);
+ dev_err(&pdev->dev, "Can't get IRQ for pwrbutton: %d\n", err);
goto free_input_dev;

}

err = input_register_device(pwr);
if (err) {

- dev_dbg(&pdev->dev, "Can't register power button: %d\n", err);
+ dev_err(&pdev->dev, "Can't register power button: %d\n", err);
goto free_irq;

Aaro Koskinen

unread,

Oct 23, 2013, 1:30:01 PM10/23/13

to

Hi,

On Wed, Oct 23, 2013 at 06:47:15PM +0200, Sebastian Reichel wrote:
> static int __exit twl4030_pwrbutton_remove(struct platform_device *pdev)
> {
> struct input_dev *pwr = platform_get_drvdata(pdev);
> - int irq = platform_get_irq(pdev, 0);
>
> - free_irq(irq, pwr);
> input_unregister_device(pwr);

The same bug still exists. You don't need to unregister manually any more,
this whole function can be just reduced to "return 0;".

A.

Felipe Balbi

unread,

Oct 23, 2013, 1:50:01 PM10/23/13

to

On Wed, Oct 23, 2013 at 08:25:24PM +0300, Aaro Koskinen wrote:
> Hi,
>
> On Wed, Oct 23, 2013 at 06:47:15PM +0200, Sebastian Reichel wrote:
> > static int __exit twl4030_pwrbutton_remove(struct platform_device *pdev)
> > {
> > struct input_dev *pwr = platform_get_drvdata(pdev);
> > - int irq = platform_get_irq(pdev, 0);
> >
> > - free_irq(irq, pwr);
> > input_unregister_device(pwr);
>
> The same bug still exists. You don't need to unregister manually any more,
> this whole function can be just reduced to "return 0;".

then it can be removed altogether, no ?

--
balbi

signature.asc

Sebastian Reichel

unread,

Oct 23, 2013, 2:00:02 PM10/23/13

to

Sebastian Reichel

unread,

Oct 23, 2013, 2:00:02 PM10/23/13

to

Use managed irq resource to simplify the driver.

Signed-off-by: Sebastian Reichel <s...@debian.org>
---

drivers/input/misc/twl4030-pwrbutton.c | 26 ++++----------------------
1 file changed, 4 insertions(+), 22 deletions(-)

diff --git a/drivers/input/misc/twl4030-pwrbutton.c b/drivers/input/misc/twl4030-pwrbutton.c
index 3efbb13..da30af0 100644
--- a/drivers/input/misc/twl4030-pwrbutton.c
+++ b/drivers/input/misc/twl4030-pwrbutton.c
@@ -58,7 +58,7 @@ static int twl4030_pwrbutton_probe(struct platform_device *pdev)

int irq = platform_get_irq(pdev, 0);

int err;

- pwr = input_allocate_device();
+ pwr = devm_input_allocate_device(&pdev->dev);
if (!pwr) {

dev_err(&pdev->dev, "Can't allocate power button\n");
return -ENOMEM;

@@ -70,40 +70,23 @@ static int twl4030_pwrbutton_probe(struct platform_device *pdev)

pwr->phys = "twl4030_pwrbutton/input0";
pwr->dev.parent = &pdev->dev;

- err = request_threaded_irq(irq, NULL, powerbutton_irq,
+ err = devm_request_threaded_irq(&pwr->dev, irq, NULL, powerbutton_irq,

IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING,
"twl4030_pwrbutton", pwr);
if (err < 0) {

dev_err(&pdev->dev, "Can't get IRQ for pwrbutton: %d\n", err);

- goto free_input_dev;
+ return err;
}

err = input_register_device(pwr);
if (err) {

dev_err(&pdev->dev, "Can't register power button: %d\n", err);

- goto free_irq;
+ return err;
}

platform_set_drvdata(pdev, pwr);

return 0;
-
-free_irq:
- free_irq(irq, pwr);
-free_input_dev:
- input_free_device(pwr);
- return err;

-}
-
-static int __exit twl4030_pwrbutton_remove(struct platform_device *pdev)
-{
- struct input_dev *pwr = platform_get_drvdata(pdev);

- int irq = platform_get_irq(pdev, 0);
-

- free_irq(irq, pwr);
- input_unregister_device(pwr);
-
- return 0;
}

#if IS_ENABLED(CONFIG_OF)
@@ -116,7 +99,6 @@ MODULE_DEVICE_TABLE(of, twl4030_pwrbutton_dt_match_table);

static struct platform_driver twl4030_pwrbutton_driver = {

.probe = twl4030_pwrbutton_probe,
- .remove = __exit_p(twl4030_pwrbutton_remove),

.driver = {
.name = "twl4030_pwrbutton",
.owner = THIS_MODULE,

Sebastian Reichel

unread,

Oct 23, 2013, 2:00:03 PM10/23/13

to

Add device tree support for twl4030 power button driver.

Signed-off-by: Sebastian Reichel <s...@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
+
+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");

Sebastian Reichel

unread,

Oct 23, 2013, 2:00:03 PM10/23/13

to

Hi,

This is the fourth iteration of DT support for the TWL4030
power button.

Changes since v3 [0]:
* remove twl4030_pwrbutton_remove function, which is no
longer needed after devm_ conversion.

[0] https://lkml.org/lkml/2013/10/23/353

-- Sebastian

Sebastian Reichel (3):
Input: twl4030-pwrbutton - add device tree support
Input: twl4030-pwrbutton: use dev_err for errors
Input: twl4030-pwrbutton: simplify driver using devm_*

.../bindings/input/twl4030-pwrbutton.txt | 13 ++++++

drivers/input/misc/twl4030-pwrbutton.c | 46 +++++++++-------------
2 files changed, 31 insertions(+), 28 deletions(-)
create mode 100644 Documentation/devicetree/bindings/input/twl4030-pwrbutton.txt

Joe Perches

unread,

Oct 23, 2013, 2:20:01 PM10/23/13

to

On Wed, 2013-10-23 at 19:54 +0200, Sebastian Reichel wrote:
> Use dev_err() to output errors instead of dev_dbg().

[]
> diff --git a/drivers/input/misc/twl4030-pwrbutton.c b/drivers/input/misc/twl4030-pwrbutton.c

[]
> @@ -60,7 +60,7 @@ static int twl4030_pwrbutton_probe(struct platform_device *pdev)
>
> pwr = input_allocate_device();
> if (!pwr) {
> - dev_dbg(&pdev->dev, "Can't allocate power button\n");
> + dev_err(&pdev->dev, "Can't allocate power button\n");
> return -ENOMEM;
> }

input_allocate_device uses kzalloc and it will emit a
standardized OOM message along with a dump_stack()
so this dev_err/dev_dbg is redundant and not necessary.

Joe Perches

unread,

Oct 23, 2013, 3:40:02 PM10/23/13

to

On Wed, 2013-10-23 at 21:31 +0200, Sebastian Reichel wrote:

> On Wed, Oct 23, 2013 at 11:17:46AM -0700, Joe Perches wrote:
> > On Wed, 2013-10-23 at 19:54 +0200, Sebastian Reichel wrote:
> > > Use dev_err() to output errors instead of dev_dbg().
> > []
> > > diff --git a/drivers/input/misc/twl4030-pwrbutton.c b/drivers/input/misc/twl4030-pwrbutton.c
> > []
> > > @@ -60,7 +60,7 @@ static int twl4030_pwrbutton_probe(struct platform_device *pdev)
> > >
> > > pwr = input_allocate_device();
> > > if (!pwr) {
> > > - dev_dbg(&pdev->dev, "Can't allocate power button\n");
> > > + dev_err(&pdev->dev, "Can't allocate power button\n");
> > > return -ENOMEM;
> > > }
> >
> > input_allocate_device uses kzalloc and it will emit a
> > standardized OOM message along with a dump_stack()
> > so this dev_err/dev_dbg is redundant and not necessary.
>

> I saw you sent a big patchset changing this for all drivers.
> Should I drop this patch?

I think the 2 other bits of this patch are fine.
Maybe resend with this section dropped?

Sebastian Reichel

unread,

Oct 23, 2013, 3:40:02 PM10/23/13

to

On Wed, Oct 23, 2013 at 11:17:46AM -0700, Joe Perches wrote:

> On Wed, 2013-10-23 at 19:54 +0200, Sebastian Reichel wrote:
> > Use dev_err() to output errors instead of dev_dbg().
> []
> > diff --git a/drivers/input/misc/twl4030-pwrbutton.c b/drivers/input/misc/twl4030-pwrbutton.c
> []
> > @@ -60,7 +60,7 @@ static int twl4030_pwrbutton_probe(struct platform_device *pdev)
> >
> > pwr = input_allocate_device();
> > if (!pwr) {
> > - dev_dbg(&pdev->dev, "Can't allocate power button\n");
> > + dev_err(&pdev->dev, "Can't allocate power button\n");
> > return -ENOMEM;
> > }
>
> input_allocate_device uses kzalloc and it will emit a
> standardized OOM message along with a dump_stack()
> so this dev_err/dev_dbg is redundant and not necessary.

I saw you sent a big patchset changing this for all drivers.
Should I drop this patch?

-- Sebastian

signature.asc

Sebastian Reichel

unread,

Oct 23, 2013, 4:30:02 PM10/23/13

to

Use managed irq resource to simplify the driver.

Signed-off-by: Sebastian Reichel <s...@debian.org>
---

drivers/input/misc/twl4030-pwrbutton.c | 26 ++++----------------------
1 file changed, 4 insertions(+), 22 deletions(-)

diff --git a/drivers/input/misc/twl4030-pwrbutton.c b/drivers/input/misc/twl4030-pwrbutton.c
index 48639ff..be1759c 100644
--- a/drivers/input/misc/twl4030-pwrbutton.c
+++ b/drivers/input/misc/twl4030-pwrbutton.c
@@ -58,7 +58,7 @@ static int twl4030_pwrbutton_probe(struct platform_device *pdev)

int irq = platform_get_irq(pdev, 0);

int err;

- pwr = input_allocate_device();
+ pwr = devm_input_allocate_device(&pdev->dev);
if (!pwr) {

dev_dbg(&pdev->dev, "Can't allocate power button\n");

static struct platform_driver twl4030_pwrbutton_driver = {

.probe = twl4030_pwrbutton_probe,
- .remove = __exit_p(twl4030_pwrbutton_remove),

.driver = {
.name = "twl4030_pwrbutton",
.owner = THIS_MODULE,

--
1.8.4.rc3

Sebastian Reichel

unread,

Oct 23, 2013, 4:30:02 PM10/23/13

to

Hi,

This is the fifth iteration of DT support for the TWL4030
power button.

Changes since v4 [0]:
* do not change dev_dbg to dev_err for input_allocate_device,
since it prints its own message. Call will be removed completly
by another patchset [1].

[0] https://lkml.org/lkml/2013/10/23/372
[1] https://lkml.org/lkml/2013/10/23/390

-- Sebastian

Sebastian Reichel (3):
Input: twl4030-pwrbutton - add device tree support
Input: twl4030-pwrbutton: use dev_err for errors
Input: twl4030-pwrbutton: simplify driver using devm_*

.../bindings/input/twl4030-pwrbutton.txt | 13 +++++++
drivers/input/misc/twl4030-pwrbutton.c | 44 +++++++++-------------
2 files changed, 30 insertions(+), 27 deletions(-)
create mode 100644 Documentation/devicetree/bindings/input/twl4030-pwrbutton.txt

Sebastian Reichel

unread,

Oct 23, 2013, 4:30:02 PM10/23/13

to

Use dev_err() to output errors instead of dev_dbg().

Signed-off-by: Sebastian Reichel <s...@debian.org>
---

drivers/input/misc/twl4030-pwrbutton.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/input/misc/twl4030-pwrbutton.c b/drivers/input/misc/twl4030-pwrbutton.c
index a3a0fe3..48639ff 100644
--- a/drivers/input/misc/twl4030-pwrbutton.c
+++ b/drivers/input/misc/twl4030-pwrbutton.c
@@ -74,13 +74,13 @@ static int twl4030_pwrbutton_probe(struct platform_device *pdev)

IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING,
"twl4030_pwrbutton", pwr);
if (err < 0) {

- dev_dbg(&pdev->dev, "Can't get IRQ for pwrbutton: %d\n", err);
+ dev_err(&pdev->dev, "Can't get IRQ for pwrbutton: %d\n", err);
goto free_input_dev;

}

err = input_register_device(pwr);
if (err) {

- dev_dbg(&pdev->dev, "Can't register power button: %d\n", err);
+ dev_err(&pdev->dev, "Can't register power button: %d\n", err);
goto free_irq;

Sebastian Reichel

unread,

Oct 23, 2013, 4:30:02 PM10/23/13

to

Add device tree support for twl4030 power button driver.

Signed-off-by: Sebastian Reichel <s...@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
+
+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");

Rob Herring

unread,

Oct 23, 2013, 6:30:02 PM10/23/13

to

So a twl4030 device is only a power button? DT should describe the h/w
not a node for a sub-function of a device.

Rob

Sebastian Reichel

unread,

Oct 23, 2013, 7:00:02 PM10/23/13

to

On Wed, Oct 23, 2013 at 05:24:14PM -0500, Rob Herring wrote:
> So a twl4030 device is only a power button? DT should describe the h/w
> not a node for a sub-function of a device.

No. TWL4030 is a companion chip for the OMAP3 processor. It provides
miscellaneous functionality, e.g.:

* RTC
* Watchdog
* Regulators
* Keypad Matrix
* USB
* Audio
* Vibrator
* GPIO
* ...

One part of the functionality is the power button. The patch
assumes, that the twl4030-pwrbutton node is used as follows:

twl {
/* ... common stuff ... */

pwrbutton {
compatible = "ti,twl4030-pwrbutton";
interrupts = <8>;
};
};

See also:
* Documentation/devicetree/bindings/mfd/twl-familly.txt
* Documentation/devicetree/bindings/watchdog/twl4030-wdt.txt
* Documentation/devicetree/bindings/sound/omap-twl4030.txt
* Documentation/devicetree/bindings/mfd/twl4030-power.txt
* Documentation/devicetree/bindings/mfd/twl4030-audio.txt
* Documentation/devicetree/bindings/gpio/gpio-twl4030.txt

-- Sebastian

signature.asc

Florian Vaussard

unread,

Oct 24, 2013, 4:00:02 AM10/24/13

to

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 <s...@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> ?

Best regards,

Florian

Sebastian Reichel

unread,

Oct 24, 2013, 4:40:01 AM10/24/13

to

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

signature.asc

Florian Vaussard

unread,

Oct 24, 2013, 5:20:02 AM10/24/13

to

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.

Grant Likely

unread,

Oct 25, 2013, 7:00:02 PM10/25/13

to

Wow, that's crazy! It is all one device so put all the bindings into a
single file.

g,