On 2020-09-21 17:36, Bjorn Andersson wrote:
On Mon 21 Sep 19:22 CDT 2020, nguyenb@xxxxxxxxxxxxxx wrote:
On 2020-09-18 12:01, Rob Herring wrote:
> On Tue, Sep 15, 2020 at 2:10 AM <nguyenb@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx>
> > >> Signed-off-by: Asutosh Das <asutoshd@xxxxxxxxxxxxxx>
> > >> Signed-off-by: Bao D. Nguyen <nguyenb@xxxxxxxxxxxxxx>
> > >> ---
> > >> 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.
It sounds like you're suggesting that we detect the UFS device using
some voltage, then depending on version we might lower it to 2.5V.
Yes, that is one possible solution.
I'm afraid I don't see any of this either documented or implemented in
these patches.
I was responding to a comment about detecting the device version and
change the voltage
based on the detection. It is not implemented in this patch. Maybe I
should stop
discussing another possible solution, even though related to the topic,
it is not
implemented in this patch.
What is this initial detection voltage and how to you configure it?
The initial voltage would be 2.9V and is lowered to 2.5V if UFS3.0+
device is detected.
We would call the regulator_set_voltage() to set to a specific voltage
level.
Regards,
Bao
Regards,
Bjorn
>
> > 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