[lkml]   [2017]   [Nov]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [PATCH v4 5/9] drivers: base: cacheinfo: arm64: Add support for ACPI based firmware tables
On 11/20/2017 10:56 AM, Sudeep Holla wrote:


>> * case there's no explicit cache node or the cache node itself in the
>> * device tree
>> + * @firmware_node: Shared with of_node. When not using DT, this may contain
>> + * pointers to other firmware based values. Particularly ACPI/PPTT
>> + * unique values.
>> * @disable_sysfs: indicates whether this node is visible to the user via
>> * sysfs or not
>> * @priv: pointer to any private data structure specific to particular
>> @@ -64,8 +67,10 @@ struct cacheinfo {
>> #define CACHE_ID BIT(4)
>> -
>> - struct device_node *of_node;
>> + union {
>> + struct device_node *of_node;
>> + void *firmware_node;
>> + };
> I would prefer
> struct device_node *of_node;
> changed to
> struct fwnode_handle *fwnode;
> You can then have
> struct pptt_fwnode {
> <.....>
> /*below fwnode allocated using acpi_alloc_fwnode_static */
> struct fwnode_handle *fwnode;
> };
> This gives a good starting point to abstract DT and ACPI.
> If not now, we can later implement fwnode.ops=pptt_cache_ops and then
> use get property for both DT and ACPI.

I'm obviously confused why this keeps coming up. On the surface it
sounds like a good idea. But then, given that I've actually implemented
a portion of it, what becomes clear is that the PPTT isn't a good match.
Converting the OF routines to use the fwnode is fairly straightforward,
but that doesn't help the ACPI situation other than to create a lot of
misleading code (and the possibility of creating nonstandard DSDT
entries). The fact that this hasn't been done for other tables
MADT/SLIT/SRAT/etc makes me wonder why we should do it for the PPTT?

Particularly, when one considers fwnode is more a DSDT<->DT abstraction
and thus has a lot of API surface that simply doesn't make any sense
given the PPTT binary tree structure. Given that most of the fwnode
routines are translating string properties (for example
fwnode_property_read_string()) it might be possible to build a
translator of some form which takes DT style properties and attempts to
map them to the ACPI PPTT tree. What this adds I can't fathom, beyond
the fact that suddenly the fwnode interface is a partial/brittle
implementation where a large subset of the fwnode_operations will tend
to be degenerate cases. The result likely will be a poorly implemented
translator which breaks or is meaningless over a large part of the
fwnode API surface.

 \ /
  Last update: 2017-11-20 19:03    [W:0.049 / U:4.940 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site