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


Groups > linux.kernel > #1495419

Re: [PATCH 06/14] ASoC: Add sun8i digital audio codec

From Mark Brown <broonie@kernel.org>
Newsgroups linux.kernel
Subject Re: [PATCH 06/14] ASoC: Add sun8i digital audio codec
Date 2016-10-04 15:10 +0200
Message-ID <soxDP-38J-5@gated-at.bofh.it> (permalink)
References <souwh-SP-1@gated-at.bofh.it> <souFY-W7-21@gated-at.bofh.it> <soxkt-2M1-9@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


[Multipart message — attachments visible in raw view] - view raw

On Tue, Oct 04, 2016 at 02:40:08PM +0200, Thomas Petazzoni wrote:

> > +/* CODEC_OFFSET represents the offset of the codec registers
> > + * and not all the DAI registers
> > + */

> This is not the proper comment style I believe for audio code, it
> should be:

> /*
>  * ...
>  */

I don't care, IIRC that's something from CodingStyle which checkpatch
moans about.

> > +	/* pcm operations */
> > +	.ops = &sun8i_codec_dai_ops,
> > +};
> > +EXPORT_SYMBOL(sun8i_codec_dai);

> This EXPORT_SYMBOL looks wrong. First because it doesn't seem to be
> used outside of this module. And second because using EXPORT_SYMBOL on
> a function defined as static doesn't make much sense, as the "static"
> qualifier limits the visibility of the symbol to the current
> compilation unit.

Also all the ASoC code is _GPL so a non-GPL export is an issue.

