Re: [PATCH v14 7/7] phy: freescale: Add HDMI PHY driver for i.MX8MQ

2024-02-21 Thread Alexander Stein
Hi Sandor,

Am Mittwoch, 21. Februar 2024, 08:47:52 CET schrieb Sandor Yu:
> Hi Alexander,
> 
> Thanks for your comments,
> 
> >
> > Hi,
> >
> > thanks for the update.
> >
> > Am Dienstag, 20. Februar 2024, 04:23:55 CET schrieb Sandor Yu:
> > > Add Cadence HDP-TX HDMI PHY driver for i.MX8MQ.
> > >
> > > Cadence HDP-TX PHY could be put in either DP mode or HDMI mode base
> > on
> > > the configuration chosen.
> > > HDMI PHY mode is configurated in the driver.
> > >
> > > Signed-off-by: Sandor Yu 
> > > Tested-by: Alexander Stein 
> >
> > This still works as before. I noticed there is a lot of code duplication 
> > with
> > patch 6. IMHO these PHY drivers should be merged into a single one where
> > the mode is configured using phy_set_mode() from cdns-mhdp8501-core.c.
> > This nicely matches my concerns regarding patch 5.
> >
> Yes, there are some registers offset are same and the clock management 
> function could be reused for DP and HDMI PHY driver.
> But because of HDMI and DP PHY totally different work mode, the functions in 
> struct phy_ops,
> Such as ->init, ->power_on/off and ->configure could not combine into a 
> single one,
> So separate DP and HDMI PHY driver should be a better resolution.

Despite some output type (DP/HDMI) specific settings, the similarities
are quite huge actually. Even though apparently DP has it's own ->init setup,
this can be delayed until ->set_mode or even ->configure.
The distinction between each mode can be done by checking phy->attrs.mode.
For a prove of concept I've hacked both drivers into a single one. I can't
test DP, but HDMI still works. Feel free to contact me in private.

Best regards,
Alexander

> B.R
> Sandor
> 
> 
> > Best regards,
> > Alexander
> >
> > > ---
> > > v13->v14:
> > >  *No change.
> > >
> > > v12->v13:
> > > - Fix build warning
> > >
> > > v11->v12:
> > > - Adjust clk disable order.
> > > - Return error code to replace -1 for function wait_for_ack().
> > > - Use bool for variable pclk_in.
> > > - Add year 2024 to copyright.
> > >
> > >  drivers/phy/freescale/Kconfig   |  10 +
> > >  drivers/phy/freescale/Makefile  |   1 +
> > >  drivers/phy/freescale/phy-fsl-imx8mq-hdmi.c | 960
> > > 
> > >  3 files changed, 971 insertions(+)
> > >  create mode 100644 drivers/phy/freescale/phy-fsl-imx8mq-hdmi.c
> > >
> > > diff --git a/drivers/phy/freescale/Kconfig
> > > b/drivers/phy/freescale/Kconfig index c39709fd700ac..14f47b7cc77ab
> > > 100644
> > > --- a/drivers/phy/freescale/Kconfig
> > > +++ b/drivers/phy/freescale/Kconfig

> [snip]

-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/




Re: [PATCH v14 7/7] phy: freescale: Add HDMI PHY driver for i.MX8MQ

2024-02-20 Thread Sandor Yu
Hi Alexander,

Thanks for your comments,

