Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <130b3742-fe0b-2030-654f-0cf8863c1479@linaro.org>
Date:   Fri, 21 Jul 2023 10:38:51 +0300
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.13.0
Subject: Re: [PATCH V1 1/1] linux: drivers: mtd: spi-nor: gigadevice.c
Content-Language: en-US
To:     Vlim <vlim@gigadevice.com>, Michael Walle <michael@walle.cc>
Cc:     "pratyush@kernel.org" <pratyush@kernel.org>,
        "miquel.raynal@bootlin.com" <miquel.raynal@bootlin.com>,
        "richard@nod.at" <richard@nod.at>,
        "vigneshr@ti.com" <vigneshr@ti.com>,
        "akumarma@amd.com" <akumarma@amd.com>,
        "linux-mtd@lists.infradead.org" <linux-mtd@lists.infradead.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "vikhyat.goyal@amd.com" <vikhyat.goyal@amd.com>
References: <20230720042834.12296-1-vlim@gigadevice.com>
 <62663146a51c2beac4eff48abe1f61f8@walle.cc>
 <TY0PR06MB56580D3FEE55389F41E33398BB3EA@TY0PR06MB5658.apcprd06.prod.outlook.com>
 <55d60aec-3b20-d9d3-aeb2-77abbcc47d16@linaro.org>
 <TY0PR06MB56581B9A28DCD7A7270CFC0ABB3FA@TY0PR06MB5658.apcprd06.prod.outlook.com>
From:   Tudor Ambarus <tudor.ambarus@linaro.org>
In-Reply-To: <TY0PR06MB56581B9A28DCD7A7270CFC0ABB3FA@TY0PR06MB5658.apcprd06.prod.outlook.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk


On 21.07.2023 09:51, Vlim wrote:
> Thanks, Tudor,
> 

you're welcome

> Is PARSE_SFDP the command to read the SFDP parameters?

it's a flag, not a command, that when set at probe time the SFDP will be
read.

> It looks to me that the examples below are both using SFDP. But I do not
> see the flag that saying SFDP is used.

not quite

> 
> { "gd25q128", INFO(0xc84018, 0, 64 * 1024, 256)
>             FLAGS(SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB)

The locking support is not part of the JEDEC SFDP standard, but vendors
can define their own SFDP tables where they describe the locking
support. So for now, if you want to enable locking you have to specify
the locking flags, whatever they are. There are different flavors of
block protection locking, with 3 or 4 block protection bits, their
order, etc. You'll have to check the datasheet.

>             NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ |
>                         SPI_NOR_QUAD_READ) },

If the flash supports SFDP then the flags from above should be replaced
with PARSE_SFDP.

We have some requirements when one updates or adds new flash entries.
They have to dump the sysfs data, including the SFDP tables, and do some
sanity check with mtd_utils. Check the following cover letter to find
out how to do it:
https://lore.kernel.org/linux-mtd/cover.1686557139.git.Takahiro.Kuwano@infineon.com/

>       { "gd25q256", INFO(0xc84019, 0, 64 * 1024, 512)
>             PARSE_SFDP
>             FLAGS(SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | SPI_NOR_TB_SR_BIT6)
>             FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
>             .fixups = &gd25q256_fixups },

these are all different types of flags. Use git blame to find the reason
why some were set.

Cheers,
ta