[...] > >> @@ -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. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html