* [PATCH v1 0/2] Supports Reading UFS's Vcc Voltage Levels from DT @ 2020-09-01 6:00 Bao D. Nguyen 2020-09-01 6:00 ` [PATCH v1 1/2] scsi: dt-bindings: ufs: Add vcc-voltage-level for UFS Bao D. Nguyen ` (2 more replies) 0 siblings, 3 replies; 21+ messages in thread From: Bao D. Nguyen @ 2020-09-01 6:00 UTC (permalink / raw) To: cang, asutoshd, martin.petersen, linux-scsi; +Cc: Bao D. Nguyen, linux-arm-msm UFS's specification supports a range of Vcc operating voltages. Allows selecting the UFS Vcc operating voltage levels by reading the UFS's vcc-voltage-level in the device tree. Bao D. Nguyen (2): scsi: dt-bindings: ufs: Add vcc-voltage-level for UFS scsi: ufs: Support reading UFS's Vcc voltage from device tree Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt | 2 ++ drivers/scsi/ufs/ufshcd-pltfrm.c | 15 ++++++++++++--- 2 files changed, 14 insertions(+), 3 deletions(-) -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v1 1/2] scsi: dt-bindings: ufs: Add vcc-voltage-level for UFS 2020-09-01 6:00 [PATCH v1 0/2] Supports Reading UFS's Vcc Voltage Levels from DT Bao D. Nguyen @ 2020-09-01 6:00 ` Bao D. Nguyen 2020-09-13 9:35 ` Avri Altman ` (2 more replies) 2020-09-01 6:00 ` [PATCH v1 2/2] scsi: ufs: Support reading UFS's Vcc voltage from device tree Bao D. Nguyen 2020-09-10 1:29 ` [PATCH v1 0/2] Supports Reading UFS's Vcc Voltage Levels from DT nguyenb 2 siblings, 3 replies; 21+ messages in thread From: Bao D. Nguyen @ 2020-09-01 6:00 UTC (permalink / raw) To: cang, asutoshd, martin.petersen, linux-scsi Cc: Bao D. Nguyen, linux-arm-msm, Rob Herring, Bjorn Andersson, Avri Altman, Vinod Koul, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list 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. - 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 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* RE: [PATCH v1 1/2] scsi: dt-bindings: ufs: Add vcc-voltage-level for UFS 2020-09-01 6:00 ` [PATCH v1 1/2] scsi: dt-bindings: ufs: Add vcc-voltage-level for UFS Bao D. Nguyen @ 2020-09-13 9:35 ` Avri Altman 2020-09-14 16:19 ` nguyenb 2020-09-14 18:35 ` Rob Herring 2020-09-15 4:41 ` Bjorn Andersson 2 siblings, 1 reply; 21+ messages in thread From: Avri Altman @ 2020-09-13 9:35 UTC (permalink / raw) To: Bao D. Nguyen, cang, asutoshd, martin.petersen, linux-scsi Cc: linux-arm-msm, Rob Herring, Bjorn Andersson, Vinod Koul, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list > > > 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. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 1/2] scsi: dt-bindings: ufs: Add vcc-voltage-level for UFS 2020-09-13 9:35 ` Avri Altman @ 2020-09-14 16:19 ` nguyenb 0 siblings, 0 replies; 21+ messages in thread From: nguyenb @ 2020-09-14 16:19 UTC (permalink / raw) To: Avri Altman Cc: cang, asutoshd, martin.petersen, linux-scsi, linux-arm-msm, Rob Herring, Bjorn Andersson, Vinod Koul, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list 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. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 1/2] scsi: dt-bindings: ufs: Add vcc-voltage-level for UFS 2020-09-01 6:00 ` [PATCH v1 1/2] scsi: dt-bindings: ufs: Add vcc-voltage-level for UFS Bao D. Nguyen 2020-09-13 9:35 ` Avri Altman @ 2020-09-14 18:35 ` Rob Herring 2020-09-15 8:10 ` nguyenb 2020-09-15 4:41 ` Bjorn Andersson 2 siblings, 1 reply; 21+ messages in thread From: Rob Herring @ 2020-09-14 18:35 UTC (permalink / raw) To: Bao D. Nguyen Cc: cang, asutoshd, martin.petersen, linux-scsi, linux-arm-msm, Bjorn Andersson, Avri Altman, Vinod Koul, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list 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. Also, properties with units must have a unit suffix. Rob ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 1/2] scsi: dt-bindings: ufs: Add vcc-voltage-level for UFS 2020-09-14 18:35 ` Rob Herring @ 2020-09-15 8:10 ` nguyenb 2020-09-18 19:01 ` Rob Herring 0 siblings, 1 reply; 21+ messages in thread From: nguyenb @ 2020-09-15 8:10 UTC (permalink / raw) To: Rob Herring Cc: cang, asutoshd, martin.petersen, linux-scsi, linux-arm-msm, Bjorn Andersson, Avri Altman, Vinod Koul, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 1/2] scsi: dt-bindings: ufs: Add vcc-voltage-level for UFS 2020-09-15 8:10 ` nguyenb @ 2020-09-18 19:01 ` Rob Herring 2020-09-22 0:22 ` nguyenb 0 siblings, 1 reply; 21+ messages in thread From: Rob Herring @ 2020-09-18 19:01 UTC (permalink / raw) To: Bao D. Nguyen Cc: Can Guo, Asutosh Das, Martin K. Petersen, SCSI, linux-arm-msm, Bjorn Andersson, Avri Altman, Vinod Koul, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list 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? > 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. Rob ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 1/2] scsi: dt-bindings: ufs: Add vcc-voltage-level for UFS 2020-09-18 19:01 ` Rob Herring @ 2020-09-22 0:22 ` nguyenb 2020-09-22 0:36 ` Bjorn Andersson 0 siblings, 1 reply; 21+ messages in thread From: nguyenb @ 2020-09-22 0:22 UTC (permalink / raw) To: Rob Herring Cc: Can Guo, Asutosh Das, Martin K. Petersen, SCSI, linux-arm-msm, Bjorn Andersson, Avri Altman, Vinod Koul, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 1/2] scsi: dt-bindings: ufs: Add vcc-voltage-level for UFS 2020-09-22 0:22 ` nguyenb @ 2020-09-22 0:36 ` Bjorn Andersson 2020-09-23 17:32 ` nguyenb 0 siblings, 1 reply; 21+ messages in thread From: Bjorn Andersson @ 2020-09-22 0:36 UTC (permalink / raw) To: nguyenb Cc: Rob Herring, Can Guo, Asutosh Das, Martin K. Petersen, SCSI, linux-arm-msm, Avri Altman, Vinod Koul, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list On Mon 21 Sep 19:22 CDT 2020, nguyenb@codeaurora.org wrote: > 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. > 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. I'm afraid I don't see any of this either documented or implemented in these patches. What is this initial detection voltage and how to you configure it? 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 1/2] scsi: dt-bindings: ufs: Add vcc-voltage-level for UFS 2020-09-22 0:36 ` Bjorn Andersson @ 2020-09-23 17:32 ` nguyenb 0 siblings, 0 replies; 21+ messages in thread From: nguyenb @ 2020-09-23 17:32 UTC (permalink / raw) To: Bjorn Andersson Cc: Rob Herring, Can Guo, Asutosh Das, Martin K. Petersen, SCSI, linux-arm-msm, Avri Altman, Vinod Koul, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list On 2020-09-21 17:36, Bjorn Andersson wrote: > On Mon 21 Sep 19:22 CDT 2020, nguyenb@codeaurora.org wrote: > >> 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. >> > > 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 1/2] scsi: dt-bindings: ufs: Add vcc-voltage-level for UFS 2020-09-01 6:00 ` [PATCH v1 1/2] scsi: dt-bindings: ufs: Add vcc-voltage-level for UFS Bao D. Nguyen 2020-09-13 9:35 ` Avri Altman 2020-09-14 18:35 ` Rob Herring @ 2020-09-15 4:41 ` Bjorn Andersson 2020-09-15 8:14 ` nguyenb 2 siblings, 1 reply; 21+ messages in thread From: Bjorn Andersson @ 2020-09-15 4:41 UTC (permalink / raw) To: Bao D. Nguyen Cc: cang, asutoshd, martin.petersen, linux-scsi, linux-arm-msm, Rob Herring, Avri Altman, Vinod Koul, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list 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 > ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 1/2] scsi: dt-bindings: ufs: Add vcc-voltage-level for UFS 2020-09-15 4:41 ` Bjorn Andersson @ 2020-09-15 8:14 ` nguyenb 2020-09-15 13:43 ` Bjorn Andersson 0 siblings, 1 reply; 21+ messages in thread From: nguyenb @ 2020-09-15 8:14 UTC (permalink / raw) To: Bjorn Andersson Cc: cang, asutoshd, martin.petersen, linux-scsi, linux-arm-msm, Rob Herring, Avri Altman, Vinod Koul, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list 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. > > 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 >> ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 1/2] scsi: dt-bindings: ufs: Add vcc-voltage-level for UFS 2020-09-15 8:14 ` nguyenb @ 2020-09-15 13:43 ` Bjorn Andersson 2020-09-15 16:47 ` nguyenb 0 siblings, 1 reply; 21+ messages in thread From: Bjorn Andersson @ 2020-09-15 13:43 UTC (permalink / raw) To: nguyenb Cc: cang, asutoshd, martin.petersen, linux-scsi, linux-arm-msm, Rob Herring, Avri Altman, Vinod Koul, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list 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 > > > > 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 > > > ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 1/2] scsi: dt-bindings: ufs: Add vcc-voltage-level for UFS 2020-09-15 13:43 ` Bjorn Andersson @ 2020-09-15 16:47 ` nguyenb 2020-09-15 18:36 ` Bjorn Andersson 0 siblings, 1 reply; 21+ messages in thread From: nguyenb @ 2020-09-15 16:47 UTC (permalink / raw) To: Bjorn Andersson Cc: cang, asutoshd, martin.petersen, linux-scsi, linux-arm-msm, Rob Herring, Avri Altman, Vinod Koul, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list 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 >> > > ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 1/2] scsi: dt-bindings: ufs: Add vcc-voltage-level for UFS 2020-09-15 16:47 ` nguyenb @ 2020-09-15 18:36 ` Bjorn Andersson 0 siblings, 0 replies; 21+ messages in thread From: Bjorn Andersson @ 2020-09-15 18:36 UTC (permalink / raw) To: nguyenb Cc: cang, asutoshd, martin.petersen, linux-scsi, linux-arm-msm, Rob Herring, Avri Altman, Vinod Koul, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list 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 > > > > > ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v1 2/2] scsi: ufs: Support reading UFS's Vcc voltage from device tree 2020-09-01 6:00 [PATCH v1 0/2] Supports Reading UFS's Vcc Voltage Levels from DT Bao D. Nguyen 2020-09-01 6:00 ` [PATCH v1 1/2] scsi: dt-bindings: ufs: Add vcc-voltage-level for UFS Bao D. Nguyen @ 2020-09-01 6:00 ` Bao D. Nguyen 2020-09-13 9:37 ` Avri Altman 2020-09-15 13:46 ` Bjorn Andersson 2020-09-10 1:29 ` [PATCH v1 0/2] Supports Reading UFS's Vcc Voltage Levels from DT nguyenb 2 siblings, 2 replies; 21+ messages in thread From: Bao D. Nguyen @ 2020-09-01 6:00 UTC (permalink / raw) To: cang, asutoshd, martin.petersen, linux-scsi Cc: Bao D. Nguyen, linux-arm-msm, Alim Akhtar, Avri Altman, James E.J. Bottomley, YueHaibing, Bean Huo, open list The UFS specifications supports a range of Vcc operating voltage from 2.4-3.6V depending on the device and manufacturers. Allows selecting the UFS Vcc voltage level by setting the UFS's entry vcc-voltage-level in the device tree. If UFS's vcc-voltage-level setting is not found in the device tree, use default values provided by the driver. 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> --- drivers/scsi/ufs/ufshcd-pltfrm.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c index 3db0af6..48f429c 100644 --- a/drivers/scsi/ufs/ufshcd-pltfrm.c +++ b/drivers/scsi/ufs/ufshcd-pltfrm.c @@ -104,10 +104,11 @@ static int ufshcd_parse_clock_info(struct ufs_hba *hba) static int ufshcd_populate_vreg(struct device *dev, const char *name, struct ufs_vreg **out_vreg) { - int ret = 0; + int len, ret = 0; char prop_name[MAX_PROP_SIZE]; struct ufs_vreg *vreg = NULL; struct device_node *np = dev->of_node; + const __be32 *prop; if (!np) { dev_err(dev, "%s: non DT initialization\n", __func__); @@ -138,8 +139,16 @@ static int ufshcd_populate_vreg(struct device *dev, const char *name, vreg->min_uV = UFS_VREG_VCC_1P8_MIN_UV; vreg->max_uV = UFS_VREG_VCC_1P8_MAX_UV; } else { - vreg->min_uV = UFS_VREG_VCC_MIN_UV; - vreg->max_uV = UFS_VREG_VCC_MAX_UV; + prop = of_get_property(np, "vcc-voltage-level", &len); + if (!prop || (len != (2 * sizeof(__be32)))) { + dev_warn(dev, "%s vcc-voltage-level property.\n", + prop ? "invalid format" : "no"); + vreg->min_uV = UFS_VREG_VCC_MIN_UV; + vreg->max_uV = UFS_VREG_VCC_MAX_UV; + } else { + vreg->min_uV = be32_to_cpup(&prop[0]); + vreg->max_uV = be32_to_cpup(&prop[1]); + } } } else if (!strcmp(name, "vccq")) { vreg->min_uV = UFS_VREG_VCCQ_MIN_UV; -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply related [flat|nested] 21+ messages in thread
* RE: [PATCH v1 2/2] scsi: ufs: Support reading UFS's Vcc voltage from device tree 2020-09-01 6:00 ` [PATCH v1 2/2] scsi: ufs: Support reading UFS's Vcc voltage from device tree Bao D. Nguyen @ 2020-09-13 9:37 ` Avri Altman 2020-09-14 18:43 ` nguyenb 2020-09-15 13:46 ` Bjorn Andersson 1 sibling, 1 reply; 21+ messages in thread From: Avri Altman @ 2020-09-13 9:37 UTC (permalink / raw) To: Bao D. Nguyen, cang, asutoshd, martin.petersen, linux-scsi Cc: linux-arm-msm, Alim Akhtar, James E.J. Bottomley, YueHaibing, Bean Huo, open list > > The UFS specifications supports a range of Vcc operating voltage > from 2.4-3.6V depending on the device and manufacturers. > Allows selecting the UFS Vcc voltage level by setting the > UFS's entry vcc-voltage-level in the device tree. If UFS's > vcc-voltage-level setting is not found in the device tree, > use default values provided by the driver. > > 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> > --- > drivers/scsi/ufs/ufshcd-pltfrm.c | 15 ++++++++++++--- > 1 file changed, 12 insertions(+), 3 deletions(-) > > diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c > index 3db0af6..48f429c 100644 > --- a/drivers/scsi/ufs/ufshcd-pltfrm.c > +++ b/drivers/scsi/ufs/ufshcd-pltfrm.c > @@ -104,10 +104,11 @@ static int ufshcd_parse_clock_info(struct ufs_hba > *hba) > static int ufshcd_populate_vreg(struct device *dev, const char *name, > struct ufs_vreg **out_vreg) > { > - int ret = 0; > + int len, ret = 0; > char prop_name[MAX_PROP_SIZE]; > struct ufs_vreg *vreg = NULL; > struct device_node *np = dev->of_node; > + const __be32 *prop; > > if (!np) { > dev_err(dev, "%s: non DT initialization\n", __func__); > @@ -138,8 +139,16 @@ static int ufshcd_populate_vreg(struct device *dev, > const char *name, > vreg->min_uV = UFS_VREG_VCC_1P8_MIN_UV; > vreg->max_uV = UFS_VREG_VCC_1P8_MAX_UV; > } else { > - vreg->min_uV = UFS_VREG_VCC_MIN_UV; > - vreg->max_uV = UFS_VREG_VCC_MAX_UV; > + prop = of_get_property(np, "vcc-voltage-level", &len); > + if (!prop || (len != (2 * sizeof(__be32)))) { > + dev_warn(dev, "%s vcc-voltage-level property.\n", > + prop ? "invalid format" : "no"); > + vreg->min_uV = UFS_VREG_VCC_MIN_UV; > + vreg->max_uV = UFS_VREG_VCC_MAX_UV; > + } else { > + vreg->min_uV = be32_to_cpup(&prop[0]); > + vreg->max_uV = be32_to_cpup(&prop[1]); > + } > } > } else if (!strcmp(name, "vccq")) { > vreg->min_uV = UFS_VREG_VCCQ_MIN_UV; > -- Maybe instead call ufshcd_populate_vreg with the new name, To not break the function flow, and just add another else if ? ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 2/2] scsi: ufs: Support reading UFS's Vcc voltage from device tree 2020-09-13 9:37 ` Avri Altman @ 2020-09-14 18:43 ` nguyenb 2020-09-15 6:51 ` Avri Altman 0 siblings, 1 reply; 21+ messages in thread From: nguyenb @ 2020-09-14 18:43 UTC (permalink / raw) To: Avri Altman Cc: cang, asutoshd, martin.petersen, linux-scsi, linux-arm-msm, Alim Akhtar, James E.J. Bottomley, YueHaibing, Bean Huo, open list On 2020-09-13 02:37, Avri Altman wrote: >> >> The UFS specifications supports a range of Vcc operating voltage >> from 2.4-3.6V depending on the device and manufacturers. >> Allows selecting the UFS Vcc voltage level by setting the >> UFS's entry vcc-voltage-level in the device tree. If UFS's >> vcc-voltage-level setting is not found in the device tree, >> use default values provided by the driver. >> >> 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> >> --- >> drivers/scsi/ufs/ufshcd-pltfrm.c | 15 ++++++++++++--- >> 1 file changed, 12 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c >> b/drivers/scsi/ufs/ufshcd-pltfrm.c >> index 3db0af6..48f429c 100644 >> --- a/drivers/scsi/ufs/ufshcd-pltfrm.c >> +++ b/drivers/scsi/ufs/ufshcd-pltfrm.c >> @@ -104,10 +104,11 @@ static int ufshcd_parse_clock_info(struct >> ufs_hba >> *hba) >> static int ufshcd_populate_vreg(struct device *dev, const char *name, >> struct ufs_vreg **out_vreg) >> { >> - int ret = 0; >> + int len, ret = 0; >> char prop_name[MAX_PROP_SIZE]; >> struct ufs_vreg *vreg = NULL; >> struct device_node *np = dev->of_node; >> + const __be32 *prop; >> >> if (!np) { >> dev_err(dev, "%s: non DT initialization\n", __func__); >> @@ -138,8 +139,16 @@ static int ufshcd_populate_vreg(struct device >> *dev, >> const char *name, >> vreg->min_uV = UFS_VREG_VCC_1P8_MIN_UV; >> vreg->max_uV = UFS_VREG_VCC_1P8_MAX_UV; >> } else { >> - vreg->min_uV = UFS_VREG_VCC_MIN_UV; >> - vreg->max_uV = UFS_VREG_VCC_MAX_UV; >> + prop = of_get_property(np, >> "vcc-voltage-level", &len); >> + if (!prop || (len != (2 * sizeof(__be32)))) { >> + dev_warn(dev, "%s vcc-voltage-level >> property.\n", >> + prop ? "invalid format" : >> "no"); >> + vreg->min_uV = UFS_VREG_VCC_MIN_UV; >> + vreg->max_uV = UFS_VREG_VCC_MAX_UV; >> + } else { >> + vreg->min_uV = be32_to_cpup(&prop[0]); >> + vreg->max_uV = be32_to_cpup(&prop[1]); >> + } >> } >> } else if (!strcmp(name, "vccq")) { >> vreg->min_uV = UFS_VREG_VCCQ_MIN_UV; >> -- > Maybe instead call ufshcd_populate_vreg with the new name, > To not break the function flow, and just add another else if ? Could you please clarify your comments? Are you suggesting to create a new function? Thank you. ^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH v1 2/2] scsi: ufs: Support reading UFS's Vcc voltage from device tree 2020-09-14 18:43 ` nguyenb @ 2020-09-15 6:51 ` Avri Altman 0 siblings, 0 replies; 21+ messages in thread From: Avri Altman @ 2020-09-15 6:51 UTC (permalink / raw) To: nguyenb Cc: cang, asutoshd, martin.petersen, linux-scsi, linux-arm-msm, Alim Akhtar, James E.J. Bottomley, YueHaibing, Bean Huo, open list > > Maybe instead call ufshcd_populate_vreg with the new name, > > To not break the function flow, and just add another else if ? > Could you please clarify your comments? Are you suggesting to create a > new function? > Thank you. No, just call ufshcd_populate_vreg with the new name, e.g. something like: diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c index 3db0af6..9798d4c 100644 --- a/drivers/scsi/ufs/ufshcd-pltfrm.c +++ b/drivers/scsi/ufs/ufshcd-pltfrm.c @@ -141,6 +141,8 @@ static int ufshcd_populate_vreg(struct device *dev, const char *name, vreg->min_uV = UFS_VREG_VCC_MIN_UV; vreg->max_uV = UFS_VREG_VCC_MAX_UV; } + } else if (!strcmp(name, "vcc-voltage-level")) { + /* add your changes here */ } else if (!strcmp(name, "vccq")) { vreg->min_uV = UFS_VREG_VCCQ_MIN_UV; vreg->max_uV = UFS_VREG_VCCQ_MAX_UV; @@ -177,8 +179,12 @@ static int ufshcd_parse_regulator_info(struct ufs_hba *hba) goto out; err = ufshcd_populate_vreg(dev, "vcc", &info->vcc); - if (err) - goto out; + if (err) { + err = ufshcd_populate_vreg(dev, "vcc-voltage-level", + &info->vcc); + if (err) + goto out; + } err = ufshcd_populate_vreg(dev, "vccq", &info->vccq); if (err) ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v1 2/2] scsi: ufs: Support reading UFS's Vcc voltage from device tree 2020-09-01 6:00 ` [PATCH v1 2/2] scsi: ufs: Support reading UFS's Vcc voltage from device tree Bao D. Nguyen 2020-09-13 9:37 ` Avri Altman @ 2020-09-15 13:46 ` Bjorn Andersson 1 sibling, 0 replies; 21+ messages in thread From: Bjorn Andersson @ 2020-09-15 13:46 UTC (permalink / raw) To: Bao D. Nguyen Cc: cang, asutoshd, martin.petersen, linux-scsi, linux-arm-msm, Alim Akhtar, Avri Altman, James E.J. Bottomley, YueHaibing, Bean Huo, open list On Tue 01 Sep 01:00 CDT 2020, Bao D. Nguyen wrote: > The UFS specifications supports a range of Vcc operating voltage > from 2.4-3.6V depending on the device and manufacturers. > Allows selecting the UFS Vcc voltage level by setting the > UFS's entry vcc-voltage-level in the device tree. If UFS's > vcc-voltage-level setting is not found in the device tree, > use default values provided by the driver. > As stated in the reply to patch 1, this is not the right approach. As long as you have static min/max values requested by the driver you should rely on the board's constraints for the regulator levels and instead remove the min_uV/max_uV code from the driver. Thanks, Bjorn > 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> > --- > drivers/scsi/ufs/ufshcd-pltfrm.c | 15 ++++++++++++--- > 1 file changed, 12 insertions(+), 3 deletions(-) > > diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c > index 3db0af6..48f429c 100644 > --- a/drivers/scsi/ufs/ufshcd-pltfrm.c > +++ b/drivers/scsi/ufs/ufshcd-pltfrm.c > @@ -104,10 +104,11 @@ static int ufshcd_parse_clock_info(struct ufs_hba *hba) > static int ufshcd_populate_vreg(struct device *dev, const char *name, > struct ufs_vreg **out_vreg) > { > - int ret = 0; > + int len, ret = 0; > char prop_name[MAX_PROP_SIZE]; > struct ufs_vreg *vreg = NULL; > struct device_node *np = dev->of_node; > + const __be32 *prop; > > if (!np) { > dev_err(dev, "%s: non DT initialization\n", __func__); > @@ -138,8 +139,16 @@ static int ufshcd_populate_vreg(struct device *dev, const char *name, > vreg->min_uV = UFS_VREG_VCC_1P8_MIN_UV; > vreg->max_uV = UFS_VREG_VCC_1P8_MAX_UV; > } else { > - vreg->min_uV = UFS_VREG_VCC_MIN_UV; > - vreg->max_uV = UFS_VREG_VCC_MAX_UV; > + prop = of_get_property(np, "vcc-voltage-level", &len); > + if (!prop || (len != (2 * sizeof(__be32)))) { > + dev_warn(dev, "%s vcc-voltage-level property.\n", > + prop ? "invalid format" : "no"); > + vreg->min_uV = UFS_VREG_VCC_MIN_UV; > + vreg->max_uV = UFS_VREG_VCC_MAX_UV; > + } else { > + vreg->min_uV = be32_to_cpup(&prop[0]); > + vreg->max_uV = be32_to_cpup(&prop[1]); > + } > } > } else if (!strcmp(name, "vccq")) { > vreg->min_uV = UFS_VREG_VCCQ_MIN_UV; > -- > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, > a Linux Foundation Collaborative Project > ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 0/2] Supports Reading UFS's Vcc Voltage Levels from DT 2020-09-01 6:00 [PATCH v1 0/2] Supports Reading UFS's Vcc Voltage Levels from DT Bao D. Nguyen 2020-09-01 6:00 ` [PATCH v1 1/2] scsi: dt-bindings: ufs: Add vcc-voltage-level for UFS Bao D. Nguyen 2020-09-01 6:00 ` [PATCH v1 2/2] scsi: ufs: Support reading UFS's Vcc voltage from device tree Bao D. Nguyen @ 2020-09-10 1:29 ` nguyenb 2 siblings, 0 replies; 21+ messages in thread From: nguyenb @ 2020-09-10 1:29 UTC (permalink / raw) To: cang, asutoshd, martin.petersen, linux-scsi; +Cc: linux-arm-msm On 2020-08-31 23:00, Bao D. Nguyen wrote: > UFS's specification supports a range of Vcc operating voltages. > Allows selecting the UFS Vcc operating voltage levels by reading > the UFS's vcc-voltage-level in the device tree. > > Bao D. Nguyen (2): > scsi: dt-bindings: ufs: Add vcc-voltage-level for UFS > scsi: ufs: Support reading UFS's Vcc voltage from device tree > > Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt | 2 ++ > drivers/scsi/ufs/ufshcd-pltfrm.c | 15 > ++++++++++++--- > 2 files changed, 14 insertions(+), 3 deletions(-) Hello, please help review the change and comment if any. Thanks! Bao ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2020-09-23 17:32 UTC | newest] Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-09-01 6:00 [PATCH v1 0/2] Supports Reading UFS's Vcc Voltage Levels from DT Bao D. Nguyen 2020-09-01 6:00 ` [PATCH v1 1/2] scsi: dt-bindings: ufs: Add vcc-voltage-level for UFS Bao D. Nguyen 2020-09-13 9:35 ` Avri Altman 2020-09-14 16:19 ` nguyenb 2020-09-14 18:35 ` Rob Herring 2020-09-15 8:10 ` nguyenb 2020-09-18 19:01 ` Rob Herring 2020-09-22 0:22 ` nguyenb 2020-09-22 0:36 ` Bjorn Andersson 2020-09-23 17:32 ` nguyenb 2020-09-15 4:41 ` Bjorn Andersson 2020-09-15 8:14 ` nguyenb 2020-09-15 13:43 ` Bjorn Andersson 2020-09-15 16:47 ` nguyenb 2020-09-15 18:36 ` Bjorn Andersson 2020-09-01 6:00 ` [PATCH v1 2/2] scsi: ufs: Support reading UFS's Vcc voltage from device tree Bao D. Nguyen 2020-09-13 9:37 ` Avri Altman 2020-09-14 18:43 ` nguyenb 2020-09-15 6:51 ` Avri Altman 2020-09-15 13:46 ` Bjorn Andersson 2020-09-10 1:29 ` [PATCH v1 0/2] Supports Reading UFS's Vcc Voltage Levels from DT nguyenb
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.