* Re: [RFT] ufs: qcom: drop custom Android boot parameters
2022-03-20 11:06 ` [RFT] ufs: qcom: drop custom Android boot parameters Krzysztof Kozlowski
@ 2022-03-21 11:22 ` Amit Pundir
2022-03-21 11:41 ` Alim Akhtar
` (3 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Amit Pundir @ 2022-03-21 11:22 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Andy Gross, Bjorn Andersson, Alim Akhtar, Avri Altman,
James E.J. Bottomley, Martin K. Petersen, linux-arm-msm,
linux-scsi, linux-kernel, Luca Weiss
On Sun, 20 Mar 2022 at 16:36, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> The QCOM UFS driver requires an androidboot.bootdevice command line
> argument matching the UFS device name. If the name is different, it
> refuses to probe. Thise androidboot.bootdevice is provided by
> stock/vendor (from an Android-based device) bootloader.
>
> This does not make sense from Linux point of view. Driver should be
> able to boot regardless of bootloader. Driver should not depend on some
> Android custom environment data.
>
No obvious regression on QCOM devboards DB845c (sdm845) and RB5
(sm8250), and Xiaomi Pocophone F1 (sdm845) running AOSP.
Tested-by: Amit Pundir <amit.pundir@linaro.org>
> Cc: Luca Weiss <luca.weiss@fairphone.com>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
>
> ---
>
> Not tested, please kindly provide tests.
>
> See also:
> https://lore.kernel.org/linux-devicetree/f61abc2b-3ce8-7b1f-3d28-8a4a03ec58eb@kernel.org/T/#u
> ---
> drivers/scsi/ufs/ufs-qcom.c | 15 ---------------
> 1 file changed, 15 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
> index 0d2e950d0865..586c0e567ff9 100644
> --- a/drivers/scsi/ufs/ufs-qcom.c
> +++ b/drivers/scsi/ufs/ufs-qcom.c
> @@ -957,18 +957,6 @@ static const struct reset_control_ops ufs_qcom_reset_ops = {
> .deassert = ufs_qcom_reset_deassert,
> };
>
> -#define ANDROID_BOOT_DEV_MAX 30
> -static char android_boot_dev[ANDROID_BOOT_DEV_MAX];
> -
> -#ifndef MODULE
> -static int __init get_android_boot_dev(char *str)
> -{
> - strlcpy(android_boot_dev, str, ANDROID_BOOT_DEV_MAX);
> - return 1;
> -}
> -__setup("androidboot.bootdevice=", get_android_boot_dev);
> -#endif
> -
> /**
> * ufs_qcom_init - bind phy with controller
> * @hba: host controller instance
> @@ -988,9 +976,6 @@ static int ufs_qcom_init(struct ufs_hba *hba)
> struct resource *res;
> struct ufs_clk_info *clki;
>
> - if (strlen(android_boot_dev) && strcmp(android_boot_dev, dev_name(dev)))
> - return -ENODEV;
> -
> host = devm_kzalloc(dev, sizeof(*host), GFP_KERNEL);
> if (!host) {
> err = -ENOMEM;
> --
> 2.32.0
>
^ permalink raw reply [flat|nested] 7+ messages in thread* RE: [RFT] ufs: qcom: drop custom Android boot parameters
2022-03-20 11:06 ` [RFT] ufs: qcom: drop custom Android boot parameters Krzysztof Kozlowski
2022-03-21 11:22 ` Amit Pundir
@ 2022-03-21 11:41 ` Alim Akhtar
2022-03-21 11:58 ` Luca Weiss
` (2 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Alim Akhtar @ 2022-03-21 11:41 UTC (permalink / raw)
To: 'Krzysztof Kozlowski', 'Andy Gross',
'Bjorn Andersson', 'Avri Altman',
'James E.J. Bottomley', 'Martin K. Petersen',
linux-arm-msm, linux-scsi, linux-kernel
Cc: 'Luca Weiss'
>-----Original Message-----
>From: Krzysztof Kozlowski [mailto:krzk@kernel.org]
>Sent: Sunday, March 20, 2022 4:36 PM
>To: Andy Gross <agross@kernel.org>; Bjorn Andersson
><bjorn.andersson@linaro.org>; Alim Akhtar <alim.akhtar@samsung.com>;
>Avri Altman <avri.altman@wdc.com>; James E.J. Bottomley
><jejb@linux.ibm.com>; Martin K. Petersen <martin.petersen@oracle.com>;
>linux-arm-msm@vger.kernel.org; linux-scsi@vger.kernel.org; linux-
>kernel@vger.kernel.org
>Cc: Krzysztof Kozlowski <krzk@kernel.org>; Luca Weiss
><luca.weiss@fairphone.com>
>Subject: [RFT] ufs: qcom: drop custom Android boot parameters
>
>The QCOM UFS driver requires an androidboot.bootdevice command line
>argument matching the UFS device name. If the name is different, it
refuses
>to probe. Thise androidboot.bootdevice is provided by stock/vendor (from
an
In case you are resending
s/thise/this
Rest looks good.
>Android-based device) bootloader.
>
>This does not make sense from Linux point of view. Driver should be able
to
>boot regardless of bootloader. Driver should not depend on some Android
>custom environment data.
>
>Cc: Luca Weiss <luca.weiss@fairphone.com>
>Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
>
>---
>
>Not tested, please kindly provide tests.
>
>See also:
>https://lore.kernel.org/linux-devicetree/f61abc2b-3ce8-7b1f-3d28-
>8a4a03ec58eb@kernel.org/T/#u
>---
> drivers/scsi/ufs/ufs-qcom.c | 15 ---------------
> 1 file changed, 15 deletions(-)
>
>diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
index
>0d2e950d0865..586c0e567ff9 100644
>--- a/drivers/scsi/ufs/ufs-qcom.c
>+++ b/drivers/scsi/ufs/ufs-qcom.c
>@@ -957,18 +957,6 @@ static const struct reset_control_ops
>ufs_qcom_reset_ops = {
> .deassert = ufs_qcom_reset_deassert,
> };
>
>-#define ANDROID_BOOT_DEV_MAX 30
>-static char android_boot_dev[ANDROID_BOOT_DEV_MAX];
>-
>-#ifndef MODULE
>-static int __init get_android_boot_dev(char *str) -{
>- strlcpy(android_boot_dev, str, ANDROID_BOOT_DEV_MAX);
>- return 1;
>-}
>-__setup("androidboot.bootdevice=", get_android_boot_dev); -#endif
>-
> /**
> * ufs_qcom_init - bind phy with controller
> * @hba: host controller instance
>@@ -988,9 +976,6 @@ static int ufs_qcom_init(struct ufs_hba *hba)
> struct resource *res;
> struct ufs_clk_info *clki;
>
>- if (strlen(android_boot_dev) && strcmp(android_boot_dev,
>dev_name(dev)))
>- return -ENODEV;
>-
> host = devm_kzalloc(dev, sizeof(*host), GFP_KERNEL);
> if (!host) {
> err = -ENOMEM;
>--
>2.32.0
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [RFT] ufs: qcom: drop custom Android boot parameters
2022-03-20 11:06 ` [RFT] ufs: qcom: drop custom Android boot parameters Krzysztof Kozlowski
2022-03-21 11:22 ` Amit Pundir
2022-03-21 11:41 ` Alim Akhtar
@ 2022-03-21 11:58 ` Luca Weiss
2022-03-21 12:13 ` Luca Weiss
2022-03-21 13:44 ` Brian Masney
2022-03-21 14:16 ` Bjorn Andersson
4 siblings, 1 reply; 7+ messages in thread
From: Luca Weiss @ 2022-03-21 11:58 UTC (permalink / raw)
To: Krzysztof Kozlowski, Andy Gross, Bjorn Andersson, Alim Akhtar,
Avri Altman, James E.J. Bottomley, Martin K. Petersen,
linux-arm-msm, linux-scsi, linux-kernel
Hi Krzysztof,
On Sun Mar 20, 2022 at 12:06 PM CET, Krzysztof Kozlowski wrote:
> The QCOM UFS driver requires an androidboot.bootdevice command line
> argument matching the UFS device name. If the name is different, it
> refuses to probe. Thise androidboot.bootdevice is provided by
> stock/vendor (from an Android-based device) bootloader.
>
> This does not make sense from Linux point of view. Driver should be
> able to boot regardless of bootloader. Driver should not depend on some
> Android custom environment data.
>
> Cc: Luca Weiss <luca.weiss@fairphone.com>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
As expected this makes UFS probe even if the node is named ufs@1d84000
on my device.
While I don't know why the code existed in the first place, it was added
back in 2015 with the introduction of the driver, so probably it's just
some remains from downstream that weren't cleaned up back then.
With this commit also 6b9afd8f96c6 ("arm64: dts: qcom: sm8250: change
ufs node name to ufshc") could be reverted (but it would probably make
more sense to rename all ufshc@ to ufs@ in a new commit).
Tested-by: Luca Weiss <luca.weiss@fairphone.com>
Regards
Luca
>
> ---
>
> Not tested, please kindly provide tests.
>
> See also:
> https://lore.kernel.org/linux-devicetree/f61abc2b-3ce8-7b1f-3d28-8a4a03ec58eb@kernel.org/T/#u
> ---
> drivers/scsi/ufs/ufs-qcom.c | 15 ---------------
> 1 file changed, 15 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
> index 0d2e950d0865..586c0e567ff9 100644
> --- a/drivers/scsi/ufs/ufs-qcom.c
> +++ b/drivers/scsi/ufs/ufs-qcom.c
> @@ -957,18 +957,6 @@ static const struct reset_control_ops ufs_qcom_reset_ops = {
> .deassert = ufs_qcom_reset_deassert,
> };
>
> -#define ANDROID_BOOT_DEV_MAX 30
> -static char android_boot_dev[ANDROID_BOOT_DEV_MAX];
> -
> -#ifndef MODULE
> -static int __init get_android_boot_dev(char *str)
> -{
> - strlcpy(android_boot_dev, str, ANDROID_BOOT_DEV_MAX);
> - return 1;
> -}
> -__setup("androidboot.bootdevice=", get_android_boot_dev);
> -#endif
> -
> /**
> * ufs_qcom_init - bind phy with controller
> * @hba: host controller instance
> @@ -988,9 +976,6 @@ static int ufs_qcom_init(struct ufs_hba *hba)
> struct resource *res;
> struct ufs_clk_info *clki;
>
> - if (strlen(android_boot_dev) && strcmp(android_boot_dev, dev_name(dev)))
> - return -ENODEV;
> -
> host = devm_kzalloc(dev, sizeof(*host), GFP_KERNEL);
> if (!host) {
> err = -ENOMEM;
> --
> 2.32.0
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [RFT] ufs: qcom: drop custom Android boot parameters
2022-03-21 11:58 ` Luca Weiss
@ 2022-03-21 12:13 ` Luca Weiss
0 siblings, 0 replies; 7+ messages in thread
From: Luca Weiss @ 2022-03-21 12:13 UTC (permalink / raw)
To: Luca Weiss, Krzysztof Kozlowski, Andy Gross, Bjorn Andersson,
Alim Akhtar, Avri Altman, James E.J. Bottomley,
Martin K. Petersen, linux-arm-msm, linux-scsi, linux-kernel
Hi all again,
On Mon Mar 21, 2022 at 12:58 PM CET, Luca Weiss wrote:
> Hi Krzysztof,
>
> On Sun Mar 20, 2022 at 12:06 PM CET, Krzysztof Kozlowski wrote:
> > The QCOM UFS driver requires an androidboot.bootdevice command line
> > argument matching the UFS device name. If the name is different, it
> > refuses to probe. Thise androidboot.bootdevice is provided by
> > stock/vendor (from an Android-based device) bootloader.
> >
> > This does not make sense from Linux point of view. Driver should be
> > able to boot regardless of bootloader. Driver should not depend on some
> > Android custom environment data.
> >
> > Cc: Luca Weiss <luca.weiss@fairphone.com>
> > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
>
> As expected this makes UFS probe even if the node is named ufs@1d84000
> on my device.
>
> While I don't know why the code existed in the first place, it was added
> back in 2015 with the introduction of the driver, so probably it's just
> some remains from downstream that weren't cleaned up back then.
I went digging a bit and found the original commit from downstream:
https://source.codeaurora.org/quic/la/kernel/msm-3.18/commit/?id=15e2c78d6a7ec73be941e058671942fd9f72b128
<quote>
Boot device can be either UFS or eMMC which means if eMMC is the boot
device, probing UFS device is not desirable as it's not going to be
used after probing. Kernel command line parameter "android.bootdevice"
tells the kernel about the boot device so look at this boot device
parameter to know whether to probe UFS device or not.
</quote>
Should be safe to remove.
Regards
Luca
>
> With this commit also 6b9afd8f96c6 ("arm64: dts: qcom: sm8250: change
> ufs node name to ufshc") could be reverted (but it would probably make
> more sense to rename all ufshc@ to ufs@ in a new commit).
>
> Tested-by: Luca Weiss <luca.weiss@fairphone.com>
>
> Regards
> Luca
>
> >
> > ---
> >
> > Not tested, please kindly provide tests.
> >
> > See also:
> > https://lore.kernel.org/linux-devicetree/f61abc2b-3ce8-7b1f-3d28-8a4a03ec58eb@kernel.org/T/#u
> > ---
> > drivers/scsi/ufs/ufs-qcom.c | 15 ---------------
> > 1 file changed, 15 deletions(-)
> >
> > diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
> > index 0d2e950d0865..586c0e567ff9 100644
> > --- a/drivers/scsi/ufs/ufs-qcom.c
> > +++ b/drivers/scsi/ufs/ufs-qcom.c
> > @@ -957,18 +957,6 @@ static const struct reset_control_ops ufs_qcom_reset_ops = {
> > .deassert = ufs_qcom_reset_deassert,
> > };
> >
> > -#define ANDROID_BOOT_DEV_MAX 30
> > -static char android_boot_dev[ANDROID_BOOT_DEV_MAX];
> > -
> > -#ifndef MODULE
> > -static int __init get_android_boot_dev(char *str)
> > -{
> > - strlcpy(android_boot_dev, str, ANDROID_BOOT_DEV_MAX);
> > - return 1;
> > -}
> > -__setup("androidboot.bootdevice=", get_android_boot_dev);
> > -#endif
> > -
> > /**
> > * ufs_qcom_init - bind phy with controller
> > * @hba: host controller instance
> > @@ -988,9 +976,6 @@ static int ufs_qcom_init(struct ufs_hba *hba)
> > struct resource *res;
> > struct ufs_clk_info *clki;
> >
> > - if (strlen(android_boot_dev) && strcmp(android_boot_dev, dev_name(dev)))
> > - return -ENODEV;
> > -
> > host = devm_kzalloc(dev, sizeof(*host), GFP_KERNEL);
> > if (!host) {
> > err = -ENOMEM;
> > --
> > 2.32.0
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFT] ufs: qcom: drop custom Android boot parameters
2022-03-20 11:06 ` [RFT] ufs: qcom: drop custom Android boot parameters Krzysztof Kozlowski
` (2 preceding siblings ...)
2022-03-21 11:58 ` Luca Weiss
@ 2022-03-21 13:44 ` Brian Masney
2022-03-21 14:16 ` Bjorn Andersson
4 siblings, 0 replies; 7+ messages in thread
From: Brian Masney @ 2022-03-21 13:44 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Andy Gross, Bjorn Andersson, Alim Akhtar, Avri Altman,
James E.J. Bottomley, Martin K. Petersen, linux-arm-msm,
linux-scsi, linux-kernel, Luca Weiss
On Sun, Mar 20, 2022 at 12:06:16PM +0100, Krzysztof Kozlowski wrote:
> The QCOM UFS driver requires an androidboot.bootdevice command line
> argument matching the UFS device name. If the name is different, it
> refuses to probe. Thise androidboot.bootdevice is provided by
> stock/vendor (from an Android-based device) bootloader.
>
> This does not make sense from Linux point of view. Driver should be
> able to boot regardless of bootloader. Driver should not depend on some
> Android custom environment data.
>
> Cc: Luca Weiss <luca.weiss@fairphone.com>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
I encountered that same code a few months ago and thought then that
this code doesn't belong here.
Reviewed-by: Brian Masney <bmasney@redhat.com>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [RFT] ufs: qcom: drop custom Android boot parameters
2022-03-20 11:06 ` [RFT] ufs: qcom: drop custom Android boot parameters Krzysztof Kozlowski
` (3 preceding siblings ...)
2022-03-21 13:44 ` Brian Masney
@ 2022-03-21 14:16 ` Bjorn Andersson
4 siblings, 0 replies; 7+ messages in thread
From: Bjorn Andersson @ 2022-03-21 14:16 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Andy Gross, Alim Akhtar, Avri Altman, James E.J. Bottomley,
Martin K. Petersen, linux-arm-msm, linux-scsi, linux-kernel,
Luca Weiss
On Sun 20 Mar 06:06 CDT 2022, Krzysztof Kozlowski wrote:
> The QCOM UFS driver requires an androidboot.bootdevice command line
> argument matching the UFS device name. If the name is different, it
> refuses to probe. Thise androidboot.bootdevice is provided by
> stock/vendor (from an Android-based device) bootloader.
>
> This does not make sense from Linux point of view. Driver should be
> able to boot regardless of bootloader. Driver should not depend on some
> Android custom environment data.
>
> Cc: Luca Weiss <luca.weiss@fairphone.com>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
>
> Not tested, please kindly provide tests.
>
> See also:
> https://lore.kernel.org/linux-devicetree/f61abc2b-3ce8-7b1f-3d28-8a4a03ec58eb@kernel.org/T/#u
> ---
> drivers/scsi/ufs/ufs-qcom.c | 15 ---------------
> 1 file changed, 15 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
> index 0d2e950d0865..586c0e567ff9 100644
> --- a/drivers/scsi/ufs/ufs-qcom.c
> +++ b/drivers/scsi/ufs/ufs-qcom.c
> @@ -957,18 +957,6 @@ static const struct reset_control_ops ufs_qcom_reset_ops = {
> .deassert = ufs_qcom_reset_deassert,
> };
>
> -#define ANDROID_BOOT_DEV_MAX 30
> -static char android_boot_dev[ANDROID_BOOT_DEV_MAX];
> -
> -#ifndef MODULE
> -static int __init get_android_boot_dev(char *str)
> -{
> - strlcpy(android_boot_dev, str, ANDROID_BOOT_DEV_MAX);
> - return 1;
> -}
> -__setup("androidboot.bootdevice=", get_android_boot_dev);
> -#endif
> -
> /**
> * ufs_qcom_init - bind phy with controller
> * @hba: host controller instance
> @@ -988,9 +976,6 @@ static int ufs_qcom_init(struct ufs_hba *hba)
> struct resource *res;
> struct ufs_clk_info *clki;
>
> - if (strlen(android_boot_dev) && strcmp(android_boot_dev, dev_name(dev)))
> - return -ENODEV;
> -
> host = devm_kzalloc(dev, sizeof(*host), GFP_KERNEL);
> if (!host) {
> err = -ENOMEM;
> --
> 2.32.0
>
^ permalink raw reply [flat|nested] 7+ messages in thread