Re: [PATCH] ASoC: rt5682: split i2c driver into separate module

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 1979 bytes --]

On Wed, May 27, 2020 at 11:57:33PM +0200, Arnd Bergmann wrote:
> On Wed, May 27, 2020 at 10:57 PM Mark Brown <broonie@kernel.org> wrote:
> > On Wed, May 27, 2020 at 09:34:13PM +0200, Arnd Bergmann wrote:

> > > +static const char *rt5682_supply_names[RT5682_NUM_SUPPLIES] = {
> > > +     "AVDD",
> > > +     "MICVDD",
> > > +     "VBAT",
> > > +};

> > I'm *fairly* sure the device needs power even with Soundwire?

> I have no idea how this is done with ACPI. I'm moving it

There are non-ACPI SoundWire systems - Qualcomm have some systems
upstream.

> back as an exported symbol. There should probably be
> some abstraction that handles this in the common part of
> the driver along with some of the other bits of
> rt5682_i2c_probe, but really don't want to shake things up
> too much and would let this be done by whoever needs to add
> DT support to the soundwire driver in the future and is able
> to test the changes.

Like I say the abstraction is generally just setting up the very basic
stuff and calling into a common init function with a few parameters.

> > This doesn't look very I2C specific either, nor do chunks of the rest of
> > the code.  The usual pattern with this stuff is to have the bus specific
> > code do bus specific stuff like setting up the regmap and then call into
> > a common init function for the shared parts of the chip.  I'd expect a
> > bit more unshared code here but not this much.

> Right, I was surprised the soundwire portion does not tie into
> device tree at all, and none of the other soundwire codecs seem
> to either and no dts files reference it, though there is some code
> for the subsystem and a binding.

The stuff that's upstream for DT platforms is for Qualcomm systems where
the reference designs are generally fully integrated with Qualcomm
components so there's limited overlap.  IIRC they also have a fairly
unusual system design with a mix of SoundWire and Slimbus usually (that
was the case for a while at least).

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]