[V5 PATCH 26/26] usb: ehci: ehci-mv: add device tree support

[mark.rutland@arm.com (Mark Rutland)]

[...]

> >> @@ -170,19 +202,36 @@ static int mv_ehci_probe(struct platform_device *pdev)
> >>       }
> >>
> >>       platform_set_drvdata(pdev, ehci_mv);
> >> -     ehci_mv->pdata = pdata;
> >>       ehci_mv->hcd = hcd;
> >>
> >> -     ehci_mv->clknum = pdata->clknum;
> >> -     for (clk_i = 0; clk_i < ehci_mv->clknum; clk_i++) {
> >> -             ehci_mv->clk[clk_i] =
> >> -                 devm_clk_get(&pdev->dev, pdata->clkname[clk_i]);
> >> -             if (IS_ERR(ehci_mv->clk[clk_i])) {
> >> -                     dev_err(&pdev->dev, "error get clck \"%s\"\n",
> >> -                             pdata->clkname[clk_i]);
> >> -                     retval = PTR_ERR(ehci_mv->clk[clk_i]);
> >> -                     goto err_clear_drvdata;
> >> +     retval = mv_ehci_parse_dt(pdev, ehci_mv);
> >> +     if (retval > 0) {
> >
> > Is this why you returned 1 from mv_ehci_parse_dt? So you only do this if you
> > don't have a dt node?
> >
> > If so, why not rip the check (and positive error code) out of mv_ehci_parse_dt,
> > and then here:
> >
> > if (pdev->dev.of_node) {
> >         retval = mv_ehci_parse_dt(pdev, ehci_mv);
> > } else
> >         fall back to mv_usb_platform_data ...
> > }
> >
> > That makes it obvious that one side depends on dt and the other's a fallback,
> > and doesn't rely on nonstandard return code behaviour.
> >
> 
> I will change it.
> 
> > Also, why not return immediately if mv_ehci_parse_dt fails?
> >
> I do not understand. if mv_ehci_parse_dt returns negative value, the
> probe will be finished immediately.

Yes, you're right. I got myself confused about the logic.

Thanks,
Mark.