* RE: [PATCH v2] scsi: ufs: Remove pre-defined initial voltage values of device powers
2020-12-01 6:51 [PATCH v2] scsi: ufs: Remove pre-defined initial voltage values of device powers Stanley Chu
@ 2020-12-01 7:01 ` Avri Altman
2020-12-01 16:43 ` Asutosh Das (asd)
2020-12-02 8:19 ` nguyenb
2 siblings, 0 replies; 5+ messages in thread
From: Avri Altman @ 2020-12-01 7:01 UTC (permalink / raw)
To: Stanley Chu, linux-scsi@vger.kernel.org,
martin.petersen@oracle.com, alim.akhtar@samsung.com,
jejb@linux.ibm.com
Cc: bjorn.andersson@linaro.org, alice.chao@mediatek.com,
bvanassche@acm.org, andy.teng@mediatek.com, cc.chou@mediatek.com,
chun-hung.wu@mediatek.com, kuohong.wang@mediatek.com,
linux-kernel@vger.kernel.org, nguyenb@codeaurora.org,
jiajie.hao@mediatek.com, cang@codeaurora.org,
linux-mediatek@lists.infradead.org, peter.wang@mediatek.com,
matthias.bgg@gmail.com, beanhuo@micron.com,
chaotian.jing@mediatek.com, linux-arm-kernel@lists.infradead.org,
asutoshd@codeaurora.org
>
> 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>
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] scsi: ufs: Remove pre-defined initial voltage values of device powers
2020-12-01 6:51 [PATCH v2] scsi: ufs: Remove pre-defined initial voltage values of device powers Stanley Chu
2020-12-01 7:01 ` Avri Altman
@ 2020-12-01 16:43 ` Asutosh Das (asd)
2020-12-02 8:19 ` nguyenb
2 siblings, 0 replies; 5+ messages in thread
From: Asutosh Das (asd) @ 2020-12-01 16:43 UTC (permalink / raw)
To: Stanley Chu, linux-scsi, martin.petersen, avri.altman,
alim.akhtar, jejb
Cc: alice.chao, bvanassche, andy.teng, cc.chou, chun-hung.wu,
kuohong.wang, linux-kernel, nguyenb, jiajie.hao, cang,
linux-mediatek, peter.wang, matthias.bgg, bjorn.andersson,
chaotian.jing, linux-arm-kernel, beanhuo
On 11/30/2020 10:51 PM, 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>
> ---
Reviewed-by: Asutosh Das <asutoshd@codeaurora.org>
> 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;
>
> out:
>
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
Linux Foundation Collaborative Project
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] scsi: ufs: Remove pre-defined initial voltage values of device powers
2020-12-01 6:51 [PATCH v2] scsi: ufs: Remove pre-defined initial voltage values of device powers Stanley Chu
2020-12-01 7:01 ` Avri Altman
2020-12-01 16:43 ` Asutosh Das (asd)
@ 2020-12-02 8:19 ` nguyenb
2020-12-02 8:22 ` Stanley Chu
2 siblings, 1 reply; 5+ messages in thread
From: nguyenb @ 2020-12-02 8:19 UTC (permalink / raw)
To: Stanley Chu
Cc: bjorn.andersson, matthias.bgg, cang, alim.akhtar, beanhuo,
bvanassche, linux-scsi, peter.wang, cc.chou, andy.teng, jejb,
chun-hung.wu, avri.altman, linux-mediatek, jiajie.hao,
chaotian.jing, linux-arm-kernel, martin.petersen, kuohong.wang,
linux-kernel, asutoshd, alice.chao
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;"?
>
> out:
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] scsi: ufs: Remove pre-defined initial voltage values of device powers
2020-12-02 8:19 ` nguyenb
@ 2020-12-02 8:22 ` Stanley Chu
0 siblings, 0 replies; 5+ messages in thread
From: Stanley Chu @ 2020-12-02 8:22 UTC (permalink / raw)
To: nguyenb
Cc: bjorn.andersson, matthias.bgg, cang, alim.akhtar, beanhuo,
bvanassche, linux-scsi, peter.wang, cc.chou, andy.teng, jejb,
chun-hung.wu, avri.altman, linux-mediatek, jiajie.hao,
chaotian.jing, linux-arm-kernel, martin.petersen, kuohong.wang,
linux-kernel, asutoshd, alice.chao
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:
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 5+ messages in thread