| Message ID | 0a9d395dc38433501f9652a9236856d0ac840b77.1598939393.git.nguyenb@codeaurora.org |
|---|---|
| State | New |
| Headers | show |
| Series | Supports Reading UFS's Vcc Voltage Levels from DT | expand |
> > > UFS's specifications supports a range of Vcc operating > voltage levels. Add documentation for the UFS's Vcc voltage > levels setting. > > Signed-off-by: Can Guo <cang@codeaurora.org> > Signed-off-by: Asutosh Das <asutoshd@codeaurora.org> > Signed-off-by: Bao D. Nguyen <nguyenb@codeaurora.org> > --- > Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt > b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt > index 415ccdd..7257b32 100644 > --- a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt > +++ b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt > @@ -23,6 +23,8 @@ Optional properties: > with "phys" attribute, provides phandle to UFS PHY node > - vdd-hba-supply : phandle to UFS host controller supply regulator node > - vcc-supply : phandle to VCC supply regulator node > +- vcc-voltage-level : specifies voltage levels for VCC supply. For ufs3.1+ devices > + Should be specified in pairs (min, max), units uV. > - vccq-supply : phandle to VCCQ supply regulator node > - vccq2-supply : phandle to VCCQ2 supply regulator node > - vcc-supply-1p8 : For embedded UFS devices, valid VCC range is 1.7-1.95V > -- Why are you removing all other docs? They are still relevant for non ufs3.1 devices.
On 2020-09-13 02:35, Avri Altman wrote: >> >> >> UFS's specifications supports a range of Vcc operating >> voltage levels. Add documentation for the UFS's Vcc voltage >> levels setting. >> >> Signed-off-by: Can Guo <cang@codeaurora.org> >> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org> >> Signed-off-by: Bao D. Nguyen <nguyenb@codeaurora.org> >> --- >> Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt >> b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt >> index 415ccdd..7257b32 100644 >> --- a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt >> +++ b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt >> @@ -23,6 +23,8 @@ Optional properties: >> with "phys" attribute, provides phandle to >> UFS PHY node >> - vdd-hba-supply : phandle to UFS host controller supply >> regulator node >> - vcc-supply : phandle to VCC supply regulator node >> +- vcc-voltage-level : specifies voltage levels for VCC supply. > For ufs3.1+ devices Thanks for the comments, Avri. I am not clear what this comment means. Could you please clarify? > >> + Should be specified in pairs (min, max), >> units uV. >> - vccq-supply : phandle to VCCQ supply regulator node >> - vccq2-supply : phandle to VCCQ2 supply regulator node >> - vcc-supply-1p8 : For embedded UFS devices, valid VCC range >> is 1.7-1.95V >> -- > Why are you removing all other docs? > They are still relevant for non ufs3.1 devices. I did not remove anything. You may be confused by the "-" used as listing in the original document.
On Tue 01 Sep 01:00 CDT 2020, Bao D. Nguyen wrote: > UFS's specifications supports a range of Vcc operating > voltage levels. Add documentation for the UFS's Vcc voltage > levels setting. > > Signed-off-by: Can Guo <cang@codeaurora.org> > Signed-off-by: Asutosh Das <asutoshd@codeaurora.org> > Signed-off-by: Bao D. Nguyen <nguyenb@codeaurora.org> > --- > Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt > index 415ccdd..7257b32 100644 > --- a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt > +++ b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt > @@ -23,6 +23,8 @@ Optional properties: > with "phys" attribute, provides phandle to UFS PHY node > - vdd-hba-supply : phandle to UFS host controller supply regulator node > - vcc-supply : phandle to VCC supply regulator node > +- vcc-voltage-level : specifies voltage levels for VCC supply. > + Should be specified in pairs (min, max), units uV. What exactly are these pairs representing? Is this supposed to be 3 pairs of (min,max) for vcc, vcc and vccq2 to be passed into a regulator_set_voltage() for each regulator? Or are these some sort of "operating points" for the vcc-supply? Regards, Bjorn > - vccq-supply : phandle to VCCQ supply regulator node > - vccq2-supply : phandle to VCCQ2 supply regulator node > - vcc-supply-1p8 : For embedded UFS devices, valid VCC range is 1.7-1.95V > -- > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, > a Linux Foundation Collaborative Project >
On 2020-09-14 11:35, Rob Herring wrote: > On Mon, Aug 31, 2020 at 11:00:47PM -0700, Bao D. Nguyen wrote: >> UFS's specifications supports a range of Vcc operating >> voltage levels. Add documentation for the UFS's Vcc voltage >> levels setting. >> >> Signed-off-by: Can Guo <cang@codeaurora.org> >> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org> >> Signed-off-by: Bao D. Nguyen <nguyenb@codeaurora.org> >> --- >> Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt >> b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt >> index 415ccdd..7257b32 100644 >> --- a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt >> +++ b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt >> @@ -23,6 +23,8 @@ Optional properties: >> with "phys" attribute, provides phandle to >> UFS PHY node >> - vdd-hba-supply : phandle to UFS host controller supply >> regulator node >> - vcc-supply : phandle to VCC supply regulator node >> +- vcc-voltage-level : specifies voltage levels for VCC supply. >> + Should be specified in pairs (min, max), >> units uV. > > The expectation is the regulator pointed to by 'vcc-supply' has the > voltage constraints. Those constraints are supposed to be the board > constraints, not the regulator operating design constraints. If that > doesn't work for your case, then it should be addressed in a common way > for the regulator binding. The UFS regulator has a min_uV and max_uV limits. Currently, the min and max are hardcoded to UFS2.1 Spec allowed values of 2.7V and 3.6V respectively. With this change, I am trying to fix a couple issues: 1. The 2.7V min value only applies to UFS2.1 devices. with UFS3.0+ devices, the VCC min should be 2.4V. Hardcoding the min_uV to 2.7V does not work for UFS3.0+ devices. 2. Allow users to select a different Vcc voltage within the allowed range. Using the min value, the UFS device is operating at marginal Vcc voltage. In addition the PMIC and the board designs may add some variables especially at extreme temperatures. We observe stability issues when using the min Vcc voltage. > > Also, properties with units must have a unit suffix. Yes, I agree. > > Rob
On 2020-09-15 06:43, Bjorn Andersson wrote: > On Tue 15 Sep 03:14 CDT 2020, nguyenb@codeaurora.org wrote: > >> On 2020-09-14 21:41, Bjorn Andersson wrote: >> > On Tue 01 Sep 01:00 CDT 2020, Bao D. Nguyen wrote: >> > >> > > UFS's specifications supports a range of Vcc operating >> > > voltage levels. Add documentation for the UFS's Vcc voltage >> > > levels setting. >> > > >> > > Signed-off-by: Can Guo <cang@codeaurora.org> >> > > Signed-off-by: Asutosh Das <asutoshd@codeaurora.org> >> > > Signed-off-by: Bao D. Nguyen <nguyenb@codeaurora.org> >> > > --- >> > > Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt | 2 ++ >> > > 1 file changed, 2 insertions(+) >> > > >> > > diff --git a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt >> > > b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt >> > > index 415ccdd..7257b32 100644 >> > > --- a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt >> > > +++ b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt >> > > @@ -23,6 +23,8 @@ Optional properties: >> > > with "phys" attribute, provides phandle >> > > to UFS PHY node >> > > - vdd-hba-supply : phandle to UFS host controller supply >> > > regulator node >> > > - vcc-supply : phandle to VCC supply regulator node >> > > +- vcc-voltage-level : specifies voltage levels for VCC supply. >> > > + Should be specified in pairs (min, max), >> > > units uV. >> > >> > What exactly are these pairs representing? >> The pair is the min and max Vcc voltage request to the PMIC chip. >> As a result, the regulator output voltage would only be in this range. >> > > If you have static min/max voltage constraints for a device on a > particular board the right way to handle this is to adjust the board's > regulator-min-microvolt and regulator-max-microvolt accordingly - and > not call regulator_set_voltage() from the river at all. > > In other words, you shouldn't add this new property to describe > something already described in the node vcc-supply points to. > > Regards, > Bjorn Thank you all for your comments. The current driver hardcoding 2.7V Vcc min voltage does not work for UFS3.0+ devices according to the UFS device JEDEC spec. However, we will try to address it in a different way. Regards, Bao > >> > >> > Is this supposed to be 3 pairs of (min,max) for vcc, vcc and vccq2 to be >> > passed into a regulator_set_voltage() for each regulator? >> Yes, that's right. I should include the other power supplies in this >> change >> as well. >> > >> > Or are these some sort of "operating points" for the vcc-supply? >> > >> > Regards, >> > Bjorn >> > >> > > - vccq-supply : phandle to VCCQ supply regulator node >> > > - vccq2-supply : phandle to VCCQ2 supply regulator node >> > > - vcc-supply-1p8 : For embedded UFS devices, valid VCC range >> > > is 1.7-1.95V >> > > -- >> > > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora >> > > Forum, >> > > a Linux Foundation Collaborative Project >> > >
On Tue 15 Sep 16:47 UTC 2020, nguyenb@codeaurora.org wrote: > On 2020-09-15 06:43, Bjorn Andersson wrote: > > On Tue 15 Sep 03:14 CDT 2020, nguyenb@codeaurora.org wrote: > > > > > On 2020-09-14 21:41, Bjorn Andersson wrote: > > > > On Tue 01 Sep 01:00 CDT 2020, Bao D. Nguyen wrote: > > > > > > > > > UFS's specifications supports a range of Vcc operating > > > > > voltage levels. Add documentation for the UFS's Vcc voltage > > > > > levels setting. > > > > > > > > > > Signed-off-by: Can Guo <cang@codeaurora.org> > > > > > Signed-off-by: Asutosh Das <asutoshd@codeaurora.org> > > > > > Signed-off-by: Bao D. Nguyen <nguyenb@codeaurora.org> > > > > > --- > > > > > Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt | 2 ++ > > > > > 1 file changed, 2 insertions(+) > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt > > > > > b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt > > > > > index 415ccdd..7257b32 100644 > > > > > --- a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt > > > > > +++ b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt > > > > > @@ -23,6 +23,8 @@ Optional properties: > > > > > with "phys" attribute, provides phandle > > > > > to UFS PHY node > > > > > - vdd-hba-supply : phandle to UFS host controller supply > > > > > regulator node > > > > > - vcc-supply : phandle to VCC supply regulator node > > > > > +- vcc-voltage-level : specifies voltage levels for VCC supply. > > > > > + Should be specified in pairs (min, max), > > > > > units uV. > > > > > > > > What exactly are these pairs representing? > > > The pair is the min and max Vcc voltage request to the PMIC chip. > > > As a result, the regulator output voltage would only be in this range. > > > > > > > If you have static min/max voltage constraints for a device on a > > particular board the right way to handle this is to adjust the board's > > regulator-min-microvolt and regulator-max-microvolt accordingly - and > > not call regulator_set_voltage() from the river at all. > > > > In other words, you shouldn't add this new property to describe > > something already described in the node vcc-supply points to. > > > > Regards, > > Bjorn > Thank you all for your comments. The current driver hardcoding 2.7V Vcc min > voltage > does not work for UFS3.0+ devices according to the UFS device JEDEC spec. > However, we will > try to address it in a different way. > Right, but what I'm saying is that you should remove the regulator_set_voltage() call from the driver and rely on the device's dts, in which case you won't have this problem. Thanks, Bjorn > Regards, > Bao > > > > > > > > > > > Is this supposed to be 3 pairs of (min,max) for vcc, vcc and vccq2 to be > > > > passed into a regulator_set_voltage() for each regulator? > > > Yes, that's right. I should include the other power supplies in this > > > change > > > as well. > > > > > > > > Or are these some sort of "operating points" for the vcc-supply? > > > > > > > > Regards, > > > > Bjorn > > > > > > > > > - vccq-supply : phandle to VCCQ supply regulator node > > > > > - vccq2-supply : phandle to VCCQ2 supply regulator node > > > > > - vcc-supply-1p8 : For embedded UFS devices, valid VCC range > > > > > is 1.7-1.95V > > > > > -- > > > > > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora > > > > > Forum, > > > > > a Linux Foundation Collaborative Project > > > > >
On 2020-09-18 12:01, Rob Herring wrote: > On Tue, Sep 15, 2020 at 2:10 AM <nguyenb@codeaurora.org> wrote: >> >> On 2020-09-14 11:35, Rob Herring wrote: >> > On Mon, Aug 31, 2020 at 11:00:47PM -0700, Bao D. Nguyen wrote: >> >> UFS's specifications supports a range of Vcc operating >> >> voltage levels. Add documentation for the UFS's Vcc voltage >> >> levels setting. >> >> >> >> Signed-off-by: Can Guo <cang@codeaurora.org> >> >> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org> >> >> Signed-off-by: Bao D. Nguyen <nguyenb@codeaurora.org> >> >> --- >> >> Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt | 2 ++ >> >> 1 file changed, 2 insertions(+) >> >> >> >> diff --git a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt >> >> b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt >> >> index 415ccdd..7257b32 100644 >> >> --- a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt >> >> +++ b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt >> >> @@ -23,6 +23,8 @@ Optional properties: >> >> with "phys" attribute, provides phandle to >> >> UFS PHY node >> >> - vdd-hba-supply : phandle to UFS host controller supply >> >> regulator node >> >> - vcc-supply : phandle to VCC supply regulator node >> >> +- vcc-voltage-level : specifies voltage levels for VCC supply. >> >> + Should be specified in pairs (min, max), >> >> units uV. >> > >> > The expectation is the regulator pointed to by 'vcc-supply' has the >> > voltage constraints. Those constraints are supposed to be the board >> > constraints, not the regulator operating design constraints. If that >> > doesn't work for your case, then it should be addressed in a common way >> > for the regulator binding. >> The UFS regulator has a min_uV and max_uV limits. Currently, the min >> and >> max are hardcoded >> to UFS2.1 Spec allowed values of 2.7V and 3.6V respectively. >> With this change, I am trying to fix a couple issues: >> 1. The 2.7V min value only applies to UFS2.1 devices. with UFS3.0+ >> devices, the VCC min should be 2.4V. >> Hardcoding the min_uV to 2.7V does not work for UFS3.0+ devices. > > Don't you know the device version attached and can adjust the voltage > based on that? Or you have to set the voltage first? Yes it is one of the solutions. Once detect the UFS device is version 3.0+, you can lower the voltage to 2.5V from the hardcoded value used by the driver. However, to change the Vcc voltage, the host needs to follow a sequence to ensure safe operations after Vcc change (device has to be in sleep mode, Vcc needs to go down to 0 then up to 2.5V.) Also same sequence is repeated for every host initialization which is inconvenient. > >> 2. Allow users to select a different Vcc voltage within the allowed >> range. >> Using the min value, the UFS device is operating at marginal Vcc >> voltage. >> In addition the PMIC and the board designs may add some variables >> especially at extreme >> temperatures. We observe stability issues when using the min Vcc >> voltage. > > Again, we have standard regulator properties for this already that you > can tune per board. Thank you for the suggestion. > > Rob
diff --git a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt index 415ccdd..7257b32 100644 --- a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt +++ b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt @@ -23,6 +23,8 @@ Optional properties: with "phys" attribute, provides phandle to UFS PHY node - vdd-hba-supply : phandle to UFS host controller supply regulator node - vcc-supply : phandle to VCC supply regulator node +- vcc-voltage-level : specifies voltage levels for VCC supply. + Should be specified in pairs (min, max), units uV. - vccq-supply : phandle to VCCQ supply regulator node - vccq2-supply : phandle to VCCQ2 supply regulator node - vcc-supply-1p8 : For embedded UFS devices, valid VCC range is 1.7-1.95V