Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1446742

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

From John Stultz <john.stultz@linaro.org>
Newsgroups linux.kernel
Subject Re: [RFC][PATCH 6/7] ASoC: hisilicon: Add hi6210 i2s audio driver for hdmi audio
Date 2016-07-20 00:00 +0200
Message-ID <rWLdv-Zu-5@gated-at.bofh.it> (permalink)
References <rVnmV-6qg-3@gated-at.bofh.it> <rVnmV-6qg-1@gated-at.bofh.it> <rVwgx-3iP-9@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Sat, Jul 16, 2016 at 4:44 AM, Mark Brown <broonie@kernel.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

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[RFC][PATCH 0/7] Add HDMI audio support for HiKey John Stultz <john.stultz@linaro.org> - 2016-07-16 04:20 +0200
  [RFC][PATCH 2/7] k3dma: Fix dma err offsets John Stultz <john.stultz@linaro.org> - 2016-07-16 04:20 +0200
    Re: [RFC][PATCH 2/7] k3dma: Fix dma err offsets zhangfei <zhangfei.gao@linaro.org> - 2016-07-18 08:50 +0200
  [RFC][PATCH 6/7] ASoC: hisilicon: Add hi6210 i2s audio driver for hdmi audio John Stultz <john.stultz@linaro.org> - 2016-07-16 04:20 +0200
    Re: [RFC][PATCH 6/7] ASoC: hisilicon: Add hi6210 i2s audio driver  for hdmi audio Mark Brown <broonie@kernel.org> - 2016-07-16 13:50 +0200
      Re: [RFC][PATCH 6/7] ASoC: hisilicon: Add hi6210 i2s audio driver for  hdmi audio John Stultz <john.stultz@linaro.org> - 2016-07-20 00:00 +0200
        Re: [RFC][PATCH 6/7] ASoC: hisilicon: Add hi6210 i2s audio driver  for hdmi audio Mark Brown <broonie@kernel.org> - 2016-07-20 02:30 +0200
  [RFC][PATCH 1/7] k3dma: Fix hisi burst clipping John Stultz <john.stultz@linaro.org> - 2016-07-16 04:20 +0200
    Re: [RFC][PATCH 1/7] k3dma: Fix hisi burst clipping zhangfei <zhangfei.gao@linaro.org> - 2016-07-18 08:40 +0200
  Re: [RFC][PATCH 0/7] Add HDMI audio support for HiKey Andy Green <andy@warmcat.com> - 2016-07-16 05:30 +0200
    Re: [RFC][PATCH 0/7] Add HDMI audio support for HiKey John Stultz <john.stultz@linaro.org> - 2016-07-16 05:40 +0200
      Re: [RFC][PATCH 0/7] Add HDMI audio support for HiKey Mark Brown <broonie@kernel.org> - 2016-07-16 13:20 +0200

csiph-web