mbox series

[RESEND,v4,0/4] ASoC: codecs: Add WSA881x Smart Speaker amplifier support

Message ID 20190822233759.12663-1-srinivas.kandagatla@linaro.org
Headers show
Series ASoC: codecs: Add WSA881x Smart Speaker amplifier support | expand

Message

Srinivas Kandagatla Aug. 22, 2019, 11:37 p.m. UTC 
Resending this series due to a typo in yaml filename!

Thanks for reviewing v3 patchset, here is v4 with addressing the comments in v3

This patchset adds support to WSA8810/WSA8815 Class-D Smart Speaker
Amplifier which is SoundWire interfaced.
This also adds support to some missing bits in SoundWire bus layer like
Device Tree support.

This patchset along with DB845c machine driver and WCD934x codec driver
has been tested on SDM845 SoC based DragonBoard DB845c with two
WSA8810 speakers.

Most of the code in this driver is rework of Qualcomm downstream drivers
used in Andriod. Credits to Banajit Goswami and Patrick Lai's Team.

TODO:
	Add thermal sensor support in WSA881x.

Thanks,
srini

Changes since v3:
 - updated slave bindings according to Rob's Suggestion.
 - moved bindings to yaml

Srinivas Kandagatla (4):
  dt-bindings: soundwire: add slave bindings
  soundwire: core: add device tree support for slave devices
  dt-bindings: ASoC: Add WSA881x bindings
  ASoC: codecs: add wsa881x amplifier support

 .../bindings/sound/qcom,wsa881x.yaml          |   44 +
 .../soundwire/soundwire-controller.yaml       |   75 ++
 drivers/soundwire/bus.c                       |    2 +
 drivers/soundwire/bus.h                       |    1 +
 drivers/soundwire/slave.c                     |   52 +
 sound/soc/codecs/Kconfig                      |   10 +
 sound/soc/codecs/Makefile                     |    2 +
 sound/soc/codecs/wsa881x.c                    | 1134 +++++++++++++++++
 8 files changed, 1320 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/qcom,wsa881x.yaml
 create mode 100644 Documentation/devicetree/bindings/soundwire/soundwire-controller.yaml
 create mode 100644 sound/soc/codecs/wsa881x.c

-- 
2.21.0

Comments

Pierre-Louis Bossart Aug. 23, 2019, 3:44 p.m. UTC | #1 
On 8/22/19 6:37 PM, Srinivas Kandagatla wrote:
> This patch adds support to parsing device tree based

> SoundWire slave devices.

> 

> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

> ---

>   drivers/soundwire/bus.c   |  2 ++

>   drivers/soundwire/bus.h   |  1 +

>   drivers/soundwire/slave.c | 52 +++++++++++++++++++++++++++++++++++++++

>   3 files changed, 55 insertions(+)

> 

> diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c

> index 49f64b2115b9..c2eaeb5c38ed 100644

> --- a/drivers/soundwire/bus.c

> +++ b/drivers/soundwire/bus.c

> @@ -77,6 +77,8 @@ int sdw_add_bus_master(struct sdw_bus *bus)

>   	 */

>   	if (IS_ENABLED(CONFIG_ACPI) && ACPI_HANDLE(bus->dev))

>   		ret = sdw_acpi_find_slaves(bus);

> +	else if (IS_ENABLED(CONFIG_OF) && bus->dev->of_node)

> +		ret = sdw_of_find_slaves(bus);

>   	else

>   		ret = -ENOTSUPP; /* No ACPI/DT so error out */

>   

> diff --git a/drivers/soundwire/bus.h b/drivers/soundwire/bus.h

> index 3048ca153f22..ee46befedbd1 100644

> --- a/drivers/soundwire/bus.h

> +++ b/drivers/soundwire/bus.h

> @@ -15,6 +15,7 @@ static inline int sdw_acpi_find_slaves(struct sdw_bus *bus)

>   }

>   #endif

>   

> +int sdw_of_find_slaves(struct sdw_bus *bus);

>   void sdw_extract_slave_id(struct sdw_bus *bus,

>   			  u64 addr, struct sdw_slave_id *id);

