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

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

[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

[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

[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.