From: Vinod Koul <vkoul@kernel.org>
To: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Cc: robh+dt@kernel.org, linux-phy@lists.infradead.org,
devicetree@vger.kernel.org, linux-amlogic@lists.infradead.org,
kishon@ti.com, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 2/2] phy: amlogic: Add a new driver for the HDMI TX PHY on Meson8/8b/8m2
Date: Tue, 20 Jul 2021 14:06:59 +0530 [thread overview]
Message-ID: <YPaLKwEKcDLWwztM@matsya> (raw)
In-Reply-To: <20210629182047.893415-3-martin.blumenstingl@googlemail.com>
On 29-06-21, 20:20, Martin Blumenstingl wrote:
> Amlogic Meson8/8b/8m2 have a built-in HDMI PHY in the HHI register
> region. Unfortunately only few register bits are documented. For
> HHI_HDMI_PHY_CNTL0 the magic numbers are taken from the 3.10 vendor
> kernel.
>
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
> drivers/phy/amlogic/Kconfig | 10 ++
> drivers/phy/amlogic/Makefile | 1 +
> drivers/phy/amlogic/phy-meson8-hdmi-tx.c | 162 +++++++++++++++++++++++
> 3 files changed, 173 insertions(+)
> create mode 100644 drivers/phy/amlogic/phy-meson8-hdmi-tx.c
>
> diff --git a/drivers/phy/amlogic/Kconfig b/drivers/phy/amlogic/Kconfig
> index db5d0cd757e3..486ca23aba32 100644
> --- a/drivers/phy/amlogic/Kconfig
> +++ b/drivers/phy/amlogic/Kconfig
> @@ -2,6 +2,16 @@
> #
> # Phy drivers for Amlogic platforms
> #
> +config PHY_MESON8_HDMI_TX
> + tristate "Meson8, Meson8b and Meson8m2 HDMI TX PHY driver"
> + depends on (ARCH_MESON && ARM) || COMPILE_TEST
> + depends on OF
> + select MFD_SYSCON
> + help
> + Enable this to support the HDMI TX PHYs found in Meson8,
> + Meson8b and Meson8m2 SoCs.
> + If unsure, say N.
> +
> config PHY_MESON8B_USB2
> tristate "Meson8, Meson8b, Meson8m2 and GXBB USB2 PHY driver"
> default ARCH_MESON
> diff --git a/drivers/phy/amlogic/Makefile b/drivers/phy/amlogic/Makefile
> index 8fa07fbd0d92..c0886c850bb0 100644
> --- a/drivers/phy/amlogic/Makefile
> +++ b/drivers/phy/amlogic/Makefile
> @@ -1,4 +1,5 @@
> # SPDX-License-Identifier: GPL-2.0-only
> +obj-$(CONFIG_PHY_MESON8_HDMI_TX) += phy-meson8-hdmi-tx.o
> obj-$(CONFIG_PHY_MESON8B_USB2) += phy-meson8b-usb2.o
> obj-$(CONFIG_PHY_MESON_GXL_USB2) += phy-meson-gxl-usb2.o
> obj-$(CONFIG_PHY_MESON_G12A_USB2) += phy-meson-g12a-usb2.o
> diff --git a/drivers/phy/amlogic/phy-meson8-hdmi-tx.c b/drivers/phy/amlogic/phy-meson8-hdmi-tx.c
> new file mode 100644
> index 000000000000..ba5a4de54811
> --- /dev/null
> +++ b/drivers/phy/amlogic/phy-meson8-hdmi-tx.c
> @@ -0,0 +1,162 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Meson8, Meson8b and Meson8m2 HDMI TX PHY.
> + *
> + * Copyright (C) 2021 Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/bits.h>
> +#include <linux/clk.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/property.h>
> +#include <linux/regmap.h>
> +
> +#define HHI_HDMI_PHY_CNTL0 0x0
> + #define HHI_HDMI_PHY_CNTL0_HDMI_CTL1 GENMASK(31, 16)
> + #define HHI_HDMI_PHY_CNTL0_HDMI_CTL0 GENMASK(15, 0)
> +
> +#define HHI_HDMI_PHY_CNTL1 0x4
> + #define HHI_HDMI_PHY_CNTL1_CLOCK_ENABLE BIT(1)
> + #define HHI_HDMI_PHY_CNTL1_SOFT_RESET BIT(0)
> +
> +#define HHI_HDMI_PHY_CNTL2 0x8
> +
> +struct phy_meson8_hdmi_tx_priv {
> + struct regmap *hhi;
> + struct clk *tmds_clk;
> + unsigned int reg_offset;
> +};
> +
> +static int phy_meson8_hdmi_tx_init(struct phy *phy)
> +{
> + struct phy_meson8_hdmi_tx_priv *priv = phy_get_drvdata(phy);
> +
> + return clk_prepare_enable(priv->tmds_clk);
> +}
> +
> +static int phy_meson8_hdmi_tx_exit(struct phy *phy)
> +{
> + struct phy_meson8_hdmi_tx_priv *priv = phy_get_drvdata(phy);
> +
> + clk_disable_unprepare(priv->tmds_clk);
> +
> + return 0;
> +}
> +
> +static int phy_meson8_hdmi_tx_power_on(struct phy *phy)
> +{
> + struct phy_meson8_hdmi_tx_priv *priv = phy_get_drvdata(phy);
> + unsigned int i;
> + u16 hdmi_ctl0;
> +
> + if (clk_get_rate(priv->tmds_clk) >= 2970UL * 1000 * 1000)
> + hdmi_ctl0 = 0x1e8b;
> + else
> + hdmi_ctl0 = 0x4d0b;
magic numbers..? I guess these are register offsets, would be better to
define..
> +
> + regmap_write(priv->hhi, priv->reg_offset + HHI_HDMI_PHY_CNTL0,
> + FIELD_PREP(HHI_HDMI_PHY_CNTL0_HDMI_CTL1, 0x08c3) |
> + FIELD_PREP(HHI_HDMI_PHY_CNTL0_HDMI_CTL0, hdmi_ctl0));
> +
> + regmap_write(priv->hhi, priv->reg_offset + HHI_HDMI_PHY_CNTL1, 0x0);
> +
> + /* Reset three times, just like the vendor driver does */
> + for (i = 0; i < 3; i++) {
> + regmap_write(priv->hhi, priv->reg_offset + HHI_HDMI_PHY_CNTL1,
> + HHI_HDMI_PHY_CNTL1_CLOCK_ENABLE |
> + HHI_HDMI_PHY_CNTL1_SOFT_RESET);
> + usleep_range(1000, 2000);
> +
> + regmap_write(priv->hhi, priv->reg_offset + HHI_HDMI_PHY_CNTL1,
> + HHI_HDMI_PHY_CNTL1_CLOCK_ENABLE);
> + usleep_range(1000, 2000);
> + }
> +
> + return 0;
> +}
> +
> +static int phy_meson8_hdmi_tx_power_off(struct phy *phy)
> +{
> + struct phy_meson8_hdmi_tx_priv *priv = phy_get_drvdata(phy);
> +
> + regmap_write(priv->hhi, priv->reg_offset + HHI_HDMI_PHY_CNTL0,
> + FIELD_PREP(HHI_HDMI_PHY_CNTL0_HDMI_CTL1, 0x0841) |
> + FIELD_PREP(HHI_HDMI_PHY_CNTL0_HDMI_CTL0, 0x8d00));
more magic..
> +
> + return 0;
> +}
> +
> +static const struct phy_ops phy_meson8_hdmi_tx_ops = {
> + .init = phy_meson8_hdmi_tx_init,
> + .exit = phy_meson8_hdmi_tx_exit,
> + .power_on = phy_meson8_hdmi_tx_power_on,
> + .power_off = phy_meson8_hdmi_tx_power_off,
> + .owner = THIS_MODULE,
> +};
> +
> +static int phy_meson8_hdmi_tx_probe(struct platform_device *pdev)
> +{
> + struct device_node *np = pdev->dev.of_node;
> + struct phy_meson8_hdmi_tx_priv *priv;
> + struct phy_provider *phy_provider;
> + struct phy *phy;
> + u32 reg[2];
> + int ret;
> +
> + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + ret = device_property_read_u32_array(&pdev->dev, "reg", reg,
> + ARRAY_SIZE(reg));
we have reg as single property, why array with 2 entries here?
> + if (ret)
> + return dev_err_probe(&pdev->dev, ret,
> + "Failed to parse the 'reg' property\n");
> +
> + priv->reg_offset = reg[0];
and mystery deepens, no use of reg[1], leftover artifacts?
> +
> + priv->hhi = syscon_node_to_regmap(np->parent);
> + if (IS_ERR(priv->hhi))
> + return PTR_ERR(priv->hhi);
> +
> + priv->tmds_clk = devm_clk_get(&pdev->dev, NULL);
> + if (IS_ERR(priv->tmds_clk))
> + return PTR_ERR(priv->tmds_clk);
> +
> + phy = devm_phy_create(&pdev->dev, np, &phy_meson8_hdmi_tx_ops);
> + if (IS_ERR(phy))
> + return PTR_ERR(phy);
> +
> + phy_set_drvdata(phy, priv);
> +
> + phy_provider = devm_of_phy_provider_register(&pdev->dev,
> + of_phy_simple_xlate);
> +
> + return PTR_ERR_OR_ZERO(phy_provider);
> +}
> +
> +static const struct of_device_id phy_meson8_hdmi_tx_of_match[] = {
> + { .compatible = "amlogic,meson8-hdmi-tx-phy" },
> + { .compatible = "amlogic,meson8b-hdmi-tx-phy" },
> + { .compatible = "amlogic,meson8m2-hdmi-tx-phy" },
> + { /* sentinel */ }
I see that all three are handled similarly, no difference!
Maybe also add a amlogic,meson8-hdmi compatible and use that?
--
~Vinod
WARNING: multiple messages have this Message-ID (diff)
From: Vinod Koul <vkoul@kernel.org>
To: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Cc: robh+dt@kernel.org, linux-phy@lists.infradead.org,
devicetree@vger.kernel.org, linux-amlogic@lists.infradead.org,
kishon@ti.com, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 2/2] phy: amlogic: Add a new driver for the HDMI TX PHY on Meson8/8b/8m2
Date: Tue, 20 Jul 2021 14:06:59 +0530 [thread overview]
Message-ID: <YPaLKwEKcDLWwztM@matsya> (raw)
In-Reply-To: <20210629182047.893415-3-martin.blumenstingl@googlemail.com>
On 29-06-21, 20:20, Martin Blumenstingl wrote:
> Amlogic Meson8/8b/8m2 have a built-in HDMI PHY in the HHI register
> region. Unfortunately only few register bits are documented. For
> HHI_HDMI_PHY_CNTL0 the magic numbers are taken from the 3.10 vendor
> kernel.
>
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
> drivers/phy/amlogic/Kconfig | 10 ++
> drivers/phy/amlogic/Makefile | 1 +
> drivers/phy/amlogic/phy-meson8-hdmi-tx.c | 162 +++++++++++++++++++++++
> 3 files changed, 173 insertions(+)
> create mode 100644 drivers/phy/amlogic/phy-meson8-hdmi-tx.c
>
> diff --git a/drivers/phy/amlogic/Kconfig b/drivers/phy/amlogic/Kconfig
> index db5d0cd757e3..486ca23aba32 100644
> --- a/drivers/phy/amlogic/Kconfig
> +++ b/drivers/phy/amlogic/Kconfig
> @@ -2,6 +2,16 @@
> #
> # Phy drivers for Amlogic platforms
> #
> +config PHY_MESON8_HDMI_TX
> + tristate "Meson8, Meson8b and Meson8m2 HDMI TX PHY driver"
> + depends on (ARCH_MESON && ARM) || COMPILE_TEST
> + depends on OF
> + select MFD_SYSCON
> + help
> + Enable this to support the HDMI TX PHYs found in Meson8,
> + Meson8b and Meson8m2 SoCs.
> + If unsure, say N.
> +
> config PHY_MESON8B_USB2
> tristate "Meson8, Meson8b, Meson8m2 and GXBB USB2 PHY driver"
> default ARCH_MESON
> diff --git a/drivers/phy/amlogic/Makefile b/drivers/phy/amlogic/Makefile
> index 8fa07fbd0d92..c0886c850bb0 100644
> --- a/drivers/phy/amlogic/Makefile
> +++ b/drivers/phy/amlogic/Makefile
> @@ -1,4 +1,5 @@
> # SPDX-License-Identifier: GPL-2.0-only
> +obj-$(CONFIG_PHY_MESON8_HDMI_TX) += phy-meson8-hdmi-tx.o
> obj-$(CONFIG_PHY_MESON8B_USB2) += phy-meson8b-usb2.o
> obj-$(CONFIG_PHY_MESON_GXL_USB2) += phy-meson-gxl-usb2.o
> obj-$(CONFIG_PHY_MESON_G12A_USB2) += phy-meson-g12a-usb2.o
> diff --git a/drivers/phy/amlogic/phy-meson8-hdmi-tx.c b/drivers/phy/amlogic/phy-meson8-hdmi-tx.c
> new file mode 100644
> index 000000000000..ba5a4de54811
> --- /dev/null
> +++ b/drivers/phy/amlogic/phy-meson8-hdmi-tx.c
> @@ -0,0 +1,162 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Meson8, Meson8b and Meson8m2 HDMI TX PHY.
> + *
> + * Copyright (C) 2021 Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/bits.h>
> +#include <linux/clk.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/property.h>
> +#include <linux/regmap.h>
> +
> +#define HHI_HDMI_PHY_CNTL0 0x0
> + #define HHI_HDMI_PHY_CNTL0_HDMI_CTL1 GENMASK(31, 16)
> + #define HHI_HDMI_PHY_CNTL0_HDMI_CTL0 GENMASK(15, 0)
> +
> +#define HHI_HDMI_PHY_CNTL1 0x4
> + #define HHI_HDMI_PHY_CNTL1_CLOCK_ENABLE BIT(1)
> + #define HHI_HDMI_PHY_CNTL1_SOFT_RESET BIT(0)
> +
> +#define HHI_HDMI_PHY_CNTL2 0x8
> +
> +struct phy_meson8_hdmi_tx_priv {
> + struct regmap *hhi;
> + struct clk *tmds_clk;
> + unsigned int reg_offset;
> +};
> +
> +static int phy_meson8_hdmi_tx_init(struct phy *phy)
> +{
> + struct phy_meson8_hdmi_tx_priv *priv = phy_get_drvdata(phy);
> +
> + return clk_prepare_enable(priv->tmds_clk);
> +}
> +
> +static int phy_meson8_hdmi_tx_exit(struct phy *phy)
> +{
> + struct phy_meson8_hdmi_tx_priv *priv = phy_get_drvdata(phy);
> +
> + clk_disable_unprepare(priv->tmds_clk);
> +
> + return 0;
> +}
> +
> +static int phy_meson8_hdmi_tx_power_on(struct phy *phy)
> +{
> + struct phy_meson8_hdmi_tx_priv *priv = phy_get_drvdata(phy);
> + unsigned int i;
> + u16 hdmi_ctl0;
> +
> + if (clk_get_rate(priv->tmds_clk) >= 2970UL * 1000 * 1000)
> + hdmi_ctl0 = 0x1e8b;
> + else
> + hdmi_ctl0 = 0x4d0b;
magic numbers..? I guess these are register offsets, would be better to
define..
> +
> + regmap_write(priv->hhi, priv->reg_offset + HHI_HDMI_PHY_CNTL0,
> + FIELD_PREP(HHI_HDMI_PHY_CNTL0_HDMI_CTL1, 0x08c3) |
> + FIELD_PREP(HHI_HDMI_PHY_CNTL0_HDMI_CTL0, hdmi_ctl0));
> +
> + regmap_write(priv->hhi, priv->reg_offset + HHI_HDMI_PHY_CNTL1, 0x0);
> +
> + /* Reset three times, just like the vendor driver does */
> + for (i = 0; i < 3; i++) {
> + regmap_write(priv->hhi, priv->reg_offset + HHI_HDMI_PHY_CNTL1,
> + HHI_HDMI_PHY_CNTL1_CLOCK_ENABLE |
> + HHI_HDMI_PHY_CNTL1_SOFT_RESET);
> + usleep_range(1000, 2000);
> +
> + regmap_write(priv->hhi, priv->reg_offset + HHI_HDMI_PHY_CNTL1,
> + HHI_HDMI_PHY_CNTL1_CLOCK_ENABLE);
> + usleep_range(1000, 2000);
> + }
> +
> + return 0;
> +}
> +
> +static int phy_meson8_hdmi_tx_power_off(struct phy *phy)
> +{
> + struct phy_meson8_hdmi_tx_priv *priv = phy_get_drvdata(phy);
> +
> + regmap_write(priv->hhi, priv->reg_offset + HHI_HDMI_PHY_CNTL0,
> + FIELD_PREP(HHI_HDMI_PHY_CNTL0_HDMI_CTL1, 0x0841) |
> + FIELD_PREP(HHI_HDMI_PHY_CNTL0_HDMI_CTL0, 0x8d00));
more magic..
> +
> + return 0;
> +}
> +
> +static const struct phy_ops phy_meson8_hdmi_tx_ops = {
> + .init = phy_meson8_hdmi_tx_init,
> + .exit = phy_meson8_hdmi_tx_exit,
> + .power_on = phy_meson8_hdmi_tx_power_on,
> + .power_off = phy_meson8_hdmi_tx_power_off,
> + .owner = THIS_MODULE,
> +};
> +
> +static int phy_meson8_hdmi_tx_probe(struct platform_device *pdev)
> +{
> + struct device_node *np = pdev->dev.of_node;
> + struct phy_meson8_hdmi_tx_priv *priv;
> + struct phy_provider *phy_provider;
> + struct phy *phy;
> + u32 reg[2];
> + int ret;
> +
> + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + ret = device_property_read_u32_array(&pdev->dev, "reg", reg,
> + ARRAY_SIZE(reg));
we have reg as single property, why array with 2 entries here?
> + if (ret)
> + return dev_err_probe(&pdev->dev, ret,
> + "Failed to parse the 'reg' property\n");
> +
> + priv->reg_offset = reg[0];
and mystery deepens, no use of reg[1], leftover artifacts?
> +
> + priv->hhi = syscon_node_to_regmap(np->parent);
> + if (IS_ERR(priv->hhi))
> + return PTR_ERR(priv->hhi);
> +
> + priv->tmds_clk = devm_clk_get(&pdev->dev, NULL);
> + if (IS_ERR(priv->tmds_clk))
> + return PTR_ERR(priv->tmds_clk);
> +
> + phy = devm_phy_create(&pdev->dev, np, &phy_meson8_hdmi_tx_ops);
> + if (IS_ERR(phy))
> + return PTR_ERR(phy);
> +
> + phy_set_drvdata(phy, priv);
> +
> + phy_provider = devm_of_phy_provider_register(&pdev->dev,
> + of_phy_simple_xlate);
> +
> + return PTR_ERR_OR_ZERO(phy_provider);
> +}
> +
> +static const struct of_device_id phy_meson8_hdmi_tx_of_match[] = {
> + { .compatible = "amlogic,meson8-hdmi-tx-phy" },
> + { .compatible = "amlogic,meson8b-hdmi-tx-phy" },
> + { .compatible = "amlogic,meson8m2-hdmi-tx-phy" },
> + { /* sentinel */ }
I see that all three are handled similarly, no difference!
Maybe also add a amlogic,meson8-hdmi compatible and use that?
--
~Vinod
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
WARNING: multiple messages have this Message-ID (diff)
From: Vinod Koul <vkoul@kernel.org>
To: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Cc: robh+dt@kernel.org, linux-phy@lists.infradead.org,
devicetree@vger.kernel.org, linux-amlogic@lists.infradead.org,
kishon@ti.com, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 2/2] phy: amlogic: Add a new driver for the HDMI TX PHY on Meson8/8b/8m2
Date: Tue, 20 Jul 2021 14:06:59 +0530 [thread overview]
Message-ID: <YPaLKwEKcDLWwztM@matsya> (raw)
In-Reply-To: <20210629182047.893415-3-martin.blumenstingl@googlemail.com>
On 29-06-21, 20:20, Martin Blumenstingl wrote:
> Amlogic Meson8/8b/8m2 have a built-in HDMI PHY in the HHI register
> region. Unfortunately only few register bits are documented. For
> HHI_HDMI_PHY_CNTL0 the magic numbers are taken from the 3.10 vendor
> kernel.
>
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
> drivers/phy/amlogic/Kconfig | 10 ++
> drivers/phy/amlogic/Makefile | 1 +
> drivers/phy/amlogic/phy-meson8-hdmi-tx.c | 162 +++++++++++++++++++++++
> 3 files changed, 173 insertions(+)
> create mode 100644 drivers/phy/amlogic/phy-meson8-hdmi-tx.c
>
> diff --git a/drivers/phy/amlogic/Kconfig b/drivers/phy/amlogic/Kconfig
> index db5d0cd757e3..486ca23aba32 100644
> --- a/drivers/phy/amlogic/Kconfig
> +++ b/drivers/phy/amlogic/Kconfig
> @@ -2,6 +2,16 @@
> #
> # Phy drivers for Amlogic platforms
> #
> +config PHY_MESON8_HDMI_TX
> + tristate "Meson8, Meson8b and Meson8m2 HDMI TX PHY driver"
> + depends on (ARCH_MESON && ARM) || COMPILE_TEST
> + depends on OF
> + select MFD_SYSCON
> + help
> + Enable this to support the HDMI TX PHYs found in Meson8,
> + Meson8b and Meson8m2 SoCs.
> + If unsure, say N.
> +
> config PHY_MESON8B_USB2
> tristate "Meson8, Meson8b, Meson8m2 and GXBB USB2 PHY driver"
> default ARCH_MESON
> diff --git a/drivers/phy/amlogic/Makefile b/drivers/phy/amlogic/Makefile
> index 8fa07fbd0d92..c0886c850bb0 100644
> --- a/drivers/phy/amlogic/Makefile
> +++ b/drivers/phy/amlogic/Makefile
> @@ -1,4 +1,5 @@
> # SPDX-License-Identifier: GPL-2.0-only
> +obj-$(CONFIG_PHY_MESON8_HDMI_TX) += phy-meson8-hdmi-tx.o
> obj-$(CONFIG_PHY_MESON8B_USB2) += phy-meson8b-usb2.o
> obj-$(CONFIG_PHY_MESON_GXL_USB2) += phy-meson-gxl-usb2.o
> obj-$(CONFIG_PHY_MESON_G12A_USB2) += phy-meson-g12a-usb2.o
> diff --git a/drivers/phy/amlogic/phy-meson8-hdmi-tx.c b/drivers/phy/amlogic/phy-meson8-hdmi-tx.c
> new file mode 100644
> index 000000000000..ba5a4de54811
> --- /dev/null
> +++ b/drivers/phy/amlogic/phy-meson8-hdmi-tx.c
> @@ -0,0 +1,162 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Meson8, Meson8b and Meson8m2 HDMI TX PHY.
> + *
> + * Copyright (C) 2021 Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/bits.h>
> +#include <linux/clk.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/property.h>
> +#include <linux/regmap.h>
> +
> +#define HHI_HDMI_PHY_CNTL0 0x0
> + #define HHI_HDMI_PHY_CNTL0_HDMI_CTL1 GENMASK(31, 16)
> + #define HHI_HDMI_PHY_CNTL0_HDMI_CTL0 GENMASK(15, 0)
> +
> +#define HHI_HDMI_PHY_CNTL1 0x4
> + #define HHI_HDMI_PHY_CNTL1_CLOCK_ENABLE BIT(1)
> + #define HHI_HDMI_PHY_CNTL1_SOFT_RESET BIT(0)
> +
> +#define HHI_HDMI_PHY_CNTL2 0x8
> +
> +struct phy_meson8_hdmi_tx_priv {
> + struct regmap *hhi;
> + struct clk *tmds_clk;
> + unsigned int reg_offset;
> +};
> +
> +static int phy_meson8_hdmi_tx_init(struct phy *phy)
> +{
> + struct phy_meson8_hdmi_tx_priv *priv = phy_get_drvdata(phy);
> +
> + return clk_prepare_enable(priv->tmds_clk);
> +}
> +
> +static int phy_meson8_hdmi_tx_exit(struct phy *phy)
> +{
> + struct phy_meson8_hdmi_tx_priv *priv = phy_get_drvdata(phy);
> +
> + clk_disable_unprepare(priv->tmds_clk);
> +
> + return 0;
> +}
> +
> +static int phy_meson8_hdmi_tx_power_on(struct phy *phy)
> +{
> + struct phy_meson8_hdmi_tx_priv *priv = phy_get_drvdata(phy);
> + unsigned int i;
> + u16 hdmi_ctl0;
> +
> + if (clk_get_rate(priv->tmds_clk) >= 2970UL * 1000 * 1000)
> + hdmi_ctl0 = 0x1e8b;
> + else
> + hdmi_ctl0 = 0x4d0b;
magic numbers..? I guess these are register offsets, would be better to
define..
> +
> + regmap_write(priv->hhi, priv->reg_offset + HHI_HDMI_PHY_CNTL0,
> + FIELD_PREP(HHI_HDMI_PHY_CNTL0_HDMI_CTL1, 0x08c3) |
> + FIELD_PREP(HHI_HDMI_PHY_CNTL0_HDMI_CTL0, hdmi_ctl0));
> +
> + regmap_write(priv->hhi, priv->reg_offset + HHI_HDMI_PHY_CNTL1, 0x0);
> +
> + /* Reset three times, just like the vendor driver does */
> + for (i = 0; i < 3; i++) {
> + regmap_write(priv->hhi, priv->reg_offset + HHI_HDMI_PHY_CNTL1,
> + HHI_HDMI_PHY_CNTL1_CLOCK_ENABLE |
> + HHI_HDMI_PHY_CNTL1_SOFT_RESET);
> + usleep_range(1000, 2000);
> +
> + regmap_write(priv->hhi, priv->reg_offset + HHI_HDMI_PHY_CNTL1,
> + HHI_HDMI_PHY_CNTL1_CLOCK_ENABLE);
> + usleep_range(1000, 2000);
> + }
> +
> + return 0;
> +}
> +
> +static int phy_meson8_hdmi_tx_power_off(struct phy *phy)
> +{
> + struct phy_meson8_hdmi_tx_priv *priv = phy_get_drvdata(phy);
> +
> + regmap_write(priv->hhi, priv->reg_offset + HHI_HDMI_PHY_CNTL0,
> + FIELD_PREP(HHI_HDMI_PHY_CNTL0_HDMI_CTL1, 0x0841) |
> + FIELD_PREP(HHI_HDMI_PHY_CNTL0_HDMI_CTL0, 0x8d00));
more magic..
> +
> + return 0;
> +}
> +
> +static const struct phy_ops phy_meson8_hdmi_tx_ops = {
> + .init = phy_meson8_hdmi_tx_init,
> + .exit = phy_meson8_hdmi_tx_exit,
> + .power_on = phy_meson8_hdmi_tx_power_on,
> + .power_off = phy_meson8_hdmi_tx_power_off,
> + .owner = THIS_MODULE,
> +};
> +
> +static int phy_meson8_hdmi_tx_probe(struct platform_device *pdev)
> +{
> + struct device_node *np = pdev->dev.of_node;
> + struct phy_meson8_hdmi_tx_priv *priv;
> + struct phy_provider *phy_provider;
> + struct phy *phy;
> + u32 reg[2];
> + int ret;
> +
> + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + ret = device_property_read_u32_array(&pdev->dev, "reg", reg,
> + ARRAY_SIZE(reg));
we have reg as single property, why array with 2 entries here?
> + if (ret)
> + return dev_err_probe(&pdev->dev, ret,
> + "Failed to parse the 'reg' property\n");
> +
> + priv->reg_offset = reg[0];
and mystery deepens, no use of reg[1], leftover artifacts?
> +
> + priv->hhi = syscon_node_to_regmap(np->parent);
> + if (IS_ERR(priv->hhi))
> + return PTR_ERR(priv->hhi);
> +
> + priv->tmds_clk = devm_clk_get(&pdev->dev, NULL);
> + if (IS_ERR(priv->tmds_clk))
> + return PTR_ERR(priv->tmds_clk);
> +
> + phy = devm_phy_create(&pdev->dev, np, &phy_meson8_hdmi_tx_ops);
> + if (IS_ERR(phy))
> + return PTR_ERR(phy);
> +
> + phy_set_drvdata(phy, priv);
> +
> + phy_provider = devm_of_phy_provider_register(&pdev->dev,
> + of_phy_simple_xlate);
> +
> + return PTR_ERR_OR_ZERO(phy_provider);
> +}
> +
> +static const struct of_device_id phy_meson8_hdmi_tx_of_match[] = {
> + { .compatible = "amlogic,meson8-hdmi-tx-phy" },
> + { .compatible = "amlogic,meson8b-hdmi-tx-phy" },
> + { .compatible = "amlogic,meson8m2-hdmi-tx-phy" },
> + { /* sentinel */ }
I see that all three are handled similarly, no difference!
Maybe also add a amlogic,meson8-hdmi compatible and use that?
--
~Vinod
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
WARNING: multiple messages have this Message-ID (diff)
From: Vinod Koul <vkoul@kernel.org>
To: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Cc: robh+dt@kernel.org, linux-phy@lists.infradead.org,
devicetree@vger.kernel.org, linux-amlogic@lists.infradead.org,
kishon@ti.com, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 2/2] phy: amlogic: Add a new driver for the HDMI TX PHY on Meson8/8b/8m2
Date: Tue, 20 Jul 2021 14:06:59 +0530 [thread overview]
Message-ID: <YPaLKwEKcDLWwztM@matsya> (raw)
In-Reply-To: <20210629182047.893415-3-martin.blumenstingl@googlemail.com>
On 29-06-21, 20:20, Martin Blumenstingl wrote:
> Amlogic Meson8/8b/8m2 have a built-in HDMI PHY in the HHI register
> region. Unfortunately only few register bits are documented. For
> HHI_HDMI_PHY_CNTL0 the magic numbers are taken from the 3.10 vendor
> kernel.
>
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
> drivers/phy/amlogic/Kconfig | 10 ++
> drivers/phy/amlogic/Makefile | 1 +
> drivers/phy/amlogic/phy-meson8-hdmi-tx.c | 162 +++++++++++++++++++++++
> 3 files changed, 173 insertions(+)
> create mode 100644 drivers/phy/amlogic/phy-meson8-hdmi-tx.c
>
> diff --git a/drivers/phy/amlogic/Kconfig b/drivers/phy/amlogic/Kconfig
> index db5d0cd757e3..486ca23aba32 100644
> --- a/drivers/phy/amlogic/Kconfig
> +++ b/drivers/phy/amlogic/Kconfig
> @@ -2,6 +2,16 @@
> #
> # Phy drivers for Amlogic platforms
> #
> +config PHY_MESON8_HDMI_TX
> + tristate "Meson8, Meson8b and Meson8m2 HDMI TX PHY driver"
> + depends on (ARCH_MESON && ARM) || COMPILE_TEST
> + depends on OF
> + select MFD_SYSCON
> + help
> + Enable this to support the HDMI TX PHYs found in Meson8,
> + Meson8b and Meson8m2 SoCs.
> + If unsure, say N.
> +
> config PHY_MESON8B_USB2
> tristate "Meson8, Meson8b, Meson8m2 and GXBB USB2 PHY driver"
> default ARCH_MESON
> diff --git a/drivers/phy/amlogic/Makefile b/drivers/phy/amlogic/Makefile
> index 8fa07fbd0d92..c0886c850bb0 100644
> --- a/drivers/phy/amlogic/Makefile
> +++ b/drivers/phy/amlogic/Makefile
> @@ -1,4 +1,5 @@
> # SPDX-License-Identifier: GPL-2.0-only
> +obj-$(CONFIG_PHY_MESON8_HDMI_TX) += phy-meson8-hdmi-tx.o
> obj-$(CONFIG_PHY_MESON8B_USB2) += phy-meson8b-usb2.o
> obj-$(CONFIG_PHY_MESON_GXL_USB2) += phy-meson-gxl-usb2.o
> obj-$(CONFIG_PHY_MESON_G12A_USB2) += phy-meson-g12a-usb2.o
> diff --git a/drivers/phy/amlogic/phy-meson8-hdmi-tx.c b/drivers/phy/amlogic/phy-meson8-hdmi-tx.c
> new file mode 100644
> index 000000000000..ba5a4de54811
> --- /dev/null
> +++ b/drivers/phy/amlogic/phy-meson8-hdmi-tx.c
> @@ -0,0 +1,162 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Meson8, Meson8b and Meson8m2 HDMI TX PHY.
> + *
> + * Copyright (C) 2021 Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/bits.h>
> +#include <linux/clk.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/property.h>
> +#include <linux/regmap.h>
> +
> +#define HHI_HDMI_PHY_CNTL0 0x0
> + #define HHI_HDMI_PHY_CNTL0_HDMI_CTL1 GENMASK(31, 16)
> + #define HHI_HDMI_PHY_CNTL0_HDMI_CTL0 GENMASK(15, 0)
> +
> +#define HHI_HDMI_PHY_CNTL1 0x4
> + #define HHI_HDMI_PHY_CNTL1_CLOCK_ENABLE BIT(1)
> + #define HHI_HDMI_PHY_CNTL1_SOFT_RESET BIT(0)
> +
> +#define HHI_HDMI_PHY_CNTL2 0x8
> +
> +struct phy_meson8_hdmi_tx_priv {
> + struct regmap *hhi;
> + struct clk *tmds_clk;
> + unsigned int reg_offset;
> +};
> +
> +static int phy_meson8_hdmi_tx_init(struct phy *phy)
> +{
> + struct phy_meson8_hdmi_tx_priv *priv = phy_get_drvdata(phy);
> +
> + return clk_prepare_enable(priv->tmds_clk);
> +}
> +
> +static int phy_meson8_hdmi_tx_exit(struct phy *phy)
> +{
> + struct phy_meson8_hdmi_tx_priv *priv = phy_get_drvdata(phy);
> +
> + clk_disable_unprepare(priv->tmds_clk);
> +
> + return 0;
> +}
> +
> +static int phy_meson8_hdmi_tx_power_on(struct phy *phy)
> +{
> + struct phy_meson8_hdmi_tx_priv *priv = phy_get_drvdata(phy);
> + unsigned int i;
> + u16 hdmi_ctl0;
> +
> + if (clk_get_rate(priv->tmds_clk) >= 2970UL * 1000 * 1000)
> + hdmi_ctl0 = 0x1e8b;
> + else
> + hdmi_ctl0 = 0x4d0b;
magic numbers..? I guess these are register offsets, would be better to
define..
> +
> + regmap_write(priv->hhi, priv->reg_offset + HHI_HDMI_PHY_CNTL0,
> + FIELD_PREP(HHI_HDMI_PHY_CNTL0_HDMI_CTL1, 0x08c3) |
> + FIELD_PREP(HHI_HDMI_PHY_CNTL0_HDMI_CTL0, hdmi_ctl0));
> +
> + regmap_write(priv->hhi, priv->reg_offset + HHI_HDMI_PHY_CNTL1, 0x0);
> +
> + /* Reset three times, just like the vendor driver does */
> + for (i = 0; i < 3; i++) {
> + regmap_write(priv->hhi, priv->reg_offset + HHI_HDMI_PHY_CNTL1,
> + HHI_HDMI_PHY_CNTL1_CLOCK_ENABLE |
> + HHI_HDMI_PHY_CNTL1_SOFT_RESET);
> + usleep_range(1000, 2000);
> +
> + regmap_write(priv->hhi, priv->reg_offset + HHI_HDMI_PHY_CNTL1,
> + HHI_HDMI_PHY_CNTL1_CLOCK_ENABLE);
> + usleep_range(1000, 2000);
> + }
> +
> + return 0;
> +}
> +
> +static int phy_meson8_hdmi_tx_power_off(struct phy *phy)
> +{
> + struct phy_meson8_hdmi_tx_priv *priv = phy_get_drvdata(phy);
> +
> + regmap_write(priv->hhi, priv->reg_offset + HHI_HDMI_PHY_CNTL0,
> + FIELD_PREP(HHI_HDMI_PHY_CNTL0_HDMI_CTL1, 0x0841) |
> + FIELD_PREP(HHI_HDMI_PHY_CNTL0_HDMI_CTL0, 0x8d00));
more magic..
> +
> + return 0;
> +}
> +
> +static const struct phy_ops phy_meson8_hdmi_tx_ops = {
> + .init = phy_meson8_hdmi_tx_init,
> + .exit = phy_meson8_hdmi_tx_exit,
> + .power_on = phy_meson8_hdmi_tx_power_on,
> + .power_off = phy_meson8_hdmi_tx_power_off,
> + .owner = THIS_MODULE,
> +};
> +
> +static int phy_meson8_hdmi_tx_probe(struct platform_device *pdev)
> +{
> + struct device_node *np = pdev->dev.of_node;
> + struct phy_meson8_hdmi_tx_priv *priv;
> + struct phy_provider *phy_provider;
> + struct phy *phy;
> + u32 reg[2];
> + int ret;
> +
> + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + ret = device_property_read_u32_array(&pdev->dev, "reg", reg,
> + ARRAY_SIZE(reg));
we have reg as single property, why array with 2 entries here?
> + if (ret)
> + return dev_err_probe(&pdev->dev, ret,
> + "Failed to parse the 'reg' property\n");
> +
> + priv->reg_offset = reg[0];
and mystery deepens, no use of reg[1], leftover artifacts?
> +
> + priv->hhi = syscon_node_to_regmap(np->parent);
> + if (IS_ERR(priv->hhi))
> + return PTR_ERR(priv->hhi);
> +
> + priv->tmds_clk = devm_clk_get(&pdev->dev, NULL);
> + if (IS_ERR(priv->tmds_clk))
> + return PTR_ERR(priv->tmds_clk);
> +
> + phy = devm_phy_create(&pdev->dev, np, &phy_meson8_hdmi_tx_ops);
> + if (IS_ERR(phy))
> + return PTR_ERR(phy);
> +
> + phy_set_drvdata(phy, priv);
> +
> + phy_provider = devm_of_phy_provider_register(&pdev->dev,
> + of_phy_simple_xlate);
> +
> + return PTR_ERR_OR_ZERO(phy_provider);
> +}
> +
> +static const struct of_device_id phy_meson8_hdmi_tx_of_match[] = {
> + { .compatible = "amlogic,meson8-hdmi-tx-phy" },
> + { .compatible = "amlogic,meson8b-hdmi-tx-phy" },
> + { .compatible = "amlogic,meson8m2-hdmi-tx-phy" },
> + { /* sentinel */ }
I see that all three are handled similarly, no difference!
Maybe also add a amlogic,meson8-hdmi compatible and use that?
--
~Vinod
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
next prev parent reply other threads:[~2021-07-20 8:37 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-29 18:20 [PATCH v2 0/2] phy: Add support for the HDMI TX PHY on Meson8/8b/8m2 Martin Blumenstingl
2021-06-29 18:20 ` Martin Blumenstingl
2021-06-29 18:20 ` Martin Blumenstingl
2021-06-29 18:20 ` Martin Blumenstingl
2021-06-29 18:20 ` [PATCH v2 1/2] dt-bindings: phy: Add the Amlogic Meson8 HDMI TX PHY bindings Martin Blumenstingl
2021-06-29 18:20 ` Martin Blumenstingl
2021-06-29 18:20 ` Martin Blumenstingl
2021-06-29 18:20 ` Martin Blumenstingl
2021-07-14 21:09 ` Rob Herring
2021-07-14 21:09 ` Rob Herring
2021-07-14 21:09 ` Rob Herring
2021-07-14 21:09 ` Rob Herring
2021-06-29 18:20 ` [PATCH v2 2/2] phy: amlogic: Add a new driver for the HDMI TX PHY on Meson8/8b/8m2 Martin Blumenstingl
2021-06-29 18:20 ` Martin Blumenstingl
2021-06-29 18:20 ` Martin Blumenstingl
2021-06-29 18:20 ` Martin Blumenstingl
2021-07-20 8:36 ` Vinod Koul [this message]
2021-07-20 8:36 ` Vinod Koul
2021-07-20 8:36 ` Vinod Koul
2021-07-20 8:36 ` Vinod Koul
2021-07-20 22:08 ` Martin Blumenstingl
2021-07-20 22:08 ` Martin Blumenstingl
2021-07-20 22:08 ` Martin Blumenstingl
2021-07-20 22:08 ` Martin Blumenstingl
2021-07-22 9:08 ` Vinod Koul
2021-07-22 9:08 ` Vinod Koul
2021-07-22 9:08 ` Vinod Koul
2021-07-22 9:08 ` Vinod Koul
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YPaLKwEKcDLWwztM@matsya \
--to=vkoul@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=kishon@ti.com \
--cc=linux-amlogic@lists.infradead.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-phy@lists.infradead.org \
--cc=martin.blumenstingl@googlemail.com \
--cc=robh+dt@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.