[PATCH PING] ASoC: dwc: Remove unnecessary conditional compilation

2015-01-27 Thread Andrew Jackson
From: Andrew Jackson 

of_match_ptr is already conditionally compiled based on
CONFIG_OF so further conditional compilation is not
required.  Remove conditional compilation surrounding
of_match_ptr.

Signed-off-by: Andrew Jackson 
---
 sound/soc/dwc/designware_i2s.c |2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
index 99cf64b..a3e97b4 100644
--- a/sound/soc/dwc/designware_i2s.c
+++ b/sound/soc/dwc/designware_i2s.c
@@ -624,9 +624,7 @@ static struct platform_driver dw_i2s_driver = {
.remove = dw_i2s_remove,
.driver = {
.name   = "designware-i2s",
-#ifdef CONFIG_OF
.of_match_table = of_match_ptr(dw_i2s_of_match),
-#endif
},
 };
 
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH PING] ASoC: dwc: Remove unnecessary conditional compilation

2015-01-27 Thread Andrew Jackson
From: Andrew Jackson andrew.jack...@arm.com

of_match_ptr is already conditionally compiled based on
CONFIG_OF so further conditional compilation is not
required.  Remove conditional compilation surrounding
of_match_ptr.

Signed-off-by: Andrew Jackson andrew.jack...@arm.com
---
 sound/soc/dwc/designware_i2s.c |2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
index 99cf64b..a3e97b4 100644
--- a/sound/soc/dwc/designware_i2s.c
+++ b/sound/soc/dwc/designware_i2s.c
@@ -624,9 +624,7 @@ static struct platform_driver dw_i2s_driver = {
.remove = dw_i2s_remove,
.driver = {
.name   = designware-i2s,
-#ifdef CONFIG_OF
.of_match_table = of_match_ptr(dw_i2s_of_match),
-#endif
},
 };
 
-- 
1.7.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v9 3/4] ASoC: tda998x: add a codec to the HDMI transmitter

2015-01-09 Thread Andrew Jackson
On 01/07/15 10:51, Jean-Francois Moine wrote:
> This patch adds a CODEC to the NXP TDA998x HDMI transmitter.
> 
> The CODEC handles both I2S and S/PDIF inputs.
> It maintains the audio format and rate constraints according
> to the HDMI device parameters (EDID) and sets dynamically the input
> ports in the TDA998x I2C driver on start/stop audio streaming.
> 
> Signed-off-by: Jean-Francois Moine 
> ---

[snip]

> +static int tda998x_codec_startup(struct snd_pcm_substream *substream,
> +struct snd_soc_dai *dai)
> +{
> +   struct snd_pcm_runtime *runtime = substream->runtime;
> +   struct snd_pcm_hw_constraint_list *rate_constraints;
> +   u8 *sad;/* Short Audio Descriptor (3 bytes) */

[...]

> +   snd_pcm_hw_constraint_minmax(runtime,
> +   SNDRV_PCM_HW_PARAM_CHANNELS,
> +   1, sad[SAD_MX_CHAN_I]);

In the light of our discussions elsewhere [1], shouldn't this
be constrained by the number of hardware channels that the TDA998x
supports too?  That is, the maximum number of channels should
be the lesser of sd[SAD_MX_CHAN_I] and number_of_I2S channels 
(or S/PDIF channels if so configured).

Andrew

[1] 
http://mailman.alsa-project.org/pipermail/alsa-devel/2015-January/086090.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v9 1/4] drm/i2c: tda998x: Add DT support for audio

2015-01-09 Thread Andrew Jackson
On 01/09/15 13:07, Russell King - ARM Linux wrote:
> On Fri, Jan 09, 2015 at 01:54:01PM +0100, Jean-Francois Moine wrote:
>> On Fri, 9 Jan 2015 11:45:29 +
>> Russell King - ARM Linux  wrote:
>>> I think we need to understand exactly how the 998x map I2S inputs to the
>>> HDMI channels to avoid making a mistake with the binding; remember, the
>>> binding isn't something that can be easily "bug fixed" at a later date
>>> as anything we come up with now has to be supported long term by the
>>> kernel.
>>
>> The DT describes the hardware configuration.
> 
> You're missing my point.
> 
> How does the driver know which of the I2S pins to enable in I2S mode?

[snip]

> My question is: how do we know which I2S inputs to enable, or are
> you suggesting that all I2S inputs should be enabled if operating in
> I2S mode irrespective of whether they may be active?
> 

Isn't it the case that for I2S:

* Word Select (WS) is always required to disambiguate left/right. So WS need
  not be specified in any device tree configuration.

* The audio inputs depend on a particular board but at least one is 
  required.  Fortunately, the TDA998x devices have all the same pin/input
  numbering so they /could/ be described as i2s0, i2s1, i2s2 and i2s3 as 
  per J-F's earlier email.  But tt isn't clear from my reading of the 
  TDA19988 datasheet (for example) whether one can skip channels (so
  does channel 0 always have to be present?).  If the channels must
  be enabled from 0 then one could simply specify the number of inputs.
  (The datasheets that I have don't indicate whether there's any 
  channel remapping performed internally by the TDA998x).

* What the driver does need to take account of is the number of inputs
  that are active so that the sound CODEC can be properly configured.


Andrew


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v9 1/4] drm/i2c: tda998x: Add DT support for audio

2015-01-09 Thread Andrew Jackson
On 01/08/15 20:04, Mark Brown wrote:
> On Thu, Jan 08, 2015 at 05:42:57PM +0100, Jean-Francois Moine wrote:
> 
>> Examples:
> 
>> - for the Cubox:
> 
>>  audio-inputs = "i2s", "spdif";
> 
>> - for some other board with I2S on the pins 3 and 4 only:
> 
>>  audio-inputs = "none", "none", "i2s", "i2s";
> 
>> - for a fully wired TDA9983B (no driver yet):
> 
>>  audio-inputs = "i2s", "i2s", "i2s", "i2s", "spdif";
> 
> I think that mostly works, though I do wonder if we need a way to
> specify the ordering of the pins (if you can make pin 3 be the first two
> I2S channels for example)?  Someone might choose a strange mapping for
> board routing reasons for example.
> 

If it helps, I've collated the pin assignments given in the various TDA 
datasheets that I can find:

Chip>   9983B   998919988   19989   
Mode>   -   S/PDIF  I2S S/PDIF  I2S S/PDIF  I2S
Pin
AP0 WS  -   WS  -   WS  -   WS
AP1 I2S#0   S/PDIF  I2S#0   S/PDIF  I2S#0   S/PDIF  I2S#0
AP2 I2S#1   -   -   S/PDIF  I2S#1   S/PDIF  I2S#1
AP3 I2S#2   -   -   -   I2S#2*  MCLK-
AP4 I2S#3   -   -   -   I2S#3*  -   -
AP5 MCLK-   -   -   -   -   -
AP6 S/PDIF  -   -   -   -   -   -
AP7 AUX -   -   -   -   -   -

WS = I2S Word Select
* Depends on package

The 9983B differs from the other devices in that the I2S and S/PDIF 
functionality is not multiplexed 
onto various pins.

Andrew

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v9 3/4] ASoC: tda998x: add a codec to the HDMI transmitter

2015-01-09 Thread Andrew Jackson
On 01/07/15 10:51, Jean-Francois Moine wrote:
 This patch adds a CODEC to the NXP TDA998x HDMI transmitter.
 
 The CODEC handles both I2S and S/PDIF inputs.
 It maintains the audio format and rate constraints according
 to the HDMI device parameters (EDID) and sets dynamically the input
 ports in the TDA998x I2C driver on start/stop audio streaming.
 
 Signed-off-by: Jean-Francois Moine moin...@free.fr
 ---

[snip]

 +static int tda998x_codec_startup(struct snd_pcm_substream *substream,
 +struct snd_soc_dai *dai)
 +{
 +   struct snd_pcm_runtime *runtime = substream-runtime;
 +   struct snd_pcm_hw_constraint_list *rate_constraints;
 +   u8 *sad;/* Short Audio Descriptor (3 bytes) */

[...]

 +   snd_pcm_hw_constraint_minmax(runtime,
 +   SNDRV_PCM_HW_PARAM_CHANNELS,
 +   1, sad[SAD_MX_CHAN_I]);

In the light of our discussions elsewhere [1], shouldn't this
be constrained by the number of hardware channels that the TDA998x
supports too?  That is, the maximum number of channels should
be the lesser of sd[SAD_MX_CHAN_I] and number_of_I2S channels 
(or S/PDIF channels if so configured).

Andrew

[1] 
http://mailman.alsa-project.org/pipermail/alsa-devel/2015-January/086090.html
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v9 1/4] drm/i2c: tda998x: Add DT support for audio

2015-01-09 Thread Andrew Jackson
On 01/08/15 20:04, Mark Brown wrote:
 On Thu, Jan 08, 2015 at 05:42:57PM +0100, Jean-Francois Moine wrote:
 
 Examples:
 
 - for the Cubox:
 
  audio-inputs = i2s, spdif;
 
 - for some other board with I2S on the pins 3 and 4 only:
 
  audio-inputs = none, none, i2s, i2s;
 
 - for a fully wired TDA9983B (no driver yet):
 
  audio-inputs = i2s, i2s, i2s, i2s, spdif;
 
 I think that mostly works, though I do wonder if we need a way to
 specify the ordering of the pins (if you can make pin 3 be the first two
 I2S channels for example)?  Someone might choose a strange mapping for
 board routing reasons for example.
 

If it helps, I've collated the pin assignments given in the various TDA 
datasheets that I can find:

Chip   9983B   998919988   19989   
Mode   -   S/PDIF  I2S S/PDIF  I2S S/PDIF  I2S
Pin
AP0 WS  -   WS  -   WS  -   WS
AP1 I2S#0   S/PDIF  I2S#0   S/PDIF  I2S#0   S/PDIF  I2S#0
AP2 I2S#1   -   -   S/PDIF  I2S#1   S/PDIF  I2S#1
AP3 I2S#2   -   -   -   I2S#2*  MCLK-
AP4 I2S#3   -   -   -   I2S#3*  -   -
AP5 MCLK-   -   -   -   -   -
AP6 S/PDIF  -   -   -   -   -   -
AP7 AUX -   -   -   -   -   -

WS = I2S Word Select
* Depends on package

The 9983B differs from the other devices in that the I2S and S/PDIF 
functionality is not multiplexed 
onto various pins.

Andrew

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v9 1/4] drm/i2c: tda998x: Add DT support for audio

2015-01-09 Thread Andrew Jackson
On 01/09/15 13:07, Russell King - ARM Linux wrote:
 On Fri, Jan 09, 2015 at 01:54:01PM +0100, Jean-Francois Moine wrote:
 On Fri, 9 Jan 2015 11:45:29 +
 Russell King - ARM Linux li...@arm.linux.org.uk wrote:
 I think we need to understand exactly how the 998x map I2S inputs to the
 HDMI channels to avoid making a mistake with the binding; remember, the
 binding isn't something that can be easily bug fixed at a later date
 as anything we come up with now has to be supported long term by the
 kernel.

 The DT describes the hardware configuration.
 
 You're missing my point.
 
 How does the driver know which of the I2S pins to enable in I2S mode?

[snip]

 My question is: how do we know which I2S inputs to enable, or are
 you suggesting that all I2S inputs should be enabled if operating in
 I2S mode irrespective of whether they may be active?
 

Isn't it the case that for I2S:

* Word Select (WS) is always required to disambiguate left/right. So WS need
  not be specified in any device tree configuration.

* The audio inputs depend on a particular board but at least one is 
  required.  Fortunately, the TDA998x devices have all the same pin/input
  numbering so they /could/ be described as i2s0, i2s1, i2s2 and i2s3 as 
  per J-F's earlier email.  But tt isn't clear from my reading of the 
  TDA19988 datasheet (for example) whether one can skip channels (so
  does channel 0 always have to be present?).  If the channels must
  be enabled from 0 then one could simply specify the number of inputs.
  (The datasheets that I have don't indicate whether there's any 
  channel remapping performed internally by the TDA998x).

* What the driver does need to take account of is the number of inputs
  that are active so that the sound CODEC can be properly configured.


Andrew


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v9 1/4] drm/i2c: tda998x: Add DT support for audio

2015-01-07 Thread Andrew Jackson
On 01/07/15 17:08, Jean-Francois Moine wrote:
> On Wed, 07 Jan 2015 14:39:13 +
> Andrew Jackson  wrote:
> 
>>> +  - audio-ports: must contain one or two values selecting the source
>>> +   in the audio port.
>>> +   The source type is given by the corresponding entry in
>>> +   the audio-port-names property.  
>>
>> I think that this entry might benefit from a little more explanation.
>> The value specified here selects which pins on the chip provide the
>> audio input doesn't it?  In the outline datasheet that I have these are
>> listed in table 17:
>>
>> Audio port   Input configuration
>>  S/PDIF  I2S-bus
>> AP0  -   WS (word select)
>> AP1  S/PDIF inputI2S-bus channel 0
>> AP2  S/PDIF inputI2S-bus channel 1
>> AP3[1]   I2S-bus channel 2
>> AP4[1]   I2S-bus channel 3
>> ACLK -   SCK (I2S-bus clock)
>>
>> [1] Depending on package.
> 
> Your table is close to the one in the TDA9983B documentation I have,
> but the pins are not exactly the same:
> 
> AP0   WS (word select)
> AP1   I2S-bus port 0
> AP2   I2S-bus port 1
> AP3   I2S-bus port 2
> AP4   I2S-bus port 3
> AP5   MCLK (master clock for S/PDIF)
> AP6   S/PDIF input
> AP7   AUX (internal test)
> ACLK  SCK (I2S-bus clock)
> 
> That's why I did not know clearly why I had to set AP2 for S/PDIF input
> and (AP0 + AP1) for I2S input in the Cubox.
> 
> Then, the only more explanation I could give is "have a look at the
> audio input format and at the register 0x1e page 0 in the documentation
> of the TDA998x chip".
> 
> BTW, the tda998x driver supports only the TDA9989, TDA19988 and
> TDA19989 chips. If the TDA9983B would be supported, the audio port
> definitions would be of no use.
> 
> So, what would you see as an explanation?
> 

I understand your difficulty!  I was just wanting something to clarify the 
meaning of the value without reference to the driver source.

You could add something like this to your existing explanation: "The value
describes which audio input pins are selected; this varies depending
on chip type so consult the section on audio port configuration in the 
relevant datasheet.".  

Andrew

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v9 3/4] ASoC: tda998x: add a codec to the HDMI transmitter

2015-01-07 Thread Andrew Jackson
On 01/07/15 10:51, Jean-Francois Moine wrote:
> This patch adds a CODEC to the NXP TDA998x HDMI transmitter.
> 
> The CODEC handles both I2S and S/PDIF inputs.
> It maintains the audio format and rate constraints according
> to the HDMI device parameters (EDID) and sets dynamically the input
> ports in the TDA998x I2C driver on start/stop audio streaming.
> 
> Signed-off-by: Jean-Francois Moine 
> ---
>  drivers/gpu/drm/i2c/Kconfig   |   1 +
>  drivers/gpu/drm/i2c/tda998x_drv.c | 139 --
>  include/sound/tda998x.h   |  23 +
>  sound/soc/codecs/Kconfig  |   4 +
>  sound/soc/codecs/Makefile |   2 +
>  sound/soc/codecs/tda998x.c| 175 
> ++
>  6 files changed, 339 insertions(+), 5 deletions(-)
>  create mode 100644 include/sound/tda998x.h
>  create mode 100644 sound/soc/codecs/tda998x.c
> 
> diff --git a/drivers/gpu/drm/i2c/Kconfig b/drivers/gpu/drm/i2c/Kconfig
> index 22c7ed6..24fc975 100644
> --- a/drivers/gpu/drm/i2c/Kconfig
> +++ b/drivers/gpu/drm/i2c/Kconfig
> @@ -28,6 +28,7 @@ config DRM_I2C_SIL164
>  config DRM_I2C_NXP_TDA998X
> tristate "NXP Semiconductors TDA998X HDMI encoder"
> default m if DRM_TILCDC
> +   select SND_SOC_TDA998X

Do you need this since sound/soc/codecs/Kconfig conditionally brings in the
CODEC driver?

> help
>   Support for NXP Semiconductors TDA998X HDMI encoders.
> 
> diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c 
> b/drivers/gpu/drm/i2c/tda998x_drv.c
> index ce1478d..a26a516 100644
> --- a/drivers/gpu/drm/i2c/tda998x_drv.c
> +++ b/drivers/gpu/drm/i2c/tda998x_drv.c
> @@ -27,6 +27,11 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +
> +#if defined(CONFIG_SND_SOC_TDA998X) || defined(CONFIG_SND_SOC_TDA998X_MODULE)
> +#define WITH_CODEC 1
> +#endif
> 
>  #define DBG(fmt, ...) DRM_DEBUG(fmt"\n", ##__VA_ARGS__)
> 
> @@ -47,6 +52,10 @@ struct tda998x_priv {
> struct drm_encoder *encoder;
> 
> u8 audio_ports[2];
> +#ifdef WITH_CODEC
> +   u8 sad[3];  /* Short Audio Descriptor */
> +   struct snd_pcm_hw_constraint_list rate_constraints;
> +#endif
>  };
> 
>  #define to_tda998x_priv(x)  ((struct tda998x_priv 
> *)to_encoder_slave(x)->slave_priv)
> @@ -730,11 +739,109 @@ tda998x_configure_audio(struct tda998x_priv *priv,
> tda998x_write_aif(priv, p);
>  }
> 
> +#ifdef WITH_CODEC
> +/* tda998x audio codec interface */
> +
> +/* return the audio parameters extracted from the last EDID */
> +static int tda998x_get_audio_var(struct device *dev,
> +   u8 **sad,
> +   struct snd_pcm_hw_constraint_list **rate_constraints)
> +{
> +   struct tda998x_priv *priv = dev_get_drvdata(dev);
> +
> +   if (!priv->encoder->crtc
> +|| !priv->is_hdmi_sink)
> +   return -ENODEV;
> +
> +   *sad = priv->sad;
> +   *rate_constraints = >rate_constraints;
> +   return 0;
> +}
> +
> +/* switch the audio port and initialize the audio parameters for streaming */
> +static int tda998x_set_audio_input(struct device *dev,
> +   int port_index,
> +   unsigned sample_rate,
> +   int sample_format)
> +{
> +   struct tda998x_priv *priv = dev_get_drvdata(dev);
> +   struct tda998x_encoder_params *p = >params;
> +
> +   if (!priv->encoder->crtc)
> +   return -ENODEV;
> +
> +   /* if no port, just disable the audio port */
> +   if (port_index == PORT_NONE) {
> +   reg_write(priv, REG_ENA_AP, 0);
> +   return 0;
> +   }
> +
> +   /* if same audio parameters, just enable the audio port */
> +   if (p->audio_cfg == priv->audio_ports[port_index] &&
> +   p->audio_sample_rate == sample_rate) {
> +   reg_write(priv, REG_ENA_AP, p->audio_cfg);
> +   return 0;
> +   }
> +
> +   p->audio_format = port_index == PORT_SPDIF ? AFMT_SPDIF : AFMT_I2S;
> +   p->audio_clk_cfg = port_index == PORT_SPDIF ? 0 : 1;
> +   p->audio_cfg = priv->audio_ports[port_index];
> +   p->audio_sample_rate = sample_rate;
> +   tda998x_configure_audio(priv, >encoder->crtc->hwmode, p);
> +   return 0;
> +}
> +
> +/* get the audio capabilities from the EDID */
> +static void tda998x_get_audio_caps(struct tda998x_priv *priv,
> +   struct drm_connector *connector)
> +{
> +   u8 *eld = connector->eld;
> +   u8 *sad;
> +   int sad_count;
> +   unsigned eld_ver, mnl;
> +   u8 max_channels, rate_mask, fmt;
> +
> +   /* adjust the hw params from the ELD (EDID) */
> +   eld_ver = eld[0] >> 3;
> +   if (eld_ver != 2 && eld_ver != 31)
> +   return;
> +
> +   mnl = eld[4] & 0x1f;
> +   if (mnl > 16)
> +   return;
> +
> +   sad_count = eld[5] >> 4;
> +   sad = eld + 20 + mnl;
> +
> + 

Re: [PATCH v9 1/4] drm/i2c: tda998x: Add DT support for audio

2015-01-07 Thread Andrew Jackson
On 01/07/15 09:10, Jean-Francois Moine wrote:
> This patch permits the definition of the audio ports from the device tree.
> 
> Signed-off-by: Jean-Francois Moine 
> ---
>  .../devicetree/bindings/drm/i2c/tda998x.txt| 18 +++
>  drivers/gpu/drm/i2c/tda998x_drv.c  | 60 
> ++
>  2 files changed, 69 insertions(+), 9 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/drm/i2c/tda998x.txt 
> b/Documentation/devicetree/bindings/drm/i2c/tda998x.txt
> index e9e4bce..e50e7cd 100644
> --- a/Documentation/devicetree/bindings/drm/i2c/tda998x.txt
> +++ b/Documentation/devicetree/bindings/drm/i2c/tda998x.txt
> @@ -17,6 +17,20 @@ Optional properties:
>- video-ports: 24 bits value which defines how the video controller
>   output is wired to the TDA998x input - default: <0x230145>
>  
> +  - audio-ports: must contain one or two values selecting the source
> + in the audio port.
> + The source type is given by the corresponding entry in
> + the audio-port-names property.

I think that this entry might benefit from a little more explanation.
The value specified here selects which pins on the chip provide the
audio input doesn't it?  In the outline datasheet that I have these are
listed in table 17:

Audio port  Input configuration
S/PDIF  I2S-bus
AP0 -   WS (word select)
AP1 S/PDIF inputI2S-bus channel 0
AP2 S/PDIF inputI2S-bus channel 1
AP3[1]  I2S-bus channel 2
AP4[1]  I2S-bus channel 3
ACLK-   SCK (I2S-bus clock)

[1] Depending on package.

Andrew

> +
> +  - audio-port-names: must contain entries matching the entries in
> + the audio-ports property.
> + Each value may be "i2s" or "spdif", giving the type of
> + the audio source.
> +
> +  - #sound-dai-cells: must be set to <1> for use with the simple-card.
> + The TDA998x audio CODEC always defines two DAIs.
> + The DAI 0 is the S/PDIF input and the DAI 1 is the I2S input.
> +
>  Example:
>  
>   tda998x: hdmi-encoder {
> @@ -26,4 +40,8 @@ Example:
>   interrupts = <27 2>;/* falling edge */
>   pinctrl-0 = <_camera>;
>   pinctrl-names = "default";
> +
> + audio-ports = <0x04>, <0x03>;
> + audio-port-names = "spdif", "i2s";
> + #sound-dai-cells = <1>;
>   };
> diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c 
> b/drivers/gpu/drm/i2c/tda998x_drv.c
> index 70658af..9d9b072 100644
> --- a/drivers/gpu/drm/i2c/tda998x_drv.c
> +++ b/drivers/gpu/drm/i2c/tda998x_drv.c
> @@ -20,6 +20,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  #include 
> @@ -44,6 +45,8 @@ struct tda998x_priv {
>   wait_queue_head_t wq_edid;
>   volatile int wq_edid_wait;
>   struct drm_encoder *encoder;
> +
> + u8 audio_ports[2];
>  };
>  
>  #define to_tda998x_priv(x)  ((struct tda998x_priv 
> *)to_encoder_slave(x)->slave_priv)
> @@ -1254,12 +1257,16 @@ static int tda998x_create(struct i2c_client *client, 
> struct tda998x_priv *priv)
>  {
>   struct device_node *np = client->dev.of_node;
>   u32 video;
> - int rev_lo, rev_hi, ret;
> + int i, rev_lo, rev_hi, ret;
> + const char *p;
>  
>   priv->vip_cntrl_0 = VIP_CNTRL_0_SWAP_A(2) | VIP_CNTRL_0_SWAP_B(3);
>   priv->vip_cntrl_1 = VIP_CNTRL_1_SWAP_C(0) | VIP_CNTRL_1_SWAP_D(1);
>   priv->vip_cntrl_2 = VIP_CNTRL_2_SWAP_E(4) | VIP_CNTRL_2_SWAP_F(5);
>  
> + priv->params.audio_frame[1] = 1;/* channels - 1 */
> + priv->params.audio_sample_rate = 48000; /* 48kHz */
> +
>   priv->current_page = 0xff;
>   priv->hdmi = client;
>   priv->cec = i2c_new_dummy(client->adapter, 0x34);
> @@ -1351,15 +1358,50 @@ static int tda998x_create(struct i2c_client *client, 
> struct tda998x_priv *priv)
>   /* enable EDID read irq: */
>   reg_set(priv, REG_INT_FLAGS_2, INT_FLAGS_2_EDID_BLK_RD);
>  
> - if (!np)
> - return 0;   /* non-DT */
> + /* get the device tree parameters */
> + if (np) {
> +
> + /* optional video properties */
> + ret = of_property_read_u32(np, "video-ports", );
> + if (ret == 0) {
> + priv->vip_cntrl_0 = video >> 16;
> + priv->vip_cntrl_1 = video >> 8;
> + priv->vip_cntrl_2 = video;
> + }
> +
> + /* optional audio properties */
> + for (i = 0; i < 2; i++) {
> + u32 port;
>  
> - /* get the optional video properties */
> - ret = of_property_read_u32(np, "video-ports", );
> - if (ret == 0) {
> - priv->vip_cntrl_0 = video >> 16;
> - priv->vip_cntrl_1 = video >> 8;
> - priv->vip_cntrl_2 = video;
> + ret = of_property_read_u32_index(np, 

Re: [PATCH v9 3/4] ASoC: tda998x: add a codec to the HDMI transmitter

2015-01-07 Thread Andrew Jackson
On 01/07/15 10:51, Jean-Francois Moine wrote:
 This patch adds a CODEC to the NXP TDA998x HDMI transmitter.
 
 The CODEC handles both I2S and S/PDIF inputs.
 It maintains the audio format and rate constraints according
 to the HDMI device parameters (EDID) and sets dynamically the input
 ports in the TDA998x I2C driver on start/stop audio streaming.
 
 Signed-off-by: Jean-Francois Moine moin...@free.fr
 ---
  drivers/gpu/drm/i2c/Kconfig   |   1 +
  drivers/gpu/drm/i2c/tda998x_drv.c | 139 --
  include/sound/tda998x.h   |  23 +
  sound/soc/codecs/Kconfig  |   4 +
  sound/soc/codecs/Makefile |   2 +
  sound/soc/codecs/tda998x.c| 175 
 ++
  6 files changed, 339 insertions(+), 5 deletions(-)
  create mode 100644 include/sound/tda998x.h
  create mode 100644 sound/soc/codecs/tda998x.c
 
 diff --git a/drivers/gpu/drm/i2c/Kconfig b/drivers/gpu/drm/i2c/Kconfig
 index 22c7ed6..24fc975 100644
 --- a/drivers/gpu/drm/i2c/Kconfig
 +++ b/drivers/gpu/drm/i2c/Kconfig
 @@ -28,6 +28,7 @@ config DRM_I2C_SIL164
  config DRM_I2C_NXP_TDA998X
 tristate NXP Semiconductors TDA998X HDMI encoder
 default m if DRM_TILCDC
 +   select SND_SOC_TDA998X

Do you need this since sound/soc/codecs/Kconfig conditionally brings in the
CODEC driver?

 help
   Support for NXP Semiconductors TDA998X HDMI encoders.
 
 diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c 
 b/drivers/gpu/drm/i2c/tda998x_drv.c
 index ce1478d..a26a516 100644
 --- a/drivers/gpu/drm/i2c/tda998x_drv.c
 +++ b/drivers/gpu/drm/i2c/tda998x_drv.c
 @@ -27,6 +27,11 @@
  #include drm/drm_encoder_slave.h
  #include drm/drm_edid.h
  #include drm/i2c/tda998x.h
 +#include sound/tda998x.h
 +
 +#if defined(CONFIG_SND_SOC_TDA998X) || defined(CONFIG_SND_SOC_TDA998X_MODULE)
 +#define WITH_CODEC 1
 +#endif
 
  #define DBG(fmt, ...) DRM_DEBUG(fmt\n, ##__VA_ARGS__)
 
 @@ -47,6 +52,10 @@ struct tda998x_priv {
 struct drm_encoder *encoder;
 
 u8 audio_ports[2];
 +#ifdef WITH_CODEC
 +   u8 sad[3];  /* Short Audio Descriptor */
 +   struct snd_pcm_hw_constraint_list rate_constraints;
 +#endif
  };
 
  #define to_tda998x_priv(x)  ((struct tda998x_priv 
 *)to_encoder_slave(x)-slave_priv)
 @@ -730,11 +739,109 @@ tda998x_configure_audio(struct tda998x_priv *priv,
 tda998x_write_aif(priv, p);
  }
 
 +#ifdef WITH_CODEC
 +/* tda998x audio codec interface */
 +
 +/* return the audio parameters extracted from the last EDID */
 +static int tda998x_get_audio_var(struct device *dev,
 +   u8 **sad,
 +   struct snd_pcm_hw_constraint_list **rate_constraints)
 +{
 +   struct tda998x_priv *priv = dev_get_drvdata(dev);
 +
 +   if (!priv-encoder-crtc
 +|| !priv-is_hdmi_sink)
 +   return -ENODEV;
 +
 +   *sad = priv-sad;
 +   *rate_constraints = priv-rate_constraints;
 +   return 0;
 +}
 +
 +/* switch the audio port and initialize the audio parameters for streaming */
 +static int tda998x_set_audio_input(struct device *dev,
 +   int port_index,
 +   unsigned sample_rate,
 +   int sample_format)
 +{
 +   struct tda998x_priv *priv = dev_get_drvdata(dev);
 +   struct tda998x_encoder_params *p = priv-params;
 +
 +   if (!priv-encoder-crtc)
 +   return -ENODEV;
 +
 +   /* if no port, just disable the audio port */
 +   if (port_index == PORT_NONE) {
 +   reg_write(priv, REG_ENA_AP, 0);
 +   return 0;
 +   }
 +
 +   /* if same audio parameters, just enable the audio port */
 +   if (p-audio_cfg == priv-audio_ports[port_index] 
 +   p-audio_sample_rate == sample_rate) {
 +   reg_write(priv, REG_ENA_AP, p-audio_cfg);
 +   return 0;
 +   }
 +
 +   p-audio_format = port_index == PORT_SPDIF ? AFMT_SPDIF : AFMT_I2S;
 +   p-audio_clk_cfg = port_index == PORT_SPDIF ? 0 : 1;
 +   p-audio_cfg = priv-audio_ports[port_index];
 +   p-audio_sample_rate = sample_rate;
 +   tda998x_configure_audio(priv, priv-encoder-crtc-hwmode, p);
 +   return 0;
 +}
 +
 +/* get the audio capabilities from the EDID */
 +static void tda998x_get_audio_caps(struct tda998x_priv *priv,
 +   struct drm_connector *connector)
 +{
 +   u8 *eld = connector-eld;
 +   u8 *sad;
 +   int sad_count;
 +   unsigned eld_ver, mnl;
 +   u8 max_channels, rate_mask, fmt;
 +
 +   /* adjust the hw params from the ELD (EDID) */
 +   eld_ver = eld[0]  3;
 +   if (eld_ver != 2  eld_ver != 31)
 +   return;
 +
 +   mnl = eld[4]  0x1f;
 +   if (mnl  16)
 +   return;
 +
 +   sad_count = eld[5]  4;
 +   sad = eld + 20 + mnl;
 +
 +   /* Start from the basic audio settings */
 +   max_channels = 1;
 +

Re: [PATCH v9 1/4] drm/i2c: tda998x: Add DT support for audio

2015-01-07 Thread Andrew Jackson
On 01/07/15 09:10, Jean-Francois Moine wrote:
 This patch permits the definition of the audio ports from the device tree.
 
 Signed-off-by: Jean-Francois Moine moin...@free.fr
 ---
  .../devicetree/bindings/drm/i2c/tda998x.txt| 18 +++
  drivers/gpu/drm/i2c/tda998x_drv.c  | 60 
 ++
  2 files changed, 69 insertions(+), 9 deletions(-)
 
 diff --git a/Documentation/devicetree/bindings/drm/i2c/tda998x.txt 
 b/Documentation/devicetree/bindings/drm/i2c/tda998x.txt
 index e9e4bce..e50e7cd 100644
 --- a/Documentation/devicetree/bindings/drm/i2c/tda998x.txt
 +++ b/Documentation/devicetree/bindings/drm/i2c/tda998x.txt
 @@ -17,6 +17,20 @@ Optional properties:
- video-ports: 24 bits value which defines how the video controller
   output is wired to the TDA998x input - default: 0x230145
  
 +  - audio-ports: must contain one or two values selecting the source
 + in the audio port.
 + The source type is given by the corresponding entry in
 + the audio-port-names property.

I think that this entry might benefit from a little more explanation.
The value specified here selects which pins on the chip provide the
audio input doesn't it?  In the outline datasheet that I have these are
listed in table 17:

Audio port  Input configuration
S/PDIF  I2S-bus
AP0 -   WS (word select)
AP1 S/PDIF inputI2S-bus channel 0
AP2 S/PDIF inputI2S-bus channel 1
AP3[1]  I2S-bus channel 2
AP4[1]  I2S-bus channel 3
ACLK-   SCK (I2S-bus clock)

[1] Depending on package.

Andrew

 +
 +  - audio-port-names: must contain entries matching the entries in
 + the audio-ports property.
 + Each value may be i2s or spdif, giving the type of
 + the audio source.
 +
 +  - #sound-dai-cells: must be set to 1 for use with the simple-card.
 + The TDA998x audio CODEC always defines two DAIs.
 + The DAI 0 is the S/PDIF input and the DAI 1 is the I2S input.
 +
  Example:
  
   tda998x: hdmi-encoder {
 @@ -26,4 +40,8 @@ Example:
   interrupts = 27 2;/* falling edge */
   pinctrl-0 = pmx_camera;
   pinctrl-names = default;
 +
 + audio-ports = 0x04, 0x03;
 + audio-port-names = spdif, i2s;
 + #sound-dai-cells = 1;
   };
 diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c 
 b/drivers/gpu/drm/i2c/tda998x_drv.c
 index 70658af..9d9b072 100644
 --- a/drivers/gpu/drm/i2c/tda998x_drv.c
 +++ b/drivers/gpu/drm/i2c/tda998x_drv.c
 @@ -20,6 +20,7 @@
  #include linux/module.h
  #include linux/irq.h
  #include sound/asoundef.h
 +#include linux/platform_device.h
  
  #include drm/drmP.h
  #include drm/drm_crtc_helper.h
 @@ -44,6 +45,8 @@ struct tda998x_priv {
   wait_queue_head_t wq_edid;
   volatile int wq_edid_wait;
   struct drm_encoder *encoder;
 +
 + u8 audio_ports[2];
  };
  
  #define to_tda998x_priv(x)  ((struct tda998x_priv 
 *)to_encoder_slave(x)-slave_priv)
 @@ -1254,12 +1257,16 @@ static int tda998x_create(struct i2c_client *client, 
 struct tda998x_priv *priv)
  {
   struct device_node *np = client-dev.of_node;
   u32 video;
 - int rev_lo, rev_hi, ret;
 + int i, rev_lo, rev_hi, ret;
 + const char *p;
  
   priv-vip_cntrl_0 = VIP_CNTRL_0_SWAP_A(2) | VIP_CNTRL_0_SWAP_B(3);
   priv-vip_cntrl_1 = VIP_CNTRL_1_SWAP_C(0) | VIP_CNTRL_1_SWAP_D(1);
   priv-vip_cntrl_2 = VIP_CNTRL_2_SWAP_E(4) | VIP_CNTRL_2_SWAP_F(5);
  
 + priv-params.audio_frame[1] = 1;/* channels - 1 */
 + priv-params.audio_sample_rate = 48000; /* 48kHz */
 +
   priv-current_page = 0xff;
   priv-hdmi = client;
   priv-cec = i2c_new_dummy(client-adapter, 0x34);
 @@ -1351,15 +1358,50 @@ static int tda998x_create(struct i2c_client *client, 
 struct tda998x_priv *priv)
   /* enable EDID read irq: */
   reg_set(priv, REG_INT_FLAGS_2, INT_FLAGS_2_EDID_BLK_RD);
  
 - if (!np)
 - return 0;   /* non-DT */
 + /* get the device tree parameters */
 + if (np) {
 +
 + /* optional video properties */
 + ret = of_property_read_u32(np, video-ports, video);
 + if (ret == 0) {
 + priv-vip_cntrl_0 = video  16;
 + priv-vip_cntrl_1 = video  8;
 + priv-vip_cntrl_2 = video;
 + }
 +
 + /* optional audio properties */
 + for (i = 0; i  2; i++) {
 + u32 port;
  
 - /* get the optional video properties */
 - ret = of_property_read_u32(np, video-ports, video);
 - if (ret == 0) {
 - priv-vip_cntrl_0 = video  16;
 - priv-vip_cntrl_1 = video  8;
 - priv-vip_cntrl_2 = video;
 + ret = of_property_read_u32_index(np, audio-ports, i, 
 port);
 + if 

Re: [PATCH v9 1/4] drm/i2c: tda998x: Add DT support for audio

2015-01-07 Thread Andrew Jackson
On 01/07/15 17:08, Jean-Francois Moine wrote:
 On Wed, 07 Jan 2015 14:39:13 +
 Andrew Jackson andrew.jack...@arm.com wrote:
 
 +  - audio-ports: must contain one or two values selecting the source
 +   in the audio port.
 +   The source type is given by the corresponding entry in
 +   the audio-port-names property.  

 I think that this entry might benefit from a little more explanation.
 The value specified here selects which pins on the chip provide the
 audio input doesn't it?  In the outline datasheet that I have these are
 listed in table 17:

 Audio port   Input configuration
  S/PDIF  I2S-bus
 AP0  -   WS (word select)
 AP1  S/PDIF inputI2S-bus channel 0
 AP2  S/PDIF inputI2S-bus channel 1
 AP3[1]   I2S-bus channel 2
 AP4[1]   I2S-bus channel 3
 ACLK -   SCK (I2S-bus clock)

 [1] Depending on package.
 
 Your table is close to the one in the TDA9983B documentation I have,
 but the pins are not exactly the same:
 
 AP0   WS (word select)
 AP1   I2S-bus port 0
 AP2   I2S-bus port 1
 AP3   I2S-bus port 2
 AP4   I2S-bus port 3
 AP5   MCLK (master clock for S/PDIF)
 AP6   S/PDIF input
 AP7   AUX (internal test)
 ACLK  SCK (I2S-bus clock)
 
 That's why I did not know clearly why I had to set AP2 for S/PDIF input
 and (AP0 + AP1) for I2S input in the Cubox.
 
 Then, the only more explanation I could give is have a look at the
 audio input format and at the register 0x1e page 0 in the documentation
 of the TDA998x chip.
 
 BTW, the tda998x driver supports only the TDA9989, TDA19988 and
 TDA19989 chips. If the TDA9983B would be supported, the audio port
 definitions would be of no use.
 
 So, what would you see as an explanation?
 

I understand your difficulty!  I was just wanting something to clarify the 
meaning of the value without reference to the driver source.

You could add something like this to your existing explanation: The value
describes which audio input pins are selected; this varies depending
on chip type so consult the section on audio port configuration in the 
relevant datasheet..  

Andrew

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] ASoC: adi: Add missing return statement.

2014-12-31 Thread Andrew Jackson
From: Andrew Jackson 

The probe routine was disabling the clock even
if the system was configured successfully.  Add
a return statement to leave clocks enabled.

Signed-off-by: Andrew Jackson 
---
Spotted while reviewing clock preparation

 sound/soc/adi/axi-i2s.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/sound/soc/adi/axi-i2s.c b/sound/soc/adi/axi-i2s.c
index 7752860..4c23381 100644
--- a/sound/soc/adi/axi-i2s.c
+++ b/sound/soc/adi/axi-i2s.c
@@ -240,6 +240,8 @@ static int axi_i2s_probe(struct platform_device *pdev)
if (ret)
goto err_clk_disable;
 
+   return 0;
+
 err_clk_disable:
clk_disable_unprepare(i2s->clk);
return ret;
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] ASoC: dwc: Remove unnecessary conditional compilation

2014-12-31 Thread Andrew Jackson
From: Andrew Jackson 

of_match_ptr is already conditionally compiled based on
CONFIG_OF so further conditional compilation is not
required.  Remove conditional compilation surrounding
of_match_ptr.

Signed-off-by: Andrew Jackson 
---
 sound/soc/dwc/designware_i2s.c |2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
index 99cf64b..a3e97b4 100644
--- a/sound/soc/dwc/designware_i2s.c
+++ b/sound/soc/dwc/designware_i2s.c
@@ -624,9 +624,7 @@ static struct platform_driver dw_i2s_driver = {
.remove = dw_i2s_remove,
.driver = {
.name   = "designware-i2s",
-#ifdef CONFIG_OF
.of_match_table = of_match_ptr(dw_i2s_of_match),
-#endif
},
 };
 
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] ASoC: dwc: Remove unnecessary conditional compilation

2014-12-31 Thread Andrew Jackson
From: Andrew Jackson andrew.jack...@arm.com

of_match_ptr is already conditionally compiled based on
CONFIG_OF so further conditional compilation is not
required.  Remove conditional compilation surrounding
of_match_ptr.

Signed-off-by: Andrew Jackson andrew.jack...@arm.com
---
 sound/soc/dwc/designware_i2s.c |2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
index 99cf64b..a3e97b4 100644
--- a/sound/soc/dwc/designware_i2s.c
+++ b/sound/soc/dwc/designware_i2s.c
@@ -624,9 +624,7 @@ static struct platform_driver dw_i2s_driver = {
.remove = dw_i2s_remove,
.driver = {
.name   = designware-i2s,
-#ifdef CONFIG_OF
.of_match_table = of_match_ptr(dw_i2s_of_match),
-#endif
},
 };
 
-- 
1.7.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] ASoC: adi: Add missing return statement.

2014-12-31 Thread Andrew Jackson
From: Andrew Jackson andrew.jack...@arm.com

The probe routine was disabling the clock even
if the system was configured successfully.  Add
a return statement to leave clocks enabled.

Signed-off-by: Andrew Jackson andrew.jack...@arm.com
---
Spotted while reviewing clock preparation

 sound/soc/adi/axi-i2s.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/sound/soc/adi/axi-i2s.c b/sound/soc/adi/axi-i2s.c
index 7752860..4c23381 100644
--- a/sound/soc/adi/axi-i2s.c
+++ b/sound/soc/adi/axi-i2s.c
@@ -240,6 +240,8 @@ static int axi_i2s_probe(struct platform_device *pdev)
if (ret)
goto err_clk_disable;
 
+   return 0;
+
 err_clk_disable:
clk_disable_unprepare(i2s-clk);
return ret;
-- 
1.7.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v5 1/6] ASoC: dwc: Switch to managed clock resource

2014-12-30 Thread Andrew Jackson
From: Andrew Jackson 

Simplify error handling during probe by using managed clock
resources.

Signed-off-by: Andrew Jackson 
---
 sound/soc/dwc/designware_i2s.c |   10 ++
 1 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
index 06d3a34..5e9d163 100644
--- a/sound/soc/dwc/designware_i2s.c
+++ b/sound/soc/dwc/designware_i2s.c
@@ -407,13 +407,13 @@ static int dw_i2s_probe(struct platform_device *pdev)
 
dev->capability = pdata->cap;
dev->i2s_clk_cfg = pdata->i2s_clk_cfg;
-   dev->clk = clk_get(>dev, NULL);
+   dev->clk = devm_clk_get(>dev, NULL);
if (IS_ERR(dev->clk))
return  PTR_ERR(dev->clk);
 
ret = clk_enable(dev->clk);
if (ret < 0)
-   goto err_clk_put;
+   return ret;
 
dev_set_drvdata(>dev, dev);
ret = snd_soc_register_component(>dev, _i2s_component,
@@ -427,19 +427,13 @@ static int dw_i2s_probe(struct platform_device *pdev)
 
 err_clk_disable:
clk_disable(dev->clk);
-err_clk_put:
-   clk_put(dev->clk);
return ret;
 }
 
 static int dw_i2s_remove(struct platform_device *pdev)
 {
-   struct dw_i2s_dev *dev = dev_get_drvdata(>dev);
-
snd_soc_unregister_component(>dev);
 
-   clk_put(dev->clk);
-
return 0;
 }
 
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v5 6/6] ASoC: dwc: Add devicetree support for Designware I2S

