Re: [PATCH resend 2/3] gpiolib: acpi: Rename honor_wakeup option to ignore_wake, add extra quirk

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 3/2/20 10:30 AM, Andy Shevchenko wrote:
On Sat, Feb 29, 2020 at 09:57:52PM +0100, Hans de Goede wrote:
On 2/28/20 12:22 PM, Hans de Goede wrote:
On 2/25/20 1:57 PM, Andy Shevchenko wrote:
On Tue, Feb 25, 2020 at 02:34:25PM +0200, Andy Shevchenko wrote:
On Tue, Feb 25, 2020 at 12:26:04PM +0100, Hans de Goede wrote:

Let's do it as a list of pairs, but in slightly different format (I see some
potential to derive a generic parser, based on users described in
Documentation/admin-guide/kernel-parameters.txt), i.e.

     ignore_wake=pin:controller[,pin:controller[,...]]

Another possible format

     ignore_wake=controller@pin[;controller@pin[;...]]

I like this one, the other one with the pin first feels wrong, the pin is
part of the controller, not the other way around.

I will rework the patch series to use the ignore_wake=controller@pin format.

Just a quick note. I've changed the separator from ; to , for some reason
grub, at least as used in Fedora with Fedora's grub2 BLS (boot loader spec)
implementation does not like it when there is a ; in the kernel commandline.

Hmm... I think it would be harder then to have less possible formats in the
command line. Do you really need right now several pins to be listed?

Yes, the existing quirk for the HP X2 10 with Cherry Trail SoC + TI PMIC,
which currently ignores wakeups on all pins needs to ignore wakeup on 2 pins.

If it's about testing, perhaps we may do it with other means.

Well it is possible to pass the ; by putting quotes around it, so we could
go with the ; if you insist, but it really makes life harder for

I will also send an email about this to Fedora grub maintainer, but for
now it is easiest to just avoid the problem.

It's definitely bug in Grub due to existing kernel users with such format.
It means Grub is unable to support kernel command line in full.

So I discussed this with the Fedora Grub maintainer, he says the problem
exists in upstream grub2 too, grub2 uses a shell like command syntax
both in its config file and in interactive mode, so if you do e.g.:

linux /boot/vmlinuz root=/dev/sda1 gpiolib_acpi.ignore_wake=INT33FF:01@0;INT0002:00@2

Then grub will see the INT0002:00@2 as a new separate commaond, this should
work:

linux /boot/vmlinuz root=/dev/sda1 gpiolib_acpi.ignore_wake="INT33FF:01@0;INT0002:00@2"

But the recommended way to edit the cmdline is by editing /etc/default/grub and
then re-running grub2-mkconfig, which clears the quotes unless we escape them
and since grub2-mkconfig is shell script inside shell script inside shell script
I don't even want to think about how many times I need to escape the quotes.

TL;DR: Using ; in kernel commandline options makes life much harder for users
and as such is something which we should try to avoid.

I appreciate that you are trying to come up with a format for the option which
looks like existing options and I like the @ use, but using ; really is not a
good example to follow and IMHO that (not a good example / idea) trumps keeping
the syntax identical to an existing option.

Regards,

Hans






[Index of Archives]     [Linux SPI]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux