From: Michael Walle <michael@walle.cc>
To: Alexander Williams <awill@google.com>
Cc: linux-kernel@vger.kernel.org, linux-mtd@lists.infradead.org,
miquel.raynal@bootlin.com, p.yadav@ti.com, richard@nod.at,
tudor.ambarus@microchip.com, vigneshr@ti.com
Subject: Re: [PATCH 2/2] mtd: spi-nor: add initial sysfs support
Date: Thu, 29 Apr 2021 17:46:36 +0200 [thread overview]
Message-ID: <9b0f13aba55e1a25054303dd1dc719eb@walle.cc> (raw)
In-Reply-To: <20210429153725.15970-1-awill@google.com>
Hi Alex,
Am 2021-04-29 17:37, schrieb Alexander Williams:
> Hi Michael,
>
>> On Thu, Mar 18, 2021 at 10:24 AM Michael Walle <michael@walle.cc>
>> wrote:
>> Add support to show the name and JEDEC identifier as well as to dump
>> the
>> SFDP table. Not all flashes list their SFDP table contents in their
>> datasheet. So having that is useful. It might also be helpful in bug
>> reports from users.
>>
>> The idea behind the sysfs module is also to have raw access to the SPI
>> NOR flash device registers, which can also be useful for debugging.
>>
>> Signed-off-by: Michael Walle <michael@walle.cc>
>> ---
>> drivers/mtd/spi-nor/Makefile | 2 +-
>> drivers/mtd/spi-nor/core.c | 5 +++
>> drivers/mtd/spi-nor/core.h | 3 ++
>> drivers/mtd/spi-nor/sysfs.c | 86
>> ++++++++++++++++++++++++++++++++++++
>> 4 files changed, 95 insertions(+), 1 deletion(-)
>> create mode 100644 drivers/mtd/spi-nor/sysfs.c
>>
>> diff --git a/drivers/mtd/spi-nor/Makefile
>> b/drivers/mtd/spi-nor/Makefile
>> index 653923896205..aff308f75987 100644
>> --- a/drivers/mtd/spi-nor/Makefile
>> +++ b/drivers/mtd/spi-nor/Makefile
>> @@ -1,6 +1,6 @@
>> # SPDX-License-Identifier: GPL-2.0
>>
>> -spi-nor-objs := core.o sfdp.o
>> +spi-nor-objs := core.o sfdp.o sysfs.o
>> spi-nor-objs += atmel.o
>> spi-nor-objs += catalyst.o
>> spi-nor-objs += eon.o
>> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
>> index 4a315cb1c4db..2eaf4ba8c0f3 100644
>> --- a/drivers/mtd/spi-nor/core.c
>> +++ b/drivers/mtd/spi-nor/core.c
>> @@ -3707,6 +3707,10 @@ static int spi_nor_probe(struct spi_mem
>> *spimem)
>> if (ret)
>> return ret;
>>
>> + ret = spi_nor_sysfs_create(nor);
>
> This appears to be racing with user space. By the time we reach
> probe(), the
> device embedded in the spi_device has already been registered, with the
> uevent
> sent out, right? udev may not see the new attributes created here.
>
> Since we reuse a preexisting device throughout spi-nor, it seems
> awfully
> challenging to be able to safely add sysfs attributes. Would it make
> sense to
> create a spi-nor-specific device/class? Or perhaps attach these
> attributes to
> the device in mtd_info like I've done in
> https://lore.kernel.org/linux-mtd/20210428052725.530939-1-awill@google.com/
> ?
Do you encounter this problem? I'm currently working on this and dropped
the sysfs_create() and use dev_groups of the driver spi nor driver.
But I'm not sure how it will be handled anyway. Because we know the
content/size SFDP only after the probe and in any case the probe could
also fail. So I don't really understand how that is handled.
I've looked at your patch and it seems that the surpress_uevent() is
rarely used in the kernel.
I don't want to attach it to an MTD device because you might have
multiple ones which has the same SPI flash device as parent. The
SFDP is really a property of the flash device and not one of the
MTD partition.
-michael
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
prev parent reply other threads:[~2021-04-29 15:47 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-18 9:24 [PATCH 0/2] mtd: spi-nor: support dumping sfdp tables Michael Walle
2021-03-18 9:24 ` [PATCH 1/2] mtd: spi-nor: sfdp: save a copy of the SFDP data Michael Walle
2021-03-22 14:21 ` Pratyush Yadav
2021-03-22 15:32 ` Michael Walle
2021-03-22 15:48 ` Michael Walle
2021-03-22 18:42 ` Pratyush Yadav
2021-03-22 22:31 ` Michael Walle
2021-03-23 9:37 ` Pratyush Yadav
2021-04-05 13:11 ` Tudor.Ambarus
2021-04-05 15:07 ` Michael Walle
2021-04-05 15:42 ` Tudor.Ambarus
2021-04-05 16:03 ` Michael Walle
2021-03-18 9:24 ` [PATCH 2/2] mtd: spi-nor: add initial sysfs support Michael Walle
2021-03-20 4:16 ` Yicong Yang
2021-03-20 19:17 ` Michael Walle
2021-03-22 14:43 ` Pratyush Yadav
2021-03-22 14:57 ` Michael Walle
2021-04-06 7:56 ` Vignesh Raghavendra
2021-04-06 8:47 ` Michael Walle
2021-04-06 11:43 ` Vignesh Raghavendra
2021-04-29 15:37 ` Alexander Williams
2021-04-29 15:46 ` Michael Walle [this message]
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=9b0f13aba55e1a25054303dd1dc719eb@walle.cc \
--to=michael@walle.cc \
--cc=awill@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mtd@lists.infradead.org \
--cc=miquel.raynal@bootlin.com \
--cc=p.yadav@ti.com \
--cc=richard@nod.at \
--cc=tudor.ambarus@microchip.com \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).