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

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

Andreas,

On 10/3/23 6:13 PM, Andreas Helbech Kleist wrote:
> Hi,
> 
> On Thu, 2023-07-27 at 15:15 +0800, bingbu.cao@intel.com wrote:
>> From: Bingbu Cao <bingbu.cao@intel.com>
>>
>> Input system driver do basic isys hardware setup and irq handling
>> and work with fwnode and v4l2 to register the ISYS v4l2 devices.
> 
>> +       media_device_pci_init(&isys->media_dev,
>> +                             pdev, IPU6_MEDIA_DEV_MODEL_NAME);
>> +
>> +       strscpy(isys->v4l2_dev.name, isys->media_dev.model,
>> +               sizeof(isys->v4l2_dev.name));
>> +
>> +       ret = media_device_register(&isys->media_dev);
>> +       if (ret < 0)
>> +               goto out_media_device_unregister;
> ...
>> +out_media_device_unregister:
>> +       media_device_unregister(&isys->media_dev);
>> +       media_device_cleanup(&isys->media_dev);
> 
> You should only call media_device_cleanup() if media_device_register()
> fails.
> 
> 
>> +static const struct pci_device_id isys_pci_tbl[] = {
>> +       { PCI_VDEVICE(INTEL, IPU6_PCI_ID) },
>> +       { PCI_VDEVICE(INTEL, IPU6SE_PCI_ID) },
>> +       { PCI_VDEVICE(INTEL, IPU6EP_ADL_P_PCI_ID) },
>> +       { PCI_VDEVICE(INTEL, IPU6EP_ADL_N_PCI_ID) },
>> +       { PCI_VDEVICE(INTEL, IPU6EP_RPL_P_PCI_ID) },
>> +       { PCI_VDEVICE(INTEL, IPU6EP_MTL_PCI_ID) },
>> +       { }
>> +};
> 
> Unused

Have you tried that whether isys driver can be auto-loaded w/o
this pci id table? It cannot on my side.

> 
>> +static const struct auxiliary_device_id ipu6_isys_id_table[] = {
>> +       {
>> +               .name = "intel_ipu6.isys",
>> +               .driver_data =
>> (kernel_ulong_t)&ipu6_isys_auxdrv_data,
>> +       },
>> +};
> 
> Missing sentinel {}.
> 
> 
> Best regards,
> Andreas
> 

-- 
Best regards,
Bingbu Cao