On 04-03-20, 17:28, Greg KH wrote: > On Wed, Mar 04, 2020 at 09:17:07AM -0600, Pierre-Louis Bossart wrote: > > > > > > > Were the above lines agreed or not? Do you see driver for master devices > > > or not? Greg was okay with as well as these patches but I am not okay > > > with the driver part for master, so I would like to see that removed. > > > > > > Different reviewers can have different reasons.. I have given bunch of > > > reasons here, BUT I have not seen a single technical reason why this > > > cannot be done. > > > > With all due respect, I consider Greg as THE reviewer for device/driver > > questions. Your earlier proposal to use platform devices was rejected by > > Greg, and we've lost an entire month in the process, so I am somewhat > > dubious on your proposal not to use a driver. > > > > If you want a technical objection, let me restate what I already mentioned: > > > > If you look at the hierarchy, we have > > > > PCI device -> PCI driver > > soundwire_master_device0 > > soundwire_slave(s) -> codec driver > > ... > > soundwire_master_deviceN > > soundwire_slave(s) -> codec driver > > > > You have not explained how I could possibly deal with power management > > without having a driver for the master_device(s). The pm_ops need to be > > inserted in a driver structure, which means we need a driver. And if we need > > a driver, then we might as well have a real driver with .probe .remove > > support, driver_register(), etc. > > To weigh in here, yes, you need such a "device" here as it isn't the PCI > device that you can use, you need your own. Just like most other busses > have this (USB has host controller drivers as one example, that create > the "root bus" device that all other USB devices hang off of.) This > "controller device" should hang off of the hardware device be it a > platform/PCI/i2c/spi/serial/whatever type of controller. That's why it > is needed. > > > I really don't see what's broken or unnecessary with these patches. > > The "wait until something else happens" does seem a bit hacky, odds are > that's not really needed if you are using the driver model correctly, > but soundwire is "odd" in places so maybe that is necessary, I'll defer > to you two on that mess :) I have no issues with adding the root bus device, so we really need the sdw_master_device. That really was a miss from me. If Pierre remove the driver parts from this set, I would merge the rest in a heartbeat. But in the mix we dont want to add a new driver which is dummy. The PCI/DT device will have a driver which is something to be used here. For Qualcomm controller, we get a DT device and driver and that does the job, it doesn't need one more driver and probe routine. If Pierre had a PCI device for SDW controller, he would not be asking for this, since Intel has a complex device, and he would like to split the functions, the need of a new driver arises. But the whole subsystem should not be burdened for that. It can be managed without that and is really an Intel issue not a subsystem one. -- ~Vinod
