[PATCH 1/4] powerpc: Add firmware_set_feature()

Michael Ellerman michael at ellerman.id.au
Thu Jan 12 21:13:38 EST 2006


On Thu, 12 Jan 2006 20:29, Paul Mackerras wrote:
> Michael Ellerman writes:
> > We have firmware_has_feature() to test for features, add
> > firmware_set_feature() so that no one needs to touch
> > ppc64_firmware_features directly.
>
> Because they might get their fingers dirty? :)

Exactly.

> Seriously, I don't think firmware_set_feature is really a help.  To me
> it's easier to understand what ppc64_firmware_features = xxx does than
> to understand firmware_set_feature(xxx) does, because with the latter
> I have to go off and look for the definition of firmware_set_feature.
> And while I am finding it I'll be wondering whether it actually does a
> firmware call or something, or if it does some extra checking, or if I
> can only call it for one feature at a time (it is firmware_set_feature
> not firmware_set_features, after all).

Well actually I just thought it made it a proper API, if we don't need 
firmware_set_feature() then why do we need firmware_has_feature().

It does do some checking, and should probably BUG_ON or refuse to build if we 
try to assign impossible features. But perhaps that's just hand holding ? :)

> Also, uninitialized variables (outside of procedures) are always set
> to 0.  This is required by the C standard.  If you add the = 0 then
> some janitor or other will just come along and remove it again. :)

Sure. I just thought it was nice to make it clear, because with 
firmware_set_feature() you're _always_ or'ing with the initial value - but 
whatever.

cheers

-- 
Michael Ellerman
IBM OzLabs

email: michael:ellerman.id.au
inmsg: mpe:jabber.org
wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
Url : http://ozlabs.org/pipermail/linuxppc64-dev/attachments/20060112/95bfcec7/attachment.pgp 


More information about the Linuxppc64-dev mailing list