Hi,
On Mon, Aug 03, 2020 at 10:44:49AM -0300, Daniel Gutson wrote:
> Currently, the intel-spi-pci driver tries to unconditionally set
> the SPI chip writeable. After discussing in the LKML, the original
> author decided that it was better to remove the attempt.
>
> Context, the intel-spi has a module argument that controls
> whether the driver attempts to turn the SPI flash chip writeable.
> The default value is FALSE (don't try to make it writeable).
> However, this flag applies only for a number of devices, coming from the
> platform driver, whereas the devices detected through the PCI driver
> (intel-spi-pci) are not subject to this check since the configuration
> takes place in intel-spi-pci which doesn't have an argument.
>
> This patch removes the code that attempts to turn the SPI chip writeable.
I think you should make the $subject to follow the convention used in
the SPI-NOR subsystem. You can see it running following command:
$ git log --oneline drivers/mtd/spi-nor/controllers/intel-spi-pci.c
In this case I think it should be:
mtd: spi-nor: intel-spi: Do not try to make the SPI flash chip writable
or something like that.
The patch itself looks good, one minor comment below.
>
> Signed-off-by: Daniel Gutson <daniel.gut...@eclypsium.com>
> ---
> drivers/mtd/spi-nor/controllers/intel-spi-pci.c | 6 ------
> 1 file changed, 6 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/controllers/intel-spi-pci.c
> b/drivers/mtd/spi-nor/controllers/intel-spi-pci.c
> index 81329f680bec..d721ba4e8b41 100644
> --- a/drivers/mtd/spi-nor/controllers/intel-spi-pci.c
> +++ b/drivers/mtd/spi-nor/controllers/intel-spi-pci.c
> @@ -41,13 +41,7 @@ static int intel_spi_pci_probe(struct pci_dev *pdev,
> if (!info)
> return -ENOMEM;
>
> - /* Try to make the chip read/write */
> pci_read_config_dword(pdev, BCR, &bcr);
> - if (!(bcr & BCR_WPD)) {
> - bcr |= BCR_WPD;
> - pci_write_config_dword(pdev, BCR, bcr);
> - pci_read_config_dword(pdev, BCR, &bcr);
> - }
> info->writeable = !!(bcr & BCR_WPD);
Perhaps we should log this in debug level (dev_dbg()) so when debugging
possible issues we can see that the chip is write protected by the BIOS.
>
> ispi = intel_spi_probe(&pdev->dev, &pdev->resource[0], info);
> --
> 2.25.1
