All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff LaBundy <jeff@labundy.com>
To: Quentin Schulz <quentin.schulz@theobroma-systems.com>
Cc: Quentin Schulz <foss+kernel@0leil.net>,
	robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org,
	heiko@sntech.de, dmitry.torokhov@gmail.com,
	klaus.goger@theobroma-systems.com, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org,
	linux-input@vger.kernel.org
Subject: Re: [PATCH 1/3] Input: add `SW_BOOT_ALT`
Date: Fri, 23 Sep 2022 12:19:11 -0500	[thread overview]
Message-ID: <Yy3qj432McMJ/5rl@nixie71> (raw)
In-Reply-To: <7add41ad-2cff-adef-08c4-dbc02f7c7c31@theobroma-systems.com>

Hi Quentin,

On Fri, Sep 23, 2022 at 10:25:58AM +0200, Quentin Schulz wrote:
> Hi Jeff,
> 
> On 9/22/22 19:20, Jeff LaBundy wrote:
> > Hi Quentin,
> > 
> > On Thu, Sep 22, 2022 at 12:12:09PM +0200, Quentin Schulz wrote:
> > > From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> > > 
> > > This event code represents the firmware source to use at boot.
> > > Value 0 means using "standard" firmware source, value 1 means using
> > > "alternative" firmware source.
> > > 
> > > For example, some hardware has the ability to force the BOOTROM to load
> > > the bootloader from a secondary firmware source (say SD card) instead of
> > > trying with the standard first and then the secondary. This event allows
> > > the userspace to know which firmware source was requested *in hardware*.
> > > 
> > > Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> > 
> > This does not seem like the right approach, especially since the switch
> > can easily be flipped after the state is already latched.
> > 
> > If the bootloader needs to pass information to the kernel (boot source or
> > otherwise), a safer and more flexible approach is to share some variables
> > in eMMC, or pass information using the kernel cmdline.
> > 
> 
> I made a terrible job at explaining what this switch is for, sorry.
> 
> Obviously, the state of the switch cannot represent which firmware boot
> source was used as only the bootloader will be able to tell (since it
> usually tries storage media in a specific order and the primary boot source
> could get corrupted at one point in time). Anyway, like you rightfully
> stated, this is useless "info" and the important one would be passed by the
> bootloader to the kernel (possibly via Device Tree fixup in case of
> Aarch64). U-Boot does this to set the memory node so this could be done
> again with a different property or something like that. Anyways, not
> something I'm really interested in.
> 
> I have a switch on my devkit which implements the BOOT_ALT#/BIOS_DISABLE#
> functionality from the Q7 standard, see section 3.1.17 Miscellaneous Signals
> in the specs:
> https://sget.org/wp-content/uploads/2018/09/Qseven-Spec_2.1.pdf
> 
> """
> BIOS_DISABLE#
> /BOOT_ALT#
> Module BIOS disable input signal. Pull low to disable
> module's on-board BIOS. Allows off-module BIOS
> implementations. This signal can also be used to disable
> standard boot firmware flash device and enable an alternative
> boot firmware source, for example a boot loader.
> """
> 
> This is basically the configuration of the firmware boot source to use for
> *the next boot*. It does not represent which boot source was used, nor which
> one will effectively be used.
> 
> In our case, this switch electrically disables eMMC and SPI flashes and only
> allow to boot from SD card (this switch is then electrically overridden by
> another GPIO at runtime by the bootloader/Linux kernel, but the state of the
> switch is still available to the user via another GPIO).

Thanks for the additional detail and the use-case is quite clear; I just
don't think input is the right home for this. Input makes more sense for
switches that a user may change during runtime with the expectation that
an event handler effects some sort of response.

Such is the case for lid open/close and headphone insertion, but here we
are just interested in the state of a muxed GPIO.

> 
> I have this switch on the board and I want to expose its state to the user,
> if this new event code is not possible/a good idea what would you suggest we
> could use?
> 
> Note that we already support the same switch but in a different way: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/rockchip/rk3399-puma-haikou.dts#n167
> We are just configuring the GPIOs into the GPIO mode with a pull-up, and
> then it's up to the user to use gpiod or gpio-sysfs to check the state of
> the GPIO used for this switch. I don't like this, very not user-friendly and
> was looking for something better :)

Actually, that's exactly what I was going to suggest. What in particular
is not user-friendly about it?

Of course, this is just my opinion as a fellow customer of input and it
is ultimately up to Dmitry.

