[PATCH v5 4/9] drivers: base cacheinfo: Add support for ACPI based firmware tables
From: Jeremy Linton <hidden>
Date: 2017-12-12 23:37:59
Also in:
linux-acpi, linux-pm, lkml
On 12/12/2017 05:02 PM, Rafael J. Wysocki wrote:
On Tue, Dec 12, 2017 at 11:55 PM, Jeremy Linton [off-list ref] wrote:quoted
Hi, On 12/12/2017 11:25 AM, Rafael J. Wysocki wrote:quoted
[cut]
(trimming list)
quoted
quoted
quoted
quoted
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.quoted
quoted
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.
I sort of agree, I think I can just change the whole of_node to a generic 'void *firmware_unique' which works fine for the PPTT code, it should also work for the DT code in cache_leaves_are_shared(). The slight gocha is there is a bit of DT code which initially runs earlier that uses of_node as an indirect parameter to a couple functions (by just passing the cacheinfo). Let me see if I can tweak that a bit. Frankly, If I understood completely all the *priv cases I suspect it might be possible to collapse *of_node into that as well. That is as long as no one decides to flush out DT on x86, or PPTT on x86.