>
> Hi,
>
> thanks for the update.
>
> Am Dienstag, 20. Februar 2024, 04:23:55 CET schrieb Sandor Yu:
> > Add Cadence HDP-TX HDMI PHY driver for i.MX8MQ.
> >
> > Cadence HDP-TX PHY could be put in either DP mode or HDMI mode base
> on
> > the configuration chosen.
> > HDMI PHY mode is configurated in the driver.
> >
> > Signed-off-by: Sandor Yu 
> > Tested-by: Alexander Stein 
>
> This still works as before. I noticed there is a lot of code duplication with
> patch 6. IMHO these PHY drivers should be merged into a single one where
> the mode is configured using phy_set_mode() from cdns-mhdp8501-core.c.
> This nicely matches my concerns regarding patch 5.
>
Yes, there are some registers offset are same and the clock management function 
could be reused for DP and HDMI PHY driver.
But because of HDMI and DP PHY totally different work mode, the functions in 
struct phy_ops,
Such as ->init, ->power_on/off and ->configure could not combine into a single 
one,
So separate DP and HDMI PHY driver should be a better resolution.

B.R
Sandor


> Best regards,
> Alexander
>
> > ---
> > v13->v14:
> >  *No change.
> >
> > v12->v13:
> > - Fix build warning
> >
> > v11->v12:
> > - Adjust clk disable order.
> > - Return error code to replace -1 for function wait_for_ack().
> > - Use bool for variable pclk_in.
> > - Add year 2024 to copyright.
> >
> >  drivers/phy/freescale/Kconfig   |  10 +
> >  drivers/phy/freescale/Makefile  |   1 +
> >  drivers/phy/freescale/phy-fsl-imx8mq-hdmi.c | 960
> > 
> >  3 files changed, 971 insertions(+)
> >  create mode 100644 drivers/phy/freescale/phy-fsl-imx8mq-hdmi.c
> >
> > diff --git a/drivers/phy/freescale/Kconfig
> > b/drivers/phy/freescale/Kconfig index c39709fd700ac..14f47b7cc77ab
> > 100644
> > --- a/drivers/phy/freescale/Kconfig
> > +++ b/drivers/phy/freescale/Kconfig
> > @@ -45,6 +45,16 @@ config PHY_FSL_IMX8MQ_DP
> > Enable this to support the Cadence HDPTX DP PHY driver
> > on i.MX8MQ SOC.
> >
> > +config PHY_FSL_IMX8MQ_HDMI
> > + tristate "Freescale i.MX8MQ HDMI PHY support"
> > + depends on OF && HAS_IOMEM
> > + depends on COMMON_CLK
> > + select GENERIC_PHY
> > + select CDNS_MHDP_HELPER
> > + help
> > +   Enable this to support the Cadence HDPTX HDMI PHY driver
> > +   on i.MX8MQ SOC.
> > +
> >  endif
> >
> >  config PHY_FSL_LYNX_28G
> > diff --git a/drivers/phy/freescale/Makefile
> > b/drivers/phy/freescale/Makefile index 47e5285209fa8..1380ac31c2ead
> > 100644
> > --- a/drivers/phy/freescale/Makefile
> > +++ b/drivers/phy/freescale/Makefile
> > @@ -1,5 +1,6 @@
> >  # SPDX-License-Identifier: GPL-2.0-only
> >  obj-$(CONFIG_PHY_FSL_IMX8MQ_DP)  +=
> phy-fsl-imx8mq-dp.o
> > +obj-$(CONFIG_PHY_FSL_IMX8MQ_HDMI)+= phy-fsl-imx8mq-hdmi.o
> >  obj-$(CONFIG_PHY_FSL_IMX8MQ_USB) += phy-fsl-imx8mq-usb.o
> >  obj-$(CONFIG_PHY_MIXEL_LVDS_PHY) += phy-fsl-imx8qm-lvds-phy.o
> >  obj-$(CONFIG_PHY_MIXEL_MIPI_DPHY)+= phy-fsl-imx8-mipi-dphy.o
> > diff --git a/drivers/phy/freescale/phy-fsl-imx8mq-hdmi.c
> > b/drivers/phy/freescale/phy-fsl-imx8mq-hdmi.c
> > new file mode 100644
> > index 0..537b1f45c91cc
> > --- /dev/null
> > +++ b/drivers/phy/freescale/phy-fsl-imx8mq-hdmi.c
> > @@ -0,0 +1,960 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Cadence High-Definition Multimedia Interface (HDMI) PHY driver
> > + *
> > + * Copyright (C) 2022-2024 NXP Semiconductor, Inc.
> > + */
> > +#include 
> > +#include  #include 
> > +#include  #include  #include
> > + #include 
> > +
> > +#define ADDR_PHY_AFE 0x8
> > +
> > +/* PHY registers */
> > +#define CMN_SSM_BIAS_TMR 0x0022
> > +#define CMN_PLLSM0_USER_DEF_CTRL 0x002f
> > +#define CMN_PSM_CLK_CTRL 0x0061
> > +#define CMN_CDIAG_REFCLK_CTRL0x0062
> > +#define CMN_PLL0_VCOCAL_START0x0081
> > +#define CMN_PLL0_VCOCAL_INIT_TMR 0x0084
> > +#define CMN_PLL0_VCOCAL_ITER_TMR 0x0085
> > +#define CMN_TXPUCAL_CTRL 0x00e0
> > +#define CMN_TXPDCAL_CTRL 0x00f0
> > +#define CMN_TXPU_ADJ_CTRL0x0108
> > +#define CMN_TXPD_ADJ_CTRL0x010c
> > +#define CMN_DIAG_PLL0_FBH_OVRD   0x01c0
> > +#define CMN_DIAG_PLL0_FBL_OVRD   0x01c1
> > +#define CMN_DIAG_PLL0_OVRD   0x01c2
> > +#define CMN_DIAG_PLL0_TEST_MODE  0x01c4
> > +#define CMN_DIAG_PLL0_V2I_TUNE   0x01c5
> > +#define CMN_DIAG_PLL0_CP_TUNE0x01c6
> > +#define CMN_DIAG_PLL0_LF_PROG0x01c7
> > +#define CMN_DIAG_PLL0_PTATIS_TUNE1   0x01c8
> > +#define CMN_DIAG_PLL0_PTATIS_TUNE2   0x01c9
> > +#define CMN_DIAG_PLL0_INCLK_CTRL 0x01ca
> > +#define CMN_DIAG_PLL0_PXL_DIVH  

