Received-SPF: pass (google.com: domain of linux-kernel+bounces-109378-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Feedback-ID: i5ec1447f:Fastmail
User-Agent: Cyrus-JMAP/3.11.0-alpha0-332-gdeb4194079-fm-20240319.002-gdeb41940
Precedence: bulk
MIME-Version: 1.0
Message-Id: <e04bf262-e024-488a-ade4-698844af20e3@app.fastmail.com>
In-Reply-To: <bdcc3052-1403-8c2f-f703-66180394c461@linux.intel.com>
References: <20240310061715.16531-1-luke@ljones.dev>
 <5f853562-cbe0-32d7-2644-d42d2bb9e060@linux.intel.com>
 <7276ec7b-9b00-4241-a4eb-a8997daeba5c@app.fastmail.com>
 <bdcc3052-1403-8c2f-f703-66180394c461@linux.intel.com>
Date: Thu, 21 Mar 2024 09:05:21 +1300
From: "Luke Jones" <luke@ljones.dev>
To: =?UTF-8?Q?Ilpo_J=C3=A4rvinen?= <ilpo.jarvinen@linux.intel.com>
Cc: platform-driver-x86@vger.kernel.org,
 "Hans de Goede" <hdegoede@redhat.com>, LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] platform/x86: asus-wmi: support toggling POST sound
Content-Type: text/plain;charset=utf-8
Content-Transfer-Encoding: quoted-printable

On Thu, 21 Mar 2024, at 12:55 AM, Ilpo J=C3=A4rvinen wrote:
> On Wed, 20 Mar 2024, Luke Jones wrote:
> > On Wed, 20 Mar 2024, at 6:48 AM, Ilpo J=C3=A4rvinen wrote:
> > > On Sun, 10 Mar 2024, Luke D. Jones wrote:
> > >=20
> > > > Add support for toggling the BIOS POST sound on some ASUS laptop=
s.
> > > >=20
> > > > Signed-off-by: Luke D. Jones <luke@ljones.dev>
> > > > ---
> > > >  .../ABI/testing/sysfs-platform-asus-wmi       |  7 +++
> > > >  drivers/platform/x86/asus-wmi.c               | 54 ++++++++++++=
+++++++
> > > >  include/linux/platform_data/x86/asus-wmi.h    |  3 ++
> > > >  3 files changed, 64 insertions(+)
> > > >=20
> > > > diff --git a/Documentation/ABI/testing/sysfs-platform-asus-wmi b=
/Documentation/ABI/testing/sysfs-platform-asus-wmi
> > > > index e32b4f0ae15f..f3c53b7453f0 100644
> > > > --- a/Documentation/ABI/testing/sysfs-platform-asus-wmi
> > > > +++ b/Documentation/ABI/testing/sysfs-platform-asus-wmi
> > > > @@ -194,3 +194,10 @@ Contact: "Luke Jones" <luke@ljones.dev>
> > > >  Description:
> > > >  Set the target temperature limit of the Nvidia dGPU:
> > > >  * min=3D75, max=3D87
> > > > +
> > > > +What: /sys/devices/platform/<platform>/boot_sound
> > > > +Date: Jun 2023
> > > > +KernelVersion: 6.9
> > > > +Contact: "Luke Jones" <luke@ljones.dev>
> > > > +Description:
> > > > + Set if the BIOS POST sound is played on boot.
>=20
> > > > @@ -2106,6 +2107,55 @@ static ssize_t panel_od_store(struct devi=
ce *dev,
> > > >  }
> > > >  static DEVICE_ATTR_RW(panel_od);
> > > > =20
> > > > +/* Bootup sound ***********************************************=
****************/
> > > > +
> > > > +static ssize_t boot_sound_show(struct device *dev,
> > > > +      struct device_attribute *attr, char *buf)
> > > > +{
> > > > + struct asus_wmi *asus =3D dev_get_drvdata(dev);
> > > > + int result;
> > > > +
> > > > + result =3D asus_wmi_get_devstate_simple(asus, ASUS_WMI_DEVID_B=
OOT_SOUND);
> > > > + if (result < 0)
> > > > + return result;
> > > > +
> > > > + return sysfs_emit(buf, "%d\n", result);
> > > > +}
> > > > +
> > > > +static ssize_t boot_sound_store(struct device *dev,
> > > > +       struct device_attribute *attr,
> > > > +       const char *buf, size_t count)
> > > > +{
> > > > + int result, err;
> > > > + u32 snd;
> > > > +
> > > > + struct asus_wmi *asus =3D dev_get_drvdata(dev);
> > > > +
> > > > + result =3D kstrtou32(buf, 10, &snd);
> > > > + if (result)
> > > > + return result;
> > > > +
> > > > + if (snd > 1)
> > > > + return -EINVAL;
> > >=20
> > > Why not just use kstrtobool()?
> >=20
> > Consistency with other methods mostly. Plus the possibility that asu=
s=20
> > might do something like add different sounds. I'll change it if a re=
vert=20
> > back to kstrtou32 later doesn't break things.
>=20
> Hi Luke,
>=20
> I'd tend to think it's not the most likely scenario. But if they still=
 do=20
> something like that, the code could do both kstrtou32() and kstrtobool=
()=20
> to keep the sysfs interface backwards compatible.
>=20
> But it isn't end of the world for me if you want to keep it as=20
> kstrtou32().
>=20
> Annoyingly the other kstrtou32()s may not be easily converted over to=20
> kstrtobool() because u32 formatting accepts 16-based values too such as
> 0x0. Perhaps hex format wouldn't be used by anyone but the risk is sti=
ll=20
> there and the benefits are not that high.

Understood, yeah. I suppose one of the reasons I preferred using kstrtou=
32 in the first place was no need for casting when making the WMI call. =
Plus all the other code I've written here is similar to this (you might =
notice that a large chunk of what is in asus-wmi now is from me).