All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
To: Mark Brown <broonie@kernel.org>
Cc: "Mylène Josserand" <mylene.josserand@free-electrons.com>,
	vinod.koul@intel.com, maxime.ripard@free-electrons.com,
	wens@csie.org, mturquette@baylibre.com, sboyd@codeaurora.org,
	lgirdwood@gmail.com, perex@perex.cz, tiwai@suse.com,
	lee.jones@linaro.org, mark.rutland@arm.com, robh+dt@kernel.org,
	linux-kernel@vger.kernel.org, dmaengine@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, linux-clk@vger.kernel.org,
	alsa-devel@alsa-project.org, devicetree@vger.kernel.org,
	linux-sunxi@googlegroups.com,
	alexandre.belloni@free-electrons.com
Subject: Re: [PATCH 06/14] ASoC: Add sun8i digital audio codec
Date: Tue, 4 Oct 2016 15:16:03 +0200	[thread overview]
Message-ID: <20161004151603.38aac5b2@free-electrons.com> (raw)
In-Reply-To: <20161004130727.srmaielkvd2z5k3s@sirena.org.uk>

Hello,

On Tue, 4 Oct 2016 15:07:27 +0200, Mark Brown wrote:

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

Correct. The

/* ..
 * ..
 */

style is mandatory for net/ and crypto code, but not in the rest of the
kernel.

> > 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.

OK.

> > > +	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().

That's why I said "Grr, pr_err, not good" :)

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

WARNING: multiple messages have this Message-ID (diff)
From: thomas.petazzoni@free-electrons.com (Thomas Petazzoni)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 06/14] ASoC: Add sun8i digital audio codec
Date: Tue, 4 Oct 2016 15:16:03 +0200	[thread overview]
Message-ID: <20161004151603.38aac5b2@free-electrons.com> (raw)
In-Reply-To: <20161004130727.srmaielkvd2z5k3s@sirena.org.uk>

Hello,

On Tue, 4 Oct 2016 15:07:27 +0200, Mark Brown wrote:

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

Correct. The

/* ..
 * ..
 */

style is mandatory for net/ and crypto code, but not in the rest of the
kernel.

> > 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.

OK.

> > > +	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().

That's why I said "Grr, pr_err, not good" :)

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

  reply	other threads:[~2016-10-04 13:16 UTC|newest]

Thread overview: 170+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-04  9:46 [PATCH 00/14] ASoc: sunxi: Add Allwinner A33 codec driver Mylène Josserand
2016-10-04  9:46 ` Mylène Josserand
     [not found] ` <cover.1475571575.git.mylene.josserand@free-electrons.com>
