| Message ID | 20240909072854.812206-1-mwalle@kernel.org |
|---|---|
| State | Accepted |
| Delegated to: | Pratyush Yadav |
| Headers | show |
| Series | mtd: spi-nor: fix flash probing | expand |
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]; > }
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
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
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
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> [...]
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!
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]; }
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(-)