Re: [PATCH] soundwire: intel: move to auxiliary bus

[Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>]

Thanks Greg and Vinod for the reviews

>>> -static int intel_master_probe(struct platform_device *pdev)
>>> +static int intel_link_probe(struct auxiliary_device *auxdev, const struct auxiliary_device_id *id)
>>>   {
>>> -	struct device *dev = &pdev->dev;
>>> +	struct device *dev = &auxdev->dev;
>>> +	struct sdw_intel_link_dev *ldev = auxiliary_dev_to_sdw_intel_link_dev(auxdev);
>>
>> Do we need another abstractions for resources here, why not aux dev
>> creation set the resources required and we skip this step...

Not sure what resources you are referring to?

this is just a container_of() and the documented way of extending the 
auxbus (see 
https://www.kernel.org/doc/html/latest/driver-api/auxiliary_bus.html#example-usage)


struct sdw_intel_link_dev {
	struct auxiliary_device auxdev;
	struct sdw_intel_link_res link_res;
};

#define auxiliary_dev_to_sdw_intel_link_dev(auxiliary_dev) \
	container_of(auxiliary_dev, struct sdw_intel_link_dev, auxdev)

>>>   	struct sdw_intel *sdw;
>>>   	struct sdw_cdns *cdns;
>>>   	struct sdw_bus *bus;
>>> @@ -1346,14 +1347,14 @@ static int intel_master_probe(struct platform_device *pdev)
>>>   	cdns = &sdw->cdns;
>>>   	bus = &cdns->bus;
>>>   
>>> -	sdw->instance = pdev->id;
>>> -	sdw->link_res = dev_get_platdata(dev);
>>> +	sdw->instance = auxdev->id;
>>
>> so auxdev has id and still we pass id as argument :( Not sure if folks
>> can fix this now
> 
> That's odd, yeah, it should be fixed.

I think we are talking about different things?

this is defined in mod_devicetable.h:

struct auxiliary_device_id {
	char name[AUXILIARY_NAME_SIZE];
	kernel_ulong_t driver_data;
};

and used for the driver probe:

	ret = auxdrv->probe(auxdev, auxiliary_match_id(auxdrv->id_table, auxdev));

but there is a separate id:

struct auxiliary_device {
	struct device dev;
	const char *name;
	u32 id;
};

which is set during the device initialization in intel_init.c

	/* we don't use an IDA since we already have a link ID */
	auxdev->id = link_id;

In the auxiliary bus design, the parent has to take care of managing 
this id, be it with an IDA or as we do here with a physical link ID that 
is unique.

in short, I don't see how I could change the code given the differences 
in concepts?