[PATCH 1/2] ASoC: ssm2602: add device tree bindings

[PATCH 1/2] ASoC: ssm2602: add device tree bindings

[Stefan Kristiansson]

Allow the ssm2602/ssm2603/ssm2604 codec driver to be
instantiated from the device tree.

Also, add Kconfig prompts to allow manual selection of both the
I2C and SPI configuration versions of the driver.

Signed-off-by: Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>
---
 .../devicetree/bindings/sound/adi,ssm2602.txt         | 19 +++++++++++++++++++
 sound/soc/codecs/Kconfig                              |  8 ++++++--
 sound/soc/codecs/ssm2602-i2c.c                        |  9 +++++++++
 sound/soc/codecs/ssm2602-spi.c                        |  9 +++++++++
 4 files changed, 43 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sound/adi,ssm2602.txt

diff --git a/Documentation/devicetree/bindings/sound/adi,ssm2602.txt b/Documentation/devicetree/bindings/sound/adi,ssm2602.txt
new file mode 100644
index 0000000..3b3302f
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/adi,ssm2602.txt
@@ -0,0 +1,19 @@
+Analog Devices SSM2602, SSM2603 and SSM2604 I2S audio CODEC devices
+
+SSM2602 support both I2C and SPI as the configuration interface,
+the selection is made by the MODE strap-in pin.
+SSM2603 and SSM2604 only support I2C as the configuration interface.
+
+Required properties:
+
+  - compatible : One of "adi,ssm2602", "adi,ssm2603" or "adi,ssm2604"
+
+  - reg : the I2C address of the device for I2C, the chip select
+          number for SPI.
+
+ Example:
+
+	ssm2602: ssm2602@1a {
+		compatible = "adi,ssm2602";
+		reg = <0x1a>;
+	};
diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index 8838838e..3649e73 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -520,12 +520,16 @@ config SND_SOC_SSM2602
 	tristate
 
 config SND_SOC_SSM2602_SPI
+	tristate "Analog Devices SSM2602 CODEC - SPI"
+	depends on SPI_MASTER
 	select SND_SOC_SSM2602
-	tristate
+	select REGMAP_SPI
 
 config SND_SOC_SSM2602_I2C
+	tristate "Analog Devices SSM2602 CODEC - I2C"
+	depends on I2C
 	select SND_SOC_SSM2602
-	tristate
+	select REGMAP_I2C
 
 config SND_SOC_STA32X
 	tristate
diff --git a/sound/soc/codecs/ssm2602-i2c.c b/sound/soc/codecs/ssm2602-i2c.c
index abd63d5..0d9779d 100644
--- a/sound/soc/codecs/ssm2602-i2c.c
+++ b/sound/soc/codecs/ssm2602-i2c.c
@@ -41,10 +41,19 @@ static const struct i2c_device_id ssm2602_i2c_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, ssm2602_i2c_id);
 
