On Thu, 30 Sept 2021 at 18:51, Jérôme Pouiller
[off-list ref] wrote:

Hello Ulf,

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

Please, see my other reply to Pali.

[...]

quoted

quoted

+
+static const struct sdio_device_id wfx_sdio_ids[] = {
+       { SDIO_DEVICE(SDIO_VENDOR_ID_SILABS, SDIO_DEVICE_ID_SILABS_WF200) },
+       { },
+};
+MODULE_DEVICE_TABLE(sdio, wfx_sdio_ids);
+
+struct sdio_driver wfx_sdio_driver = {
+       .name = "wfx-sdio",
+       .id_table = wfx_sdio_ids,
+       .probe = wfx_sdio_probe,
+       .remove = wfx_sdio_remove,
+       .drv = {
+               .owner = THIS_MODULE,
+               .of_match_table = wfx_sdio_of_match,

Is there no power management? Or do you intend to add that on top?

It seems we already have had this discussion:

  https://lore.kernel.org/netdev/CAPDyKFqJf=vUqpQg3suDCadKrFTkQWFTY_qp=+yDK=_Lu9gJGg@mail.gmail.com/#r (local)

In this thread, Kalle said:

quoted

Many mac80211 drivers do so that the device is powered off during
interface down (ifconfig wlan0 down), and as mac80211 does interface
down automatically during suspend, suspend then works without extra
handlers.

Yeah, it's been a while since I looked at this, thanks for the pointer.

[...]

Kind regards
Uffe