diff mbox series

[v2,1/3] pwm: bcm-kona: Use pwmchip_add() instead of pwmchip_add_with_polarity()

Message ID 20201207134556.25217-2-uwe@kleine-koenig.org
State Accepted
Headers show
Series pwm: get rid of pwmchip_add_with_polarity() | expand

Commit Message

Uwe Kleine-König Dec. 7, 2020, 1:45 p.m. UTC
The only side effect of this change is that pwm->state.polarity is
initialized to PWM_POLARITY_NORMAL instead of PWM_POLARITY_INVERSED.
However all other members of pwm->state are uninitialized and consumers
are expected to provide the right polarity (either by setting it explicitly
or by using a helper like pwm_init_state() that overwrites .polarity
anyhow with a value independent of the initial value).

The eventual goal is to remove pwmchip_add_with_polarity() and so simplify
the data flow in the PWM core.

Signed-off-by: Uwe Kleine-König <uwe@kleine-koenig.org>
---
 drivers/pwm/pwm-bcm-kona.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Thierry Reding March 22, 2021, 11:27 a.m. UTC | #1
On Mon, Dec 07, 2020 at 02:45:54PM +0100, Uwe Kleine-König wrote:
> The only side effect of this change is that pwm->state.polarity is
> initialized to PWM_POLARITY_NORMAL instead of PWM_POLARITY_INVERSED.
> However all other members of pwm->state are uninitialized and consumers
> are expected to provide the right polarity (either by setting it explicitly
> or by using a helper like pwm_init_state() that overwrites .polarity
> anyhow with a value independent of the initial value).
> 
> The eventual goal is to remove pwmchip_add_with_polarity() and so simplify
> the data flow in the PWM core.
> 
> Signed-off-by: Uwe Kleine-König <uwe@kleine-koenig.org>
> ---
>  drivers/pwm/pwm-bcm-kona.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

I was initially reluctant to apply this because the purpose of this API
was to mark chips as deviating from the default polarity. That is to say
that this chip's default is to produce an inversed PWM. However, I see
that the driver already deals with that properly during ->apply(), so
there is no reason for this to stay around.

The details are now a bit blurry, but I think initially the idea had
been to let the core handle this in some way (e.g. by rejecting a
polarity setting if ->set_polarity() wasn't implemented and the polarity
was not INVERSED for chips like this), but that's not happening in
practice.

Going forward this should all be dealt with at the driver level. Drivers
that support both polarities should just program them appropriately and
drivers that support only one should flat out refuse to set the other
one. One potential issue is for drivers that will use "normal" polarity
by default, but will work fine with "inverse" polarity (and,
correspondingly, an inverted duty-cycle/period ratio). I guess for those
they could always try with "normal" polarity and if that fails retry
with an inverted polarity and invert duty-cycle/period ratio.

Applied, thanks.

Thierry
diff mbox series

Patch

diff --git a/drivers/pwm/pwm-bcm-kona.c b/drivers/pwm/pwm-bcm-kona.c
index 16c5898b934a..40ecfb2bb632 100644
--- a/drivers/pwm/pwm-bcm-kona.c
+++ b/drivers/pwm/pwm-bcm-kona.c
@@ -303,7 +303,7 @@  static int kona_pwmc_probe(struct platform_device *pdev)
 
 	clk_disable_unprepare(kp->clk);
 
-	ret = pwmchip_add_with_polarity(&kp->chip, PWM_POLARITY_INVERSED);
+	ret = pwmchip_add(&kp->chip);
 	if (ret < 0)
 		dev_err(&pdev->dev, "failed to add PWM chip: %d\n", ret);