Re: [EXTERNAL] Re: [PATCH v1 2/2] ASoC: tas2781: Add tas5832 support

From: Krzysztof Kozlowski

Date: Tue Mar 17 2026 - 03:07:00 EST


On 17/03/2026 07:05, Ding, Shenghao wrote:
>>>>>
>>>>>>>>>>>>> static const struct of_device_id tasdevice_of_match[] = {
>>>>>
>>>>>>>>>>>>> { .compatible = "ti,tas5827" },
>>>>>>>>>>>>> { .compatible = "ti,tas5828" },
>>>>>>>>>>>>> { .compatible = "ti,tas5830" },
>>>>>>>>>>>>> + { .compatible = "ti,tas5832" },
>>>>>>>>>>>>
>>> ...
>>>>>
>>>>> I don't know what you are trying to get from them, but I²C
>>>>> enumeration on DT platforms works in a way that it takes two tables
>>>>> into account, hence, if there is no compatible (with given part
>>>>> number) there will be no matching name.
>>>>>
>>>>> AFAIK it has to have a compatible to make it work in such a case.
>>>>> What did I miss?
>>>>
>>>> Show me please then how the OF table is used here at all to get the name.
>>>
>>> This code will also be used for ACPI devices. To ensure consistent bin
>>> file naming between ACPI and DTS devices, the mapping table
>>> tasdevice_id[] has been introduced.
>>
>> We need to stop discussing with irrelevant arguments, really.
>>
>> I asked to show me how this is going to be used in DT. No answer to this part.
>>
>> Now you claim this OF ID will be used for ACPI devices, but YOU HAVE already
>> ACPI table there, so that's clearly invalid argument. Do you understand how
>> patch/email review works? Where the comments appear and what is being
>> discussed?
>>
>>> The code retrieves the corresponding name of the chip from
>>> tasdevice_id[] based on its order in tasdevice_of_match[] or
>> tasdevice_acpi_match[].
>>
>> Based on the order in table? No, that's crazy buggy solution to tie order of
>> entries in both tables. And it makes no sense... and I am sure code does not
>> do it, so again irrelevant argument.
>
> It should be noted that the current implementation relies on id_table

OF ID table is the ID table which we talk here about. Please read again
where do the comments appear, which part is being discussed.

> (rather than OF_ID) to obtain the chip name and chip ID. The chip ID i
> s specifically reserved for handling exceptional cases, for example,
> the workaround for the TAS2781’s lower speaker impedance issue.
>
> Since the 58xx family lacks a dedicated register to query the chip ID,
> both the supported chip ID and name are stored in id_table.
> This approach allows compatibility with both DT-based (Device Tree)
> and ACPI-based (Advanced Configuration and Power Interface) device.
>
> if (ACPI_HANDLE(&i2c->dev)) {
> acpi_id = acpi_match_device(i2c->dev.driver->acpi_match_table,
> &i2c->dev);
> if (!acpi_id) {
> dev_err(&i2c->dev, "No driver data\n");
> ret = -EINVAL;
> goto err;
> }
> tas_priv->chip_id = acpi_id->driver_data;
> tas_priv->isacpi = true;
> } else {
> tas_priv->chip_id = (uintptr_t)i2c_get_match_data(i2c);
> tas_priv->isacpi = false;
> }

I do not see the point you are making. All last replies felt off topic
to me, so just to be sure - do you understand how i2c_get_match_data()
works? That it calls device_get_match_data() which will return what?
What exactly?

Best regards,
Krzysztof