[PATCH 0/1] ufs: mediatek: Fix probe failure on MT8395 SoC

[PATCH 0/1] ufs: mediatek: Fix probe failure on MT8395 SoC

[Mary Guillemard]

Hello,

This series fixes probe failure on MT8395 SoC caused by LSDBS field
in the CAP register being set while the controller only support
UFSHCI 2.1.

This is based on 6.11/scsi-fixes as it requires the LSDBS quirk from [1].

This was tested on a Radxa NIO 12L with [2] and appropriate dt changes.

[1]https://lore.kernel.org/linux-scsi/20240816-ufs-bug-fix-v3-1-e6fe0e18e2a3@linaro.org/
[2]https://lore.kernel.org/all/20240612074309.50278-1-angelogioacchino.delregno@collabora.com/

Mary Guillemard (1):
  scsi: ufs-mediatek: Add UFSHCD_QUIRK_BROKEN_LSDBS_CAP

 drivers/ufs/host/ufs-mediatek.c | 3 +++
 1 file changed, 3 insertions(+)


base-commit: cbaac68987b8699397df29413b33bd51f5255255
-- 
2.46.0

[PATCH 1/1] scsi: ufs-mediatek: Add UFSHCD_QUIRK_BROKEN_LSDBS_CAP

[Mary Guillemard]

MT8183 supports UFSHCI 2.1 spec, but report a bogus value of 1 in the
reserved part for the Legacy Single Doorbell Support (LSDBS) capability.

This set UFSHCD_QUIRK_BROKEN_LSDBS_CAP when MCQ support is explicitly
disabled, allowing the device to be properly registered.

Signed-off-by: Mary Guillemard <mary@mary.zone>
---
 drivers/ufs/host/ufs-mediatek.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/ufs/host/ufs-mediatek.c b/drivers/ufs/host/ufs-mediatek.c
index 02c9064284e1..9a5919434c4e 100644
--- a/drivers/ufs/host/ufs-mediatek.c
+++ b/drivers/ufs/host/ufs-mediatek.c
@@ -1026,6 +1026,9 @@ static int ufs_mtk_init(struct ufs_hba *hba)
 	if (host->caps & UFS_MTK_CAP_DISABLE_AH8)
 		hba->caps |= UFSHCD_CAP_HIBERN8_WITH_CLK_GATING;
 
