Re: [RFC PATCH 1/2] ASoC: mediatek: add btcvsd driver

[KaiChieh Chuang <kaichieh.chuang@mediatek.com>]

On Thu, 2019-01-24 at 19:25 +0000, Mark Brown wrote:
> On Thu, Jan 24, 2019 at 04:58:41PM +0800, KaiChieh Chuang wrote:
> 
> > The driver function for transferring/receiving
> > BT encoded data to/from BT hardware.
> 
> It's really nice to see someone submitting a driver for actual radio
> hardware, this is great to see!  
> 
> The driver looks pretty good, I've got a few quite minor comments below
> but the main one is that given that this doesn't have any DAIs or DAPM
> it looks like it doesn't integrate with the rest of the sound card much
> so perhaps it could just be a free standing ALSA driver?  If there's
> inputs that let you link it to the rest of the sound card which will be
> added later then that's obviously a good reason to keep it in ASoC (I'd
> not be surprised if there were) but otherwise I'm not sure what
> advantage ASoC brings you.

Our btcvsd radio chip is tightly bound with baseband SoC,
Currently, the mtk-btcvsd resides in mtk afe sound cards,
such as mt6797-mt6351 sound card.

"free standing ALSA driver" i'm not quite sure what you mean by this,
do you mean something like usb sound card (sound/usb/)?
Unlike normal BT SCO chip is transfer with PCM format,
we transfer encoded data directly. It's a MTK hardware-proprietary,
i'm not sure a standalone ALSA driver can do any good here.
Thats why I decided to put it under sound/soc/mediatek/.


> There's quite a few of these dev_info() calls which look like they might
> be a bit noisy and could perhaps be dev_dbg() instead.

I'll cut down the log in next patch

> Some of these enums (all of them except the narrow/wide band one) should
> be simple number controls ending with Switch as covered in
> control-names.rst, this helps UIs display them better.

I'll update this in next patch

> > +	SND_SOC_BYTES_TLV("btcvsd_tx_timestamp",
> > +			  sizeof(struct mtk_btcvsd_snd_time_buffer_info),
> > +			  btcvsd_tx_timestamp_get, NULL),
> 
> There's an ALSA timestamping API - see commit 4eeaaeaea1ce (ALSA: core:
> add hooks for audio timestamps) and related commits.  I don't know if
> that applies here and might be a better fit or not?

We would like to keep the exact time stamp during copy (read/write).
and the remain data in kernel temp buffer. I think its a little different
to ALSA API. I would prefer keeping this for now?