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

[Ulf Hansson <ulf.hansson@linaro.org>]

On Sat, 6 Nov 2021 at 15:31, H. Nikolaus Schaller <hns@goldelico.com> wrote:
>
> Hi Jérôme,
>
> > Am 05.11.2021 um 15:27 schrieb Jérôme Pouiller <jerome.pouiller@silabs.com>:
> >
> > On Friday 5 November 2021 10:05:47 CET H. Nikolaus Schaller 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>
> >
> > Thank you for to have taken care of that (Maybe, you would like to add a
> > "Co-developed-by:" tag).
>
> Well, I just have taken your and Ulf's proposal and done 90% copy&paste.
> So there wasn't much development, just editing...
>
> >
> >
> >> ---
> >> drivers/mmc/core/card.h   |  3 +++
> >> drivers/mmc/core/quirks.h | 17 +++++++++++++++++
> >> 2 files changed, 20 insertions(+)
> >>
> >> +static inline bool mmc_fixup_of_compatible_match(struct mmc_card *card,
> >> +                                                const char *const *compat_list)

After a second thought, I am not sure we really need a list of
compatibles here. The quirks we may want to apply should be specific
per device and most likely not shared among a family of devices, don't
you think?

> >> +{
> >> +       struct device_node *np;
> >> +
> >> +       for_each_child_of_node(mmc_dev(card->host)->of_node, np) {
> >> +               if (of_device_compatible_match(np, compat_list))
> >> +                       return true;
> >
> > Intel robot complains about of_device_compatible_match():
> >
> >    ERROR: modpost: "of_device_compatible_match" [drivers/mmc/core/mmc_core.ko] undefined!
> >
> > I think we have to add this line:
> >
> >    EXPORT_SYMBOL(of_device_compatible_match);
> >
> > in drivers/of/base.c

If we change to use one compatible string, rather than a list - then
we can use of_device_is_compatible() instead, which is already
properly exported with EXPORT_SYMBOL().

>
> I had seen the krobot message as well but could not figure out
> what it meant...
>
> But with your hint it indeed looks like an omission in drivers/of/base.c
> Having something exported in include/linux/of.h but code not
> marked EXPORT_SYMBOL...
>
> That needs a separate patch. I'll add one with a
>
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
>
> and some Fixes: tag. Since it has a different audience I think
> I should post it separately.
>
> BTW: krobot noted the same issue for mmc_of_find_child_device()
> in drivers/mmc/core/core.c (which we do not touch in this series).
> But maybe it should be fixed as well.

If there is an existing problem, please send a separate fix/patch for that.

>
> So let's wait for more comments and then I may distribute a [PATCH v1].
> Or should I do a [PATCH v5] to continue version counting?

I suggest moving from RFC into using PATCH v1, as this isn't really an
RFC any more.

>
> BR,
> Nikolaus
>

Kind regards
Uffe