On 09.06.23 09:29, Sascha Hauer wrote:
> On Thu, Jun 08, 2023 at 09:24:18AM +0200, Ahmad Fatoum wrote:
>> Best practice is for each board to populate $global.system.partitions
>> or $global.fastboot.partitions with a string exporting its flashable
>> devices in a descriptive manner, e.g. "/dev/mmc0(eMMC),/dev/mmc1(SD)".
>>
>> This often goes into BSPs though, so upstream boards are left without
>> default partitions, making use a bit cumbersome. Make this easier
>> by providing three new magic specifiers:
>>
>> - nvmem: exports all registered NVMEM devices (e.g. EEPROMs, Fuse banks)
>> - block: exports all registered block devices (e.g. eMMC and SD)
>> - auto: currently equivalent to "nvmem,block". May be extended
>> to raw MTD and UBI in future
>>
>> This makes it easy to export devices on any board:
>>
>> usbgadget -A auto -b
>>
>> or
>>
>> usbgadget -S auto,/tmp/fitimage(fitimage)c
>>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@xxxxxxxxxxxxxx>
>> ---
>> Documentation/user/usb.rst | 17 +++++++++++++++++
>> common/block.c | 16 ++++++++++++++++
>> common/file-list.c | 29 +++++++++++++++++++++++++++--
>> drivers/nvmem/core.c | 16 ++++++++++++++++
>> include/block.h | 6 ++++++
>> include/linux/nvmem-consumer.h | 8 ++++++++
>> 6 files changed, 90 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/user/usb.rst b/Documentation/user/usb.rst
>> index f2f57ead98d4..6fed0f619b32 100644
>> --- a/Documentation/user/usb.rst
>> +++ b/Documentation/user/usb.rst
>> @@ -73,6 +73,23 @@ Example:
>>
>> /dev/nand0.barebox.bb(barebox)sr,/kernel(kernel)rc
>>
>> +Board code authors are encouraged to provide a default environment containing
>> +partitions with descriptive names. For boards where this is not specified,
>> +there exist a number of **partition** specifiers for automatically generating entries:
>> +
>> +* ``nvmem`` exports all registered NVMEM devices (e.g. EEPROMs, Fuse banks)
>
> Blindly exporting NVMEM devices is not a good idea. As the description
> says it's used for fuse banks. These are write-once and modifying them
> is potentially dangerous. Also it's fastboot which means you can't even
> read them before modifying them and there is no way to only partially
> write them.
>
> There might be good reasons to export some specific NVMEM device, but
> then this should be explicitly exported and not by default.
The few fuse bank drivers that can actually blow fuses have a device
variable that needs to be written to unlock write access.
But yes, the inability to write them partially makes the write
useless. I will just resend without nvmem.
>> +static bool file_list_handle_spec(struct file_list *files, const char *spec)
>> +{
>> + unsigned count = 0;
>> +
>> + if (!strcmp(spec, "auto")) {
>> + count += file_list_add_blockdevs(files);
>> + count += file_list_add_nvmemdevs(files);
>> + } else if (!strcmp(spec, "block")) {
>> + count += file_list_add_blockdevs(files);
>> + } else if (!strcmp(spec, "nvmem")) {
>> + count += file_list_add_nvmemdevs(files);
>> + } else {
>> + return false;
>> + }
>
> Since you are talking about future extensions you could parse "auto"
> first and set a flag variable for it:
>
> bool auto = false;
>
> if (!strcmp(spec, "auto"))
> auto = true;
> if (!strcmp(spec, "block") || auto)
> count += file_list_add_blockdevs(files);
> if (!strcmp(spec, "nvmem") || auto)
> count += file_list_add_nvmemdevs(files);
>
> That would make it a bit easier to integrate the future extensions into
> the code.
Ok.
>
> Sascha
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
