[RFC PATCH 0/2] mmc: allow to rely on the DT to apply quirks

[RFC PATCH 0/2] mmc: allow to rely on the DT to apply quirks

[Jerome Pouiller]

From: Jérôme Pouiller <jerome.pouiller@silabs.com>

Hi Ulf,

I try to write a patch to problem discussed here[1].

This patch is not complete. Especially, it does not contains the necessary
FIXUP macro in card.h. I would like to have your comment on the concept.

[1] https://lore.kernel.org/netdev/CAPDyKFr62Kykg3=9WiXAV8UToqjw8gj4t6bbM7pGQ+iGGQRLmg@mail.gmail.com/

Jérôme Pouiller (2):
  mmc: rewrite mmc_fixup_device()
  mmc: allow to match the device tree to apply quirks

 drivers/mmc/core/card.h   |  3 ++
 drivers/mmc/core/quirks.h | 59 ++++++++++++++++++++++++++++-----------
 2 files changed, 45 insertions(+), 17 deletions(-)

-- 
2.33.0

[RFC PATCH 1/2] mmc: rewrite mmc_fixup_device()

[Jerome Pouiller]

From: Jérôme Pouiller <jerome.pouiller@silabs.com>

Currently, mmc_fixup_device() is a bit difficult to read because of
particularly long condition.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/mmc/core/quirks.h | 41 +++++++++++++++++++++++----------------
 1 file changed, 24 insertions(+), 17 deletions(-)

diff --git a/drivers/mmc/core/quirks.h b/drivers/mmc/core/quirks.h
index d68e6e513a4f..c7ef2d14b359 100644
--- a/drivers/mmc/core/quirks.h
+++ b/drivers/mmc/core/quirks.h
@@ -152,22 +152,29 @@ static inline void mmc_fixup_device(struct mmc_card *card,
 	u64 rev = cid_rev_card(card);
 
 	for (f = table; f->vendor_fixup; f++) {
-		if ((f->manfid == CID_MANFID_ANY ||
-		     f->manfid == card->cid.manfid) &&
-		    (f->oemid == CID_OEMID_ANY ||
-		     f->oemid == card->cid.oemid) &&
-		    (f->name == CID_NAME_ANY ||
-		     !strncmp(f->name, card->cid.prod_name,
-			      sizeof(card->cid.prod_name))) &&
-		    (f->cis_vendor == card->cis.vendor ||
-		     f->cis_vendor == (u16) SDIO_ANY_ID) &&
-		    (f->cis_device == card->cis.device ||
-		     f->cis_device == (u16) SDIO_ANY_ID) &&
-		    (f->ext_csd_rev == EXT_CSD_REV_ANY ||
-		     f->ext_csd_rev == card->ext_csd.rev) &&
-		    rev >= f->rev_start && rev <= f->rev_end) {
-			dev_dbg(&card->dev, "calling %ps\n", f->vendor_fixup);
-			f->vendor_fixup(card, f->data);
-		}
+		if (f->manfid != CID_MANFID_ANY &&
+		    f->manfid != card->cid.manfid)
+			continue;
+		if (f->oemid != CID_OEMID_ANY &&
+		    f->oemid != card->cid.oemid)
+			continue;
+		if (f->name != CID_NAME_ANY &&
+		    strncmp(f->name, card->cid.prod_name,
+			    sizeof(card->cid.prod_name)))
+			continue;
+		if (f->cis_vendor != (u16)SDIO_ANY_ID &&
+		    f->cis_vendor != card->cis.vendor)
+			continue;
+		if (f->cis_device != (u16)SDIO_ANY_ID &&
+		    f->cis_device != card->cis.device)
+			continue;
+		if (f->ext_csd_rev != EXT_CSD_REV_ANY &&
+		    f->ext_csd_rev != card->ext_csd.rev)
+			continue;
+		if (rev < f->rev_start || rev > f->rev_end)
+			continue;
+
+		dev_dbg(&card->dev, "calling %ps\n", f->vendor_fixup);
+		f->vendor_fixup(card, f->data);
 	}
 }
-- 
2.33.0

[RFC PATCH 2/2] mmc: allow to match the device tree to apply quirks

[Jerome Pouiller]

From: Jérôme Pouiller <jerome.pouiller@silabs.com>

MMC subsystem provides a way to apply quirks when a device match some
properties (VID, PID, etc...) Unfortunately, some SDIO devices does not
comply with the SDIO specification and does not provide reliable VID/PID
(eg. Silabs WF200).

So, the drivers for these devices rely on device tree to identify the
device.

This patch allows the MMC to also rely on the device tree to apply a
quirk.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/mmc/core/card.h   |  3 +++
 drivers/mmc/core/quirks.h | 18 ++++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/drivers/mmc/core/card.h b/drivers/mmc/core/card.h
index 7bd392d55cfa..2f73f8567e14 100644
--- a/drivers/mmc/core/card.h
+++ b/drivers/mmc/core/card.h
@@ -59,6 +59,9 @@ struct mmc_fixup {
 	/* for MMC cards */
 	unsigned int ext_csd_rev;
 
+	/* Match against functions declared in device tree */
+	const char *const *of_compatible;
+
 	void (*vendor_fixup)(struct mmc_card *card, int data);
 	int data;
 };
