Re: [PATCH 10/15] media: intel/ipu6: add input system driver

[Bingbu Cao <bingbu.cao@linux.intel.com>]

Hans,

>>>> but I think it
>>>> is correct for IPU6 as well. The reason is that isys_driver is an
>>>> auxiliary_driver, so I don't think 
>>>>
>>>>     MODULE_DEVICE_TABLE(pci, isys_pci_tbl);
>>>>
>>>> has any effect. The PCI probe happens in ipu6_pci_probe in ipu6.c
>>>> (because it has a pci_device_id table as well), and the isys_driver
>>>> is
>>>> probed indirectly by ipu6-bus.c.
>>>
>>> So the MODULE_DEVICE_TABLE(pci, isys_pci_tbl) indeed does not
>>> belong in this auxbus driver, instead it should use some sort
>>> of auxbus MODULE_DEVICE_TABLE() to autoload based on its
>>> auxbus modalias.
>>>
>>> But it does have an effect, modprobe will load both the main
>>> ipu6 driver registering the aux devices as well as this driver
>>> based on the modalias of the PCI device because with this
>>> MODULE_DEVICE_TABLE(pci, isys_pci_tbl); statement both drivers
>>> match that PCI modalias.
>>
>> All right. But since the main driver contains the same table, I don't
>> think there's any need to have it here?
>>
>>> But the correct thing to do here would be to switch to
>>> an auxbus based MODULE_DEVICE_TABLE() for the isys driver.
>>
>> The isys_driver already has an auxiliary_device_id table. I'm not sure
>> if that's what you mean?
>>
>> From the bottom of ipu6-isys.c in PATCH 10/15:
>>
>> +static const struct auxiliary_device_id ipu6_isys_id_table[] = {
>> +	{
>> +		.name = "intel_ipu6.isys",
>> +		.driver_data = (kernel_ulong_t)&ipu6_isys_auxdrv_data,
>> +	},
>> +};
> 
> Right, so this needs a:
> 
> MODULE_DEVICE_TABLE(auxiliary, ipu6_isys_id_table);

Ack, thanks. I will send v2 this week including the fixes.
> 
> And then the:
> 
> MODULE_DEVICE_TABLE(pci, isys_pci_tbl)
> 
> and any other mention of isys_pci_tbl can be dropped.
> 
> Regards,
> 
> Hans
> 

-- 
Best regards,
Bingbu Cao