linux-kernel - Re: [RFC][PATCH 6/7] ASoC: hisilicon: Add hi6210 i2s audio driver for hdmi audio

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] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 19 Jul 2016 14:59:31 -0700
From:	John Stultz <john.stultz@...aro.org>
To:	Mark Brown <broonie@...nel.org>
Cc:	lkml <linux-kernel@...r.kernel.org>,
	Andy Green <andy.green@...aro.org>,
	Zhangfei Gao <zhangfei.gao@...aro.org>,
	Jingoo Han <jg1.han@...sung.com>,
	Krzysztof Kozlowski <k.kozlowski@...sung.com>,
	Maxime Ripard <maxime.ripard@...e-electrons.com>,
	Vinod Koul <vinod.koul@...el.com>,
	Dan Williams <dan.j.williams@...el.com>,
	Liam Girdwood <lgirdwood@...il.com>,
	Jaroslav Kysela <perex@...ex.cz>,
	Takashi Iwai <tiwai@...e.com>, Wei Xu <xuwei5@...ilicon.com>,
	Rob Herring <robh+dt@...nel.org>,
	Andy Green <andy@...mcat.com>,
	Dave Long <dave.long@...aro.org>,
	Guodong Xu <guodong.xu@...aro.org>
Subject: Re: [RFC][PATCH 6/7] ASoC: hisilicon: Add hi6210 i2s audio driver for
 hdmi audio

On Sat, Jul 16, 2016 at 4:44 AM, Mark Brown <broonie@...nel.org> wrote:
> On Fri, Jul 15, 2016 at 07:13:26PM -0700, John Stultz wrote:
>>
>> +     .cpu_dai_name = "f7118000.hi6210_i2s",
>
> Why is this not connected up in DT - as things stand the card has zero
> reuse potential.  Though given that this has
>
>> +     .codec_name = "0.hi6210_hdmi_card",
>
> If you've got a CODEC with hdmi_card in the name that suggests there's a
> serious abstraction problem going on somewhere.

Yea. So I've been splitting up the patch and looking at how this might
be wired up via devicetree instead.

I've basically stripped the hdmi-card driver down and it really just
seems to be:

static struct snd_soc_dai_driver hi6210_hdmi_dai = {
        .name = "hi6210_hdmi_dai",
        .playback = {
                .channels_min = 2,
                .channels_max = 2,
                .rates = SNDRV_PCM_RATE_48000,
                .formats = SNDRV_PCM_FMTBIT_S16_LE,
        },
};

Then the probe/remove logic to register the codec, and connected it up
with the simple card driver.

Though it seems kind of silly to have a whole driver (and devicetree
entry for the probe hook) just to fill and install the above
structure. Is there a simpler way to specify that via a DT node
instead?


>> +static const struct of_device_id hi6210_i2s_dt_ids[] = {
>> +     { .compatible = "hisilicon,hi6210-i2s" },
>> +     { /* sentinel */ }
>> +};
>
> The code makes this look like it's not just an I2S controller.

I'm not sure I follow this? The dt ids make it seem like this driver
is not an i2s controller?

I think I've addressed the other issue you've pointed out in this
patch, so I'll likely be sending out another revision later today.

thanks
-john