2014-12-30 Thread Andrew Jackson
From: Andrew Jackson 

Allow the driver to be configured through a device tree rather than platform
data.

Signed-off-by: Andrew Jackson 
---
 sound/soc/dwc/Kconfig  |1 +
 sound/soc/dwc/designware_i2s.c |  193 +++-
 2 files changed, 151 insertions(+), 43 deletions(-)

diff --git a/sound/soc/dwc/Kconfig b/sound/soc/dwc/Kconfig
index e334900..d50e085 100644
--- a/sound/soc/dwc/Kconfig
+++ b/sound/soc/dwc/Kconfig
@@ -1,6 +1,7 @@
 config SND_DESIGNWARE_I2S
tristate "Synopsys I2S Device Driver"
depends on CLKDEV_LOOKUP
+   select SND_SOC_GENERIC_DMAENGINE_PCM
help
 Say Y or M if you want to add support for I2S driver for
 Synopsys desigwnware I2S device. The device supports upto
diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
index e1c0e2c..99cf64b 100644
--- a/sound/soc/dwc/designware_i2s.c
+++ b/sound/soc/dwc/designware_i2s.c
@@ -22,6 +22,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /* common register for all channel */
 #define IER0x000
@@ -82,6 +83,11 @@
 #define MAX_CHANNEL_NUM8
 #define MIN_CHANNEL_NUM2
 
