From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933586AbcFQL4h (ORCPT ); Fri, 17 Jun 2016 07:56:37 -0400 Received: from bear.ext.ti.com ([198.47.19.11]:41185 "EHLO bear.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755587AbcFQL4c (ORCPT ); Fri, 17 Jun 2016 07:56:32 -0400 Subject: Re: [PATCH v5 2/2] phy: rockchip-inno-usb2: add a new driver for Rockchip usb2phy To: Frank Wang , , , , , , , , , , References: <1465783810-18756-1-git-send-email-frank.wang@rock-chips.com> <1465783810-18756-3-git-send-email-frank.wang@rock-chips.com> CC: , , , , , , , From: Kishon Vijay Abraham I Message-ID: <5763E506.1060500@ti.com> Date: Fri, 17 Jun 2016 17:24:46 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.2 MIME-Version: 1.0 In-Reply-To: <1465783810-18756-3-git-send-email-frank.wang@rock-chips.com> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, On Monday 13 June 2016 07:40 AM, Frank Wang wrote: > 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 > --- > > Changes in v5: > - Added 'reg' in the data block to match the different phy-blocks in dt. > > Changes in v4: > - Removed some processes related to 'vbus_host-supply'. > > Changes in v3: > - Resolved the mapping defect between fixed value in driver and the property > in devicetree. > - Optimized 480m output clock register function. > - Code cleanup. > > Changes in v2: > - Changed vbus_host operation from gpio to regulator in *_probe. > - Improved the fault treatment relate to 480m clock register. > - Cleaned up some meaningless codes in *_clk480m_disable. > - made more clear the comment of *_sm_work. > > drivers/phy/Kconfig | 7 + > drivers/phy/Makefile | 1 + > drivers/phy/phy-rockchip-inno-usb2.c | 645 ++++++++++++++++++++++++++++++++++ > 3 files changed, 653 insertions(+) > create mode 100644 drivers/phy/phy-rockchip-inno-usb2.c > > diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig > index b869b98..29ef15c 100644 > --- a/drivers/phy/Kconfig > +++ b/drivers/phy/Kconfig > @@ -347,6 +347,13 @@ config PHY_ROCKCHIP_USB > help > Enable this to support the Rockchip USB 2.0 PHY. > > +config PHY_ROCKCHIP_INNO_USB2 > + tristate "Rockchip INNO USB2PHY Driver" > + depends on ARCH_ROCKCHIP && OF depends on COMMON_CLK, since this is also a clock provider. > + select GENERIC_PHY > + help > + Support for Rockchip USB2.0 PHY with Innosilicon IP block. > + > config PHY_ROCKCHIP_EMMC > tristate "Rockchip EMMC PHY Driver" > depends on ARCH_ROCKCHIP && OF > diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile > index 9c3e73c..4963fbc 100644 > --- a/drivers/phy/Makefile > +++ b/drivers/phy/Makefile > @@ -38,6 +38,7 @@ phy-exynos-usb2-$(CONFIG_PHY_S5PV210_USB2) += phy-s5pv210-usb2.o > obj-$(CONFIG_PHY_EXYNOS5_USBDRD) += phy-exynos5-usbdrd.o > obj-$(CONFIG_PHY_QCOM_APQ8064_SATA) += phy-qcom-apq8064-sata.o > obj-$(CONFIG_PHY_ROCKCHIP_USB) += phy-rockchip-usb.o > +obj-$(CONFIG_PHY_ROCKCHIP_INNO_USB2) += phy-rockchip-inno-usb2.o > obj-$(CONFIG_PHY_ROCKCHIP_EMMC) += phy-rockchip-emmc.o > obj-$(CONFIG_PHY_ROCKCHIP_DP) += phy-rockchip-dp.o > obj-$(CONFIG_PHY_QCOM_IPQ806X_SATA) += phy-qcom-ipq806x-sata.o > diff --git a/drivers/phy/phy-rockchip-inno-usb2.c b/drivers/phy/phy-rockchip-inno-usb2.c > new file mode 100644 > index 0000000..fc599c7 > --- /dev/null > +++ b/drivers/phy/phy-rockchip-inno-usb2.c > @@ -0,0 +1,645 @@ > +/* > + * Rockchip USB2.0 PHY with Innosilicon IP block driver > + * > + * Copyright (C) 2016 Fuzhou Rockchip Electronics Co., Ltd > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define BIT_WRITEABLE_SHIFT 16 > +#define SCHEDULE_DELAY (60 * HZ) > + > +enum rockchip_usb2phy_port_id { > + USB2PHY_PORT_OTG, > + USB2PHY_PORT_HOST, > + USB2PHY_NUM_PORTS, > +}; > + > +enum rockchip_usb2phy_host_state { > + PHY_STATE_HS_ONLINE = 0, > + PHY_STATE_DISCONNECT = 1, > + PHY_STATE_HS_CONNECT = 2, > + PHY_STATE_FS_CONNECT = 4, > +}; > + > +struct usb2phy_reg { > + unsigned int offset; > + unsigned int bitend; > + unsigned int bitstart; > + unsigned int disable; > + unsigned int enable; > +}; > + > +/** > + * struct rockchip_usb2phy_port_cfg: usb-phy port configuration. > + * @phy_sus: phy suspend register. > + * @ls_det_en: linestate detection enable register. > + * @ls_det_st: linestate detection state register. > + * @ls_det_clr: linestate detection clear register. > + * @utmi_ls: utmi linestate state register. > + * @utmi_hstdet: utmi host disconnect register. > + */ > +struct rockchip_usb2phy_port_cfg { > + struct usb2phy_reg phy_sus; > + struct usb2phy_reg ls_det_en; > + struct usb2phy_reg ls_det_st; > + struct usb2phy_reg ls_det_clr; > + struct usb2phy_reg utmi_ls; > + struct usb2phy_reg utmi_hstdet; > +}; > + > +/** > + * struct rockchip_usb2phy_cfg: usb-phy configuration. > + * @reg: the address offset of grf for usb-phy config. > + * @num_ports: specify how many ports that the phy has. > + * @clkout_ctl: keep on/turn off output clk of phy. > + */ > +struct rockchip_usb2phy_cfg { > + unsigned int reg; > + unsigned int num_ports; > + struct usb2phy_reg clkout_ctl; > + const struct rockchip_usb2phy_port_cfg port_cfgs[USB2PHY_NUM_PORTS]; > +}; > + > +/** > + * struct rockchip_usb2phy_port: usb-phy port data. > + * @port_id: flag for otg port or host port. > + * @suspended: phy suspended flag. > + * @ls_irq: IRQ number assigned for linestate detection. > + * @mutex: for register updating in sm_work. > + * @sm_work: OTG state machine work. > + * @phy_cfg: port register configuration, assigned by driver data. > + */ > +struct rockchip_usb2phy_port { > + struct phy *phy; > + unsigned int port_id; > + bool suspended; > + int ls_irq; > + struct mutex mutex; > + struct delayed_work sm_work; > + const struct rockchip_usb2phy_port_cfg *port_cfg; > +}; > + > +/** > + * struct rockchip_usb2phy: usb2.0 phy driver data. > + * @grf: General Register Files regmap. > + * @clk: clock struct of phy input clk. > + * @clk480m: clock struct of phy output clk. > + * @clk_hw: clock struct of phy output clk management. > + * @phy_cfg: phy register configuration, assigned by driver data. > + * @ports: phy port instance. > + */ > +struct rockchip_usb2phy { > + struct device *dev; > + struct regmap *grf; > + struct clk *clk; > + struct clk *clk480m; > + struct clk_hw clk480m_hw; > + const struct rockchip_usb2phy_cfg *phy_cfg; > + struct rockchip_usb2phy_port ports[USB2PHY_NUM_PORTS]; > +}; > + > +static inline int property_enable(struct rockchip_usb2phy *rphy, > + const struct usb2phy_reg *reg, bool en) > +{ > + unsigned int val, mask, tmp; > + > + tmp = en ? reg->enable : reg->disable; > + mask = GENMASK(reg->bitend, reg->bitstart); > + val = (tmp << reg->bitstart) | (mask << BIT_WRITEABLE_SHIFT); > + > + return regmap_write(rphy->grf, reg->offset, val); > +} > + > +static inline bool property_enabled(struct rockchip_usb2phy *rphy, > + const struct usb2phy_reg *reg) > +{ > + int ret; > + unsigned int tmp, orig; > + unsigned int mask = GENMASK(reg->bitend, reg->bitstart); > + > + ret = regmap_read(rphy->grf, reg->offset, &orig); > + if (ret) > + return false; > + > + tmp = (orig & mask) >> reg->bitstart; > + return tmp == reg->enable; > +} > + > +static int rockchip_usb2phy_clk480m_enable(struct clk_hw *hw) > +{ > + struct rockchip_usb2phy *rphy = > + container_of(hw, struct rockchip_usb2phy, clk480m_hw); > + int ret; > + > + /* turn on 480m clk output if it is off */ > + if (!property_enabled(rphy, &rphy->phy_cfg->clkout_ctl)) { > + ret = property_enable(rphy, &rphy->phy_cfg->clkout_ctl, true); > + if (ret) > + return ret; > + > + /* waitting for the clk become stable */ > + mdelay(1); > + } > + > + return 0; > +} > + > +static void rockchip_usb2phy_clk480m_disable(struct clk_hw *hw) > +{ > + struct rockchip_usb2phy *rphy = > + container_of(hw, struct rockchip_usb2phy, clk480m_hw); > + > + /* turn off 480m clk output */ > + property_enable(rphy, &rphy->phy_cfg->clkout_ctl, false); > +} > + > +static int rockchip_usb2phy_clk480m_enabled(struct clk_hw *hw) > +{ > + struct rockchip_usb2phy *rphy = > + container_of(hw, struct rockchip_usb2phy, clk480m_hw); > + > + return property_enabled(rphy, &rphy->phy_cfg->clkout_ctl); > +} > + > +static unsigned long > +rockchip_usb2phy_clk480m_recalc_rate(struct clk_hw *hw, > + unsigned long parent_rate) > +{ > + return 480000000; > +} > + > +static const struct clk_ops rockchip_usb2phy_clkout_ops = { > + .enable = rockchip_usb2phy_clk480m_enable, > + .disable = rockchip_usb2phy_clk480m_disable, > + .is_enabled = rockchip_usb2phy_clk480m_enabled, > + .recalc_rate = rockchip_usb2phy_clk480m_recalc_rate, > +}; > + > +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); > + > + if (rphy->clk) > + clk_put(rphy->clk); > +} > + > +static int > +rockchip_usb2phy_clk480m_register(struct rockchip_usb2phy *rphy) > +{ > + struct device_node *node = rphy->dev->of_node; > + struct clk_init_data init; > + const char *clk_name; > + int ret; > + > + init.flags = 0; > + init.name = "clk_usbphy_480m"; > + init.ops = &rockchip_usb2phy_clkout_ops; > + > + /* optional override of the clockname */ > + of_property_read_string(node, "clock-output-names", &init.name); > + > + rphy->clk = of_clk_get_by_name(node, "phyclk"); > + if (IS_ERR(rphy->clk)) { > + rphy->clk = NULL; > + init.parent_names = NULL; > + init.num_parents = 0; > + } else { > + clk_name = __clk_get_name(rphy->clk); > + init.parent_names = &clk_name; > + init.num_parents = 1; > + } > + > + rphy->clk480m_hw.init = &init; > + > + /* register the clock */ > + rphy->clk480m = clk_register(rphy->dev, &rphy->clk480m_hw); > + if (IS_ERR(rphy->clk480m)) { > + ret = PTR_ERR(rphy->clk480m); > + goto err_register; > + } > + > + ret = of_clk_add_provider(node, of_clk_src_simple_get, rphy->clk480m); > + 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 0; > + > +err_unreg_action: > + of_clk_del_provider(node); > +err_clk_provider: > + clk_unregister(rphy->clk480m); > +err_register: > + if (rphy->clk) > + clk_put(rphy->clk); > + return ret; > +} I'm seeing lot of similarities specifically w.r.t to clock handling part in drivers/phy/phy-rockchip-usb.c. Why not just re-use that driver? > + > +static int rockchip_usb2phy_init(struct phy *phy) > +{ > + struct rockchip_usb2phy_port *rport = phy_get_drvdata(phy); > + struct rockchip_usb2phy *rphy = dev_get_drvdata(phy->dev.parent); > + int ret; > + > + if (rport->port_id == USB2PHY_PORT_HOST) { > + /* clear linestate and enable linestate detect irq */ > + mutex_lock(&rport->mutex); > + > + ret = property_enable(rphy, &rport->port_cfg->ls_det_clr, true); > + if (ret) { > + mutex_unlock(&rport->mutex); > + return ret; > + } > + > + ret = property_enable(rphy, &rport->port_cfg->ls_det_en, true); > + if (ret) { > + mutex_unlock(&rport->mutex); > + return ret; > + } > + > + mutex_unlock(&rport->mutex); > + schedule_delayed_work(&rport->sm_work, SCHEDULE_DELAY); > + } > + > + return 0; > +} > + > +static int rockchip_usb2phy_resume(struct phy *phy) > +{ > + struct rockchip_usb2phy_port *rport = phy_get_drvdata(phy); > + struct rockchip_usb2phy *rphy = dev_get_drvdata(phy->dev.parent); > + int ret; > + > + dev_dbg(&rport->phy->dev, "port resume\n"); > + > + ret = clk_prepare_enable(rphy->clk480m); > + if (ret) > + return ret; > + > + ret = property_enable(rphy, &rport->port_cfg->phy_sus, false); > + if (ret) > + return ret; > + > + rport->suspended = false; > + return 0; > +} > + > +static int rockchip_usb2phy_suspend(struct phy *phy) > +{ > + struct rockchip_usb2phy_port *rport = phy_get_drvdata(phy); > + struct rockchip_usb2phy *rphy = dev_get_drvdata(phy->dev.parent); > + int ret; > + > + dev_dbg(&rport->phy->dev, "port suspend\n"); > + > + ret = property_enable(rphy, &rport->port_cfg->phy_sus, true); > + if (ret) > + return ret; > + > + rport->suspended = true; > + clk_disable_unprepare(rphy->clk480m); > + return 0; > +} > + > +static int rockchip_usb2phy_exit(struct phy *phy) > +{ > + struct rockchip_usb2phy_port *rport = phy_get_drvdata(phy); > + > + if (rport->port_id == USB2PHY_PORT_HOST) > + cancel_delayed_work_sync(&rport->sm_work); > + > + return 0; > +} > + > +static const struct phy_ops rockchip_usb2phy_ops = { > + .init = rockchip_usb2phy_init, > + .exit = rockchip_usb2phy_exit, > + .power_on = rockchip_usb2phy_resume, > + .power_off = rockchip_usb2phy_suspend, > + .owner = THIS_MODULE, > +}; > + > +/* > + * 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 may invoke *phy_suspend or *phy_resume which will invoke some > + * clk related APIs, so do not invoke it from interrupt context directly. > + */ > +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); Why are you scheduling the work again? Interrupt handlers can invoke this right? > +} > + > +static irqreturn_t rockchip_usb2phy_linestate_irq(int irq, void *data) > +{ > + struct rockchip_usb2phy_port *rport = data; > + struct rockchip_usb2phy *rphy = dev_get_drvdata(rport->phy->dev.parent); > + > + if (!property_enabled(rphy, &rport->port_cfg->ls_det_st)) > + return IRQ_NONE; > + > + mutex_lock(&rport->mutex); > + > + /* disable linestate detect irq and clear its status */ > + property_enable(rphy, &rport->port_cfg->ls_det_en, false); > + property_enable(rphy, &rport->port_cfg->ls_det_clr, true); > + > + mutex_unlock(&rport->mutex); > + > + /* > + * In this case for host phy port, a new device is plugged in, > + * meanwhile, if the phy port is suspended, we need rearm the work to > + * resume it and mange its states; otherwise, we do nothing about that. > + */ > + if (rport->suspended && rport->port_id == USB2PHY_PORT_HOST) > + rockchip_usb2phy_sm_work(&rport->sm_work.work); > + > + return IRQ_HANDLED; > +} > + > +static int rockchip_usb2phy_host_port_init(struct rockchip_usb2phy *rphy, > + struct rockchip_usb2phy_port *rport, > + struct device_node *child_np) > +{ > + int ret; > + > + rport->port_id = USB2PHY_PORT_HOST; > + rport->port_cfg = &rphy->phy_cfg->port_cfgs[USB2PHY_PORT_HOST]; > + > + mutex_init(&rport->mutex); > + INIT_DELAYED_WORK(&rport->sm_work, rockchip_usb2phy_sm_work); > + > + rport->ls_irq = of_irq_get_byname(child_np, "linestate"); > + if (rport->ls_irq < 0) { > + dev_err(rphy->dev, "no linestate irq provided\n"); > + return rport->ls_irq; > + } > + > + ret = devm_request_threaded_irq(rphy->dev, rport->ls_irq, NULL, > + rockchip_usb2phy_linestate_irq, > + IRQF_ONESHOT, > + "rockchip_usb2phy", rport); > + if (ret) { > + dev_err(rphy->dev, "failed to request irq handle\n"); > + return ret; > + } > + > + return 0; > +} > + > +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 rockchip_usb2phy_cfg *phy_cfgs; > + const struct of_device_id *match; > + unsigned int reg; > + 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 -EINVAL; > + > + rphy->grf = syscon_node_to_regmap(dev->parent->of_node); > + if (IS_ERR(rphy->grf)) > + return PTR_ERR(rphy->grf); > + > + if (of_property_read_u32(np, "reg", ®)) { > + dev_err(dev, "the reg property is not assigned in %s node\n", > + np->name); > + return -EINVAL; > + } > + > + rphy->dev = dev; > + phy_cfgs = match->data; > + platform_set_drvdata(pdev, rphy); > + > + /* find out a proper config which can be matched with dt. */ > + index = 0; > + while (phy_cfgs[index].reg) { > + if (phy_cfgs[index].reg == reg) { Why not pass these config values from dt? Moreover finding the config using register offset is bound to break. Thanks Kishon