[PATCH u-boot-dm + u-boot-spi v2 0/7] Support SPI NORs and OF partitions in `mtd list`

[Patrice CHOTARD]

Hi Marek

On 2/10/21 3:14 PM, Marek Behun wrote:
> On Wed, 10 Feb 2021 14:48:23 +0100
> Patrice CHOTARD <patrice.chotard at foss.st.com> wrote:
> 
>> +Patrick
>>
>> Hi All
>>
>> I have tested this series on stm32mp1-ev1 board which embeds 1 nand and 2 identical spi-nor. 
>> spi-nor DT node is the following:
>>
>> &qspi {
>> 	pinctrl-names = "default", "sleep";
>> 	pinctrl-0 = <&qspi_clk_pins_a &qspi_bk1_pins_a &qspi_bk2_pins_a>;
>> 	pinctrl-1 = <&qspi_clk_sleep_pins_a &qspi_bk1_sleep_pins_a &qspi_bk2_sleep_pins_a>;
>> 	reg = <0x58003000 0x1000>, <0x70000000 0x4000000>;
>> 	#address-cells = <1>;
>> 	#size-cells = <0>;
>> 	status = "okay";
>>
>> 	flash0: mx66l51235l at 0 {
>> 		compatible = "jedec,spi-nor";
>> 		reg = <0>;
>> 		spi-rx-bus-width = <4>;
>> 		spi-max-frequency = <108000000>;
>> 		#address-cells = <1>;
>> 		#size-cells = <1>;
>> 	};
>>
>> 	flash1: mx66l51235l at 1 {
>> 		compatible = "jedec,spi-nor";
>> 		reg = <1>;
>> 		spi-rx-bus-width = <4>;
>> 		spi-max-frequency = <108000000>;
>> 		#address-cells = <1>;
>> 		#size-cells = <1>;
>> 	};
>> };
>>
>> As is, this series is not able to differentiate the both identical spi-nor using the MTD framework. 
>> Both spi-nor have the same name "mx66l51235l" as shown below, and usage of mtd command is not possible to reach
>> both spi-nor:
>>
>> STM32MP> mtd list  
>> List of MTD devices:
>> * nand0
>>   - type: NAND flash
>>   - block size: 0x40000 bytes
>>   - min I/O: 0x1000 bytes
>>   - OOB size: 224 bytes
>>   - OOB available: 118 bytes
>>   - ECC strength: 8 bits
>>   - ECC step size: 512 bytes
>>   - bitflip threshold: 6 bits
>>   - 0x000000000000-0x000040000000 : "nand0"
>> * mx66l51235l
>>   - device: mx66l51235l at 0
>>   - parent: spi at 58003000
>>   - driver: jedec_spi_nor
>>   - type: NOR flash
>>   - block size: 0x10000 bytes
>>   - min I/O: 0x1 bytes
>>   - 0x000000000000-0x000004000000 : "mx66l51235l"
>> * mx66l51235l
>>   - device: mx66l51235l at 1
>>   - parent: spi at 58003000
>>   - driver: jedec_spi_nor
>>   - type: NOR flash
>>   - block size: 0x10000 bytes
>>   - min I/O: 0x1 bytes
>>   - 0x000000000000-0x000004000000 : "mx66l51235l"
>>
>> The following patch fix it :
>>
>> @@ -2528,11 +2528,11 @@ int spi_nor_scan(struct spi_nor *nor)
>>  	ret = spi_nor_init_params(nor, info, &params);
>>  	if (ret)
>>  		return ret;
>>  
>>  	if (!mtd->name)
>> -		mtd->name = info->name;
>> +		mtd->name = (char *)nor->dev->name;
>>  	mtd->dev = nor->dev;
>>  	mtd->priv = nor;
>>  	mtd->type = MTD_NORFLASH;
>>  	mtd->writesize = 1;
>>  	mtd->flags = MTD_CAP_NORFLASH;
>>
>>
>>
>> Now it looks like :
>>
>> STM32MP> mtd list  
>> List of MTD devices:
>> * nand0
>>   - type: NAND flash
>>   - block size: 0x40000 bytes
>>   - min I/O: 0x1000 bytes
>>   - OOB size: 224 bytes
>>   - OOB available: 118 bytes
>>   - ECC strength: 8 bits
>>   - ECC step size: 512 bytes
>>   - bitflip threshold: 6 bits
>>   - 0x000000000000-0x000040000000 : "nand0"
>> * mx66l51235l at 0
>>   - device: mx66l51235l at 0
>>   - parent: spi at 58003000
>>   - driver: jedec_spi_nor
>>   - type: NOR flash
>>   - block size: 0x10000 bytes
>>   - min I/O: 0x1 bytes
>>   - 0x000000000000-0x000004000000 : "mx66l51235l at 0"
>> * mx66l51235l at 1
>>   - device: mx66l51235l at 1
>>   - parent: spi at 58003000
>>   - driver: jedec_spi_nor
>>   - type: NOR flash
>>   - block size: 0x10000 bytes
>>   - min I/O: 0x1 bytes
>>   - 0x000000000000-0x000004000000 : "mx66l51235l at 1"
>> STM32MP>   
>>
>> Everything goes fine, i observed one side effect regarding "sf probe" command:
>>
>> STM32MP> sf probe  
>> SF: Detected mx66l51235l at 0 with page size 256 Bytes, erase size 64 KiB, total 64 MiB
>> STM32MP> sf probe 1    
>> SF: Detected mx66l51235l at 1 with page size 256 Bytes, erase size 64 KiB, total 64 MiB
>>
>> Here the flash name is mx66l51235l at 0 or mx66l51235l at 1 and no more mx66l51235l.
> 
> I too tried that change (using (char *)nor->dev->name as mtd name).
> 
> The thing is that the name "mx66l51235l" is determined from the SPI NOR
> ID (not the device-tree), so if you solder a different NOR, the name
> will be different. Meanwhile dts name is always the same.

Ok got it, i didn't pay attention to the fact nor->dev->name doesn't come from spi-nor-ids but from DT .... :-(

> 
> I think the mtd command should at least inform the user what kind of SPI
> NOR is soldered on the board.

Fully agree

> 
> So one solution is to use device-tree name to select the NOR, but still
> print the name determined from SPI ID somewhere.
> 
> Second solution is to allow selecting the SPI by number, i.e. (the n-th
> enumerated SPI).

and what about <name from SPI id>@<chip select> as shown in my previous email ?

> 
> (BTW regarding your dts node names, the node names should be spi-nor at 0
>  and spi-nor at 1. Yes, I know that spi-nor is not documented in
>  device-tree bindings yet, but this would be consistent with other
>  device-tree bindings in Linux.)

You are right

Patrice

> 
> Marek
>