> 
> Hope I explained myself a bit better this time, lemme know if I can clarify
> anything.
> 
> Thanks!
> Quentin

Kind regards,
Jeff LaBundy

WARNING: multiple messages have this Message-ID (diff)
From: Jeff LaBundy <jeff@labundy.com>
To: Quentin Schulz <quentin.schulz@theobroma-systems.com>
Cc: Quentin Schulz <foss+kernel@0leil.net>,
	robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org,
	heiko@sntech.de, dmitry.torokhov@gmail.com,
	klaus.goger@theobroma-systems.com, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org,
	linux-input@vger.kernel.org
Subject: Re: [PATCH 1/3] Input: add `SW_BOOT_ALT`
Date: Fri, 23 Sep 2022 12:19:11 -0500	[thread overview]
Message-ID: <Yy3qj432McMJ/5rl@nixie71> (raw)
In-Reply-To: <7add41ad-2cff-adef-08c4-dbc02f7c7c31@theobroma-systems.com>

Hi Quentin,

On Fri, Sep 23, 2022 at 10:25:58AM +0200, Quentin Schulz wrote:
> Hi Jeff,
> 
> On 9/22/22 19:20, Jeff LaBundy wrote:
> > Hi Quentin,
> > 
> > On Thu, Sep 22, 2022 at 12:12:09PM +0200, Quentin Schulz wrote:
> > > From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> > > 
> > > This event code represents the firmware source to use at boot.
> > > Value 0 means using "standard" firmware source, value 1 means using
> > > "alternative" firmware source.
> > > 
> > > For example, some hardware has the ability to force the BOOTROM to load
> > > the bootloader from a secondary firmware source (say SD card) instead of
> > > trying with the standard first and then the secondary. This event allows
> > > the userspace to know which firmware source was requested *in hardware*.
> > > 
> > > Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> > 
> > This does not seem like the right approach, especially since the switch
> > can easily be flipped after the state is already latched.
> > 
> > If the bootloader needs to pass information to the kernel (boot source or
> > otherwise), a safer and more flexible approach is to share some variables
> > in eMMC, or pass information using the kernel cmdline.
> > 
> 
> I made a terrible job at explaining what this switch is for, sorry.
> 
> Obviously, the state of the switch cannot represent which firmware boot
> source was used as only the bootloader will be able to tell (since it
> usually tries storage media in a specific order and the primary boot source
> could get corrupted at one point in time). Anyway, like you rightfully
> stated, this is useless "info" and the important one would be passed by the
> bootloader to the kernel (possibly via Device Tree fixup in case of
> Aarch64). U-Boot does this to set the memory node so this could be done
> again with a different property or something like that. Anyways, not
> something I'm really interested in.
> 
> I have a switch on my devkit which implements the BOOT_ALT#/BIOS_DISABLE#
> functionality from the Q7 standard, see section 3.1.17 Miscellaneous Signals
> in the specs:
> https://sget.org/wp-content/uploads/2018/09/Qseven-Spec_2.1.pdf
> 
> """
> BIOS_DISABLE#
> /BOOT_ALT#
> Module BIOS disable input signal. Pull low to disable
> module's on-board BIOS. Allows off-module BIOS
> implementations. This signal can also be used to disable
> standard boot firmware flash device and enable an alternative
> boot firmware source, for example a boot loader.
> """
> 
> This is basically the configuration of the firmware boot source to use for
> *the next boot*. It does not represent which boot source was used, nor which
> one will effectively be used.
> 
> In our case, this switch electrically disables eMMC and SPI flashes and only
> allow to boot from SD card (this switch is then electrically overridden by
> another GPIO at runtime by the bootloader/Linux kernel, but the state of the
> switch is still available to the user via another GPIO).

Thanks for the additional detail and the use-case is quite clear; I just
don't think input is the right home for this. Input makes more sense for
switches that a user may change during runtime with the expectation that
an event handler effects some sort of response.

Such is the case for lid open/close and headphone insertion, but here we
are just interested in the state of a muxed GPIO.

> 
> I have this switch on the board and I want to expose its state to the user,
> if this new event code is not possible/a good idea what would you suggest we
> could use?
> 
> Note that we already support the same switch but in a different way: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/rockchip/rk3399-puma-haikou.dts#n167
> We are just configuring the GPIOs into the GPIO mode with a pull-up, and
> then it's up to the user to use gpiod or gpio-sysfs to check the state of
> the GPIO used for this switch. I don't like this, very not user-friendly and
> was looking for something better :)

Actually, that's exactly what I was going to suggest. What in particular
is not user-friendly about it?