>   

> diff --git a/drivers/soundwire/slave.c b/drivers/soundwire/slave.c

> index f39a5815e25d..3ef265d2ee89 100644

> --- a/drivers/soundwire/slave.c

> +++ b/drivers/soundwire/slave.c

> @@ -2,6 +2,7 @@

>   // Copyright(c) 2015-17 Intel Corporation.

>   

>   #include <linux/acpi.h>

> +#include <linux/of.h>

>   #include <linux/soundwire/sdw.h>

>   #include <linux/soundwire/sdw_type.h>

>   #include "bus.h"

> @@ -35,6 +36,7 @@ static int sdw_slave_add(struct sdw_bus *bus,

>   

>   	slave->dev.release = sdw_slave_release;

>   	slave->dev.bus = &sdw_bus_type;

> +	slave->dev.of_node = of_node_get(to_of_node(fwnode));

>   	slave->bus = bus;

>   	slave->status = SDW_SLAVE_UNATTACHED;

>   	slave->dev_num = 0;

> @@ -112,3 +114,53 @@ int sdw_acpi_find_slaves(struct sdw_bus *bus)

>   }

>   

>   #endif

> +

> +/*

> + * sdw_of_find_slaves() - Find Slave devices in master device tree node

> + * @bus: SDW bus instance

> + *

> + * Scans Master DT node for SDW child Slave devices and registers it.

> + */

> +int sdw_of_find_slaves(struct sdw_bus *bus)

> +{

> +	struct device *dev = bus->dev;

> +	struct device_node *node;

> +

> +	for_each_child_of_node(bus->dev->of_node, node) {

> +		int link_id, sdw_version, ret, len;

> +		const char *compat = NULL;

> +		struct sdw_slave_id id;

> +		const __be32 *addr;

> +

> +		compat = of_get_property(node, "compatible", NULL);

> +		if (!compat)

> +			continue;

> +

> +		ret = sscanf(compat, "sdw%01x%04hx%04hx%02hhx", &sdw_version,

> +			     &id.mfg_id, &id.part_id, &id.class_id);

> +

> +		if (ret != 4) {

> +			dev_err(dev, "Invalid compatible string found %s\n",

> +				compat);

> +			continue;

> +		}

> +

> +		addr = of_get_property(node, "reg", &len);

> +		if (!addr || (len < 2 * sizeof(u32))) {

> +			dev_err(dev, "Invalid Instance and Link ID\n");

> +			continue;

> +		}

> +

> +		id.unique_id = be32_to_cpup(addr++);

> +		link_id = be32_to_cpup(addr);


So here the link ID is obviously not in the address, so you are not 
using the MIPI spec as we discussed initially?

> +		id.sdw_version = sdw_version;

> +

> +		/* Check for link_id match */

> +		if (link_id != bus->link_id)

> +			continue;

> +

> +		sdw_slave_add(bus, &id, of_fwnode_handle(node));

> +	}

> +

> +	return 0;

> +}

>
Srinivas Kandagatla Aug. 23, 2019, 3:47 p.m. UTC | #2 
On 23/08/2019 16:44, Pierre-Louis Bossart wrote:
> 

> 

> On 8/22/19 6:37 PM, Srinivas Kandagatla wrote:

>> This patch adds support to parsing device tree based

>> SoundWire slave devices.

>>

>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

>> ---

>>   drivers/soundwire/bus.c   |  2 ++

>>   drivers/soundwire/bus.h   |  1 +

>>   drivers/soundwire/slave.c | 52 +++++++++++++++++++++++++++++++++++++++

>>   3 files changed, 55 insertions(+)

>>

>> diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c

>> index 49f64b2115b9..c2eaeb5c38ed 100644

>> --- a/drivers/soundwire/bus.c

>> +++ b/drivers/soundwire/bus.c

>> @@ -77,6 +77,8 @@ int sdw_add_bus_master(struct sdw_bus *bus)

>>        */

>>       if (IS_ENABLED(CONFIG_ACPI) && ACPI_HANDLE(bus->dev))

>>           ret = sdw_acpi_find_slaves(bus);

