Hans,
On 11/5/23 10:43 PM, Hans de Goede wrote:
> Hi,
>
>
> On 10/24/23 13:29, 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.
>>
>> Signed-off-by: Bingbu Cao <bingbu.cao@xxxxxxxxx>
>> Reported-by: Hans de Goede <hdegoede@xxxxxxxxxx>
>> Suggested-by: Andreas Helbech Kleist <andreaskleist@xxxxxxxxx>
>
> Thank you. Just one small remark, not a full review:
>
> <snip>
>
>> diff --git a/drivers/media/pci/intel/ipu6/ipu6.c b/drivers/media/pci/intel/ipu6/ipu6.c
>> new file mode 100644
>> index 000000000000..84960a283daf
>> --- /dev/null
>> +++ b/drivers/media/pci/intel/ipu6/ipu6.c
>
> <snip>
>
>> +static struct ipu6_bus_device *
>> +ipu6_isys_init(struct pci_dev *pdev, struct device *parent,
>> + struct ipu6_buttress_ctrl *ctrl, void __iomem *base,
>> + const struct ipu6_isys_internal_pdata *ipdata)
>> +{
>> + struct fwnode_handle *fwnode = dev_fwnode(&pdev->dev);
>> + struct ipu6_bus_device *isys_adev;
>> + struct ipu6_isys_pdata *pdata;
>> + int ret;
>> +
>> + /* check fwnode at first, fallback into bridge if no fwnode graph */
>> + ret = ipu6_isys_check_fwnode_graph(fwnode);
>> + if (ret) {
>> + if (fwnode && !IS_ERR_OR_NULL(fwnode->secondary)) {
>> + dev_err(&pdev->dev,
>> + "fwnode graph has no endpoints connection\n");
>> + return ERR_PTR(-EINVAL);
>> + }
>> +
>> + ret = ipu_bridge_init(&pdev->dev, ipu_bridge_parse_ssdb);
>> + if (ret) {
>> + if (ret != -EPROBE_DEFER)
>> + dev_err_probe(&pdev->dev, ret,
>> + "IPU6 bridge init failed\n");
>
> dev_err_probe() already silences logging of -EPROBE_DEFER errors itself,
> so this "if (ret != -EPROBE_DEFER)" guard here is not necessary.
Good catch, thanks.
>
> Regards,
>
> Hans
>
>
>
>
--
Best regards,
Bingbu Cao
