Hi Kuninori Miromoto, >-----Original Message----- >From: Kuninori Morimoto [mailto:kuninori.morimoto.gx@xxxxxxxxxxx] >Sent: Thursday, December 21, 2017 6:24 PM >To: Ryan Lee <RyanS.Lee@xxxxxxxxxxxxxxxxxxx> >Cc: lgirdwood@xxxxxxxxx; broonie@xxxxxxxxxx; robh+dt@xxxxxxxxxx; >mark.rutland@xxxxxxx; perex@xxxxxxxx; tiwai@xxxxxxxx; arnd@xxxxxxxx; >afd@xxxxxx; robert.jarzmik@xxxxxxx; supercraig0719@xxxxxxxxx; >jbrunet@xxxxxxxxxxxx; dannenberg@xxxxxx; romain.perier@xxxxxxxxxxxxx; >bryce.ferguson@xxxxxxxxxxxxxxxxxxx; m-stecklein@xxxxxx; alsa-devel@alsa- >project.org; devicetree@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; >ryan.lee.maxim@xxxxxxxxx >Subject: Re: [PATCH] ASoC: max98373: Added Amplifier Driver > >EXTERNAL EMAIL > > > >Hi Ryan > >> Signed-off-by: Ryan Lee <ryans.lee@xxxxxxxxxxxxxxxxxxx> >> --- >> >> 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 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
