On Wed, Nov 02, 2022 at 04:05:34PM -0400, Krzysztof Kozlowski wrote:
> On 31/10/2022 10:56, Manivannan Sadhasivam wrote:
> >>> if (hs_gear > UFS_HS_G2)
> >>> return UFS_HS_G2;
> >>> + } else if (host->hw_ver.major > 0x3) {
> >>> + /*
> >>> + * Starting from UFS controller v4, Qcom supports dual gear mode (i.e., the
> >>> + * controller/PHY can be configured to run in two gear speeds). But that
> >>> + * requires an agreement between the UFS controller and the device. Below
> >>> + * code tries to find the max gear of both and decides which gear to use.
> >>> + *
> >>> + * First get the max gear supported by the UFS device if available.
> >>> + * If the property is not defined in devicetree, then use the default gear.
> >>> + */
> >>> + ret = of_property_read_u32(dev->of_node, "max-gear", &max_gear);
> >>> + if (ret)
> >>> + goto err_out;
> >>
> >> Can we detect the UFS device's max gear somehow? If not, the 'max-gear'
> >> property name doesn't sound good. Maybe calling it 'device-gear' would be
> >> better.
> >>
> >
> > UFS device probing depends on PHY init sequence. So technically we cannot know
> > the max gear of the device without using an init sequence, but this information
> > is static and should be known to a board manufacturer. That's why I decided to
> > use this property. Another option is to use a fixed init sequence for probing
> > the device and do a re-init after knowing it's max gear but that is not
> > recommended.
> >
>
> Why it is not recommended? By whom? You init on some default low gear
> (support for some is mandated by UFS spec) and then allow faster gears
> while you know the capabilities.
>
This approach is what used in Qcom downstream. I learned that when they tried
submitting to mailing list, it got rejected. So I came up with this approach.
Thanks,
Mani
> > We need "max" keyword because this property specifies the maximum gear at which
> > the device could operate and not necessarily the gear at which it operates.
> > Maybe, "max-device-gear" would make it clear.
>
>
> Best regards,
> Krzysztof
>
--
மணிவண்ணன் சதாசிவம்