diff --git a/drivers/mmc/core/quirks.h b/drivers/mmc/core/quirks.h
index c7ef2d14b359..ef368386e711 100644
--- a/drivers/mmc/core/quirks.h
+++ b/drivers/mmc/core/quirks.h
@@ -10,6 +10,7 @@
  *
  */
 
+#include <linux/of.h>
 #include <linux/mmc/sdio_ids.h>
 
 #include "card.h"
@@ -145,6 +146,20 @@ static const struct mmc_fixup __maybe_unused sdio_fixup_methods[] = {
 	END_FIXUP
 };
 
+static inline bool mmc_fixup_of_compatible_match(struct mmc_card *card,
+						 const char *const *compat_list)
+{
+	struct device_node *of_node;
+	int i;
+
+	for (i = 0; i < 7; i++) {
+		of_node = mmc_of_find_child_device(card->host, i);
+		if (of_node && of_device_compatible_match(of_node, compat_list))
+			return true;
+	}
+	return false;
+}
+
 static inline void mmc_fixup_device(struct mmc_card *card,
 				    const struct mmc_fixup *table)
 {
@@ -173,6 +188,9 @@ static inline void mmc_fixup_device(struct mmc_card *card,
 			continue;
 		if (rev < f->rev_start || rev > f->rev_end)
 			continue;
+		if (f->of_compatible != NULL &&
+		    !mmc_fixup_of_compatible_match(card, f->of_compatible))
+			continue;
 
 		dev_dbg(&card->dev, "calling %ps\n", f->vendor_fixup);
 		f->vendor_fixup(card, f->data);
-- 
2.33.0

Re: [RFC PATCH 2/2] mmc: allow to match the device tree to apply quirks

[Ulf Hansson]

On Thu, 14 Oct 2021 at 16:30, Jerome Pouiller
<Jerome.Pouiller@silabs.com> wrote:
>
> From: Jérôme Pouiller <jerome.pouiller@silabs.com>
>
> MMC subsystem provides a way to apply quirks when a device match some
> properties (VID, PID, etc...) Unfortunately, some SDIO devices does not
> comply with the SDIO specification and does not provide reliable VID/PID
> (eg. Silabs WF200).
>
> So, the drivers for these devices rely on device tree to identify the
> device.
>
> This patch allows the MMC to also rely on the device tree to apply a
> quirk.
>
> Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
> ---
>  drivers/mmc/core/card.h   |  3 +++
>  drivers/mmc/core/quirks.h | 18 ++++++++++++++++++
>  2 files changed, 21 insertions(+)
>
> diff --git a/drivers/mmc/core/card.h b/drivers/mmc/core/card.h
> index 7bd392d55cfa..2f73f8567e14 100644
> --- a/drivers/mmc/core/card.h
> +++ b/drivers/mmc/core/card.h
> @@ -59,6 +59,9 @@ struct mmc_fixup {
>         /* for MMC cards */
>         unsigned int ext_csd_rev;
>
> +       /* Match against functions declared in device tree */
> +       const char *const *of_compatible;

const char is probably sufficient.

> +
>         void (*vendor_fixup)(struct mmc_card *card, int data);
>         int data;
>  };
> diff --git a/drivers/mmc/core/quirks.h b/drivers/mmc/core/quirks.h
> index c7ef2d14b359..ef368386e711 100644
> --- a/drivers/mmc/core/quirks.h
> +++ b/drivers/mmc/core/quirks.h
> @@ -10,6 +10,7 @@
>   *
>   */
>
> +#include <linux/of.h>
>  #include <linux/mmc/sdio_ids.h>
>
>  #include "card.h"
> @@ -145,6 +146,20 @@ static const struct mmc_fixup __maybe_unused sdio_fixup_methods[] = {
>         END_FIXUP
>  };
>
> +static inline bool mmc_fixup_of_compatible_match(struct mmc_card *card,
> +                                                const char *const *compat_list)
> +{
> +       struct device_node *of_node;
> +       int i;
> +
> +       for (i = 0; i < 7; i++) {

We need to iterate up '8', to cover all functional devices.

The function numbers are from 1->7, while we use '0' for the
"mmc-card" compatible.

> +               of_node = mmc_of_find_child_device(card->host, i);
> +               if (of_node && of_device_compatible_match(of_node, compat_list))
> +                       return true;
> +       }

Another option, which would avoid doing the iteration over and over
again in mmc_of_find_child_device(), would be to simply do the parsing
of the compatible in the child nodes, right here. In other words, just
use for_each_child_of_node().

> +       return false;
> +}
> +
>  static inline void mmc_fixup_device(struct mmc_card *card,
>                                     const struct mmc_fixup *table)
>  {
> @@ -173,6 +188,9 @@ static inline void mmc_fixup_device(struct mmc_card *card,
>                         continue;
>                 if (rev < f->rev_start || rev > f->rev_end)
>                         continue;
> +               if (f->of_compatible != NULL &&
> +                   !mmc_fixup_of_compatible_match(card, f->of_compatible))
> +                       continue;
>
>                 dev_dbg(&card->dev, "calling %ps\n", f->vendor_fixup);
>                 f->vendor_fixup(card, f->data);

Kind regards
Uffe