Of course, this is just my opinion as a fellow customer of input and it
is ultimately up to Dmitry.

> 
> Hope I explained myself a bit better this time, lemme know if I can clarify
> anything.
> 
> Thanks!
> Quentin

Kind regards,
Jeff LaBundy

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

WARNING: multiple messages have this Message-ID (diff)
From: Jeff LaBundy <jeff@labundy.com>
To: Quentin Schulz <quentin.schulz@theobroma-systems.com>
Cc: Quentin Schulz <foss+kernel@0leil.net>,
	robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org,
	heiko@sntech.de, dmitry.torokhov@gmail.com,
	klaus.goger@theobroma-systems.com, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org,
	linux-input@vger.kernel.org
Subject: Re: [PATCH 1/3] Input: add `SW_BOOT_ALT`
Date: Fri, 23 Sep 2022 12:19:11 -0500	[thread overview]
Message-ID: <Yy3qj432McMJ/5rl@nixie71> (raw)
In-Reply-To: <7add41ad-2cff-adef-08c4-dbc02f7c7c31@theobroma-systems.com>

Hi Quentin,

On Fri, Sep 23, 2022 at 10:25:58AM +0200, Quentin Schulz wrote:
> Hi Jeff,
> 
> On 9/22/22 19:20, Jeff LaBundy wrote:
> > Hi Quentin,
> > 
> > On Thu, Sep 22, 2022 at 12:12:09PM +0200, Quentin Schulz wrote:
> > > From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> > > 
> > > This event code represents the firmware source to use at boot.
> > > Value 0 means using "standard" firmware source, value 1 means using
> > > "alternative" firmware source.
> > > 
> > > For example, some hardware has the ability to force the BOOTROM to load
> > > the bootloader from a secondary firmware source (say SD card) instead of
> > > trying with the standard first and then the secondary. This event allows
> > > the userspace to know which firmware source was requested *in hardware*.
> > > 
> > > Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> > 
> > This does not seem like the right approach, especially since the switch
> > can easily be flipped after the state is already latched.
> > 
> > If the bootloader needs to pass information to the kernel (boot source or
> > otherwise), a safer and more flexible approach is to share some variables
> > in eMMC, or pass information using the kernel cmdline.
> > 
> 
> I made a terrible job at explaining what this switch is for, sorry.
> 
> Obviously, the state of the switch cannot represent which firmware boot
> source was used as only the bootloader will be able to tell (since it
> usually tries storage media in a specific order and the primary boot source
> could get corrupted at one point in time). Anyway, like you rightfully
> stated, this is useless "info" and the important one would be passed by the
> bootloader to the kernel (possibly via Device Tree fixup in case of
> Aarch64). U-Boot does this to set the memory node so this could be done
> again with a different property or something like that. Anyways, not
> something I'm really interested in.
> 
> I have a switch on my devkit which implements the BOOT_ALT#/BIOS_DISABLE#
> functionality from the Q7 standard, see section 3.1.17 Miscellaneous Signals
> in the specs:
> https://sget.org/wp-content/uploads/2018/09/Qseven-Spec_2.1.pdf
> 
> """
> BIOS_DISABLE#
> /BOOT_ALT#
> Module BIOS disable input signal. Pull low to disable
> module's on-board BIOS. Allows off-module BIOS
> implementations. This signal can also be used to disable
> standard boot firmware flash device and enable an alternative
> boot firmware source, for example a boot loader.
> """
> 
> This is basically the configuration of the firmware boot source to use for
> *the next boot*. It does not represent which boot source was used, nor which
> one will effectively be used.
> 
> In our case, this switch electrically disables eMMC and SPI flashes and only
> allow to boot from SD card (this switch is then electrically overridden by
> another GPIO at runtime by the bootloader/Linux kernel, but the state of the
> switch is still available to the user via another GPIO).

Thanks for the additional detail and the use-case is quite clear; I just
don't think input is the right home for this. Input makes more sense for
switches that a user may change during runtime with the expectation that
an event handler effects some sort of response.

Such is the case for lid open/close and headphone insertion, but here we
are just interested in the state of a muxed GPIO.

> 
> I have this switch on the board and I want to expose its state to the user,
> if this new event code is not possible/a good idea what would you suggest we
> could use?
> 
> Note that we already support the same switch but in a different way: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/rockchip/rk3399-puma-haikou.dts#n167
> We are just configuring the GPIOs into the GPIO mode with a pull-up, and
> then it's up to the user to use gpiod or gpio-sysfs to check the state of
> the GPIO used for this switch. I don't like this, very not user-friendly and
> was looking for something better :)