+static const struct of_device_id ssm2602_of_match[] = {
+	{ .compatible = "adi,ssm2602", },
+	{ .compatible = "adi,ssm2603", },
+	{ .compatible = "adi,ssm2604", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ssm2602_of_match);
+
 static struct i2c_driver ssm2602_i2c_driver = {
 	.driver = {
 		.name = "ssm2602",
 		.owner = THIS_MODULE,
+		.of_match_table = ssm2602_of_match,
 	},
 	.probe = ssm2602_i2c_probe,
 	.remove = ssm2602_i2c_remove,
diff --git a/sound/soc/codecs/ssm2602-spi.c b/sound/soc/codecs/ssm2602-spi.c
index 2bf55e2..fb4cc46 100644
--- a/sound/soc/codecs/ssm2602-spi.c
+++ b/sound/soc/codecs/ssm2602-spi.c
@@ -26,10 +26,19 @@ static int ssm2602_spi_remove(struct spi_device *spi)
 	return 0;
 }
 
+static const struct of_device_id ssm2602_of_match[] = {
+	{ .compatible = "adi,ssm2602", },
+	{ .compatible = "adi,ssm2603", },
+	{ .compatible = "adi,ssm2604", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ssm2602_of_match);
+
 static struct spi_driver ssm2602_spi_driver = {
 	.driver = {
 		.name	= "ssm2602",
 		.owner	= THIS_MODULE,
+		.of_match_table = ssm2602_of_match,
 	},
 	.probe		= ssm2602_spi_probe,
 	.remove		= ssm2602_spi_remove,
-- 
1.9.1

Re: [PATCH 1/2] ASoC: ssm2602: add device tree bindings

[Lars-Peter Clausen]

On 09/28/2014 09:57 AM, Stefan Kristiansson wrote:
> Allow the ssm2602/ssm2603/ssm2604 codec driver to be
> instantiated from the device tree.
>
> Also, add Kconfig prompts to allow manual selection of both the
> I2C and SPI configuration versions of the driver.
>
> Signed-off-by: Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>

Looks mostly good, but you should Cc both the devicetree and the ASoC 
maintainers.

[...]
> diff --git a/sound/soc/codecs/ssm2602-i2c.c b/sound/soc/codecs/ssm2602-i2c.c
> index abd63d5..0d9779d 100644
> --- a/sound/soc/codecs/ssm2602-i2c.c
> +++ b/sound/soc/codecs/ssm2602-i2c.c
> @@ -41,10 +41,19 @@ static const struct i2c_device_id ssm2602_i2c_id[] = {
>   };
>   MODULE_DEVICE_TABLE(i2c, ssm2602_i2c_id);
>
> +static const struct of_device_id ssm2602_of_match[] = {
> +	{ .compatible = "adi,ssm2602", },
> +	{ .compatible = "adi,ssm2603", },
> +	{ .compatible = "adi,ssm2604", },

Since the driver is supposed to behave different depending on the device you 
should set the driver data similar to like it is done in the ssm2602_i2c_id 
table and add code in the probe function to extract the driver data.

> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, ssm2602_of_match);
> +
>   static struct i2c_driver ssm2602_i2c_driver = {
>   	.driver = {
>   		.name = "ssm2602",
>   		.owner = THIS_MODULE,
> +		.of_match_table = ssm2602_of_match,
>   	},
>   	.probe = ssm2602_i2c_probe,
>   	.remove = ssm2602_i2c_remove,
> diff --git a/sound/soc/codecs/ssm2602-spi.c b/sound/soc/codecs/ssm2602-spi.c
> index 2bf55e2..fb4cc46 100644
> --- a/sound/soc/codecs/ssm2602-spi.c
> +++ b/sound/soc/codecs/ssm2602-spi.c
> @@ -26,10 +26,19 @@ static int ssm2602_spi_remove(struct spi_device *spi)
>   	return 0;
>   }
>
> +static const struct of_device_id ssm2602_of_match[] = {
> +	{ .compatible = "adi,ssm2602", },
> +	{ .compatible = "adi,ssm2603", },
> +	{ .compatible = "adi,ssm2604", },

The ssm2603 and ssm2604 do not have a SPI interface.

> +	{ }
> +};
[...]

Re: [PATCH 1/2] ASoC: ssm2602: add device tree bindings

[Stefan Kristiansson]

On Mon, Sep 29, 2014 at 10:50:24AM +0200, Lars-Peter Clausen wrote:
> On 09/28/2014 09:57 AM, Stefan Kristiansson wrote:
> >Allow the ssm2602/ssm2603/ssm2604 codec driver to be
> >instantiated from the device tree.
> >
> >Also, add Kconfig prompts to allow manual selection of both the
> >I2C and SPI configuration versions of the driver.
> >
> >Signed-off-by: Stefan Kristiansson <stefan.kristiansson-MbMCFXIvDHJFcC0YU169RA@public.gmane.org>
> 
> Looks mostly good, but you should Cc both the devicetree and the
> ASoC maintainers.
> 

Ok, I'll make sure to do that in v2.

> [...]
> >diff --git a/sound/soc/codecs/ssm2602-i2c.c b/sound/soc/codecs/ssm2602-i2c.c
> >index abd63d5..0d9779d 100644
> >--- a/sound/soc/codecs/ssm2602-i2c.c
> >+++ b/sound/soc/codecs/ssm2602-i2c.c
> >@@ -41,10 +41,19 @@ static const struct i2c_device_id ssm2602_i2c_id[] = {
> >  };
> >  MODULE_DEVICE_TABLE(i2c, ssm2602_i2c_id);
> >
> >+static const struct of_device_id ssm2602_of_match[] = {
> >+	{ .compatible = "adi,ssm2602", },
> >+	{ .compatible = "adi,ssm2603", },
> >+	{ .compatible = "adi,ssm2604", },
> 
> Since the driver is supposed to behave different depending on the
> device you should set the driver data similar to like it is done in
> the ssm2602_i2c_id table and add code in the probe function to
> extract the driver data.
> 

As far as I understand, that is already covered for, ssm2602_i2c_probe() get
called with the i2c_device_id struct set according to the compatible string.
I.e. if I set it to "adi,ssm2603" in the .dts, ssm2602_i2c_probe() is called
with id->name = "ssm2603" and id->driver_data = 0 and if I set it to
"adi,ssm2604" I get id->name = "ssm2603" and id->driver_data = 1.

However, when looking into that, I noticed this in ssm2602_probe()

	 ssm2602->type = SSM2602;

so the value from id->driver_data is never used.
(it should be ssm2602->type = type;)

I can send a seperate patch that fixes this, but I only have a ssm2603,
so I can't test if it works.

> >+static const struct of_device_id ssm2602_of_match[] = {
> >+	{ .compatible = "adi,ssm2602", },
> >+	{ .compatible = "adi,ssm2603", },
> >+	{ .compatible = "adi,ssm2604", },
> 
> The ssm2603 and ssm2604 do not have a SPI interface.
>

Right, I'll remove those from that list.

Stefan
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] ASoC: ssm2602: add device tree bindings

[Stefan Kristiansson]

On Mon, Sep 29, 2014 at 10:50:24AM +0200, Lars-Peter Clausen wrote:
> On 09/28/2014 09:57 AM, Stefan Kristiansson wrote:
> >Allow the ssm2602/ssm2603/ssm2604 codec driver to be
> >instantiated from the device tree.
> >
> >Also, add Kconfig prompts to allow manual selection of both the
> >I2C and SPI configuration versions of the driver.
> >
> >Signed-off-by: Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>
> 
> Looks mostly good, but you should Cc both the devicetree and the
> ASoC maintainers.
> 

Ok, I'll make sure to do that in v2.

> [...]
> >diff --git a/sound/soc/codecs/ssm2602-i2c.c b/sound/soc/codecs/ssm2602-i2c.c
> >index abd63d5..0d9779d 100644
> >--- a/sound/soc/codecs/ssm2602-i2c.c
> >+++ b/sound/soc/codecs/ssm2602-i2c.c
> >@@ -41,10 +41,19 @@ static const struct i2c_device_id ssm2602_i2c_id[] = {
> >  };
> >  MODULE_DEVICE_TABLE(i2c, ssm2602_i2c_id);
> >
> >+static const struct of_device_id ssm2602_of_match[] = {
> >+	{ .compatible = "adi,ssm2602", },
> >+	{ .compatible = "adi,ssm2603", },
> >+	{ .compatible = "adi,ssm2604", },
> 
> Since the driver is supposed to behave different depending on the
> device you should set the driver data similar to like it is done in
> the ssm2602_i2c_id table and add code in the probe function to
> extract the driver data.
> 

As far as I understand, that is already covered for, ssm2602_i2c_probe() get
called with the i2c_device_id struct set according to the compatible string.
I.e. if I set it to "adi,ssm2603" in the .dts, ssm2602_i2c_probe() is called
with id->name = "ssm2603" and id->driver_data = 0 and if I set it to
"adi,ssm2604" I get id->name = "ssm2603" and id->driver_data = 1.

However, when looking into that, I noticed this in ssm2602_probe()

	 ssm2602->type = SSM2602;

so the value from id->driver_data is never used.
(it should be ssm2602->type = type;)

I can send a seperate patch that fixes this, but I only have a ssm2603,
so I can't test if it works.

> >+static const struct of_device_id ssm2602_of_match[] = {
> >+	{ .compatible = "adi,ssm2602", },
> >+	{ .compatible = "adi,ssm2603", },
> >+	{ .compatible = "adi,ssm2604", },
> 
> The ssm2603 and ssm2604 do not have a SPI interface.
>

Right, I'll remove those from that list.

Stefan

Re: [PATCH 1/2] ASoC: ssm2602: add device tree bindings

[Lars-Peter Clausen]

On 09/29/2014 04:34 PM, Stefan Kristiansson wrote:
> On Mon, Sep 29, 2014 at 10:50:24AM +0200, Lars-Peter Clausen wrote:
>> On 09/28/2014 09:57 AM, Stefan Kristiansson wrote:
>>> Allow the ssm2602/ssm2603/ssm2604 codec driver to be
>>> instantiated from the device tree.
>>>
>>> Also, add Kconfig prompts to allow manual selection of both the
>>> I2C and SPI configuration versions of the driver.
>>>
>>> Signed-off-by: Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>
>>
>> Looks mostly good, but you should Cc both the devicetree and the
>> ASoC maintainers.
>>
>
> Ok, I'll make sure to do that in v2.
>
>> [...]
>>> diff --git a/sound/soc/codecs/ssm2602-i2c.c b/sound/soc/codecs/ssm2602-i2c.c
>>> index abd63d5..0d9779d 100644
>>> --- a/sound/soc/codecs/ssm2602-i2c.c
>>> +++ b/sound/soc/codecs/ssm2602-i2c.c
>>> @@ -41,10 +41,19 @@ static const struct i2c_device_id ssm2602_i2c_id[] = {
>>>   };
>>>   MODULE_DEVICE_TABLE(i2c, ssm2602_i2c_id);
>>>
>>> +static const struct of_device_id ssm2602_of_match[] = {
>>> +	{ .compatible = "adi,ssm2602", },
>>> +	{ .compatible = "adi,ssm2603", },
>>> +	{ .compatible = "adi,ssm2604", },
>>
>> Since the driver is supposed to behave different depending on the
>> device you should set the driver data similar to like it is done in
>> the ssm2602_i2c_id table and add code in the probe function to
>> extract the driver data.
>>
>
> As far as I understand, that is already covered for, ssm2602_i2c_probe() get
> called with the i2c_device_id struct set according to the compatible string.
> I.e. if I set it to "adi,ssm2603" in the .dts, ssm2602_i2c_probe() is called
> with id->name = "ssm2603" and id->driver_data = 0 and if I set it to
> "adi,ssm2604" I get id->name = "ssm2603" and id->driver_data = 1.

Ok, looks like the I2C core extracts the string after the "," and than 
matches the I2C device id table against it.

>
> However, when looking into that, I noticed this in ssm2602_probe()
>
> 	 ssm2602->type = SSM2602;
>
> so the value from id->driver_data is never used.
> (it should be ssm2602->type = type;)

Yes, that's clearly a bug.

[...]

Re: [PATCH 1/2] ASoC: ssm2602: add device tree bindings

[Stefan Kristiansson]

On Mon, Sep 29, 2014 at 05:34:52PM +0300, Stefan Kristiansson wrote:
> ... if I set it to
> "adi,ssm2604" I get id->name = "ssm2603" and id->driver_data = 1.
> 

Err, copy-paste mistake,that should have of course have read
'if I set it to "adi,ssm2604" I get id->name = "ssm2604"'
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] ASoC: ssm2602: add device tree bindings

[Stefan Kristiansson]

On Mon, Sep 29, 2014 at 05:34:52PM +0300, Stefan Kristiansson wrote:
> ... if I set it to
> "adi,ssm2604" I get id->name = "ssm2603" and id->driver_data = 1.
> 

Err, copy-paste mistake,that should have of course have read
'if I set it to "adi,ssm2604" I get id->name = "ssm2604"'