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