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 ratified
> + by the MIPI Alliance. SoundWire is used for transporting data
> + typically related to audio functions. SoundWire interface is
> + optimized to integrate audio devices in mobile or mobile inspired
> + systems
> +
> +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
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