Re: [PATCH v14 7/7] phy: freescale: Add HDMI PHY driver for i.MX8MQ

2024-02-20 Thread Alexander Stein
Hi,

thanks for the update.

Am Dienstag, 20. Februar 2024, 04:23:55 CET schrieb Sandor Yu:
> Add Cadence HDP-TX HDMI PHY driver for i.MX8MQ.
> 
> Cadence HDP-TX PHY could be put in either DP mode or
> HDMI mode base on the configuration chosen.
> HDMI PHY mode is configurated in the driver.
> 
> Signed-off-by: Sandor Yu 
> Tested-by: Alexander Stein 

This still works as before. I noticed there is a lot of code duplication
with patch 6. IMHO these PHY drivers should be merged into a single
one where the mode is configured using phy_set_mode() from
cdns-mhdp8501-core.c. This nicely matches my concerns regarding patch 5.

Best regards,
Alexander

> ---
> v13->v14:
>  *No change.
> 
> v12->v13:
> - Fix build warning
> 
> v11->v12:
> - Adjust clk disable order.
> - Return error code to replace -1 for function wait_for_ack().
> - Use bool for variable pclk_in.
> - Add year 2024 to copyright.
> 
>  drivers/phy/freescale/Kconfig   |  10 +
>  drivers/phy/freescale/Makefile  |   1 +
>  drivers/phy/freescale/phy-fsl-imx8mq-hdmi.c | 960 
>  3 files changed, 971 insertions(+)
>  create mode 100644 drivers/phy/freescale/phy-fsl-imx8mq-hdmi.c
> 
> diff --git a/drivers/phy/freescale/Kconfig b/drivers/phy/freescale/Kconfig
> index c39709fd700ac..14f47b7cc77ab 100644
> --- a/drivers/phy/freescale/Kconfig
> +++ b/drivers/phy/freescale/Kconfig
> @@ -45,6 +45,16 @@ config PHY_FSL_IMX8MQ_DP
> Enable this to support the Cadence HDPTX DP PHY driver
> on i.MX8MQ SOC.
>  
> +config PHY_FSL_IMX8MQ_HDMI
> + tristate "Freescale i.MX8MQ HDMI PHY support"
> + depends on OF && HAS_IOMEM
> + depends on COMMON_CLK
> + select GENERIC_PHY
> + select CDNS_MHDP_HELPER
> + help
> +   Enable this to support the Cadence HDPTX HDMI PHY driver
> +   on i.MX8MQ SOC.
> +
>  endif
>  
>  config PHY_FSL_LYNX_28G
> diff --git a/drivers/phy/freescale/Makefile b/drivers/phy/freescale/Makefile
> index 47e5285209fa8..1380ac31c2ead 100644
> --- a/drivers/phy/freescale/Makefile
> +++ b/drivers/phy/freescale/Makefile
> @@ -1,5 +1,6 @@
>  # SPDX-License-Identifier: GPL-2.0-only
>  obj-$(CONFIG_PHY_FSL_IMX8MQ_DP)  += phy-fsl-imx8mq-dp.o
> +obj-$(CONFIG_PHY_FSL_IMX8MQ_HDMI)+= phy-fsl-imx8mq-hdmi.o
>  obj-$(CONFIG_PHY_FSL_IMX8MQ_USB) += phy-fsl-imx8mq-usb.o
>  obj-$(CONFIG_PHY_MIXEL_LVDS_PHY) += phy-fsl-imx8qm-lvds-phy.o
>  obj-$(CONFIG_PHY_MIXEL_MIPI_DPHY)+= phy-fsl-imx8-mipi-dphy.o
> diff --git a/drivers/phy/freescale/phy-fsl-imx8mq-hdmi.c 
> b/drivers/phy/freescale/phy-fsl-imx8mq-hdmi.c
> new file mode 100644
> index 0..537b1f45c91cc
> --- /dev/null
> +++ b/drivers/phy/freescale/phy-fsl-imx8mq-hdmi.c
> @@ -0,0 +1,960 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Cadence High-Definition Multimedia Interface (HDMI) PHY driver
> + *
> + * Copyright (C) 2022-2024 NXP Semiconductor, Inc.
> + */
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define ADDR_PHY_AFE 0x8
> +
> +/* PHY registers */
> +#define CMN_SSM_BIAS_TMR 0x0022
> +#define CMN_PLLSM0_USER_DEF_CTRL 0x002f
> +#define CMN_PSM_CLK_CTRL 0x0061
> +#define CMN_CDIAG_REFCLK_CTRL0x0062
> +#define CMN_PLL0_VCOCAL_START0x0081
> +#define CMN_PLL0_VCOCAL_INIT_TMR 0x0084
> +#define CMN_PLL0_VCOCAL_ITER_TMR 0x0085
> +#define CMN_TXPUCAL_CTRL 0x00e0
> +#define CMN_TXPDCAL_CTRL 0x00f0
> +#define CMN_TXPU_ADJ_CTRL0x0108
> +#define CMN_TXPD_ADJ_CTRL0x010c
> +#define CMN_DIAG_PLL0_FBH_OVRD   0x01c0
> +#define CMN_DIAG_PLL0_FBL_OVRD   0x01c1
> +#define CMN_DIAG_PLL0_OVRD   0x01c2
> +#define CMN_DIAG_PLL0_TEST_MODE  0x01c4
> +#define CMN_DIAG_PLL0_V2I_TUNE   0x01c5
> +#define CMN_DIAG_PLL0_CP_TUNE0x01c6
> +#define CMN_DIAG_PLL0_LF_PROG0x01c7
> +#define CMN_DIAG_PLL0_PTATIS_TUNE1   0x01c8
> +#define CMN_DIAG_PLL0_PTATIS_TUNE2   0x01c9
> +#define CMN_DIAG_PLL0_INCLK_CTRL 0x01ca
> +#define CMN_DIAG_PLL0_PXL_DIVH   0x01cb
> +#define CMN_DIAG_PLL0_PXL_DIVL   0x01cc
> +#define CMN_DIAG_HSCLK_SEL   0x01e0
> +#define XCVR_PSM_RCTRL   0x4001
> +#define TX_TXCC_CAL_SCLR_MULT_0  0x4047
> +#define TX_TXCC_CPOST_MULT_00_0  0x404c
> +#define XCVR_DIAG_PLLDRC_CTRL0x40e0
> +#define XCVR_DIAG_PLLDRC_CTRL0x40e0
> +#define XCVR_DIAG_HSCLK_SEL  0x40e1
> +#define XCVR_DIAG_BIDI_CTRL  0x40e8
> +#define TX_PSC_A00x4100
> +#define TX_PSC_A1 

