[1/2] dt-bindings: phy-imx8mq-usb: add compatible string for imx8mp usb phy

Add "fsl,imx8mp-usb-phy" compatible string for imx8mp usb phy, which is
similar with imx8mq usb phy but with some different customizations.

Signed-off-by: Li Jun <jun.li@nxp.com>

On 20-06-08 21:11:32, Li Jun wrote:
> Add "fsl,imx8mp-usb-phy" compatible string for imx8mp usb phy, which is
> similar with imx8mq usb phy but with some different customizations.
> 
> Signed-off-by: Li Jun <jun.li@nxp.com>
> 

Reviewed-by: Peter Chen <peter.chen@nxp.com>

Peter

> diff --git a/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.txt b/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.txt
> index ed47e5c..7c70f2a 100644
> --- a/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.txt
> +++ b/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.txt
> @@ -1,7 +1,7 @@
>  * Freescale i.MX8MQ USB3 PHY binding
>  
>  Required properties:
> -- compatible:	Should be "fsl,imx8mq-usb-phy"
> +- compatible:	Should be "fsl,imx8mq-usb-phy" or "fsl,imx8mp-usb-phy"
>  - #phys-cells:	must be 0 (see phy-bindings.txt in this directory)
>  - reg:		The base address and length of the registers
>  - clocks:	phandles to the clocks for each clock listed in clock-names
> -- 
> 2.7.4
>

