Messages in this thread | | | Subject | Re: [RFC PATCH v1 2/2] scsi: ufs: ufs-exynos: implement exynos isr | From | Bart Van Assche <> | Date | Fri, 6 Aug 2021 09:37:08 -0700 |
| |
On 8/5/21 11:34 PM, Kiwoong Kim wrote: > Based on some events in the real world
Which events? Please clarify.
> I implement > this to block the host's working in some abnormal > conditions using an vendor specific interrupt for > cases that some contexts of a pending request in the > host isn't the same with those of its corresponding UPIUs > if they should have been the same exactly.
The entire patch description sounds very vague to me. Please make the description more clear.
> +enum exynos_ufs_vs_interrupt { > + /* > + * This occurs when information of a pending request isn't > + * the same with incoming UPIU for the request. For example, > + * if UFS driver rings with task tag #1, subsequential UPIUs > + * for this must have one as the value of task tag. But if > + * it's corrutped until the host receives it or incoming UPIUs > + * has an unexpected value for task tag, this raises. > + */ > + RX_UPIU_HIT_ERROR = 1 << 19, > +};
The above description needs to be improved. If a request is submitted with task tag one, only one UPIU can have that task tag instead of all subsequent UPIUs.
> hci_writel(ufs, UFS_SW_RST_MASK, HCI_SW_RST); > - > do { > if (!(hci_readl(ufs, HCI_SW_RST) & UFS_SW_RST_MASK)) > - goto out; > + return 0; > } while (time_before(jiffies, timeout));
Since the above loop is a busy-waiting loop, please insert an msleep() or cpu_relax() call.
> + * some unexpected events could happen, such as tranferring ^^^^^^^^^^^ Please fix the spelling of this word.
Thanks,
Bart.
| |