Re: [PATCH 18/18] gpio: mvebu: Make use of devm_pwmchip_alloc() function

[Bartosz Golaszewski <brgl@xxxxxxxx>]

On Tue, Jul 18, 2023 at 8:19 PM Uwe Kleine-König
<u.kleine-koenig@xxxxxxxxxxxxxx> wrote:
>
> This prepares the pwm sub-driver to further changes of the pwm core
> outlined in the commit introducing devm_pwmchip_alloc(). There is no
> intended semantical change and the driver should behave as before.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx>
> ---
>  drivers/gpio/gpio-mvebu.c | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
> index a35958e7adf6..9557cac807f9 100644
> --- a/drivers/gpio/gpio-mvebu.c
> +++ b/drivers/gpio/gpio-mvebu.c
> @@ -98,7 +98,6 @@ struct mvebu_pwm {
>         u32                      offset;
>         unsigned long            clk_rate;
>         struct gpio_desc        *gpiod;
> -       struct pwm_chip          chip;
>         spinlock_t               lock;
>         struct mvebu_gpio_chip  *mvchip;
>
> @@ -614,7 +613,7 @@ static const struct regmap_config mvebu_gpio_regmap_config = {
>   */
>  static struct mvebu_pwm *to_mvebu_pwm(struct pwm_chip *chip)
>  {
> -       return container_of(chip, struct mvebu_pwm, chip);
> +       return pwmchip_priv(chip);
>  }
>
>  static int mvebu_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
> @@ -789,6 +788,7 @@ static int mvebu_pwm_probe(struct platform_device *pdev,
>  {
>         struct device *dev = &pdev->dev;
>         struct mvebu_pwm *mvpwm;
> +       struct pwm_chip *chip;
>         void __iomem *base;
>         u32 offset;
>         u32 set;
> @@ -813,9 +813,11 @@ static int mvebu_pwm_probe(struct platform_device *pdev,
>         if (IS_ERR(mvchip->clk))
>                 return PTR_ERR(mvchip->clk);
>
> -       mvpwm = devm_kzalloc(dev, sizeof(struct mvebu_pwm), GFP_KERNEL);
> -       if (!mvpwm)
> +       chip = devm_pwmchip_alloc(dev, sizeof(struct mvebu_pwm));
> +       if (!chip)
>                 return -ENOMEM;
> +       mvpwm = pwmchip_priv(chip);
> +
>         mvchip->mvpwm = mvpwm;
>         mvpwm->mvchip = mvchip;
>         mvpwm->offset = offset;
> @@ -868,13 +870,12 @@ static int mvebu_pwm_probe(struct platform_device *pdev,
>                 return -EINVAL;
>         }
>
> -       mvpwm->chip.dev = dev;
> -       mvpwm->chip.ops = &mvebu_pwm_ops;
> -       mvpwm->chip.npwm = mvchip->chip.ngpio;
> +       chip->ops = &mvebu_pwm_ops;
> +       chip->npwm = mvchip->chip.ngpio;
>
>         spin_lock_init(&mvpwm->lock);
>
> -       return devm_pwmchip_add(dev, &mvpwm->chip);
> +       return devm_pwmchip_add(dev, chip);
>  }
>
>  #ifdef CONFIG_DEBUG_FS
> --
> 2.39.2
>

Looks good to me (although I have my reservations about the concept of
foo_alloc() for subsystems in the kernel...). How do you want this to
go upstream?

Bart