> > +	.component_driver = {
> > +		.dapm_widgets		= sun8i_codec_dapm_widgets,
> > +		.num_dapm_widgets	= ARRAY_SIZE(sun8i_codec_dapm_widgets),
> > +		.dapm_routes		= sun8i_codec_dapm_routes,
> > +		.num_dapm_routes	= ARRAY_SIZE(sun8i_codec_dapm_routes),

> I'm probably missing something, but in the sun4i-codec.c driver, those
> fields are initialized directly in the snd_soc_codec_driver structure,
> not in the .component_driver sub-structure.

We're in the process of pushing everything out to component level, this
update should be made in the old code if it's not happened already.

> > +	if (clk_prepare_enable(scodec->clk_module))
> > +		pr_err("err:open failed;\n");

> Grr, pr_err, not good. Plus you want to return with an error from the
> probe() function.

Also when printing an error message use dev_err().

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


Thread

[PATCH 00/14] ASoc: sunxi: Add Allwinner A33 codec driver Mylène Josserand          <mylene.josserand@free-electrons.com> - 2016-10-04 11:50 +0200
  [PATCH 08/14] dt-bindings: sound: Add sun8i analog codec documentation Mylène Josserand          <mylene.josserand@free-electrons.com> - 2016-10-04 11:50 +0200
    Re: [PATCH 08/14] dt-bindings: sound: Add sun8i analog codec  documentation Mark Brown <broonie@kernel.org> - 2016-10-04 12:40 +0200
    Re: [PATCH 08/14] dt-bindings: sound: Add sun8i analog codec  documentation Maxime Ripard <maxime.ripard@free-electrons.com> - 2016-10-04 18:30 +0200
      Re: [PATCH 08/14] dt-bindings: sound: Add sun8i analog codec documentation Chen-Yu Tsai <wens@csie.org> - 2016-10-05 05:10 +0200
        Re: [PATCH 08/14] dt-bindings: sound: Add sun8i analog codec  documentation Mylene Josserand <mylene.josserand@free-electrons.com> - 2016-10-05 14:10 +0200
  [PATCH 11/14] ARM: dts: sun8i: Add analog codec on prcm node Mylène Josserand          <mylene.josserand@free-electrons.com> - 2016-10-04 11:50 +0200
  [PATCH 01/14] dma: sun6i-dma: Add burst case of 4 Mylène Josserand          <mylene.josserand@free-electrons.com> - 2016-10-04 11:50 +0200
    Re: [PATCH 01/14] dma: sun6i-dma: Add burst case of 4 Jean-Francois Moine <moinejf@free.fr> - 2016-10-04 12:50 +0200
      Re: [PATCH 01/14] dma: sun6i-dma: Add burst case of 4 Thomas Petazzoni <thomas.petazzoni@free-electrons.com> - 2016-10-04 14:20 +0200
        Re: [PATCH 01/14] dma: sun6i-dma: Add burst case of 4 Jean-Francois Moine <moinejf@free.fr> - 2016-10-04 15:50 +0200
          Re: [PATCH 01/14] dma: sun6i-dma: Add burst case of 4 Vinod Koul <vinod.koul@intel.com> - 2016-10-04 17:40 +0200
      Re: [PATCH 01/14] dma: sun6i-dma: Add burst case of 4 Maxime Ripard <maxime.ripard@free-electrons.com> - 2016-10-04 19:00 +0200
  [PATCH 13/14] ARM: dts: sun8i: parrot: Enable audio nodes Mylène Josserand          <mylene.josserand@free-electrons.com> - 2016-10-04 11:50 +0200
  [PATCH 10/14] dt-bindings: sound: Add sun8i audio card documentation Mylène Josserand          <mylene.josserand@free-electrons.com> - 2016-10-04 11:50 +0200
    Re: [PATCH 10/14] dt-bindings: sound: Add sun8i audio card  documentation Maxime Ripard <maxime.ripard@free-electrons.com> - 2016-10-04 18:40 +0200
  [PATCH 12/14] ARM: dts: sun8i: Add audio codec, dai and card for A33 Mylène Josserand          <mylene.josserand@free-electrons.com> - 2016-10-04 11:50 +0200
  [PATCH 03/14] ASoC: sun4i-i2s: Add apb reset Mylène Josserand          <mylene.josserand@free-electrons.com> - 2016-10-04 11:50 +0200
    Re: [PATCH 03/14] ASoC: sun4i-i2s: Add apb reset Thomas Petazzoni <thomas.petazzoni@free-electrons.com> - 2016-10-04 14:20 +0200
      Re: [PATCH 03/14] ASoC: sun4i-i2s: Add apb reset Maxime Ripard <maxime.ripard@free-electrons.com> - 2016-10-04 17:50 +0200
        Re: [PATCH 03/14] ASoC: sun4i-i2s: Add apb reset Mylene Josserand <mylene.josserand@free-electrons.com> - 2016-10-05 11:50 +0200
    Re: [PATCH 03/14] ASoC: sun4i-i2s: Add apb reset Code Kipper <codekipper@gmail.com> - 2016-10-04 14:30 +0200
  [PATCH 02/14] clk: ccu-sun8i-a33: Add CLK_SET_RATE_PARENT to ac-dig Mylène Josserand          <mylene.josserand@free-electrons.com> - 2016-10-04 12:00 +0200
    Re: [PATCH 02/14] clk: ccu-sun8i-a33: Add CLK_SET_RATE_PARENT to  ac-dig Thomas Petazzoni <thomas.petazzoni@free-electrons.com> - 2016-10-04 14:20 +0200
      Re: [PATCH 02/14] clk: ccu-sun8i-a33: Add CLK_SET_RATE_PARENT to  ac-dig Mylene Josserand <mylene.josserand@free-electrons.com> - 2016-10-05 11:40 +0200
  [PATCH 05/14] mfd: sun6i-prcm: Add sun8i analog codec as subnode Mylène Josserand          <mylene.josserand@free-electrons.com> - 2016-10-04 12:00 +0200
    Re: [PATCH 05/14] mfd: sun6i-prcm: Add sun8i analog codec as  subnode Jean-Francois Moine <moinejf@free.fr> - 2016-10-04 13:00 +0200
  [PATCH 04/14] ASoC: Add sun8i analog codec driver Mylène Josserand          <mylene.josserand@free-electrons.com> - 2016-10-04 12:00 +0200
    Re: [PATCH 04/14] ASoC: Add sun8i analog codec driver Code Kipper <codekipper@gmail.com> - 2016-10-04 12:30 +0200
      Re: [PATCH 04/14] ASoC: Add sun8i analog codec driver Chen-Yu Tsai <wens@csie.org> - 2016-10-04 13:00 +0200
  [PATCH 07/14] ASoC: Add sun8i audio card Mylène Josserand          <mylene.josserand@free-electrons.com> - 2016-10-04 12:00 +0200
    Re: [PATCH 07/14] ASoC: Add sun8i audio card Code Kipper <codekipper@gmail.com> - 2016-10-04 12:20 +0200
      Re: [PATCH 07/14] ASoC: Add sun8i audio card Chen-Yu Tsai <wens@csie.org> - 2016-10-04 13:10 +0200
    Re: [PATCH 07/14] ASoC: Add sun8i audio card Thomas Petazzoni <thomas.petazzoni@free-electrons.com> - 2016-10-04 14:30 +0200
    Re: [PATCH 07/14] ASoC: Add sun8i audio card Code Kipper <codekipper@gmail.com> - 2016-10-05 08:10 +0200
      Re: [PATCH 07/14] ASoC: Add sun8i audio card Jean-Francois Moine <moinejf@free.fr> - 2016-10-05 12:10 +0200
  [PATCH 06/14] ASoC: Add sun8i digital audio codec Mylène Josserand          <mylene.josserand@free-electrons.com> - 2016-10-04 12:00 +0200
    Re: [PATCH 06/14] ASoC: Add sun8i digital audio codec Thomas Petazzoni <thomas.petazzoni@free-electrons.com> - 2016-10-04 14:50 +0200
      Re: [PATCH 06/14] ASoC: Add sun8i digital audio codec Mark Brown <broonie@kernel.org> - 2016-10-04 15:10 +0200
        Re: [PATCH 06/14] ASoC: Add sun8i digital audio codec Thomas Petazzoni <thomas.petazzoni@free-electrons.com> - 2016-10-04 15:20 +0200
      Re: [PATCH 06/14] ASoC: Add sun8i digital audio codec Maxime Ripard <maxime.ripard@free-electrons.com> - 2016-10-04 18:20 +0200
      Re: [PATCH 06/14] ASoC: Add sun8i digital audio codec Mylene Josserand <mylene.josserand@free-electrons.com> - 2016-10-05 14:00 +0200
    Re: [PATCH 06/14] ASoC: Add sun8i digital audio codec Maxime Ripard <maxime.ripard@free-electrons.com> - 2016-10-04 18:20 +0200
      Re: [PATCH 06/14] ASoC: Add sun8i digital audio codec Alexandre Belloni <alexandre.belloni@free-electrons.com> - 2016-10-06 20:30 +0200
  [PATCH 09/14] dt-bindings: sound: Add sun8i codec documentation Mylène Josserand          <mylene.josserand@free-electrons.com> - 2016-10-04 12:00 +0200
    Re: [PATCH 09/14] dt-bindings: sound: Add sun8i codec documentation Maxime Ripard <maxime.ripard@free-electrons.com> - 2016-10-04 18:30 +0200

csiph-web