+union dw_i2s_snd_dma_data {
+   struct i2s_dma_data pd;
+   struct snd_dmaengine_dai_dma_data dt;
+};
+
 struct dw_i2s_dev {
void __iomem *i2s_base;
struct clk *clk;
@@ -90,8 +96,8 @@ struct dw_i2s_dev {
struct device *dev;
 
/* data related to DMA transfers b/w i2s and DMAC */
-   struct i2s_dma_data play_dma_data;
-   struct i2s_dma_data capture_dma_data;
+   union dw_i2s_snd_dma_data play_dma_data;
+   union dw_i2s_snd_dma_data capture_dma_data;
struct i2s_clk_config_data config;
int (*i2s_clk_cfg)(struct i2s_clk_config_data *config);
 };
@@ -178,7 +184,7 @@ static int dw_i2s_startup(struct snd_pcm_substream 
*substream,
struct snd_soc_dai *cpu_dai)
 {
struct dw_i2s_dev *dev = snd_soc_dai_get_drvdata(cpu_dai);
-   struct i2s_dma_data *dma_data = NULL;
+   union dw_i2s_snd_dma_data *dma_data = NULL;
 
if (!(dev->capability & DWC_I2S_RECORD) &&
(substream->stream == SNDRV_PCM_STREAM_CAPTURE))
@@ -267,13 +273,21 @@ static int dw_i2s_hw_params(struct snd_pcm_substream 
*substream,
 
config->sample_rate = params_rate(params);
 
-   if (!dev->i2s_clk_cfg)
-   return -EINVAL;
+   if (dev->i2s_clk_cfg) {
+   ret = dev->i2s_clk_cfg(config);
+   if (ret < 0) {
+   dev_err(dev->dev, "runtime audio clk config fail\n");
+   return ret;
+   }
+   } else {
+   u32 bitclk = config->sample_rate * config->data_width * 2;
 
-   ret = dev->i2s_clk_cfg(config);
-   if (ret < 0) {
-   dev_err(dev->dev, "runtime audio clk config fail\n");
-   return ret;
+   ret = clk_set_rate(dev->clk, bitclk);
+   if (ret) {
+   dev_err(dev->dev, "Can't set I2S clock rate: %d\n",
+   ret);
+   return ret;
+   }
}
 
return 0;
@@ -368,6 +382,11 @@ static int dw_i2s_resume(struct snd_soc_dai *dai)
  * block parameter.
  */
 
+/* Maximum bit resolution of a channel - not uniformly spaced */
+static const u32 fifo_width[COMP_MAX_WORDSIZE] = {
+   12, 16, 20, 24, 32, 0, 0, 0
+};
+
 /* Width of (DMA) bus */
 static const u32 bus_widths[COMP_MAX_DATA_WIDTH] = {
DMA_SLAVE_BUSWIDTH_1_BYTE,
@@ -388,10 +407,9 @@ static const u32 formats[COMP_MAX_WORDSIZE] = {
0
 };
 
-static int dw_configure_dai_by_pd(struct dw_i2s_dev *dev,
+static int dw_configure_dai(struct dw_i2s_dev *dev,
   struct snd_soc_dai_driver *dw_i2s_dai,
-  struct resource *res,
-  const struct i2s_platform_data *pdata)
+  unsigned int rates)
 {
/*
 * Read component parameter registers to extract
@@ -399,23 +417,7 @@ static int dw_configure_dai_by_pd(struct dw_i2s_dev *dev,
 */
u32 comp1 = i2s_read_reg(dev->i2s_base, I2S_COMP_PARAM_1);
u32 comp2 = i2s_read_reg(dev->i2s_base, I2S_COMP_PARAM_2);
-   u32 idx = COMP1_APB_DATA_WIDTH(comp1);
-
-   if (WARN_ON(idx >= ARRAY_SIZE(bus_widths)))
-   return -EINVAL;
-
-   /* Set DMA slaves info */
-
-   dev->play_dma_data.data = pdata->play_dma_data;
-   dev->capture_dma_data.data = pdata->capture_dma_data;
-   dev->play_dma_data.addr = res->start + I2S_TXDMA;
-   dev->capture_dma_data.addr = res->start + I2S_RXDMA;
-   dev->play_dma_data.max_burst = 16;
-   dev->capture_dma_data.max_burst = 16;
-   dev-&g

[PATCH v5 3/6] ASoC: dwc: Read I2S block configuration from registers

2014-12-30 Thread Andrew Jackson
From: Andrew Jackson 

The I2S block provides component parameter registers which
describe how the block is instantiated.  Use these registers
to extract the block's configuration rather than relying on
platform data.

Signed-off-by: Andrew Jackson 
---
 sound/soc/dwc/designware_i2s.c |   96 +++
 1 files changed, 86 insertions(+), 10 deletions(-)

diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
index 08608c1..b3e7567 100644
--- a/sound/soc/dwc/designware_i2s.c
+++ b/sound/soc/dwc/designware_i2s.c
@@ -54,6 +54,31 @@
 #define I2S_COMP_VERSION   0x01F8
 #define I2S_COMP_TYPE  0x01FC
 
+/*
+ * Component parameter register fields - define the I2S block's
+ * configuration.
+ */
+#defineCOMP1_TX_WORDSIZE_3(r)  (((r) & GENMASK(27, 25)) >> 25)
+#defineCOMP1_TX_WORDSIZE_2(r)  (((r) & GENMASK(24, 22)) >> 22)
+#defineCOMP1_TX_WORDSIZE_1(r)  (((r) & GENMASK(21, 19)) >> 19)
+#defineCOMP1_TX_WORDSIZE_0(r)  (((r) & GENMASK(18, 16)) >> 16)
+#defineCOMP1_TX_CHANNELS(r)(((r) & GENMASK(10, 9)) >> 9)
+#defineCOMP1_RX_CHANNELS(r)(((r) & GENMASK(8, 7)) >> 7)
+#defineCOMP1_RX_ENABLED(r) (((r) & BIT(6)) >> 6)
+#defineCOMP1_TX_ENABLED(r) (((r) & BIT(5)) >> 5)
+#defineCOMP1_MODE_EN(r)(((r) & BIT(4)) >> 4)
+#defineCOMP1_FIFO_DEPTH_GLOBAL(r)  (((r) & GENMASK(3, 2)) >> 2)
+#defineCOMP1_APB_DATA_WIDTH(r) (((r) & GENMASK(1, 0)) >> 0)
+
+#defineCOMP2_RX_WORDSIZE_3(r)  (((r) & GENMASK(12, 10)) >> 10)
+#defineCOMP2_RX_WORDSIZE_2(r)  (((r) & GENMASK(9, 7)) >> 7)
+#defineCOMP2_RX_WORDSIZE_1(r)  (((r) & GENMASK(5, 3)) >> 3)
+#defineCOMP2_RX_WORDSIZE_0(r)  (((r) & GENMASK(2, 0)) >> 0)
+
+/* Number of entries in WORDSIZE and DATA_WIDTH parameter registers */
+#defineCOMP_MAX_WORDSIZE   (1 << 3)
+#defineCOMP_MAX_DATA_WIDTH (1 << 2)
+
 #define MAX_CHANNEL_NUM8
 #define MIN_CHANNEL_NUM2
 
@@ -335,11 +360,50 @@ static int dw_i2s_resume(struct snd_soc_dai *dai)
 #define dw_i2s_resume  NULL
 #endif
 
-static void dw_configure_dai_by_pd(struct dw_i2s_dev *dev,
+/*
+ * The following tables allow a direct lookup of various parameters
+ * defined in the I2S block's configuration in terms of sound system
+ * parameters.  Each table is sized to the number of entries possible
+ * according to the number of configuration bits describing an I2S
+ * block parameter.
+ */
+
+/* Width of (DMA) bus */
+static const u32 bus_widths[COMP_MAX_DATA_WIDTH] = {
+   DMA_SLAVE_BUSWIDTH_1_BYTE,
+   DMA_SLAVE_BUSWIDTH_2_BYTES,
+   DMA_SLAVE_BUSWIDTH_4_BYTES,
+   DMA_SLAVE_BUSWIDTH_UNDEFINED
+};
+
+/* PCM format to support channel resolution */
+static const u32 formats[COMP_MAX_WORDSIZE] = {
+   SNDRV_PCM_FMTBIT_S16_LE,
+   SNDRV_PCM_FMTBIT_S16_LE,
+   SNDRV_PCM_FMTBIT_S24_LE,
+   SNDRV_PCM_FMTBIT_S24_LE,
+   SNDRV_PCM_FMTBIT_S32_LE,
+   0,
+   0,
+   0
+};
+
+static int dw_configure_dai_by_pd(struct dw_i2s_dev *dev,
   struct snd_soc_dai_driver *dw_i2s_dai,
   struct resource *res,
   const struct i2s_platform_data *pdata)
 {
+   /*
+* Read component parameter registers to extract
+* the I2S block's configuration.
+*/
+   u32 comp1 = i2s_read_reg(dev->i2s_base, I2S_COMP_PARAM_1);
+   u32 comp2 = i2s_read_reg(dev->i2s_base, I2S_COMP_PARAM_2);
+   u32 idx = COMP1_APB_DATA_WIDTH(comp1);
+
+   if (WARN_ON(idx >= ARRAY_SIZE(bus_widths)))
+   return -EINVAL;
+
/* Set DMA slaves info */
 
dev->play_dma_data.data = pdata->play_dma_data;
@@ -348,26 +412,36 @@ static void dw_configure_dai_by_pd(struct dw_i2s_dev *dev,
dev->capture_dma_data.addr = res->start + I2S_RXDMA;
dev->play_dma_data.max_burst = 16;
dev->capture_dma_data.max_burst = 16;
-   dev->play_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
-   dev->capture_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
+   dev->play_dma_data.addr_width = bus_widths[idx];
+   dev->capture_dma_data.addr_width = bus_widths[idx];
dev->play_dma_data.filter = pdata->filter;
dev->capture_dma_data.filter = pdata->filter;
 
-   if (pdata->cap & DWC_I2S_PLAY) {
+   if (COMP1_TX_ENABLED(comp1)) {
dev_dbg(dev->dev, " designware: play supported\n");
+   idx = COMP1_TX_WORDSIZE_0(comp1);
+   if (WARN_ON(idx >= ARRAY_SIZE(formats)))
+   return -EINVAL;
dw_i2s_dai->playback.channels_m

[PATCH v5 5/6] ASoC: dwc: Add documentation for I2S DT

2014-12-30 Thread Andrew Jackson
From: Andrew Jackson 

Add documentation for Designware I2S hardware block.  The block requires
one clock (for audio sampling) and DMA channels for receive and transmit.

Signed-off-by: Andrew Jackson 
---
 .../devicetree/bindings/sound/designware-i2s.txt   |   31 
 1 files changed, 31 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sound/designware-i2s.txt

diff --git a/Documentation/devicetree/bindings/sound/designware-i2s.txt 
b/Documentation/devicetree/bindings/sound/designware-i2s.txt
new file mode 100644
index 000..7bb5424
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/designware-i2s.txt
@@ -0,0 +1,31 @@
+DesignWare I2S controller
+
+Required properties:
+ - compatible : Must be "snps,designware-i2s"
+ - reg : Must contain the I2S core's registers location and length
+ - clocks : Pairs of phandle and specifier referencing the controller's
+   clocks. The controller expects one clock: the clock used as the sampling
+   rate reference clock sample.
+ - clock-names : "i2sclk" for the sample rate reference clock.
+ - dmas: Pairs of phandle and specifier for the DMA channels that are used by
+   the core. The core expects one or two dma channels: one for transmit and
+   one for receive.
+ - dma-names : "tx" for the transmit channel, "rx" for the receive channel.
+
+For more details on the 'dma', 'dma-names', 'clock' and 'clock-names'
+properties please check:
+   * resource-names.txt
+   * clock/clock-bindings.txt
+   * dma/dma.txt
+
+Example:
+
+   soc_i2s: i2s@7ff9 {
+   compatible = "snps,designware-i2s";
+   reg = <0x0 0x7ff9 0x0 0x1000>;
+   clocks = <_i2sclk 0>;
+   clock-names = "i2sclk";
+   #sound-dai-cells = <0>;
+   dmas = < 5>;
+   dma-names = "tx";
+   };
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v5 2/6] ASoC: dwc: Prepare clock before use

2014-12-30 Thread Andrew Jackson
From: Andrew Jackson 

Some I2S clocks may require some time to get the clock ready
for operation and so need to be prepared before they are enabled.
So, prepare the clock as well as enabling it, but combine the
two through clk_prepare_enable.

Signed-off-by: Andrew Jackson 
---
 sound/soc/dwc/designware_i2s.c |7 +--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
index 5e9d163..08608c1 100644
--- a/sound/soc/dwc/designware_i2s.c
+++ b/sound/soc/dwc/designware_i2s.c
@@ -411,7 +411,7 @@ static int dw_i2s_probe(struct platform_device *pdev)
if (IS_ERR(dev->clk))
return  PTR_ERR(dev->clk);
 
-   ret = clk_enable(dev->clk);
+   ret = clk_prepare_enable(dev->clk);
if (ret < 0)
return ret;
 
@@ -426,13 +426,16 @@ static int dw_i2s_probe(struct platform_device *pdev)
return 0;
 
 err_clk_disable:
-   clk_disable(dev->clk);
+   clk_disable_unprepare(dev->clk);
return ret;
 }
 
 static int dw_i2s_remove(struct platform_device *pdev)
 {
+   struct dw_i2s_dev *dev = dev_get_drvdata(>dev);
+
snd_soc_unregister_component(>dev);
+   clk_disable_unprepare(dev->clk);
 
return 0;
 }
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v5 4/6] ASoC: dwc: Register components with managed interface

2014-12-30 Thread Andrew Jackson
From: Andrew Jackson 

Register SOC component using managed interface to
simplify error handling and future introduction of
device tree.

Signed-off-by: Andrew Jackson 
---
 sound/soc/dwc/designware_i2s.c |3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
index b3e7567..e1c0e2c 100644
--- a/sound/soc/dwc/designware_i2s.c
+++ b/sound/soc/dwc/designware_i2s.c
@@ -492,7 +492,7 @@ static int dw_i2s_probe(struct platform_device *pdev)
return ret;
 
dev_set_drvdata(>dev, dev);
-   ret = snd_soc_register_component(>dev, _i2s_component,
+   ret = devm_snd_soc_register_component(>dev, _i2s_component,
 dw_i2s_dai, 1);
if (ret != 0) {
dev_err(>dev, "not able to register dai\n");
@@ -510,7 +510,6 @@ static int dw_i2s_remove(struct platform_device *pdev)
 {
struct dw_i2s_dev *dev = dev_get_drvdata(>dev);
 
-   snd_soc_unregister_component(>dev);
clk_disable_unprepare(dev->clk);
 
return 0;
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v5 0/6] ASoC: dwc: Add device tree support to designware I2S

2014-12-30 Thread Andrew Jackson
From: Andrew Jackson 

This patch set extends the DesignWare I2S driver to provide device
tree support and fixes a couple of small faults.

Changes v4->v5
  + Remove all clk_put calls [Andrew Jackson]
  + Call clk_disable_unprepare [Andrew Jackson]

Changes v3->v4
  + Drop applied patches
  + Use combined function to prepare clock [Mark Brown]
  + Use managed clock resources to avoid clk_put [Mark Brown]
  + Read configuration prameters from hardware for both platform data
and device tree [Mark Brown]
  + Re-order patch sequence [Mark Brown]
  + Change union name to avoid future collisions [Mark Brown]
  + Check return code from clk_set_rate [Mark Brown]
  + Check parametsrs read from hardware agaist array limits and
add further comments [Mark Brown]
  + Add of_match_ptr [Mark Brown]

Changes v2->v3
  + Drop applied patch
  + Flush FIFOs in prepare rather than hw_params [Lars-Peter Clausen]

Changes v1->v2
  + Drop negative use count patch [Mark Brown]
  + Remove unnecessary debug print messages [Lars-Peter Clausen]
  + Rewrite iteration as for loop rather than do...while [Mark Brown]
  + Reorder patches to send fixes first [Mark Brown]
  + Simplify device tree code [Mark Brown]
  + Split device tree patch in two [Mark Brown]
  + Expand explanatory comment on channel configuration [Rajeev Kumar]

Arnd: I've not forgotten about updating the spear entries and
  will submit as a separate patch.

Andrew Jackson (6):
  ASoC: dwc: Switch to managed clock resource
  ASoC: dwc: Prepare clock before use
  ASoC: dwc: Read I2S block configuration from registers
  ASoC: dwc: Register components with managed interface
  ASoC: dwc: Add documentation for I2S DT
  ASoC: dwc: Add devicetree support for Designware I2S

 .../devicetree/bindings/sound/designware-i2s.txt   |   31 +++
 sound/soc/dwc/Kconfig  |1 +
 sound/soc/dwc/designware_i2s.c |  283 
 3 files changed, 263 insertions(+), 52 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sound/designware-i2s.txt

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v5 4/6] ASoC: dwc: Register components with managed interface

2014-12-30 Thread Andrew Jackson
From: Andrew Jackson andrew.jack...@arm.com

Register SOC component using managed interface to
simplify error handling and future introduction of
device tree.

Signed-off-by: Andrew Jackson andrew.jack...@arm.com
---
 sound/soc/dwc/designware_i2s.c |3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
index b3e7567..e1c0e2c 100644
--- a/sound/soc/dwc/designware_i2s.c
+++ b/sound/soc/dwc/designware_i2s.c
@@ -492,7 +492,7 @@ static int dw_i2s_probe(struct platform_device *pdev)
return ret;
 
dev_set_drvdata(pdev-dev, dev);
-   ret = snd_soc_register_component(pdev-dev, dw_i2s_component,
+   ret = devm_snd_soc_register_component(pdev-dev, dw_i2s_component,
 dw_i2s_dai, 1);
if (ret != 0) {
dev_err(pdev-dev, not able to register dai\n);
@@ -510,7 +510,6 @@ static int dw_i2s_remove(struct platform_device *pdev)
 {
struct dw_i2s_dev *dev = dev_get_drvdata(pdev-dev);
 
-   snd_soc_unregister_component(pdev-dev);
clk_disable_unprepare(dev-clk);
 
return 0;
-- 
1.7.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v5 0/6] ASoC: dwc: Add device tree support to designware I2S

2014-12-30 Thread Andrew Jackson
From: Andrew Jackson andrew.jack...@arm.com

This patch set extends the DesignWare I2S driver to provide device
tree support and fixes a couple of small faults.

Changes v4-v5
  + Remove all clk_put calls [Andrew Jackson]
  + Call clk_disable_unprepare [Andrew Jackson]

Changes v3-v4
  + Drop applied patches
  + Use combined function to prepare clock [Mark Brown]
  + Use managed clock resources to avoid clk_put [Mark Brown]
  + Read configuration prameters from hardware for both platform data
and device tree [Mark Brown]
  + Re-order patch sequence [Mark Brown]
  + Change union name to avoid future collisions [Mark Brown]
  + Check return code from clk_set_rate [Mark Brown]
  + Check parametsrs read from hardware agaist array limits and
add further comments [Mark Brown]
  + Add of_match_ptr [Mark Brown]

Changes v2-v3
  + Drop applied patch
  + Flush FIFOs in prepare rather than hw_params [Lars-Peter Clausen]

Changes v1-v2
  + Drop negative use count patch [Mark Brown]
  + Remove unnecessary debug print messages [Lars-Peter Clausen]
  + Rewrite iteration as for loop rather than do...while [Mark Brown]
  + Reorder patches to send fixes first [Mark Brown]
  + Simplify device tree code [Mark Brown]
  + Split device tree patch in two [Mark Brown]
  + Expand explanatory comment on channel configuration [Rajeev Kumar]

Arnd: I've not forgotten about updating the spear entries and
  will submit as a separate patch.

Andrew Jackson (6):
  ASoC: dwc: Switch to managed clock resource
  ASoC: dwc: Prepare clock before use
  ASoC: dwc: Read I2S block configuration from registers
  ASoC: dwc: Register components with managed interface
  ASoC: dwc: Add documentation for I2S DT
  ASoC: dwc: Add devicetree support for Designware I2S

 .../devicetree/bindings/sound/designware-i2s.txt   |   31 +++
 sound/soc/dwc/Kconfig  |1 +
 sound/soc/dwc/designware_i2s.c |  283 
 3 files changed, 263 insertions(+), 52 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sound/designware-i2s.txt

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v5 2/6] ASoC: dwc: Prepare clock before use

2014-12-30 Thread Andrew Jackson
From: Andrew Jackson andrew.jack...@arm.com

Some I2S clocks may require some time to get the clock ready
for operation and so need to be prepared before they are enabled.
So, prepare the clock as well as enabling it, but combine the
two through clk_prepare_enable.

Signed-off-by: Andrew Jackson andrew.jack...@arm.com
---
 sound/soc/dwc/designware_i2s.c |7 +--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
index 5e9d163..08608c1 100644
--- a/sound/soc/dwc/designware_i2s.c
+++ b/sound/soc/dwc/designware_i2s.c
@@ -411,7 +411,7 @@ static int dw_i2s_probe(struct platform_device *pdev)
if (IS_ERR(dev-clk))
return  PTR_ERR(dev-clk);
 
-   ret = clk_enable(dev-clk);
+   ret = clk_prepare_enable(dev-clk);
if (ret  0)
return ret;
 
@@ -426,13 +426,16 @@ static int dw_i2s_probe(struct platform_device *pdev)
return 0;
 
 err_clk_disable:
-   clk_disable(dev-clk);
+   clk_disable_unprepare(dev-clk);
return ret;
 }
 
 static int dw_i2s_remove(struct platform_device *pdev)
 {
+   struct dw_i2s_dev *dev = dev_get_drvdata(pdev-dev);
+
snd_soc_unregister_component(pdev-dev);
+   clk_disable_unprepare(dev-clk);
 
return 0;
 }
-- 
1.7.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v5 5/6] ASoC: dwc: Add documentation for I2S DT

2014-12-30 Thread Andrew Jackson
From: Andrew Jackson andrew.jack...@arm.com

Add documentation for Designware I2S hardware block.  The block requires
one clock (for audio sampling) and DMA channels for receive and transmit.

Signed-off-by: Andrew Jackson andrew.jack...@arm.com
---
 .../devicetree/bindings/sound/designware-i2s.txt   |   31 
 1 files changed, 31 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sound/designware-i2s.txt

diff --git a/Documentation/devicetree/bindings/sound/designware-i2s.txt 
b/Documentation/devicetree/bindings/sound/designware-i2s.txt
new file mode 100644
index 000..7bb5424
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/designware-i2s.txt
@@ -0,0 +1,31 @@
+DesignWare I2S controller
+
+Required properties:
+ - compatible : Must be snps,designware-i2s
+ - reg : Must contain the I2S core's registers location and length
+ - clocks : Pairs of phandle and specifier referencing the controller's
+   clocks. The controller expects one clock: the clock used as the sampling
+   rate reference clock sample.
+ - clock-names : i2sclk for the sample rate reference clock.
+ - dmas: Pairs of phandle and specifier for the DMA channels that are used by
+   the core. The core expects one or two dma channels: one for transmit and
+   one for receive.
+ - dma-names : tx for the transmit channel, rx for the receive channel.
+
+For more details on the 'dma', 'dma-names', 'clock' and 'clock-names'
+properties please check:
+   * resource-names.txt
+   * clock/clock-bindings.txt
+   * dma/dma.txt
+
+Example:
+
+   soc_i2s: i2s@7ff9 {
+   compatible = snps,designware-i2s;
+   reg = 0x0 0x7ff9 0x0 0x1000;
+   clocks = scpi_i2sclk 0;
+   clock-names = i2sclk;
+   #sound-dai-cells = 0;
+   dmas = dma0 5;
+   dma-names = tx;
+   };
-- 
1.7.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v5 6/6] ASoC: dwc: Add devicetree support for Designware I2S

2014-12-30 Thread Andrew Jackson
From: Andrew Jackson andrew.jack...@arm.com

Allow the driver to be configured through a device tree rather than platform
data.

Signed-off-by: Andrew Jackson andrew.jack...@arm.com
---
 sound/soc/dwc/Kconfig  |1 +
 sound/soc/dwc/designware_i2s.c |  193 +++-
 2 files changed, 151 insertions(+), 43 deletions(-)

diff --git a/sound/soc/dwc/Kconfig b/sound/soc/dwc/Kconfig
index e334900..d50e085 100644
--- a/sound/soc/dwc/Kconfig
+++ b/sound/soc/dwc/Kconfig
@@ -1,6 +1,7 @@
 config SND_DESIGNWARE_I2S
tristate Synopsys I2S Device Driver
depends on CLKDEV_LOOKUP
+   select SND_SOC_GENERIC_DMAENGINE_PCM
help
 Say Y or M if you want to add support for I2S driver for
 Synopsys desigwnware I2S device. The device supports upto
diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
index e1c0e2c..99cf64b 100644
--- a/sound/soc/dwc/designware_i2s.c
+++ b/sound/soc/dwc/designware_i2s.c
@@ -22,6 +22,7 @@
 #include sound/pcm.h
 #include sound/pcm_params.h
 #include sound/soc.h
+#include sound/dmaengine_pcm.h
 
 /* common register for all channel */
 #define IER0x000
@@ -82,6 +83,11 @@
 #define MAX_CHANNEL_NUM8
 #define MIN_CHANNEL_NUM2
 
+union dw_i2s_snd_dma_data {
+   struct i2s_dma_data pd;
+   struct snd_dmaengine_dai_dma_data dt;
+};
+
 struct dw_i2s_dev {
void __iomem *i2s_base;
struct clk *clk;
@@ -90,8 +96,8 @@ struct dw_i2s_dev {
struct device *dev;
 
/* data related to DMA transfers b/w i2s and DMAC */
-   struct i2s_dma_data play_dma_data;
-   struct i2s_dma_data capture_dma_data;
+   union dw_i2s_snd_dma_data play_dma_data;
+   union dw_i2s_snd_dma_data capture_dma_data;
struct i2s_clk_config_data config;
int (*i2s_clk_cfg)(struct i2s_clk_config_data *config);
 };
@@ -178,7 +184,7 @@ static int dw_i2s_startup(struct snd_pcm_substream 
*substream,
struct snd_soc_dai *cpu_dai)
 {
struct dw_i2s_dev *dev = snd_soc_dai_get_drvdata(cpu_dai);
-   struct i2s_dma_data *dma_data = NULL;
+   union dw_i2s_snd_dma_data *dma_data = NULL;
 
if (!(dev-capability  DWC_I2S_RECORD) 
(substream-stream == SNDRV_PCM_STREAM_CAPTURE))
@@ -267,13 +273,21 @@ static int dw_i2s_hw_params(struct snd_pcm_substream 
*substream,
 
config-sample_rate = params_rate(params);
 
-   if (!dev-i2s_clk_cfg)
-   return -EINVAL;
+   if (dev-i2s_clk_cfg) {
+   ret = dev-i2s_clk_cfg(config);
+   if (ret  0) {
+   dev_err(dev-dev, runtime audio clk config fail\n);
+   return ret;
+   }
+   } else {
+   u32 bitclk = config-sample_rate * config-data_width * 2;
 
-   ret = dev-i2s_clk_cfg(config);
-   if (ret  0) {
-   dev_err(dev-dev, runtime audio clk config fail\n);
-   return ret;
+   ret = clk_set_rate(dev-clk, bitclk);
+   if (ret) {
+   dev_err(dev-dev, Can't set I2S clock rate: %d\n,
+   ret);
+   return ret;
+   }
}
 
return 0;
@@ -368,6 +382,11 @@ static int dw_i2s_resume(struct snd_soc_dai *dai)
  * block parameter.
  */
 
+/* Maximum bit resolution of a channel - not uniformly spaced */
+static const u32 fifo_width[COMP_MAX_WORDSIZE] = {
+   12, 16, 20, 24, 32, 0, 0, 0
+};
+
 /* Width of (DMA) bus */
 static const u32 bus_widths[COMP_MAX_DATA_WIDTH] = {
DMA_SLAVE_BUSWIDTH_1_BYTE,
@@ -388,10 +407,9 @@ static const u32 formats[COMP_MAX_WORDSIZE] = {
0
 };
 
-static int dw_configure_dai_by_pd(struct dw_i2s_dev *dev,
+static int dw_configure_dai(struct dw_i2s_dev *dev,
   struct snd_soc_dai_driver *dw_i2s_dai,
-  struct resource *res,
-  const struct i2s_platform_data *pdata)
+  unsigned int rates)
 {
/*
 * Read component parameter registers to extract
@@ -399,23 +417,7 @@ static int dw_configure_dai_by_pd(struct dw_i2s_dev *dev,
 */
u32 comp1 = i2s_read_reg(dev-i2s_base, I2S_COMP_PARAM_1);
u32 comp2 = i2s_read_reg(dev-i2s_base, I2S_COMP_PARAM_2);
-   u32 idx = COMP1_APB_DATA_WIDTH(comp1);
-
-   if (WARN_ON(idx = ARRAY_SIZE(bus_widths)))
-   return -EINVAL;
-
-   /* Set DMA slaves info */
-
-   dev-play_dma_data.data = pdata-play_dma_data;
-   dev-capture_dma_data.data = pdata-capture_dma_data;
-   dev-play_dma_data.addr = res-start + I2S_TXDMA;
-   dev-capture_dma_data.addr = res-start + I2S_RXDMA;
-   dev-play_dma_data.max_burst = 16;
-   dev-capture_dma_data.max_burst = 16;
-   dev-play_dma_data.addr_width = bus_widths[idx];
-   dev

[PATCH v5 3/6] ASoC: dwc: Read I2S block configuration from registers

2014-12-30 Thread Andrew Jackson
From: Andrew Jackson andrew.jack...@arm.com

The I2S block provides component parameter registers which
describe how the block is instantiated.  Use these registers
to extract the block's configuration rather than relying on
platform data.

Signed-off-by: Andrew Jackson andrew.jack...@arm.com
---
 sound/soc/dwc/designware_i2s.c |   96 +++
 1 files changed, 86 insertions(+), 10 deletions(-)

diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
index 08608c1..b3e7567 100644
--- a/sound/soc/dwc/designware_i2s.c
+++ b/sound/soc/dwc/designware_i2s.c
@@ -54,6 +54,31 @@
 #define I2S_COMP_VERSION   0x01F8
 #define I2S_COMP_TYPE  0x01FC
 
+/*
+ * Component parameter register fields - define the I2S block's
+ * configuration.
+ */
+#defineCOMP1_TX_WORDSIZE_3(r)  (((r)  GENMASK(27, 25))  25)
+#defineCOMP1_TX_WORDSIZE_2(r)  (((r)  GENMASK(24, 22))  22)
+#defineCOMP1_TX_WORDSIZE_1(r)  (((r)  GENMASK(21, 19))  19)
+#defineCOMP1_TX_WORDSIZE_0(r)  (((r)  GENMASK(18, 16))  16)
+#defineCOMP1_TX_CHANNELS(r)(((r)  GENMASK(10, 9))  9)
+#defineCOMP1_RX_CHANNELS(r)(((r)  GENMASK(8, 7))  7)
+#defineCOMP1_RX_ENABLED(r) (((r)  BIT(6))  6)
+#defineCOMP1_TX_ENABLED(r) (((r)  BIT(5))  5)
+#defineCOMP1_MODE_EN(r)(((r)  BIT(4))  4)
+#defineCOMP1_FIFO_DEPTH_GLOBAL(r)  (((r)  GENMASK(3, 2))  2)
+#defineCOMP1_APB_DATA_WIDTH(r) (((r)  GENMASK(1, 0))  0)
+
+#defineCOMP2_RX_WORDSIZE_3(r)  (((r)  GENMASK(12, 10))  10)
+#defineCOMP2_RX_WORDSIZE_2(r)  (((r)  GENMASK(9, 7))  7)
+#defineCOMP2_RX_WORDSIZE_1(r)  (((r)  GENMASK(5, 3))  3)
+#defineCOMP2_RX_WORDSIZE_0(r)  (((r)  GENMASK(2, 0))  0)
+
+/* Number of entries in WORDSIZE and DATA_WIDTH parameter registers */
+#defineCOMP_MAX_WORDSIZE   (1  3)
+#defineCOMP_MAX_DATA_WIDTH (1  2)
+
 #define MAX_CHANNEL_NUM8
 #define MIN_CHANNEL_NUM2
 
@@ -335,11 +360,50 @@ static int dw_i2s_resume(struct snd_soc_dai *dai)
 #define dw_i2s_resume  NULL
 #endif
 
-static void dw_configure_dai_by_pd(struct dw_i2s_dev *dev,
+/*
+ * The following tables allow a direct lookup of various parameters
+ * defined in the I2S block's configuration in terms of sound system
+ * parameters.  Each table is sized to the number of entries possible
+ * according to the number of configuration bits describing an I2S
+ * block parameter.
+ */
+
+/* Width of (DMA) bus */
+static const u32 bus_widths[COMP_MAX_DATA_WIDTH] = {
+   DMA_SLAVE_BUSWIDTH_1_BYTE,
+   DMA_SLAVE_BUSWIDTH_2_BYTES,
+   DMA_SLAVE_BUSWIDTH_4_BYTES,
+   DMA_SLAVE_BUSWIDTH_UNDEFINED
+};
+
+/* PCM format to support channel resolution */
+static const u32 formats[COMP_MAX_WORDSIZE] = {
+   SNDRV_PCM_FMTBIT_S16_LE,
+   SNDRV_PCM_FMTBIT_S16_LE,
+   SNDRV_PCM_FMTBIT_S24_LE,
+   SNDRV_PCM_FMTBIT_S24_LE,
+   SNDRV_PCM_FMTBIT_S32_LE,
+   0,
+   0,
+   0
+};
+
+static int dw_configure_dai_by_pd(struct dw_i2s_dev *dev,
   struct snd_soc_dai_driver *dw_i2s_dai,
   struct resource *res,
   const struct i2s_platform_data *pdata)
 {
+   /*
+* Read component parameter registers to extract
+* the I2S block's configuration.
+*/
+   u32 comp1 = i2s_read_reg(dev-i2s_base, I2S_COMP_PARAM_1);
+   u32 comp2 = i2s_read_reg(dev-i2s_base, I2S_COMP_PARAM_2);
+   u32 idx = COMP1_APB_DATA_WIDTH(comp1);
+
+   if (WARN_ON(idx = ARRAY_SIZE(bus_widths)))
+   return -EINVAL;
+
/* Set DMA slaves info */
 
dev-play_dma_data.data = pdata-play_dma_data;
@@ -348,26 +412,36 @@ static void dw_configure_dai_by_pd(struct dw_i2s_dev *dev,
dev-capture_dma_data.addr = res-start + I2S_RXDMA;
dev-play_dma_data.max_burst = 16;
dev-capture_dma_data.max_burst = 16;
-   dev-play_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
-   dev-capture_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
+   dev-play_dma_data.addr_width = bus_widths[idx];
+   dev-capture_dma_data.addr_width = bus_widths[idx];
dev-play_dma_data.filter = pdata-filter;
dev-capture_dma_data.filter = pdata-filter;
 
-   if (pdata-cap  DWC_I2S_PLAY) {
+   if (COMP1_TX_ENABLED(comp1)) {
dev_dbg(dev-dev,  designware: play supported\n);
+   idx = COMP1_TX_WORDSIZE_0(comp1);
+   if (WARN_ON(idx = ARRAY_SIZE(formats)))
+   return -EINVAL;
dw_i2s_dai-playback.channels_min = MIN_CHANNEL_NUM;
-   dw_i2s_dai-playback.channels_max = pdata-channel;
-   dw_i2s_dai-playback.formats = pdata-snd_fmts;
+   dw_i2s_dai-playback.channels_max =
+   1  (COMP1_TX_CHANNELS(comp1) + 1

[PATCH v5 1/6] ASoC: dwc: Switch to managed clock resource

2014-12-30 Thread Andrew Jackson
From: Andrew Jackson andrew.jack...@arm.com

Simplify error handling during probe by using managed clock
resources.

Signed-off-by: Andrew Jackson andrew.jack...@arm.com
---
 sound/soc/dwc/designware_i2s.c |   10 ++
 1 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
index 06d3a34..5e9d163 100644
--- a/sound/soc/dwc/designware_i2s.c
+++ b/sound/soc/dwc/designware_i2s.c
@@ -407,13 +407,13 @@ static int dw_i2s_probe(struct platform_device *pdev)
 
dev-capability = pdata-cap;
dev-i2s_clk_cfg = pdata-i2s_clk_cfg;
-   dev-clk = clk_get(pdev-dev, NULL);
+   dev-clk = devm_clk_get(pdev-dev, NULL);
if (IS_ERR(dev-clk))
return  PTR_ERR(dev-clk);
 
ret = clk_enable(dev-clk);
if (ret  0)
-   goto err_clk_put;
+   return ret;
 
dev_set_drvdata(pdev-dev, dev);
ret = snd_soc_register_component(pdev-dev, dw_i2s_component,
@@ -427,19 +427,13 @@ static int dw_i2s_probe(struct platform_device *pdev)
 
 err_clk_disable:
clk_disable(dev-clk);
-err_clk_put:
-   clk_put(dev-clk);
return ret;
 }
 
 static int dw_i2s_remove(struct platform_device *pdev)
 {
-   struct dw_i2s_dev *dev = dev_get_drvdata(pdev-dev);
-
snd_soc_unregister_component(pdev-dev);
 
-   clk_put(dev-clk);
-
return 0;
 }
 
-- 
1.7.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4 1/5] ASoC: dwc: Prepare clock before use

2014-12-23 Thread Andrew Jackson
On 12/23/14 13:56, Andrew Jackson wrote:
> From: Andrew Jackson 
> 
> Some I2S clocks may require some time to get the clock ready
> for operation and so need to be prepared before they are enabled.
> So, prepare the clock as well as enabling it, but combine the
> two through clk_prepare_enable.

More turkey-focussed issues: this should have a clk_disable_unprepare too.  Sigh

Andrew

> Signed-off-by: Andrew Jackson 
> ---
>  sound/soc/dwc/designware_i2s.c |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
> index 06d3a34..9573ec7 100644
> --- a/sound/soc/dwc/designware_i2s.c
> +++ b/sound/soc/dwc/designware_i2s.c
> @@ -411,7 +411,7 @@ static int dw_i2s_probe(struct platform_device *pdev)
>   if (IS_ERR(dev->clk))
>   return  PTR_ERR(dev->clk);
>  
> - ret = clk_enable(dev->clk);
> + ret = clk_prepare_enable(dev->clk);
>   if (ret < 0)
>   goto err_clk_put;
>  
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4 2/5] ASoC: dwc: Switch to managed clock resource

2014-12-23 Thread Andrew Jackson
On 12/23/14 13:56, Andrew Jackson wrote:
> From: Andrew Jackson 
> 
> Simplify error handling during probe by using managed clock
> resources.

In my haste to get to the Christmas turkey I missed a clk_put elsewhere.  Will 
fix and resubmit.

 Andrew

> Signed-off-by: Andrew Jackson 
> ---
>  sound/soc/dwc/designware_i2s.c |6 ++
>  1 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
> index 9573ec7..b3dc535 100644
> --- a/sound/soc/dwc/designware_i2s.c
> +++ b/sound/soc/dwc/designware_i2s.c
> @@ -407,13 +407,13 @@ static int dw_i2s_probe(struct platform_device *pdev)
>  
>   dev->capability = pdata->cap;
>   dev->i2s_clk_cfg = pdata->i2s_clk_cfg;
> - dev->clk = clk_get(>dev, NULL);
> + dev->clk = devm_clk_get(>dev, NULL);
>   if (IS_ERR(dev->clk))
>   return  PTR_ERR(dev->clk);
>  
>   ret = clk_prepare_enable(dev->clk);
>   if (ret < 0)
> - goto err_clk_put;
> + return ret;
>  
>   dev_set_drvdata(>dev, dev);
>   ret = snd_soc_register_component(>dev, _i2s_component,
> @@ -427,8 +427,6 @@ static int dw_i2s_probe(struct platform_device *pdev)
>  
>  err_clk_disable:
>   clk_disable(dev->clk);
> -err_clk_put:
> - clk_put(dev->clk);
>   return ret;
>  }
>  
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v4 1/5] ASoC: dwc: Prepare clock before use

2014-12-23 Thread Andrew Jackson
From: Andrew Jackson 

Some I2S clocks may require some time to get the clock ready
for operation and so need to be prepared before they are enabled.
So, prepare the clock as well as enabling it, but combine the
two through clk_prepare_enable.

Signed-off-by: Andrew Jackson 
---
 sound/soc/dwc/designware_i2s.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
index 06d3a34..9573ec7 100644
--- a/sound/soc/dwc/designware_i2s.c
+++ b/sound/soc/dwc/designware_i2s.c
@@ -411,7 +411,7 @@ static int dw_i2s_probe(struct platform_device *pdev)
if (IS_ERR(dev->clk))
return  PTR_ERR(dev->clk);
 
-   ret = clk_enable(dev->clk);
+   ret = clk_prepare_enable(dev->clk);
if (ret < 0)
goto err_clk_put;
 
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v4 0/5] ASoC: dwc: Add device tree support to designware I2S

2014-12-23 Thread Andrew Jackson
From: Andrew Jackson 

This patch set extends the DesignWare I2S driver to provide device
tree support and fixes a couple of small faults.

Changes v3->v4
  + Drop applied patches
  + Use combined function to prepare clock [Mark Brown]
  + Use managed clock resources to avoid clk_put [Mark Brown]
  + Read configuration prameters from hardware for both platform data
and device tree [Mark Brown]
  + Re-order patch sequence [Mark Brown]
  + Change union name to avoid future collisions [Mark Brown]
  + Check return code from clk_set_rate [Mark Brown]
  + Check parametsrs read from hardware agaist array limits and
add further comments [Mark Brown]
  + Add of_match_ptr [Mark Brown]

Changes v2->v3
  + Drop applied patch
  + Flush FIFOs in prepare rather than hw_params [Lars-Peter Clausen]

Changes v1->v2
  + Drop negative use count patch [Mark Brown]
  + Remove unnecessary debug print messages [Lars-Peter Clausen]
  + Rewrite iteration as for loop rather than do...while [Mark Brown]
  + Reorder patches to send fixes first [Mark Brown]
  + Simplify device tree code [Mark Brown]
  + Split device tree patch in two [Mark Brown]
  + Expand explanatory comment on channel configuration [Rajeev Kumar]

Arnd: I've not forgotten about updating the spear entries and
  will submit as a separate patch.

Andrew Jackson (5):
  ASoC: dwc: Prepare clock before use
  ASoC: dwc: Switch to managed clock resource
  ASoC: dwc: Read I2S block configuration from registers
  ASoC: dwc: Add documentation for I2S DT
  ASoC: dwc: Add devicetree support for Designware I2S

 .../devicetree/bindings/sound/designware-i2s.txt   |   31 +++
 sound/soc/dwc/Kconfig  |1 +
 sound/soc/dwc/designware_i2s.c |  275 
 3 files changed, 260 insertions(+), 47 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sound/designware-i2s.txt

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v4 3/5] ASoC: dwc: Read I2S block configuration from registers

2014-12-23 Thread Andrew Jackson
From: Andrew Jackson 

The I2S block provides component parameter registers which
describe how the block is instantiated.  Use these registers
to extract the block's configuration rather than relying on
platform data.

Signed-off-by: Andrew Jackson 
---
 sound/soc/dwc/designware_i2s.c |   96 +++
 1 files changed, 86 insertions(+), 10 deletions(-)

diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
index b3dc535..c89b9eb 100644
--- a/sound/soc/dwc/designware_i2s.c
+++ b/sound/soc/dwc/designware_i2s.c
@@ -54,6 +54,31 @@
 #define I2S_COMP_VERSION   0x01F8
 #define I2S_COMP_TYPE  0x01FC
 
+/*
+ * Component parameter register fields - define the I2S block's
+ * configuration.
+ */
+#defineCOMP1_TX_WORDSIZE_3(r)  (((r) & GENMASK(27, 25)) >> 25)
+#defineCOMP1_TX_WORDSIZE_2(r)  (((r) & GENMASK(24, 22)) >> 22)
+#defineCOMP1_TX_WORDSIZE_1(r)  (((r) & GENMASK(21, 19)) >> 19)
+#defineCOMP1_TX_WORDSIZE_0(r)  (((r) & GENMASK(18, 16)) >> 16)
+#defineCOMP1_TX_CHANNELS(r)(((r) & GENMASK(10, 9)) >> 9)
+#defineCOMP1_RX_CHANNELS(r)(((r) & GENMASK(8, 7)) >> 7)
+#defineCOMP1_RX_ENABLED(r) (((r) & BIT(6)) >> 6)
+#defineCOMP1_TX_ENABLED(r) (((r) & BIT(5)) >> 5)
+#defineCOMP1_MODE_EN(r)(((r) & BIT(4)) >> 4)
+#defineCOMP1_FIFO_DEPTH_GLOBAL(r)  (((r) & GENMASK(3, 2)) >> 2)
+#defineCOMP1_APB_DATA_WIDTH(r) (((r) & GENMASK(1, 0)) >> 0)
+
+#defineCOMP2_RX_WORDSIZE_3(r)  (((r) & GENMASK(12, 10)) >> 10)
+#defineCOMP2_RX_WORDSIZE_2(r)  (((r) & GENMASK(9, 7)) >> 7)
+#defineCOMP2_RX_WORDSIZE_1(r)  (((r) & GENMASK(5, 3)) >> 3)
+#defineCOMP2_RX_WORDSIZE_0(r)  (((r) & GENMASK(2, 0)) >> 0)
+
+/* Number of entries in WORDSIZE and DATA_WIDTH parameter registers */
+#defineCOMP_MAX_WORDSIZE   (1 << 3)
+#defineCOMP_MAX_DATA_WIDTH (1 << 2)
+
 #define MAX_CHANNEL_NUM8
 #define MIN_CHANNEL_NUM2
 
@@ -335,11 +360,50 @@ static int dw_i2s_resume(struct snd_soc_dai *dai)
 #define dw_i2s_resume  NULL
 #endif
 
-static void dw_configure_dai_by_pd(struct dw_i2s_dev *dev,
+/*
+ * The following tables allow a direct lookup of various parameters
+ * defined in the I2S block's configuration in terms of sound system
+ * parameters.  Each table is sized to the number of entries possible
+ * according to the number of configuration bits describing an I2S
+ * block parameter.
+ */
+
+/* Width of (DMA) bus */
+static const u32 bus_widths[COMP_MAX_DATA_WIDTH] = {
+   DMA_SLAVE_BUSWIDTH_1_BYTE,
+   DMA_SLAVE_BUSWIDTH_2_BYTES,
+   DMA_SLAVE_BUSWIDTH_4_BYTES,
+   DMA_SLAVE_BUSWIDTH_UNDEFINED
+};
+
+/* PCM format to support channel resolution */
+static const u32 formats[COMP_MAX_WORDSIZE] = {
+   SNDRV_PCM_FMTBIT_S16_LE,
+   SNDRV_PCM_FMTBIT_S16_LE,
+   SNDRV_PCM_FMTBIT_S24_LE,
+   SNDRV_PCM_FMTBIT_S24_LE,
+   SNDRV_PCM_FMTBIT_S32_LE,
+   0,
+   0,
+   0
+};
+
+static int dw_configure_dai_by_pd(struct dw_i2s_dev *dev,
   struct snd_soc_dai_driver *dw_i2s_dai,
   struct resource *res,
   const struct i2s_platform_data *pdata)
 {
+   /*
+* Read component parameter registers to extract
+* the I2S block's configuration.
+*/
+   u32 comp1 = i2s_read_reg(dev->i2s_base, I2S_COMP_PARAM_1);
+   u32 comp2 = i2s_read_reg(dev->i2s_base, I2S_COMP_PARAM_2);
+   u32 idx = COMP1_APB_DATA_WIDTH(comp1);
+
+   if (WARN_ON(idx >= ARRAY_SIZE(bus_widths)))
+   return -EINVAL;
+
/* Set DMA slaves info */
 
dev->play_dma_data.data = pdata->play_dma_data;
@@ -348,26 +412,36 @@ static void dw_configure_dai_by_pd(struct dw_i2s_dev *dev,
dev->capture_dma_data.addr = res->start + I2S_RXDMA;
dev->play_dma_data.max_burst = 16;
dev->capture_dma_data.max_burst = 16;
-   dev->play_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
-   dev->capture_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
+   dev->play_dma_data.addr_width = bus_widths[idx];
+   dev->capture_dma_data.addr_width = bus_widths[idx];
dev->play_dma_data.filter = pdata->filter;
dev->capture_dma_data.filter = pdata->filter;
 
-   if (pdata->cap & DWC_I2S_PLAY) {
+   if (COMP1_TX_ENABLED(comp1)) {
dev_dbg(dev->dev, " designware: play supported\n");
+   idx = COMP1_TX_WORDSIZE_0(comp1);
+   if (WARN_ON(idx >= ARRAY_SIZE(formats)))
+   return -EINVAL;
dw_i2s_dai->playback.channels_m

[PATCH v4 4/5] ASoC: dwc: Add documentation for I2S DT

2014-12-23 Thread Andrew Jackson
From: Andrew Jackson 

Add documentation for Designware I2S hardware block.  The block requires
one clock (for audio sampling) and DMA channels for receive and transmit.

Signed-off-by: Andrew Jackson 
---
 .../devicetree/bindings/sound/designware-i2s.txt   |   31 
 1 files changed, 31 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sound/designware-i2s.txt

diff --git a/Documentation/devicetree/bindings/sound/designware-i2s.txt 
b/Documentation/devicetree/bindings/sound/designware-i2s.txt
new file mode 100644
index 000..7bb5424
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/designware-i2s.txt
@@ -0,0 +1,31 @@
+DesignWare I2S controller
+
+Required properties:
+ - compatible : Must be "snps,designware-i2s"
+ - reg : Must contain the I2S core's registers location and length
+ - clocks : Pairs of phandle and specifier referencing the controller's
+   clocks. The controller expects one clock: the clock used as the sampling
+   rate reference clock sample.
+ - clock-names : "i2sclk" for the sample rate reference clock.
+ - dmas: Pairs of phandle and specifier for the DMA channels that are used by
+   the core. The core expects one or two dma channels: one for transmit and
+   one for receive.
+ - dma-names : "tx" for the transmit channel, "rx" for the receive channel.
+
+For more details on the 'dma', 'dma-names', 'clock' and 'clock-names'
+properties please check:
+   * resource-names.txt
+   * clock/clock-bindings.txt
+   * dma/dma.txt
+
+Example:
+
+   soc_i2s: i2s@7ff9 {
+   compatible = "snps,designware-i2s";
+   reg = <0x0 0x7ff9 0x0 0x1000>;
+   clocks = <_i2sclk 0>;
+   clock-names = "i2sclk";
+   #sound-dai-cells = <0>;
+   dmas = < 5>;
+   dma-names = "tx";
+   };
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v4 2/5] ASoC: dwc: Switch to managed clock resource

2014-12-23 Thread Andrew Jackson
From: Andrew Jackson 

Simplify error handling during probe by using managed clock
resources.

Signed-off-by: Andrew Jackson 
---
 sound/soc/dwc/designware_i2s.c |6 ++
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
index 9573ec7..b3dc535 100644
--- a/sound/soc/dwc/designware_i2s.c
+++ b/sound/soc/dwc/designware_i2s.c
@@ -407,13 +407,13 @@ static int dw_i2s_probe(struct platform_device *pdev)
 
dev->capability = pdata->cap;
dev->i2s_clk_cfg = pdata->i2s_clk_cfg;
-   dev->clk = clk_get(>dev, NULL);
+   dev->clk = devm_clk_get(>dev, NULL);
if (IS_ERR(dev->clk))
return  PTR_ERR(dev->clk);
 
ret = clk_prepare_enable(dev->clk);
if (ret < 0)
-   goto err_clk_put;
+   return ret;
 
dev_set_drvdata(>dev, dev);
ret = snd_soc_register_component(>dev, _i2s_component,
@@ -427,8 +427,6 @@ static int dw_i2s_probe(struct platform_device *pdev)
 
 err_clk_disable:
clk_disable(dev->clk);
-err_clk_put:
-   clk_put(dev->clk);
return ret;
 }
 
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v4 5/5] ASoC: dwc: Add devicetree support for Designware I2S

2014-12-23 Thread Andrew Jackson
From: Andrew Jackson 

Allow the driver to be configured through a device tree rather than platform
data.

Signed-off-by: Andrew Jackson 
---
 sound/soc/dwc/Kconfig  |1 +
 sound/soc/dwc/designware_i2s.c |  193 +++-
 2 files changed, 151 insertions(+), 43 deletions(-)

diff --git a/sound/soc/dwc/Kconfig b/sound/soc/dwc/Kconfig
index e334900..d50e085 100644
--- a/sound/soc/dwc/Kconfig
+++ b/sound/soc/dwc/Kconfig
@@ -1,6 +1,7 @@
 config SND_DESIGNWARE_I2S
tristate "Synopsys I2S Device Driver"
depends on CLKDEV_LOOKUP
+   select SND_SOC_GENERIC_DMAENGINE_PCM
help
 Say Y or M if you want to add support for I2S driver for
 Synopsys desigwnware I2S device. The device supports upto
diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
index c89b9eb..9e95274 100644
--- a/sound/soc/dwc/designware_i2s.c
+++ b/sound/soc/dwc/designware_i2s.c
@@ -22,6 +22,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /* common register for all channel */
 #define IER0x000
@@ -82,6 +83,11 @@
 #define MAX_CHANNEL_NUM8
 #define MIN_CHANNEL_NUM2
 
+union dw_i2s_snd_dma_data {
+   struct i2s_dma_data pd;
+   struct snd_dmaengine_dai_dma_data dt;
+};
+
 struct dw_i2s_dev {
void __iomem *i2s_base;
struct clk *clk;
@@ -90,8 +96,8 @@ struct dw_i2s_dev {
struct device *dev;
 
/* data related to DMA transfers b/w i2s and DMAC */
-   struct i2s_dma_data play_dma_data;
-   struct i2s_dma_data capture_dma_data;
+   union dw_i2s_snd_dma_data play_dma_data;
+   union dw_i2s_snd_dma_data capture_dma_data;
struct i2s_clk_config_data config;
int (*i2s_clk_cfg)(struct i2s_clk_config_data *config);
 };
@@ -178,7 +184,7 @@ static int dw_i2s_startup(struct snd_pcm_substream 
*substream,
struct snd_soc_dai *cpu_dai)
 {
struct dw_i2s_dev *dev = snd_soc_dai_get_drvdata(cpu_dai);
-   struct i2s_dma_data *dma_data = NULL;
+   union dw_i2s_snd_dma_data *dma_data = NULL;
 
if (!(dev->capability & DWC_I2S_RECORD) &&
(substream->stream == SNDRV_PCM_STREAM_CAPTURE))
@@ -267,13 +273,21 @@ static int dw_i2s_hw_params(struct snd_pcm_substream 
*substream,
 
config->sample_rate = params_rate(params);
 
-   if (!dev->i2s_clk_cfg)
-   return -EINVAL;
+   if (dev->i2s_clk_cfg) {
+   ret = dev->i2s_clk_cfg(config);
+   if (ret < 0) {
+   dev_err(dev->dev, "runtime audio clk config fail\n");
+   return ret;
+   }
+   } else {
+   u32 bitclk = config->sample_rate * config->data_width * 2;
 
-   ret = dev->i2s_clk_cfg(config);
-   if (ret < 0) {
-   dev_err(dev->dev, "runtime audio clk config fail\n");
-   return ret;
+   ret = clk_set_rate(dev->clk, bitclk);
+   if (ret) {
+   dev_err(dev->dev, "Can't set I2S clock rate: %d\n",
+   ret);
+   return ret;
+   }
}
 
return 0;
@@ -368,6 +382,11 @@ static int dw_i2s_resume(struct snd_soc_dai *dai)
  * block parameter.
  */
 
+/* Maximum bit resolution of a channel - not uniformly spaced */
+static const u32 fifo_width[COMP_MAX_WORDSIZE] = {
+   12, 16, 20, 24, 32, 0, 0, 0
+};
+
 /* Width of (DMA) bus */
 static const u32 bus_widths[COMP_MAX_DATA_WIDTH] = {
DMA_SLAVE_BUSWIDTH_1_BYTE,
@@ -388,10 +407,9 @@ static const u32 formats[COMP_MAX_WORDSIZE] = {
0
 };
 
-static int dw_configure_dai_by_pd(struct dw_i2s_dev *dev,
+static int dw_configure_dai(struct dw_i2s_dev *dev,
   struct snd_soc_dai_driver *dw_i2s_dai,
-  struct resource *res,
-  const struct i2s_platform_data *pdata)
+  unsigned int rates)
 {
/*
 * Read component parameter registers to extract
@@ -399,23 +417,7 @@ static int dw_configure_dai_by_pd(struct dw_i2s_dev *dev,
 */
u32 comp1 = i2s_read_reg(dev->i2s_base, I2S_COMP_PARAM_1);
u32 comp2 = i2s_read_reg(dev->i2s_base, I2S_COMP_PARAM_2);
-   u32 idx = COMP1_APB_DATA_WIDTH(comp1);
-
-   if (WARN_ON(idx >= ARRAY_SIZE(bus_widths)))
-   return -EINVAL;
-
-   /* Set DMA slaves info */
-
-   dev->play_dma_data.data = pdata->play_dma_data;
-   dev->capture_dma_data.data = pdata->capture_dma_data;
-   dev->play_dma_data.addr = res->start + I2S_TXDMA;
-   dev->capture_dma_data.addr = res->start + I2S_RXDMA;
-   dev->play_dma_data.max_burst = 16;
-   dev->capture_dma_data.max_burst = 16;
-   dev-&g

[PATCH v4 2/5] ASoC: dwc: Switch to managed clock resource

2014-12-23 Thread Andrew Jackson
From: Andrew Jackson andrew.jack...@arm.com

Simplify error handling during probe by using managed clock
resources.

Signed-off-by: Andrew Jackson andrew.jack...@arm.com
---
 sound/soc/dwc/designware_i2s.c |6 ++
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
index 9573ec7..b3dc535 100644
--- a/sound/soc/dwc/designware_i2s.c
+++ b/sound/soc/dwc/designware_i2s.c
@@ -407,13 +407,13 @@ static int dw_i2s_probe(struct platform_device *pdev)
 
dev-capability = pdata-cap;
dev-i2s_clk_cfg = pdata-i2s_clk_cfg;
-   dev-clk = clk_get(pdev-dev, NULL);
+   dev-clk = devm_clk_get(pdev-dev, NULL);
if (IS_ERR(dev-clk))
return  PTR_ERR(dev-clk);
 
ret = clk_prepare_enable(dev-clk);
if (ret  0)
-   goto err_clk_put;
+   return ret;
 
dev_set_drvdata(pdev-dev, dev);
ret = snd_soc_register_component(pdev-dev, dw_i2s_component,
@@ -427,8 +427,6 @@ static int dw_i2s_probe(struct platform_device *pdev)
 
 err_clk_disable:
clk_disable(dev-clk);
-err_clk_put:
-   clk_put(dev-clk);
return ret;
 }
 
-- 
1.7.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v4 5/5] ASoC: dwc: Add devicetree support for Designware I2S

2014-12-23 Thread Andrew Jackson
From: Andrew Jackson andrew.jack...@arm.com

Allow the driver to be configured through a device tree rather than platform
data.

Signed-off-by: Andrew Jackson andrew.jack...@arm.com
---
 sound/soc/dwc/Kconfig  |1 +
 sound/soc/dwc/designware_i2s.c |  193 +++-
 2 files changed, 151 insertions(+), 43 deletions(-)

diff --git a/sound/soc/dwc/Kconfig b/sound/soc/dwc/Kconfig
index e334900..d50e085 100644
--- a/sound/soc/dwc/Kconfig
+++ b/sound/soc/dwc/Kconfig
@@ -1,6 +1,7 @@
 config SND_DESIGNWARE_I2S
tristate Synopsys I2S Device Driver
depends on CLKDEV_LOOKUP
+   select SND_SOC_GENERIC_DMAENGINE_PCM
help
 Say Y or M if you want to add support for I2S driver for
 Synopsys desigwnware I2S device. The device supports upto
diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
index c89b9eb..9e95274 100644
--- a/sound/soc/dwc/designware_i2s.c
+++ b/sound/soc/dwc/designware_i2s.c
@@ -22,6 +22,7 @@
 #include sound/pcm.h
 #include sound/pcm_params.h
 #include sound/soc.h
+#include sound/dmaengine_pcm.h
 
 /* common register for all channel */
 #define IER0x000
@@ -82,6 +83,11 @@
 #define MAX_CHANNEL_NUM8
 #define MIN_CHANNEL_NUM2
 
+union dw_i2s_snd_dma_data {
+   struct i2s_dma_data pd;
+   struct snd_dmaengine_dai_dma_data dt;
+};
+
 struct dw_i2s_dev {
void __iomem *i2s_base;
struct clk *clk;
@@ -90,8 +96,8 @@ struct dw_i2s_dev {
struct device *dev;
 
/* data related to DMA transfers b/w i2s and DMAC */
-   struct i2s_dma_data play_dma_data;
-   struct i2s_dma_data capture_dma_data;
+   union dw_i2s_snd_dma_data play_dma_data;
+   union dw_i2s_snd_dma_data capture_dma_data;
struct i2s_clk_config_data config;
int (*i2s_clk_cfg)(struct i2s_clk_config_data *config);
 };
@@ -178,7 +184,7 @@ static int dw_i2s_startup(struct snd_pcm_substream 
*substream,
struct snd_soc_dai *cpu_dai)
 {
struct dw_i2s_dev *dev = snd_soc_dai_get_drvdata(cpu_dai);
-   struct i2s_dma_data *dma_data = NULL;
+   union dw_i2s_snd_dma_data *dma_data = NULL;
 
if (!(dev-capability  DWC_I2S_RECORD) 
(substream-stream == SNDRV_PCM_STREAM_CAPTURE))
@@ -267,13 +273,21 @@ static int dw_i2s_hw_params(struct snd_pcm_substream 
*substream,
 
config-sample_rate = params_rate(params);
 
-   if (!dev-i2s_clk_cfg)
-   return -EINVAL;
+   if (dev-i2s_clk_cfg) {
+   ret = dev-i2s_clk_cfg(config);
+   if (ret  0) {
+   dev_err(dev-dev, runtime audio clk config fail\n);
+   return ret;
+   }
+   } else {
+   u32 bitclk = config-sample_rate * config-data_width * 2;
 
-   ret = dev-i2s_clk_cfg(config);
-   if (ret  0) {
-   dev_err(dev-dev, runtime audio clk config fail\n);
-   return ret;
+   ret = clk_set_rate(dev-clk, bitclk);
+   if (ret) {
+   dev_err(dev-dev, Can't set I2S clock rate: %d\n,
+   ret);
+   return ret;
+   }
}
 
return 0;
@@ -368,6 +382,11 @@ static int dw_i2s_resume(struct snd_soc_dai *dai)
  * block parameter.
  */
 
+/* Maximum bit resolution of a channel - not uniformly spaced */
+static const u32 fifo_width[COMP_MAX_WORDSIZE] = {
+   12, 16, 20, 24, 32, 0, 0, 0
+};
+
 /* Width of (DMA) bus */
 static const u32 bus_widths[COMP_MAX_DATA_WIDTH] = {
DMA_SLAVE_BUSWIDTH_1_BYTE,
@@ -388,10 +407,9 @@ static const u32 formats[COMP_MAX_WORDSIZE] = {
0
 };
 
-static int dw_configure_dai_by_pd(struct dw_i2s_dev *dev,
+static int dw_configure_dai(struct dw_i2s_dev *dev,
   struct snd_soc_dai_driver *dw_i2s_dai,
-  struct resource *res,
-  const struct i2s_platform_data *pdata)
+  unsigned int rates)
 {
/*
 * Read component parameter registers to extract
@@ -399,23 +417,7 @@ static int dw_configure_dai_by_pd(struct dw_i2s_dev *dev,
 */
u32 comp1 = i2s_read_reg(dev-i2s_base, I2S_COMP_PARAM_1);
u32 comp2 = i2s_read_reg(dev-i2s_base, I2S_COMP_PARAM_2);
-   u32 idx = COMP1_APB_DATA_WIDTH(comp1);
-
-   if (WARN_ON(idx = ARRAY_SIZE(bus_widths)))
-   return -EINVAL;
-
-   /* Set DMA slaves info */
-
-   dev-play_dma_data.data = pdata-play_dma_data;
-   dev-capture_dma_data.data = pdata-capture_dma_data;
-   dev-play_dma_data.addr = res-start + I2S_TXDMA;
-   dev-capture_dma_data.addr = res-start + I2S_RXDMA;
-   dev-play_dma_data.max_burst = 16;
-   dev-capture_dma_data.max_burst = 16;
-   dev-play_dma_data.addr_width = bus_widths[idx];
-   dev

[PATCH v4 3/5] ASoC: dwc: Read I2S block configuration from registers

2014-12-23 Thread Andrew Jackson
From: Andrew Jackson andrew.jack...@arm.com

The I2S block provides component parameter registers which
describe how the block is instantiated.  Use these registers
to extract the block's configuration rather than relying on
platform data.

Signed-off-by: Andrew Jackson andrew.jack...@arm.com
---
 sound/soc/dwc/designware_i2s.c |   96 +++
 1 files changed, 86 insertions(+), 10 deletions(-)

diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
index b3dc535..c89b9eb 100644
--- a/sound/soc/dwc/designware_i2s.c
+++ b/sound/soc/dwc/designware_i2s.c
@@ -54,6 +54,31 @@
 #define I2S_COMP_VERSION   0x01F8
 #define I2S_COMP_TYPE  0x01FC
 
+/*
+ * Component parameter register fields - define the I2S block's
+ * configuration.
+ */
+#defineCOMP1_TX_WORDSIZE_3(r)  (((r)  GENMASK(27, 25))  25)
+#defineCOMP1_TX_WORDSIZE_2(r)  (((r)  GENMASK(24, 22))  22)
+#defineCOMP1_TX_WORDSIZE_1(r)  (((r)  GENMASK(21, 19))  19)
+#defineCOMP1_TX_WORDSIZE_0(r)  (((r)  GENMASK(18, 16))  16)
+#defineCOMP1_TX_CHANNELS(r)(((r)  GENMASK(10, 9))  9)
+#defineCOMP1_RX_CHANNELS(r)(((r)  GENMASK(8, 7))  7)
+#defineCOMP1_RX_ENABLED(r) (((r)  BIT(6))  6)
+#defineCOMP1_TX_ENABLED(r) (((r)  BIT(5))  5)
+#defineCOMP1_MODE_EN(r)(((r)  BIT(4))  4)
+#defineCOMP1_FIFO_DEPTH_GLOBAL(r)  (((r)  GENMASK(3, 2))  2)
+#defineCOMP1_APB_DATA_WIDTH(r) (((r)  GENMASK(1, 0))  0)
+
+#defineCOMP2_RX_WORDSIZE_3(r)  (((r)  GENMASK(12, 10))  10)
+#defineCOMP2_RX_WORDSIZE_2(r)  (((r)  GENMASK(9, 7))  7)
+#defineCOMP2_RX_WORDSIZE_1(r)  (((r)  GENMASK(5, 3))  3)
+#defineCOMP2_RX_WORDSIZE_0(r)  (((r)  GENMASK(2, 0))  0)
+
+/* Number of entries in WORDSIZE and DATA_WIDTH parameter registers */
+#defineCOMP_MAX_WORDSIZE   (1  3)
+#defineCOMP_MAX_DATA_WIDTH (1  2)
+
 #define MAX_CHANNEL_NUM8
 #define MIN_CHANNEL_NUM2
 
@@ -335,11 +360,50 @@ static int dw_i2s_resume(struct snd_soc_dai *dai)
 #define dw_i2s_resume  NULL
 #endif
 
-static void dw_configure_dai_by_pd(struct dw_i2s_dev *dev,
+/*
+ * The following tables allow a direct lookup of various parameters
+ * defined in the I2S block's configuration in terms of sound system
+ * parameters.  Each table is sized to the number of entries possible
+ * according to the number of configuration bits describing an I2S
+ * block parameter.
+ */
+
+/* Width of (DMA) bus */
+static const u32 bus_widths[COMP_MAX_DATA_WIDTH] = {
+   DMA_SLAVE_BUSWIDTH_1_BYTE,
+   DMA_SLAVE_BUSWIDTH_2_BYTES,
+   DMA_SLAVE_BUSWIDTH_4_BYTES,
+   DMA_SLAVE_BUSWIDTH_UNDEFINED
+};
+
+/* PCM format to support channel resolution */
+static const u32 formats[COMP_MAX_WORDSIZE] = {
+   SNDRV_PCM_FMTBIT_S16_LE,
+   SNDRV_PCM_FMTBIT_S16_LE,
+   SNDRV_PCM_FMTBIT_S24_LE,
+   SNDRV_PCM_FMTBIT_S24_LE,
+   SNDRV_PCM_FMTBIT_S32_LE,
+   0,
+   0,
+   0
+};
+
+static int dw_configure_dai_by_pd(struct dw_i2s_dev *dev,
   struct snd_soc_dai_driver *dw_i2s_dai,
   struct resource *res,
   const struct i2s_platform_data *pdata)
 {
+   /*
+* Read component parameter registers to extract
+* the I2S block's configuration.
+*/
+   u32 comp1 = i2s_read_reg(dev-i2s_base, I2S_COMP_PARAM_1);
+   u32 comp2 = i2s_read_reg(dev-i2s_base, I2S_COMP_PARAM_2);
+   u32 idx = COMP1_APB_DATA_WIDTH(comp1);
+
+   if (WARN_ON(idx = ARRAY_SIZE(bus_widths)))
+   return -EINVAL;
+
/* Set DMA slaves info */
 
dev-play_dma_data.data = pdata-play_dma_data;
@@ -348,26 +412,36 @@ static void dw_configure_dai_by_pd(struct dw_i2s_dev *dev,
dev-capture_dma_data.addr = res-start + I2S_RXDMA;
dev-play_dma_data.max_burst = 16;
dev-capture_dma_data.max_burst = 16;
-   dev-play_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
-   dev-capture_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
+   dev-play_dma_data.addr_width = bus_widths[idx];
+   dev-capture_dma_data.addr_width = bus_widths[idx];
dev-play_dma_data.filter = pdata-filter;
dev-capture_dma_data.filter = pdata-filter;
 
-   if (pdata-cap  DWC_I2S_PLAY) {
+   if (COMP1_TX_ENABLED(comp1)) {
dev_dbg(dev-dev,  designware: play supported\n);
+   idx = COMP1_TX_WORDSIZE_0(comp1);
+   if (WARN_ON(idx = ARRAY_SIZE(formats)))
+   return -EINVAL;
dw_i2s_dai-playback.channels_min = MIN_CHANNEL_NUM;
-   dw_i2s_dai-playback.channels_max = pdata-channel;
-   dw_i2s_dai-playback.formats = pdata-snd_fmts;
+   dw_i2s_dai-playback.channels_max =
+   1  (COMP1_TX_CHANNELS(comp1) + 1

[PATCH v4 4/5] ASoC: dwc: Add documentation for I2S DT

2014-12-23 Thread Andrew Jackson
From: Andrew Jackson andrew.jack...@arm.com

Add documentation for Designware I2S hardware block.  The block requires
one clock (for audio sampling) and DMA channels for receive and transmit.

Signed-off-by: Andrew Jackson andrew.jack...@arm.com
---
 .../devicetree/bindings/sound/designware-i2s.txt   |   31 
 1 files changed, 31 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sound/designware-i2s.txt

diff --git a/Documentation/devicetree/bindings/sound/designware-i2s.txt 
b/Documentation/devicetree/bindings/sound/designware-i2s.txt
new file mode 100644
index 000..7bb5424
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/designware-i2s.txt
@@ -0,0 +1,31 @@
+DesignWare I2S controller
+
+Required properties:
+ - compatible : Must be snps,designware-i2s
+ - reg : Must contain the I2S core's registers location and length
+ - clocks : Pairs of phandle and specifier referencing the controller's
+   clocks. The controller expects one clock: the clock used as the sampling
+   rate reference clock sample.
+ - clock-names : i2sclk for the sample rate reference clock.
+ - dmas: Pairs of phandle and specifier for the DMA channels that are used by
+   the core. The core expects one or two dma channels: one for transmit and
+   one for receive.
+ - dma-names : tx for the transmit channel, rx for the receive channel.
+
+For more details on the 'dma', 'dma-names', 'clock' and 'clock-names'
+properties please check:
+   * resource-names.txt
+   * clock/clock-bindings.txt
+   * dma/dma.txt
+
+Example:
+
+   soc_i2s: i2s@7ff9 {
+   compatible = snps,designware-i2s;
+   reg = 0x0 0x7ff9 0x0 0x1000;
+   clocks = scpi_i2sclk 0;
+   clock-names = i2sclk;
+   #sound-dai-cells = 0;
+   dmas = dma0 5;
+   dma-names = tx;
+   };
-- 
1.7.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v4 1/5] ASoC: dwc: Prepare clock before use

2014-12-23 Thread Andrew Jackson
From: Andrew Jackson andrew.jack...@arm.com

Some I2S clocks may require some time to get the clock ready
for operation and so need to be prepared before they are enabled.
So, prepare the clock as well as enabling it, but combine the
two through clk_prepare_enable.

Signed-off-by: Andrew Jackson andrew.jack...@arm.com
---
 sound/soc/dwc/designware_i2s.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
index 06d3a34..9573ec7 100644
--- a/sound/soc/dwc/designware_i2s.c
+++ b/sound/soc/dwc/designware_i2s.c
@@ -411,7 +411,7 @@ static int dw_i2s_probe(struct platform_device *pdev)
if (IS_ERR(dev-clk))
return  PTR_ERR(dev-clk);
 
-   ret = clk_enable(dev-clk);
+   ret = clk_prepare_enable(dev-clk);
if (ret  0)
goto err_clk_put;
 
-- 
1.7.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v4 0/5] ASoC: dwc: Add device tree support to designware I2S

2014-12-23 Thread Andrew Jackson
From: Andrew Jackson andrew.jack...@arm.com

This patch set extends the DesignWare I2S driver to provide device
tree support and fixes a couple of small faults.

Changes v3-v4
  + Drop applied patches
  + Use combined function to prepare clock [Mark Brown]
  + Use managed clock resources to avoid clk_put [Mark Brown]
  + Read configuration prameters from hardware for both platform data
and device tree [Mark Brown]
  + Re-order patch sequence [Mark Brown]
  + Change union name to avoid future collisions [Mark Brown]
  + Check return code from clk_set_rate [Mark Brown]
  + Check parametsrs read from hardware agaist array limits and
add further comments [Mark Brown]
  + Add of_match_ptr [Mark Brown]

Changes v2-v3
  + Drop applied patch
  + Flush FIFOs in prepare rather than hw_params [Lars-Peter Clausen]

Changes v1-v2
  + Drop negative use count patch [Mark Brown]
  + Remove unnecessary debug print messages [Lars-Peter Clausen]
  + Rewrite iteration as for loop rather than do...while [Mark Brown]
  + Reorder patches to send fixes first [Mark Brown]
  + Simplify device tree code [Mark Brown]
  + Split device tree patch in two [Mark Brown]
  + Expand explanatory comment on channel configuration [Rajeev Kumar]

Arnd: I've not forgotten about updating the spear entries and
  will submit as a separate patch.

Andrew Jackson (5):
  ASoC: dwc: Prepare clock before use
  ASoC: dwc: Switch to managed clock resource
  ASoC: dwc: Read I2S block configuration from registers
  ASoC: dwc: Add documentation for I2S DT
  ASoC: dwc: Add devicetree support for Designware I2S

 .../devicetree/bindings/sound/designware-i2s.txt   |   31 +++
 sound/soc/dwc/Kconfig  |1 +
 sound/soc/dwc/designware_i2s.c |  275 
 3 files changed, 260 insertions(+), 47 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sound/designware-i2s.txt

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4 2/5] ASoC: dwc: Switch to managed clock resource

2014-12-23 Thread Andrew Jackson
On 12/23/14 13:56, Andrew Jackson wrote:
 From: Andrew Jackson andrew.jack...@arm.com
 
 Simplify error handling during probe by using managed clock
 resources.

In my haste to get to the Christmas turkey I missed a clk_put elsewhere.  Will 
fix and resubmit.

 Andrew

 Signed-off-by: Andrew Jackson andrew.jack...@arm.com
 ---
  sound/soc/dwc/designware_i2s.c |6 ++
  1 files changed, 2 insertions(+), 4 deletions(-)
 
 diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
 index 9573ec7..b3dc535 100644
 --- a/sound/soc/dwc/designware_i2s.c
 +++ b/sound/soc/dwc/designware_i2s.c
 @@ -407,13 +407,13 @@ static int dw_i2s_probe(struct platform_device *pdev)
  
   dev-capability = pdata-cap;
   dev-i2s_clk_cfg = pdata-i2s_clk_cfg;
 - dev-clk = clk_get(pdev-dev, NULL);
 + dev-clk = devm_clk_get(pdev-dev, NULL);
   if (IS_ERR(dev-clk))
   return  PTR_ERR(dev-clk);
  
   ret = clk_prepare_enable(dev-clk);
   if (ret  0)
 - goto err_clk_put;
 + return ret;
  
   dev_set_drvdata(pdev-dev, dev);
   ret = snd_soc_register_component(pdev-dev, dw_i2s_component,
 @@ -427,8 +427,6 @@ static int dw_i2s_probe(struct platform_device *pdev)
  
  err_clk_disable:
   clk_disable(dev-clk);
 -err_clk_put:
 - clk_put(dev-clk);
   return ret;
  }
  
 

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4 1/5] ASoC: dwc: Prepare clock before use

2014-12-23 Thread Andrew Jackson
On 12/23/14 13:56, Andrew Jackson wrote:
 From: Andrew Jackson andrew.jack...@arm.com
 
 Some I2S clocks may require some time to get the clock ready
 for operation and so need to be prepared before they are enabled.
 So, prepare the clock as well as enabling it, but combine the
 two through clk_prepare_enable.

More turkey-focussed issues: this should have a clk_disable_unprepare too.  Sigh

Andrew

 Signed-off-by: Andrew Jackson andrew.jack...@arm.com
 ---
  sound/soc/dwc/designware_i2s.c |2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)
 
 diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
 index 06d3a34..9573ec7 100644
 --- a/sound/soc/dwc/designware_i2s.c
 +++ b/sound/soc/dwc/designware_i2s.c
 @@ -411,7 +411,7 @@ static int dw_i2s_probe(struct platform_device *pdev)
   if (IS_ERR(dev-clk))
   return  PTR_ERR(dev-clk);
  
 - ret = clk_enable(dev-clk);
 + ret = clk_prepare_enable(dev-clk);
   if (ret  0)
   goto err_clk_put;
  
 

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v3 4/5] ASoC: dwc: Add devicetree support for Designware I2S

2014-12-22 Thread Andrew Jackson
On 12/22/14 14:10, Mark Brown wrote:
> On Fri, Dec 19, 2014 at 04:18:08PM +0000, Andrew Jackson wrote:
> 
>> +union snd_dma_data {
>> +struct i2s_dma_data pd;
>> +struct snd_dmaengine_dai_dma_data dt;
>> +};
>> +
> 
> This is a driver local union with a very generic name, it seems likely
> that this will collide in future causing build breaks

I'll change it.

> 
>> -ret = dev->i2s_clk_cfg(config);
>> -if (ret < 0) {
>> -dev_err(dev->dev, "runtime audio clk config fail\n");
>> -return ret;
>> +/* TODO: Validate sample rate against permissible set */
>> +bitclk = config->sample_rate * config->data_width * 2;
>> +clk_set_rate(dev->clk, bitclk);
>>  }
> 
> This is ignoring errors in clk_set_rate().
> 
>> +/* Maximum resolution of a channel - not uniformly spaced */
>> +static const u32 fifo_width[] = {
>> +12, 16, 20, 24, 32, 0, 0, 0
>> +};
>> +
>> +/* Width of (DMA) bus */
>> +static const u32 bus_widths[] = {
>> +DMA_SLAVE_BUSWIDTH_1_BYTE,
>> +DMA_SLAVE_BUSWIDTH_2_BYTES,
>> +DMA_SLAVE_BUSWIDTH_4_BYTES,
>> +DMA_SLAVE_BUSWIDTH_UNDEFINED
>> +};
> 
>> +u32 bus_width = bus_widths[COMP1_APB_DATA_WIDTH(comp1)];
>> +u32 fifo_depth = 1 << (1 + COMP1_FIFO_DEPTH_GLOBAL(comp1));
>> +u32 max_size;
> 
> I'd feel a lot more comfortable if there were bounds checking on these
> array indexes, especially since the arrays aren't explicitly sized and
> instead just have the number of elements that is (hopefully) safe with
> no comments or anything.  As things stand this is all using really
> fraigle idioms, this could easily be broken if someone is updating the
> driver for new IP features or even just cleaning up the code.

I will add robustness.  

> 
>> -dev->i2s_clk_cfg = pdata->i2s_clk_cfg;
>> -dev->clk = clk_get(>dev, NULL);
>> +dev->clk = clk_get(>dev, NULL);
>> +} else {
>> +dw_configure_dai_by_dt(dev, dw_i2s_dai, res);
>> +
>> +dev->clk = devm_clk_get(>dev, "i2sclk");
>> +}
> 
> This changes from clk_get() to devm_clk_get() but I'm not seeing
> anything that removes clk_put() calls.
> 
>> +#ifdef CONFIG_OF
>> +.of_match_table = dw_i2s_of_match,
>> +#endif
> 
> of_match_ptr().
> 

Thanks for all the comments.

Andrew
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v3 5/5] ASoC: dwc: Add documentation for I2S DT

2014-12-22 Thread Andrew Jackson
On 12/22/14 14:26, Mark Brown wrote:
> On Fri, Dec 19, 2014 at 04:18:09PM +0000, Andrew Jackson wrote:
> 
>> Add documentation for Designware I2S hardware block.  The block requires
>> two clocks (one for audio sampling, the other for APB) and DMA channels
>> for receive and transmit.
> 
> You should generally include the binding before the code to parse it,
> both because the binding is required in order to tell if the code is
> doing the right thing and also because people will often not even look
> at code with a missing binding.

Fair enough: I'll reorder the (remaining) patches.

>> + - clocks : Pairs of phandle and specifier referencing the controller's 
>> clocks.
>> +   The controller expects two clocks, the clock used for the APB interface 
>> and
>> +   the clock used as the sampling rate reference clock sample.
>> + - clock-names : "apb_plck" for the clock to the APB interface, "i2sclk" 
>> for the sample
>> +   rate reference clock.
> 
> This is a name based lookup of clocks but the code doesn't use
> apb_pclk at all; it needs to or the binding needs to say that apb_pclk
> must be the first listed clock (which would not be good).

I can remove apb_pclk: I was modelling the device tree entry on 
various PLxxx examples (c.f. amba-pl011) which also reference an AMBA clock
but don't use it.  (The effect being to document what clock the block is
driven by.)

>> +soc_i2s: i2s@7ff9 {
>> +compatible = "snps,designware-i2s";
>> +reg = <0x0 0x7ff9 0x0 0x1000>;
>> +clocks = <_i2sclk 0>, <_refclk100mhz>;
>> +clock-names = "i2sclk", "apb_pclk";
>> +#sound-dai-cells = <0>;
>> +dmas = < 5>;
>> +dma-names = "tx";
>> +};
> 
> This omits a lot of configurability that is in platform data and
> replaces it by reading back the parameters from the hardware.  If this
> is a viable approach to that configuration you should do this for both
> platform data and device tree rather than only device tree.  The point
> with keeping platform data is that it's not good to make the device DT
> only, improving the usability of platform data in a way that happens to
> also make the DT case easier is totally fine.  If we can determine how
> the IP is configured from the hardware that's both less work and more
> robust no matter how the device is instantiated.
> 

I agree.  I didn't do it like this originally because it wasn't clear
whether or not the original driver catered for some custom IP and I 
wanted to ensure that I didn't break the existing driver.  I'm happy to
switch both platform data and device tree to reading their parameters
from the hardware.

Andrew

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v3 2/5] ASoC: dwc: Iterate over all channels

2014-12-22 Thread Andrew Jackson
On 12/22/14 13:53, Mark Brown wrote:
> On Fri, Dec 19, 2014 at 04:18:06PM +0000, Andrew Jackson wrote:
> 
>> The Designware core can be configured with up to four stereo channels.
>> Each stereo channel is individually configured so, when the driver's
>> hw_params call is made, each requested stereo channel has to be
>> programmed.
> 
> This is quite unclear to someone who doesn't know the hardware, is this
> a bug fix or a new feature?  It looks like it's a fix...
> 

It is a fix.  In the Designware core, each stereo channel is configured
individually.  So, when hw_params is called to configure N channels, 
N/2 stereo channels need to be configured in the core.  The existing
code doesn't do this and will only configure the highest numbered 
channel.

Andrew

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v3 2/5] ASoC: dwc: Iterate over all channels

2014-12-22 Thread Andrew Jackson
On 12/22/14 13:53, Mark Brown wrote:
 On Fri, Dec 19, 2014 at 04:18:06PM +, Andrew Jackson wrote:
 
 The Designware core can be configured with up to four stereo channels.
 Each stereo channel is individually configured so, when the driver's
 hw_params call is made, each requested stereo channel has to be
 programmed.
 
 This is quite unclear to someone who doesn't know the hardware, is this
 a bug fix or a new feature?  It looks like it's a fix...
 

It is a fix.  In the Designware core, each stereo channel is configured
individually.  So, when hw_params is called to configure N channels, 
N/2 stereo channels need to be configured in the core.  The existing
code doesn't do this and will only configure the highest numbered 
channel.

Andrew

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v3 5/5] ASoC: dwc: Add documentation for I2S DT

2014-12-22 Thread Andrew Jackson
On 12/22/14 14:26, Mark Brown wrote:
 On Fri, Dec 19, 2014 at 04:18:09PM +, Andrew Jackson wrote:
 
 Add documentation for Designware I2S hardware block.  The block requires
 two clocks (one for audio sampling, the other for APB) and DMA channels
 for receive and transmit.
 
 You should generally include the binding before the code to parse it,
 both because the binding is required in order to tell if the code is
 doing the right thing and also because people will often not even look
 at code with a missing binding.

Fair enough: I'll reorder the (remaining) patches.

 + - clocks : Pairs of phandle and specifier referencing the controller's 
 clocks.
 +   The controller expects two clocks, the clock used for the APB interface 
 and
 +   the clock used as the sampling rate reference clock sample.
 + - clock-names : apb_plck for the clock to the APB interface, i2sclk 
 for the sample
 +   rate reference clock.
 
 This is a name based lookup of clocks but the code doesn't use
 apb_pclk at all; it needs to or the binding needs to say that apb_pclk
 must be the first listed clock (which would not be good).

I can remove apb_pclk: I was modelling the device tree entry on 
various PLxxx examples (c.f. amba-pl011) which also reference an AMBA clock
but don't use it.  (The effect being to document what clock the block is
driven by.)

 +soc_i2s: i2s@7ff9 {
 +compatible = snps,designware-i2s;
 +reg = 0x0 0x7ff9 0x0 0x1000;
 +clocks = scpi_i2sclk 0, soc_refclk100mhz;
 +clock-names = i2sclk, apb_pclk;
 +#sound-dai-cells = 0;
 +dmas = dma0 5;
 +dma-names = tx;
 +};
 
 This omits a lot of configurability that is in platform data and
 replaces it by reading back the parameters from the hardware.  If this
 is a viable approach to that configuration you should do this for both
 platform data and device tree rather than only device tree.  The point
 with keeping platform data is that it's not good to make the device DT
 only, improving the usability of platform data in a way that happens to
 also make the DT case easier is totally fine.  If we can determine how
 the IP is configured from the hardware that's both less work and more
 robust no matter how the device is instantiated.
 

I agree.  I didn't do it like this originally because it wasn't clear
whether or not the original driver catered for some custom IP and I 
wanted to ensure that I didn't break the existing driver.  I'm happy to
switch both platform data and device tree to reading their parameters
from the hardware.

Andrew

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v3 4/5] ASoC: dwc: Add devicetree support for Designware I2S

2014-12-22 Thread Andrew Jackson
On 12/22/14 14:10, Mark Brown wrote:
 On Fri, Dec 19, 2014 at 04:18:08PM +, Andrew Jackson wrote:
 
 +union snd_dma_data {
 +struct i2s_dma_data pd;
 +struct snd_dmaengine_dai_dma_data dt;
 +};
 +
 
 This is a driver local union with a very generic name, it seems likely
 that this will collide in future causing build breaks

I'll change it.

 
 -ret = dev-i2s_clk_cfg(config);
 -if (ret  0) {
 -dev_err(dev-dev, runtime audio clk config fail\n);
 -return ret;
 +/* TODO: Validate sample rate against permissible set */
 +bitclk = config-sample_rate * config-data_width * 2;
 +clk_set_rate(dev-clk, bitclk);
  }
 
 This is ignoring errors in clk_set_rate().
 
 +/* Maximum resolution of a channel - not uniformly spaced */
 +static const u32 fifo_width[] = {
 +12, 16, 20, 24, 32, 0, 0, 0
 +};
 +
 +/* Width of (DMA) bus */
 +static const u32 bus_widths[] = {
 +DMA_SLAVE_BUSWIDTH_1_BYTE,
 +DMA_SLAVE_BUSWIDTH_2_BYTES,
 +DMA_SLAVE_BUSWIDTH_4_BYTES,
 +DMA_SLAVE_BUSWIDTH_UNDEFINED
 +};
 
 +u32 bus_width = bus_widths[COMP1_APB_DATA_WIDTH(comp1)];
 +u32 fifo_depth = 1  (1 + COMP1_FIFO_DEPTH_GLOBAL(comp1));
 +u32 max_size;
 
 I'd feel a lot more comfortable if there were bounds checking on these
 array indexes, especially since the arrays aren't explicitly sized and
 instead just have the number of elements that is (hopefully) safe with
 no comments or anything.  As things stand this is all using really
 fraigle idioms, this could easily be broken if someone is updating the
 driver for new IP features or even just cleaning up the code.

I will add robustness.  

 
 -dev-i2s_clk_cfg = pdata-i2s_clk_cfg;
 -dev-clk = clk_get(pdev-dev, NULL);
 +dev-clk = clk_get(pdev-dev, NULL);
 +} else {
 +dw_configure_dai_by_dt(dev, dw_i2s_dai, res);
 +
 +dev-clk = devm_clk_get(pdev-dev, i2sclk);
 +}
 
 This changes from clk_get() to devm_clk_get() but I'm not seeing
 anything that removes clk_put() calls.
 
 +#ifdef CONFIG_OF
 +.of_match_table = dw_i2s_of_match,
 +#endif
 
 of_match_ptr().
 

Thanks for all the comments.

Andrew
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v3 3/5] ASoC: dwc: Reorder code in preparation for DT support

2014-12-19 Thread Andrew Jackson
From: Andrew Jackson 

Move code that configures the DAI and DMA into a separate function.  This
reduces the size of the dw_i2s_probe function and will make it easier to
add support for device tree to the driver.

Signed-off-by: Andrew Jackson 
---
 sound/soc/dwc/designware_i2s.c |   73 +--
 1 files changed, 39 insertions(+), 34 deletions(-)

diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
index 2ba1e2e..06d3a34 100644
--- a/sound/soc/dwc/designware_i2s.c
+++ b/sound/soc/dwc/designware_i2s.c
@@ -335,13 +335,47 @@ static int dw_i2s_resume(struct snd_soc_dai *dai)
 #define dw_i2s_resume  NULL
 #endif
 
+static void dw_configure_dai_by_pd(struct dw_i2s_dev *dev,
+  struct snd_soc_dai_driver *dw_i2s_dai,
+  struct resource *res,
+  const struct i2s_platform_data *pdata)
+{
+   /* Set DMA slaves info */
+
+   dev->play_dma_data.data = pdata->play_dma_data;
+   dev->capture_dma_data.data = pdata->capture_dma_data;
+   dev->play_dma_data.addr = res->start + I2S_TXDMA;
+   dev->capture_dma_data.addr = res->start + I2S_RXDMA;
+   dev->play_dma_data.max_burst = 16;
+   dev->capture_dma_data.max_burst = 16;
+   dev->play_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
+   dev->capture_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
+   dev->play_dma_data.filter = pdata->filter;
+   dev->capture_dma_data.filter = pdata->filter;
+
+   if (pdata->cap & DWC_I2S_PLAY) {
+   dev_dbg(dev->dev, " designware: play supported\n");
+   dw_i2s_dai->playback.channels_min = MIN_CHANNEL_NUM;
+   dw_i2s_dai->playback.channels_max = pdata->channel;
+   dw_i2s_dai->playback.formats = pdata->snd_fmts;
+   dw_i2s_dai->playback.rates = pdata->snd_rates;
+   }
+
+   if (pdata->cap & DWC_I2S_RECORD) {
+   dev_dbg(dev->dev, "designware: record supported\n");
+   dw_i2s_dai->capture.channels_min = MIN_CHANNEL_NUM;
+   dw_i2s_dai->capture.channels_max = pdata->channel;
+   dw_i2s_dai->capture.formats = pdata->snd_fmts;
+   dw_i2s_dai->capture.rates = pdata->snd_rates;
+   }
+}
+
 static int dw_i2s_probe(struct platform_device *pdev)
 {
const struct i2s_platform_data *pdata = pdev->dev.platform_data;
struct dw_i2s_dev *dev;
struct resource *res;
int ret;
-   unsigned int cap;
struct snd_soc_dai_driver *dw_i2s_dai;
 
if (!pdata) {
@@ -368,23 +402,11 @@ static int dw_i2s_probe(struct platform_device *pdev)
if (IS_ERR(dev->i2s_base))
return PTR_ERR(dev->i2s_base);
 
-   cap = pdata->cap;
-   dev->capability = cap;
-   dev->i2s_clk_cfg = pdata->i2s_clk_cfg;
-
-   /* Set DMA slaves info */
-
-   dev->play_dma_data.data = pdata->play_dma_data;
-   dev->capture_dma_data.data = pdata->capture_dma_data;
-   dev->play_dma_data.addr = res->start + I2S_TXDMA;
-   dev->capture_dma_data.addr = res->start + I2S_RXDMA;
-   dev->play_dma_data.max_burst = 16;
-   dev->capture_dma_data.max_burst = 16;
-   dev->play_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
-   dev->capture_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
-   dev->play_dma_data.filter = pdata->filter;
-   dev->capture_dma_data.filter = pdata->filter;
+   dev->dev = >dev;
+   dw_configure_dai_by_pd(dev, dw_i2s_dai, res, pdata);
 
+   dev->capability = pdata->cap;
+   dev->i2s_clk_cfg = pdata->i2s_clk_cfg;
dev->clk = clk_get(>dev, NULL);
if (IS_ERR(dev->clk))
return  PTR_ERR(dev->clk);
@@ -393,23 +415,6 @@ static int dw_i2s_probe(struct platform_device *pdev)
if (ret < 0)
goto err_clk_put;
 
-   if (cap & DWC_I2S_PLAY) {
-   dev_dbg(>dev, " designware: play supported\n");
-   dw_i2s_dai->playback.channels_min = MIN_CHANNEL_NUM;
-   dw_i2s_dai->playback.channels_max = pdata->channel;
-   dw_i2s_dai->playback.formats = pdata->snd_fmts;
-   dw_i2s_dai->playback.rates = pdata->snd_rates;
-   }
-
-   if (cap & DWC_I2S_RECORD) {
-   dev_dbg(>dev, "designware: record supported\n");
-   dw_i2s_dai->capture.channels_min = MIN_CHANNEL_NUM;
-   dw_i2s_dai->capture.channels_max = pdata->channel;
-   dw_i2s_dai->capture.formats = pdata->snd_fmts;
-   dw_i2s_dai->capture.rates = pdata->snd_rates;
-   }
-
-   dev->d

[PATCH v3 0/5] ASoC: dwc: Add device tree support to designware I2S

2014-12-19 Thread Andrew Jackson
From: Andrew Jackson 

This patch set extends the DesignWare I2S driver to provide device
tree support and fixes a couple of small faults.

Changes v2->v3
  + Drop applied patch
  + Flush FIFOs in prepare rather than hw_params [Lars-Peter Clausen]

Changes v1->v2
  + Drop negative use count patch [Mark Brown]
  + Remove unnecessary debug print messages [Lars-Peter Clausen]
  + Rewrite iteration as for loop rather than do...while [Mark Brown]
  + Reorder patches to send fixes first [Mark Brown]
  + Simplify device tree code [Mark Brown]
  + Split device tree patch in two [Mark Brown]
  + Expand explanatory comment on channel configuration [Rajeev Kumar]

Arnd: I've not forgotten about updating the spear entries and
  will submit as a separate patch.

Andrew Jackson (5):
  ASoC: dwc: Ensure FIFOs are flushed to prevent channel swap
  ASoC: dwc: Iterate over all channels
  ASoC: dwc: Reorder code in preparation for DT support
  ASoC: dwc: Add devicetree support for Designware I2S
  ASoC: dwc: Add documentation for I2S DT

 .../devicetree/bindings/sound/designware-i2s.txt   |   32 +++
 sound/soc/dwc/Kconfig  |1 +
 sound/soc/dwc/designware_i2s.c |  293 +++-
 3 files changed, 258 insertions(+), 68 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sound/designware-i2s.txt

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v3 5/5] ASoC: dwc: Add documentation for I2S DT

2014-12-19 Thread Andrew Jackson
From: Andrew Jackson 

Add documentation for Designware I2S hardware block.  The block requires
two clocks (one for audio sampling, the other for APB) and DMA channels
for receive and transmit.

Signed-off-by: Andrew Jackson 
---
 .../devicetree/bindings/sound/designware-i2s.txt   |   32 
 1 files changed, 32 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sound/designware-i2s.txt

diff --git a/Documentation/devicetree/bindings/sound/designware-i2s.txt 
b/Documentation/devicetree/bindings/sound/designware-i2s.txt
new file mode 100644
index 000..cdee591
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/designware-i2s.txt
@@ -0,0 +1,32 @@
+DesignWare I2S controller
+
+Required properties:
+ - compatible : Must be "snps,designware-i2s"
+ - reg : Must contain I2S core's registers location and length
+ - clocks : Pairs of phandle and specifier referencing the controller's clocks.
+   The controller expects two clocks, the clock used for the APB interface and
+   the clock used as the sampling rate reference clock sample.
+ - clock-names : "apb_plck" for the clock to the APB interface, "i2sclk" for 
the sample
+   rate reference clock.
+ - dmas: Pairs of phandle and specifier for the DMA channels that are used by
+   the core. The core expects one or two dma channels, one for transmit and 
one for
+   receive.
+ - dma-names : "tx" for the transmit channel, "rx" for the receive channel.
+
+For more details on the 'dma', 'dma-names', 'clock' and 'clock-names' 
properties
+please check:
+   * resource-names.txt
+   * clock/clock-bindings.txt
+   * dma/dma.txt
+
+Example:
+
+   soc_i2s: i2s@7ff9 {
+   compatible = "snps,designware-i2s";
+   reg = <0x0 0x7ff9 0x0 0x1000>;
+   clocks = <_i2sclk 0>, <_refclk100mhz>;
+   clock-names = "i2sclk", "apb_pclk";
+   #sound-dai-cells = <0>;
+   dmas = < 5>;
+   dma-names = "tx";
+   };
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v3 4/5] ASoC: dwc: Add devicetree support for Designware I2S

2014-12-19 Thread Andrew Jackson
From: Andrew Jackson 

Allow the driver to be configured through a device tree rather than platform
data.  When using device-tree, read the I2S block's configuration from the
relevant registers: this reduces the amount of information required in
the device tree.

Signed-off-by: Andrew Jackson 
---
 sound/soc/dwc/Kconfig  |1 +
 sound/soc/dwc/designware_i2s.c |  199 ++--
 2 files changed, 171 insertions(+), 29 deletions(-)

diff --git a/sound/soc/dwc/Kconfig b/sound/soc/dwc/Kconfig
index e334900..d50e085 100644
--- a/sound/soc/dwc/Kconfig
+++ b/sound/soc/dwc/Kconfig
@@ -1,6 +1,7 @@
 config SND_DESIGNWARE_I2S
tristate "Synopsys I2S Device Driver"
depends on CLKDEV_LOOKUP
+   select SND_SOC_GENERIC_DMAENGINE_PCM
help
 Say Y or M if you want to add support for I2S driver for
 Synopsys desigwnware I2S device. The device supports upto
diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
index 06d3a34..7905c52 100644
--- a/sound/soc/dwc/designware_i2s.c
+++ b/sound/soc/dwc/designware_i2s.c
@@ -22,6 +22,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /* common register for all channel */
 #define IER0x000
@@ -54,9 +55,35 @@
 #define I2S_COMP_VERSION   0x01F8
 #define I2S_COMP_TYPE  0x01FC
 
+/*
+ * Component parameter register fields - define the I2S block's
+ * configuration.
+ */
+#defineCOMP1_TX_WORDSIZE_3(r)  (((r) & GENMASK(27, 25)) >> 25)
+#defineCOMP1_TX_WORDSIZE_2(r)  (((r) & GENMASK(24, 22)) >> 22)
+#defineCOMP1_TX_WORDSIZE_1(r)  (((r) & GENMASK(21, 19)) >> 19)
+#defineCOMP1_TX_WORDSIZE_0(r)  (((r) & GENMASK(18, 16)) >> 16)
+#defineCOMP1_TX_CHANNELS(r)(((r) & GENMASK(10, 9)) >> 9)
+#defineCOMP1_RX_CHANNELS(r)(((r) & GENMASK(8, 7)) >> 7)
+#defineCOMP1_RX_ENABLED(r) (((r) & BIT(6)) >> 6)
+#defineCOMP1_TX_ENABLED(r) (((r) & BIT(5)) >> 5)
+#defineCOMP1_MODE_EN(r)(((r) & BIT(4)) >> 4)
+#defineCOMP1_FIFO_DEPTH_GLOBAL(r)  (((r) & GENMASK(3, 2)) >> 2)
+#defineCOMP1_APB_DATA_WIDTH(r) (((r) & GENMASK(1, 0)) >> 0)
+
+#defineCOMP2_RX_WORDSIZE_3(r)  (((r) & GENMASK(12, 10)) >> 10)
+#defineCOMP2_RX_WORDSIZE_2(r)  (((r) & GENMASK(9, 7)) >> 7)
+#defineCOMP2_RX_WORDSIZE_1(r)  (((r) & GENMASK(5, 3)) >> 3)
+#defineCOMP2_RX_WORDSIZE_0(r)  (((r) & GENMASK(2, 0)) >> 0)
+
 #define MAX_CHANNEL_NUM8
 #define MIN_CHANNEL_NUM2
 
+union snd_dma_data {
+   struct i2s_dma_data pd;
+   struct snd_dmaengine_dai_dma_data dt;
+};
+
 struct dw_i2s_dev {
void __iomem *i2s_base;
struct clk *clk;
@@ -65,8 +92,8 @@ struct dw_i2s_dev {
struct device *dev;
 
/* data related to DMA transfers b/w i2s and DMAC */
-   struct i2s_dma_data play_dma_data;
-   struct i2s_dma_data capture_dma_data;
+   union snd_dma_data play_dma_data;
+   union snd_dma_data capture_dma_data;
struct i2s_clk_config_data config;
int (*i2s_clk_cfg)(struct i2s_clk_config_data *config);
 };
@@ -153,7 +180,7 @@ static int dw_i2s_startup(struct snd_pcm_substream 
*substream,
struct snd_soc_dai *cpu_dai)
 {
struct dw_i2s_dev *dev = snd_soc_dai_get_drvdata(cpu_dai);
-   struct i2s_dma_data *dma_data = NULL;
+   union snd_dma_data *dma_data = NULL;
 
if (!(dev->capability & DWC_I2S_RECORD) &&
(substream->stream == SNDRV_PCM_STREAM_CAPTURE))
@@ -242,13 +269,18 @@ static int dw_i2s_hw_params(struct snd_pcm_substream 
*substream,
 
config->sample_rate = params_rate(params);
 
-   if (!dev->i2s_clk_cfg)
-   return -EINVAL;
+   if (dev->i2s_clk_cfg) {
+   ret = dev->i2s_clk_cfg(config);
+   if (ret < 0) {
+   dev_err(dev->dev, "runtime audio clk config fail\n");
+   return ret;
+   }
+   } else {
+   u32 bitclk;
 
-   ret = dev->i2s_clk_cfg(config);
-   if (ret < 0) {
-   dev_err(dev->dev, "runtime audio clk config fail\n");
-   return ret;
+   /* TODO: Validate sample rate against permissible set */
+   bitclk = config->sample_rate * config->data_width * 2;
+   clk_set_rate(dev->clk, bitclk);
}
 
return 0;
@@ -335,6 +367,31 @@ static int dw_i2s_resume(struct snd_soc_dai *dai)
 #define dw_i2s_resume  NULL
 #endif
 
+/* Maximum resolution of a channel - not uniformly spaced */
+static const u32 fifo_width[] = {
+   12, 16, 20, 24, 32, 0, 0, 0
+};
+
+/* Width of (DMA) bus */
+static const u32 bus_wi

[PATCH v3 2/5] ASoC: dwc: Iterate over all channels

2014-12-19 Thread Andrew Jackson
From: Andrew Jackson 

The Designware core can be configured with up to four stereo channels.
Each stereo channel is individually configured so, when the driver's
hw_params call is made, each requested stereo channel has to be
programmed.

Signed-off-by: Andrew Jackson 
---
 sound/soc/dwc/designware_i2s.c |   35 ---
 1 files changed, 16 insertions(+), 19 deletions(-)

diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
index 5c13303..2ba1e2e 100644
--- a/sound/soc/dwc/designware_i2s.c
+++ b/sound/soc/dwc/designware_i2s.c
@@ -209,16 +209,9 @@ static int dw_i2s_hw_params(struct snd_pcm_substream 
*substream,
 
switch (config->chan_nr) {
case EIGHT_CHANNEL_SUPPORT:
-   ch_reg = 3;
-   break;
case SIX_CHANNEL_SUPPORT:
-   ch_reg = 2;
-   break;
case FOUR_CHANNEL_SUPPORT:
-   ch_reg = 1;
-   break;
case TWO_CHANNEL_SUPPORT:
-   ch_reg = 0;
break;
default:
dev_err(dev->dev, "channel not supported\n");
@@ -227,18 +220,22 @@ static int dw_i2s_hw_params(struct snd_pcm_substream 
*substream,
 
i2s_disable_channels(dev, substream->stream);
 
-   if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
-   i2s_write_reg(dev->i2s_base, TCR(ch_reg), xfer_resolution);
-   i2s_write_reg(dev->i2s_base, TFCR(ch_reg), 0x02);
-   irq = i2s_read_reg(dev->i2s_base, IMR(ch_reg));
-   i2s_write_reg(dev->i2s_base, IMR(ch_reg), irq & ~0x30);
-   i2s_write_reg(dev->i2s_base, TER(ch_reg), 1);
-   } else {
-   i2s_write_reg(dev->i2s_base, RCR(ch_reg), xfer_resolution);
-   i2s_write_reg(dev->i2s_base, RFCR(ch_reg), 0x07);
-   irq = i2s_read_reg(dev->i2s_base, IMR(ch_reg));
-   i2s_write_reg(dev->i2s_base, IMR(ch_reg), irq & ~0x03);
-   i2s_write_reg(dev->i2s_base, RER(ch_reg), 1);
+   for (ch_reg = 0; ch_reg < (config->chan_nr / 2); ch_reg++) {
+   if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+   i2s_write_reg(dev->i2s_base, TCR(ch_reg),
+ xfer_resolution);
+   i2s_write_reg(dev->i2s_base, TFCR(ch_reg), 0x02);
+   irq = i2s_read_reg(dev->i2s_base, IMR(ch_reg));
+   i2s_write_reg(dev->i2s_base, IMR(ch_reg), irq & ~0x30);
+   i2s_write_reg(dev->i2s_base, TER(ch_reg), 1);
+   } else {
+   i2s_write_reg(dev->i2s_base, RCR(ch_reg),
+ xfer_resolution);
+   i2s_write_reg(dev->i2s_base, RFCR(ch_reg), 0x07);
+   irq = i2s_read_reg(dev->i2s_base, IMR(ch_reg));
+   i2s_write_reg(dev->i2s_base, IMR(ch_reg), irq & ~0x03);
+   i2s_write_reg(dev->i2s_base, RER(ch_reg), 1);
+   }
}
 
i2s_write_reg(dev->i2s_base, CCR, ccr);
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v3 1/5] ASoC: dwc: Ensure FIFOs are flushed to prevent channel swap

2014-12-19 Thread Andrew Jackson
From: Andrew Jackson 

Flush the FIFOs when the stream is prepared for use.  This avoids
an inadvertent swapping of the left/right channels if the FIFOs are
not empty at startup.

Signed-off-by: Andrew Jackson 
---
 sound/soc/dwc/designware_i2s.c |   14 ++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
index f81e747..5c13303 100644
--- a/sound/soc/dwc/designware_i2s.c
+++ b/sound/soc/dwc/designware_i2s.c
@@ -263,6 +263,19 @@ static void dw_i2s_shutdown(struct snd_pcm_substream 
*substream,
snd_soc_dai_set_dma_data(dai, substream, NULL);
 }
 
+static int dw_i2s_prepare(struct snd_pcm_substream *substream,
+ struct snd_soc_dai *dai)
+{
+   struct dw_i2s_dev *dev = snd_soc_dai_get_drvdata(dai);
+
+   if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+   i2s_write_reg(dev->i2s_base, TXFFR, 1);
+   else
+   i2s_write_reg(dev->i2s_base, RXFFR, 1);
+
+   return 0;
+}
+
 static int dw_i2s_trigger(struct snd_pcm_substream *substream,
int cmd, struct snd_soc_dai *dai)
 {
@@ -294,6 +307,7 @@ static struct snd_soc_dai_ops dw_i2s_dai_ops = {
.startup= dw_i2s_startup,
.shutdown   = dw_i2s_shutdown,
.hw_params  = dw_i2s_hw_params,
+   .prepare= dw_i2s_prepare,
.trigger= dw_i2s_trigger,
 };
 
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v3 1/5] ASoC: dwc: Ensure FIFOs are flushed to prevent channel swap

2014-12-19 Thread Andrew Jackson
From: Andrew Jackson andrew.jack...@arm.com

Flush the FIFOs when the stream is prepared for use.  This avoids
an inadvertent swapping of the left/right channels if the FIFOs are
not empty at startup.

Signed-off-by: Andrew Jackson andrew.jack...@arm.com
---
 sound/soc/dwc/designware_i2s.c |   14 ++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
index f81e747..5c13303 100644
--- a/sound/soc/dwc/designware_i2s.c
+++ b/sound/soc/dwc/designware_i2s.c
@@ -263,6 +263,19 @@ static void dw_i2s_shutdown(struct snd_pcm_substream 
*substream,
snd_soc_dai_set_dma_data(dai, substream, NULL);
 }
 
+static int dw_i2s_prepare(struct snd_pcm_substream *substream,
+ struct snd_soc_dai *dai)
+{
+   struct dw_i2s_dev *dev = snd_soc_dai_get_drvdata(dai);
+
+   if (substream-stream == SNDRV_PCM_STREAM_PLAYBACK)
+   i2s_write_reg(dev-i2s_base, TXFFR, 1);
+   else
+   i2s_write_reg(dev-i2s_base, RXFFR, 1);
+
+   return 0;
+}
+
 static int dw_i2s_trigger(struct snd_pcm_substream *substream,
int cmd, struct snd_soc_dai *dai)
 {
@@ -294,6 +307,7 @@ static struct snd_soc_dai_ops dw_i2s_dai_ops = {
.startup= dw_i2s_startup,
.shutdown   = dw_i2s_shutdown,
.hw_params  = dw_i2s_hw_params,
+   .prepare= dw_i2s_prepare,
.trigger= dw_i2s_trigger,
 };
 
-- 
1.7.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v3 3/5] ASoC: dwc: Reorder code in preparation for DT support

2014-12-19 Thread Andrew Jackson
From: Andrew Jackson andrew.jack...@arm.com

Move code that configures the DAI and DMA into a separate function.  This
reduces the size of the dw_i2s_probe function and will make it easier to
add support for device tree to the driver.

Signed-off-by: Andrew Jackson andrew.jack...@arm.com
---
 sound/soc/dwc/designware_i2s.c |   73 +--
 1 files changed, 39 insertions(+), 34 deletions(-)

diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
index 2ba1e2e..06d3a34 100644
--- a/sound/soc/dwc/designware_i2s.c
+++ b/sound/soc/dwc/designware_i2s.c
@@ -335,13 +335,47 @@ static int dw_i2s_resume(struct snd_soc_dai *dai)
 #define dw_i2s_resume  NULL
 #endif
 
+static void dw_configure_dai_by_pd(struct dw_i2s_dev *dev,
+  struct snd_soc_dai_driver *dw_i2s_dai,
+  struct resource *res,
+  const struct i2s_platform_data *pdata)
+{
+   /* Set DMA slaves info */
+
+   dev-play_dma_data.data = pdata-play_dma_data;
+   dev-capture_dma_data.data = pdata-capture_dma_data;
+   dev-play_dma_data.addr = res-start + I2S_TXDMA;
+   dev-capture_dma_data.addr = res-start + I2S_RXDMA;
+   dev-play_dma_data.max_burst = 16;
+   dev-capture_dma_data.max_burst = 16;
+   dev-play_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
+   dev-capture_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
+   dev-play_dma_data.filter = pdata-filter;
+   dev-capture_dma_data.filter = pdata-filter;
+
+   if (pdata-cap  DWC_I2S_PLAY) {
+   dev_dbg(dev-dev,  designware: play supported\n);
+   dw_i2s_dai-playback.channels_min = MIN_CHANNEL_NUM;
+   dw_i2s_dai-playback.channels_max = pdata-channel;
+   dw_i2s_dai-playback.formats = pdata-snd_fmts;
+   dw_i2s_dai-playback.rates = pdata-snd_rates;
+   }
+
+   if (pdata-cap  DWC_I2S_RECORD) {
+   dev_dbg(dev-dev, designware: record supported\n);
+   dw_i2s_dai-capture.channels_min = MIN_CHANNEL_NUM;
+   dw_i2s_dai-capture.channels_max = pdata-channel;
+   dw_i2s_dai-capture.formats = pdata-snd_fmts;
+   dw_i2s_dai-capture.rates = pdata-snd_rates;
+   }
+}
+
 static int dw_i2s_probe(struct platform_device *pdev)
 {
const struct i2s_platform_data *pdata = pdev-dev.platform_data;
struct dw_i2s_dev *dev;
struct resource *res;
int ret;
-   unsigned int cap;
struct snd_soc_dai_driver *dw_i2s_dai;
 
if (!pdata) {
@@ -368,23 +402,11 @@ static int dw_i2s_probe(struct platform_device *pdev)
if (IS_ERR(dev-i2s_base))
return PTR_ERR(dev-i2s_base);
 
-   cap = pdata-cap;
-   dev-capability = cap;
-   dev-i2s_clk_cfg = pdata-i2s_clk_cfg;
-
-   /* Set DMA slaves info */
-
-   dev-play_dma_data.data = pdata-play_dma_data;
-   dev-capture_dma_data.data = pdata-capture_dma_data;
-   dev-play_dma_data.addr = res-start + I2S_TXDMA;
-   dev-capture_dma_data.addr = res-start + I2S_RXDMA;
-   dev-play_dma_data.max_burst = 16;
-   dev-capture_dma_data.max_burst = 16;
-   dev-play_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
-   dev-capture_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
-   dev-play_dma_data.filter = pdata-filter;
-   dev-capture_dma_data.filter = pdata-filter;
+   dev-dev = pdev-dev;
+   dw_configure_dai_by_pd(dev, dw_i2s_dai, res, pdata);
 
+   dev-capability = pdata-cap;
+   dev-i2s_clk_cfg = pdata-i2s_clk_cfg;
dev-clk = clk_get(pdev-dev, NULL);
if (IS_ERR(dev-clk))
return  PTR_ERR(dev-clk);
@@ -393,23 +415,6 @@ static int dw_i2s_probe(struct platform_device *pdev)
if (ret  0)
goto err_clk_put;
 
-   if (cap  DWC_I2S_PLAY) {
-   dev_dbg(pdev-dev,  designware: play supported\n);
-   dw_i2s_dai-playback.channels_min = MIN_CHANNEL_NUM;
-   dw_i2s_dai-playback.channels_max = pdata-channel;
-   dw_i2s_dai-playback.formats = pdata-snd_fmts;
-   dw_i2s_dai-playback.rates = pdata-snd_rates;
-   }
-
-   if (cap  DWC_I2S_RECORD) {
-   dev_dbg(pdev-dev, designware: record supported\n);
-   dw_i2s_dai-capture.channels_min = MIN_CHANNEL_NUM;
-   dw_i2s_dai-capture.channels_max = pdata-channel;
-   dw_i2s_dai-capture.formats = pdata-snd_fmts;
-   dw_i2s_dai-capture.rates = pdata-snd_rates;
-   }
-
-   dev-dev = pdev-dev;
dev_set_drvdata(pdev-dev, dev);
ret = snd_soc_register_component(pdev-dev, dw_i2s_component,
 dw_i2s_dai, 1);
-- 
1.7.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http

[PATCH v3 0/5] ASoC: dwc: Add device tree support to designware I2S

2014-12-19 Thread Andrew Jackson
From: Andrew Jackson andrew.jack...@arm.com

This patch set extends the DesignWare I2S driver to provide device
tree support and fixes a couple of small faults.

Changes v2-v3
  + Drop applied patch
  + Flush FIFOs in prepare rather than hw_params [Lars-Peter Clausen]

Changes v1-v2
  + Drop negative use count patch [Mark Brown]
  + Remove unnecessary debug print messages [Lars-Peter Clausen]
  + Rewrite iteration as for loop rather than do...while [Mark Brown]
  + Reorder patches to send fixes first [Mark Brown]
  + Simplify device tree code [Mark Brown]
  + Split device tree patch in two [Mark Brown]
  + Expand explanatory comment on channel configuration [Rajeev Kumar]

Arnd: I've not forgotten about updating the spear entries and
  will submit as a separate patch.

Andrew Jackson (5):
  ASoC: dwc: Ensure FIFOs are flushed to prevent channel swap
  ASoC: dwc: Iterate over all channels
  ASoC: dwc: Reorder code in preparation for DT support
  ASoC: dwc: Add devicetree support for Designware I2S
  ASoC: dwc: Add documentation for I2S DT

 .../devicetree/bindings/sound/designware-i2s.txt   |   32 +++
 sound/soc/dwc/Kconfig  |1 +
 sound/soc/dwc/designware_i2s.c |  293 +++-
 3 files changed, 258 insertions(+), 68 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sound/designware-i2s.txt

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v3 5/5] ASoC: dwc: Add documentation for I2S DT

2014-12-19 Thread Andrew Jackson
From: Andrew Jackson andrew.jack...@arm.com

Add documentation for Designware I2S hardware block.  The block requires
two clocks (one for audio sampling, the other for APB) and DMA channels
for receive and transmit.

Signed-off-by: Andrew Jackson andrew.jack...@arm.com
---
 .../devicetree/bindings/sound/designware-i2s.txt   |   32 
 1 files changed, 32 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sound/designware-i2s.txt

diff --git a/Documentation/devicetree/bindings/sound/designware-i2s.txt 
b/Documentation/devicetree/bindings/sound/designware-i2s.txt
new file mode 100644
index 000..cdee591
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/designware-i2s.txt
@@ -0,0 +1,32 @@
+DesignWare I2S controller
+
+Required properties:
+ - compatible : Must be snps,designware-i2s
+ - reg : Must contain I2S core's registers location and length
+ - clocks : Pairs of phandle and specifier referencing the controller's clocks.
+   The controller expects two clocks, the clock used for the APB interface and
+   the clock used as the sampling rate reference clock sample.
+ - clock-names : apb_plck for the clock to the APB interface, i2sclk for 
the sample
+   rate reference clock.
+ - dmas: Pairs of phandle and specifier for the DMA channels that are used by
+   the core. The core expects one or two dma channels, one for transmit and 
one for
+   receive.
+ - dma-names : tx for the transmit channel, rx for the receive channel.
+
+For more details on the 'dma', 'dma-names', 'clock' and 'clock-names' 
properties
+please check:
+   * resource-names.txt
+   * clock/clock-bindings.txt
+   * dma/dma.txt
+
+Example:
+
+   soc_i2s: i2s@7ff9 {
+   compatible = snps,designware-i2s;
+   reg = 0x0 0x7ff9 0x0 0x1000;
+   clocks = scpi_i2sclk 0, soc_refclk100mhz;
+   clock-names = i2sclk, apb_pclk;
+   #sound-dai-cells = 0;
+   dmas = dma0 5;
+   dma-names = tx;
+   };
-- 
1.7.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v3 4/5] ASoC: dwc: Add devicetree support for Designware I2S

2014-12-19 Thread Andrew Jackson
From: Andrew Jackson andrew.jack...@arm.com

Allow the driver to be configured through a device tree rather than platform
data.  When using device-tree, read the I2S block's configuration from the
relevant registers: this reduces the amount of information required in
the device tree.

Signed-off-by: Andrew Jackson andrew.jack...@arm.com
---
 sound/soc/dwc/Kconfig  |1 +
 sound/soc/dwc/designware_i2s.c |  199 ++--
 2 files changed, 171 insertions(+), 29 deletions(-)

diff --git a/sound/soc/dwc/Kconfig b/sound/soc/dwc/Kconfig
index e334900..d50e085 100644
--- a/sound/soc/dwc/Kconfig
+++ b/sound/soc/dwc/Kconfig
@@ -1,6 +1,7 @@
 config SND_DESIGNWARE_I2S
tristate Synopsys I2S Device Driver
depends on CLKDEV_LOOKUP
+   select SND_SOC_GENERIC_DMAENGINE_PCM
help
 Say Y or M if you want to add support for I2S driver for
 Synopsys desigwnware I2S device. The device supports upto
diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
index 06d3a34..7905c52 100644
--- a/sound/soc/dwc/designware_i2s.c
+++ b/sound/soc/dwc/designware_i2s.c
@@ -22,6 +22,7 @@
 #include sound/pcm.h
 #include sound/pcm_params.h
 #include sound/soc.h
+#include sound/dmaengine_pcm.h
 
 /* common register for all channel */
 #define IER0x000
@@ -54,9 +55,35 @@
 #define I2S_COMP_VERSION   0x01F8
 #define I2S_COMP_TYPE  0x01FC
 
+/*
+ * Component parameter register fields - define the I2S block's
+ * configuration.
+ */
+#defineCOMP1_TX_WORDSIZE_3(r)  (((r)  GENMASK(27, 25))  25)
+#defineCOMP1_TX_WORDSIZE_2(r)  (((r)  GENMASK(24, 22))  22)
+#defineCOMP1_TX_WORDSIZE_1(r)  (((r)  GENMASK(21, 19))  19)
+#defineCOMP1_TX_WORDSIZE_0(r)  (((r)  GENMASK(18, 16))  16)
+#defineCOMP1_TX_CHANNELS(r)(((r)  GENMASK(10, 9))  9)
+#defineCOMP1_RX_CHANNELS(r)(((r)  GENMASK(8, 7))  7)
+#defineCOMP1_RX_ENABLED(r) (((r)  BIT(6))  6)
+#defineCOMP1_TX_ENABLED(r) (((r)  BIT(5))  5)
+#defineCOMP1_MODE_EN(r)(((r)  BIT(4))  4)
+#defineCOMP1_FIFO_DEPTH_GLOBAL(r)  (((r)  GENMASK(3, 2))  2)
+#defineCOMP1_APB_DATA_WIDTH(r) (((r)  GENMASK(1, 0))  0)
+
+#defineCOMP2_RX_WORDSIZE_3(r)  (((r)  GENMASK(12, 10))  10)
+#defineCOMP2_RX_WORDSIZE_2(r)  (((r)  GENMASK(9, 7))  7)
+#defineCOMP2_RX_WORDSIZE_1(r)  (((r)  GENMASK(5, 3))  3)
+#defineCOMP2_RX_WORDSIZE_0(r)  (((r)  GENMASK(2, 0))  0)
+
 #define MAX_CHANNEL_NUM8
 #define MIN_CHANNEL_NUM2
 
+union snd_dma_data {
+   struct i2s_dma_data pd;
+   struct snd_dmaengine_dai_dma_data dt;
+};
+
 struct dw_i2s_dev {
void __iomem *i2s_base;
struct clk *clk;
@@ -65,8 +92,8 @@ struct dw_i2s_dev {
struct device *dev;
 
/* data related to DMA transfers b/w i2s and DMAC */
-   struct i2s_dma_data play_dma_data;
-   struct i2s_dma_data capture_dma_data;
+   union snd_dma_data play_dma_data;
+   union snd_dma_data capture_dma_data;
struct i2s_clk_config_data config;
int (*i2s_clk_cfg)(struct i2s_clk_config_data *config);
 };
@@ -153,7 +180,7 @@ static int dw_i2s_startup(struct snd_pcm_substream 
*substream,
struct snd_soc_dai *cpu_dai)
 {
struct dw_i2s_dev *dev = snd_soc_dai_get_drvdata(cpu_dai);
-   struct i2s_dma_data *dma_data = NULL;
+   union snd_dma_data *dma_data = NULL;
 
if (!(dev-capability  DWC_I2S_RECORD) 
(substream-stream == SNDRV_PCM_STREAM_CAPTURE))
@@ -242,13 +269,18 @@ static int dw_i2s_hw_params(struct snd_pcm_substream 
*substream,
 
config-sample_rate = params_rate(params);
 
-   if (!dev-i2s_clk_cfg)
-   return -EINVAL;
+   if (dev-i2s_clk_cfg) {
+   ret = dev-i2s_clk_cfg(config);
+   if (ret  0) {
+   dev_err(dev-dev, runtime audio clk config fail\n);
+   return ret;
+   }
+   } else {
+   u32 bitclk;
 
-   ret = dev-i2s_clk_cfg(config);
-   if (ret  0) {
-   dev_err(dev-dev, runtime audio clk config fail\n);
-   return ret;
+   /* TODO: Validate sample rate against permissible set */
+   bitclk = config-sample_rate * config-data_width * 2;
+   clk_set_rate(dev-clk, bitclk);
}
 
return 0;
@@ -335,6 +367,31 @@ static int dw_i2s_resume(struct snd_soc_dai *dai)
 #define dw_i2s_resume  NULL
 #endif
 
+/* Maximum resolution of a channel - not uniformly spaced */
+static const u32 fifo_width[] = {
+   12, 16, 20, 24, 32, 0, 0, 0
+};
+
+/* Width of (DMA) bus */
+static const u32 bus_widths[] = {
+   DMA_SLAVE_BUSWIDTH_1_BYTE,
+   DMA_SLAVE_BUSWIDTH_2_BYTES,
+   DMA_SLAVE_BUSWIDTH_4_BYTES,
+   DMA_SLAVE_BUSWIDTH_UNDEFINED
+};
+
+/* PCM format to support channel

[PATCH v3 2/5] ASoC: dwc: Iterate over all channels

2014-12-19 Thread Andrew Jackson
From: Andrew Jackson andrew.jack...@arm.com

The Designware core can be configured with up to four stereo channels.
Each stereo channel is individually configured so, when the driver's
hw_params call is made, each requested stereo channel has to be
programmed.

Signed-off-by: Andrew Jackson andrew.jack...@arm.com
---
 sound/soc/dwc/designware_i2s.c |   35 ---
 1 files changed, 16 insertions(+), 19 deletions(-)

diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
index 5c13303..2ba1e2e 100644
--- a/sound/soc/dwc/designware_i2s.c
+++ b/sound/soc/dwc/designware_i2s.c
@@ -209,16 +209,9 @@ static int dw_i2s_hw_params(struct snd_pcm_substream 
*substream,
 
switch (config-chan_nr) {
case EIGHT_CHANNEL_SUPPORT:
-   ch_reg = 3;
-   break;
case SIX_CHANNEL_SUPPORT:
-   ch_reg = 2;
-   break;
case FOUR_CHANNEL_SUPPORT:
-   ch_reg = 1;
-   break;
case TWO_CHANNEL_SUPPORT:
-   ch_reg = 0;
break;
default:
dev_err(dev-dev, channel not supported\n);
@@ -227,18 +220,22 @@ static int dw_i2s_hw_params(struct snd_pcm_substream 
*substream,
 
i2s_disable_channels(dev, substream-stream);
 
-   if (substream-stream == SNDRV_PCM_STREAM_PLAYBACK) {
-   i2s_write_reg(dev-i2s_base, TCR(ch_reg), xfer_resolution);
-   i2s_write_reg(dev-i2s_base, TFCR(ch_reg), 0x02);
-   irq = i2s_read_reg(dev-i2s_base, IMR(ch_reg));
-   i2s_write_reg(dev-i2s_base, IMR(ch_reg), irq  ~0x30);
-   i2s_write_reg(dev-i2s_base, TER(ch_reg), 1);
-   } else {
-   i2s_write_reg(dev-i2s_base, RCR(ch_reg), xfer_resolution);
-   i2s_write_reg(dev-i2s_base, RFCR(ch_reg), 0x07);
-   irq = i2s_read_reg(dev-i2s_base, IMR(ch_reg));
-   i2s_write_reg(dev-i2s_base, IMR(ch_reg), irq  ~0x03);
-   i2s_write_reg(dev-i2s_base, RER(ch_reg), 1);
+   for (ch_reg = 0; ch_reg  (config-chan_nr / 2); ch_reg++) {
+   if (substream-stream == SNDRV_PCM_STREAM_PLAYBACK) {
+   i2s_write_reg(dev-i2s_base, TCR(ch_reg),
+ xfer_resolution);
+   i2s_write_reg(dev-i2s_base, TFCR(ch_reg), 0x02);
+   irq = i2s_read_reg(dev-i2s_base, IMR(ch_reg));
+   i2s_write_reg(dev-i2s_base, IMR(ch_reg), irq  ~0x30);
+   i2s_write_reg(dev-i2s_base, TER(ch_reg), 1);
+   } else {
+   i2s_write_reg(dev-i2s_base, RCR(ch_reg),
+ xfer_resolution);
+   i2s_write_reg(dev-i2s_base, RFCR(ch_reg), 0x07);
+   irq = i2s_read_reg(dev-i2s_base, IMR(ch_reg));
+   i2s_write_reg(dev-i2s_base, IMR(ch_reg), irq  ~0x03);
+   i2s_write_reg(dev-i2s_base, RER(ch_reg), 1);
+   }
}
 
i2s_write_reg(dev-i2s_base, CCR, ccr);
-- 
1.7.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v2 2/6] ASoC: dwc: Ensure FIFOs are flushed to prevent channel swap

2014-12-15 Thread Andrew Jackson
On 12/12/14 09:37, Lars-Peter Clausen wrote:
> On 12/12/2014 10:25 AM, Andrew Jackson wrote:
>> From: Andrew Jackson 
>>
>> If the FIFOs aren't flushed, the left/right channels may be swapped:
>> this may occur if the FIFOs are not empty when the streams start.
>>
>> Signed-off-by: Andrew Jackson 
>> ---
>>   sound/soc/dwc/designware_i2s.c |2 ++
>>   1 files changed, 2 insertions(+), 0 deletions(-)
>>
>> diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
>> index ef771ea..b9d6a25 100644
>> --- a/sound/soc/dwc/designware_i2s.c
>> +++ b/sound/soc/dwc/designware_i2s.c
>> @@ -228,12 +228,14 @@ static int dw_i2s_hw_params(struct snd_pcm_substream 
>> *substream,
>>  i2s_disable_channels(dev, substream->stream);
>>
>>  if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
>> +i2s_write_reg(dev->i2s_base, TXFFR, 1);
>>  i2s_write_reg(dev->i2s_base, TCR(ch_reg), xfer_resolution);
>>  i2s_write_reg(dev->i2s_base, TFCR(ch_reg), 0x02);
>>  irq = i2s_read_reg(dev->i2s_base, IMR(ch_reg));
>>  i2s_write_reg(dev->i2s_base, IMR(ch_reg), irq & ~0x30);
>>  i2s_write_reg(dev->i2s_base, TER(ch_reg), 1);
>>  } else {
>> +i2s_write_reg(dev->i2s_base, RXFFR, 1);
>>  i2s_write_reg(dev->i2s_base, RCR(ch_reg), xfer_resolution);
>>  i2s_write_reg(dev->i2s_base, RFCR(ch_reg), 0x07);
>>  irq = i2s_read_reg(dev->i2s_base, IMR(ch_reg));
>>
> 
> This should probably go into the prepare callback. prepare is for example 
> also called when recovering from a underrun/overrun. Whereas hwparams is 
> only called during initial setup of the stream.

That's a good idea: thank you.  I'll rework the patch with this in mind.

Andrew

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v2 2/6] ASoC: dwc: Ensure FIFOs are flushed to prevent channel swap

2014-12-15 Thread Andrew Jackson
On 12/12/14 09:37, Lars-Peter Clausen wrote:
 On 12/12/2014 10:25 AM, Andrew Jackson wrote:
 From: Andrew Jackson andrew.jack...@arm.com

 If the FIFOs aren't flushed, the left/right channels may be swapped:
 this may occur if the FIFOs are not empty when the streams start.

 Signed-off-by: Andrew Jackson andrew.jack...@arm.com
 ---
   sound/soc/dwc/designware_i2s.c |2 ++
   1 files changed, 2 insertions(+), 0 deletions(-)

 diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
 index ef771ea..b9d6a25 100644
 --- a/sound/soc/dwc/designware_i2s.c
 +++ b/sound/soc/dwc/designware_i2s.c
 @@ -228,12 +228,14 @@ static int dw_i2s_hw_params(struct snd_pcm_substream 
 *substream,
  i2s_disable_channels(dev, substream-stream);

  if (substream-stream == SNDRV_PCM_STREAM_PLAYBACK) {
 +i2s_write_reg(dev-i2s_base, TXFFR, 1);
  i2s_write_reg(dev-i2s_base, TCR(ch_reg), xfer_resolution);
  i2s_write_reg(dev-i2s_base, TFCR(ch_reg), 0x02);
  irq = i2s_read_reg(dev-i2s_base, IMR(ch_reg));
  i2s_write_reg(dev-i2s_base, IMR(ch_reg), irq  ~0x30);
  i2s_write_reg(dev-i2s_base, TER(ch_reg), 1);
  } else {
 +i2s_write_reg(dev-i2s_base, RXFFR, 1);
  i2s_write_reg(dev-i2s_base, RCR(ch_reg), xfer_resolution);
  i2s_write_reg(dev-i2s_base, RFCR(ch_reg), 0x07);
  irq = i2s_read_reg(dev-i2s_base, IMR(ch_reg));

 
 This should probably go into the prepare callback. prepare is for example 
 also called when recovering from a underrun/overrun. Whereas hwparams is 
 only called during initial setup of the stream.

That's a good idea: thank you.  I'll rework the patch with this in mind.

Andrew

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v2 1/6] ASoC: dwc: Remove unnecessary debug messages and tests

2014-12-12 Thread Andrew Jackson
On 12/12/14 09:31, Joe Perches wrote:
> On Fri, 2014-12-12 at 09:25 +0000, Andrew Jackson wrote:
>> The devm_XXX allocation functions print a message on failure,
>> so additional messages are not required.
> []
>> diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
> []
>> @@ -345,26 +345,17 @@ static int dw_i2s_probe(struct platform_device *pdev)
>>  }
>>  
>>  dw_i2s_dai = devm_kzalloc(>dev, sizeof(*dw_i2s_dai), GFP_KERNEL);
>> -if (!dw_i2s_dai) {
>> -dev_err(>dev, "mem allocation failed for dai driver\n");
>> +if (!dw_i2s_dai)
>>  return -ENOMEM;
>> -}
> 
> ok.
>  
>>  dw_i2s_dai->ops = _i2s_dai_ops;
>>  dw_i2s_dai->suspend = dw_i2s_suspend;
>>  dw_i2s_dai->resume = dw_i2s_resume;
>>  
>>  res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> -if (!res) {
>> -dev_err(>dev, "no i2s resource defined\n");
>> -return -ENODEV;
>> -}
>> -
> 
> Why delete this?

Lars-Peter said that it was unnecessary: see 
<http://lists.infradead.org/pipermail/linux-arm-kernel/2014-December/308416.html>.
> 
>>  dev->i2s_base = devm_ioremap_resource(>dev, res);
>> -if (IS_ERR(dev->i2s_base)) {
>> -dev_err(>dev, "ioremap fail for i2s_region\n");
>> +if (IS_ERR(dev->i2s_base))
>>  return PTR_ERR(dev->i2s_base);
>> -}
> 
> or this?

Ditto

Andrew

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v2 6/6] ASoC: dwc: Add documentation for I2S DT

2014-12-12 Thread Andrew Jackson
From: Andrew Jackson 

Add documentation for Designware I2S hardware block.  The block requires
two clocks (one for audio sampling, the other for APB) and DMA channels
for receive and transmit.

Signed-off-by: Andrew Jackson 
---
 .../devicetree/bindings/sound/designware-i2s.txt   |   32 
 1 files changed, 32 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sound/designware-i2s.txt

diff --git a/Documentation/devicetree/bindings/sound/designware-i2s.txt 
b/Documentation/devicetree/bindings/sound/designware-i2s.txt
new file mode 100644
index 000..cdee591
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/designware-i2s.txt
@@ -0,0 +1,32 @@
+DesignWare I2S controller
+
+Required properties:
+ - compatible : Must be "snps,designware-i2s"
+ - reg : Must contain I2S core's registers location and length
+ - clocks : Pairs of phandle and specifier referencing the controller's clocks.
+   The controller expects two clocks, the clock used for the APB interface and
+   the clock used as the sampling rate reference clock sample.
+ - clock-names : "apb_plck" for the clock to the APB interface, "i2sclk" for 
the sample
+   rate reference clock.
+ - dmas: Pairs of phandle and specifier for the DMA channels that are used by
+   the core. The core expects one or two dma channels, one for transmit and 
one for
+   receive.
+ - dma-names : "tx" for the transmit channel, "rx" for the receive channel.
+
+For more details on the 'dma', 'dma-names', 'clock' and 'clock-names' 
properties
+please check:
+   * resource-names.txt
+   * clock/clock-bindings.txt
+   * dma/dma.txt
+
+Example:
+
+   soc_i2s: i2s@7ff9 {
+   compatible = "snps,designware-i2s";
+   reg = <0x0 0x7ff9 0x0 0x1000>;
+   clocks = <_i2sclk 0>, <_refclk100mhz>;
+   clock-names = "i2sclk", "apb_pclk";
+   #sound-dai-cells = <0>;
+   dmas = < 5>;
+   dma-names = "tx";
+   };
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v2 5/6] ASoC: dwc: Add devicetree support for Designware I2S

2014-12-12 Thread Andrew Jackson
From: Andrew Jackson 

Allow the driver to be configured through a device tree rather than platform
data.  When using device-tree, read the I2S block's configuration from the
relevant registers: this reduces the amount of information required in
the device tree.

Signed-off-by: Andrew Jackson 
---
 sound/soc/dwc/Kconfig  |1 +
 sound/soc/dwc/designware_i2s.c |  199 ++--
 2 files changed, 171 insertions(+), 29 deletions(-)

diff --git a/sound/soc/dwc/Kconfig b/sound/soc/dwc/Kconfig
index e334900..d50e085 100644
--- a/sound/soc/dwc/Kconfig
+++ b/sound/soc/dwc/Kconfig
@@ -1,6 +1,7 @@
 config SND_DESIGNWARE_I2S
tristate "Synopsys I2S Device Driver"
depends on CLKDEV_LOOKUP
+   select SND_SOC_GENERIC_DMAENGINE_PCM
help
 Say Y or M if you want to add support for I2S driver for
 Synopsys desigwnware I2S device. The device supports upto
diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
index 343f5e7..9e1f547 100644
--- a/sound/soc/dwc/designware_i2s.c
+++ b/sound/soc/dwc/designware_i2s.c
@@ -22,6 +22,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /* common register for all channel */
 #define IER0x000
@@ -54,9 +55,35 @@
 #define I2S_COMP_VERSION   0x01F8
 #define I2S_COMP_TYPE  0x01FC
 
+/*
+ * Component parameter register fields - define the I2S block's
+ * configuration.
+ */
+#defineCOMP1_TX_WORDSIZE_3(r)  (((r) & GENMASK(27, 25)) >> 25)
+#defineCOMP1_TX_WORDSIZE_2(r)  (((r) & GENMASK(24, 22)) >> 22)
+#defineCOMP1_TX_WORDSIZE_1(r)  (((r) & GENMASK(21, 19)) >> 19)
+#defineCOMP1_TX_WORDSIZE_0(r)  (((r) & GENMASK(18, 16)) >> 16)
+#defineCOMP1_TX_CHANNELS(r)(((r) & GENMASK(10, 9)) >> 9)
+#defineCOMP1_RX_CHANNELS(r)(((r) & GENMASK(8, 7)) >> 7)
+#defineCOMP1_RX_ENABLED(r) (((r) & BIT(6)) >> 6)
+#defineCOMP1_TX_ENABLED(r) (((r) & BIT(5)) >> 5)
+#defineCOMP1_MODE_EN(r)(((r) & BIT(4)) >> 4)
+#defineCOMP1_FIFO_DEPTH_GLOBAL(r)  (((r) & GENMASK(3, 2)) >> 2)
+#defineCOMP1_APB_DATA_WIDTH(r) (((r) & GENMASK(1, 0)) >> 0)
+
+#defineCOMP2_RX_WORDSIZE_3(r)  (((r) & GENMASK(12, 10)) >> 10)
+#defineCOMP2_RX_WORDSIZE_2(r)  (((r) & GENMASK(9, 7)) >> 7)
+#defineCOMP2_RX_WORDSIZE_1(r)  (((r) & GENMASK(5, 3)) >> 3)
+#defineCOMP2_RX_WORDSIZE_0(r)  (((r) & GENMASK(2, 0)) >> 0)
+
 #define MAX_CHANNEL_NUM8
 #define MIN_CHANNEL_NUM2
 
+union snd_dma_data {
+   struct i2s_dma_data pd;
+   struct snd_dmaengine_dai_dma_data dt;
+};
+
 struct dw_i2s_dev {
void __iomem *i2s_base;
struct clk *clk;
@@ -65,8 +92,8 @@ struct dw_i2s_dev {
struct device *dev;
 
/* data related to DMA transfers b/w i2s and DMAC */
-   struct i2s_dma_data play_dma_data;
-   struct i2s_dma_data capture_dma_data;
+   union snd_dma_data play_dma_data;
+   union snd_dma_data capture_dma_data;
struct i2s_clk_config_data config;
int (*i2s_clk_cfg)(struct i2s_clk_config_data *config);
 };
@@ -153,7 +180,7 @@ static int dw_i2s_startup(struct snd_pcm_substream 
*substream,
struct snd_soc_dai *cpu_dai)
 {
struct dw_i2s_dev *dev = snd_soc_dai_get_drvdata(cpu_dai);
-   struct i2s_dma_data *dma_data = NULL;
+   union snd_dma_data *dma_data = NULL;
 
if (!(dev->capability & DWC_I2S_RECORD) &&
(substream->stream == SNDRV_PCM_STREAM_CAPTURE))
@@ -244,13 +271,18 @@ static int dw_i2s_hw_params(struct snd_pcm_substream 
*substream,
 
config->sample_rate = params_rate(params);
 
-   if (!dev->i2s_clk_cfg)
-   return -EINVAL;
+   if (dev->i2s_clk_cfg) {
+   ret = dev->i2s_clk_cfg(config);
+   if (ret < 0) {
+   dev_err(dev->dev, "runtime audio clk config fail\n");
+   return ret;
+   }
+   } else {
+   u32 bitclk;
 
-   ret = dev->i2s_clk_cfg(config);
-   if (ret < 0) {
-   dev_err(dev->dev, "runtime audio clk config fail\n");
-   return ret;
+   /* TODO: Validate sample rate against permissible set */
+   bitclk = config->sample_rate * config->data_width * 2;
+   clk_set_rate(dev->clk, bitclk);
}
 
return 0;
@@ -323,6 +355,31 @@ static int dw_i2s_resume(struct snd_soc_dai *dai)
 #define dw_i2s_resume  NULL
 #endif
 
+/* Maximum resolution of a channel - not uniformly spaced */
+static const u32 fifo_width[] = {
+   12, 16, 20, 24, 32, 0, 0, 0
+};
+
+/* Width of (DMA) bus */
+static const u32 bus_wi

[PATCH v2 2/6] ASoC: dwc: Ensure FIFOs are flushed to prevent channel swap

2014-12-12 Thread Andrew Jackson
From: Andrew Jackson 

If the FIFOs aren't flushed, the left/right channels may be swapped:
this may occur if the FIFOs are not empty when the streams start.

Signed-off-by: Andrew Jackson 
---
 sound/soc/dwc/designware_i2s.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
index ef771ea..b9d6a25 100644
--- a/sound/soc/dwc/designware_i2s.c
+++ b/sound/soc/dwc/designware_i2s.c
@@ -228,12 +228,14 @@ static int dw_i2s_hw_params(struct snd_pcm_substream 
*substream,
i2s_disable_channels(dev, substream->stream);
 
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+   i2s_write_reg(dev->i2s_base, TXFFR, 1);
i2s_write_reg(dev->i2s_base, TCR(ch_reg), xfer_resolution);
i2s_write_reg(dev->i2s_base, TFCR(ch_reg), 0x02);
irq = i2s_read_reg(dev->i2s_base, IMR(ch_reg));
i2s_write_reg(dev->i2s_base, IMR(ch_reg), irq & ~0x30);
i2s_write_reg(dev->i2s_base, TER(ch_reg), 1);
} else {
+   i2s_write_reg(dev->i2s_base, RXFFR, 1);
i2s_write_reg(dev->i2s_base, RCR(ch_reg), xfer_resolution);
i2s_write_reg(dev->i2s_base, RFCR(ch_reg), 0x07);
irq = i2s_read_reg(dev->i2s_base, IMR(ch_reg));
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v2 4/6] ASoC: dwc: Reorder code in preparation for DT support

2014-12-12 Thread Andrew Jackson
From: Andrew Jackson 

Move code that configures the DAI and DMA into a separate function.  This
reduces the size of the dw_i2s_probe function and will make it easier to
add support for device tree to the driver.

Signed-off-by: Andrew Jackson 
---
 sound/soc/dwc/designware_i2s.c |   73 +--
 1 files changed, 39 insertions(+), 34 deletions(-)

diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
index 1d26ded..343f5e7 100644
--- a/sound/soc/dwc/designware_i2s.c
+++ b/sound/soc/dwc/designware_i2s.c
@@ -323,13 +323,47 @@ static int dw_i2s_resume(struct snd_soc_dai *dai)
 #define dw_i2s_resume  NULL
 #endif
 
+static void dw_configure_dai_by_pd(struct dw_i2s_dev *dev,
+  struct snd_soc_dai_driver *dw_i2s_dai,
+  struct resource *res,
+  const struct i2s_platform_data *pdata)
+{
+   /* Set DMA slaves info */
+
+   dev->play_dma_data.data = pdata->play_dma_data;
+   dev->capture_dma_data.data = pdata->capture_dma_data;
+   dev->play_dma_data.addr = res->start + I2S_TXDMA;
+   dev->capture_dma_data.addr = res->start + I2S_RXDMA;
+   dev->play_dma_data.max_burst = 16;
+   dev->capture_dma_data.max_burst = 16;
+   dev->play_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
+   dev->capture_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
+   dev->play_dma_data.filter = pdata->filter;
+   dev->capture_dma_data.filter = pdata->filter;
+
+   if (pdata->cap & DWC_I2S_PLAY) {
+   dev_dbg(dev->dev, " designware: play supported\n");
+   dw_i2s_dai->playback.channels_min = MIN_CHANNEL_NUM;
+   dw_i2s_dai->playback.channels_max = pdata->channel;
+   dw_i2s_dai->playback.formats = pdata->snd_fmts;
+   dw_i2s_dai->playback.rates = pdata->snd_rates;
+   }
+
+   if (pdata->cap & DWC_I2S_RECORD) {
+   dev_dbg(dev->dev, "designware: record supported\n");
+   dw_i2s_dai->capture.channels_min = MIN_CHANNEL_NUM;
+   dw_i2s_dai->capture.channels_max = pdata->channel;
+   dw_i2s_dai->capture.formats = pdata->snd_fmts;
+   dw_i2s_dai->capture.rates = pdata->snd_rates;
+   }
+}
+
 static int dw_i2s_probe(struct platform_device *pdev)
 {
const struct i2s_platform_data *pdata = pdev->dev.platform_data;
struct dw_i2s_dev *dev;
struct resource *res;
int ret;
-   unsigned int cap;
struct snd_soc_dai_driver *dw_i2s_dai;
 
if (!pdata) {
@@ -356,23 +390,11 @@ static int dw_i2s_probe(struct platform_device *pdev)
if (IS_ERR(dev->i2s_base))
return PTR_ERR(dev->i2s_base);
 
-   cap = pdata->cap;
-   dev->capability = cap;
-   dev->i2s_clk_cfg = pdata->i2s_clk_cfg;
-
-   /* Set DMA slaves info */
-
-   dev->play_dma_data.data = pdata->play_dma_data;
-   dev->capture_dma_data.data = pdata->capture_dma_data;
-   dev->play_dma_data.addr = res->start + I2S_TXDMA;
-   dev->capture_dma_data.addr = res->start + I2S_RXDMA;
-   dev->play_dma_data.max_burst = 16;
-   dev->capture_dma_data.max_burst = 16;
-   dev->play_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
-   dev->capture_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
-   dev->play_dma_data.filter = pdata->filter;
-   dev->capture_dma_data.filter = pdata->filter;
+   dev->dev = >dev;
+   dw_configure_dai_by_pd(dev, dw_i2s_dai, res, pdata);
 
+   dev->capability = pdata->cap;
+   dev->i2s_clk_cfg = pdata->i2s_clk_cfg;
dev->clk = clk_get(>dev, NULL);
if (IS_ERR(dev->clk))
return  PTR_ERR(dev->clk);
@@ -381,23 +403,6 @@ static int dw_i2s_probe(struct platform_device *pdev)
if (ret < 0)
goto err_clk_put;
 
-   if (cap & DWC_I2S_PLAY) {
-   dev_dbg(>dev, " designware: play supported\n");
-   dw_i2s_dai->playback.channels_min = MIN_CHANNEL_NUM;
-   dw_i2s_dai->playback.channels_max = pdata->channel;
-   dw_i2s_dai->playback.formats = pdata->snd_fmts;
-   dw_i2s_dai->playback.rates = pdata->snd_rates;
-   }
-
-   if (cap & DWC_I2S_RECORD) {
-   dev_dbg(>dev, "designware: record supported\n");
-   dw_i2s_dai->capture.channels_min = MIN_CHANNEL_NUM;
-   dw_i2s_dai->capture.channels_max = pdata->channel;
-   dw_i2s_dai->capture.formats = pdata->snd_fmts;
-   dw_i2s_dai->capture.rates = pdata->snd_rates;
-   }
-
-   dev->d

[PATCH v2 0/6] ASoC: dwc: Add device tree support to designware I2S

2014-12-12 Thread Andrew Jackson
From: Andrew Jackson 

This patch set extends the DesignWare I2S driver to provide device
tree support and fixes a couple of small faults.

Changes v1->v2
  + Drop negative use count patch [Mark Brown]
  + Remove unnecessary debug print messages [Lars-Peter Clausen]
  + Rewrite iteration as for loop rather than do...while [Mark Brown]
  + Reorder patches to send fixes first [Mark Brown]
  + Simplify device tree code [Mark Brown]
  + Split device tree patch in two [Mark Brown]
  + Expand explanatory comment on channel configuration [Rajeev Kumar]

Arnd: I've not forgotten about updating the spear entries

Andrew Jackson (6):
  ASoC: dwc: Remove unnecessary debug messages and tests
  ASoC: dwc: Ensure FIFOs are flushed to prevent channel swap
  ASoC: dwc: Iterate over all channels
  ASoC: dwc: Reorder code in preparation for DT support
  ASoC: dwc: Add devicetree support for Designware I2S
  ASoC: dwc: Add documentation for I2S DT

 .../devicetree/bindings/sound/designware-i2s.txt   |   32 +++
 sound/soc/dwc/Kconfig  |1 +
 sound/soc/dwc/designware_i2s.c |  294 ++--
 3 files changed, 248 insertions(+), 79 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sound/designware-i2s.txt

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v2 3/6] ASoC: dwc: Iterate over all channels

2014-12-12 Thread Andrew Jackson
From: Andrew Jackson 

The Designware core can be configured with up to four stereo channels.
Each stereo channel is individually configured so, when the driver's
hw_params call is made, each requested stereo channel has to be
programmed.

Signed-off-by: Andrew Jackson 
---
 sound/soc/dwc/designware_i2s.c |   39 ++-
 1 files changed, 18 insertions(+), 21 deletions(-)

diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
index b9d6a25..1d26ded 100644
--- a/sound/soc/dwc/designware_i2s.c
+++ b/sound/soc/dwc/designware_i2s.c
@@ -209,16 +209,9 @@ static int dw_i2s_hw_params(struct snd_pcm_substream 
*substream,
 
switch (config->chan_nr) {
case EIGHT_CHANNEL_SUPPORT:
-   ch_reg = 3;
-   break;
case SIX_CHANNEL_SUPPORT:
-   ch_reg = 2;
-   break;
case FOUR_CHANNEL_SUPPORT:
-   ch_reg = 1;
-   break;
case TWO_CHANNEL_SUPPORT:
-   ch_reg = 0;
break;
default:
dev_err(dev->dev, "channel not supported\n");
@@ -227,20 +220,24 @@ static int dw_i2s_hw_params(struct snd_pcm_substream 
*substream,
 
i2s_disable_channels(dev, substream->stream);
 
-   if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
-   i2s_write_reg(dev->i2s_base, TXFFR, 1);
-   i2s_write_reg(dev->i2s_base, TCR(ch_reg), xfer_resolution);
-   i2s_write_reg(dev->i2s_base, TFCR(ch_reg), 0x02);
-   irq = i2s_read_reg(dev->i2s_base, IMR(ch_reg));
-   i2s_write_reg(dev->i2s_base, IMR(ch_reg), irq & ~0x30);
-   i2s_write_reg(dev->i2s_base, TER(ch_reg), 1);
-   } else {
-   i2s_write_reg(dev->i2s_base, RXFFR, 1);
-   i2s_write_reg(dev->i2s_base, RCR(ch_reg), xfer_resolution);
-   i2s_write_reg(dev->i2s_base, RFCR(ch_reg), 0x07);
-   irq = i2s_read_reg(dev->i2s_base, IMR(ch_reg));
-   i2s_write_reg(dev->i2s_base, IMR(ch_reg), irq & ~0x03);
-   i2s_write_reg(dev->i2s_base, RER(ch_reg), 1);
+   for (ch_reg = 0; ch_reg < (config->chan_nr / 2); ch_reg++) {
+   if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+   i2s_write_reg(dev->i2s_base, TXFFR, 1);
+   i2s_write_reg(dev->i2s_base, TCR(ch_reg),
+ xfer_resolution);
+   i2s_write_reg(dev->i2s_base, TFCR(ch_reg), 0x02);
+   irq = i2s_read_reg(dev->i2s_base, IMR(ch_reg));
+   i2s_write_reg(dev->i2s_base, IMR(ch_reg), irq & ~0x30);
+   i2s_write_reg(dev->i2s_base, TER(ch_reg), 1);
+   } else {
+   i2s_write_reg(dev->i2s_base, RXFFR, 1);
+   i2s_write_reg(dev->i2s_base, RCR(ch_reg),
+ xfer_resolution);
+   i2s_write_reg(dev->i2s_base, RFCR(ch_reg), 0x07);
+   irq = i2s_read_reg(dev->i2s_base, IMR(ch_reg));
+   i2s_write_reg(dev->i2s_base, IMR(ch_reg), irq & ~0x03);
+   i2s_write_reg(dev->i2s_base, RER(ch_reg), 1);
+   }
}
 
i2s_write_reg(dev->i2s_base, CCR, ccr);
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v2 1/6] ASoC: dwc: Remove unnecessary debug messages and tests

2014-12-12 Thread Andrew Jackson
From: Andrew Jackson 

The devm_XXX allocation functions print a message on failure,
so additional messages are not required.

Signed-off-by: Andrew Jackson 
---
 sound/soc/dwc/designware_i2s.c |   13 ++---
 1 files changed, 2 insertions(+), 11 deletions(-)

diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
index d202c7c..ef771ea 100644
--- a/sound/soc/dwc/designware_i2s.c
+++ b/sound/soc/dwc/designware_i2s.c
@@ -345,26 +345,17 @@ static int dw_i2s_probe(struct platform_device *pdev)
}
 
dw_i2s_dai = devm_kzalloc(>dev, sizeof(*dw_i2s_dai), GFP_KERNEL);
-   if (!dw_i2s_dai) {
-   dev_err(>dev, "mem allocation failed for dai driver\n");
+   if (!dw_i2s_dai)
return -ENOMEM;
-   }
 
dw_i2s_dai->ops = _i2s_dai_ops;
dw_i2s_dai->suspend = dw_i2s_suspend;
dw_i2s_dai->resume = dw_i2s_resume;
 
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-   if (!res) {
-   dev_err(>dev, "no i2s resource defined\n");
-   return -ENODEV;
-   }
-
dev->i2s_base = devm_ioremap_resource(>dev, res);
-   if (IS_ERR(dev->i2s_base)) {
-   dev_err(>dev, "ioremap fail for i2s_region\n");
+   if (IS_ERR(dev->i2s_base))
return PTR_ERR(dev->i2s_base);
-   }
 
cap = pdata->cap;
dev->capability = cap;
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v2 1/6] ASoC: dwc: Remove unnecessary debug messages and tests

2014-12-12 Thread Andrew Jackson
From: Andrew Jackson andrew.jack...@arm.com

The devm_XXX allocation functions print a message on failure,
so additional messages are not required.

Signed-off-by: Andrew Jackson andrew.jack...@arm.com
---
 sound/soc/dwc/designware_i2s.c |   13 ++---
 1 files changed, 2 insertions(+), 11 deletions(-)

diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
index d202c7c..ef771ea 100644
--- a/sound/soc/dwc/designware_i2s.c
+++ b/sound/soc/dwc/designware_i2s.c
@@ -345,26 +345,17 @@ static int dw_i2s_probe(struct platform_device *pdev)
}
 
dw_i2s_dai = devm_kzalloc(pdev-dev, sizeof(*dw_i2s_dai), GFP_KERNEL);
-   if (!dw_i2s_dai) {
-   dev_err(pdev-dev, mem allocation failed for dai driver\n);
+   if (!dw_i2s_dai)
return -ENOMEM;
-   }
 
dw_i2s_dai-ops = dw_i2s_dai_ops;
dw_i2s_dai-suspend = dw_i2s_suspend;
dw_i2s_dai-resume = dw_i2s_resume;
 
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-   if (!res) {
-   dev_err(pdev-dev, no i2s resource defined\n);
-   return -ENODEV;
-   }
-
dev-i2s_base = devm_ioremap_resource(pdev-dev, res);
-   if (IS_ERR(dev-i2s_base)) {
-   dev_err(pdev-dev, ioremap fail for i2s_region\n);
+   if (IS_ERR(dev-i2s_base))
return PTR_ERR(dev-i2s_base);
-   }
 
cap = pdata-cap;
dev-capability = cap;
-- 
1.7.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v2 3/6] ASoC: dwc: Iterate over all channels

2014-12-12 Thread Andrew Jackson
From: Andrew Jackson andrew.jack...@arm.com

The Designware core can be configured with up to four stereo channels.
Each stereo channel is individually configured so, when the driver's
hw_params call is made, each requested stereo channel has to be
programmed.

Signed-off-by: Andrew Jackson andrew.jack...@arm.com
---
 sound/soc/dwc/designware_i2s.c |   39 ++-
 1 files changed, 18 insertions(+), 21 deletions(-)

diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
index b9d6a25..1d26ded 100644
--- a/sound/soc/dwc/designware_i2s.c
+++ b/sound/soc/dwc/designware_i2s.c
@@ -209,16 +209,9 @@ static int dw_i2s_hw_params(struct snd_pcm_substream 
*substream,
 
switch (config-chan_nr) {
case EIGHT_CHANNEL_SUPPORT:
-   ch_reg = 3;
-   break;
case SIX_CHANNEL_SUPPORT:
-   ch_reg = 2;
-   break;
case FOUR_CHANNEL_SUPPORT:
-   ch_reg = 1;
-   break;
case TWO_CHANNEL_SUPPORT:
-   ch_reg = 0;
break;
default:
dev_err(dev-dev, channel not supported\n);
@@ -227,20 +220,24 @@ static int dw_i2s_hw_params(struct snd_pcm_substream 
*substream,
 
i2s_disable_channels(dev, substream-stream);
 
-   if (substream-stream == SNDRV_PCM_STREAM_PLAYBACK) {
-   i2s_write_reg(dev-i2s_base, TXFFR, 1);
-   i2s_write_reg(dev-i2s_base, TCR(ch_reg), xfer_resolution);
-   i2s_write_reg(dev-i2s_base, TFCR(ch_reg), 0x02);
-   irq = i2s_read_reg(dev-i2s_base, IMR(ch_reg));
-   i2s_write_reg(dev-i2s_base, IMR(ch_reg), irq  ~0x30);
-   i2s_write_reg(dev-i2s_base, TER(ch_reg), 1);
-   } else {
-   i2s_write_reg(dev-i2s_base, RXFFR, 1);
-   i2s_write_reg(dev-i2s_base, RCR(ch_reg), xfer_resolution);
-   i2s_write_reg(dev-i2s_base, RFCR(ch_reg), 0x07);
-   irq = i2s_read_reg(dev-i2s_base, IMR(ch_reg));
-   i2s_write_reg(dev-i2s_base, IMR(ch_reg), irq  ~0x03);
-   i2s_write_reg(dev-i2s_base, RER(ch_reg), 1);
+   for (ch_reg = 0; ch_reg  (config-chan_nr / 2); ch_reg++) {
+   if (substream-stream == SNDRV_PCM_STREAM_PLAYBACK) {
+   i2s_write_reg(dev-i2s_base, TXFFR, 1);
+   i2s_write_reg(dev-i2s_base, TCR(ch_reg),
+ xfer_resolution);
+   i2s_write_reg(dev-i2s_base, TFCR(ch_reg), 0x02);
+   irq = i2s_read_reg(dev-i2s_base, IMR(ch_reg));
+   i2s_write_reg(dev-i2s_base, IMR(ch_reg), irq  ~0x30);
+   i2s_write_reg(dev-i2s_base, TER(ch_reg), 1);
+   } else {
+   i2s_write_reg(dev-i2s_base, RXFFR, 1);
+   i2s_write_reg(dev-i2s_base, RCR(ch_reg),
+ xfer_resolution);
+   i2s_write_reg(dev-i2s_base, RFCR(ch_reg), 0x07);
+   irq = i2s_read_reg(dev-i2s_base, IMR(ch_reg));
+   i2s_write_reg(dev-i2s_base, IMR(ch_reg), irq  ~0x03);
+   i2s_write_reg(dev-i2s_base, RER(ch_reg), 1);
+   }
}
 
i2s_write_reg(dev-i2s_base, CCR, ccr);
-- 
1.7.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v2 0/6] ASoC: dwc: Add device tree support to designware I2S

2014-12-12 Thread Andrew Jackson
From: Andrew Jackson andrew.jack...@arm.com

This patch set extends the DesignWare I2S driver to provide device
tree support and fixes a couple of small faults.

Changes v1-v2
  + Drop negative use count patch [Mark Brown]
  + Remove unnecessary debug print messages [Lars-Peter Clausen]
  + Rewrite iteration as for loop rather than do...while [Mark Brown]
  + Reorder patches to send fixes first [Mark Brown]
  + Simplify device tree code [Mark Brown]
  + Split device tree patch in two [Mark Brown]
  + Expand explanatory comment on channel configuration [Rajeev Kumar]

Arnd: I've not forgotten about updating the spear entries

Andrew Jackson (6):
  ASoC: dwc: Remove unnecessary debug messages and tests
  ASoC: dwc: Ensure FIFOs are flushed to prevent channel swap
  ASoC: dwc: Iterate over all channels
  ASoC: dwc: Reorder code in preparation for DT support
  ASoC: dwc: Add devicetree support for Designware I2S
  ASoC: dwc: Add documentation for I2S DT

 .../devicetree/bindings/sound/designware-i2s.txt   |   32 +++
 sound/soc/dwc/Kconfig  |1 +
 sound/soc/dwc/designware_i2s.c |  294 ++--
 3 files changed, 248 insertions(+), 79 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sound/designware-i2s.txt

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v2 4/6] ASoC: dwc: Reorder code in preparation for DT support

2014-12-12 Thread Andrew Jackson
From: Andrew Jackson andrew.jack...@arm.com

Move code that configures the DAI and DMA into a separate function.  This
reduces the size of the dw_i2s_probe function and will make it easier to
add support for device tree to the driver.

Signed-off-by: Andrew Jackson andrew.jack...@arm.com
---
 sound/soc/dwc/designware_i2s.c |   73 +--
 1 files changed, 39 insertions(+), 34 deletions(-)

diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
index 1d26ded..343f5e7 100644
--- a/sound/soc/dwc/designware_i2s.c
+++ b/sound/soc/dwc/designware_i2s.c
@@ -323,13 +323,47 @@ static int dw_i2s_resume(struct snd_soc_dai *dai)
 #define dw_i2s_resume  NULL
 #endif
 
+static void dw_configure_dai_by_pd(struct dw_i2s_dev *dev,
+  struct snd_soc_dai_driver *dw_i2s_dai,
+  struct resource *res,
+  const struct i2s_platform_data *pdata)
+{
+   /* Set DMA slaves info */
+
+   dev-play_dma_data.data = pdata-play_dma_data;
+   dev-capture_dma_data.data = pdata-capture_dma_data;
+   dev-play_dma_data.addr = res-start + I2S_TXDMA;
+   dev-capture_dma_data.addr = res-start + I2S_RXDMA;
+   dev-play_dma_data.max_burst = 16;
+   dev-capture_dma_data.max_burst = 16;
+   dev-play_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
+   dev-capture_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
+   dev-play_dma_data.filter = pdata-filter;
+   dev-capture_dma_data.filter = pdata-filter;
+
+   if (pdata-cap  DWC_I2S_PLAY) {
+   dev_dbg(dev-dev,  designware: play supported\n);
+   dw_i2s_dai-playback.channels_min = MIN_CHANNEL_NUM;
+   dw_i2s_dai-playback.channels_max = pdata-channel;
+   dw_i2s_dai-playback.formats = pdata-snd_fmts;
+   dw_i2s_dai-playback.rates = pdata-snd_rates;
+   }
+
+   if (pdata-cap  DWC_I2S_RECORD) {
+   dev_dbg(dev-dev, designware: record supported\n);
+   dw_i2s_dai-capture.channels_min = MIN_CHANNEL_NUM;
+   dw_i2s_dai-capture.channels_max = pdata-channel;
+   dw_i2s_dai-capture.formats = pdata-snd_fmts;
+   dw_i2s_dai-capture.rates = pdata-snd_rates;
+   }
+}
+
 static int dw_i2s_probe(struct platform_device *pdev)
 {
const struct i2s_platform_data *pdata = pdev-dev.platform_data;
struct dw_i2s_dev *dev;
struct resource *res;
int ret;
-   unsigned int cap;
struct snd_soc_dai_driver *dw_i2s_dai;
 
if (!pdata) {
@@ -356,23 +390,11 @@ static int dw_i2s_probe(struct platform_device *pdev)
if (IS_ERR(dev-i2s_base))
return PTR_ERR(dev-i2s_base);
 
-   cap = pdata-cap;
-   dev-capability = cap;
-   dev-i2s_clk_cfg = pdata-i2s_clk_cfg;
-
-   /* Set DMA slaves info */
-
-   dev-play_dma_data.data = pdata-play_dma_data;
-   dev-capture_dma_data.data = pdata-capture_dma_data;
-   dev-play_dma_data.addr = res-start + I2S_TXDMA;
-   dev-capture_dma_data.addr = res-start + I2S_RXDMA;
-   dev-play_dma_data.max_burst = 16;
-   dev-capture_dma_data.max_burst = 16;
-   dev-play_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
-   dev-capture_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
-   dev-play_dma_data.filter = pdata-filter;
-   dev-capture_dma_data.filter = pdata-filter;
+   dev-dev = pdev-dev;
+   dw_configure_dai_by_pd(dev, dw_i2s_dai, res, pdata);
 
+   dev-capability = pdata-cap;
+   dev-i2s_clk_cfg = pdata-i2s_clk_cfg;
dev-clk = clk_get(pdev-dev, NULL);
if (IS_ERR(dev-clk))
return  PTR_ERR(dev-clk);
@@ -381,23 +403,6 @@ static int dw_i2s_probe(struct platform_device *pdev)
if (ret  0)
goto err_clk_put;
 
-   if (cap  DWC_I2S_PLAY) {
-   dev_dbg(pdev-dev,  designware: play supported\n);
-   dw_i2s_dai-playback.channels_min = MIN_CHANNEL_NUM;
-   dw_i2s_dai-playback.channels_max = pdata-channel;
-   dw_i2s_dai-playback.formats = pdata-snd_fmts;
-   dw_i2s_dai-playback.rates = pdata-snd_rates;
-   }
-
-   if (cap  DWC_I2S_RECORD) {
-   dev_dbg(pdev-dev, designware: record supported\n);
-   dw_i2s_dai-capture.channels_min = MIN_CHANNEL_NUM;
-   dw_i2s_dai-capture.channels_max = pdata-channel;
-   dw_i2s_dai-capture.formats = pdata-snd_fmts;
-   dw_i2s_dai-capture.rates = pdata-snd_rates;
-   }
-
-   dev-dev = pdev-dev;
dev_set_drvdata(pdev-dev, dev);
ret = snd_soc_register_component(pdev-dev, dw_i2s_component,
 dw_i2s_dai, 1);
-- 
1.7.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http

[PATCH v2 2/6] ASoC: dwc: Ensure FIFOs are flushed to prevent channel swap

2014-12-12 Thread Andrew Jackson
From: Andrew Jackson andrew.jack...@arm.com

If the FIFOs aren't flushed, the left/right channels may be swapped:
this may occur if the FIFOs are not empty when the streams start.

Signed-off-by: Andrew Jackson andrew.jack...@arm.com
---
 sound/soc/dwc/designware_i2s.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
index ef771ea..b9d6a25 100644
--- a/sound/soc/dwc/designware_i2s.c
+++ b/sound/soc/dwc/designware_i2s.c
@@ -228,12 +228,14 @@ static int dw_i2s_hw_params(struct snd_pcm_substream 
*substream,
i2s_disable_channels(dev, substream-stream);
 
if (substream-stream == SNDRV_PCM_STREAM_PLAYBACK) {
+   i2s_write_reg(dev-i2s_base, TXFFR, 1);
i2s_write_reg(dev-i2s_base, TCR(ch_reg), xfer_resolution);
i2s_write_reg(dev-i2s_base, TFCR(ch_reg), 0x02);
irq = i2s_read_reg(dev-i2s_base, IMR(ch_reg));
i2s_write_reg(dev-i2s_base, IMR(ch_reg), irq  ~0x30);
i2s_write_reg(dev-i2s_base, TER(ch_reg), 1);
} else {
+   i2s_write_reg(dev-i2s_base, RXFFR, 1);
i2s_write_reg(dev-i2s_base, RCR(ch_reg), xfer_resolution);
i2s_write_reg(dev-i2s_base, RFCR(ch_reg), 0x07);
irq = i2s_read_reg(dev-i2s_base, IMR(ch_reg));
-- 
1.7.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v2 6/6] ASoC: dwc: Add documentation for I2S DT

2014-12-12 Thread Andrew Jackson
From: Andrew Jackson andrew.jack...@arm.com

Add documentation for Designware I2S hardware block.  The block requires
two clocks (one for audio sampling, the other for APB) and DMA channels
for receive and transmit.

Signed-off-by: Andrew Jackson andrew.jack...@arm.com
---
 .../devicetree/bindings/sound/designware-i2s.txt   |   32 
 1 files changed, 32 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sound/designware-i2s.txt

diff --git a/Documentation/devicetree/bindings/sound/designware-i2s.txt 
b/Documentation/devicetree/bindings/sound/designware-i2s.txt
new file mode 100644
index 000..cdee591
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/designware-i2s.txt
@@ -0,0 +1,32 @@
+DesignWare I2S controller
+
+Required properties:
+ - compatible : Must be snps,designware-i2s
+ - reg : Must contain I2S core's registers location and length
+ - clocks : Pairs of phandle and specifier referencing the controller's clocks.
+   The controller expects two clocks, the clock used for the APB interface and
+   the clock used as the sampling rate reference clock sample.
+ - clock-names : apb_plck for the clock to the APB interface, i2sclk for 
the sample
+   rate reference clock.
+ - dmas: Pairs of phandle and specifier for the DMA channels that are used by
+   the core. The core expects one or two dma channels, one for transmit and 
one for
+   receive.
+ - dma-names : tx for the transmit channel, rx for the receive channel.
+
+For more details on the 'dma', 'dma-names', 'clock' and 'clock-names' 
properties
+please check:
+   * resource-names.txt
+   * clock/clock-bindings.txt
+   * dma/dma.txt
+
+Example:
+
+   soc_i2s: i2s@7ff9 {
+   compatible = snps,designware-i2s;
+   reg = 0x0 0x7ff9 0x0 0x1000;
+   clocks = scpi_i2sclk 0, soc_refclk100mhz;
+   clock-names = i2sclk, apb_pclk;
+   #sound-dai-cells = 0;
+   dmas = dma0 5;
+   dma-names = tx;
+   };
-- 
1.7.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v2 5/6] ASoC: dwc: Add devicetree support for Designware I2S

2014-12-12 Thread Andrew Jackson
From: Andrew Jackson andrew.jack...@arm.com

Allow the driver to be configured through a device tree rather than platform
data.  When using device-tree, read the I2S block's configuration from the
relevant registers: this reduces the amount of information required in
the device tree.

Signed-off-by: Andrew Jackson andrew.jack...@arm.com
---
 sound/soc/dwc/Kconfig  |1 +
 sound/soc/dwc/designware_i2s.c |  199 ++--
 2 files changed, 171 insertions(+), 29 deletions(-)

diff --git a/sound/soc/dwc/Kconfig b/sound/soc/dwc/Kconfig
index e334900..d50e085 100644
--- a/sound/soc/dwc/Kconfig
+++ b/sound/soc/dwc/Kconfig
@@ -1,6 +1,7 @@
 config SND_DESIGNWARE_I2S
tristate Synopsys I2S Device Driver
depends on CLKDEV_LOOKUP
+   select SND_SOC_GENERIC_DMAENGINE_PCM
help
 Say Y or M if you want to add support for I2S driver for
 Synopsys desigwnware I2S device. The device supports upto
diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
index 343f5e7..9e1f547 100644
--- a/sound/soc/dwc/designware_i2s.c
+++ b/sound/soc/dwc/designware_i2s.c
@@ -22,6 +22,7 @@
 #include sound/pcm.h
 #include sound/pcm_params.h
 #include sound/soc.h
+#include sound/dmaengine_pcm.h
 
 /* common register for all channel */
 #define IER0x000
@@ -54,9 +55,35 @@
 #define I2S_COMP_VERSION   0x01F8
 #define I2S_COMP_TYPE  0x01FC
 
+/*
+ * Component parameter register fields - define the I2S block's
+ * configuration.
+ */
+#defineCOMP1_TX_WORDSIZE_3(r)  (((r)  GENMASK(27, 25))  25)
+#defineCOMP1_TX_WORDSIZE_2(r)  (((r)  GENMASK(24, 22))  22)
+#defineCOMP1_TX_WORDSIZE_1(r)  (((r)  GENMASK(21, 19))  19)
+#defineCOMP1_TX_WORDSIZE_0(r)  (((r)  GENMASK(18, 16))  16)
+#defineCOMP1_TX_CHANNELS(r)(((r)  GENMASK(10, 9))  9)
+#defineCOMP1_RX_CHANNELS(r)(((r)  GENMASK(8, 7))  7)
+#defineCOMP1_RX_ENABLED(r) (((r)  BIT(6))  6)
+#defineCOMP1_TX_ENABLED(r) (((r)  BIT(5))  5)
+#defineCOMP1_MODE_EN(r)(((r)  BIT(4))  4)
+#defineCOMP1_FIFO_DEPTH_GLOBAL(r)  (((r)  GENMASK(3, 2))  2)
+#defineCOMP1_APB_DATA_WIDTH(r) (((r)  GENMASK(1, 0))  0)
+
+#defineCOMP2_RX_WORDSIZE_3(r)  (((r)  GENMASK(12, 10))  10)
+#defineCOMP2_RX_WORDSIZE_2(r)  (((r)  GENMASK(9, 7))  7)
+#defineCOMP2_RX_WORDSIZE_1(r)  (((r)  GENMASK(5, 3))  3)
+#defineCOMP2_RX_WORDSIZE_0(r)  (((r)  GENMASK(2, 0))  0)
+
 #define MAX_CHANNEL_NUM8
 #define MIN_CHANNEL_NUM2
 
+union snd_dma_data {
+   struct i2s_dma_data pd;
+   struct snd_dmaengine_dai_dma_data dt;
+};
+
 struct dw_i2s_dev {
void __iomem *i2s_base;
struct clk *clk;
@@ -65,8 +92,8 @@ struct dw_i2s_dev {
struct device *dev;
 
/* data related to DMA transfers b/w i2s and DMAC */
-   struct i2s_dma_data play_dma_data;
-   struct i2s_dma_data capture_dma_data;
+   union snd_dma_data play_dma_data;
+   union snd_dma_data capture_dma_data;
struct i2s_clk_config_data config;
int (*i2s_clk_cfg)(struct i2s_clk_config_data *config);
 };
@@ -153,7 +180,7 @@ static int dw_i2s_startup(struct snd_pcm_substream 
*substream,
struct snd_soc_dai *cpu_dai)
 {
struct dw_i2s_dev *dev = snd_soc_dai_get_drvdata(cpu_dai);
-   struct i2s_dma_data *dma_data = NULL;
+   union snd_dma_data *dma_data = NULL;
 
if (!(dev-capability  DWC_I2S_RECORD) 
(substream-stream == SNDRV_PCM_STREAM_CAPTURE))
@@ -244,13 +271,18 @@ static int dw_i2s_hw_params(struct snd_pcm_substream 
*substream,
 
config-sample_rate = params_rate(params);
 
-   if (!dev-i2s_clk_cfg)
-   return -EINVAL;
+   if (dev-i2s_clk_cfg) {
+   ret = dev-i2s_clk_cfg(config);
+   if (ret  0) {
+   dev_err(dev-dev, runtime audio clk config fail\n);
+   return ret;
+   }
+   } else {
+   u32 bitclk;
 
-   ret = dev-i2s_clk_cfg(config);
-   if (ret  0) {
-   dev_err(dev-dev, runtime audio clk config fail\n);
-   return ret;
+   /* TODO: Validate sample rate against permissible set */
+   bitclk = config-sample_rate * config-data_width * 2;
+   clk_set_rate(dev-clk, bitclk);
}
 
return 0;
@@ -323,6 +355,31 @@ static int dw_i2s_resume(struct snd_soc_dai *dai)
 #define dw_i2s_resume  NULL
 #endif
 
+/* Maximum resolution of a channel - not uniformly spaced */
+static const u32 fifo_width[] = {
+   12, 16, 20, 24, 32, 0, 0, 0
+};
+
+/* Width of (DMA) bus */
+static const u32 bus_widths[] = {
+   DMA_SLAVE_BUSWIDTH_1_BYTE,
+   DMA_SLAVE_BUSWIDTH_2_BYTES,
+   DMA_SLAVE_BUSWIDTH_4_BYTES,
+   DMA_SLAVE_BUSWIDTH_UNDEFINED
+};
+
+/* PCM format to support channel

Re: [PATCH v2 1/6] ASoC: dwc: Remove unnecessary debug messages and tests

2014-12-12 Thread Andrew Jackson
On 12/12/14 09:31, Joe Perches wrote:
 On Fri, 2014-12-12 at 09:25 +, Andrew Jackson wrote:
 The devm_XXX allocation functions print a message on failure,
 so additional messages are not required.
 []
 diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
 []
 @@ -345,26 +345,17 @@ static int dw_i2s_probe(struct platform_device *pdev)
  }
  
  dw_i2s_dai = devm_kzalloc(pdev-dev, sizeof(*dw_i2s_dai), GFP_KERNEL);
 -if (!dw_i2s_dai) {
 -dev_err(pdev-dev, mem allocation failed for dai driver\n);
 +if (!dw_i2s_dai)
  return -ENOMEM;
 -}
 
 ok.
  
  dw_i2s_dai-ops = dw_i2s_dai_ops;
  dw_i2s_dai-suspend = dw_i2s_suspend;
  dw_i2s_dai-resume = dw_i2s_resume;
  
  res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 -if (!res) {
 -dev_err(pdev-dev, no i2s resource defined\n);
 -return -ENODEV;
 -}
 -
 
 Why delete this?

Lars-Peter said that it was unnecessary: see 
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-December/308416.html.
 
  dev-i2s_base = devm_ioremap_resource(pdev-dev, res);
 -if (IS_ERR(dev-i2s_base)) {
 -dev_err(pdev-dev, ioremap fail for i2s_region\n);
 +if (IS_ERR(dev-i2s_base))
  return PTR_ERR(dev-i2s_base);
 -}
 
 or this?

Ditto

Andrew

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 4/5] ASoC: dwc: Add devicetree support for Designware I2S

2014-12-04 Thread Andrew Jackson
On 12/04/14 07:43, rajeev kumar wrote:
> Sorry for the last blank mail.
> 
> On Wed, Dec 3, 2014 at 10:09 PM, Andrew Jackson  
> wrote:
>> Convert to driver to use either platform_data or device-tree for 
>> configuration
>> of the device.  When using device-tree, the I2S block's configuration is read
>> from the relevant registers: this reduces the amount of information required 
>> in
>> the device tree.
>>
>> Signed-off-by: Andrew Jackson 
>> ---
>>  .../devicetree/bindings/sound/designware-i2s.txt   |   32 +++
>>  sound/soc/dwc/Kconfig  |1 +
>>  sound/soc/dwc/designware_i2s.c |  238 
>> 
>>  3 files changed, 222 insertions(+), 49 deletions(-)
>>  create mode 100644 
>> Documentation/devicetree/bindings/sound/designware-i2s.txt
>>
>> diff --git a/Documentation/devicetree/bindings/sound/designware-i2s.txt 
>> b/Documentation/devicetree/bindings/sound/designware-i2s.txt
>> new file mode 100644
>> index 000..cdee591
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/sound/designware-i2s.txt
>> @@ -0,0 +1,32 @@
>> +DesignWare I2S controller
>> +
>> +Required properties:
>> + - compatible : Must be "snps,designware-i2s"
>> + - reg : Must contain I2S core's registers location and length
>> + - clocks : Pairs of phandle and specifier referencing the controller's 
>> clocks.
>> +   The controller expects two clocks, the clock used for the APB interface 
>> and
>> +   the clock used as the sampling rate reference clock sample.
>> + - clock-names : "apb_plck" for the clock to the APB interface, "i2sclk" 
>> for the sample
>> +   rate reference clock.
>> + - dmas: Pairs of phandle and specifier for the DMA channels that are used 
>> by
>> +   the core. The core expects one or two dma channels, one for transmit and 
>> one for
>> +   receive.
>> + - dma-names : "tx" for the transmit channel, "rx" for the receive channel.
>> +
>> +For more details on the 'dma', 'dma-names', 'clock' and 'clock-names' 
>> properties
>> +please check:
>> +   * resource-names.txt
>> +   * clock/clock-bindings.txt
>> +   * dma/dma.txt
>> +
>> +Example:
>> +
>> +   soc_i2s: i2s@7ff9 {
>> +   compatible = "snps,designware-i2s";
>> +   reg = <0x0 0x7ff9 0x0 0x1000>;
>> +   clocks = <_i2sclk 0>, <_refclk100mhz>;
>> +   clock-names = "i2sclk", "apb_pclk";
>> +   #sound-dai-cells = <0>;
>> +   dmas = < 5>;
>> +   dma-names = "tx";
>> +   };
>> diff --git a/sound/soc/dwc/Kconfig b/sound/soc/dwc/Kconfig
>> index e334900..d50e085 100644
>> --- a/sound/soc/dwc/Kconfig
>> +++ b/sound/soc/dwc/Kconfig
>> @@ -1,6 +1,7 @@
>>  config SND_DESIGNWARE_I2S
>> tristate "Synopsys I2S Device Driver"
>> depends on CLKDEV_LOOKUP
>> +   select SND_SOC_GENERIC_DMAENGINE_PCM
>> help
>>  Say Y or M if you want to add support for I2S driver for
>>  Synopsys desigwnware I2S device. The device supports upto
>> diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
>> index c497ada..083779d 100644
>> --- a/sound/soc/dwc/designware_i2s.c
>> +++ b/sound/soc/dwc/designware_i2s.c
>> @@ -22,6 +22,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>
>>  /* common register for all channel */
>>  #define IER0x000
>> @@ -54,19 +55,46 @@
>>  #define I2S_COMP_VERSION   0x01F8
>>  #define I2S_COMP_TYPE  0x01FC
>>
>> +/*
>> + * Component parameter register fields - define the I2S block's
>> + * configuration.
>> + */
>> +#defineCOMP1_TX_WORDSIZE_3(r)  (((r) & GENMASK(27, 25)) >> 25)
>> +#defineCOMP1_TX_WORDSIZE_2(r)  (((r) & GENMASK(24, 22)) >> 22)
>> +#defineCOMP1_TX_WORDSIZE_1(r)  (((r) & GENMASK(21, 19)) >> 19)
>> +#defineCOMP1_TX_WORDSIZE_0(r)  (((r) & GENMASK(18, 16)) >> 16)
>> +#defineCOMP1_TX_CHANNELS(r)(((r) & GENMASK(10, 9)) >> 9)
>> +#defineCOMP1_RX_CHANNELS(r)(((r) & GENMASK(8, 7)) >> 7)
>> +#defineCOMP1_RX_ENABLED(r) (((r) & BIT(6)) >> 6)
>> +#defineCOMP1_TX_ENABLED(r) (((r) & BIT

Re: [PATCH 4/5] ASoC: dwc: Add devicetree support for Designware I2S

2014-12-04 Thread Andrew Jackson
On 12/03/14 20:13, Arnd Bergmann wrote:
> On Wednesday 03 December 2014 16:39:08 Andrew Jackson wrote:
>> Convert to driver to use either platform_data or device-tree for 
>> configuration
>> of the device.  When using device-tree, the I2S block's configuration is read
>> from the relevant registers: this reduces the amount of information required 
>> in
>> the device tree.
>>
>> Signed-off-by: Andrew Jackson 
> 
> I don't think we even have to worry about the platform_data case here:
> the only platform using this hardware in Linux is arm/mach-spear, and
> it defines a device node with a binding that is similar to the one you
> document here but that is not implemented in the driver.
> 
> So, I think for all practical purposes we can assume that nobody cares
> if you make incompatible changes as long as you don't introduce build
> regression.
> 
> Also, please adapt thearch/arm/boot/dts/spear13*.dts{,i} files
> as good as you can. They are broken in other ways too that you don't
> have to fix, just make them conform to the binding you add.
> 

Will do.  Although I had noticed those DT entries, I couldn't find anything in 
support of them.  I wasn't sure whether someone else was waiting in the wings 
(so as to speak) with a DT enabled Designware I2S driver. 

>   Arnd
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 4/5] ASoC: dwc: Add devicetree support for Designware I2S

2014-12-04 Thread Andrew Jackson
On 12/03/14 18:23, Mark Brown wrote:
> On Wed, Dec 03, 2014 at 04:39:08PM +0000, Andrew Jackson wrote:
> 
>> Convert to driver to use either platform_data or device-tree for 
>> configuration
>> of the device.  When using device-tree, the I2S block's configuration is read
>> from the relevant registers: this reduces the amount of information required 
>> in
>> the device tree.
> 
> This really needs to be split into two or more patches, there's a whole
> bunch of refactoring to support this DT stuff which should be separate
> from the DT addition itself.  Right now it's hard to tell what each
> individual bit of the code is supposed to be doing, the patch is far too
> large and doing far too many individual things.

I will have look at how it might be split.  The majority of the new code is in 
reading and processing the device's configuration: I didn't want to change the 
platform data handling to do that because some of the comments in the driver 
suggested that there were ST specific changes to the Designware IP.  I wasn't 
in a position to know whether, if I changed the configuration reading, the 
driver would still function correctly on the SPEAR platform.

>> +if (dev->using_pd) {
>> +ret = dev->i2s_clk_cfg(config);
>> +if (ret < 0) {
>> +dev_err(dev->dev, "runtime audio clk config fail\n");
>> +return ret;
>> +}
>> +} else {
>> +u32 bitclk;
> 
> Having this whole separate path for using platform data feels icky, we
> don't want to have completely separate flows like this.  Checking for
> the callbacks being there is probably fine but just having totally
> separate code paths is a bit icky.

I wasn't very happy either but making the test explicit seemed reasonable at 
the time.  I'll change the code to test for the presence of the callback 
instead.

Andrew



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 3/5] ASoC: dwc: Iterate over all channels

2014-12-04 Thread Andrew Jackson
On 12/04/14 06:55, rajeev kumar wrote:
> On Wed, Dec 3, 2014 at 10:59 PM, Mark Brown  wrote:
>> On Wed, Dec 03, 2014 at 04:39:01PM +0000, Andrew Jackson wrote:
>>
>>> + /* Iterate over set of channels - independently controlled.
>>> +  */
>>> + do {
>>> + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
>>> + i2s_write_reg(dev->i2s_base, TCR(ch_reg),
>>> +   xfer_resolution);
>>> + i2s_write_reg(dev->i2s_base, TFCR(ch_reg), 0x02);
>>> + irq = i2s_read_reg(dev->i2s_base, IMR(ch_reg));
>>> + i2s_write_reg(dev->i2s_base, IMR(ch_reg), irq & 
>>> ~0x30);
>>> + i2s_write_reg(dev->i2s_base, TER(ch_reg), 1);
>>> + } else {
>>> + i2s_write_reg(dev->i2s_base, RCR(ch_reg),
>>> +   xfer_resolution);
>>> + i2s_write_reg(dev->i2s_base, RFCR(ch_reg), 0x07);
>>> + irq = i2s_read_reg(dev->i2s_base, IMR(ch_reg));
>>> + i2s_write_reg(dev->i2s_base, IMR(ch_reg), irq & 
>>> ~0x03);
>>> + i2s_write_reg(dev->i2s_base, RER(ch_reg), 1);
>>> + }
>>> + } while (ch_reg-- > 0);
>>
>> The normal way to write an iteration would be with a for loop - why are
>> we not doing that?
>>
>> Also I see that you've not sent these as a single thread - please use
>> --thread.
> 
> designware i2s has individual register for channel support. It is like
> TCR(0), TCR(1) and so on.. So a macro is defined to capture these
> please check below #define
> 
> #define TCR(x)  (0x40 * x + 0x034)
> 
> and the same is true for capture also. So there is no need for
> iteration. Only writing to the particular register will do the work.

However params_channels returns the number of channels that the device needs to 
configure.  So, for example, when there are four channels two sets of channel 
registers need to be programmed.


Andrew
> 
> ~Rajeev
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/5] ASoC: dwc: Don't allow negative use counts

2014-12-04 Thread Andrew Jackson
On 12/04/14 06:43, rajeev kumar wrote:
> On Thu, Dec 4, 2014 at 12:10 PM, rajeev kumar
>  wrote:
>> On Wed, Dec 3, 2014 at 10:55 PM, Mark Brown  wrote:
>>> On Wed, Dec 03, 2014 at 04:38:55PM +, Andrew Jackson wrote:
>>>
>>>>   case SNDRV_PCM_TRIGGER_STOP:
>>>>   case SNDRV_PCM_TRIGGER_SUSPEND:
>>>>   case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
>>>> - dev->active--;
>>>> + if (dev->active > 0)
>>>> + dev->active--;
>>>
>>> How is this triggering - this sounds like you're papering over some
>>> other bug somewhere?
> 
> This check can be removed as it is not going to triggered.

As I said in my email to Mark, I couldn't convince myself that STOP/SUSPEND 
would only be called once so it seemed like a loophole which /might/ result in 
the device not functioning correctly.

If this can never happen, I'll drop the patch.

Andrew

> B'rgds
> ~Rajeev
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/5] ASoC: dwc: Allocate resources with devm_ioremap_resource

2014-12-04 Thread Andrew Jackson
On 12/03/14 18:26, Mark Brown wrote:
> On Wed, Dec 03, 2014 at 04:38:46PM +0000, Andrew Jackson wrote:
>> Prepare for the introduction of device-tree support by re-ordering some
>> of the allocations and using devm_iomap_resource to simplify IO mapping.
> 
> Applied, thanks.
> 

Lars-Peter Clausen had some comments on this patch so I was intending to 
resubmit it.  Would you prefer that I submit a patch to my original?

  Andrew
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 3/5] ASoC: dwc: Iterate over all channels

2014-12-04 Thread Andrew Jackson
On 12/03/14 17:29, Mark Brown wrote:
> On Wed, Dec 03, 2014 at 04:39:01PM +0000, Andrew Jackson wrote:
> 
>> +/* Iterate over set of channels - independently controlled.
>> + */
>> +do {
>> +if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
>> +i2s_write_reg(dev->i2s_base, TCR(ch_reg),
>> +  xfer_resolution);
>> +i2s_write_reg(dev->i2s_base, TFCR(ch_reg), 0x02);
>> +irq = i2s_read_reg(dev->i2s_base, IMR(ch_reg));
>> +i2s_write_reg(dev->i2s_base, IMR(ch_reg), irq & ~0x30);
>> +i2s_write_reg(dev->i2s_base, TER(ch_reg), 1);
>> +} else {
>> +i2s_write_reg(dev->i2s_base, RCR(ch_reg),
>> +  xfer_resolution);
>> +i2s_write_reg(dev->i2s_base, RFCR(ch_reg), 0x07);
>> +irq = i2s_read_reg(dev->i2s_base, IMR(ch_reg));
>> +i2s_write_reg(dev->i2s_base, IMR(ch_reg), irq & ~0x03);
>> +i2s_write_reg(dev->i2s_base, RER(ch_reg), 1);
>> +}
>> +} while (ch_reg-- > 0);
> 
> The normal way to write an iteration would be with a for loop - why are
> we not doing that?

The intention was to minimise the changes, excluding whitespace, between this 
version and the original.  Also, it is a perfectly valid looping construct.  
I'm happy to rework it into a for loop.

> Also I see that you've not sent these as a single thread - please use
> --thread.
> 

Andrew
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/5] ASoC: dwc: Don't allow negative use counts

2014-12-04 Thread Andrew Jackson
On 12/03/14 17:25, Mark Brown wrote:
> On Wed, Dec 03, 2014 at 04:38:55PM +0000, Andrew Jackson wrote:
> 
>>  case SNDRV_PCM_TRIGGER_STOP:
>>  case SNDRV_PCM_TRIGGER_SUSPEND:
>>  case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
>> -dev->active--;
>> +if (dev->active > 0)
>> +dev->active--;
> 
> How is this triggering - this sounds like you're papering over some
> other bug somewhere?
> 

When I looked at the code paths I couldn't convince myself that STOP wouldn't 
be called more than once.  Then actuve would be negative and the device might 
not be restartable.  I didn't have a problem per se, it was just that it seemed 
to be something of a loophole.

   Andrew

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 0/5] ASoC: dwc: Add device tree support to designware I2S

2014-12-04 Thread Andrew Jackson
On 12/03/14 17:21, Mark Brown wrote:
> On Wed, Dec 03, 2014 at 04:38:36PM +0000, Andrew Jackson wrote:
>> This patch set extends the DesignWare I2S driver to provide device
>> tree support and fixes a couple of small faults.
> 
> If you're sending fixes the fixes should be the first things in the
> series - the idea is that we should be able to send the fixes to Linus
> and stable separate to any new features.
> 

Oh, sorry about that, I will reorder the patches before resubmission.

   Andrew

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [alsa-devel] [PATCH 1/5] ASoC: dwc: Allocate resources with devm_ioremap_resource

2014-12-04 Thread Andrew Jackson
On 12/03/14 16:44, Lars-Peter Clausen wrote:
> On 12/03/2014 05:38 PM, Andrew Jackson wrote:
> [,,,[
>> +dw_i2s_dai = devm_kzalloc(>dev, sizeof(*dw_i2s_dai), GFP_KERNEL);
>> +if (!dw_i2s_dai) {
>> +dev_err(>dev, "mem allocation failed for dai driver\n");
> 
> All the memory alloc functions already print a error message.

I will remove the error message(s).

>>  return -ENOMEM;
>>  }
>>
>> +dw_i2s_dai->ops = _i2s_dai_ops;
>> +dw_i2s_dai->suspend = dw_i2s_suspend;
>> +dw_i2s_dai->resume = dw_i2s_resume;
> 
> This seems to be separate from the devm_ioremap_resource() change.

It seemed reasonable to assign these known values to the dw_i2s_dai as soon as 
it had been allocated. Would you prefer this in a separate patch?

>> +
>> +res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +if (!res) {
> 
> You don't actually have to check it devm_ioremap_resource does this for you.

Oh, thanks, paranoia was taking over.

>> +dev_err(>dev, "no i2s resource defined\n");
>> +return -ENODEV;
>> +}
>> +
>> +dev->i2s_base = devm_ioremap_resource(>dev, res);
>> +if (IS_ERR(dev->i2s_base)) {
>> +dev_err(>dev, "ioremap fail for i2s_region\n");
> 
> Same here devm_ioremap_resource() will already print a appropriate error 
> message.
> 
>> +return PTR_ERR(dev->i2s_base);
>> +}
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [alsa-devel] [PATCH 1/5] ASoC: dwc: Allocate resources with devm_ioremap_resource

2014-12-04 Thread Andrew Jackson
On 12/03/14 16:44, Lars-Peter Clausen wrote:
 On 12/03/2014 05:38 PM, Andrew Jackson wrote:
 [,,,[
 +dw_i2s_dai = devm_kzalloc(pdev-dev, sizeof(*dw_i2s_dai), GFP_KERNEL);
 +if (!dw_i2s_dai) {
 +dev_err(pdev-dev, mem allocation failed for dai driver\n);
 
 All the memory alloc functions already print a error message.

I will remove the error message(s).

  return -ENOMEM;
  }

 +dw_i2s_dai-ops = dw_i2s_dai_ops;
 +dw_i2s_dai-suspend = dw_i2s_suspend;
 +dw_i2s_dai-resume = dw_i2s_resume;
 
 This seems to be separate from the devm_ioremap_resource() change.

It seemed reasonable to assign these known values to the dw_i2s_dai as soon as 
it had been allocated. Would you prefer this in a separate patch?

 +
 +res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 +if (!res) {
 
 You don't actually have to check it devm_ioremap_resource does this for you.

Oh, thanks, paranoia was taking over.

 +dev_err(pdev-dev, no i2s resource defined\n);
 +return -ENODEV;
 +}
 +
 +dev-i2s_base = devm_ioremap_resource(pdev-dev, res);
 +if (IS_ERR(dev-i2s_base)) {
 +dev_err(pdev-dev, ioremap fail for i2s_region\n);
 
 Same here devm_ioremap_resource() will already print a appropriate error 
 message.
 
 +return PTR_ERR(dev-i2s_base);
 +}
 
 

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 0/5] ASoC: dwc: Add device tree support to designware I2S

2014-12-04 Thread Andrew Jackson
On 12/03/14 17:21, Mark Brown wrote:
 On Wed, Dec 03, 2014 at 04:38:36PM +, Andrew Jackson wrote:
 This patch set extends the DesignWare I2S driver to provide device
 tree support and fixes a couple of small faults.
 
 If you're sending fixes the fixes should be the first things in the
 series - the idea is that we should be able to send the fixes to Linus
 and stable separate to any new features.
 

Oh, sorry about that, I will reorder the patches before resubmission.

   Andrew

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/5] ASoC: dwc: Don't allow negative use counts

2014-12-04 Thread Andrew Jackson
On 12/03/14 17:25, Mark Brown wrote:
 On Wed, Dec 03, 2014 at 04:38:55PM +, Andrew Jackson wrote:
 
  case SNDRV_PCM_TRIGGER_STOP:
  case SNDRV_PCM_TRIGGER_SUSPEND:
  case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
 -dev-active--;
 +if (dev-active  0)
 +dev-active--;
 
 How is this triggering - this sounds like you're papering over some
 other bug somewhere?
 

When I looked at the code paths I couldn't convince myself that STOP wouldn't 
be called more than once.  Then actuve would be negative and the device might 
not be restartable.  I didn't have a problem per se, it was just that it seemed 
to be something of a loophole.

   Andrew

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 3/5] ASoC: dwc: Iterate over all channels

2014-12-04 Thread Andrew Jackson
On 12/03/14 17:29, Mark Brown wrote:
 On Wed, Dec 03, 2014 at 04:39:01PM +, Andrew Jackson wrote:
 
 +/* Iterate over set of channels - independently controlled.
 + */
 +do {
 +if (substream-stream == SNDRV_PCM_STREAM_PLAYBACK) {
 +i2s_write_reg(dev-i2s_base, TCR(ch_reg),
 +  xfer_resolution);
 +i2s_write_reg(dev-i2s_base, TFCR(ch_reg), 0x02);
 +irq = i2s_read_reg(dev-i2s_base, IMR(ch_reg));
 +i2s_write_reg(dev-i2s_base, IMR(ch_reg), irq  ~0x30);
 +i2s_write_reg(dev-i2s_base, TER(ch_reg), 1);
 +} else {
 +i2s_write_reg(dev-i2s_base, RCR(ch_reg),
 +  xfer_resolution);
 +i2s_write_reg(dev-i2s_base, RFCR(ch_reg), 0x07);
 +irq = i2s_read_reg(dev-i2s_base, IMR(ch_reg));
 +i2s_write_reg(dev-i2s_base, IMR(ch_reg), irq  ~0x03);
 +i2s_write_reg(dev-i2s_base, RER(ch_reg), 1);
 +}
 +} while (ch_reg--  0);
 
 The normal way to write an iteration would be with a for loop - why are
 we not doing that?

The intention was to minimise the changes, excluding whitespace, between this 
version and the original.  Also, it is a perfectly valid looping construct.  
I'm happy to rework it into a for loop.

 Also I see that you've not sent these as a single thread - please use
 --thread.
 

Andrew
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/5] ASoC: dwc: Allocate resources with devm_ioremap_resource

2014-12-04 Thread Andrew Jackson
On 12/03/14 18:26, Mark Brown wrote:
 On Wed, Dec 03, 2014 at 04:38:46PM +, Andrew Jackson wrote:
 Prepare for the introduction of device-tree support by re-ordering some
 of the allocations and using devm_iomap_resource to simplify IO mapping.
 
 Applied, thanks.
 

Lars-Peter Clausen had some comments on this patch so I was intending to 
resubmit it.  Would you prefer that I submit a patch to my original?

  Andrew
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/5] ASoC: dwc: Don't allow negative use counts

2014-12-04 Thread Andrew Jackson
On 12/04/14 06:43, rajeev kumar wrote:
 On Thu, Dec 4, 2014 at 12:10 PM, rajeev kumar
 rajeevkumar.li...@gmail.com wrote:
 On Wed, Dec 3, 2014 at 10:55 PM, Mark Brown broo...@kernel.org wrote:
 On Wed, Dec 03, 2014 at 04:38:55PM +, Andrew Jackson wrote:

   case SNDRV_PCM_TRIGGER_STOP:
   case SNDRV_PCM_TRIGGER_SUSPEND:
   case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
 - dev-active--;
 + if (dev-active  0)
 + dev-active--;

 How is this triggering - this sounds like you're papering over some
 other bug somewhere?
 
 This check can be removed as it is not going to triggered.

As I said in my email to Mark, I couldn't convince myself that STOP/SUSPEND 
would only be called once so it seemed like a loophole which /might/ result in 
the device not functioning correctly.

If this can never happen, I'll drop the patch.

Andrew

 B'rgds
 ~Rajeev
 

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


  1   2   >