On 20-06-08 21:11:33, Li Jun wrote:
> Add initial support for imx8mp usb phy support, imx8mp usb has
> a silimar phy as imx8mq, which has some different customizations
> on clock and low power design when SoC integration.
> 
> Signed-off-by: Li Jun <jun.li@nxp.com>
> 
> diff --git a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> index 0c4833d..030bf4e 100644
> --- a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> +++ b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> @@ -4,12 +4,16 @@
>  #include <linux/clk.h>
>  #include <linux/io.h>
>  #include <linux/module.h>
> +#include <linux/delay.h>
>  #include <linux/phy/phy.h>
>  #include <linux/platform_device.h>
> +#include <linux/of_platform.h>
>  #include <linux/regulator/consumer.h>
>  
>  #define PHY_CTRL0			0x0
>  #define PHY_CTRL0_REF_SSP_EN		BIT(2)
> +#define PHY_CTRL0_FSEL_MASK		GENMASK(10, 5)
> +#define PHY_CTRL0_FSEL_24M		(0x2a << 5)
>  
>  #define PHY_CTRL1			0x4
>  #define PHY_CTRL1_RESET			BIT(0)
> @@ -20,6 +24,11 @@
>  
>  #define PHY_CTRL2			0x8
>  #define PHY_CTRL2_TXENABLEN0		BIT(8)
> +#define PHY_CTRL2_OTG_DISABLE		BIT(9)
> +
> +#define PHY_CTRL6			0x18
> +#define PHY_CTRL6_ALT_CLK_EN		BIT(1)
> +#define PHY_CTRL6_ALT_CLK_SEL		BIT(0)
>  
>  struct imx8mq_usb_phy {
>  	struct phy *phy;
> @@ -54,6 +63,44 @@ static int imx8mq_usb_phy_init(struct phy *phy)
>  	return 0;
>  }
>  
> +static int imx8mp_usb_phy_init(struct phy *phy)
> +{
> +	struct imx8mq_usb_phy *imx_phy = phy_get_drvdata(phy);
> +	u32 value;
> +
> +	/* USB3.0 PHY signal fsel for 24M ref */
> +	value = readl(imx_phy->base + PHY_CTRL0);
> +	value &= ~PHY_CTRL0_FSEL_MASK;
> +	value |= PHY_CTRL0_FSEL_24M;
> +	writel(value, imx_phy->base + PHY_CTRL0);
> +
> +	/* Disable alt_clk_en and use internal MPLL clocks */
> +	value = readl(imx_phy->base + PHY_CTRL6);
> +	value &= ~(PHY_CTRL6_ALT_CLK_SEL | PHY_CTRL6_ALT_CLK_EN);
> +	writel(value, imx_phy->base + PHY_CTRL6);
> +
> +	value = readl(imx_phy->base + PHY_CTRL1);
> +	value &= ~(PHY_CTRL1_VDATSRCENB0 | PHY_CTRL1_VDATDETENB0);
> +	value |= PHY_CTRL1_RESET | PHY_CTRL1_ATERESET;
> +	writel(value, imx_phy->base + PHY_CTRL1);
> +
> +	value = readl(imx_phy->base + PHY_CTRL0);
> +	value |= PHY_CTRL0_REF_SSP_EN;
> +	writel(value, imx_phy->base + PHY_CTRL0);
> +
> +	value = readl(imx_phy->base + PHY_CTRL2);
> +	value |= PHY_CTRL2_TXENABLEN0 | PHY_CTRL2_OTG_DISABLE;
> +	writel(value, imx_phy->base + PHY_CTRL2);
> +
> +	udelay(10);
> +
> +	value = readl(imx_phy->base + PHY_CTRL1);
> +	value &= ~(PHY_CTRL1_RESET | PHY_CTRL1_ATERESET);
> +	writel(value, imx_phy->base + PHY_CTRL1);
> +
> +	return 0;
> +}
> +
>  static int imx8mq_phy_power_on(struct phy *phy)
>  {
>  	struct imx8mq_usb_phy *imx_phy = phy_get_drvdata(phy);
> @@ -83,12 +130,29 @@ static struct phy_ops imx8mq_usb_phy_ops = {
>  	.owner		= THIS_MODULE,
>  };
>  
> +static struct phy_ops imx8mp_usb_phy_ops = {
> +	.init		= imx8mp_usb_phy_init,
> +	.power_on	= imx8mq_phy_power_on,
> +	.power_off	= imx8mq_phy_power_off,
> +	.owner		= THIS_MODULE,
> +};
> +
> +static const struct of_device_id imx8mq_usb_phy_of_match[] = {
> +	{.compatible = "fsl,imx8mq-usb-phy",
> +	 .data = &imx8mq_usb_phy_ops,},
> +	{.compatible = "fsl,imx8mp-usb-phy",
> +	 .data = &imx8mp_usb_phy_ops,},
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, imx8mq_usb_phy_of_match);
> +
>  static int imx8mq_usb_phy_probe(struct platform_device *pdev)
>  {
>  	struct phy_provider *phy_provider;
>  	struct device *dev = &pdev->dev;
>  	struct imx8mq_usb_phy *imx_phy;
>  	struct resource *res;
> +	const struct of_device_id *of_id;
>  
>  	imx_phy = devm_kzalloc(dev, sizeof(*imx_phy), GFP_KERNEL);
>  	if (!imx_phy)
> @@ -105,7 +169,12 @@ static int imx8mq_usb_phy_probe(struct platform_device *pdev)
>  	if (IS_ERR(imx_phy->base))
>  		return PTR_ERR(imx_phy->base);
>  
> -	imx_phy->phy = devm_phy_create(dev, NULL, &imx8mq_usb_phy_ops);
> +	of_id = of_match_device(imx8mq_usb_phy_of_match, dev);
> +	if (!of_id)
> +		return -ENODEV;
> +
> +	imx_phy->phy = devm_phy_create(dev, NULL, (const struct phy_ops *)
> +					of_id->data);
>  	if (IS_ERR(imx_phy->phy))
>  		return PTR_ERR(imx_phy->phy);

Reviewed-by: Peter Chen <peter.chen@nxp.com>

>  
> @@ -120,12 +189,6 @@ static int imx8mq_usb_phy_probe(struct platform_device *pdev)
>  	return PTR_ERR_OR_ZERO(phy_provider);
>  }
>  
> -static const struct of_device_id imx8mq_usb_phy_of_match[] = {
> -	{.compatible = "fsl,imx8mq-usb-phy",},
> -	{ },
> -};
> -MODULE_DEVICE_TABLE(of, imx8mq_usb_phy_of_match);
> -
>  static struct platform_driver imx8mq_usb_phy_driver = {
>  	.probe	= imx8mq_usb_phy_probe,
>  	.driver = {
> -- 
> 2.7.4
>

On Mon, 08 Jun 2020 21:11:32 +0800, Li Jun wrote:
> Add "fsl,imx8mp-usb-phy" compatible string for imx8mp usb phy, which is
> similar with imx8mq usb phy but with some different customizations.
> 
> Signed-off-by: Li Jun <jun.li@nxp.com>
> 

Acked-by: Rob Herring <robh@kernel.org>

+Vinod

> -----Original Message-----
> From: Jun Li <jun.li@nxp.com>
> Sent: Monday, June 8, 2020 9:43 PM
> To: kishon@ti.com; robh+dt@kernel.org
> Cc: shawnguo@kernel.org; s.hauer@pengutronix.de; kernel@pengutronix.de;
> festevam@gmail.com; dl-linux-imx <linux-imx@nxp.com>; Jun Li <jun.li@nxp.com>;
> devicetree@vger.kernel.org; Peter Chen <peter.chen@nxp.com>
> Subject: [PATCH 2/2] phy: freescale: imx8mq-usb: add support for imx8mp usb phy
> 
> Add initial support for imx8mp usb phy support, imx8mp usb has a silimar phy as
> imx8mq, which has some different customizations on clock and low power design when
> SoC integration.
> 
> Signed-off-by: Li Jun <jun.li@nxp.com>
> 
> diff --git a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> index 0c4833d..030bf4e 100644
> --- a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> +++ b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> @@ -4,12 +4,16 @@
>  #include <linux/clk.h>
>  #include <linux/io.h>
>  #include <linux/module.h>
> +#include <linux/delay.h>
>  #include <linux/phy/phy.h>
>  #include <linux/platform_device.h>
> +#include <linux/of_platform.h>
>  #include <linux/regulator/consumer.h>
> 
>  #define PHY_CTRL0			0x0
>  #define PHY_CTRL0_REF_SSP_EN		BIT(2)
> +#define PHY_CTRL0_FSEL_MASK		GENMASK(10, 5)
> +#define PHY_CTRL0_FSEL_24M		(0x2a << 5)
> 
>  #define PHY_CTRL1			0x4
>  #define PHY_CTRL1_RESET			BIT(0)
> @@ -20,6 +24,11 @@
> 
>  #define PHY_CTRL2			0x8
>  #define PHY_CTRL2_TXENABLEN0		BIT(8)
> +#define PHY_CTRL2_OTG_DISABLE		BIT(9)
> +
> +#define PHY_CTRL6			0x18
> +#define PHY_CTRL6_ALT_CLK_EN		BIT(1)
> +#define PHY_CTRL6_ALT_CLK_SEL		BIT(0)
> 
>  struct imx8mq_usb_phy {
>  	struct phy *phy;
> @@ -54,6 +63,44 @@ static int imx8mq_usb_phy_init(struct phy *phy)
>  	return 0;
>  }
> 
> +static int imx8mp_usb_phy_init(struct phy *phy) {
> +	struct imx8mq_usb_phy *imx_phy = phy_get_drvdata(phy);
> +	u32 value;
> +
> +	/* USB3.0 PHY signal fsel for 24M ref */
> +	value = readl(imx_phy->base + PHY_CTRL0);
> +	value &= ~PHY_CTRL0_FSEL_MASK;
> +	value |= PHY_CTRL0_FSEL_24M;
> +	writel(value, imx_phy->base + PHY_CTRL0);
> +
> +	/* Disable alt_clk_en and use internal MPLL clocks */
> +	value = readl(imx_phy->base + PHY_CTRL6);
> +	value &= ~(PHY_CTRL6_ALT_CLK_SEL | PHY_CTRL6_ALT_CLK_EN);
> +	writel(value, imx_phy->base + PHY_CTRL6);
> +
> +	value = readl(imx_phy->base + PHY_CTRL1);
> +	value &= ~(PHY_CTRL1_VDATSRCENB0 | PHY_CTRL1_VDATDETENB0);
> +	value |= PHY_CTRL1_RESET | PHY_CTRL1_ATERESET;
> +	writel(value, imx_phy->base + PHY_CTRL1);
> +
> +	value = readl(imx_phy->base + PHY_CTRL0);
> +	value |= PHY_CTRL0_REF_SSP_EN;
> +	writel(value, imx_phy->base + PHY_CTRL0);
> +
> +	value = readl(imx_phy->base + PHY_CTRL2);
> +	value |= PHY_CTRL2_TXENABLEN0 | PHY_CTRL2_OTG_DISABLE;
> +	writel(value, imx_phy->base + PHY_CTRL2);
> +
> +	udelay(10);
> +
> +	value = readl(imx_phy->base + PHY_CTRL1);
> +	value &= ~(PHY_CTRL1_RESET | PHY_CTRL1_ATERESET);
> +	writel(value, imx_phy->base + PHY_CTRL1);
> +
> +	return 0;
> +}
> +
>  static int imx8mq_phy_power_on(struct phy *phy)  {
>  	struct imx8mq_usb_phy *imx_phy = phy_get_drvdata(phy); @@ -83,12 +130,29 @@
> static struct phy_ops imx8mq_usb_phy_ops = {
>  	.owner		= THIS_MODULE,
>  };
> 
> +static struct phy_ops imx8mp_usb_phy_ops = {
> +	.init		= imx8mp_usb_phy_init,
> +	.power_on	= imx8mq_phy_power_on,
> +	.power_off	= imx8mq_phy_power_off,
> +	.owner		= THIS_MODULE,
> +};
> +
> +static const struct of_device_id imx8mq_usb_phy_of_match[] = {
> +	{.compatible = "fsl,imx8mq-usb-phy",
> +	 .data = &imx8mq_usb_phy_ops,},
> +	{.compatible = "fsl,imx8mp-usb-phy",
> +	 .data = &imx8mp_usb_phy_ops,},
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, imx8mq_usb_phy_of_match);
> +
>  static int imx8mq_usb_phy_probe(struct platform_device *pdev)  {
>  	struct phy_provider *phy_provider;
>  	struct device *dev = &pdev->dev;
>  	struct imx8mq_usb_phy *imx_phy;
>  	struct resource *res;
> +	const struct of_device_id *of_id;
> 
>  	imx_phy = devm_kzalloc(dev, sizeof(*imx_phy), GFP_KERNEL);
>  	if (!imx_phy)
> @@ -105,7 +169,12 @@ static int imx8mq_usb_phy_probe(struct platform_device *pdev)
>  	if (IS_ERR(imx_phy->base))
>  		return PTR_ERR(imx_phy->base);
> 
> -	imx_phy->phy = devm_phy_create(dev, NULL, &imx8mq_usb_phy_ops);
> +	of_id = of_match_device(imx8mq_usb_phy_of_match, dev);
> +	if (!of_id)
> +		return -ENODEV;
> +
> +	imx_phy->phy = devm_phy_create(dev, NULL, (const struct phy_ops *)
> +					of_id->data);
>  	if (IS_ERR(imx_phy->phy))
>  		return PTR_ERR(imx_phy->phy);
> 
> @@ -120,12 +189,6 @@ static int imx8mq_usb_phy_probe(struct platform_device *pdev)
>  	return PTR_ERR_OR_ZERO(phy_provider);
>  }
> 
> -static const struct of_device_id imx8mq_usb_phy_of_match[] = {
> -	{.compatible = "fsl,imx8mq-usb-phy",},
> -	{ },
> -};
> -MODULE_DEVICE_TABLE(of, imx8mq_usb_phy_of_match);
> -
>  static struct platform_driver imx8mq_usb_phy_driver = {
>  	.probe	= imx8mq_usb_phy_probe,
>  	.driver = {
> --
> 2.7.4

A gentle ping...

Thanks
Li Jun

On 16-08-20, 03:26, Jun Li wrote:
> +Vinod

> > -static const struct of_device_id imx8mq_usb_phy_of_match[] = {
> > -	{.compatible = "fsl,imx8mq-usb-phy",},
> > -	{ },
> > -};
> > -MODULE_DEVICE_TABLE(of, imx8mq_usb_phy_of_match);
> > -
> >  static struct platform_driver imx8mq_usb_phy_driver = {
> >  	.probe	= imx8mq_usb_phy_probe,
> >  	.driver = {
> > --
> > 2.7.4
> 
> A gentle ping...

Sorry I dont have this in my inbox, can you please rebase and resend

Hi,

On 20-06-08 21:11, Li Jun wrote:

...

> +static int imx8mp_usb_phy_init(struct phy *phy)
> +{
> +	struct imx8mq_usb_phy *imx_phy = phy_get_drvdata(phy);
> +	u32 value;
> +
> +	/* USB3.0 PHY signal fsel for 24M ref */
> +	value = readl(imx_phy->base + PHY_CTRL0);
> +	value &= ~PHY_CTRL0_FSEL_MASK;
> +	value |= PHY_CTRL0_FSEL_24M;
> +	writel(value, imx_phy->base + PHY_CTRL0);
> +
> +	/* Disable alt_clk_en and use internal MPLL clocks */
> +	value = readl(imx_phy->base + PHY_CTRL6);
> +	value &= ~(PHY_CTRL6_ALT_CLK_SEL | PHY_CTRL6_ALT_CLK_EN);
> +	writel(value, imx_phy->base + PHY_CTRL6);
> +
> +	value = readl(imx_phy->base + PHY_CTRL1);
> +	value &= ~(PHY_CTRL1_VDATSRCENB0 | PHY_CTRL1_VDATDETENB0);
> +	value |= PHY_CTRL1_RESET | PHY_CTRL1_ATERESET;
> +	writel(value, imx_phy->base + PHY_CTRL1);
> +
> +	value = readl(imx_phy->base + PHY_CTRL0);
> +	value |= PHY_CTRL0_REF_SSP_EN;
> +	writel(value, imx_phy->base + PHY_CTRL0);
> +
> +	value = readl(imx_phy->base + PHY_CTRL2);
> +	value |= PHY_CTRL2_TXENABLEN0 | PHY_CTRL2_OTG_DISABLE;
> +	writel(value, imx_phy->base + PHY_CTRL2);
> +
> +	udelay(10);

Nit:
Do we need the active wait here or is it also possible to use
usleep_range() here?

> +
> +	value = readl(imx_phy->base + PHY_CTRL1);
> +	value &= ~(PHY_CTRL1_RESET | PHY_CTRL1_ATERESET);
> +	writel(value, imx_phy->base + PHY_CTRL1);
> +
> +	return 0;
> +}