Thread (48 messages) 48 messages, 8 authors, 2018-01-05

[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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help