* [PATCH] input: pwm-beeper: Add devicetree probing support
@ 2012-09-24 7:37 Sascha Hauer
2012-09-24 12:55 ` Rob Herring
0 siblings, 1 reply; 10+ messages in thread
From: Sascha Hauer @ 2012-09-24 7:37 UTC (permalink / raw)
To: linux-input
Cc: Dmitry Torokhov, Lars-Peter Clausen, Thierry Reding, kernel,
devicetree-discuss, Sascha Hauer
A very simple binding, the only property is the phandle to the PWM.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
Documentation/devicetree/bindings/input/pwm-beeper.txt | 7 +++++++
drivers/input/misc/pwm-beeper.c | 11 ++++++++++-
2 files changed, 17 insertions(+), 1 deletion(-)
create mode 100644 Documentation/devicetree/bindings/input/pwm-beeper.txt
diff --git a/Documentation/devicetree/bindings/input/pwm-beeper.txt b/Documentation/devicetree/bindings/input/pwm-beeper.txt
new file mode 100644
index 0000000..7388b82
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/pwm-beeper.txt
@@ -0,0 +1,7 @@
+* PWM beeper device tree bindings
+
+Registers a PWM device as beeper.
+
+Required properties:
+- compatible: should be "pwm-beeper"
+- pwms: phandle to the physical pwm device
diff --git a/drivers/input/misc/pwm-beeper.c b/drivers/input/misc/pwm-beeper.c
index fc84c8a..a6aa48c 100644
--- a/drivers/input/misc/pwm-beeper.c
+++ b/drivers/input/misc/pwm-beeper.c
@@ -75,7 +75,10 @@ static int __devinit pwm_beeper_probe(struct platform_device *pdev)
if (!beeper)
return -ENOMEM;
- beeper->pwm = pwm_request(pwm_id, "pwm beeper");
+ if (pdev->dev.platform_data)
+ beeper->pwm = pwm_request(pwm_id, "pwm beeper");
+ else
+ beeper->pwm = pwm_get(&pdev->dev, NULL);
if (IS_ERR(beeper->pwm)) {
error = PTR_ERR(beeper->pwm);
@@ -171,6 +174,11 @@ static SIMPLE_DEV_PM_OPS(pwm_beeper_pm_ops,
#define PWM_BEEPER_PM_OPS NULL
#endif
+static const struct of_device_id pwm_beeper_match[] = {
+ { .compatible = "pwm-beeper", },
+ { },
+};
+
static struct platform_driver pwm_beeper_driver = {
.probe = pwm_beeper_probe,
.remove = __devexit_p(pwm_beeper_remove),
@@ -178,6 +186,7 @@ static struct platform_driver pwm_beeper_driver = {
.name = "pwm-beeper",
.owner = THIS_MODULE,
.pm = PWM_BEEPER_PM_OPS,
+ .of_match_table = pwm_beeper_match,
},
};
module_platform_driver(pwm_beeper_driver);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH] input: pwm-beeper: Add devicetree probing support
2012-09-24 7:37 [PATCH] input: pwm-beeper: Add devicetree probing support Sascha Hauer
@ 2012-09-24 12:55 ` Rob Herring
2012-09-24 15:56 ` Dmitry Torokhov
0 siblings, 1 reply; 10+ messages in thread
From: Rob Herring @ 2012-09-24 12:55 UTC (permalink / raw)
To: Sascha Hauer
Cc: linux-input, devicetree-discuss, Dmitry Torokhov,
Lars-Peter Clausen, kernel
On 09/24/2012 02:37 AM, Sascha Hauer wrote:
> A very simple binding, the only property is the phandle to the PWM.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Acked-by: Rob Herring <rob.herring@calxeda.com>
> ---
> Documentation/devicetree/bindings/input/pwm-beeper.txt | 7 +++++++
> drivers/input/misc/pwm-beeper.c | 11 ++++++++++-
> 2 files changed, 17 insertions(+), 1 deletion(-)
> create mode 100644 Documentation/devicetree/bindings/input/pwm-beeper.txt
>
> diff --git a/Documentation/devicetree/bindings/input/pwm-beeper.txt b/Documentation/devicetree/bindings/input/pwm-beeper.txt
> new file mode 100644
> index 0000000..7388b82
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/pwm-beeper.txt
> @@ -0,0 +1,7 @@
> +* PWM beeper device tree bindings
> +
> +Registers a PWM device as beeper.
> +
> +Required properties:
> +- compatible: should be "pwm-beeper"
> +- pwms: phandle to the physical pwm device
> diff --git a/drivers/input/misc/pwm-beeper.c b/drivers/input/misc/pwm-beeper.c
> index fc84c8a..a6aa48c 100644
> --- a/drivers/input/misc/pwm-beeper.c
> +++ b/drivers/input/misc/pwm-beeper.c
> @@ -75,7 +75,10 @@ static int __devinit pwm_beeper_probe(struct platform_device *pdev)
> if (!beeper)
> return -ENOMEM;
>
> - beeper->pwm = pwm_request(pwm_id, "pwm beeper");
> + if (pdev->dev.platform_data)
> + beeper->pwm = pwm_request(pwm_id, "pwm beeper");
> + else
> + beeper->pwm = pwm_get(&pdev->dev, NULL);
>
> if (IS_ERR(beeper->pwm)) {
> error = PTR_ERR(beeper->pwm);
> @@ -171,6 +174,11 @@ static SIMPLE_DEV_PM_OPS(pwm_beeper_pm_ops,
> #define PWM_BEEPER_PM_OPS NULL
> #endif
>
> +static const struct of_device_id pwm_beeper_match[] = {
> + { .compatible = "pwm-beeper", },
> + { },
> +};
> +
> static struct platform_driver pwm_beeper_driver = {
> .probe = pwm_beeper_probe,
> .remove = __devexit_p(pwm_beeper_remove),
> @@ -178,6 +186,7 @@ static struct platform_driver pwm_beeper_driver = {
> .name = "pwm-beeper",
> .owner = THIS_MODULE,
> .pm = PWM_BEEPER_PM_OPS,
> + .of_match_table = pwm_beeper_match,
> },
> };
> module_platform_driver(pwm_beeper_driver);
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] input: pwm-beeper: Add devicetree probing support
2012-09-24 12:55 ` Rob Herring
@ 2012-09-24 15:56 ` Dmitry Torokhov
2012-09-24 16:22 ` Lars-Peter Clausen
0 siblings, 1 reply; 10+ messages in thread
From: Dmitry Torokhov @ 2012-09-24 15:56 UTC (permalink / raw)
To: Rob Herring
Cc: Sascha Hauer, linux-input, devicetree-discuss, Lars-Peter Clausen,
kernel
On Mon, Sep 24, 2012 at 07:55:38AM -0500, Rob Herring wrote:
> On 09/24/2012 02:37 AM, Sascha Hauer wrote:
> > A very simple binding, the only property is the phandle to the PWM.
> >
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>
> Acked-by: Rob Herring <rob.herring@calxeda.com>
>
> > ---
> > Documentation/devicetree/bindings/input/pwm-beeper.txt | 7 +++++++
> > drivers/input/misc/pwm-beeper.c | 11 ++++++++++-
> > 2 files changed, 17 insertions(+), 1 deletion(-)
> > create mode 100644 Documentation/devicetree/bindings/input/pwm-beeper.txt
> >
> > diff --git a/Documentation/devicetree/bindings/input/pwm-beeper.txt b/Documentation/devicetree/bindings/input/pwm-beeper.txt
> > new file mode 100644
> > index 0000000..7388b82
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/input/pwm-beeper.txt
> > @@ -0,0 +1,7 @@
> > +* PWM beeper device tree bindings
> > +
> > +Registers a PWM device as beeper.
> > +
> > +Required properties:
> > +- compatible: should be "pwm-beeper"
> > +- pwms: phandle to the physical pwm device
> > diff --git a/drivers/input/misc/pwm-beeper.c b/drivers/input/misc/pwm-beeper.c
> > index fc84c8a..a6aa48c 100644
> > --- a/drivers/input/misc/pwm-beeper.c
> > +++ b/drivers/input/misc/pwm-beeper.c
> > @@ -75,7 +75,10 @@ static int __devinit pwm_beeper_probe(struct platform_device *pdev)
> > if (!beeper)
> > return -ENOMEM;
> >
> > - beeper->pwm = pwm_request(pwm_id, "pwm beeper");
> > + if (pdev->dev.platform_data)
> > + beeper->pwm = pwm_request(pwm_id, "pwm beeper");
> > + else
> > + beeper->pwm = pwm_get(&pdev->dev, NULL);
Hmm, pwm_id == 0 is a valid ID I think, but your change makes it go into
DT branch, potentially breaking it.
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] input: pwm-beeper: Add devicetree probing support
2012-09-24 15:56 ` Dmitry Torokhov
@ 2012-09-24 16:22 ` Lars-Peter Clausen
2012-09-24 18:49 ` Sascha Hauer
0 siblings, 1 reply; 10+ messages in thread
From: Lars-Peter Clausen @ 2012-09-24 16:22 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Rob Herring, Sascha Hauer, linux-input, devicetree-discuss,
kernel
On 09/24/2012 05:56 PM, Dmitry Torokhov wrote:
> On Mon, Sep 24, 2012 at 07:55:38AM -0500, Rob Herring wrote:
>> On 09/24/2012 02:37 AM, Sascha Hauer wrote:
>>> A very simple binding, the only property is the phandle to the PWM.
>>>
>>> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>>
>> Acked-by: Rob Herring <rob.herring@calxeda.com>
>>
>>> ---
>>> Documentation/devicetree/bindings/input/pwm-beeper.txt | 7 +++++++
>>> drivers/input/misc/pwm-beeper.c | 11 ++++++++++-
>>> 2 files changed, 17 insertions(+), 1 deletion(-)
>>> create mode 100644 Documentation/devicetree/bindings/input/pwm-beeper.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/input/pwm-beeper.txt b/Documentation/devicetree/bindings/input/pwm-beeper.txt
>>> new file mode 100644
>>> index 0000000..7388b82
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/input/pwm-beeper.txt
>>> @@ -0,0 +1,7 @@
>>> +* PWM beeper device tree bindings
>>> +
>>> +Registers a PWM device as beeper.
>>> +
>>> +Required properties:
>>> +- compatible: should be "pwm-beeper"
>>> +- pwms: phandle to the physical pwm device
>>> diff --git a/drivers/input/misc/pwm-beeper.c b/drivers/input/misc/pwm-beeper.c
>>> index fc84c8a..a6aa48c 100644
>>> --- a/drivers/input/misc/pwm-beeper.c
>>> +++ b/drivers/input/misc/pwm-beeper.c
>>> @@ -75,7 +75,10 @@ static int __devinit pwm_beeper_probe(struct platform_device *pdev)
>>> if (!beeper)
>>> return -ENOMEM;
>>>
>>> - beeper->pwm = pwm_request(pwm_id, "pwm beeper");
>>> + if (pdev->dev.platform_data)
>>> + beeper->pwm = pwm_request(pwm_id, "pwm beeper");
>>> + else
>>> + beeper->pwm = pwm_get(&pdev->dev, NULL);
>
> Hmm, pwm_id == 0 is a valid ID I think, but your change makes it go into
> DT branch, potentially breaking it.
Yes, this a bit tricky, but we only have a single in-tree user of the
pwm-beeper which uses a id != 0. And now that all the PWM providers have
been converted to the new generic PWM framework the old legacy API will go
away soon anyway. So this if () else branch should hopefully only be
necessary for a transitional period of 1-2 releases. So I think this change
should be OK.
But I think the patch is missing a change to the Kconfig entry to allow the
driver to be selected if the generic PWM framework is available.
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -444,7 +444,7 @@ config INPUT_PCF8574
config INPUT_PWM_BEEPER
tristate "PWM beeper support"
- depends on HAVE_PWM
+ depends on HAVE_PWM || PWM
help
Say Y here to get support for PWM based beeper devices.
- Lars
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] input: pwm-beeper: Add devicetree probing support
2012-09-24 16:22 ` Lars-Peter Clausen
@ 2012-09-24 18:49 ` Sascha Hauer
2012-09-24 19:05 ` Lars-Peter Clausen
0 siblings, 1 reply; 10+ messages in thread
From: Sascha Hauer @ 2012-09-24 18:49 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: Dmitry Torokhov, Rob Herring, linux-input, devicetree-discuss,
kernel
On Mon, Sep 24, 2012 at 06:22:33PM +0200, Lars-Peter Clausen wrote:
> On 09/24/2012 05:56 PM, Dmitry Torokhov wrote:
> > On Mon, Sep 24, 2012 at 07:55:38AM -0500, Rob Herring wrote:
> >> On 09/24/2012 02:37 AM, Sascha Hauer wrote:
> >>> A very simple binding, the only property is the phandle to the PWM.
> >>>
> >>> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> >>
> >> Acked-by: Rob Herring <rob.herring@calxeda.com>
> >>
> >>> ---
> >>> Documentation/devicetree/bindings/input/pwm-beeper.txt | 7 +++++++
> >>> drivers/input/misc/pwm-beeper.c | 11 ++++++++++-
> >>> 2 files changed, 17 insertions(+), 1 deletion(-)
> >>> create mode 100644 Documentation/devicetree/bindings/input/pwm-beeper.txt
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/input/pwm-beeper.txt b/Documentation/devicetree/bindings/input/pwm-beeper.txt
> >>> new file mode 100644
> >>> index 0000000..7388b82
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/input/pwm-beeper.txt
> >>> @@ -0,0 +1,7 @@
> >>> +* PWM beeper device tree bindings
> >>> +
> >>> +Registers a PWM device as beeper.
> >>> +
> >>> +Required properties:
> >>> +- compatible: should be "pwm-beeper"
> >>> +- pwms: phandle to the physical pwm device
> >>> diff --git a/drivers/input/misc/pwm-beeper.c b/drivers/input/misc/pwm-beeper.c
> >>> index fc84c8a..a6aa48c 100644
> >>> --- a/drivers/input/misc/pwm-beeper.c
> >>> +++ b/drivers/input/misc/pwm-beeper.c
> >>> @@ -75,7 +75,10 @@ static int __devinit pwm_beeper_probe(struct platform_device *pdev)
> >>> if (!beeper)
> >>> return -ENOMEM;
> >>>
> >>> - beeper->pwm = pwm_request(pwm_id, "pwm beeper");
> >>> + if (pdev->dev.platform_data)
> >>> + beeper->pwm = pwm_request(pwm_id, "pwm beeper");
> >>> + else
> >>> + beeper->pwm = pwm_get(&pdev->dev, NULL);
> >
> > Hmm, pwm_id == 0 is a valid ID I think, but your change makes it go into
> > DT branch, potentially breaking it.
My bad, I missed that platform_data is casted to an unsigned long. I
thought I would test for a pointer.
The obvious clean way would be to use a pointer for platform_data, but
given that this will vanish anyway soon, I think we could just test for
existence of dev->of_node instead of dev->platform_data.
>
> Yes, this a bit tricky, but we only have a single in-tree user of the
> pwm-beeper which uses a id != 0. And now that all the PWM providers have
> been converted to the new generic PWM framework the old legacy API will go
> away soon anyway. So this if () else branch should hopefully only be
> necessary for a transitional period of 1-2 releases. So I think this change
> should be OK.
>
> But I think the patch is missing a change to the Kconfig entry to allow the
> driver to be selected if the generic PWM framework is available.
>
> --- a/drivers/input/misc/Kconfig
> +++ b/drivers/input/misc/Kconfig
> @@ -444,7 +444,7 @@ config INPUT_PCF8574
>
> config INPUT_PWM_BEEPER
> tristate "PWM beeper support"
> - depends on HAVE_PWM
> + depends on HAVE_PWM || PWM
Is this the preferred way to do this? Instead of doing the above I added
a 'select HAVE_PWM' to the pwm framework instead. I found a patch for that,
but there were comments to it that this is not good
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] input: pwm-beeper: Add devicetree probing support
2012-09-24 18:49 ` Sascha Hauer
@ 2012-09-24 19:05 ` Lars-Peter Clausen
2012-09-24 19:19 ` Sascha Hauer
2012-09-24 19:42 ` Thierry Reding
0 siblings, 2 replies; 10+ messages in thread
From: Lars-Peter Clausen @ 2012-09-24 19:05 UTC (permalink / raw)
To: Sascha Hauer
Cc: Dmitry Torokhov, Rob Herring, linux-input, devicetree-discuss,
kernel, Thierry Reding
On 09/24/2012 08:49 PM, Sascha Hauer wrote:
> On Mon, Sep 24, 2012 at 06:22:33PM +0200, Lars-Peter Clausen wrote:
>> On 09/24/2012 05:56 PM, Dmitry Torokhov wrote:
>>> On Mon, Sep 24, 2012 at 07:55:38AM -0500, Rob Herring wrote:
>>>> On 09/24/2012 02:37 AM, Sascha Hauer wrote:
>>>>> A very simple binding, the only property is the phandle to the PWM.
>>>>>
>>>>> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>>>>
>>>> Acked-by: Rob Herring <rob.herring@calxeda.com>
>>>>
>>>>> ---
>>>>> Documentation/devicetree/bindings/input/pwm-beeper.txt | 7 +++++++
>>>>> drivers/input/misc/pwm-beeper.c | 11 ++++++++++-
>>>>> 2 files changed, 17 insertions(+), 1 deletion(-)
>>>>> create mode 100644 Documentation/devicetree/bindings/input/pwm-beeper.txt
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/input/pwm-beeper.txt b/Documentation/devicetree/bindings/input/pwm-beeper.txt
>>>>> new file mode 100644
>>>>> index 0000000..7388b82
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/input/pwm-beeper.txt
>>>>> @@ -0,0 +1,7 @@
>>>>> +* PWM beeper device tree bindings
>>>>> +
>>>>> +Registers a PWM device as beeper.
>>>>> +
>>>>> +Required properties:
>>>>> +- compatible: should be "pwm-beeper"
>>>>> +- pwms: phandle to the physical pwm device
>>>>> diff --git a/drivers/input/misc/pwm-beeper.c b/drivers/input/misc/pwm-beeper.c
>>>>> index fc84c8a..a6aa48c 100644
>>>>> --- a/drivers/input/misc/pwm-beeper.c
>>>>> +++ b/drivers/input/misc/pwm-beeper.c
>>>>> @@ -75,7 +75,10 @@ static int __devinit pwm_beeper_probe(struct platform_device *pdev)
>>>>> if (!beeper)
>>>>> return -ENOMEM;
>>>>>
>>>>> - beeper->pwm = pwm_request(pwm_id, "pwm beeper");
>>>>> + if (pdev->dev.platform_data)
>>>>> + beeper->pwm = pwm_request(pwm_id, "pwm beeper");
>>>>> + else
>>>>> + beeper->pwm = pwm_get(&pdev->dev, NULL);
>>>
>>> Hmm, pwm_id == 0 is a valid ID I think, but your change makes it go into
>>> DT branch, potentially breaking it.
>
> My bad, I missed that platform_data is casted to an unsigned long. I
> thought I would test for a pointer.
> The obvious clean way would be to use a pointer for platform_data, but
> given that this will vanish anyway soon, I think we could just test for
> existence of dev->of_node instead of dev->platform_data.
I think the plan is to convert the existing board file platforms to pwm_table
and then remove the old pwm_request API. So this wouldn't work too well if we'd
test for of_node. Maybe we can just run pwm_get unconditionally and fallback to
pwm_request if it failed. That's also what the PWM backlight driver currently does.
>
>>
>> Yes, this a bit tricky, but we only have a single in-tree user of the
>> pwm-beeper which uses a id != 0. And now that all the PWM providers have
>> been converted to the new generic PWM framework the old legacy API will go
>> away soon anyway. So this if () else branch should hopefully only be
>> necessary for a transitional period of 1-2 releases. So I think this change
>> should be OK.
>>
>> But I think the patch is missing a change to the Kconfig entry to allow the
>> driver to be selected if the generic PWM framework is available.
>>
>> --- a/drivers/input/misc/Kconfig
>> +++ b/drivers/input/misc/Kconfig
>> @@ -444,7 +444,7 @@ config INPUT_PCF8574
>>
>> config INPUT_PWM_BEEPER
>> tristate "PWM beeper support"
>> - depends on HAVE_PWM
>> + depends on HAVE_PWM || PWM
>
> Is this the preferred way to do this? Instead of doing the above I added
> a 'select HAVE_PWM' to the pwm framework instead. I found a patch for that,
> but there were comments to it that this is not good
>
Thierry said that this is his preferred solution. Given that HAVE_PWM will be
extinct soon anyway I think it is fine.
- Lars
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] input: pwm-beeper: Add devicetree probing support
2012-09-24 19:05 ` Lars-Peter Clausen
@ 2012-09-24 19:19 ` Sascha Hauer
2012-09-24 19:42 ` Thierry Reding
1 sibling, 0 replies; 10+ messages in thread
From: Sascha Hauer @ 2012-09-24 19:19 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: Dmitry Torokhov, Rob Herring, linux-input, devicetree-discuss,
kernel, Thierry Reding
On Mon, Sep 24, 2012 at 09:05:59PM +0200, Lars-Peter Clausen wrote:
> On 09/24/2012 08:49 PM, Sascha Hauer wrote:
> >
> > My bad, I missed that platform_data is casted to an unsigned long. I
> > thought I would test for a pointer.
> > The obvious clean way would be to use a pointer for platform_data, but
> > given that this will vanish anyway soon, I think we could just test for
> > existence of dev->of_node instead of dev->platform_data.
>
> I think the plan is to convert the existing board file platforms to pwm_table
> and then remove the old pwm_request API. So this wouldn't work too well if we'd
> test for of_node. Maybe we can just run pwm_get unconditionally and fallback to
> pwm_request if it failed. That's also what the PWM backlight driver currently does.
Fine with me.
>
> >
> >>
> >> Yes, this a bit tricky, but we only have a single in-tree user of the
> >> pwm-beeper which uses a id != 0. And now that all the PWM providers have
> >> been converted to the new generic PWM framework the old legacy API will go
> >> away soon anyway. So this if () else branch should hopefully only be
> >> necessary for a transitional period of 1-2 releases. So I think this change
> >> should be OK.
> >>
> >> But I think the patch is missing a change to the Kconfig entry to allow the
> >> driver to be selected if the generic PWM framework is available.
> >>
> >> --- a/drivers/input/misc/Kconfig
> >> +++ b/drivers/input/misc/Kconfig
> >> @@ -444,7 +444,7 @@ config INPUT_PCF8574
> >>
> >> config INPUT_PWM_BEEPER
> >> tristate "PWM beeper support"
> >> - depends on HAVE_PWM
> >> + depends on HAVE_PWM || PWM
> >
> > Is this the preferred way to do this? Instead of doing the above I added
> > a 'select HAVE_PWM' to the pwm framework instead. I found a patch for that,
> > but there were comments to it that this is not good
> >
>
> Thierry said that this is his preferred solution. Given that HAVE_PWM will be
> extinct soon anyway I think it is fine.
Ok. Will send an updated patch tomorrow.
Thanks
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] input: pwm-beeper: Add devicetree probing support
2012-09-24 19:05 ` Lars-Peter Clausen
2012-09-24 19:19 ` Sascha Hauer
@ 2012-09-24 19:42 ` Thierry Reding
1 sibling, 0 replies; 10+ messages in thread
From: Thierry Reding @ 2012-09-24 19:42 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: Sascha Hauer, Dmitry Torokhov, Rob Herring, linux-input,
devicetree-discuss, kernel
[-- Attachment #1: Type: text/plain, Size: 5019 bytes --]
On Mon, Sep 24, 2012 at 09:05:59PM +0200, Lars-Peter Clausen wrote:
> On 09/24/2012 08:49 PM, Sascha Hauer wrote:
> > On Mon, Sep 24, 2012 at 06:22:33PM +0200, Lars-Peter Clausen wrote:
> >> On 09/24/2012 05:56 PM, Dmitry Torokhov wrote:
> >>> On Mon, Sep 24, 2012 at 07:55:38AM -0500, Rob Herring wrote:
> >>>> On 09/24/2012 02:37 AM, Sascha Hauer wrote:
> >>>>> A very simple binding, the only property is the phandle to the PWM.
> >>>>>
> >>>>> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> >>>>
> >>>> Acked-by: Rob Herring <rob.herring@calxeda.com>
> >>>>
> >>>>> ---
> >>>>> Documentation/devicetree/bindings/input/pwm-beeper.txt | 7 +++++++
> >>>>> drivers/input/misc/pwm-beeper.c | 11 ++++++++++-
> >>>>> 2 files changed, 17 insertions(+), 1 deletion(-)
> >>>>> create mode 100644 Documentation/devicetree/bindings/input/pwm-beeper.txt
> >>>>>
> >>>>> diff --git a/Documentation/devicetree/bindings/input/pwm-beeper.txt b/Documentation/devicetree/bindings/input/pwm-beeper.txt
> >>>>> new file mode 100644
> >>>>> index 0000000..7388b82
> >>>>> --- /dev/null
> >>>>> +++ b/Documentation/devicetree/bindings/input/pwm-beeper.txt
> >>>>> @@ -0,0 +1,7 @@
> >>>>> +* PWM beeper device tree bindings
> >>>>> +
> >>>>> +Registers a PWM device as beeper.
> >>>>> +
> >>>>> +Required properties:
> >>>>> +- compatible: should be "pwm-beeper"
> >>>>> +- pwms: phandle to the physical pwm device
> >>>>> diff --git a/drivers/input/misc/pwm-beeper.c b/drivers/input/misc/pwm-beeper.c
> >>>>> index fc84c8a..a6aa48c 100644
> >>>>> --- a/drivers/input/misc/pwm-beeper.c
> >>>>> +++ b/drivers/input/misc/pwm-beeper.c
> >>>>> @@ -75,7 +75,10 @@ static int __devinit pwm_beeper_probe(struct platform_device *pdev)
> >>>>> if (!beeper)
> >>>>> return -ENOMEM;
> >>>>>
> >>>>> - beeper->pwm = pwm_request(pwm_id, "pwm beeper");
> >>>>> + if (pdev->dev.platform_data)
> >>>>> + beeper->pwm = pwm_request(pwm_id, "pwm beeper");
> >>>>> + else
> >>>>> + beeper->pwm = pwm_get(&pdev->dev, NULL);
> >>>
> >>> Hmm, pwm_id == 0 is a valid ID I think, but your change makes it go into
> >>> DT branch, potentially breaking it.
> >
> > My bad, I missed that platform_data is casted to an unsigned long. I
> > thought I would test for a pointer.
> > The obvious clean way would be to use a pointer for platform_data, but
> > given that this will vanish anyway soon, I think we could just test for
> > existence of dev->of_node instead of dev->platform_data.
>
> I think the plan is to convert the existing board file platforms to pwm_table
> and then remove the old pwm_request API. So this wouldn't work too well if we'd
> test for of_node. Maybe we can just run pwm_get unconditionally and fallback to
> pwm_request if it failed. That's also what the PWM backlight driver currently does.
I agree. Calling pwm_get() will automatically switch to using the
entries from a PWM table if the board adds it.
> >> Yes, this a bit tricky, but we only have a single in-tree user of the
> >> pwm-beeper which uses a id != 0. And now that all the PWM providers have
> >> been converted to the new generic PWM framework the old legacy API will go
> >> away soon anyway. So this if () else branch should hopefully only be
> >> necessary for a transitional period of 1-2 releases. So I think this change
> >> should be OK.
> >>
> >> But I think the patch is missing a change to the Kconfig entry to allow the
> >> driver to be selected if the generic PWM framework is available.
> >>
> >> --- a/drivers/input/misc/Kconfig
> >> +++ b/drivers/input/misc/Kconfig
> >> @@ -444,7 +444,7 @@ config INPUT_PCF8574
> >>
> >> config INPUT_PWM_BEEPER
> >> tristate "PWM beeper support"
> >> - depends on HAVE_PWM
> >> + depends on HAVE_PWM || PWM
> >
> > Is this the preferred way to do this? Instead of doing the above I added
> > a 'select HAVE_PWM' to the pwm framework instead. I found a patch for that,
> > but there were comments to it that this is not good
> >
>
> Thierry said that this is his preferred solution. Given that HAVE_PWM will be
> extinct soon anyway I think it is fine.
The reason is that some platforms, actually the majority, do not define
HAVE_PWM property. We ran into that problem when the PWM tree was first
added to linux-next because builds would break on PowerPC, which is one
of the architectures that doesn't define HAVE_PWM.
Conceptually what we're saying here is that either the legacy PWM API
(HAVE_PWM) or the new PWM framework (PWM) can satisfy the dependency.
Note that if you make the change above, it is no longer enough to depend
on HAVE_PWM because that doesn't provide the pwm_get() function.
Also we should be able to remove HAVE_PWM for 3.7 already as all legacy
implementations have been moved. The plan is to get rid of pwm_request()
and pwm_free() for 3.8. All that's required is converting all the board
files to use PWM lookup tables.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] input: pwm-beeper: Add devicetree probing support
@ 2012-09-25 7:15 Sascha Hauer
2012-09-27 19:39 ` Thierry Reding
0 siblings, 1 reply; 10+ messages in thread
From: Sascha Hauer @ 2012-09-25 7:15 UTC (permalink / raw)
To: linux-input
Cc: Dmitry Torokhov, Lars-Peter Clausen, Thierry Reding, kernel,
Sascha Hauer
A very simple binding, the only property is the phandle to the PWM.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
changes since v1:
- first use new pwm API, if that fails, use legacy API
- add 'depends on PWM'
Documentation/devicetree/bindings/input/pwm-beeper.txt | 7 +++++++
drivers/input/misc/Kconfig | 2 +-
drivers/input/misc/pwm-beeper.c | 12 +++++++++++-
3 files changed, 19 insertions(+), 2 deletions(-)
create mode 100644 Documentation/devicetree/bindings/input/pwm-beeper.txt
diff --git a/Documentation/devicetree/bindings/input/pwm-beeper.txt b/Documentation/devicetree/bindings/input/pwm-beeper.txt
new file mode 100644
index 0000000..7388b82
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/pwm-beeper.txt
@@ -0,0 +1,7 @@
+* PWM beeper device tree bindings
+
+Registers a PWM device as beeper.
+
+Required properties:
+- compatible: should be "pwm-beeper"
+- pwms: phandle to the physical pwm device
diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 7c0f1ec..ff5dbd6 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -444,7 +444,7 @@ config INPUT_PCF8574
config INPUT_PWM_BEEPER
tristate "PWM beeper support"
- depends on HAVE_PWM
+ depends on HAVE_PWM || PWM
help
Say Y here to get support for PWM based beeper devices.
diff --git a/drivers/input/misc/pwm-beeper.c b/drivers/input/misc/pwm-beeper.c
index fc84c8a..dfb3c50 100644
--- a/drivers/input/misc/pwm-beeper.c
+++ b/drivers/input/misc/pwm-beeper.c
@@ -75,7 +75,11 @@ static int __devinit pwm_beeper_probe(struct platform_device *pdev)
if (!beeper)
return -ENOMEM;
- beeper->pwm = pwm_request(pwm_id, "pwm beeper");
+ beeper->pwm = pwm_get(&pdev->dev, NULL);
+ if (IS_ERR(beeper->pwm)) {
+ dev_err(&pdev->dev, "unable to request PWM, trying legacy API\n");
+ beeper->pwm = pwm_request(pwm_id, "pwm beeper");
+ }
if (IS_ERR(beeper->pwm)) {
error = PTR_ERR(beeper->pwm);
@@ -171,6 +175,11 @@ static SIMPLE_DEV_PM_OPS(pwm_beeper_pm_ops,
#define PWM_BEEPER_PM_OPS NULL
#endif
+static const struct of_device_id pwm_beeper_match[] = {
+ { .compatible = "pwm-beeper", },
+ { },
+};
+
static struct platform_driver pwm_beeper_driver = {
.probe = pwm_beeper_probe,
.remove = __devexit_p(pwm_beeper_remove),
@@ -178,6 +187,7 @@ static struct platform_driver pwm_beeper_driver = {
.name = "pwm-beeper",
.owner = THIS_MODULE,
.pm = PWM_BEEPER_PM_OPS,
+ .of_match_table = pwm_beeper_match,
},
};
module_platform_driver(pwm_beeper_driver);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH] input: pwm-beeper: Add devicetree probing support
2012-09-25 7:15 Sascha Hauer
@ 2012-09-27 19:39 ` Thierry Reding
0 siblings, 0 replies; 10+ messages in thread
From: Thierry Reding @ 2012-09-27 19:39 UTC (permalink / raw)
To: Sascha Hauer; +Cc: linux-input, Dmitry Torokhov, Lars-Peter Clausen, kernel
[-- Attachment #1: Type: text/plain, Size: 1104 bytes --]
On Tue, Sep 25, 2012 at 09:15:51AM +0200, Sascha Hauer wrote:
[...]
> diff --git a/drivers/input/misc/pwm-beeper.c b/drivers/input/misc/pwm-beeper.c
> index fc84c8a..dfb3c50 100644
> --- a/drivers/input/misc/pwm-beeper.c
> +++ b/drivers/input/misc/pwm-beeper.c
[...]
> @@ -171,6 +175,11 @@ static SIMPLE_DEV_PM_OPS(pwm_beeper_pm_ops,
> #define PWM_BEEPER_PM_OPS NULL
> #endif
>
> +static const struct of_device_id pwm_beeper_match[] = {
> + { .compatible = "pwm-beeper", },
> + { },
> +};
> +
This is usually protected by #ifdef CONFIG_OF...
> static struct platform_driver pwm_beeper_driver = {
> .probe = pwm_beeper_probe,
> .remove = __devexit_p(pwm_beeper_remove),
> @@ -178,6 +187,7 @@ static struct platform_driver pwm_beeper_driver = {
> .name = "pwm-beeper",
> .owner = THIS_MODULE,
> .pm = PWM_BEEPER_PM_OPS,
> + .of_match_table = pwm_beeper_match,
... and this of_match_ptr(pwm_beeper_match). They aren't technically
necessary but I guess it makes sense to not keep them around for !OF
configurations in order to save some bytes.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2012-09-27 19:39 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-24 7:37 [PATCH] input: pwm-beeper: Add devicetree probing support Sascha Hauer
2012-09-24 12:55 ` Rob Herring
2012-09-24 15:56 ` Dmitry Torokhov
2012-09-24 16:22 ` Lars-Peter Clausen
2012-09-24 18:49 ` Sascha Hauer
2012-09-24 19:05 ` Lars-Peter Clausen
2012-09-24 19:19 ` Sascha Hauer
2012-09-24 19:42 ` Thierry Reding
-- strict thread matches above, loose matches on Subject: below --
2012-09-25 7:15 Sascha Hauer
2012-09-27 19:39 ` Thierry Reding
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).