Hi,
This is the v6 series to add i.MX8qxp LVDS PHY mode support for the Mixel PHY in the Freescale i.MX8qxp SoC. Comparing to v5, this version only rebases the series upon v5.17-rc1.
The Mixel PHY is MIPI DPHY + LVDS PHY combo, which can works in either MIPI DPHY mode or LVDS PHY mode. The PHY mode is controlled by i.MX8qxp SCU firmware. The PHY driver would call a SCU function to configure the mode.
The PHY driver is already supporting the Mixel MIPI DPHY in i.MX8mq SoC, where it appears to be a single MIPI DPHY.
Patch 1/5 sets PHY mode in the Northwest Logic MIPI DSI host controller bridge driver, since i.MX8qxp SoC embeds this controller IP to support MIPI DSI displays together with the Mixel PHY.
Patch 2/5 allows LVDS PHYs to be configured through the generic PHY functions and through a custom structure added to the generic PHY configuration union.
Patch 3/5 converts mixel,mipi-dsi-phy plain text dt binding to json-schema.
Patch 4/5 adds dt binding support for the Mixel combo PHY in i.MX8qxp SoC.
Patch 5/5 adds the i.MX8qxp LVDS PHY mode support in the Mixel PHY driver.
Welcome comments, thanks.
v5->v6: * Rebase the series upon v5.17-rc1. * Set PHY mode in ->mode_set() instead of ->pre_enable() in the nwl-dsi bridge driver in patch 1/5 due to the rebase. * Drop Guido's R-b tag on patch 1/5 due to the rebase. * Resend with linux-phy ML Cc'ed and reviewer mail addresses updated for patch 1/5.
v4->v5: * Align kernel-doc style of include/linux/phy/phy-lvds.h to include/linux/phy/phy.h for patch 2/5. (Vinod) * Trivial tweaks on patch 2/5. * Drop Robert's R-b tag on patch 2/5.
v3->v4: * Add all R-b tags received from v3 on relevant patches and respin. (Robert)
v2->v3: * Improve readability of mixel_dphy_set_mode() in the Mixel PHY driver. (Guido) * Improve the 'clock-names' property in the PHY dt binding.
v1->v2: * Convert mixel,mipi-dsi-phy plain text dt binding to json-schema. (Guido) * Print invalid PHY mode in dmesg from the Mixel PHY driver. (Guido) * Add Guido's R-b tag on the patch for the nwl-dsi drm bridge driver.
Liu Ying (5): drm/bridge: nwl-dsi: Set PHY mode in nwl_dsi_mode_set() phy: Add LVDS configuration options dt-bindings: phy: Convert mixel,mipi-dsi-phy to json-schema dt-bindings: phy: mixel: mipi-dsi-phy: Add Mixel combo PHY support for i.MX8qxp phy: freescale: phy-fsl-imx8-mipi-dphy: Add i.MX8qxp LVDS PHY mode support
.../bindings/phy/mixel,mipi-dsi-phy.txt | 29 -- .../bindings/phy/mixel,mipi-dsi-phy.yaml | 107 +++++++ drivers/gpu/drm/bridge/nwl-dsi.c | 6 + .../phy/freescale/phy-fsl-imx8-mipi-dphy.c | 269 +++++++++++++++++- include/linux/phy/phy-lvds.h | 32 +++ include/linux/phy/phy.h | 4 + 6 files changed, 407 insertions(+), 40 deletions(-) delete mode 100644 Documentation/devicetree/bindings/phy/mixel,mipi-dsi-phy.txt create mode 100644 Documentation/devicetree/bindings/phy/mixel,mipi-dsi-phy.yaml create mode 100644 include/linux/phy/phy-lvds.h
The Northwest Logic MIPI DSI host controller embedded in i.MX8qxp works with a Mixel MIPI DPHY + LVDS PHY combo to support either a MIPI DSI display or a LVDS display. So, this patch calls phy_set_mode() from nwl_dsi_mode_set() to set PHY mode to MIPI DPHY explicitly.
Cc: Guido Günther agx@sigxcpu.org Cc: Robert Chiras robert.chiras@nxp.com Cc: Martin Kepplinger martin.kepplinger@puri.sm Cc: Andrzej Hajda andrzej.hajda@intel.com Cc: Neil Armstrong narmstrong@baylibre.com Cc: Laurent Pinchart Laurent.pinchart@ideasonboard.com Cc: Jonas Karlman jonas@kwiboo.se Cc: Jernej Skrabec jernej.skrabec@gmail.com Cc: David Airlie airlied@linux.ie Cc: Daniel Vetter daniel@ffwll.ch Cc: NXP Linux Team linux-imx@nxp.com Signed-off-by: Liu Ying victor.liu@nxp.com --- v5->v6: * Rebase the series upon v5.17-rc1. * Set PHY mode in ->mode_set() instead of ->pre_enable() in the nwl-dsi bridge driver due to the rebase. * Drop Guido's R-b tag due to the rebase. * Resend with Andrzej's and Jernej's mail addressed updated.
v4->v5: * No change.
v3->v4: * No change.
v2->v3: * No change.
v1->v2: * Add Guido's R-b tag.
drivers/gpu/drm/bridge/nwl-dsi.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/bridge/nwl-dsi.c b/drivers/gpu/drm/bridge/nwl-dsi.c index a7389a0facfb..5fbfc68a46a5 100644 --- a/drivers/gpu/drm/bridge/nwl-dsi.c +++ b/drivers/gpu/drm/bridge/nwl-dsi.c @@ -673,6 +673,12 @@ static int nwl_dsi_mode_set(struct nwl_dsi *dsi) return ret; }
+ ret = phy_set_mode(dsi->phy, PHY_MODE_MIPI_DPHY); + if (ret < 0) { + DRM_DEV_ERROR(dev, "Failed to set DSI phy mode: %d\n", ret); + goto uninit_phy; + } + ret = phy_configure(dsi->phy, phy_cfg); if (ret < 0) { DRM_DEV_ERROR(dev, "Failed to configure DSI phy: %d\n", ret);
This patch allows LVDS PHYs to be configured through the generic functions and through a custom structure added to the generic union.
The parameters added here are based on common LVDS PHY implementation practices. The set of parameters should cover all potential users.
Cc: Kishon Vijay Abraham I kishon@ti.com Cc: Vinod Koul vkoul@kernel.org Cc: NXP Linux Team linux-imx@nxp.com Signed-off-by: Liu Ying victor.liu@nxp.com --- v5->v6: * Rebase upon v5.17-rc1.
v4->v5: * Align kernel-doc style to include/linux/phy/phy.h. (Vinod) * Trivial tweaks. * Drop Robert's R-b tag.
v3->v4: * Add Robert's R-b tag.
v2->v3: * No change.
v1->v2: * No change.
include/linux/phy/phy-lvds.h | 32 ++++++++++++++++++++++++++++++++ include/linux/phy/phy.h | 4 ++++ 2 files changed, 36 insertions(+) create mode 100644 include/linux/phy/phy-lvds.h
diff --git a/include/linux/phy/phy-lvds.h b/include/linux/phy/phy-lvds.h new file mode 100644 index 000000000000..7a2f4747f624 --- /dev/null +++ b/include/linux/phy/phy-lvds.h @@ -0,0 +1,32 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright 2020 NXP + */ + +#ifndef __PHY_LVDS_H_ +#define __PHY_LVDS_H_ + +/** + * struct phy_configure_opts_lvds - LVDS configuration set + * @bits_per_lane_and_dclk_cycle: Number of bits per data lane and + * differential clock cycle. + * @differential_clk_rate: Clock rate, in Hertz, of the LVDS + * differential clock. + * @lanes: Number of active, consecutive, + * data lanes, starting from lane 0, + * used for the transmissions. + * @is_slave: Boolean, true if the phy is a slave + * which works together with a master + * phy to support dual link transmission, + * otherwise a regular phy or a master phy. + * + * This structure is used to represent the configuration state of a LVDS phy. + */ +struct phy_configure_opts_lvds { + unsigned int bits_per_lane_and_dclk_cycle; + unsigned long differential_clk_rate; + unsigned int lanes; + bool is_slave; +}; + +#endif /* __PHY_LVDS_H_ */ diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h index f3286f4cd306..b1413757fcc3 100644 --- a/include/linux/phy/phy.h +++ b/include/linux/phy/phy.h @@ -17,6 +17,7 @@ #include <linux/regulator/consumer.h>
#include <linux/phy/phy-dp.h> +#include <linux/phy/phy-lvds.h> #include <linux/phy/phy-mipi-dphy.h>
struct phy; @@ -57,10 +58,13 @@ enum phy_media { * the MIPI_DPHY phy mode. * @dp: Configuration set applicable for phys supporting * the DisplayPort protocol. + * @lvds: Configuration set applicable for phys supporting + * the LVDS phy mode. */ union phy_configure_opts { struct phy_configure_opts_mipi_dphy mipi_dphy; struct phy_configure_opts_dp dp; + struct phy_configure_opts_lvds lvds; };
/**
On 02-04-22, 13:24, Liu Ying wrote:
This patch allows LVDS PHYs to be configured through the generic functions and through a custom structure added to the generic union.
The parameters added here are based on common LVDS PHY implementation practices. The set of parameters should cover all potential users.
Cc: Kishon Vijay Abraham I kishon@ti.com Cc: Vinod Koul vkoul@kernel.org Cc: NXP Linux Team linux-imx@nxp.com Signed-off-by: Liu Ying victor.liu@nxp.com
v5->v6:
- Rebase upon v5.17-rc1.
v4->v5:
- Align kernel-doc style to include/linux/phy/phy.h. (Vinod)
- Trivial tweaks.
- Drop Robert's R-b tag.
v3->v4:
- Add Robert's R-b tag.
v2->v3:
- No change.
v1->v2:
- No change.
include/linux/phy/phy-lvds.h | 32 ++++++++++++++++++++++++++++++++ include/linux/phy/phy.h | 4 ++++ 2 files changed, 36 insertions(+) create mode 100644 include/linux/phy/phy-lvds.h
diff --git a/include/linux/phy/phy-lvds.h b/include/linux/phy/phy-lvds.h new file mode 100644 index 000000000000..7a2f4747f624 --- /dev/null +++ b/include/linux/phy/phy-lvds.h @@ -0,0 +1,32 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/*
- Copyright 2020 NXP
2022 now
- */
+#ifndef __PHY_LVDS_H_ +#define __PHY_LVDS_H_
+/**
- struct phy_configure_opts_lvds - LVDS configuration set
- @bits_per_lane_and_dclk_cycle: Number of bits per data lane and
differential clock cycle.
What does it mean by bits per data lane and differential clock cycle?
- @differential_clk_rate: Clock rate, in Hertz, of the LVDS
differential clock.
- @lanes: Number of active, consecutive,
data lanes, starting from lane 0,
used for the transmissions.
- @is_slave: Boolean, true if the phy is a slave
which works together with a master
phy to support dual link transmission,
otherwise a regular phy or a master phy.
- This structure is used to represent the configuration state of a LVDS phy.
- */
+struct phy_configure_opts_lvds {
- unsigned int bits_per_lane_and_dclk_cycle;
- unsigned long differential_clk_rate;
- unsigned int lanes;
- bool is_slave;
+};
Where is the user of this new configuration? Can you post that patch for reference as well please
Hi Vinod,
On Wed, 2022-04-13 at 11:41 +0530, Vinod Koul wrote:
On 02-04-22, 13:24, Liu Ying wrote:
This patch allows LVDS PHYs to be configured through the generic functions and through a custom structure added to the generic union.
The parameters added here are based on common LVDS PHY implementation practices. The set of parameters should cover all potential users.
Cc: Kishon Vijay Abraham I kishon@ti.com Cc: Vinod Koul vkoul@kernel.org Cc: NXP Linux Team linux-imx@nxp.com Signed-off-by: Liu Ying victor.liu@nxp.com
v5->v6:
- Rebase upon v5.17-rc1.
v4->v5:
- Align kernel-doc style to include/linux/phy/phy.h. (Vinod)
- Trivial tweaks.
- Drop Robert's R-b tag.
v3->v4:
- Add Robert's R-b tag.
v2->v3:
- No change.
v1->v2:
- No change.
include/linux/phy/phy-lvds.h | 32 ++++++++++++++++++++++++++++++++ include/linux/phy/phy.h | 4 ++++ 2 files changed, 36 insertions(+) create mode 100644 include/linux/phy/phy-lvds.h
diff --git a/include/linux/phy/phy-lvds.h b/include/linux/phy/phy- lvds.h new file mode 100644 index 000000000000..7a2f4747f624 --- /dev/null +++ b/include/linux/phy/phy-lvds.h @@ -0,0 +1,32 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/*
- Copyright 2020 NXP
2022 now
I may change it to 'Copyright 2020,2022 NXP'.
- */
+#ifndef __PHY_LVDS_H_ +#define __PHY_LVDS_H_
+/**
- struct phy_configure_opts_lvds - LVDS configuration set
- @bits_per_lane_and_dclk_cycle: Number of bits per data lane
and
differential clock cycle.What does it mean by bits per data lane and differential clock cycle?
Please check Documentation/devicetree/bindings/display/panel/lvds.yaml. lvds.yaml metions slot. 'bits_per_lane_and_dclk_cycle' means the number of slots. But, I don't find the word 'slot' in my lvds relevant specs which mentioned in lvds.yaml, so 'slots' is probably not a generic name(lvds.yaml is for display panel). So, I use 'bits_per_lane_and_dclk_cycle' as the name tells what it means.
- @differential_clk_rate: Clock rate, in Hertz, of the
LVDS
differential clock.
- @lanes: Number of active, consecutive,
data lanes, starting from lane0,
used for the transmissions.
- @is_slave: Boolean, true if the
phy is a slave
which works together with amaster
phy to support dual linktransmission,
otherwise a regular phy or amaster phy.
- This structure is used to represent the configuration state of
a LVDS phy.
- */
+struct phy_configure_opts_lvds {
- unsigned int bits_per_lane_and_dclk_cycle;
- unsigned long differential_clk_rate;
- unsigned int lanes;
- bool is_slave;
+};
Where is the user of this new configuration? Can you post that patch for reference as well please
Patch 5/5 uses it. Also, I posted two drm bridge drivers[1][2] which use it.
[1] https://patchwork.kernel.org/project/dri-devel/patch/1617172405-12962-13-git... [2] https://patchwork.kernel.org/project/dri-devel/patch/1617172405-12962-14-git...
Regards, Liu Ying
On 13-04-22, 18:04, Liu Ying wrote:
Hi Vinod,
On Wed, 2022-04-13 at 11:41 +0530, Vinod Koul wrote:
On 02-04-22, 13:24, Liu Ying wrote:
This patch allows LVDS PHYs to be configured through the generic functions and through a custom structure added to the generic union.
The parameters added here are based on common LVDS PHY implementation practices. The set of parameters should cover all potential users.
Cc: Kishon Vijay Abraham I kishon@ti.com Cc: Vinod Koul vkoul@kernel.org Cc: NXP Linux Team linux-imx@nxp.com Signed-off-by: Liu Ying victor.liu@nxp.com
v5->v6:
- Rebase upon v5.17-rc1.
v4->v5:
- Align kernel-doc style to include/linux/phy/phy.h. (Vinod)
- Trivial tweaks.
- Drop Robert's R-b tag.
v3->v4:
- Add Robert's R-b tag.
v2->v3:
- No change.
v1->v2:
- No change.
include/linux/phy/phy-lvds.h | 32 ++++++++++++++++++++++++++++++++ include/linux/phy/phy.h | 4 ++++ 2 files changed, 36 insertions(+) create mode 100644 include/linux/phy/phy-lvds.h
diff --git a/include/linux/phy/phy-lvds.h b/include/linux/phy/phy- lvds.h new file mode 100644 index 000000000000..7a2f4747f624 --- /dev/null +++ b/include/linux/phy/phy-lvds.h @@ -0,0 +1,32 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/*
- Copyright 2020 NXP
2022 now
I may change it to 'Copyright 2020,2022 NXP'.
- */
+#ifndef __PHY_LVDS_H_ +#define __PHY_LVDS_H_
+/**
- struct phy_configure_opts_lvds - LVDS configuration set
- @bits_per_lane_and_dclk_cycle: Number of bits per data lane
and
differential clock cycle.What does it mean by bits per data lane and differential clock cycle?
Please check Documentation/devicetree/bindings/display/panel/lvds.yaml. lvds.yaml metions slot. 'bits_per_lane_and_dclk_cycle' means the number of slots. But, I don't find the word 'slot' in my lvds relevant specs which mentioned in lvds.yaml, so 'slots' is probably not a generic name(lvds.yaml is for display panel). So, I use 'bits_per_lane_and_dclk_cycle' as the name tells what it means.
variable name is fine, explanation for bit per lane and differential clock cycle didnt help, maybe add better explanation of what this variable means
- @differential_clk_rate: Clock rate, in Hertz, of the
LVDS
differential clock.
- @lanes: Number of active, consecutive,
data lanes, starting from lane0,
used for the transmissions.
- @is_slave: Boolean, true if the
phy is a slave
which works together with amaster
phy to support dual linktransmission,
otherwise a regular phy or amaster phy.
- This structure is used to represent the configuration state of
a LVDS phy.
- */
+struct phy_configure_opts_lvds {
- unsigned int bits_per_lane_and_dclk_cycle;
- unsigned long differential_clk_rate;
- unsigned int lanes;
- bool is_slave;
+};
Where is the user of this new configuration? Can you post that patch for reference as well please
Patch 5/5 uses it. Also, I posted two drm bridge drivers[1][2] which use it.
That is phy driver not the user
[1] https://patchwork.kernel.org/project/dri-devel/patch/1617172405-12962-13-git... [2] https://patchwork.kernel.org/project/dri-devel/patch/1617172405-12962-14-git...
this is helpful, thanks
On Wed, 2022-04-13 at 16:19 +0530, Vinod Koul wrote:
On 13-04-22, 18:04, Liu Ying wrote:
Hi Vinod,
On Wed, 2022-04-13 at 11:41 +0530, Vinod Koul wrote:
On 02-04-22, 13:24, Liu Ying wrote:
This patch allows LVDS PHYs to be configured through the generic functions and through a custom structure added to the generic union.
The parameters added here are based on common LVDS PHY implementation practices. The set of parameters should cover all potential users.
Cc: Kishon Vijay Abraham I kishon@ti.com Cc: Vinod Koul vkoul@kernel.org Cc: NXP Linux Team linux-imx@nxp.com Signed-off-by: Liu Ying victor.liu@nxp.com
[...]
- */
+#ifndef __PHY_LVDS_H_ +#define __PHY_LVDS_H_
+/**
- struct phy_configure_opts_lvds - LVDS configuration set
- @bits_per_lane_and_dclk_cycle: Number of bits per data
lane and
differential clockcycle.
What does it mean by bits per data lane and differential clock cycle?
Please check Documentation/devicetree/bindings/display/panel/lvds.yaml. lvds.yaml metions slot. 'bits_per_lane_and_dclk_cycle' means the number of slots. But, I don't find the word 'slot' in my lvds relevant specs which mentioned in lvds.yaml, so 'slots' is probably not a generic name(lvds.yaml is for display panel). So, I use 'bits_per_lane_and_dclk_cycle' as the name tells what it means.
variable name is fine, explanation for bit per lane and differential clock cycle didnt help, maybe add better explanation of what this variable means
I may add an example diagram as below...
- @differential_clk_rate: Clock rate, in Hertz,
of the LVDS
differential clock.
- @lanes: Number of active,
consecutive,
data lanes, startingfrom lane 0,
used for thetransmissions.
- @is_slave: Boolean, true if the
phy is a slave
which works togetherwith a master
phy to support duallink transmission,
otherwise a regular phyor a master phy.
---------------------------------8<------------------------------------ + * This is an example with 4 lanes and 7 bits per data lane and differential + * clock cycle: + * + * :: + * |<------------- one differential clock cycle -------- ----->| + * ________________ ____________ _____ + * Clock _______________________/ + * ______ ______ ______ ______ ______ ______ ____ __ + * Lane0 ><_bit0_><_bit1_><_bit2_><_bit3_><_bit4_><_bit5_><_bit 6_>< + * Lane1 ><_bit0_><_bit1_><_bit2_><_bit3_><_bit4_><_bit5_><_bit 6_>< + * Lane2 ><_bit0_><_bit1_><_bit2_><_bit3_><_bit4_><_bit5_><_bit 6_>< + * Lane3 ><_bit0_><_bit1_><_bit2_><_bit3_><_bit4_><_bit5_><_bit 6_>< ---------------------------------8<------------------------------------
What do you think?
Regards, Liu Ying
On 13-04-22, 20:39, Liu Ying wrote:
On Wed, 2022-04-13 at 16:19 +0530, Vinod Koul wrote:
On 13-04-22, 18:04, Liu Ying wrote:
Hi Vinod,
On Wed, 2022-04-13 at 11:41 +0530, Vinod Koul wrote:
On 02-04-22, 13:24, Liu Ying wrote:
This patch allows LVDS PHYs to be configured through the generic functions and through a custom structure added to the generic union.
The parameters added here are based on common LVDS PHY implementation practices. The set of parameters should cover all potential users.
Cc: Kishon Vijay Abraham I kishon@ti.com Cc: Vinod Koul vkoul@kernel.org Cc: NXP Linux Team linux-imx@nxp.com Signed-off-by: Liu Ying victor.liu@nxp.com
[...]
- */
+#ifndef __PHY_LVDS_H_ +#define __PHY_LVDS_H_
+/**
- struct phy_configure_opts_lvds - LVDS configuration set
- @bits_per_lane_and_dclk_cycle: Number of bits per data
lane and
differential clockcycle.
What does it mean by bits per data lane and differential clock cycle?
Please check Documentation/devicetree/bindings/display/panel/lvds.yaml. lvds.yaml metions slot. 'bits_per_lane_and_dclk_cycle' means the number of slots. But, I don't find the word 'slot' in my lvds relevant specs which mentioned in lvds.yaml, so 'slots' is probably not a generic name(lvds.yaml is for display panel). So, I use 'bits_per_lane_and_dclk_cycle' as the name tells what it means.
variable name is fine, explanation for bit per lane and differential clock cycle didnt help, maybe add better explanation of what this variable means
I may add an example diagram as below...
Not really a diagram, you can add if you like.. But something which explains in a sentence or few about the variable.
bits per lane per differential clock cycle ?
On Thu, 2022-04-14 at 11:07 +0530, Vinod Koul wrote:
On 13-04-22, 20:39, Liu Ying wrote:
On Wed, 2022-04-13 at 16:19 +0530, Vinod Koul wrote:
On 13-04-22, 18:04, Liu Ying wrote:
Hi Vinod,
On Wed, 2022-04-13 at 11:41 +0530, Vinod Koul wrote:
On 02-04-22, 13:24, Liu Ying wrote:
This patch allows LVDS PHYs to be configured through the generic functions and through a custom structure added to the generic union.
The parameters added here are based on common LVDS PHY implementation practices. The set of parameters should cover all potential users.
Cc: Kishon Vijay Abraham I kishon@ti.com Cc: Vinod Koul vkoul@kernel.org Cc: NXP Linux Team linux-imx@nxp.com Signed-off-by: Liu Ying victor.liu@nxp.com
[...]
- */
+#ifndef __PHY_LVDS_H_ +#define __PHY_LVDS_H_
+/**
- struct phy_configure_opts_lvds - LVDS configuration set
- @bits_per_lane_and_dclk_cycle: Number of bits per data
lane and
differential clockcycle.
What does it mean by bits per data lane and differential clock cycle?
Please check Documentation/devicetree/bindings/display/panel/lvds.yaml. lvds.yaml metions slot. 'bits_per_lane_and_dclk_cycle' means the number of slots. But, I don't find the word 'slot' in my lvds relevant specs which mentioned in lvds.yaml, so 'slots' is probably not a generic name(lvds.yaml is for display panel). So, I use 'bits_per_lane_and_dclk_cycle' as the name tells what it means.
variable name is fine, explanation for bit per lane and differential clock cycle didnt help, maybe add better explanation of what this variable means
I may add an example diagram as below...
Not really a diagram, you can add if you like.. But something which explains in a sentence or few about the variable.
Ok, no diagram.
bits per lane per differential clock cycle ?
Ok, will use this explaination.
Regards, Liu Ying
This patch converts the mixel,mipi-dsi-phy binding to DT schema format using json-schema.
Comparing to the plain text version, the new binding adds the 'assigned-clocks', 'assigned-clock-parents' and 'assigned-clock-rates' properites, otherwise 'make dtbs_check' would complain that there are mis-matches. Also, the new binding requires the 'power-domains' property since all potential SoCs that embed this PHY would provide a power domain for it. The example of the new binding takes reference to the latest dphy node in imx8mq.dtsi.
Cc: Guido Günther agx@sigxcpu.org Cc: Kishon Vijay Abraham I kishon@ti.com Cc: Vinod Koul vkoul@kernel.org Cc: Rob Herring robh+dt@kernel.org Cc: NXP Linux Team linux-imx@nxp.com Reviewed-by: Rob Herring robh@kernel.org Reviewed-by: Guido Günther agx@sigxcpu.org Signed-off-by: Liu Ying victor.liu@nxp.com --- v5->v6: * No change.
v4->v5: * No change.
v3->v4: * Add Rob's and Guido's R-b tags.
v2->v3: * Improve the 'clock-names' property by dropping 'items:'.
v1->v2: * Newly introduced in v2. (Guido)
.../bindings/phy/mixel,mipi-dsi-phy.txt | 29 -------- .../bindings/phy/mixel,mipi-dsi-phy.yaml | 72 +++++++++++++++++++ 2 files changed, 72 insertions(+), 29 deletions(-) delete mode 100644 Documentation/devicetree/bindings/phy/mixel,mipi-dsi-phy.txt create mode 100644 Documentation/devicetree/bindings/phy/mixel,mipi-dsi-phy.yaml
diff --git a/Documentation/devicetree/bindings/phy/mixel,mipi-dsi-phy.txt b/Documentation/devicetree/bindings/phy/mixel,mipi-dsi-phy.txt deleted file mode 100644 index 9b23407233c0..000000000000 --- a/Documentation/devicetree/bindings/phy/mixel,mipi-dsi-phy.txt +++ /dev/null @@ -1,29 +0,0 @@ -Mixel DSI PHY for i.MX8 - -The Mixel MIPI-DSI PHY IP block is e.g. found on i.MX8 platforms (along the -MIPI-DSI IP from Northwest Logic). It represents the physical layer for the -electrical signals for DSI. - -Required properties: -- compatible: Must be: - - "fsl,imx8mq-mipi-dphy" -- clocks: Must contain an entry for each entry in clock-names. -- clock-names: Must contain the following entries: - - "phy_ref": phandle and specifier referring to the DPHY ref clock -- reg: the register range of the PHY controller -- #phy-cells: number of cells in PHY, as defined in - Documentation/devicetree/bindings/phy/phy-bindings.txt - this must be <0> - -Optional properties: -- power-domains: phandle to power domain - -Example: - dphy: dphy@30a0030 { - compatible = "fsl,imx8mq-mipi-dphy"; - clocks = <&clk IMX8MQ_CLK_DSI_PHY_REF>; - clock-names = "phy_ref"; - reg = <0x30a00300 0x100>; - power-domains = <&pd_mipi0>; - #phy-cells = <0>; - }; diff --git a/Documentation/devicetree/bindings/phy/mixel,mipi-dsi-phy.yaml b/Documentation/devicetree/bindings/phy/mixel,mipi-dsi-phy.yaml new file mode 100644 index 000000000000..c34f2e6d6bd5 --- /dev/null +++ b/Documentation/devicetree/bindings/phy/mixel,mipi-dsi-phy.yaml @@ -0,0 +1,72 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/phy/mixel,mipi-dsi-phy.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Mixel DSI PHY for i.MX8 + +maintainers: + - Guido Günther agx@sigxcpu.org + +description: | + The Mixel MIPI-DSI PHY IP block is e.g. found on i.MX8 platforms (along the + MIPI-DSI IP from Northwest Logic). It represents the physical layer for the + electrical signals for DSI. + +properties: + compatible: + enum: + - fsl,imx8mq-mipi-dphy + + reg: + maxItems: 1 + + clocks: + maxItems: 1 + + clock-names: + const: phy_ref + + assigned-clocks: + maxItems: 1 + + assigned-clock-parents: + maxItems: 1 + + assigned-clock-rates: + maxItems: 1 + + "#phy-cells": + const: 0 + + power-domains: + maxItems: 1 + +required: + - compatible + - reg + - clocks + - clock-names + - assigned-clocks + - assigned-clock-parents + - assigned-clock-rates + - "#phy-cells" + - power-domains + +additionalProperties: false + +examples: + - | + #include <dt-bindings/clock/imx8mq-clock.h> + dphy: dphy@30a0030 { + compatible = "fsl,imx8mq-mipi-dphy"; + reg = <0x30a00300 0x100>; + clocks = <&clk IMX8MQ_CLK_DSI_PHY_REF>; + clock-names = "phy_ref"; + assigned-clocks = <&clk IMX8MQ_CLK_DSI_PHY_REF>; + assigned-clock-parents = <&clk IMX8MQ_VIDEO_PLL1_OUT>; + assigned-clock-rates = <24000000>; + #phy-cells = <0>; + power-domains = <&pgc_mipi>; + };
Add support for Mixel MIPI DPHY + LVDS PHY combo IP as found on Freescale i.MX8qxp SoC.
Cc: Guido Günther agx@sigxcpu.org Cc: Kishon Vijay Abraham I kishon@ti.com Cc: Vinod Koul vkoul@kernel.org Cc: Rob Herring robh+dt@kernel.org Cc: NXP Linux Team linux-imx@nxp.com Reviewed-by: Rob Herring robh@kernel.org Reviewed-by: Guido Günther agx@sigxcpu.org Signed-off-by: Liu Ying victor.liu@nxp.com --- v5->v6: * No change.
v4->v5: * No change.
v3->v4: * Add Rob's and Guido's R-b tags.
v2->v3: * No change.
v1->v2: * Add the binding for i.MX8qxp Mixel combo PHY based on the converted binding. (Guido)
.../bindings/phy/mixel,mipi-dsi-phy.yaml | 41 +++++++++++++++++-- 1 file changed, 38 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/phy/mixel,mipi-dsi-phy.yaml b/Documentation/devicetree/bindings/phy/mixel,mipi-dsi-phy.yaml index c34f2e6d6bd5..786cfd71cb7e 100644 --- a/Documentation/devicetree/bindings/phy/mixel,mipi-dsi-phy.yaml +++ b/Documentation/devicetree/bindings/phy/mixel,mipi-dsi-phy.yaml @@ -14,10 +14,14 @@ description: | MIPI-DSI IP from Northwest Logic). It represents the physical layer for the electrical signals for DSI.
+ The Mixel PHY IP block found on i.MX8qxp is a combo PHY that can work + in either MIPI-DSI PHY mode or LVDS PHY mode. + properties: compatible: enum: - fsl,imx8mq-mipi-dphy + - fsl,imx8qxp-mipi-dphy
reg: maxItems: 1 @@ -40,6 +44,11 @@ properties: "#phy-cells": const: 0
+ fsl,syscon: + $ref: /schemas/types.yaml#/definitions/phandle + description: | + A phandle which points to Control and Status Registers(CSR) module. + power-domains: maxItems: 1
@@ -48,12 +57,38 @@ required: - reg - clocks - clock-names - - assigned-clocks - - assigned-clock-parents - - assigned-clock-rates - "#phy-cells" - power-domains
+allOf: + - if: + properties: + compatible: + contains: + const: fsl,imx8mq-mipi-dphy + then: + properties: + fsl,syscon: false + + required: + - assigned-clocks + - assigned-clock-parents + - assigned-clock-rates + + - if: + properties: + compatible: + contains: + const: fsl,imx8qxp-mipi-dphy + then: + properties: + assigned-clocks: false + assigned-clock-parents: false + assigned-clock-rates: false + + required: + - fsl,syscon + additionalProperties: false
examples:
i.MX8qxp SoC embeds a Mixel MIPI DPHY + LVDS PHY combo which supports either a MIPI DSI display or a LVDS display. The PHY mode is controlled by SCU firmware and the driver would call a SCU firmware function to configure the PHY mode. The single LVDS PHY has 4 data lanes to support a LVDS display. Also, with a master LVDS PHY and a slave LVDS PHY, they may work together to support a LVDS display with 8 data lanes(usually, dual LVDS link display). Note that this patch supports the LVDS PHY mode only for the i.MX8qxp Mixel combo PHY, i.e., the MIPI DPHY mode is yet to be supported, so for now error would be returned from ->set_mode() if MIPI DPHY mode is passed over to it for the combo PHY.
Cc: Guido Günther agx@sigxcpu.org Cc: Robert Chiras robert.chiras@nxp.com Cc: Kishon Vijay Abraham I kishon@ti.com Cc: Vinod Koul vkoul@kernel.org Cc: Shawn Guo shawnguo@kernel.org Cc: Sascha Hauer s.hauer@pengutronix.de Cc: Pengutronix Kernel Team kernel@pengutronix.de Cc: Fabio Estevam festevam@gmail.com Cc: NXP Linux Team linux-imx@nxp.com Reviewed-by: Guido Günther agx@sigxcpu.org Signed-off-by: Liu Ying victor.liu@nxp.com --- v5->v6: * No change.
v4->v5: * No change.
v3->v4: * Add Guido's R-b tag.
v2->v3: * Improve readability of mixel_dphy_set_mode(). (Guido)
v1->v2: * Print invalid PHY mode in dmesg. (Guido)
.../phy/freescale/phy-fsl-imx8-mipi-dphy.c | 269 +++++++++++++++++- 1 file changed, 258 insertions(+), 11 deletions(-)
diff --git a/drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c b/drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c index a95572b397ca..af1ecda091c7 100644 --- a/drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c +++ b/drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c @@ -4,17 +4,31 @@ * Copyright 2019 Purism SPC */
+#include <linux/bitfield.h> #include <linux/clk.h> #include <linux/clk-provider.h> #include <linux/delay.h> +#include <linux/firmware/imx/ipc.h> +#include <linux/firmware/imx/svc/misc.h> #include <linux/io.h> #include <linux/kernel.h> +#include <linux/mfd/syscon.h> #include <linux/module.h> #include <linux/of.h> #include <linux/of_platform.h> #include <linux/phy/phy.h> #include <linux/platform_device.h> #include <linux/regmap.h> +#include <dt-bindings/firmware/imx/rsrc.h> + +/* Control and Status Registers(CSR) */ +#define PHY_CTRL 0x00 +#define CCM_MASK GENMASK(7, 5) +#define CCM(n) FIELD_PREP(CCM_MASK, (n)) +#define CA_MASK GENMASK(4, 2) +#define CA(n) FIELD_PREP(CA_MASK, (n)) +#define RFB BIT(1) +#define LVDS_EN BIT(0)
/* DPHY registers */ #define DPHY_PD_DPHY 0x00 @@ -55,8 +69,15 @@ #define PWR_ON 0 #define PWR_OFF 1
+#define MIN_VCO_FREQ 640000000 +#define MAX_VCO_FREQ 1500000000 + +#define MIN_LVDS_REFCLK_FREQ 24000000 +#define MAX_LVDS_REFCLK_FREQ 150000000 + enum mixel_dphy_devtype { MIXEL_IMX8MQ, + MIXEL_IMX8QXP, };
struct mixel_dphy_devdata { @@ -65,6 +86,7 @@ struct mixel_dphy_devdata { u8 reg_rxlprp; u8 reg_rxcdrp; u8 reg_rxhs_settle; + bool is_combo; /* MIPI DPHY and LVDS PHY combo */ };
static const struct mixel_dphy_devdata mixel_dphy_devdata[] = { @@ -74,6 +96,10 @@ static const struct mixel_dphy_devdata mixel_dphy_devdata[] = { .reg_rxlprp = 0x40, .reg_rxcdrp = 0x44, .reg_rxhs_settle = 0x48, + .is_combo = false, + }, + [MIXEL_IMX8QXP] = { + .is_combo = true, }, };
@@ -95,8 +121,12 @@ struct mixel_dphy_cfg { struct mixel_dphy_priv { struct mixel_dphy_cfg cfg; struct regmap *regmap; + struct regmap *lvds_regmap; struct clk *phy_ref_clk; const struct mixel_dphy_devdata *devdata; + struct imx_sc_ipc *ipc_handle; + bool is_slave; + int id; };
static const struct regmap_config mixel_dphy_regmap_config = { @@ -317,7 +347,8 @@ static int mixel_dphy_set_pll_params(struct phy *phy) return 0; }
-static int mixel_dphy_configure(struct phy *phy, union phy_configure_opts *opts) +static int +mixel_dphy_configure_mipi_dphy(struct phy *phy, union phy_configure_opts *opts) { struct mixel_dphy_priv *priv = phy_get_drvdata(phy); struct mixel_dphy_cfg cfg = { 0 }; @@ -345,15 +376,121 @@ static int mixel_dphy_configure(struct phy *phy, union phy_configure_opts *opts) return 0; }
+static int +mixel_dphy_configure_lvds_phy(struct phy *phy, union phy_configure_opts *opts) +{ + struct mixel_dphy_priv *priv = phy_get_drvdata(phy); + struct phy_configure_opts_lvds *lvds_opts = &opts->lvds; + unsigned long data_rate; + unsigned long fvco; + u32 rsc; + u32 co; + int ret; + + priv->is_slave = lvds_opts->is_slave; + + /* LVDS interface pins */ + regmap_write(priv->lvds_regmap, PHY_CTRL, CCM(0x5) | CA(0x4) | RFB); + + /* enable MODE8 only for slave LVDS PHY */ + rsc = priv->id ? IMX_SC_R_MIPI_1 : IMX_SC_R_MIPI_0; + ret = imx_sc_misc_set_control(priv->ipc_handle, rsc, IMX_SC_C_DUAL_MODE, + lvds_opts->is_slave); + if (ret) { + dev_err(&phy->dev, "Failed to configure MODE8: %d\n", ret); + return ret; + } + + /* + * Choose an appropriate divider ratio to meet the requirement of + * PLL VCO frequency range. + * + * ----- 640MHz ~ 1500MHz ------------ --------------- + * | VCO | ----------------> | CO divider | -> | LVDS data rate| + * ----- FVCO ------------ --------------- + * 1/2/4/8 div 7 * differential_clk_rate + */ + data_rate = 7 * lvds_opts->differential_clk_rate; + for (co = 1; co <= 8; co *= 2) { + fvco = data_rate * co; + + if (fvco >= MIN_VCO_FREQ) + break; + } + + if (fvco < MIN_VCO_FREQ || fvco > MAX_VCO_FREQ) { + dev_err(&phy->dev, "VCO frequency %lu is out of range\n", fvco); + return -ERANGE; + } + + /* + * CO is configurable, while CN and CM are not, + * as fixed ratios 1 and 7 are applied respectively. + */ + phy_write(phy, __ffs(co), DPHY_CO); + + /* set reference clock rate */ + clk_set_rate(priv->phy_ref_clk, lvds_opts->differential_clk_rate); + + return ret; +} + +static int mixel_dphy_configure(struct phy *phy, union phy_configure_opts *opts) +{ + if (phy->attrs.mode == PHY_MODE_MIPI_DPHY) + return mixel_dphy_configure_mipi_dphy(phy, opts); + else if (phy->attrs.mode == PHY_MODE_LVDS) + return mixel_dphy_configure_lvds_phy(phy, opts); + + dev_err(&phy->dev, + "Failed to configure PHY with invalid PHY mode: %d\n", + phy->attrs.mode); + return -EINVAL; +} + +static int +mixel_dphy_validate_lvds_phy(struct phy *phy, union phy_configure_opts *opts) +{ + struct phy_configure_opts_lvds *lvds_cfg = &opts->lvds; + + if (lvds_cfg->bits_per_lane_and_dclk_cycle != 7) { + dev_err(&phy->dev, "Invalid bits per LVDS data lane: %u\n", + lvds_cfg->bits_per_lane_and_dclk_cycle); + return -EINVAL; + } + + if (lvds_cfg->lanes != 4) { + dev_err(&phy->dev, "Invalid LVDS data lanes: %u\n", + lvds_cfg->lanes); + return -EINVAL; + } + + if (lvds_cfg->differential_clk_rate < MIN_LVDS_REFCLK_FREQ || + lvds_cfg->differential_clk_rate > MAX_LVDS_REFCLK_FREQ) { + dev_err(&phy->dev, + "Invalid LVDS differential clock rate: %lu\n", + lvds_cfg->differential_clk_rate); + return -EINVAL; + } + + return 0; +} + static int mixel_dphy_validate(struct phy *phy, enum phy_mode mode, int submode, union phy_configure_opts *opts) { - struct mixel_dphy_cfg cfg = { 0 }; + if (mode == PHY_MODE_MIPI_DPHY) { + struct mixel_dphy_cfg mipi_dphy_cfg = { 0 };
- if (mode != PHY_MODE_MIPI_DPHY) - return -EINVAL; + return mixel_dphy_config_from_opts(phy, &opts->mipi_dphy, + &mipi_dphy_cfg); + } else if (mode == PHY_MODE_LVDS) { + return mixel_dphy_validate_lvds_phy(phy, opts); + }
- return mixel_dphy_config_from_opts(phy, &opts->mipi_dphy, &cfg); + dev_err(&phy->dev, + "Failed to validate PHY with invalid PHY mode: %d\n", mode); + return -EINVAL; }
static int mixel_dphy_init(struct phy *phy) @@ -373,26 +510,74 @@ static int mixel_dphy_exit(struct phy *phy) return 0; }
-static int mixel_dphy_power_on(struct phy *phy) +static int mixel_dphy_power_on_mipi_dphy(struct phy *phy) { struct mixel_dphy_priv *priv = phy_get_drvdata(phy); u32 locked; int ret;
- ret = clk_prepare_enable(priv->phy_ref_clk); - if (ret < 0) - return ret; - phy_write(phy, PWR_ON, DPHY_PD_PLL); ret = regmap_read_poll_timeout(priv->regmap, DPHY_LOCK, locked, locked, PLL_LOCK_SLEEP, PLL_LOCK_TIMEOUT); if (ret < 0) { dev_err(&phy->dev, "Could not get DPHY lock (%d)!\n", ret); - goto clock_disable; + return ret; } phy_write(phy, PWR_ON, DPHY_PD_DPHY);
+ return 0; +} + +static int mixel_dphy_power_on_lvds_phy(struct phy *phy) +{ + struct mixel_dphy_priv *priv = phy_get_drvdata(phy); + u32 locked; + int ret; + + regmap_update_bits(priv->lvds_regmap, PHY_CTRL, LVDS_EN, LVDS_EN); + + phy_write(phy, PWR_ON, DPHY_PD_DPHY); + phy_write(phy, PWR_ON, DPHY_PD_PLL); + + /* do not wait for slave LVDS PHY being locked */ + if (priv->is_slave) + return 0; + + ret = regmap_read_poll_timeout(priv->regmap, DPHY_LOCK, locked, + locked, PLL_LOCK_SLEEP, + PLL_LOCK_TIMEOUT); + if (ret < 0) { + dev_err(&phy->dev, "Could not get LVDS PHY lock (%d)!\n", ret); + return ret; + } + + return 0; +} + +static int mixel_dphy_power_on(struct phy *phy) +{ + struct mixel_dphy_priv *priv = phy_get_drvdata(phy); + int ret; + + ret = clk_prepare_enable(priv->phy_ref_clk); + if (ret < 0) + return ret; + + if (phy->attrs.mode == PHY_MODE_MIPI_DPHY) { + ret = mixel_dphy_power_on_mipi_dphy(phy); + } else if (phy->attrs.mode == PHY_MODE_LVDS) { + ret = mixel_dphy_power_on_lvds_phy(phy); + } else { + dev_err(&phy->dev, + "Failed to power on PHY with invalid PHY mode: %d\n", + phy->attrs.mode); + ret = -EINVAL; + } + + if (ret) + goto clock_disable; + return 0; clock_disable: clk_disable_unprepare(priv->phy_ref_clk); @@ -406,16 +591,51 @@ static int mixel_dphy_power_off(struct phy *phy) phy_write(phy, PWR_OFF, DPHY_PD_PLL); phy_write(phy, PWR_OFF, DPHY_PD_DPHY);
+ if (phy->attrs.mode == PHY_MODE_LVDS) + regmap_update_bits(priv->lvds_regmap, PHY_CTRL, LVDS_EN, 0); + clk_disable_unprepare(priv->phy_ref_clk);
return 0; }
+static int mixel_dphy_set_mode(struct phy *phy, enum phy_mode mode, int submode) +{ + struct mixel_dphy_priv *priv = phy_get_drvdata(phy); + int ret; + + if (priv->devdata->is_combo && mode != PHY_MODE_LVDS) { + dev_err(&phy->dev, "Failed to set PHY mode for combo PHY\n"); + return -EINVAL; + } + + if (!priv->devdata->is_combo && mode != PHY_MODE_MIPI_DPHY) { + dev_err(&phy->dev, "Failed to set PHY mode to MIPI DPHY\n"); + return -EINVAL; + } + + if (priv->devdata->is_combo) { + u32 rsc = priv->id ? IMX_SC_R_MIPI_1 : IMX_SC_R_MIPI_0; + + ret = imx_sc_misc_set_control(priv->ipc_handle, + rsc, IMX_SC_C_MODE, + mode == PHY_MODE_LVDS); + if (ret) { + dev_err(&phy->dev, + "Failed to set PHY mode via SCU ipc: %d\n", ret); + return ret; + } + } + + return 0; +} + static const struct phy_ops mixel_dphy_phy_ops = { .init = mixel_dphy_init, .exit = mixel_dphy_exit, .power_on = mixel_dphy_power_on, .power_off = mixel_dphy_power_off, + .set_mode = mixel_dphy_set_mode, .configure = mixel_dphy_configure, .validate = mixel_dphy_validate, .owner = THIS_MODULE, @@ -424,6 +644,8 @@ static const struct phy_ops mixel_dphy_phy_ops = { static const struct of_device_id mixel_dphy_of_match[] = { { .compatible = "fsl,imx8mq-mipi-dphy", .data = &mixel_dphy_devdata[MIXEL_IMX8MQ] }, + { .compatible = "fsl,imx8qxp-mipi-dphy", + .data = &mixel_dphy_devdata[MIXEL_IMX8QXP] }, { /* sentinel */ }, }; MODULE_DEVICE_TABLE(of, mixel_dphy_of_match); @@ -436,6 +658,7 @@ static int mixel_dphy_probe(struct platform_device *pdev) struct mixel_dphy_priv *priv; struct phy *phy; void __iomem *base; + int ret;
if (!np) return -ENODEV; @@ -467,6 +690,30 @@ static int mixel_dphy_probe(struct platform_device *pdev) dev_dbg(dev, "phy_ref clock rate: %lu\n", clk_get_rate(priv->phy_ref_clk));
+ if (priv->devdata->is_combo) { + priv->lvds_regmap = + syscon_regmap_lookup_by_phandle(np, "fsl,syscon"); + if (IS_ERR(priv->lvds_regmap)) { + ret = PTR_ERR(priv->lvds_regmap); + dev_err_probe(dev, ret, "Failed to get LVDS regmap\n"); + return ret; + } + + priv->id = of_alias_get_id(np, "mipi_dphy"); + if (priv->id < 0) { + dev_err(dev, "Failed to get phy node alias id: %d\n", + priv->id); + return priv->id; + } + + ret = imx_scu_get_handle(&priv->ipc_handle); + if (ret) { + dev_err_probe(dev, ret, + "Failed to get SCU ipc handle\n"); + return ret; + } + } + dev_set_drvdata(dev, priv);
phy = devm_phy_create(dev, np, &mixel_dphy_phy_ops);
On 02-04-22, 13:24, Liu Ying wrote:
i.MX8qxp SoC embeds a Mixel MIPI DPHY + LVDS PHY combo which supports either a MIPI DSI display or a LVDS display. The PHY mode is controlled by SCU firmware and the driver would call a SCU firmware function to configure the PHY mode. The single LVDS PHY has 4 data lanes to support a LVDS display. Also, with a master LVDS PHY and a slave LVDS PHY, they may work together to support a LVDS display with 8 data lanes(usually, dual LVDS link display). Note that this patch supports the LVDS PHY mode only for the i.MX8qxp Mixel combo PHY, i.e., the MIPI DPHY mode is yet to be supported, so for now error would be returned from ->set_mode() if MIPI DPHY mode is passed over to it for the combo PHY.
Cc: Guido Günther agx@sigxcpu.org Cc: Robert Chiras robert.chiras@nxp.com Cc: Kishon Vijay Abraham I kishon@ti.com Cc: Vinod Koul vkoul@kernel.org Cc: Shawn Guo shawnguo@kernel.org Cc: Sascha Hauer s.hauer@pengutronix.de Cc: Pengutronix Kernel Team kernel@pengutronix.de Cc: Fabio Estevam festevam@gmail.com Cc: NXP Linux Team linux-imx@nxp.com Reviewed-by: Guido Günther agx@sigxcpu.org Signed-off-by: Liu Ying victor.liu@nxp.com
v5->v6:
- No change.
v4->v5:
- No change.
v3->v4:
- Add Guido's R-b tag.
v2->v3:
- Improve readability of mixel_dphy_set_mode(). (Guido)
v1->v2:
- Print invalid PHY mode in dmesg. (Guido)
.../phy/freescale/phy-fsl-imx8-mipi-dphy.c | 269 +++++++++++++++++- 1 file changed, 258 insertions(+), 11 deletions(-)
diff --git a/drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c b/drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c index a95572b397ca..af1ecda091c7 100644 --- a/drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c +++ b/drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c @@ -4,17 +4,31 @@
- Copyright 2019 Purism SPC
*/
+#include <linux/bitfield.h> #include <linux/clk.h> #include <linux/clk-provider.h> #include <linux/delay.h> +#include <linux/firmware/imx/ipc.h> +#include <linux/firmware/imx/svc/misc.h> #include <linux/io.h> #include <linux/kernel.h> +#include <linux/mfd/syscon.h> #include <linux/module.h> #include <linux/of.h> #include <linux/of_platform.h> #include <linux/phy/phy.h> #include <linux/platform_device.h> #include <linux/regmap.h> +#include <dt-bindings/firmware/imx/rsrc.h>
+/* Control and Status Registers(CSR) */ +#define PHY_CTRL 0x00 +#define CCM_MASK GENMASK(7, 5) +#define CCM(n) FIELD_PREP(CCM_MASK, (n)) +#define CA_MASK GENMASK(4, 2) +#define CA(n) FIELD_PREP(CA_MASK, (n)) +#define RFB BIT(1) +#define LVDS_EN BIT(0)
/* DPHY registers */ #define DPHY_PD_DPHY 0x00 @@ -55,8 +69,15 @@ #define PWR_ON 0 #define PWR_OFF 1
+#define MIN_VCO_FREQ 640000000 +#define MAX_VCO_FREQ 1500000000
+#define MIN_LVDS_REFCLK_FREQ 24000000 +#define MAX_LVDS_REFCLK_FREQ 150000000
enum mixel_dphy_devtype { MIXEL_IMX8MQ,
- MIXEL_IMX8QXP,
};
struct mixel_dphy_devdata { @@ -65,6 +86,7 @@ struct mixel_dphy_devdata { u8 reg_rxlprp; u8 reg_rxcdrp; u8 reg_rxhs_settle;
- bool is_combo; /* MIPI DPHY and LVDS PHY combo */
};
static const struct mixel_dphy_devdata mixel_dphy_devdata[] = { @@ -74,6 +96,10 @@ static const struct mixel_dphy_devdata mixel_dphy_devdata[] = { .reg_rxlprp = 0x40, .reg_rxcdrp = 0x44, .reg_rxhs_settle = 0x48,
.is_combo = false,- },
- [MIXEL_IMX8QXP] = {
.is_combo = true,};
@@ -95,8 +121,12 @@ struct mixel_dphy_cfg { struct mixel_dphy_priv { struct mixel_dphy_cfg cfg; struct regmap *regmap;
- struct regmap *lvds_regmap; struct clk *phy_ref_clk; const struct mixel_dphy_devdata *devdata;
- struct imx_sc_ipc *ipc_handle;
- bool is_slave;
- int id;
};
static const struct regmap_config mixel_dphy_regmap_config = { @@ -317,7 +347,8 @@ static int mixel_dphy_set_pll_params(struct phy *phy) return 0; }
-static int mixel_dphy_configure(struct phy *phy, union phy_configure_opts *opts) +static int +mixel_dphy_configure_mipi_dphy(struct phy *phy, union phy_configure_opts *opts)
Please dont change things like this.. A patch should do only one thing :)
{ struct mixel_dphy_priv *priv = phy_get_drvdata(phy); struct mixel_dphy_cfg cfg = { 0 }; @@ -345,15 +376,121 @@ static int mixel_dphy_configure(struct phy *phy, union phy_configure_opts *opts) return 0; }
+static int +mixel_dphy_configure_lvds_phy(struct phy *phy, union phy_configure_opts *opts) +{
- struct mixel_dphy_priv *priv = phy_get_drvdata(phy);
- struct phy_configure_opts_lvds *lvds_opts = &opts->lvds;
- unsigned long data_rate;
- unsigned long fvco;
- u32 rsc;
- u32 co;
- int ret;
- priv->is_slave = lvds_opts->is_slave;
I think you should check if lvds_opts is valid before dereferencing it..
- /* LVDS interface pins */
- regmap_write(priv->lvds_regmap, PHY_CTRL, CCM(0x5) | CA(0x4) | RFB);
can we avoid magic numbers?
- /* enable MODE8 only for slave LVDS PHY */
- rsc = priv->id ? IMX_SC_R_MIPI_1 : IMX_SC_R_MIPI_0;
- ret = imx_sc_misc_set_control(priv->ipc_handle, rsc, IMX_SC_C_DUAL_MODE,
lvds_opts->is_slave);- if (ret) {
dev_err(&phy->dev, "Failed to configure MODE8: %d\n", ret);return ret;- }
- /*
* Choose an appropriate divider ratio to meet the requirement of* PLL VCO frequency range.** ----- 640MHz ~ 1500MHz ------------ ---------------* | VCO | ----------------> | CO divider | -> | LVDS data rate|* ----- FVCO ------------ ---------------* 1/2/4/8 div 7 * differential_clk_rate*/- data_rate = 7 * lvds_opts->differential_clk_rate;
- for (co = 1; co <= 8; co *= 2) {
fvco = data_rate * co;if (fvco >= MIN_VCO_FREQ)break;- }
- if (fvco < MIN_VCO_FREQ || fvco > MAX_VCO_FREQ) {
dev_err(&phy->dev, "VCO frequency %lu is out of range\n", fvco);return -ERANGE;- }
- /*
* CO is configurable, while CN and CM are not,* as fixed ratios 1 and 7 are applied respectively.*/- phy_write(phy, __ffs(co), DPHY_CO);
- /* set reference clock rate */
- clk_set_rate(priv->phy_ref_clk, lvds_opts->differential_clk_rate);
- return ret;
+}
+static int mixel_dphy_configure(struct phy *phy, union phy_configure_opts *opts) +{
- if (phy->attrs.mode == PHY_MODE_MIPI_DPHY)
return mixel_dphy_configure_mipi_dphy(phy, opts);- else if (phy->attrs.mode == PHY_MODE_LVDS)
return mixel_dphy_configure_lvds_phy(phy, opts);- dev_err(&phy->dev,
"Failed to configure PHY with invalid PHY mode: %d\n",phy->attrs.mode);
single line for this please
- return -EINVAL;
+}
+static int +mixel_dphy_validate_lvds_phy(struct phy *phy, union phy_configure_opts *opts) +{
- struct phy_configure_opts_lvds *lvds_cfg = &opts->lvds;
- if (lvds_cfg->bits_per_lane_and_dclk_cycle != 7) {
dev_err(&phy->dev, "Invalid bits per LVDS data lane: %u\n",lvds_cfg->bits_per_lane_and_dclk_cycle);return -EINVAL;- }
- if (lvds_cfg->lanes != 4) {
dev_err(&phy->dev, "Invalid LVDS data lanes: %u\n",lvds_cfg->lanes);
here too, make it better read
return -EINVAL;- }
- if (lvds_cfg->differential_clk_rate < MIN_LVDS_REFCLK_FREQ ||
lvds_cfg->differential_clk_rate > MAX_LVDS_REFCLK_FREQ) {dev_err(&phy->dev,"Invalid LVDS differential clock rate: %lu\n",lvds_cfg->differential_clk_rate);return -EINVAL;- }
- return 0;
+}
static int mixel_dphy_validate(struct phy *phy, enum phy_mode mode, int submode, union phy_configure_opts *opts) {
- struct mixel_dphy_cfg cfg = { 0 };
- if (mode == PHY_MODE_MIPI_DPHY) {
struct mixel_dphy_cfg mipi_dphy_cfg = { 0 };
- if (mode != PHY_MODE_MIPI_DPHY)
return -EINVAL;
return mixel_dphy_config_from_opts(phy, &opts->mipi_dphy,&mipi_dphy_cfg);
the line should be justified to previos line open brace (checkpatch --strict will warn you about this style miss)
Hi Vinod,
On Wed, 2022-04-13 at 11:51 +0530, Vinod Koul wrote:
On 02-04-22, 13:24, Liu Ying wrote:
i.MX8qxp SoC embeds a Mixel MIPI DPHY + LVDS PHY combo which supports either a MIPI DSI display or a LVDS display. The PHY mode is controlled by SCU firmware and the driver would call a SCU firmware function to configure the PHY mode. The single LVDS PHY has 4 data lanes to support a LVDS display. Also, with a master LVDS PHY and a slave LVDS PHY, they may work together to support a LVDS display with 8 data lanes(usually, dual LVDS link display). Note that this patch supports the LVDS PHY mode only for the i.MX8qxp Mixel combo PHY, i.e., the MIPI DPHY mode is yet to be supported, so for now error would be returned from ->set_mode() if MIPI DPHY mode is passed over to it for the combo PHY.
Cc: Guido Günther agx@sigxcpu.org Cc: Robert Chiras robert.chiras@nxp.com Cc: Kishon Vijay Abraham I kishon@ti.com Cc: Vinod Koul vkoul@kernel.org Cc: Shawn Guo shawnguo@kernel.org Cc: Sascha Hauer s.hauer@pengutronix.de Cc: Pengutronix Kernel Team kernel@pengutronix.de Cc: Fabio Estevam festevam@gmail.com Cc: NXP Linux Team linux-imx@nxp.com Reviewed-by: Guido Günther agx@sigxcpu.org Signed-off-by: Liu Ying victor.liu@nxp.com
v5->v6:
- No change.
v4->v5:
- No change.
v3->v4:
- Add Guido's R-b tag.
v2->v3:
- Improve readability of mixel_dphy_set_mode(). (Guido)
v1->v2:
- Print invalid PHY mode in dmesg. (Guido)
.../phy/freescale/phy-fsl-imx8-mipi-dphy.c | 269 +++++++++++++++++- 1 file changed, 258 insertions(+), 11 deletions(-)
diff --git a/drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c b/drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c index a95572b397ca..af1ecda091c7 100644 --- a/drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c +++ b/drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c @@ -4,17 +4,31 @@
- Copyright 2019 Purism SPC
*/
+#include <linux/bitfield.h> #include <linux/clk.h> #include <linux/clk-provider.h> #include <linux/delay.h> +#include <linux/firmware/imx/ipc.h> +#include <linux/firmware/imx/svc/misc.h> #include <linux/io.h> #include <linux/kernel.h> +#include <linux/mfd/syscon.h> #include <linux/module.h> #include <linux/of.h> #include <linux/of_platform.h> #include <linux/phy/phy.h> #include <linux/platform_device.h> #include <linux/regmap.h> +#include <dt-bindings/firmware/imx/rsrc.h>
+/* Control and Status Registers(CSR) */ +#define PHY_CTRL 0x00 +#define CCM_MASK GENMASK(7, 5) +#define CCM(n) FIELD_PREP(CCM_MASK, (n)) +#define CA_MASK GENMASK(4, 2) +#define CA(n) FIELD_PREP(CA_MASK, (n)) +#define RFB BIT(1) +#define LVDS_EN BIT(0)
/* DPHY registers */ #define DPHY_PD_DPHY 0x00 @@ -55,8 +69,15 @@ #define PWR_ON 0 #define PWR_OFF 1
+#define MIN_VCO_FREQ 640000000 +#define MAX_VCO_FREQ 1500000000
+#define MIN_LVDS_REFCLK_FREQ 24000000 +#define MAX_LVDS_REFCLK_FREQ 150000000
enum mixel_dphy_devtype { MIXEL_IMX8MQ,
- MIXEL_IMX8QXP,
};
struct mixel_dphy_devdata { @@ -65,6 +86,7 @@ struct mixel_dphy_devdata { u8 reg_rxlprp; u8 reg_rxcdrp; u8 reg_rxhs_settle;
- bool is_combo; /* MIPI DPHY and LVDS PHY combo */
};
static const struct mixel_dphy_devdata mixel_dphy_devdata[] = { @@ -74,6 +96,10 @@ static const struct mixel_dphy_devdata mixel_dphy_devdata[] = { .reg_rxlprp = 0x40, .reg_rxcdrp = 0x44, .reg_rxhs_settle = 0x48,
.is_combo = false,- },
- [MIXEL_IMX8QXP] = {
.is_combo = true,};
@@ -95,8 +121,12 @@ struct mixel_dphy_cfg { struct mixel_dphy_priv { struct mixel_dphy_cfg cfg; struct regmap *regmap;
- struct regmap *lvds_regmap; struct clk *phy_ref_clk; const struct mixel_dphy_devdata *devdata;
- struct imx_sc_ipc *ipc_handle;
- bool is_slave;
- int id;
};
static const struct regmap_config mixel_dphy_regmap_config = { @@ -317,7 +347,8 @@ static int mixel_dphy_set_pll_params(struct phy *phy) return 0; }
-static int mixel_dphy_configure(struct phy *phy, union phy_configure_opts *opts) +static int +mixel_dphy_configure_mipi_dphy(struct phy *phy, union phy_configure_opts *opts)
Please dont change things like this.. A patch should do only one thing :)
The function name is changed by purpose - the phy in i.MX8qxp SoC is a combo phy(lvds or mipi dphy). The new function name can be distinguished from the added function mixel_dphy_configure_lvds_phy().
{ struct mixel_dphy_priv *priv = phy_get_drvdata(phy); struct mixel_dphy_cfg cfg = { 0 }; @@ -345,15 +376,121 @@ static int mixel_dphy_configure(struct phy *phy, union phy_configure_opts *opts) return 0; }
+static int +mixel_dphy_configure_lvds_phy(struct phy *phy, union phy_configure_opts *opts) +{
- struct mixel_dphy_priv *priv = phy_get_drvdata(phy);
- struct phy_configure_opts_lvds *lvds_opts = &opts->lvds;
- unsigned long data_rate;
- unsigned long fvco;
- u32 rsc;
- u32 co;
- int ret;
- priv->is_slave = lvds_opts->is_slave;
I think you should check if lvds_opts is valid before dereferencing it..
In next verstion, I may check !opts in mixel_dphy_configure() for both lvds and mipi dphy.
- /* LVDS interface pins */
- regmap_write(priv->lvds_regmap, PHY_CTRL, CCM(0x5) | CA(0x4) |
RFB);
can we avoid magic numbers?
Will add macros CCM_1_2V and CA_3_51MA in next version.
- /* enable MODE8 only for slave LVDS PHY */
- rsc = priv->id ? IMX_SC_R_MIPI_1 : IMX_SC_R_MIPI_0;
- ret = imx_sc_misc_set_control(priv->ipc_handle, rsc,
IMX_SC_C_DUAL_MODE,
lvds_opts->is_slave);- if (ret) {
dev_err(&phy->dev, "Failed to configure MODE8: %d\n",ret);
return ret;- }
- /*
* Choose an appropriate divider ratio to meet the requirementof
* PLL VCO frequency range.** ----- 640MHz ~ 1500MHz ------------ ---------------* | VCO | ----------------> | CO divider | -> | LVDS datarate|
* ----- FVCO ------------ ---------------* 1/2/4/8 div 7 *differential_clk_rate
*/- data_rate = 7 * lvds_opts->differential_clk_rate;
- for (co = 1; co <= 8; co *= 2) {
fvco = data_rate * co;if (fvco >= MIN_VCO_FREQ)break;- }
- if (fvco < MIN_VCO_FREQ || fvco > MAX_VCO_FREQ) {
dev_err(&phy->dev, "VCO frequency %lu is out ofrange\n", fvco);
return -ERANGE;- }
- /*
* CO is configurable, while CN and CM are not,* as fixed ratios 1 and 7 are applied respectively.*/- phy_write(phy, __ffs(co), DPHY_CO);
- /* set reference clock rate */
- clk_set_rate(priv->phy_ref_clk, lvds_opts-
differential_clk_rate);
- return ret;
+}
+static int mixel_dphy_configure(struct phy *phy, union phy_configure_opts *opts) +{
- if (phy->attrs.mode == PHY_MODE_MIPI_DPHY)
return mixel_dphy_configure_mipi_dphy(phy, opts);- else if (phy->attrs.mode == PHY_MODE_LVDS)
return mixel_dphy_configure_lvds_phy(phy, opts);- dev_err(&phy->dev,
"Failed to configure PHY with invalid PHY mode: %d\n",phy-attrs.mode);
single line for this please
Will do.
- return -EINVAL;
+}
+static int +mixel_dphy_validate_lvds_phy(struct phy *phy, union phy_configure_opts *opts) +{
- struct phy_configure_opts_lvds *lvds_cfg = &opts->lvds;
- if (lvds_cfg->bits_per_lane_and_dclk_cycle != 7) {
dev_err(&phy->dev, "Invalid bits per LVDS data lane:%u\n",
lvds_cfg-bits_per_lane_and_dclk_cycle);
return -EINVAL;- }
- if (lvds_cfg->lanes != 4) {
dev_err(&phy->dev, "Invalid LVDS data lanes: %u\n",lvds_cfg->lanes);here too, make it better read
Ok.
return -EINVAL;- }
- if (lvds_cfg->differential_clk_rate < MIN_LVDS_REFCLK_FREQ ||
lvds_cfg->differential_clk_rate > MAX_LVDS_REFCLK_FREQ) {dev_err(&phy->dev,"Invalid LVDS differential clock rate: %lu\n",lvds_cfg-differential_clk_rate);
return -EINVAL;- }
- return 0;
+}
static int mixel_dphy_validate(struct phy *phy, enum phy_mode mode, int submode, union phy_configure_opts *opts) {
- struct mixel_dphy_cfg cfg = { 0 };
- if (mode == PHY_MODE_MIPI_DPHY) {
struct mixel_dphy_cfg mipi_dphy_cfg = { 0 };
- if (mode != PHY_MODE_MIPI_DPHY)
return -EINVAL;
return mixel_dphy_config_from_opts(phy, &opts-mipi_dphy,
&mipi_dphy_cfg);
the line should be justified to previos line open brace (checkpatch --strict will warn you about this style miss)
Ok. Will suppress all 'checkpatch --strict' warnings.
Regards, Liu Ying
dri-devel@lists.freedesktop.org
-
Liu Ying -
Vinod Koul