[PATCH v14 7/7] phy: freescale: Add HDMI PHY driver for i.MX8MQ

2024-02-19 Thread Sandor Yu
Add Cadence HDP-TX HDMI PHY driver for i.MX8MQ.

Cadence HDP-TX PHY could be put in either DP mode or
HDMI mode base on the configuration chosen.
HDMI PHY mode is configurated in the driver.

Signed-off-by: Sandor Yu 
Tested-by: Alexander Stein 
---
v13->v14:
 *No change.

v12->v13:
- Fix build warning

v11->v12:
- Adjust clk disable order.
- Return error code to replace -1 for function wait_for_ack().
- Use bool for variable pclk_in.
- Add year 2024 to copyright.

 drivers/phy/freescale/Kconfig   |  10 +
 drivers/phy/freescale/Makefile  |   1 +
 drivers/phy/freescale/phy-fsl-imx8mq-hdmi.c | 960 
 3 files changed, 971 insertions(+)
 create mode 100644 drivers/phy/freescale/phy-fsl-imx8mq-hdmi.c

diff --git a/drivers/phy/freescale/Kconfig b/drivers/phy/freescale/Kconfig
index c39709fd700ac..14f47b7cc77ab 100644
--- a/drivers/phy/freescale/Kconfig
+++ b/drivers/phy/freescale/Kconfig
@@ -45,6 +45,16 @@ config PHY_FSL_IMX8MQ_DP
  Enable this to support the Cadence HDPTX DP PHY driver
  on i.MX8MQ SOC.
 
