Messages in this thread | | | Date | Thu, 26 May 2022 23:12:19 +0300 | Subject | Re: [RESEND PATCH] scsi: ufs: sysfs: support writing boot_lun attr | From | - <> |
| |
Hi,
My usecase is enabling boot slot switching on Android A/B devices, where the active LUN has to be changed in order to facilitate e.g. dual-booting Android and mainline Linux. A similar interface is exposed by Android, albeit via ioctl. I've tested this patch and confirmed it enabled the necessary functionality.
On 25/05/2022 21:34, Avri Altman wrote: > Hi, >> Expands sysfs boot_lun attribute to be writable. Necessary to enable >> proper support for LUN switching on some UFS devices. > Can you please elaborate why is it necessary? > What use case are you running? > >> Signed-off-by: Nia Espera <a5b6@riseup.net> >> --- >> drivers/scsi/ufs/ufs-sysfs.c | 67 +++++++++++++++++++++++++++++++++++- >> 1 file changed, 66 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/scsi/ufs/ufs-sysfs.c b/drivers/scsi/ufs/ufs-sysfs.c >> index 5c405ff7b6ea..7bf5d6c3d0ec 100644 >> --- a/drivers/scsi/ufs/ufs-sysfs.c >> +++ b/drivers/scsi/ufs/ufs-sysfs.c >> @@ -1047,6 +1047,71 @@ static inline bool ufshcd_is_wb_attrs(enum attr_idn >> idn) >> idn <= QUERY_ATTR_IDN_CURR_WB_BUFF_SIZE; >> } >> >> +static ssize_t boot_lun_enabled_show(struct device *dev, >> + struct device_attribute *attr, char *buf) >> +{ >> + struct ufs_hba *hba = dev_get_drvdata(dev); >> + u32 slot; >> + int ret; >> + u8 index = 0; >> + >> + down(&hba->host_sem); >> + if (!ufshcd_is_user_access_allowed(hba)) { >> + up(&hba->host_sem); >> + return -EBUSY; >> + } >> + if (ufshcd_is_wb_attrs(QUERY_ATTR_IDN_BOOT_LU_EN)) > Clearly bBootLunEn is not a WB attribute. > >> + index = ufshcd_wb_get_query_index(hba); >> + ufshcd_rpm_get_sync(hba); >> + >> + ret = ufshcd_query_attr(hba, UPIU_QUERY_OPCODE_READ_ATTR, >> + QUERY_ATTR_IDN_BOOT_LU_EN, index, 0, &slot); >> + >> + ufshcd_rpm_put_sync(hba); >> + if (ret) { >> + ret = -EINVAL; >> + goto out; >> + } >> + >> + ret = sysfs_emit(buf, "0x%08X\n", slot); >> +out: >> + up(&hba->host_sem); >> + return ret; >> +} >> + >> +static ssize_t boot_lun_enabled_store(struct device *dev, >> + struct device_attribute *attr, >> + const char *buf, size_t count) >> +{ >> + struct ufs_hba *hba = dev_get_drvdata(dev); >> + u32 slot; >> + int ret; >> + u8 index = 0; >> + >> + if (kstrtouint(buf, 0, &slot) < 0) >> + return -EINVAL; > You need to verify that no one set bBootLunEn = 0x0 because the device won't boot. > Better check explicitly that slot != bBootLunEn and its either 1 or 2. > > Thanks, > Avri > >> + >> + down(&hba->host_sem); >> + if (!ufshcd_is_user_access_allowed(hba)) { >> + up(&hba->host_sem); >> + return -EBUSY; >> + } >> + if (ufshcd_is_wb_attrs(QUERY_ATTR_IDN_BOOT_LU_EN)) >> + index = ufshcd_wb_get_query_index(hba); >> + ufshcd_rpm_get_sync(hba); >> + >> + ret = ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_WRITE_ATTR, >> + QUERY_ATTR_IDN_BOOT_LU_EN, index, 0, &slot); >> + ufshcd_rpm_put_sync(hba); >> + if (ret) { >> + ret = -EINVAL; >> + goto out; >> + } >> +out: >> + up(&hba->host_sem); >> + return ret ? ret : count; >> +} >> + >> #define UFS_ATTRIBUTE(_name, _uname) \ >> static ssize_t _name##_show(struct device *dev, \ >> struct device_attribute *attr, char *buf) \ >> @@ -1077,8 +1142,8 @@ out: \ >> return ret; \ >> } \ >> static DEVICE_ATTR_RO(_name) >> +static DEVICE_ATTR_RW(boot_lun_enabled); >> >> -UFS_ATTRIBUTE(boot_lun_enabled, _BOOT_LU_EN); >> UFS_ATTRIBUTE(max_data_size_hpb_single_cmd, _MAX_HPB_SINGLE_CMD); >> UFS_ATTRIBUTE(current_power_mode, _POWER_MODE); >> UFS_ATTRIBUTE(active_icc_level, _ACTIVE_ICC_LVL); >> -- >> 2.36.1
| |