On 09/24/2012 08:49 PM, Sascha Hauer wrote:

On Mon, Sep 24, 2012 at 06:22:33PM +0200, Lars-Peter Clausen wrote:

quoted

On 09/24/2012 05:56 PM, Dmitry Torokhov wrote:

quoted

On Mon, Sep 24, 2012 at 07:55:38AM -0500, Rob Herring wrote:

quoted

On 09/24/2012 02:37 AM, Sascha Hauer wrote:

quoted

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 <redacted>

quoted

---
 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.

quoted

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