+config PHY_FSL_IMX8MQ_HDMI
+   tristate "Freescale i.MX8MQ HDMI PHY support"
+   depends on OF && HAS_IOMEM
+   depends on COMMON_CLK
+   select GENERIC_PHY
+   select CDNS_MHDP_HELPER
+   help
+ Enable this to support the Cadence HDPTX HDMI PHY driver
+ on i.MX8MQ SOC.
+
 endif
 
 config PHY_FSL_LYNX_28G
diff --git a/drivers/phy/freescale/Makefile b/drivers/phy/freescale/Makefile
index 47e5285209fa8..1380ac31c2ead 100644
--- a/drivers/phy/freescale/Makefile
+++ b/drivers/phy/freescale/Makefile
@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0-only
 obj-$(CONFIG_PHY_FSL_IMX8MQ_DP)+= phy-fsl-imx8mq-dp.o
+obj-$(CONFIG_PHY_FSL_IMX8MQ_HDMI)  += phy-fsl-imx8mq-hdmi.o
 obj-$(CONFIG_PHY_FSL_IMX8MQ_USB)   += phy-fsl-imx8mq-usb.o
 obj-$(CONFIG_PHY_MIXEL_LVDS_PHY)   += phy-fsl-imx8qm-lvds-phy.o
 obj-$(CONFIG_PHY_MIXEL_MIPI_DPHY)  += phy-fsl-imx8-mipi-dphy.o
