Hi Frank, Am Dienstag, 31. Mai 2016, 14:40:11 schrieb Frank Wang: > The newer SoCs (rk3366, rk3399) take a different usb-phy IP block > than rk3288 and before, and most of phy-related registers are also > different from the past, so a new phy driver is required necessarily. > > Signed-off-by: Frank Wang <frank.wang@xxxxxxxxxxxxxx> > --- [...] > +static void rockchip_usb2phy_clk480m_disable(struct clk_hw *hw) > +{ > + struct rockchip_usb2phy *rphy = > + container_of(hw, struct rockchip_usb2phy, clk480m_hw); > + int index; > + > + /* make sure all ports in suspended mode */ > + for (index = 0; index != rphy->phy_cfg->num_ports; index++) > + if (!rphy->ports[index].suspended) > + return; This function can only get called when all clk-references have disabled the clock, so you should never reach that point that one phy port is not suspended? > + > + /* turn off 480m clk output */ > + property_enable(rphy, &rphy->phy_cfg->clkout_ctl, false); > +} > + [...] add something like: static void rockchip_usb2phy_clk480m_unregister(void *data) { struct rockchip_usb2phy *rphy = data; of_clk_del_provider(rphy->dev->of_node); clk_unregister(rphy->clk480m); } > +static struct clk * > +rockchip_usb2phy_clk480m_register(struct rockchip_usb2phy *rphy) > +{ > + struct device_node *node = rphy->dev->of_node; > + struct clk *clk; > + struct clk_init_data init; int ret; > + > + init.name = "clk_usbphy_480m"; > + init.ops = &rockchip_usb2phy_clkout_ops; > + init.flags = CLK_IS_ROOT; > + init.parent_names = NULL; > + init.num_parents = 0; > + rphy->clk480m_hw.init = &init; > + > + /* optional override of the clockname */ > + of_property_read_string(node, "clock-output-names", &init.name); > + > + /* register the clock */ > + clk = clk_register(rphy->dev, &rphy->clk480m_hw); if (IS_ERR(clk)) return clk: ret = of_clk_add_provider(node, of_clk_src_simple_get, clk); if (ret < 0) goto err_clk_provider; ret = devm_add_action(rphy->dev, rockchip_usb2phy_clk480m_unregister, rphy); if (ret < 0) goto err_unreg_action; return clk; err_unreg_action: of_clk_del_provider(node); err_clk_provider: clk_unregister(clk); return ERR_PTR(ret); > +} [...] > +/* > + * The function manage host-phy port state and suspend/resume phy port > + * to save power. > + * > + * we rely on utmi_linestate and utmi_hostdisconnect to identify whether > + * FS/HS is disconnect or not. Besides, we do not need care it is FS > + * disconnected or HS disconnected, actually, we just only need get the > + * device is disconnected at last through rearm the delayed work, > + * to suspend the phy port in _PHY_STATE_DISCONNECT_ case. > + * > + * NOTE: It will invoke some clk related APIs, so do not invoke it from > + * interrupt context. This does not seem to match the code, as I don't see any clk_* calls, > + */ > +static void rockchip_usb2phy_sm_work(struct work_struct *work) > +{ > + struct rockchip_usb2phy_port *rport = > + container_of(work, struct rockchip_usb2phy_port, sm_work.work); > + struct rockchip_usb2phy *rphy = dev_get_drvdata(rport->phy->dev.parent); > + unsigned int sh = rport->port_cfg->utmi_hstdet.bitend - > + rport->port_cfg->utmi_hstdet.bitstart + 1; > + unsigned int ul, uhd, state; > + unsigned int ul_mask, uhd_mask; > + int ret; > + > + mutex_lock(&rport->mutex); > + > + ret = regmap_read(rphy->grf, rport->port_cfg->utmi_ls.offset, &ul); > + if (ret < 0) > + goto next_schedule; > + > + ret = regmap_read(rphy->grf, rport->port_cfg->utmi_hstdet.offset, > + &uhd); > + if (ret < 0) > + goto next_schedule; > + > + uhd_mask = GENMASK(rport->port_cfg->utmi_hstdet.bitend, > + rport->port_cfg->utmi_hstdet.bitstart); > + ul_mask = GENMASK(rport->port_cfg->utmi_ls.bitend, > + rport->port_cfg->utmi_ls.bitstart); > + > + /* stitch on utmi_ls and utmi_hstdet as phy state */ > + state = ((uhd & uhd_mask) >> rport->port_cfg->utmi_hstdet.bitstart) | > + (((ul & ul_mask) >> rport->port_cfg->utmi_ls.bitstart) << sh); > + > + switch (state) { > + case PHY_STATE_HS_ONLINE: > + dev_dbg(&rport->phy->dev, "HS online\n"); > + break; > + case PHY_STATE_FS_CONNECT: > + /* > + * For FS device, the online state share with connect state > + * from utmi_ls and utmi_hstdet register, so we distinguish > + * them via suspended flag. > + */ > + if (!rport->suspended) { > + dev_dbg(&rport->phy->dev, "FS online\n"); > + break; > + } > + /* fall through */ > + case PHY_STATE_HS_CONNECT: > + if (rport->suspended) { > + dev_dbg(&rport->phy->dev, "HS/FS connected\n"); > + rockchip_usb2phy_resume(rport->phy); > + rport->suspended = false; > + } > + break; > + case PHY_STATE_DISCONNECT: > + if (!rport->suspended) { > + dev_dbg(&rport->phy->dev, "HS/FS disconnected\n"); > + rockchip_usb2phy_suspend(rport->phy); > + rport->suspended = true; > + } > + > + /* > + * activate the linestate detection to get the next device > + * plug-in irq. > + */ > + property_enable(rphy, &rport->port_cfg->ls_det_clr, true); > + property_enable(rphy, &rport->port_cfg->ls_det_en, true); > + > + /* > + * we don't need to rearm the delayed work when the phy port > + * is suspended. > + */ > + mutex_unlock(&rport->mutex); > + return; > + default: > + dev_dbg(&rport->phy->dev, "unknown phy state\n"); > + break; > + } > + > +next_schedule: > + mutex_unlock(&rport->mutex); > + schedule_delayed_work(&rport->sm_work, SCHEDULE_DELAY); > +} [...] > +static int rockchip_usb2phy_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct device_node *np = dev->of_node; > + struct device_node *child_np; > + struct phy_provider *provider; > + struct rockchip_usb2phy *rphy; > + const struct of_device_id *match; > + int index, ret; > + > + rphy = devm_kzalloc(dev, sizeof(*rphy), GFP_KERNEL); > + if (!rphy) > + return -ENOMEM; > + > + match = of_match_device(dev->driver->of_match_table, dev); > + if (!match || !match->data) { > + dev_err(dev, "phy configs are not assigned!\n"); > + return -EINVAL; > + } > + > + if (!dev->parent || !dev->parent->of_node) > + return -ENOMEM; > + > + rphy->grf = syscon_node_to_regmap(dev->parent->of_node); > + if (IS_ERR(rphy->grf)) > + return PTR_ERR(rphy->grf); > + > + rphy->dev = dev; > + rphy->phy_cfg = match->data; > + platform_set_drvdata(pdev, rphy); > + > + rphy->clk480m = rockchip_usb2phy_clk480m_register(rphy); > + if (IS_ERR(rphy->clk480m)) > + return PTR_ERR(rphy->clk480m); > + > + rphy->vbus_host_gpio = > + devm_gpiod_get_optional(dev, "vbus_host", GPIOD_OUT_HIGH); > + if (!rphy->vbus_host_gpio) > + dev_info(dev, "host_vbus is not assigned!\n"); > + else if (IS_ERR(rphy->vbus_host_gpio)) > + return PTR_ERR(rphy->vbus_host_gpio); > + > + index = 0; > + for_each_child_of_node(np, child_np) { > + struct rockchip_usb2phy_port *rport = &rphy->ports[index]; > + struct phy *phy; > + > + phy = devm_phy_create(dev, child_np, &rockchip_usb2phy_ops); > + if (IS_ERR(phy)) { > + dev_err(dev, "failed to create phy\n"); > + ret = PTR_ERR(phy); > + goto put_child; > + } > + > + rport->phy = phy; > + rport->port_cfg = &rphy->phy_cfg->port_cfgs[index]; > + > + /* initialize otg/host port separately */ > + if (!of_node_cmp(child_np->name, "host-port")) { > + ret = rockchip_usb2phy_host_port_init(rphy, rport, > + child_np); > + if (ret) > + goto put_child; > + } > + > + phy_set_drvdata(rport->phy, rport); > + index++; > + } > + > + provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate); > + return PTR_ERR_OR_ZERO(provider); > + > +put_child: > + of_node_put(child_np); > + of_clk_del_provider(np); > + clk_unregister(rphy->clk480m); these two can go away, once you have the devm_action described near your clk_register function. > + return ret; > +} Heiko -- 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