diff mbox series

[RFC,v4,4/6] mmc: core: add new calls to mmc_fixup_device(sdio_card_init_methods)

Message ID 73440c0f227778e57167dd9fedd350637a1d737a.1636103151.git.hns@goldelico.com (mailing list archive)
State New, archived
Headers show
Series mmc: core: extend mmc_fixup_device and transplant ti,wl1251 quirks from to be retired omap_hsmmc | expand

Commit Message

H. Nikolaus Schaller Nov. 5, 2021, 9:05 a.m. UTC
This allows to add quirks based on device tree instead of having
card specific code in the host ops.

We call it just after where host->ops->init_card() can be optionally
called.

Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 drivers/mmc/core/mmc.c  | 1 +
 drivers/mmc/core/sd.c   | 2 ++
 drivers/mmc/core/sdio.c | 1 +
 3 files changed, 4 insertions(+)

Comments

Ulf Hansson Nov. 8, 2021, 3:08 p.m. UTC | #1
On Fri, 5 Nov 2021 at 10:06, H. Nikolaus Schaller <hns@goldelico.com> wrote:
>
> This allows to add quirks based on device tree instead of having
> card specific code in the host ops.
>
> We call it just after where host->ops->init_card() can be optionally
> called.
>
> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> ---
>  drivers/mmc/core/mmc.c  | 1 +
>  drivers/mmc/core/sd.c   | 2 ++
>  drivers/mmc/core/sdio.c | 1 +
>  3 files changed, 4 insertions(+)
>
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index 29e58ffae3797..19cd138acaec9 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -1634,6 +1634,7 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
>          */
>         if (host->ops->init_card)
>                 host->ops->init_card(host, card);
> +       mmc_fixup_device(card, sdio_card_init_methods);
>
>         /*
>          * For native busses:  set card RCA and quit open drain mode.
> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
> index 4646b7a03db6b..0d174fdf47164 100644
> --- a/drivers/mmc/core/sd.c
> +++ b/drivers/mmc/core/sd.c
> @@ -23,6 +23,7 @@
>  #include "host.h"
>  #include "bus.h"
>  #include "mmc_ops.h"
> +#include "quirks.h"
>  #include "sd.h"
>  #include "sd_ops.h"
>
> @@ -1427,6 +1428,7 @@ static int mmc_sd_init_card(struct mmc_host *host, u32 ocr,
>          */
>         if (host->ops->init_card)
>                 host->ops->init_card(host, card);
> +       mmc_fixup_device(card, sdio_card_init_methods);
>
>         /*
>          * For native busses:  get card RCA and quit open drain mode.
> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
> index 68edf7a615be5..cf8ee66990508 100644
> --- a/drivers/mmc/core/sdio.c
> +++ b/drivers/mmc/core/sdio.c
> @@ -707,6 +707,7 @@ static int mmc_sdio_init_card(struct mmc_host *host, u32 ocr,
>          */
>         if (host->ops->init_card)
>                 host->ops->init_card(host, card);
> +       mmc_fixup_device(card, sdio_card_init_methods);
>
>         /*
>          * If the host and card support UHS-I mode request the card
> --
> 2.33.0
>

As the quirk is for SDIO cards, we don't need to call
mmc_fixup_device(card, sdio_card_init_methods) - other than from
mmc_sdio_init_card(). Additionally, for sd/mmc we should not be using
'sdio_card_init_methods'.

That said, it looks also reasonable to me, to squash $subject patch with patch3.

Kind regards
Uffe
Jérôme Pouiller Nov. 8, 2021, 3:39 p.m. UTC | #2
On Friday 5 November 2021 10:05:49 CET H. Nikolaus Schaller wrote:
> This allows to add quirks based on device tree instead of having
> card specific code in the host ops.
> 
> We call it just after where host->ops->init_card() can be optionally
> called.
> 
> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>

[...]
> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
> index 68edf7a615be5..cf8ee66990508 100644
> --- a/drivers/mmc/core/sdio.c
> +++ b/drivers/mmc/core/sdio.c
> @@ -707,6 +707,7 @@ static int mmc_sdio_init_card(struct mmc_host *host, u32 ocr,
>          */
>         if (host->ops->init_card)
>                 host->ops->init_card(host, card);
> +       mmc_fixup_device(card, sdio_card_init_methods);

