The flash name is unreliable as we saw flash ID collisions. Mark the
flash name as obsolete and print the manufacturer and device ID where
name was printed.
JaimeLiao (1):
mtd: spi-nor: sysfs: hide the flash name if not set
Tudor Ambarus (3):
mtd: spi-nor: print flash ID instead of name
mtd: spi-nor: mark the flash name as obsolete
mtd: spi-nor: drop superfluous debug prints
.../ABI/testing/sysfs-bus-spi-devices-spi-nor | 3 +++
drivers/mtd/spi-nor/core.c | 22 +++----------------
drivers/mtd/spi-nor/core.h | 2 +-
drivers/mtd/spi-nor/sysfs.c | 2 ++
4 files changed, 9 insertions(+), 20 deletions(-)
--
2.34.1
We saw flash ID collisions which make the flash name unreliable. Print
the manufacgturer and device ID instead of the flash name.
Lower the print to dev_dbg to stop polluting the kernel log.
Suggested-by: Miquel Raynal <[email protected]>
Signed-off-by: Tudor Ambarus <[email protected]>
---
drivers/mtd/spi-nor/core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 503fed90c2fa..ca5bd93d1f17 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -3558,8 +3558,8 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
/* No mtd_info fields should be used up to this point. */
spi_nor_set_mtd_info(nor);
- dev_info(dev, "%s (%lld Kbytes)\n", info->name,
- (long long)mtd->size >> 10);
+ dev_dbg(dev, "Manufacturer and device ID: %*phN\n",
+ SPI_NOR_MAX_ID_LEN, nor->id);
dev_dbg(dev,
"mtd .name = %s, .size = 0x%llx (%lldMiB), "
--
2.34.1
The flash name is unreliable as we saw flash ID collisions. Mark the
name as obsolete.
Signed-off-by: Tudor Ambarus <[email protected]>
---
drivers/mtd/spi-nor/core.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 29ed67725b18..d36c0e072954 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -465,7 +465,7 @@ struct spi_nor_id {
* struct flash_info - SPI NOR flash_info entry.
* @id: pointer to struct spi_nor_id or NULL, which means "no ID" (mostly
* older chips).
- * @name: the name of the flash.
+ * @name: (obsolete) the name of the flash. Do not set it for new additions.
* @size: the size of the flash in bytes.
* @sector_size: (optional) the size listed here is what works with
* SPINOR_OP_SE, which isn't necessarily called a "sector" by
--
2.34.1
From: JaimeLiao <[email protected]>
The flash name is not reliable as we saw flash ID collisions.
Hide the flash name if not set.
Signed-off-by: JaimeLiao <[email protected]>
[ta: update commit subject and description]
Signed-off-by: Tudor Ambarus <[email protected]>
---
Documentation/ABI/testing/sysfs-bus-spi-devices-spi-nor | 3 +++
drivers/mtd/spi-nor/sysfs.c | 2 ++
2 files changed, 5 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-bus-spi-devices-spi-nor b/Documentation/ABI/testing/sysfs-bus-spi-devices-spi-nor
index c800621eff95..6d7be97bf7d1 100644
--- a/Documentation/ABI/testing/sysfs-bus-spi-devices-spi-nor
+++ b/Documentation/ABI/testing/sysfs-bus-spi-devices-spi-nor
@@ -25,6 +25,9 @@ KernelVersion: 5.14
Contact: [email protected]
Description: (RO) Part name of the SPI NOR flash.
+ The attribute is not present if the jedec_id founded in
+ ID table but flash name didn't include in it.
+
What: /sys/bus/spi/devices/.../spi-nor/sfdp
Date: April 2021
diff --git a/drivers/mtd/spi-nor/sysfs.c b/drivers/mtd/spi-nor/sysfs.c
index 2dfdc555a69f..96064e4babf0 100644
--- a/drivers/mtd/spi-nor/sysfs.c
+++ b/drivers/mtd/spi-nor/sysfs.c
@@ -78,6 +78,8 @@ static umode_t spi_nor_sysfs_is_visible(struct kobject *kobj,
if (attr == &dev_attr_manufacturer.attr && !nor->manufacturer)
return 0;
+ if (attr == &dev_attr_partname.attr && !nor->info->name)
+ return 0;
if (attr == &dev_attr_jedec_id.attr && !nor->info->id && !nor->id)
return 0;
--
2.34.1
The mtd data hall be obtained with the mtd ioctls or with new debugs
entries if one cares. Drop the debug prints.
Signed-off-by: Tudor Ambarus <[email protected]>
---
drivers/mtd/spi-nor/core.c | 16 ----------------
1 file changed, 16 deletions(-)
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index ca5bd93d1f17..a708c3448809 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -3561,22 +3561,6 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
dev_dbg(dev, "Manufacturer and device ID: %*phN\n",
SPI_NOR_MAX_ID_LEN, nor->id);
- dev_dbg(dev,
- "mtd .name = %s, .size = 0x%llx (%lldMiB), "
- ".erasesize = 0x%.8x (%uKiB) .numeraseregions = %d\n",
- mtd->name, (long long)mtd->size, (long long)(mtd->size >> 20),
- mtd->erasesize, mtd->erasesize / 1024, mtd->numeraseregions);
-
- if (mtd->numeraseregions)
- for (i = 0; i < mtd->numeraseregions; i++)
- dev_dbg(dev,
- "mtd.eraseregions[%d] = { .offset = 0x%llx, "
- ".erasesize = 0x%.8x (%uKiB), "
- ".numblocks = %d }\n",
- i, (long long)mtd->eraseregions[i].offset,
- mtd->eraseregions[i].erasesize,
- mtd->eraseregions[i].erasesize / 1024,
- mtd->eraseregions[i].numblocks);
return 0;
}
EXPORT_SYMBOL_GPL(spi_nor_scan);
--
2.34.1
I missed to drop some unused variables. Will drop them if everything
else is fine.
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index a708c3448809..92c992eb73d5 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -3492,9 +3492,7 @@ int spi_nor_scan(struct spi_nor *nor, const char
*name,
{
const struct flash_info *info;
struct device *dev = nor->dev;
- struct mtd_info *mtd = &nor->mtd;
int ret;
- int i;
ret = spi_nor_check(nor);
if (ret)
Am 2023-12-15 09:21, schrieb Tudor Ambarus:
> We saw flash ID collisions which make the flash name unreliable. Print
> the manufacgturer and device ID instead of the flash name.
>
> Lower the print to dev_dbg to stop polluting the kernel log.
>
> Suggested-by: Miquel Raynal <[email protected]>
> Signed-off-by: Tudor Ambarus <[email protected]>
Reviewed-by: Michael Walle <[email protected]>
Am 2023-12-15 09:21, schrieb Tudor Ambarus:
> The flash name is unreliable as we saw flash ID collisions. Mark the
> name as obsolete.
>
> Signed-off-by: Tudor Ambarus <[email protected]>
Reviewed-by: Michael Walle <[email protected]>
Am 2023-12-15 09:21, schrieb Tudor Ambarus:
> From: JaimeLiao <[email protected]>
>
> The flash name is not reliable as we saw flash ID collisions.
> Hide the flash name if not set.
>
> Signed-off-by: JaimeLiao <[email protected]>
> [ta: update commit subject and description]
> Signed-off-by: Tudor Ambarus <[email protected]>
Great!
Reviewed-by: Michael Walle <[email protected]>
-michael
Am 2023-12-15 09:21, schrieb Tudor Ambarus:
> The mtd data hall be obtained with the mtd ioctls or with new debugs
shall, debugfs
> entries if one cares. Drop the debug prints.
>
> Signed-off-by: Tudor Ambarus <[email protected]>
With the above and the unused variables fixed:
Reviewed-by: Michael Walle <[email protected]>
Miquel raised a valid point that if the rootfs is on this very
flash and if there is an error, you cannot use debugfs or the
ioctls. But I doubt that the info about the erase regions will
help much in that case.
-michael
On Fri, 15 Dec 2023 10:21:34 +0200, Tudor Ambarus wrote:
> The flash name is unreliable as we saw flash ID collisions. Mark the
> flash name as obsolete and print the manufacturer and device ID where
> name was printed.
>
> JaimeLiao (1):
> mtd: spi-nor: sysfs: hide the flash name if not set
>
> [...]
Dropped unused variables, fixed typos and
applied to git://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git,
spi-nor/next branch. Thanks!
[1/4] mtd: spi-nor: print flash ID instead of name
https://git.kernel.org/mtd/c/9fcb0999345e
[2/4] mtd: spi-nor: mark the flash name as obsolete
https://git.kernel.org/mtd/c/15eb8303bb42
[3/4] mtd: spi-nor: sysfs: hide the flash name if not set
https://git.kernel.org/mtd/c/62f92e62c1ef
[4/4] mtd: spi-nor: drop superfluous debug prints
https://git.kernel.org/mtd/c/7bf018ea5cb6
Cheers,
--
Tudor Ambarus <[email protected]>
On Fri, Dec 15 2023, Tudor Ambarus wrote:
> We saw flash ID collisions which make the flash name unreliable. Print
> the manufacgturer and device ID instead of the flash name.
Typo. s/manufacgturer/manufacturer/
>
> Lower the print to dev_dbg to stop polluting the kernel log.
FWIW, I find these prints to be somewhat useful. they serve to
"announce" that the kernel probed a device successfully. This can be
somewhat helpful when reading logs from customers trying to figure out
why something doesn't work.
I don't have any strong opinions of course since it is not such a big
deal but I wanted to point out that these prints _are_ somewhat useful.
Acked-by: Pratyush Yadav <[email protected]>
> Suggested-by: Miquel Raynal <[email protected]>
> Signed-off-by: Tudor Ambarus <[email protected]>
--
Regards,
Pratyush Yadav
On Fri, Dec 15 2023, Tudor Ambarus wrote:
> The flash name is unreliable as we saw flash ID collisions. Mark the
> name as obsolete.
>
> Signed-off-by: Tudor Ambarus <[email protected]>
Reviewed-by: Pratyush Yadav <[email protected]>
--
Regards,
Pratyush Yadav
On Fri, Dec 15 2023, Tudor Ambarus wrote:
> From: JaimeLiao <[email protected]>
>
> The flash name is not reliable as we saw flash ID collisions.
> Hide the flash name if not set.
>
> Signed-off-by: JaimeLiao <[email protected]>
> [ta: update commit subject and description]
> Signed-off-by: Tudor Ambarus <[email protected]>
> ---
> Documentation/ABI/testing/sysfs-bus-spi-devices-spi-nor | 3 +++
> drivers/mtd/spi-nor/sysfs.c | 2 ++
> 2 files changed, 5 insertions(+)
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-spi-devices-spi-nor b/Documentation/ABI/testing/sysfs-bus-spi-devices-spi-nor
> index c800621eff95..6d7be97bf7d1 100644
> --- a/Documentation/ABI/testing/sysfs-bus-spi-devices-spi-nor
> +++ b/Documentation/ABI/testing/sysfs-bus-spi-devices-spi-nor
> @@ -25,6 +25,9 @@ KernelVersion: 5.14
> Contact: [email protected]
> Description: (RO) Part name of the SPI NOR flash.
>
> + The attribute is not present if the jedec_id founded in
> + ID table but flash name didn't include in it.
Nitpick: strange sentence construction makes it a bit confusing.
Perhaps:
The attribute is not present if the JEDEC ID table for the flash
does not include its name.
> +
>
> What: /sys/bus/spi/devices/.../spi-nor/sfdp
> Date: April 2021
> diff --git a/drivers/mtd/spi-nor/sysfs.c b/drivers/mtd/spi-nor/sysfs.c
> index 2dfdc555a69f..96064e4babf0 100644
> --- a/drivers/mtd/spi-nor/sysfs.c
> +++ b/drivers/mtd/spi-nor/sysfs.c
> @@ -78,6 +78,8 @@ static umode_t spi_nor_sysfs_is_visible(struct kobject *kobj,
>
> if (attr == &dev_attr_manufacturer.attr && !nor->manufacturer)
> return 0;
> + if (attr == &dev_attr_partname.attr && !nor->info->name)
> + return 0;
Do you want to show the name for the "spi-nor-generic" flash? If not
then perhaps drop its name as well?
> if (attr == &dev_attr_jedec_id.attr && !nor->info->id && !nor->id)
> return 0;
--
Regards,
Pratyush Yadav
On Fri, Dec 15 2023, Tudor Ambarus wrote:
> I missed to drop some unused variables. Will drop them if everything
> else is fine.
>
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index a708c3448809..92c992eb73d5 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -3492,9 +3492,7 @@ int spi_nor_scan(struct spi_nor *nor, const char
> *name,
> {
> const struct flash_info *info;
> struct device *dev = nor->dev;
> - struct mtd_info *mtd = &nor->mtd;
> int ret;
> - int i;
>
> ret = spi_nor_check(nor);
> if (ret)
With these,
Reviewed-by: Pratyush Yadav <[email protected]>
--
Regards,
Pratyush Yadav
On 1/29/24 13:26, Pratyush Yadav wrote:
> On Fri, Dec 15 2023, Tudor Ambarus wrote:
>
>> I missed to drop some unused variables. Will drop them if everything
>> else is fine.
>>
>> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
>> index a708c3448809..92c992eb73d5 100644
>> --- a/drivers/mtd/spi-nor/core.c
>> +++ b/drivers/mtd/spi-nor/core.c
>> @@ -3492,9 +3492,7 @@ int spi_nor_scan(struct spi_nor *nor, const char
>> *name,
>> {
>> const struct flash_info *info;
>> struct device *dev = nor->dev;
>> - struct mtd_info *mtd = &nor->mtd;
>> int ret;
>> - int i;
>>
>> ret = spi_nor_check(nor);
>> if (ret)
>
>
> With these,
>
> Reviewed-by: Pratyush Yadav <[email protected]>
>
Hi, Pratyush,
All in these series were already applied, as I specified in a reply.
Please check patchwork for patches that are not yet handled.
Cheers,
ta
On Mon, Jan 29 2024, Tudor Ambarus wrote:
> On 1/29/24 13:26, Pratyush Yadav wrote:
>> On Fri, Dec 15 2023, Tudor Ambarus wrote:
>>
>>> I missed to drop some unused variables. Will drop them if everything
>>> else is fine.
>>>
>>> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
>>> index a708c3448809..92c992eb73d5 100644
>>> --- a/drivers/mtd/spi-nor/core.c
>>> +++ b/drivers/mtd/spi-nor/core.c
>>> @@ -3492,9 +3492,7 @@ int spi_nor_scan(struct spi_nor *nor, const char
>>> *name,
>>> {
>>> const struct flash_info *info;
>>> struct device *dev = nor->dev;
>>> - struct mtd_info *mtd = &nor->mtd;
>>> int ret;
>>> - int i;
>>>
>>> ret = spi_nor_check(nor);
>>> if (ret)
>>
>>
>> With these,
>>
>> Reviewed-by: Pratyush Yadav <[email protected]>
>>
> Hi, Pratyush,
>
> All in these series were already applied, as I specified in a reply.
> Please check patchwork for patches that are not yet handled.
Ah sorry, I didn't see that. I was browsing through my email backlog and
these patches just caught my eye.
Will look at the newer ones.
--
Regards,
Pratyush Yadav