This patchset adds support for HDMI at SoCs from Exynos4 family. The patches are rebased on kishon/next. Additionally, The patchset contains small fixes to PHY and CLK frameworks. I preferred to keep all the patches together for the first version of the RFC.
The interesting part might be 'propagation of clk_set_parent()'. This feature allows to remove the usage of artificial clocks in drivers. Such a situation happens for Exynos HDMI and 'mout_hdmi' where the clock is not even mentioned in some versions of SoC's documentation. Since enabling and setting rate can be propagated I think that clk_set_parent() should also be propagated. This would simplify driver's code and make it less dependant on SoC's version.
Another interesting feature refers to simple PHY driver. This driver register a PHY interface that operates by setting a special bit in platform register. This situation is very common in Exynos SoCs. The current version supports only one phy per node. The code might be modified to support multiple phys from single simple-phy provider to avoid creation of multiple nodes in DT.
All comments are welcome.
Regards, Tomasz Stanislawski
Tomasz Stanislawski (12): clk: propagate parent change up one level clk: exynos4: export sclk_hdmiphy clock clk: exynos4: enable clk_set_parent() propagation for sclk_hdmi and sclk_mixer clocks phy: Add simple-phy driver phy: use of_phy_simple_xlate for NULL xlate function Revert "drm/exynos: add mout_hdmi clock in hdmi driver to change parent" drm: exynos: hdmi: use hdmiphy as PHY drm: exynos: hdmi: simplify extracting hpd-gpio from DT drm: exynos: add compatibles for HDMI and Mixer chips and exynos4210 SoC arm: dts: exynos4: add i2c controller for HDMIPHY arm: dts: exynos4: add HDMI devices arm: dts: universal_c210: add HDMI devices
.../devicetree/bindings/clock/exynos4-clock.txt | 1 + arch/arm/boot/dts/exynos4.dtsi | 43 +++++++ arch/arm/boot/dts/exynos4210-universal_c210.dts | 53 ++++++++ arch/arm/boot/dts/exynos4210.dtsi | 4 + drivers/clk/clk.c | 6 + drivers/clk/samsung/clk-exynos4.c | 10 +- drivers/gpu/drm/exynos/exynos_hdmi.c | 41 +++---- drivers/gpu/drm/exynos/exynos_mixer.c | 3 + drivers/phy/Kconfig | 5 + drivers/phy/Makefile | 1 + drivers/phy/phy-core.c | 2 +- drivers/phy/phy-simple.c | 128 ++++++++++++++++++++ include/linux/clk-provider.h | 1 + 13 files changed, 269 insertions(+), 29 deletions(-) create mode 100644 drivers/phy/phy-simple.c
This patch adds support for propagation of setup of clock's parent one level up.
This feature is helpful when a driver changes topology of its clocks using clk_set_parent(). The problem occurs when on one platform/SoC driver's clock is located at MUX output but on the other platform/SoC there is a gated proxy clock between the MUX and driver's clock. In such a case, driver's code has to be modified to use one clock for enabling and the other clock for setup of a parent.
The code updates are avoided by propagating setup of a parent up one level.
Additionally, this patch adds CLK_SET_PARENT_PARENT (sorry for naming) flag to inform clk-core that clk_set_parent() should be propagated.
Signed-off-by: Tomasz Stanislawski t.stanislaws@samsung.com --- drivers/clk/clk.c | 6 ++++++ include/linux/clk-provider.h | 1 + 2 files changed, 7 insertions(+)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index a004769..14eda80 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -1595,6 +1595,12 @@ int clk_set_parent(struct clk *clk, struct clk *parent)
/* try finding the new parent index */ if (parent) { + if ((clk->flags & CLK_SET_PARENT_PARENT) + && clk->num_parents == 1) { + ret = clk_set_parent(clk->parent, parent); + goto out; + } + p_index = clk_fetch_parent_index(clk, parent); p_rate = parent->rate; if (p_index == clk->num_parents) { diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h index 73bdb69..83c98d5 100644 --- a/include/linux/clk-provider.h +++ b/include/linux/clk-provider.h @@ -29,6 +29,7 @@ #define CLK_IS_BASIC BIT(5) /* Basic clk, can't do a to_clk_foo() */ #define CLK_GET_RATE_NOCACHE BIT(6) /* do not use the cached clk rate */ #define CLK_SET_RATE_NO_REPARENT BIT(7) /* don't re-parent on rate change */ +#define CLK_SET_PARENT_PARENT BIT(8) /* propagate parent change up one level */
struct clk_hw;
Export sclk_hdmiphy clock to be usable from DT.
Signed-off-by: Tomasz Stanislawski t.stanislaws@samsung.com --- .../devicetree/bindings/clock/exynos4-clock.txt | 1 + drivers/clk/samsung/clk-exynos4.c | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/clock/exynos4-clock.txt b/Documentation/devicetree/bindings/clock/exynos4-clock.txt index c6bf8a6..cb08f5d 100644 --- a/Documentation/devicetree/bindings/clock/exynos4-clock.txt +++ b/Documentation/devicetree/bindings/clock/exynos4-clock.txt @@ -46,6 +46,7 @@ Exynos4 SoC and this is specified where applicable. mout_mpll_user_c 18 Exynos4x12 mout_core 19 mout_apll 20 + sclk_hdmiphy 21
[Clock Gate for Special Clocks] diff --git a/drivers/clk/samsung/clk-exynos4.c b/drivers/clk/samsung/clk-exynos4.c index ad5ff50..df79ca6 100644 --- a/drivers/clk/samsung/clk-exynos4.c +++ b/drivers/clk/samsung/clk-exynos4.c @@ -148,7 +148,7 @@ enum exynos4_clks { xxti, xusbxti, fin_pll, fout_apll, fout_mpll, fout_epll, fout_vpll, sclk_apll, sclk_mpll, sclk_epll, sclk_vpll, arm_clk, aclk200, aclk100, aclk160, aclk133, mout_mpll_user_t, mout_mpll_user_c, mout_core, - mout_apll, /* 20 */ + mout_apll, sclk_hdmiphy, /* 21 */
/* gate for special clocks (sclk) */ sclk_fimc0 = 128, sclk_fimc1, sclk_fimc2, sclk_fimc3, sclk_cam0, @@ -354,7 +354,7 @@ static struct samsung_fixed_rate_clock exynos4_fixed_rate_ext_clks[] __initdata /* fixed rate clocks generated inside the soc */ static struct samsung_fixed_rate_clock exynos4_fixed_rate_clks[] __initdata = { FRATE(none, "sclk_hdmi24m", NULL, CLK_IS_ROOT, 24000000), - FRATE(none, "sclk_hdmiphy", NULL, CLK_IS_ROOT, 27000000), + FRATE(sclk_hdmiphy, "sclk_hdmiphy", NULL, CLK_IS_ROOT, 27000000), FRATE(none, "sclk_usbphy0", NULL, CLK_IS_ROOT, 48000000), };
This patch enables clk_set_parent() propagation for clocks used by s5p-tv and exynos-drm drivers.
Signed-off-by: Tomasz Stanislawski t.stanislaws@samsung.com --- drivers/clk/samsung/clk-exynos4.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/samsung/clk-exynos4.c b/drivers/clk/samsung/clk-exynos4.c index df79ca6..1f58b7c 100644 --- a/drivers/clk/samsung/clk-exynos4.c +++ b/drivers/clk/samsung/clk-exynos4.c @@ -605,7 +605,8 @@ static struct samsung_gate_clock exynos4_gate_clks[] __initdata = { * the device name and clock alias names specified below for some * of the clocks can be removed. */ - GATE(sclk_hdmi, "sclk_hdmi", "mout_hdmi", SRC_MASK_TV, 0, 0, 0), + GATE(sclk_hdmi, "sclk_hdmi", "mout_hdmi", SRC_MASK_TV, 0, + CLK_SET_PARENT_PARENT, 0), GATE(sclk_spdif, "sclk_spdif", "mout_spdif", SRC_MASK_PERIL1, 8, 0, 0), GATE(jpeg, "jpeg", "aclk160", GATE_IP_CAM, 6, 0, 0), GATE(mie0, "mie0", "aclk160", GATE_IP_LCD0, 1, 0, 0), @@ -801,7 +802,8 @@ static struct samsung_gate_clock exynos4210_gate_clks[] __initdata = { E4210_SRC_MASK_LCD1, 12, CLK_SET_RATE_PARENT, 0), GATE(sclk_sata, "sclk_sata", "div_sata", SRC_MASK_FSYS, 24, CLK_SET_RATE_PARENT, 0), - GATE(sclk_mixer, "sclk_mixer", "mout_mixer", SRC_MASK_TV, 4, 0, 0), + GATE(sclk_mixer, "sclk_mixer", "mout_mixer", SRC_MASK_TV, 4, + CLK_SET_PARENT_PARENT, 0), GATE(sclk_dac, "sclk_dac", "mout_dac", SRC_MASK_TV, 8, 0, 0), GATE(tsadc, "tsadc", "aclk100", GATE_IP_PERIL, 15, 0, 0),
Add simple-phy driver to support a single register PHY interfaces present on Exynos4 SoC.
Signed-off-by: Tomasz Stanislawski t.stanislaws@samsung.com --- drivers/phy/Kconfig | 5 ++ drivers/phy/Makefile | 1 + drivers/phy/phy-simple.c | 128 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 134 insertions(+) create mode 100644 drivers/phy/phy-simple.c
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig index ac239ac..619c657 100644 --- a/drivers/phy/Kconfig +++ b/drivers/phy/Kconfig @@ -38,4 +38,9 @@ config TWL4030_USB This transceiver supports high and full speed devices plus, in host mode, low speed.
+config PHY_SIMPLE + tristate "Simple PHY driver" + help + Support for PHY controllers configured using single register. + endmenu diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile index 0dd8a98..3d68e19 100644 --- a/drivers/phy/Makefile +++ b/drivers/phy/Makefile @@ -5,3 +5,4 @@ obj-$(CONFIG_GENERIC_PHY) += phy-core.o obj-$(CONFIG_OMAP_USB2) += phy-omap-usb2.o obj-$(CONFIG_TWL4030_USB) += phy-twl4030-usb.o +obj-$(CONFIG_PHY_SIMPLE) += phy-simple.o diff --git a/drivers/phy/phy-simple.c b/drivers/phy/phy-simple.c new file mode 100644 index 0000000..4a28af7 --- /dev/null +++ b/drivers/phy/phy-simple.c @@ -0,0 +1,128 @@ +/* + * Simple PHY driver + * + * Copyright (C) 2013 Samsung Electronics Co., Ltd. + * Author: Tomasz Stanislawski t.stanislaws@samsung.com + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include <linux/io.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/of_address.h> +#include <linux/phy/phy.h> +#include <linux/platform_device.h> +#include <linux/spinlock.h> + +struct simple_phy { + spinlock_t slock; + u32 on_value; + u32 off_value; + u32 mask; + void __iomem *regs; +}; + +static int sphy_set(struct simple_phy *sphy, bool on) +{ + u32 reg; + + spin_lock(&sphy->slock); + + reg = readl(sphy->regs); + reg &= ~sphy->mask; + reg |= sphy->mask & (on ? sphy->on_value : sphy->off_value); + writel(reg, sphy->regs); + + spin_unlock(&sphy->slock); + + return 0; +} + +static int simple_phy_power_on(struct phy *phy) +{ + return sphy_set(phy_get_drvdata(phy), 1); +} + +static int simple_phy_power_off(struct phy *phy) +{ + return sphy_set(phy_get_drvdata(phy), 0); +} + +static struct phy_ops simple_phy_ops = { + .power_on = simple_phy_power_on, + .power_off = simple_phy_power_off, + .owner = THIS_MODULE, +}; + +static int simple_phy_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct simple_phy *sphy; + struct resource *res; + struct phy_provider *phy_provider; + struct phy *phy; + + sphy = devm_kzalloc(dev, sizeof(*sphy), GFP_KERNEL); + if (!sphy) + return -ENOMEM; + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + + sphy->regs = devm_ioremap_resource(dev, res); + if (IS_ERR(sphy->regs)) { + dev_err(dev, "failed to ioremap registers\n"); + return PTR_ERR(sphy->regs); + } + + spin_lock_init(&sphy->slock); + + phy_provider = devm_of_phy_provider_register(dev, NULL); + if (IS_ERR(phy_provider)) { + dev_err(dev, "failed to register PHY provider\n"); + return PTR_ERR(phy_provider); + } + + phy = devm_phy_create(dev, &simple_phy_ops, NULL); + if (IS_ERR(phy)) { + dev_err(dev, "failed to create PHY\n"); + return PTR_ERR(phy); + } + + sphy->mask = 1; + sphy->on_value = ~0; + sphy->off_value = 0; + + of_property_read_u32(dev->of_node, "mask", &sphy->mask); + of_property_read_u32(dev->of_node, "on-value", &sphy->on_value); + of_property_read_u32(dev->of_node, "off-value", &sphy->off_value); + + phy_set_drvdata(phy, sphy); + + dev_info(dev, "probe successful\n"); + + return 0; +} + +static const struct of_device_id simple_phy_of_match[] = { + { .compatible = "simple-phy" }, + { }, +}; +MODULE_DEVICE_TABLE(of, simple_phy_of_match); + +static struct platform_driver simple_phy_driver = { + .probe = simple_phy_probe, + .driver = { + .of_match_table = simple_phy_of_match, + .name = "simple-phy", + .owner = THIS_MODULE, + } +}; +module_platform_driver(simple_phy_driver); + +MODULE_DESCRIPTION("Simple PHY driver"); +MODULE_AUTHOR("Tomasz Stanislawski t.stanislaws@samsung.com"); +MODULE_LICENSE("GPL v2");
Hi,
On Monday 21 October 2013 07:48 PM, Tomasz Stanislawski wrote:
Add simple-phy driver to support a single register PHY interfaces present on Exynos4 SoC.
How are these PHY interfaces modelled in the SoC? Where does the register actually reside?
Signed-off-by: Tomasz Stanislawski t.stanislaws@samsung.com
drivers/phy/Kconfig | 5 ++ drivers/phy/Makefile | 1 + drivers/phy/phy-simple.c | 128 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 134 insertions(+) create mode 100644 drivers/phy/phy-simple.c
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig index ac239ac..619c657 100644 --- a/drivers/phy/Kconfig +++ b/drivers/phy/Kconfig @@ -38,4 +38,9 @@ config TWL4030_USB This transceiver supports high and full speed devices plus, in host mode, low speed.
+config PHY_SIMPLE
- tristate "Simple PHY driver"
This is too generic a name to be used. Lets name it something specific to what it is used for (EXYNOS/HDMI.. ?).
- help
Support for PHY controllers configured using single register.endmenu diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile index 0dd8a98..3d68e19 100644 --- a/drivers/phy/Makefile +++ b/drivers/phy/Makefile @@ -5,3 +5,4 @@ obj-$(CONFIG_GENERIC_PHY) += phy-core.o obj-$(CONFIG_OMAP_USB2) += phy-omap-usb2.o obj-$(CONFIG_TWL4030_USB) += phy-twl4030-usb.o +obj-$(CONFIG_PHY_SIMPLE) += phy-simple.o diff --git a/drivers/phy/phy-simple.c b/drivers/phy/phy-simple.c new file mode 100644 index 0000000..4a28af7 --- /dev/null +++ b/drivers/phy/phy-simple.c @@ -0,0 +1,128 @@ +/*
- Simple PHY driver
- Copyright (C) 2013 Samsung Electronics Co., Ltd.
- Author: Tomasz Stanislawski t.stanislaws@samsung.com
- This program is free software; you can redistribute it and/or modify
- it under the terms of the GNU General Public License version 2 as
- published by the Free Software Foundation.
- */
+#include <linux/io.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/of_address.h> +#include <linux/phy/phy.h> +#include <linux/platform_device.h> +#include <linux/spinlock.h>
+struct simple_phy {
- spinlock_t slock;
- u32 on_value;
- u32 off_value;
- u32 mask;
- void __iomem *regs;
+};
+static int sphy_set(struct simple_phy *sphy, bool on) +{
- u32 reg;
- spin_lock(&sphy->slock);
Lets add spin_lock only when it is absolutely necessary. When your PHY provider implements only a single PHY, it is not needed. phy_power_on and phy_power_off is already protected by the framework.
- reg = readl(sphy->regs);
- reg &= ~sphy->mask;
- reg |= sphy->mask & (on ? sphy->on_value : sphy->off_value);
- writel(reg, sphy->regs);
- spin_unlock(&sphy->slock);
- return 0;
+}
+static int simple_phy_power_on(struct phy *phy) +{
- return sphy_set(phy_get_drvdata(phy), 1);
+}
+static int simple_phy_power_off(struct phy *phy) +{
- return sphy_set(phy_get_drvdata(phy), 0);
+}
+static struct phy_ops simple_phy_ops = {
- .power_on = simple_phy_power_on,
- .power_off = simple_phy_power_off,
- .owner = THIS_MODULE,
+};
+static int simple_phy_probe(struct platform_device *pdev) +{
- struct device *dev = &pdev->dev;
- struct simple_phy *sphy;
- struct resource *res;
- struct phy_provider *phy_provider;
- struct phy *phy;
- sphy = devm_kzalloc(dev, sizeof(*sphy), GFP_KERNEL);
- if (!sphy)
return -ENOMEM;- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- sphy->regs = devm_ioremap_resource(dev, res);
- if (IS_ERR(sphy->regs)) {
dev_err(dev, "failed to ioremap registers\n");return PTR_ERR(sphy->regs);- }
- spin_lock_init(&sphy->slock);
- phy_provider = devm_of_phy_provider_register(dev, NULL);
pass 'of_phy_simple_xlate' instead of NULL.
- if (IS_ERR(phy_provider)) {
dev_err(dev, "failed to register PHY provider\n");return PTR_ERR(phy_provider);- }
- phy = devm_phy_create(dev, &simple_phy_ops, NULL);
- if (IS_ERR(phy)) {
dev_err(dev, "failed to create PHY\n");return PTR_ERR(phy);- }
- sphy->mask = 1;
- sphy->on_value = ~0;
- sphy->off_value = 0;
- of_property_read_u32(dev->of_node, "mask", &sphy->mask);
This means your driver will depend on dt data to describe how the register should look like. Not a good idea.
- of_property_read_u32(dev->of_node, "on-value", &sphy->on_value);
- of_property_read_u32(dev->of_node, "off-value", &sphy->off_value);
- phy_set_drvdata(phy, sphy);
- dev_info(dev, "probe successful\n");
Lets not make the boot noisy.
Thanks Kishon
Hi, Please refer to the comments below.
On 10/24/2013 05:52 PM, Kishon Vijay Abraham I wrote:
Hi,
On Monday 21 October 2013 07:48 PM, Tomasz Stanislawski wrote:
Add simple-phy driver to support a single register PHY interfaces present on Exynos4 SoC.
How are these PHY interfaces modelled in the SoC? Where does the register actually reside?
Initially, I was planning to add PHY for HDMI_PHY register in power management register set on s5pv310 soc.
However other PHYs use very similar interface (setting bit 0). This includes DAC_PHY, ADC_PHY, PCIe_PHY, SATA_PHY. Moreover it suits well to USBDEVICE_PHY, USBHOST_PHY. That is why I thought about using something more generic to handle all those phys without introducing a herd of 200-lines-long drivers.
Signed-off-by: Tomasz Stanislawski t.stanislaws@samsung.com
drivers/phy/Kconfig | 5 ++ drivers/phy/Makefile | 1 + drivers/phy/phy-simple.c | 128 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 134 insertions(+) create mode 100644 drivers/phy/phy-simple.c
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig index ac239ac..619c657 100644 --- a/drivers/phy/Kconfig +++ b/drivers/phy/Kconfig @@ -38,4 +38,9 @@ config TWL4030_USB This transceiver supports high and full speed devices plus, in host mode, low speed.
+config PHY_SIMPLE
- tristate "Simple PHY driver"
This is too generic a name to be used. Lets name it something specific to what it is used for (EXYNOS/HDMI.. ?).
Ok. It could be renamed to EXYNOS-SIMPLE-PHY or EXYNOS-1BIT-PHY or EXYNOS-GENERIC-PHY or something similar. Any ideas?
- help
Support for PHY controllers configured using single register.endmenu diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile index 0dd8a98..3d68e19 100644 --- a/drivers/phy/Makefile +++ b/drivers/phy/Makefile @@ -5,3 +5,4 @@ obj-$(CONFIG_GENERIC_PHY) += phy-core.o obj-$(CONFIG_OMAP_USB2) += phy-omap-usb2.o obj-$(CONFIG_TWL4030_USB) += phy-twl4030-usb.o +obj-$(CONFIG_PHY_SIMPLE) += phy-simple.o diff --git a/drivers/phy/phy-simple.c b/drivers/phy/phy-simple.c new file mode 100644 index 0000000..4a28af7 --- /dev/null +++ b/drivers/phy/phy-simple.c @@ -0,0 +1,128 @@ +/*
- Simple PHY driver
- Copyright (C) 2013 Samsung Electronics Co., Ltd.
- Author: Tomasz Stanislawski t.stanislaws@samsung.com
- This program is free software; you can redistribute it and/or modify
- it under the terms of the GNU General Public License version 2 as
- published by the Free Software Foundation.
- */
+#include <linux/io.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/of_address.h> +#include <linux/phy/phy.h> +#include <linux/platform_device.h> +#include <linux/spinlock.h>
+struct simple_phy {
- spinlock_t slock;
- u32 on_value;
- u32 off_value;
- u32 mask;
- void __iomem *regs;
+};
+static int sphy_set(struct simple_phy *sphy, bool on) +{
- u32 reg;
- spin_lock(&sphy->slock);
Lets add spin_lock only when it is absolutely necessary. When your PHY provider implements only a single PHY, it is not needed. phy_power_on and phy_power_off is already protected by the framework.
ok
- reg = readl(sphy->regs);
- reg &= ~sphy->mask;
- reg |= sphy->mask & (on ? sphy->on_value : sphy->off_value);
- writel(reg, sphy->regs);
- spin_unlock(&sphy->slock);
- return 0;
+}
+static int simple_phy_power_on(struct phy *phy) +{
- return sphy_set(phy_get_drvdata(phy), 1);
+}
+static int simple_phy_power_off(struct phy *phy) +{
- return sphy_set(phy_get_drvdata(phy), 0);
+}
+static struct phy_ops simple_phy_ops = {
- .power_on = simple_phy_power_on,
- .power_off = simple_phy_power_off,
- .owner = THIS_MODULE,
+};
+static int simple_phy_probe(struct platform_device *pdev) +{
- struct device *dev = &pdev->dev;
- struct simple_phy *sphy;
- struct resource *res;
- struct phy_provider *phy_provider;
- struct phy *phy;
- sphy = devm_kzalloc(dev, sizeof(*sphy), GFP_KERNEL);
- if (!sphy)
return -ENOMEM;- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- sphy->regs = devm_ioremap_resource(dev, res);
- if (IS_ERR(sphy->regs)) {
dev_err(dev, "failed to ioremap registers\n");return PTR_ERR(sphy->regs);- }
- spin_lock_init(&sphy->slock);
- phy_provider = devm_of_phy_provider_register(dev, NULL);
pass 'of_phy_simple_xlate' instead of NULL.
- if (IS_ERR(phy_provider)) {
dev_err(dev, "failed to register PHY provider\n");return PTR_ERR(phy_provider);- }
- phy = devm_phy_create(dev, &simple_phy_ops, NULL);
- if (IS_ERR(phy)) {
dev_err(dev, "failed to create PHY\n");return PTR_ERR(phy);- }
- sphy->mask = 1;
- sphy->on_value = ~0;
- sphy->off_value = 0;
- of_property_read_u32(dev->of_node, "mask", &sphy->mask);
This means your driver will depend on dt data to describe how the register should look like. Not a good idea.
I can remove it. No problem. The driver can justt use fixed mask=1,on-value=1,off-value=0. Adding mentioned attributes greatly improves driver flexibility but such a flexibility is not needed currently for s5pv310 phys.
But frankly, I do not exactly follow what is a rationale for such police in DT. It forces developer to write a lot of redundant code. Moreover, some clock drivers seams to violate it. Clock "picochip,pc3x3-gated-clk" is an example. One can find similar tricks in pinctrl.
- of_property_read_u32(dev->of_node, "on-value", &sphy->on_value);
- of_property_read_u32(dev->of_node, "off-value", &sphy->off_value);
- phy_set_drvdata(phy, sphy);
- dev_info(dev, "probe successful\n");
Lets not make the boot noisy.
ok. s/dev_info/dev_dbg is good enough?
Thanks Kishon
Could you take a look on other patches in this RFC?
Regards, Tomasz Stanislawski
Hi,
On Friday 25 October 2013 01:21 PM, Tomasz Stanislawski wrote:
Hi, Please refer to the comments below.
On 10/24/2013 05:52 PM, Kishon Vijay Abraham I wrote:
Hi,
On Monday 21 October 2013 07:48 PM, Tomasz Stanislawski wrote:
Add simple-phy driver to support a single register PHY interfaces present on Exynos4 SoC.
How are these PHY interfaces modelled in the SoC? Where does the register actually reside?
Initially, I was planning to add PHY for HDMI_PHY register in power management register set on s5pv310 soc.
If that register is part of the power management register space, I don't think it justifies creating a PHY driver for it.
However other PHYs use very similar interface (setting bit 0). This includes DAC_PHY, ADC_PHY, PCIe_PHY, SATA_PHY. Moreover it suits well to USBDEVICE_PHY, USBHOST_PHY.
How is it currently being done for these drivers? Is it being done in the patches sent by Kamil or Vivek?
Thanks Kishon
Use default handler of_phy_simple_xlate() when NULL is passed as argument to of_phy_provider_register().
Signed-off-by: Tomasz Stanislawski t.stanislaws@samsung.com --- drivers/phy/phy-core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c index 03cf8fb..c38ae1e7 100644 --- a/drivers/phy/phy-core.c +++ b/drivers/phy/phy-core.c @@ -575,7 +575,7 @@ struct phy_provider *__of_phy_provider_register(struct device *dev,
phy_provider->dev = dev; phy_provider->owner = owner; - phy_provider->of_xlate = of_xlate; + phy_provider->of_xlate = of_xlate ? of_xlate : of_phy_simple_xlate;
mutex_lock(&phy_provider_mutex); list_add_tail(&phy_provider->list, &phy_provider_list);
Hi,
On Monday 21 October 2013 07:48 PM, Tomasz Stanislawski wrote:
Use default handler of_phy_simple_xlate() when NULL is passed as argument to of_phy_provider_register().
Signed-off-by: Tomasz Stanislawski t.stanislaws@samsung.com
drivers/phy/phy-core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c index 03cf8fb..c38ae1e7 100644 --- a/drivers/phy/phy-core.c +++ b/drivers/phy/phy-core.c @@ -575,7 +575,7 @@ struct phy_provider *__of_phy_provider_register(struct device *dev,
phy_provider->dev = dev; phy_provider->owner = owner;
- phy_provider->of_xlate = of_xlate;
- phy_provider->of_xlate = of_xlate ? of_xlate : of_phy_simple_xlate;
Lets allow the phy provider to pass the correct of_xlate (of_phy_simple_xlate is exported anyway). Instead you can modify the patch to check for of_xlate and do a WARN if it is NULL.
Thanks Kishon
This reverts commit 59956d35a8618235ea715280b49447bb36f2c975. --- drivers/gpu/drm/exynos/exynos_hdmi.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c index a0e10ae..fcfa23a 100644 --- a/drivers/gpu/drm/exynos/exynos_hdmi.c +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c @@ -83,7 +83,6 @@ struct hdmi_resources { struct clk *sclk_pixel; struct clk *sclk_hdmiphy; struct clk *hdmiphy; - struct clk *mout_hdmi; struct regulator_bulk_data *regul_bulk; int regul_count; }; @@ -1113,7 +1112,7 @@ static void hdmi_v13_mode_apply(struct hdmi_context *hdata) }
clk_disable_unprepare(hdata->res.sclk_hdmi); - clk_set_parent(hdata->res.mout_hdmi, hdata->res.sclk_hdmiphy); + clk_set_parent(hdata->res.sclk_hdmi, hdata->res.sclk_hdmiphy); clk_prepare_enable(hdata->res.sclk_hdmi);
/* enable HDMI and timing generator */ @@ -1280,7 +1279,7 @@ static void hdmi_v14_mode_apply(struct hdmi_context *hdata) }
clk_disable_unprepare(hdata->res.sclk_hdmi); - clk_set_parent(hdata->res.mout_hdmi, hdata->res.sclk_hdmiphy); + clk_set_parent(hdata->res.sclk_hdmi, hdata->res.sclk_hdmiphy); clk_prepare_enable(hdata->res.sclk_hdmi);
/* enable HDMI and timing generator */ @@ -1306,7 +1305,7 @@ static void hdmiphy_conf_reset(struct hdmi_context *hdata) u32 reg;
clk_disable_unprepare(hdata->res.sclk_hdmi); - clk_set_parent(hdata->res.mout_hdmi, hdata->res.sclk_pixel); + clk_set_parent(hdata->res.sclk_hdmi, hdata->res.sclk_pixel); clk_prepare_enable(hdata->res.sclk_hdmi);
/* operation mode */ @@ -1815,13 +1814,8 @@ static int hdmi_resources_init(struct hdmi_context *hdata) DRM_ERROR("failed to get clock 'hdmiphy'\n"); goto fail; } - res->mout_hdmi = devm_clk_get(dev, "mout_hdmi"); - if (IS_ERR(res->mout_hdmi)) { - DRM_ERROR("failed to get clock 'mout_hdmi'\n"); - goto fail; - }
- clk_set_parent(res->mout_hdmi, res->sclk_pixel); + clk_set_parent(res->sclk_hdmi, res->sclk_pixel);
res->regul_bulk = devm_kzalloc(dev, ARRAY_SIZE(supply) * sizeof(res->regul_bulk[0]), GFP_KERNEL);
The HDMIPHY (physical interface) is controlled by a single bit in a power controller's regiter. It was implemented as clock. It was a simple but effective hack.
This patch makes HDMI driver to control HDMIPHY via PHY interface.
Signed-off-by: Tomasz Stanislawski t.stanislaws@samsung.com --- drivers/gpu/drm/exynos/exynos_hdmi.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c index fcfa23a..e36588a 100644 --- a/drivers/gpu/drm/exynos/exynos_hdmi.c +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c @@ -34,6 +34,7 @@ #include <linux/io.h> #include <linux/of.h> #include <linux/of_gpio.h> +#include <linux/phy/phy.h>
#include <drm/exynos_drm.h>
@@ -82,7 +83,7 @@ struct hdmi_resources { struct clk *sclk_hdmi; struct clk *sclk_pixel; struct clk *sclk_hdmiphy; - struct clk *hdmiphy; + struct phy *hdmiphy; struct regulator_bulk_data *regul_bulk; int regul_count; }; @@ -1685,7 +1686,7 @@ static void hdmi_poweron(struct hdmi_context *hdata) if (regulator_bulk_enable(res->regul_count, res->regul_bulk)) DRM_DEBUG_KMS("failed to enable regulator bulk\n");
- clk_prepare_enable(res->hdmiphy); + phy_power_on(res->hdmiphy); clk_prepare_enable(res->hdmi); clk_prepare_enable(res->sclk_hdmi);
@@ -1710,7 +1711,7 @@ static void hdmi_poweroff(struct hdmi_context *hdata)
clk_disable_unprepare(res->sclk_hdmi); clk_disable_unprepare(res->hdmi); - clk_disable_unprepare(res->hdmiphy); + phy_power_off(res->hdmiphy); regulator_bulk_disable(res->regul_count, res->regul_bulk);
mutex_lock(&hdata->hdmi_mutex); @@ -1809,9 +1810,9 @@ static int hdmi_resources_init(struct hdmi_context *hdata) DRM_ERROR("failed to get clock 'sclk_hdmiphy'\n"); goto fail; } - res->hdmiphy = devm_clk_get(dev, "hdmiphy"); + res->hdmiphy = devm_phy_get(dev, "hdmiphy"); if (IS_ERR(res->hdmiphy)) { - DRM_ERROR("failed to get clock 'hdmiphy'\n"); + DRM_ERROR("failed to get phy 'hdmiphy'\n"); goto fail; }
This patch eliminates redundant checks while retrieving HPD gpio from DT during HDMI's probe().
Signed-off-by: Tomasz Stanislawski t.stanislaws@samsung.com --- drivers/gpu/drm/exynos/exynos_hdmi.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c index e36588a..5adb5c1 100644 --- a/drivers/gpu/drm/exynos/exynos_hdmi.c +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c @@ -1858,23 +1858,18 @@ static struct s5p_hdmi_platform_data *drm_hdmi_dt_parse_pdata { struct device_node *np = dev->of_node; struct s5p_hdmi_platform_data *pd; - u32 value;
pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL); if (!pd) - goto err_data; + return NULL;
- if (!of_find_property(np, "hpd-gpio", &value)) { + pd->hpd_gpio = of_get_named_gpio_flags(np, "hpd-gpio", 0, NULL); + if (pd->hpd_gpio < 0) { DRM_ERROR("no hpd gpio property found\n"); - goto err_data; + return NULL; }
- pd->hpd_gpio = of_get_named_gpio(np, "hpd-gpio", 0); - return pd; - -err_data: - return NULL; }
static struct of_device_id hdmi_match_types[] = {
This patch add proper compatibles for Mixer and HDMI chip available on exynos4210 SoCs.
Signed-off-by: Tomasz Stanislawski t.stanislaws@samsung.com --- drivers/gpu/drm/exynos/exynos_hdmi.c | 3 +++ drivers/gpu/drm/exynos/exynos_mixer.c | 3 +++ 2 files changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c index 5adb5c1..ae21fa5 100644 --- a/drivers/gpu/drm/exynos/exynos_hdmi.c +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c @@ -1874,6 +1874,9 @@ static struct s5p_hdmi_platform_data *drm_hdmi_dt_parse_pdata
static struct of_device_id hdmi_match_types[] = { { + .compatible = "samsung,exynos4210-hdmi", + .data = (void *)HDMI_TYPE13, + }, { .compatible = "samsung,exynos5-hdmi", .data = (void *)HDMI_TYPE14, }, { diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c index 63bc5f9..892afb5 100644 --- a/drivers/gpu/drm/exynos/exynos_mixer.c +++ b/drivers/gpu/drm/exynos/exynos_mixer.c @@ -1161,6 +1161,9 @@ static struct platform_device_id mixer_driver_types[] = {
static struct of_device_id mixer_match_types[] = { { + .compatible = "samsung,exynos4-mixer", + .data = &exynos4_mxr_drv_data, + }, { .compatible = "samsung,exynos5-mixer", .data = &exynos5250_mxr_drv_data, }, {
This patch adds DT nodes for I2C controller dedicated for HDMIPHY.
Signed-off-by: Tomasz Stanislawski t.stanislaws@samsung.com --- arch/arm/boot/dts/exynos4.dtsi | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
diff --git a/arch/arm/boot/dts/exynos4.dtsi b/arch/arm/boot/dts/exynos4.dtsi index caadc02..a5f6b8b 100644 --- a/arch/arm/boot/dts/exynos4.dtsi +++ b/arch/arm/boot/dts/exynos4.dtsi @@ -36,6 +36,7 @@ i2c5 = &i2c_5; i2c6 = &i2c_6; i2c7 = &i2c_7; + i2c8 = &i2c_8; csis0 = &csis_0; csis1 = &csis_1; fimc0 = &fimc_0; @@ -399,6 +400,21 @@ status = "disabled"; };
+ i2c_8: i2c@138E0000 { + #address-cells = <1>; + #size-cells = <0>; + compatible = "samsung,s3c2440-hdmiphy-i2c"; + reg = <0x138E0000 0x100>; + interrupts = <0 93 0>; + clocks = <&clock 325>; + clock-names = "i2c"; + + hdmiphy@38 { + compatible = "samsung,exynos5-hdmiphy"; + reg = <0x38>; + }; + }; + spi_0: spi@13920000 { compatible = "samsung,exynos4210-spi"; reg = <0x13920000 0x100>;
This patch adds DT nodes for HDMI related devices on SoCs from Exynos4 family.
Signed-off-by: Tomasz Stanislawski t.stanislaws@samsung.com --- arch/arm/boot/dts/exynos4.dtsi | 27 +++++++++++++++++++++++++++ arch/arm/boot/dts/exynos4210.dtsi | 4 ++++ 2 files changed, 31 insertions(+)
diff --git a/arch/arm/boot/dts/exynos4.dtsi b/arch/arm/boot/dts/exynos4.dtsi index a5f6b8b..a5df9fe 100644 --- a/arch/arm/boot/dts/exynos4.dtsi +++ b/arch/arm/boot/dts/exynos4.dtsi @@ -522,4 +522,31 @@ samsung,power-domain = <&pd_lcd0>; status = "disabled"; }; + + hdmi_phy: hdmi-phy@10020700 { + compatible = "simple-phy"; + reg = <0x10020700 0x4>; + mask = <1>; + #phy-cells = <0>; + }; + + hdmi: hdmi@12D00000 { + reg = <0x12D00000 0x70000>; + interrupts = <0 92 0>; + clock-names = "hdmi", "sclk_hdmi", "sclk_pixel", "sclk_hdmiphy"; + clocks = <&clock 271>, <&clock 136>, <&clock 139>, <&clock 21>; + phys = <&hdmi_phy>; + phy-names = "hdmiphy"; + status = "disabled"; + samsung,power-domain = <&pd_tv>; + }; + + mixer: mixer@12C10000 { + compatible = "samsung,exynos4-mixer"; + interrupts = <0 91 0>; + reg = <0x12C10000 0x2100>, <0x12c00000 0x300>; + clock-names = "mixer", "sclk_hdmi", "vp", "sclk_dac", "sclk_mixer"; + clocks = <&clock 269>, <&clock 136>, <&clock 268>, <&clock 138>, <&clock 137>; + samsung,power-domain = <&pd_tv>; + }; }; diff --git a/arch/arm/boot/dts/exynos4210.dtsi b/arch/arm/boot/dts/exynos4210.dtsi index 057d682..75fd845 100644 --- a/arch/arm/boot/dts/exynos4210.dtsi +++ b/arch/arm/boot/dts/exynos4210.dtsi @@ -155,4 +155,8 @@ samsung,lcd-wb; }; }; + + hdmi: hdmi@12D00000 { + compatible = "samsung,exynos4210-hdmi"; + }; };
This patch adds configuration of HDMI devices on Universal C210 board.
Signed-off-by: Tomasz Stanislawski t.stanislaws@samsung.com --- arch/arm/boot/dts/exynos4210-universal_c210.dts | 53 +++++++++++++++++++++++ 1 file changed, 53 insertions(+)
diff --git a/arch/arm/boot/dts/exynos4210-universal_c210.dts b/arch/arm/boot/dts/exynos4210-universal_c210.dts index 889cdad..2d94a02 100644 --- a/arch/arm/boot/dts/exynos4210-universal_c210.dts +++ b/arch/arm/boot/dts/exynos4210-universal_c210.dts @@ -349,4 +349,57 @@ compatible = "samsung,s5p6440-pwm"; status = "okay"; }; + + hdmi_en: voltage-regulator-hdmi-5v { + compatible = "regulator-fixed"; + regulator-name = "HDMI_5V"; + regulator-min-microvolt = <5000000>; + regulator-max-microvolt = <5000000>; + gpio = <&gpe0 1 0>; + enable-active-high; + }; + + i2c-ddc { + compatible = "i2c-gpio"; + gpios = <&gpe4 2 0 &gpe4 3 0>; + i2c-gpio,delay-us = <100>; + #address-cells = <1>; + #size-cells = <0>; + + pinctrl-0 = <&i2c_ddc_bus>; + pinctrl-names = "default"; + status = "okay"; + + hdmiddc@72 { + compatible = "samsung,exynos5-hdmiddc"; + reg = <0x72>; + }; + }; + + hdmi: hdmi@12D00000 { + hpd-gpio = <&gpx3 7 0>; + pinctrl-names = "default"; + pinctrl-0 = <&hdmi_hpd>; + hdmi-en-supply = <&hdmi_en>; + vdd-supply = <&ldo3_reg>; + vdd_osc-supply = <&ldo4_reg>; + vdd_pll-supply = <&ldo3_reg>; + status = "okay"; + }; +}; + +&pinctrl_1 { + hdmi_hpd: hdmi-hpd { + samsung,pins = "gpx3-7"; + samsung,pin-pud = <0>; + }; +}; + +&pinctrl_0 { + i2c_ddc_bus: i2c-ddc-bus { + samsung,pins = "gpe4-2", "gpe4-3"; + samsung,pin-function = <2>; + samsung,pin-pud = <3>; + samsung,pin-drv = <0>; + }; };
Hi Tomasz,
I have merged the re-factoring patch set from Sean Paul. Can you re-base your patch set at top of exynos-drm-next?
Thanks, Inki Dae
2013/10/21 Tomasz Stanislawski t.stanislaws@samsung.com:
This patchset adds support for HDMI at SoCs from Exynos4 family. The patches are rebased on kishon/next. Additionally, The patchset contains small fixes to PHY and CLK frameworks. I preferred to keep all the patches together for the first version of the RFC.
The interesting part might be 'propagation of clk_set_parent()'. This feature allows to remove the usage of artificial clocks in drivers. Such a situation happens for Exynos HDMI and 'mout_hdmi' where the clock is not even mentioned in some versions of SoC's documentation. Since enabling and setting rate can be propagated I think that clk_set_parent() should also be propagated. This would simplify driver's code and make it less dependant on SoC's version.
Another interesting feature refers to simple PHY driver. This driver register a PHY interface that operates by setting a special bit in platform register. This situation is very common in Exynos SoCs. The current version supports only one phy per node. The code might be modified to support multiple phys from single simple-phy provider to avoid creation of multiple nodes in DT.
All comments are welcome.
Regards, Tomasz Stanislawski
Tomasz Stanislawski (12): clk: propagate parent change up one level clk: exynos4: export sclk_hdmiphy clock clk: exynos4: enable clk_set_parent() propagation for sclk_hdmi and sclk_mixer clocks phy: Add simple-phy driver phy: use of_phy_simple_xlate for NULL xlate function Revert "drm/exynos: add mout_hdmi clock in hdmi driver to change parent" drm: exynos: hdmi: use hdmiphy as PHY drm: exynos: hdmi: simplify extracting hpd-gpio from DT drm: exynos: add compatibles for HDMI and Mixer chips and exynos4210 SoC arm: dts: exynos4: add i2c controller for HDMIPHY arm: dts: exynos4: add HDMI devices arm: dts: universal_c210: add HDMI devices
.../devicetree/bindings/clock/exynos4-clock.txt | 1 + arch/arm/boot/dts/exynos4.dtsi | 43 +++++++ arch/arm/boot/dts/exynos4210-universal_c210.dts | 53 ++++++++ arch/arm/boot/dts/exynos4210.dtsi | 4 + drivers/clk/clk.c | 6 + drivers/clk/samsung/clk-exynos4.c | 10 +- drivers/gpu/drm/exynos/exynos_hdmi.c | 41 +++---- drivers/gpu/drm/exynos/exynos_mixer.c | 3 + drivers/phy/Kconfig | 5 + drivers/phy/Makefile | 1 + drivers/phy/phy-core.c | 2 +- drivers/phy/phy-simple.c | 128 ++++++++++++++++++++ include/linux/clk-provider.h | 1 + 13 files changed, 269 insertions(+), 29 deletions(-) create mode 100644 drivers/phy/phy-simple.c
-- 1.7.9.5
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
2013/10/29 Kukjin Kim kgene.kim@samsung.com:
On 10/28/13 06:42, Inki Dae wrote:
Hi Tomasz,
I have merged the re-factoring patch set from Sean Paul. Can you re-base your patch set at top of exynos-drm-next?
Basically, RFC is not patch for merge. So Tomasz needs to re-submit after addressing comments from RFC.
There must definitely be your misunderstanding. I have never merged this RFC patch set. For review, shouldn't this RFC patch set be rebased at top of latest exynos-drm-next? :)
Thanks, Inki Dae
Thanks, Kukjin -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
dri-devel@lists.freedesktop.org
-
Inki Dae -
Kishon Vijay Abraham I -
Kukjin Kim -
Tomasz Stanislawski