Re: [PATCH] mtd: spi-nor: intel-spi: Split intel-spi reading from writing

[Greg Kroah-Hartman]

On Wed, Oct 28, 2020 at 06:43:59PM -0300, Daniel Gutson wrote:
> This patch separates the writing part of the intel-spi drivers
> so the 'dangerous' part can be set/unset independently.
> This way, the kernel can be configured to include the reading
> parts of the driver which can be used without
> the dangerous write operations that can turn the system
> unbootable.
> 
> Signed-off-by: Daniel Gutson <daniel.gutson@xxxxxxxxxxxxx>
> ---
>  drivers/mtd/spi-nor/controllers/Kconfig     | 39 ++++++++++++---------
>  drivers/mtd/spi-nor/controllers/intel-spi.c | 12 +++++--
>  2 files changed, 33 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/controllers/Kconfig b/drivers/mtd/spi-nor/controllers/Kconfig
> index 5c0e0ec2e6d1..491c755fea49 100644
> --- a/drivers/mtd/spi-nor/controllers/Kconfig
> +++ b/drivers/mtd/spi-nor/controllers/Kconfig
> @@ -31,34 +31,41 @@ config SPI_INTEL_SPI
>  	tristate
>  
>  config SPI_INTEL_SPI_PCI
> -	tristate "Intel PCH/PCU SPI flash PCI driver (DANGEROUS)"
> +	tristate "Intel PCH/PCU SPI flash PCI driver"
>  	depends on X86 && PCI
>  	select SPI_INTEL_SPI
>  	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.
> -
> -	  Say N here unless you know what you are doing. Overwriting the
> -	  SPI flash may render the system unbootable.
> +	  This enables read only 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 read the SPI chip directly
> +	  from Linux.
>  
>  	  To compile this driver as a module, choose M here: the module
>  	  will be called intel-spi-pci.
>  
>  config SPI_INTEL_SPI_PLATFORM
> -	tristate "Intel PCH/PCU SPI flash platform driver (DANGEROUS)"
> +	tristate "Intel PCH/PCU SPI flash platform driver"
>  	depends on X86
>  	select SPI_INTEL_SPI
>  	help
> -	  This enables platform support for the Intel PCH/PCU SPI
> +	  This enables read only 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.
> +	  Intel hardware and is used to hold BIOS and other persistent settings.
> +	  Using this driver it is possible to read the SPI chip directly
> +	  from Linux.
> +
> +	  To compile this driver as a module, choose M here: the module
> +	  will be called intel-spi-pci.
> +
> +config SPI_INTEL_SPI_WRITE
> +	bool "Intel PCH/PCU SPI flash drivers write operations (DANGEROUS)"
> +	depends on SPI_INTEL_SPI_PCI || SPI_INTEL_SPI_PLATFORM
> +	help
> +	  This enables full read/write support for the Intel PCH/PCU SPI
> +	  controller.
> +	  Using this option it may be 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.
> -
> -	  To compile this driver as a module, choose M here: the module
> -	  will be called intel-spi-platform.
> diff --git a/drivers/mtd/spi-nor/controllers/intel-spi.c b/drivers/mtd/spi-nor/controllers/intel-spi.c
> index b54a56a68100..8d8053395c3d 100644
> --- a/drivers/mtd/spi-nor/controllers/intel-spi.c
> +++ b/drivers/mtd/spi-nor/controllers/intel-spi.c
> @@ -266,6 +266,7 @@ static int intel_spi_read_block(struct intel_spi *ispi, void *buf, size_t size)
>  	return 0;
>  }
>  
> +#ifdef CONFIG_SPI_INTEL_SPI_WRITE

<snip>

Please do not add #ifdef to .c files, that's not the proper kernel
coding style at all, and just makes maintaining this file much much
harder over time.

Split things out into two different files if you really need to do this.

thanks,

greg k-h