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 <[email protected]>
---
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(+)
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),
--
2.40.1
On Tue, 30 Apr 2024 09:25:42 +0200,
Baojun Xu wrote:
>
> 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 <[email protected]>
>
> ---
> 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(+)
>
> 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
A nitpicking: better to align with other lines (i.e. with *-objs
instead of *-y).
The main problem here is, though, that this commit will break the
build, because you introduced a Kconfig that can be enabled, while the
corresponding code for snd-hda-scodec-tas2781-spi isn't present yet.
This is bad for the git-bisection.
You'd need to reorganize better how the code is added piece-by-piece.
thanks,
Takashi
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).
--
With Best Regards,
Andy Shevchenko
On Tue, Apr 30, 2024 at 03:25:42PM +0800, Baojun Xu wrote:
> 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.
..
> + {"TIAS2781", },
Is this for real? I mean you really abused ACPI specifications to get this into
the real products on the market?! If so, never do this again.
_This_ one is non-conforming abuse of ACPI specification and has not to be here
at all. But, if the above is the case, provide the models of the devices and
the excerpt of the DSDT where this broken ID happen, also add a big comment
explaining how comes this happened and that you promise to never happen this
again.
..
> + { "TIAS2781", (unsigned long)&tas2781_hda },
Ditto.
..
> +config SND_HDA_SCODEC_TAS2781_SPI
> + tristate "Build TAS2781 HD-audio side codec support for SPI Bus"
> + depends on SPI_MASTER
> + depends on ACPI
No compile test?
> + 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.
--
With Best Regards,
Andy Shevchenko
Hi Andy
Thanks for your comments, answer in line:
> From: Andy Shevchenko <[email protected]>
> Sent: 30 April 2024 21:45
> To: Takashi Iwai
> Cc: Xu, Baojun; [email protected]; [email protected]; [email protected]; [email protected]; Lu, Kevin; Ding, Shenghao; Navada Kanyana, Mukund; [email protected]; P O, Vijeth; Holalu Yogendra, Niranjan; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
> 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 [email protected]
>
> 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 <[email protected]>
> > 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.
--
With Best Regards,
Andy Shevchenko
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