On 29/10/2024 18:23, Michael Nemanov wrote: > sdio.c implements SDIO transport functions. These are bound into > struct cc33xx_if_operations and accessed via io.h in order to abstract > multiple transport interfaces such as SPI in the future. > The CC33xx driver supports the SDIO in-band IRQ option so the IRQ from > the device received here as well. > Unlike wl1xxx products, there is no longer mapping between > HW and SDIO / SPI address space of any kind. > There are only 3 valid addresses for control, data and status > transactions each with a predefined structure. > > Signed-off-by: Michael Nemanov <michael.nemanov@xxxxxx> > --- > drivers/net/wireless/ti/cc33xx/io.c | 129 +++++++ > drivers/net/wireless/ti/cc33xx/io.h | 26 ++ > drivers/net/wireless/ti/cc33xx/sdio.c | 530 ++++++++++++++++++++++++++ > 3 files changed, 685 insertions(+) > create mode 100644 drivers/net/wireless/ti/cc33xx/io.c > create mode 100644 drivers/net/wireless/ti/cc33xx/io.h > create mode 100644 drivers/net/wireless/ti/cc33xx/sdio.c > > diff --git a/drivers/net/wireless/ti/cc33xx/io.c b/drivers/net/wireless/ti/cc33xx/io.c > new file mode 100644 > index 000000000000..59696004efe9 > --- /dev/null > +++ b/drivers/net/wireless/ti/cc33xx/io.c > @@ -0,0 +1,129 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (C) 2022-2024 Texas Instruments Incorporated - https://www.ti.com/ > + */ > + > +#include "cc33xx.h" > +#include "debug.h" > +#include "io.h" > +#include "tx.h" > + > +bool cc33xx_set_block_size(struct cc33xx *cc) > +{ > + if (cc->if_ops->set_block_size) { > + cc->if_ops->set_block_size(cc->dev, CC33XX_BUS_BLOCK_SIZE); > + cc33xx_debug(DEBUG_CC33xx, > + "Set BLKsize to %d", CC33XX_BUS_BLOCK_SIZE); > + return true; > + } > + > + cc33xx_debug(DEBUG_CC33xx, "Could not set BLKsize"); > + return false; > +} > + > +void cc33xx_disable_interrupts_nosync(struct cc33xx *cc) > +{ > + cc->if_ops->disable_irq(cc->dev); > +} > + > +void cc33xx_irq(void *cookie); Why do you need forward declaration of non-static function? If you need it, it means you had W=1 warning which you fixed incorrect way. Regardless, be sure this code has 0 warnings on clang with W=1. > +void cc33xx_enable_interrupts(struct cc33xx *cc) > +{ > + cc->if_ops->enable_irq(cc->dev); > + > + cc33xx_irq(cc); > +} ... > +static const struct cc33xx_if_operations sdio_ops_inband_irq = { > + .interface_claim = cc33xx_sdio_claim, > + .interface_release = cc33xx_sdio_release, > + .read = cc33xx_sdio_raw_read, > + .write = cc33xx_sdio_raw_write, > + .power = cc33xx_sdio_set_power, > + .set_block_size = cc33xx_sdio_set_block_size, > + .set_irq_handler = cc33xx_set_irq_handler, > + .disable_irq = cc33xx_sdio_disable_irq, > + .enable_irq = cc33xx_sdio_enable_irq, > +}; > + > +#ifdef CONFIG_OF > +static const struct cc33xx_family_data cc33xx_data = { > + .name = "cc33xx", > + .cfg_name = "ti-connectivity/cc33xx-conf.bin", > + .nvs_name = "ti-connectivity/cc33xx-nvs.bin", > +}; > + > +static const struct of_device_id cc33xx_sdio_of_match_table[] = { > + { .compatible = "ti,cc33xx", .data = &cc33xx_data }, This is supposed to be your base variant. > + { } > +}; Missing MODULE_DEVICE_TABLE... or you do not autoload this based on OF matching? That's a bit surprising, I don't remember how SDIO bus handles it. But for other buses this is unexpected and usually not correct. > + ... > + > +static struct sdio_driver cc33xx_sdio_driver = { > + .name = "cc33xx_sdio", > + .id_table = cc33xx_devices, > + .probe = sdio_cc33xx_probe, > + .remove = sdio_cc33xx_remove, > +#ifdef CONFIG_PM > + .drv = { > + .pm = &cc33xx_sdio_pm_ops, > + }, > +#endif /* CONFIG_PM */ > +}; > + > +MODULE_DEVICE_TABLE(sdio, cc33xx_devices); This is always next to the table. > + > +module_sdio_driver(cc33xx_sdio_driver); > + > +MODULE_LICENSE("GPL v2"); > +MODULE_DESCRIPTION("SDIO transport for Texas Instruments CC33xx WLAN driver"); > +MODULE_AUTHOR("Michael Nemanov <michael.nemanov@xxxxxx>"); > +MODULE_AUTHOR("Sabeeh Khan <sabeeh-khan@xxxxxx>"); Best regards, Krzysztof
