Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

[PATCH 0/3 v2] Add I2S/ADV7511 audio support for ARC AXS10x boards

46 views
Skip to first unread message

Jose Abreu

unread,
Mar 28, 2016, 10:37:11 AM3/28/16
to linux-s...@lists.infradead.org, linux-...@vger.kernel.org, dri-...@lists.freedesktop.org, alsa-...@alsa-project.org, devic...@vger.kernel.org, air...@linux.ie, lgir...@gmail.com, bro...@kernel.org, pe...@perex.cz, ti...@suse.com, laurent.pinc...@ideasonboard.com, wsa+r...@sang-engineering.com, la...@metafoo.de, ville....@linux.intel.com, nar...@opensource.wolfsonmicro.com, alexande...@amd.com, Maruthi.B...@amd.com, buyi...@gmail.com, ti...@linaro.org, yiti...@tangramtek.com, Alexey....@synopsys.com, rob...@kernel.org, pawel...@arm.com, mark.r...@arm.com, ijc+dev...@hellion.org.uk, ga...@codeaurora.org, Vineet...@synopsys.com, CARLOS....@synopsys.com, Jose Abreu
ARC AXS10x platforms consist of a mainboard with several peripherals.
One of those peripherals is an HDMI output port controlled by the ADV7511
transmitter.

This patch set adds audio for the ADV7511 transmitter and I2S audio for
the AXS10x platform.

Changes v1 -> v2:
* DT bindings moved to separate patch (as suggested by Alexey Brodkin)
* Removed defconfigs entries (as suggested by Alexey Brodkin)

Jose Abreu (3):
drm/i2c/adv7511: Add audio support
ASoC: dwc: Add I2S HDMI audio support
arc: axs10x: Add support for Designware I2S on DT

arch/arc/boot/dts/axs10x_mb.dtsi | 49 +-
drivers/gpu/drm/i2c/Kconfig | 11 +
drivers/gpu/drm/i2c/Makefile | 2 +
drivers/gpu/drm/i2c/adv7511.c | 1024 -----------------------------------
drivers/gpu/drm/i2c/adv7511.h | 41 ++
drivers/gpu/drm/i2c/adv7511_audio.c | 310 +++++++++++
drivers/gpu/drm/i2c/adv7511_core.c | 1005 ++++++++++++++++++++++++++++++++++
include/sound/soc-dai.h | 1 +
sound/soc/dwc/Kconfig | 1 +
sound/soc/dwc/designware_i2s.c | 385 ++++++++++++-
10 files changed, 1788 insertions(+), 1041 deletions(-)
delete mode 100644 drivers/gpu/drm/i2c/adv7511.c
create mode 100644 drivers/gpu/drm/i2c/adv7511_audio.c
create mode 100644 drivers/gpu/drm/i2c/adv7511_core.c

--
1.9.1


Alexey Brodkin

unread,
Mar 28, 2016, 11:36:07 AM3/28/16
to Jose Abreu
Hi Jose,

On Mon, 2016-03-28 at 15:36 +0100, Jose Abreu wrote:
> HDMI audio support was added to the AXS board using an
> I2S cpu driver and a custom platform driver.
>
> The platform driver supports two channels @ 16 bits with
> rates 32k, 44.1k and 48k. ALSA Simple audio card is used to
> glue the cpu, platform and codec driver (adv7511).
>
> Signed-off-by: Jose Abreu <joa...@synopsys.com>
> ---
>
> No changes v1 -> v2.
>
>  sound/soc/dwc/Kconfig          |   1 +
>  sound/soc/dwc/designware_i2s.c | 385 +++++++++++++++++++++++++++++++++++++++--
>  2 files changed, 373 insertions(+), 13 deletions(-)
>
> diff --git a/sound/soc/dwc/Kconfig b/sound/soc/dwc/Kconfig
> index d50e085..bc3fae7 100644
> --- a/sound/soc/dwc/Kconfig
> +++ b/sound/soc/dwc/Kconfig
> @@ -2,6 +2,7 @@ config SND_DESIGNWARE_I2S
>   tristate "Synopsys I2S Device Driver"
>   depends on CLKDEV_LOOKUP
>   select SND_SOC_GENERIC_DMAENGINE_PCM
> + select SND_SIMPLE_CARD
>   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 bff258d..0f2f588 100644
> --- a/sound/soc/dwc/designware_i2s.c
> +++ b/sound/soc/dwc/designware_i2s.c
> @@ -84,11 +84,37 @@
>  #define MAX_CHANNEL_NUM 8
>  #define MIN_CHANNEL_NUM 2
>  
> +/* FPGA Version Info */
> +#define FPGA_VER_INFO 0xE0011230
> +#define FPGA_VER_27M 0x000FBED9
> +
> +/* PLL registers addresses */
> +#define PLL_IDIV_ADDR 0xE00100A0
> +#define PLL_FBDIV_ADDR 0xE00100A4
> +#define PLL_ODIV0_ADDR 0xE00100A8
> +#define PLL_ODIV1_ADDR 0xE00100AC

Well I think all is not acceptable.
See all these FPGA_VER_xxx as well as PLL_xxx
are strictly ARC SDP specific things and have nothing to do with generic driver.

That's so pity we don't have a driver for all clocks/PLLs on ARC SDP yet.
So as of now I may only propose to use hard-coded fixed clocks as I did with
ARC PGU, see "pguclk" here:
http://lists.infradead.org/pipermail/linux-snps-arc/2016-March/000790.html

Again I'll try to implement missing clock driver sometime soon because
more and more stuff requires it but for now let's use a work-around.

> +struct dw_i2s_pll {
> + unsigned int rate;
> + unsigned int data_width;
> + unsigned int idiv;
> + unsigned int fbdiv;
> + unsigned int odiv0;
> + unsigned int odiv1;
> +};
> +
> +static const struct dw_i2s_pll dw_i2s_pll_cfg_27m[] = {
> + /* 27Mhz */
> + { 32000, 16, 0x104, 0x451, 0x10E38, 0x2000 },
> + { 44100, 16, 0x104, 0x596, 0x10D35, 0x2000 },
> + { 48000, 16, 0x208, 0xA28, 0x10B2C, 0x2000 },
> + { 0, 0, 0, 0, 0, 0 },
>  };
>  
> +static const struct dw_i2s_pll dw_i2s_pll_cfg_28m[] = {
> + /* 28.224Mhz */
> + { 32000, 16, 0x82, 0x105, 0x107DF, 0x2000 },
> + { 44100, 16, 0x28A, 0x1, 0x10001, 0x2000 },
> + { 48000, 16, 0xA28, 0x187, 0x10042, 0x2000 },
> + { 0, 0, 0, 0, 0, 0 },
> +};

These 2 hunks as well should go in ARC SDP clocks.

> +static int i2s_pll_cfg(struct i2s_clk_config_data *config)
> +{
> + const struct dw_i2s_pll *pll_cfg;
> + u32 rate = config->sample_rate;
> + u32 data_width = config->data_width;
> + int i;
> +
> + if (readl((void *)FPGA_VER_INFO) <= FPGA_VER_27M)
> + pll_cfg = dw_i2s_pll_cfg_27m;
> + else
> + pll_cfg = dw_i2s_pll_cfg_28m;
> +
> + for (i = 0; pll_cfg[i].rate != 0; i++) {
> + if ((pll_cfg[i].rate == rate) &&
> + (pll_cfg[i].data_width == data_width)) {
> + writel(pll_cfg[i].idiv, (void *)PLL_IDIV_ADDR);
> + writel(pll_cfg[i].fbdiv, (void *)PLL_FBDIV_ADDR);
> + writel(pll_cfg[i].odiv0, (void *)PLL_ODIV0_ADDR);
> + writel(pll_cfg[i].odiv1, (void *)PLL_ODIV1_ADDR);
> + return 0;
> + }
> + }
> +
> + return -EINVAL;
> +}

Ditto.

-Alexey

Jose Abreu

unread,
Mar 28, 2016, 12:08:05 PM3/28/16
to Alexey Brodkin
Hi Alexey,
Yes, this is a workaround that we are using so that the driver works in ARC SDP
platforms. The driver still has the functionality to operate using a clock
driver (it must be declared in device tree) but if the clock handle is not
declared the driver will assume that must use the internal PLL config options.
This is currently the only option to make it work in ARC SDP.

I will send a v3 soon without this workaround and when the missing clock drivers
are implemented I will re-test this.
Best regards,
Jose Miguel Abreu

Archit Taneja

unread,
Mar 29, 2016, 4:05:47 AM3/29/16
to Jose Abreu, linux-s...@lists.infradead.org, linux-...@vger.kernel.org, dri-...@lists.freedesktop.org, alsa-...@alsa-project.org, devic...@vger.kernel.org, ti...@linaro.org, mark.r...@arm.com, bro...@kernel.org, Alexey....@synopsys.com, lgir...@gmail.com, yiti...@tangramtek.com, wsa+r...@sang-engineering.com, laurent.pinc...@ideasonboard.com, nar...@opensource.wolfsonmicro.com, Maruthi.B...@amd.com, pawel...@arm.com, ijc+dev...@hellion.org.uk, Vineet...@synopsys.com, buyi...@gmail.com, pe...@perex.cz, ti...@suse.com, rob...@kernel.org, ga...@codeaurora.org, alexande...@amd.com, CARLOS....@synopsys.com
Hi,

