| Message ID | 20240430072544.1877-2-baojun.xu@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ALSA: hda/tas2781: Add tas2781 driver for SPI. | expand |
Hi Andy Thanks for your comments, answer in line: > From: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Sent: 30 April 2024 21:45 > To: Takashi Iwai > Cc: Xu, Baojun; robh+dt@kernel.org; lgirdwood@gmail.com; perex@perex.cz; pierre-louis.bossart@linux.intel.com; Lu, Kevin; Ding, Shenghao; Navada Kanyana, Mukund; 13916275206@139.com; P O, Vijeth; Holalu Yogendra, Niranjan; alsa-devel@alsa-project.org; linux-kernel@vger.kernel.org; liam.r.girdwood@intel.com; yung-chuan.liao@linux.intel.com; broonie@kernel.org; soyer@irl.hu > Subject: [EXTERNAL] Re: [PATCH v4 1/3] ALSA: hda/tas2781: Add tas2781 hda driver based on SPI > > On Tue, Apr 30, 2024 at 02: 58: 10PM +0200, Takashi Iwai wrote: > On Tue, 30 Apr 2024 09: 25: 42 +0200, Baojun Xu wrote: .. . > > snd-hda-cs-dsp-ctls-objs := hda_cs_dsp_ctl. o > > snd-hda-scodec-component-objs := hda_component. o > > ZjQcmQRYFpfptBannerStart > This message was sent from outside of Texas Instruments. > Do not click links or open attachments unless you recognize the source of this email and know the content is safe. If you wish to report this message to IT Security, please forward the message as an attachment to phishing@list.ti.com > > ZjQcmQRYFpfptBannerEnd > > On Tue, Apr 30, 2024 at 02:58:10PM +0200, Takashi Iwai wrote: > > On Tue, 30 Apr 2024 09:25:42 +0200, Baojun Xu wrote: > > ... > > > > snd-hda-cs-dsp-ctls-objs := hda_cs_dsp_ctl.o > > > snd-hda-scodec-component-objs := hda_component.o > > > snd-hda-scodec-tas2781-i2c-objs := tas2781_hda_i2c.o > > > +snd-hda-scodec-tas2781-spi-y := tas2781_hda_spi.o tas2781_spi_fwlib.o > > > > A nitpicking: better to align with other lines (i.e. with *-objs > > instead of *-y). > > I object this. The better approach is to have a precursor patch that switches > to y over objs (the latter is for user space code / tools). I also do not fully understand why set it as "y", as you know, I follow CONFIG_SND_HDA_SCODEC_TAS2781_I2C, as it do not set to "y". > > -- > With Best Regards, > Andy Shevchenko > > >
On Mon, May 06, 2024 at 07:44:41AM +0000, Xu, Baojun wrote: > > From: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > Sent: 30 April 2024 21:45 > > On Tue, Apr 30, 2024 at 02: 58: 10PM +0200, Takashi Iwai wrote: > > On Tue, Apr 30, 2024 at 02:58:10PM +0200, Takashi Iwai wrote: > > > On Tue, 30 Apr 2024 09:25:42 +0200, Baojun Xu wrote: ... > > > > snd-hda-cs-dsp-ctls-objs := hda_cs_dsp_ctl.o > > > > snd-hda-scodec-component-objs := hda_component.o > > > > snd-hda-scodec-tas2781-i2c-objs := tas2781_hda_i2c.o > > > > +snd-hda-scodec-tas2781-spi-y := tas2781_hda_spi.o tas2781_spi_fwlib.o > > > > > > A nitpicking: better to align with other lines (i.e. with *-objs > > > instead of *-y). > > > > I object this. The better approach is to have a precursor patch that switches > > to y over objs (the latter is for user space code / tools). > > I also do not fully understand why set it as "y", as you know, I follow > CONFIG_SND_HDA_SCODEC_TAS2781_I2C, as it do not set to "y". While you see no side effects, the 'objs' is reserved for user space, while 'y' should be used in the kernel code. See Documentation/kbuild/makefiles.rst "Composite Host Programs", mind the word "host" meaning.
On Tue, 30 Apr 2024 15:45:28 +0200, Andy Shevchenko wrote: > > On Tue, Apr 30, 2024 at 02:58:10PM +0200, Takashi Iwai wrote: > > On Tue, 30 Apr 2024 09:25:42 +0200, Baojun Xu wrote: > > ... > > > > snd-hda-cs-dsp-ctls-objs := hda_cs_dsp_ctl.o > > > snd-hda-scodec-component-objs := hda_component.o > > > snd-hda-scodec-tas2781-i2c-objs := tas2781_hda_i2c.o > > > +snd-hda-scodec-tas2781-spi-y := tas2781_hda_spi.o tas2781_spi_fwlib.o > > > > A nitpicking: better to align with other lines (i.e. with *-objs > > instead of *-y). > > I object this. The better approach is to have a precursor patch that switches > to y over objs (the latter is for user space code / tools). Indeed it can be a good cleanup, yeah. Let me try. thanks, Takashi
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index d1464324de95..51af181ccf62 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -1765,6 +1765,8 @@ static bool acpi_device_enumeration_by_parent(struct acpi_device *device) {"CSC3557", }, {"INT33FE", }, {"INT3515", }, + {"TXNW2781", }, + {"TIAS2781", }, /* Non-conforming _HID for Cirrus Logic already released */ {"CLSA0100", }, {"CLSA0101", }, diff --git a/drivers/platform/x86/serial-multi-instantiate.c b/drivers/platform/x86/serial-multi-instantiate.c index 97b9c6392230..d1c766f17b26 100644 --- a/drivers/platform/x86/serial-multi-instantiate.c +++ b/drivers/platform/x86/serial-multi-instantiate.c @@ -368,6 +368,17 @@ static const struct smi_node cs35l57_hda = { .bus_type = SMI_AUTO_DETECT, }; +static const struct smi_node tas2781_hda = { + .instances = { + { "tas2781-hda", IRQ_RESOURCE_AUTO, 0 }, + { "tas2781-hda", IRQ_RESOURCE_AUTO, 0 }, + { "tas2781-hda", IRQ_RESOURCE_AUTO, 0 }, + { "tas2781-hda", IRQ_RESOURCE_AUTO, 0 }, + {} + }, + .bus_type = SMI_AUTO_DETECT, +}; + /* * Note new device-ids must also be added to ignore_serial_bus_ids in * drivers/acpi/scan.c: acpi_device_enumeration_by_parent(). @@ -380,6 +391,8 @@ static const struct acpi_device_id smi_acpi_ids[] = { { "CSC3556", (unsigned long)&cs35l56_hda }, { "CSC3557", (unsigned long)&cs35l57_hda }, { "INT3515", (unsigned long)&int3515_data }, + { "TXNW2781", (unsigned long)&tas2781_hda }, + { "TIAS2781", (unsigned long)&tas2781_hda }, /* Non-conforming _HID for Cirrus Logic already released */ { "CLSA0100", (unsigned long)&cs35l41_hda }, { "CLSA0101", (unsigned long)&cs35l41_hda }, diff --git a/sound/pci/hda/Kconfig b/sound/pci/hda/Kconfig index f806636242ee..15f0e66b77e5 100644 --- a/sound/pci/hda/Kconfig +++ b/sound/pci/hda/Kconfig @@ -202,6 +202,20 @@ config SND_HDA_SCODEC_TAS2781_I2C comment "Set to Y if you want auto-loading the side codec driver" depends on SND_HDA=y && SND_HDA_SCODEC_TAS2781_I2C=m +config SND_HDA_SCODEC_TAS2781_SPI + tristate "Build TAS2781 HD-audio side codec support for SPI Bus" + depends on SPI_MASTER + depends on ACPI + depends on EFI + depends on SND_SOC + select CRC32_SARWATE + help + Say Y or M here to include TAS2781 SPI HD-audio side codec support + in snd-hda-intel driver, such as ALC287. + +comment "Set to Y if you want auto-loading the side codec driver" + depends on SND_HDA=y && SND_HDA_SCODEC_TAS2781_SPI=m + config SND_HDA_CODEC_REALTEK tristate "Build Realtek HD-audio codec support" select SND_HDA_GENERIC diff --git a/sound/pci/hda/Makefile b/sound/pci/hda/Makefile index 13e04e1f65de..2d5d4d841d87 100644 --- a/sound/pci/hda/Makefile +++ b/sound/pci/hda/Makefile @@ -39,6 +39,7 @@ snd-hda-scodec-cs35l56-spi-objs := cs35l56_hda_spi.o snd-hda-cs-dsp-ctls-objs := hda_cs_dsp_ctl.o snd-hda-scodec-component-objs := hda_component.o snd-hda-scodec-tas2781-i2c-objs := tas2781_hda_i2c.o +snd-hda-scodec-tas2781-spi-y := tas2781_hda_spi.o tas2781_spi_fwlib.o # common driver obj-$(CONFIG_SND_HDA) := snd-hda-codec.o @@ -70,6 +71,7 @@ obj-$(CONFIG_SND_HDA_SCODEC_CS35L56_SPI) += snd-hda-scodec-cs35l56-spi.o obj-$(CONFIG_SND_HDA_CS_DSP_CONTROLS) += snd-hda-cs-dsp-ctls.o obj-$(CONFIG_SND_HDA_SCODEC_COMPONENT) += snd-hda-scodec-component.o obj-$(CONFIG_SND_HDA_SCODEC_TAS2781_I2C) += snd-hda-scodec-tas2781-i2c.o +obj-$(CONFIG_SND_HDA_SCODEC_TAS2781_SPI) += snd-hda-scodec-tas2781-spi.o # this must be the last entry after codec drivers; # otherwise the codec patches won't be hooked before the PCI probe diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index 70d80b6af3fe..1070a1c0edec 100644 --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -6913,6 +6913,11 @@ static void tas2781_fixup_i2c(struct hda_codec *cdc, comp_generic_fixup(cdc, action, "i2c", "TIAS2781", "-%s:00", 1); } +static void tas2781_fixup_spi_two(struct hda_codec *cdc, const struct hda_fixup *fix, int action) +{ + comp_generic_fixup(cdc, action, "spi", "TXNW2781", "-%s:00-tas2781-hda.%d", 2); +} + static void yoga7_14arb7_fixup_i2c(struct hda_codec *cdc, const struct hda_fixup *fix, int action) { @@ -7451,6 +7456,7 @@ enum { ALC236_FIXUP_DELL_DUAL_CODECS, ALC287_FIXUP_CS35L41_I2C_2_THINKPAD_ACPI, ALC287_FIXUP_TAS2781_I2C, + ALC245_FIXUP_TAS2781_SPI_2_HP_GPIO_LED, ALC287_FIXUP_YOGA7_14ARB7_I2C, ALC245_FIXUP_HP_MUTE_LED_COEFBIT, ALC245_FIXUP_HP_X360_MUTE_LEDS, @@ -9618,6 +9624,12 @@ static const struct hda_fixup alc269_fixups[] = { .chained = true, .chain_id = ALC285_FIXUP_THINKPAD_HEADSET_JACK, }, + [ALC245_FIXUP_TAS2781_SPI_2_HP_GPIO_LED] = { + .type = HDA_FIXUP_FUNC, + .v.func = tas2781_fixup_spi_two, + .chained = true, + .chain_id = ALC285_FIXUP_HP_GPIO_LED, + }, [ALC287_FIXUP_YOGA7_14ARB7_I2C] = { .type = HDA_FIXUP_FUNC, .v.func = yoga7_14arb7_fixup_i2c, @@ -10074,6 +10086,7 @@ static const struct snd_pci_quirk alc269_fixup_tbl[] = { SND_PCI_QUIRK(0x103c, 0x8b8d, "HP", ALC236_FIXUP_HP_GPIO_LED), SND_PCI_QUIRK(0x103c, 0x8b8f, "HP", ALC245_FIXUP_CS35L41_SPI_4_HP_GPIO_LED), SND_PCI_QUIRK(0x103c, 0x8b92, "HP", ALC245_FIXUP_CS35L41_SPI_2_HP_GPIO_LED), + SND_PCI_QUIRK(0x103c, 0x8b93, "HP", ALC245_FIXUP_TAS2781_SPI_2_HP_GPIO_LED), SND_PCI_QUIRK(0x103c, 0x8b96, "HP", ALC236_FIXUP_HP_MUTE_LED_MICMUTE_VREF), SND_PCI_QUIRK(0x103c, 0x8b97, "HP", ALC236_FIXUP_HP_MUTE_LED_MICMUTE_VREF), SND_PCI_QUIRK(0x103c, 0x8bdd, "HP Envy 17", ALC287_FIXUP_CS35L41_I2C_2),
Integrate tas2781 hda spi driver configs for HP (Varcolac). Every tas2781 SPI node was added by serial-multi-instantie.c as a SPI device. The code support Realtek as the primary codec. Signed-off-by: Baojun Xu <baojun.xu@ti.com> --- v4: - Add old hardware id "TIAS2781" for compatible with old production - Add 2 devices in struct smi_node tas2781_hda, to compatible with 4 AMPs v3: - Move HID up to above /* Non-conforming _HID ... */ in scan.c, for avoid misunderstanding. - Move HID up to above /* Non-conforming _HID ... */ in serial-multi-instantiate.c, for avoid misunderstanding. - Change objs to y for snd-hda-scodec-tas2781-spi- in Makefile. --- drivers/acpi/scan.c | 2 ++ drivers/platform/x86/serial-multi-instantiate.c | 13 +++++++++++++ sound/pci/hda/Kconfig | 14 ++++++++++++++ sound/pci/hda/Makefile | 2 ++ sound/pci/hda/patch_realtek.c | 13 +++++++++++++ 5 files changed, 44 insertions(+)