Re: [PATCH v2] PM / devfreq: use _visible attribute to replace create/remove_sysfs_files()

["zhangpengjie (A)" <zhangpengjie2@huawei.com>]

On 10/31/2025 10:41 PM, Jonathan Cameron wrote:
> On Tue, 28 Oct 2025 10:24:58 +0800
> Pengjie Zhang <zhangpengjie2@huawei.com> wrote:
>
>>   static ssize_t polling_interval_store(struct device *dev,
>> @@ -1828,15 +1831,22 @@ static ssize_t polling_interval_store(struct device *dev,
>>   	unsigned int value;
>>   	int ret;
>>   
>> -	if (!df->governor)
>> +	mutex_lock(&devfreq_list_lock);
> As below I'd use guard() to simplify this.  Applies in various other places in this
> patch.
>>   
>> -/* Remove the specific sysfs files which depend on each governor. */
>> -static void remove_sysfs_files(struct devfreq *devfreq,
>> -				const struct devfreq_governor *gov)
>> +static bool gov_group_visible(struct kobject *kobj)
>>   {
>> -	if (IS_SUPPORTED_ATTR(gov->attrs, POLLING_INTERVAL))
>> -		sysfs_remove_file(&devfreq->dev.kobj,
>> -				&dev_attr_polling_interval.attr);
>> -	if (IS_SUPPORTED_ATTR(gov->attrs, TIMER))
>> -		sysfs_remove_file(&devfreq->dev.kobj, &dev_attr_timer.attr);
>> +	struct device *dev = kobj_to_dev(kobj);
>> +	struct devfreq *df;
>> +
>> +	if (!dev)
>> +		return false;
>> +
>> +	df = to_devfreq(dev);
>> +	if (!df)
>> +		return false;
> Isn't to_devfreq() just a container_of() wrapper?
> If that's the case it will always be there if dev is not NULL.
>
> As such I not seeing how this group is likely to ever be
> invisible.
>
Thank you for your review and comments.  I will address all your feedback

in the necessary revisions and submit the third version shortly.

Best regards,

Pengjie Zhang