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

From: Will Deacon <will@kernel.org>
To: Shaokun Zhang <zhangshaokun@hisilicon.com>
Cc: Mark Rutland <mark.rutland@arm.com>, Qi Liu <liuqi115@huawei.com>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] drivers/perf: hisi: Fix DEVICE_ATTR style tests warning for later PMU driver
Date: Tue, 15 Sep 2020 11:30:45 +0100	[thread overview]
Message-ID: <20200915103044.GA25957@willie-the-truck> (raw)
In-Reply-To: <36f4e162-2a6a-0dae-f7a8-212e4bfee7e0@hisilicon.com>

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
static symbols, but I don't think it's a good idea to go beyond that.

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel