Re: [PATCH 1/8] soundwire: bus_type: add master_device/driver support - Pierre-Louis Bossart

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
To: Vinod Koul <vkoul@kernel.org>
Cc: alsa-devel@alsa-project.org, tiwai@suse.de,
	gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org,
	Ranjani Sridharan <ranjani.sridharan@linux.intel.com>,
	Hui Wang <hui.wang@canonical.com>,
	broonie@kernel.org, srinivas.kandagatla@linaro.org,
	jank@cadence.com, slawomir.blauciak@intel.com,
	Sanyog Kale <sanyog.r.kale@intel.com>,
	Bard liao <yung-chuan.liao@linux.intel.com>,
	Rander Wang <rander.wang@linux.intel.com>
Subject: Re: [PATCH 1/8] soundwire: bus_type: add master_device/driver support
Date: Mon, 16 Mar 2020 14:15:02 -0500	[thread overview]
Message-ID: <3c32830c-cd12-867f-a763-7c3e385cb1e9@linux.intel.com> (raw)
In-Reply-To: <20200314094904.GP4885@vkoul-mobl>

>> 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.

>> 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."

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.

> 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 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'?