diff --git a/drivers/phy/freescale/phy-fsl-imx8mq-hdmi.c 
b/drivers/phy/freescale/phy-fsl-imx8mq-hdmi.c
new file mode 100644
index 0..537b1f45c91cc
--- /dev/null
+++ b/drivers/phy/freescale/phy-fsl-imx8mq-hdmi.c
@@ -0,0 +1,960 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Cadence High-Definition Multimedia Interface (HDMI) PHY driver
+ *
+ * Copyright (C) 2022-2024 NXP Semiconductor, Inc.
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define ADDR_PHY_AFE   0x8
+
+/* PHY registers */
+#define CMN_SSM_BIAS_TMR   0x0022
+#define CMN_PLLSM0_USER_DEF_CTRL   0x002f
+#define CMN_PSM_CLK_CTRL   0x0061
+#define CMN_CDIAG_REFCLK_CTRL  0x0062
+#define CMN_PLL0_VCOCAL_START  0x0081
+#define CMN_PLL0_VCOCAL_INIT_TMR   0x0084
+#define CMN_PLL0_VCOCAL_ITER_TMR   0x0085
+#define CMN_TXPUCAL_CTRL   0x00e0
+#define CMN_TXPDCAL_CTRL   0x00f0
+#define CMN_TXPU_ADJ_CTRL  0x0108
+#define CMN_TXPD_ADJ_CTRL  0x010c
+#define CMN_DIAG_PLL0_FBH_OVRD 0x01c0
+#define CMN_DIAG_PLL0_FBL_OVRD 0x01c1
+#define CMN_DIAG_PLL0_OVRD 0x01c2
+#define CMN_DIAG_PLL0_TEST_MODE0x01c4
+#define CMN_DIAG_PLL0_V2I_TUNE 0x01c5
+#define CMN_DIAG_PLL0_CP_TUNE  0x01c6
+#define CMN_DIAG_PLL0_LF_PROG  0x01c7
+#define CMN_DIAG_PLL0_PTATIS_TUNE1 0x01c8
+#define CMN_DIAG_PLL0_PTATIS_TUNE2 0x01c9
+#define CMN_DIAG_PLL0_INCLK_CTRL   0x01ca
+#define CMN_DIAG_PLL0_PXL_DIVH 0x01cb
+#define CMN_DIAG_PLL0_PXL_DIVL 0x01cc
+#define CMN_DIAG_HSCLK_SEL 0x01e0
+#define XCVR_PSM_RCTRL 0x4001
+#define TX_TXCC_CAL_SCLR_MULT_00x4047
+#define TX_TXCC_CPOST_MULT_00_00x404c
+#define XCVR_DIAG_PLLDRC_CTRL  0x40e0
+#define XCVR_DIAG_PLLDRC_CTRL  0x40e0
+#define XCVR_DIAG_HSCLK_SEL0x40e1
+#define XCVR_DIAG_BIDI_CTRL0x40e8
+#define TX_PSC_A0  0x4100
+#define TX_PSC_A1  0x4101
+#define TX_PSC_A2  0x4102
+#define TX_PSC_A3  0x4103
+#define TX_DIAG_TX_CTRL0x41e0
+#define TX_DIAG_TX_DRV 0x41e1
+#define TX_DIAG_BGREF_PREDRV_DELAY 0x41e7
+#define TX_DIAG_ACYA_0 0x41ff
+#define TX_DIAG_ACYA_1 0x43ff
+#define TX_DIAG_ACYA_2 0x45ff
+#define TX_DIAG_ACYA_3 0x47ff
+#define TX_ANA_CTRL_REG_1  0x5020
+#define TX_ANA_CTRL_REG_2