+	if (host->caps & UFS_MTK_CAP_DISABLE_MCQ)
+		hba->quirks |= UFSHCD_QUIRK_BROKEN_LSDBS_CAP;
+
 	ufs_mtk_init_clocks(hba);
 
 	/*
-- 
2.46.0

Re: [PATCH 1/1] scsi: ufs-mediatek: Add UFSHCD_QUIRK_BROKEN_LSDBS_CAP

[Manivannan Sadhasivam]

On Mon, Aug 19, 2024 at 12:24:42AM +0200, Mary Guillemard wrote:
> MT8183 supports UFSHCI 2.1 spec, but report a bogus value of 1 in the
> reserved part for the Legacy Single Doorbell Support (LSDBS) capability.
> 

Wow... I never thought that this quirk will be used outside of Qcom SoCs...

> This set UFSHCD_QUIRK_BROKEN_LSDBS_CAP when MCQ support is explicitly
> disabled, allowing the device to be properly registered.
> 
> Signed-off-by: Mary Guillemard <mary@mary.zone>
> ---
>  drivers/ufs/host/ufs-mediatek.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/ufs/host/ufs-mediatek.c b/drivers/ufs/host/ufs-mediatek.c
> index 02c9064284e1..9a5919434c4e 100644
> --- a/drivers/ufs/host/ufs-mediatek.c
> +++ b/drivers/ufs/host/ufs-mediatek.c
> @@ -1026,6 +1026,9 @@ static int ufs_mtk_init(struct ufs_hba *hba)
>  	if (host->caps & UFS_MTK_CAP_DISABLE_AH8)
>  		hba->caps |= UFSHCD_CAP_HIBERN8_WITH_CLK_GATING;
>  
> +	if (host->caps & UFS_MTK_CAP_DISABLE_MCQ)

How can this be the deciding factor? You said above that the issue is with
MT8183 SoC. So why not just use the quirk only for that platform?

- Mani

> +		hba->quirks |= UFSHCD_QUIRK_BROKEN_LSDBS_CAP;
> +
>  	ufs_mtk_init_clocks(hba);
>  
>  	/*
> -- 
> 2.46.0
> 
> 

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH 1/1] scsi: ufs-mediatek: Add UFSHCD_QUIRK_BROKEN_LSDBS_CAP

[Mary Guillemard]

On Mon, Aug 19, 2024 at 05:38:52PM +0530, Manivannan Sadhasivam wrote:
> On Mon, Aug 19, 2024 at 12:24:42AM +0200, Mary Guillemard wrote:
> > MT8183 supports UFSHCI 2.1 spec, but report a bogus value of 1 in the
> > reserved part for the Legacy Single Doorbell Support (LSDBS) capability.
> > 
> 
> Wow... I never thought that this quirk will be used outside of Qcom SoCs...
>

Yeah I found that by trial and error some weeks ago and noticed your
serie while looking to upstream this change, quite funny to see other
vendors having the same quirk here.
 
> > This set UFSHCD_QUIRK_BROKEN_LSDBS_CAP when MCQ support is explicitly
> > disabled, allowing the device to be properly registered.
> > 
> > Signed-off-by: Mary Guillemard <mary@mary.zone>
> > ---
> >  drivers/ufs/host/ufs-mediatek.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/ufs/host/ufs-mediatek.c b/drivers/ufs/host/ufs-mediatek.c
> > index 02c9064284e1..9a5919434c4e 100644
> > --- a/drivers/ufs/host/ufs-mediatek.c
> > +++ b/drivers/ufs/host/ufs-mediatek.c
> > @@ -1026,6 +1026,9 @@ static int ufs_mtk_init(struct ufs_hba *hba)
> >  	if (host->caps & UFS_MTK_CAP_DISABLE_AH8)
> >  		hba->caps |= UFSHCD_CAP_HIBERN8_WITH_CLK_GATING;
> >  
> > +	if (host->caps & UFS_MTK_CAP_DISABLE_MCQ)
> 
> How can this be the deciding factor? You said above that the issue is with
> MT8183 SoC. So why not just use the quirk only for that platform?
> 
> - Mani
>

So my current assumption is that it also affect other Mediatek SoCs
that are also based on UFS 2.1 spec but I cannot check this.

Instead, we know that if MCQ isn't supported, we must fallback to LSDB
as there is no other ways to drive the device.

UFS_MTK_CAP_DISABLE_MCQ (mediatek,ufs-disable-mcq) being unused upstream,
I think that's an acceptable fix.

Another way to handle this would be to add a new dt property and add it
to ufs_mtk_host_caps but I feel that my approach should be enough. 

> > +		hba->quirks |= UFSHCD_QUIRK_BROKEN_LSDBS_CAP;
> > +
> >  	ufs_mtk_init_clocks(hba);
> >  
> >  	/*
> > -- 
> > 2.46.0
> > 
> > 
> 
> -- 
> மணிவண்ணன் சதாசிவம்

- Mary

Re: [PATCH 1/1] scsi: ufs-mediatek: Add UFSHCD_QUIRK_BROKEN_LSDBS_CAP

[Peter Wang (王信友)]

On Mon, 2024-08-19 at 20:17 +0200, Mary Guillemard wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  On Mon, Aug 19, 2024 at 05:38:52PM +0530, Manivannan Sadhasivam
> wrote:
> > On Mon, Aug 19, 2024 at 12:24:42AM +0200, Mary Guillemard wrote:
> > > MT8183 supports UFSHCI 2.1 spec, but report a bogus value of 1 in
> the
> > > reserved part for the Legacy Single Doorbell Support (LSDBS)
> capability.
> > > 
> > 
> > Wow... I never thought that this quirk will be used outside of Qcom
> SoCs...
> >
> 
> Yeah I found that by trial and error some weeks ago and noticed your
> serie while looking to upstream this change, quite funny to see other
> vendors having the same quirk here.
>  
> > > This set UFSHCD_QUIRK_BROKEN_LSDBS_CAP when MCQ support is
> explicitly
> > > disabled, allowing the device to be properly registered.
> > > 
> > > Signed-off-by: Mary Guillemard <mary@mary.zone>
> > > ---
> > >  drivers/ufs/host/ufs-mediatek.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/drivers/ufs/host/ufs-mediatek.c
> b/drivers/ufs/host/ufs-mediatek.c
> > > index 02c9064284e1..9a5919434c4e 100644
> > > --- a/drivers/ufs/host/ufs-mediatek.c
> > > +++ b/drivers/ufs/host/ufs-mediatek.c
> > > @@ -1026,6 +1026,9 @@ static int ufs_mtk_init(struct ufs_hba
> *hba)
> > >  if (host->caps & UFS_MTK_CAP_DISABLE_AH8)
> > >  hba->caps |= UFSHCD_CAP_HIBERN8_WITH_CLK_GATING;
> > >  
> > > +if (host->caps & UFS_MTK_CAP_DISABLE_MCQ)
> > 
> > How can this be the deciding factor? You said above that the issue
> is with
> > MT8183 SoC. So why not just use the quirk only for that platform?
> > 
> > - Mani
> >
> 
> So my current assumption is that it also affect other Mediatek SoCs
> that are also based on UFS 2.1 spec but I cannot check this.
> 
> Instead, we know that if MCQ isn't supported, we must fallback to
> LSDB
> as there is no other ways to drive the device.
> 
> UFS_MTK_CAP_DISABLE_MCQ (mediatek,ufs-disable-mcq) being unused
> upstream,
> I think that's an acceptable fix.
> 
> Another way to handle this would be to add a new dt property and add
> it
> to ufs_mtk_host_caps but I feel that my approach should be enough. 
> 

Hi Mary,

Yes, the MT8395 indeed requires the LSDBS flag, 
but not every MediaTek legacy chip does.
So setting the LSDBS flag here is appropriate.

Thanks.
Peter


> > > +hba->quirks |= UFSHCD_QUIRK_BROKEN_LSDBS_CAP;
> > > +
> > >  ufs_mtk_init_clocks(hba);
> > >  
> > >  /*
> > > -- 
> > > 2.46.0
> > > 
> > > 
> > 
> > -- 
> > மணிவண்ணன் சதாசிவம்
> 
> - Mary
> 

Re: [PATCH 1/1] scsi: ufs-mediatek: Add UFSHCD_QUIRK_BROKEN_LSDBS_CAP

[Manivannan Sadhasivam]

On Mon, Aug 19, 2024 at 08:17:10PM +0200, Mary Guillemard wrote:
> On Mon, Aug 19, 2024 at 05:38:52PM +0530, Manivannan Sadhasivam wrote:
> > On Mon, Aug 19, 2024 at 12:24:42AM +0200, Mary Guillemard wrote:
> > > MT8183 supports UFSHCI 2.1 spec, but report a bogus value of 1 in the
> > > reserved part for the Legacy Single Doorbell Support (LSDBS) capability.
> > > 
> > 
> > Wow... I never thought that this quirk will be used outside of Qcom SoCs...
> >
> 
> Yeah I found that by trial and error some weeks ago and noticed your
> serie while looking to upstream this change, quite funny to see other
> vendors having the same quirk here.
>  
> > > This set UFSHCD_QUIRK_BROKEN_LSDBS_CAP when MCQ support is explicitly
> > > disabled, allowing the device to be properly registered.
> > > 
> > > Signed-off-by: Mary Guillemard <mary@mary.zone>
> > > ---
> > >  drivers/ufs/host/ufs-mediatek.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/drivers/ufs/host/ufs-mediatek.c b/drivers/ufs/host/ufs-mediatek.c
> > > index 02c9064284e1..9a5919434c4e 100644
> > > --- a/drivers/ufs/host/ufs-mediatek.c
> > > +++ b/drivers/ufs/host/ufs-mediatek.c
> > > @@ -1026,6 +1026,9 @@ static int ufs_mtk_init(struct ufs_hba *hba)
> > >  	if (host->caps & UFS_MTK_CAP_DISABLE_AH8)
> > >  		hba->caps |= UFSHCD_CAP_HIBERN8_WITH_CLK_GATING;
> > >  
> > > +	if (host->caps & UFS_MTK_CAP_DISABLE_MCQ)
> > 
> > How can this be the deciding factor? You said above that the issue is with
> > MT8183 SoC. So why not just use the quirk only for that platform?
> > 
> > - Mani
> >
> 
> So my current assumption is that it also affect other Mediatek SoCs
> that are also based on UFS 2.1 spec but I cannot check this.
> 
> Instead, we know that if MCQ isn't supported, we must fallback to LSDB
> as there is no other ways to drive the device.
> 
> UFS_MTK_CAP_DISABLE_MCQ (mediatek,ufs-disable-mcq) being unused upstream,
> I think that's an acceptable fix.
> 

If you use this quirk, then you need to use the corresponding DT property. But
using the 'mediatek,ufs-disable-mcq' property for 2.1 controller doesn't make
sense as MCQ is for controllers >= 4.0.

> Another way to handle this would be to add a new dt property and add it
> to ufs_mtk_host_caps but I feel that my approach should be enough. 
> 

No need to add a DT property. Just use the SoC specific compatible as I did for
SM8550 SoC.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH 1/1] scsi: ufs-mediatek: Add UFSHCD_QUIRK_BROKEN_LSDBS_CAP

[Bart Van Assche]

On 8/19/24 11:09 PM, Manivannan Sadhasivam wrote:
> On Mon, Aug 19, 2024 at 08:17:10PM +0200, Mary Guillemard wrote:
>> On Mon, Aug 19, 2024 at 05:38:52PM +0530, Manivannan Sadhasivam wrote:
>>> On Mon, Aug 19, 2024 at 12:24:42AM +0200, Mary Guillemard wrote:
>>>> +	if (host->caps & UFS_MTK_CAP_DISABLE_MCQ)
>>>
>>> How can this be the deciding factor? You said above that the issue is with
>>> MT8183 SoC. So why not just use the quirk only for that platform?
>>
>> So my current assumption is that it also affect other Mediatek SoCs
>> that are also based on UFS 2.1 spec but I cannot check this.
>>
>> Instead, we know that if MCQ isn't supported, we must fallback to LSDB
>> as there is no other ways to drive the device.
>>
>> UFS_MTK_CAP_DISABLE_MCQ (mediatek,ufs-disable-mcq) being unused upstream,
>> I think that's an acceptable fix.
>>
> 
> If you use this quirk, then you need to use the corresponding DT property. But
> using the 'mediatek,ufs-disable-mcq' property for 2.1 controller doesn't make
> sense as MCQ is for controllers >= 4.0.
> 
>> Another way to handle this would be to add a new dt property and add it
>> to ufs_mtk_host_caps but I feel that my approach should be enough.
>>
> 
> No need to add a DT property. Just use the SoC specific compatible as I did for
> SM8550 SoC.

Mary, do you plan to implement Manivannan's feedback?

Thanks,

Bart.

Re: [PATCH 1/1] scsi: ufs-mediatek: Add UFSHCD_QUIRK_BROKEN_LSDBS_CAP

[Mary Guillemard]

On Tue, Aug 20, 2024 at 02:50:58PM -0700, Bart Van Assche wrote:
> On 8/19/24 11:09 PM, Manivannan Sadhasivam wrote:
> > On Mon, Aug 19, 2024 at 08:17:10PM +0200, Mary Guillemard wrote:
> > > On Mon, Aug 19, 2024 at 05:38:52PM +0530, Manivannan Sadhasivam wrote:
> > > > On Mon, Aug 19, 2024 at 12:24:42AM +0200, Mary Guillemard wrote:
> > > > > +	if (host->caps & UFS_MTK_CAP_DISABLE_MCQ)
> > > > 
> > > > How can this be the deciding factor? You said above that the issue is with
> > > > MT8183 SoC. So why not just use the quirk only for that platform?
> > > 
> > > So my current assumption is that it also affect other Mediatek SoCs
> > > that are also based on UFS 2.1 spec but I cannot check this.
> > > 
> > > Instead, we know that if MCQ isn't supported, we must fallback to LSDB
> > > as there is no other ways to drive the device.
> > > 
> > > UFS_MTK_CAP_DISABLE_MCQ (mediatek,ufs-disable-mcq) being unused upstream,
> > > I think that's an acceptable fix.
> > > 
> > 
> > If you use this quirk, then you need to use the corresponding DT property. But
> > using the 'mediatek,ufs-disable-mcq' property for 2.1 controller doesn't make
> > sense as MCQ is for controllers >= 4.0.
> > 
> > > Another way to handle this would be to add a new dt property and add it
> > > to ufs_mtk_host_caps but I feel that my approach should be enough.
> > > 
> > 
> > No need to add a DT property. Just use the SoC specific compatible as I did for
> > SM8550 SoC.
> 
> Mary, do you plan to implement Manivannan's feedback?
> 
> Thanks,
> 
> Bart.
>

Hello Bart,

I think that considering Peter's reply, explicitly checking for the
MT8183 controller isn't required.

I also think it could be required for at least the MT8192 and MT8195
considering they are apparently also based on UFS 2.1 spec [1].

However, if you want me to add an explicit check, I will happily send a
v2.

Thanks,

Mary.

[1]https://corp.mediatek.com/news-events/press-releases/mediatek-announces-new-mt8192-and-mt8195-chipsets-designed-for-next-generation-of-chromebooks

Re: [PATCH 1/1] scsi: ufs-mediatek: Add UFSHCD_QUIRK_BROKEN_LSDBS_CAP

[Manivannan Sadhasivam]

On Wed, Aug 21, 2024 at 11:32:06PM +0200, Mary Guillemard wrote:
> On Tue, Aug 20, 2024 at 02:50:58PM -0700, Bart Van Assche wrote:
> > On 8/19/24 11:09 PM, Manivannan Sadhasivam wrote:
> > > On Mon, Aug 19, 2024 at 08:17:10PM +0200, Mary Guillemard wrote:
> > > > On Mon, Aug 19, 2024 at 05:38:52PM +0530, Manivannan Sadhasivam wrote:
> > > > > On Mon, Aug 19, 2024 at 12:24:42AM +0200, Mary Guillemard wrote:
> > > > > > +	if (host->caps & UFS_MTK_CAP_DISABLE_MCQ)
> > > > > 
> > > > > How can this be the deciding factor? You said above that the issue is with
> > > > > MT8183 SoC. So why not just use the quirk only for that platform?
> > > > 
> > > > So my current assumption is that it also affect other Mediatek SoCs
> > > > that are also based on UFS 2.1 spec but I cannot check this.
> > > > 
> > > > Instead, we know that if MCQ isn't supported, we must fallback to LSDB
> > > > as there is no other ways to drive the device.
> > > > 
> > > > UFS_MTK_CAP_DISABLE_MCQ (mediatek,ufs-disable-mcq) being unused upstream,
> > > > I think that's an acceptable fix.
> > > > 
> > > 
> > > If you use this quirk, then you need to use the corresponding DT property. But
> > > using the 'mediatek,ufs-disable-mcq' property for 2.1 controller doesn't make
> > > sense as MCQ is for controllers >= 4.0.
> > > 
> > > > Another way to handle this would be to add a new dt property and add it
> > > > to ufs_mtk_host_caps but I feel that my approach should be enough.
> > > > 
> > > 
> > > No need to add a DT property. Just use the SoC specific compatible as I did for
> > > SM8550 SoC.
> > 
> > Mary, do you plan to implement Manivannan's feedback?
> > 
> > Thanks,
> > 
> > Bart.
> >
> 
> Hello Bart,
> 
> I think that considering Peter's reply, explicitly checking for the
> MT8183 controller isn't required.
> 
> I also think it could be required for at least the MT8192 and MT8195
> considering they are apparently also based on UFS 2.1 spec [1].
> 

How can you add a quirk that is specifically meant for 4.x controllers to 2.1
controllers? It doesn't make sense.

Also it is weird that the existing DT files doesn't have ufshc nodes for any
SoCs, but the SoCs are supporting UFSHC.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH 1/1] scsi: ufs-mediatek: Add UFSHCD_QUIRK_BROKEN_LSDBS_CAP

[Peter Wang (王信友)]

On Mon, 2024-08-19 at 00:24 +0200, Mary Guillemard wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  MT8183 supports UFSHCI 2.1 spec, but report a bogus value of 1 in
> the
> reserved part for the Legacy Single Doorbell Support (LSDBS)
> capability.
> 
> This set UFSHCD_QUIRK_BROKEN_LSDBS_CAP when MCQ support is explicitly
> disabled, allowing the device to be properly registered.
> 
> Signed-off-by: Mary Guillemard <mary@mary.zone>
> ---
>  drivers/ufs/host/ufs-mediatek.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/ufs/host/ufs-mediatek.c b/drivers/ufs/host/ufs-
> mediatek.c
> index 02c9064284e1..9a5919434c4e 100644
> --- a/drivers/ufs/host/ufs-mediatek.c
> +++ b/drivers/ufs/host/ufs-mediatek.c
> @@ -1026,6 +1026,9 @@ static int ufs_mtk_init(struct ufs_hba *hba)
>  	if (host->caps & UFS_MTK_CAP_DISABLE_AH8)
>  		hba->caps |= UFSHCD_CAP_HIBERN8_WITH_CLK_GATING;
>  
> +	if (host->caps & UFS_MTK_CAP_DISABLE_MCQ)
> +		hba->quirks |= UFSHCD_QUIRK_BROKEN_LSDBS_CAP;
> +
>  	ufs_mtk_init_clocks(hba);
>  
>  	/*
> -- 
> 2.46.0


Reviewed-by: Peter Wang <peter.wang@mediatek.com>

Re: [PATCH 1/1] scsi: ufs-mediatek: Add UFSHCD_QUIRK_BROKEN_LSDBS_CAP

[Martin K. Petersen]

On Mon, 19 Aug 2024 00:24:42 +0200, Mary Guillemard wrote:

> MT8183 supports UFSHCI 2.1 spec, but report a bogus value of 1 in the
> reserved part for the Legacy Single Doorbell Support (LSDBS) capability.
> 
> This set UFSHCD_QUIRK_BROKEN_LSDBS_CAP when MCQ support is explicitly
> disabled, allowing the device to be properly registered.
> 
> 
> [...]

Applied to 6.11/scsi-fixes, thanks!

[1/1] scsi: ufs-mediatek: Add UFSHCD_QUIRK_BROKEN_LSDBS_CAP
      https://git.kernel.org/mkp/scsi/c/0f9592ae26ff

-- 
Martin K. Petersen	Oracle Linux Engineering