[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <OS0PR01MB59222A9E3B642799B308379586F3A@OS0PR01MB5922.jpnprd01.prod.outlook.com>
Date: Sun, 10 Sep 2023 07:52:29 +0000
From: Biju Das <biju.das.jz@...renesas.com>
To: Shenghao Ding <shenghao-ding@...com>,
"tiwai@...e.de" <tiwai@...e.de>
CC: "robh+dt@...nel.org" <robh+dt@...nel.org>,
"andriy.shevchenko@...ux.intel.com"
<andriy.shevchenko@...ux.intel.com>,
"lgirdwood@...il.com" <lgirdwood@...il.com>,
"perex@...ex.cz" <perex@...ex.cz>,
"pierre-louis.bossart@...ux.intel.com"
<pierre-louis.bossart@...ux.intel.com>,
"kevin-lu@...com" <kevin-lu@...com>,
"13916275206@....com" <13916275206@....com>,
"alsa-devel@...a-project.org" <alsa-devel@...a-project.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"liam.r.girdwood@...el.com" <liam.r.girdwood@...el.com>,
"mengdong.lin@...el.com" <mengdong.lin@...el.com>,
"baojun.xu@...com" <baojun.xu@...com>,
"thomas.gfeller@...rop.com" <thomas.gfeller@...rop.com>,
"peeyush@...com" <peeyush@...com>, "navada@...com" <navada@...com>,
"broonie@...nel.org" <broonie@...nel.org>,
"gentuser@...il.com" <gentuser@...il.com>
Subject: RE: [PATCH v1] ALSA: hda/tas2781: Support ACPI_ID both TXNW2781 and
TIAS2781
Hi Shenghao Ding,
Thanks for the patch.
> Subject: [PATCH v1] ALSA: hda/tas2781: Support ACPI_ID both TXNW2781 and
> TIAS2781
>
> Support ACPI_ID both TXNW2781 and TIAS2781, TXNW2781 is the only one legal
> ACPI ID, TIAS2781 is the widely-used ACPI ID named by our customers, so far
> it is not registered. We have discussed this with them, they requested
> TIAS2781 must be supported for the laptops already released to market,
> their new laptops will switch to TXNW2781.
>
> Signed-off-by: Shenghao Ding <shenghao-ding@...com>
>
> ---
> Changes in v1:
> - Add TXNW2781 into tas2781_acpi_hda_match and move it to the top
> - Redefine tas2781_generic_fixup, remove hid param
> - TIAS2781 has been used by our customers, see following dstd.dsl. We
> have discussed this with them, they requested TIAS2781 must be
> supported for the laptops already released to market, their new
> laptops will switch to TXNW2781
> Name (_HID, "TIAS2781") // _HID: Hardware ID
> Name (_UID, Zero) // _UID: Unique ID
> Method (_SUB, 0, NotSerialized) // _SUB: Subsystem ID
> {
> If ((SPID == Zero))
> {
> Return ("17AA3886")
> }
>
> If ((SPID == One))
> {
> Return ("17AA3884")
> }
> }
> - Add TXNW2781 support in comp_match_tas2781_dev_name
> ---
> sound/pci/hda/patch_realtek.c | 36 ++++++++++++++++++---------------
> sound/pci/hda/tas2781_hda_i2c.c | 33 ++++++++++++++++++------------
> 2 files changed, 40 insertions(+), 29 deletions(-)
>
> diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
> index b7e78bfcff..6dae58a8ef 100644
> --- a/sound/pci/hda/patch_realtek.c
> +++ b/sound/pci/hda/patch_realtek.c
> @@ -6770,24 +6770,35 @@ static int comp_match_cs35l41_dev_name(struct
> device *dev, void *data)
> return !strcmp(d + n, tmp);
> }
>
> +/* TIAS2781 is the unofficial ACPI id, but widely used in current devices.
> + * TXNW2781 is the official ACPI id, and will be used in the new devices.
> + * Check TIAS2781 or TXNW2781
> + */
> static int comp_match_tas2781_dev_name(struct device *dev,
> void *data)
> {
> - struct scodec_dev_name *p = data;
> + const char c[][10] = { "TXNW2781", "TIAS2781" };
This you should get from match_data().
See below.
> const char *d = dev_name(dev);
> - int n = strlen(p->bus);
> + const char *bus = data;
> + int n = strlen(bus), i;
> char tmp[32];
>
> /* check the bus name */
> - if (strncmp(d, p->bus, n))
> + if (strncmp(d, bus, n))
> return 0;
> /* skip the bus number */
> if (isdigit(d[n]))
> n++;
> - /* the rest must be exact matching */
> - snprintf(tmp, sizeof(tmp), "-%s:00", p->hid);
>
> - return !strcmp(d + n, tmp);
> + for (i = 0; i < ARRAY_SIZE(c); i++) {
> + /* the rest must be exact matching */
> + snprintf(tmp, sizeof(tmp), "-%s:00", c[i]);
> +
> + if (!strcmp(d + n, tmp))
> + return 1;
> + }
> +
> + return 0;
> }
>
> static void cs35l41_generic_fixup(struct hda_codec *cdc, int action, const
> char *bus, @@ -6824,24 +6835,17 @@ static void cs35l41_generic_fixup(struct
> hda_codec *cdc, int action, const char }
>
> static void tas2781_generic_fixup(struct hda_codec *cdc, int action,
> - const char *bus, const char *hid)
> + const char *bus)
> {
> struct device *dev = hda_codec_dev(cdc);
> struct alc_spec *spec = cdc->spec;
> - struct scodec_dev_name *rec;
> int ret;
>
> switch (action) {
> case HDA_FIXUP_ACT_PRE_PROBE:
> - rec = devm_kmalloc(dev, sizeof(*rec), GFP_KERNEL);
> - if (!rec)
> - return;
> - rec->bus = bus;
> - rec->hid = hid;
> - rec->index = 0;
> spec->comps[0].codec = cdc;
> component_match_add(dev, &spec->match,
> - comp_match_tas2781_dev_name, rec);
> + comp_match_tas2781_dev_name, (void *)bus);
> ret = component_master_add_with_match(dev, &comp_master_ops,
> spec->match);
> if (ret)
> @@ -6888,7 +6892,7 @@ static void
> alc287_fixup_legion_16ithg6_speakers(struct hda_codec *cdc, const st
> static void tas2781_fixup_i2c(struct hda_codec *cdc,
> const struct hda_fixup *fix, int action) {
> - tas2781_generic_fixup(cdc, action, "i2c", "TIAS2781");
> + tas2781_generic_fixup(cdc, action, "i2c");
> }
>
> /* for alc295_fixup_hp_top_speakers */
> diff --git a/sound/pci/hda/tas2781_hda_i2c.c
> b/sound/pci/hda/tas2781_hda_i2c.c index fb80280293..8493952305 100644
> --- a/sound/pci/hda/tas2781_hda_i2c.c
> +++ b/sound/pci/hda/tas2781_hda_i2c.c
> @@ -65,6 +65,16 @@ enum calib_data {
> CALIB_MAX
> };
>
> +/* TIAS2781 is the unofficial ACPI id, but widely used in current devices.
> + * TXNW2781 is the official ACPI id, and will be used in the new devices.
> + */
> +static const struct acpi_device_id tas2781_acpi_hda_match[] = {
> + {"TIAS2781", 0 },
> + {"TXNW2781", 1 },
> + {}
> +};
> +MODULE_DEVICE_TABLE(acpi, tas2781_acpi_hda_match);
> +
> static int tas2781_get_i2c_res(struct acpi_resource *ares, void *data) {
> struct tasdevice_priv *tas_priv = data; @@ -644,20 +654,23 @@ static
> void tas2781_hda_remove(struct device *dev) static int
> tas2781_hda_i2c_probe(struct i2c_client *clt) {
> struct tasdevice_priv *tas_priv;
> - const char *device_name;
> - int ret;
> + int ret, i;
>
> - if (strstr(dev_name(&clt->dev), "TIAS2781"))
> - device_name = "TIAS2781";
> - else
> - return -ENODEV;
> + /* Check TIAS2781 or TXNW2781 */
> + for (i = 0; i < ARRAY_SIZE(tas2781_acpi_hda_match); i++)
Why not aviding for loop as it can be retrieved directly
using i2c_get_match_data()?
Update the ACPI/ID table to use pointer to the device_name
as data in the table.
Then,
device_name = i2c_get_match_data(client);
if (!device_name && strstr(dev_name(&clt->dev), device_name)))
return dev_err_probe(tas_priv->dev, -ENODEV,
"Device not available\n");
Cheers,
Biju
> +static const struct acpi_device_id tas2781_acpi_hda_match[] = {
> + {"TIAS2781", 0 },
> + {"TXNW2781", 1 },
> + {}
> +};
> + if (strstr(dev_name(&clt->dev), tas2781_acpi_hda_match[i].id))
> + break;
> +
> + if (i == ARRAY_SIZE(tas2781_acpi_hda_match))
> + return dev_err_probe(tas_priv->dev, -ENODEV,
> + "Device not available\n");
>
> tas_priv = tasdevice_kzalloc(clt);
> if (!tas_priv)
> return -ENOMEM;
>
> tas_priv->irq_info.irq = clt->irq;
> - ret = tas2781_read_acpi(tas_priv, device_name);
> + ret = tas2781_read_acpi(tas_priv, tas2781_acpi_hda_match[i].id);
> if (ret)
> return dev_err_probe(tas_priv->dev, ret,
> "Platform not supported\n");
> @@ -822,12 +835,6 @@ static const struct i2c_device_id tas2781_hda_i2c_id[]
> = {
> {}
> };
>
> -static const struct acpi_device_id tas2781_acpi_hda_match[] = {
> - {"TIAS2781", 0 },
> - {}
> -};
> -MODULE_DEVICE_TABLE(acpi, tas2781_acpi_hda_match);
> -
> static struct i2c_driver tas2781_hda_i2c_driver = {
> .driver = {
> .name = "tas2781-hda",
> --
> 2.34.1
Powered by blists - more mailing lists