[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <3abc216a-1af5-4cc2-a70c-e406943be274@kylinos.cn>
Date: Fri, 23 May 2025 11:08:09 +0800
From: Pei Xiao <xiaopei01@...inos.cn>
To: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc: gregkh@...uxfoundation.org, linux-kernel@...r.kernel.org,
rafael@...nel.org, dakr@...nel.org, bartosz.golaszewski@...aro.org
Subject: Re: [PATCH] devres: Move remaining devm_alloc_percpu and
devm_device_add_group to devres.h
在 2025/5/22 20:54, Andy Shevchenko 写道:
> On Thu, May 22, 2025 at 05:01:26PM +0800, Pei Xiao wrote:
>> Since commit f5e5631df596("devres: Move devm_*_action*() APIs to
>> devres.h"), But devm_alloc_percpu() and devm_device_add_group didn't be
>> moved.
>>
>> so move it.The changes improve header organization by keeping all
>> resource-managed device APIs in the dedicated devres.h header,
>> reducing cross-header dependencies and making the interfaces
>> easier to locate.
> Thanks for the patch, my comments below.
>
> ...
>
>> -/**
>> - * devm_alloc_percpu - Resource-managed alloc_percpu
>> - * @dev: Device to allocate per-cpu memory for
>> - * @type: Type to allocate per-cpu memory for
>> - *
>> - * Managed alloc_percpu. Per-cpu memory allocated with this function is
>> - * automatically freed on driver detach.
>> - *
>> - * RETURNS:
>> - * Pointer to allocated memory on success, NULL on failure.
>> - */
>> -#define devm_alloc_percpu(dev, type) \
>> - ((typeof(type) __percpu *)__devm_alloc_percpu((dev), sizeof(type), \
>> - __alignof__(type)))
>> -
>> -void __percpu *__devm_alloc_percpu(struct device *dev, size_t size,
>> - size_t align);
>> -void devm_free_percpu(struct device *dev, void __percpu *pdata);
> Don't you need to cleanup the header inclusions as well?
>
> ...
>
>> -int __must_check devm_device_add_group(struct device *dev,
>> - const struct attribute_group *grp);
> I'm not sure about this. The percpu seems standalone piece, but this has
> relations with the other group related definitions just above.
>
> ...
>
>> +#include <linux/sysfs.h>
>> +#include <asm/percpu.h>
> What for are these new inclusions, please?
I add these for build error:
In file included from drivers/gpio/gpiolib-devres.c:9:
./include/linux/device/devres.h:189:15: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘*’ token
189 | void __percpu *__devm_alloc_percpu(struct device *dev, size_t size,
| ^
./include/linux/device/devres.h:191:57: error: expected ‘;’, ‘,’ or ‘)’ before ‘*’ token
191 | void devm_free_percpu(struct device *dev, void __percpu *pdata);
| ^
./include/linux/device/devres.h:194:25: warning: ‘struct attribute_group’ declared inside parameter list will not be visible outside of this definition or declaration
194 | const struct attribute_group *grp);
| ^~~~~~~~~~~~~~~
> ...
>
>> +/**
>> + * devm_alloc_percpu - Resource-managed alloc_percpu
>> + * @dev: Device to allocate per-cpu memory for
>> + * @type: Type to allocate per-cpu memory for
>> + *
>> + * Managed alloc_percpu. Per-cpu memory allocated with this function is
>> + * automatically freed on driver detach.
>> + *
>> + * RETURNS:
> Please, check that all of the kernel-doc are in align (using same style).
ok,get it
>> + * Pointer to allocated memory on success, NULL on failure.
>> + */
>> +#define devm_alloc_percpu(dev, type) \
>> + ((typeof(type) __percpu *)__devm_alloc_percpu((dev), sizeof(type), \
>> + __alignof__(type)))
> Just make it one line. it will be less than 100.
ok
>> +void __percpu *__devm_alloc_percpu(struct device *dev, size_t size,
>> + size_t align);
> Ditto.
ok.
>> +void devm_free_percpu(struct device *dev, void __percpu *pdata);
> Please, take your time to understand what is behind the __percpu and
> why the asm/percpu.h is redundant here.
>
Powered by blists - more mailing lists