Re: [PATCH v5 1/4] ACPI: NHLT: Reintroduce types the table consists of

[Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>]

On 3/6/24 10:17, Cezary Rojewski wrote:
> On 2024-03-04 9:46 PM, Pierre-Louis Bossart wrote:
>> On 3/4/24 14:34, Cezary Rojewski wrote:
>>> On 2024-03-04 9:22 PM, Pierre-Louis Bossart wrote:
>>>> On 3/4/24 13:49, Andy Shevchenko wrote:
>>>>> On Mon, Mar 04, 2024 at 10:57:39AM -0600, Pierre-Louis Bossart wrote:
>>>>>>> +/* Values for link_type field above */
>>>>>>> +
>>>>>>> +#define ACPI_NHLT_LINKTYPE_HDA            0
>>>>>>> +#define ACPI_NHLT_LINKTYPE_DSP            1
>>>>>>> +#define ACPI_NHLT_LINKTYPE_PDM            2
>>>>>>> +#define ACPI_NHLT_LINKTYPE_SSP            3
>>>>>>> +#define ACPI_NHLT_LINKTYPE_SLIMBUS        4
>>>>>>> +#define ACPI_NHLT_LINKTYPE_SDW            5
>>>>>>> +#define ACPI_NHLT_LINKTYPE_UAOL            6
>>>>>>
>>>>>> More than half of those values are not used. Is there really any
>>>>>> benefit
>>>>>> in exposing them?
>>>>>
>>>>> Sometimes a code is the (only) documentation. Since it's a global
>>>>> header and
>>>>> part of ACPICA we probably better to expose all bits that are defined.
>>>>
>>>> NHLT is an Intel-only solution - no other company uses it.
>>>> Intel does not have any designs where SlimBus is productized.
>>>>
>>>> I fail to see the wisdom of exposing a non-existent option with
>>>> LINKTYPE_SLIMBUS. It's not because this case was listed in a document
>>>> that we have to add the information verbatim in a open-source header.
>>>>
>>>> Likewise for SoundWire we do NOT use NHLT at all...
>>>>
>>>> Options 4 and 5 are completely irrelevant. 0 and 1 most likely as well.
>>>
>>> Hello,
>>>
>>> How relevant or not given field is in LINKTYPE enumeration is..
>>> irrelevant. Those values are reserved since the dawn of the table.
>>> Renaming those with range of RESERVED_X(s) is hardly an alternative. On
>>> top of that, specs which have been publicly shared since 2016 _do_ list
>>> the non-I2S/PDW constants when describing LINKTYPE.
>>
>> I maintain that all those values, while spec-defined, should be treated
>> as not supported. It's not unusual in engineering to change directions
>> and back-annotate, demote or cleanup initial designs. Change is the only
>> constant.
> 
> What's the proposal here? Would comment suffice or there is something
> else you have in mind?

I would be fine with a comment along the lines of 'defined in spec, not
used' and 'used on all SKL+ platforms'.