Thread (34 messages) 34 messages, 4 authors, 2021-10-06

Re: [PATCH v7 08/24] wfx: add bus_sdio.c

From: Ulf Hansson <hidden>
Date: 2021-10-06 15:02:54
Also in: linux-devicetree, linux-mmc, lkml, netdev

On Tue, 5 Oct 2021 at 10:14, Jérôme Pouiller [off-list ref] wrote:
On Friday 1 October 2021 17:23:16 CEST Ulf Hansson wrote:
quoted
On Thu, 30 Sept 2021 at 19:06, Pali Rohár [off-list ref] wrote:
quoted
On Thursday 30 September 2021 18:51:09 Jérôme Pouiller wrote:
quoted
On Thursday 30 September 2021 12:07:55 CEST Ulf Hansson wrote:
quoted
On Mon, 20 Sept 2021 at 18:12, Jerome Pouiller
[off-list ref] wrote:
quoted
From: Jérôme Pouiller <jerome.pouiller@silabs.com>

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/net/wireless/silabs/wfx/bus_sdio.c | 261 +++++++++++++++++++++
 1 file changed, 261 insertions(+)
 create mode 100644 drivers/net/wireless/silabs/wfx/bus_sdio.c
diff --git a/drivers/net/wireless/silabs/wfx/bus_sdio.c b/drivers/net/wireless/silabs/wfx/bus_sdio.c
[...]
quoted
+
+static int wfx_sdio_probe(struct sdio_func *func,
+                         const struct sdio_device_id *id)
+{
+       struct device_node *np = func->dev.of_node;
+       struct wfx_sdio_priv *bus;
+       int ret;
+
+       if (func->num != 1) {
+               dev_err(&func->dev, "SDIO function number is %d while it should always be 1 (unsupported chip?)\n",
+                       func->num);
+               return -ENODEV;
+       }
+
+       bus = devm_kzalloc(&func->dev, sizeof(*bus), GFP_KERNEL);
+       if (!bus)
+               return -ENOMEM;
+
+       if (!np || !of_match_node(wfx_sdio_of_match, np)) {
+               dev_warn(&func->dev, "no compatible device found in DT\n");
+               return -ENODEV;
+       }
+
+       bus->func = func;
+       bus->of_irq = irq_of_parse_and_map(np, 0);
+       sdio_set_drvdata(func, bus);
+       func->card->quirks |= MMC_QUIRK_LENIENT_FN0 |
+                             MMC_QUIRK_BLKSZ_FOR_BYTE_MODE |
+                             MMC_QUIRK_BROKEN_BYTE_MODE_512;
I would rather see that you add an SDIO_FIXUP for the SDIO card, to
the sdio_fixup_methods[], in drivers/mmc/core/quirks.h, instead of
this.
In the current patch, these quirks are applied only if the device appears
in the device tree (see the condition above). If I implement them in
drivers/mmc/core/quirks.h they will be applied as soon as the device is
detected. Is it what we want?

Note: we already have had a discussion about the strange VID/PID declared
by this device:
  https://www.spinics.net/lists/netdev/msg692577.html
Yes, vendor id 0x0000 is invalid per SDIO spec. So based on this vendor
id, it is not possible to write any quirk in mmc/sdio generic code.

Ulf, but maybe it could be possible to write quirk based on OF
compatible string?
Yes, that would be better in my opinion.

We already have DT bindings to describe embedded SDIO cards (a subnode
to the mmc controller node), so we should be able to extend that I
think.
So, this feature does not yet exist? Do you consider it is a blocker for
the current patch?
Yes, sorry. I think we should avoid unnecessary hacks in SDIO func
drivers, especially those that deserve to be fixed in the mmc core.

Moreover, we already support the similar thing for eMMC cards, plus
that most parts are already done for SDIO too.
To be honest, I don't really want to take over this change in mmc/core.
I understand. Allow me a couple of days, then I can post a patch to
help you out.
--
Jérôme Pouiller
Kind regards
Uffe
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help