| Message ID | 20201012104648.985256-1-Jerome.Pouiller@silabs.com |
|---|---|
| Headers | show |
| Series | wfx: get out from the staging area | expand |
On Thu, 15 Oct 2020 at 16:03, Jérôme Pouiller <jerome.pouiller@silabs.com> wrote: > > On Wednesday 14 October 2020 14:43:34 CEST Pali Rohár wrote: > > On Wednesday 14 October 2020 13:52:15 Jérôme Pouiller wrote: > > > On Tuesday 13 October 2020 22:11:56 CEST Pali Rohár wrote: > > > > On Monday 12 October 2020 12:46:32 Jerome Pouiller wrote: > > > > > +#define SDIO_VENDOR_ID_SILABS 0x0000 > > > > > +#define SDIO_DEVICE_ID_SILABS_WF200 0x1000 > > > > > +static const struct sdio_device_id wfx_sdio_ids[] = { > > > > > + { SDIO_DEVICE(SDIO_VENDOR_ID_SILABS, SDIO_DEVICE_ID_SILABS_WF200) }, > > > > > > > > Please move ids into common include file include/linux/mmc/sdio_ids.h > > > > where are all SDIO ids. Now all drivers have ids defined in that file. > > > > > > > > > + // FIXME: ignore VID/PID and only rely on device tree > > > > > + // { SDIO_DEVICE(SDIO_ANY_ID, SDIO_ANY_ID) }, > > > > > > > > What is the reason for ignoring vendor and device ids? > > > > > > The device has a particularity, its VID/PID is 0000:1000 (as you can see > > > above). This value is weird. The risk of collision with another device is > > > high. > > > > Those ids looks strange. You are from Silabs, can you check internally > > in Silabs if ids are really correct? And which sdio vendor id you in > > Silabs got assigned for your products? > > I confirm these ids are the ones burned in the WF200. We have to deal with > that :( . Yep. Unfortunately this isn't so uncommon when targeting the embedded types of devices. The good thing is, that we already have bindings allowing us to specify this. > > > > I know that sdio devices with multiple functions may have different sdio > > vendor/device id particular function and in common CIS (function 0). > > > > Could not be a problem that on one place is vendor/device id correct and > > on other place is that strange value? > > > > I have sent following patch (now part of upstream kernel) which exports > > these ids to userspace: > > https://lore.kernel.org/linux-mmc/20200527110858.17504-2-pali@kernel.org/T/#u > > > > Also for debugging ids and information about sdio cards, I sent another > > patch which export additional data: > > https://lore.kernel.org/linux-mmc/20200727133837.19086-1-pali@kernel.org/T/#u > > > > Could you try them and look at /sys/class/mmc_host/ attribute outputs? > > Here is: > > # cd /sys/class/mmc_host/ && grep -r . mmc1/ > mmc1/power/runtime_suspended_time:0 > grep: mmc1/power/autosuspend_delay_ms: Input/output error > mmc1/power/runtime_active_time:0 > mmc1/power/control:auto > mmc1/power/runtime_status:unsupported > mmc1/mmc1:0001/vendor:0x0000 > mmc1/mmc1:0001/rca:0x0001 > mmc1/mmc1:0001/device:0x1000 > mmc1/mmc1:0001/mmc1:0001:1/vendor:0x0000 > mmc1/mmc1:0001/mmc1:0001:1/device:0x1000 > grep: mmc1/mmc1:0001/mmc1:0001:1/info4: No data available > mmc1/mmc1:0001/mmc1:0001:1/power/runtime_suspended_time:0 > grep: mmc1/mmc1:0001/mmc1:0001:1/power/autosuspend_delay_ms: Input/output error > mmc1/mmc1:0001/mmc1:0001:1/power/runtime_active_time:0 > mmc1/mmc1:0001/mmc1:0001:1/power/control:auto > mmc1/mmc1:0001/mmc1:0001:1/power/runtime_status:unsupported > mmc1/mmc1:0001/mmc1:0001:1/class:0x00 > grep: mmc1/mmc1:0001/mmc1:0001:1/info2: No data available > mmc1/mmc1:0001/mmc1:0001:1/modalias:sdio:c00v0000d1000 > mmc1/mmc1:0001/mmc1:0001:1/revision:0.0 > mmc1/mmc1:0001/mmc1:0001:1/uevent:OF_NAME=mmc > mmc1/mmc1:0001/mmc1:0001:1/uevent:OF_FULLNAME=/soc/sdhci@7e300000/mmc@1 > mmc1/mmc1:0001/mmc1:0001:1/uevent:OF_COMPATIBLE_0=silabs,wfx-sdio > mmc1/mmc1:0001/mmc1:0001:1/uevent:OF_COMPATIBLE_N=1 > mmc1/mmc1:0001/mmc1:0001:1/uevent:SDIO_CLASS=00 > mmc1/mmc1:0001/mmc1:0001:1/uevent:SDIO_ID=0000:1000 > mmc1/mmc1:0001/mmc1:0001:1/uevent:SDIO_REVISION=0.0 > mmc1/mmc1:0001/mmc1:0001:1/uevent:MODALIAS=sdio:c00v0000d1000 > grep: mmc1/mmc1:0001/mmc1:0001:1/info3: No data available > grep: mmc1/mmc1:0001/mmc1:0001:1/info1: No data available > mmc1/mmc1:0001/ocr:0x00200000 > grep: mmc1/mmc1:0001/info4: No data available > mmc1/mmc1:0001/power/runtime_suspended_time:0 > grep: mmc1/mmc1:0001/power/autosuspend_delay_ms: Input/output error > mmc1/mmc1:0001/power/runtime_active_time:0 > mmc1/mmc1:0001/power/control:auto > mmc1/mmc1:0001/power/runtime_status:unsupported > grep: mmc1/mmc1:0001/info2: No data available > mmc1/mmc1:0001/type:SDIO > mmc1/mmc1:0001/revision:0.0 > mmc1/mmc1:0001/uevent:MMC_TYPE=SDIO > mmc1/mmc1:0001/uevent:SDIO_ID=0000:1000 > mmc1/mmc1:0001/uevent:SDIO_REVISION=0.0 > grep: mmc1/mmc1:0001/info3: No data available > grep: mmc1/mmc1:0001/info1: No data available > > Please have a look at the Documentation/devicetree/bindings/mmc/mmc-controller.yaml, there you find that from a child node of the mmc host's node, we can specify an embedded SDIO functional device. In sdio_add_func(), which calls sdio_set_of_node() - we assign the func->dev.of_node its corresponding child node for the SDIO func. Allowing the sdio functional driver to be matched with the SDIO func device. Perhaps what is missing, is that we may want to avoid probing an in-correct sdio driver, based upon buggy SDIO ids. I don't think we take some actions in the mmc core to prevent this, but maybe it's not a big problem anyway. When it comes to documenting the buggy SDIO ids, please add some information about this in the common SDIO headers. It's nice to have a that information, if any issue comes up later on. Kind regards Uffe
From: Jérôme Pouiller <jerome.pouiller@silabs.com> I think the wfx driver is now mature enough to be accepted in the drivers/net/wireless directory. As requested by Kalle[1], I send one file per patch. At the end, all the patches will be squashed (therefore, I didn't bother to write real commit messages). Here is a diagram of the global architecture that may help to understand the code: ,------------------------------------. | mac80211 | `------------------------------------' ,------------+-----------+-----------. | sta | | | | scan | | | | main | | | +------------+ data_tx | | | key | | data_rx | | hif_tx_mib | queue | | | hif_tx | | | | hif_rx | | | | hif_api_* | | | +------------+-----------+-----------+--------. | bh | fwio | +------------------------------------+--------+ | hwio | +---------------------------------------------+ | bus_sdio | | bus_spi | `---------------------------------------------' ,---------------------------------------------. | spi / sdio | `---------------------------------------------' Roughly, I have sent the files from the bottom to the top. Below the differences with the files from drivers/staging/wfx/: v1: - Drop the function name in the warning message (Kalle) - Replace goto by return in wfx_send_pdata_pds() (Kalle, Dan) - Improve error label in wfx_send_pdata_pds() (Kalle) [1] https://lore.kernel.org/lkml/87ft6p2n0h.fsf@codeaurora.org/ *** BLURB HERE *** Jérôme Pouiller (23): dt-bindings: introduce silabs,wfx.yaml wfx: add Makefile/Kconfig wfx: add wfx.h wfx: add main.c/main.h wfx: add bus.h wfx: add bus_spi.c wfx: add bus_sdio.c wfx: add hwio.c/hwio.h wfx: add fwio.c/fwio.h wfx: add bh.c/bh.h wfx: add hif_api_*.h wfx: add hif_tx*.c/hif_tx*.h wfx: add key.c/key.h wfx: add hif_rx.c/hif_rx.h wfx: add data_rx.c/data_rx.h wfx: add queue.c/queue.h wfx: add data_tx.c/data_tx.h wfx: add sta.c/sta.h wfx: add scan.c/scan.h wfx: add debug.c/debug.h wfx: add traces.h wfx: remove from the staging area wfx: get out from the staging area .../bindings/net/wireless/silabs,wfx.yaml | 0 MAINTAINERS | 3 ++- drivers/net/wireless/Kconfig | 1 + drivers/net/wireless/Makefile | 1 + drivers/net/wireless/silabs/Kconfig | 18 ++++++++++++++++++ drivers/net/wireless/silabs/Makefile | 3 +++ .../wireless/silabs}/wfx/Kconfig | 0 .../wireless/silabs}/wfx/Makefile | 0 .../{staging => net/wireless/silabs}/wfx/bh.c | 0 .../{staging => net/wireless/silabs}/wfx/bh.h | 0 .../{staging => net/wireless/silabs}/wfx/bus.h | 0 .../wireless/silabs}/wfx/bus_sdio.c | 0 .../wireless/silabs}/wfx/bus_spi.c | 0 .../wireless/silabs}/wfx/data_rx.c | 0 .../wireless/silabs}/wfx/data_rx.h | 0 .../wireless/silabs}/wfx/data_tx.c | 2 +- .../wireless/silabs}/wfx/data_tx.h | 0 .../wireless/silabs}/wfx/debug.c | 0 .../wireless/silabs}/wfx/debug.h | 0 .../wireless/silabs}/wfx/fwio.c | 0 .../wireless/silabs}/wfx/fwio.h | 0 .../wireless/silabs}/wfx/hif_api_cmd.h | 0 .../wireless/silabs}/wfx/hif_api_general.h | 0 .../wireless/silabs}/wfx/hif_api_mib.h | 0 .../wireless/silabs}/wfx/hif_rx.c | 0 .../wireless/silabs}/wfx/hif_rx.h | 0 .../wireless/silabs}/wfx/hif_tx.c | 0 .../wireless/silabs}/wfx/hif_tx.h | 0 .../wireless/silabs}/wfx/hif_tx_mib.c | 0 .../wireless/silabs}/wfx/hif_tx_mib.h | 0 .../wireless/silabs}/wfx/hwio.c | 0 .../wireless/silabs}/wfx/hwio.h | 0 .../{staging => net/wireless/silabs}/wfx/key.c | 0 .../{staging => net/wireless/silabs}/wfx/key.h | 0 .../wireless/silabs}/wfx/main.c | 7 +++---- .../wireless/silabs}/wfx/main.h | 0 .../wireless/silabs}/wfx/queue.c | 0 .../wireless/silabs}/wfx/queue.h | 0 .../wireless/silabs}/wfx/scan.c | 0 .../wireless/silabs}/wfx/scan.h | 0 .../{staging => net/wireless/silabs}/wfx/sta.c | 0 .../{staging => net/wireless/silabs}/wfx/sta.h | 0 .../wireless/silabs}/wfx/traces.h | 0 .../{staging => net/wireless/silabs}/wfx/wfx.h | 0 drivers/staging/Kconfig | 2 -- drivers/staging/Makefile | 1 - drivers/staging/wfx/TODO | 6 ------ 47 files changed, 29 insertions(+), 15 deletions(-) rename {drivers/staging/wfx/Documentation => Documentation}/devicetree/bindings/net/wireless/silabs,wfx.yaml (100%) create mode 100644 drivers/net/wireless/silabs/Kconfig create mode 100644 drivers/net/wireless/silabs/Makefile rename drivers/{staging => net/wireless/silabs}/wfx/Kconfig (100%) rename drivers/{staging => net/wireless/silabs}/wfx/Makefile (100%) rename drivers/{staging => net/wireless/silabs}/wfx/bh.c (100%) rename drivers/{staging => net/wireless/silabs}/wfx/bh.h (100%) rename drivers/{staging => net/wireless/silabs}/wfx/bus.h (100%) rename drivers/{staging => net/wireless/silabs}/wfx/bus_sdio.c (100%) rename drivers/{staging => net/wireless/silabs}/wfx/bus_spi.c (100%) rename drivers/{staging => net/wireless/silabs}/wfx/data_rx.c (100%) rename drivers/{staging => net/wireless/silabs}/wfx/data_rx.h (100%) rename drivers/{staging => net/wireless/silabs}/wfx/data_tx.c (99%) rename drivers/{staging => net/wireless/silabs}/wfx/data_tx.h (100%) rename drivers/{staging => net/wireless/silabs}/wfx/debug.c (100%) rename drivers/{staging => net/wireless/silabs}/wfx/debug.h (100%) rename drivers/{staging => net/wireless/silabs}/wfx/fwio.c (100%) rename drivers/{staging => net/wireless/silabs}/wfx/fwio.h (100%) rename drivers/{staging => net/wireless/silabs}/wfx/hif_api_cmd.h (100%) rename drivers/{staging => net/wireless/silabs}/wfx/hif_api_general.h (100%) rename drivers/{staging => net/wireless/silabs}/wfx/hif_api_mib.h (100%) rename drivers/{staging => net/wireless/silabs}/wfx/hif_rx.c (100%) rename drivers/{staging => net/wireless/silabs}/wfx/hif_rx.h (100%) rename drivers/{staging => net/wireless/silabs}/wfx/hif_tx.c (100%) rename drivers/{staging => net/wireless/silabs}/wfx/hif_tx.h (100%) rename drivers/{staging => net/wireless/silabs}/wfx/hif_tx_mib.c (100%) rename drivers/{staging => net/wireless/silabs}/wfx/hif_tx_mib.h (100%) rename drivers/{staging => net/wireless/silabs}/wfx/hwio.c (100%) rename drivers/{staging => net/wireless/silabs}/wfx/hwio.h (100%) rename drivers/{staging => net/wireless/silabs}/wfx/key.c (100%) rename drivers/{staging => net/wireless/silabs}/wfx/key.h (100%) rename drivers/{staging => net/wireless/silabs}/wfx/main.c (99%) rename drivers/{staging => net/wireless/silabs}/wfx/main.h (100%) rename drivers/{staging => net/wireless/silabs}/wfx/queue.c (100%) rename drivers/{staging => net/wireless/silabs}/wfx/queue.h (100%) rename drivers/{staging => net/wireless/silabs}/wfx/scan.c (100%) rename drivers/{staging => net/wireless/silabs}/wfx/scan.h (100%) rename drivers/{staging => net/wireless/silabs}/wfx/sta.c (100%) rename drivers/{staging => net/wireless/silabs}/wfx/sta.h (100%) rename drivers/{staging => net/wireless/silabs}/wfx/traces.h (100%) rename drivers/{staging => net/wireless/silabs}/wfx/wfx.h (100%) delete mode 100644 drivers/staging/wfx/TODO