Re: [PATCH 01/18] pwm: Provide devm_pwmchip_alloc() function

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Tue, Jul 18, 2023 at 08:18:32PM +0200, Uwe Kleine-König wrote:
> This function allocates a struct pwm_chip and driver data. Compared to
> the status quo the split into pwm_chip and driver data is new, otherwise
> it doesn't change anything relevant (yet).
> 
> The intention is that after all drivers are switched to use this
> allocation function, its possible to add a struct device to struct
> pwm_chip to properly track the latter's lifetime without touching all
> drivers again. Proper lifetime tracking is a necessary precondition to
> introduce character device support for PWMs (that implements atomic
> setting and doesn't suffer from the sysfs overhead of the /sys/class/pwm
> userspace support).
> 
> The new function pwmchip_priv() (obviously?) only works for chips
> allocated with devm_pwmchip_alloc().

...

> +void *pwmchip_priv(struct pwm_chip *chip)
> +{
> +	return (char *)chip + ALIGN(sizeof(*chip), 32);

Why 32? I haven't found any explanation on the choice. I can understand arch
minimum align, but hard coded value is a bit hard to get.

> +}

...

> +struct pwm_chip *devm_pwmchip_alloc(struct device *parent, size_t sizeof_priv)
> +{
> +	struct pwm_chip *chip;
> +	size_t alloc_size;

> +	alloc_size = ALIGN(sizeof(*chip), 32) + sizeof_priv;

Ditto.

Shouldn't it use a macro from overflow.h?

> +	chip = devm_kzalloc(parent, alloc_size, GFP_KERNEL);
> +	if (!chip)
> +		return NULL;
> +
> +	chip->dev = parent;
> +
> +	return chip;
> +}

-- 
With Best Regards,
Andy Shevchenko