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
--
2.25.1
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 <[email protected]>
Cc: Robert Chiras <[email protected]>
Cc: Martin Kepplinger <[email protected]>
Cc: Andrzej Hajda <[email protected]>
Cc: Neil Armstrong <[email protected]>
Cc: Laurent Pinchart <[email protected]>
Cc: Jonas Karlman <[email protected]>
Cc: Jernej Skrabec <[email protected]>
Cc: David Airlie <[email protected]>
Cc: Daniel Vetter <[email protected]>
Cc: NXP Linux Team <[email protected]>
Signed-off-by: Liu Ying <[email protected]>
---
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);
--
2.25.1
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 <[email protected]>
Cc: Kishon Vijay Abraham I <[email protected]>
Cc: Vinod Koul <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: NXP Linux Team <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
Reviewed-by: Guido Günther <[email protected]>
Signed-off-by: Liu Ying <[email protected]>
---
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 <[email protected]>
+
+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>;
+ };
--
2.25.1
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 <[email protected]>
Cc: Vinod Koul <[email protected]>
Cc: NXP Linux Team <[email protected]>
Signed-off-by: Liu Ying <[email protected]>
---
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;
};
/**
--
2.25.1
Add support for Mixel MIPI DPHY + LVDS PHY combo IP
as found on Freescale i.MX8qxp SoC.
Cc: Guido Günther <[email protected]>
Cc: Kishon Vijay Abraham I <[email protected]>
Cc: Vinod Koul <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: NXP Linux Team <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
Reviewed-by: Guido Günther <[email protected]>
Signed-off-by: Liu Ying <[email protected]>
---
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:
--
2.25.1
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 <[email protected]>
> Cc: Vinod Koul <[email protected]>
> Cc: NXP Linux Team <[email protected]>
> Signed-off-by: Liu Ying <[email protected]>
> ---
> 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
--
~Vinod
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 <[email protected]>
> > > > Cc: Vinod Koul <[email protected]>
> > > > Cc: NXP Linux Team <[email protected]>
> > > > Signed-off-by: Liu Ying <[email protected]>
> > > > ---
[...]
> > > > + */
> > > > +
> > > > +#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
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, 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.
---------------------------------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, 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 <[email protected]>
> > > Cc: Vinod Koul <[email protected]>
> > > Cc: NXP Linux Team <[email protected]>
> > > Signed-off-by: Liu Ying <[email protected]>
> > > ---
> > > 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 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
>
> 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/[email protected]/
> [2]
> https://patchwork.kernel.org/project/dri-devel/patch/[email protected]/
this is helpful, thanks
--
~Vinod
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 <[email protected]>
> > Cc: Vinod Koul <[email protected]>
> > Cc: NXP Linux Team <[email protected]>
> > Signed-off-by: Liu Ying <[email protected]>
> > ---
> > 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 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
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/[email protected]/
[2]
https://patchwork.kernel.org/project/dri-devel/patch/[email protected]/
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 <[email protected]>
> > > > > Cc: Vinod Koul <[email protected]>
> > > > > Cc: NXP Linux Team <[email protected]>
> > > > > Signed-off-by: Liu Ying <[email protected]>
> > > > > ---
>
> [...]
>
> > > > > + */
> > > > > +
> > > > > +#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
>
> 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 ?
--
~Vinod
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 <[email protected]>
> > > > > > Cc: Vinod Koul <[email protected]>
> > > > > > Cc: NXP Linux Team <[email protected]>
> > > > > > Signed-off-by: Liu Ying <[email protected]>
> > > > > > ---
> >
> > [...]
> >
> > > > > > + */
> > > > > > +
> > > > > > +#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
> >
> > 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