Re: [PATCH v11 2/3] ASoC: qcom: dt-bindings: Add sc7180 machine bindings

On Wed, Oct 21, 2020 at 02:51:33AM +0800, Cheng-yi Chiang wrote:

On Tue, Oct 20, 2020 at 10:37 PM Mark Brown [off-list ref] wrote:

quoted

If the device has both front and rear mics and only one can be active at
once that seems obvious and sensible.  If the devices only have one of
these then this seems like a bad idea.

trogdor board: only front mic.
pompom board: having both front mic and rear mic. Only one of them
will be used at a time. It is toggled by mixer control backed by a
gpio.

My proposed solution: instead of using compatible strings, expose only
dmic-gpio property.
When the machine driver sees this property, it uses the dapm widgets
and controls created in the machine driver.

Yes, that is what I would expect.

quoted

I don't understand what "logic scattered in various dtsi files" means,
sorry.

I mean I don't want to use device property to pass in widget name,
type, text and callbacks.
Let me give an example:

- Board trogdor uses front mic, rt5682, and max98357a.
- Board pompom is based on board trogdor, but it has front mic and rear mic.
If we somehow managed to add the code to pass in widget, route, type,
text, and callbacks needed for dmic control, we will need to put a
bunch of properties in trogdor-pompom.dtsi file.

Most of this code is already there as part of the generic card
infrastructure, the only thing that stands out for me is the GPIO to
switch between the front and rear mics.

- Board ABC is based on trogdor as well, and it has front mic and rear
mic, but with a different speaker amp.

To use widget, route, type, text and callbacks for front mic and rear
mic, in trogdor-ABC.dtsi file we would copy some properties used in
trogdor-pompom.dtsi file. To support the different combination of
codec, we would need some modification of the route and widget.

It shouldn't be hugely difficult to split the DT files up usually, and
ideally they'd be small enough that just having an entirely new sound
bit isn't the end of the world.  Again I'm just not clear what you're
seeing here.

quoted

The CODEC change is going to be described in the DT no matter what -
you'll have a reference to the CODEC node but it may make sense if
there's enough custom code around it.  For front vs rear mic the
simplest thing would just be to not mention which if this is a hardware
fixed thing, otherwise a control.

Would you suggest checking whether the codec node is a rt5682 node,
and call required PLL calls accordingly ?

Potentially, or there might be so little shared that it's just a
separate machine driver.

"For front vs rear mic the simplest thing would just be to not mention
which if this is a hardware fixed thing, otherwise a control."
Sorry I am not sure if I understand this correctly. Please correct me
if I am wrong.

- For default case having 1 mic: not mention this at all
- For front mic / rear mic case: see gpio property and use an
additional control.

Yes.

"These feel more like things that fit with compatible" regarding
replacing alc5682 with adau7002. Please let me know which one solution
you prefer:
-  deriving this information from codec node
-  deriving this information from different sound card name

To an extent this depends on how different the CODECs and general setup
are but a different CODEC is something that often justifies a separate
compatible.  Of course you also have an awful lot of systems that work
with the generic card drivers and all different kinds of CPU and CODEC,
usually because the driver doesn't need to know anything about the
implementation of either.