Re: [PATCH v2 1/8] media: Rename the IPU PCI device table header and add IPU7 PCI IDs

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

Hi, Sakari,

On 5/28/25 7:37 PM, Sakari Ailus wrote:
> Hi Bingbu,
> 
> On Wed, May 28, 2025 at 05:18:54PM +0800, Bingbu Cao wrote:
>> Sakari,
>>
>> On 5/28/25 3:51 PM, bingbu.cao@intel.com wrote:
>>> From: Bingbu Cao <bingbu.cao@intel.com>
>>>
>>> Rename the IPU PCI device table header file and add the IPU7 PCI device
>>> IDs.
>>>
>>> Signed-off-by: Bingbu Cao <bingbu.cao@intel.com>
>>> ---
>>>  drivers/media/pci/intel/ipu6/ipu6.c                |  2 +-
>>>  drivers/media/pci/intel/ivsc/mei_csi.c             |  2 +-
>>>  .../media/{ipu6-pci-table.h => ipu-pci-table.h}    | 14 +++++++++++---
>>>  3 files changed, 13 insertions(+), 5 deletions(-)
>>>  rename include/media/{ipu6-pci-table.h => ipu-pci-table.h} (68%)
>>>
>>> diff --git a/drivers/media/pci/intel/ipu6/ipu6.c b/drivers/media/pci/intel/ipu6/ipu6.c
>>> index 277af7cda8ee..aa859220ba94 100644
>>> --- a/drivers/media/pci/intel/ipu6/ipu6.c
>>> +++ b/drivers/media/pci/intel/ipu6/ipu6.c
>>> @@ -21,7 +21,7 @@
>>>  #include <linux/types.h>
>>>  
>>>  #include <media/ipu-bridge.h>
>>> -#include <media/ipu6-pci-table.h>
>>> +#include <media/ipu-pci-table.h>
>>>  
>>>  #include "ipu6.h"
>>>  #include "ipu6-bus.h"
>>> diff --git a/drivers/media/pci/intel/ivsc/mei_csi.c b/drivers/media/pci/intel/ivsc/mei_csi.c
>>> index 92d871a378ba..47fc48c7619b 100644
>>> --- a/drivers/media/pci/intel/ivsc/mei_csi.c
>>> +++ b/drivers/media/pci/intel/ivsc/mei_csi.c
>>> @@ -27,7 +27,7 @@
>>>  #include <linux/workqueue.h>
>>>  
>>>  #include <media/ipu-bridge.h>
>>> -#include <media/ipu6-pci-table.h>
>>> +#include <media/ipu-pci-table.h>
>>>  #include <media/v4l2-async.h>
>>>  #include <media/v4l2-ctrls.h>
>>>  #include <media/v4l2-fwnode.h>
>>> diff --git a/include/media/ipu6-pci-table.h b/include/media/ipu-pci-table.h
>>> similarity index 68%
>>> rename from include/media/ipu6-pci-table.h
>>> rename to include/media/ipu-pci-table.h
>>> index 0899d9d2f978..528080c6f742 100644
>>> --- a/include/media/ipu6-pci-table.h
>>> +++ b/include/media/ipu-pci-table.h
>>> @@ -3,8 +3,8 @@
>>>   * Copyright (C) 2024 Intel Corporation
>>>   */
>>>  
>>> -#ifndef __IPU6_PCI_TBL_H__
>>> -#define __IPU6_PCI_TBL_H__
>>> +#ifndef __IPU_PCI_TBL_H__
>>> +#define __IPU_PCI_TBL_H__
>>>  
>>>  #include <linux/pci.h>
>>>  
>>> @@ -15,6 +15,9 @@
>>>  #define PCI_DEVICE_ID_INTEL_IPU6EP_RPLP		0xa75d
>>>  #define PCI_DEVICE_ID_INTEL_IPU6EP_MTL		0x7d19
>>>  
>>> +#define PCI_DEVICE_ID_INTEL_IPU7_PCI_ID		0x645d
>>> +#define PCI_DEVICE_ID_INTEL_IPU7P5_PCI_ID	0xb05d
>>> +
>>>  static const struct pci_device_id ipu6_pci_tbl[] = {
>>>  	{ PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_IPU6) },
>>>  	{ PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_IPU6SE) },
>>> @@ -25,4 +28,9 @@ static const struct pci_device_id ipu6_pci_tbl[] = {
>>>  	{ }
>>>  };
>>>  
>>> -#endif /* __IPU6_PCI_TBL_H__ */
>>> +static const struct pci_device_id ipu7_pci_tbl[] = {
>>
>> I got media-ci build errors:
>> In file included from drivers/media/pci/intel/ivsc/mei_csi.c:30:
>> ./include/media/ipu-pci-table.h:31:35: warning: ‘ipu7_pci_tbl’
>> defined but not used [-Wunused-const-variable=]
>>
>> Do you think is it OK using 1 table to cover IPU6 and IPU7?
> 
> Static const tables in a header isn't that great to begin with, but the
> alternative would be another module which is way overkill. Two headers
> would make sense in this case I think -- the tables are separate in any
> case.

ivsc is another user of the IPU6 table, but I don't see that other driver
need IPU7 table so far. So I want to keep the ipu7 table private without
touching the table.h (Drop 1/8).

> 
>>
>>> +	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_IPU7_PCI_ID)},
>>> +	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_IPU7P5_PCI_ID)},
>>> +	{ }
>>> +};
>>> +#endif /* __IPU_PCI_TBL_H__ */
>>>
>>
> 

-- 
Best regards,
Bingbu Cao