Re: [v2 PATCH 2/4] phy: Add USB Type-C PHY driver for rk3399

From: Chris Zhong
Date: Sun Jun 19 2016 - 20:33:25 EST


Hi Kishon

On 06/17/2016 08:54 PM, Kishon Vijay Abraham I wrote:

On Monday 13 June 2016 03:09 PM, Chris Zhong wrote:
Add a PHY provider driver for the rk3399 SoC Type-c PHY. The USB
Type-C PHY is designed to support the USB3 and DP applications. The
PHY basically has two main components: USB3 and DisplyPort. USB3
operates in SuperSpeed mode and the DP can operate at RBR, HBR and
HBR2 data rates.

Signed-off-by: Chris Zhong <zyw@xxxxxxxxxxxxxx>
Signed-off-by: Kever Yang <kever.yang@xxxxxxxxxxxxxx>

---

Changes in v2:
- select RESET_CONTROLLER
- alphabetic order
- modify some spelling mistakes
- make mode cleaner
- use bool for enable/disable
- check all of the return value
- return a better err number
- use more readx_poll_timeout()
- clk_disable_unprepare(tcphy->clk_ref);
- remove unuse functions, rockchip_typec_phy_power_on/off
- remove unnecessary typecast from void *
- use dts node to distinguish between phys.

Changes in v1:
- update the licence note
- init core clock to 50MHz
- use extcon API
- remove unused global
- add some comments for magic num
- change usleep_range(1000, 2000) tousleep_range(1000, 1050)
- remove __func__ from dev_err
- return err number when get clk failed
- remove ADDR_ADJ define
- use devm_clk_get(&pdev->dev, "tcpdcore")

drivers/phy/Kconfig | 8 +
drivers/phy/Makefile | 1 +
drivers/phy/phy-rockchip-typec.c | 952 +++++++++++++++++++++++++++++++++
include/linux/phy/phy-rockchip-typec.h | 20 +
4 files changed, 981 insertions(+)
create mode 100644 drivers/phy/phy-rockchip-typec.c
create mode 100644 include/linux/phy/phy-rockchip-typec.h

diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 26566db..ec87b3a 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -351,6 +351,14 @@ config PHY_ROCKCHIP_DP
help
Enable this to support the Rockchip Display Port PHY.
+config PHY_ROCKCHIP_TYPEC
+ tristate "Rockchip TYPEC PHY Driver"
+ depends on ARCH_ROCKCHIP && OF
+ select GENERIC_PHY
+ select RESET_CONTROLLER
+ help
+ Enable this to support the Rockchip USB TYPEC PHY.
+
config PHY_ST_SPEAR1310_MIPHY
tristate "ST SPEAR1310-MIPHY driver"
select GENERIC_PHY
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index 24596a9..91fa413 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -39,6 +39,7 @@ obj-$(CONFIG_PHY_QCOM_APQ8064_SATA) += phy-qcom-apq8064-sata.o
obj-$(CONFIG_PHY_ROCKCHIP_USB) += phy-rockchip-usb.o
obj-$(CONFIG_PHY_ROCKCHIP_EMMC) += phy-rockchip-emmc.o
obj-$(CONFIG_PHY_ROCKCHIP_DP) += phy-rockchip-dp.o
+obj-$(CONFIG_PHY_ROCKCHIP_TYPEC) += phy-rockchip-typec.o
obj-$(CONFIG_PHY_QCOM_IPQ806X_SATA) += phy-qcom-ipq806x-sata.o
obj-$(CONFIG_PHY_ST_SPEAR1310_MIPHY) += phy-spear1310-miphy.o
obj-$(CONFIG_PHY_ST_SPEAR1340_MIPHY) += phy-spear1340-miphy.o
diff --git a/drivers/phy/phy-rockchip-typec.c b/drivers/phy/phy-rockchip-typec.c
new file mode 100644
index 0000000..230e074
--- /dev/null
+++ b/drivers/phy/phy-rockchip-typec.c
<snip>

