[PATCH] drivers/perf: hisi: Fix DEVICE_ATTR style tests warning for later PMU driver

Shaokun Zhang zhangshaokun at hisilicon.com
Tue Sep 15 21:41:02 EDT 2020


Hi Will,

在 2020/9/15 18:30, Will Deacon 写道:
> On Tue, Sep 15, 2020 at 10:39:28AM +0800, Shaokun Zhang wrote:
>> 在 2020/9/14 20:29, Will Deacon 写道:
>>> On Sat, Sep 12, 2020 at 05:03:06PM +0800, Shaokun Zhang wrote:
>>>> Since commit 001804689b0d ("checkpatch: add a few DEVICE_ATTR style
>>>> tests") has checked DEVICE_ATTR style, let's cleanup the sysfs interface
>>>> to get rid of the warning for later HiSilicon uncore PMU drivers.
>>>> Otherwise the warning is throwed by checkpatch.pl for new drivers as
>>>> follow:
>>>> WARNING: Consider renaming function(s) 'hisi_cpumask_sysfs_show' to
>>>> 'cpumask_show'
>>>
>>> [...]
>>>
>>>> diff --git a/drivers/perf/hisilicon/hisi_uncore_pmu.c b/drivers/perf/hisilicon/hisi_uncore_pmu.c
>>>> index 97aff877a4e7..e2612a73edf6 100644
>>>> --- a/drivers/perf/hisilicon/hisi_uncore_pmu.c
>>>> +++ b/drivers/perf/hisilicon/hisi_uncore_pmu.c
>>>> @@ -54,14 +54,14 @@ EXPORT_SYMBOL_GPL(hisi_event_sysfs_show);
>>>>  /*
>>>>   * sysfs cpumask attributes. For uncore PMU, we only have a single CPU to show
>>>>   */
>>>> -ssize_t hisi_cpumask_sysfs_show(struct device *dev,
>>>> -				struct device_attribute *attr, char *buf)
>>>> +ssize_t cpumask_show(struct device *dev, struct device_attribute *attr,
>>>> +		     char *buf)
>>>>  {
>>>>  	struct hisi_pmu *hisi_pmu = to_hisi_pmu(dev_get_drvdata(dev));
>>>>  
>>>>  	return sprintf(buf, "%d\n", hisi_pmu->on_cpu);
>>>>  }
>>>> -EXPORT_SYMBOL_GPL(hisi_cpumask_sysfs_show);
>>>> +EXPORT_SYMBOL_GPL(cpumask_show);
>>>
>>> This seems like a colossally bad idea to me.
>>
>> Apologies. We are developing new uncore PMU module drivers which will be sent to commuinity
>> later and do checkpatch.pl that introduces this warning. Since the interface is not new
>> developed, so you suggest to keep it the same, right?
> 
> I don't think checkpatch warnings hold much value, but in this case having a
> global (exported!) 'cpumask_show' symbol sounds like it is going to conflict
> with anybody else trying to fix similar warnings. By all means cleanup the

Oh, correct, I don't consider it enough.

> static symbols, but I don't think it's a good idea to go beyond that.

Got it.

Thanks,
Shaokun

> 
> Will
> .
> 



More information about the linux-arm-kernel mailing list