Hi,
This series aims to add Freescale i.MX8qm LVDS PHY driver andi dt-binding
support.
The PHY IP is from Mixel, Inc.
The PHY IP supports two LVDS PHYs, thus two LVDS channels.
Each LVDS PHY may work by itself to support a LVDS display device.
When two LVDS PHYs are enabled simultaneously, PHY configurations and reference
clock rate have to be the same since there is only one set of PHY registers.
In this case, the two LVDS PHYs are usually used to support a dual LVDS link
display device, one as master PHY and the other as slave PHY.
Patch 1/2 adds dt-binding for the PHY IP.
Patch 2/2 adds PHY driver support.
Liu Ying (2):
dt-bindings: phy: Add Freescale i.MX8qm Mixel LVDS PHY binding
phy: freescale: Add i.MX8qm Mixel LVDS PHY support
.../bindings/phy/mixel,lvds-phy.yaml | 64 +++
drivers/phy/freescale/Kconfig | 9 +
drivers/phy/freescale/Makefile | 1 +
.../phy/freescale/phy-fsl-imx8qm-lvds-phy.c | 448 ++++++++++++++++++
4 files changed, 522 insertions(+)
create mode 100644 Documentation/devicetree/bindings/phy/mixel,lvds-phy.yaml
create mode 100644 drivers/phy/freescale/phy-fsl-imx8qm-lvds-phy.c
--
2.25.1
This patch adds bindings for Mixel LVDS PHY found on
Freescale i.MX8qm SoC.
Signed-off-by: Liu Ying <[email protected]>
---
.../bindings/phy/mixel,lvds-phy.yaml | 64 +++++++++++++++++++
1 file changed, 64 insertions(+)
create mode 100644 Documentation/devicetree/bindings/phy/mixel,lvds-phy.yaml
diff --git a/Documentation/devicetree/bindings/phy/mixel,lvds-phy.yaml b/Documentation/devicetree/bindings/phy/mixel,lvds-phy.yaml
new file mode 100644
index 000000000000..de964ffb9356
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/mixel,lvds-phy.yaml
@@ -0,0 +1,64 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/mixel,lvds-phy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Mixel LVDS PHY for Freescale i.MX8qm SoC
+
+maintainers:
+ - Liu Ying <[email protected]>
+
+description: |
+ The Mixel LVDS PHY IP block is found on Freescale i.MX8qm SoC.
+ It converts two groups of four 7/10 bits of CMOS data into two
+ groups of four data lanes of LVDS data streams. A phase-locked
+ transmit clock is transmitted in parallel with each group of
+ data streams over a fifth LVDS link. Every cycle of the transmit
+ clock, 56/80 bits of input data are sampled and transmitted
+ through the two groups of LVDS data streams. Together with the
+ transmit clocks, the two groups of LVDS data streams form two
+ LVDS channels.
+
+ The Mixel LVDS PHY found on Freescale i.MX8qm SoC is controlled
+ by Control and Status Registers(CSR) module in the SoC. The CSR
+ module, as a system controller, contains the PHY's registers.
+
+properties:
+ compatible:
+ const: fsl,imx8qm-lvds-phy
+
+ "#phy-cells":
+ const: 1
+ description: |
+ Cell allows setting the LVDS channel index of the PHY.
+ Index 0 is for LVDS channel0 and index 1 is for LVDS channel1.
+
+ clocks:
+ maxItems: 1
+
+ clock-names:
+ const: phy_ref
+
+ power-domains:
+ maxItems: 1
+
+required:
+ - compatible
+ - "#phy-cells"
+ - clocks
+ - clock-names
+ - power-domains
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/firmware/imx/rsrc.h>
+ phy {
+ compatible = "fsl,imx8qm-lvds-phy";
+ #phy-cells = <1>;
+ clocks = <&clk IMX_SC_R_LVDS_0 IMX_SC_PM_CLK_PHY>;
+ clock-names = "phy_ref";
+ power-domains = <&pd IMX_SC_R_LVDS_0>;
+ };
--
2.25.1
This patch adds Freescale i.MX8qm LVDS PHY support.
The PHY IP is from Mixel, Inc.
Signed-off-by: Liu Ying <[email protected]>
---
drivers/phy/freescale/Kconfig | 9 +
drivers/phy/freescale/Makefile | 1 +
.../phy/freescale/phy-fsl-imx8qm-lvds-phy.c | 448 ++++++++++++++++++
3 files changed, 458 insertions(+)
create mode 100644 drivers/phy/freescale/phy-fsl-imx8qm-lvds-phy.c
diff --git a/drivers/phy/freescale/Kconfig b/drivers/phy/freescale/Kconfig
index f9c54cd02036..853958fb2c06 100644
--- a/drivers/phy/freescale/Kconfig
+++ b/drivers/phy/freescale/Kconfig
@@ -8,6 +8,15 @@ config PHY_FSL_IMX8MQ_USB
select GENERIC_PHY
default ARCH_MXC && ARM64
+config PHY_MIXEL_LVDS_PHY
+ tristate "Mixel LVDS PHY support"
+ depends on OF
+ select GENERIC_PHY
+ select REGMAP_MMIO
+ help
+ Enable this to add support for the Mixel LVDS PHY as found
+ on NXP's i.MX8qm SoC.
+
config PHY_MIXEL_MIPI_DPHY
tristate "Mixel MIPI DSI PHY support"
depends on OF && HAS_IOMEM
diff --git a/drivers/phy/freescale/Makefile b/drivers/phy/freescale/Makefile
index 3518d5dbe8a7..cedb328bc4d2 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_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
obj-$(CONFIG_PHY_FSL_IMX8M_PCIE) += phy-fsl-imx8m-pcie.o
obj-$(CONFIG_PHY_FSL_LYNX_28G) += phy-fsl-lynx-28g.o
diff --git a/drivers/phy/freescale/phy-fsl-imx8qm-lvds-phy.c b/drivers/phy/freescale/phy-fsl-imx8qm-lvds-phy.c
new file mode 100644
index 000000000000..bdd11079b7b0
--- /dev/null
+++ b/drivers/phy/freescale/phy-fsl-imx8qm-lvds-phy.c
@@ -0,0 +1,448 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2017-2020,2022 NXP
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/clk.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
+
+#define REG_SET 0x4
+#define REG_CLR 0x8
+
+#define PHY_CTRL 0x0
+#define M_MASK GENMASK(18, 17)
+#define M(n) FIELD_PREP(M_MASK, (n))
+#define CCM_MASK GENMASK(16, 14)
+#define CCM(n) FIELD_PREP(CCM_MASK, (n))
+#define CA_MASK GENMASK(13, 11)
+#define CA(n) FIELD_PREP(CA_MASK, (n))
+#define TST_MASK GENMASK(10, 5)
+#define TST(n) FIELD_PREP(TST_MASK, (n))
+#define CH_EN(id) BIT(3 + (id))
+#define NB BIT(2)
+#define RFB BIT(1)
+#define PD BIT(0)
+
+/* Power On Reset(POR) value */
+#define CTRL_RESET_VAL (M(0x0) | CCM(0x4) | CA(0x4) | TST(0x25))
+
+/* PHY initialization value and mask */
+#define CTRL_INIT_MASK (M_MASK | CCM_MASK | CA_MASK | TST_MASK | NB | RFB)
+#define CTRL_INIT_VAL (M(0x0) | CCM(0x5) | CA(0x4) | TST(0x25) | RFB)
+
+#define PHY_STATUS 0x10
+#define LOCK BIT(0)
+
+#define PHY_NUM 2
+
+#define MIN_CLKIN_FREQ 25000000
+#define MAX_CLKIN_FREQ 165000000
+
+#define PLL_LOCK_SLEEP 10
+#define PLL_LOCK_TIMEOUT 1000
+
+struct mixel_lvds_phy {
+ struct phy *phy;
+ struct phy_configure_opts_lvds cfg;
+ unsigned int id;
+};
+
+struct mixel_lvds_phy_priv {
+ struct regmap *regmap;
+ struct mutex lock; /* protect remap access and cfg of our own */
+ struct clk *phy_ref_clk;
+ struct mixel_lvds_phy *phys[PHY_NUM];
+};
+
+static int mixel_lvds_phy_init(struct phy *phy)
+{
+ struct mixel_lvds_phy_priv *priv = dev_get_drvdata(phy->dev.parent);
+
+ mutex_lock(&priv->lock);
+ regmap_update_bits(priv->regmap,
+ PHY_CTRL, CTRL_INIT_MASK, CTRL_INIT_VAL);
+ mutex_unlock(&priv->lock);
+
+ return 0;
+}
+
+static int mixel_lvds_phy_power_on(struct phy *phy)
+{
+ struct mixel_lvds_phy_priv *priv = dev_get_drvdata(phy->dev.parent);
+ struct mixel_lvds_phy *lvds_phy = phy_get_drvdata(phy);
+ struct mixel_lvds_phy *companion = priv->phys[lvds_phy->id ^ 1];
+ struct phy_configure_opts_lvds *cfg = &lvds_phy->cfg;
+ u32 val = 0;
+ u32 locked;
+ int ret;
+
+ ret = clk_prepare_enable(priv->phy_ref_clk);
+ if (ret < 0) {
+ dev_err(&phy->dev,
+ "failed to enable PHY reference clock: %d\n", ret);
+ return ret;
+ }
+
+ /* The master PHY would power on the slave PHY. */
+ if (cfg->is_slave)
+ return 0;
+
+ mutex_lock(&priv->lock);
+ if (cfg->bits_per_lane_and_dclk_cycle == 7) {
+ if (cfg->differential_clk_rate < 44000000)
+ val |= M(0x2);
+ else if (cfg->differential_clk_rate < 90000000)
+ val |= M(0x1);
+ else
+ val |= M(0x0);
+ } else {
+ val = NB;
+
+ if (cfg->differential_clk_rate < 32000000)
+ val |= M(0x2);
+ else if (cfg->differential_clk_rate < 63000000)
+ val |= M(0x1);
+ else
+ val |= M(0x0);
+ }
+ regmap_update_bits(priv->regmap, PHY_CTRL, M_MASK | NB, val);
+
+ /*
+ * Enable two channels synchronously,
+ * if the companion PHY is a slave PHY.
+ */
+ if (companion->cfg.is_slave)
+ val = CH_EN(0) | CH_EN(1);
+ else
+ val = CH_EN(lvds_phy->id);
+ regmap_write(priv->regmap, PHY_CTRL + REG_SET, val);
+
+ ret = regmap_read_poll_timeout(priv->regmap, PHY_STATUS, locked,
+ locked, PLL_LOCK_SLEEP,
+ PLL_LOCK_TIMEOUT);
+ if (ret < 0) {
+ dev_err(&phy->dev, "failed to get PHY lock: %d\n", ret);
+ clk_disable_unprepare(priv->phy_ref_clk);
+ }
+ mutex_unlock(&priv->lock);
+
+ return ret;
+}
+
+static int mixel_lvds_phy_power_off(struct phy *phy)
+{
+ struct mixel_lvds_phy_priv *priv = dev_get_drvdata(phy->dev.parent);
+ struct mixel_lvds_phy *lvds_phy = phy_get_drvdata(phy);
+
+ mutex_lock(&priv->lock);
+ regmap_write(priv->regmap, PHY_CTRL + REG_CLR, CH_EN(lvds_phy->id));
+ mutex_unlock(&priv->lock);
+
+ clk_disable_unprepare(priv->phy_ref_clk);
+
+ return 0;
+}
+
+static int mixel_lvds_phy_configure(struct phy *phy,
+ union phy_configure_opts *opts)
+{
+ struct mixel_lvds_phy_priv *priv = dev_get_drvdata(phy->dev.parent);
+ struct phy_configure_opts_lvds *cfg = &opts->lvds;
+ int ret;
+
+ ret = clk_set_rate(priv->phy_ref_clk, cfg->differential_clk_rate);
+ if (ret)
+ dev_err(&phy->dev,
+ "failed to set PHY reference clock rate(%lu): %d\n",
+ cfg->differential_clk_rate, ret);
+
+ return ret;
+}
+
+/* Assume the master PHY's configuration set is cached first. */
+static int mixel_lvds_phy_check_slave(struct phy *slave_phy)
+{
+ struct device *dev = &slave_phy->dev;
+ struct mixel_lvds_phy_priv *priv = dev_get_drvdata(dev->parent);
+ struct mixel_lvds_phy *slv = phy_get_drvdata(slave_phy);
+ struct mixel_lvds_phy *mst = priv->phys[slv->id ^ 1];
+ struct phy_configure_opts_lvds *mst_cfg = &mst->cfg;
+ struct phy_configure_opts_lvds *slv_cfg = &slv->cfg;
+
+ if (mst_cfg->bits_per_lane_and_dclk_cycle !=
+ slv_cfg->bits_per_lane_and_dclk_cycle) {
+ dev_err(dev, "number bits mismatch(mst: %u vs slv: %u)\n",
+ mst_cfg->bits_per_lane_and_dclk_cycle,
+ slv_cfg->bits_per_lane_and_dclk_cycle);
+ return -EINVAL;
+ }
+
+ if (mst_cfg->differential_clk_rate !=
+ slv_cfg->differential_clk_rate) {
+ dev_err(dev, "dclk rate mismatch(mst: %lu vs slv: %lu)\n",
+ mst_cfg->differential_clk_rate,
+ slv_cfg->differential_clk_rate);
+ return -EINVAL;
+ }
+
+ if (mst_cfg->lanes != slv_cfg->lanes) {
+ dev_err(dev, "lanes mismatch(mst: %u vs slv: %u)\n",
+ mst_cfg->lanes, slv_cfg->lanes);
+ return -EINVAL;
+ }
+
+ if (mst_cfg->is_slave == slv_cfg->is_slave) {
+ dev_err(dev, "master PHY is not found\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int mixel_lvds_phy_validate(struct phy *phy, enum phy_mode mode,
+ int submode, union phy_configure_opts *opts)
+{
+ struct mixel_lvds_phy_priv *priv = dev_get_drvdata(phy->dev.parent);
+ struct mixel_lvds_phy *lvds_phy = phy_get_drvdata(phy);
+ struct phy_configure_opts_lvds *cfg = &opts->lvds;
+ int ret = 0;
+
+ if (mode != PHY_MODE_LVDS) {
+ dev_err(&phy->dev, "invalid PHY mode(%d)\n", mode);
+ return -EINVAL;
+ }
+
+ if (cfg->bits_per_lane_and_dclk_cycle != 7 &&
+ cfg->bits_per_lane_and_dclk_cycle != 10) {
+ dev_err(&phy->dev, "invalid bits per data lane(%u)\n",
+ cfg->bits_per_lane_and_dclk_cycle);
+ return -EINVAL;
+ }
+
+ if (cfg->lanes != 4 && cfg->lanes != 3) {
+ dev_err(&phy->dev, "invalid data lanes(%u)\n", cfg->lanes);
+ return -EINVAL;
+ }
+
+ if (cfg->differential_clk_rate < MIN_CLKIN_FREQ ||
+ cfg->differential_clk_rate > MAX_CLKIN_FREQ) {
+ dev_err(&phy->dev, "invalid differential clock rate(%lu)\n",
+ cfg->differential_clk_rate);
+ return -EINVAL;
+ }
+
+ mutex_lock(&priv->lock);
+ /* cache configuration set of our own for check */
+ memcpy(&lvds_phy->cfg, cfg, sizeof(*cfg));
+
+ if (cfg->is_slave) {
+ ret = mixel_lvds_phy_check_slave(phy);
+ if (ret)
+ dev_err(&phy->dev,
+ "failed to check slave PHY: %d\n", ret);
+ }
+ mutex_unlock(&priv->lock);
+
+ return ret;
+}
+
+static const struct phy_ops mixel_lvds_phy_ops = {
+ .init = mixel_lvds_phy_init,
+ .power_on = mixel_lvds_phy_power_on,
+ .power_off = mixel_lvds_phy_power_off,
+ .configure = mixel_lvds_phy_configure,
+ .validate = mixel_lvds_phy_validate,
+ .owner = THIS_MODULE,
+};
+
+static int mixel_lvds_phy_reset(struct device *dev)
+{
+ struct mixel_lvds_phy_priv *priv = dev_get_drvdata(dev);
+ int ret;
+
+ ret = pm_runtime_get_sync(dev);
+ if (ret < 0) {
+ dev_err(dev, "failed to get PM runtime: %d\n", ret);
+ return ret;
+ }
+
+ regmap_write(priv->regmap, PHY_CTRL, CTRL_RESET_VAL);
+
+ ret = pm_runtime_put(dev);
+ if (ret < 0)
+ dev_err(dev, "failed to put PM runtime: %d\n", ret);
+
+ return ret;
+}
+
+static struct phy *mixel_lvds_phy_xlate(struct device *dev,
+ struct of_phandle_args *args)
+{
+ struct mixel_lvds_phy_priv *priv = dev_get_drvdata(dev);
+ unsigned int phy_id;
+
+ if (args->args_count != 1) {
+ dev_err(dev,
+ "invalid argument number(%d) for 'phys' property\n",
+ args->args_count);
+ return ERR_PTR(-EINVAL);
+ }
+
+ phy_id = args->args[0];
+
+ if (phy_id >= PHY_NUM) {
+ dev_err(dev, "invalid PHY index(%d)\n", phy_id);
+ return ERR_PTR(-ENODEV);
+ }
+
+ return priv->phys[phy_id]->phy;
+}
+
+static int mixel_lvds_phy_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct phy_provider *phy_provider;
+ struct mixel_lvds_phy_priv *priv;
+ struct mixel_lvds_phy *lvds_phy;
+ struct phy *phy;
+ int i;
+ int ret;
+
+ if (!dev->of_node)
+ return -ENODEV;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ priv->regmap = syscon_node_to_regmap(dev->of_node->parent);
+ if (IS_ERR(priv->regmap)) {
+ ret = PTR_ERR(priv->regmap);
+ dev_err_probe(dev, ret, "failed to get regmap\n");
+ return ret;
+ }
+
+ priv->phy_ref_clk = devm_clk_get(dev, "phy_ref");
+ if (IS_ERR(priv->phy_ref_clk)) {
+ ret = PTR_ERR(priv->phy_ref_clk);
+ dev_err_probe(dev, ret, "failed to get PHY reference clock\n");
+ return ret;
+ }
+
+ mutex_init(&priv->lock);
+
+ dev_set_drvdata(dev, priv);
+
+ pm_runtime_enable(dev);
+
+ ret = mixel_lvds_phy_reset(dev);
+ if (ret) {
+ dev_err(dev, "failed to do POR reset: %d\n", ret);
+ return ret;
+ }
+
+ for (i = 0; i < PHY_NUM; i++) {
+ lvds_phy = devm_kzalloc(dev, sizeof(*lvds_phy), GFP_KERNEL);
+ if (!lvds_phy) {
+ ret = -ENOMEM;
+ goto err;
+ }
+
+ phy = devm_phy_create(dev, NULL, &mixel_lvds_phy_ops);
+ if (IS_ERR(phy)) {
+ ret = PTR_ERR(phy);
+ dev_err(dev, "failed to create PHY for channel%d: %d\n",
+ i, ret);
+ goto err;
+ }
+
+ lvds_phy->phy = phy;
+ lvds_phy->id = i;
+ priv->phys[i] = lvds_phy;
+
+ phy_set_drvdata(phy, lvds_phy);
+ }
+
+ phy_provider = devm_of_phy_provider_register(dev, mixel_lvds_phy_xlate);
+ if (IS_ERR(phy_provider)) {
+ ret = PTR_ERR(phy_provider);
+ dev_err(dev, "failed to register PHY provider: %d\n", ret);
+ goto err;
+ }
+
+ return 0;
+err:
+ pm_runtime_disable(dev);
+
+ return ret;
+}
+
+static int mixel_lvds_phy_remove(struct platform_device *pdev)
+{
+ pm_runtime_disable(&pdev->dev);
+
+ return 0;
+}
+
+static int __maybe_unused mixel_lvds_phy_runtime_suspend(struct device *dev)
+{
+ struct mixel_lvds_phy_priv *priv = dev_get_drvdata(dev);
+
+ /* power down */
+ mutex_lock(&priv->lock);
+ regmap_write(priv->regmap, PHY_CTRL + REG_SET, PD);
+ mutex_unlock(&priv->lock);
+
+ dev_dbg(dev, "runtime suspended\n");
+
+ return 0;
+}
+
+static int __maybe_unused mixel_lvds_phy_runtime_resume(struct device *dev)
+{
+ struct mixel_lvds_phy_priv *priv = dev_get_drvdata(dev);
+
+ /* power up + control initialization */
+ mutex_lock(&priv->lock);
+ regmap_update_bits(priv->regmap, PHY_CTRL,
+ CTRL_INIT_MASK | PD, CTRL_INIT_VAL);
+ mutex_unlock(&priv->lock);
+
+ dev_dbg(dev, "runtime resumed\n");
+
+ return 0;
+}
+
+static const struct dev_pm_ops mixel_lvds_phy_pm_ops = {
+ SET_RUNTIME_PM_OPS(mixel_lvds_phy_runtime_suspend,
+ mixel_lvds_phy_runtime_resume, NULL)
+};
+
+static const struct of_device_id mixel_lvds_phy_of_match[] = {
+ { .compatible = "fsl,imx8qm-lvds-phy" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, mixel_lvds_phy_of_match);
+
+static struct platform_driver mixel_lvds_phy_driver = {
+ .probe = mixel_lvds_phy_probe,
+ .remove = mixel_lvds_phy_remove,
+ .driver = {
+ .pm = &mixel_lvds_phy_pm_ops,
+ .name = "mixel-lvds-phy",
+ .of_match_table = mixel_lvds_phy_of_match,
+ }
+};
+module_platform_driver(mixel_lvds_phy_driver);
+
+MODULE_DESCRIPTION("Mixel LVDS PHY driver");
+MODULE_AUTHOR("Liu Ying <[email protected]>");
+MODULE_LICENSE("GPL");
--
2.25.1
On 18/06/2022 11:22, Liu Ying wrote:
> This patch adds bindings for Mixel LVDS PHY found on
> Freescale i.MX8qm SoC.
>
> Signed-off-by: Liu Ying <[email protected]>
> ---
> .../bindings/phy/mixel,lvds-phy.yaml | 64 +++++++++++++++++++
> 1 file changed, 64 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/phy/mixel,lvds-phy.yaml
>
> diff --git a/Documentation/devicetree/bindings/phy/mixel,lvds-phy.yaml b/Documentation/devicetree/bindings/phy/mixel,lvds-phy.yaml
> new file mode 100644
> index 000000000000..de964ffb9356
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/mixel,lvds-phy.yaml
> @@ -0,0 +1,64 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/phy/mixel,lvds-phy.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Mixel LVDS PHY for Freescale i.MX8qm SoC
If Mixel is a vendor, it needs it's vendor prefix documented and used in
compatible. Filename should match compatible. If it is not a vendor,
then filename should be "fsl,imx8qm-lvds-phy.yaml"
> +
> +maintainers:
> + - Liu Ying <[email protected]>
> +
> +description: |
> + The Mixel LVDS PHY IP block is found on Freescale i.MX8qm SoC.
> + It converts two groups of four 7/10 bits of CMOS data into two
> + groups of four data lanes of LVDS data streams. A phase-locked
> + transmit clock is transmitted in parallel with each group of
> + data streams over a fifth LVDS link. Every cycle of the transmit
> + clock, 56/80 bits of input data are sampled and transmitted
> + through the two groups of LVDS data streams. Together with the
> + transmit clocks, the two groups of LVDS data streams form two
> + LVDS channels.
> +
> + The Mixel LVDS PHY found on Freescale i.MX8qm SoC is controlled
> + by Control and Status Registers(CSR) module in the SoC. The CSR
> + module, as a system controller, contains the PHY's registers.
> +
> +properties:
> + compatible:
> + const: fsl,imx8qm-lvds-phy
> +
> + "#phy-cells":
> + const: 1
> + description: |
> + Cell allows setting the LVDS channel index of the PHY.
> + Index 0 is for LVDS channel0 and index 1 is for LVDS channel1.
> +
> + clocks:
> + maxItems: 1
> +
> + clock-names:
> + const: phy_ref
Maybe just skip the clock-names, it's not bringing anything useful,
unless you expect some more clocks to be documented later? (but in such
case question would be why they are not documented now)
> +
> + power-domains:
> + maxItems: 1
> +
> +required:
> + - compatible
> + - "#phy-cells"
> + - clocks
> + - clock-names
> + - power-domains
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/firmware/imx/rsrc.h>
> + phy {
> + compatible = "fsl,imx8qm-lvds-phy";
> + #phy-cells = <1>;
> + clocks = <&clk IMX_SC_R_LVDS_0 IMX_SC_PM_CLK_PHY>;
> + clock-names = "phy_ref";
> + power-domains = <&pd IMX_SC_R_LVDS_0>;
> + };
Best regards,
Krzysztof
On 18/06/2022 11:22, Liu Ying wrote:
> This patch adds Freescale i.MX8qm LVDS PHY support.
Don't use "This patch".
https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95
> The PHY IP is from Mixel, Inc.
>
> Signed-off-by: Liu Ying <[email protected]>
> +static int mixel_lvds_phy_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct phy_provider *phy_provider;
> + struct mixel_lvds_phy_priv *priv;
> + struct mixel_lvds_phy *lvds_phy;
> + struct phy *phy;
> + int i;
> + int ret;
> +
> + if (!dev->of_node)
> + return -ENODEV;
> +
> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + priv->regmap = syscon_node_to_regmap(dev->of_node->parent);
> + if (IS_ERR(priv->regmap)) {
> + ret = PTR_ERR(priv->regmap);
> + dev_err_probe(dev, ret, "failed to get regmap\n");
> + return ret;
All such calls are one-liners.
> + }
> +
> + priv->phy_ref_clk = devm_clk_get(dev, "phy_ref");
> + if (IS_ERR(priv->phy_ref_clk)) {
> + ret = PTR_ERR(priv->phy_ref_clk);
> + dev_err_probe(dev, ret, "failed to get PHY reference clock\n");
> + return ret;
Again, one line instead of three.
> + }
> +
> + mutex_init(&priv->lock);
> +
> + dev_set_drvdata(dev, priv);
> +
> + pm_runtime_enable(dev);
> +
> + ret = mixel_lvds_phy_reset(dev);
> + if (ret) {
> + dev_err(dev, "failed to do POR reset: %d\n", ret);
> + return ret;
> + }
> +
> + for (i = 0; i < PHY_NUM; i++) {
> + lvds_phy = devm_kzalloc(dev, sizeof(*lvds_phy), GFP_KERNEL);
> + if (!lvds_phy) {
> + ret = -ENOMEM;
> + goto err;
> + }
> +
> + phy = devm_phy_create(dev, NULL, &mixel_lvds_phy_ops);
> + if (IS_ERR(phy)) {
> + ret = PTR_ERR(phy);
> + dev_err(dev, "failed to create PHY for channel%d: %d\n",
> + i, ret);
> + goto err;
> + }
> +
> + lvds_phy->phy = phy;
> + lvds_phy->id = i;
> + priv->phys[i] = lvds_phy;
> +
> + phy_set_drvdata(phy, lvds_phy);
> + }
> +
> + phy_provider = devm_of_phy_provider_register(dev, mixel_lvds_phy_xlate);
> + if (IS_ERR(phy_provider)) {
> + ret = PTR_ERR(phy_provider);
> + dev_err(dev, "failed to register PHY provider: %d\n", ret);
> + goto err;
> + }
> +
> + return 0;
> +err:
> + pm_runtime_disable(dev);
> +
> + return ret;
> +}
> +
> +static int mixel_lvds_phy_remove(struct platform_device *pdev)
> +{
> + pm_runtime_disable(&pdev->dev);
> +
> + return 0;
> +}
> +
> +static int __maybe_unused mixel_lvds_phy_runtime_suspend(struct device *dev)
> +{
> + struct mixel_lvds_phy_priv *priv = dev_get_drvdata(dev);
> +
> + /* power down */
> + mutex_lock(&priv->lock);
> + regmap_write(priv->regmap, PHY_CTRL + REG_SET, PD);
> + mutex_unlock(&priv->lock);
> +
> + dev_dbg(dev, "runtime suspended\n");
> +
> + return 0;
> +}
> +
> +static int __maybe_unused mixel_lvds_phy_runtime_resume(struct device *dev)
> +{
> + struct mixel_lvds_phy_priv *priv = dev_get_drvdata(dev);
> +
> + /* power up + control initialization */
> + mutex_lock(&priv->lock);
> + regmap_update_bits(priv->regmap, PHY_CTRL,
> + CTRL_INIT_MASK | PD, CTRL_INIT_VAL);
> + mutex_unlock(&priv->lock);
> +
> + dev_dbg(dev, "runtime resumed\n");
No such debug messages.
Best regards,
Krzysztof
On Sun, 2022-06-19 at 14:11 +0200, Krzysztof Kozlowski wrote:
> On 18/06/2022 11:22, Liu Ying wrote:
> > This patch adds bindings for Mixel LVDS PHY found on
> > Freescale i.MX8qm SoC.
> >
> > Signed-off-by: Liu Ying <[email protected]>
> > ---
> > .../bindings/phy/mixel,lvds-phy.yaml | 64
> > +++++++++++++++++++
> > 1 file changed, 64 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/phy/mixel,lvds-phy.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/phy/mixel,lvds-
> > phy.yaml b/Documentation/devicetree/bindings/phy/mixel,lvds-
> > phy.yaml
> > new file mode 100644
> > index 000000000000..de964ffb9356
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/phy/mixel,lvds-phy.yaml
> > @@ -0,0 +1,64 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id:
> > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetree.org%2Fschemas%2Fphy%2Fmixel%2Clvds-phy.yaml%23&data=05%7C01%7Cvictor.liu%40nxp.com%7Cb68f9012ddcc44bc10cd08da51ece7f7%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637912375188731290%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=MdBILPYmfYeWeCXXNxy1mu1NcU0b6EW3QztYc294dd4%3D&reserved=0
> > +$schema:
> > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetree.org%2Fmeta-schemas%2Fcore.yaml%23&data=05%7C01%7Cvictor.liu%40nxp.com%7Cb68f9012ddcc44bc10cd08da51ece7f7%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637912375188731290%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=MhHwku8rbDLAZAQh1T9CGFULMkk5MaNoj3LQnQJ6VXM%3D&reserved=0
> > +
> > +title: Mixel LVDS PHY for Freescale i.MX8qm SoC
>
> If Mixel is a vendor, it needs it's vendor prefix documented and used
> in
> compatible. Filename should match compatible. If it is not a vendor,
> then filename should be "fsl,imx8qm-lvds-phy.yaml"
Mixel is a vendor. I'll document the vendor prefix and set
'fsl,imx8qm-lvds-phy' and 'mixel,lvds-phy' as compatible's enum.
I'll keep the filename as-is.
>
> > +
> > +maintainers:
> > + - Liu Ying <[email protected]>
> > +
> > +description: |
> > + The Mixel LVDS PHY IP block is found on Freescale i.MX8qm SoC.
> > + It converts two groups of four 7/10 bits of CMOS data into two
> > + groups of four data lanes of LVDS data streams. A phase-locked
> > + transmit clock is transmitted in parallel with each group of
> > + data streams over a fifth LVDS link. Every cycle of the transmit
> > + clock, 56/80 bits of input data are sampled and transmitted
> > + through the two groups of LVDS data streams. Together with the
> > + transmit clocks, the two groups of LVDS data streams form two
> > + LVDS channels.
> > +
> > + The Mixel LVDS PHY found on Freescale i.MX8qm SoC is controlled
> > + by Control and Status Registers(CSR) module in the SoC. The CSR
> > + module, as a system controller, contains the PHY's registers.
> > +
> > +properties:
> > + compatible:
> > + const: fsl,imx8qm-lvds-phy
> > +
> > + "#phy-cells":
> > + const: 1
> > + description: |
> > + Cell allows setting the LVDS channel index of the PHY.
> > + Index 0 is for LVDS channel0 and index 1 is for LVDS
> > channel1.
> > +
> > + clocks:
> > + maxItems: 1
> > +
> > + clock-names:
> > + const: phy_ref
>
> Maybe just skip the clock-names, it's not bringing anything useful,
> unless you expect some more clocks to be documented later? (but in
> such
> case question would be why they are not documented now)
I'll skip it, because it is the only clock required by the PHY IP that
I'm aware of.
Thanks,
Liu Ying
>
> > +
> > + power-domains:
> > + maxItems: 1
> > +
> > +required:
> > + - compatible
> > + - "#phy-cells"
> > + - clocks
> > + - clock-names
> > + - power-domains
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/firmware/imx/rsrc.h>
> > + phy {
> > + compatible = "fsl,imx8qm-lvds-phy";
> > + #phy-cells = <1>;
> > + clocks = <&clk IMX_SC_R_LVDS_0 IMX_SC_PM_CLK_PHY>;
> > + clock-names = "phy_ref";
> > + power-domains = <&pd IMX_SC_R_LVDS_0>;
> > + };
>
>
> Best regards,
> Krzysztof
On Sun, 2022-06-19 at 14:15 +0200, Krzysztof Kozlowski wrote:
> On 18/06/2022 11:22, Liu Ying wrote:
> > This patch adds Freescale i.MX8qm LVDS PHY support.
>
>
> Don't use "This patch".
Fair enough. Won't use it.
>
https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Felixir.bootlin.com%2Flinux%2Fv5.17.1%2Fsource%2FDocumentation%2Fprocess%2Fsubmitting-patches.rst%23L95&data=05%7C01%7Cvictor.liu%40nxp.com%7C82050bf711fb4a8eb28108da51ed5912%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637912377082315453%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=nloK3581LSb7%2BUF%2FTMR4b5J4GYRw4SKKRfK%2FRfP3UrM%3D&reserved=0
>
> > The PHY IP is from Mixel, Inc.
> >
> > Signed-off-by: Liu Ying <[email protected]>
>
>
>
> > +static int mixel_lvds_phy_probe(struct platform_device *pdev)
> > +{
> > + struct device *dev = &pdev->dev;
> > + struct phy_provider *phy_provider;
> > + struct mixel_lvds_phy_priv *priv;
> > + struct mixel_lvds_phy *lvds_phy;
> > + struct phy *phy;
> > + int i;
> > + int ret;
> > +
> > + if (!dev->of_node)
> > + return -ENODEV;
> > +
> > + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> > + if (!priv)
> > + return -ENOMEM;
> > +
> > + priv->regmap = syscon_node_to_regmap(dev->of_node->parent);
> > + if (IS_ERR(priv->regmap)) {
> > + ret = PTR_ERR(priv->regmap);
> > + dev_err_probe(dev, ret, "failed to get regmap\n");
> > + return ret;
>
> All such calls are one-liners.
Will do.
>
> > + }
> > +
> > + priv->phy_ref_clk = devm_clk_get(dev, "phy_ref");
> > + if (IS_ERR(priv->phy_ref_clk)) {
> > + ret = PTR_ERR(priv->phy_ref_clk);
> > + dev_err_probe(dev, ret, "failed to get PHY reference
> > clock\n");
> > + return ret;
>
> Again, one line instead of three.
Will do.
>
> > + }
> > +
> > + mutex_init(&priv->lock);
> > +
> > + dev_set_drvdata(dev, priv);
> > +
> > + pm_runtime_enable(dev);
> > +
> > + ret = mixel_lvds_phy_reset(dev);
> > + if (ret) {
> > + dev_err(dev, "failed to do POR reset: %d\n", ret);
> > + return ret;
> > + }
> > +
> > + for (i = 0; i < PHY_NUM; i++) {
> > + lvds_phy = devm_kzalloc(dev, sizeof(*lvds_phy),
> > GFP_KERNEL);
> > + if (!lvds_phy) {
> > + ret = -ENOMEM;
> > + goto err;
> > + }
> > +
> > + phy = devm_phy_create(dev, NULL, &mixel_lvds_phy_ops);
> > + if (IS_ERR(phy)) {
> > + ret = PTR_ERR(phy);
> > + dev_err(dev, "failed to create PHY for
> > channel%d: %d\n",
> > + i, ret);
> > + goto err;
> > + }
> > +
> > + lvds_phy->phy = phy;
> > + lvds_phy->id = i;
> > + priv->phys[i] = lvds_phy;
> > +
> > + phy_set_drvdata(phy, lvds_phy);
> > + }
> > +
> > + phy_provider = devm_of_phy_provider_register(dev,
> > mixel_lvds_phy_xlate);
> > + if (IS_ERR(phy_provider)) {
> > + ret = PTR_ERR(phy_provider);
> > + dev_err(dev, "failed to register PHY provider: %d\n",
> > ret);
> > + goto err;
> > + }
> > +
> > + return 0;
> > +err:
> > + pm_runtime_disable(dev);
> > +
> > + return ret;
> > +}
> > +
> > +static int mixel_lvds_phy_remove(struct platform_device *pdev)
> > +{
> > + pm_runtime_disable(&pdev->dev);
> > +
> > + return 0;
> > +}
> > +
> > +static int __maybe_unused mixel_lvds_phy_runtime_suspend(struct
> > device *dev)
> > +{
> > + struct mixel_lvds_phy_priv *priv = dev_get_drvdata(dev);
> > +
> > + /* power down */
> > + mutex_lock(&priv->lock);
> > + regmap_write(priv->regmap, PHY_CTRL + REG_SET, PD);
> > + mutex_unlock(&priv->lock);
> > +
> > + dev_dbg(dev, "runtime suspended\n");
> > +
> > + return 0;
> > +}
> > +
> > +static int __maybe_unused mixel_lvds_phy_runtime_resume(struct
> > device *dev)
> > +{
> > + struct mixel_lvds_phy_priv *priv = dev_get_drvdata(dev);
> > +
> > + /* power up + control initialization */
> > + mutex_lock(&priv->lock);
> > + regmap_update_bits(priv->regmap, PHY_CTRL,
> > + CTRL_INIT_MASK | PD, CTRL_INIT_VAL);
> > + mutex_unlock(&priv->lock);
> > +
> > + dev_dbg(dev, "runtime resumed\n");
>
> No such debug messages.
Will remove the debug messages.
Thanks,
Liu Ying
>
>
> Best regards,
> Krzysztof
On 20/06/2022 05:06, Liu Ying wrote:
> On Sun, 2022-06-19 at 14:11 +0200, Krzysztof Kozlowski wrote:
>> On 18/06/2022 11:22, Liu Ying wrote:
>>> This patch adds bindings for Mixel LVDS PHY found on
>>> Freescale i.MX8qm SoC.
>>>
>>> Signed-off-by: Liu Ying <[email protected]>
>>> ---
>>> .../bindings/phy/mixel,lvds-phy.yaml | 64
>>> +++++++++++++++++++
>>> 1 file changed, 64 insertions(+)
>>> create mode 100644
>>> Documentation/devicetree/bindings/phy/mixel,lvds-phy.yaml
>>
>>> diff --git a/Documentation/devicetree/bindings/phy/mixel,lvds-
>>> phy.yaml b/Documentation/devicetree/bindings/phy/mixel,lvds-
>>> phy.yaml
>>> new file mode 100644
>>> index 000000000000..de964ffb9356
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/phy/mixel,lvds-phy.yaml
>>> @@ -0,0 +1,64 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id:
>>> https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetree.org%2Fschemas%2Fphy%2Fmixel%2Clvds-phy.yaml%23&data=05%7C01%7Cvictor.liu%40nxp.com%7Cb68f9012ddcc44bc10cd08da51ece7f7%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637912375188731290%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=MdBILPYmfYeWeCXXNxy1mu1NcU0b6EW3QztYc294dd4%3D&reserved=0
>>> +$schema:
>>> https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetree.org%2Fmeta-schemas%2Fcore.yaml%23&data=05%7C01%7Cvictor.liu%40nxp.com%7Cb68f9012ddcc44bc10cd08da51ece7f7%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637912375188731290%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=MhHwku8rbDLAZAQh1T9CGFULMkk5MaNoj3LQnQJ6VXM%3D&reserved=0
>>> +
>>> +title: Mixel LVDS PHY for Freescale i.MX8qm SoC
>>
>> If Mixel is a vendor, it needs it's vendor prefix documented and used
>> in
>> compatible. Filename should match compatible. If it is not a vendor,
>> then filename should be "fsl,imx8qm-lvds-phy.yaml"
>
> Mixel is a vendor. I'll document the vendor prefix and set
> 'fsl,imx8qm-lvds-phy' and 'mixel,lvds-phy' as compatible's enum.
> I'll keep the filename as-is.
mixel,lvds-phy is not a good compatible (and filename) because it is not
specific about component version. Please use specific compatibles only
thus also specific filename (filename should match first compatible).
Best regards,
Krzysztof
On Mon, 2022-06-20 at 09:35 +0200, Krzysztof Kozlowski wrote:
> On 20/06/2022 05:06, Liu Ying wrote:
> > On Sun, 2022-06-19 at 14:11 +0200, Krzysztof Kozlowski wrote:
> > > On 18/06/2022 11:22, Liu Ying wrote:
> > > > This patch adds bindings for Mixel LVDS PHY found on
> > > > Freescale i.MX8qm SoC.
> > > >
> > > > Signed-off-by: Liu Ying <[email protected]>
> > > > ---
> > > > .../bindings/phy/mixel,lvds-phy.yaml | 64
> > > > +++++++++++++++++++
> > > > 1 file changed, 64 insertions(+)
> > > > create mode 100644
> > > > Documentation/devicetree/bindings/phy/mixel,lvds-phy.yaml
> > > > diff --git a/Documentation/devicetree/bindings/phy/mixel,lvds-
> > > > phy.yaml b/Documentation/devicetree/bindings/phy/mixel,lvds-
> > > > phy.yaml
> > > > new file mode 100644
> > > > index 000000000000..de964ffb9356
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/phy/mixel,lvds-phy.yaml
> > > > @@ -0,0 +1,64 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > +%YAML 1.2
> > > > +---
> > > > +$id:
> > > >
https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetree.org%2Fschemas%2Fphy%2Fmixel%2Clvds-phy.yaml%23&data=05%7C01%7Cvictor.liu%40nxp.com%7Cc7d6216ac12148ec95d008da528f78eb%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637913073407696040%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=m588CiXOaJhWFbpEal3MjZaHtvOTUOVIujydIdPxSHg%3D&reserved=0
> > > > +$schema:
> > > >
https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetree.org%2Fmeta-schemas%2Fcore.yaml%23&data=05%7C01%7Cvictor.liu%40nxp.com%7Cc7d6216ac12148ec95d008da528f78eb%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637913073407696040%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=IGn5f05QrzyX05k%2FKgTitdq6OYN2FmdpsO6qHXun55Y%3D&reserved=0
> > > > +
> > > > +title: Mixel LVDS PHY for Freescale i.MX8qm SoC
> > >
> > > If Mixel is a vendor, it needs it's vendor prefix documented and
> > > used
> > > in
> > > compatible. Filename should match compatible. If it is not a
> > > vendor,
> > > then filename should be "fsl,imx8qm-lvds-phy.yaml"
> >
> > Mixel is a vendor. I'll document the vendor prefix and set
> > 'fsl,imx8qm-lvds-phy' and 'mixel,lvds-phy' as compatible's enum.
> > I'll keep the filename as-is.
>
> mixel,lvds-phy is not a good compatible (and filename) because it is
> not
> specific about component version. Please use specific compatibles
> only
> thus also specific filename (filename should match first compatible).
All the information about component version I can find in the PHY IP's
data sheet is '28FDSOI-LVDS-1250-8CH-TX-PLL'. '28FDSOI' looks like the
technology name. Do you think the compatible name can be
'mixel,lvds-1250-8ch-tx-pll'? Or, any better option?
If the compatible name is ok, does the below compabitle property look
ok?
===================8<==============
compatible:
enum:
- mixel,lvds-1250-8ch-tx-pll
- fsl,imx8qm-lvds-phy
===================8<==============
I'll change the filename accordingly.
Thanks,
Liu Ying
On 20/06/2022 09:56, Liu Ying wrote:
> On Mon, 2022-06-20 at 09:35 +0200, Krzysztof Kozlowski wrote:
>> On 20/06/2022 05:06, Liu Ying wrote:
>>> On Sun, 2022-06-19 at 14:11 +0200, Krzysztof Kozlowski wrote:
>>>> On 18/06/2022 11:22, Liu Ying wrote:
>>>>> This patch adds bindings for Mixel LVDS PHY found on
>>>>> Freescale i.MX8qm SoC.
>>>>>
>>>>> Signed-off-by: Liu Ying <[email protected]>
>>>>> ---
>>>>> .../bindings/phy/mixel,lvds-phy.yaml | 64
>>>>> +++++++++++++++++++
>>>>> 1 file changed, 64 insertions(+)
>>>>> create mode 100644
>>>>> Documentation/devicetree/bindings/phy/mixel,lvds-phy.yaml
>>>>> diff --git a/Documentation/devicetree/bindings/phy/mixel,lvds-
>>>>> phy.yaml b/Documentation/devicetree/bindings/phy/mixel,lvds-
>>>>> phy.yaml
>>>>> new file mode 100644
>>>>> index 000000000000..de964ffb9356
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/phy/mixel,lvds-phy.yaml
>>>>> @@ -0,0 +1,64 @@
>>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>>> +%YAML 1.2
>>>>> +---
>>>>> +$id:
>>>>>
> https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetree.org%2Fschemas%2Fphy%2Fmixel%2Clvds-phy.yaml%23&data=05%7C01%7Cvictor.liu%40nxp.com%7Cc7d6216ac12148ec95d008da528f78eb%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637913073407696040%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=m588CiXOaJhWFbpEal3MjZaHtvOTUOVIujydIdPxSHg%3D&reserved=0
>>>>> +$schema:
>>>>>
> https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetree.org%2Fmeta-schemas%2Fcore.yaml%23&data=05%7C01%7Cvictor.liu%40nxp.com%7Cc7d6216ac12148ec95d008da528f78eb%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637913073407696040%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=IGn5f05QrzyX05k%2FKgTitdq6OYN2FmdpsO6qHXun55Y%3D&reserved=0
>>>>> +
>>>>> +title: Mixel LVDS PHY for Freescale i.MX8qm SoC
>>>>
>>>> If Mixel is a vendor, it needs it's vendor prefix documented and
>>>> used
>>>> in
>>>> compatible. Filename should match compatible. If it is not a
>>>> vendor,
>>>> then filename should be "fsl,imx8qm-lvds-phy.yaml"
>>>
>>> Mixel is a vendor. I'll document the vendor prefix and set
>>> 'fsl,imx8qm-lvds-phy' and 'mixel,lvds-phy' as compatible's enum.
>>> I'll keep the filename as-is.
>>
>> mixel,lvds-phy is not a good compatible (and filename) because it is
>> not
>> specific about component version. Please use specific compatibles
>> only
>> thus also specific filename (filename should match first compatible).
>
> All the information about component version I can find in the PHY IP's
> data sheet is '28FDSOI-LVDS-1250-8CH-TX-PLL'. '28FDSOI' looks like the
> technology name. Do you think the compatible name can be
> 'mixel,lvds-1250-8ch-tx-pll'? Or, any better option?
>
> If the compatible name is ok, does the below compabitle property look
> ok?
>
> ===================8<==============
> compatible:
> enum:
> - mixel,lvds-1250-8ch-tx-pll
> - fsl,imx8qm-lvds-phy
> ===================8<==============
https://mixel.com/wp-content/mixel/pdf/2018/MXL-LVDS-1250-8CH-TX-PLL_SS_28FDSOI.pdf
mentions entire name as a device name, so with 28FDSOI, but your choice
is also good.
Best regards,
Krzysztof
On Mon, 2022-06-20 at 12:38 +0200, Krzysztof Kozlowski wrote:
> On 20/06/2022 09:56, Liu Ying wrote:
> > On Mon, 2022-06-20 at 09:35 +0200, Krzysztof Kozlowski wrote:
> > > On 20/06/2022 05:06, Liu Ying wrote:
> > > > On Sun, 2022-06-19 at 14:11 +0200, Krzysztof Kozlowski wrote:
> > > > > On 18/06/2022 11:22, Liu Ying wrote:
> > > > > > This patch adds bindings for Mixel LVDS PHY found on
> > > > > > Freescale i.MX8qm SoC.
> > > > > >
> > > > > > Signed-off-by: Liu Ying <[email protected]>
> > > > > > ---
> > > > > > .../bindings/phy/mixel,lvds-phy.yaml | 64
> > > > > > +++++++++++++++++++
> > > > > > 1 file changed, 64 insertions(+)
> > > > > > create mode 100644
> > > > > > Documentation/devicetree/bindings/phy/mixel,lvds-phy.yaml
> > > > > > diff --git
> > > > > > a/Documentation/devicetree/bindings/phy/mixel,lvds-
> > > > > > phy.yaml
> > > > > > b/Documentation/devicetree/bindings/phy/mixel,lvds-
> > > > > > phy.yaml
> > > > > > new file mode 100644
> > > > > > index 000000000000..de964ffb9356
> > > > > > --- /dev/null
> > > > > > +++ b/Documentation/devicetree/bindings/phy/mixel,lvds-
> > > > > > phy.yaml
> > > > > > @@ -0,0 +1,64 @@
> > > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > > > +%YAML 1.2
> > > > > > +---
> > > > > > +$id:
> > > > > >
> >
> >
https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetree.org%2Fschemas%2Fphy%2Fmixel%2Clvds-phy.yaml%23&data=05%7C01%7Cvictor.liu%40nxp.com%7C968d5a1e2f5c4ccdd77908da52a9148d%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C637913183390683087%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=h8JmjvGBnTcMn2Uk3CVgq%2BB4%2BlbCBA5iDD7OhtFZyZo%3D&reserved=0
> > > > > > +$schema:
> > > > > >
> >
> >
https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetree.org%2Fmeta-schemas%2Fcore.yaml%23&data=05%7C01%7Cvictor.liu%40nxp.com%7C968d5a1e2f5c4ccdd77908da52a9148d%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C637913183390683087%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=bLsmtGPC8tzZSHPqblHOhx0gnsgFHlKVu0P8dN6G15s%3D&reserved=0
> > > > > > +
> > > > > > +title: Mixel LVDS PHY for Freescale i.MX8qm SoC
> > > > >
> > > > > If Mixel is a vendor, it needs it's vendor prefix documented
> > > > > and
> > > > > used
> > > > > in
> > > > > compatible. Filename should match compatible. If it is not a
> > > > > vendor,
> > > > > then filename should be "fsl,imx8qm-lvds-phy.yaml"
> > > >
> > > > Mixel is a vendor. I'll document the vendor prefix and set
> > > > 'fsl,imx8qm-lvds-phy' and 'mixel,lvds-phy' as compatible's
> > > > enum.
> > > > I'll keep the filename as-is.
> > >
> > > mixel,lvds-phy is not a good compatible (and filename) because it
> > > is
> > > not
> > > specific about component version. Please use specific compatibles
> > > only
> > > thus also specific filename (filename should match first
> > > compatible).
> >
> > All the information about component version I can find in the PHY
> > IP's
> > data sheet is '28FDSOI-LVDS-1250-8CH-TX-PLL'. '28FDSOI' looks like
> > the
> > technology name. Do you think the compatible name can be
> > 'mixel,lvds-1250-8ch-tx-pll'? Or, any better option?
> >
> > If the compatible name is ok, does the below compabitle property
> > look
> > ok?
> >
> > ===================8<==============
> > compatible:
> > enum:
> > - mixel,lvds-1250-8ch-tx-pll
> > - fsl,imx8qm-lvds-phy
> > ===================8<==============
>
>
https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmixel.com%2Fwp-content%2Fmixel%2Fpdf%2F2018%2FMXL-LVDS-1250-8CH-TX-PLL_SS_28FDSOI.pdf&data=05%7C01%7Cvictor.liu%40nxp.com%7C968d5a1e2f5c4ccdd77908da52a9148d%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C637913183390683087%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=PWK82Pf%2FbBti6PXNq9wvFbjM6QmnZWcj7bWSh2FYAzA%3D&reserved=0
>
> mentions entire name as a device name, so with 28FDSOI, but your
> choice
> is also good.
Will use the entire name with 28FDSOI.
Thanks,
Liu Ying