+static const struct phy_ops rockchip_tcphy_ops = {
+ .owner = THIS_MODULE,
no ops?
+};
+
+static int tcphy_pd_event(struct notifier_block *nb,
+ unsigned long event, void *priv)
+{
+ struct rockchip_typec_phy *tcphy;
+ struct extcon_dev *edev = priv;
+ int value = edev->state;
+ int mode;
+ u8 is_plugged, dfp;
+
+ tcphy = container_of(nb, struct rockchip_typec_phy, event_nb);
+
+ is_plugged = GET_PLUGGED(value);
+ tcphy->flip = GET_FLIP(value);
+ dfp = GET_DFP(value);
+ tcphy->map = GET_PIN_MAP(value);
+
+ if (is_plugged) {
+ if (!dfp)
+ mode = MODE_UFP_USB;
+ else if (tcphy->map & (PIN_MAP_B | PIN_MAP_D | PIN_MAP_F))
+ mode = MODE_DFP_USB | MODE_DFP_DP;
+ else if (tcphy->map & (PIN_MAP_A | PIN_MAP_C | PIN_MAP_E))
+ mode = MODE_DFP_DP;
+ else
+ mode = MODE_DFP_USB;
+ } else {
+ mode = MODE_DISCONNECT;
+ }
+
+ if (tcphy->mode != mode) {
+ tcphy->mode = mode;
+ schedule_delayed_work_on(0, &tcphy->event_wq, 0);
+ }
+
+ return 0;
+}
+
+static void tcphy_event_wq(struct work_struct *work)
+{
+ struct rockchip_typec_phy *tcphy;
+ int ret;
+
+ tcphy = container_of(work, struct rockchip_typec_phy, event_wq.work);
+
+ if (tcphy->mode == MODE_DISCONNECT) {
+ tcphy_phy_deinit(tcphy);
+ /* remove hpd sign for DP */
+ if (tcphy->hpd_status) {
+ regmap_write(tcphy->grf_regs, GRF_SOC_CON26,
+ DPTX_HPD_SEL_MASK | DPTX_HPD_DEL);
+ tcphy->hpd_status = 0;
+ }
+ } else {
+ ret = tcphy_phy_init(tcphy);
+ if (ret)
+ return;
+
+ if (tcphy->mode & (MODE_UFP_USB | MODE_DFP_USB))
+ tcphy_usb3_init(tcphy);
+
+ if (tcphy->mode & MODE_DFP_DP) {
+ ret = tcphy_dp_init(tcphy);
+
+ /* set hpd sign for DP, if DP phy is ready */
+ if (!ret) {
+ regmap_write(tcphy->grf_regs, GRF_SOC_CON26,
+ DPTX_HPD_SEL_MASK | DPTX_HPD_SEL);
+ tcphy->hpd_status = 1;
+ }
+ }
+ }
+}
+
+static int tcphy_get_param(struct device *dev,
+ struct usb3phy_reg *reg,
+ const char *name)
+{
+ int ret, buffer[3];
+
+ ret = of_property_read_u32_array(dev->of_node, name, buffer, 3);
+ if (ret) {
+ dev_err(dev, "Can not parse %s\n", name);
+ return ret;
+ }
+
+ reg->offset = buffer[0];
+ reg->enable_bit = buffer[1];
+ reg->write_enable = buffer[2];
+ return 0;
+}
+
+static int tcphy_parse_dt(struct rockchip_typec_phy *tcphy,
+ struct device *dev)
+{
+ struct rockchip_usb3phy_port_cfg *cfg = &tcphy->port_cfgs;
+ int ret;
+
+ ret = tcphy_get_param(dev, &cfg->typec_conn_dir,
+ "rockchip,typec_conn_dir");
+ if (ret)
+ return ret;
+
+ ret = tcphy_get_param(dev, &cfg->usb3tousb2_en,
+ "rockchip,usb3tousb2_en");
+ if (ret)
+ return ret;
+
+ ret = tcphy_get_param(dev, &cfg->external_psm,
+ "rockchip,external_psm");
+ if (ret)
+ return ret;
+
+ ret = tcphy_get_param(dev, &cfg->pipe_status,
+ "rockchip,pipe_status");
+ if (ret)
+ return ret;
+
+ ret = tcphy_get_param(dev, &cfg->uphy_dp_sel,
+ "rockchip,uphy_dp_sel");
+ if (ret)
+ return ret;
+
+ tcphy->grf_regs = syscon_regmap_lookup_by_phandle(dev->of_node,
+ "rockchip,grf");
+ if (IS_ERR(tcphy->grf_regs)) {
+ dev_err(dev, "could not find grf dt node\n");
+ return PTR_ERR(tcphy->grf_regs);
+ }
+
+ tcphy->clk_core = devm_clk_get(dev, "tcpdcore");
+ if (IS_ERR(tcphy->clk_core)) {
+ dev_err(dev, "could not get uphy core clock\n");
+ return PTR_ERR(tcphy->clk_core);
+ }
+
+ tcphy->clk_ref = devm_clk_get(dev, "tcpdphy_ref");
+ if (IS_ERR(tcphy->clk_ref)) {
+ dev_err(dev, "could not get uphy ref clock\n");
+ return PTR_ERR(tcphy->clk_ref);
+ }
+
+ tcphy->phy_rst = devm_reset_control_get(dev, "tcphy");
+ if (IS_ERR(tcphy->phy_rst)) {
+ dev_err(dev, "no phy_rst reset control found\n");
+ return PTR_ERR(tcphy->phy_rst);
+ }
+
+ tcphy->pipe_rst = devm_reset_control_get(dev, "tcphy_pipe");
+ if (IS_ERR(tcphy->pipe_rst)) {
+ dev_err(dev, "no pipe_rst reset control found\n");
+ return PTR_ERR(tcphy->pipe_rst);
+ }
+
+ tcphy->uphy_rst = devm_reset_control_get(dev, "uphy_tcphy");
+ if (IS_ERR(tcphy->uphy_rst)) {
+ dev_err(dev, "no uphy_rst reset control found\n");
+ return PTR_ERR(tcphy->uphy_rst);
+ }
+
+ return 0;
+}
+
+static int rockchip_typec_phy_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct rockchip_typec_phy *tcphy;
+ struct resource *res;
+ struct phy_provider *phy_provider;
+ int ret;
+
+ tcphy = devm_kzalloc(dev, sizeof(*tcphy), GFP_KERNEL);
+ if (!tcphy)
+ return -ENOMEM;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ tcphy->base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(tcphy->base)) {
+ dev_err(dev, "failed to remap phy regs\n");
+ return PTR_ERR(tcphy->base);
+ }
+
+ tcphy_parse_dt(tcphy, dev);
+
+ tcphy->dev = dev;
+ platform_set_drvdata(pdev, tcphy);
+
+ tcphy->mode = MODE_DISCONNECT;
+
+ tcphy->phy = devm_phy_create(dev, NULL, &rockchip_tcphy_ops);

your phy_ops doesn't have any ops populated. Is there a reason to use the phy
framework?

Thanks
Kishon


Yes, I am thinking about it, remove it would be better.