On Mon, 9 Dec 2024 at 10:38, Sandor Yu [off-list ref] wrote:

quoted

-----Original Message-----
From: Dmitry Baryshkov <redacted>
Sent: 2024年11月26日 22:43
To: Sandor Yu <redacted>
Cc: andrzej.hajda@intel.com; neil.armstrong@linaro.org; Laurent Pinchart
[off-list ref]; jonas@kwiboo.se;
jernej.skrabec@gmail.com; airlied@gmail.com; daniel@ffwll.ch;
robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
shawnguo@kernel.org; s.hauer@pengutronix.de; festevam@gmail.com;
vkoul@kernel.org; dri-devel@lists.freedesktop.org;
devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
linux-kernel@vger.kernel.org; linux-phy@lists.infradead.org;
mripard@kernel.org; kernel@pengutronix.de; dl-linux-imx
[off-list ref]; Oliver Brown [off-list ref];
alexander.stein@ew.tq-group.com; sam@ravnborg.org
Subject: [EXT] Re: [PATCH v19 6/8] phy: freescale: Add DisplayPort/HDMI
Combo-PHY driver for i.MX8MQ

Caution: This is an external email. Please take care when clicking links or
opening attachments. When in doubt, report the message using the 'Report
this email' button


On Tue, Nov 26, 2024 at 10:11:51PM +0800, Sandor Yu wrote:

quoted

Add Cadence HDP-TX DisplayPort and 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.
DisplayPort or HDMI PHY mode is configured in the driver.

Signed-off-by: Sandor Yu <redacted>
Signed-off-by: Alexander Stein <redacted>
---
v18->v19:
- Simplify the PLL tables by removing unused and constant data
- Remove PHY power management, controller driver will handle them.
- Remove enum dp_link_rate
- Introduce read_pll_timeout.
- Update clock management as devm_clk_get_enabled() introduced.
- Remove cdns_hdptx_phy_init() and cdns_hdptx_phy_remove().

v17->v18:
- fix build error as code rebase to latest kernel version.

 drivers/phy/freescale/Kconfig                |   10 +
 drivers/phy/freescale/Makefile               |    1 +
 drivers/phy/freescale/phy-fsl-imx8mq-hdptx.c | 1237

++++++++++++++++++

quoted

 3 files changed, 1248 insertions(+)
 create mode 100644 drivers/phy/freescale/phy-fsl-imx8mq-hdptx.c

diff --git a/drivers/phy/freescale/Kconfig b/drivers/phy/freescale/Kconfig
index dcd9acff6d01a..bbd17e9556cc3 100644
--- a/drivers/phy/freescale/Kconfig
+++ b/drivers/phy/freescale/Kconfig

@@ -35,6 +35,16 @@ config PHY_FSL_IMX8M_PCIE
        Enable this to add support for the PCIE PHY as found on
        i.MX8M family of SOCs.

+config PHY_FSL_IMX8MQ_HDPTX
+     tristate "Freescale i.MX8MQ DP/HDMI PHY support"
+     depends on OF && HAS_IOMEM
+     depends on COMMON_CLK
+     depends on CDNS_MHDP_HELPER

You should not be depending on the symbol that can not be selected by
the user.

OK.

quoted

quoted

+     select GENERIC_PHY
+     help
+       Enable this to support the Cadence HDPTX DP/HDMI PHY driver
+       on i.MX8MQ SOC.
+
 config PHY_FSL_IMX8QM_HSIO
      tristate "Freescale i.MX8QM HSIO PHY"
      depends on OF && HAS_IOMEM

diff --git a/drivers/phy/freescale/Makefile b/drivers/phy/freescale/Makefile
index 658eac7d0a622..a946b87905498 100644
--- a/drivers/phy/freescale/Makefile
+++ b/drivers/phy/freescale/Makefile

@@ -1,4 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0-only
+obj-$(CONFIG_PHY_FSL_IMX8MQ_HDPTX)   += phy-fsl-imx8mq-hdptx.o
 obj-$(CONFIG_PHY_FSL_IMX8MQ_USB)     += phy-fsl-imx8mq-usb.o
 obj-$(CONFIG_PHY_MIXEL_LVDS_PHY)     +=

phy-fsl-imx8qm-lvds-phy.o

quoted

 obj-$(CONFIG_PHY_MIXEL_MIPI_DPHY)    += phy-fsl-imx8-mipi-dphy.o

diff --git a/drivers/phy/freescale/phy-fsl-imx8mq-hdptx.c

b/drivers/phy/freescale/phy-fsl-imx8mq-hdptx.c

quoted

new file mode 100644
index 0000000000000..e99487622d43c

--- /dev/null
+++ b/drivers/phy/freescale/phy-fsl-imx8mq-hdptx.c

@@ -0,0 +1,1237 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Cadence DP/HDMI PHY driver
+ *
+ * Copyright (C) 2022-2024 NXP Semiconductor, Inc.
+ */
+#include <drm/bridge/cdns-mhdp-helper.h>

Still, I'd ask for drivers/soc instead.

If put it in drivers/soc, should I create a new directory called drivers/soc/cadence or put it in drivers/soc/imx?

The mhdp helpers will share code between cdns-mhdp8501 and cdns-mhdp8546.
I prefer creating a new directory, drivers/soc/cadence, but the problem is that cadence not a real SoC.

I think drivers/soc/cadence is fine. Please explain your decision in
the cover letter and in the commit message, this will help other
maintainers to understand you.

-- 
With best wishes
Dmitry