sdio_read_common_cis(card) is called a bit after this line. I think it 
will overwrite all the card->cis fields. This does not conflict with what 
your are doing in wl1251_quirk()?
H. Nikolaus Schaller Nov. 8, 2021, 4:04 p.m. UTC | #3
> Am 08.11.2021 um 16:39 schrieb Jérôme Pouiller <jerome.pouiller@silabs.com>:
> 
> On Friday 5 November 2021 10:05:49 CET H. Nikolaus Schaller wrote:
>> This allows to add quirks based on device tree instead of having
>> card specific code in the host ops.
>> 
>> We call it just after where host->ops->init_card() can be optionally
>> called.
>> 
>> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> 
> [...]
>> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
>> index 68edf7a615be5..cf8ee66990508 100644
>> --- a/drivers/mmc/core/sdio.c
>> +++ b/drivers/mmc/core/sdio.c
>> @@ -707,6 +707,7 @@ static int mmc_sdio_init_card(struct mmc_host *host, u32 ocr,
>>         */
>>        if (host->ops->init_card)
>>                host->ops->init_card(host, card);
>> +       mmc_fixup_device(card, sdio_card_init_methods);
> 
> sdio_read_common_cis(card) is called a bit after this line. I think it 
> will overwrite all the card->cis fields. This does not conflict with what 
> your are doing in wl1251_quirk()?

No, because the wl1251_quirk sets MMC_QUIRK_NONSTD_SDIO which
skips reading CIS. The key issue with the wl1251 seems to be
that it reports random CIS tuples if we try to probe without
quirks (I have no further idea about the wl1251 than moving the
quirks around...).
H. Nikolaus Schaller Nov. 8, 2021, 4:07 p.m. UTC | #4
> Am 08.11.2021 um 16:08 schrieb Ulf Hansson <ulf.hansson@linaro.org>:
> 
> On Fri, 5 Nov 2021 at 10:06, H. Nikolaus Schaller <hns@goldelico.com> wrote:
>> 
>> This allows to add quirks based on device tree instead of having
>> card specific code in the host ops.
>> 
>> We call it just after where host->ops->init_card() can be optionally
>> called.
>> 
>> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
>> ---
>> drivers/mmc/core/mmc.c  | 1 +
>> drivers/mmc/core/sd.c   | 2 ++
>> drivers/mmc/core/sdio.c | 1 +
>> 3 files changed, 4 insertions(+)
>> 
>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>> index 29e58ffae3797..19cd138acaec9 100644
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -1634,6 +1634,7 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
>>         */
>>        if (host->ops->init_card)
>>                host->ops->init_card(host, card);
>> +       mmc_fixup_device(card, sdio_card_init_methods);
>> 
>>        /*
>>         * For native busses:  set card RCA and quit open drain mode.
>> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
>> index 4646b7a03db6b..0d174fdf47164 100644
>> --- a/drivers/mmc/core/sd.c
>> +++ b/drivers/mmc/core/sd.c
>> @@ -23,6 +23,7 @@
>> #include "host.h"
>> #include "bus.h"
>> #include "mmc_ops.h"
>> +#include "quirks.h"
>> #include "sd.h"
>> #include "sd_ops.h"
>> 
>> @@ -1427,6 +1428,7 @@ static int mmc_sd_init_card(struct mmc_host *host, u32 ocr,
>>         */
>>        if (host->ops->init_card)
>>                host->ops->init_card(host, card);
>> +       mmc_fixup_device(card, sdio_card_init_methods);
>> 
>>        /*
>>         * For native busses:  get card RCA and quit open drain mode.
>> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
>> index 68edf7a615be5..cf8ee66990508 100644
>> --- a/drivers/mmc/core/sdio.c
>> +++ b/drivers/mmc/core/sdio.c
>> @@ -707,6 +707,7 @@ static int mmc_sdio_init_card(struct mmc_host *host, u32 ocr,
>>         */
>>        if (host->ops->init_card)
>>                host->ops->init_card(host, card);
>> +       mmc_fixup_device(card, sdio_card_init_methods);
>> 
>>        /*
>>         * If the host and card support UHS-I mode request the card
>> --
>> 2.33.0
>> 
> 
> As the quirk is for SDIO cards, we don't need to call
> mmc_fixup_device(card, sdio_card_init_methods) - other than from
> mmc_sdio_init_card().

Ok. Well, the old code did have some logic for some SD_COMBO.
But I have no idea if that is needed.

> Additionally, for sd/mmc we should not be using
> 'sdio_card_init_methods'.

Ok ,I see.

> 
> That said, it looks also reasonable to me, to squash $subject patch with patch3.

Ok.

BR and thanks,
Nikolaus
diff mbox series

Patch

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 29e58ffae3797..19cd138acaec9 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1634,6 +1634,7 @@  static int mmc_init_card(struct mmc_host *host, u32 ocr,
 	 */
 	if (host->ops->init_card)
 		host->ops->init_card(host, card);
+	mmc_fixup_device(card, sdio_card_init_methods);
 
 	/*
 	 * For native busses:  set card RCA and quit open drain mode.
diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
index 4646b7a03db6b..0d174fdf47164 100644
--- a/drivers/mmc/core/sd.c
+++ b/drivers/mmc/core/sd.c
@@ -23,6 +23,7 @@ 
 #include "host.h"
 #include "bus.h"
 #include "mmc_ops.h"
+#include "quirks.h"
 #include "sd.h"
 #include "sd_ops.h"
 
@@ -1427,6 +1428,7 @@  static int mmc_sd_init_card(struct mmc_host *host, u32 ocr,
 	 */
 	if (host->ops->init_card)
 		host->ops->init_card(host, card);
+	mmc_fixup_device(card, sdio_card_init_methods);
 
 	/*
 	 * For native busses:  get card RCA and quit open drain mode.
diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index 68edf7a615be5..cf8ee66990508 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -707,6 +707,7 @@  static int mmc_sdio_init_card(struct mmc_host *host, u32 ocr,
 	 */
 	if (host->ops->init_card)
 		host->ops->init_card(host, card);
+	mmc_fixup_device(card, sdio_card_init_methods);
 
 	/*
 	 * If the host and card support UHS-I mode request the card