On 27/05/2022 07:17, Avri Altman wrote:
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?
NAK with prejudice.
Hi Avri,
Could you explain why the NAK here? Boot LUN switching is used on a lot
of embedded devices to implement A/B updates, Android devices are just
one such example.
Distributions like postmarketOS [1] aim to support upstream Linux on
mobile devices, particularly those that are no longer supported by the
vendor. Being able to make use of features like A/B updates is something
that I expect more distributions to be considering in the future, as we
start to see more Linux devices with support for features like this.
If safety is a concern, or if the values are device specific, we can
look at protecting the write functionality and configuration behind DT
properties, or coming up with another alternative.
[1]: https://postmarketos.org
Kind regards,
Caleb (they/he)
Thanks,
Avri
Signed-off-by: Nia Espera <a5b6@xxxxxxxxxx>
---
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
