[PATCH] mtd: spi-nor: intel-spi: Add support for second flash chip

[PATCH] mtd: spi-nor: intel-spi: Add support for second flash chip

[Mika Westerberg]

Intel SPI flash controller has been supporting two chip selects long
time already even if the most common configuration is to have single
flash chip for the BIOS and related data. This adds support for the
second chip select if we find out that there are two flash components
(this information is available in the mandatory flash descriptor on the
first chip). The second chip is exposed as is without any partition
information with name "chip1". The first chip continues work as is.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/mtd/spi-nor/controllers/intel-spi.c | 208 ++++++++++++++------
 1 file changed, 150 insertions(+), 58 deletions(-)

diff --git a/drivers/mtd/spi-nor/controllers/intel-spi.c b/drivers/mtd/spi-nor/controllers/intel-spi.c
index a413892ff449..f9a900c4dd40 100644
--- a/drivers/mtd/spi-nor/controllers/intel-spi.c
+++ b/drivers/mtd/spi-nor/controllers/intel-spi.c
@@ -118,6 +118,11 @@
 #define ERASE_64K_OPCODE_SHIFT		16
 #define ERASE_64K_OPCODE_MASK		(0xff << ERASE_OPCODE_SHIFT)
 
+/* Flash descriptor fields */
+#define FLVALSIG_MAGIC			0x0ff0a55a
+#define FLMAP0_NC_MASK			GENMASK(9, 8)
+#define FLMAP0_NC_SHIFT			8
+
 #define INTEL_SPI_TIMEOUT		5000 /* ms */
 #define INTEL_SPI_FIFO_SZ		64
 
@@ -125,7 +130,8 @@
  * struct intel_spi - Driver private data
  * @dev: Device pointer
  * @info: Pointer to board specific info
- * @nor: SPI NOR layer structure
+ * @nor: SPI NOR layer structures
+ * @nc: Number of flash chips
  * @base: Beginning of MMIO space
  * @pregs: Start of protection registers
  * @sregs: Start of software sequencer registers
