Platform-driver-x86 archive mirror
 help / color / mirror / Atom feed
From: "Luke Jones" <luke@ljones.dev>
To: "Ilpo Järvinen" <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
Date: Thu, 21 Mar 2024 09:05:21 +1300	[thread overview]
Message-ID: <e04bf262-e024-488a-ade4-698844af20e3@app.fastmail.com> (raw)
In-Reply-To: <bdcc3052-1403-8c2f-f703-66180394c461@linux.intel.com>

On Thu, 21 Mar 2024, at 12:55 AM, Ilpo Järvinen wrote:
> On Wed, 20 Mar 2024, Luke Jones wrote:
> > On Wed, 20 Mar 2024, at 6:48 AM, Ilpo Järvinen wrote:
> > > On Sun, 10 Mar 2024, Luke D. Jones wrote:
> > > 
> > > > Add support for toggling the BIOS POST sound on some ASUS laptops.
> > > > 
> > > > 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(+)
> > > > 
> > > > 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=75, max=87
> > > > +
> > > > +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 *dev,
> > > >  }
> > > >  static DEVICE_ATTR_RW(panel_od);
> > > >  
> > > > +/* Bootup sound ***************************************************************/
> > > > +
> > > > +static ssize_t boot_sound_show(struct device *dev,
> > > > +      struct device_attribute *attr, char *buf)
> > > > +{
> > > > + struct asus_wmi *asus = dev_get_drvdata(dev);
> > > > + int result;
> > > > +
> > > > + result = asus_wmi_get_devstate_simple(asus, ASUS_WMI_DEVID_BOOT_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 = dev_get_drvdata(dev);
> > > > +
> > > > + result = kstrtou32(buf, 10, &snd);
> > > > + if (result)
> > > > + return result;
> > > > +
> > > > + if (snd > 1)
> > > > + return -EINVAL;
> > > 
> > > Why not just use kstrtobool()?
> > 
> > Consistency with other methods mostly. Plus the possibility that asus 
> > might do something like add different sounds. I'll change it if a revert 
> > 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 
> something like that, the code could do both kstrtou32() and kstrtobool() 
> to keep the sysfs interface backwards compatible.
> 
> But it isn't end of the world for me if you want to keep it as 
> kstrtou32().
> 
> Annoyingly the other kstrtou32()s may not be easily converted over to 
> 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 
> there and the benefits are not that high.

Understood, yeah. I suppose one of the reasons I preferred using kstrtou32 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).

      reply	other threads:[~2024-03-20 20:05 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-10  6:17 [PATCH] platform/x86: asus-wmi: support toggling POST sound Luke D. Jones
2024-03-19 17:48 ` Ilpo Järvinen
2024-03-20  1:18   ` Luke Jones
2024-03-20 11:55     ` Ilpo Järvinen
2024-03-20 20:05       ` Luke Jones [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e04bf262-e024-488a-ade4-698844af20e3@app.fastmail.com \
    --to=luke@ljones.dev \
    --cc=hdegoede@redhat.com \
    --cc=ilpo.jarvinen@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=platform-driver-x86@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).