From: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
To: Michael Ellerman <michael@ellerman.id.au>
Cc: linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH] powerpc/kernel/sysfs: cleanup set up macros for pmc/non pmc sprs
Date: Tue, 01 Oct 2013 12:14:41 +0530 [thread overview]
Message-ID: <524A6F59.9060004@linux.vnet.ibm.com> (raw)
In-Reply-To: <20131001045035.GC17966@concordia>
On Tuesday 01 October 2013 10:20 AM, Michael Ellerman wrote:
> On Mon, Sep 30, 2013 at 04:47:29PM +0530, Madhavan Srinivasan wrote:
>> Currently pmc setup macros are used for non pmc sprs. This patch
>> add new set of macros and cleans up the code to use the new setup macro
>> for non pmc sprs.
>
> Hi Maddy,
>
> Firstly you should use "PMC" not pmc, it's an acronym. You should also
> spell out what it means the first time you use it, eg:
>
> Currently the PMC (Performance Monitor Counter) macros are used ..
>
> Secondly you need to say _why_ it is a bad idea to use the PMC macros
> for non-PMC SPRs.
>
Will change it.
>> diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c
>> index 27a90b9..73b6f9f 100644
>> --- a/arch/powerpc/kernel/sysfs.c
>> +++ b/arch/powerpc/kernel/sysfs.c
>> @@ -139,6 +139,37 @@ static ssize_t __used \
>> return count; \
>> }
>>
>> +#define SYSFS_SPRSETUP(NAME, ADDRESS) \
>> +static void read_##NAME(void *val) \
>> +{ \
>> + *(unsigned long *)val = mfspr(ADDRESS); \
>> +} \
>> +static void write_##NAME(void *val) \
>> +{ \
>> + mtspr(ADDRESS, *(unsigned long *)val); \
>> +} \
>> +static ssize_t show_##NAME(struct device *dev, \
>> + struct device_attribute *attr, \
>> + char *buf) \
>> +{ \
>> + struct cpu *cpu = container_of(dev, struct cpu, dev); \
>> + unsigned long val; \
>> + smp_call_function_single(cpu->dev.id, read_##NAME, &val, 1); \
>> + return sprintf(buf, "%lx\n", val); \
>> +} \
>> +static ssize_t __used \
>> + store_##NAME(struct device *dev, struct device_attribute *attr, \
>> + const char *buf, size_t count) \
>> +{ \
>> + struct cpu *cpu = container_of(dev, struct cpu, dev); \
>> + unsigned long val; \
>> + int ret = sscanf(buf, "%lx", &val); \
>> + if (ret != 1) \
>> + return -EINVAL; \
>> + smp_call_function_single(cpu->dev.id, write_##NAME, &val, 1); \
>> + return count; \
>> +}
>
> This is basically a complete copy of the SYSFS_PMCSETUP() macro, except
> for the one line removal of the call to ppc_enable_pmcs().
>
> You should be able to do better, by defining a macro that does all the
> boiler plate and takes an "extra" argument, which for PMCs is
> "ppc_enable_pmcs()" and for regular SPRs is empty.
>
Sorry about that. Will make the changes.
> cheers
>
Thanks for the feedback
prev parent reply other threads:[~2013-10-01 6:44 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-09-30 11:17 [PATCH] powerpc/kernel/sysfs: cleanup set up macros for pmc/non pmc sprs Madhavan Srinivasan
2013-10-01 4:50 ` Michael Ellerman
2013-10-01 6:44 ` Madhavan Srinivasan [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=524A6F59.9060004@linux.vnet.ibm.com \
--to=maddy@linux.vnet.ibm.com \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=michael@ellerman.id.au \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).