On 1/12/20 11:18 PM, Vinod Koul wrote:
On 10-01-20, 10:08, Pierre-Louis Bossart wrote:
The "big" difference is that probe is called by core (asoc) and not by
driver onto themselves.. IMO that needs to go away.
What I did is not different from what existed already with platform devices.
They were manually created, weren't they?
Manual creation of device based on a requirement is different, did I ask
you why you are creating device :)
I am simple asking you not to call probe in the driver. If you need
that, move it to core! We do not want these kind of things in the
drivers...
What core are you talking about?
soundwire core ofcourse! IMO All that which goes into soundwire-bus-objs is
considered as soundwire core part and rest are drivers intel, qc, so on!
This master code was added to the bus: v
v
soundwire-bus-objs := bus_type.o bus.o master.o slave.o mipi_disco.o
stream.o
obj-$(CONFIG_SOUNDWIRE) += soundwire-bus.o
and the API is also part of the sdw.h include file. That seems to meet
exactly what you describe above, no?
git grep sdw_master_device_add (reformatted output)
drivers/soundwire/intel_init.c:
md = sdw_master_device_add(&intel_sdw_driver,
drivers/soundwire/master.c:
*sdw_master_device_add(struct sdw_master_driver *driver,
drivers/soundwire/master.c:
EXPORT_SYMBOL_GPL(sdw_master_device_add);
include/linux/soundwire/sdw.h:
*sdw_master_device_add(struct sdw_master_driver *driver,
So, what exactly is the issue?
We are not 'calling the probe in the [Intel] driver' as you state it, we
use a SoundWire core API which in turn will create a device. The device
core takes care of calling the probe, see the master.c code which is NOT
Intel-specific.
The SOF intel driver needs to create a device, which will then be bound with
a SoundWire master driver.
What I am doing is no different from what your team did with
platform_register_device, I am really lost on what you are asking.
Again repeating myself, you call an API to do that is absolutely fine,
but we don't do that in drivers or open code these things
That is still quite unclear, what 'open-coding' are you referring to?
I am starting to wonder if you missed the addition of the master
functionality in the previous patch:
[PATCH v5 08/17] soundwire: add initial definitions for sdw_master_device
What this patch 9 does is call the core-defined API and implement the
intel-specific master driver.
FWIW, the implementation here follows what was suggested for Greybus 'Host
Devices' [1] [2], so it's not like I am creating any sort of dangerous
precedent.
[1]
https://elixir.bootlin.com/linux/latest/source/drivers/greybus/es2.c#L1275
[2] https://elixir.bootlin.com/linux/latest/source/drivers/greybus/hd.c#L124
And if you look closely all this work is done by core not by drivers!
Drivers _should_ never do all this, it is the job of core to do that for
you.
Please look at the code again, you have a USB probe that will manually call
the GreyBus device creation.
static int ap_probe(struct usb_interface *interface,
const struct usb_device_id *id)
{
hd = gb_hd_create(&es2_driver, &udev->dev,
static struct usb_driver es2_ap_driver = {
.name = "es2_ap_driver",
.probe = ap_probe, <<< code above
.disconnect = ap_disconnect,
.id_table = id_table,
.soft_unbind = 1,
};
Look closely the driver es2 calls into greybus core hd.c and gets the
work done, subtle but a big differances in the approaches..
I am sorry, I have absolutely no idea what you are referring to.
The code I copy/pasted here makes no call to the greybus core, it's ap_probe
-> gb_hd_create. No core involved. If I am mistaken, please show me what I
got wrong.
1. es2_ap_driver is host controller driver
2. gb_hd_create() is an API provided by greybus core!
same in my code...
es2 driver doesn't open code creation like you are doing in intel driver,
it doesn't call probe on its own, greybus does that
This is very common pattern in linux kernel subsytems, drivers dont do
these things, the respective subsystem core does that... see about es2
driver and implementation of gb_hd_create(). See callers of
platform_register_device() and its implementation.
I don't know how else I can explain this to you, is something wrong in
how I conveyed this info or you... or something else, I dont know!!!
the new 'master' functionality is part of the bus code, so please
clarify what you see as problematic for the partition.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
