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;
MIME-Version: 1.0
References: <20220808030420.8633-1-luke@ljones.dev> <20220808030420.8633-4-luke@ljones.dev>
 <CAHp75VdNa8csSF5Je87kOzC3EQ4yetZqpxXFZu1D7C4A6XCTNg@mail.gmail.com> <X5LBGR.15DO7CCYH6ER@ljones.dev>
In-Reply-To: <X5LBGR.15DO7CCYH6ER@ljones.dev>
From:   Andy Shevchenko <andy.shevchenko@gmail.com>
Date:   Tue, 9 Aug 2022 10:29:09 +0200
Message-ID: <CAHp75Ve4kFJz-4QqpUYTujQRkOkFZ+w01eHRwZyvQAA+GkNG9A@mail.gmail.com>
Subject: Re: [PATCH v2 3/6] asus-wmi: Implement TUF laptop keyboard power states
To:     Luke Jones <luke@ljones.dev>
Cc:     Hans de Goede <hdegoede@redhat.com>,
        Mark Gross <markgross@kernel.org>,
        Platform Driver <platform-driver-x86@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Tue, Aug 9, 2022 at 1:27 AM Luke Jones <luke@ljones.dev> wrote:

...

> >>  +       if (sscanf(buf, "%hhd %hhd %hhd %hhd %hhd", &save, &boot,
> >> &awake, &sleep, &keyboard) != 5)
> >>  +               return -EINVAL;
> >
> > Same Q here: wouldn't it be better to put each of the parameters to a
> > separate sysfs node? Or look at the LED ABI (that what Pavel mentioned
> > for multi-color patterns) and see if there are already some
> > established ways of how to represent necessary information?
>
> Same argument I make for the RGB mode nodes. But here I think it's
> probably even more pertinent. The reasons I would like to keep this as
> one node are:
>
> - It's separate to the RGB part
> - We can't read the device to set defaults on boot

Hmm... Maybe it's done via one of the WMI calls?

> - Because of the above, if we set a default and the user wants to
> change perhaps "sleep", then we're going to have to write some
> incorrect guess data since the write requires all the flags at once
> - One way to improve the UX is to add _show, but then this has to
> display incorrect data on boot
> - We end up with 5 more nodes
>
> The same reasons above apply to the RGB nodes, which right now I'm of
> two minds about. We'll see which way the RGB mode patch goes after some
> daily use.

I just realized that in previous mail I mentioned Device Tree which is
irrelevant here. We can't use it on x86 traditional platforms, so it
means that platform should somehow pass the data to the OS one way or
another. If there is no way to read back (bad designed interfaces),
then we can only reset to whatever user provides.

-- 
With Best Regards,
Andy Shevchenko