Re: [PATCH 1/8] soundwire: bus_type: add master_device/driver support

[Vinod Koul <vkoul@kernel.org>]

On 16-03-20, 14:15, Pierre-Louis Bossart wrote:
> 
> 
> > > It's really down to your objection to the use of 'struct driver'... For ASoC
> > > support we only need the .name and .pm_ops, so there's really no possible
> > > path forward otherwise.
> > 
> > It means that we cannot have a solution which is Intel specific into
> > core. If you has a standalone controller you do not need this.
> 
> A 'struct driver' is not Intel-specific, sorry.

We are discussing 'struct sdw_master_driver'. Please be very specific in
you replies and do not use incorrect terminology which confuses people.

Sorry a 'struct sdw_master_driver' IMHO is. As I have said it is not
needed if you have standalone controller even in Intel case, and rest of
the world.

> > > Like I said, we have 3 options
> > 
> > Repeating the already discussed doesn't help. I have already told you the
> > constraint to work is not to add Intel specific change into core.
> > 
> > I have already said that expect the driver part I dont have objections
> > to rest of this series and am ready to merge
> > 
> > > a) stay with platform devices for now. You will need to have a conversation
> > > with Greg on this.
> > > 
> > > b) use a minimal sdw_master_device with a minimal 'struct driver' use.
> > > 
> > > c) use a more elaborate solution suggested in this patchset and yes that
> > > means the Qualcomm driver would need to change a bit.
> > > 
> > > Pick one or suggest something that is implementable. The first version of
> > > the patches was provided in October, the last RFC was provided on January
> > > 31, time's up. At the moment you are preventing ASoC integration from moving
> > > forward.
> > 
> > In opensource review we go back and forth and we debate and come to a
> > common conclusion. Choosing a specific set of solutions and constraining
> > yourself to pick one does not help.
> 
> First off, the ask to move away from platform devices came from Greg. Not
> me. All I did here was suggest solutions, one reviewed by Greg as 'sane' and
> 'nice work'. Greg essentially wrote the book on devices/drivers so his
> review means I am not completely senile just yet.
> 
> You pushed back with two proposals that don't account for power management
> and the driver name required for ASoC. That was on top on another suggestion
> to use platform devices that was shot down by Greg himself with language I
> can't quote here.
> 
> Please re-read my words: my ask was "Pick one or suggest something that is
> implementable."

IMO the path I suggested is implementable..
> 
> You don't pick one and don't suggest anything implementable either, so
> there's really not much I can do, can I? I don't have a solution without a
> 'struct driver', and you don't want it.
> 
> The only short-term path forward I see is to ask Greg to agree to keep the
> platform devices for now.

And I guess you didn't talk to your Intel colleagues... Please talk to
them on how they did it.


> 
> > I have only _one_ constraint no platform specific change in core. If that
> > is satisfied I will go with it. Sorry but this is non-negotiable for me.
> 
> How is a 'struct driver' platform specific?
> 
> > Ask yourself, do you need this intrusive core change if you had this
> > exact same controller(s) but only as standalone one...
> 
> That would really not change anything. There would be some sort of ID (PCI
> or something else) for the controller and multiple masters below. The

If it is single master?

If it is multiple master with different ACPI ID for each master?

Would you still need 'struct sdw_master_driver'?

> ACPI/DisCo spec does not account for masters so they would have to be
> created by hand.


> 
> Again how is a 'struct driver' an 'intrusive change'?

Again do not confuse by using incorrect terminology.

Again 'struct sdw_master_driver' for a specific Intel hw configuration is.

-- 
~Vinod