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

[Michael Nazzareno Trimarchi]

Hi

On Thu, Mar 1, 2018 at 8:43 AM, MylÃne Josserand
<mylene.josserand@xxxxxxxxxxx> wrote:
> Hello,
>
> Thank you for the review.
>
> On Tue, 27 Feb 2018 22:51:40 +0100
> Thomas Petazzoni <thomas.petazzoni@xxxxxxxxxxx> 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@xxxxxxxxxxx>
>> > ---
>> >  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@xxxxxxxxxxxxxxxx
> 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               |