Re: [PATCH 3/5] pwm: atmel-tcb: Put per-channel data into driver data

[claudiu beznea <claudiu.beznea@tuxon.dev>]

On 19.07.2023 22:20, Uwe Kleine-König wrote:
> This simplifies the code, reduces the number of memory allocations and
> pointer dereferences.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Reviewed-by: Claudiu Beznea <claudiu.beznea@tuxon.dev>

> ---
>   drivers/pwm/pwm-atmel-tcb.c | 29 +++++++++--------------------
>   1 file changed, 9 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/pwm/pwm-atmel-tcb.c b/drivers/pwm/pwm-atmel-tcb.c
> index 2826fc216d29..ae274bd7907d 100644
> --- a/drivers/pwm/pwm-atmel-tcb.c
> +++ b/drivers/pwm/pwm-atmel-tcb.c
> @@ -57,7 +57,7 @@ struct atmel_tcb_pwm_chip {
>   	struct clk *clk;
>   	struct clk *gclk;
>   	struct clk *slow_clk;
> -	struct atmel_tcb_pwm_device *pwms[NPWM];
> +	struct atmel_tcb_pwm_device pwms[NPWM];
>   	struct atmel_tcb_channel bkup;
>   };
>   
> @@ -73,7 +73,7 @@ static int atmel_tcb_pwm_set_polarity(struct pwm_chip *chip,
>   				      enum pwm_polarity polarity)
>   {
>   	struct atmel_tcb_pwm_chip *tcbpwmc = to_tcb_chip(chip);
> -	struct atmel_tcb_pwm_device *tcbpwm = tcbpwmc->pwms[pwm->hwpwm];
> +	struct atmel_tcb_pwm_device *tcbpwm = &tcbpwmc->pwms[pwm->hwpwm];
>   
>   	tcbpwm->polarity = polarity;
>   
> @@ -84,19 +84,13 @@ static int atmel_tcb_pwm_request(struct pwm_chip *chip,
>   				 struct pwm_device *pwm)
>   {
>   	struct atmel_tcb_pwm_chip *tcbpwmc = to_tcb_chip(chip);
> -	struct atmel_tcb_pwm_device *tcbpwm;
> +	struct atmel_tcb_pwm_device *tcbpwm = &tcbpwmc->pwms[pwm->hwpwm];
>   	unsigned cmr;
>   	int ret;
>   
> -	tcbpwm = devm_kzalloc(chip->dev, sizeof(*tcbpwm), GFP_KERNEL);
> -	if (!tcbpwm)
> -		return -ENOMEM;
> -
>   	ret = clk_prepare_enable(tcbpwmc->clk);
> -	if (ret) {
> -		devm_kfree(chip->dev, tcbpwm);
> +	if (ret)
>   		return ret;
> -	}
>   
>   	tcbpwm->polarity = PWM_POLARITY_NORMAL;
>   	tcbpwm->duty = 0;
> @@ -131,25 +125,20 @@ static int atmel_tcb_pwm_request(struct pwm_chip *chip,
>   	regmap_write(tcbpwmc->regmap, ATMEL_TC_REG(tcbpwmc->channel, CMR), cmr);
>   	spin_unlock(&tcbpwmc->lock);
>   
> -	tcbpwmc->pwms[pwm->hwpwm] = tcbpwm;
> -
>   	return 0;
>   }
>   
>   static void atmel_tcb_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
>   {
>   	struct atmel_tcb_pwm_chip *tcbpwmc = to_tcb_chip(chip);
> -	struct atmel_tcb_pwm_device *tcbpwm = tcbpwmc->pwms[pwm->hwpwm];
>   
>   	clk_disable_unprepare(tcbpwmc->clk);
> -	tcbpwmc->pwms[pwm->hwpwm] = NULL;
> -	devm_kfree(chip->dev, tcbpwm);
>   }
>   
>   static void atmel_tcb_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
>   {
>   	struct atmel_tcb_pwm_chip *tcbpwmc = to_tcb_chip(chip);
> -	struct atmel_tcb_pwm_device *tcbpwm = tcbpwmc->pwms[pwm->hwpwm];
> +	struct atmel_tcb_pwm_device *tcbpwm = &tcbpwmc->pwms[pwm->hwpwm];
>   	unsigned cmr;
>   	enum pwm_polarity polarity = tcbpwm->polarity;
>   
> @@ -206,7 +195,7 @@ static void atmel_tcb_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
>   static int atmel_tcb_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
>   {
>   	struct atmel_tcb_pwm_chip *tcbpwmc = to_tcb_chip(chip);
> -	struct atmel_tcb_pwm_device *tcbpwm = tcbpwmc->pwms[pwm->hwpwm];
> +	struct atmel_tcb_pwm_device *tcbpwm = &tcbpwmc->pwms[pwm->hwpwm];
>   	u32 cmr;
>   	enum pwm_polarity polarity = tcbpwm->polarity;
>   
> @@ -291,7 +280,7 @@ static int atmel_tcb_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
>   				int duty_ns, int period_ns)
>   {
>   	struct atmel_tcb_pwm_chip *tcbpwmc = to_tcb_chip(chip);
> -	struct atmel_tcb_pwm_device *tcbpwm = tcbpwmc->pwms[pwm->hwpwm];
> +	struct atmel_tcb_pwm_device *tcbpwm = &tcbpwmc->pwms[pwm->hwpwm];
>   	struct atmel_tcb_pwm_device *atcbpwm = NULL;
>   	int i = 0;
>   	int slowclk = 0;
> @@ -338,9 +327,9 @@ static int atmel_tcb_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
>   	period = div_u64(period_ns, min);
>   
>   	if (pwm->hwpwm == 0)
> -		atcbpwm = tcbpwmc->pwms[1];
> +		atcbpwm = &tcbpwmc->pwms[1];
>   	else
> -		atcbpwm = tcbpwmc->pwms[0];
> +		atcbpwm = &tcbpwmc->pwms[0];
>   
>   	/*
>   	 * PWM devices provided by the TCB driver are grouped by 2.