Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <12102f7419c5e44cd2133aa769e25dbd16f4e0c9.camel@codux.tech>
Subject: Re: CSC3551 and devices missing related _DSD bits
From:   Armas Spann <armas@codux.tech>
To:     Luke Jones <luke@ljones.dev>,
        Stuart Henderson <stuarth@opensource.cirrus.com>,
        Takashi Iwai <tiwai@suse.de>
Cc:     linux-kernel@vger.kernel.org, tiwai@suse.com,
        sbinding@opensource.cirrus.com, perex@perex.cz,
        tangmeng@uniontech.com, andy.chi@canonical.com, p.jungkamp@gmx.net,
        kasper93@gmail.com, yangyuchi66@gmail.com, ealex95@gmail.com,
        james.schulman@cirrus.com, david.rhodes@cirrus.com,
        tanureal@opensource.cirrus.com, rf@opensource.cirrus.com,
        patches@opensource.cirrus.com, alsa-devel@alsa-project.org
Date:   Sun, 28 May 2023 02:02:46 +0200
In-Reply-To: <19d69a5da8f1d4017ac14ed87f927ab82278073d.camel@ljones.dev>
References: <1991650.PYKUYFuaPT@fedora> <87jzx3zaf8.wl-tiwai@suse.de>
         <b4c202b2-ab29-e2aa-b141-0c967b2c1645@opensource.cirrus.com>
         <19d69a5da8f1d4017ac14ed87f927ab82278073d.camel@ljones.dev>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.48.1 
MIME-Version: 1.0
Precedence: bulk

Hi Takashi, Hi Stuart (and of course, all others in here),

would you mind to evaluate this small (pseudo-)patch to be harmless?=20
(concerning the blow-up theory the first answer in this converstion)

I won't push it upstream right now but I want to know if this patch
might be harmfull. I'm owning a GA402XY myself and we digged out that
the initial setting of the cr3551 can be done via:

diff --git a/sound/pci/hda/cs35l41_hda.c b/sound/pci/hda/cs35l41_hda.c
index 75020edd39e7..eaa06751bd48 100644
--- a/sound/pci/hda/cs35l41_hda.c
+++ b/sound/pci/hda/cs35l41_hda.c
@@ -1243,6 +1243,12 @@ static int cs35l41_no_acpi_dsd(struct
cs35l41_hda *cs35l41, struct device *physd
 		hw_cfg->bst_type =3D CS35L41_EXT_BOOST;
 		hw_cfg->gpio1.func =3D CS35l41_VSPK_SWITCH;
 		hw_cfg->gpio1.valid =3D true;
+	} else if (strncmp(hid, "CSC3551", 7) =3D=3D 0 && strcmp(cs35l41-
>acpi_subsystem_id, "10431463") =3D=3D 0) {
+		// TESTING - (Hook for GA402X)
+		dev_warn(cs35l41->dev, "Warning: ASUS didn't provide
the needed ACPI _DSD properties for GA402X series, using defaults..");
+		hw_cfg->bst_type =3D CS35L41_EXT_BOOST;
+		hw_cfg->gpio1.func =3D CS35l41_VSPK_SWITCH;
+		hw_cfg->gpio1.valid =3D true;
 	} else {
 		/*
 		 * Note: CLSA010(0/1) are special cases which use a
slightly different design.

--

Which for our devices(GA402XY) enables the DAC to be used (it's still
quiet, as we don't know/set the right limits for boost/ind/cap at the
moment).

The above will be called in our HDA_Quirk
(sound/pci/hda/patch_realtek.c)

```pseudo
	[ALC285_FIXUP_ASUS_GA402XY] =3D {
		.type =3D HDA_FIXUP_FUNC,
		.v.func =3D cs35l41_fixup_i2c_two,
		// ....
	},
```

The cs3551 init be loaded by the above quirk wich is bound to and
checks its ID internally again(acpi_subsystem_id):

```pseudo
SND_PCI_QUIRK(0x1043, 0x1463, "Asus Zephyrus G14 2023",
ALC285_FIXUP_ASUS_GA402XY),
```


Many thanks in advance!

Best regards
Armas


On Thu, 2023-05-25 at 09:30 +1200, Luke Jones wrote:
> On Wed, 2023-05-24 at 17:36 +0100, Stuart Henderson wrote:
> >=20
> > > The problem is that this can really easily blow up your machine
> > > if
> > > some incorrect bit is applied.=C2=A0 And more easily applicable, more
> > > chance to break by novice users, simply by believing what a chat
> > > bot
> > > speaks :)
> > > That's the very reason why this kind of change should be via ACPI
> > > table officially set up by the vendor.=C2=A0 That said, the question
> > > is
> > > only who and how can be responsible for this kind of change.=C2=A0
> > > It's
> > > no technical issue, per se.
> > >=20
> > > If BIOS can't be updated, at least, the configuration change has
> > > to
> > > be
> > > confirmed by ASUS people.=C2=A0 If ASUS still ignores the inquires an=
d
> > > requests, we may put the quirk but with a bit fat warning (and
> > > maybe
> > > complaints to ASUS) to be shown in the log as a very last resort.
> > >=20
> > > Let's see what happens.
> >=20
> > Thanks Takashi.
> >=20
> > Just a note to say we're not ignoring this and are investigating
> > the=20
> > best way to support released laptops with ACPI incompatible with
> > Linux.=C2=A0=20
> > We're hoping this is going to be less of an issue going forward.=C2=A0
> > Please=20
> > bear with us while we look into this.
> >=20
>=20
> This is great news, thank you Stuart. If you need testing done at all
> on a wide range please reach out to me and I will enlist the help of
> those with the affected laptops I mentioned.