Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Michael Walle <michael@walle.cc>
To:     f.suligoi@asem.it
Cc:     linux-kernel@vger.kernel.org, linux-mtd@lists.infradead.org,
        miquel.raynal@bootlin.com, richard@nod.at,
        tudor.ambarus@microchip.com, vigneshr@ti.com, michael@walle.cc,
        Pratyush Yadav <p.yadav@ti.com>
Subject: Re: [PATCH v1] mtd: spi-nor: add ACPI support for non-JEDEC SPI-NOR
Date:   Mon, 31 May 2021 09:44:24 +0200
Message-Id: <20210531074424.16086-1-michael@walle.cc>
In-Reply-To: <20210126143748.1546187-1-f.suligoi@asem.it>
References: <20210126143748.1546187-1-f.suligoi@asem.it>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

Hi,

> In a x86 machine, an additional device can be described
> inside the BIOS ACPI tables.
> For example, an I2C GPIO expander, a LED, etc.,
> can be successfully declared in ACPI, so that the related
> device driver can start automatically at the boot.
> 
> But for the SPI NOR devices, the ACPI description works
> for JEDEC-compatible devices only.
> 
> For example, an Everspin MR25H40 MRAM device (non-JEDEC
> compatible), declared using the following table,
> does not work:
> 
>     Scope (\_SB.SPI1)
>     {
>         Device (NVR0)
>         {
>             Name (_HID, "PRP0001")
>             Name (_DDN, "Everspin MR25H40 MRAM")
>             Name (_CRS, ResourceTemplate () {
>                 SpiSerialBus (
>                 1,                      // Chip select
>                 PolarityLow,            // Chip select is active low
>                 FourWireMode,           // Full duplex
>                 8,                      // Bits per word is 8 (byte)
>                 ControllerInitiated,    // Don't care
>                 10000000,               // 10 MHz
>                 ClockPolarityLow,       // SPI mode 0 ClockPolarityLow
>                 ClockPhaseFirst,        // SPI mode 0 ClockPhaseFirst
>                 "\\_SB.SPI1",           // SPI host controller
>                 0,                      // Must be 0
>                 ResourceConsumer,
>                 ,
>                 )
>             })
> 
>             Name (_DSD, Package () {
>                 ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>                 Package () {
>                     Package () {"compatible", "mr25h40"},
>                 }
>             })
>         }
>     }

This isn't the correct compatible string, according to the devicetree
binding [1], there is no compatible = "mr25h40" binding. The correct entry
would be
 compatible = "everspin,mr25h40", "jedec,spi-nor";

I'm not familiar with ACPI and don't know wether you can specify a list,
nor am I convinced that it will work then. But it should be worth a try.

[1] Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml

> 
> To enable the detection of a non-JEDEC device described
> in a BIOS ACPI table, it is necessary to add its id name
> in the spi_nor_of_table structure.
> With this change, all the SPI NOR devices (JEDEC and
> non-JEDEC) can be detected by the kernel (and the above
> example of ACPI table finally works).
> 
> Signed-off-by: Flavio Suligoi <f.suligoi@asem.it>
> ---
>  drivers/mtd/spi-nor/core.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 6ae7d4c2d2b6..b6fb8b15c439 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -3755,6 +3755,19 @@ static const struct of_device_id spi_nor_of_table[] = {
>  	 * JEDEC READ ID opcode (0x9F). Use this, if possible.
>  	 */
>  	{ .compatible = "jedec,spi-nor" },
> +	{ .compatible = "m25p05-nonjedec" },
> +	{ .compatible = "m25p10-nonjedec" },
> +	{ .compatible = "m25p20-nonjedec" },
> +	{ .compatible = "m25p40-nonjedec" },
> +	{ .compatible = "m25p80-nonjedec" },
> +	{ .compatible = "m25p16-nonjedec" },
> +	{ .compatible = "m25p32-nonjedec" },
> +	{ .compatible = "m25p64-nonjedec" },
> +	{ .compatible = "m25p128-nonjedec" },
> +	{ .compatible = "mr25h128" },
> +	{ .compatible = "mr25h256" },
> +	{ .compatible = "mr25h10" },
> +	{ .compatible = "mr25h40" },
>  	{ /* sentinel */ },
>  };
>  MODULE_DEVICE_TABLE(of, spi_nor_of_table);
> -- 
> 2.25.1