On Sun, May 28, 2017 at 9:40 PM, Hauke Mehrtens <hauke@xxxxxxxxxx> wrote:
> This driver starts the DWC2 core(s) built into the XWAY SoCs and provides
> the PHY interfaces for each core. The phy instances can be passed to the
> dwc2 driver, which already supports the generic phy interface.
> +static void ltq_rcu_usb2_start_cores(struct platform_device *pdev)
It should return int. See below.
> +{
> + /* Power on the USB core. */
> + if (clk_prepare_enable(priv->ctrl_gate_clk)) {
You basically shadow the error, why?
> + dev_err(dev, "failed to enable CTRL gate\n");
> + return;
> + }
> + if (clk_prepare_enable(priv->phy_gate_clk)) {
> + dev_err(dev, "failed to enable PHY gate\n");
> + return;
> + }
Ditto.
> +static int ltq_rcu_usb2_of_probe(struct device_node *phynode,
> + struct ltq_rcu_usb2_priv *priv)
> +{
> + struct device *dev = priv->dev;
> + const struct of_device_id *match =
> + of_match_node(ltq_rcu_usb2_phy_of_match, phynode);
> + int ret;
> +
> + if (!match) {
> + dev_err(dev, "Not a compatible Lantiq RCU USB PHY\n");
> + return -EINVAL;
> + }
Can it ever happen?
> +
> + priv->reg_bits = match->data;
I think there is a helper to get driver data directly from node.
> + if (priv->reg_bits->have_ana_cfg) {
> + ret = device_property_read_u32(dev, "offset-ana",
> + &priv->ana_cfg1_reg_offset);
> + if (ret) {
> + dev_dbg(dev, "Failed to get RCU ANA CFG1 reg offset\n");
> + return ret;
> + }
> + }
ret = device_property_...(...);
if (ret && priv->reg_bits->have_ana_cfg) {
...
return ret;
}
?
> + priv->dev = &pdev->dev;
> + dev_set_drvdata(priv->dev, priv);
Move this to the end of function. Ideally it should be run if and only
if the function returns 0.
> + provider = devm_of_phy_provider_register(&pdev->dev,
> + of_phy_simple_xlate);
> +
> + return PTR_ERR_OR_ZERO(provider);
I would do explicitly, though it's up to you and maintainer.
if (IS_ERR(provider))
return PTR_ERR();
return 0;
--
With Best Regards,
Andy Shevchenko