2016-10-04  9:46   ` [PATCH 01/14] dma: sun6i-dma: Add burst case of 4 Mylène Josserand
2016-10-04  9:46     ` Mylène Josserand
2016-10-04 10:40     ` Jean-Francois Moine
2016-10-04 10:40       ` Jean-Francois Moine
2016-10-04 10:40       ` Jean-Francois Moine
2016-10-04 12:12       ` Thomas Petazzoni
2016-10-04 12:12         ` Thomas Petazzoni
2016-10-04 12:12         ` Thomas Petazzoni
2016-10-04 12:12         ` Thomas Petazzoni
2016-10-04 13:46         ` Jean-Francois Moine
2016-10-04 13:46           ` Jean-Francois Moine
2016-10-04 13:46           ` Jean-Francois Moine
2016-10-04 13:46           ` Jean-Francois Moine
2016-10-04 15:40           ` Vinod Koul
2016-10-04 15:40             ` Vinod Koul
2016-10-04 15:40             ` Vinod Koul
2016-10-04 16:55       ` Maxime Ripard
2016-10-04 16:55         ` Maxime Ripard
2016-10-23 16:31         ` Jean-Francois Moine
2016-10-23 16:31           ` Jean-Francois Moine
2016-10-23 16:31           ` Jean-Francois Moine
2016-10-23 16:31           ` Jean-Francois Moine
2016-10-27 17:51           ` Maxime Ripard
2016-10-27 17:51             ` Maxime Ripard
2016-10-27 17:51             ` Maxime Ripard
2016-10-30  2:06             ` Chen-Yu Tsai
2016-10-30  2:06               ` Chen-Yu Tsai
2016-10-30  2:06               ` Chen-Yu Tsai
2016-10-30  2:06               ` Chen-Yu Tsai
2016-10-30  6:16               ` Jean-Francois Moine
2016-10-30  6:16                 ` Jean-Francois Moine
2016-10-30  6:16                 ` Jean-Francois Moine
2016-10-30  6:16                 ` Jean-Francois Moine
2016-11-01 13:46               ` Koul, Vinod
2016-11-01 13:46                 ` Koul, Vinod
2016-11-01 13:46                 ` Koul, Vinod
2016-11-01 13:46                 ` Koul, Vinod
2016-11-01 14:55                 ` Chen-Yu Tsai
2016-11-01 14:55                   ` Chen-Yu Tsai
2016-11-01 14:55                   ` Chen-Yu Tsai
2016-11-01 14:55                   ` Chen-Yu Tsai
2016-11-14  4:54                   ` Vinod Koul
2016-11-14  4:54                     ` Vinod Koul
2016-11-14  4:54                     ` Vinod Koul
2016-11-14  4:54                     ` Vinod Koul
2016-10-04  9:46   ` [PATCH 02/14] clk: ccu-sun8i-a33: Add CLK_SET_RATE_PARENT to ac-dig Mylène Josserand
2016-10-04  9:46     ` Mylène Josserand
2016-10-04 12:12     ` Thomas Petazzoni
2016-10-04 12:12       ` Thomas Petazzoni
2016-10-04 12:12       ` Thomas Petazzoni
2016-10-04 12:12       ` Thomas Petazzoni
2016-10-05  9:37       ` Mylene Josserand
2016-10-05  9:37         ` Mylene Josserand
2016-10-04  9:46   ` [PATCH 03/14] ASoC: sun4i-i2s: Add apb reset Mylène Josserand
2016-10-04  9:46     ` Mylène Josserand
2016-10-04 12:15     ` Thomas Petazzoni
2016-10-04 12:15       ` Thomas Petazzoni
2016-10-04 12:15       ` Thomas Petazzoni
2016-10-04 15:42       ` Maxime Ripard
2016-10-04 15:42         ` Maxime Ripard
2016-10-05  9:43         ` Mylene Josserand
2016-10-05  9:43           ` Mylene Josserand
2016-10-04 12:22     ` Code Kipper
2016-10-04 12:22       ` Code Kipper
2016-10-04 12:22       ` Code Kipper
2016-10-04  9:46   ` [PATCH 04/14] ASoC: Add sun8i analog codec driver Mylène Josserand
2016-10-04  9:46     ` Mylène Josserand
2016-10-04  9:46     ` Mylène Josserand
2016-10-04 10:21     ` Code Kipper
2016-10-04 10:21       ` Code Kipper
2016-10-04 10:21       ` Code Kipper
2016-10-04 10:21       ` Code Kipper
2016-10-04 10:56       ` Chen-Yu Tsai
2016-10-04 10:56         ` Chen-Yu Tsai
2016-10-04 10:56         ` Chen-Yu Tsai
2016-10-04 10:56         ` Chen-Yu Tsai
2016-10-04  9:46   ` [PATCH 05/14] mfd: sun6i-prcm: Add sun8i analog codec as subnode Mylène Josserand
2016-10-04  9:46     ` Mylène Josserand
2016-10-04 10:52     ` Jean-Francois Moine
2016-10-04 10:52       ` Jean-Francois Moine
2016-10-04 10:52       ` Jean-Francois Moine
2016-10-26 14:05     ` Lee Jones
2016-10-26 14:05       ` Lee Jones
2016-10-26 14:05       ` Lee Jones
2016-10-04  9:46   ` [PATCH 06/14] ASoC: Add sun8i digital audio codec Mylène Josserand
2016-10-04  9:46     ` Mylène Josserand
2016-10-04 12:40     ` Thomas Petazzoni
2016-10-04 12:40       ` Thomas Petazzoni
2016-10-04 12:40       ` Thomas Petazzoni
2016-10-04 13:07       ` Mark Brown
2016-10-04 13:07         ` Mark Brown
2016-10-04 13:07         ` Mark Brown
2016-10-04 13:16         ` Thomas Petazzoni [this message]
2016-10-04 13:16           ` Thomas Petazzoni
2016-10-04 16:09       ` Maxime Ripard
2016-10-04 16:09         ` Maxime Ripard
2016-10-04 16:09         ` Maxime Ripard
2016-10-05 11:54       ` Mylene Josserand
2016-10-05 11:54         ` Mylene Josserand
2016-10-04 16:15     ` Maxime Ripard
2016-10-04 16:15       ` Maxime Ripard
2016-10-06 16:06       ` Icenowy Zheng
2016-10-06 16:06         ` Icenowy Zheng
2016-10-06 16:06         ` Icenowy Zheng
2016-10-06 18:23         ` Alexandre Belloni
2016-10-06 18:23           ` Alexandre Belloni
2016-10-06 18:23           ` Alexandre Belloni
     [not found]     ` <85cbd9926e52d0aa03f6bbfd8794373d8db491e0.1475571575.git.mylene.josserand-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
