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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 10/20/23 12:47, Andreas Helbech Kleist wrote:
> Hi Bingbu,
> 
> On Thu, 2023-10-19 at 16:28 +0800, Bingbu Cao wrote:
>> On 10/3/23 6:13 PM, Andreas Helbech Kleist wrote:
>>> On Thu, 2023-07-27 at 15:15 +0800, bingbu.cao@xxxxxxxxx wrote:
>>>
>>>> +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.
> 
> I've only tried it on my WIP IPU4 hack of this driver

Oh a IPU4 version of this driver would be very interesting,
I was actually thinking about looking into this myself
because I have a Chuwi Hi13 tablet which uses the IPU4 and
it would be nice to get the camera working there (and on
other IPU4 using devices).

Can you give some more info on your IPU4 work, e.g. :

1. Does it work?
2. Which sensor are you using it with ?
3. Which device are you testing on ?
4. Do you have a public git branch with this somewhere ?

I'm afraid that I won't have time to look into this
myself anytime soon, but I'm very interested in this.

> 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.

But the correct thing to do here would be to switch to
an auxbus based MODULE_DEVICE_TABLE() for the isys driver.

Regards,

Hans







[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux