Received-SPF: pass (google.com: domain of linux-kernel+bounces-108886-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
From: =?UTF-8?q?Ilpo=20J=C3=A4rvinen?= <ilpo.jarvinen@linux.intel.com>
Date: Wed, 20 Mar 2024 13:55:52 +0200 (EET)
To: Luke Jones <luke@ljones.dev>
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
In-Reply-To: <7276ec7b-9b00-4241-a4eb-a8997daeba5c@app.fastmail.com>
Message-ID: <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>
Precedence: bulk
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="8323328-1644597700-1710935752=:1037"

  This message is in MIME format.  The first part should be readable text,
  while the remaining parts are likely unreadable without MIME-aware tools.

--8323328-1644597700-1710935752=:1037
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: QUOTED-PRINTABLE

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 laptops.
> > >=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/Docu=
mentation/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.

> > > @@ -2106,6 +2107,55 @@ static ssize_t panel_od_store(struct device *d=
ev,
> > >  }
> > >  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_BOOT_S=
OUND);
> > > + 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 asus=20
> might do something like add different sounds. I'll change it if a revert=
=20
> back to kstrtou32 later doesn't break things.

Hi Luke,

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.

But it isn't end of the world for me if you want to keep it as=20
kstrtou32().

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 still=20
there and the benefits are not that high.


--=20
 i.

--8323328-1644597700-1710935752=:1037--