Messages in this thread | | | From | "Rafael J. Wysocki" <> | Date | Wed, 13 Dec 2017 00:02:58 +0100 | Subject | Re: [PATCH v5 4/9] drivers: base cacheinfo: Add support for ACPI based firmware tables |
| |
On Tue, Dec 12, 2017 at 11:55 PM, Jeremy Linton <jeremy.linton@arm.com> wrote: > Hi, > > > On 12/12/2017 11:25 AM, Rafael J. Wysocki wrote: >>
[cut]
>>>> >>>> >>>> >>>> What about converting this to using struct fwnode instead of adding >>>> fields to it? >>> >>> >>> >>> I didn't really want to add another field here, but I've also pointed out >>> how I thought converting it to a fwnode wasn't a good choice. >>> >>> https://lkml.org/lkml/2017/11/20/502 >>> >>> Mostly because IMHO its even more misleading (lacking any >>> fwnode_operations) >>> than misusing the of_node as a void *. >> >> >> I'm not sure what you mean. > > > Converting the DT drivers/cacheinfo.c code to use a fwnode_handle is > straightforward. But IMHO it doesn't solve the readability problem of either > casting the ACPI/PPTT token directly to the resulting fwnode_handle *, or > alternatively an actual fwnode_handle with bogus fwnode_operations to wrap > that token.
I'm not talking about that at all.
>> >> Anyway, the idea is to have one pointer in there instead of two that >> cannot be used at the same time and there's no reason why of_node >> should be special. > > > Avoid two pointers for size, or readability? Because the last > version had a union with of_node, which isn't strictly necessary as I can > just cast the pptt token to of_node. There is exactly one line of code after > that which uses the token and it doesn't care about type.
So call this field "token" or similar. Don't call it "of_node" and don't introduce another "firmware_node" thing in addition to that. That just is a mess, sorry.
Thanks, Rafael
| |