lkml.org 
[lkml]   [2017]   [Dec]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [PATCH] ASoC: max98373: Added Amplifier Driver
Date
Hi Kuninori Miromoto,

>-----Original Message-----
>From: Kuninori Morimoto [mailto:kuninori.morimoto.gx@renesas.com]
>Sent: Thursday, December 21, 2017 6:24 PM
>To: Ryan Lee <RyanS.Lee@maximintegrated.com>
>Cc: lgirdwood@gmail.com; broonie@kernel.org; robh+dt@kernel.org;
>mark.rutland@arm.com; perex@perex.cz; tiwai@suse.com; arnd@arndb.de;
>afd@ti.com; robert.jarzmik@free.fr; supercraig0719@gmail.com;
>jbrunet@baylibre.com; dannenberg@ti.com; romain.perier@collabora.com;
>bryce.ferguson@rockwellcollins.com; m-stecklein@ti.com; alsa-devel@alsa-
>project.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
>ryan.lee.maxim@gmail.com
>Subject: Re: [PATCH] ASoC: max98373: Added Amplifier Driver
>
>EXTERNAL EMAIL
>
>
>
>Hi Ryan
>
>> Signed-off-by: Ryan Lee <ryans.lee@maximintegrated.com>
>> ---
>>
>> Created max98373 amplifier driver.
>>
>> .../devicetree/bindings/sound/max98373.txt | 43 +
>> sound/soc/codecs/Kconfig | 5 +
>> sound/soc/codecs/Makefile | 2 +
>> sound/soc/codecs/max98373.c | 996
>+++++++++++++++++++++
>> sound/soc/codecs/max98373.h | 225 +++++
>> 5 files changed, 1271 insertions(+)
>> create mode 100644
>> Documentation/devicetree/bindings/sound/max98373.txt
>> create mode 100644 sound/soc/codecs/max98373.c create mode 100644
>> sound/soc/codecs/max98373.h
>(snip)
>> +struct max98373_priv {
>> + struct regmap *regmap;
>> + struct snd_soc_codec *codec;
>> + unsigned int sysclk;
>> + unsigned int v_slot;
>> + unsigned int i_slot;
>> + unsigned int spkfb_slot;
>> + bool interleave_mode;
>> + unsigned int ch_size;
>> + unsigned int iface;
>> + bool tdm_mode;
>> +};
>
>About this max98373->codec.
>This user is only max98373_set_clock(), and it is called from
>max98373_dai_hw_params().
>You are getting *codec from dai->codec in this function, and max98373 is
>came from it.
>This means, we can remove max98373->codec ?

Thanks for your feedback.
I will remove max98373->codec and change related things.

>
>About max98373->regmap.
>You are using devm_regmap_init_i2c(), and keeping it on max98373.
>Can you check snd_soc_component_add() which is called from
>snd_soc_add_component().
>It will set component->regmap if you called devm_regmap_init_i2c().
>I think you can use snd_soc_component_read/write instead of
>regmap_read/write.
>Then, we can remove max98373->regmap too ?
>Ahh, you want to check Revision ID.
>then, snd_soc_component_init_regmap() can help you ?

I'm sorry but I don't fully understand the benefit of this.
Keeping regmap information in private driver data is very common and I can see many ASoC drivers are using it.
I was able to see only a few driver use ' snd_soc_component_read'.
I would like to keep regmap_read/write if it is still acceptable.

>
>Best regards
>---
>Kuninori Morimoto

\
 
 \ /
  Last update: 2017-12-25 15:31    [W:0.040 / U:16.828 seconds]
©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site