linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] pwm: Introduce single-PWM of_xlate function
@ 2021-04-23 21:33 Bjorn Andersson
  2021-04-24 11:32 ` Uwe Kleine-König
  0 siblings, 1 reply; 4+ messages in thread
From: Bjorn Andersson @ 2021-04-23 21:33 UTC (permalink / raw)
  To: Thierry Reding, Uwe Kleine-König, Lee Jones; +Cc: linux-pwm, linux-kernel

The existing pxa driver and the upcoming addition of PWM support in the
TI sn565dsi86 DSI/eDP bridge driver both has a single PWM channel and
thereby a need for a of_xlate function with the period as its single
argument.

Introduce a common helper function in the core that can be used as
of_xlate by such drivers and migrate the pxa driver to use this.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---

Changes since v1:
- Included the pwm.h change in the commit...

 drivers/pwm/core.c    | 24 ++++++++++++++++++++++++
 drivers/pwm/pwm-pxa.c | 16 +---------------
 include/linux/pwm.h   |  2 ++
 3 files changed, 27 insertions(+), 15 deletions(-)

diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index c4d5c0667137..25ed23cdf0d3 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -176,6 +176,30 @@ of_pwm_simple_xlate(struct pwm_chip *pc, const struct of_phandle_args *args)
 	return pwm;
 }
 
+struct pwm_device *
+of_pwm_single_xlate(struct pwm_chip *pc, const struct of_phandle_args *args)
+{
+	struct pwm_device *pwm;
+
+	/* sanity check driver is single PWM */
+	if (pc->of_pwm_n_cells != 1)
+		return ERR_PTR(-EINVAL);
+
+	/* validate that one cell is specified */
+	if (args->args_count != 1)
+		return ERR_PTR(-EINVAL);
+
+	pwm = pwm_request_from_chip(pc, 0, NULL);
+	if (IS_ERR(pwm))
+		return pwm;
+
+	pwm->args.period = args->args[0];
+	pwm->args.polarity = PWM_POLARITY_NORMAL;
+
+	return pwm;
+}
+EXPORT_SYMBOL_GPL(of_pwm_single_xlate);
+
 static void of_pwmchip_add(struct pwm_chip *chip)
 {
 	if (!chip->dev || !chip->dev->of_node)
diff --git a/drivers/pwm/pwm-pxa.c b/drivers/pwm/pwm-pxa.c
index cfb683827d32..8cd82fb54483 100644
--- a/drivers/pwm/pwm-pxa.c
+++ b/drivers/pwm/pwm-pxa.c
@@ -148,20 +148,6 @@ static const struct platform_device_id *pxa_pwm_get_id_dt(struct device *dev)
 	return id ? id->data : NULL;
 }
 
-static struct pwm_device *
-pxa_pwm_of_xlate(struct pwm_chip *pc, const struct of_phandle_args *args)
-{
-	struct pwm_device *pwm;
-
-	pwm = pwm_request_from_chip(pc, 0, NULL);
-	if (IS_ERR(pwm))
-		return pwm;
-
-	pwm->args.period = args->args[0];
-
-	return pwm;
-}
-
 static int pwm_probe(struct platform_device *pdev)
 {
 	const struct platform_device_id *id = platform_get_device_id(pdev);
@@ -187,7 +173,7 @@ static int pwm_probe(struct platform_device *pdev)
 	pwm->chip.npwm = (id->driver_data & HAS_SECONDARY_PWM) ? 2 : 1;
 
 	if (IS_ENABLED(CONFIG_OF)) {
-		pwm->chip.of_xlate = pxa_pwm_of_xlate;
+		pwm->chip.of_xlate = of_pwm_single_xlate;
 		pwm->chip.of_pwm_n_cells = 1;
 	}
 
diff --git a/include/linux/pwm.h b/include/linux/pwm.h
index 5bb90af4997e..0ac242a5b5e4 100644
--- a/include/linux/pwm.h
+++ b/include/linux/pwm.h
@@ -405,6 +405,8 @@ struct pwm_device *pwm_request_from_chip(struct pwm_chip *chip,
 
 struct pwm_device *of_pwm_xlate_with_flags(struct pwm_chip *pc,
 		const struct of_phandle_args *args);
+struct pwm_device *of_pwm_single_xlate(struct pwm_chip *pc,
+				       const struct of_phandle_args *args);
 
 struct pwm_device *pwm_get(struct device *dev, const char *con_id);
 struct pwm_device *of_pwm_get(struct device *dev, struct device_node *np,
-- 
2.31.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] pwm: Introduce single-PWM of_xlate function
  2021-04-23 21:33 [PATCH v2] pwm: Introduce single-PWM of_xlate function Bjorn Andersson
@ 2021-04-24 11:32 ` Uwe Kleine-König
  2021-04-26 15:24   ` Bjorn Andersson
  0 siblings, 1 reply; 4+ messages in thread
From: Uwe Kleine-König @ 2021-04-24 11:32 UTC (permalink / raw)
  To: Bjorn Andersson; +Cc: Thierry Reding, Lee Jones, linux-pwm, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1132 bytes --]

Hello,

On Fri, Apr 23, 2021 at 04:33:04PM -0500, Bjorn Andersson wrote:
> The existing pxa driver and the upcoming addition of PWM support in the
> TI sn565dsi86 DSI/eDP bridge driver both has a single PWM channel and
> thereby a need for a of_xlate function with the period as its single
> argument.
> 
> Introduce a common helper function in the core that can be used as
> of_xlate by such drivers and migrate the pxa driver to use this.
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>

I'm OK with the idea as such. I'd like to see the semantic expanded a
bit however such that the function can parse

	pwms = <&mypwm 50000>;

and also

	pwms = <&mypwm 500000 PWM_POLARITY_INVERTED>;

. You suggetion only covers the former.

See
https://lore.kernel.org/r/20210315111124.2475274-2-u.kleine-koenig@pengutronix.de
for my first attempt to unify of_pwm_xlate_with_flags and
of_pwm_simple_xlate accordingly.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] pwm: Introduce single-PWM of_xlate function
  2021-04-24 11:32 ` Uwe Kleine-König