On 03/28/2016 08:06 PM, Jose Abreu wrote:
> This patch adds audio support for the ADV7511 HDMI transmitter
> using ALSA SoC.
>
> The code was ported from Analog Devices linux tree from
> commit 1770c4a1e32b ("Merge remote-tracking branch
> 'xilinx/master' into xcomm_zynq"), which is available at:
> - https://github.com/analogdevicesinc/linux/
>
> The main core file was renamed from adv7511.c to adv7511_core.c
> so that audio and video compile into a single adv7511.ko module
> and to keep up with Analog Devices kernel tree.
>
> The audio can be disabled using menu-config so it is possible
> to use only video mode.
>
> The HDMI mode is automatically started at boot and the audio
> (when enabled) registers as a codec into ALSA.

Is there a reason why we set the mode to HDMI at probe itself?
Shouldn't it be okay to set the mode later once we read the
EDID off the panel?

Some more comments below.

>
> SPDIF DAI format was also added to ASoC as it is required
> by adv7511 audio.
>
> Signed-off-by: Jose Abreu <joa...@synopsys.com>
> ---
>
> No changes v1 -> v2.
>
> drivers/gpu/drm/i2c/Kconfig | 11 +
> drivers/gpu/drm/i2c/Makefile | 2 +
> drivers/gpu/drm/i2c/adv7511.c | 1024 -----------------------------------
> drivers/gpu/drm/i2c/adv7511.h | 41 ++
> drivers/gpu/drm/i2c/adv7511_audio.c | 310 +++++++++++
> drivers/gpu/drm/i2c/adv7511_core.c | 1005 ++++++++++++++++++++++++++++++++++
> include/sound/soc-dai.h | 1 +
> 7 files changed, 1370 insertions(+), 1024 deletions(-)
> delete mode 100644 drivers/gpu/drm/i2c/adv7511.c
> create mode 100644 drivers/gpu/drm/i2c/adv7511_audio.c
> create mode 100644 drivers/gpu/drm/i2c/adv7511_core.c

<snip>

> +
> +static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
> +{
> + struct adv7511_link_config link_config;
> + struct adv7511 *adv7511;
> + struct device *dev = &i2c->dev;
> + unsigned int val;
> + int ret;
> +
> + if (!dev->of_node)
> + return -EINVAL;
> +
> + adv7511 = devm_kzalloc(dev, sizeof(*adv7511), GFP_KERNEL);
> + if (!adv7511)
> + return -ENOMEM;
> +
> + adv7511->powered = false;
> + adv7511->status = connector_status_disconnected;
> +
> + ret = adv7511_parse_dt(dev->of_node, &link_config);
> + if (ret)
> + return ret;
> +
> + /*
> + * The power down GPIO is optional. If present, toggle it from active to
> + * inactive to wake up the encoder.
> + */
> + adv7511->gpio_pd = devm_gpiod_get_optional(dev, "pd", GPIOD_OUT_HIGH);
> + if (IS_ERR(adv7511->gpio_pd))
> + return PTR_ERR(adv7511->gpio_pd);
> +
> + if (adv7511->gpio_pd) {
> + mdelay(5);
> + gpiod_set_value_cansleep(adv7511->gpio_pd, 0);
> + }
> +
> + adv7511->regmap = devm_regmap_init_i2c(i2c, &adv7511_regmap_config);
> + if (IS_ERR(adv7511->regmap))
> + return PTR_ERR(adv7511->regmap);
> +
> + ret = regmap_read(adv7511->regmap, ADV7511_REG_CHIP_REVISION, &val);
> + if (ret)
> + return ret;
> + dev_dbg(dev, "Rev. %d\n", val);
> +
> + ret = regmap_register_patch(adv7511->regmap, adv7511_fixed_registers,
> + ARRAY_SIZE(adv7511_fixed_registers));
> + if (ret)
> + return ret;
> +
> + regmap_write(adv7511->regmap, ADV7511_REG_EDID_I2C_ADDR, edid_i2c_addr);
> + regmap_write(adv7511->regmap, ADV7511_REG_PACKET_I2C_ADDR,
> + packet_i2c_addr);
> + regmap_write(adv7511->regmap, ADV7511_REG_CEC_I2C_ADDR, cec_i2c_addr);
> + adv7511_packet_disable(adv7511, 0xffff);
> +
> + adv7511->i2c_main = i2c;
> + adv7511->i2c_edid = i2c_new_dummy(i2c->adapter, edid_i2c_addr >> 1);
> + if (!adv7511->i2c_edid)
> + return -ENOMEM;
> +
> + if (i2c->irq) {
> + init_waitqueue_head(&adv7511->wq);
> +
> + ret = devm_request_threaded_irq(dev, i2c->irq, NULL,
> + adv7511_irq_handler,
> + IRQF_ONESHOT, dev_name(dev),
> + adv7511);
> + if (ret)
> + goto err_i2c_unregister_device;
> + }
> +
> + /* CEC is unused for now */
> + regmap_write(adv7511->regmap, ADV7511_REG_CEC_CTRL,
> + ADV7511_CEC_CTRL_POWER_DOWN);
> +
> + adv7511_power_off(adv7511);
> +
> + i2c_set_clientdata(i2c, adv7511);
> +
> +#ifdef CONFIG_DRM_I2C_ADV7511_AUDIO
> + adv7511_audio_init(&i2c->dev);
> +#endif

If we intend to have more audio funcs being used by the core in the
future, it would be nice to have NOP audio funcs rather than having
multiple #ifdef checks in the driver when CONFIG_DRM_I2C_ADV7511_AUDIO
isn't set.

> +
> + adv7511_set_link_config(adv7511, &link_config);
> +
> + /* Enable HDMI mode */
> + regmap_update_bits(adv7511->regmap, ADV7511_REG_HDCP_HDMI_CFG,
> + ADV7511_HDMI_CFG_MODE_MASK,
> + ADV7511_HDMI_CFG_MODE_HDMI);
> +
> + return 0;
> +
> +err_i2c_unregister_device:
> + i2c_unregister_device(adv7511->i2c_edid);
> +
> + return ret;
> +}
> +
> +static int adv7511_remove(struct i2c_client *i2c)
> +{
> + struct adv7511 *adv7511 = i2c_get_clientdata(i2c);
> +

Are we missing a call to adv7511_audio_exit() here?

Thanks,
Archit

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum, hosted by The Linux Foundation

Jose Abreu

unread,
Mar 29, 2016, 6:52:35 AM3/29/16
to Archit Taneja, Jose Abreu, linux-s...@lists.infradead.org, linux-...@vger.kernel.org, dri-...@lists.freedesktop.org, alsa-...@alsa-project.org, devic...@vger.kernel.org, ti...@linaro.org, mark.r...@arm.com, bro...@kernel.org, Alexey....@synopsys.com, lgir...@gmail.com, yiti...@tangramtek.com, wsa+r...@sang-engineering.com, laurent.pinc...@ideasonboard.com, nar...@opensource.wolfsonmicro.com, Maruthi.B...@amd.com, pawel...@arm.com, ijc+dev...@hellion.org.uk, Vineet...@synopsys.com, buyi...@gmail.com, pe...@perex.cz, ti...@suse.com, rob...@kernel.org, ga...@codeaurora.org, alexande...@amd.com, CARLOS....@synopsys.com
Hi Archit,

On 29-03-2016 09:05, Archit Taneja wrote:
> Hi,
>
> On 03/28/2016 08:06 PM, Jose Abreu wrote:
>> This patch adds audio support for the ADV7511 HDMI transmitter
>> using ALSA SoC.
>>
>> The code was ported from Analog Devices linux tree from
>> commit 1770c4a1e32b ("Merge remote-tracking branch
>> 'xilinx/master' into xcomm_zynq"), which is available at:
>> - https://github.com/analogdevicesinc/linux/
>>
>> The main core file was renamed from adv7511.c to adv7511_core.c
>> so that audio and video compile into a single adv7511.ko module
>> and to keep up with Analog Devices kernel tree.
>>
>> The audio can be disabled using menu-config so it is possible
>> to use only video mode.
>>
>> The HDMI mode is automatically started at boot and the audio
>> (when enabled) registers as a codec into ALSA.
>
> Is there a reason why we set the mode to HDMI at probe itself?
> Shouldn't it be okay to set the mode later once we read the
> EDID off the panel?
>
> Some more comments below.
>

Well, when I was using this in kernel 3.18 (with an older version of the driver)
I noticed that DVI mode was being used even when HDMI was connected so I forced
the driver to start in HDMI mode. There were some changes in the driver so it is
possible that this is no longer needed. Should I drop it?
I will move this ifdef to adv751_audio and use NOP functions.

>> +
>> + adv7511_set_link_config(adv7511, &link_config);
>> +
>> + /* Enable HDMI mode */
>> + regmap_update_bits(adv7511->regmap, ADV7511_REG_HDCP_HDMI_CFG,
>> + ADV7511_HDMI_CFG_MODE_MASK,
>> + ADV7511_HDMI_CFG_MODE_HDMI);
>> +
>> + return 0;
>> +
>> +err_i2c_unregister_device:
>> + i2c_unregister_device(adv7511->i2c_edid);
>> +
>> + return ret;
>> +}
>> +
>> +static int adv7511_remove(struct i2c_client *i2c)
>> +{
>> + struct adv7511 *adv7511 = i2c_get_clientdata(i2c);
>> +
>
> Are we missing a call to adv7511_audio_exit() here?

I followed the code in Analog Devices tree where there is no call to
audio_exit() but indeed you are correct. I will add this call.

>
> Thanks,
> Archit
>

Thanks for your comments!

Mark Brown

unread,
Mar 29, 2016, 1:00:51 PM3/29/16
to Jose Abreu, linux-s...@lists.infradead.org, linux-...@vger.kernel.org, dri-...@lists.freedesktop.org, alsa-...@alsa-project.org, devic...@vger.kernel.org, air...@linux.ie, lgir...@gmail.com, pe...@perex.cz, ti...@suse.com, laurent.pinc...@ideasonboard.com, wsa+r...@sang-engineering.com, la...@metafoo.de, ville....@linux.intel.com, nar...@opensource.wolfsonmicro.com, alexande...@amd.com, Maruthi.B...@amd.com, buyi...@gmail.com, ti...@linaro.org, yiti...@tangramtek.com, Alexey....@synopsys.com, rob...@kernel.org, pawel...@arm.com, mark.r...@arm.com, ijc+dev...@hellion.org.uk, ga...@codeaurora.org, Vineet...@synopsys.com, CARLOS....@synopsys.com
On Mon, Mar 28, 2016 at 03:36:08PM +0100, Jose Abreu wrote:
> ARC AXS10x platforms consist of a mainboard with several peripherals.
> One of those peripherals is an HDMI output port controlled by the ADV7511
> transmitter.

I'm going to tell you the same thing I tell everyone else working on
HDMI audio integration: you all need to talk to each other and review
each other's code and unless there is a very good reason for it there
should be at least some code sharing. For example take a look at the
patches Jyri Sarha has been posting recently. I don't have detailed
knowledge of HDMI and the range of hardware that's out there for it but
I am seeing a number of different people posting patch serieses that
look a lot like each other and like they should be sharing things.

Please also try to keep your CC lists reasonable, the set of people
you've copied on this stuff is enormous and I'm having trouble seeing
why a lot of tehm are included.
signature.asc

Archit Taneja

unread,
Mar 29, 2016, 1:03:58 PM3/29/16
to Jose Abreu, linux-s...@lists.infradead.org, linux-...@vger.kernel.org, dri-...@lists.freedesktop.org, alsa-...@alsa-project.org, devic...@vger.kernel.org, ti...@linaro.org, mark.r...@arm.com, bro...@kernel.org, Alexey....@synopsys.com, lgir...@gmail.com, yiti...@tangramtek.com, wsa+r...@sang-engineering.com, laurent.pinc...@ideasonboard.com, nar...@opensource.wolfsonmicro.com, Maruthi.B...@amd.com, pawel...@arm.com, ijc+dev...@hellion.org.uk, Vineet...@synopsys.com, buyi...@gmail.com, pe...@perex.cz, ti...@suse.com, rob...@kernel.org, ga...@codeaurora.org, alexande...@amd.com, CARLOS....@synopsys.com
Mode selection works fine with ADV7533 on a 4.5 kernel. I'm assuming it
should work out of the box for ADV7511 too. We should drop this.
Thanks, I think it should help in the longer run.

>
>>> +
>>> + adv7511_set_link_config(adv7511, &link_config);
>>> +
>>> + /* Enable HDMI mode */
>>> + regmap_update_bits(adv7511->regmap, ADV7511_REG_HDCP_HDMI_CFG,
>>> + ADV7511_HDMI_CFG_MODE_MASK,
>>> + ADV7511_HDMI_CFG_MODE_HDMI);
>>> +
>>> + return 0;
>>> +
>>> +err_i2c_unregister_device:
>>> + i2c_unregister_device(adv7511->i2c_edid);
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +static int adv7511_remove(struct i2c_client *i2c)
>>> +{
>>> + struct adv7511 *adv7511 = i2c_get_clientdata(i2c);
>>> +
>>
>> Are we missing a call to adv7511_audio_exit() here?
>
> I followed the code in Analog Devices tree where there is no call to
> audio_exit() but indeed you are correct. I will add this call.
>

Since we have 3 files for adv7511 now, could we also move the driver
to a separate folder? The long term plan is to convert all the i2c
slave encoder drivers to bridges. Keeping them together would be nicer
when we migrate this driver to the bridge folder.

Thanks,
Archit

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Mark Brown

unread,
Mar 29, 2016, 1:32:22 PM3/29/16
to Jose Abreu, linux-s...@lists.infradead.org, linux-...@vger.kernel.org, dri-...@lists.freedesktop.org, alsa-...@alsa-project.org, devic...@vger.kernel.org, air...@linux.ie, lgir...@gmail.com, pe...@perex.cz, ti...@suse.com, laurent.pinc...@ideasonboard.com, wsa+r...@sang-engineering.com, la...@metafoo.de, ville....@linux.intel.com, nar...@opensource.wolfsonmicro.com, alexande...@amd.com, Maruthi.B...@amd.com, buyi...@gmail.com, ti...@linaro.org, yiti...@tangramtek.com, Alexey....@synopsys.com, rob...@kernel.org, pawel...@arm.com, mark.r...@arm.com, ijc+dev...@hellion.org.uk, ga...@codeaurora.org, Vineet...@synopsys.com, CARLOS....@synopsys.com
On Mon, Mar 28, 2016 at 03:36:10PM +0100, Jose Abreu wrote:
> HDMI audio support was added to the AXS board using an
> I2S cpu driver and a custom platform driver.
>
> The platform driver supports two channels @ 16 bits with
> rates 32k, 44.1k and 48k. ALSA Simple audio card is used to
> glue the cpu, platform and codec driver (adv7511).

> sound/soc/dwc/Kconfig | 1 +
> sound/soc/dwc/designware_i2s.c | 385 +++++++++++++++++++++++++++++++++++++++--

Your changelog appears to describe the writing of a machine driver but
this is a large patch adding code to an I2S controller driver. This
means I can't review your patch since I can't tell what it is supposed
to do. If you've added functionality to this driver you need to send
one or more patches each of which adds a single feature to the driver
together with a changelog which describes what that feature is.

Glancing at the patch I'm not 100% sure that the features you're adding
are part of the Synopsis device but I'm not entirely sure.

> 2 files changed, 373 insertions(+), 13 deletions(-)
>
> diff --git a/sound/soc/dwc/Kconfig b/sound/soc/dwc/Kconfig
> index d50e085..bc3fae7 100644
> --- a/sound/soc/dwc/Kconfig
> +++ b/sound/soc/dwc/Kconfig
> @@ -2,6 +2,7 @@ config SND_DESIGNWARE_I2S
> tristate "Synopsys I2S Device Driver"
> depends on CLKDEV_LOOKUP
> select SND_SOC_GENERIC_DMAENGINE_PCM
> + select SND_SIMPLE_CARD

No, this doesn't make sense - the fact that someone has used a Synopsis
I2S controller doesn't mean that they have a system which uses
simple-card. If the user wants to use simple-card they need to enable
it separately, this is the same pattern we follow for all CPU controller
drivers.
signature.asc

Mark Brown

unread,
Mar 29, 2016, 2:22:59 PM3/29/16
to Jose Abreu, ti...@linaro.org, mark.r...@arm.com, alsa-...@alsa-project.org, lgir...@gmail.com, Alexey....@synopsys.com, dri-...@lists.freedesktop.org, linux-...@vger.kernel.org, yiti...@tangramtek.com, wsa+r...@sang-engineering.com, laurent.pinc...@ideasonboard.com, nar...@opensource.wolfsonmicro.com, linux-s...@lists.infradead.org, devic...@vger.kernel.org, Maruthi.B...@amd.com, pawel...@arm.com, ijc+dev...@hellion.org.uk, Vineet...@synopsys.com, buyi...@gmail.com, rob...@kernel.org, pe...@perex.cz, ti...@suse.com, CARLOS....@synopsys.com, ga...@codeaurora.org, alexande...@amd.com
On Tue, Mar 29, 2016 at 07:03:01PM +0100, Jose Abreu wrote:

> The major part of this patch is the adding of an ALSA platform driver so that
> audio comes out of the box in AXS boards but we also added functionalities to
> the i2s driver and performed one bug fix related with the mask/unmask of
> interrupts. I will split the patches but they will depend on each other.

If you want to add a new platform driver you need to add a new platform
driver, not shove the code into an existing driver for a seperate IP.
signature.asc

Emil Velikov

unread,
Mar 30, 2016, 5:58:54 AM3/30/16
to Jose Abreu, linux-s...@lists.infradead.org, Linux-Kernel@Vger. Kernel. Org, ML dri-devel, alsa-...@alsa-project.org, devicetree, Jon Medhurst, Mark Rutland, bro...@kernel.org, Alexey Brodkin, lgir...@gmail.com, yiti...@tangramtek.com, wsa+r...@sang-engineering.com, laurent.pinc...@ideasonboard.com, nar...@opensource.wolfsonmicro.com, Maruthi.B...@amd.com, Pawel Moll, Ian Campbell, Vineet...@synopsys.com, buyi...@gmail.com, pe...@perex.cz, ti...@suse.com, Rob Herring, Kumar Gala, Alex Deucher, CARLOS....@synopsys.com
Hi Jose,

On 28 March 2016 at 15:36, Jose Abreu <Jose....@synopsys.com> wrote:
> This patch adds audio support for the ADV7511 HDMI transmitter
> using ALSA SoC.
>
> The code was ported from Analog Devices linux tree from
> commit 1770c4a1e32b ("Merge remote-tracking branch
> 'xilinx/master' into xcomm_zynq"), which is available at:
> - https://github.com/analogdevicesinc/linux/
>
> The main core file was renamed from adv7511.c to adv7511_core.c
> so that audio and video compile into a single adv7511.ko module
> and to keep up with Analog Devices kernel tree.
>
> The audio can be disabled using menu-config so it is possible
> to use only video mode.
>
> The HDMI mode is automatically started at boot and the audio
> (when enabled) registers as a codec into ALSA.
>
> SPDIF DAI format was also added to ASoC as it is required
> by adv7511 audio.
>
> Signed-off-by: Jose Abreu <joa...@synopsys.com>
> ---
>
> No changes v1 -> v2.
>
> drivers/gpu/drm/i2c/Kconfig | 11 +
> drivers/gpu/drm/i2c/Makefile | 2 +
> drivers/gpu/drm/i2c/adv7511.c | 1024 -----------------------------------
> drivers/gpu/drm/i2c/adv7511.h | 41 ++
> drivers/gpu/drm/i2c/adv7511_audio.c | 310 +++++++++++
> drivers/gpu/drm/i2c/adv7511_core.c | 1005 ++++++++++++++++++++++++++++++++++
Please keep the file rename separate (and use -M flag when generating
the patch) from the introduction of audio support. Having to check 1k
LOC movement alongside the introduction of new one is a bit...
suboptimal.

Regards,
Emil

Jose Abreu

unread,
Mar 31, 2016, 5:37:49 AM3/31/16
to Mark Brown, Jose Abreu, ti...@linaro.org, mark.r...@arm.com, alsa-...@alsa-project.org, Alexey....@synopsys.com, dri-...@lists.freedesktop.org, linux-...@vger.kernel.org, yiti...@tangramtek.com, wsa+r...@sang-engineering.com, laurent.pinc...@ideasonboard.com, ti...@suse.com, nar...@opensource.wolfsonmicro.com, linux-s...@lists.infradead.org, devic...@vger.kernel.org, Maruthi.B...@amd.com, pawel...@arm.com, ijc+dev...@hellion.org.uk, Vineet...@synopsys.com, buyi...@gmail.com, rob...@kernel.org, pe...@perex.cz, lgir...@gmail.com, CARLOS....@synopsys.com, ga...@codeaurora.org, alexande...@amd.com
Hi Mark,
I can separate the platform driver into a new file but they will have to be
compiled into the same module as the new additions to the i2s driver depend on
functions of the platform driver (see i2s_irq_handler()). Or should I divide
this into two modules and add a Kconfig option to the platform driver? Besides
this I first wanted the driver to be compiled into the same module so that it is
compatible with kernel 3.18 where simple audio card requires that platform
driver == cpu driver.

Jose Abreu

unread,
Mar 31, 2016, 8:58:19 AM3/31/16
to Archit Taneja, Jose Abreu, linux-s...@lists.infradead.org, linux-...@vger.kernel.org, dri-...@lists.freedesktop.org, alsa-...@alsa-project.org, devic...@vger.kernel.org, ti...@linaro.org, mark.r...@arm.com, bro...@kernel.org, Alexey....@synopsys.com, lgir...@gmail.com, yiti...@tangramtek.com, wsa+r...@sang-engineering.com, laurent.pinc...@ideasonboard.com, nar...@opensource.wolfsonmicro.com, Maruthi.B...@amd.com, pawel...@arm.com, ijc+dev...@hellion.org.uk, Vineet...@synopsys.com, buyi...@gmail.com, pe...@perex.cz, ti...@suse.com, rob...@kernel.org, ga...@codeaurora.org, alexande...@amd.com, CARLOS....@synopsys.com
Hi Archit,
Ok, will drop.
Ok, will move to separate folder.

>
> Thanks,
> Archit

Mark Brown

unread,
Mar 31, 2016, 12:57:34 PM3/31/16
to Jose Abreu, ti...@linaro.org, mark.r...@arm.com, alsa-...@alsa-project.org, Alexey....@synopsys.com, dri-...@lists.freedesktop.org, linux-...@vger.kernel.org, yiti...@tangramtek.com, wsa+r...@sang-engineering.com, laurent.pinc...@ideasonboard.com, ti...@suse.com, nar...@opensource.wolfsonmicro.com, linux-s...@lists.infradead.org, devic...@vger.kernel.org, Maruthi.B...@amd.com, pawel...@arm.com, ijc+dev...@hellion.org.uk, Vineet...@synopsys.com, buyi...@gmail.com, rob...@kernel.org, pe...@perex.cz, lgir...@gmail.com, CARLOS....@synopsys.com, ga...@codeaurora.org, alexande...@amd.com
On Thu, Mar 31, 2016 at 10:37:20AM +0100, Jose Abreu wrote:
> On 29-03-2016 19:22, Mark Brown wrote:

> > If you want to add a new platform driver you need to add a new platform
> > driver, not shove the code into an existing driver for a seperate IP.

> I can separate the platform driver into a new file but they will have to be
> compiled into the same module as the new additions to the i2s driver depend on
> functions of the platform driver (see i2s_irq_handler()). Or should I divide

No, that's not at all acceptable. The Designware IP is not specific to
your system, you can't make it depend on your platform driver. The
kernel needs to work on other people's systems too. You need to work
through and/or extend the abstractions the framework provides to
separate the drivers for different IPs.

> this into two modules and add a Kconfig option to the platform driver? Besides
> this I first wanted the driver to be compiled into the same module so that it is
> compatible with kernel 3.18 where simple audio card requires that platform
> driver == cpu driver.

That's not OK upstream, we're working on the current kernel not on
random old kernels. We don't carry compatibility code to enable current
kernel code to be run on years old kernels.
signature.asc

Laurent Pinchart

unread,
Apr 1, 2016, 1:10:27 PM4/1/16
to dri-...@lists.freedesktop.org, Jose Abreu, linux-s...@lists.infradead.org, linux-...@vger.kernel.org, alsa-...@alsa-project.org, devic...@vger.kernel.org, ti...@linaro.org, mark.r...@arm.com, bro...@kernel.org, Alexey....@synopsys.com, lgir...@gmail.com, yiti...@tangramtek.com, wsa+r...@sang-engineering.com, laurent.pinc...@ideasonboard.com, nar...@opensource.wolfsonmicro.com, Maruthi.B...@amd.com, pawel...@arm.com, ijc+dev...@hellion.org.uk, Vineet...@synopsys.com, buyi...@gmail.com, pe...@perex.cz, ti...@suse.com, rob...@kernel.org, ga...@codeaurora.org, alexande...@amd.com, CARLOS....@synopsys.com
Hi Jose,

Thank you for the patch.

On Monday 28 Mar 2016 15:36:09 Jose Abreu wrote:
> This patch adds audio support for the ADV7511 HDMI transmitter
> using ALSA SoC.
>
> The code was ported from Analog Devices linux tree from
> commit 1770c4a1e32b ("Merge remote-tracking branch
> 'xilinx/master' into xcomm_zynq"), which is available at:
> - https://github.com/analogdevicesinc/linux/
>
> The main core file was renamed from adv7511.c to adv7511_core.c
> so that audio and video compile into a single adv7511.ko module
> and to keep up with Analog Devices kernel tree.
>
> The audio can be disabled using menu-config so it is possible
> to use only video mode.
>
> The HDMI mode is automatically started at boot and the audio
> (when enabled) registers as a codec into ALSA.
>
> SPDIF DAI format was also added to ASoC as it is required
> by adv7511 audio.
>
> Signed-off-by: Jose Abreu <joa...@synopsys.com>
> ---
>
> No changes v1 -> v2.
>
> drivers/gpu/drm/i2c/Kconfig | 11 +
> drivers/gpu/drm/i2c/Makefile | 2 +
> drivers/gpu/drm/i2c/adv7511.c | 1024 -------------------------------
> drivers/gpu/drm/i2c/adv7511.h | 41 ++
> drivers/gpu/drm/i2c/adv7511_audio.c | 310 +++++++++++
> drivers/gpu/drm/i2c/adv7511_core.c | 1005 ++++++++++++++++++++++++++++++

Please use git-format-patch -M to detect renames if you send a new version of
this series, it will help with review.

> include/sound/soc-dai.h | 1 +
> 7 files changed, 1370 insertions(+), 1024 deletions(-)
> delete mode 100644 drivers/gpu/drm/i2c/adv7511.c
> create mode 100644 drivers/gpu/drm/i2c/adv7511_audio.c
> create mode 100644 drivers/gpu/drm/i2c/adv7511_core.c

[snip]

> diff --git a/drivers/gpu/drm/i2c/adv7511_core.c
> b/drivers/gpu/drm/i2c/adv7511_core.c new file mode 100644
> index 0000000..d54256a
> --- /dev/null
> +++ b/drivers/gpu/drm/i2c/adv7511_core.c

[snip]
Shouldn't we condition this to the audio channel being somehow described in DT
? If a board doesn't route audio signals to the ADV7511 audio input, there's
no need to register an audio codec.

> +
> + adv7511_set_link_config(adv7511, &link_config);
> +
> + /* Enable HDMI mode */
> + regmap_update_bits(adv7511->regmap, ADV7511_REG_HDCP_HDMI_CFG,
> + ADV7511_HDMI_CFG_MODE_MASK,
> + ADV7511_HDMI_CFG_MODE_HDMI);
> +
> + return 0;
> +
> +err_i2c_unregister_device:
> + i2c_unregister_device(adv7511->i2c_edid);
> +
> + return ret;
> +}

--
Regards,

Laurent Pinchart

Jose Abreu

unread,
Apr 4, 2016, 5:11:46 AM4/4/16
to Laurent Pinchart, dri-...@lists.freedesktop.org, Jose Abreu, linux-s...@lists.infradead.org, linux-...@vger.kernel.org, alsa-...@alsa-project.org, devic...@vger.kernel.org, ti...@linaro.org, mark.r...@arm.com, bro...@kernel.org, Alexey....@synopsys.com, lgir...@gmail.com, yiti...@tangramtek.com, wsa+r...@sang-engineering.com, laurent.pinc...@ideasonboard.com, nar...@opensource.wolfsonmicro.com, Maruthi.B...@amd.com, pawel...@arm.com, ijc+dev...@hellion.org.uk, Vineet...@synopsys.com, buyi...@gmail.com, pe...@perex.cz, ti...@suse.com, rob...@kernel.org, ga...@codeaurora.org, alexande...@amd.com, CARLOS....@synopsys.com
Hi Laurent,
Ok, will do that in next version.
I can do this but the audio is already conditionally compiled using menuconfig.
Is it really necessary to add this extra layer of condition?

>> +
>> + adv7511_set_link_config(adv7511, &link_config);
>> +
>> + /* Enable HDMI mode */
>> + regmap_update_bits(adv7511->regmap, ADV7511_REG_HDCP_HDMI_CFG,
>> + ADV7511_HDMI_CFG_MODE_MASK,
>> + ADV7511_HDMI_CFG_MODE_HDMI);
>> +
>> + return 0;
>> +
>> +err_i2c_unregister_device:
>> + i2c_unregister_device(adv7511->i2c_edid);
>> +
>> + return ret;
>> +}

Laurent Pinchart

unread,
Apr 4, 2016, 5:41:34 PM4/4/16
to Jose Abreu, dri-...@lists.freedesktop.org, linux-s...@lists.infradead.org, linux-...@vger.kernel.org, alsa-...@alsa-project.org, devic...@vger.kernel.org, ti...@linaro.org, mark.r...@arm.com, bro...@kernel.org, Alexey....@synopsys.com, lgir...@gmail.com, yiti...@tangramtek.com, wsa+r...@sang-engineering.com, laurent.pinc...@ideasonboard.com, nar...@opensource.wolfsonmicro.com, Maruthi.B...@amd.com, pawel...@arm.com, ijc+dev...@hellion.org.uk, Vineet...@synopsys.com, buyi...@gmail.com, pe...@perex.cz, ti...@suse.com, rob...@kernel.org, ga...@codeaurora.org, alexande...@amd.com, CARLOS....@synopsys.com
Hi Jose,

On Monday 04 Apr 2016 10:05:39 Jose Abreu wrote:
> On 01-04-2016 18:10, Laurent Pinchart wrote:
> > On Monday 28 Mar 2016 15:36:09 Jose Abreu wrote:
> >> This patch adds audio support for the ADV7511 HDMI transmitter
> >> using ALSA SoC.
> >>
> >> The code was ported from Analog Devices linux tree from
> >> commit 1770c4a1e32b ("Merge remote-tracking branch
> >>
> >> 'xilinx/master' into xcomm_zynq"), which is available at:
> >> - https://github.com/analogdevicesinc/linux/
> >>
> >> The main core file was renamed from adv7511.c to adv7511_core.c
> >> so that audio and video compile into a single adv7511.ko module
> >> and to keep up with Analog Devices kernel tree.
> >>
> >> The audio can be disabled using menu-config so it is possible
> >> to use only video mode.
> >>
> >> The HDMI mode is automatically started at boot and the audio
> >> (when enabled) registers as a codec into ALSA.
> >>
> >> SPDIF DAI format was also added to ASoC as it is required
> >> by adv7511 audio.
> >>
> >> Signed-off-by: Jose Abreu <joa...@synopsys.com>
> >> ---
> >>
> >> No changes v1 -> v2.
> >>
> >> drivers/gpu/drm/i2c/Kconfig | 11 +
> >> drivers/gpu/drm/i2c/Makefile | 2 +
> >> drivers/gpu/drm/i2c/adv7511.c | 1024 ----------------------------
> >> drivers/gpu/drm/i2c/adv7511.h | 41 ++
> >> drivers/gpu/drm/i2c/adv7511_audio.c | 310 +++++++++++
> >> drivers/gpu/drm/i2c/adv7511_core.c | 1005 ++++++++++++++++++++++++++++
> >
The idea is that enabling support for ADV7511 audio in the kernel isn't
coupled with whether the system includes audio support. It would be confusing,
and would also waste resources, to create a Linux sound device when no sound
channel is routed on the board.

> >> +
> >> + adv7511_set_link_config(adv7511, &link_config);
> >> +
> >> + /* Enable HDMI mode */
> >> + regmap_update_bits(adv7511->regmap, ADV7511_REG_HDCP_HDMI_CFG,
> >> + ADV7511_HDMI_CFG_MODE_MASK,
> >> + ADV7511_HDMI_CFG_MODE_HDMI);
> >> +
> >> + return 0;
> >> +
> >> +err_i2c_unregister_device:
> >> + i2c_unregister_device(adv7511->i2c_edid);
> >> +
> >> + return ret;
> >> +}

--
Regards,

Laurent Pinchart

Jose Abreu

unread,
Apr 5, 2016, 7:03:03 AM4/5/16
to Laurent Pinchart, Jose Abreu, dri-...@lists.freedesktop.org, linux-s...@lists.infradead.org, linux-...@vger.kernel.org, alsa-...@alsa-project.org, devic...@vger.kernel.org, ti...@linaro.org, mark.r...@arm.com, bro...@kernel.org, Alexey....@synopsys.com, lgir...@gmail.com, yiti...@tangramtek.com, wsa+r...@sang-engineering.com, laurent.pinc...@ideasonboard.com, nar...@opensource.wolfsonmicro.com, Maruthi.B...@amd.com, pawel...@arm.com, ijc+dev...@hellion.org.uk, Vineet...@synopsys.com, buyi...@gmail.com, pe...@perex.cz, ti...@suse.com, rob...@kernel.org, ga...@codeaurora.org, alexande...@amd.com, CARLOS....@synopsys.com
Hi Laurent,
Ok, will do that in next version. Another question: I am facing a problem when
compiling ALSA as a module, as the ADV7511 audio is not embedded into ALSA the
compilation fails with undefined references to ALSA functions. The solution that
I am planning to use is to add a default value to the ADV7511 kconfig entry so
that the driver is compiled as module when ALSA is a module and as embedded if
ALSA is embedded. Is this okay or is there another solution without moving the
ADV7511 audio to the ALSA directory?

>
>>>> +
>>>> + adv7511_set_link_config(adv7511, &link_config);
>>>> +
>>>> + /* Enable HDMI mode */
>>>> + regmap_update_bits(adv7511->regmap, ADV7511_REG_HDCP_HDMI_CFG,
>>>> + ADV7511_HDMI_CFG_MODE_MASK,
>>>> + ADV7511_HDMI_CFG_MODE_HDMI);
>>>> +
>>>> + return 0;
>>>> +
>>>> +err_i2c_unregister_device:
>>>> + i2c_unregister_device(adv7511->i2c_edid);
>>>> +
>>>> + return ret;
>>>> +}

Laurent Pinchart

unread,
Apr 5, 2016, 12:04:08 PM4/5/16
to Jose Abreu, dri-...@lists.freedesktop.org, linux-s...@lists.infradead.org, linux-...@vger.kernel.org, alsa-...@alsa-project.org, devic...@vger.kernel.org, ti...@linaro.org, mark.r...@arm.com, bro...@kernel.org, Alexey....@synopsys.com, lgir...@gmail.com, yiti...@tangramtek.com, wsa+r...@sang-engineering.com, laurent.pinc...@ideasonboard.com, nar...@opensource.wolfsonmicro.com, Maruthi.B...@amd.com, pawel...@arm.com, ijc+dev...@hellion.org.uk, Vineet...@synopsys.com, buyi...@gmail.com, pe...@perex.cz, ti...@suse.com, rob...@kernel.org, ga...@codeaurora.org, alexande...@amd.com, CARLOS....@synopsys.com
Hi Jose,
You can write the Kconfig dependency as

depends on DRM_I2C_ADV7511
depends on SND_SOC=y || (SND_SOC && DRM_I2C_ADV7511=m)

> >>>> +
> >>>> + adv7511_set_link_config(adv7511, &link_config);
> >>>> +
> >>>> + /* Enable HDMI mode */
> >>>> + regmap_update_bits(adv7511->regmap, ADV7511_REG_HDCP_HDMI_CFG,
> >>>> + ADV7511_HDMI_CFG_MODE_MASK,
> >>>> + ADV7511_HDMI_CFG_MODE_HDMI);
> >>>> +
> >>>> + return 0;
> >>>> +
> >>>> +err_i2c_unregister_device:
> >>>> + i2c_unregister_device(adv7511->i2c_edid);
> >>>> +
> >>>> + return ret;
> >>>> +}

--
Regards,

Laurent Pinchart

Jose Abreu

unread,
Apr 5, 2016, 1:08:55 PM4/5/16
to linux-s...@lists.infradead.org, linux-...@vger.kernel.org, dri-...@lists.freedesktop.org, alsa-...@alsa-project.org, bro...@kernel.org, CARLOS....@synopsys.com, Alexey....@synopsys.com, Vineet...@synopsys.com, arc...@codeaurora.org, air...@linux.ie, lgir...@gmail.com, pe...@perex.cz, ti...@suse.com, Jose Abreu
ARC AXS10x platforms consist of a mainboard with several peripherals.
One of those peripherals is an HDMI output port controlled by the ADV7511
transmitter.

This patch set adds audio for the ADV7511 transmitter and I2S audio for
the AXS10x platform.

Changes v2 -> v3:
* Removed pll_config functions (as suggested by Alexey Brodkin)
* Removed HDMI start at adv7511_core (as suggested by Archit Taneja)
* Use NOP functions for adv7511_audio (as suggested by Archit Taneja)
* Added adv7511_audio_exit() function (as suggested by Archit Taneja)
* Moved adv7511 to its own folder (as suggested by Archit Taneja)
* Separated file rename of adv7511_core (as suggested by Emil Velikov)
* Compile adv7511 as module if ALSA SoC is compiled as module
* Load adv7511 audio only if declared in device tree (as suggested by Laurent Pinchart)
* Dropped custom platform driver, using now ALSA DMA engine
* Dropped IRQ handler for I2S

Changes v1 -> v2:
* DT bindings moved to separate patch (as suggested by Alexey Brodkin)
* Removed defconfigs entries (as suggested by Alexey Brodkin)

Jose Abreu (3):
drm/i2c/adv7511: Rename and move to separate folder
drm/i2c/adv7511: Add audio support
ASoC: dwc: Unmask I2S interrupts only for enabled channels

.../bindings/display/bridge/adi,adv7511.txt | 3 +
drivers/gpu/drm/i2c/Kconfig | 6 +-
drivers/gpu/drm/i2c/Makefile | 2 +-
drivers/gpu/drm/i2c/adv7511/Kconfig | 18 ++
drivers/gpu/drm/i2c/adv7511/Makefile | 3 +
drivers/gpu/drm/i2c/{ => adv7511}/adv7511.h | 53 ++++
drivers/gpu/drm/i2c/adv7511/adv7511_audio.c | 310 +++++++++++++++++++++
.../drm/i2c/{adv7511.c => adv7511/adv7511_core.c} | 43 +--
include/sound/soc-dai.h | 1 +
sound/soc/dwc/designware_i2s.c | 5 +-
10 files changed, 406 insertions(+), 38 deletions(-)
create mode 100644 drivers/gpu/drm/i2c/adv7511/Kconfig
create mode 100644 drivers/gpu/drm/i2c/adv7511/Makefile
rename drivers/gpu/drm/i2c/{ => adv7511}/adv7511.h (90%)
create mode 100644 drivers/gpu/drm/i2c/adv7511/adv7511_audio.c
rename drivers/gpu/drm/i2c/{adv7511.c => adv7511/adv7511_core.c} (97%)

--
1.9.1


Mark Brown

unread,
Apr 5, 2016, 2:57:25 PM4/5/16
to Jose Abreu, linux-s...@lists.infradead.org, linux-...@vger.kernel.org, dri-...@lists.freedesktop.org, alsa-...@alsa-project.org, CARLOS....@synopsys.com, Alexey....@synopsys.com, Vineet...@synopsys.com, arc...@codeaurora.org, air...@linux.ie, lgir...@gmail.com, pe...@perex.cz, ti...@suse.com
On Tue, Apr 05, 2016 at 06:08:02PM +0100, Jose Abreu wrote:
> There is no need to unmask all interrupts at I2S start. This
> can cause performance issues in slower platforms.
>
> Unmask only the interrupts for the used channels.

This appears to be completely unrelated to the other two patches in the
series. Please don't mix unrelated changes in a single series, it
creates interdependencies that don't really exist - if you've got a
separate change or set of changes send them separately.
signature.asc

Jose Abreu

unread,
Apr 7, 2016, 12:54:41 PM4/7/16
to linux-s...@lists.infradead.org, linux-...@vger.kernel.org, dri-...@lists.freedesktop.org, alsa-...@alsa-project.org, bro...@kernel.org, CARLOS....@synopsys.com, Alexey....@synopsys.com, Vineet...@synopsys.com, arc...@codeaurora.org, air...@linux.ie, lgir...@gmail.com, pe...@perex.cz, ti...@suse.com, Jose Abreu
ARC AXS10x platforms consist of a mainboard with several peripherals.
One of those peripherals is an HDMI output port controlled by the ADV7511
transmitter.

This patch set adds audio for the ADV7511 transmitter and I2S audio for
the AXS10x platform.

Changes v3 -> v4:
* Reintroduced custom PCM driver (see note below)
* Use DT boolean to switch between ALSA DMA engine PCM or custom PCM
* Use fifo depth to program I2S FCR
* Update I2S documentation

Changes v2 -> v3:
* Removed pll_config functions (as suggested by Alexey Brodkin)
* Removed HDMI start at adv7511_core (as suggested by Archit Taneja)
* Use NOP functions for adv7511_audio (as suggested by Archit Taneja)
* Added adv7511_audio_exit() function (as suggested by Archit Taneja)
* Moved adv7511 to its own folder (as suggested by Archit Taneja)
* Separated file rename of adv7511_core (as suggested by Emil Velikov)
* Compile adv7511 as module if ALSA SoC is compiled as module
* Load adv7511 audio only if declared in device tree (as suggested by Laurent Pinchart)
* Dropped custom platform driver, using now ALSA DMA engine
* Dropped IRQ handler for I2S

Changes v1 -> v2:
* DT bindings moved to separate patch (as suggested by Alexey Brodkin)
* Removed defconfigs entries (as suggested by Alexey Brodkin)


NOTE:
Although the mainline I2S driver uses ALSA DMA engine,
this controller can be built without DMA support so it
was necessary to add this custom platform driver so that
HDMI audio works in AXS boards.

Jose Abreu (5):
drm/i2c/adv7511: Rename and move to separate folder
drm/i2c/adv7511: Add audio support
ASoC: dwc: Use fifo depth to program FCR
ASoC: dwc: Add custom PCM driver
ASoC: dwc: Update DOCUMENTATION for I2S Driver

.../bindings/display/bridge/adi,adv7511.txt | 3 +
.../devicetree/bindings/sound/designware-i2s.txt | 5 +
drivers/gpu/drm/i2c/Kconfig | 6 +-
drivers/gpu/drm/i2c/Makefile | 2 +-
drivers/gpu/drm/i2c/adv7511/Kconfig | 18 ++
drivers/gpu/drm/i2c/adv7511/Makefile | 3 +
drivers/gpu/drm/i2c/{ => adv7511}/adv7511.h | 53 ++++
drivers/gpu/drm/i2c/adv7511/adv7511_audio.c | 310 +++++++++++++++++++++
.../drm/i2c/{adv7511.c => adv7511/adv7511_core.c} | 43 +--
include/sound/soc-dai.h | 1 +
sound/soc/dwc/Kconfig | 9 +
sound/soc/dwc/Makefile | 1 +
sound/soc/dwc/designware.h | 70 +++++
sound/soc/dwc/designware_i2s.c | 106 +++++--
sound/soc/dwc/designware_pcm.c | 230 +++++++++++++++
15 files changed, 796 insertions(+), 64 deletions(-)
create mode 100644 drivers/gpu/drm/i2c/adv7511/Kconfig
create mode 100644 drivers/gpu/drm/i2c/adv7511/Makefile
rename drivers/gpu/drm/i2c/{ => adv7511}/adv7511.h (90%)
create mode 100644 drivers/gpu/drm/i2c/adv7511/adv7511_audio.c
rename drivers/gpu/drm/i2c/{adv7511.c => adv7511/adv7511_core.c} (97%)
create mode 100644 sound/soc/dwc/designware.h
create mode 100644 sound/soc/dwc/designware_pcm.c

--
1.9.1


Mark Brown

unread,
Apr 7, 2016, 1:53:48 PM4/7/16
to Jose Abreu, linux-s...@lists.infradead.org, linux-...@vger.kernel.org, dri-...@lists.freedesktop.org, alsa-...@alsa-project.org, CARLOS....@synopsys.com, Alexey....@synopsys.com, Vineet...@synopsys.com, arc...@codeaurora.org, air...@linux.ie, lgir...@gmail.com, pe...@perex.cz, ti...@suse.com
On Thu, Apr 07, 2016 at 05:53:59PM +0100, Jose Abreu wrote:

> + Optional properties:
> + - snps,use-dmaengine: If set the driver will use ALSA DMA engine. If set
> + it is required to use the properties 'dmas' and 'dma-names'.

This is not a good interface, it's describing Linux internal APIs. If
the device needs to operate in PIO mode it should just do that.
signature.asc

Jose Abreu

unread,
Apr 8, 2016, 6:07:27 AM4/8/16
to Mark Brown, Jose Abreu, linux-s...@lists.infradead.org, linux-...@vger.kernel.org, dri-...@lists.freedesktop.org, alsa-...@alsa-project.org, CARLOS....@synopsys.com, Alexey....@synopsys.com, Vineet...@synopsys.com, arc...@codeaurora.org, air...@linux.ie, lgir...@gmail.com, pe...@perex.cz, ti...@suse.com
Hi Mark,
I added this interface because there is no direct way to check if DMA is
available on the I2S controller so it is not possible to automatically change
between DMA and PIO mode. As the I2S controller can be built with or without DMA
support it is necessary to somehow check if DMA is enabled or not and according
to that use either ALSA DMA engine or the custom platform driver sent in these
patches. I did not want to remove drivers functionality so I added this property
to the DT. This way a user can select between DMA and PIO mode.

Is there a better option to do this without removing the possibility of using
ALSA DMA engine in the driver?

Lars-Peter Clausen

unread,
Apr 8, 2016, 11:46:34 AM4/8/16
to Jose Abreu, linux-s...@lists.infradead.org, linux-...@vger.kernel.org, dri-...@lists.freedesktop.org, alsa-...@alsa-project.org, arc...@codeaurora.org, air...@linux.ie, Vineet...@synopsys.com, Alexey....@synopsys.com, CARLOS....@synopsys.com, lgir...@gmail.com, bro...@kernel.org, ti...@suse.com
On 04/07/2016 06:53 PM, Jose Abreu wrote:
> This patch adds audio support for the ADV7511 HDMI transmitter
> using ALSA SoC.
>
> The code was ported from Analog Devices linux tree from
> commit 1770c4a1e32b ("Merge remote-tracking branch
> 'xilinx/master' into xcomm_zynq"), which is available at:
> - https://github.com/analogdevicesinc/linux/

The reason why this driver is still out of tree, is because there has been
no conclusion yet on how to go forward with the whole HDMI integration. So
this is not going to get merged until that issue has been addressed.

[...]
> +- adi,enable-audio: If set the ADV7511 driver will register a codec interface
> + into ALSA SoC.

This is not a description of the hardware.

[...]
> diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h
> index 964b7de..539c091 100644
> --- a/include/sound/soc-dai.h
> +++ b/include/sound/soc-dai.h
> @@ -33,6 +33,7 @@ struct snd_compr_stream;
> #define SND_SOC_DAIFMT_DSP_B 5 /* L data MSB during FRM LRC */
> #define SND_SOC_DAIFMT_AC97 6 /* AC97 */
> #define SND_SOC_DAIFMT_PDM 7 /* Pulse density modulation */
> +#define SND_SOC_DAIFMT_SPDIF 8 /* SPDIF */

This needs to be a separate patch.

Lars-Peter Clausen

unread,
Apr 8, 2016, 11:52:33 AM4/8/16
to Jose Abreu, Mark Brown, alsa-...@alsa-project.org, lgir...@gmail.com, air...@linux.ie, Vineet...@synopsys.com, Alexey....@synopsys.com, linux-...@vger.kernel.org, dri-...@lists.freedesktop.org, CARLOS....@synopsys.com, ti...@suse.com, linux-s...@lists.infradead.org, arc...@codeaurora.org
On 04/08/2016 12:06 PM, Jose Abreu wrote:
> Hi Mark,
>
>
> On 07-04-2016 18:53, Mark Brown wrote:
>> On Thu, Apr 07, 2016 at 05:53:59PM +0100, Jose Abreu wrote:
>>
>>> + Optional properties:
>>> + - snps,use-dmaengine: If set the driver will use ALSA DMA engine. If set
>>> + it is required to use the properties 'dmas' and 'dma-names'.
>> This is not a good interface, it's describing Linux internal APIs. If
>> the device needs to operate in PIO mode it should just do that.
>
> I added this interface because there is no direct way to check if DMA is
> available on the I2S controller so it is not possible to automatically change
> between DMA and PIO mode. As the I2S controller can be built with or without DMA
> support it is necessary to somehow check if DMA is enabled or not and according
> to that use either ALSA DMA engine or the custom platform driver sent in these
> patches. I did not want to remove drivers functionality so I added this property
> to the DT. This way a user can select between DMA and PIO mode.

That's OK, but you need to describe the hardware, not the indented behavior
of the software driver.

Jose Abreu

unread,
Apr 8, 2016, 12:09:05 PM4/8/16
to Lars-Peter Clausen, Jose Abreu, Mark Brown, alsa-...@alsa-project.org, lgir...@gmail.com, air...@linux.ie, Vineet...@synopsys.com, Alexey....@synopsys.com, linux-...@vger.kernel.org, dri-...@lists.freedesktop.org, CARLOS....@synopsys.com, ti...@suse.com, linux-s...@lists.infradead.org, arc...@codeaurora.org
Hi Lars,
Is this okay: "snps,use-dmaengine: Set this boolean paramater if I2S controller
has DMA support. If set the properties 'dmas' and 'dma-names' must be also set" ?

Jose Abreu

unread,
Apr 8, 2016, 12:13:54 PM4/8/16
to Lars-Peter Clausen, Jose Abreu, linux-s...@lists.infradead.org, linux-...@vger.kernel.org, dri-...@lists.freedesktop.org, alsa-...@alsa-project.org, arc...@codeaurora.org, air...@linux.ie, Vineet...@synopsys.com, Alexey....@synopsys.com, lgir...@gmail.com, CARLOS....@synopsys.com, bro...@kernel.org, ti...@suse.com
Hi Lars,


On 08-04-2016 16:46, Lars-Peter Clausen wrote:
> On 04/07/2016 06:53 PM, Jose Abreu wrote:
>> This patch adds audio support for the ADV7511 HDMI transmitter
>> using ALSA SoC.
>>
>> The code was ported from Analog Devices linux tree from
>> commit 1770c4a1e32b ("Merge remote-tracking branch
>> 'xilinx/master' into xcomm_zynq"), which is available at:
>> - https://github.com/analogdevicesinc/linux/
> The reason why this driver is still out of tree, is because there has been
> no conclusion yet on how to go forward with the whole HDMI integration. So
> this is not going to get merged until that issue has been addressed.

Ok, will then drop the patches related with adv7511 but will update with your
comments so that when this HDMI issue is addressed I can send again the patches.
Is this okay?

>
> [...]
>> +- adi,enable-audio: If set the ADV7511 driver will register a codec interface
>> + into ALSA SoC.
> This is not a description of the hardware.

Is this okay: "adi,enable-audio: Set this boolean parameter if ADV7511
transmitter routes audio signals" ?

>
> [...]
>> diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h
>> index 964b7de..539c091 100644
>> --- a/include/sound/soc-dai.h
>> +++ b/include/sound/soc-dai.h
>> @@ -33,6 +33,7 @@ struct snd_compr_stream;
>> #define SND_SOC_DAIFMT_DSP_B 5 /* L data MSB during FRM LRC */
>> #define SND_SOC_DAIFMT_AC97 6 /* AC97 */
>> #define SND_SOC_DAIFMT_PDM 7 /* Pulse density modulation */
>> +#define SND_SOC_DAIFMT_SPDIF 8 /* SPDIF */
> This needs to be a separate patch.

Ok.

>
>
> _______________________________________________
> linux-snps-arc mailing list
> linux-s...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Lars-Peter Clausen

unread,
Apr 9, 2016, 10:55:54 AM4/9/16
to Jose Abreu, Mark Brown, alsa-...@alsa-project.org, lgir...@gmail.com, air...@linux.ie, Vineet...@synopsys.com, Alexey....@synopsys.com, linux-...@vger.kernel.org, dri-...@lists.freedesktop.org, CARLOS....@synopsys.com, ti...@suse.com, linux-s...@lists.infradead.org, arc...@codeaurora.org
The description is better. But the name of the property is still imperative
rather then descriptive. It tells the software what should be done rather
then describing what the hardware looks like.

Since there is already the dmas property which is present if a DMA is
connected and is absent when no DMA is present it should be enough to just
check that property rather than requiring an additional one.

Lars-Peter Clausen

unread,
Apr 9, 2016, 11:03:06 AM4/9/16
to Jose Abreu, linux-s...@lists.infradead.org, linux-...@vger.kernel.org, dri-...@lists.freedesktop.org, alsa-...@alsa-project.org, arc...@codeaurora.org, air...@linux.ie, Vineet...@synopsys.com, Alexey....@synopsys.com, lgir...@gmail.com, CARLOS....@synopsys.com, bro...@kernel.org, ti...@suse.com
On 04/08/2016 06:12 PM, Jose Abreu wrote:
[...]
>>
>> [...]
>>> +- adi,enable-audio: If set the ADV7511 driver will register a codec interface
>>> + into ALSA SoC.
>> This is not a description of the hardware.
>
> Is this okay: "adi,enable-audio: Set this boolean parameter if ADV7511
> transmitter routes audio signals" ?

I don't think we need this property. There is no problem with registering
the audio part unconditionally. As long as there is no connection we wont
create a sound card that is exposed to userspace.

Jose Abreu

unread,
Apr 11, 2016, 5:25:43 AM4/11/16
to Lars-Peter Clausen, Jose Abreu, Mark Brown, alsa-...@alsa-project.org, Vineet...@synopsys.com, Alexey....@synopsys.com, lgir...@gmail.com, dri-...@lists.freedesktop.org, linux-...@vger.kernel.org, CARLOS....@synopsys.com, ti...@suse.com, linux-s...@lists.infradead.org
Hi Lars,
Ok, will then use the DMA property to decide which mode to use: PIO or DMA.

>
> _______________________________________________
> dri-devel mailing list
> dri-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Jose Abreu

unread,
Apr 11, 2016, 5:27:32 AM4/11/16
to Lars-Peter Clausen, Jose Abreu, linux-s...@lists.infradead.org, linux-...@vger.kernel.org, dri-...@lists.freedesktop.org, alsa-...@alsa-project.org, arc...@codeaurora.org, air...@linux.ie, Vineet...@synopsys.com, Alexey....@synopsys.com, lgir...@gmail.com, CARLOS....@synopsys.com, bro...@kernel.org, ti...@suse.com
Hi Lars,
This change was suggested by Laurent Pinchart and was introduced in v3. Quoting
Laurent:
"The idea is that enabling support for ADV7511 audio in the kernel isn't coupled
with whether the system includes audio support. It would be confusing, and would
also waste resources, to create a Linux sound device when no sound channel is
routed on the board."

Should I revert this?

Lars-Peter Clausen

unread,
Apr 11, 2016, 5:34:00 AM4/11/16
to Jose Abreu, linux-s...@lists.infradead.org, linux-...@vger.kernel.org, dri-...@lists.freedesktop.org, alsa-...@alsa-project.org, arc...@codeaurora.org, air...@linux.ie, Vineet...@synopsys.com, Alexey....@synopsys.com, lgir...@gmail.com, CARLOS....@synopsys.com, bro...@kernel.org, ti...@suse.com
I wouldn't care too much about this at this point, the extra amount of
resources required for registering the CODEC (but not the sound card) is
just a few bytes (sizeof(struct snd_soc_codec)).

Nevertheless what we should do is describe the hardware and from this
information infer whether there is a audio connection or not and if there is
none we might skip registering the CODEC. In my opinion this hardware
description should be modeled using of-graph, having a connection between
the SoC side and the adv7511 SPDIF or I2S port.

Jose Abreu

unread,
Apr 11, 2016, 7:33:07 AM4/11/16
to Lars-Peter Clausen, Jose Abreu, linux-s...@lists.infradead.org, linux-...@vger.kernel.org, dri-...@lists.freedesktop.org, alsa-...@alsa-project.org, arc...@codeaurora.org, air...@linux.ie, Vineet...@synopsys.com, Alexey....@synopsys.com, lgir...@gmail.com, CARLOS....@synopsys.com, bro...@kernel.org, ti...@suse.com
Hi Lars,
You mean something like this:

sound_playback: sound_playback {
compatible = "simple-audio-card";
[...]
simple-audio-card,format = "i2s";
[...]
}

adv7511@xx {
compatible = "adi,adv7511";
[...]

ports {
[...]
/* Audio Output */
port@x {
reg = <x>;
endpoint {
remote-endpoint = <&sound_playback>;

Lars-Peter Clausen

unread,
Apr 11, 2016, 8:24:16 AM4/11/16
to Jose Abreu, linux-s...@lists.infradead.org, linux-...@vger.kernel.org, dri-...@lists.freedesktop.org, alsa-...@alsa-project.org, arc...@codeaurora.org, air...@linux.ie, Vineet...@synopsys.com, Alexey....@synopsys.com, lgir...@gmail.com, CARLOS....@synopsys.com, bro...@kernel.org, ti...@suse.com
Yes, something like that. Not exactly like that, but similar. One of the
core concepts of of-graph is that there is always a description of the
connection from both sides, this way each side can independently figure out
where it is connected.

Currently there is also zero support of of-graph in ASoC, so a bit of work
is required to get this integrated properly.

Jose Abreu

unread,
Apr 11, 2016, 10:09:18 AM4/11/16
to Lars-Peter Clausen, Jose Abreu, linux-s...@lists.infradead.org, linux-...@vger.kernel.org, dri-...@lists.freedesktop.org, alsa-...@alsa-project.org, arc...@codeaurora.org, air...@linux.ie, Vineet...@synopsys.com, Alexey....@synopsys.com, lgir...@gmail.com, CARLOS....@synopsys.com, bro...@kernel.org, ti...@suse.com
Hi Lars,
I also believe this would be the better option but in the meantime can't I
integrate the audio like it is being done in the dw-hdmi driver[1]? In this
driver the audio is registered as a sound card and is conditionally built using
Kconfig, just like I was doing in the previous versions. I know you said the
HDMI audio is still an open issue but it seems that for this case it was accepted.

[1] http://lxr.free-electrons.com/source/drivers/gpu/drm/bridge/dw-hdmi-ahb-audio.c

Lars-Peter Clausen

unread,
Apr 11, 2016, 3:34:41 PM4/11/16
to Jose Abreu, linux-s...@lists.infradead.org, linux-...@vger.kernel.org, dri-...@lists.freedesktop.org, alsa-...@alsa-project.org, arc...@codeaurora.org, air...@linux.ie, Vineet...@synopsys.com, Alexey....@synopsys.com, lgir...@gmail.com, CARLOS....@synopsys.com, bro...@kernel.org, ti...@suse.com
On 04/11/2016 04:08 PM, Jose Abreu wrote:
[...]
>> Currently there is also zero support of of-graph in ASoC, so a bit of work
>> is required to get this integrated properly.
>>
>
> I also believe this would be the better option but in the meantime can't I
> integrate the audio like it is being done in the dw-hdmi driver[1]? In this
> driver the audio is registered as a sound card and is conditionally built using
> Kconfig, just like I was doing in the previous versions. I know you said the
> HDMI audio is still an open issue but it seems that for this case it was accepted.
>
> [1] http://lxr.free-electrons.com/source/drivers/gpu/drm/bridge/dw-hdmi-ahb-audio.c

Maybe, but I'm not sure it would work in this case. Resources are probably
better invested in working towards a proper solution.

Jose Abreu

unread,
Apr 12, 2016, 8:57:00 AM4/12/16
to linux-s...@lists.infradead.org, alsa-...@alsa-project.org, linux-...@vger.kernel.org, devic...@vger.kernel.org, lgir...@gmail.com, bro...@kernel.org, pe...@perex.cz, ti...@suse.com, rob...@kernel.org, CARLOS....@synopsys.com, la...@metafoo.de, Jose Abreu
Hi all,

This is v5 of these patches. In this version I dropped the ADV7511 audio patch
because, quoting Lars-Peter Clausen:
"The reason why this driver is still out of tree, is because there has been no
conclusion yet on how to go forward with the whole HDMI integration. So this
is not going to get merged until that issue has been addressed."

So, until that issue is addressed I will not send any ADV7511 audio related
patches but if for some reason anyone wants to merge it I can send the
pathes again. V4, which includes the ADV7511 audio patches is available at:
http://mailman.alsa-project.org/pipermail/alsa-devel/2016-April/106650.html

Looking forward to your comments!

Best regards,
Jose Miguel Abreu

-----

ARC AXS10x platforms consist of a mainboard with several peripherals.
One of those peripherals is an HDMI output port controlled by the ADV7511
transmitter.

This patch set adds I2S audio for the AXS10x platform.

Changes v4 -> v5
* Resolve undefined references when compiling as module
* Dropped adv7511 audio patches
* Use DMA properties in I2S to check which mode to use: PIO or DMA (as suggested by Lars-Peter Clausen)

Changes v3 -> v4:
* Reintroduced custom PCM driver (see note below)
* Use DT boolean to switch between ALSA DMA engine PCM or custom PCM
* Use fifo depth to program I2S FCR
* Update I2S documentation

Changes v2 -> v3:
* Removed pll_config functions (as suggested by Alexey Brodkin)
* Removed HDMI start at adv7511_core (as suggested by Archit Taneja)
* Use NOP functions for adv7511_audio (as suggested by Archit Taneja)
* Added adv7511_audio_exit() function (as suggested by Archit Taneja)
* Moved adv7511 to its own folder (as suggested by Archit Taneja)
* Separated file rename of adv7511_core (as suggested by Emil Velikov)
* Compile adv7511 as module if ALSA SoC is compiled as module
* Load adv7511 audio only if declared in device tree (as suggested by Laurent Pinchart)
* Dropped custom platform driver, using now ALSA DMA engine
* Dropped IRQ handler for I2S

Changes v1 -> v2:
* DT bindings moved to separate patch (as suggested by Alexey Brodkin)
* Removed defconfigs entries (as suggested by Alexey Brodkin)


NOTE:
Although the mainline I2S driver uses ALSA DMA engine,
this controller can be built without DMA support so it
was necessary to add this custom platform driver so that
HDMI audio works in AXS boards.

Jose Abreu (2):
ASoC: dwc: Add custom PCM driver
ASoC: dwc: Update DOCUMENTATION for I2S Driver

.../devicetree/bindings/sound/designware-i2s.txt | 9 +-
sound/soc/dwc/Kconfig | 9 +
sound/soc/dwc/Makefile | 1 +
sound/soc/dwc/designware.h | 71 +++++++
sound/soc/dwc/designware_i2s.c | 94 ++++++---
sound/soc/dwc/designware_pcm.c | 230 +++++++++++++++++++++
6 files changed, 385 insertions(+), 29 deletions(-)

Rob Herring

unread,
Apr 13, 2016, 4:17:22 PM4/13/16
to Jose Abreu, linux-s...@lists.infradead.org, alsa-...@alsa-project.org, linux-...@vger.kernel.org, devic...@vger.kernel.org, lgir...@gmail.com, bro...@kernel.org, pe...@perex.cz, ti...@suse.com, CARLOS....@synopsys.com, la...@metafoo.de
On Tue, Apr 12, 2016 at 01:56:28PM +0100, Jose Abreu wrote:
> This patch updates documentation for the Designware I2S
> driver.
>
> Signed-off-by: Jose Abreu <joa...@synopsys.com>
> ---
>
> Changes v4 -> v5:
> * interrupts is now required property
> * Drop 'snps-use-dmaengine' property
>
> This patch was only introduced in v4.
>
> Documentation/devicetree/bindings/sound/designware-i2s.txt | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)

Acked-by: Rob Herring <ro...@kernel.org>
0 new messages