Hi Bingbu,
Thank your for the patch series. I'm working on adding support for IPU4
(as Claus Stovgaard mentioned) and have run into various issues,
resulting in these comments.
On Thu, 2023-07-27 at 15:15 +0800, bingbu.cao@xxxxxxxxx wrote:
> From: Bingbu Cao <bingbu.cao@xxxxxxxxx>
>
> Intel Image Processing Unit 6th Gen includes input and processing
> systems
> but the hardware presents itself as a single PCI device in system.
>
> IPU6 PCI device driver basically does PCI configurations and load
> the firmware binary, initialises IPU virtual bus, and sets up
> platform
> specific variants to support multiple IPU6 devices in single device
> driver.
> +extern struct ipu6_isys_internal_pdata isys_ipdata;
> +extern struct ipu6_psys_internal_pdata psys_ipdata;
> +extern const struct ipu6_buttress_ctrl isys_buttress_ctrl;
> +extern const struct ipu6_buttress_ctrl psys_buttress_ctrl;
They are only used internally in ipu6.c, so no need for extern
declarations.
> +static int ipu6_pci_probe(struct pci_dev *pdev, const struct
> pci_device_id *id)
> +{
...
> + isp->bus_ready_to_probe = true;
This variable is written but never read.
> +static void ipu6_pci_remove(struct pci_dev *pdev)
> +{
...
> + ipu6_bus_del_devices(pdev);
...
> + ipu6_mmu_cleanup(isp->psys->mmu);
> + ipu6_mmu_cleanup(isp->isys->mmu);
I think ipu6_mmu_cleanup() should be done before ipu6_bus_del_devices()
like in the ipu6_pci_probe() error path.
> +struct ipu6_device {
...
> + bool bus_ready_to_probe;
Remove unused variable (see comment earlier).
I'm looking forward to the next version of the patch series.
Best Regards,
Andreas
