From: Jérôme Pouiller <[email protected]>
Signed-off-by: Jérôme Pouiller <[email protected]>
---
drivers/net/wireless/silabs/wfx/bus_sdio.c | 259 +++++++++++++++++++++
1 file changed, 259 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
new file mode 100644
index 000000000000..55b6728416ba
--- /dev/null
+++ b/drivers/net/wireless/silabs/wfx/bus_sdio.c
@@ -0,0 +1,259 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * SDIO interface.
+ *
+ * Copyright (c) 2017-2020, Silicon Laboratories, Inc.
+ * Copyright (c) 2010, ST-Ericsson
+ */
+#include <linux/module.h>
+#include <linux/mmc/sdio.h>
+#include <linux/mmc/sdio_func.h>
+#include <linux/mmc/sdio_ids.h>
+#include <linux/mmc/card.h>
+#include <linux/interrupt.h>
+#include <linux/of_irq.h>
+#include <linux/irq.h>
+
+#include "bus.h"
+#include "wfx.h"
+#include "hwio.h"
+#include "main.h"
+#include "bh.h"
+
+static const struct wfx_platform_data wfx_sdio_pdata = {
+ .file_fw = "wfm_wf200",
+ .file_pds = "wf200.pds",
+};
+
+struct wfx_sdio_priv {
+ struct sdio_func *func;
+ struct wfx_dev *core;
+ u8 buf_id_tx;
+ u8 buf_id_rx;
+ int of_irq;
+};
+
+static int wfx_sdio_copy_from_io(void *priv, unsigned int reg_id,
+ void *dst, size_t count)
+{
+ struct wfx_sdio_priv *bus = priv;
+ unsigned int sdio_addr = reg_id << 2;
+ int ret;
+
+ WARN(reg_id > 7, "chip only has 7 registers");
+ WARN(((uintptr_t)dst) & 3, "unaligned buffer size");
+ WARN(count & 3, "unaligned buffer address");
+
+ /* Use queue mode buffers */
+ if (reg_id == WFX_REG_IN_OUT_QUEUE)
+ sdio_addr |= (bus->buf_id_rx + 1) << 7;
+ ret = sdio_memcpy_fromio(bus->func, dst, sdio_addr, count);
+ if (!ret && reg_id == WFX_REG_IN_OUT_QUEUE)
+ bus->buf_id_rx = (bus->buf_id_rx + 1) % 4;
+
+ return ret;
+}
+
+static int wfx_sdio_copy_to_io(void *priv, unsigned int reg_id,
+ const void *src, size_t count)
+{
+ struct wfx_sdio_priv *bus = priv;
+ unsigned int sdio_addr = reg_id << 2;
+ int ret;
+
+ WARN(reg_id > 7, "chip only has 7 registers");
+ WARN(((uintptr_t)src) & 3, "unaligned buffer size");
+ WARN(count & 3, "unaligned buffer address");
+
+ /* Use queue mode buffers */
+ if (reg_id == WFX_REG_IN_OUT_QUEUE)
+ sdio_addr |= bus->buf_id_tx << 7;
+ /* FIXME: discards 'const' qualifier for src */
+ ret = sdio_memcpy_toio(bus->func, sdio_addr, (void *)src, count);
+ if (!ret && reg_id == WFX_REG_IN_OUT_QUEUE)
+ bus->buf_id_tx = (bus->buf_id_tx + 1) % 32;
+
+ return ret;
+}
+
+static void wfx_sdio_lock(void *priv)
+{
+ struct wfx_sdio_priv *bus = priv;
+
+ sdio_claim_host(bus->func);
+}
+
+static void wfx_sdio_unlock(void *priv)
+{
+ struct wfx_sdio_priv *bus = priv;
+
+ sdio_release_host(bus->func);
+}
+
+static void wfx_sdio_irq_handler(struct sdio_func *func)
+{
+ struct wfx_sdio_priv *bus = sdio_get_drvdata(func);
+
+ wfx_bh_request_rx(bus->core);
+}
+
+static irqreturn_t wfx_sdio_irq_handler_ext(int irq, void *priv)
+{
+ struct wfx_sdio_priv *bus = priv;
+
+ sdio_claim_host(bus->func);
+ wfx_bh_request_rx(bus->core);
+ sdio_release_host(bus->func);
+ return IRQ_HANDLED;
+}
+
+static int wfx_sdio_irq_subscribe(void *priv)
+{
+ struct wfx_sdio_priv *bus = priv;
+ u32 flags;
+ int ret;
+ u8 cccr;
+
+ if (!bus->of_irq) {
+ sdio_claim_host(bus->func);
+ ret = sdio_claim_irq(bus->func, wfx_sdio_irq_handler);
+ sdio_release_host(bus->func);
+ return ret;
+ }
+
+ sdio_claim_host(bus->func);
+ cccr = sdio_f0_readb(bus->func, SDIO_CCCR_IENx, NULL);
+ cccr |= BIT(0);
+ cccr |= BIT(bus->func->num);
+ sdio_f0_writeb(bus->func, cccr, SDIO_CCCR_IENx, NULL);
+ sdio_release_host(bus->func);
+ flags = irq_get_trigger_type(bus->of_irq);
+ if (!flags)
+ flags = IRQF_TRIGGER_HIGH;
+ flags |= IRQF_ONESHOT;
+ return devm_request_threaded_irq(&bus->func->dev, bus->of_irq, NULL,
+ wfx_sdio_irq_handler_ext, flags,
+ "wfx", bus);
+}
+
+static int wfx_sdio_irq_unsubscribe(void *priv)
+{
+ struct wfx_sdio_priv *bus = priv;
+ int ret;
+
+ if (bus->of_irq)
+ devm_free_irq(&bus->func->dev, bus->of_irq, bus);
+ sdio_claim_host(bus->func);
+ ret = sdio_release_irq(bus->func);
+ sdio_release_host(bus->func);
+ return ret;
+}
+
+static size_t wfx_sdio_align_size(void *priv, size_t size)
+{
+ struct wfx_sdio_priv *bus = priv;
+
+ return sdio_align_size(bus->func, size);
+}
+
+static const struct hwbus_ops wfx_sdio_hwbus_ops = {
+ .copy_from_io = wfx_sdio_copy_from_io,
+ .copy_to_io = wfx_sdio_copy_to_io,
+ .irq_subscribe = wfx_sdio_irq_subscribe,
+ .irq_unsubscribe = wfx_sdio_irq_unsubscribe,
+ .lock = wfx_sdio_lock,
+ .unlock = wfx_sdio_unlock,
+ .align_size = wfx_sdio_align_size,
+};
+
+static const struct of_device_id wfx_sdio_of_match[] = {
+ { .compatible = "silabs,wfx-sdio" },
+ { .compatible = "silabs,wf200" },
+ { },
+};
+MODULE_DEVICE_TABLE(of, wfx_sdio_of_match);
+
+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;
+
+ sdio_claim_host(func);
+ ret = sdio_enable_func(func);
+ /* Block of 64 bytes is more efficient than 512B for frame sizes < 4k */
+ sdio_set_block_size(func, 64);
+ sdio_release_host(func);
+ if (ret)
+ goto err0;
+
+ bus->core = wfx_init_common(&func->dev, &wfx_sdio_pdata,
+ &wfx_sdio_hwbus_ops, bus);
+ if (!bus->core) {
+ ret = -EIO;
+ goto err1;
+ }
+
+ ret = wfx_probe(bus->core);
+ if (ret)
+ goto err1;
+
+ return 0;
+
+err1:
+ sdio_claim_host(func);
+ sdio_disable_func(func);
+ sdio_release_host(func);
+err0:
+ return ret;
+}
+
+static void wfx_sdio_remove(struct sdio_func *func)
+{
+ struct wfx_sdio_priv *bus = sdio_get_drvdata(func);
+
+ wfx_release(bus->core);
+ sdio_claim_host(func);
+ sdio_disable_func(func);
+ sdio_release_host(func);
+}
+
+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,
+ }
+};
--
2.30.2
On Mon, 22 Mar 2021 at 18:14, Jérôme Pouiller
<[email protected]> wrote:
>
> Hello Ulf,
>
> On Monday 22 March 2021 13:20:35 CET Ulf Hansson wrote:
> > On Mon, 15 Mar 2021 at 14:25, Jerome Pouiller
> > <[email protected]> wrote:
> > >
> > > From: Jérôme Pouiller <[email protected]>
> > >
> > > Signed-off-by: Jérôme Pouiller <[email protected]>
> > > ---
> > > drivers/net/wireless/silabs/wfx/bus_sdio.c | 259 +++++++++++++++++++++
> > > 1 file changed, 259 insertions(+)
> > > create mode 100644 drivers/net/wireless/silabs/wfx/bus_sdio.c
> >
> > [...]
> >
> > > +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,
> >
> > It's not mandatory to support power management, like system
> > suspend/resume. However, as this looks like this is a driver for an
> > embedded SDIO device, you probably want this.
> >
> > If that is the case, please assign the dev_pm_ops here and implement
> > the ->suspend|resume() callbacks.
>
> I have no platform to test suspend/resume, so I have only a
> theoretical understanding of this subject.
I see.
>
> I understanding is that with the current implementation, the
> device will be powered off on suspend and then totally reset
> (including reloading of the firmware) on resume. I am wrong?
You are correct, for a *removable* SDIO card. In this case, the
mmc/sdio core will remove the corresponding SDIO card/device and its
corresponding SDIO func devices at system suspend. It will then be
redetected at system resume (and the SDIO func driver re-probed).
Although, as this is an embedded SDIO device, per definition it's not
a removable card (MMC_CAP_NONREMOVABLE should be set for the
corresponding mmc host), the SDIO card will stick around and instead
the ->suspend|resume() callback needs to be implemented for the SDIO
func driver.
>
> This behavior sounds correct to me. You would expect something
> more?
Yes, see above.
Kind regards
Uffe
On Fri, 1 Oct 2021 at 14:31, Kalle Valo <[email protected]> wrote:
>
> Hi Ulf,
>
> sorry for the late reply, my Gnus tells me it took me 24 weeks to reply :)
>
> Ulf Hansson <[email protected]> writes:
>
> > On Wed, 7 Apr 2021 at 14:00, Kalle Valo <[email protected]> wrote:
> >>
> >> Ulf Hansson <[email protected]> writes:
> >>
> >> >> If I follow what has been done in other drivers I would write something
> >> >> like:
> >> >>
> >> >> static int wfx_sdio_suspend(struct device *dev)
> >> >> {
> >> >> struct sdio_func *func = dev_to_sdio_func(dev);
> >> >> struct wfx_sdio_priv *bus = sdio_get_drvdata(func);
> >> >>
> >> >> config_reg_write_bits(bus->core, CFG_IRQ_ENABLE_DATA, 0);
> >> >> // Necessary to keep device firmware in RAM
> >> >> return sdio_set_host_pm_flags(func, MMC_PM_KEEP_POWER);
> >> >
> >> > This will tell the mmc/sdio core to keep the SDIO card powered on
> >> > during system suspend. Thus, it doesn't need to re-initialize it at
> >> > system resume - and the firmware should not need to be re-programmed.
> >> >
> >> > On the other hand, if you don't plan to support system wakeups, it
> >> > would probably be better to power off the card, to avoid wasting
> >> > energy while the system is suspended. I assume that means you need to
> >> > re-program the firmware as well. Normally, it's these kinds of things
> >> > that need to be managed from a ->resume() callback.
> >>
> >> 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.
> >
> > That sounds simple. :-)
>
> Indeed, I was omitting a lot of details :) My comment was more like a
> general remark to all different bus techonologies, not just about SDIO.
> And I'm not saying that all wireless drivers do that, but some of them
> do. Though I don't have any numbers how many.
>
> > Would you mind elaborating on what is actually being powered off at
> > interface down - and thus also I am curious what happens at a typical
> > interface up?
>
> In general in the drivers that do we this the firmware is completely
> turned off and all memory is reset during interface down. And firmware
> is started from the scratch during interface up. Also one benefit from
> this is that firmware state is reset, the wireless firmwares are
> notarious being buggy.
>
> > Even if we don't want to use system wakeups (wake-on-lan), the SDIO
> > core and the SDIO func driver still need to somewhat agree on how to
> > manage the power for the card during system suspend, I think.
> >
> > For example, for a non-removable SDIO card, the SDIO/MMC core may
> > decide to power off the card in system suspend. Then it needs to
> > restore power to the card and re-initialize it at system resume, of
> > course. This doesn't mean that the actual corresponding struct device
> > for it, gets removed/re-added, thus the SDIO func driver isn't being
> > re-probed after the system has resumed. Although, since the SDIO card
> > was re-initialized, it's likely that the FW may need to be
> > re-programmed after the system has been resumed.
> >
> > Are you saying that re-programming the FW is always happening at
> > interface up, when there are none system suspend/resume callbacks
> > assigned for the SDIO func driver?
>
> Yes, that's what I was trying to say. But take all this with grain of
> salt, I'm not very familiar with SDIO! And funnily enough, I checked
> what we do in ath10k_sdio driver during suspend has conflicting code and
> documentation:
>
> /* Empty handlers so that mmc subsystem doesn't remove us entirely during
> * suspend. We instead follow cfg80211 suspend/resume handlers.
> */
> static int ath10k_sdio_pm_suspend(struct device *device)
> {
> struct sdio_func *func = dev_to_sdio_func(device);
> struct ath10k_sdio *ar_sdio = sdio_get_drvdata(func);
> struct ath10k *ar = ar_sdio->ar;
> mmc_pm_flag_t pm_flag, pm_caps;
> int ret;
>
> if (!device_may_wakeup(ar->dev))
> return 0;
>
> ath10k_sdio_set_mbox_sleep(ar, true);
>
> pm_flag = MMC_PM_KEEP_POWER;
>
> ret = sdio_set_host_pm_flags(func, pm_flag);
> if (ret) {
> pm_caps = sdio_get_host_pm_caps(func);
> ath10k_warn(ar, "failed to set sdio host pm flags (0x%x, 0x%x): %d\n",
> pm_flag, pm_caps, ret);
> return ret;
> }
>
> return ret;
Just to confirm, the code looks reasonable to me, even if the comment
above looks a bit odd/outdated. :-)
*) Because the SDIO driver's ->suspend|resume() callbacks have been
assigned, the mmc core will not remove the corresponding SDIO
func/card's struct device.
**) If system wakeup *isn't* going to be enabled, the early return
with 0, will allow the mmc core to power off the SDIO card/func device
during system suspend. Vice versa, it will then restore power to it
and re-initialize it during system resume.
***) If system wakeup *is* going to be enabled, MMC_PM_KEEP_POWER flag
will prevent the mmc core from powering off the SDIO card/func device
during system suspend. Depending on if the wakeup irq is in-band or
out-band, MMC_PM_WAKE_SDIO_IRQ could be set too.
That said, note that ->probe() of the SDIO func driver, will not be
called for a non-removable SDIO func/card to re-program the FW after a
system suspend/resume. That needs to be managed from the SDIO func
driver's system resume callback - or deferring to upper common network
layers (interface up?), which seems to be the case here.
Kind regards
Uffe