2022-07-11 08:41:56

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH 2/3] mmc: sdhci-msm: add MSM8998 SDCC specific compatible

Add a MSM8998-specific SDCC compatible, because using only a generic
qcom,sdhci-msm-v4 fallback is deprecated.

Signed-off-by: Krzysztof Kozlowski <[email protected]>
---
drivers/mmc/host/sdhci-msm.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index e395411fb6fd..bb169c1c2b5e 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -2447,6 +2447,7 @@ static const struct of_device_id sdhci_msm_dt_match[] = {
{.compatible = "qcom,msm8992-sdhci", .data = &sdhci_msm_mci_var},
{.compatible = "qcom,msm8994-sdhci", .data = &sdhci_msm_mci_var},
{.compatible = "qcom,msm8996-sdhci", .data = &sdhci_msm_mci_var},
+ {.compatible = "qcom,msm8998-sdhci", .data = &sdhci_msm_mci_var},
/*
* Add entries for sdcc version 5.0 here. For SDCC version 5.0.0,
* MCI registers are removed from SDCC interface and some registers
--
2.34.1


2022-07-11 09:04:01

by Konrad Dybcio

[permalink] [raw]
Subject: Re: [PATCH 2/3] mmc: sdhci-msm: add MSM8998 SDCC specific compatible



On 11.07.2022 10:27, Krzysztof Kozlowski wrote:
> Add a MSM8998-specific SDCC compatible, because using only a generic
> qcom,sdhci-msm-v4 fallback is deprecated.
>
> Signed-off-by: Krzysztof Kozlowski <[email protected]>
> ---
Reviewed-by: Konrad Dybcio <[email protected]>

Konrad
> drivers/mmc/host/sdhci-msm.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index e395411fb6fd..bb169c1c2b5e 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -2447,6 +2447,7 @@ static const struct of_device_id sdhci_msm_dt_match[] = {
> {.compatible = "qcom,msm8992-sdhci", .data = &sdhci_msm_mci_var},
> {.compatible = "qcom,msm8994-sdhci", .data = &sdhci_msm_mci_var},
> {.compatible = "qcom,msm8996-sdhci", .data = &sdhci_msm_mci_var},
> + {.compatible = "qcom,msm8998-sdhci", .data = &sdhci_msm_mci_var},
> /*
> * Add entries for sdcc version 5.0 here. For SDCC version 5.0.0,
> * MCI registers are removed from SDCC interface and some registers

2022-07-11 15:10:24

by Doug Anderson

[permalink] [raw]
Subject: Re: [PATCH 2/3] mmc: sdhci-msm: add MSM8998 SDCC specific compatible

Hi,

On Mon, Jul 11, 2022 at 1:27 AM Krzysztof Kozlowski
<[email protected]> wrote:
>
> Add a MSM8998-specific SDCC compatible, because using only a generic
> qcom,sdhci-msm-v4 fallback is deprecated.
>
> Signed-off-by: Krzysztof Kozlowski <[email protected]>
> ---
> drivers/mmc/host/sdhci-msm.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index e395411fb6fd..bb169c1c2b5e 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -2447,6 +2447,7 @@ static const struct of_device_id sdhci_msm_dt_match[] = {
> {.compatible = "qcom,msm8992-sdhci", .data = &sdhci_msm_mci_var},
> {.compatible = "qcom,msm8994-sdhci", .data = &sdhci_msm_mci_var},
> {.compatible = "qcom,msm8996-sdhci", .data = &sdhci_msm_mci_var},
> + {.compatible = "qcom,msm8998-sdhci", .data = &sdhci_msm_mci_var},

FWIW I'm _against_ this change.

In my mind while it is correct to specify both the specific and
generic compatible string in the device tree, the driver itself should
rely on just the generic compatible string until there is a reason to
use the specific one (like we needed to for sdm845 and sc7180).

I think I pointed that out before, but somehow all of the specific
device tree strings have snuck their way into the driver without me
paying attention. :(

-Doug

2022-07-12 07:07:19

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 2/3] mmc: sdhci-msm: add MSM8998 SDCC specific compatible

On 11/07/2022 17:08, Doug Anderson wrote:
> Hi,
>
> On Mon, Jul 11, 2022 at 1:27 AM Krzysztof Kozlowski
> <[email protected]> wrote:
>>
>> Add a MSM8998-specific SDCC compatible, because using only a generic
>> qcom,sdhci-msm-v4 fallback is deprecated.
>>
>> Signed-off-by: Krzysztof Kozlowski <[email protected]>
>> ---
>> drivers/mmc/host/sdhci-msm.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
>> index e395411fb6fd..bb169c1c2b5e 100644
>> --- a/drivers/mmc/host/sdhci-msm.c
>> +++ b/drivers/mmc/host/sdhci-msm.c
>> @@ -2447,6 +2447,7 @@ static const struct of_device_id sdhci_msm_dt_match[] = {
>> {.compatible = "qcom,msm8992-sdhci", .data = &sdhci_msm_mci_var},
>> {.compatible = "qcom,msm8994-sdhci", .data = &sdhci_msm_mci_var},
>> {.compatible = "qcom,msm8996-sdhci", .data = &sdhci_msm_mci_var},
>> + {.compatible = "qcom,msm8998-sdhci", .data = &sdhci_msm_mci_var},
>
> FWIW I'm _against_ this change.
>
> In my mind while it is correct to specify both the specific and
> generic compatible string in the device tree, the driver itself should
> rely on just the generic compatible string until there is a reason to
> use the specific one (like we needed to for sdm845 and sc7180).
>
> I think I pointed that out before, but somehow all of the specific
> device tree strings have snuck their way into the driver without me
> paying attention. :(

I thought it's existing practice for some time, but it's a fresh commit
466614a9765c ("mmc: sdhci-msm: Add SoC specific compatibles"). I agree
that it does not make much sense to add each compatible to the driver,
so how about reverting 466614a9765c?

Best regards,
Krzysztof

2022-07-12 14:58:03

by Doug Anderson

[permalink] [raw]
Subject: Re: [PATCH 2/3] mmc: sdhci-msm: add MSM8998 SDCC specific compatible

Hi,

On Mon, Jul 11, 2022 at 11:47 PM Krzysztof Kozlowski
<[email protected]> wrote:
>
> On 11/07/2022 17:08, Doug Anderson wrote:
> > Hi,
> >
> > On Mon, Jul 11, 2022 at 1:27 AM Krzysztof Kozlowski
> > <[email protected]> wrote:
> >>
> >> Add a MSM8998-specific SDCC compatible, because using only a generic
> >> qcom,sdhci-msm-v4 fallback is deprecated.
> >>
> >> Signed-off-by: Krzysztof Kozlowski <[email protected]>
> >> ---
> >> drivers/mmc/host/sdhci-msm.c | 1 +
> >> 1 file changed, 1 insertion(+)
> >>
> >> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> >> index e395411fb6fd..bb169c1c2b5e 100644
> >> --- a/drivers/mmc/host/sdhci-msm.c
> >> +++ b/drivers/mmc/host/sdhci-msm.c
> >> @@ -2447,6 +2447,7 @@ static const struct of_device_id sdhci_msm_dt_match[] = {
> >> {.compatible = "qcom,msm8992-sdhci", .data = &sdhci_msm_mci_var},
> >> {.compatible = "qcom,msm8994-sdhci", .data = &sdhci_msm_mci_var},
> >> {.compatible = "qcom,msm8996-sdhci", .data = &sdhci_msm_mci_var},
> >> + {.compatible = "qcom,msm8998-sdhci", .data = &sdhci_msm_mci_var},
> >
> > FWIW I'm _against_ this change.
> >
> > In my mind while it is correct to specify both the specific and
> > generic compatible string in the device tree, the driver itself should
> > rely on just the generic compatible string until there is a reason to
> > use the specific one (like we needed to for sdm845 and sc7180).
> >
> > I think I pointed that out before, but somehow all of the specific
> > device tree strings have snuck their way into the driver without me
> > paying attention. :(
>
> I thought it's existing practice for some time, but it's a fresh commit
> 466614a9765c ("mmc: sdhci-msm: Add SoC specific compatibles"). I agree
> that it does not make much sense to add each compatible to the driver,
> so how about reverting 466614a9765c?

That would be my vote.

-Doug