2016-10-05 10:19       ` Icenowy Zheng
2016-10-05 10:19         ` Icenowy Zheng
2016-10-05 10:19         ` Icenowy Zheng
2016-10-04  9:46   ` [PATCH 07/14] ASoC: Add sun8i audio card Mylène Josserand
2016-10-04  9:46     ` Mylène Josserand
2016-10-04 10:16     ` Code Kipper
2016-10-04 10:16       ` Code Kipper
2016-10-04 10:16       ` Code Kipper
2016-10-04 10:16       ` Code Kipper
2016-10-04 10:59       ` Chen-Yu Tsai
2016-10-04 10:59         ` Chen-Yu Tsai
2016-10-04 10:59         ` Chen-Yu Tsai
2016-10-04 10:59         ` Chen-Yu Tsai
2016-10-04 12:25     ` Thomas Petazzoni
2016-10-04 12:25       ` Thomas Petazzoni
2016-10-04 12:25       ` Thomas Petazzoni
2016-10-05  6:04     ` Code Kipper
2016-10-05  6:04       ` Code Kipper
2016-10-05  6:04       ` Code Kipper
2016-10-05  6:04       ` Code Kipper
2016-10-05 10:03       ` Jean-Francois Moine
2016-10-05 10:03         ` Jean-Francois Moine
2016-10-05 10:03         ` Jean-Francois Moine
2016-10-05 10:03         ` Jean-Francois Moine
2016-10-04  9:46   ` [PATCH 08/14] dt-bindings: sound: Add sun8i analog codec documentation Mylène Josserand
2016-10-04  9:46     ` Mylène Josserand
2016-10-04 10:32     ` Mark Brown
2016-10-04 10:32       ` Mark Brown
2016-10-04 10:32       ` Mark Brown
2016-10-04 16:24     ` Maxime Ripard
2016-10-04 16:24       ` Maxime Ripard
2016-10-04 16:24       ` Maxime Ripard
2016-10-05  2:59       ` Chen-Yu Tsai
2016-10-05  2:59         ` Chen-Yu Tsai
2016-10-05  2:59         ` Chen-Yu Tsai
2016-10-05  2:59         ` Chen-Yu Tsai
2016-10-05 12:08         ` Mylene Josserand
2016-10-05 12:08           ` Mylene Josserand
2016-10-05 12:08           ` Mylene Josserand
2016-10-04  9:46   ` [PATCH 09/14] dt-bindings: sound: Add sun8i " Mylène Josserand
2016-10-04  9:46     ` Mylène Josserand
2016-10-04  9:46     ` Mylène Josserand
2016-10-04 16:19     ` Maxime Ripard
2016-10-04 16:19       ` Maxime Ripard
2016-10-04  9:46   ` [PATCH 10/14] dt-bindings: sound: Add sun8i audio card documentation Mylène Josserand
2016-10-04  9:46     ` Mylène Josserand
2016-10-04  9:46     ` Mylène Josserand
2016-10-04 16:32     ` Maxime Ripard
2016-10-04 16:32       ` Maxime Ripard
2016-10-04 16:32       ` Maxime Ripard
2016-10-04  9:46   ` [PATCH 11/14] ARM: dts: sun8i: Add analog codec on prcm node Mylène Josserand
2016-10-04  9:46     ` Mylène Josserand
2016-10-04  9:46   ` [PATCH 12/14] ARM: dts: sun8i: Add audio codec, dai and card for A33 Mylène Josserand
2016-10-04  9:46     ` Mylène Josserand
2016-10-04  9:46   ` [PATCH 13/14] ARM: dts: sun8i: parrot: Enable audio nodes Mylène Josserand
2016-10-04  9:46     ` Mylène Josserand
2016-10-04  9:46   ` [PATCH 14/14] ARM: dts: sun8i: sinlinx: " Mylène Josserand
2016-10-04  9:46     ` Mylène Josserand
     [not found] ` <cover.1475569400.git.mylene.josserand-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
2016-10-07  8:35   ` [PATCH 00/14] ASoc: sunxi: Add Allwinner A33 codec driver Icenowy Zheng
2016-10-07  8:35     ` Icenowy Zheng
2016-10-07  8:35     ` Icenowy Zheng

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161004151603.38aac5b2@free-electrons.com \
    --to=thomas.petazzoni@free-electrons.com \
    --cc=alexandre.belloni@free-electrons.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dmaengine@vger.kernel.org \
    --cc=lee.jones@linaro.org \
    --cc=lgirdwood@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sunxi@googlegroups.com \
    --cc=mark.rutland@arm.com \
    --cc=maxime.ripard@free-electrons.com \
    --cc=mturquette@baylibre.com \
    --cc=mylene.josserand@free-electrons.com \
    --cc=perex@perex.cz \
    --cc=robh+dt@kernel.org \
    --cc=sboyd@codeaurora.org \
    --cc=tiwai@suse.com \
    --cc=vinod.koul@intel.com \
    --cc=wens@csie.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.