LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Takashi Iwai <tiwai@suse.de>
To: "Arnaud Pouliquen" <arnaud.pouliquen@st.com>
Cc: "Mark Brown" <broonie@kernel.org>,
	"Olivier MOYSAN" <olivier.moysan@st.com>,
	"alsa-devel@alsa-project.org" <alsa-devel@alsa-project.org>,
	"mark.rutland@arm.com" <mark.rutland@arm.com>,
	"rmk@arm.linux.org.uk" <rmk@arm.linux.org.uk>,
	"lgirdwood@gmail.com" <lgirdwood@gmail.com>,
	"mcoquelin.stm32@gmail.com" <mcoquelin.stm32@gmail.com>,
	"robh@kernel.org" <robh@kernel.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"Alexandre TORGUE" <alexandre.torgue@st.com>,
	"Benjamin GAIGNARD" <benjamin.gaignard@st.com>,
	"kernel@stlinux.com" <kernel@stlinux.com>,
	"jsarha@ti.com" <jsarha@ti.com>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [alsa-devel] [PATCH 0/3] ASoC: stm32: sai: add support of iec958 controls
Date: Tue, 05 Jun 2018 20:29:37 +0200	[thread overview]
Message-ID: <s5hpo15xf1q.wl-tiwai@suse.de> (raw)
In-Reply-To: <2ae6c5b6-872a-9aaa-6ca4-e29adb7c8bc0@st.com>

On Tue, 05 Jun 2018 17:50:57 +0200,
Arnaud Pouliquen wrote:
> 
> Hi Takashi,
> 
> On 04/17/2018 01:17 PM, Mark Brown wrote:
> > On Tue, Apr 17, 2018 at 08:29:17AM +0000, Olivier MOYSAN wrote:
> > 
> >> I guess the blocking patch in this patchset is the patch "add IEC958 
> >> channel status control helper". This patch has been reviewed several 
> >> times, but did not get a ack so far.
> >> If you think these helpers will not be merged, I will reintegrate the 
> >> corresponding code in stm driver.
> >> Please let me know, if I need to prepare a v2 without helpers, or if we 
> >> can go further in the review of iec helpers patch ?
> > 
> > I don't mind either way but you're right here, I'm waiting for Takashi
> > to review the first patch.  I'd probably be OK with it just integrated
> > into the driver if we have to go that way though.
> 
> Gentlemen reminder for this patch set. We would appreciate to have your
> feedback on iec helper.
> From our point of view it could be useful to have a generic management
> of the iec controls based on helpers instead of redefining them in DAIs.
> Having the same behavior for these controls could be useful to ensure
> coherence of the control management used by application (for instance
> Gstreamer uses it to determine iec raw mode capability for iec61937 streams)

Oh sorry for the late reply, I've totally overlooked the thread.

And, another sorry: the patchset doesn't look convincing enough to
me.

First off, the provided API definition appears somewhat
unconventional, the mixture of the ops, the static data and the
dynamic data.

Moreover, this is only for your driver, ATM.  If it were an API that
does clean up the already existing usages, I'd happily apply it. There
are lots of drivers creating and controlling the IEC958 ctls even
now.

Also, the patchset requires more fine-tuning, in anyways.  The changes
in create_iec958_consumre() are basically irrelevant, should be split
off as an individual fix.  Also, the new function doesn't create the
"XXX Mask" controls.  And the byte comparison should be replaced with
memcmp(), etc, etc.

So, please proceed rather with the open codes for now.  If you can
provide a patch that cleans up the *multiple* driver codes, again,
I'll happily take it.  But it can be done anytime later, too.


thanks,

Takashi

  reply	other threads:[~2018-06-05 18:29 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-13 16:27 Olivier Moysan
2018-03-13 16:27 ` [PATCH 1/3] ALSA: pcm: add IEC958 channel status control helper Olivier Moysan
2018-03-13 16:27 ` [PATCH 2/3] ASoC: stm32: sai: add iec958 controls support Olivier Moysan
2018-06-05 15:52   ` Arnaud Pouliquen
2018-03-13 16:27 ` [PATCH 3/3] ASoC: dmaengine_pcm: document process callback Olivier Moysan
2018-03-13 16:47   ` Applied "ASoC: dmaengine_pcm: document process callback" to the asoc tree Mark Brown
2018-04-17  8:29 ` [PATCH 0/3] ASoC: stm32: sai: add support of iec958 controls Olivier MOYSAN
2018-04-17 11:17   ` Mark Brown
2018-05-17 13:03     ` Olivier MOYSAN
2018-06-05 15:50     ` [alsa-devel] " Arnaud Pouliquen
2018-06-05 18:29       ` Takashi Iwai [this message]
2018-06-06  9:31         ` Arnaud Pouliquen
2018-06-06  9:47           ` Takashi Iwai
2018-06-07 16:02             ` Arnaud Pouliquen

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=s5hpo15xf1q.wl-tiwai@suse.de \
    --to=tiwai@suse.de \
    --cc=alexandre.torgue@st.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=arnaud.pouliquen@st.com \
    --cc=benjamin.gaignard@st.com \
    --cc=broonie@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=jsarha@ti.com \
    --cc=kernel@stlinux.com \
    --cc=lgirdwood@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mcoquelin.stm32@gmail.com \
    --cc=olivier.moysan@st.com \
    --cc=rmk@arm.linux.org.uk \
    --cc=robh@kernel.org \
    --subject='Re: [alsa-devel] [PATCH 0/3] ASoC: stm32: sai: add support of iec958 controls' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).