Given that the PCI driver handles controllers that only work with
hardware sequencing, we can remove the dangerous tag.
This patch is the second part of Mika's suggestion [1].
The first part was accepted in [2].
[1] https://lore.kernel.org/r/[email protected]/
[2] https://lore.kernel.org/linux-spi/[email protected]/
Mauro Lima (1):
spi: intel: Remove DANGEROUS tag from pci driver
drivers/spi/Kconfig | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
base-commit: 6437d7e4505debf3e2ea6cf1d04e9f8afd834445
--
2.39.1
Modern CPUs exposes this controller as PCI device that only uses
hardware sequencing capabilities which is safer than software
sequencing.
Leave the platform driver as *DANGEROUS* and update help text since
most of these controllers are using software sequencing.
Signed-off-by: Mauro Lima <[email protected]>
---
drivers/spi/Kconfig | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 87fc2bd16b72..3a362c450cb6 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -447,7 +447,7 @@ config SPI_INTEL
tristate
config SPI_INTEL_PCI
- tristate "Intel PCH/PCU SPI flash PCI driver (DANGEROUS)"
+ tristate "Intel PCH/PCU SPI flash PCI driver"
depends on PCI
depends on X86 || COMPILE_TEST
depends on SPI_MEM
@@ -455,8 +455,9 @@ config SPI_INTEL_PCI
help
This enables PCI support for the Intel PCH/PCU SPI controller in
master mode. This controller is present in modern Intel hardware
- and is used to hold BIOS and other persistent settings. Using
- this driver it is possible to upgrade BIOS directly from Linux.
+ and is used to hold BIOS and other persistent settings. This
+ driver only supports hardware sequencing mode. Using this
+ driver it is possible to upgrade BIOS directly from Linux.
Say N here unless you know what you are doing. Overwriting the
SPI flash may render the system unbootable.
@@ -471,10 +472,10 @@ config SPI_INTEL_PLATFORM
select SPI_INTEL
help
This enables platform support for the Intel PCH/PCU SPI
- controller in master mode. This controller is present in modern
- Intel hardware and is used to hold BIOS and other persistent
- settings. Using this driver it is possible to upgrade BIOS
- directly from Linux.
+ controller in master mode that is used to hold BIOS and other
+ persistent settings. Most of these controllers are using
+ software sequencing mode. Using this driver it is possible to
+ upgrade BIOS directly from Linux.
Say N here unless you know what you are doing. Overwriting the
SPI flash may render the system unbootable.
--
2.39.1
Hi,
One nit: In $subject please use PCI instead of pci.
On Mon, Feb 06, 2023 at 03:31:43PM -0300, Mauro Lima wrote:
> Modern CPUs exposes this controller as PCI device that only uses
> hardware sequencing capabilities which is safer than software
> sequencing.
> Leave the platform driver as *DANGEROUS* and update help text since
> most of these controllers are using software sequencing.
>
> Signed-off-by: Mauro Lima <[email protected]>
Acked-by: Mika Westerberg <[email protected]>
On Mon, Feb 06, 2023 at 03:31:42PM -0300, Mauro Lima wrote:
> Given that the PCI driver handles controllers that only work with
> hardware sequencing, we can remove the dangerous tag.
> This patch is the second part of Mika's suggestion [1].
> The first part was accepted in [2].
Please don't send cover letters for single patches, if there is anything
that needs saying put it in the changelog of the patch or after the ---
if it's administrative stuff. This reduces mail volume and ensures that
any important information is recorded in the changelog rather than being
lost.
> Modern CPUs exposes this controller as PCI device that only uses
> hardware sequencing capabilities which is safer than software
> sequencing.
> Leave the platform driver as *DANGEROUS* and update help text since
> most of these controllers are using software sequencing.
Out of curiosity, what is hardware sequencing? Maybe this should
be explained a bit more in the Kconfig help text. Looks like the
dangerous was there because you can update the bios and that
could eventually lead to a bricked mainboard. So hardware
sequencing helps there? how?
-michael
Hi,
On Tue, Feb 07, 2023 at 02:52:54PM +0100, Michael Walle wrote:
> > Modern CPUs exposes this controller as PCI device that only uses
> > hardware sequencing capabilities which is safer than software
> > sequencing.
> > Leave the platform driver as *DANGEROUS* and update help text since
> > most of these controllers are using software sequencing.
>
> Out of curiosity, what is hardware sequencing? Maybe this should
> be explained a bit more in the Kconfig help text. Looks like the
> dangerous was there because you can update the bios and that
> could eventually lead to a bricked mainboard. So hardware
> sequencing helps there? how?
Hardware sequencing means the controller exposes just a bunch of "high
level" operations to the software. Such as read, write, erase and so on
but does not allow running the actual "low level" SPI-NOR opcodes.
Software sequencing on the other hand allows running pretty much any
opcode and this is what caused problems for certain Lenovo laptops few
years back that then resulted adding DANGEROUS to the Kconfig.
Typically the flash is locked by the BIOS so ordinary users cannot
really overwrite it, even by accident.
Hi Mika,
Am 2023-02-07 15:03, schrieb Mika Westerberg:
> On Tue, Feb 07, 2023 at 02:52:54PM +0100, Michael Walle wrote:
>> > Modern CPUs exposes this controller as PCI device that only uses
>> > hardware sequencing capabilities which is safer than software
>> > sequencing.
>> > Leave the platform driver as *DANGEROUS* and update help text since
>> > most of these controllers are using software sequencing.
>>
>> Out of curiosity, what is hardware sequencing? Maybe this should
>> be explained a bit more in the Kconfig help text. Looks like the
>> dangerous was there because you can update the bios and that
>> could eventually lead to a bricked mainboard. So hardware
>> sequencing helps there? how?
>
> Hardware sequencing means the controller exposes just a bunch of "high
> level" operations to the software.
Ok, I figured it would have been something to do with the SPI driver
just supporting these high level ops. But even with that background
it was hard to connect that to the "hardware sequencing". The help
text should be somewhat understandable to the user/distro
people/whoever,
right? So I'd suggest to explain that a bit more in detail, or don't
use the term hardware sequencing at all. I'm not sure.
> Such as read, write, erase and so on
> but does not allow running the actual "low level" SPI-NOR opcodes.
> Software sequencing on the other hand allows running pretty much any
> opcode and this is what caused problems for certain Lenovo laptops few
> years back that then resulted adding DANGEROUS to the Kconfig.
That information should go into the commit message.
> Typically the flash is locked by the BIOS so ordinary users cannot
> really overwrite it, even by accident.
I see, thanks for the explanation!
-michael
On Mon, 06 Feb 2023 15:31:42 -0300, Mauro Lima wrote:
> Given that the PCI driver handles controllers that only work with
> hardware sequencing, we can remove the dangerous tag.
> This patch is the second part of Mika's suggestion [1].
> The first part was accepted in [2].
>
> [1] https://lore.kernel.org/r/[email protected]/
> [2] https://lore.kernel.org/linux-spi/[email protected]/
>
> [...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
Thanks!
[1/1] spi: intel: Remove DANGEROUS tag from pci driver
commit: 7db738b5fea4533fa217dfb05c506c15bd0964d9
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
Hi,
On Tue, Feb 07, 2023 at 03:11:26PM +0100, Michael Walle wrote:
> Hi Mika,
>
> Am 2023-02-07 15:03, schrieb Mika Westerberg:
> > On Tue, Feb 07, 2023 at 02:52:54PM +0100, Michael Walle wrote:
> > > > Modern CPUs exposes this controller as PCI device that only uses
> > > > hardware sequencing capabilities which is safer than software
> > > > sequencing.
> > > > Leave the platform driver as *DANGEROUS* and update help text since
> > > > most of these controllers are using software sequencing.
> > >
> > > Out of curiosity, what is hardware sequencing? Maybe this should
> > > be explained a bit more in the Kconfig help text. Looks like the
> > > dangerous was there because you can update the bios and that
> > > could eventually lead to a bricked mainboard. So hardware
> > > sequencing helps there? how?
> >
> > Hardware sequencing means the controller exposes just a bunch of "high
> > level" operations to the software.
>
> Ok, I figured it would have been something to do with the SPI driver
> just supporting these high level ops. But even with that background
> it was hard to connect that to the "hardware sequencing". The help
> text should be somewhat understandable to the user/distro people/whoever,
> right? So I'd suggest to explain that a bit more in detail, or don't
> use the term hardware sequencing at all. I'm not sure.
I agree it should be made more understandable for the distro folks. At
least add some explanation why it is OK to select this.
Mauro, can you do that in the next version?
> > Such as read, write, erase and so on
> > but does not allow running the actual "low level" SPI-NOR opcodes.
> > Software sequencing on the other hand allows running pretty much any
> > opcode and this is what caused problems for certain Lenovo laptops few
> > years back that then resulted adding DANGEROUS to the Kconfig.
>
> That information should go into the commit message.
+1
Hi all,
On Tue, Feb 7, 2023 at 2:25 PM Mika Westerberg
<[email protected]> wrote:
>
> Hi,
>
> On Tue, Feb 07, 2023 at 03:11:26PM +0100, Michael Walle wrote:
> > Hi Mika,
> >
> > Am 2023-02-07 15:03, schrieb Mika Westerberg:
> > > On Tue, Feb 07, 2023 at 02:52:54PM +0100, Michael Walle wrote:
> > > > > Modern CPUs exposes this controller as PCI device that only uses
> > > > > hardware sequencing capabilities which is safer than software
> > > > > sequencing.
> > > > > Leave the platform driver as *DANGEROUS* and update help text since
> > > > > most of these controllers are using software sequencing.
> > > >
> > > > Out of curiosity, what is hardware sequencing? Maybe this should
> > > > be explained a bit more in the Kconfig help text. Looks like the
> > > > dangerous was there because you can update the bios and that
> > > > could eventually lead to a bricked mainboard. So hardware
> > > > sequencing helps there? how?
> > >
> > > Hardware sequencing means the controller exposes just a bunch of "high
> > > level" operations to the software.
> >
> > Ok, I figured it would have been something to do with the SPI driver
> > just supporting these high level ops. But even with that background
> > it was hard to connect that to the "hardware sequencing". The help
> > text should be somewhat understandable to the user/distro people/whoever,
> > right? So I'd suggest to explain that a bit more in detail, or don't
> > use the term hardware sequencing at all. I'm not sure.
>
> I agree it should be made more understandable for the distro folks. At
> least add some explanation why it is OK to select this.
I agree with this.
> Mauro, can you do that in the next version?
Sure thing.
> > > Such as read, write, erase and so on
> > > but does not allow running the actual "low level" SPI-NOR opcodes.
> > > Software sequencing on the other hand allows running pretty much any
> > > opcode and this is what caused problems for certain Lenovo laptops few
> > > years back that then resulted adding DANGEROUS to the Kconfig.
> >
> > That information should go into the commit message.
>
> +1
Sorry about this, still learning :)
Thanks all for your comments and time.