@ 2021-04-26 15:24   ` Bjorn Andersson
  2021-04-26 16:19     ` Uwe Kleine-König
  0 siblings, 1 reply; 4+ messages in thread
From: Bjorn Andersson @ 2021-04-26 15:24 UTC (permalink / raw)
  To: Uwe Kleine-K?nig; +Cc: Thierry Reding, Lee Jones, linux-pwm, linux-kernel

On Sat 24 Apr 06:32 CDT 2021, Uwe Kleine-K?nig wrote:

> Hello,
> 
> On Fri, Apr 23, 2021 at 04:33:04PM -0500, Bjorn Andersson wrote:
> > The existing pxa driver and the upcoming addition of PWM support in the
> > TI sn565dsi86 DSI/eDP bridge driver both has a single PWM channel and
> > thereby a need for a of_xlate function with the period as its single
> > argument.
> > 
> > Introduce a common helper function in the core that can be used as
> > of_xlate by such drivers and migrate the pxa driver to use this.
> > 
> > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> 
> I'm OK with the idea as such. I'd like to see the semantic expanded a
> bit however such that the function can parse
> 
> 	pwms = <&mypwm 50000>;
> 
> and also
> 
> 	pwms = <&mypwm 500000 PWM_POLARITY_INVERTED>;
> 
> . You suggetion only covers the former.

One concern though is that a single-channel pwm with the optional flag
would syntactically be indistinguishable from a multi-channel property
without flags. Presumably the values are out of range though, so I
suppose there's no problem in practice.

Please let me know if you think there's any merit to this concern and
I'll respin the patch accordingly.

Thanks,
Bjorn

> 
> See
> https://lore.kernel.org/r/20210315111124.2475274-2-u.kleine-koenig@pengutronix.de
> for my first attempt to unify of_pwm_xlate_with_flags and
> of_pwm_simple_xlate accordingly.
> 
> Best regards
> Uwe
> 
> -- 
> Pengutronix e.K.                           | Uwe Kleine-König            |
> Industrial Linux Solutions                 | https://www.pengutronix.de/ |



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] pwm: Introduce single-PWM of_xlate function
  2021-04-26 15:24   ` Bjorn Andersson
@ 2021-04-26 16:19     ` Uwe Kleine-König
  0 siblings, 0 replies; 4+ messages in thread
From: Uwe Kleine-König @ 2021-04-26 16:19 UTC (permalink / raw)
  To: Bjorn Andersson; +Cc: Thierry Reding, Lee Jones, linux-pwm, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1696 bytes --]

On Mon, Apr 26, 2021 at 10:24:27AM -0500, Bjorn Andersson wrote:
> On Sat 24 Apr 06:32 CDT 2021, Uwe Kleine-K?nig wrote:
> 
> > Hello,
> > 
> > On Fri, Apr 23, 2021 at 04:33:04PM -0500, Bjorn Andersson wrote:
> > > The existing pxa driver and the upcoming addition of PWM support in the
> > > TI sn565dsi86 DSI/eDP bridge driver both has a single PWM channel and
> > > thereby a need for a of_xlate function with the period as its single
> > > argument.
> > > 
> > > Introduce a common helper function in the core that can be used as
> > > of_xlate by such drivers and migrate the pxa driver to use this.
> > > 
> > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> > 
> > I'm OK with the idea as such. I'd like to see the semantic expanded a
> > bit however such that the function can parse
> > 
> > 	pwms = <&mypwm 50000>;
> > 
> > and also
> > 
> > 	pwms = <&mypwm 500000 PWM_POLARITY_INVERTED>;
> > 
> > . You suggetion only covers the former.
> 
> One concern though is that a single-channel pwm with the optional flag
> would syntactically be indistinguishable from a multi-channel property
> without flags. Presumably the values are out of range though, so I
> suppose there's no problem in practice.

I personally have no problem with it, for clk-providers it is also
normal that some need an id and others don't. If we have such concerns
(Thierry?) we should insist that new drivers always require an channel
id (which is always 0 for single-channel PWMs).

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-04-26 16:19 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-23 21:33 [PATCH v2] pwm: Introduce single-PWM of_xlate function Bjorn Andersson
2021-04-24 11:32 ` Uwe Kleine-König
2021-04-26 15:24   ` Bjorn Andersson
2021-04-26 16:19     ` Uwe Kleine-König

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