Re: [alsa-devel] [PATCH v1 1/4] ASoC: codecs: pcm179x: Add PCM1789 id

Michael Nazzareno Trimarchi
2018-03-01T09:04:20Z

Hi

On Thu, Mar 1, 2018 at 8:43 AM, Mylène Josserand
<mylene.josserand@bootlin.com> wrote:
> Hello,
>
> Thank you for the review.
>
> On Tue, 27 Feb 2018 22:51:40 +0100
> Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote:
>
>> Hello,
>>
>> On Tue, 27 Feb 2018 22:24:30 +0100, Mylène Josserand wrote:
>> > To prepare the support for the PCM1789, add a new compatible
>> > and use the i2c_id to handle, later, the differences between
>> > these two DACs even if they are pretty similar.
>> >
>> > Signed-off-by: Mylène Josserand <mylene.josserand@bootlin.com>
>> > ---
>> > Documentation/devicetree/bindings/sound/pcm179x.txt | 2 +-
>>
>> The DT binding change should be in a separate patch.
>>
>> > sound/soc/codecs/pcm179x-i2c.c | 6 ++++--
>> > sound/soc/codecs/pcm179x.c | 3 ++-
>> > sound/soc/codecs/pcm179x.h | 8 +++++++-
>>
>> And this should be together with the PCM1789 support patch. Otherwise
>> your series is not bisectable: if we apply only PATCH 1/4, the driver
>> supports the new compatible string, but it doesn't have the actual code
>> to handle PCM1789. Am I missing something here ?
>
> No, you are right, I will merge it with patch 02.
>

Can you please include me in next series?

I have several hardware running on pcm179x family

Michael

>>
>> > - return pcm179x_common_init(&client->dev, regmap);
>> > + return pcm179x_common_init(&client->dev, regmap, id->driver_data);
>>
>> This can be done in a preparation patch.
>>
>> > }
>> >
>> > static const struct of_device_id pcm179x_of_match[] = {
>> > { .compatible = "ti,pcm1792a", },
>> > + { .compatible = "ti,pcm1789", },
>> > { }
>> > };
>> > MODULE_DEVICE_TABLE(of, pcm179x_of_match);
>> >
>> > static const struct i2c_device_id pcm179x_i2c_ids[] = {
>> > - { "pcm179x", 0 },
>> > + { "pcm179x", PCM179X },
>>
>> And also this addition.
>>
>> > + { "pcm1789", PCM1789 },
>> > { }
>> > };
>> > MODULE_DEVICE_TABLE(i2c, pcm179x_i2c_ids);
>> > diff --git a/sound/soc/codecs/pcm179x.c b/sound/soc/codecs/pcm179x.c
>> > index 4b311c06f97d..81cbf09319f6 100644
>> > --- a/sound/soc/codecs/pcm179x.c
>> > +++ b/sound/soc/codecs/pcm179x.c
>> > @@ -218,7 +218,8 @@ static const struct snd_soc_component_driver soc_component_dev_pcm179x = {
>> > .non_legacy_dai_naming = 1,
>> > };
>> >
>> > -int pcm179x_common_init(struct device *dev, struct regmap *regmap)
>> > +int pcm179x_common_init(struct device *dev, struct regmap *regmap,
>> > + enum pcm17xx_type type)
>>
>> And this done.
>>
>> > {
>> > struct pcm179x_private *pcm179x;
>> >
>> > diff --git a/sound/soc/codecs/pcm179x.h b/sound/soc/codecs/pcm179x.h
>> > index cf8681c9a373..8c08689e3b8b 100644
>> > --- a/sound/soc/codecs/pcm179x.h
>> > +++ b/sound/soc/codecs/pcm179x.h
>> > @@ -17,11 +17,17 @@
>> > #ifndef __PCM179X_H__
>> > #define __PCM179X_H__
>> >
>> > +enum pcm17xx_type {
>> > + PCM179X,
>>
>> And this one.
>>
>> > + PCM1789,
>> > +};
>> > +
>> > #define PCM1792A_FORMATS (SNDRV_PCM_FMTBIT_S32_LE | SNDRV_PCM_FMTBIT_S24_LE | \
>> > SNDRV_PCM_FMTBIT_S16_LE)
>> >
>> > extern const struct regmap_config pcm179x_regmap_config;
>> >
>> > -int pcm179x_common_init(struct device *dev, struct regmap *regmap);
>> > +int pcm179x_common_init(struct device *dev, struct regmap *regmap,
>> > + enum pcm17xx_type type);
>>
>> And this one. Just as a "preparation patch" to support multiple codecs
>> in the existing pcm179x.c driver.
>>
>> Thanks!
>>
>> Thomas
>
> Thanks,
>
> Mylène
>
> --
> Mylène Josserand, Bootlin (formerly Free Electrons)
> Embedded Linux and Kernel engineering
> http://bootlin.com
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

--
| Michael Nazzareno Trimarchi Amarula Solutions BV |
| COO - Founder Cruquiuskade 47 |
| +31(0)851119172 Amsterdam 1018 AM NL |
| [`as] http://www.amarulasolutions.com |