On Tue, Sep 1, 2015 at 1:04 AM, Yakir Yang [off-list ref] wrote:

This phy driver would control the Rockchip DisplayPort module
phy clock and phy power, it is relate to analogix_dp-rockchip
dp driver. If you want DP works rightly on rockchip platform,
then you should select both of them.

Signed-off-by: Yakir Yang <redacted>
---
Changes in v4:
- Take Kishon suggest, add commit message, and remove the redundant
  rockchip_dp_phy_init() function, move those code to probe() method.
  And remove driver .owner number.

Changes in v3:
- Take Heiko suggest, add rockchip dp phy driver,
  collect the phy clocks and power control.

Changes in v2: None

 .../devicetree/bindings/phy/rockchip-dp-phy.txt    |  26 ++++

It is preferred that you split binding doc's from driver changes.

Please state the size of the list and order of what each entry if more than one.

+- clocks: from common clock binding: handle to dp clock.
+       of memory mapped region.
+- clock-names: from common clock binding:
+       Required elements: "sclk_dp" "sclk_dp_24m"
+
+- rockchip,grf: this soc should set GRF regs, so need get grf here.

I have no idea what GRF means.

Do you need to do this every time? This can go in probe.

+       if (ret < 0) {
+               dev_err(dp->dev, "cannot set clock phy_24m %d\n", ret);
+               return ret;
+       }
+
+       ret = clk_prepare_enable(dp->phy_24m);
+       if (ret < 0) {
+               dev_err(dp->dev, "cannot enable clock phy_24m %d\n", ret);
+               return ret;
+       }
+
+       return 0;
+}
+
+static int rockchip_dp_phy_clk_disable(struct rockchip_dp_phy *dp)
+{
+       clk_disable_unprepare(dp->phy_24m);
+
+       return 0;
+}
+
+static int rockchip_set_phy_state(struct phy *phy, bool enable)
+{
+       struct rockchip_dp_phy *dp = phy_get_drvdata(phy);
+
+       if (enable) {
+               rockchip_dp_phy_clk_enable(dp);
+               writel(DP_PHY_SIDDQ_WRITE_EN | DP_PHY_SIDDQ_ON, dp->regs);
+       } else {
+               rockchip_dp_phy_clk_disable(dp);
+               writel(DP_PHY_SIDDQ_WRITE_EN | DP_PHY_SIDDQ_OFF, dp->regs);
+       }
+
+       return 0;
+}

I would just inline all 3 of these functions. The wrappers don't
doesn't do anything, but add 2 levels of function calls.

+
+static int rockchip_dp_phy_power_on(struct phy *phy)
+{
+       return rockchip_set_phy_state(phy, true);
+}
+
+static int rockchip_dp_phy_power_off(struct phy *phy)
+{
+       return rockchip_set_phy_state(phy, false);
+}
+
+static struct phy_ops rockchip_dp_phy_ops = {
+       .power_on       = rockchip_dp_phy_power_on,
+       .power_off      = rockchip_dp_phy_power_off,
+       .owner          = THIS_MODULE,
+};
+
+static int rockchip_dp_phy_probe(struct platform_device *pdev)
+{
+       struct device *dev = &pdev->dev;
+       struct device_node *np = dev->of_node;
+       struct phy_provider *phy_provider;
+       struct rockchip_dp_phy *dp;
+       struct resource *res;
+       struct phy *phy;
+       int ret;
+
+       dp = devm_kzalloc(dev, sizeof(*dp), GFP_KERNEL);
+       if (IS_ERR(dp))
+               return -ENOMEM;
+
+       dp->dev = dev;
+
+       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+       dp->regs = devm_ioremap_resource(dev, res);
+       if (IS_ERR(dp->regs))
+               return PTR_ERR(dp->regs);
+
+       dp->phy_24m = devm_clk_get(dev, "24m");
+       if (IS_ERR(dp->phy_24m)) {
+               dev_err(dev, "cannot get clock 24m\n");
+               return PTR_ERR(dp->phy_24m);
+       }

The binding says there are 2 clocks.

+
+       dp->grf = syscon_regmap_lookup_by_phandle(np, "rockchip,grf");
+       if (IS_ERR(dp->grf)) {
+               dev_err(dev, "rk3288-dp needs rockchip,grf property\n");
+               return PTR_ERR(dp->grf);
+       }
+
+       ret = regmap_write(dp->grf, GRF_SOC_CON12,
+                          GRF_EDP_REF_CLK_SEL_INTER |
+                          (GRF_EDP_REF_CLK_SEL_INTER << 16));
+       if (ret != 0) {
+               dev_err(dp->dev, "Could not config GRF edp ref clk: %d\n", ret);
+               return ret;
+       }
+
+       phy = devm_phy_create(dev, NULL, &rockchip_dp_phy_ops, NULL);
+       if (IS_ERR(phy)) {
+               dev_err(dev, "failed to create phy\n");
+               return PTR_ERR(phy);
+       }
+       phy_set_drvdata(phy, dp);
+
+       phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+
+       return PTR_ERR_OR_ZERO(phy_provider);
+}
+
+static const struct of_device_id rockchip_dp_phy_dt_ids[] = {
+       { .compatible = "rockchip,rk3288-dp-phy" },
+       {}
+};
+
+MODULE_DEVICE_TABLE(of, rockchip_dp_phy_dt_ids);
+
+static struct platform_driver rockchip_dp_phy_driver = {
+       .probe          = rockchip_dp_phy_probe,
+       .driver         = {
+               .name   = "rockchip-dp-phy",
+               .of_match_table = rockchip_dp_phy_dt_ids,
+       },
+};
+
+module_platform_driver(rockchip_dp_phy_driver);
+
+MODULE_AUTHOR("Yakir Yang [off-list ref]");
+MODULE_DESCRIPTION("Rockchip DP PHY driver");
+MODULE_LICENSE("GPL v2");
--
2.1.2