On Thu, 19 Oct 2017 05:03:18 +0200, Vinod Koul wrote:
--- /dev/null +++ b/drivers/soundwire/Kconfig @@ -0,0 +1,22 @@ +# +# SoundWire subsystem configuration +#
+menuconfig SOUNDWIRE
- bool "SoundWire support"
- ---help---
SoundWire is a 2-Pin interface with data and clock line ratifiedby the MIPI Alliance. SoundWire is used for transporting datatypically related to audio functions. SoundWire interface isoptimized to integrate audio devices in mobile or mobile inspiredsystems+if SOUNDWIRE
+comment "SoundWire Devices"
+config SOUNDWIRE_BUS
- tristate
- default SOUNDWIRE
Does it make sense to be tristate? Since CONFIG_SOUNDWIRE is a bool, the above would be also only either Y or N. If it's Y and others select M, it'll be still Y.
--- /dev/null +++ b/drivers/soundwire/bus_type.c +/**
- sdw_get_device_id: find the matching SoundWire device id
- @slave: SoundWire Slave device
- @drv: SoundWire Slave Driver
Inconsistent upper/lower letters in these two lines.
- 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) {
Please indentation properly.
return id;}
Superfluous braces for a single-line.
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)
I'd put const to slave argument, as it won't be modified.
--- a/include/linux/mod_devicetable.h +++ b/include/linux/mod_devicetable.h @@ -228,6 +228,13 @@ struct hda_device_id { unsigned long driver_data; };
+struct sdw_device_id {
- __u16 mfg_id;
- __u16 part_id;
- __u8 class_id;
- kernel_ulong_t driver_data;
Better to think of alignment.
--- /dev/null +++ b/include/linux/soundwire/sdw.h
....
+/**
- struct sdw_bus: SoundWire bus
- @dev: Master linux device
- @link_id: Link id number, can be 0 to N, unique for each Master
- @slaves: list of Slaves on this bus
- @assigned: logical addresses assigned, Index 0 (broadcast) would be unused
- @bus_lock: bus lock
- */
+struct sdw_bus {
- struct device *dev;
- unsigned int link_id;
- struct list_head slaves;
- bool assigned[SDW_MAX_DEVICES + 1];
Why not a bitmap?
thanks,
Takashi