[alsa-devel] [RFC PATCH 0/9] soundwire: add Master device support, GreyBus style
The current use of platform devices leads to limitations in terms of API and problematic reference counts flagged during the review of sysfs patches.
As suggested by Greg KH, this patch series introduces a 'Master device' and gets rid of platform devices. The code is inspired by GreyBus, e.g. 'gb_hd_driver' is translated for the SoundWire context as 'sdw_md_driver' and likewise 'gb_host_device' is translated as 'sdw_master_device'. There are differences in the way devices are created, using device_register instead of the multiple steps used in GreyBus.
All I know about GreyBus is that it's based on Unipro, so the code updates are based on a first-order analysis, it could very well be I missed concepts while trying to reverse-engineer the code in staging/greybus/. I focused on the concepts mainly and it's also possible that the dual-license for new files or plan-vanilla EXPORT_SYMBOL are not appropriate for all the changes in this series. Please don't take errors as deliberate intent to work-around GPL but rather as an indicator of maturity of the code only developed in the last few days.
These changes make it possible to provide new callbacks, e.g splitting the bus startup from the initializations in probe, which is very much desired on Intel platforms to detect the machine driver as early as possible before the hardware is fully powered/enable. These patches will be provided as a separate RFC later today to illustrate the benefit of these changes.
To make the code more consistent the first series of patches are renames. I did not rename 'sdw_slave' or 'module_sdw_driver' to avoid compatibility issues since there are codec drivers almost ready for integration. I don't have any specific opinion on if/when additional renames should be done, as long as there is a means to clearly identify what is specific to a SoundWire Master I am fine.
Feedback and reviews welcome.
Bard Liao (1): soundwire: add device driver to sdw_md_driver
Pierre-Louis Bossart (8): soundwire: renames to prepare support for master drivers/devices soundwire: rename dev_to_sdw_dev macro soundwire: rename drv_to_sdw_slave_driver macro soundwire: bus_type: rename sdw_drv_ to sdw_slave_drv soundwire: intel: rename res field as link_res soundwire: add support for sdw_slave_type soundwire: add initial definitions for sdw_master_device soundwire: intel: remove platform devices and provide new interface
drivers/base/regmap/regmap-sdw.c | 4 +- drivers/soundwire/Makefile | 2 +- drivers/soundwire/bus.c | 2 +- drivers/soundwire/bus_type.c | 60 +++--- drivers/soundwire/intel.c | 117 ++++++----- drivers/soundwire/intel.h | 22 +-- drivers/soundwire/intel_init.c | 293 +++++++++++++++++++--------- drivers/soundwire/master.c | 80 ++++++++ drivers/soundwire/slave.c | 9 +- include/linux/soundwire/sdw.h | 39 +++- include/linux/soundwire/sdw_intel.h | 86 +++++++- include/linux/soundwire/sdw_type.h | 34 +++- 12 files changed, 551 insertions(+), 197 deletions(-) create mode 100644 drivers/soundwire/master.c
Add clearer references to sdw_slave_driver for internal macros
No change for sdw_driver and module_sdw_driver to avoid compatibility issues with existing codec devices
No functionality change.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- drivers/soundwire/bus_type.c | 21 +++++++++++---------- include/linux/soundwire/sdw_type.h | 18 ++++++++++-------- 2 files changed, 21 insertions(+), 18 deletions(-)
diff --git a/drivers/soundwire/bus_type.c b/drivers/soundwire/bus_type.c index 4a465f55039f..370b94752662 100644 --- a/drivers/soundwire/bus_type.c +++ b/drivers/soundwire/bus_type.c @@ -34,7 +34,7 @@ sdw_get_device_id(struct sdw_slave *slave, struct sdw_driver *drv) static int sdw_bus_match(struct device *dev, struct device_driver *ddrv) { struct sdw_slave *slave = dev_to_sdw_dev(dev); - struct sdw_driver *drv = drv_to_sdw_driver(ddrv); + struct sdw_driver *drv = drv_to_sdw_slave_driver(ddrv);
return !!sdw_get_device_id(slave, drv); } @@ -70,7 +70,7 @@ EXPORT_SYMBOL_GPL(sdw_bus_type); static int sdw_drv_probe(struct device *dev) { struct sdw_slave *slave = dev_to_sdw_dev(dev); - struct sdw_driver *drv = drv_to_sdw_driver(dev->driver); + struct sdw_driver *drv = drv_to_sdw_slave_driver(dev->driver); const struct sdw_device_id *id; int ret;
@@ -116,7 +116,7 @@ static int sdw_drv_probe(struct device *dev) static int sdw_drv_remove(struct device *dev) { struct sdw_slave *slave = dev_to_sdw_dev(dev); - struct sdw_driver *drv = drv_to_sdw_driver(dev->driver); + struct sdw_driver *drv = drv_to_sdw_slave_driver(dev->driver); int ret = 0;
if (drv->remove) @@ -130,20 +130,21 @@ static int sdw_drv_remove(struct device *dev) static void sdw_drv_shutdown(struct device *dev) { struct sdw_slave *slave = dev_to_sdw_dev(dev); - struct sdw_driver *drv = drv_to_sdw_driver(dev->driver); + struct sdw_driver *drv = drv_to_sdw_slave_driver(dev->driver);
if (drv->shutdown) drv->shutdown(slave); }
/** - * __sdw_register_driver() - register a SoundWire Slave driver + * __sdw_register_slave_driver() - register a SoundWire Slave driver * @drv: driver to register * @owner: owning module/driver * * Return: zero on success, else a negative error code. */ -int __sdw_register_driver(struct sdw_driver *drv, struct module *owner) +int __sdw_register_slave_driver(struct sdw_driver *drv, + struct module *owner) { drv->driver.bus = &sdw_bus_type;
@@ -164,17 +165,17 @@ int __sdw_register_driver(struct sdw_driver *drv, struct module *owner)
return driver_register(&drv->driver); } -EXPORT_SYMBOL_GPL(__sdw_register_driver); +EXPORT_SYMBOL_GPL(__sdw_register_slave_driver);
/** - * sdw_unregister_driver() - unregisters the SoundWire Slave driver + * sdw_unregister_slave_driver() - unregisters the SoundWire Slave driver * @drv: driver to unregister */ -void sdw_unregister_driver(struct sdw_driver *drv) +void sdw_unregister_slave_driver(struct sdw_driver *drv) { driver_unregister(&drv->driver); } -EXPORT_SYMBOL_GPL(sdw_unregister_driver); +EXPORT_SYMBOL_GPL(sdw_unregister_slave_driver);
static int __init sdw_bus_init(void) { diff --git a/include/linux/soundwire/sdw_type.h b/include/linux/soundwire/sdw_type.h index aaa7f4267c14..abaa21278152 100644 --- a/include/linux/soundwire/sdw_type.h +++ b/include/linux/soundwire/sdw_type.h @@ -6,13 +6,15 @@
extern struct bus_type sdw_bus_type;
-#define drv_to_sdw_driver(_drv) container_of(_drv, struct sdw_driver, driver) +#define drv_to_sdw_slave_driver(_drv) \ + container_of(_drv, struct sdw_driver, driver)
-#define sdw_register_driver(drv) \ - __sdw_register_driver(drv, THIS_MODULE) +#define sdw_register_slave_driver(drv) \ + __sdw_register_slave_driver(drv, THIS_MODULE)
-int __sdw_register_driver(struct sdw_driver *drv, struct module *owner); -void sdw_unregister_driver(struct sdw_driver *drv); +int __sdw_register_slave_driver(struct sdw_driver *drv, + struct module *owner); +void sdw_unregister_slave_driver(struct sdw_driver *drv);
int sdw_slave_modalias(const struct sdw_slave *slave, char *buf, size_t size);
@@ -24,7 +26,7 @@ int sdw_slave_modalias(const struct sdw_slave *slave, char *buf, size_t size); * module init/exit. This eliminates a lot of boilerplate. Each module may only * use this macro once, and calling it replaces module_init() and module_exit() */ -#define module_sdw_driver(__sdw_driver) \ - module_driver(__sdw_driver, sdw_register_driver, \ - sdw_unregister_driver) +#define module_sdw_driver(__sdw_slave_driver) \ + module_driver(__sdw_slave_driver, sdw_register_slave_driver, \ + sdw_unregister_slave_driver) #endif /* __SOUNDWIRE_TYPES_H */
Since we want to introduce master devices, rename macro so that we have consistency between slave and master device access, following the Grey Bus example.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- drivers/base/regmap/regmap-sdw.c | 4 ++-- drivers/soundwire/bus.c | 2 +- drivers/soundwire/bus_type.c | 9 +++++---- drivers/soundwire/slave.c | 2 +- include/linux/soundwire/sdw.h | 3 ++- 5 files changed, 11 insertions(+), 9 deletions(-)
diff --git a/drivers/base/regmap/regmap-sdw.c b/drivers/base/regmap/regmap-sdw.c index 50a66382d87d..d1fc0c22180a 100644 --- a/drivers/base/regmap/regmap-sdw.c +++ b/drivers/base/regmap/regmap-sdw.c @@ -10,7 +10,7 @@ static int regmap_sdw_write(void *context, unsigned int reg, unsigned int val) { struct device *dev = context; - struct sdw_slave *slave = dev_to_sdw_dev(dev); + struct sdw_slave *slave = to_sdw_slave_device(dev);
return sdw_write(slave, reg, val); } @@ -18,7 +18,7 @@ static int regmap_sdw_write(void *context, unsigned int reg, unsigned int val) static int regmap_sdw_read(void *context, unsigned int reg, unsigned int *val) { struct device *dev = context; - struct sdw_slave *slave = dev_to_sdw_dev(dev); + struct sdw_slave *slave = to_sdw_slave_device(dev); int read;
read = sdw_read(slave, reg); diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c index fc53dbe57f85..2f44ed34bd0c 100644 --- a/drivers/soundwire/bus.c +++ b/drivers/soundwire/bus.c @@ -110,7 +110,7 @@ EXPORT_SYMBOL(sdw_add_bus_master);
static int sdw_delete_slave(struct device *dev, void *data) { - struct sdw_slave *slave = dev_to_sdw_dev(dev); + struct sdw_slave *slave = to_sdw_slave_device(dev); struct sdw_bus *bus = slave->bus;
sdw_slave_debugfs_exit(slave); diff --git a/drivers/soundwire/bus_type.c b/drivers/soundwire/bus_type.c index 370b94752662..071605ca01fa 100644 --- a/drivers/soundwire/bus_type.c +++ b/drivers/soundwire/bus_type.c @@ -33,7 +33,7 @@ sdw_get_device_id(struct sdw_slave *slave, struct sdw_driver *drv)
static int sdw_bus_match(struct device *dev, struct device_driver *ddrv) { - struct sdw_slave *slave = dev_to_sdw_dev(dev); + struct sdw_slave *slave = to_sdw_slave_device(dev); struct sdw_driver *drv = drv_to_sdw_slave_driver(ddrv);
return !!sdw_get_device_id(slave, drv); @@ -69,7 +69,7 @@ EXPORT_SYMBOL_GPL(sdw_bus_type);
static int sdw_drv_probe(struct device *dev) { - struct sdw_slave *slave = dev_to_sdw_dev(dev); + struct sdw_slave *slave = to_sdw_slave_device(dev); struct sdw_driver *drv = drv_to_sdw_slave_driver(dev->driver); const struct sdw_device_id *id; int ret; @@ -115,8 +115,9 @@ static int sdw_drv_probe(struct device *dev)
static int sdw_drv_remove(struct device *dev) { - struct sdw_slave *slave = dev_to_sdw_dev(dev); + struct sdw_slave *slave = to_sdw_slave_device(dev); struct sdw_driver *drv = drv_to_sdw_slave_driver(dev->driver); + int ret = 0;
if (drv->remove) @@ -129,7 +130,7 @@ static int sdw_drv_remove(struct device *dev)
static void sdw_drv_shutdown(struct device *dev) { - struct sdw_slave *slave = dev_to_sdw_dev(dev); + struct sdw_slave *slave = to_sdw_slave_device(dev); struct sdw_driver *drv = drv_to_sdw_slave_driver(dev->driver);
if (drv->shutdown) diff --git a/drivers/soundwire/slave.c b/drivers/soundwire/slave.c index 48a63ca130d2..40e41796a499 100644 --- a/drivers/soundwire/slave.c +++ b/drivers/soundwire/slave.c @@ -9,7 +9,7 @@
static void sdw_slave_release(struct device *dev) { - struct sdw_slave *slave = dev_to_sdw_dev(dev); + struct sdw_slave *slave = to_sdw_slave_device(dev);
kfree(slave); } diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h index 688b40e65c89..f004de91ce24 100644 --- a/include/linux/soundwire/sdw.h +++ b/include/linux/soundwire/sdw.h @@ -561,7 +561,8 @@ struct sdw_slave { u16 dev_num; };
-#define dev_to_sdw_dev(_dev) container_of(_dev, struct sdw_slave, dev) +#define to_sdw_slave_device(d) \ + container_of(d, struct sdw_slave, dev)
struct sdw_driver { const char *name;
Align with previous renames and shorten macro
No functionality change
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- drivers/soundwire/bus_type.c | 9 ++++----- include/linux/soundwire/sdw_type.h | 3 ++- 2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/soundwire/bus_type.c b/drivers/soundwire/bus_type.c index 071605ca01fa..ae56fdd434af 100644 --- a/drivers/soundwire/bus_type.c +++ b/drivers/soundwire/bus_type.c @@ -34,7 +34,7 @@ sdw_get_device_id(struct sdw_slave *slave, struct sdw_driver *drv) static int sdw_bus_match(struct device *dev, struct device_driver *ddrv) { struct sdw_slave *slave = to_sdw_slave_device(dev); - struct sdw_driver *drv = drv_to_sdw_slave_driver(ddrv); + struct sdw_driver *drv = to_sdw_slave_driver(ddrv);
return !!sdw_get_device_id(slave, drv); } @@ -70,7 +70,7 @@ EXPORT_SYMBOL_GPL(sdw_bus_type); static int sdw_drv_probe(struct device *dev) { struct sdw_slave *slave = to_sdw_slave_device(dev); - struct sdw_driver *drv = drv_to_sdw_slave_driver(dev->driver); + struct sdw_driver *drv = to_sdw_slave_driver(dev->driver); const struct sdw_device_id *id; int ret;
@@ -116,8 +116,7 @@ static int sdw_drv_probe(struct device *dev) static int sdw_drv_remove(struct device *dev) { struct sdw_slave *slave = to_sdw_slave_device(dev); - struct sdw_driver *drv = drv_to_sdw_slave_driver(dev->driver); - + struct sdw_driver *drv = to_sdw_slave_driver(dev->driver); int ret = 0;
if (drv->remove) @@ -131,7 +130,7 @@ static int sdw_drv_remove(struct device *dev) static void sdw_drv_shutdown(struct device *dev) { struct sdw_slave *slave = to_sdw_slave_device(dev); - struct sdw_driver *drv = drv_to_sdw_slave_driver(dev->driver); + struct sdw_driver *drv = to_sdw_slave_driver(dev->driver);
if (drv->shutdown) drv->shutdown(slave); diff --git a/include/linux/soundwire/sdw_type.h b/include/linux/soundwire/sdw_type.h index abaa21278152..7d4bc6a979bf 100644 --- a/include/linux/soundwire/sdw_type.h +++ b/include/linux/soundwire/sdw_type.h @@ -6,7 +6,7 @@
extern struct bus_type sdw_bus_type;
-#define drv_to_sdw_slave_driver(_drv) \ +#define to_sdw_slave_driver(_drv) \ container_of(_drv, struct sdw_driver, driver)
#define sdw_register_slave_driver(drv) \ @@ -29,4 +29,5 @@ int sdw_slave_modalias(const struct sdw_slave *slave, char *buf, size_t size); #define module_sdw_driver(__sdw_slave_driver) \ module_driver(__sdw_slave_driver, sdw_register_slave_driver, \ sdw_unregister_slave_driver) + #endif /* __SOUNDWIRE_TYPES_H */
Before we add master driver support, make sure there is no ambiguity and no occirrences of sdw_drv_ functions.
No functionality change.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- drivers/soundwire/bus_type.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/soundwire/bus_type.c b/drivers/soundwire/bus_type.c index ae56fdd434af..9407ebf30012 100644 --- a/drivers/soundwire/bus_type.c +++ b/drivers/soundwire/bus_type.c @@ -67,7 +67,7 @@ struct bus_type sdw_bus_type = { }; EXPORT_SYMBOL_GPL(sdw_bus_type);
-static int sdw_drv_probe(struct device *dev) +static int sdw_slave_drv_probe(struct device *dev) { struct sdw_slave *slave = to_sdw_slave_device(dev); struct sdw_driver *drv = to_sdw_slave_driver(dev->driver); @@ -113,7 +113,7 @@ static int sdw_drv_probe(struct device *dev) return 0; }
-static int sdw_drv_remove(struct device *dev) +static int sdw_slave_drv_remove(struct device *dev) { struct sdw_slave *slave = to_sdw_slave_device(dev); struct sdw_driver *drv = to_sdw_slave_driver(dev->driver); @@ -127,7 +127,7 @@ static int sdw_drv_remove(struct device *dev) return ret; }
-static void sdw_drv_shutdown(struct device *dev) +static void sdw_slave_drv_shutdown(struct device *dev) { struct sdw_slave *slave = to_sdw_slave_device(dev); struct sdw_driver *drv = to_sdw_slave_driver(dev->driver); @@ -155,13 +155,13 @@ int __sdw_register_slave_driver(struct sdw_driver *drv, }
drv->driver.owner = owner; - drv->driver.probe = sdw_drv_probe; + drv->driver.probe = sdw_slave_drv_probe;
if (drv->remove) - drv->driver.remove = sdw_drv_remove; + drv->driver.remove = sdw_slave_drv_remove;
if (drv->shutdown) - drv->driver.shutdown = sdw_drv_shutdown; + drv->driver.shutdown = sdw_slave_drv_shutdown;
return driver_register(&drv->driver); }
There are too many fields called 'res' so add prefix to make it easier to track what the structures are.
Pure rename, no functionality change
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- drivers/soundwire/intel.c | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-)
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index 44e7afee83b5..8a1f6c627788 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -103,7 +103,7 @@ enum intel_pdi_type { struct sdw_intel { struct sdw_cdns cdns; int instance; - struct sdw_intel_link_res *res; + struct sdw_intel_link_res *link_res; #ifdef CONFIG_DEBUG_FS struct dentry *debugfs; #endif @@ -193,8 +193,8 @@ static ssize_t intel_sprintf(void __iomem *mem, bool l, static int intel_reg_show(struct seq_file *s_file, void *data) { struct sdw_intel *sdw = s_file->private; - void __iomem *s = sdw->res->shim; - void __iomem *a = sdw->res->alh; + void __iomem *s = sdw->link_res->shim; + void __iomem *a = sdw->link_res->alh; char *buf; ssize_t ret; int i, j; @@ -289,7 +289,7 @@ static void intel_debugfs_exit(struct sdw_intel *sdw) {} static int intel_link_power_up(struct sdw_intel *sdw) { unsigned int link_id = sdw->instance; - void __iomem *shim = sdw->res->shim; + void __iomem *shim = sdw->link_res->shim; int spa_mask, cpa_mask; int link_control, ret;
@@ -309,7 +309,7 @@ static int intel_link_power_up(struct sdw_intel *sdw)
static int intel_shim_init(struct sdw_intel *sdw) { - void __iomem *shim = sdw->res->shim; + void __iomem *shim = sdw->link_res->shim; unsigned int link_id = sdw->instance; int sync_reg, ret; u16 ioctl = 0, act = 0; @@ -370,7 +370,7 @@ static int intel_shim_init(struct sdw_intel *sdw) static void intel_pdi_init(struct sdw_intel *sdw, struct sdw_cdns_stream_config *config) { - void __iomem *shim = sdw->res->shim; + void __iomem *shim = sdw->link_res->shim; unsigned int link_id = sdw->instance; int pcm_cap, pdm_cap;
@@ -404,7 +404,7 @@ static void intel_pdi_init(struct sdw_intel *sdw, static int intel_pdi_get_ch_cap(struct sdw_intel *sdw, unsigned int pdi_num, bool pcm) { - void __iomem *shim = sdw->res->shim; + void __iomem *shim = sdw->link_res->shim; unsigned int link_id = sdw->instance; int count;
@@ -476,7 +476,7 @@ static int intel_pdi_ch_update(struct sdw_intel *sdw) static void intel_pdi_shim_configure(struct sdw_intel *sdw, struct sdw_cdns_pdi *pdi) { - void __iomem *shim = sdw->res->shim; + void __iomem *shim = sdw->link_res->shim; unsigned int link_id = sdw->instance; int pdi_conf = 0;
@@ -505,7 +505,7 @@ intel_pdi_shim_configure(struct sdw_intel *sdw, struct sdw_cdns_pdi *pdi) static void intel_pdi_alh_configure(struct sdw_intel *sdw, struct sdw_cdns_pdi *pdi) { - void __iomem *alh = sdw->res->alh; + void __iomem *alh = sdw->link_res->alh; unsigned int link_id = sdw->instance; unsigned int conf;
@@ -528,7 +528,7 @@ static int intel_config_stream(struct sdw_intel *sdw, struct snd_soc_dai *dai, struct snd_pcm_hw_params *hw_params, int link_id) { - struct sdw_intel_link_res *res = sdw->res; + struct sdw_intel_link_res *res = sdw->link_res;
if (res->ops && res->ops->config_stream && res->arg) return res->ops->config_stream(res->arg, @@ -545,7 +545,7 @@ static int intel_pre_bank_switch(struct sdw_bus *bus) { struct sdw_cdns *cdns = bus_to_cdns(bus); struct sdw_intel *sdw = cdns_to_intel(cdns); - void __iomem *shim = sdw->res->shim; + void __iomem *shim = sdw->link_res->shim; int sync_reg;
/* Write to register only for multi-link */ @@ -564,7 +564,7 @@ static int intel_post_bank_switch(struct sdw_bus *bus) { struct sdw_cdns *cdns = bus_to_cdns(bus); struct sdw_intel *sdw = cdns_to_intel(cdns); - void __iomem *shim = sdw->res->shim; + void __iomem *shim = sdw->link_res->shim; int sync_reg, ret;
/* Write to register only for multi-link */ @@ -920,9 +920,9 @@ static int intel_probe(struct platform_device *pdev) return -ENOMEM;
sdw->instance = pdev->id; - sdw->res = dev_get_platdata(&pdev->dev); + sdw->link_res = dev_get_platdata(&pdev->dev); sdw->cdns.dev = &pdev->dev; - sdw->cdns.registers = sdw->res->registers; + sdw->cdns.registers = sdw->link_res->registers; sdw->cdns.instance = sdw->instance; sdw->cdns.msg_count = 0; sdw->cdns.bus.dev = &pdev->dev; @@ -962,11 +962,12 @@ static int intel_probe(struct platform_device *pdev) intel_pdi_ch_update(sdw);
/* Acquire IRQ */ - ret = request_threaded_irq(sdw->res->irq, sdw_cdns_irq, sdw_cdns_thread, + ret = request_threaded_irq(sdw->link_res->irq, + sdw_cdns_irq, sdw_cdns_thread, IRQF_SHARED, KBUILD_MODNAME, &sdw->cdns); if (ret < 0) { dev_err(sdw->cdns.dev, "unable to grab IRQ %d, disabling device\n", - sdw->res->irq); + sdw->link_res->irq); goto err_init; }
@@ -997,7 +998,7 @@ static int intel_probe(struct platform_device *pdev) err_interrupt: sdw_cdns_enable_interrupt(&sdw->cdns, false); err_dai: - free_irq(sdw->res->irq, sdw); + free_irq(sdw->link_res->irq, sdw); err_init: sdw_delete_bus_master(&sdw->cdns.bus); return ret; @@ -1012,7 +1013,7 @@ static int intel_remove(struct platform_device *pdev) if (!sdw->cdns.bus.prop.hw_disabled) { intel_debugfs_exit(sdw); sdw_cdns_enable_interrupt(&sdw->cdns, false); - free_irq(sdw->res->irq, sdw); + free_irq(sdw->link_res->irq, sdw); snd_soc_unregister_component(sdw->cdns.dev); } sdw_delete_bus_master(&sdw->cdns.bus);
Currently the bus does not have any explicit support for master devices. Add explicit support for sdw_slave_type, so that in follow-up patches we can add support for the sdw_md_type (md==Master Device), following the Grey Bus example.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- drivers/soundwire/bus_type.c | 9 ++++++++- drivers/soundwire/slave.c | 7 ++++++- include/linux/soundwire/sdw_type.h | 6 ++++++ 3 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/drivers/soundwire/bus_type.c b/drivers/soundwire/bus_type.c index 9407ebf30012..5df095f4e12f 100644 --- a/drivers/soundwire/bus_type.c +++ b/drivers/soundwire/bus_type.c @@ -49,9 +49,16 @@ int sdw_slave_modalias(const struct sdw_slave *slave, char *buf, size_t size)
static int sdw_uevent(struct device *dev, struct kobj_uevent_env *env) { - struct sdw_slave *slave = dev_to_sdw_dev(dev); + struct sdw_slave *slave; char modalias[32];
+ if (is_sdw_slave(dev)) { + slave = to_sdw_slave_device(dev); + } else { + dev_warn(dev, "uevent for unknown Soundwire type\n"); + return -EINVAL; + } + sdw_slave_modalias(slave, modalias, sizeof(modalias));
if (add_uevent_var(env, "MODALIAS=%s", modalias)) diff --git a/drivers/soundwire/slave.c b/drivers/soundwire/slave.c index 40e41796a499..89854ae8414f 100644 --- a/drivers/soundwire/slave.c +++ b/drivers/soundwire/slave.c @@ -14,6 +14,11 @@ static void sdw_slave_release(struct device *dev) kfree(slave); }
+struct device_type sdw_slave_type = { + .name = "sdw_slave", + .release = sdw_slave_release, +}; + static int sdw_slave_add(struct sdw_bus *bus, struct sdw_slave_id *id, struct fwnode_handle *fwnode) { @@ -34,9 +39,9 @@ static int sdw_slave_add(struct sdw_bus *bus, bus->link_id, id->mfg_id, id->part_id, id->class_id, id->unique_id);
- slave->dev.release = sdw_slave_release; slave->dev.bus = &sdw_bus_type; slave->dev.of_node = of_node_get(to_of_node(fwnode)); + slave->dev.type = &sdw_slave_type; slave->bus = bus; slave->status = SDW_SLAVE_UNATTACHED; slave->dev_num = 0; diff --git a/include/linux/soundwire/sdw_type.h b/include/linux/soundwire/sdw_type.h index 7d4bc6a979bf..c681b3426478 100644 --- a/include/linux/soundwire/sdw_type.h +++ b/include/linux/soundwire/sdw_type.h @@ -5,6 +5,12 @@ #define __SOUNDWIRE_TYPES_H
extern struct bus_type sdw_bus_type; +extern struct device_type sdw_slave_type; + +static inline int is_sdw_slave(const struct device *dev) +{ + return dev->type == &sdw_slave_type; +}
#define to_sdw_slave_driver(_drv) \ container_of(_drv, struct sdw_driver, driver)
Since we want an explicit support for the SoundWire Master device, add the definitions, following the Grey Bus example.
Open: do we need to set a variable when dealing with the master uevent?
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- drivers/soundwire/Makefile | 2 +- drivers/soundwire/bus_type.c | 16 +++--- drivers/soundwire/master.c | 78 ++++++++++++++++++++++++++++++ include/linux/soundwire/sdw.h | 35 ++++++++++++++ include/linux/soundwire/sdw_type.h | 9 ++++ 5 files changed, 133 insertions(+), 7 deletions(-) create mode 100644 drivers/soundwire/master.c
diff --git a/drivers/soundwire/Makefile b/drivers/soundwire/Makefile index 563894e5ecaf..89b29819dd3a 100644 --- a/drivers/soundwire/Makefile +++ b/drivers/soundwire/Makefile @@ -4,7 +4,7 @@ #
#Bus Objs -soundwire-bus-objs := bus_type.o bus.o slave.o mipi_disco.o stream.o +soundwire-bus-objs := bus_type.o bus.o master.o slave.o mipi_disco.o stream.o obj-$(CONFIG_SOUNDWIRE) += soundwire-bus.o
ifdef CONFIG_DEBUG_FS diff --git a/drivers/soundwire/bus_type.c b/drivers/soundwire/bus_type.c index 5df095f4e12f..cf33f63773f0 100644 --- a/drivers/soundwire/bus_type.c +++ b/drivers/soundwire/bus_type.c @@ -49,21 +49,25 @@ int sdw_slave_modalias(const struct sdw_slave *slave, char *buf, size_t size)
static int sdw_uevent(struct device *dev, struct kobj_uevent_env *env) { + struct sdw_master_device *md; struct sdw_slave *slave; char modalias[32];
- if (is_sdw_slave(dev)) { + if (is_sdw_md(dev)) { + md = to_sdw_master_device(dev); + /* TODO: do we need to call add_uevent_var() ? */ + } else if (is_sdw_slave(dev)) { slave = to_sdw_slave_device(dev); + + sdw_slave_modalias(slave, modalias, sizeof(modalias)); + + if (add_uevent_var(env, "MODALIAS=%s", modalias)) + return -ENOMEM; } else { dev_warn(dev, "uevent for unknown Soundwire type\n"); return -EINVAL; }
- sdw_slave_modalias(slave, modalias, sizeof(modalias)); - - if (add_uevent_var(env, "MODALIAS=%s", modalias)) - return -ENOMEM; - return 0; }
diff --git a/drivers/soundwire/master.c b/drivers/soundwire/master.c new file mode 100644 index 000000000000..d9d09759b9c3 --- /dev/null +++ b/drivers/soundwire/master.c @@ -0,0 +1,78 @@ +// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) +// Copyright(c) 2019 Intel Corporation. + +#include <linux/device.h> +#include <linux/acpi.h> +#include <linux/soundwire/sdw.h> +#include <linux/soundwire/sdw_type.h> +#include "bus.h" + +static ssize_t bus_id_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct sdw_master_device *md = to_sdw_master_device(dev); + + return sprintf(buf, "%d\n", md->link_id); +} +static DEVICE_ATTR_RO(bus_id); + +static struct attribute *bus_attrs[] = { + &dev_attr_bus_id.attr, + NULL +}; +ATTRIBUTE_GROUPS(bus); + +static void sdw_md_release(struct device *dev) +{ + struct sdw_master_device *md = to_sdw_master_device(dev); + + kfree(md); +} + +struct device_type sdw_md_type = { + .name = "soundwire_master", + .release = sdw_md_release, +}; + +struct sdw_master_device *sdw_md_add(struct sdw_md_driver *driver, + struct device *parent, + struct fwnode_handle *fwnode, + int link_id) +{ + struct sdw_master_device *md; + int ret; + + if (!driver->probe) { + dev_err(parent, "mandatory probe callback missing\n"); + return ERR_PTR(-EINVAL); + } + + md = kzalloc(sizeof(*md), GFP_KERNEL); + if (!md) + return ERR_PTR(-ENOMEM); + + md->link_id = link_id; + + md->driver = driver; + + md->dev.parent = parent; + md->dev.fwnode = fwnode; + md->dev.bus = &sdw_bus_type; + md->dev.type = &sdw_md_type; + md->dev.groups = bus_groups; + md->dev.dma_mask = md->dev.parent->dma_mask; + dev_set_name(&md->dev, "sdw-master-%d", md->link_id); + + ret = device_register(&md->dev); + if (ret) { + dev_err(parent, "Failed to add master: ret %d\n", ret); + /* + * On err, don't free but drop ref as this will be freed + * when release method is invoked. + */ + put_device(&md->dev); + } + + return md; +} +EXPORT_SYMBOL(sdw_md_add); diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h index f004de91ce24..6289924b0336 100644 --- a/include/linux/soundwire/sdw.h +++ b/include/linux/soundwire/sdw.h @@ -564,6 +564,16 @@ struct sdw_slave { #define to_sdw_slave_device(d) \ container_of(d, struct sdw_slave, dev)
+struct sdw_master_device { + struct device dev; + int link_id; + struct sdw_md_driver *driver; + void *pdata; /* core does not touch */ +}; + +#define to_sdw_master_device(d) \ + container_of(d, struct sdw_master_device, dev) + struct sdw_driver { const char *name;
@@ -578,6 +588,26 @@ struct sdw_driver { struct device_driver driver; };
+struct sdw_md_driver { + /* initializations and allocations */ + int (*probe)(struct sdw_master_device *md, void *link_ctx); + /* hardware enablement, all clock/power dependencies are available */ + int (*startup)(struct sdw_master_device *md); + /* hardware disabled */ + int (*shutdown)(struct sdw_master_device *md); + /* free all resources */ + int (*remove)(struct sdw_master_device *md); + /* + * enable/disable driver control while in clock-stop mode, + * typically in always-on/D0ix modes. When the driver yields + * control, another entity in the system (typically firmware + * running on an always-on microprocessor) is responsible to + * tracking Slave-initiated wakes + */ + int (*autonomous_clock_stop_enable)(struct sdw_master_device *md, + bool state); +}; + #define SDW_SLAVE_ENTRY(_mfg_id, _part_id, _drv_data) \ { .mfg_id = (_mfg_id), .part_id = (_part_id), \ .driver_data = (unsigned long)(_drv_data) } @@ -767,6 +797,11 @@ struct sdw_bus { int sdw_add_bus_master(struct sdw_bus *bus); void sdw_delete_bus_master(struct sdw_bus *bus);
+struct sdw_master_device *sdw_md_add(struct sdw_md_driver *driver, + struct device *parent, + struct fwnode_handle *fwnode, + int link_id); + /** * sdw_port_config: Master or Slave Port configuration * diff --git a/include/linux/soundwire/sdw_type.h b/include/linux/soundwire/sdw_type.h index c681b3426478..463d6d018d56 100644 --- a/include/linux/soundwire/sdw_type.h +++ b/include/linux/soundwire/sdw_type.h @@ -6,15 +6,24 @@
extern struct bus_type sdw_bus_type; extern struct device_type sdw_slave_type; +extern struct device_type sdw_md_type;
static inline int is_sdw_slave(const struct device *dev) { return dev->type == &sdw_slave_type; }
+static inline int is_sdw_md(const struct device *dev) +{ + return dev->type == &sdw_md_type; +} + #define to_sdw_slave_driver(_drv) \ container_of(_drv, struct sdw_driver, driver)
+#define to_sdw_md_driver(_drv) \ + container_of(_drv, struct sdw_md_driver, driver) + #define sdw_register_slave_driver(drv) \ __sdw_register_slave_driver(drv, THIS_MODULE)
Use sdw_master_device and driver instead of platform devices
To quote GregKH:
"Don't mess with a platform device unless you really have no other possible choice. And even then, don't do it and try to do something else. Platform devices are really abused, don't perpetuate it "
In addition, rather than a plain-vanilla init/exit, this patch provides 3 steps in the initialization (ACPI scan, probe, startup) which make it easier to verify support and allocate required resources as early as possible, and conversely help make the startup lighter-weight with only hardware register setup.
The data structures are also consolidated in a single file and comments added to help follow what is used for what.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- drivers/soundwire/intel.c | 90 +++++---- drivers/soundwire/intel.h | 22 +-- drivers/soundwire/intel_init.c | 293 +++++++++++++++++++--------- include/linux/soundwire/sdw_intel.h | 86 +++++++- 4 files changed, 344 insertions(+), 147 deletions(-)
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index 8a1f6c627788..267e0fad7494 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -92,8 +92,6 @@ #define SDW_ALH_STRMZCFG_DMAT GENMASK(7, 0) #define SDW_ALH_STRMZCFG_CHN GENMASK(19, 16)
-#define SDW_INTEL_QUIRK_MASK_BUS_DISABLE BIT(1) - enum intel_pdi_type { INTEL_PDI_IN = 0, INTEL_PDI_OUT = 1, @@ -909,24 +907,23 @@ static int intel_init(struct sdw_intel *sdw) /* * probe and init */ -static int intel_probe(struct platform_device *pdev) +static int intel_master_probe(struct sdw_master_device *md, void *link_ctx) { - struct sdw_cdns_stream_config config; struct sdw_intel *sdw; int ret;
- sdw = devm_kzalloc(&pdev->dev, sizeof(*sdw), GFP_KERNEL); + sdw = devm_kzalloc(&md->dev, sizeof(*sdw), GFP_KERNEL); if (!sdw) return -ENOMEM;
- sdw->instance = pdev->id; - sdw->link_res = dev_get_platdata(&pdev->dev); - sdw->cdns.dev = &pdev->dev; + sdw->instance = md->link_id; + sdw->link_res = link_ctx; + sdw->cdns.dev = &md->dev; sdw->cdns.registers = sdw->link_res->registers; - sdw->cdns.instance = sdw->instance; + sdw->cdns.instance = md->link_id; sdw->cdns.msg_count = 0; - sdw->cdns.bus.dev = &pdev->dev; - sdw->cdns.bus.link_id = pdev->id; + sdw->cdns.bus.dev = &md->dev; + sdw->cdns.bus.link_id = md->link_id;
sdw_cdns_probe(&sdw->cdns);
@@ -934,16 +931,50 @@ static int intel_probe(struct platform_device *pdev) sdw_intel_ops.read_prop = intel_prop_read; sdw->cdns.bus.ops = &sdw_intel_ops;
- platform_set_drvdata(pdev, sdw); + md->pdata = sdw; + + /* set driver data, accessed by snd_soc_dai_set_drvdata() */ + dev_set_drvdata(&md->dev, &sdw->cdns);
ret = sdw_add_bus_master(&sdw->cdns.bus); if (ret) { - dev_err(&pdev->dev, "sdw_add_bus_master fail: %d\n", ret); + dev_err(&md->dev, "sdw_add_bus_master fail: %d\n", ret); return ret; }
if (sdw->cdns.bus.prop.hw_disabled) { - dev_info(&pdev->dev, "SoundWire master %d is disabled, ignoring\n", + dev_info(&md->dev, "SoundWire master %d is disabled, ignoring\n", + sdw->cdns.bus.link_id); + return 0; + } + + /* Acquire IRQ */ + ret = request_threaded_irq(sdw->link_res->irq, + sdw_cdns_irq, sdw_cdns_thread, + IRQF_SHARED, KBUILD_MODNAME, &sdw->cdns); + if (ret < 0) { + dev_err(sdw->cdns.dev, "unable to grab IRQ %d, disabling device\n", + sdw->link_res->irq); + goto err_init; + } + + return 0; + +err_init: + sdw_delete_bus_master(&sdw->cdns.bus); + return ret; +} + +static int intel_master_startup(struct sdw_master_device *md) +{ + struct sdw_cdns_stream_config config; + struct sdw_intel *sdw; + int ret; + + sdw = md->pdata; + + if (sdw->cdns.bus.prop.hw_disabled) { + dev_info(&md->dev, "SoundWire master %d is disabled, ignoring\n", sdw->cdns.bus.link_id); return 0; } @@ -961,16 +992,6 @@ static int intel_probe(struct platform_device *pdev)
intel_pdi_ch_update(sdw);
- /* Acquire IRQ */ - ret = request_threaded_irq(sdw->link_res->irq, - sdw_cdns_irq, sdw_cdns_thread, - IRQF_SHARED, KBUILD_MODNAME, &sdw->cdns); - if (ret < 0) { - dev_err(sdw->cdns.dev, "unable to grab IRQ %d, disabling device\n", - sdw->link_res->irq); - goto err_init; - } - ret = sdw_cdns_enable_interrupt(&sdw->cdns, true); if (ret < 0) { dev_err(sdw->cdns.dev, "cannot enable interrupts\n"); @@ -997,18 +1018,17 @@ static int intel_probe(struct platform_device *pdev)
err_interrupt: sdw_cdns_enable_interrupt(&sdw->cdns, false); -err_dai: - free_irq(sdw->link_res->irq, sdw); err_init: + free_irq(sdw->link_res->irq, sdw); sdw_delete_bus_master(&sdw->cdns.bus); return ret; }
-static int intel_remove(struct platform_device *pdev) +static int intel_master_remove(struct sdw_master_device *md) { struct sdw_intel *sdw;
- sdw = platform_get_drvdata(pdev); + sdw = md->pdata;
if (!sdw->cdns.bus.prop.hw_disabled) { intel_debugfs_exit(sdw); @@ -1021,16 +1041,12 @@ static int intel_remove(struct platform_device *pdev) return 0; }
-static struct platform_driver sdw_intel_drv = { - .probe = intel_probe, - .remove = intel_remove, - .driver = { - .name = "int-sdw", - - }, +struct sdw_md_driver intel_sdw_driver = { + .probe = intel_master_probe, + .startup = intel_master_startup, + .remove = intel_master_remove, }; - -module_platform_driver(sdw_intel_drv); +EXPORT_SYMBOL(intel_sdw_driver);
MODULE_LICENSE("Dual BSD/GPL"); MODULE_ALIAS("platform:int-sdw"); diff --git a/drivers/soundwire/intel.h b/drivers/soundwire/intel.h index d923b6262330..25cc51e15ef5 100644 --- a/drivers/soundwire/intel.h +++ b/drivers/soundwire/intel.h @@ -4,24 +4,8 @@ #ifndef __SDW_INTEL_LOCAL_H #define __SDW_INTEL_LOCAL_H
-/** - * struct sdw_intel_link_res - Soundwire link resources - * @registers: Link IO registers base - * @shim: Audio shim pointer - * @alh: ALH (Audio Link Hub) pointer - * @irq: Interrupt line - * @ops: Shim callback ops - * @arg: Shim callback ops argument - * - * This is set as pdata for each link instance. - */ -struct sdw_intel_link_res { - void __iomem *registers; - void __iomem *shim; - void __iomem *alh; - int irq; - const struct sdw_intel_ops *ops; - void *arg; -}; +#define SDW_INTEL_QUIRK_MASK_BUS_DISABLE BIT(1) + +extern struct sdw_md_driver intel_sdw_driver;
#endif /* __SDW_INTEL_LOCAL_H */ diff --git a/drivers/soundwire/intel_init.c b/drivers/soundwire/intel_init.c index d488c44fcbae..47124fc13a4a 100644 --- a/drivers/soundwire/intel_init.c +++ b/drivers/soundwire/intel_init.c @@ -11,7 +11,7 @@ #include <linux/export.h> #include <linux/iomap.h> #include <linux/module.h> -#include <linux/platform_device.h> +#include <linux/soundwire/sdw.h> #include <linux/soundwire/sdw_intel.h> #include "intel.h"
@@ -27,28 +27,45 @@ static int link_mask; module_param_named(sdw_link_mask, link_mask, int, 0444); MODULE_PARM_DESC(sdw_link_mask, "Intel link mask (one bit per link)");
-struct sdw_link_data { - struct sdw_intel_link_res res; - struct platform_device *pdev; -}; +static bool is_link_enabled(struct fwnode_handle *fw_node, int i) +{ + struct fwnode_handle *link; + char name[32]; + u32 quirk_mask = 0;
-struct sdw_intel_ctx { - int count; - struct sdw_link_data *links; -}; + /* Find master handle */ + snprintf(name, sizeof(name), + "mipi-sdw-link-%d-subproperties", i); + + link = fwnode_get_named_child_node(fw_node, name); + if (!link) + return false;
-static int sdw_intel_cleanup_pdev(struct sdw_intel_ctx *ctx) + fwnode_property_read_u32(link, + "intel-quirk-mask", + &quirk_mask); + + if (quirk_mask & SDW_INTEL_QUIRK_MASK_BUS_DISABLE) + return false; + + return true; +} + +static int sdw_intel_cleanup(struct sdw_intel_ctx *ctx) { - struct sdw_link_data *link = ctx->links; + struct sdw_intel_link_res *link = ctx->links; + struct sdw_master_device *md; int i;
if (!link) return 0;
- for (i = 0; i < ctx->count; i++) { - if (link->pdev) - platform_device_unregister(link->pdev); - link++; + for (i = 0; i < ctx->count; i++, link++) { + md = link->md; + if (md) { + md->driver->remove(md); + put_device(&md->dev); + } }
kfree(ctx->links); @@ -57,115 +74,187 @@ static int sdw_intel_cleanup_pdev(struct sdw_intel_ctx *ctx) return 0; }
-static struct sdw_intel_ctx -*sdw_intel_add_controller(struct sdw_intel_res *res) +static int +sdw_intel_scan_controller(struct sdw_intel_acpi_info *info) { - struct platform_device_info pdevinfo; - struct platform_device *pdev; - struct sdw_link_data *link; - struct sdw_intel_ctx *ctx; struct acpi_device *adev; int ret, i; u8 count; - u32 caps;
- if (acpi_bus_get_device(res->handle, &adev)) - return NULL; + if (acpi_bus_get_device(info->handle, &adev)) + return -EINVAL;
/* Found controller, find links supported */ count = 0; ret = fwnode_property_read_u8_array(acpi_fwnode_handle(adev), "mipi-sdw-master-count", &count, 1);
- /* Don't fail on error, continue and use hw value */ + /* + * In theory we could check the number of links supported in + * hardware, but in that step we cannot assume SoundWire IP is + * powered. + * + * In addition, if the BIOS doesn't even provide this + * 'master-count' property then all the inits based on link + * masks will fail as well. + * + * We will check the hardware capabilities in the startup() step + */ + if (ret) { dev_err(&adev->dev, "Failed to read mipi-sdw-master-count: %d\n", ret); - count = SDW_MAX_LINKS; + return -EINVAL; }
- /* Check SNDWLCAP.LCOUNT */ - caps = ioread32(res->mmio_base + SDW_SHIM_BASE + SDW_SHIM_LCAP); - caps &= GENMASK(2, 0); - - /* Check HW supported vs property value and use min of two */ - count = min_t(u8, caps, count); - /* Check count is within bounds */ if (count > SDW_MAX_LINKS) { dev_err(&adev->dev, "Link count %d exceeds max %d\n", count, SDW_MAX_LINKS); - return NULL; + return -EINVAL; } else if (!count) { dev_warn(&adev->dev, "No SoundWire links detected\n"); - return NULL; + return -EINVAL; } + dev_dbg(&adev->dev, "Detected %d SDW Link devices\n", count);
+ info->count = count; + + for (i = 0; i < count; i++) { + if (link_mask && !(link_mask & BIT(i))) { + dev_dbg(&adev->dev, + "Link %d masked, will not be enabled\n", i); + continue; + } + + if (!is_link_enabled(acpi_fwnode_handle(adev), i)) + continue; + + info->link_mask |= BIT(i); + } + + return 0; +} + +static struct sdw_intel_ctx +*sdw_intel_probe_controller(struct sdw_intel_res *res) +{ + struct sdw_intel_link_res *link; + struct sdw_intel_ctx *ctx; + struct acpi_device *adev; + struct sdw_master_device *md; + u32 link_mask; + int count; + int i; + + if (acpi_bus_get_device(res->handle, &adev)) + return NULL; + + if (!res || !res->count) + return NULL; + + count = res->count; dev_dbg(&adev->dev, "Creating %d SDW Link devices\n", count);
ctx = kzalloc(sizeof(*ctx), GFP_KERNEL); if (!ctx) return NULL;
- ctx->count = count; - ctx->links = kcalloc(ctx->count, sizeof(*ctx->links), GFP_KERNEL); + ctx->links = kcalloc(count, sizeof(*ctx->links), GFP_KERNEL); if (!ctx->links) goto link_err;
+ ctx->count = count; + ctx->mmio_base = res->mmio_base; + ctx->link_mask = res->link_mask; + ctx->handle = res->handle; + link = ctx->links; + link_mask = ctx->link_mask;
/* Create SDW Master devices */ - for (i = 0; i < count; i++) { - if (link_mask && !(link_mask & BIT(i))) { - dev_dbg(&adev->dev, - "Link %d masked, will not be enabled\n", i); - link++; + for (i = 0; i < count; i++, link++) { + if (link_mask && !(link_mask & BIT(i))) continue; - }
- link->res.irq = res->irq; - link->res.registers = res->mmio_base + SDW_LINK_BASE - + (SDW_LINK_SIZE * i); - link->res.shim = res->mmio_base + SDW_SHIM_BASE; - link->res.alh = res->mmio_base + SDW_ALH_BASE; - - link->res.ops = res->ops; - link->res.arg = res->arg; - - memset(&pdevinfo, 0, sizeof(pdevinfo)); - - pdevinfo.parent = res->parent; - pdevinfo.name = "int-sdw"; - pdevinfo.id = i; - pdevinfo.fwnode = acpi_fwnode_handle(adev); - pdevinfo.data = &link->res; - pdevinfo.size_data = sizeof(link->res); - - pdev = platform_device_register_full(&pdevinfo); - if (IS_ERR(pdev)) { - dev_err(&adev->dev, - "platform device creation failed: %ld\n", - PTR_ERR(pdev)); - goto pdev_err; - } + md = sdw_md_add(&intel_sdw_driver, + &adev->dev, + acpi_fwnode_handle(adev), + i);
- link->pdev = pdev; - link++; + if (IS_ERR(md)) { + dev_err(&adev->dev, "Could not create link %d\n", i); + goto err; + } + link->md = md; + link->mmio_base = res->mmio_base; + link->registers = res->mmio_base + SDW_LINK_BASE + + (SDW_LINK_SIZE * i); + link->shim = res->mmio_base + SDW_SHIM_BASE; + link->alh = res->mmio_base + SDW_ALH_BASE; + link->irq = res->irq; + link->ops = res->ops; + link->arg = res->arg; + + /* let the SoundWire master driver to its probe */ + md->driver->probe(md, link); }
return ctx;
-pdev_err: - sdw_intel_cleanup_pdev(ctx); +err: + sdw_intel_cleanup(ctx); link_err: kfree(ctx); return NULL; }
+static int +sdw_intel_startup_controller(struct sdw_intel_ctx *ctx) +{ + struct acpi_device *adev; + struct sdw_intel_link_res *link; + struct sdw_master_device *md; + u32 caps; + int i; + + if (acpi_bus_get_device(ctx->handle, &adev)) + return -EINVAL; + + /* Check SNDWLCAP.LCOUNT */ + caps = ioread32(ctx->mmio_base + SDW_SHIM_BASE + SDW_SHIM_LCAP); + caps &= GENMASK(2, 0); + + /* Check HW supported vs property value */ + if (caps < ctx->count) { + dev_err(&adev->dev, + "BIOS master count is larger than hardware capabilities\n"); + return -EINVAL; + } + + if (!ctx->links) + return -EINVAL; + + link = ctx->links; + link_mask = ctx->link_mask; + + /* Create SDW Master devices */ + for (i = 0; i < ctx->count; i++, link++) { + if (link_mask && !(link_mask & BIT(i))) + continue; + + md = link->md; + + md->driver->startup(md); + } + + return 0; +} + static acpi_status sdw_intel_acpi_cb(acpi_handle handle, u32 level, void *cdata, void **return_value) { - struct sdw_intel_res *res = cdata; + struct sdw_intel_acpi_info *info = cdata; struct acpi_device *adev; acpi_status status; u64 adr; @@ -179,7 +268,7 @@ static acpi_status sdw_intel_acpi_cb(acpi_handle handle, u32 level, return AE_NOT_FOUND; }
- res->handle = handle; + info->handle = handle;
/* * On some Intel platforms, multiple children of the HDAS @@ -196,40 +285,68 @@ static acpi_status sdw_intel_acpi_cb(acpi_handle handle, u32 level, }
/** - * sdw_intel_init() - SoundWire Intel init routine + * sdw_intel_acpi_scan() - SoundWire Intel init routine * @parent_handle: ACPI parent handle - * @res: resource data + * @info: description of what firmware/DSDT tables expose * - * This scans the namespace and creates SoundWire link controller devices - * based on the info queried. + * This scans the namespace and queries firmware to figure out which + * links to enable. A follow-up use of sdw_intel_probe() and + * sdw_intel_startup() is required for creation of devices and bus + * startup */ -void *sdw_intel_init(acpi_handle *parent_handle, struct sdw_intel_res *res) +int sdw_intel_acpi_scan(acpi_handle *parent_handle, + struct sdw_intel_acpi_info *info) { acpi_status status;
status = acpi_walk_namespace(ACPI_TYPE_DEVICE, parent_handle, 1, sdw_intel_acpi_cb, - NULL, res, NULL); + NULL, info, NULL); if (ACPI_FAILURE(status)) - return NULL; + return -ENODEV;
- return sdw_intel_add_controller(res); + return sdw_intel_scan_controller(info); } -EXPORT_SYMBOL(sdw_intel_init); +EXPORT_SYMBOL(sdw_intel_acpi_scan);
+/** + * sdw_intel_probe() - SoundWire Intel probe routine + * @parent_handle: ACPI parent handle + * @res: resource data + * + * This creates SoundWire Master and Slave devices below the controller. + * All the information necessary is stored in the context, and the res + * argument pointer can be freed after this step. + */ +struct sdw_intel_ctx +*sdw_intel_probe(struct sdw_intel_res *res) +{ + return sdw_intel_probe_controller(res); +} +EXPORT_SYMBOL(sdw_intel_probe); + +/** + * sdw_intel_startup() - SoundWire Intel startup + * @ctx: SoundWire context allocated in the probe + * + */ +int sdw_intel_startup(struct sdw_intel_ctx *ctx) +{ + return sdw_intel_startup_controller(ctx); +} +EXPORT_SYMBOL(sdw_intel_startup); /** * sdw_intel_exit() - SoundWire Intel exit - * @arg: callback context + * @ctx: SoundWire context allocated in the probe * * Delete the controller instances created and cleanup */ -void sdw_intel_exit(void *arg) +void sdw_intel_exit(struct sdw_intel_ctx *ctx) { - struct sdw_intel_ctx *ctx = arg; - - sdw_intel_cleanup_pdev(ctx); + sdw_intel_cleanup(ctx); kfree(ctx); + ctx = NULL; } EXPORT_SYMBOL(sdw_intel_exit);
diff --git a/include/linux/soundwire/sdw_intel.h b/include/linux/soundwire/sdw_intel.h index c9427cb6020b..0ce3e4023074 100644 --- a/include/linux/soundwire/sdw_intel.h +++ b/include/linux/soundwire/sdw_intel.h @@ -16,24 +16,104 @@ struct sdw_intel_ops { };
/** - * struct sdw_intel_res - Soundwire Intel resource structure + * struct sdw_intel_acpi_info - Soundwire Intel information found in ACPI tables + * @handle: ACPI controller handle + * @count: link count found with "sdw-master-count" property + * @link_mask: bit-wise mask listing links enabled by BIOS menu + * + * this structure could be expanded to e.g. provide all the _ADR + * information in case the link_mask is not sufficient to identify + * platform capabilities. + */ +struct sdw_intel_acpi_info { + acpi_handle handle; + int count; + u32 link_mask; +}; + +/** + * struct sdw_intel_res - Soundwire Intel global resource structure, + * typically populated by the DSP driver + * + * @count: link count (may be filtered by DSP driver) * @mmio_base: mmio base of SoundWire registers * @irq: interrupt number * @handle: ACPI parent handle * @parent: parent device * @ops: callback ops * @arg: callback arg + * @link_mask: bit-wise mask listing links selected by the DSP driver */ struct sdw_intel_res { + int count; void __iomem *mmio_base; int irq; acpi_handle handle; struct device *parent; const struct sdw_intel_ops *ops; void *arg; + u32 link_mask; };
-void *sdw_intel_init(acpi_handle *parent_handle, struct sdw_intel_res *res); -void sdw_intel_exit(void *arg); +/** + * struct sdw_intel_link_res - Soundwire Intel link resource structure, + * typically populated by the controller driver. + * @md: Master device + * @mmio_base: mmio base of SoundWire registers + * @registers: Link IO registers base + * @shim: Audio shim pointer + * @alh: ALH (Audio Link Hub) pointer + * @irq: Interrupt line + * @ops: Shim callback ops + * @arg: Shim callback ops argument + */ +struct sdw_intel_link_res { + struct sdw_master_device *md; + void __iomem *mmio_base; /* not strictly needed, useful for debug */ + void __iomem *registers; + void __iomem *shim; + void __iomem *alh; + int irq; + const struct sdw_intel_ops *ops; + void *arg; +}; + +/** + * struct sdw_intel_ctx - context allocated by the controller + * driver probe + * @count: link count + * @mmio_base: mmio base of SoundWire registers, only used to check + * hardware capabilities after all power dependencies are settled. + * @arg: Shim callback ops argument + */ +struct sdw_intel_ctx { + int count; + void __iomem *mmio_base; + u32 link_mask; + acpi_handle handle; + struct sdw_intel_link_res *links; +}; + +/* + * On Intel platforms, the SoundWire IP has dependencies on power + * rails shared with the DSP, and the initialization steps are split + * in three. First an ACPI scan to check what the firmware describes + * in DSDT tables, then an allocation step (with no hardware + * configuration but with all the relevant devices created) and last + * the actual hardware configuration. The final stage is a global + * interrupt enable which is controlled by the DSP driver. Splitting + * these phases helps simplify the boot flow and make early decisions + * on e.g. which machine driver to select (I2S mode, HDaudio or + * SoundWire). + */ +int sdw_intel_acpi_scan(acpi_handle *parent_handle, + struct sdw_intel_acpi_info *info); + +struct sdw_intel_ctx * +sdw_intel_probe(struct sdw_intel_res *res); + +int sdw_intel_startup(struct sdw_intel_ctx *ctx); + +void sdw_intel_exit(struct sdw_intel_ctx *ctx);
#endif
On Mon, Sep 16, 2019 at 04:23:41PM -0500, Pierre-Louis Bossart wrote:
+/**
- sdw_intel_probe() - SoundWire Intel probe routine
- @parent_handle: ACPI parent handle
- @res: resource data
- This creates SoundWire Master and Slave devices below the controller.
- All the information necessary is stored in the context, and the res
- argument pointer can be freed after this step.
- */
+struct sdw_intel_ctx +*sdw_intel_probe(struct sdw_intel_res *res) +{
- return sdw_intel_probe_controller(res);
+} +EXPORT_SYMBOL(sdw_intel_probe);
+/**
- sdw_intel_startup() - SoundWire Intel startup
- @ctx: SoundWire context allocated in the probe
- */
+int sdw_intel_startup(struct sdw_intel_ctx *ctx) +{
- return sdw_intel_startup_controller(ctx);
+} +EXPORT_SYMBOL(sdw_intel_startup);
Why are you exporting these functions if no one calls them?
thanks,
greg k-h
On 9/17/19 12:55 AM, Greg KH wrote:
On Mon, Sep 16, 2019 at 04:23:41PM -0500, Pierre-Louis Bossart wrote:
+/**
- sdw_intel_probe() - SoundWire Intel probe routine
- @parent_handle: ACPI parent handle
- @res: resource data
- This creates SoundWire Master and Slave devices below the controller.
- All the information necessary is stored in the context, and the res
- argument pointer can be freed after this step.
- */
+struct sdw_intel_ctx +*sdw_intel_probe(struct sdw_intel_res *res) +{
- return sdw_intel_probe_controller(res);
+} +EXPORT_SYMBOL(sdw_intel_probe);
+/**
- sdw_intel_startup() - SoundWire Intel startup
- @ctx: SoundWire context allocated in the probe
- */
+int sdw_intel_startup(struct sdw_intel_ctx *ctx) +{
- return sdw_intel_startup_controller(ctx);
+} +EXPORT_SYMBOL(sdw_intel_startup);
Why are you exporting these functions if no one calls them?
They are used in the next series, see '[RFC PATCH 04/12] ASoC: SOF: Intel: add SoundWire configuration interface'
+int hda_sdw_startup(struct snd_sof_dev *sdev) +{ + struct sof_intel_hda_dev *hdev; + int ret; + + hdev = sdev->pdata->hw_pdata; + + ret = sdw_intel_startup(hdev->sdw); + if (ret < 0) + return ret; + hda_sdw_int_enable(sdev, true); + + return ret; +}
These 4 functions sdw_intel_acpi_scan, sdw_intel_probe, sdw_intel_startup and sdw_intel_exit are the interface between the ASoC world and the Soundwire/Intel module.
I split the patches in two series to make the review and integration easier on maintainers. The first one is strictly contained within the driver/soundwire directory while will impact the soundwire and ASoC trees.
On Tue, Sep 17, 2019 at 09:29:52AM -0500, Pierre-Louis Bossart wrote:
On 9/17/19 12:55 AM, Greg KH wrote:
On Mon, Sep 16, 2019 at 04:23:41PM -0500, Pierre-Louis Bossart wrote:
+/**
- sdw_intel_probe() - SoundWire Intel probe routine
- @parent_handle: ACPI parent handle
- @res: resource data
- This creates SoundWire Master and Slave devices below the controller.
- All the information necessary is stored in the context, and the res
- argument pointer can be freed after this step.
- */
+struct sdw_intel_ctx +*sdw_intel_probe(struct sdw_intel_res *res) +{
- return sdw_intel_probe_controller(res);
+} +EXPORT_SYMBOL(sdw_intel_probe);
+/**
- sdw_intel_startup() - SoundWire Intel startup
- @ctx: SoundWire context allocated in the probe
- */
+int sdw_intel_startup(struct sdw_intel_ctx *ctx) +{
- return sdw_intel_startup_controller(ctx);
+} +EXPORT_SYMBOL(sdw_intel_startup);
Why are you exporting these functions if no one calls them?
They are used in the next series, see '[RFC PATCH 04/12] ASoC: SOF: Intel: add SoundWire configuration interface'
That wasn't obvious :)
Also, why not EXPORT_SYMBOL_GPL()? :)
thanks,
greg k-h
On 9/18/19 7:06 AM, Greg KH wrote:
On Tue, Sep 17, 2019 at 09:29:52AM -0500, Pierre-Louis Bossart wrote:
On 9/17/19 12:55 AM, Greg KH wrote:
On Mon, Sep 16, 2019 at 04:23:41PM -0500, Pierre-Louis Bossart wrote:
+/**
- sdw_intel_probe() - SoundWire Intel probe routine
- @parent_handle: ACPI parent handle
- @res: resource data
- This creates SoundWire Master and Slave devices below the controller.
- All the information necessary is stored in the context, and the res
- argument pointer can be freed after this step.
- */
+struct sdw_intel_ctx +*sdw_intel_probe(struct sdw_intel_res *res) +{
- return sdw_intel_probe_controller(res);
+} +EXPORT_SYMBOL(sdw_intel_probe);
+/**
- sdw_intel_startup() - SoundWire Intel startup
- @ctx: SoundWire context allocated in the probe
- */
+int sdw_intel_startup(struct sdw_intel_ctx *ctx) +{
- return sdw_intel_startup_controller(ctx);
+} +EXPORT_SYMBOL(sdw_intel_startup);
Why are you exporting these functions if no one calls them?
They are used in the next series, see '[RFC PATCH 04/12] ASoC: SOF: Intel: add SoundWire configuration interface'
That wasn't obvious :)
Also, why not EXPORT_SYMBOL_GPL()? :)
Since the beginning of this SoundWire work, the intent what that the code could be reused in non-GPL open-source circles, hence the dual license and EXPORT_SYMBOL. That said, there are cases where the code only makes sense for Linux, or relies on symbols that are exported with EXPORT_SYMBOL_GPL, in those cases we rely on GPLv2 and EXPORT_SYMBOL_GPL. For this series I added a disclaimer in the cover letter that those parts need to be reviewed further to make sure there are no conflicts with GPL. This is an RFC-level contribution to check if my understanding of the bus/device/driver model is aligned with recommendations. I've already made local improvements by fixing bisect issues, removing warnings, improved some sequences, and that GPL question will be revisited before I send a formal patch.
On Wed, Sep 18, 2019 at 08:48:33AM -0500, Pierre-Louis Bossart wrote:
On 9/18/19 7:06 AM, Greg KH wrote:
On Tue, Sep 17, 2019 at 09:29:52AM -0500, Pierre-Louis Bossart wrote:
On 9/17/19 12:55 AM, Greg KH wrote:
On Mon, Sep 16, 2019 at 04:23:41PM -0500, Pierre-Louis Bossart wrote:
+/**
- sdw_intel_probe() - SoundWire Intel probe routine
- @parent_handle: ACPI parent handle
- @res: resource data
- This creates SoundWire Master and Slave devices below the controller.
- All the information necessary is stored in the context, and the res
- argument pointer can be freed after this step.
- */
+struct sdw_intel_ctx +*sdw_intel_probe(struct sdw_intel_res *res) +{
- return sdw_intel_probe_controller(res);
+} +EXPORT_SYMBOL(sdw_intel_probe);
+/**
- sdw_intel_startup() - SoundWire Intel startup
- @ctx: SoundWire context allocated in the probe
- */
+int sdw_intel_startup(struct sdw_intel_ctx *ctx) +{
- return sdw_intel_startup_controller(ctx);
+} +EXPORT_SYMBOL(sdw_intel_startup);
Why are you exporting these functions if no one calls them?
They are used in the next series, see '[RFC PATCH 04/12] ASoC: SOF: Intel: add SoundWire configuration interface'
That wasn't obvious :)
Also, why not EXPORT_SYMBOL_GPL()? :)
Since the beginning of this SoundWire work, the intent what that the code could be reused in non-GPL open-source circles, hence the dual license and EXPORT_SYMBOL.
Hah, you _have_ talked to your lawyers about this, right?
You have a chance to do something like this for header files, for .c files, good luck. That's going to be a hard road to go down. Many have tried in the past, all but 1 have failed.
That said, there are cases where the code only makes sense for Linux, or relies on symbols that are exported with EXPORT_SYMBOL_GPL, in those cases we rely on GPLv2 and EXPORT_SYMBOL_GPL. For this series I added a disclaimer in the cover letter that those parts need to be reviewed further to make sure there are no conflicts with GPL.
Please do that with your lawyers, do not require developers to do legal work for you, that's just mean :(
thanks,
greg k-h
On Wed, Sep 18, 2019 at 03:53:02PM +0200, Greg KH wrote:
On Wed, Sep 18, 2019 at 08:48:33AM -0500, Pierre-Louis Bossart wrote:
On 9/18/19 7:06 AM, Greg KH wrote:
On Tue, Sep 17, 2019 at 09:29:52AM -0500, Pierre-Louis Bossart wrote:
On 9/17/19 12:55 AM, Greg KH wrote:
On Mon, Sep 16, 2019 at 04:23:41PM -0500, Pierre-Louis Bossart wrote:
+/**
- sdw_intel_probe() - SoundWire Intel probe routine
- @parent_handle: ACPI parent handle
- @res: resource data
- This creates SoundWire Master and Slave devices below the controller.
- All the information necessary is stored in the context, and the res
- argument pointer can be freed after this step.
- */
+struct sdw_intel_ctx +*sdw_intel_probe(struct sdw_intel_res *res) +{
- return sdw_intel_probe_controller(res);
+} +EXPORT_SYMBOL(sdw_intel_probe);
+/**
- sdw_intel_startup() - SoundWire Intel startup
- @ctx: SoundWire context allocated in the probe
- */
+int sdw_intel_startup(struct sdw_intel_ctx *ctx) +{
- return sdw_intel_startup_controller(ctx);
+} +EXPORT_SYMBOL(sdw_intel_startup);
Why are you exporting these functions if no one calls them?
They are used in the next series, see '[RFC PATCH 04/12] ASoC: SOF: Intel: add SoundWire configuration interface'
That wasn't obvious :)
Also, why not EXPORT_SYMBOL_GPL()? :)
Since the beginning of this SoundWire work, the intent what that the code could be reused in non-GPL open-source circles, hence the dual license and EXPORT_SYMBOL.
Hah, you _have_ talked to your lawyers about this, right?
You have a chance to do something like this for header files, for .c files, good luck. That's going to be a hard road to go down. Many have tried in the past, all but 1 have failed.
Also note, the last I checked, the _default_ license for Linux kernel code from Intel was GPLv2. If you got an exception for this, please work with your legal council on how to do this "properly" as that was part of getting that exception, right?
If you didn't get the exception, um, you have some people to go talk to, and how come I am the one asking you about this? :(
thanks,
greg k-h
On 9/18/19 8:54 AM, Greg KH wrote:
On Wed, Sep 18, 2019 at 03:53:02PM +0200, Greg KH wrote:
On Wed, Sep 18, 2019 at 08:48:33AM -0500, Pierre-Louis Bossart wrote:
On 9/18/19 7:06 AM, Greg KH wrote:
On Tue, Sep 17, 2019 at 09:29:52AM -0500, Pierre-Louis Bossart wrote:
On 9/17/19 12:55 AM, Greg KH wrote:
On Mon, Sep 16, 2019 at 04:23:41PM -0500, Pierre-Louis Bossart wrote: > +/** > + * sdw_intel_probe() - SoundWire Intel probe routine > + * @parent_handle: ACPI parent handle > + * @res: resource data > + * > + * This creates SoundWire Master and Slave devices below the controller. > + * All the information necessary is stored in the context, and the res > + * argument pointer can be freed after this step. > + */ > +struct sdw_intel_ctx > +*sdw_intel_probe(struct sdw_intel_res *res) > +{ > + return sdw_intel_probe_controller(res); > +} > +EXPORT_SYMBOL(sdw_intel_probe); > + > +/** > + * sdw_intel_startup() - SoundWire Intel startup > + * @ctx: SoundWire context allocated in the probe > + * > + */ > +int sdw_intel_startup(struct sdw_intel_ctx *ctx) > +{ > + return sdw_intel_startup_controller(ctx); > +} > +EXPORT_SYMBOL(sdw_intel_startup);
Why are you exporting these functions if no one calls them?
They are used in the next series, see '[RFC PATCH 04/12] ASoC: SOF: Intel: add SoundWire configuration interface'
That wasn't obvious :)
Also, why not EXPORT_SYMBOL_GPL()? :)
Since the beginning of this SoundWire work, the intent what that the code could be reused in non-GPL open-source circles, hence the dual license and EXPORT_SYMBOL.
Hah, you _have_ talked to your lawyers about this, right?
You have a chance to do something like this for header files, for .c files, good luck. That's going to be a hard road to go down. Many have tried in the past, all but 1 have failed.
Also note, the last I checked, the _default_ license for Linux kernel code from Intel was GPLv2. If you got an exception for this, please work with your legal council on how to do this "properly" as that was part of getting that exception, right?
If you didn't get the exception, um, you have some people to go talk to, and how come I am the one asking you about this? :(
All the legal due-diligence was done when SoundWire was initially contributed in 2018. You asked that question at the time and I will point you to the email exchange Alan Cox and you had on this topic [1].
On Wed, Sep 18, 2019 at 10:14:51AM -0500, Pierre-Louis Bossart wrote:
On 9/18/19 8:54 AM, Greg KH wrote:
On Wed, Sep 18, 2019 at 03:53:02PM +0200, Greg KH wrote:
On Wed, Sep 18, 2019 at 08:48:33AM -0500, Pierre-Louis Bossart wrote:
On 9/18/19 7:06 AM, Greg KH wrote:
On Tue, Sep 17, 2019 at 09:29:52AM -0500, Pierre-Louis Bossart wrote:
On 9/17/19 12:55 AM, Greg KH wrote: > On Mon, Sep 16, 2019 at 04:23:41PM -0500, Pierre-Louis Bossart wrote: > > +/** > > + * sdw_intel_probe() - SoundWire Intel probe routine > > + * @parent_handle: ACPI parent handle > > + * @res: resource data > > + * > > + * This creates SoundWire Master and Slave devices below the controller. > > + * All the information necessary is stored in the context, and the res > > + * argument pointer can be freed after this step. > > + */ > > +struct sdw_intel_ctx > > +*sdw_intel_probe(struct sdw_intel_res *res) > > +{ > > + return sdw_intel_probe_controller(res); > > +} > > +EXPORT_SYMBOL(sdw_intel_probe); > > + > > +/** > > + * sdw_intel_startup() - SoundWire Intel startup > > + * @ctx: SoundWire context allocated in the probe > > + * > > + */ > > +int sdw_intel_startup(struct sdw_intel_ctx *ctx) > > +{ > > + return sdw_intel_startup_controller(ctx); > > +} > > +EXPORT_SYMBOL(sdw_intel_startup); > > Why are you exporting these functions if no one calls them?
They are used in the next series, see '[RFC PATCH 04/12] ASoC: SOF: Intel: add SoundWire configuration interface'
That wasn't obvious :)
Also, why not EXPORT_SYMBOL_GPL()? :)
Since the beginning of this SoundWire work, the intent what that the code could be reused in non-GPL open-source circles, hence the dual license and EXPORT_SYMBOL.
Hah, you _have_ talked to your lawyers about this, right?
You have a chance to do something like this for header files, for .c files, good luck. That's going to be a hard road to go down. Many have tried in the past, all but 1 have failed.
Also note, the last I checked, the _default_ license for Linux kernel code from Intel was GPLv2. If you got an exception for this, please work with your legal council on how to do this "properly" as that was part of getting that exception, right?
If you didn't get the exception, um, you have some people to go talk to, and how come I am the one asking you about this? :(
All the legal due-diligence was done when SoundWire was initially contributed in 2018. You asked that question at the time and I will point you to the email exchange Alan Cox and you had on this topic [1].
Yes, that is fine, what I am saying here is that you are now asking the community to do this for you. You said in this thread:
For this series I added a disclaimer in the cover letter that those parts need to be reviewed further to make sure there are no conflicts with GPL.
Why are you sending code out that you think might have conflicts before your lawyers have reviewed it? That's just screaming for problems in the future (hint, you distributed something in the previous emails...)
Again, go and get this sorted out before dumping that kind of work on the community as this is something that you are having to deal with (i.e. it is self-inflicted). Don't make others do this for you here in public.
Otherwise I will probably just purposefully tell you the wrong thing, and then watch what kind of fun your lawyers will have :)
thanks,
greg k-h
From: Bard Liao yung-chuan.liao@linux.intel.com
Setting an device driver is necessary for ASoC to register DAI components.
Signed-off-by: Bard Liao yung-chuan.liao@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- drivers/soundwire/intel.c | 4 ++++ drivers/soundwire/master.c | 2 ++ include/linux/soundwire/sdw.h | 1 + 3 files changed, 7 insertions(+)
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index 267e0fad7494..c3dba6cf7730 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -1042,6 +1042,10 @@ static int intel_master_remove(struct sdw_master_device *md) }
struct sdw_md_driver intel_sdw_driver = { + .driver = { + .name = "intel-sdw", + .owner = THIS_MODULE, + }, .probe = intel_master_probe, .startup = intel_master_startup, .remove = intel_master_remove, diff --git a/drivers/soundwire/master.c b/drivers/soundwire/master.c index d9d09759b9c3..adf11d9f5469 100644 --- a/drivers/soundwire/master.c +++ b/drivers/soundwire/master.c @@ -73,6 +73,8 @@ struct sdw_master_device *sdw_md_add(struct sdw_md_driver *driver, put_device(&md->dev); }
+ md->dev.driver = &driver->driver; + return md; } EXPORT_SYMBOL(sdw_md_add); diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h index 6289924b0336..e22bc037c196 100644 --- a/include/linux/soundwire/sdw.h +++ b/include/linux/soundwire/sdw.h @@ -606,6 +606,7 @@ struct sdw_md_driver { */ int (*autonomous_clock_stop_enable)(struct sdw_master_device *md, bool state); + struct device_driver driver; };
#define SDW_SLAVE_ENTRY(_mfg_id, _part_id, _drv_data) \
participants (2)
-
Greg KH
-
Pierre-Louis Bossart