Actually, that's exactly what I was going to suggest. What in particular
is not user-friendly about it?

Of course, this is just my opinion as a fellow customer of input and it
is ultimately up to Dmitry.

> 
> Hope I explained myself a bit better this time, lemme know if I can clarify
> anything.
> 
> Thanks!
> Quentin

Kind regards,
Jeff LaBundy

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2022-09-23 17:19 UTC|newest]

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-22 10:12 [PATCH 0/3] add support for Theobroma Systems PX30-µQ7 (Ringneck) with Haikou devkit Quentin Schulz
2022-09-22 10:12 ` Quentin Schulz
2022-09-22 10:12 ` [PATCH 1/3] Input: add `SW_BOOT_ALT` Quentin Schulz
2022-09-22 10:12   ` Quentin Schulz
2022-09-22 10:12   ` Quentin Schulz
2022-09-22 17:20   ` Jeff LaBundy
2022-09-22 17:20     ` Jeff LaBundy
2022-09-22 17:20     ` Jeff LaBundy
2022-09-23  8:25     ` Quentin Schulz
2022-09-23  8:25       ` Quentin Schulz
2022-09-23  8:25       ` Quentin Schulz
2022-09-23 17:19       ` Jeff LaBundy [this message]
2022-09-23 17:19         ` Jeff LaBundy
2022-09-23 17:19         ` Jeff LaBundy
2022-09-23 17:24         ` Dmitry Torokhov
2022-09-23 17:24           ` Dmitry Torokhov
2022-09-23 17:24           ` Dmitry Torokhov
2022-09-26  9:37         ` Quentin Schulz
2022-09-26  9:37           ` Quentin Schulz
2022-09-26  9:37           ` Quentin Schulz
2022-09-27  1:56           ` Jeff LaBundy
2022-09-27  1:56             ` Jeff LaBundy
2022-09-27  1:56             ` Jeff LaBundy
2022-09-27  8:51             ` Quentin Schulz
2022-09-27  8:51               ` Quentin Schulz
2022-09-27  8:51               ` Quentin Schulz
2022-09-27 12:57               ` Robin Murphy
2022-09-27 12:57                 ` Robin Murphy
2022-09-27 12:57                 ` Robin Murphy
2022-09-22 10:12 ` [PATCH 2/3] arm64: dts: rockchip: add PX30-µQ7 (Ringneck) SoM with Haikou baseboard Quentin Schulz
2022-09-22 10:12   ` Quentin Schulz
2022-09-22 10:12   ` Quentin Schulz
2022-09-22 11:56   ` Quentin Schulz
2022-09-22 11:56     ` Quentin Schulz
2022-09-22 11:56     ` Quentin Schulz
2022-09-22 12:45   ` Krzysztof Kozlowski
2022-09-22 12:45     ` Krzysztof Kozlowski
2022-09-22 12:45     ` Krzysztof Kozlowski
2022-09-23  9:55     ` Quentin Schulz
2022-09-23  9:55       ` Quentin Schulz
2022-09-23  9:55       ` Quentin Schulz
2022-09-23 10:28       ` Krzysztof Kozlowski
2022-09-23 10:28         ` Krzysztof Kozlowski
2022-09-23 10:28         ` Krzysztof Kozlowski
2022-09-22 10:12 ` [PATCH 3/3] dt-bindings: arm: rockchip: add Theobroma Systems PX30-µQ7 (Ringneck) with Haikou Quentin Schulz
2022-09-22 10:12   ` Quentin Schulz
2022-09-22 10:12   ` Quentin Schulz
2022-09-22 12:40   ` Krzysztof Kozlowski
2022-09-22 12:40     ` Krzysztof Kozlowski
2022-09-22 12:40     ` Krzysztof Kozlowski
2022-09-22 12:41     ` Krzysztof Kozlowski
2022-09-22 12:41       ` Krzysztof Kozlowski
2022-09-22 12:41       ` Krzysztof Kozlowski

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=Yy3qj432McMJ/5rl@nixie71 \
    --to=jeff@labundy.com \
    --cc=devicetree@vger.kernel.org \
    --cc=dmitry.torokhov@gmail.com \
    --cc=foss+kernel@0leil.net \
    --cc=heiko@sntech.de \
    --cc=klaus.goger@theobroma-systems.com \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=quentin.schulz@theobroma-systems.com \
    --cc=robh+dt@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.