On Thu, Oct 19, 2017 at 08:33:18AM +0530, Vinod Koul wrote:

Are you _sure_ that code that interacts with the driver core can have a
dual-license here?  Have you explained to lawyers what you are doing
here (wrapping gpl-only symbols with non-gpl-only exports)?

And why dual license something that will only ever work on Linux?

And finally, put a real SPDX header up there so that people don't have
to parse that horrid amount of text to try to determine exactly what
that license is.

+ *
+ */
+
+#include <linux/acpi.h>
+#include <linux/device.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/pm_domain.h>
+#include <linux/pm_runtime.h>
+#include <linux/soundwire/sdw.h>
+#include "bus.h"
+
+/**
+ * sdw_get_device_id: find the matching SoundWire device id
+ *
+ * @slave: SoundWire Slave device
+ * @drv: SoundWire Slave Driver
+ *
+ * The match is done by comparing the mfg_id and part_id from the
+ * struct sdw_device_id. class_id is unused, as it is a placeholder
+ * in MIPI Spec.
+ */
+static const struct sdw_device_id *
+sdw_get_device_id(struct sdw_slave *slave, struct sdw_driver *drv)
+{
+	const struct sdw_device_id *id = drv->id_table;
+
+	while (id && id->mfg_id) {
+		if (slave->id.mfg_id == id->mfg_id &&
+				slave->id.part_id == id->part_id) {
+			return id;
+		}
+		id++;
+	}
+
+	return NULL;
+}
+
+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);
+
+	return !!sdw_get_device_id(slave, drv);
+}
+
+int sdw_slave_modalias(struct sdw_slave *slave, char *buf, size_t size)
+{
+	/* modalias is sdw:m<mfg_id>p<part_id> */
+
+	return snprintf(buf, size, "sdw:m%04Xp%04X\n",
+			slave->id.mfg_id, slave->id.part_id);
+}
+
+static int sdw_uevent(struct device *dev, struct kobj_uevent_env *env)
+{
+	struct sdw_slave *slave = dev_to_sdw_dev(dev);
+	char modalias[32];
+
+	sdw_slave_modalias(slave, modalias, sizeof(modalias));
+
+	if (add_uevent_var(env, "MODALIAS=%s", modalias))
+		return -ENOMEM;
+
+	return 0;
+}
+
+struct bus_type sdw_bus_type = {
+	.name = "soundwire",
+	.match = sdw_bus_match,
+	.uevent = sdw_uevent,
+};
+EXPORT_SYMBOL(sdw_bus_type);

EXPORT_SYMBOL_GPL()?

No release callback?  Who frees the device?

+
+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);
+	const struct sdw_device_id *id;
+	int ret;
+
+	id = sdw_get_device_id(slave, drv);
+	if (!id)
+		return -ENODEV;
+
+	/*
+	 * attach to power domain but don't turn on (last arg)
+	 */
+	ret = dev_pm_domain_attach(dev, false);
+	if (ret) {
+		dev_err(dev, "Failed to attach PM domain: %d\n", ret);
+		return ret;
+	}
+
+	ret = drv->probe(slave, id);
+	if (ret) {
+		dev_err(dev, "Probe of %s failed: %d\n", drv->name, ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+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);
+
+	if (drv->remove)
+		drv->remove(slave);

No catching the error value?

+
+	dev_pm_domain_detach(dev, false);
+
+	return 0;
+}
+
+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);
+
+	if (drv->shutdown)
+		drv->shutdown(slave);
+}
+
+/**
+ * __sdw_register_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)
+{
+	drv->driver.bus = &sdw_bus_type;
+
+	if (!drv->probe) {
+		pr_err("driver %s didn't provide SDW probe routine\n",
+							drv->name);
+		return -EINVAL;
+	}
+
+	drv->driver.owner = owner;
+	drv->driver.probe = sdw_drv_probe;
+
+	if (drv->remove)
+		drv->driver.remove = sdw_drv_remove;
+
+	if (drv->shutdown)
+		drv->driver.shutdown = sdw_drv_shutdown;
+
+	return driver_register(&drv->driver);
+}
+EXPORT_SYMBOL(__sdw_register_driver);
+
+/*
+ * sdw_unregister_driver: unregisters the SoundWire Slave driver
+ *
+ * @drv: driver to unregister
+ */
+void sdw_unregister_driver(struct sdw_driver *drv)
+{
+	driver_unregister(&drv->driver);
+}
+EXPORT_SYMBOL(sdw_unregister_driver);

EXPORT_SYMBOL_GPL()?  Same for all the exports here (I have to ask,
especially given what you are wrapping...)

thanks,

greg k-h