Hi,

On Friday 29 June 2018 02:08 PM, Kunihiko Hayashi wrote:

I'd like to avoid configure the PHY this way, since it's impossible to know
which register is being configured.

+
+struct uniphier_u3hsphy_trim_param {
+	unsigned int rterm;
+	unsigned int sel_t;
+	unsigned int hs_i;
+};
+
+#define trim_param_is_valid(p)	((p)->rterm || (p)->sel_t || (p)->hs_i)
+
+struct uniphier_u3hsphy_priv {
+	struct device *dev;
+	void __iomem *base;
+	int nclks;
+	struct clk *clk[MAX_CLKS], *clk_phy, *clk_phy_ext;
+	int nrsts;
+	struct reset_control *rst[MAX_RSTS], *rst_phy;
+	const struct uniphier_u3hsphy_soc_data *data;
+};
+
+struct uniphier_u3hsphy_soc_data {
+	const char *clock_names[MAX_CLKS];
+	const char *reset_names[MAX_RSTS];
+	int nparams;
+	const struct uniphier_u3hsphy_param param[MAX_PHY_PARAMS];
+	u32 config0;
+	u32 config1;
+	void (*trim_func)(struct uniphier_u3hsphy_priv *priv, u32 *pconfig,
+			  struct uniphier_u3hsphy_trim_param *pt);
+};
+
+static void uniphier_u3hsphy_trim_ld20(struct uniphier_u3hsphy_priv *priv,
+				       u32 *pconfig,
+				       struct uniphier_u3hsphy_trim_param *pt)
+{
+	*pconfig &= ~HSPHY_CFG0_RTERM_MASK;
+	*pconfig |= FIELD_PREP(HSPHY_CFG0_RTERM_MASK, pt->rterm);
+
+	*pconfig &= ~HSPHY_CFG0_SEL_T_MASK;
+	*pconfig |= FIELD_PREP(HSPHY_CFG0_SEL_T_MASK, pt->sel_t);
+
+	*pconfig &= ~HSPHY_CFG0_HS_I_MASK;
+	*pconfig |= FIELD_PREP(HSPHY_CFG0_HS_I_MASK,  pt->hs_i);
+}
+
+static int uniphier_u3hsphy_get_nvparam(struct uniphier_u3hsphy_priv *priv,
+					const char *name, unsigned int *val)
+{
+	struct nvmem_cell *cell;
+	u8 *buf;
+
+	cell = devm_nvmem_cell_get(priv->dev, name);
+	if (IS_ERR(cell))
+		return PTR_ERR(cell);
+
+	buf = nvmem_cell_read(cell, NULL);
+	if (IS_ERR(buf))
+		return PTR_ERR(buf);
+
+	*val = *buf;
+
+	kfree(buf);
+
+	return 0;
+}
+
+static int uniphier_u3hsphy_get_nvparams(struct uniphier_u3hsphy_priv *priv,
+					 struct uniphier_u3hsphy_trim_param *pt)
+{
+	int ret;
+
+	ret = uniphier_u3hsphy_get_nvparam(priv, "rterm", &pt->rterm);
+	if (ret)
+		return ret;
+
+	ret = uniphier_u3hsphy_get_nvparam(priv, "sel_t", &pt->sel_t);
+	if (ret)
+		return ret;
+
+	ret = uniphier_u3hsphy_get_nvparam(priv, "hs_i", &pt->hs_i);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int uniphier_u3hsphy_update_config(struct uniphier_u3hsphy_priv *priv,
+					  u32 *pconfig)
+{
+	struct uniphier_u3hsphy_trim_param trim;
+	int ret, trimmed = 0;
+
+	if (priv->data->trim_func) {
+		ret = uniphier_u3hsphy_get_nvparams(priv, &trim);
+		if (ret == -EPROBE_DEFER)
+			return ret;
+
+		/*
+		 * call trim_func only when trimming parameters that aren't
+		 * all-zero can be acquired. All-zero parameters mean nothing
+		 * has been written to nvmem.
+		 */
+		if (!ret && trim_param_is_valid(&trim)) {
+			priv->data->trim_func(priv, pconfig, &trim);
+			trimmed = 1;
+		} else {
+			dev_dbg(priv->dev, "can't get parameter from nvmem\n");
+		}
+	}
+
+	/* use default parameters without trimming values */
+	if (!trimmed) {
+		*pconfig &= ~HSPHY_CFG0_HSDISC_MASK;
+		*pconfig |= FIELD_PREP(HSPHY_CFG0_HSDISC_MASK, 3);
+	}
+
+	return 0;
+}
+
+static void uniphier_u3hsphy_set_param(struct uniphier_u3hsphy_priv *priv,
+				       const struct uniphier_u3hsphy_param *p)
+{
+	u32 val;
+
+	val = readl(priv->base + HSPHY_CFG1);
+	val &= ~HSPHY_CFG1_ADR_MASK;
+	val |= FIELD_PREP(HSPHY_CFG1_ADR_MASK, p->addr) | HSPHY_CFG1_ADR_EN;
+	writel(val, priv->base + HSPHY_CFG1);
+
+	val = readl(priv->base + HSPHY_CFG1);
+	val &= ~HSPHY_CFG1_ADR_EN;
+	writel(val, priv->base + HSPHY_CFG1);
+
+	val = readl(priv->base + HSPHY_CFG1);
+	val &= ~FIELD_PREP(HSPHY_CFG1_DAT_MASK, p->mask);
+	val |=  FIELD_PREP(HSPHY_CFG1_DAT_MASK, p->val) | HSPHY_CFG1_DAT_EN;
+	writel(val, priv->base + HSPHY_CFG1);
+
+	val = readl(priv->base + HSPHY_CFG1);
+	val &= ~HSPHY_CFG1_DAT_EN;
+	writel(val, priv->base + HSPHY_CFG1);
+}
+
+static int uniphier_u3hsphy_init(struct phy *phy)
+{
+	struct uniphier_u3hsphy_priv *priv = phy_get_drvdata(phy);
+	u32 config0, config1;
+	int i, ret;
+
+	ret = clk_prepare_enable(priv->clk_phy_ext);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare_enable(priv->clk_phy);
+	if (ret)
+		goto out_clk_ext_disable;
+
+	ret = reset_control_deassert(priv->rst_phy);
+	if (ret)
+		goto out_clk_disable;
+
+	if (!priv->data->config0 && !priv->data->config1)
+		return 0;
+
+	config0 = priv->data->config0;
+	config1 = priv->data->config1;
+
+	ret = uniphier_u3hsphy_update_config(priv, &config0);
+	if (ret)
+		goto out_rst_assert;
+
+	writel(config0, priv->base + HSPHY_CFG0);
+	writel(config1, priv->base + HSPHY_CFG1);
+
+	for (i = 0; i < priv->data->nparams; i++)
+		uniphier_u3hsphy_set_param(priv, &priv->data->param[i]);
+
+	return 0;
+
+out_rst_assert:
+	reset_control_assert(priv->rst_phy);
+out_clk_disable:
+	clk_disable_unprepare(priv->clk_phy);
+out_clk_ext_disable:
+	clk_disable_unprepare(priv->clk_phy_ext);
+
+	return ret;
+}
+
+static int uniphier_u3hsphy_exit(struct phy *phy)
+{
+	struct uniphier_u3hsphy_priv *priv = phy_get_drvdata(phy);
+
+	reset_control_assert(priv->rst_phy);
+	clk_disable_unprepare(priv->clk_phy);
+	clk_disable_unprepare(priv->clk_phy_ext);
+
+	return 0;
+}
+
+static const struct phy_ops uniphier_u3hsphy_ops = {
+	.init           = uniphier_u3hsphy_init,
+	.exit           = uniphier_u3hsphy_exit,
+	.owner          = THIS_MODULE,
+};
+
+static int uniphier_u3hsphy_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct uniphier_u3hsphy_priv *priv;
+	struct phy_provider *phy_provider;
+	struct resource *res;
+	struct phy *phy;
+	struct clk *clk;
+	struct reset_control *rst;
+	const char *name;
+	int i, ret, nc, nr;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->dev = dev;
+	priv->data = of_device_get_match_data(dev);
+	if (WARN_ON(!priv->data ||
+		    priv->data->nparams > MAX_PHY_PARAMS))
+		return -EINVAL;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	priv->base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(priv->base))
+		return PTR_ERR(priv->base);
+
+	for (i = 0; i < MAX_CLKS; i++) {
+		name = priv->data->clock_names[i];
+		if (!name)
+			break;
+		clk = devm_clk_get(dev, name);
+		/* "phy-ext" is optional */
+		if (!strcmp(name, "phy-ext")) {
+			if (PTR_ERR(clk) == -ENOENT)
+				clk = NULL;
+			priv->clk_phy_ext = clk;
+		} else if (!strcmp(name, "phy")) {
+			priv->clk_phy = clk;
+		} else {
+			priv->clk[priv->nclks++] = clk;

Only "link" clock will be here? Do we need an array for this?

I think the above can be replaced with 3 separate clk_get calls instead of
storing clock names in uniphier_u3hsphy_soc_data.

Thanks
Kishon