>> +    else if (IS_ENABLED(CONFIG_OF) && bus->dev->of_node)

>> +        ret = sdw_of_find_slaves(bus);

>>       else

>>           ret = -ENOTSUPP; /* No ACPI/DT so error out */

>> diff --git a/drivers/soundwire/bus.h b/drivers/soundwire/bus.h

>> index 3048ca153f22..ee46befedbd1 100644

>> --- a/drivers/soundwire/bus.h

>> +++ b/drivers/soundwire/bus.h

>> @@ -15,6 +15,7 @@ static inline int sdw_acpi_find_slaves(struct 

>> sdw_bus *bus)

>>   }

>>   #endif

>> +int sdw_of_find_slaves(struct sdw_bus *bus);

>>   void sdw_extract_slave_id(struct sdw_bus *bus,

>>                 u64 addr, struct sdw_slave_id *id);

>> diff --git a/drivers/soundwire/slave.c b/drivers/soundwire/slave.c

>> index f39a5815e25d..3ef265d2ee89 100644

>> --- a/drivers/soundwire/slave.c

>> +++ b/drivers/soundwire/slave.c

>> @@ -2,6 +2,7 @@

>>   // Copyright(c) 2015-17 Intel Corporation.

>>   #include <linux/acpi.h>

>> +#include <linux/of.h>

>>   #include <linux/soundwire/sdw.h>

>>   #include <linux/soundwire/sdw_type.h>

>>   #include "bus.h"

>> @@ -35,6 +36,7 @@ static int sdw_slave_add(struct sdw_bus *bus,

>>       slave->dev.release = sdw_slave_release;

>>       slave->dev.bus = &sdw_bus_type;

>> +    slave->dev.of_node = of_node_get(to_of_node(fwnode));

>>       slave->bus = bus;

>>       slave->status = SDW_SLAVE_UNATTACHED;

>>       slave->dev_num = 0;

>> @@ -112,3 +114,53 @@ int sdw_acpi_find_slaves(struct sdw_bus *bus)

>>   }

>>   #endif

>> +

>> +/*

>> + * sdw_of_find_slaves() - Find Slave devices in master device tree node

>> + * @bus: SDW bus instance

>> + *

>> + * Scans Master DT node for SDW child Slave devices and registers it.

>> + */

>> +int sdw_of_find_slaves(struct sdw_bus *bus)

>> +{

>> +    struct device *dev = bus->dev;

>> +    struct device_node *node;

>> +

>> +    for_each_child_of_node(bus->dev->of_node, node) {

>> +        int link_id, sdw_version, ret, len;

>> +        const char *compat = NULL;

>> +        struct sdw_slave_id id;

>> +        const __be32 *addr;

>> +

>> +        compat = of_get_property(node, "compatible", NULL);

>> +        if (!compat)

>> +            continue;

>> +

>> +        ret = sscanf(compat, "sdw%01x%04hx%04hx%02hhx", &sdw_version,

>> +                 &id.mfg_id, &id.part_id, &id.class_id);

>> +

>> +        if (ret != 4) {

>> +            dev_err(dev, "Invalid compatible string found %s\n",

>> +                compat);

>> +            continue;

>> +        }

>> +

>> +        addr = of_get_property(node, "reg", &len);

>> +        if (!addr || (len < 2 * sizeof(u32))) {

>> +            dev_err(dev, "Invalid Instance and Link ID\n");

>> +            continue;

>> +        }

>> +

>> +        id.unique_id = be32_to_cpup(addr++);

>> +        link_id = be32_to_cpup(addr);

> 

> So here the link ID is obviously not in the address, so you are not 

> using the MIPI spec as we discussed initially?


No, Rob rejected that approach (https://lkml.org/lkml/2019/8/22/490) 
with the fact that compatible string should be constant for each instance.

--srini
> 

>> +        id.sdw_version = sdw_version;

>> +

>> +        /* Check for link_id match */

>> +        if (link_id != bus->link_id)

>> +            continue;

>> +

>> +        sdw_slave_add(bus, &id, of_fwnode_handle(node));

>> +    }

>> +

>> +    return 0;

>> +}

>>