Re: [PATCH 2/2] configs: spacemit: k3: enable mtdparts on Pico-ITX in main app

[Yixun Lan <dlan@kernel.org>]

Hi E

On 07:35 Wed 21 Oct     , E Shattow wrote:
> Hi Yixun,
> 
> On 10/21/26 00:56, Yixun Lan wrote:
> > Hi E Shattow,
> > 
> > 
> > On 12:51 Tue 20 Oct     , E Shattow wrote:
> >> add default mtdparts and mtdids consistent with k3-pico-itx.dts
> >>
> >> Signed-off-by: E Shattow <e@freeshell.de>
> >> ---
> >>  configs/spacemit_k3_defconfig | 5 +++++
> >>  1 file changed, 5 insertions(+)
> >>
> >> diff --git a/configs/spacemit_k3_defconfig b/configs/spacemit_k3_defconfig
> >> index b333e708d01b..2ca912556a88 100644
> >> --- a/configs/spacemit_k3_defconfig
> >> +++ b/configs/spacemit_k3_defconfig
> >> @@ -26,6 +26,7 @@ CONFIG_SYS_PBSIZE=276
> >>  CONFIG_HUSH_PARSER=y
> >>  CONFIG_CMD_GPIO=y
> >>  CONFIG_CMD_I2C=y
> >> +CONFIG_CMD_MTD=y
> >>  CONFIG_CMD_DHCP=y
> >>  CONFIG_CMD_DNS=y
> >>  CONFIG_CMD_MII=y
> >> @@ -36,6 +37,9 @@ CONFIG_CMD_PMIC=y
> >>  CONFIG_CMD_REGULATOR=y
> >>  CONFIG_CMD_FAT=y
> >>  CONFIG_CMD_FS_GENERIC=y
> >> +CONFIG_CMD_MTDPARTS=y
> >> +CONFIG_MTDIDS_DEFAULT="nor0=spi0.0"
> >> +CONFIG_MTDPARTS_DEFAULT="spi0.0:128k(bootinfo),512k(fsbl),64k(env),1024k(esos),384k(opensbi),-(uboot)"
> 
> > or make it more specific? s/spi0.0/d420c000.spi/ 
> 
> Yes and no, I do prefer an automatic layout (and layout name) be
> generated from device tree at runtime. I guess that may possibly become
> "qspi.flash0", or "fdt.soc.d420c000.spi.qspi.flash.0", etc.
> 
> The output of 'mtd list' tells us the parent:
> 
> => mtd list
> List of MTD devices:
> * nor0
>   - device: flash@0
>   - parent: spi@d420c000
> 
> Also, then, the output of 'mtdparts' tells us the partition layout:
> 
> => mtdpart
> 
> device nor0 <spi0.0>, # parts = 6
>  #: name                size            offset          mask_flags
>  0: bootinfo            0x00020000      0x00000000      0
>  1: fsbl                0x00080000      0x00020000      0
>  2: env                 0x00010000      0x000a0000      0
>  3: esos                0x00100000      0x000b0000      0
>  4: opensbi             0x00060000      0x001b0000      0
>  5: uboot               0x005f0000      0x00210000      0
> 
> active partition: nor0,0 - (bootinfo) 0x00020000 @ 0x00000000
> 
> defaults:
> mtdids  : nor0=spi0.0
> mtdparts:
> spi0.0:128k(bootinfo),512k(fsbl),64k(env),1024k(esos),384k(opensbi),-(uboot)
> 
> I consider the partition layout identifier style as you suggest:
> 
> $ egrep --exclude-dir=.git -or 'mtdparts=[^ ]+)' | cut -d: -f2 | sort |
> uniq -c | egrep -v '[0-9a-fA-F]{7}' | sort -k1,1 -n
> 
> ...
>       3 mtdparts=nand_mtd
>       3 mtdparts=uniphier-nand.0
>       4 mtdparts=nand0
>       4 mtdparts=spi-flash.0
>       5 mtdparts=nor0
>       5 mtdparts=spi_flash
>       7 mtdparts=boot
>       7 mtdparts=omap2-nand.0
>       8 mtdparts=nand
>      11 mtdparts=nand.0
>      12 mtdparts=orion_nand
>      13 mtdparts=spi0.0
>      16 mtdparts=gpmi-nand
>      50 mtdparts=atmel_nand
> 
> 
> $ egrep --exclude-dir=.git -or 'mtdparts=[^ ]+)' | cut -d: -f2 | sort |
> uniq -c | egrep '[0-9a-fA-F]{7}' | sort -k1,1 -n
>       1 mtdparts=10b80000.nand.0
>       1 mtdparts=1fc000000.nor_flash
>       1 mtdparts=21e0000.qspi
>       1 mtdparts=30bb0000.qspi
>       1 mtdparts=30bb0000.spi
>       1 mtdparts=8000000.nand
>       1 mtdparts=fe000000.nor_flash
>       1 mtdparts=ffb90000.nand.0
>       1 mtdparts=mtdparts=47040000.spi.0
>       1 mtdparts=[mtdparts=]edb7312-nor
>       1 mtdparts=mtdparts=edb7312-nor
>       2 mtdparts=02008000.spi.1
>       2 mtdparts=8000000.nor
>       4 mtdparts=1550000.spi
>       4 mtdparts=1550000.spi-0
>       4 mtdparts=47040000.spi.0
>       4 mtdparts=ec000000.nor
>       4 mtdparts=ef000000.nor
>       4 mtdparts=fef000000.nor
>      10 mtdparts=ff705000.spi.0
>      16 mtdparts=60000000.nor
>      16 mtdparts=ff800000.flash
>      22 mtdparts=fe8000000.nor
> 
> 
> For the initial patch here I instead prefer the minimal and simple style
> for mtdparts layout template names and to re-use what is popular in
> U-Boot source tree. The use of hexadecimal bus address for the layout
> name is difficult for me to read, and not clear to me what is the
> benefit to doing this manually-entered data address?
> 
> => env print
> ...
> mtddevnum=0
> mtdids=nor0=spi0.0
> mtdparts=spi0.0:128k(bootinfo),512k(fsbl),64k(env),1024k(esos),384k(opensbi),-(uboot)
> partition=nor0,0
> ...
> 
> Compared to a style of repeating the bus address:
> 
> => env print
> ...
> mtddevnum=0
> mtdids=nor0=d420c000.spi
> mtdparts=d420c000.spi:128k(bootinfo),512k(fsbl),64k(env),1024k(esos),384k(opensbi),-(uboot)
> partition=nor0,0
> ...
> 
> Let me know if you do yet prefer the bus address repeated for the name
> of partition layout. I'd like to see this as part of a runtime-generated
> mtdparts configuration, so anything is okay with me for the initial
> manual layout here.
> 
I've done a further check.. so quote from <uboot>/cmd/mtdparts.c

 * mtdparts=[mtdparts=]<mtd-def>[;<mtd-def>...]
 *
 * <mtd-def>  := <mtd-id>:<part-def>[,<part-def>...]
 * <mtd-id>   := unique device tag used by linux kernel to find mtd device (mtd->name)

from kernel side, check file drivers/mtd/parsers/cmdlinepart.c which
has the same format, on the linux console (if no mtdparts enabled)

# cat /sys/class/mtd/mtd0/name
d420c000.spi

so with bus address it's able to support cmdlinepart, although we
currently use ofpart, but IMO, it's correct and in case people need it

> > 
> >>  CONFIG_DEVICE_TREE_INCLUDES="spacemit-k3-u-boot.dtsi"
> >>  CONFIG_ENV_OVERWRITE=y
> >>  CONFIG_ENV_IS_IN_SPI_FLASH=y
> >> @@ -49,6 +53,7 @@ CONFIG_DM_I2C=y
> >>  CONFIG_SYS_I2C_SPACEMIT_K1=y
> >>  CONFIG_MISC=y
> >>  CONFIG_SPI_FLASH_WINBOND=y
> >> +CONFIG_SPI_FLASH_MTD=y
> >>  CONFIG_PHY_GIGE=y
> >>  CONFIG_DWC_ETH_QOS=y
> >>  CONFIG_DWC_ETH_QOS_SPACEMIT=y
> >> -- 
> >> 2.53.0
> >>
> > 
> 
> Thanks,
> 
> -E

-- 
Yixun Lan (dlan)