Re: [PATCH 2/2] phy: freescale: Add i.MX8qm Mixel LVDS PHY support

[Liu Ying]

On Sun, 2022-06-19 at 14:15 +0200, Krzysztof Kozlowski wrote:
> On 18/06/2022 11:22, Liu Ying wrote:
> > This patch adds Freescale i.MX8qm LVDS PHY support.
> 
> 
> Don't use "This patch".

Fair enough. Won't use it.

> 
https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Felixir.bootlin.com%2Flinux%2Fv5.17.1%2Fsource%2FDocumentation%2Fprocess%2Fsubmitting-patches.rst%23L95&data=05%7C01%7Cvictor.liu%40nxp.com%7C82050bf711fb4a8eb28108da51ed5912%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637912377082315453%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=nloK3581LSb7%2BUF%2FTMR4b5J4GYRw4SKKRfK%2FRfP3UrM%3D&reserved=0
> 
> > The PHY IP is from Mixel, Inc.
> > 
> > Signed-off-by: Liu Ying <victor.liu@xxxxxxx>
> 
> 
> 
> > +static int mixel_lvds_phy_probe(struct platform_device *pdev)
> > +{
> > +	struct device *dev = &pdev->dev;
> > +	struct phy_provider *phy_provider;
> > +	struct mixel_lvds_phy_priv *priv;
> > +	struct mixel_lvds_phy *lvds_phy;
> > +	struct phy *phy;
> > +	int i;
> > +	int ret;
> > +
> > +	if (!dev->of_node)
> > +		return -ENODEV;
> > +
> > +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> > +	if (!priv)
> > +		return -ENOMEM;
> > +
> > +	priv->regmap = syscon_node_to_regmap(dev->of_node->parent);
> > +	if (IS_ERR(priv->regmap)) {
> > +		ret = PTR_ERR(priv->regmap);
> > +		dev_err_probe(dev, ret, "failed to get regmap\n");
> > +		return ret;
> 
> All such calls are one-liners.

Will do.

> 
> > +	}
> > +
> > +	priv->phy_ref_clk = devm_clk_get(dev, "phy_ref");
> > +	if (IS_ERR(priv->phy_ref_clk)) {
> > +		ret = PTR_ERR(priv->phy_ref_clk);
> > +		dev_err_probe(dev, ret, "failed to get PHY reference
> > clock\n");
> > +		return ret;
> 
> Again, one line instead of three.

Will do.

> 
> > +	}
> > +
> > +	mutex_init(&priv->lock);
> > +
> > +	dev_set_drvdata(dev, priv);
> > +
> > +	pm_runtime_enable(dev);
> > +
> > +	ret = mixel_lvds_phy_reset(dev);
> > +	if (ret) {
> > +		dev_err(dev, "failed to do POR reset: %d\n", ret);
> > +		return ret;
> > +	}
> > +
> > +	for (i = 0; i < PHY_NUM; i++) {
> > +		lvds_phy = devm_kzalloc(dev, sizeof(*lvds_phy),
> > GFP_KERNEL);
> > +		if (!lvds_phy) {
> > +			ret = -ENOMEM;
> > +			goto err;
> > +		}
> > +
> > +		phy = devm_phy_create(dev, NULL, &mixel_lvds_phy_ops);
> > +		if (IS_ERR(phy)) {
> > +			ret = PTR_ERR(phy);
> > +			dev_err(dev, "failed to create PHY for
> > channel%d: %d\n",
> > +				i, ret);
> > +			goto err;
> > +		}
> > +
> > +		lvds_phy->phy = phy;
> > +		lvds_phy->id = i;
> > +		priv->phys[i] = lvds_phy;
> > +
> > +		phy_set_drvdata(phy, lvds_phy);
> > +	}
> > +
> > +	phy_provider = devm_of_phy_provider_register(dev,
> > mixel_lvds_phy_xlate);
> > +	if (IS_ERR(phy_provider)) {
> > +		ret = PTR_ERR(phy_provider);
> > +		dev_err(dev, "failed to register PHY provider: %d\n",
> > ret);
> > +		goto err;
> > +	}
> > +
> > +	return 0;
> > +err:
> > +	pm_runtime_disable(dev);
> > +
> > +	return ret;
> > +}
> > +
> > +static int mixel_lvds_phy_remove(struct platform_device *pdev)
> > +{
> > +	pm_runtime_disable(&pdev->dev);
> > +
> > +	return 0;
> > +}
> > +
> > +static int __maybe_unused mixel_lvds_phy_runtime_suspend(struct
> > device *dev)
> > +{
> > +	struct mixel_lvds_phy_priv *priv = dev_get_drvdata(dev);
> > +
> > +	/* power down */
> > +	mutex_lock(&priv->lock);
> > +	regmap_write(priv->regmap, PHY_CTRL + REG_SET, PD);
> > +	mutex_unlock(&priv->lock);
> > +
> > +	dev_dbg(dev, "runtime suspended\n");
> > +
> > +	return 0;
> > +}
> > +
> > +static int __maybe_unused mixel_lvds_phy_runtime_resume(struct
> > device *dev)
> > +{
> > +	struct mixel_lvds_phy_priv *priv = dev_get_drvdata(dev);
> > +
> > +	/* power up + control initialization */
> > +	mutex_lock(&priv->lock);
> > +	regmap_update_bits(priv->regmap, PHY_CTRL,
> > +			   CTRL_INIT_MASK | PD, CTRL_INIT_VAL);
> > +	mutex_unlock(&priv->lock);
> > +
> > +	dev_dbg(dev, "runtime resumed\n");
> 
> No such debug messages.

Will remove the debug messages.

Thanks,
Liu Ying

> 
> 
> Best regards,
> Krzysztof