diff mbox series

[2/3] mmc: sdhci-msm: add MSM8998 SDCC specific compatible

Message ID 20220711082709.39102-2-krzysztof.kozlowski@linaro.org
State New
Headers show
Series [1/3] dt-bindings: mmc: sdhci-msm: add MSM8998 | expand

Commit Message

Krzysztof Kozlowski July 11, 2022, 8:27 a.m. UTC
Add a MSM8998-specific SDCC compatible, because using only a generic
qcom,sdhci-msm-v4 fallback is deprecated.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 drivers/mmc/host/sdhci-msm.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Krzysztof Kozlowski July 12, 2022, 6:47 a.m. UTC | #1
On 11/07/2022 17:08, Doug Anderson wrote:
> Hi,
> 
> On Mon, Jul 11, 2022 at 1:27 AM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> Add a MSM8998-specific SDCC compatible, because using only a generic
>> qcom,sdhci-msm-v4 fallback is deprecated.
>>
>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> ---
>>  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
Doug Anderson July 12, 2022, 2:38 p.m. UTC | #2
Hi,

On Mon, Jul 11, 2022 at 11:47 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 11/07/2022 17:08, Doug Anderson wrote:
> > Hi,
> >
> > On Mon, Jul 11, 2022 at 1:27 AM Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> >>
> >> Add a MSM8998-specific SDCC compatible, because using only a generic
> >> qcom,sdhci-msm-v4 fallback is deprecated.
> >>
> >> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> >> ---
> >>  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
diff mbox series

Patch

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