[PATCH v1 1/5] docs: firmware-guide: ACPI: Add a PWM example

[PATCH v1 1/5] docs: firmware-guide: ACPI: Add a PWM example

[Andy Shevchenko]

When PWM support for ACPI has been added into the kernel, it missed
the documentation update. Hence update documentation here.

Fixes: 4a6ef8e37c4d ("pwm: Add support referencing PWMs from ACPI")
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 .../firmware-guide/acpi/enumeration.rst       | 32 +++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/Documentation/firmware-guide/acpi/enumeration.rst b/Documentation/firmware-guide/acpi/enumeration.rst
index 9f0d5c854fa4..0813508b45b0 100644
--- a/Documentation/firmware-guide/acpi/enumeration.rst
+++ b/Documentation/firmware-guide/acpi/enumeration.rst
@@ -258,6 +258,38 @@ input driver::
 		.id_table	= mpu3050_ids,
 	};
 
+Reference to PWM device
+=======================
+
+Sometimes a device can be a consumer of PWM channel. Obviously OS would like
+to know which one. To provide this mapping the special property has been
+introduced, i.e.::
+
+    Device (DEV)
+    {
+        Name (_DSD, Package ()
+        {
+            ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+            Package () {
+                Package () { "compatible", Package () { "pwm-leds" } },
+                Package () { "label", "alarm-led" },
+                Package () { "pwms",
+                    Package () {
+                        "\\_SB.PCI0.PWM",  // <PWM device reference>
+                        0,                 // <PWM index>
+                        600000,            // <PWM period>
+                        0,                 // <PWM flags>
+                    }
+                }
+            }
+
+        })
+        ...
+
+In the above example the PWM-based LED driver references to the PWM channel 0
+of \_SB.PCI0.PWM device with initial period setting equal to 600 us (note that
+value is given in nanoseconds).
+
 GPIO support
 ============
 
-- 
2.30.2

[PATCH v1 2/5] pwm: core: Always require PWM flags to be provided

[Andy Shevchenko]

It makes little sense to make PWM flags optional since in case
of multi-channel consumer the flags can be optional only for
the last listed channel. Thus always require PWM flags to be
provided.

Fixes: 4a6ef8e37c4d ("pwm: Add support referencing PWMs from ACPI")
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/pwm/core.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index c165c5822703..25f7b3370672 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -852,8 +852,10 @@ static struct pwm_chip *device_to_pwmchip(struct device *dev)
  *
  * This is analogous to of_pwm_get() except con_id is not yet supported.
  * ACPI entries must look like
- * Package () {"pwms", Package ()
- *     { <PWM device reference>, <PWM index>, <PWM period> [, <PWM flags>]}}
+ * Package () { "pwms", Package () {
+ *		<PWM device reference>, <PWM index>, <PWM period>, <PWM flags>,
+ *	}
+ * }
  *
  * Returns: A pointer to the requested PWM device or an ERR_PTR()-encoded
  * error code on failure.
@@ -877,7 +879,7 @@ static struct pwm_device *acpi_pwm_get(struct fwnode_handle *fwnode)
 	if (!acpi)
 		return ERR_PTR(-EINVAL);
 
-	if (args.nargs < 2)
+	if (args.nargs < 3)
 		return ERR_PTR(-EPROTO);
 
 	chip = device_to_pwmchip(&acpi->dev);
@@ -891,7 +893,7 @@ static struct pwm_device *acpi_pwm_get(struct fwnode_handle *fwnode)
 	pwm->args.period = args.args[1];
 	pwm->args.polarity = PWM_POLARITY_NORMAL;
 
-	if (args.nargs > 2 && args.args[2] & PWM_POLARITY_INVERTED)
+	if (args.args[2] & PWM_POLARITY_INVERTED)
 		pwm->args.polarity = PWM_POLARITY_INVERSED;
 #endif
 
-- 
2.30.2

[PATCH v1 3/5] pwm: core: Convert to use fwnode for matching

[Andy Shevchenko]

When we traverse the list of the registered PWM controllers,
use fwnode to match. This will help for further cleanup.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/pwm/core.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index 25f7b3370672..338d8ee369db 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -691,14 +691,14 @@ int pwm_adjust_config(struct pwm_device *pwm)
 }
 EXPORT_SYMBOL_GPL(pwm_adjust_config);
 
