[PATCH v5] ufs: core: Add HID support
Bean Huo
huobean at gmail.com
Wed May 21 07:56:50 PDT 2025
On Wed, 2025-05-21 at 22:32 +0800, Huan Tang wrote:
> > > > +static const char *ufs_hid_state_to_string(enum ufs_hid_state state)
> > > > +{
> > > > + switch (state) {
> > > > + case HID_IDLE:
> > > > + return "idle";
> > > > + case ANALYSIS_IN_PROGRESS:
> > > > + return "analysis_in_progress";
> > > > + case DEFRAG_REQUIRED:
> > > > + return "defrag_required";
> > > > + case DEFRAG_IN_PROGRESS:
> > > > + return "defrag_in_progress";
> > > > + case DEFRAG_COMPLETED:
> > > > + return "defrag_completed";
> > > > + case DEFRAG_NOT_REQUIRED:
> > > > + return "defrag_not_required";
> > > > + default:
> > > > + return "unknown";
> > > > + }
> > > > +}
> > >
> > > The enum ufs_hid_state values are contiguous and start from 0, maybe change switch-state to :
> > >
> > > #define NUM_UFS_HID_STATES 6
> > > static const char *ufs_hid_states[NUM_UFS_HID_STATES] = {
> > > "idle",
> > > "analysis_in_progress",
> > > "defrag_required",
> > > "defrag_in_progress",
> > > "defrag_completed",
> > > "defrag_not_required"
> > > };
>
> > If this change is made, please use the designated initializer syntax
> > ([label] = "text"). This will make it easier to verify that the
> > enumeration labels and the strings match.
>
> Hi bart and bean sir,
>
> Thank you for your comments and guidance!
>
> What do you think of the following changes?
>
> ufs.h
> +/* bHIDState attribute values */
> +enum ufs_hid_state {
> + HID_IDLE = 0,
> + ANALYSIS_IN_PROGRESS = 1,
> + DEFRAG_REQUIRED = 2,
> + DEFRAG_IN_PROGRESS = 3,
> + DEFRAG_COMPLETED = 4,
> + DEFRAG_NOT_REQUIRED = 5,
> + NUM_UFS_HID_STATES = 6,
> +};
>
> ufs-sysfs.c
> +static const char * const ufs_hid_states[] = {
> + [HID_IDLE] = "idle",
> + [ANALYSIS_IN_PROGRESS] = "analysis_in_progress",
> + [DEFRAG_REQUIRED] = "defrag_required",
> + [DEFRAG_IN_PROGRESS] = "defrag_in_progress",
> + [DEFRAG_COMPLETED] = "defrag_completed",
> + [DEFRAG_NOT_REQUIRED] = "defrag_not_required",
> +};
> +
> +static const char *ufs_hid_state_to_string(enum ufs_hid_state state)
> +{
> + if (state < NUM_UFS_HID_STATES)
> + return ufs_hid_states[state];
> +
> + return "unknown";
> +}
>
> ...
>
> +static ssize_t state_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct ufs_hba *hba = dev_get_drvdata(dev);
> + u32 value;
> + int ret;
> +
> + ret = hid_query_attr(hba, UPIU_QUERY_OPCODE_READ_ATTR,
> + QUERY_ATTR_IDN_HID_STATE, &value);
> + if (ret)
> + return ret;
> +
> + return sysfs_emit(buf, "%s\n", ufs_hid_state_to_string(value));
> +}
>
> Thanks
> Huan
>
>
>
looks good to me, with this change, please add my reviewed-by tag in the next version.
Kind regards,
Bean
More information about the Linux-mediatek
mailing list