Message ID | 20201201065114.1001-1-stanley.chu@mediatek.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [v2] scsi: ufs: Remove pre-defined initial voltage values of device powers | expand |
> > UFS specficication allows different VCC configurations for UFS devices, > for example, > (1). 2.70V - 3.60V (Activated by default in UFS core driver) > (2). 1.70V - 1.95V (Activated if "vcc-supply-1p8" is declared in > device tree) > (3). 2.40V - 2.70V (Supported since UFS 3.x) > > With the introduction of UFS 3.x products, an issue is happening that > UFS driver will use wrong "min_uV-max_uV" values to configure the > voltage of VCC regulator on UFU 3.x products with the configuration (3) > used. > > To solve this issue, we simply remove pre-defined initial VCC voltage > values in UFS core driver with below reasons, > > 1. UFS specifications do not define how to detect the VCC configuration > supported by attached device. > > 2. Device tree already supports standard regulator properties. > > Therefore VCC voltage shall be defined correctly in device tree, and > shall not changed by UFS driver. What UFS driver needs to do is simply > enable or disable the VCC regulator only. > > Similar change is applied to VCCQ and VCCQ2 as well. > > Note that we keep struct ufs_vreg unchanged. This is allow vendors to > configure proper min_uV and max_uV of any regulators to make > regulator_set_voltage() works during regulator toggling flow. > Without specific vendor configurations, min_uV and max_uV will be NULL > by default and UFS core driver will enable or disable the regulator > only without adjusting its voltage. > > Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org> > Signed-off-by: Stanley Chu <stanley.chu@mediatek.com> Acked-by: Avri Altman <avri.altman@wdc.com>
On Wed, 2020-12-02 at 00:19 -0800, nguyenb@codeaurora.org wrote: > On 2020-11-30 22:51, Stanley Chu wrote: > > UFS specficication allows different VCC configurations for UFS devices, > > for example, > > (1). 2.70V - 3.60V (Activated by default in UFS core driver) > > (2). 1.70V - 1.95V (Activated if "vcc-supply-1p8" is declared in > > device tree) > > (3). 2.40V - 2.70V (Supported since UFS 3.x) > > > > With the introduction of UFS 3.x products, an issue is happening that > > UFS driver will use wrong "min_uV-max_uV" values to configure the > > voltage of VCC regulator on UFU 3.x products with the configuration (3) > > used. > > > > To solve this issue, we simply remove pre-defined initial VCC voltage > > values in UFS core driver with below reasons, > > > > 1. UFS specifications do not define how to detect the VCC configuration > > supported by attached device. > > > > 2. Device tree already supports standard regulator properties. > > > > Therefore VCC voltage shall be defined correctly in device tree, and > > shall not changed by UFS driver. What UFS driver needs to do is simply > > enable or disable the VCC regulator only. > > > > Similar change is applied to VCCQ and VCCQ2 as well. > > > > Note that we keep struct ufs_vreg unchanged. This is allow vendors to > > configure proper min_uV and max_uV of any regulators to make > > regulator_set_voltage() works during regulator toggling flow. > > Without specific vendor configurations, min_uV and max_uV will be NULL > > by default and UFS core driver will enable or disable the regulator > > only without adjusting its voltage. > > > > Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org> > > Signed-off-by: Stanley Chu <stanley.chu@mediatek.com> > > --- > > drivers/scsi/ufs/ufshcd-pltfrm.c | 16 ---------------- > > 1 file changed, 16 deletions(-) > > > > diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c > > b/drivers/scsi/ufs/ufshcd-pltfrm.c > > index a6f76399b3ae..09e2f04bf4f6 100644 > > --- a/drivers/scsi/ufs/ufshcd-pltfrm.c > > +++ b/drivers/scsi/ufs/ufshcd-pltfrm.c > > @@ -133,22 +133,6 @@ static int ufshcd_populate_vreg(struct device > > *dev, const char *name, > > vreg->max_uA = 0; > > } > > > > - if (!strcmp(name, "vcc")) { > > - if (of_property_read_bool(np, "vcc-supply-1p8")) { > > - 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; > > - } > > - } else if (!strcmp(name, "vccq")) { > > - vreg->min_uV = UFS_VREG_VCCQ_MIN_UV; > > - vreg->max_uV = UFS_VREG_VCCQ_MAX_UV; > > - } else if (!strcmp(name, "vccq2")) { > > - vreg->min_uV = UFS_VREG_VCCQ2_MIN_UV; > > - vreg->max_uV = UFS_VREG_VCCQ2_MAX_UV; > > - } > > - > > goto out; > Do we need this "goto out;"? Will remove it in next version. Thanks for remind. Stanley Chu > > > > > out:
diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c index a6f76399b3ae..09e2f04bf4f6 100644 --- a/drivers/scsi/ufs/ufshcd-pltfrm.c +++ b/drivers/scsi/ufs/ufshcd-pltfrm.c @@ -133,22 +133,6 @@ static int ufshcd_populate_vreg(struct device *dev, const char *name, vreg->max_uA = 0; } - if (!strcmp(name, "vcc")) { - if (of_property_read_bool(np, "vcc-supply-1p8")) { - 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; - } - } else if (!strcmp(name, "vccq")) { - vreg->min_uV = UFS_VREG_VCCQ_MIN_UV; - vreg->max_uV = UFS_VREG_VCCQ_MAX_UV; - } else if (!strcmp(name, "vccq2")) { - vreg->min_uV = UFS_VREG_VCCQ2_MIN_UV; - vreg->max_uV = UFS_VREG_VCCQ2_MAX_UV; - } - goto out; out: