[PATCH] pwm: samsung: Implement .apply() callback

[PATCH] pwm: samsung: Implement .apply() callback

[Uwe Kleine-König]

To eventually get rid of all legacy drivers convert this driver to the
modern world implementing .apply().

The size check for state->period is moved to .apply() to make sure that
the values of state->duty_cycle and state->period are passed to
pwm_samsung_config without change while they are discarded to int.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/pwm/pwm-samsung.c | 54 ++++++++++++++++++++++++++++++---------
 1 file changed, 42 insertions(+), 12 deletions(-)

diff --git a/drivers/pwm/pwm-samsung.c b/drivers/pwm/pwm-samsung.c
index 0a4ff55fad04..9c5b4f515641 100644
--- a/drivers/pwm/pwm-samsung.c
+++ b/drivers/pwm/pwm-samsung.c
@@ -321,14 +321,6 @@ static int __pwm_samsung_config(struct pwm_chip *chip, struct pwm_device *pwm,
 	struct samsung_pwm_channel *chan = pwm_get_chip_data(pwm);
 	u32 tin_ns = chan->tin_ns, tcnt, tcmp, oldtcmp;
 
-	/*
-	 * We currently avoid using 64bit arithmetic by using the
-	 * fact that anything faster than 1Hz is easily representable
-	 * by 32bits.
-	 */
-	if (period_ns > NSEC_PER_SEC)
-		return -ERANGE;
-
 	tcnt = readl(our_chip->base + REG_TCNTB(pwm->hwpwm));
 	oldtcmp = readl(our_chip->base + REG_TCMPB(pwm->hwpwm));
 
@@ -438,13 +430,51 @@ static int pwm_samsung_set_polarity(struct pwm_chip *chip,
 	return 0;
 }
 
+static int pwm_samsung_apply(struct pwm_chip *chip, struct pwm_device *pwm,
+			     const struct pwm_state *state)
+{
+	int err, enabled = pwm->state.enabled;
+
+	if (state->polarity != pwm->state.polarity) {
+		if (enabled) {
+			pwm_samsung_disable(chip, pwm);
+			enabled = false;
+		}
+
+		err = pwm_samsung_set_polarity(chip, pwm, state->polarity);
+		if (err)
+			return err;
+	}
+
+	if (!state->enabled) {
+		if (enabled)
+			pwm_samsung_disable(chip, pwm);
+
+		return 0;
+	}
+
+	/*
+	 * We currently avoid using 64bit arithmetic by using the
+	 * fact that anything faster than 1Hz is easily representable
+	 * by 32bits.
+	 */
+	if (state->period > NSEC_PER_SEC)
+		return -ERANGE;
+
+	err = pwm_samsung_config(chip, pwm, state->duty_cycle, state->period);
+	if (err)
+		return err;
+
+	if (!pwm->state.enabled)
+		err = pwm_samsung_enable(chip, pwm);
+
+	return err;
+}
+
 static const struct pwm_ops pwm_samsung_ops = {
 	.request	= pwm_samsung_request,
 	.free		= pwm_samsung_free,
-	.enable		= pwm_samsung_enable,
-	.disable	= pwm_samsung_disable,
-	.config		= pwm_samsung_config,
-	.set_polarity	= pwm_samsung_set_polarity,
+	.apply		= pwm_samsung_apply,
 	.owner		= THIS_MODULE,
 };
 

base-commit: ed14d36498c8d15be098df4af9ca324f96e9de74
-- 
2.35.1

[PATCH] pwm: samsung: Implement .apply() callback

[Uwe Kleine-König]

To eventually get rid of all legacy drivers convert this driver to the
modern world implementing .apply().

The size check for state->period is moved to .apply() to make sure that
the values of state->duty_cycle and state->period are passed to
pwm_samsung_config without change while they are discarded to int.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/pwm/pwm-samsung.c | 54 ++++++++++++++++++++++++++++++---------
 1 file changed, 42 insertions(+), 12 deletions(-)

diff --git a/drivers/pwm/pwm-samsung.c b/drivers/pwm/pwm-samsung.c
index 0a4ff55fad04..9c5b4f515641 100644
--- a/drivers/pwm/pwm-samsung.c
+++ b/drivers/pwm/pwm-samsung.c
@@ -321,14 +321,6 @@ static int __pwm_samsung_config(struct pwm_chip *chip, struct pwm_device *pwm,
 	struct samsung_pwm_channel *chan = pwm_get_chip_data(pwm);
 	u32 tin_ns = chan->tin_ns, tcnt, tcmp, oldtcmp;
 
-	/*
-	 * We currently avoid using 64bit arithmetic by using the
-	 * fact that anything faster than 1Hz is easily representable
-	 * by 32bits.
-	 */
-	if (period_ns > NSEC_PER_SEC)
-		return -ERANGE;
-
 	tcnt = readl(our_chip->base + REG_TCNTB(pwm->hwpwm));
 	oldtcmp = readl(our_chip->base + REG_TCMPB(pwm->hwpwm));
 
@@ -438,13 +430,51 @@ static int pwm_samsung_set_polarity(struct pwm_chip *chip,
 	return 0;
 }
 
+static int pwm_samsung_apply(struct pwm_chip *chip, struct pwm_device *pwm,
+			     const struct pwm_state *state)
+{
+	int err, enabled = pwm->state.enabled;
+
+	if (state->polarity != pwm->state.polarity) {
+		if (enabled) {
+			pwm_samsung_disable(chip, pwm);
+			enabled = false;
+		}
+
+		err = pwm_samsung_set_polarity(chip, pwm, state->polarity);
+		if (err)
+			return err;
+	}
+
+	if (!state->enabled) {
+		if (enabled)
+			pwm_samsung_disable(chip, pwm);
+
+		return 0;
+	}
+
+	/*
+	 * We currently avoid using 64bit arithmetic by using the
+	 * fact that anything faster than 1Hz is easily representable
+	 * by 32bits.
+	 */
+	if (state->period > NSEC_PER_SEC)
+		return -ERANGE;
+
+	err = pwm_samsung_config(chip, pwm, state->duty_cycle, state->period);
+	if (err)
+		return err;
+
+	if (!pwm->state.enabled)
+		err = pwm_samsung_enable(chip, pwm);
+
+	return err;
+}
+
 static const struct pwm_ops pwm_samsung_ops = {
 	.request	= pwm_samsung_request,
 	.free		= pwm_samsung_free,
-	.enable		= pwm_samsung_enable,
-	.disable	= pwm_samsung_disable,
-	.config		= pwm_samsung_config,
-	.set_polarity	= pwm_samsung_set_polarity,
+	.apply		= pwm_samsung_apply,
 	.owner		= THIS_MODULE,
 };
 

base-commit: ed14d36498c8d15be098df4af9ca324f96e9de74
-- 
2.35.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] pwm: samsung: Implement .apply() callback

[Krzysztof Kozlowski]

On 28/03/2022 09:34, Uwe Kleine-König wrote:
> To eventually get rid of all legacy drivers convert this driver to the
> modern world implementing .apply().
> 
> The size check for state->period is moved to .apply() to make sure that
> the values of state->duty_cycle and state->period are passed to
> pwm_samsung_config without change while they are discarded to int.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>  drivers/pwm/pwm-samsung.c | 54 ++++++++++++++++++++++++++++++---------
>  1 file changed, 42 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/pwm/pwm-samsung.c b/drivers/pwm/pwm-samsung.c
> index 0a4ff55fad04..9c5b4f515641 100644
> --- a/drivers/pwm/pwm-samsung.c
> +++ b/drivers/pwm/pwm-samsung.c
> @@ -321,14 +321,6 @@ static int __pwm_samsung_config(struct pwm_chip *chip, struct pwm_device *pwm,
>  	struct samsung_pwm_channel *chan = pwm_get_chip_data(pwm);
>  	u32 tin_ns = chan->tin_ns, tcnt, tcmp, oldtcmp;
>  
> -	/*
> -	 * We currently avoid using 64bit arithmetic by using the
> -	 * fact that anything faster than 1Hz is easily representable
> -	 * by 32bits.
> -	 */
> -	if (period_ns > NSEC_PER_SEC)
> -		return -ERANGE;
> -
>  	tcnt = readl(our_chip->base + REG_TCNTB(pwm->hwpwm));
>  	oldtcmp = readl(our_chip->base + REG_TCMPB(pwm->hwpwm));
>  
> @@ -438,13 +430,51 @@ static int pwm_samsung_set_polarity(struct pwm_chip *chip,
>  	return 0;
>  }
>  
> +static int pwm_samsung_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> +			     const struct pwm_state *state)
> +{
> +	int err, enabled = pwm->state.enabled;
> +
> +	if (state->polarity != pwm->state.polarity) {
> +		if (enabled) {
> +			pwm_samsung_disable(chip, pwm);
> +			enabled = false;
> +		}
> +
> +		err = pwm_samsung_set_polarity(chip, pwm, state->polarity);
> +		if (err)
> +			return err;
> +	}
> +
> +	if (!state->enabled) {
> +		if (enabled)
> +			pwm_samsung_disable(chip, pwm);
> +
> +		return 0;
> +	}
> +
> +	/*
> +	 * We currently avoid using 64bit arithmetic by using the
> +	 * fact that anything faster than 1Hz is easily representable
> +	 * by 32bits.
> +	 */
> +	if (state->period > NSEC_PER_SEC)

Isn't this changing a bit logic in case of setting wrong period and
inverted signal?

Before, the config would return early and do nothing. Now, you disable
the PWM, check the condition here and exit with PWM disabled. I think
this should reverse the tasks done, or the check should be done early.

> +		return -ERANGE;
> +
> +	err = pwm_samsung_config(chip, pwm, state->duty_cycle, state->period);
> +	if (err)
> +		return err;

Best regards,
Krzysztof

Re: [PATCH] pwm: samsung: Implement .apply() callback

[Krzysztof Kozlowski]

On 28/03/2022 09:34, Uwe Kleine-König wrote:
> To eventually get rid of all legacy drivers convert this driver to the
> modern world implementing .apply().
> 
> The size check for state->period is moved to .apply() to make sure that
> the values of state->duty_cycle and state->period are passed to
> pwm_samsung_config without change while they are discarded to int.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>  drivers/pwm/pwm-samsung.c | 54 ++++++++++++++++++++++++++++++---------
>  1 file changed, 42 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/pwm/pwm-samsung.c b/drivers/pwm/pwm-samsung.c
> index 0a4ff55fad04..9c5b4f515641 100644
> --- a/drivers/pwm/pwm-samsung.c
> +++ b/drivers/pwm/pwm-samsung.c
> @@ -321,14 +321,6 @@ static int __pwm_samsung_config(struct pwm_chip *chip, struct pwm_device *pwm,
>  	struct samsung_pwm_channel *chan = pwm_get_chip_data(pwm);
>  	u32 tin_ns = chan->tin_ns, tcnt, tcmp, oldtcmp;
>  
> -	/*
> -	 * We currently avoid using 64bit arithmetic by using the
> -	 * fact that anything faster than 1Hz is easily representable
> -	 * by 32bits.
> -	 */
> -	if (period_ns > NSEC_PER_SEC)
> -		return -ERANGE;
> -
>  	tcnt = readl(our_chip->base + REG_TCNTB(pwm->hwpwm));
>  	oldtcmp = readl(our_chip->base + REG_TCMPB(pwm->hwpwm));
>  
> @@ -438,13 +430,51 @@ static int pwm_samsung_set_polarity(struct pwm_chip *chip,
>  	return 0;
>  }
>  
> +static int pwm_samsung_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> +			     const struct pwm_state *state)
> +{
> +	int err, enabled = pwm->state.enabled;
> +
> +	if (state->polarity != pwm->state.polarity) {
> +		if (enabled) {
> +			pwm_samsung_disable(chip, pwm);
> +			enabled = false;
> +		}
> +
> +		err = pwm_samsung_set_polarity(chip, pwm, state->polarity);
> +		if (err)
> +			return err;
> +	}
> +
> +	if (!state->enabled) {
> +		if (enabled)
> +			pwm_samsung_disable(chip, pwm);
> +
> +		return 0;
> +	}
> +
> +	/*
> +	 * We currently avoid using 64bit arithmetic by using the
> +	 * fact that anything faster than 1Hz is easily representable
> +	 * by 32bits.
> +	 */
> +	if (state->period > NSEC_PER_SEC)

Isn't this changing a bit logic in case of setting wrong period and
inverted signal?

Before, the config would return early and do nothing. Now, you disable
the PWM, check the condition here and exit with PWM disabled. I think
this should reverse the tasks done, or the check should be done early.

> +		return -ERANGE;
> +
> +	err = pwm_samsung_config(chip, pwm, state->duty_cycle, state->period);
> +	if (err)
> +		return err;

Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] pwm: samsung: Implement .apply() callback

[Uwe Kleine-König]

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

On Fri, Apr 22, 2022 at 05:11:02PM +0200, Krzysztof Kozlowski wrote:
> On 28/03/2022 09:34, Uwe Kleine-König wrote:
> > To eventually get rid of all legacy drivers convert this driver to the
> > modern world implementing .apply().
> > 
> > The size check for state->period is moved to .apply() to make sure that
> > the values of state->duty_cycle and state->period are passed to
> > pwm_samsung_config without change while they are discarded to int.
> > 
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > ---
> >  drivers/pwm/pwm-samsung.c | 54 ++++++++++++++++++++++++++++++---------
> >  1 file changed, 42 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/pwm/pwm-samsung.c b/drivers/pwm/pwm-samsung.c
> > index 0a4ff55fad04..9c5b4f515641 100644
> > --- a/drivers/pwm/pwm-samsung.c
> > +++ b/drivers/pwm/pwm-samsung.c
> > @@ -321,14 +321,6 @@ static int __pwm_samsung_config(struct pwm_chip *chip, struct pwm_device *pwm,
> >  	struct samsung_pwm_channel *chan = pwm_get_chip_data(pwm);
> >  	u32 tin_ns = chan->tin_ns, tcnt, tcmp, oldtcmp;
> >  
> > -	/*
> > -	 * We currently avoid using 64bit arithmetic by using the
> > -	 * fact that anything faster than 1Hz is easily representable
> > -	 * by 32bits.
> > -	 */
> > -	if (period_ns > NSEC_PER_SEC)
> > -		return -ERANGE;
> > -
> >  	tcnt = readl(our_chip->base + REG_TCNTB(pwm->hwpwm));
> >  	oldtcmp = readl(our_chip->base + REG_TCMPB(pwm->hwpwm));
> >  
> > @@ -438,13 +430,51 @@ static int pwm_samsung_set_polarity(struct pwm_chip *chip,
> >  	return 0;
> >  }
> >  
> > +static int pwm_samsung_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> > +			     const struct pwm_state *state)
> > +{
> > +	int err, enabled = pwm->state.enabled;
> > +
> > +	if (state->polarity != pwm->state.polarity) {
> > +		if (enabled) {
> > +			pwm_samsung_disable(chip, pwm);
> > +			enabled = false;
> > +		}
> > +
> > +		err = pwm_samsung_set_polarity(chip, pwm, state->polarity);
> > +		if (err)
> > +			return err;
> > +	}
> > +
> > +	if (!state->enabled) {
> > +		if (enabled)
> > +			pwm_samsung_disable(chip, pwm);
> > +
> > +		return 0;
> > +	}
> > +
> > +	/*
> > +	 * We currently avoid using 64bit arithmetic by using the
> > +	 * fact that anything faster than 1Hz is easily representable
> > +	 * by 32bits.
> > +	 */
> > +	if (state->period > NSEC_PER_SEC)
> 
> Isn't this changing a bit logic in case of setting wrong period and
> inverted signal?
> 
> Before, the config would return early and do nothing. Now, you disable
> the PWM, check the condition here and exit with PWM disabled. I think
> this should reverse the tasks done, or the check should be done early.

The intension here is to just push the legacy implementation of .apply()
(i.e.  pwm_apply_legacy()) into the driver, to be able to get rid of the
legacy handing in the core. I think the problem you point out is real,
but it is not introduced by my change which doesn't change behaviour.

The problem is just that it's not possible to "see" that period is
invalid at the time the polarity is changed and so it exists since at
least 5ec803edcb703fe379836f13560b79dfac79b01d, my patch just made it
more obvious.

So yes, there is potential to further improve the driver, but that's out
of scope for this 1:1 conversion patch.

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

Re: [PATCH] pwm: samsung: Implement .apply() callback

[Uwe Kleine-König]

[-- Attachment #1.1: Type: text/plain, Size: 3558 bytes --]

On Fri, Apr 22, 2022 at 05:11:02PM +0200, Krzysztof Kozlowski wrote:
> On 28/03/2022 09:34, Uwe Kleine-König wrote:
> > To eventually get rid of all legacy drivers convert this driver to the
> > modern world implementing .apply().
> > 
> > The size check for state->period is moved to .apply() to make sure that
> > the values of state->duty_cycle and state->period are passed to
> > pwm_samsung_config without change while they are discarded to int.
> > 
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > ---
> >  drivers/pwm/pwm-samsung.c | 54 ++++++++++++++++++++++++++++++---------
> >  1 file changed, 42 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/pwm/pwm-samsung.c b/drivers/pwm/pwm-samsung.c
> > index 0a4ff55fad04..9c5b4f515641 100644
> > --- a/drivers/pwm/pwm-samsung.c
> > +++ b/drivers/pwm/pwm-samsung.c
> > @@ -321,14 +321,6 @@ static int __pwm_samsung_config(struct pwm_chip *chip, struct pwm_device *pwm,
> >  	struct samsung_pwm_channel *chan = pwm_get_chip_data(pwm);
> >  	u32 tin_ns = chan->tin_ns, tcnt, tcmp, oldtcmp;
> >  
> > -	/*
> > -	 * We currently avoid using 64bit arithmetic by using the
> > -	 * fact that anything faster than 1Hz is easily representable
> > -	 * by 32bits.
> > -	 */
> > -	if (period_ns > NSEC_PER_SEC)
> > -		return -ERANGE;
> > -
> >  	tcnt = readl(our_chip->base + REG_TCNTB(pwm->hwpwm));
> >  	oldtcmp = readl(our_chip->base + REG_TCMPB(pwm->hwpwm));
> >  
> > @@ -438,13 +430,51 @@ static int pwm_samsung_set_polarity(struct pwm_chip *chip,
> >  	return 0;
> >  }
> >  
> > +static int pwm_samsung_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> > +			     const struct pwm_state *state)
> > +{
> > +	int err, enabled = pwm->state.enabled;
> > +
> > +	if (state->polarity != pwm->state.polarity) {
> > +		if (enabled) {
> > +			pwm_samsung_disable(chip, pwm);
> > +			enabled = false;
> > +		}
> > +
> > +		err = pwm_samsung_set_polarity(chip, pwm, state->polarity);
> > +		if (err)
> > +			return err;
> > +	}
> > +
> > +	if (!state->enabled) {
> > +		if (enabled)
> > +			pwm_samsung_disable(chip, pwm);
> > +
> > +		return 0;
> > +	}
> > +
> > +	/*
> > +	 * We currently avoid using 64bit arithmetic by using the
> > +	 * fact that anything faster than 1Hz is easily representable
> > +	 * by 32bits.
> > +	 */
> > +	if (state->period > NSEC_PER_SEC)
> 
> Isn't this changing a bit logic in case of setting wrong period and
> inverted signal?
> 
> Before, the config would return early and do nothing. Now, you disable
> the PWM, check the condition here and exit with PWM disabled. I think
> this should reverse the tasks done, or the check should be done early.

The intension here is to just push the legacy implementation of .apply()
(i.e.  pwm_apply_legacy()) into the driver, to be able to get rid of the
legacy handing in the core. I think the problem you point out is real,
but it is not introduced by my change which doesn't change behaviour.

The problem is just that it's not possible to "see" that period is
invalid at the time the polarity is changed and so it exists since at
least 5ec803edcb703fe379836f13560b79dfac79b01d, my patch just made it
more obvious.

So yes, there is potential to further improve the driver, but that's out
of scope for this 1:1 conversion patch.

Best regards
Uwe

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

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

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] pwm: samsung: Implement .apply() callback

[Krzysztof Kozlowski]

On 25/04/2022 19:16, Uwe Kleine-König wrote:

>>> +	/*
>>> +	 * We currently avoid using 64bit arithmetic by using the
>>> +	 * fact that anything faster than 1Hz is easily representable
>>> +	 * by 32bits.
>>> +	 */
>>> +	if (state->period > NSEC_PER_SEC)
>>
>> Isn't this changing a bit logic in case of setting wrong period and
>> inverted signal?
>>
>> Before, the config would return early and do nothing. Now, you disable
>> the PWM, check the condition here and exit with PWM disabled. I think
>> this should reverse the tasks done, or the check should be done early.
> 
> The intension here is to just push the legacy implementation of .apply()
> (i.e.  pwm_apply_legacy()) into the driver, to be able to get rid of the
> legacy handing in the core. I think the problem you point out is real,
> but it is not introduced by my change which doesn't change behaviour.
> 
> The problem is just that it's not possible to "see" that period is
> invalid at the time the polarity is changed and so it exists since at
> least 5ec803edcb703fe379836f13560b79dfac79b01d, my patch just made it
> more obvious.
> 
> So yes, there is potential to further improve the driver, but that's out
> of scope for this 1:1 conversion patch.

Sounds reasonable, thanks for explanation. Everything else was looking
good, so:

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>




Best regards,
Krzysztof

Re: [PATCH] pwm: samsung: Implement .apply() callback

[Krzysztof Kozlowski]

On 25/04/2022 19:16, Uwe Kleine-König wrote:

>>> +	/*
>>> +	 * We currently avoid using 64bit arithmetic by using the
>>> +	 * fact that anything faster than 1Hz is easily representable
>>> +	 * by 32bits.
>>> +	 */
>>> +	if (state->period > NSEC_PER_SEC)
>>
>> Isn't this changing a bit logic in case of setting wrong period and
>> inverted signal?
>>
>> Before, the config would return early and do nothing. Now, you disable
>> the PWM, check the condition here and exit with PWM disabled. I think
>> this should reverse the tasks done, or the check should be done early.
> 
> The intension here is to just push the legacy implementation of .apply()
> (i.e.  pwm_apply_legacy()) into the driver, to be able to get rid of the
> legacy handing in the core. I think the problem you point out is real,
> but it is not introduced by my change which doesn't change behaviour.
> 
> The problem is just that it's not possible to "see" that period is
> invalid at the time the polarity is changed and so it exists since at
> least 5ec803edcb703fe379836f13560b79dfac79b01d, my patch just made it
> more obvious.
> 
> So yes, there is potential to further improve the driver, but that's out
> of scope for this 1:1 conversion patch.

Sounds reasonable, thanks for explanation. Everything else was looking
good, so:

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>




Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] pwm: samsung: Implement .apply() callback

[Thierry Reding]

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

On Mon, Mar 28, 2022 at 09:34:34AM +0200, Uwe Kleine-König wrote:
> To eventually get rid of all legacy drivers convert this driver to the
> modern world implementing .apply().
> 
> The size check for state->period is moved to .apply() to make sure that
> the values of state->duty_cycle and state->period are passed to
> pwm_samsung_config without change while they are discarded to int.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>  drivers/pwm/pwm-samsung.c | 54 ++++++++++++++++++++++++++++++---------
>  1 file changed, 42 insertions(+), 12 deletions(-)

Applied, thanks.

Thierry

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

Re: [PATCH] pwm: samsung: Implement .apply() callback

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 637 bytes --]

On Mon, Mar 28, 2022 at 09:34:34AM +0200, Uwe Kleine-König wrote:
> To eventually get rid of all legacy drivers convert this driver to the
> modern world implementing .apply().
> 
> The size check for state->period is moved to .apply() to make sure that
> the values of state->duty_cycle and state->period are passed to
> pwm_samsung_config without change while they are discarded to int.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>  drivers/pwm/pwm-samsung.c | 54 ++++++++++++++++++++++++++++++---------
>  1 file changed, 42 insertions(+), 12 deletions(-)

Applied, thanks.

Thierry

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

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] pwm: samsung: Implement .apply() callback

[Uwe Kleine-König]

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

On Sun, Jun 19, 2022 at 10:21:54PM +0200, Uwe Kleine-König wrote:
> Hello,
> 
> just a short note about a few things I noticed about this patch status
> mail:
> 
> The To is strange, didn't try to understand it and just keep it to
> learn, if it does the right thing. Still looks wrong. (For reference:
> 	To: =?utf-8?q?Uwe_Kleine-K=C3=B6nig_=3Cu=2Ekleine-koenig=40pengutronix=2Ede=3E?=@ci.codeaurora.org
> )

I learned it doesn't work, my mailer daemon told me, this won't work.

Ah, I reported something like that already earlier, see
https://lore.kernel.org/tools/20211018134444.k5k45xiy652cklyl@meerkat.local
expanded Cc: accordingly. I keep the context below around for the new
recipents. Note there is another issue pointed out there.

Best regards
Uwe

> On Mon, Jun 06, 2022 at 08:24:43AM +0000, patchwork-bot+linux-samsung-soc@kernel.org wrote:
> > This patch was applied to pinctrl/samsung.git (for-next)
> > by Thierry Reding <thierry.reding@gmail.com>:
> > 
> > On Mon, 28 Mar 2022 09:34:34 +0200 you wrote:
> > > To eventually get rid of all legacy drivers convert this driver to the
> > > modern world implementing .apply().
> > > 
> > > The size check for state->period is moved to .apply() to make sure that
> > > the values of state->duty_cycle and state->period are passed to
> > > pwm_samsung_config without change while they are discarded to int.
> > > 
> > > [...]
> > 
> > Here is the summary with links:
> >   - pwm: samsung: Implement .apply() callback
> >     https://git.kernel.org/pinctrl/samsung/c/daa986d5f8d8
> 
> This is a bit misleading, because the patch was actually merged via
> Thierry's tree into v5.19-rc1 and (I guess) only entered the
> samsung-pinctrl tree via an update to a new upstream version.



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