@@ -143,7 +149,8 @@
 struct intel_spi {
 	struct device *dev;
 	const struct intel_spi_boardinfo *info;
-	struct spi_nor nor;
+	struct spi_nor nor[2];
+	size_t nc;
 	void __iomem *base;
 	void __iomem *pregs;
 	void __iomem *sregs;
@@ -554,14 +561,23 @@ static int intel_spi_sw_cycle(struct intel_spi *ispi, u8 opcode, size_t len,
 	return 0;
 }
 
+static void intel_spi_set_addr(const struct spi_nor *nor, loff_t addr)
+{
+	const struct intel_spi *ispi = nor->priv;
+
+	/* Pick correct flash component */
+	if (nor == &ispi->nor[1])
+		addr += ispi->nor[0].mtd.size;
+	writel(addr, ispi->base + FADDR);
+}
+
 static int intel_spi_read_reg(struct spi_nor *nor, u8 opcode, u8 *buf,
 			      size_t len)
 {
 	struct intel_spi *ispi = nor->priv;
 	int ret;
 
-	/* Address of the first chip */
-	writel(0, ispi->base + FADDR);
+	intel_spi_set_addr(nor, 0);
 
 	if (ispi->swseq_reg)
 		ret = intel_spi_sw_cycle(ispi, opcode, len,
@@ -620,7 +636,7 @@ static int intel_spi_write_reg(struct spi_nor *nor, u8 opcode, const u8 *buf,
 	if (opcode == SPINOR_OP_WRDI)
 		return 0;
 
-	writel(0, ispi->base + FADDR);
+	intel_spi_set_addr(nor, 0);
 
 	/* Write the value beforehand */
 	ret = intel_spi_write_block(ispi, buf, len);
@@ -633,12 +649,49 @@ static int intel_spi_write_reg(struct spi_nor *nor, u8 opcode, const u8 *buf,
 	return intel_spi_hw_cycle(ispi, opcode, len);
 }
 
+static ssize_t __intel_spi_read(struct intel_spi *ispi, struct spi_nor *nor,
+				loff_t from, size_t len, u_char *read_buf)
+{
+	u32 val, status;
+	int ret;
+
+	if (nor)
+		intel_spi_set_addr(nor, from);
+	else
+		writel(from, ispi->base + FADDR);
+
+	val = readl(ispi->base + HSFSTS_CTL);
+	val &= ~(HSFSTS_CTL_FDBC_MASK | HSFSTS_CTL_FCYCLE_MASK);
+	val |= HSFSTS_CTL_AEL | HSFSTS_CTL_FCERR | HSFSTS_CTL_FDONE;
+	val |= (len - 1) << HSFSTS_CTL_FDBC_SHIFT;
+	val |= HSFSTS_CTL_FCYCLE_READ;
+	val |= HSFSTS_CTL_FGO;
+	writel(val, ispi->base + HSFSTS_CTL);
+
+	ret = intel_spi_wait_hw_busy(ispi);
+	if (ret)
+		return ret;
+
+	status = readl(ispi->base + HSFSTS_CTL);
+	if (status & HSFSTS_CTL_FCERR)
+		ret = -EIO;
+	else if (status & HSFSTS_CTL_AEL)
+		ret = -EACCES;
+
+	if (ret < 0) {
+		dev_err(ispi->dev, "read error: %llx: %#x\n", from,
+			status);
+		return ret;
+	}
+
+	return intel_spi_read_block(ispi, read_buf, len);
+}
+
 static ssize_t intel_spi_read(struct spi_nor *nor, loff_t from, size_t len,
 			      u_char *read_buf)
 {
 	struct intel_spi *ispi = nor->priv;
 	size_t block_size, retlen = 0;
-	u32 val, status;
 	ssize_t ret;
 
 	/*
@@ -665,33 +718,7 @@ static ssize_t intel_spi_read(struct spi_nor *nor, loff_t from, size_t len,
 		block_size = min_t(loff_t, from + block_size,
 				   round_up(from + 1, SZ_4K)) - from;
 
-		writel(from, ispi->base + FADDR);
-
-		val = readl(ispi->base + HSFSTS_CTL);
-		val &= ~(HSFSTS_CTL_FDBC_MASK | HSFSTS_CTL_FCYCLE_MASK);
-		val |= HSFSTS_CTL_AEL | HSFSTS_CTL_FCERR | HSFSTS_CTL_FDONE;
-		val |= (block_size - 1) << HSFSTS_CTL_FDBC_SHIFT;
-		val |= HSFSTS_CTL_FCYCLE_READ;
-		val |= HSFSTS_CTL_FGO;
-		writel(val, ispi->base + HSFSTS_CTL);
-
-		ret = intel_spi_wait_hw_busy(ispi);
-		if (ret)
-			return ret;
-
-		status = readl(ispi->base + HSFSTS_CTL);
-		if (status & HSFSTS_CTL_FCERR)
-			ret = -EIO;
-		else if (status & HSFSTS_CTL_AEL)
-			ret = -EACCES;
-
-		if (ret < 0) {
-			dev_err(ispi->dev, "read error: %llx: %#x\n", from,
-				status);
-			return ret;
-		}
-
-		ret = intel_spi_read_block(ispi, read_buf, block_size);
+		ret = __intel_spi_read(ispi, nor, from, block_size, read_buf);
 		if (ret)
 			return ret;
 
@@ -722,7 +749,7 @@ static ssize_t intel_spi_write(struct spi_nor *nor, loff_t to, size_t len,
 		block_size = min_t(loff_t, to + block_size,
 				   round_up(to + 1, SZ_4K)) - to;
 
-		writel(to, ispi->base + FADDR);
+		intel_spi_set_addr(nor, to);
 
 		val = readl(ispi->base + HSFSTS_CTL);
 		val &= ~(HSFSTS_CTL_FDBC_MASK | HSFSTS_CTL_FCYCLE_MASK);
@@ -785,7 +812,7 @@ static int intel_spi_erase(struct spi_nor *nor, loff_t offs)
 
 	if (ispi->swseq_erase) {
 		while (len > 0) {
-			writel(offs, ispi->base + FADDR);
+			intel_spi_set_addr(nor, offs);
 
 			ret = intel_spi_sw_cycle(ispi, nor->erase_opcode,
 						 0, OPTYPE_WRITE_WITH_ADDR);
@@ -803,7 +830,7 @@ static int intel_spi_erase(struct spi_nor *nor, loff_t offs)
 	ispi->atomic_preopcode = 0;
 
 	while (len > 0) {
-		writel(offs, ispi->base + FADDR);
+		intel_spi_set_addr(nor, offs);
 
 		val = readl(ispi->base + HSFSTS_CTL);
 		val &= ~(HSFSTS_CTL_FDBC_MASK | HSFSTS_CTL_FCYCLE_MASK);
@@ -851,6 +878,37 @@ static bool intel_spi_is_protected(const struct intel_spi *ispi,
 	return false;
 }
 
+static int intel_spi_read_desc(struct intel_spi *ispi)
+{
+	u32 buf[2], nc;
+	ssize_t ret;
+
+	ret = __intel_spi_read(ispi, NULL, 0x10, sizeof(buf), (u_char *)buf);
+	if (ret < 0) {
+		dev_warn(ispi->dev, "failed to read descriptor\n");
+		return ret;
+	}
+
+	dev_dbg(ispi->dev, "FLVALSIG=0x%08x\n", buf[0]);
+	dev_dbg(ispi->dev, "FLMAP0=0x%08x\n", buf[1]);
+
+	if (buf[0] != FLVALSIG_MAGIC) {
+		dev_warn(ispi->dev, "descriptor signature not valid\n");
+		return -ENODEV;
+	}
+
+	nc = (buf[1] & FLMAP0_NC_MASK) >> FLMAP0_NC_SHIFT;
+	if (nc == 0)
+		ispi->nc = 1;
+	else if (nc == 1)
+		ispi->nc = 2;
+	else
+		return -EINVAL;
+
+	dev_dbg(ispi->dev, "%zd flash components found\n", ispi->nc);
+	return 0;
+}
+
 /*
  * There will be a single partition holding all enabled flash regions. We
  * call this "BIOS".
@@ -861,8 +919,6 @@ static void intel_spi_fill_partition(struct intel_spi *ispi,
 	u64 end;
 	int i;
 
-	memset(part, 0, sizeof(*part));
-
 	/* Start from the mandatory descriptor region */
 	part->size = 4096;
 	part->name = "BIOS";
@@ -889,7 +945,7 @@ static void intel_spi_fill_partition(struct intel_spi *ispi,
 			ispi->writeable = false;
 
 		end = (limit << 12) + 4096;
-		if (end > part->size)
+		if (end > part->size && end <= ispi->nor[0].mtd.size)
 			part->size = end;
 	}
 }
@@ -902,15 +958,47 @@ static const struct spi_nor_controller_ops intel_spi_controller_ops = {
 	.erase = intel_spi_erase,
 };
 
-struct intel_spi *intel_spi_probe(struct device *dev,
-	struct resource *mem, const struct intel_spi_boardinfo *info)
+static int intel_spi_add_chip(struct intel_spi *ispi, struct spi_nor *nor,
+			      const char *name)
 {
 	const struct spi_nor_hwcaps hwcaps = {
 		.mask = SNOR_HWCAPS_READ |
 			SNOR_HWCAPS_READ_FAST |
 			SNOR_HWCAPS_PP,
 	};
-	struct mtd_partition part;
+	struct mtd_partition parts[1];
+	int ret, nr_parts = 0;
+
+	nor->dev = ispi->dev;
+	nor->priv = ispi;
+	nor->controller_ops = &intel_spi_controller_ops;
+
+	memset(parts, 0, sizeof(parts));
+
+	ret = spi_nor_scan(nor, NULL, &hwcaps);
+	if (ret) {
+		dev_info(ispi->dev, "failed to locate the chip\n");
+		return ret;
+	}
+
+	/* Only the first chip has partition */
+	if (nor == &ispi->nor[0]) {
+		intel_spi_fill_partition(ispi, &parts[0]);
+		nr_parts = 1;
+	}
+
+	/* Prevent writes if not explicitly enabled */
+	if (!ispi->writeable || !writeable)
+		nor->mtd.flags &= ~MTD_WRITEABLE;
+	if (name)
+		nor->mtd.name = name;
+
+	return mtd_device_register(&nor->mtd, parts, nr_parts);
+}
+
+struct intel_spi *intel_spi_probe(struct device *dev,
+	struct resource *mem, const struct intel_spi_boardinfo *info)
+{
 	struct intel_spi *ispi;
 	int ret;
 
@@ -933,33 +1021,37 @@ struct intel_spi *intel_spi_probe(struct device *dev,
 	if (ret)
 		return ERR_PTR(ret);
 
-	ispi->nor.dev = ispi->dev;
-	ispi->nor.priv = ispi;
-	ispi->nor.controller_ops = &intel_spi_controller_ops;
-
-	ret = spi_nor_scan(&ispi->nor, NULL, &hwcaps);
-	if (ret) {
-		dev_info(dev, "failed to locate the chip\n");
+	ret = intel_spi_read_desc(ispi);
+	if (ret)
 		return ERR_PTR(ret);
-	}
-
-	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);
+	ret = intel_spi_add_chip(ispi, &ispi->nor[0], NULL);
 	if (ret)
 		return ERR_PTR(ret);
 
+	if (ispi->nc > 1) {
+		ret = intel_spi_add_chip(ispi, &ispi->nor[1], "chip1");
+		if (ret) {
+			mtd_device_unregister(&ispi->nor[0].mtd);
+			return ERR_PTR(ret);
+		}
+	}
+
 	return ispi;
 }
 EXPORT_SYMBOL_GPL(intel_spi_probe);
 
 int intel_spi_remove(struct intel_spi *ispi)
 {
-	return mtd_device_unregister(&ispi->nor.mtd);
+	int i, ret;
+
+	for (i = ispi->nc - 1; i >= 0; i--) {
+		ret = mtd_device_unregister(&ispi->nor[i].mtd);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
 }
 EXPORT_SYMBOL_GPL(intel_spi_remove);
 
-- 
2.30.2


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: spi-nor: intel-spi: Add support for second flash chip

[Pratyush Yadav]

Hi,

On 25/05/21 07:03PM, Mika Westerberg wrote:
> Intel SPI flash controller has been supporting two chip selects long
> time already even if the most common configuration is to have single
> flash chip for the BIOS and related data. This adds support for the
> second chip select if we find out that there are two flash components
> (this information is available in the mandatory flash descriptor on the
> first chip). The second chip is exposed as is without any partition
> information with name "chip1". The first chip continues work as is.
> 
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> ---
>  drivers/mtd/spi-nor/controllers/intel-spi.c | 208 ++++++++++++++------

Aren't the drivers in controllers/ supposed to move to use SPI MEM? I 
don't know if this has been discussed before, but I would like all 
drivers in controllers/ to move to SPI MEM API at some point in the 
future. This would let us drop support for this "legacy" controller API 
from SPI NOR, cleaning up the core quite a bit. No more if (nor->spimem) 
needed anywhere.

I wonder what other folks think about this. I vote for freezing all new 
features in controllers/. If you want something new, move to SPI MEM.

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: spi-nor: intel-spi: Add support for second flash chip

[Mika Westerberg]

Hi,

On Wed, May 26, 2021 at 12:44:16AM +0530, Pratyush Yadav wrote:
> Hi,
> 
> On 25/05/21 07:03PM, Mika Westerberg wrote:
> > Intel SPI flash controller has been supporting two chip selects long
> > time already even if the most common configuration is to have single
> > flash chip for the BIOS and related data. This adds support for the
> > second chip select if we find out that there are two flash components
> > (this information is available in the mandatory flash descriptor on the
> > first chip). The second chip is exposed as is without any partition
> > information with name "chip1". The first chip continues work as is.
> > 
> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > ---
> >  drivers/mtd/spi-nor/controllers/intel-spi.c | 208 ++++++++++++++------
> 
> Aren't the drivers in controllers/ supposed to move to use SPI MEM? I 
> don't know if this has been discussed before, but I would like all 
> drivers in controllers/ to move to SPI MEM API at some point in the 
> future. This would let us drop support for this "legacy" controller API 
> from SPI NOR, cleaning up the core quite a bit. No more if (nor->spimem) 
> needed anywhere.

What is SPI MEM? :) Looking at the mainline v5.13-rc3 controllers/ there
is no single driver "converted" to SPI MEM, at least from a quick
clance.

> I wonder what other folks think about this. I vote for freezing all new 
> features in controllers/. If you want something new, move to SPI MEM.

I think at this point it is not reasonable to expect that all controller
drivers move to your new framework that has not even landed the
mainline ;-) Or did I miss something?

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: spi-nor: intel-spi: Add support for second flash chip

[Mika Westerberg]

On Wed, May 26, 2021 at 12:12:56PM +0300, Mika Westerberg wrote:
> Hi,
> 
> On Wed, May 26, 2021 at 12:44:16AM +0530, Pratyush Yadav wrote:
> > Hi,
> > 
> > On 25/05/21 07:03PM, Mika Westerberg wrote:
> > > Intel SPI flash controller has been supporting two chip selects long
> > > time already even if the most common configuration is to have single
> > > flash chip for the BIOS and related data. This adds support for the
> > > second chip select if we find out that there are two flash components
> > > (this information is available in the mandatory flash descriptor on the
> > > first chip). The second chip is exposed as is without any partition
> > > information with name "chip1". The first chip continues work as is.
> > > 
> > > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > > ---
> > >  drivers/mtd/spi-nor/controllers/intel-spi.c | 208 ++++++++++++++------
> > 
> > Aren't the drivers in controllers/ supposed to move to use SPI MEM? I 
> > don't know if this has been discussed before, but I would like all 
> > drivers in controllers/ to move to SPI MEM API at some point in the 
> > future. This would let us drop support for this "legacy" controller API 
> > from SPI NOR, cleaning up the core quite a bit. No more if (nor->spimem) 
> > needed anywhere.
> 
> What is SPI MEM? :) Looking at the mainline v5.13-rc3 controllers/ there
> is no single driver "converted" to SPI MEM, at least from a quick
> clance.
> 
> > I wonder what other folks think about this. I vote for freezing all new 
> > features in controllers/. If you want something new, move to SPI MEM.
> 
> I think at this point it is not reasonable to expect that all controller
> drivers move to your new framework that has not even landed the
> mainline ;-) Or did I miss something?

Oh, I see now this commit:

a314f6367787 ("mtd: spi-nor: Convert cadence-quadspi to use spi-mem framework")

So "SPI MEM" means generic SPI subsystem for memory mapped devices.
Unfortunately Intel controller at least is not capable of running
generic SPI transactions. It only supports accessing SPI-NOR flashes and
for those there is small set of commands that supports. I don't think it
is even possible to convert the driver to generic SPI subsystem.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: spi-nor: intel-spi: Add support for second flash chip

[Michael Walle]

Am 2021-05-26 11:24, schrieb Mika Westerberg:
> On Wed, May 26, 2021 at 12:12:56PM +0300, Mika Westerberg wrote:
>> Hi,
>> 
>> On Wed, May 26, 2021 at 12:44:16AM +0530, Pratyush Yadav wrote:
>> > Hi,
>> >
>> > On 25/05/21 07:03PM, Mika Westerberg wrote:
>> > > Intel SPI flash controller has been supporting two chip selects long
>> > > time already even if the most common configuration is to have single
>> > > flash chip for the BIOS and related data. This adds support for the
>> > > second chip select if we find out that there are two flash components
>> > > (this information is available in the mandatory flash descriptor on the
>> > > first chip). The second chip is exposed as is without any partition
>> > > information with name "chip1". The first chip continues work as is.
>> > >
>> > > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
>> > > ---
>> > >  drivers/mtd/spi-nor/controllers/intel-spi.c | 208 ++++++++++++++------
>> >
>> > Aren't the drivers in controllers/ supposed to move to use SPI MEM? I
>> > don't know if this has been discussed before, but I would like all
>> > drivers in controllers/ to move to SPI MEM API at some point in the
>> > future. This would let us drop support for this "legacy" controller API
>> > from SPI NOR, cleaning up the core quite a bit. No more if (nor->spimem)
>> > needed anywhere.
>> 
>> What is SPI MEM? :) Looking at the mainline v5.13-rc3 controllers/ 
>> there
>> is no single driver "converted" to SPI MEM, at least from a quick
>> clance.
>> 
>> > I wonder what other folks think about this. I vote for freezing all new
>> > features in controllers/. If you want something new, move to SPI MEM.
>> 
>> I think at this point it is not reasonable to expect that all 
>> controller
>> drivers move to your new framework that has not even landed the
>> mainline ;-) Or did I miss something?
> 
> Oh, I see now this commit:
> 
> a314f6367787 ("mtd: spi-nor: Convert cadence-quadspi to use spi-mem 
> framework")
> 
> So "SPI MEM" means generic SPI subsystem for memory mapped devices.
> Unfortunately Intel controller at least is not capable of running
> generic SPI transactions. It only supports accessing SPI-NOR flashes 
> and
> for those there is small set of commands that supports. I don't think 
> it
> is even possible to convert the driver to generic SPI subsystem.

AFAIK it stands for SPI memory device (memory mapped is not a 
requirement).
Eg. spi-nxp-fspi doesn't support generic SPI devices either, but just 
SPI
flashes. So I'd guess SPI MEM is exactly what you are looking for.

-michael

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: spi-nor: intel-spi: Add support for second flash chip

[Mika Westerberg]

Hi,

On Wed, May 26, 2021 at 11:31:58AM +0200, Michael Walle wrote:
> > Oh, I see now this commit:
> > 
> > a314f6367787 ("mtd: spi-nor: Convert cadence-quadspi to use spi-mem
> > framework")
> > 
> > So "SPI MEM" means generic SPI subsystem for memory mapped devices.
> > Unfortunately Intel controller at least is not capable of running
> > generic SPI transactions. It only supports accessing SPI-NOR flashes and
> > for those there is small set of commands that supports. I don't think it
> > is even possible to convert the driver to generic SPI subsystem.
> 
> AFAIK it stands for SPI memory device (memory mapped is not a requirement).
> Eg. spi-nxp-fspi doesn't support generic SPI devices either, but just SPI
> flashes. So I'd guess SPI MEM is exactly what you are looking for.

OK, I see that there is ->mem_ops that can be used to implement
different higher level commands. What I'm not seeing is that how the
child SPI flash is created using this scheme? DeviceTree and ACPI are
supported fine but what about scanning? I mean the intel_spi driver has
this:

  spi_nor_scan(&ispi->nor, NULL, &hwcaps);

But if the driver is to be moved under drivers/spi/* you can't really
call these functions anymore or can you? Or the point is to keep the
driver under controllers/ and just call spi_nor_scan(), and in addition
implement the new mem_ops?

Thanks in advance and sorry about many questions but there does not seem
to be a conversion guide nor any (non-DT/ACPI) examples that I can take
a look. :-)

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: spi-nor: intel-spi: Add support for second flash chip

[Mika Westerberg]

Hi guys,

On Wed, May 26, 2021 at 01:28:16PM +0300, Mika Westerberg wrote:
> Hi,
> 
> On Wed, May 26, 2021 at 11:31:58AM +0200, Michael Walle wrote:
> > > Oh, I see now this commit:
> > > 
> > > a314f6367787 ("mtd: spi-nor: Convert cadence-quadspi to use spi-mem
> > > framework")
> > > 
> > > So "SPI MEM" means generic SPI subsystem for memory mapped devices.
> > > Unfortunately Intel controller at least is not capable of running
> > > generic SPI transactions. It only supports accessing SPI-NOR flashes and
> > > for those there is small set of commands that supports. I don't think it
> > > is even possible to convert the driver to generic SPI subsystem.
> > 
> > AFAIK it stands for SPI memory device (memory mapped is not a requirement).
> > Eg. spi-nxp-fspi doesn't support generic SPI devices either, but just SPI
> > flashes. So I'd guess SPI MEM is exactly what you are looking for.
> 
> OK, I see that there is ->mem_ops that can be used to implement
> different higher level commands. What I'm not seeing is that how the
> child SPI flash is created using this scheme? DeviceTree and ACPI are
> supported fine but what about scanning? I mean the intel_spi driver has
> this:
> 
>   spi_nor_scan(&ispi->nor, NULL, &hwcaps);
> 
> But if the driver is to be moved under drivers/spi/* you can't really
> call these functions anymore or can you? Or the point is to keep the
> driver under controllers/ and just call spi_nor_scan(), and in addition
> implement the new mem_ops?
> 
> Thanks in advance and sorry about many questions but there does not seem
> to be a conversion guide nor any (non-DT/ACPI) examples that I can take
> a look. :-)

Can you provide some guidance here? So in order to use the generic SPI
subsystem with "SPI MEM" parts of it, I would need to be able to create
the child SPI-NOR flash device without using ACPI or DT (as these
systems do not have any ACPI/DT description), or use spi_nor_scan() but
none of the driver under drivers/spi are calling it.

Thanks!

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: spi-nor: intel-spi: Add support for second flash chip

[Mika Westerberg]

Hi Tudor,

On Mon, May 31, 2021 at 02:29:59PM +0300, Mika Westerberg wrote:
> Hi guys,
> 
> On Wed, May 26, 2021 at 01:28:16PM +0300, Mika Westerberg wrote:
> > Hi,
> > 
> > On Wed, May 26, 2021 at 11:31:58AM +0200, Michael Walle wrote:
> > > > Oh, I see now this commit:
> > > > 
> > > > a314f6367787 ("mtd: spi-nor: Convert cadence-quadspi to use spi-mem
> > > > framework")
> > > > 
> > > > So "SPI MEM" means generic SPI subsystem for memory mapped devices.
> > > > Unfortunately Intel controller at least is not capable of running
> > > > generic SPI transactions. It only supports accessing SPI-NOR flashes and
> > > > for those there is small set of commands that supports. I don't think it
> > > > is even possible to convert the driver to generic SPI subsystem.
> > > 
> > > AFAIK it stands for SPI memory device (memory mapped is not a requirement).
> > > Eg. spi-nxp-fspi doesn't support generic SPI devices either, but just SPI
> > > flashes. So I'd guess SPI MEM is exactly what you are looking for.
> > 
> > OK, I see that there is ->mem_ops that can be used to implement
> > different higher level commands. What I'm not seeing is that how the
> > child SPI flash is created using this scheme? DeviceTree and ACPI are
> > supported fine but what about scanning? I mean the intel_spi driver has
> > this:
> > 
> >   spi_nor_scan(&ispi->nor, NULL, &hwcaps);
> > 
> > But if the driver is to be moved under drivers/spi/* you can't really
> > call these functions anymore or can you? Or the point is to keep the
> > driver under controllers/ and just call spi_nor_scan(), and in addition
> > implement the new mem_ops?
> > 
> > Thanks in advance and sorry about many questions but there does not seem
> > to be a conversion guide nor any (non-DT/ACPI) examples that I can take
> > a look. :-)
> 
> Can you provide some guidance here? So in order to use the generic SPI
> subsystem with "SPI MEM" parts of it, I would need to be able to create
> the child SPI-NOR flash device without using ACPI or DT (as these
> systems do not have any ACPI/DT description), or use spi_nor_scan() but
> none of the driver under drivers/spi are calling it.

As the main SPI-NOR maintainer, what's your take on this?

Thanks!

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: spi-nor: intel-spi: Add support for second flash chip

[Pratyush Yadav]

+Mark, linux-spi list,

On 03/06/21 02:07PM, Mika Westerberg wrote:
> Hi Tudor,
> 
> On Mon, May 31, 2021 at 02:29:59PM +0300, Mika Westerberg wrote:
> > Hi guys,
> > 
> > On Wed, May 26, 2021 at 01:28:16PM +0300, Mika Westerberg wrote:
> > > Hi,
> > > 
> > > On Wed, May 26, 2021 at 11:31:58AM +0200, Michael Walle wrote:
> > > > > Oh, I see now this commit:
> > > > > 
> > > > > a314f6367787 ("mtd: spi-nor: Convert cadence-quadspi to use spi-mem
> > > > > framework")
> > > > > 
> > > > > So "SPI MEM" means generic SPI subsystem for memory mapped devices.
> > > > > Unfortunately Intel controller at least is not capable of running
> > > > > generic SPI transactions. It only supports accessing SPI-NOR flashes and
> > > > > for those there is small set of commands that supports. I don't think it
> > > > > is even possible to convert the driver to generic SPI subsystem.
> > > > 
> > > > AFAIK it stands for SPI memory device (memory mapped is not a requirement).
> > > > Eg. spi-nxp-fspi doesn't support generic SPI devices either, but just SPI
> > > > flashes. So I'd guess SPI MEM is exactly what you are looking for.
> > > 
> > > OK, I see that there is ->mem_ops that can be used to implement
> > > different higher level commands. What I'm not seeing is that how the
> > > child SPI flash is created using this scheme? DeviceTree and ACPI are
> > > supported fine but what about scanning? I mean the intel_spi driver has
> > > this:
> > > 
> > >   spi_nor_scan(&ispi->nor, NULL, &hwcaps);
> > > 
> > > But if the driver is to be moved under drivers/spi/* you can't really
> > > call these functions anymore or can you? Or the point is to keep the
> > > driver under controllers/ and just call spi_nor_scan(), and in addition
> > > implement the new mem_ops?
> > > 
> > > Thanks in advance and sorry about many questions but there does not seem
> > > to be a conversion guide nor any (non-DT/ACPI) examples that I can take
> > > a look. :-)
> > 
> > Can you provide some guidance here? So in order to use the generic SPI
> > subsystem with "SPI MEM" parts of it, I would need to be able to create
> > the child SPI-NOR flash device without using ACPI or DT (as these
> > systems do not have any ACPI/DT description), or use spi_nor_scan() but
> > none of the driver under drivers/spi are calling it.
> 
> As the main SPI-NOR maintainer, what's your take on this?

I think this is more of a SPI or SPI MEM question, and less of a SPI NOR 
question. SPI MEM would call spi_nor_probe() which in turn calls 
spi_nor_scan().

So the question that needs to be answered is how to probe SPI MEM based
drivers without ACPI/DT.

> 
> Thanks!

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

Re: [PATCH] mtd: spi-nor: intel-spi: Add support for second flash chip

[Pratyush Yadav]

+Mark, linux-spi list,

On 03/06/21 02:07PM, Mika Westerberg wrote:
> Hi Tudor,
> 
> On Mon, May 31, 2021 at 02:29:59PM +0300, Mika Westerberg wrote:
> > Hi guys,
> > 
> > On Wed, May 26, 2021 at 01:28:16PM +0300, Mika Westerberg wrote:
> > > Hi,
> > > 
> > > On Wed, May 26, 2021 at 11:31:58AM +0200, Michael Walle wrote:
> > > > > Oh, I see now this commit:
> > > > > 
> > > > > a314f6367787 ("mtd: spi-nor: Convert cadence-quadspi to use spi-mem
> > > > > framework")
> > > > > 
> > > > > So "SPI MEM" means generic SPI subsystem for memory mapped devices.
> > > > > Unfortunately Intel controller at least is not capable of running
> > > > > generic SPI transactions. It only supports accessing SPI-NOR flashes and
> > > > > for those there is small set of commands that supports. I don't think it
> > > > > is even possible to convert the driver to generic SPI subsystem.
> > > > 
> > > > AFAIK it stands for SPI memory device (memory mapped is not a requirement).
> > > > Eg. spi-nxp-fspi doesn't support generic SPI devices either, but just SPI
> > > > flashes. So I'd guess SPI MEM is exactly what you are looking for.
> > > 
> > > OK, I see that there is ->mem_ops that can be used to implement
> > > different higher level commands. What I'm not seeing is that how the
> > > child SPI flash is created using this scheme? DeviceTree and ACPI are
> > > supported fine but what about scanning? I mean the intel_spi driver has
> > > this:
> > > 
> > >   spi_nor_scan(&ispi->nor, NULL, &hwcaps);
> > > 
> > > But if the driver is to be moved under drivers/spi/* you can't really
> > > call these functions anymore or can you? Or the point is to keep the
> > > driver under controllers/ and just call spi_nor_scan(), and in addition
> > > implement the new mem_ops?
> > > 
> > > Thanks in advance and sorry about many questions but there does not seem
> > > to be a conversion guide nor any (non-DT/ACPI) examples that I can take
> > > a look. :-)
> > 
> > Can you provide some guidance here? So in order to use the generic SPI
> > subsystem with "SPI MEM" parts of it, I would need to be able to create
> > the child SPI-NOR flash device without using ACPI or DT (as these
> > systems do not have any ACPI/DT description), or use spi_nor_scan() but
> > none of the driver under drivers/spi are calling it.
> 
> As the main SPI-NOR maintainer, what's your take on this?

I think this is more of a SPI or SPI MEM question, and less of a SPI NOR 
question. SPI MEM would call spi_nor_probe() which in turn calls 
spi_nor_scan().

So the question that needs to be answered is how to probe SPI MEM based
drivers without ACPI/DT.

> 
> Thanks!

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: spi-nor: intel-spi: Add support for second flash chip

[Mika Westerberg]

On Thu, Jun 03, 2021 at 11:38:45PM +0530, Pratyush Yadav wrote:
> +Mark, linux-spi list,
> 
> On 03/06/21 02:07PM, Mika Westerberg wrote:
> > Hi Tudor,
> > 
> > On Mon, May 31, 2021 at 02:29:59PM +0300, Mika Westerberg wrote:
> > > Hi guys,
> > > 
> > > On Wed, May 26, 2021 at 01:28:16PM +0300, Mika Westerberg wrote:
> > > > Hi,
> > > > 
> > > > On Wed, May 26, 2021 at 11:31:58AM +0200, Michael Walle wrote:
> > > > > > Oh, I see now this commit:
> > > > > > 
> > > > > > a314f6367787 ("mtd: spi-nor: Convert cadence-quadspi to use spi-mem
> > > > > > framework")
> > > > > > 
> > > > > > So "SPI MEM" means generic SPI subsystem for memory mapped devices.
> > > > > > Unfortunately Intel controller at least is not capable of running
> > > > > > generic SPI transactions. It only supports accessing SPI-NOR flashes and
> > > > > > for those there is small set of commands that supports. I don't think it
> > > > > > is even possible to convert the driver to generic SPI subsystem.
> > > > > 
> > > > > AFAIK it stands for SPI memory device (memory mapped is not a requirement).
> > > > > Eg. spi-nxp-fspi doesn't support generic SPI devices either, but just SPI
> > > > > flashes. So I'd guess SPI MEM is exactly what you are looking for.
> > > > 
> > > > OK, I see that there is ->mem_ops that can be used to implement
> > > > different higher level commands. What I'm not seeing is that how the
> > > > child SPI flash is created using this scheme? DeviceTree and ACPI are
> > > > supported fine but what about scanning? I mean the intel_spi driver has
> > > > this:
> > > > 
> > > >   spi_nor_scan(&ispi->nor, NULL, &hwcaps);
> > > > 
> > > > But if the driver is to be moved under drivers/spi/* you can't really
> > > > call these functions anymore or can you? Or the point is to keep the
> > > > driver under controllers/ and just call spi_nor_scan(), and in addition
> > > > implement the new mem_ops?
> > > > 
> > > > Thanks in advance and sorry about many questions but there does not seem
> > > > to be a conversion guide nor any (non-DT/ACPI) examples that I can take
> > > > a look. :-)
> > > 
> > > Can you provide some guidance here? So in order to use the generic SPI
> > > subsystem with "SPI MEM" parts of it, I would need to be able to create
> > > the child SPI-NOR flash device without using ACPI or DT (as these
> > > systems do not have any ACPI/DT description), or use spi_nor_scan() but
> > > none of the driver under drivers/spi are calling it.
> > 
> > As the main SPI-NOR maintainer, what's your take on this?
> 
> I think this is more of a SPI or SPI MEM question, and less of a SPI NOR 
> question. SPI MEM would call spi_nor_probe() which in turn calls 
> spi_nor_scan().
> 
> So the question that needs to be answered is how to probe SPI MEM based
> drivers without ACPI/DT.

Yes, exactly. With ACPI/DT the SPI core handles this after the SPI
master device is registered and that would result spi_nor_probe() to be
called for the children. However, with this one there is no ACPI node
for the controller (it is PCI enumerated) so there would need to be some
way to create that child device. In the old days that would be "platform
data" but that's pretty much frowned upon these days ;-)

Re: [PATCH] mtd: spi-nor: intel-spi: Add support for second flash chip

[Mika Westerberg]

On Thu, Jun 03, 2021 at 11:38:45PM +0530, Pratyush Yadav wrote:
> +Mark, linux-spi list,
> 
> On 03/06/21 02:07PM, Mika Westerberg wrote:
> > Hi Tudor,
> > 
> > On Mon, May 31, 2021 at 02:29:59PM +0300, Mika Westerberg wrote:
> > > Hi guys,
> > > 
> > > On Wed, May 26, 2021 at 01:28:16PM +0300, Mika Westerberg wrote:
> > > > Hi,
> > > > 
> > > > On Wed, May 26, 2021 at 11:31:58AM +0200, Michael Walle wrote:
> > > > > > Oh, I see now this commit:
> > > > > > 
> > > > > > a314f6367787 ("mtd: spi-nor: Convert cadence-quadspi to use spi-mem
> > > > > > framework")
> > > > > > 
> > > > > > So "SPI MEM" means generic SPI subsystem for memory mapped devices.
> > > > > > Unfortunately Intel controller at least is not capable of running
> > > > > > generic SPI transactions. It only supports accessing SPI-NOR flashes and
> > > > > > for those there is small set of commands that supports. I don't think it
> > > > > > is even possible to convert the driver to generic SPI subsystem.
> > > > > 
> > > > > AFAIK it stands for SPI memory device (memory mapped is not a requirement).
> > > > > Eg. spi-nxp-fspi doesn't support generic SPI devices either, but just SPI
> > > > > flashes. So I'd guess SPI MEM is exactly what you are looking for.
> > > > 
> > > > OK, I see that there is ->mem_ops that can be used to implement
> > > > different higher level commands. What I'm not seeing is that how the
> > > > child SPI flash is created using this scheme? DeviceTree and ACPI are
> > > > supported fine but what about scanning? I mean the intel_spi driver has
> > > > this:
> > > > 
> > > >   spi_nor_scan(&ispi->nor, NULL, &hwcaps);
> > > > 
> > > > But if the driver is to be moved under drivers/spi/* you can't really
> > > > call these functions anymore or can you? Or the point is to keep the
> > > > driver under controllers/ and just call spi_nor_scan(), and in addition
> > > > implement the new mem_ops?
> > > > 
> > > > Thanks in advance and sorry about many questions but there does not seem
> > > > to be a conversion guide nor any (non-DT/ACPI) examples that I can take
> > > > a look. :-)
> > > 
> > > Can you provide some guidance here? So in order to use the generic SPI
> > > subsystem with "SPI MEM" parts of it, I would need to be able to create
> > > the child SPI-NOR flash device without using ACPI or DT (as these
> > > systems do not have any ACPI/DT description), or use spi_nor_scan() but
> > > none of the driver under drivers/spi are calling it.
> > 
> > As the main SPI-NOR maintainer, what's your take on this?
> 
> I think this is more of a SPI or SPI MEM question, and less of a SPI NOR 
> question. SPI MEM would call spi_nor_probe() which in turn calls 
> spi_nor_scan().
> 
> So the question that needs to be answered is how to probe SPI MEM based
> drivers without ACPI/DT.

Yes, exactly. With ACPI/DT the SPI core handles this after the SPI
master device is registered and that would result spi_nor_probe() to be
called for the children. However, with this one there is no ACPI node
for the controller (it is PCI enumerated) so there would need to be some
way to create that child device. In the old days that would be "platform
data" but that's pretty much frowned upon these days ;-)

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: spi-nor: intel-spi: Add support for second flash chip

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 599 bytes --]

On Fri, Jun 04, 2021 at 02:28:08PM +0300, Mika Westerberg wrote:

> Yes, exactly. With ACPI/DT the SPI core handles this after the SPI
> master device is registered and that would result spi_nor_probe() to be
> called for the children. However, with this one there is no ACPI node
> for the controller (it is PCI enumerated) so there would need to be some
> way to create that child device. In the old days that would be "platform
> data" but that's pretty much frowned upon these days ;-)

No, that's totally fine and normal - it's just like probing a MFD, we do
it all the time for child devices.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] mtd: spi-nor: intel-spi: Add support for second flash chip

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 599 bytes --]

On Fri, Jun 04, 2021 at 02:28:08PM +0300, Mika Westerberg wrote:

> Yes, exactly. With ACPI/DT the SPI core handles this after the SPI
> master device is registered and that would result spi_nor_probe() to be
> called for the children. However, with this one there is no ACPI node
> for the controller (it is PCI enumerated) so there would need to be some
> way to create that child device. In the old days that would be "platform
> data" but that's pretty much frowned upon these days ;-)

No, that's totally fine and normal - it's just like probing a MFD, we do
it all the time for child devices.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 144 bytes --]

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: spi-nor: intel-spi: Add support for second flash chip

[Mika Westerberg]

On Fri, Jun 04, 2021 at 12:53:39PM +0100, Mark Brown wrote:
> On Fri, Jun 04, 2021 at 02:28:08PM +0300, Mika Westerberg wrote:
> 
> > Yes, exactly. With ACPI/DT the SPI core handles this after the SPI
> > master device is registered and that would result spi_nor_probe() to be
> > called for the children. However, with this one there is no ACPI node
> > for the controller (it is PCI enumerated) so there would need to be some
> > way to create that child device. In the old days that would be "platform
> > data" but that's pretty much frowned upon these days ;-)
> 
> No, that's totally fine and normal - it's just like probing a MFD, we do
> it all the time for child devices.

Okay, thanks! Then I think I have all the questions answered and can try
to convert the driver over the "SPI MEM" framework.

Re: [PATCH] mtd: spi-nor: intel-spi: Add support for second flash chip

[Mika Westerberg]

On Fri, Jun 04, 2021 at 12:53:39PM +0100, Mark Brown wrote:
> On Fri, Jun 04, 2021 at 02:28:08PM +0300, Mika Westerberg wrote:
> 
> > Yes, exactly. With ACPI/DT the SPI core handles this after the SPI
> > master device is registered and that would result spi_nor_probe() to be
> > called for the children. However, with this one there is no ACPI node
> > for the controller (it is PCI enumerated) so there would need to be some
> > way to create that child device. In the old days that would be "platform
> > data" but that's pretty much frowned upon these days ;-)
> 
> No, that's totally fine and normal - it's just like probing a MFD, we do
> it all the time for child devices.

Okay, thanks! Then I think I have all the questions answered and can try
to convert the driver over the "SPI MEM" framework.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/