All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael Walle" <mwalle@kernel.org>
To: "Christian Marangi" <ansuelsmth@gmail.com>,
	"Tudor Ambarus" <tudor.ambarus@linaro.org>,
	"Pratyush Yadav" <pratyush@kernel.org>,
	"Miquel Raynal" <miquel.raynal@bootlin.com>,
	"Richard Weinberger" <richard@nod.at>,
	"Vignesh Raghavendra" <vigneshr@ti.com>,
	<linux-kernel@vger.kernel.org>, <linux-mtd@lists.infradead.org>
Cc: "David Bauer" <mail@david-bauer.net>
Subject: Re: [PATCH v2] mtd: spi-nor: Add support for BoHong bh25q128as
Date: Tue, 20 Feb 2024 21:15:09 +0100	[thread overview]
Message-ID: <CZA6KB8QFMAB.3DOO2VSKCZZMR@kernel.org> (raw)
In-Reply-To: <20240220200325.19089-1-ansuelsmth@gmail.com>

Hi,

On Tue Feb 20, 2024 at 9:03 PM CET, Christian Marangi wrote:
> From: David Bauer <mail@david-bauer.net>
>
> Add MTD support for the BoHong bh25q128as SPI NOR chip.
> The chip has 16MB of total capacity, divided into a total of 256
> sectors, each 64KB sized. The chip also supports 4KB sectors.
> Additionally, it supports dual and quad read modes.

Thanks for your patch!

> Datasheet is public and can be found at the referenced link.
>
> Functionality was verified on an Tenbay WR1800K / MTK MT7621 board.
>
> Link: https://www.e-interlink.com.tw/userUpload/files/BH25Q128AS_v1_0.pdf
> Signed-off-by: David Bauer <mail@david-bauer.net>
> [ reworked to new flash_info format ]
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> ---
> Read/write/erase test are done by flashing and installing an OpenWrt
> image on the spi and by using overlayfs on it confirming correct
> read/write.
>
> root@OpenWrt:/tmp/tmp2# cat /sys/bus/spi/devices/spi0.0/spi-nor/partname 
> bh25q128as
> root@OpenWrt:/tmp/tmp2# cat /sys/bus/spi/devices/spi0.0/spi-nor/jedec_id 
> 684018
> root@OpenWrt:/tmp/tmp2# cat /sys/bus/spi/devices/spi0.0/spi-nor/manufacturer 
> bohong
>
> root@OpenWrt:/# xxd -p /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
> 53464450000101ff00000109300000ff68000103600000ffffffffffffff
> ffffffffffffffffffffffffffffffffffffe520f1ffffffff0744eb086b
> 083b42bbeeffffffffff00ffffff44eb0c200f5210d800ffffffffffffff
> ffffffffffff003600279ef97764fcebffff

Thanks! That's surprising, because in the past thread we were told,
this flash doesn't have SFDP :) Even better then. But..

