| Message ID | 20210930100719.2176-2-mika.westerberg@linux.intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | mtd: spi-nor / spi / MFD: Convert intel-spi to SPI MEM | expand |
Hi all! On Thu, Sep 30, 2021 at 7:07 AM Mika Westerberg <mika.westerberg@linux.intel.com> wrote: > > Currently the driver tries to disable the BIOS write protection > automatically even if this is not what the user wants. For this reason > modify the driver so that by default it does not touch the write > protection. Only if specifically asked by the user (setting writeable=1 > command line parameter) the driver tries to disable the BIOS write > protection. > > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> > --- > drivers/mfd/lpc_ich.c | 59 +++++++++++++++++-- > .../mtd/spi-nor/controllers/intel-spi-pci.c | 29 +++++---- > drivers/mtd/spi-nor/controllers/intel-spi.c | 41 ++++++------- > include/linux/platform_data/x86/intel-spi.h | 6 +- > 4 files changed, 96 insertions(+), 39 deletions(-) > > diff --git a/drivers/mfd/lpc_ich.c b/drivers/mfd/lpc_ich.c > index f10e53187f67..9ffab9aafd81 100644 > --- a/drivers/mfd/lpc_ich.c > +++ b/drivers/mfd/lpc_ich.c > @@ -63,6 +63,8 @@ > #define SPIBASE_BYT 0x54 > #define SPIBASE_BYT_SZ 512 > #define SPIBASE_BYT_EN BIT(1) > +#define BYT_BCR 0xfc > +#define BYT_BCR_WPD BIT(0) > > #define SPIBASE_LPT 0x3800 > #define SPIBASE_LPT_SZ 512 > @@ -1084,12 +1086,57 @@ static int lpc_ich_init_wdt(struct pci_dev *dev) > return ret; > } > > +static bool lpc_ich_byt_set_writeable(void __iomem *base, void *data) > +{ > + u32 val; > + > + val = readl(base + BYT_BCR); > + if (!(val & BYT_BCR_WPD)) { > + val |= BYT_BCR_WPD; > + writel(val, base + BYT_BCR); > + val = readl(base + BYT_BCR); > + } > + > + return val & BYT_BCR_WPD; > +} > + > +static bool lpc_ich_lpt_set_writeable(void __iomem *base, void *data) > +{ > + struct pci_dev *pdev = data; > + u32 bcr; > + > + 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); > + } > + > + return bcr & BCR_WPD; > +} > + > +static bool lpc_ich_bxt_set_writeable(void __iomem *base, void *data) > +{ > + unsigned int spi = PCI_DEVFN(13, 2); > + struct pci_bus *bus = data; > + u32 bcr; > + > + pci_bus_read_config_dword(bus, spi, BCR, &bcr); > + if (!(bcr & BCR_WPD)) { > + bcr |= BCR_WPD; > + pci_bus_write_config_dword(bus, spi, BCR, bcr); > + pci_bus_read_config_dword(bus, spi, BCR, &bcr); > + } > + > + return bcr & BCR_WPD; > +} > + > static int lpc_ich_init_spi(struct pci_dev *dev) > { > struct lpc_ich_priv *priv = pci_get_drvdata(dev); > struct resource *res = &intel_spi_res[0]; > struct intel_spi_boardinfo *info; > - u32 spi_base, rcba, bcr; > + u32 spi_base, rcba; > > info = devm_kzalloc(&dev->dev, sizeof(*info), GFP_KERNEL); > if (!info) > @@ -1103,6 +1150,8 @@ static int lpc_ich_init_spi(struct pci_dev *dev) > if (spi_base & SPIBASE_BYT_EN) { > res->start = spi_base & ~(SPIBASE_BYT_SZ - 1); > res->end = res->start + SPIBASE_BYT_SZ - 1; > + > + info->set_writeable = lpc_ich_byt_set_writeable; > } > break; > > @@ -1113,8 +1162,8 @@ static int lpc_ich_init_spi(struct pci_dev *dev) > res->start = spi_base + SPIBASE_LPT; > res->end = res->start + SPIBASE_LPT_SZ - 1; > > - pci_read_config_dword(dev, BCR, &bcr); > - info->writeable = !!(bcr & BCR_WPD); > + info->set_writeable = lpc_ich_lpt_set_writeable; > + info->data = dev; > } > break; > > @@ -1135,8 +1184,8 @@ static int lpc_ich_init_spi(struct pci_dev *dev) > res->start = spi_base & 0xfffffff0; > res->end = res->start + SPIBASE_APL_SZ - 1; > > - pci_bus_read_config_dword(bus, spi, BCR, &bcr); > - info->writeable = !!(bcr & BCR_WPD); > + info->set_writeable = lpc_ich_bxt_set_writeable; > + info->data = bus; > } > > pci_bus_write_config_byte(bus, p2sb, 0xe1, 0x1); > diff --git a/drivers/mtd/spi-nor/controllers/intel-spi-pci.c b/drivers/mtd/spi-nor/controllers/intel-spi-pci.c > index 1bc53b8bb88a..508f7ca098ef 100644 > --- a/drivers/mtd/spi-nor/controllers/intel-spi-pci.c > +++ b/drivers/mtd/spi-nor/controllers/intel-spi-pci.c > @@ -16,12 +16,30 @@ > #define BCR 0xdc > #define BCR_WPD BIT(0) > > +static bool intel_spi_pci_set_writeable(void __iomem *base, void *data) > +{ > + struct pci_dev *pdev = data; > + u32 bcr; > + > + /* 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); > + } > + > + return bcr & BCR_WPD; > +} > + > static const struct intel_spi_boardinfo bxt_info = { > .type = INTEL_SPI_BXT, > + .set_writeable = intel_spi_pci_set_writeable, > }; > > static const struct intel_spi_boardinfo cnl_info = { > .type = INTEL_SPI_CNL, > + .set_writeable = intel_spi_pci_set_writeable, > }; > > static int intel_spi_pci_probe(struct pci_dev *pdev, > @@ -29,7 +47,6 @@ static int intel_spi_pci_probe(struct pci_dev *pdev, > { > struct intel_spi_boardinfo *info; > struct intel_spi *ispi; > - u32 bcr; > int ret; > > ret = pcim_enable_device(pdev); > @@ -41,15 +58,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); > - > + info->data = pdev; > ispi = intel_spi_probe(&pdev->dev, &pdev->resource[0], info); > if (IS_ERR(ispi)) > return PTR_ERR(ispi); > diff --git a/drivers/mtd/spi-nor/controllers/intel-spi.c b/drivers/mtd/spi-nor/controllers/intel-spi.c > index a413892ff449..f35597cbea0c 100644 > --- a/drivers/mtd/spi-nor/controllers/intel-spi.c > +++ b/drivers/mtd/spi-nor/controllers/intel-spi.c > @@ -131,7 +131,6 @@ > * @sregs: Start of software sequencer registers > * @nregions: Maximum number of regions > * @pr_num: Maximum number of protected range registers > - * @writeable: Is the chip writeable > * @locked: Is SPI setting locked > * @swseq_reg: Use SW sequencer in register reads/writes > * @swseq_erase: Use SW sequencer in erase operation > @@ -149,7 +148,6 @@ struct intel_spi { > void __iomem *sregs; > size_t nregions; > size_t pr_num; > - bool writeable; > bool locked; > bool swseq_reg; > bool swseq_erase; > @@ -304,6 +302,14 @@ static int intel_spi_wait_sw_busy(struct intel_spi *ispi) > INTEL_SPI_TIMEOUT * 1000); > } > > +static bool intel_spi_set_writeable(struct intel_spi *ispi) > +{ > + if (!ispi->info->set_writeable) > + return false; > + > + return ispi->info->set_writeable(ispi->base, ispi->info->data); > +} > + > static int intel_spi_init(struct intel_spi *ispi) > { > u32 opmenu0, opmenu1, lvscc, uvscc, val; > @@ -316,19 +322,6 @@ static int intel_spi_init(struct intel_spi *ispi) > ispi->nregions = BYT_FREG_NUM; > ispi->pr_num = BYT_PR_NUM; > ispi->swseq_reg = true; > - > - if (writeable) { > - /* Disable write protection */ > - val = readl(ispi->base + BYT_BCR); > - if (!(val & BYT_BCR_WPD)) { > - val |= BYT_BCR_WPD; > - writel(val, ispi->base + BYT_BCR); > - val = readl(ispi->base + BYT_BCR); > - } > - > - ispi->writeable = !!(val & BYT_BCR_WPD); > - } > - > break; > > case INTEL_SPI_LPT: > @@ -358,6 +351,12 @@ static int intel_spi_init(struct intel_spi *ispi) > return -EINVAL; > } > > + /* Try to disable write protection if user asked to do so */ > + if (writeable && !intel_spi_set_writeable(ispi)) { > + dev_warn(ispi->dev, "can't disable chip write protection\n"); > + writeable = false; > + } > + > /* Disable #SMI generation from HW sequencer */ > val = readl(ispi->base + HSFSTS_CTL); > val &= ~HSFSTS_CTL_FSMIE; > @@ -884,9 +883,12 @@ static void intel_spi_fill_partition(struct intel_spi *ispi, > /* > * If any of the regions have protection bits set, make the > * whole partition read-only to be on the safe side. > + * > + * Also if the user did not ask the chip to be writeable > + * mask the bit too. > */ > - if (intel_spi_is_protected(ispi, base, limit)) > - ispi->writeable = false; > + if (!writeable || intel_spi_is_protected(ispi, base, limit)) > + part->mask_flags |= MTD_WRITEABLE; > > end = (limit << 12) + 4096; > if (end > part->size) > @@ -927,7 +929,6 @@ struct intel_spi *intel_spi_probe(struct device *dev, > > ispi->dev = dev; > ispi->info = info; > - ispi->writeable = info->writeable; > > ret = intel_spi_init(ispi); > if (ret) > @@ -945,10 +946,6 @@ struct intel_spi *intel_spi_probe(struct device *dev, > > intel_spi_fill_partition(ispi, &part); > > - /* Prevent writes if not explicitly enabled */ > - if (!ispi->writeable || !writeable) > - ispi->nor.mtd.flags &= ~MTD_WRITEABLE; > - > ret = mtd_device_register(&ispi->nor.mtd, &part, 1); > if (ret) > return ERR_PTR(ret); > diff --git a/include/linux/platform_data/x86/intel-spi.h b/include/linux/platform_data/x86/intel-spi.h > index 7f53a5c6f35e..7dda3f690465 100644 > --- a/include/linux/platform_data/x86/intel-spi.h > +++ b/include/linux/platform_data/x86/intel-spi.h > @@ -19,11 +19,13 @@ enum intel_spi_type { > /** > * struct intel_spi_boardinfo - Board specific data for Intel SPI driver > * @type: Type which this controller is compatible with > - * @writeable: The chip is writeable > + * @set_writeable: Try to make the chip writeable (optional) > + * @data: Data to be passed to @set_writeable can be %NULL > */ > struct intel_spi_boardinfo { > enum intel_spi_type type; > - bool writeable; > + bool (*set_writeable)(void __iomem *base, void *data); > + void *data; > }; > > #endif /* INTEL_SPI_PDATA_H */ > -- > 2.33.0 > Question for maintainers: With these changes is it safe to remove the *(DANGEROUS)* tag from menuconfig? Thanks, Mauro.
Hi, On Fri, Oct 01, 2021 at 05:23:23PM -0300, Mauro Lima wrote: > Question for maintainers: With these changes is it safe to remove the > *(DANGEROUS)* tag from menuconfig? I don't think we want to remove that. This driver is not for regular users, at least in its current form.
Hi Mika On Mon, Oct 4, 2021 at 2:18 AM Mika Westerberg <mika.westerberg@linux.intel.com> wrote: > > Hi, > > On Fri, Oct 01, 2021 at 05:23:23PM -0300, Mauro Lima wrote: > > Question for maintainers: With these changes is it safe to remove the > > *(DANGEROUS)* tag from menuconfig? > > I don't think we want to remove that. This driver is not for regular > users, at least in its current form. Do we know why this is still dangerous for the user? My plan is to make a sys/class driver to query write protection status of the SPI, this will be used by fwupd to gather information about vendors, also should be easy for the user to use 'cat' and see the information from userspace. For this to be possible we need kernel engineers to compile the kernel with this driver enabled, but the (DANGEROUS) tag is a no go for most of them. Is there anything I can do to make this possible? Thanks
Hi, On Tue, Oct 12, 2021 at 03:49:22PM -0300, Mauro Lima wrote: > Hi Mika > > On Mon, Oct 4, 2021 at 2:18 AM Mika Westerberg > <mika.westerberg@linux.intel.com> wrote: > > > > Hi, > > > > On Fri, Oct 01, 2021 at 05:23:23PM -0300, Mauro Lima wrote: > > > Question for maintainers: With these changes is it safe to remove the > > > *(DANGEROUS)* tag from menuconfig? > > > > I don't think we want to remove that. This driver is not for regular > > users, at least in its current form. > Do we know why this is still dangerous for the user? There was a bug in the driver in the past (that was already fixed but it did not yet reach the stable trees or something like that). At this unfortunate time there was no DANGEROUS in the name so Ubuntu kernel went and enabled it. Combined with the bug certain Lenovo laptops BIOS turned into read-only which prevented booting from non-default devices. This happened even when the driver did not do any "write" or "erase" operations, just clearing the status register or so. We don't want that to happen again. > My plan is to make a sys/class driver to query write protection status > of the SPI, this will be > used by fwupd to gather information about vendors, also should be easy > for the user to use > 'cat' and see the information from userspace. For this to be possible > we need kernel engineers > to compile the kernel with this driver enabled, but the (DANGEROUS) > tag is a no go for most > of them. > Is there anything I can do to make this possible? IMHO we can make certain parts of the driver, that are known not to cause any issues available without the DANGEROUS. I mean the controller exposes some information that I think you are intersted in and that does not cause anything to be sent to the flash chip itself.
Hi! On Wed, Oct 13, 2021 at 6:04 AM Mika Westerberg <mika.westerberg@linux.intel.com> wrote: > > Hi, > > On Tue, Oct 12, 2021 at 03:49:22PM -0300, Mauro Lima wrote: > > Hi Mika > > > > On Mon, Oct 4, 2021 at 2:18 AM Mika Westerberg > > <mika.westerberg@linux.intel.com> wrote: > > > > > > Hi, > > > > > > On Fri, Oct 01, 2021 at 05:23:23PM -0300, Mauro Lima wrote: > > > > Question for maintainers: With these changes is it safe to remove the > > > > *(DANGEROUS)* tag from menuconfig? > > > > > > I don't think we want to remove that. This driver is not for regular > > > users, at least in its current form. > > Do we know why this is still dangerous for the user? > > There was a bug in the driver in the past (that was already fixed but it > did not yet reach the stable trees or something like that). At this > unfortunate time there was no DANGEROUS in the name so Ubuntu kernel > went and enabled it. Combined with the bug certain Lenovo laptops BIOS > turned into read-only which prevented booting from non-default devices. > > This happened even when the driver did not do any "write" or "erase" > operations, just clearing the status register or so. > > We don't want that to happen again. > > > My plan is to make a sys/class driver to query write protection status > > of the SPI, this will be > > used by fwupd to gather information about vendors, also should be easy > > for the user to use > > 'cat' and see the information from userspace. For this to be possible > > we need kernel engineers > > to compile the kernel with this driver enabled, but the (DANGEROUS) > > tag is a no go for most > > of them. > > Is there anything I can do to make this possible? > > IMHO we can make certain parts of the driver, that are known not to > cause any issues available without the DANGEROUS. I mean the controller > exposes some information that I think you are intersted in and that does > not cause anything to be sent to the flash chip itself. This will work for me. Thanks!
diff --git a/drivers/mfd/lpc_ich.c b/drivers/mfd/lpc_ich.c index f10e53187f67..9ffab9aafd81 100644 --- a/drivers/mfd/lpc_ich.c +++ b/drivers/mfd/lpc_ich.c @@ -63,6 +63,8 @@ #define SPIBASE_BYT 0x54 #define SPIBASE_BYT_SZ 512 #define SPIBASE_BYT_EN BIT(1) +#define BYT_BCR 0xfc +#define BYT_BCR_WPD BIT(0) #define SPIBASE_LPT 0x3800 #define SPIBASE_LPT_SZ 512 @@ -1084,12 +1086,57 @@ static int lpc_ich_init_wdt(struct pci_dev *dev) return ret; } +static bool lpc_ich_byt_set_writeable(void __iomem *base, void *data) +{ + u32 val; + + val = readl(base + BYT_BCR); + if (!(val & BYT_BCR_WPD)) { + val |= BYT_BCR_WPD; + writel(val, base + BYT_BCR); + val = readl(base + BYT_BCR); + } + + return val & BYT_BCR_WPD; +} + +static bool lpc_ich_lpt_set_writeable(void __iomem *base, void *data) +{ + struct pci_dev *pdev = data; + u32 bcr; + + 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); + } + + return bcr & BCR_WPD; +} + +static bool lpc_ich_bxt_set_writeable(void __iomem *base, void *data) +{ + unsigned int spi = PCI_DEVFN(13, 2); + struct pci_bus *bus = data; + u32 bcr; + + pci_bus_read_config_dword(bus, spi, BCR, &bcr); + if (!(bcr & BCR_WPD)) { + bcr |= BCR_WPD; + pci_bus_write_config_dword(bus, spi, BCR, bcr); + pci_bus_read_config_dword(bus, spi, BCR, &bcr); + } + + return bcr & BCR_WPD; +} + static int lpc_ich_init_spi(struct pci_dev *dev) { struct lpc_ich_priv *priv = pci_get_drvdata(dev); struct resource *res = &intel_spi_res[0]; struct intel_spi_boardinfo *info; - u32 spi_base, rcba, bcr; + u32 spi_base, rcba; info = devm_kzalloc(&dev->dev, sizeof(*info), GFP_KERNEL); if (!info) @@ -1103,6 +1150,8 @@ static int lpc_ich_init_spi(struct pci_dev *dev) if (spi_base & SPIBASE_BYT_EN) { res->start = spi_base & ~(SPIBASE_BYT_SZ - 1); res->end = res->start + SPIBASE_BYT_SZ - 1; + + info->set_writeable = lpc_ich_byt_set_writeable; } break; @@ -1113,8 +1162,8 @@ static int lpc_ich_init_spi(struct pci_dev *dev) res->start = spi_base + SPIBASE_LPT; res->end = res->start + SPIBASE_LPT_SZ - 1; - pci_read_config_dword(dev, BCR, &bcr); - info->writeable = !!(bcr & BCR_WPD); + info->set_writeable = lpc_ich_lpt_set_writeable; + info->data = dev; } break; @@ -1135,8 +1184,8 @@ static int lpc_ich_init_spi(struct pci_dev *dev) res->start = spi_base & 0xfffffff0; res->end = res->start + SPIBASE_APL_SZ - 1; - pci_bus_read_config_dword(bus, spi, BCR, &bcr); - info->writeable = !!(bcr & BCR_WPD); + info->set_writeable = lpc_ich_bxt_set_writeable; + info->data = bus; } pci_bus_write_config_byte(bus, p2sb, 0xe1, 0x1); diff --git a/drivers/mtd/spi-nor/controllers/intel-spi-pci.c b/drivers/mtd/spi-nor/controllers/intel-spi-pci.c index 1bc53b8bb88a..508f7ca098ef 100644 --- a/drivers/mtd/spi-nor/controllers/intel-spi-pci.c +++ b/drivers/mtd/spi-nor/controllers/intel-spi-pci.c @@ -16,12 +16,30 @@ #define BCR 0xdc #define BCR_WPD BIT(0) +static bool intel_spi_pci_set_writeable(void __iomem *base, void *data) +{ + struct pci_dev *pdev = data; + u32 bcr; + + /* 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); + } + + return bcr & BCR_WPD; +} + static const struct intel_spi_boardinfo bxt_info = { .type = INTEL_SPI_BXT, + .set_writeable = intel_spi_pci_set_writeable, }; static const struct intel_spi_boardinfo cnl_info = { .type = INTEL_SPI_CNL, + .set_writeable = intel_spi_pci_set_writeable, }; static int intel_spi_pci_probe(struct pci_dev *pdev, @@ -29,7 +47,6 @@ static int intel_spi_pci_probe(struct pci_dev *pdev, { struct intel_spi_boardinfo *info; struct intel_spi *ispi; - u32 bcr; int ret; ret = pcim_enable_device(pdev); @@ -41,15 +58,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); - + info->data = pdev; ispi = intel_spi_probe(&pdev->dev, &pdev->resource[0], info); if (IS_ERR(ispi)) return PTR_ERR(ispi); diff --git a/drivers/mtd/spi-nor/controllers/intel-spi.c b/drivers/mtd/spi-nor/controllers/intel-spi.c index a413892ff449..f35597cbea0c 100644 --- a/drivers/mtd/spi-nor/controllers/intel-spi.c +++ b/drivers/mtd/spi-nor/controllers/intel-spi.c @@ -131,7 +131,6 @@ * @sregs: Start of software sequencer registers * @nregions: Maximum number of regions * @pr_num: Maximum number of protected range registers - * @writeable: Is the chip writeable * @locked: Is SPI setting locked * @swseq_reg: Use SW sequencer in register reads/writes * @swseq_erase: Use SW sequencer in erase operation @@ -149,7 +148,6 @@ struct intel_spi { void __iomem *sregs; size_t nregions; size_t pr_num; - bool writeable; bool locked; bool swseq_reg; bool swseq_erase; @@ -304,6 +302,14 @@ static int intel_spi_wait_sw_busy(struct intel_spi *ispi) INTEL_SPI_TIMEOUT * 1000); } +static bool intel_spi_set_writeable(struct intel_spi *ispi) +{ + if (!ispi->info->set_writeable) + return false; + + return ispi->info->set_writeable(ispi->base, ispi->info->data); +} + static int intel_spi_init(struct intel_spi *ispi) { u32 opmenu0, opmenu1, lvscc, uvscc, val; @@ -316,19 +322,6 @@ static int intel_spi_init(struct intel_spi *ispi) ispi->nregions = BYT_FREG_NUM; ispi->pr_num = BYT_PR_NUM; ispi->swseq_reg = true; - - if (writeable) { - /* Disable write protection */ - val = readl(ispi->base + BYT_BCR); - if (!(val & BYT_BCR_WPD)) { - val |= BYT_BCR_WPD; - writel(val, ispi->base + BYT_BCR); - val = readl(ispi->base + BYT_BCR); - } - - ispi->writeable = !!(val & BYT_BCR_WPD); - } - break; case INTEL_SPI_LPT: @@ -358,6 +351,12 @@ static int intel_spi_init(struct intel_spi *ispi) return -EINVAL; } + /* Try to disable write protection if user asked to do so */ + if (writeable && !intel_spi_set_writeable(ispi)) { + dev_warn(ispi->dev, "can't disable chip write protection\n"); + writeable = false; + } + /* Disable #SMI generation from HW sequencer */ val = readl(ispi->base + HSFSTS_CTL); val &= ~HSFSTS_CTL_FSMIE; @@ -884,9 +883,12 @@ static void intel_spi_fill_partition(struct intel_spi *ispi, /* * If any of the regions have protection bits set, make the * whole partition read-only to be on the safe side. + * + * Also if the user did not ask the chip to be writeable + * mask the bit too. */ - if (intel_spi_is_protected(ispi, base, limit)) - ispi->writeable = false; + if (!writeable || intel_spi_is_protected(ispi, base, limit)) + part->mask_flags |= MTD_WRITEABLE; end = (limit << 12) + 4096; if (end > part->size) @@ -927,7 +929,6 @@ struct intel_spi *intel_spi_probe(struct device *dev, ispi->dev = dev; ispi->info = info; - ispi->writeable = info->writeable; ret = intel_spi_init(ispi); if (ret) @@ -945,10 +946,6 @@ struct intel_spi *intel_spi_probe(struct device *dev, intel_spi_fill_partition(ispi, &part); - /* Prevent writes if not explicitly enabled */ - if (!ispi->writeable || !writeable) - ispi->nor.mtd.flags &= ~MTD_WRITEABLE; - ret = mtd_device_register(&ispi->nor.mtd, &part, 1); if (ret) return ERR_PTR(ret); diff --git a/include/linux/platform_data/x86/intel-spi.h b/include/linux/platform_data/x86/intel-spi.h index 7f53a5c6f35e..7dda3f690465 100644 --- a/include/linux/platform_data/x86/intel-spi.h +++ b/include/linux/platform_data/x86/intel-spi.h @@ -19,11 +19,13 @@ enum intel_spi_type { /** * struct intel_spi_boardinfo - Board specific data for Intel SPI driver * @type: Type which this controller is compatible with - * @writeable: The chip is writeable + * @set_writeable: Try to make the chip writeable (optional) + * @data: Data to be passed to @set_writeable can be %NULL */ struct intel_spi_boardinfo { enum intel_spi_type type; - bool writeable; + bool (*set_writeable)(void __iomem *base, void *data); + void *data; }; #endif /* INTEL_SPI_PDATA_H */
Currently the driver tries to disable the BIOS write protection automatically even if this is not what the user wants. For this reason modify the driver so that by default it does not touch the write protection. Only if specifically asked by the user (setting writeable=1 command line parameter) the driver tries to disable the BIOS write protection. Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> --- drivers/mfd/lpc_ich.c | 59 +++++++++++++++++-- .../mtd/spi-nor/controllers/intel-spi-pci.c | 29 +++++---- drivers/mtd/spi-nor/controllers/intel-spi.c | 41 ++++++------- include/linux/platform_data/x86/intel-spi.h | 6 +- 4 files changed, 96 insertions(+), 39 deletions(-)