mtd: spi-nor: fix flash probing

[PATCH] mtd: spi-nor: fix flash probing

Posted by Michael Walle 1 year, 5 months ago

Fix flash probing by name. Flash entries without a name are allowed
since commit 15eb8303bb42 ("mtd: spi-nor: mark the flash name as
obsolete"). But it was just until recently that a flash entry without a
name was actually introduced. This triggers a bug in the legacy probe by
name path. Skip entries without a name to fix it.

Fixes: 2095e7da8049 ("mtd: spi-nor: spansion: Add support for S28HS256T")
Reported-by: Jon Hunter <jonathanh@nvidia.com>
Closes: https://lore.kernel.org/r/66c8ebb0-1324-4ad9-9926-8d4eb7e1e63a@nvidia.com/
Tested-by: Jon Hunter <jonathanh@nvidia.com>
Signed-off-by: Michael Walle <mwalle@kernel.org>
---
 drivers/mtd/spi-nor/core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index d8e551fd2e2a..101ee5b0ddeb 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -3281,7 +3281,8 @@ static const struct flash_info *spi_nor_match_name(struct spi_nor *nor,
 
 	for (i = 0; i < ARRAY_SIZE(manufacturers); i++) {
 		for (j = 0; j < manufacturers[i]->nparts; j++) {
-			if (!strcmp(name, manufacturers[i]->parts[j].name)) {
+			if (manufacturers[i]->parts[j].name &&
+			    !strcmp(name, manufacturers[i]->parts[j].name)) {
 				nor->manufacturer = manufacturers[i];
 				return &manufacturers[i]->parts[j];
 			}
-- 
2.39.2

Re: [PATCH] mtd: spi-nor: fix flash probing

Posted by Pratyush Yadav 1 year, 5 months ago

Hi Michael,

Thanks for the quick fix!

On Mon, Sep 09 2024, Michael Walle wrote:

> Fix flash probing by name. Flash entries without a name are allowed
> since commit 15eb8303bb42 ("mtd: spi-nor: mark the flash name as
> obsolete"). But it was just until recently that a flash entry without a
> name was actually introduced. This triggers a bug in the legacy probe by
> name path. Skip entries without a name to fix it.

We also use name in a couple other places:

1. spi_nor_get_flash_info() prints jinfo->name and info->name() in the
   dev_warn() call.

2. spi_nor_params_show() prints nor->info->name for the debugfs entry.

Since both of these are prints, it should probably be fine since they
should be converted to "(null)" instead. Still, something we should fix.
I don't think those are critical so those can be fixed as follow up
patches. I unfortunately am swamped and don't have the time to fix them
right now.

I also did a quick check to make sure we don't have other flashes
without a name.

Reviewed-by: Pratyush Yadav <pratyush@kernel.org>

Applied to spi-nor/next. Thanks!

>
> Fixes: 2095e7da8049 ("mtd: spi-nor: spansion: Add support for S28HS256T")
> Reported-by: Jon Hunter <jonathanh@nvidia.com>
> Closes: https://lore.kernel.org/r/66c8ebb0-1324-4ad9-9926-8d4eb7e1e63a@nvidia.com/
> Tested-by: Jon Hunter <jonathanh@nvidia.com>
> Signed-off-by: Michael Walle <mwalle@kernel.org>
[...]

-- 
Regards,
Pratyush Yadav

Re: [PATCH] mtd: spi-nor: fix flash probing

Posted by Tudor Ambarus 1 year, 5 months ago


On 9/9/24 8:28 AM, Michael Walle wrote:
> Fix flash probing by name. Flash entries without a name are allowed
> since commit 15eb8303bb42 ("mtd: spi-nor: mark the flash name as
> obsolete"). But it was just until recently that a flash entry without a
> name was actually introduced. This triggers a bug in the legacy probe by
> name path. Skip entries without a name to fix it.
> 
> Fixes: 2095e7da8049 ("mtd: spi-nor: spansion: Add support for S28HS256T")
> Reported-by: Jon Hunter <jonathanh@nvidia.com>
> Closes: https://lore.kernel.org/r/66c8ebb0-1324-4ad9-9926-8d4eb7e1e63a@nvidia.com/
> Tested-by: Jon Hunter <jonathanh@nvidia.com>
> Signed-off-by: Michael Walle <mwalle@kernel.org>

Reviewed-by: Tudor Ambarus <tudor.ambarus@linaro.org>

> ---
>  drivers/mtd/spi-nor/core.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index d8e551fd2e2a..101ee5b0ddeb 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -3281,7 +3281,8 @@ static const struct flash_info *spi_nor_match_name(struct spi_nor *nor,
>  
>  	for (i = 0; i < ARRAY_SIZE(manufacturers); i++) {
>  		for (j = 0; j < manufacturers[i]->nparts; j++) {
> -			if (!strcmp(name, manufacturers[i]->parts[j].name)) {
> +			if (manufacturers[i]->parts[j].name &&
> +			    !strcmp(name, manufacturers[i]->parts[j].name)) {
>  				nor->manufacturer = manufacturers[i];
>  				return &manufacturers[i]->parts[j];
>  			}

Re: [PATCH] mtd: spi-nor: fix flash probing

Posted by Miquel Raynal 1 year, 5 months ago

Hey,

tudor.ambarus@linaro.org wrote on Mon, 9 Sep 2024 08:35:23 +0100:

> On 9/9/24 8:28 AM, Michael Walle wrote:
> > Fix flash probing by name. Flash entries without a name are allowed
> > since commit 15eb8303bb42 ("mtd: spi-nor: mark the flash name as
> > obsolete"). But it was just until recently that a flash entry without a
> > name was actually introduced. This triggers a bug in the legacy probe by
> > name path. Skip entries without a name to fix it.
> > 
> > Fixes: 2095e7da8049 ("mtd: spi-nor: spansion: Add support for S28HS256T")
> > Reported-by: Jon Hunter <jonathanh@nvidia.com>
> > Closes: https://lore.kernel.org/r/66c8ebb0-1324-4ad9-9926-8d4eb7e1e63a@nvidia.com/
> > Tested-by: Jon Hunter <jonathanh@nvidia.com>
> > Signed-off-by: Michael Walle <mwalle@kernel.org>  
> 
> Reviewed-by: Tudor Ambarus <tudor.ambarus@linaro.org>

This patch will only apply on top of spi-nor/next. It would spend more
time in -next if one of you applied it to this branch before sending
the MR?

Cheers,
Miquèl

Re: [PATCH] mtd: spi-nor: fix flash probing

Posted by Tudor Ambarus 1 year, 5 months ago


On 9/9/24 8:52 AM, Miquel Raynal wrote:
> Hey,
> 
Hiya

> tudor.ambarus@linaro.org wrote on Mon, 9 Sep 2024 08:35:23 +0100:
> 
>> On 9/9/24 8:28 AM, Michael Walle wrote:
>>> Fix flash probing by name. Flash entries without a name are allowed
>>> since commit 15eb8303bb42 ("mtd: spi-nor: mark the flash name as
>>> obsolete"). But it was just until recently that a flash entry without a
>>> name was actually introduced. This triggers a bug in the legacy probe by
>>> name path. Skip entries without a name to fix it.
>>>
>>> Fixes: 2095e7da8049 ("mtd: spi-nor: spansion: Add support for S28HS256T")
>>> Reported-by: Jon Hunter <jonathanh@nvidia.com>
>>> Closes: https://lore.kernel.org/r/66c8ebb0-1324-4ad9-9926-8d4eb7e1e63a@nvidia.com/
>>> Tested-by: Jon Hunter <jonathanh@nvidia.com>
>>> Signed-off-by: Michael Walle <mwalle@kernel.org>  
>>
>> Reviewed-by: Tudor Ambarus <tudor.ambarus@linaro.org>
> 
> This patch will only apply on top of spi-nor/next. It would spend more
> time in -next if one of you applied it to this branch before sending
> the MR?
> 

The patch is intended for spi-nor/next indeed, as the blamed commit was
just recently queued. No need to take it through mtd/fixes. Pratyush
will take it via spi-nor/next.

Cheers,
ta

Re: [PATCH] mtd: spi-nor: fix flash probing

Posted by Miquel Raynal 1 year, 5 months ago

Hi Tudor,

> >>> Fixes: 2095e7da8049 ("mtd: spi-nor: spansion: Add support for S28HS256T")
> >>> Reported-by: Jon Hunter <jonathanh@nvidia.com>
> >>> Closes: https://lore.kernel.org/r/66c8ebb0-1324-4ad9-9926-8d4eb7e1e63a@nvidia.com/
> >>> Tested-by: Jon Hunter <jonathanh@nvidia.com>
> >>> Signed-off-by: Michael Walle <mwalle@kernel.org>    
> >>
> >> Reviewed-by: Tudor Ambarus <tudor.ambarus@linaro.org>  
> > 
> > This patch will only apply on top of spi-nor/next. It would spend more
> > time in -next if one of you applied it to this branch before sending
> > the MR?
> >   
> 
> The patch is intended for spi-nor/next indeed, as the blamed commit was
> just recently queued. No need to take it through mtd/fixes. Pratyush
> will take it via spi-nor/next.

Ok perfect, thanks!

Miquèl

Re: [PATCH] mtd: spi-nor: fix flash probing

Posted by Pratyush Yadav 1 year, 5 months ago

On Mon, Sep 09 2024, Miquel Raynal wrote:

> Hi Tudor,
>
>> >>> Fixes: 2095e7da8049 ("mtd: spi-nor: spansion: Add support for S28HS256T")
>> >>> Reported-by: Jon Hunter <jonathanh@nvidia.com>
>> >>> Closes: https://lore.kernel.org/r/66c8ebb0-1324-4ad9-9926-8d4eb7e1e63a@nvidia.com/
>> >>> Tested-by: Jon Hunter <jonathanh@nvidia.com>
>> >>> Signed-off-by: Michael Walle <mwalle@kernel.org>    
>> >>
>> >> Reviewed-by: Tudor Ambarus <tudor.ambarus@linaro.org>  
>> > 
>> > This patch will only apply on top of spi-nor/next. It would spend more
>> > time in -next if one of you applied it to this branch before sending
>> > the MR?
>> >   
>> 
>> The patch is intended for spi-nor/next indeed, as the blamed commit was
>> just recently queued. No need to take it through mtd/fixes. Pratyush
>> will take it via spi-nor/next.

Right.

I have applied this to spi-nor/next. I plan to send the pull request by
Wednesday/Thursday so it should get at least a couple days of testing in
linux-next.

>
> Ok perfect, thanks!

-- 
Regards,
Pratyush Yadav