Hi Matthias, On 06.02.24 18:55, Matthias Kaehlcke wrote: > Hi Javier, > > a few comments inline > > On Tue, Feb 06, 2024 at 02:59:29PM +0100, Javier Carrasco wrote: >> +static struct onboard_dev *_find_onboard_dev(struct device *dev) >> +{ >> + struct platform_device *pdev; >> + struct device_node *np; >> + struct onboard_dev *onboard_dev; >> + >> + pdev = of_find_device_by_node(dev->of_node); >> + if (!pdev) { >> + np = of_parse_phandle(dev->of_node, "peer-hub", 0); >> + if (!np) { >> + dev_err(dev, "failed to find device node for peer hub\n"); >> + return ERR_PTR(-EINVAL); >> + } >> + >> + pdev = of_find_device_by_node(np); >> + of_node_put(np); >> + >> + if (!pdev) >> + return ERR_PTR(-ENODEV); >> + } > > The above branch should probably be guarded by 'if (!onboard_dev->pdata->is_hub)', > this is also a change for ""usb: misc: onboard_dev: add support for non-hub devices" > I am not sure how to guard the branch like that because onboard_dev is retrieved by means of pdev->dev, which is not available if of_find_device_by_node returns NULL. The non-hub device will not have a peer-hub property according to its bindings anyway, right? Thanks again for your feedback and best regards, Javier Carrasco