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
--
1.8.4.rc3
Use dev_err() to output errors instead of dev_dbg().
Signed-off-by: Sebastian Reichel <[email protected]>
---
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;
}
--
1.8.4.rc3
Use managed irq resource to simplify the driver.
Signed-off-by: Sebastian Reichel <[email protected]>
---
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");
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,
--
1.8.4.rc3
Add device tree support for twl4030 power button driver.
Signed-off-by: Sebastian Reichel <[email protected]>
---
.../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");
--
1.8.4.rc3
On 10/23/2013 03:26 PM, Sebastian Reichel wrote:
> Add device tree support for twl4030 power button driver.
>
> Signed-off-by: Sebastian Reichel <[email protected]>
> ---
> .../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>;
> + };
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
> 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");
>
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
On Thu, 24 Oct 2013 00:49:42 +0200, Sebastian Reichel <[email protected]> wrote:
> 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
Wow, that's crazy! It is all one device so put all the bindings into a
single file.
g,