-static struct pwm_chip *of_node_to_pwmchip(struct device_node *np)
+static struct pwm_chip *fwnode_to_pwmchip(struct fwnode_handle *fwnode)
 {
 	struct pwm_chip *chip;
 
 	mutex_lock(&pwm_lock);
 
 	list_for_each_entry(chip, &pwm_chips, list)
-		if (chip->dev && chip->dev->of_node == np) {
+		if (chip->dev && dev_fwnode(chip->dev) == fwnode) {
 			mutex_unlock(&pwm_lock);
 			return chip;
 		}
@@ -777,7 +777,7 @@ struct pwm_device *of_pwm_get(struct device *dev, struct device_node *np,
 		return ERR_PTR(err);
 	}
 
-	pc = of_node_to_pwmchip(args.np);
+	pc = fwnode_to_pwmchip(of_fwnode_handle(args.np));
 	if (IS_ERR(pc)) {
 		if (PTR_ERR(pc) != -EPROBE_DEFER)
 			pr_err("%s(): PWM chip not found\n", __func__);
-- 
2.30.2

[PATCH v1 4/5] pwm: core: Reuse fwnode_to_pwmchip() in ACPI case

[Andy Shevchenko]

In ACPI case we may use matching by fwnode as provided via
fwnode_to_pwmchip(). This makes device_to_pwmchip() not needed
anymore.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/pwm/core.c | 31 +------------------------------
 1 file changed, 1 insertion(+), 30 deletions(-)

diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index 338d8ee369db..2223a9b6644b 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -819,28 +819,6 @@ struct pwm_device *of_pwm_get(struct device *dev, struct device_node *np,
 }
 EXPORT_SYMBOL_GPL(of_pwm_get);
 
-#if IS_ENABLED(CONFIG_ACPI)
-static struct pwm_chip *device_to_pwmchip(struct device *dev)
-{
-	struct pwm_chip *chip;
-
-	mutex_lock(&pwm_lock);
-
-	list_for_each_entry(chip, &pwm_chips, list) {
-		struct acpi_device *adev = ACPI_COMPANION(chip->dev);
-
-		if ((chip->dev == dev) || (adev && &adev->dev == dev)) {
-			mutex_unlock(&pwm_lock);
-			return chip;
-		}
-	}
-
-	mutex_unlock(&pwm_lock);
-
-	return ERR_PTR(-EPROBE_DEFER);
-}
-#endif
-
 /**
  * acpi_pwm_get() - request a PWM via parsing "pwms" property in ACPI
  * @fwnode: firmware node to get the "pwm" property from
@@ -863,9 +841,7 @@ static struct pwm_chip *device_to_pwmchip(struct device *dev)
 static struct pwm_device *acpi_pwm_get(struct fwnode_handle *fwnode)
 {
 	struct pwm_device *pwm = ERR_PTR(-ENODEV);
-#if IS_ENABLED(CONFIG_ACPI)
 	struct fwnode_reference_args args;
-	struct acpi_device *acpi;
 	struct pwm_chip *chip;
 	int ret;
 
@@ -875,14 +851,10 @@ static struct pwm_device *acpi_pwm_get(struct fwnode_handle *fwnode)
 	if (ret < 0)
 		return ERR_PTR(ret);
 
-	acpi = to_acpi_device_node(args.fwnode);
-	if (!acpi)
-		return ERR_PTR(-EINVAL);
-
 	if (args.nargs < 3)
 		return ERR_PTR(-EPROTO);
 
-	chip = device_to_pwmchip(&acpi->dev);
+	chip = fwnode_to_pwmchip(args.fwnode);
 	if (IS_ERR(chip))
 		return ERR_CAST(chip);
 
@@ -895,7 +867,6 @@ static struct pwm_device *acpi_pwm_get(struct fwnode_handle *fwnode)
 
 	if (args.args[2] & PWM_POLARITY_INVERTED)
 		pwm->args.polarity = PWM_POLARITY_INVERSED;
-#endif
 
 	return pwm;
 }
-- 
2.30.2

[PATCH v1 5/5] pwm: core: Unify fwnode checks in the module

[Andy Shevchenko]

Historically we have two different approaches on how to check type of fwnode.
Unify them using the latest and greatest fwnode related APIs.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/pwm/core.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index 2223a9b6644b..135eb5af9191 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -838,7 +838,7 @@ EXPORT_SYMBOL_GPL(of_pwm_get);
  * Returns: A pointer to the requested PWM device or an ERR_PTR()-encoded
  * error code on failure.
  */
-static struct pwm_device *acpi_pwm_get(struct fwnode_handle *fwnode)
+static struct pwm_device *acpi_pwm_get(const struct fwnode_handle *fwnode)
 {
 	struct pwm_device *pwm = ERR_PTR(-ENODEV);
 	struct fwnode_reference_args args;
@@ -922,6 +922,7 @@ void pwm_remove_table(struct pwm_lookup *table, size_t num)
  */
 struct pwm_device *pwm_get(struct device *dev, const char *con_id)
 {
+	const struct fwnode_handle *fwnode = dev ? dev_fwnode(dev) : NULL;
 	const char *dev_id = dev ? dev_name(dev) : NULL;
 	struct pwm_device *pwm;
 	struct pwm_chip *chip;
@@ -932,12 +933,12 @@ struct pwm_device *pwm_get(struct device *dev, const char *con_id)
 	int err;
 
 	/* look up via DT first */
-	if (IS_ENABLED(CONFIG_OF) && dev && dev->of_node)
-		return of_pwm_get(dev, dev->of_node, con_id);
+	if (is_of_node(fwnode))
+		return of_pwm_get(dev, to_of_node(fwnode), con_id);
 
 	/* then lookup via ACPI */
-	if (dev && is_acpi_node(dev->fwnode)) {
-		pwm = acpi_pwm_get(dev->fwnode);
+	if (is_acpi_node(fwnode)) {
+		pwm = acpi_pwm_get(fwnode);
 		if (!IS_ERR(pwm) || PTR_ERR(pwm) != -ENOENT)
 			return pwm;
 	}
-- 
2.30.2

Re: [PATCH v1 1/5] docs: firmware-guide: ACPI: Add a PWM example

[Andy Shevchenko]

On Mon, May 31, 2021 at 06:43:47PM +0300, Andy Shevchenko wrote:
> When PWM support for ACPI has been added into the kernel, it missed
> the documentation update. Hence update documentation here.

I am actually in doubt that original commit message provides that short period
intentionally. Possibly it was misinterpretation of nanoseconds (which is
expected by PWM framework) with microseconds than might sound logical for LED.

Perhaps we may add 000 to the value (1) and replace us by ms in the text (2).

...

> +                    Package () {
> +                        "\\_SB.PCI0.PWM",  // <PWM device reference>
> +                        0,                 // <PWM index>

> +                        600000,            // <PWM period>

(1)

> +                        0,                 // <PWM flags>
> +                    }

> +In the above example the PWM-based LED driver references to the PWM channel 0
> +of \_SB.PCI0.PWM device with initial period setting equal to 600 us (note that

(2)

> +value is given in nanoseconds).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/5] docs: firmware-guide: ACPI: Add a PWM example

[Andy Shevchenko]

On Mon, May 31, 2021 at 07:09:13PM +0300, Andy Shevchenko wrote:
> On Mon, May 31, 2021 at 06:43:47PM +0300, Andy Shevchenko wrote:
> > When PWM support for ACPI has been added into the kernel, it missed
> > the documentation update. Hence update documentation here.
> 
> I am actually in doubt that original commit message provides that short period
> intentionally. Possibly it was misinterpretation of nanoseconds (which is
> expected by PWM framework) with microseconds than might sound logical for LED.
> 
> Perhaps we may add 000 to the value (1) and replace us by ms in the text (2).

Okay, I will send an extended v2 and I think I will use 600 ms in the
documentation example.

-- 
With Best Regards,
Andy Shevchenko