>
> root@OpenWrt:/# sha256sum /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
> e2e374124e998c9c430a5a4c368ded374186637f48301dcb3943b81af2987995  /sys/bus/spi/devices/spi0.0/spi-np
>
> root@OpenWrt:/tmp/tmp2# hexdump -Cv /sys/bus/spi/devices/spi0.0/spi-nor/sfdp 
> 00000000  53 46 44 50 00 01 01 ff  00 00 01 09 30 00 00 ff  |SFDP........0...|
> 00000010  68 00 01 03 60 00 00 ff  ff ff ff ff ff ff ff ff  |h...`...........|
> 00000020  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
> 00000030  e5 20 f1 ff ff ff ff 07  44 eb 08 6b 08 3b 42 bb  |. ......D..k.;B.|
> 00000040  ee ff ff ff ff ff 00 ff  ff ff 44 eb 0c 20 0f 52  |..........D.. .R|
> 00000050  10 d8 00 ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
> 00000060  00 36 00 27 9e f9 77 64  fc eb ff ff              |.6.'..wd....|
> 0000006c
>
> root@OpenWrt:/tmp/tmp2# sha256sum /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
> e2e374124e998c9c430a5a4c368ded374186637f48301dcb3943b81af2987995  /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
>
> root@OpenWrt:/tmp/tmp2# cat /sys/kernel/debug/spi-nor/spi0.0/capabilities
> Supported read modes by the flash
>  1S-1S-1S
>   opcode	0x03
>   mode cycles	0
>   dummy cycles	0
>  1S-1S-2S
>   opcode	0x3b
>   mode cycles	0
>   dummy cycles	8
>  1S-2S-2S
>   opcode	0xbb
>   mode cycles	2
>   dummy cycles	2
>  1S-1S-4S
>   opcode	0x6b
>   mode cycles	0
>   dummy cycles	8
>  1S-4S-4S
>   opcode	0xeb
>   mode cycles	2
>   dummy cycles	4
>
> Supported page program modes by the flash
>  1S-1S-1S
>   opcode	0x02
>
> root@OpenWrt:/tmp/tmp2# cat /sys/kernel/debug/spi-nor/spi0.0/params
> name		bh25q128as
> id		68 40 18
> size		16.0 MiB
> write size	1
> page size	256
> address nbytes	3
> flags		HAS_16BIT_SR
>
> opcodes
>  read		0x03
>   dummy cycles	0
>  erase		0xd8
>  program	0x02
>  8D extension	none
>
> protocols
>  read		1S-1S-1S
>  write		1S-1S-1S
>  register	1S-1S-1S
>
> erase commands
>  20 (4.00 KiB) [1]
>  52 (32.0 KiB) [2]
>  d8 (64.0 KiB) [3]
>  c7 (16.0 MiB)
>
> sector map
>  region (in hex)   | erase mask | flags
>  ------------------+------------+----------
>  00000000-00ffffff |     [ 123] | 
>
> Changes v2:
> - Drop wrong copyright header
> - Drop name
> - Add requested info about sfpd and others
>
>  drivers/mtd/spi-nor/Makefile |  1 +
>  drivers/mtd/spi-nor/bohong.c | 18 ++++++++++++++++++
>  drivers/mtd/spi-nor/core.c   |  1 +
>  drivers/mtd/spi-nor/core.h   |  1 +
>  4 files changed, 21 insertions(+)
>  create mode 100644 drivers/mtd/spi-nor/bohong.c
>
> diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
> index 5e68468b72fc..c8849cf5124f 100644
> --- a/drivers/mtd/spi-nor/Makefile
> +++ b/drivers/mtd/spi-nor/Makefile
> @@ -2,6 +2,7 @@
>  
>  spi-nor-objs			:= core.o sfdp.o swp.o otp.o sysfs.o
>  spi-nor-objs			+= atmel.o
> +spi-nor-objs			+= bohong.o
>  spi-nor-objs			+= eon.o
>  spi-nor-objs			+= esmt.o
>  spi-nor-objs			+= everspin.o
> diff --git a/drivers/mtd/spi-nor/bohong.c b/drivers/mtd/spi-nor/bohong.c
> new file mode 100644
> index 000000000000..8ba4f3a32040
> --- /dev/null
> +++ b/drivers/mtd/spi-nor/bohong.c
> @@ -0,0 +1,18 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/mtd/spi-nor.h>
> +
> +#include "core.h"
> +
> +static const struct flash_info bohong_parts[] = {
> +	{
> +		.id = SNOR_ID(0x68, 0x40, 0x18),
> +		.size = SZ_16M,
> +		.no_sfdp_flags = SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ,

.. the SFDP will likely make your patch superfluous, because all of
this should be parsed from the SFDP. Did you try your flash without
any flash_info entries at all?

Regarding your question about the names, the identification should
work by the IDs only. Maybe there will be a userspace tool which
will have a flashdb instead. That is a mapping from IDs to
manufacturer and part name.

-michael

> +	},
> +};
> +
> +const struct spi_nor_manufacturer spi_nor_bohong = {
> +	.parts = bohong_parts,
> +	.nparts = ARRAY_SIZE(bohong_parts),
> +};
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 4129764fad8c..29c28ee683a1 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -2037,6 +2037,7 @@ int spi_nor_sr2_bit7_quad_enable(struct spi_nor *nor)
>  
>  static const struct spi_nor_manufacturer *manufacturers[] = {
>  	&spi_nor_atmel,
> +	&spi_nor_bohong,
>  	&spi_nor_eon,
>  	&spi_nor_esmt,
>  	&spi_nor_everspin,
> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> index d36c0e072954..c293568ae827 100644
> --- a/drivers/mtd/spi-nor/core.h
> +++ b/drivers/mtd/spi-nor/core.h
> @@ -601,6 +601,7 @@ struct sfdp {
>  
>  /* Manufacturer drivers. */
>  extern const struct spi_nor_manufacturer spi_nor_atmel;
> +extern const struct spi_nor_manufacturer spi_nor_bohong;
>  extern const struct spi_nor_manufacturer spi_nor_eon;
>  extern const struct spi_nor_manufacturer spi_nor_esmt;
>  extern const struct spi_nor_manufacturer spi_nor_everspin;


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

WARNING: multiple messages have this Message-ID (diff)
From: "Michael Walle" <mwalle@kernel.org>
To: "Christian Marangi" <ansuelsmth@gmail.com>,
	"Tudor Ambarus" <tudor.ambarus@linaro.org>,
	"Pratyush Yadav" <pratyush@kernel.org>,
	"Miquel Raynal" <miquel.raynal@bootlin.com>,
	"Richard Weinberger" <richard@nod.at>,
	"Vignesh Raghavendra" <vigneshr@ti.com>,
	<linux-kernel@vger.kernel.org>, <linux-mtd@lists.infradead.org>
Cc: "David Bauer" <mail@david-bauer.net>
Subject: Re: [PATCH v2] mtd: spi-nor: Add support for BoHong bh25q128as
Date: Tue, 20 Feb 2024 21:15:09 +0100	[thread overview]
Message-ID: <CZA6KB8QFMAB.3DOO2VSKCZZMR@kernel.org> (raw)
In-Reply-To: <20240220200325.19089-1-ansuelsmth@gmail.com>

Hi,

On Tue Feb 20, 2024 at 9:03 PM CET, Christian Marangi wrote:
> From: David Bauer <mail@david-bauer.net>
>
> Add MTD support for the BoHong bh25q128as SPI NOR chip.
> The chip has 16MB of total capacity, divided into a total of 256
> sectors, each 64KB sized. The chip also supports 4KB sectors.
> Additionally, it supports dual and quad read modes.

Thanks for your patch!

> Datasheet is public and can be found at the referenced link.
>
> Functionality was verified on an Tenbay WR1800K / MTK MT7621 board.
>
> Link: https://www.e-interlink.com.tw/userUpload/files/BH25Q128AS_v1_0.pdf
> Signed-off-by: David Bauer <mail@david-bauer.net>
> [ reworked to new flash_info format ]
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> ---
> Read/write/erase test are done by flashing and installing an OpenWrt
> image on the spi and by using overlayfs on it confirming correct
> read/write.
>
> root@OpenWrt:/tmp/tmp2# cat /sys/bus/spi/devices/spi0.0/spi-nor/partname 
> bh25q128as
> root@OpenWrt:/tmp/tmp2# cat /sys/bus/spi/devices/spi0.0/spi-nor/jedec_id 
> 684018
> root@OpenWrt:/tmp/tmp2# cat /sys/bus/spi/devices/spi0.0/spi-nor/manufacturer 
> bohong
>
> root@OpenWrt:/# xxd -p /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
> 53464450000101ff00000109300000ff68000103600000ffffffffffffff
> ffffffffffffffffffffffffffffffffffffe520f1ffffffff0744eb086b
> 083b42bbeeffffffffff00ffffff44eb0c200f5210d800ffffffffffffff
> ffffffffffff003600279ef97764fcebffff

Thanks! That's surprising, because in the past thread we were told,
this flash doesn't have SFDP :) Even better then. But..

>
> root@OpenWrt:/# sha256sum /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
> e2e374124e998c9c430a5a4c368ded374186637f48301dcb3943b81af2987995  /sys/bus/spi/devices/spi0.0/spi-np
>
> root@OpenWrt:/tmp/tmp2# hexdump -Cv /sys/bus/spi/devices/spi0.0/spi-nor/sfdp 
> 00000000  53 46 44 50 00 01 01 ff  00 00 01 09 30 00 00 ff  |SFDP........0...|
> 00000010  68 00 01 03 60 00 00 ff  ff ff ff ff ff ff ff ff  |h...`...........|
> 00000020  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
> 00000030  e5 20 f1 ff ff ff ff 07  44 eb 08 6b 08 3b 42 bb  |. ......D..k.;B.|
> 00000040  ee ff ff ff ff ff 00 ff  ff ff 44 eb 0c 20 0f 52  |..........D.. .R|
> 00000050  10 d8 00 ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
> 00000060  00 36 00 27 9e f9 77 64  fc eb ff ff              |.6.'..wd....|
> 0000006c
>
> root@OpenWrt:/tmp/tmp2# sha256sum /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
> e2e374124e998c9c430a5a4c368ded374186637f48301dcb3943b81af2987995  /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
>
> root@OpenWrt:/tmp/tmp2# cat /sys/kernel/debug/spi-nor/spi0.0/capabilities
> Supported read modes by the flash
>  1S-1S-1S
>   opcode	0x03
>   mode cycles	0
>   dummy cycles	0
>  1S-1S-2S
>   opcode	0x3b
>   mode cycles	0
>   dummy cycles	8
>  1S-2S-2S
>   opcode	0xbb
>   mode cycles	2
>   dummy cycles	2
>  1S-1S-4S
>   opcode	0x6b
>   mode cycles	0
>   dummy cycles	8
>  1S-4S-4S
>   opcode	0xeb
>   mode cycles	2
>   dummy cycles	4
>
> Supported page program modes by the flash
>  1S-1S-1S
>   opcode	0x02
>
> root@OpenWrt:/tmp/tmp2# cat /sys/kernel/debug/spi-nor/spi0.0/params
> name		bh25q128as
> id		68 40 18
> size		16.0 MiB
> write size	1
> page size	256
> address nbytes	3
> flags		HAS_16BIT_SR
>
> opcodes
>  read		0x03
>   dummy cycles	0
>  erase		0xd8
>  program	0x02
>  8D extension	none
>
> protocols
>  read		1S-1S-1S
>  write		1S-1S-1S
>  register	1S-1S-1S
>
> erase commands
>  20 (4.00 KiB) [1]
>  52 (32.0 KiB) [2]
>  d8 (64.0 KiB) [3]
>  c7 (16.0 MiB)
>
> sector map
>  region (in hex)   | erase mask | flags
>  ------------------+------------+----------
>  00000000-00ffffff |     [ 123] | 
>
> Changes v2:
> - Drop wrong copyright header
> - Drop name
> - Add requested info about sfpd and others
>
>  drivers/mtd/spi-nor/Makefile |  1 +
>  drivers/mtd/spi-nor/bohong.c | 18 ++++++++++++++++++
>  drivers/mtd/spi-nor/core.c   |  1 +
>  drivers/mtd/spi-nor/core.h   |  1 +
>  4 files changed, 21 insertions(+)
>  create mode 100644 drivers/mtd/spi-nor/bohong.c
>
> diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
> index 5e68468b72fc..c8849cf5124f 100644
> --- a/drivers/mtd/spi-nor/Makefile
> +++ b/drivers/mtd/spi-nor/Makefile
> @@ -2,6 +2,7 @@
>  
>  spi-nor-objs			:= core.o sfdp.o swp.o otp.o sysfs.o
>  spi-nor-objs			+= atmel.o
> +spi-nor-objs			+= bohong.o
>  spi-nor-objs			+= eon.o
>  spi-nor-objs			+= esmt.o
>  spi-nor-objs			+= everspin.o
> diff --git a/drivers/mtd/spi-nor/bohong.c b/drivers/mtd/spi-nor/bohong.c
> new file mode 100644
> index 000000000000..8ba4f3a32040
> --- /dev/null
> +++ b/drivers/mtd/spi-nor/bohong.c
> @@ -0,0 +1,18 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/mtd/spi-nor.h>
> +
> +#include "core.h"
> +
> +static const struct flash_info bohong_parts[] = {
> +	{
> +		.id = SNOR_ID(0x68, 0x40, 0x18),
> +		.size = SZ_16M,
> +		.no_sfdp_flags = SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ,

.. the SFDP will likely make your patch superfluous, because all of
this should be parsed from the SFDP. Did you try your flash without
any flash_info entries at all?

Regarding your question about the names, the identification should
work by the IDs only. Maybe there will be a userspace tool which
will have a flashdb instead. That is a mapping from IDs to
manufacturer and part name.

-michael

> +	},
> +};
> +
> +const struct spi_nor_manufacturer spi_nor_bohong = {
> +	.parts = bohong_parts,
> +	.nparts = ARRAY_SIZE(bohong_parts),
> +};
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 4129764fad8c..29c28ee683a1 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -2037,6 +2037,7 @@ int spi_nor_sr2_bit7_quad_enable(struct spi_nor *nor)
>  
>  static const struct spi_nor_manufacturer *manufacturers[] = {
>  	&spi_nor_atmel,
> +	&spi_nor_bohong,
>  	&spi_nor_eon,
>  	&spi_nor_esmt,
>  	&spi_nor_everspin,
> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> index d36c0e072954..c293568ae827 100644
> --- a/drivers/mtd/spi-nor/core.h
> +++ b/drivers/mtd/spi-nor/core.h
> @@ -601,6 +601,7 @@ struct sfdp {
>  
>  /* Manufacturer drivers. */
>  extern const struct spi_nor_manufacturer spi_nor_atmel;
> +extern const struct spi_nor_manufacturer spi_nor_bohong;
>  extern const struct spi_nor_manufacturer spi_nor_eon;
>  extern const struct spi_nor_manufacturer spi_nor_esmt;
>  extern const struct spi_nor_manufacturer spi_nor_everspin;


  reply	other threads:[~2024-02-20 20:15 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-20 20:03 [PATCH v2] mtd: spi-nor: Add support for BoHong bh25q128as Christian Marangi
2024-02-20 20:03 ` Christian Marangi
2024-02-20 20:15 ` Michael Walle [this message]
2024-02-20 20:15   ` Michael Walle
2024-02-22  9:19 ` Tudor Ambarus
2024-02-22  9:19   ` Tudor Ambarus
2024-02-24 11:18   ` Christian Marangi
2024-02-24 11:18     ` Christian Marangi
2024-02-26  8:51     ` Michael Walle
2024-02-26  8:51       ` Michael Walle
2024-02-26  8:53       ` Tudor Ambarus
2024-02-26  8:53         ` Tudor Ambarus
2024-02-27  9:33         ` Christian Marangi
2024-02-27  9:33           ` Christian Marangi
2024-02-27 10:08           ` Tudor Ambarus
2024-02-27 10:08             ` Tudor Ambarus

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CZA6KB8QFMAB.3DOO2VSKCZZMR@kernel.org \
    --to=mwalle@kernel.org \
    --cc=ansuelsmth@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=mail@david-bauer.net \
    --cc=miquel.raynal@bootlin.com \
    --cc=pratyush@kernel.org \
    --cc=richard@nod.at \
    --cc=tudor.ambarus@linaro.org \
    --cc=vigneshr@ti.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.