On 2023-08-09 7:00 AM, andy.shevchenko@xxxxxxxxx wrote:
Fri, Jul 21, 2023 at 05:48:10PM +0200, Cezary Rojewski kirjoitti:
Device configuration structures are plenty so declare a struct for each
known variant. As neither of them shall be accessed without verifying
the memory block first, introduce macros to make it easy to do so.
Link: https://github.com/acpica/acpica/pull/881
Thinking of this over night (as I replied in the above)...
Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
Sorry, but seems I have to retract my tag and even more, NAK to the ACPICA changes.
I have thought that this is something new to the header there, but it appears that
it duplicates (in a wrong way in my opinion) existing data types.
Existing data types are crafted (as far as I get them) in a way to be able to be
combined in the union. In the similar way how _CRS is parsed in DSDT (first that
comes to my mind). Hence that "simplification" is quite wrong in a few ways:
- it breaks ACPICA agreement on naming schema
- it duplicates existing data
- it made it even partially
- it is fine and correct in ACPICA to have long dereferenced data, again see
for the union of acpi_object
I trully believe now that the above change in ACPICA must be reverted.
Again, sorry for this late bad news from my side. I have no clue why
it was merged, perhaps lack of review? Or anything subtle I so miserably
missed?
First, you took the review seriously and provided a ton of valid
feedback. And your reviews and expertise helped me grow as a developer,
so from my perspective no need to sorry about spotting bad things late.
Now, I admit, a bit surprised given the number of revisions and age of
the initial patchset. The cover-letter, attached for each revision, made
the intentions clear. Our goal is to help actual users of NHLT i.e.:
audio teams. While part of ACPICA, NHLT-code is hidden within sound/ so
no one asks questions. Leaving things at status quo does not improve the
situation. Thus I believe simple "no" is not an option here. To make the
code better overall, relevant pieces should be made part of drivers/acpi.
Original problems stem from the fact that audio teams were not looped in
during initial integration of NHLT-code. Turned out that no users
utilize it in its current form. The problems are subtle, but a
discussion wouldn't hurt.
To avoid double posting, should we continue the discussion here or in
the PR on github?
Kind regards,
Czarek
