Hey Neil Please see inline answers.
________________________________ From: Neil Armstrong <[email protected]> Sent: Monday, June 1, 2026 4:55 PM To: Raz Ben Yehuda <[email protected]>; [email protected] <[email protected]> Subject: Re: [PATCH 3/5] ufs: Extend the ufs user interface EXTERNAL EMAIL: Do not click any links or open any attachments unless you trust the sender and know the content is safe. On 6/1/26 13:26, Raz Ben Yehuda wrote: > Provide a user interface and help to ufs tool. > query [lun] [idn]: > (0h) Device desc > (7h) Geometry desc > (8h) Power desc > (2h) Unit desc > (1h) Config desc > > set_lun [LUN] [parm] > lu_enable > boot_lun_id > lu_write_protect > memory_type > data_reliability > num_allocunits > logical_block_size > provisioning_type > context_capabilities > lu_num_write_booster_buffer_allocunits > > set_dev_desc [parm] [value] > descriptor_idn, device, device_class > device_sub_class, number_lu, boot_enable > descr_access_en, init_power_mode, high_priority_lun > secure_removal_type, initactive_icc_level > queue_depth, write_booster_buffer_preserve_user_space_en > write_booster_buffer_type, spec_version, manufacturer_id > periodic_rtc_update, device_version, psa_max_data_size > extended_ufs_features_support, > num_shared_write_booster_buffer_allocunits > write_booster_buffer_preserve_user_space_en > > set_cfg_desc [parm] [value] > length, descriptor_idn, conf_desc_continue > boot_enable, descr_access_en, initpower_mode > high_priority_lun, secure_removal_type, init_active_icc_level > periodic_rtc_update, reserved_HPB, rpmb_region_enable > rpmb_region1_size, rpmb_region2_size, rpmb_region3_size > write_booster_buffer_preserve_user_space_en > write_booster_buffer_type > num_shared_write_booster_buffer_allocunits > > flag [flag name] > deviceinit, permanent_wpe, pwr_on_wpe, bkops_en, life_span_mode > purge_enable, fphy_resource_removal, busy_rtc > permanently_disable_fw_update, write_booster_en > wb_buf_flush_en, wb_buf_flush_h8 > > Examples: > > To create lun 4 please: > use ufs set_lun 4 num_allocunits xxx to set size > ufs commit > scsi scan > > To change some value in the configuration descriptor > ufs set_cfg_desc num_shared_write_booster_buffer_allocunits 100000 > ufs set_cfg_desc write_booster_buffer_type 1 > ufs commit > > Signed-off-by: Raz Ben Yehuda <[email protected]> > --- > cmd/ufs.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++++++----- > 1 file changed, 86 insertions(+), 8 deletions(-) > > diff --git a/cmd/ufs.c b/cmd/ufs.c > index 790dab50f18..bb0e2fd3a67 100644 > --- a/cmd/ufs.c > +++ b/cmd/ufs.c > @@ -8,31 +8,109 @@ > #include <command.h> > #include <ufs.h> > #include <vsprintf.h> > -#include <linux/string.h> Why ? string,h not needed. > > static int do_ufs(struct cmd_tbl *cmdtp, int flag, int argc, char *const > argv[]) > { > - int dev, ret; > + int dev; > > if (argc >= 2) { > if (!strcmp(argv[1], "init")) { > if (argc == 3) { > dev = dectoul(argv[2], NULL); > - ret = ufs_probe_dev(dev); > - if (ret) > + if (!ufs_probe_dev(dev)) > return CMD_RET_FAILURE; Why ? Raz: Correct. thx. > } else { > ufs_probe(); > } > - > return CMD_RET_SUCCESS; > } > - } > +#if IS_ENABLED(CONFIG_UFS_TOOL) > + if (!strcmp(argv[1], "query")) { > + int lun = -1; > + int idn = -1; > + > + if (argc <= 3) { > + printf("usage: lun idn"); > + return CMD_RET_FAILURE; > + } > + > + lun = dectoul(argv[2], NULL); > + idn = dectoul(argv[3], NULL); > + > + return ufs_tool_query(lun, idn); > + } > + > + if (!strcmp(argv[1], "set_lun")) { > + int lun; > + u32 val; > + > + if (argc <= 4) { > + ufs_tool_help(); > + return CMD_RET_FAILURE; > + } > + lun = dectoul(argv[2], NULL); > + val = dectoul(argv[4], NULL); > + return ufs_tool_set_lun(lun, argv[3], val); > + } > + if (!strcmp(argv[1], "set_dev_desc")) { > + const char *name; > + u32 val; > > - return CMD_RET_USAGE; > + if (argc <= 3) { > + ufs_tool_help(); > + return CMD_RET_FAILURE; > + } > + name = argv[2]; > + val = dectoul(argv[3], NULL); > + return ufs_tool_set_device_desc(name, val); > + } > + if (!strcmp(argv[1], "set_cfg_desc")) { > + const char *name; > + u32 val; > + > + if (argc <= 3) { > + ufs_tool_help(); > + return CMD_RET_FAILURE; > + } > + name = argv[2]; > + val = dectoul(argv[3], NULL); > + return ufs_tool_set_config_desc(name, val); > + } > + > + if (!strcmp(argv[1], "flag")) { > + if (argc == 2) { > + printf("usage: flag [flag name]"); > + ufs_tool_help(); > + return CMD_RET_FAILURE; > + } > + return ufs_tool_query_flag(argv[2]); > + } > + > + if (!strcmp(argv[1], "commit")) > + return ufs_tool_commit(); > + > + if (!strcmp(argv[1], "clearall")) > + return ufs_tool_clearall(); > + if (!strcmp(argv[1], "luns")) { > + ufs_tool_print_luns(); Make the UFS device selectable everywhere, and return the ufs_tool_print_luns error Raz: Why ? these commands do not require any device. > + return 0; > + } > + } > + ufs_tool_help(); > +#endif > + return CMD_RET_FAILURE; > } > > -U_BOOT_CMD(ufs, 3, 1, do_ufs, > +U_BOOT_CMD(ufs, 5, 1, do_ufs, > "UFS sub-system", > "init [dev] - init UFS subsystem\n" > +#if IS_ENABLED(CONFIG_UFS_TOOL) > + "query [LUN] [IDN]\n" > + "set_lun [LUN] [parm name] [value]\n" > + "set_cfg_des [parm name] [value]\n" > + "set_dev_desc [parm name] [value]\n" > + "luns - prints luns\n" > + "flag [flag name] Query Flags\n" > + "commit - writes luns configuration\n" > +#endif > );
