linux-kernel - Re: [PATCH 1/2 v3] drm/bridge: adv7511: Add Audio support.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CALAqxLXHJmWB8DsR=1NgnZXD6-A=Uwwq06DTUDOLpS2yEHFb9Q@mail.gmail.com>
Date:   Fri, 16 Sep 2016 16:08:26 -0700
From:   John Stultz <john.stultz@...aro.org>
To:     Lars-Peter Clausen <lars@...afoo.de>
Cc:     lkml <linux-kernel@...r.kernel.org>,
        David Airlie <airlied@...ux.ie>,
        Archit Taneja <architt@...eaurora.org>,
        Laurent Pinchart <laurent.pinchart@...asonboard.com>,
        Wolfram Sang <wsa+renesas@...g-engineering.com>,
        Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
        Ville Syrjälä <ville.syrjala@...ux.intel.com>,
        Boris Brezillon <boris.brezillon@...e-electrons.com>,
        Andy Green <andy@...mcat.com>,
        Dave Long <dave.long@...aro.org>,
        Guodong Xu <guodong.xu@...aro.org>,
        Zhangfei Gao <zhangfei.gao@...aro.org>,
        Mark Brown <broonie@...nel.org>,
        Jose Abreu <joabreu@...opsys.com>,
        "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>
Subject: Re: [PATCH 1/2 v3] drm/bridge: adv7511: Add Audio support.

On Fri, Sep 16, 2016 at 3:51 AM, Lars-Peter Clausen <lars@...afoo.de> wrote:
> On 09/07/2016 01:22 AM, John Stultz wrote:
>> This patch adds support to Audio for both adv7511 and adv7533
>> bridge chips.
>>
>> This patch was originally from [1] by Lars-Peter Clausen <lars@...afoo.de>
>> and was adapted by Archit Taneja <architt@...eaurora.org> and
>> Srinivas Kandagatla <srinivas.kandagatla@...aro.org>.
>>
>> Then I heavily reworked it to use the hdmi-codec driver. And also
>> folded in some audio packet initialization done by Andy Green
>> <andy.green@...aro.org>. So credit to them, but blame to me.
>>
>> [1] https://github.com/analogdevicesinc/linux/blob/xcomm_zynq/drivers/gpu/drm/i2c/adv7511_audio.c
>>
>> Cc: David Airlie <airlied@...ux.ie>
>> Cc: Archit Taneja <architt@...eaurora.org>
>> Cc: Laurent Pinchart <laurent.pinchart@...asonboard.com>
>> Cc: Wolfram Sang <wsa+renesas@...g-engineering.com>
>> Cc: Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
>> Cc: "Ville Syrjälä" <ville.syrjala@...ux.intel.com>
>> Cc: Boris Brezillon <boris.brezillon@...e-electrons.com>
>> Cc: Andy Green <andy@...mcat.com>
>> Cc: Dave Long <dave.long@...aro.org>
>> Cc: Guodong Xu <guodong.xu@...aro.org>
>> Cc: Zhangfei Gao <zhangfei.gao@...aro.org>
>> Cc: Mark Brown <broonie@...nel.org>
>> Cc: Lars-Peter Clausen <lars@...afoo.de>
>> Cc: Jose Abreu <joabreu@...opsys.com>
>> Cc: dri-devel@...ts.freedesktop.org
>> Signed-off-by: John Stultz <john.stultz@...aro.org>
>
> I'm not to fond of the hdmi-codec stuff, but within its context this looks
> ok. Thanks for getting this upstream ready. Just one tiny bit regarding the
> Kconfig entry.
>
> If that is fixed feel free to add on the next revision:
>
> Acked-by: Lars-Peter Clausen <lars@...afoo.de>
>
>> ---
>> v3:
>> * Allowed audio support to be configured in or out, as suggested by Laurent
>> * Minor cleanups suggested by Laurent
>> * Folded in Andy's audio packet initialization patch, as suggested by Archit
>>
>>  drivers/gpu/drm/bridge/adv7511/Kconfig         |   9 ++
>>  drivers/gpu/drm/bridge/adv7511/Makefile        |   1 +
>>  drivers/gpu/drm/bridge/adv7511/adv7511.h       |  16 ++
>>  drivers/gpu/drm/bridge/adv7511/adv7511_audio.c | 213 +++++++++++++++++++++++++
>>  drivers/gpu/drm/bridge/adv7511/adv7511_drv.c   |   4 +
>>  5 files changed, 243 insertions(+)
>>  create mode 100644 drivers/gpu/drm/bridge/adv7511/adv7511_audio.c
>>
>> diff --git a/drivers/gpu/drm/bridge/adv7511/Kconfig b/drivers/gpu/drm/bridge/adv7511/Kconfig
>> index d2b0499..b303ad1 100644
>> --- a/drivers/gpu/drm/bridge/adv7511/Kconfig
>> +++ b/drivers/gpu/drm/bridge/adv7511/Kconfig
>> @@ -3,9 +3,18 @@ config DRM_I2C_ADV7511
>>       depends on OF
>>       select DRM_KMS_HELPER
>>       select REGMAP_I2C
>> +     select SND_SOC_HDMI_CODEC if SND_SOC
>
> This seems to be redundant considering the select done by DRM_I2C_ADV7511_AUDIO.
>
>>       help
>>         Support for the Analog Device ADV7511(W) and ADV7513 HDMI encoders.
>>
>> +config DRM_I2C_ADV7511_AUDIO
>> +     tristate "ADV7511 HDMI Audio driver"
>
> This should be bool. It either gets built into the ADV7511 driver (which
> itself could either be a module or built-in) or not. But setting this option
> itself to module wont work.

Ah! Thanks for the catch here!

I really appreciate the feedback! I'll integrate those changes and
respin the patch here shortly!

thanks
-john