Re: [PATCH V2] powerpc/kernel/sysfs: disable writing to purr in non-powernv

[Benjamin Herrenschmidt <benh@kernel.crashing.org>]

On Tue, 2013-10-01 at 16:31 +1000, Michael Ellerman wrote:

> > 1)Changed the test for to hypervisor mode instead of platform
> 
> I think Ben's wrong about that.
> 
> Almost all existing code uses FW_FEATURE_LPAR to differentiate
> hypervisor vs guest mode, so I think we should do the same here.

I didn't object to using the FW test, it's a reasonable way to do it, I
objected to using the platform as an indication (powernv vs. pseries)

Ben.

> So it would be:
> 
> > +	if (cpu_has_feature(CPU_FTR_PURR)) {
> > +		if (!firmware_has_feature(FW_FEATURE_LPAR))
> > +			add_write_permission_dev_attr((void *)&dev_attr_purr);
> >  		device_create_file(s, &dev_attr_purr);
> > +	}
> 
> 
> > +static void add_write_permission_dev_attr(void *ptr)
> > +{
> > +	struct device_attribute *attr = (struct device_attribute *)ptr;
> > +
> > +	attr->attr.mode |= (unsigned short) 0200;
> > +}
> 
> Why does it take a void *, which then requires a cast at the call site?
> 
> And do you need the cast to short? If so shouldn't you use umode_t
> directly?
> 
> cheers