* [PATCH V3 2/3] phy: Add driver for Exynos DP PHY
@ 2013-07-01 5:24 ` Jingoo Han
0 siblings, 0 replies; 9+ messages in thread
From: Jingoo Han @ 2013-07-01 5:24 UTC (permalink / raw)
To: linux-arm-kernel, linux-samsung-soc
Cc: 'Kishon Vijay Abraham I',
linux-media, 'Kukjin Kim', 'Sylwester Nawrocki',
'Felipe Balbi', 'Tomasz Figa',
devicetree-discuss, 'Inki Dae', 'Donghwa Lee',
'Kyungmin Park',
'Jean-Christophe PLAGNIOL-VILLARD',
linux-fbdev, 'Hui Wang',
Jingoo Han
Add a PHY provider driver for the Samsung Exynos SoC DP PHY.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>
Acked-by: Felipe Balbi <balbi@ti.com>
---
.../devicetree/bindings/phy/samsung-phy.txt | 8 ++
drivers/phy/Kconfig | 5 +
drivers/phy/Makefile | 1 +
drivers/phy/phy-exynos-dp-video.c | 118 ++++++++++++++++++++
4 files changed, 132 insertions(+)
create mode 100644 drivers/phy/phy-exynos-dp-video.c
diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt b/Documentation/devicetree/bindings/phy/samsung-phy.txt
index 5ff208c..3fb656a 100644
--- a/Documentation/devicetree/bindings/phy/samsung-phy.txt
+++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt
@@ -12,3 +12,11 @@ the PHY specifier identifies the PHY and its meaning is as follows:
1 - MIPI DSIM 0,
2 - MIPI CSIS 1,
3 - MIPI DSIM 1.
+
+Samsung EXYNOS SoC series DP PHY
+-------------------------------------------------
+
+Required properties:
+- compatible : should be "samsung,exynos5250-dp-video-phy";
+- reg : offset and length of the DP PHY register set;
+- #phy-cells : from the generic phy bindings, must be 0;
\ No newline at end of file
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 6f446d0..760f55a 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -19,4 +19,9 @@ config PHY_EXYNOS_MIPI_VIDEO
help
Support for MIPI CSI-2 and MIPI DSI DPHY found on Samsung
S5P and EXYNOS SoCs.
+
+config PHY_EXYNOS_DP_VIDEO
+ tristate "EXYNOS SoC series DP PHY driver"
+ help
+ Support for DP PHY found on Samsung EXYNOS SoCs.
endif
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index 71d8841..0fd1340 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -4,3 +4,4 @@
obj-$(CONFIG_GENERIC_PHY) += phy-core.o
obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO) += phy-exynos-mipi-video.o
+obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO) += phy-exynos-dp-video.o
diff --git a/drivers/phy/phy-exynos-dp-video.c b/drivers/phy/phy-exynos-dp-video.c
new file mode 100644
index 0000000..75e1d11
--- /dev/null
+++ b/drivers/phy/phy-exynos-dp-video.c
@@ -0,0 +1,118 @@
+/*
+ * Samsung EXYNOS SoC series DP PHY driver
+ *
+ * Copyright (C) 2013 Samsung Electronics Co., Ltd.
+ * Author: Jingoo Han <jg1.han@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>
+
+/* DPTX_PHY_CONTROL register */
+#define EXYNOS_DPTX_PHY_ENABLE (1 << 0)
+
+struct exynos_dp_video_phy {
+ void __iomem *regs;
+};
+
+static int __set_phy_state(struct exynos_dp_video_phy *state, unsigned int on)
+{
+ void __iomem *addr;
+ u32 reg;
+
+ addr = state->regs;
+
+ reg = readl(addr);
+ if (on)
+ reg |= EXYNOS_DPTX_PHY_ENABLE;
+ else
+ reg &= ~EXYNOS_DPTX_PHY_ENABLE;
+ writel(reg, addr);
+
+ return 0;
+}
+
+static int exynos_dp_video_phy_power_on(struct phy *phy)
+{
+ struct exynos_dp_video_phy *state = phy_get_drvdata(phy);
+
+ return __set_phy_state(state, 1);
+}
+
+static int exynos_dp_video_phy_power_off(struct phy *phy)
+{
+ struct exynos_dp_video_phy *state = phy_get_drvdata(phy);
+
+ return __set_phy_state(state, 0);
+}
+
+static struct phy_ops exynos_dp_video_phy_ops = {
+ .power_on = exynos_dp_video_phy_power_on,
+ .power_off = exynos_dp_video_phy_power_off,
+ .owner = THIS_MODULE,
+};
+
+static int exynos_dp_video_phy_probe(struct platform_device *pdev)
+{
+ struct exynos_dp_video_phy *state;
+ struct device *dev = &pdev->dev;
+ struct resource *res;
+ struct phy_provider *phy_provider;
+ struct phy *phy;
+
+ state = devm_kzalloc(dev, sizeof(*state), GFP_KERNEL);
+ if (!state)
+ return -ENOMEM;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+
+ state->regs = devm_ioremap_resource(dev, res);
+ if (IS_ERR(state->regs))
+ return PTR_ERR(state->regs);
+
+ dev_set_drvdata(dev, state);
+
+ phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+ if (IS_ERR(phy_provider))
+ return PTR_ERR(phy_provider);
+
+ phy = devm_phy_create(dev, 0, &exynos_dp_video_phy_ops, "dp");
+ if (IS_ERR(phy)) {
+ dev_err(dev, "failed to create DP PHY\n");
+ return PTR_ERR(phy);
+ }
+ phy_set_drvdata(phy, state);
+
+ return 0;
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id exynos_dp_video_phy_of_match[] = {
+ { .compatible = "samsung,exynos5250-dp-video-phy" },
+ { },
+};
+MODULE_DEVICE_TABLE(of, exynos_dp_video_phy_of_match);
+#endif
+
+static struct platform_driver exynos_dp_video_phy_driver = {
+ .probe = exynos_dp_video_phy_probe,
+ .driver = {
+ .name = "exynos-dp-video-phy",
+ .owner = THIS_MODULE,
+ .of_match_table = exynos_dp_video_phy_of_match,
+ }
+};
+module_platform_driver(exynos_dp_video_phy_driver);
+
+MODULE_AUTHOR("Jingoo Han <jg1.han@samsung.com>");
+MODULE_DESCRIPTION("Samsung EXYNOS SoC DP PHY driver");
+MODULE_LICENSE("GPL v2");
--
1.7.10.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH V3 2/3] phy: Add driver for Exynos DP PHY
@ 2013-07-01 5:24 ` Jingoo Han
0 siblings, 0 replies; 9+ messages in thread
From: Jingoo Han @ 2013-07-01 5:24 UTC (permalink / raw)
To: linux-arm-kernel
Add a PHY provider driver for the Samsung Exynos SoC DP PHY.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>
Acked-by: Felipe Balbi <balbi@ti.com>
---
.../devicetree/bindings/phy/samsung-phy.txt | 8 ++
drivers/phy/Kconfig | 5 +
drivers/phy/Makefile | 1 +
drivers/phy/phy-exynos-dp-video.c | 118 ++++++++++++++++++++
4 files changed, 132 insertions(+)
create mode 100644 drivers/phy/phy-exynos-dp-video.c
diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt b/Documentation/devicetree/bindings/phy/samsung-phy.txt
index 5ff208c..3fb656a 100644
--- a/Documentation/devicetree/bindings/phy/samsung-phy.txt
+++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt
@@ -12,3 +12,11 @@ the PHY specifier identifies the PHY and its meaning is as follows:
1 - MIPI DSIM 0,
2 - MIPI CSIS 1,
3 - MIPI DSIM 1.
+
+Samsung EXYNOS SoC series DP PHY
+-------------------------------------------------
+
+Required properties:
+- compatible : should be "samsung,exynos5250-dp-video-phy";
+- reg : offset and length of the DP PHY register set;
+- #phy-cells : from the generic phy bindings, must be 0;
\ No newline at end of file
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 6f446d0..760f55a 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -19,4 +19,9 @@ config PHY_EXYNOS_MIPI_VIDEO
help
Support for MIPI CSI-2 and MIPI DSI DPHY found on Samsung
S5P and EXYNOS SoCs.
+
+config PHY_EXYNOS_DP_VIDEO
+ tristate "EXYNOS SoC series DP PHY driver"
+ help
+ Support for DP PHY found on Samsung EXYNOS SoCs.
endif
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index 71d8841..0fd1340 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -4,3 +4,4 @@
obj-$(CONFIG_GENERIC_PHY) += phy-core.o
obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO) += phy-exynos-mipi-video.o
+obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO) += phy-exynos-dp-video.o
diff --git a/drivers/phy/phy-exynos-dp-video.c b/drivers/phy/phy-exynos-dp-video.c
new file mode 100644
index 0000000..75e1d11
--- /dev/null
+++ b/drivers/phy/phy-exynos-dp-video.c
@@ -0,0 +1,118 @@
+/*
+ * Samsung EXYNOS SoC series DP PHY driver
+ *
+ * Copyright (C) 2013 Samsung Electronics Co., Ltd.
+ * Author: Jingoo Han <jg1.han@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>
+
+/* DPTX_PHY_CONTROL register */
+#define EXYNOS_DPTX_PHY_ENABLE (1 << 0)
+
+struct exynos_dp_video_phy {
+ void __iomem *regs;
+};
+
+static int __set_phy_state(struct exynos_dp_video_phy *state, unsigned int on)
+{
+ void __iomem *addr;
+ u32 reg;
+
+ addr = state->regs;
+
+ reg = readl(addr);
+ if (on)
+ reg |= EXYNOS_DPTX_PHY_ENABLE;
+ else
+ reg &= ~EXYNOS_DPTX_PHY_ENABLE;
+ writel(reg, addr);
+
+ return 0;
+}
+
+static int exynos_dp_video_phy_power_on(struct phy *phy)
+{
+ struct exynos_dp_video_phy *state = phy_get_drvdata(phy);
+
+ return __set_phy_state(state, 1);
+}
+
+static int exynos_dp_video_phy_power_off(struct phy *phy)
+{
+ struct exynos_dp_video_phy *state = phy_get_drvdata(phy);
+
+ return __set_phy_state(state, 0);
+}
+
+static struct phy_ops exynos_dp_video_phy_ops = {
+ .power_on = exynos_dp_video_phy_power_on,
+ .power_off = exynos_dp_video_phy_power_off,
+ .owner = THIS_MODULE,
+};
+
+static int exynos_dp_video_phy_probe(struct platform_device *pdev)
+{
+ struct exynos_dp_video_phy *state;
+ struct device *dev = &pdev->dev;
+ struct resource *res;
+ struct phy_provider *phy_provider;
+ struct phy *phy;
+
+ state = devm_kzalloc(dev, sizeof(*state), GFP_KERNEL);
+ if (!state)
+ return -ENOMEM;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+
+ state->regs = devm_ioremap_resource(dev, res);
+ if (IS_ERR(state->regs))
+ return PTR_ERR(state->regs);
+
+ dev_set_drvdata(dev, state);
+
+ phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+ if (IS_ERR(phy_provider))
+ return PTR_ERR(phy_provider);
+
+ phy = devm_phy_create(dev, 0, &exynos_dp_video_phy_ops, "dp");
+ if (IS_ERR(phy)) {
+ dev_err(dev, "failed to create DP PHY\n");
+ return PTR_ERR(phy);
+ }
+ phy_set_drvdata(phy, state);
+
+ return 0;
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id exynos_dp_video_phy_of_match[] = {
+ { .compatible = "samsung,exynos5250-dp-video-phy" },
+ { },
+};
+MODULE_DEVICE_TABLE(of, exynos_dp_video_phy_of_match);
+#endif
+
+static struct platform_driver exynos_dp_video_phy_driver = {
+ .probe = exynos_dp_video_phy_probe,
+ .driver = {
+ .name = "exynos-dp-video-phy",
+ .owner = THIS_MODULE,
+ .of_match_table = exynos_dp_video_phy_of_match,
+ }
+};
+module_platform_driver(exynos_dp_video_phy_driver);
+
+MODULE_AUTHOR("Jingoo Han <jg1.han@samsung.com>");
+MODULE_DESCRIPTION("Samsung EXYNOS SoC DP PHY driver");
+MODULE_LICENSE("GPL v2");
--
1.7.10.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH V3 2/3] phy: Add driver for Exynos DP PHY
@ 2013-07-01 5:24 ` Jingoo Han
0 siblings, 0 replies; 9+ messages in thread
From: Jingoo Han @ 2013-07-01 5:24 UTC (permalink / raw)
To: linux-arm-kernel
Add a PHY provider driver for the Samsung Exynos SoC DP PHY.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>
Acked-by: Felipe Balbi <balbi@ti.com>
---
.../devicetree/bindings/phy/samsung-phy.txt | 8 ++
drivers/phy/Kconfig | 5 +
drivers/phy/Makefile | 1 +
drivers/phy/phy-exynos-dp-video.c | 118 ++++++++++++++++++++
4 files changed, 132 insertions(+)
create mode 100644 drivers/phy/phy-exynos-dp-video.c
diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt b/Documentation/devicetree/bindings/phy/samsung-phy.txt
index 5ff208c..3fb656a 100644
--- a/Documentation/devicetree/bindings/phy/samsung-phy.txt
+++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt
@@ -12,3 +12,11 @@ the PHY specifier identifies the PHY and its meaning is as follows:
1 - MIPI DSIM 0,
2 - MIPI CSIS 1,
3 - MIPI DSIM 1.
+
+Samsung EXYNOS SoC series DP PHY
+-------------------------------------------------
+
+Required properties:
+- compatible : should be "samsung,exynos5250-dp-video-phy";
+- reg : offset and length of the DP PHY register set;
+- #phy-cells : from the generic phy bindings, must be 0;
\ No newline at end of file
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 6f446d0..760f55a 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -19,4 +19,9 @@ config PHY_EXYNOS_MIPI_VIDEO
help
Support for MIPI CSI-2 and MIPI DSI DPHY found on Samsung
S5P and EXYNOS SoCs.
+
+config PHY_EXYNOS_DP_VIDEO
+ tristate "EXYNOS SoC series DP PHY driver"
+ help
+ Support for DP PHY found on Samsung EXYNOS SoCs.
endif
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index 71d8841..0fd1340 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -4,3 +4,4 @@
obj-$(CONFIG_GENERIC_PHY) += phy-core.o
obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO) += phy-exynos-mipi-video.o
+obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO) += phy-exynos-dp-video.o
diff --git a/drivers/phy/phy-exynos-dp-video.c b/drivers/phy/phy-exynos-dp-video.c
new file mode 100644
index 0000000..75e1d11
--- /dev/null
+++ b/drivers/phy/phy-exynos-dp-video.c
@@ -0,0 +1,118 @@
+/*
+ * Samsung EXYNOS SoC series DP PHY driver
+ *
+ * Copyright (C) 2013 Samsung Electronics Co., Ltd.
+ * Author: Jingoo Han <jg1.han@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>
+
+/* DPTX_PHY_CONTROL register */
+#define EXYNOS_DPTX_PHY_ENABLE (1 << 0)
+
+struct exynos_dp_video_phy {
+ void __iomem *regs;
+};
+
+static int __set_phy_state(struct exynos_dp_video_phy *state, unsigned int on)
+{
+ void __iomem *addr;
+ u32 reg;
+
+ addr = state->regs;
+
+ reg = readl(addr);
+ if (on)
+ reg |= EXYNOS_DPTX_PHY_ENABLE;
+ else
+ reg &= ~EXYNOS_DPTX_PHY_ENABLE;
+ writel(reg, addr);
+
+ return 0;
+}
+
+static int exynos_dp_video_phy_power_on(struct phy *phy)
+{
+ struct exynos_dp_video_phy *state = phy_get_drvdata(phy);
+
+ return __set_phy_state(state, 1);
+}
+
+static int exynos_dp_video_phy_power_off(struct phy *phy)
+{
+ struct exynos_dp_video_phy *state = phy_get_drvdata(phy);
+
+ return __set_phy_state(state, 0);
+}
+
+static struct phy_ops exynos_dp_video_phy_ops = {
+ .power_on = exynos_dp_video_phy_power_on,
+ .power_off = exynos_dp_video_phy_power_off,
+ .owner = THIS_MODULE,
+};
+
+static int exynos_dp_video_phy_probe(struct platform_device *pdev)
+{
+ struct exynos_dp_video_phy *state;
+ struct device *dev = &pdev->dev;
+ struct resource *res;
+ struct phy_provider *phy_provider;
+ struct phy *phy;
+
+ state = devm_kzalloc(dev, sizeof(*state), GFP_KERNEL);
+ if (!state)
+ return -ENOMEM;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+
+ state->regs = devm_ioremap_resource(dev, res);
+ if (IS_ERR(state->regs))
+ return PTR_ERR(state->regs);
+
+ dev_set_drvdata(dev, state);
+
+ phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+ if (IS_ERR(phy_provider))
+ return PTR_ERR(phy_provider);
+
+ phy = devm_phy_create(dev, 0, &exynos_dp_video_phy_ops, "dp");
+ if (IS_ERR(phy)) {
+ dev_err(dev, "failed to create DP PHY\n");
+ return PTR_ERR(phy);
+ }
+ phy_set_drvdata(phy, state);
+
+ return 0;
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id exynos_dp_video_phy_of_match[] = {
+ { .compatible = "samsung,exynos5250-dp-video-phy" },
+ { },
+};
+MODULE_DEVICE_TABLE(of, exynos_dp_video_phy_of_match);
+#endif
+
+static struct platform_driver exynos_dp_video_phy_driver = {
+ .probe = exynos_dp_video_phy_probe,
+ .driver = {
+ .name = "exynos-dp-video-phy",
+ .owner = THIS_MODULE,
+ .of_match_table = exynos_dp_video_phy_of_match,
+ }
+};
+module_platform_driver(exynos_dp_video_phy_driver);
+
+MODULE_AUTHOR("Jingoo Han <jg1.han@samsung.com>");
+MODULE_DESCRIPTION("Samsung EXYNOS SoC DP PHY driver");
+MODULE_LICENSE("GPL v2");
--
1.7.10.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH V3 2/3] phy: Add driver for Exynos DP PHY
2013-07-01 5:24 ` Jingoo Han
(?)
@ 2013-07-01 19:48 ` Sylwester Nawrocki
-1 siblings, 0 replies; 9+ messages in thread
From: Sylwester Nawrocki @ 2013-07-01 19:48 UTC (permalink / raw)
To: Jingoo Han
Cc: linux-arm-kernel, linux-samsung-soc,
'Kishon Vijay Abraham I',
linux-media, 'Kukjin Kim', 'Sylwester Nawrocki',
'Felipe Balbi', 'Tomasz Figa',
devicetree-discuss, 'Inki Dae', 'Donghwa Lee',
'Kyungmin Park',
'Jean-Christophe PLAGNIOL-VILLARD',
linux-fbdev, 'Hui Wang'
On 07/01/2013 07:24 AM, Jingoo Han wrote:
> Add a PHY provider driver for the Samsung Exynos SoC DP PHY.
>
> Signed-off-by: Jingoo Han<jg1.han@samsung.com>
> Cc: Sylwester Nawrocki<s.nawrocki@samsung.com>
> Acked-by: Felipe Balbi<balbi@ti.com>
> ---
> .../devicetree/bindings/phy/samsung-phy.txt | 8 ++
> drivers/phy/Kconfig | 5 +
> drivers/phy/Makefile | 1 +
> drivers/phy/phy-exynos-dp-video.c | 118 ++++++++++++++++++++
> 4 files changed, 132 insertions(+)
> create mode 100644 drivers/phy/phy-exynos-dp-video.c
>
> diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt b/Documentation/devicetree/bindings/phy/samsung-phy.txt
> index 5ff208c..3fb656a 100644
> --- a/Documentation/devicetree/bindings/phy/samsung-phy.txt
> +++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt
> @@ -12,3 +12,11 @@ the PHY specifier identifies the PHY and its meaning is as follows:
> 1 - MIPI DSIM 0,
> 2 - MIPI CSIS 1,
> 3 - MIPI DSIM 1.
> +
> +Samsung EXYNOS SoC series DP PHY
I would make it "Samsung EXYNOS SoC series Display Port PHY"
> +-------------------------------------------------
> +
> +Required properties:
> +- compatible : should be "samsung,exynos5250-dp-video-phy";
> +- reg : offset and length of the DP PHY register set;
> +- #phy-cells : from the generic phy bindings, must be 0;
s/phy/PHY ?
> \ No newline at end of file
Missing new line character.
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index 6f446d0..760f55a 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -19,4 +19,9 @@ config PHY_EXYNOS_MIPI_VIDEO
> help
> Support for MIPI CSI-2 and MIPI DSI DPHY found on Samsung
> S5P and EXYNOS SoCs.
> +
> +config PHY_EXYNOS_DP_VIDEO
> + tristate "EXYNOS SoC series DP PHY driver"
"EXYNOS SoC series Display Port PHY driver" ?
> + help
> + Support for DP PHY found on Samsung EXYNOS SoCs.
s/DP/Display Port ?
> endif
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index 71d8841..0fd1340 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -4,3 +4,4 @@
>
> obj-$(CONFIG_GENERIC_PHY) += phy-core.o
> obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO) += phy-exynos-mipi-video.o
> +obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO) += phy-exynos-dp-video.o
> diff --git a/drivers/phy/phy-exynos-dp-video.c b/drivers/phy/phy-exynos-dp-video.c
> new file mode 100644
> index 0000000..75e1d11
> --- /dev/null
> +++ b/drivers/phy/phy-exynos-dp-video.c
> @@ -0,0 +1,118 @@
> +/*
> + * Samsung EXYNOS SoC series DP PHY driver
s/DP/Display Port ?
> + *
> + * Copyright (C) 2013 Samsung Electronics Co., Ltd.
> + * Author: Jingoo Han<jg1.han@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>
> +
> +/* DPTX_PHY_CONTROL register */
> +#define EXYNOS_DPTX_PHY_ENABLE (1<< 0)
> +
> +struct exynos_dp_video_phy {
> + void __iomem *regs;
> +};
> +
> +static int __set_phy_state(struct exynos_dp_video_phy *state, unsigned int on)
> +{
> + void __iomem *addr;
How about just dropping this local variable ?
> + u32 reg;
> +
> + addr = state->regs;
> +
> + reg = readl(addr);
> + if (on)
> + reg |= EXYNOS_DPTX_PHY_ENABLE;
> + else
> + reg&= ~EXYNOS_DPTX_PHY_ENABLE;
> + writel(reg, addr);
> +
> + return 0;
> +}
> +
> +static int exynos_dp_video_phy_power_on(struct phy *phy)
> +{
> + struct exynos_dp_video_phy *state = phy_get_drvdata(phy);
> +
> + return __set_phy_state(state, 1);
> +}
> +
> +static int exynos_dp_video_phy_power_off(struct phy *phy)
> +{
> + struct exynos_dp_video_phy *state = phy_get_drvdata(phy);
> +
> + return __set_phy_state(state, 0);
> +}
> +
> +static struct phy_ops exynos_dp_video_phy_ops = {
> + .power_on = exynos_dp_video_phy_power_on,
> + .power_off = exynos_dp_video_phy_power_off,
> + .owner = THIS_MODULE,
> +};
> +
> +static int exynos_dp_video_phy_probe(struct platform_device *pdev)
> +{
> + struct exynos_dp_video_phy *state;
> + struct device *dev =&pdev->dev;
> + struct resource *res;
> + struct phy_provider *phy_provider;
> + struct phy *phy;
> +
> + state = devm_kzalloc(dev, sizeof(*state), GFP_KERNEL);
> + if (!state)
> + return -ENOMEM;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +
> + state->regs = devm_ioremap_resource(dev, res);
> + if (IS_ERR(state->regs))
> + return PTR_ERR(state->regs);
> +
> + dev_set_drvdata(dev, state);
I can't see any corresponding dev_get_drvdata(), it should be safe
to remove this assignment.
> + phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> + if (IS_ERR(phy_provider))
> + return PTR_ERR(phy_provider);
> +
> + phy = devm_phy_create(dev, 0,&exynos_dp_video_phy_ops, "dp");
The label could be NULL, since there is no need to support non-dt config.
> + if (IS_ERR(phy)) {
> + dev_err(dev, "failed to create DP PHY\n");
> + return PTR_ERR(phy);
> + }
> + phy_set_drvdata(phy, state);
> +
> + return 0;
> +}
> +
> +#ifdef CONFIG_OF
You don't use of_match_ptr() macro to assign of_match_table to the driver
structure below. This means build with CONFIG_OF disabled is broken now.
I would just remove that #ifdef and would make the driver depends on
CONFIG_OF.
> +static const struct of_device_id exynos_dp_video_phy_of_match[] = {
> + { .compatible = "samsung,exynos5250-dp-video-phy" },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, exynos_dp_video_phy_of_match);
> +#endif
> +
> +static struct platform_driver exynos_dp_video_phy_driver = {
> + .probe = exynos_dp_video_phy_probe,
> + .driver = {
> + .name = "exynos-dp-video-phy",
> + .owner = THIS_MODULE,
> + .of_match_table = exynos_dp_video_phy_of_match,
> + }
> +};
> +module_platform_driver(exynos_dp_video_phy_driver);
> +
> +MODULE_AUTHOR("Jingoo Han<jg1.han@samsung.com>");
> +MODULE_DESCRIPTION("Samsung EXYNOS SoC DP PHY driver");
> +MODULE_LICENSE("GPL v2");
Thanks,
Sylwester
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH V3 2/3] phy: Add driver for Exynos DP PHY
@ 2013-07-01 19:48 ` Sylwester Nawrocki
0 siblings, 0 replies; 9+ messages in thread
From: Sylwester Nawrocki @ 2013-07-01 19:48 UTC (permalink / raw)
To: linux-arm-kernel
On 07/01/2013 07:24 AM, Jingoo Han wrote:
> Add a PHY provider driver for the Samsung Exynos SoC DP PHY.
>
> Signed-off-by: Jingoo Han<jg1.han@samsung.com>
> Cc: Sylwester Nawrocki<s.nawrocki@samsung.com>
> Acked-by: Felipe Balbi<balbi@ti.com>
> ---
> .../devicetree/bindings/phy/samsung-phy.txt | 8 ++
> drivers/phy/Kconfig | 5 +
> drivers/phy/Makefile | 1 +
> drivers/phy/phy-exynos-dp-video.c | 118 ++++++++++++++++++++
> 4 files changed, 132 insertions(+)
> create mode 100644 drivers/phy/phy-exynos-dp-video.c
>
> diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt b/Documentation/devicetree/bindings/phy/samsung-phy.txt
> index 5ff208c..3fb656a 100644
> --- a/Documentation/devicetree/bindings/phy/samsung-phy.txt
> +++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt
> @@ -12,3 +12,11 @@ the PHY specifier identifies the PHY and its meaning is as follows:
> 1 - MIPI DSIM 0,
> 2 - MIPI CSIS 1,
> 3 - MIPI DSIM 1.
> +
> +Samsung EXYNOS SoC series DP PHY
I would make it "Samsung EXYNOS SoC series Display Port PHY"
> +-------------------------------------------------
> +
> +Required properties:
> +- compatible : should be "samsung,exynos5250-dp-video-phy";
> +- reg : offset and length of the DP PHY register set;
> +- #phy-cells : from the generic phy bindings, must be 0;
s/phy/PHY ?
> \ No newline at end of file
Missing new line character.
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index 6f446d0..760f55a 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -19,4 +19,9 @@ config PHY_EXYNOS_MIPI_VIDEO
> help
> Support for MIPI CSI-2 and MIPI DSI DPHY found on Samsung
> S5P and EXYNOS SoCs.
> +
> +config PHY_EXYNOS_DP_VIDEO
> + tristate "EXYNOS SoC series DP PHY driver"
"EXYNOS SoC series Display Port PHY driver" ?
> + help
> + Support for DP PHY found on Samsung EXYNOS SoCs.
s/DP/Display Port ?
> endif
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index 71d8841..0fd1340 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -4,3 +4,4 @@
>
> obj-$(CONFIG_GENERIC_PHY) += phy-core.o
> obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO) += phy-exynos-mipi-video.o
> +obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO) += phy-exynos-dp-video.o
> diff --git a/drivers/phy/phy-exynos-dp-video.c b/drivers/phy/phy-exynos-dp-video.c
> new file mode 100644
> index 0000000..75e1d11
> --- /dev/null
> +++ b/drivers/phy/phy-exynos-dp-video.c
> @@ -0,0 +1,118 @@
> +/*
> + * Samsung EXYNOS SoC series DP PHY driver
s/DP/Display Port ?
> + *
> + * Copyright (C) 2013 Samsung Electronics Co., Ltd.
> + * Author: Jingoo Han<jg1.han@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>
> +
> +/* DPTX_PHY_CONTROL register */
> +#define EXYNOS_DPTX_PHY_ENABLE (1<< 0)
> +
> +struct exynos_dp_video_phy {
> + void __iomem *regs;
> +};
> +
> +static int __set_phy_state(struct exynos_dp_video_phy *state, unsigned int on)
> +{
> + void __iomem *addr;
How about just dropping this local variable ?
> + u32 reg;
> +
> + addr = state->regs;
> +
> + reg = readl(addr);
> + if (on)
> + reg |= EXYNOS_DPTX_PHY_ENABLE;
> + else
> + reg&= ~EXYNOS_DPTX_PHY_ENABLE;
> + writel(reg, addr);
> +
> + return 0;
> +}
> +
> +static int exynos_dp_video_phy_power_on(struct phy *phy)
> +{
> + struct exynos_dp_video_phy *state = phy_get_drvdata(phy);
> +
> + return __set_phy_state(state, 1);
> +}
> +
> +static int exynos_dp_video_phy_power_off(struct phy *phy)
> +{
> + struct exynos_dp_video_phy *state = phy_get_drvdata(phy);
> +
> + return __set_phy_state(state, 0);
> +}
> +
> +static struct phy_ops exynos_dp_video_phy_ops = {
> + .power_on = exynos_dp_video_phy_power_on,
> + .power_off = exynos_dp_video_phy_power_off,
> + .owner = THIS_MODULE,
> +};
> +
> +static int exynos_dp_video_phy_probe(struct platform_device *pdev)
> +{
> + struct exynos_dp_video_phy *state;
> + struct device *dev =&pdev->dev;
> + struct resource *res;
> + struct phy_provider *phy_provider;
> + struct phy *phy;
> +
> + state = devm_kzalloc(dev, sizeof(*state), GFP_KERNEL);
> + if (!state)
> + return -ENOMEM;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +
> + state->regs = devm_ioremap_resource(dev, res);
> + if (IS_ERR(state->regs))
> + return PTR_ERR(state->regs);
> +
> + dev_set_drvdata(dev, state);
I can't see any corresponding dev_get_drvdata(), it should be safe
to remove this assignment.
> + phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> + if (IS_ERR(phy_provider))
> + return PTR_ERR(phy_provider);
> +
> + phy = devm_phy_create(dev, 0,&exynos_dp_video_phy_ops, "dp");
The label could be NULL, since there is no need to support non-dt config.
> + if (IS_ERR(phy)) {
> + dev_err(dev, "failed to create DP PHY\n");
> + return PTR_ERR(phy);
> + }
> + phy_set_drvdata(phy, state);
> +
> + return 0;
> +}
> +
> +#ifdef CONFIG_OF
You don't use of_match_ptr() macro to assign of_match_table to the driver
structure below. This means build with CONFIG_OF disabled is broken now.
I would just remove that #ifdef and would make the driver depends on
CONFIG_OF.
> +static const struct of_device_id exynos_dp_video_phy_of_match[] = {
> + { .compatible = "samsung,exynos5250-dp-video-phy" },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, exynos_dp_video_phy_of_match);
> +#endif
> +
> +static struct platform_driver exynos_dp_video_phy_driver = {
> + .probe = exynos_dp_video_phy_probe,
> + .driver = {
> + .name = "exynos-dp-video-phy",
> + .owner = THIS_MODULE,
> + .of_match_table = exynos_dp_video_phy_of_match,
> + }
> +};
> +module_platform_driver(exynos_dp_video_phy_driver);
> +
> +MODULE_AUTHOR("Jingoo Han<jg1.han@samsung.com>");
> +MODULE_DESCRIPTION("Samsung EXYNOS SoC DP PHY driver");
> +MODULE_LICENSE("GPL v2");
Thanks,
Sylwester
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH V3 2/3] phy: Add driver for Exynos DP PHY
@ 2013-07-01 19:48 ` Sylwester Nawrocki
0 siblings, 0 replies; 9+ messages in thread
From: Sylwester Nawrocki @ 2013-07-01 19:48 UTC (permalink / raw)
To: linux-arm-kernel
On 07/01/2013 07:24 AM, Jingoo Han wrote:
> Add a PHY provider driver for the Samsung Exynos SoC DP PHY.
>
> Signed-off-by: Jingoo Han<jg1.han@samsung.com>
> Cc: Sylwester Nawrocki<s.nawrocki@samsung.com>
> Acked-by: Felipe Balbi<balbi@ti.com>
> ---
> .../devicetree/bindings/phy/samsung-phy.txt | 8 ++
> drivers/phy/Kconfig | 5 +
> drivers/phy/Makefile | 1 +
> drivers/phy/phy-exynos-dp-video.c | 118 ++++++++++++++++++++
> 4 files changed, 132 insertions(+)
> create mode 100644 drivers/phy/phy-exynos-dp-video.c
>
> diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt b/Documentation/devicetree/bindings/phy/samsung-phy.txt
> index 5ff208c..3fb656a 100644
> --- a/Documentation/devicetree/bindings/phy/samsung-phy.txt
> +++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt
> @@ -12,3 +12,11 @@ the PHY specifier identifies the PHY and its meaning is as follows:
> 1 - MIPI DSIM 0,
> 2 - MIPI CSIS 1,
> 3 - MIPI DSIM 1.
> +
> +Samsung EXYNOS SoC series DP PHY
I would make it "Samsung EXYNOS SoC series Display Port PHY"
> +-------------------------------------------------
> +
> +Required properties:
> +- compatible : should be "samsung,exynos5250-dp-video-phy";
> +- reg : offset and length of the DP PHY register set;
> +- #phy-cells : from the generic phy bindings, must be 0;
s/phy/PHY ?
> \ No newline at end of file
Missing new line character.
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index 6f446d0..760f55a 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -19,4 +19,9 @@ config PHY_EXYNOS_MIPI_VIDEO
> help
> Support for MIPI CSI-2 and MIPI DSI DPHY found on Samsung
> S5P and EXYNOS SoCs.
> +
> +config PHY_EXYNOS_DP_VIDEO
> + tristate "EXYNOS SoC series DP PHY driver"
"EXYNOS SoC series Display Port PHY driver" ?
> + help
> + Support for DP PHY found on Samsung EXYNOS SoCs.
s/DP/Display Port ?
> endif
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index 71d8841..0fd1340 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -4,3 +4,4 @@
>
> obj-$(CONFIG_GENERIC_PHY) += phy-core.o
> obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO) += phy-exynos-mipi-video.o
> +obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO) += phy-exynos-dp-video.o
> diff --git a/drivers/phy/phy-exynos-dp-video.c b/drivers/phy/phy-exynos-dp-video.c
> new file mode 100644
> index 0000000..75e1d11
> --- /dev/null
> +++ b/drivers/phy/phy-exynos-dp-video.c
> @@ -0,0 +1,118 @@
> +/*
> + * Samsung EXYNOS SoC series DP PHY driver
s/DP/Display Port ?
> + *
> + * Copyright (C) 2013 Samsung Electronics Co., Ltd.
> + * Author: Jingoo Han<jg1.han@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>
> +
> +/* DPTX_PHY_CONTROL register */
> +#define EXYNOS_DPTX_PHY_ENABLE (1<< 0)
> +
> +struct exynos_dp_video_phy {
> + void __iomem *regs;
> +};
> +
> +static int __set_phy_state(struct exynos_dp_video_phy *state, unsigned int on)
> +{
> + void __iomem *addr;
How about just dropping this local variable ?
> + u32 reg;
> +
> + addr = state->regs;
> +
> + reg = readl(addr);
> + if (on)
> + reg |= EXYNOS_DPTX_PHY_ENABLE;
> + else
> + reg&= ~EXYNOS_DPTX_PHY_ENABLE;
> + writel(reg, addr);
> +
> + return 0;
> +}
> +
> +static int exynos_dp_video_phy_power_on(struct phy *phy)
> +{
> + struct exynos_dp_video_phy *state = phy_get_drvdata(phy);
> +
> + return __set_phy_state(state, 1);
> +}
> +
> +static int exynos_dp_video_phy_power_off(struct phy *phy)
> +{
> + struct exynos_dp_video_phy *state = phy_get_drvdata(phy);
> +
> + return __set_phy_state(state, 0);
> +}
> +
> +static struct phy_ops exynos_dp_video_phy_ops = {
> + .power_on = exynos_dp_video_phy_power_on,
> + .power_off = exynos_dp_video_phy_power_off,
> + .owner = THIS_MODULE,
> +};
> +
> +static int exynos_dp_video_phy_probe(struct platform_device *pdev)
> +{
> + struct exynos_dp_video_phy *state;
> + struct device *dev =&pdev->dev;
> + struct resource *res;
> + struct phy_provider *phy_provider;
> + struct phy *phy;
> +
> + state = devm_kzalloc(dev, sizeof(*state), GFP_KERNEL);
> + if (!state)
> + return -ENOMEM;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +
> + state->regs = devm_ioremap_resource(dev, res);
> + if (IS_ERR(state->regs))
> + return PTR_ERR(state->regs);
> +
> + dev_set_drvdata(dev, state);
I can't see any corresponding dev_get_drvdata(), it should be safe
to remove this assignment.
> + phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> + if (IS_ERR(phy_provider))
> + return PTR_ERR(phy_provider);
> +
> + phy = devm_phy_create(dev, 0,&exynos_dp_video_phy_ops, "dp");
The label could be NULL, since there is no need to support non-dt config.
> + if (IS_ERR(phy)) {
> + dev_err(dev, "failed to create DP PHY\n");
> + return PTR_ERR(phy);
> + }
> + phy_set_drvdata(phy, state);
> +
> + return 0;
> +}
> +
> +#ifdef CONFIG_OF
You don't use of_match_ptr() macro to assign of_match_table to the driver
structure below. This means build with CONFIG_OF disabled is broken now.
I would just remove that #ifdef and would make the driver depends on
CONFIG_OF.
> +static const struct of_device_id exynos_dp_video_phy_of_match[] = {
> + { .compatible = "samsung,exynos5250-dp-video-phy" },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, exynos_dp_video_phy_of_match);
> +#endif
> +
> +static struct platform_driver exynos_dp_video_phy_driver = {
> + .probe = exynos_dp_video_phy_probe,
> + .driver = {
> + .name = "exynos-dp-video-phy",
> + .owner = THIS_MODULE,
> + .of_match_table = exynos_dp_video_phy_of_match,
> + }
> +};
> +module_platform_driver(exynos_dp_video_phy_driver);
> +
> +MODULE_AUTHOR("Jingoo Han<jg1.han@samsung.com>");
> +MODULE_DESCRIPTION("Samsung EXYNOS SoC DP PHY driver");
> +MODULE_LICENSE("GPL v2");
Thanks,
Sylwester
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH V3 2/3] phy: Add driver for Exynos DP PHY
2013-07-01 19:48 ` Sylwester Nawrocki
(?)
@ 2013-07-02 6:58 ` Jingoo Han
-1 siblings, 0 replies; 9+ messages in thread
From: Jingoo Han @ 2013-07-02 6:58 UTC (permalink / raw)
To: 'Sylwester Nawrocki'
Cc: linux-arm-kernel, linux-samsung-soc,
'Kishon Vijay Abraham I',
linux-media, 'Kukjin Kim', 'Sylwester Nawrocki',
'Felipe Balbi', 'Tomasz Figa',
devicetree-discuss, 'Inki Dae', 'Donghwa Lee',
'Kyungmin Park',
'Jean-Christophe PLAGNIOL-VILLARD',
linux-fbdev, 'Hui Wang',
Jingoo Han
On Tuesday, July 02, 2013 4:48 AM, Sylwester Nawrocki wrote:
> On 07/01/2013 07:24 AM, Jingoo Han wrote:
> > Add a PHY provider driver for the Samsung Exynos SoC DP PHY.
> >
> > Signed-off-by: Jingoo Han<jg1.han@samsung.com>
> > Cc: Sylwester Nawrocki<s.nawrocki@samsung.com>
> > Acked-by: Felipe Balbi<balbi@ti.com>
> > ---
> > .../devicetree/bindings/phy/samsung-phy.txt | 8 ++
> > drivers/phy/Kconfig | 5 +
> > drivers/phy/Makefile | 1 +
> > drivers/phy/phy-exynos-dp-video.c | 118 ++++++++++++++++++++
> > 4 files changed, 132 insertions(+)
> > create mode 100644 drivers/phy/phy-exynos-dp-video.c
> >
> > diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt
> b/Documentation/devicetree/bindings/phy/samsung-phy.txt
> > index 5ff208c..3fb656a 100644
> > --- a/Documentation/devicetree/bindings/phy/samsung-phy.txt
> > +++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt
> > @@ -12,3 +12,11 @@ the PHY specifier identifies the PHY and its meaning is as follows:
> > 1 - MIPI DSIM 0,
> > 2 - MIPI CSIS 1,
> > 3 - MIPI DSIM 1.
> > +
> > +Samsung EXYNOS SoC series DP PHY
>
> I would make it "Samsung EXYNOS SoC series Display Port PHY"
OK.
>
> > +-------------------------------------------------
> > +
> > +Required properties:
> > +- compatible : should be "samsung,exynos5250-dp-video-phy";
> > +- reg : offset and length of the DP PHY register set;
> > +- #phy-cells : from the generic phy bindings, must be 0;
>
> s/phy/PHY ?
OK.
>
> > \ No newline at end of file
>
> Missing new line character.
OK.
>
> > diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> > index 6f446d0..760f55a 100644
> > --- a/drivers/phy/Kconfig
> > +++ b/drivers/phy/Kconfig
> > @@ -19,4 +19,9 @@ config PHY_EXYNOS_MIPI_VIDEO
> > help
> > Support for MIPI CSI-2 and MIPI DSI DPHY found on Samsung
> > S5P and EXYNOS SoCs.
> > +
> > +config PHY_EXYNOS_DP_VIDEO
> > + tristate "EXYNOS SoC series DP PHY driver"
>
> "EXYNOS SoC series Display Port PHY driver" ?
>
> > + help
> > + Support for DP PHY found on Samsung EXYNOS SoCs.
>
> s/DP/Display Port ?
OK.
>
> > endif
> > diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> > index 71d8841..0fd1340 100644
> > --- a/drivers/phy/Makefile
> > +++ b/drivers/phy/Makefile
> > @@ -4,3 +4,4 @@
> >
> > obj-$(CONFIG_GENERIC_PHY) += phy-core.o
> > obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO) += phy-exynos-mipi-video.o
> > +obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO) += phy-exynos-dp-video.o
> > diff --git a/drivers/phy/phy-exynos-dp-video.c b/drivers/phy/phy-exynos-dp-video.c
> > new file mode 100644
> > index 0000000..75e1d11
> > --- /dev/null
> > +++ b/drivers/phy/phy-exynos-dp-video.c
> > @@ -0,0 +1,118 @@
> > +/*
> > + * Samsung EXYNOS SoC series DP PHY driver
>
> s/DP/Display Port ?
OK.
>
> > + *
> > + * Copyright (C) 2013 Samsung Electronics Co., Ltd.
> > + * Author: Jingoo Han<jg1.han@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>
> > +
> > +/* DPTX_PHY_CONTROL register */
> > +#define EXYNOS_DPTX_PHY_ENABLE (1<< 0)
> > +
> > +struct exynos_dp_video_phy {
> > + void __iomem *regs;
> > +};
> > +
> > +static int __set_phy_state(struct exynos_dp_video_phy *state, unsigned int on)
> > +{
> > + void __iomem *addr;
>
> How about just dropping this local variable ?
OK.
I will drop this local variable.
>
> > + u32 reg;
> > +
> > + addr = state->regs;
> > +
> > + reg = readl(addr);
> > + if (on)
> > + reg |= EXYNOS_DPTX_PHY_ENABLE;
> > + else
> > + reg&= ~EXYNOS_DPTX_PHY_ENABLE;
> > + writel(reg, addr);
> > +
> > + return 0;
> > +}
> > +
> > +static int exynos_dp_video_phy_power_on(struct phy *phy)
> > +{
> > + struct exynos_dp_video_phy *state = phy_get_drvdata(phy);
> > +
> > + return __set_phy_state(state, 1);
> > +}
> > +
> > +static int exynos_dp_video_phy_power_off(struct phy *phy)
> > +{
> > + struct exynos_dp_video_phy *state = phy_get_drvdata(phy);
> > +
> > + return __set_phy_state(state, 0);
> > +}
> > +
> > +static struct phy_ops exynos_dp_video_phy_ops = {
> > + .power_on = exynos_dp_video_phy_power_on,
> > + .power_off = exynos_dp_video_phy_power_off,
> > + .owner = THIS_MODULE,
> > +};
> > +
> > +static int exynos_dp_video_phy_probe(struct platform_device *pdev)
> > +{
> > + struct exynos_dp_video_phy *state;
> > + struct device *dev =&pdev->dev;
> > + struct resource *res;
> > + struct phy_provider *phy_provider;
> > + struct phy *phy;
> > +
> > + state = devm_kzalloc(dev, sizeof(*state), GFP_KERNEL);
> > + if (!state)
> > + return -ENOMEM;
> > +
> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +
> > + state->regs = devm_ioremap_resource(dev, res);
> > + if (IS_ERR(state->regs))
> > + return PTR_ERR(state->regs);
> > +
> > + dev_set_drvdata(dev, state);
>
> I can't see any corresponding dev_get_drvdata(), it should be safe
> to remove this assignment.
OK.
I will remove 'dev_set_drvdata(dev, state)'.
>
> > + phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> > + if (IS_ERR(phy_provider))
> > + return PTR_ERR(phy_provider);
> > +
> > + phy = devm_phy_create(dev, 0,&exynos_dp_video_phy_ops, "dp");
>
> The label could be NULL, since there is no need to support non-dt config.
OK.
I will replace "dp" with NULL.
>
> > + if (IS_ERR(phy)) {
> > + dev_err(dev, "failed to create DP PHY\n");
> > + return PTR_ERR(phy);
> > + }
> > + phy_set_drvdata(phy, state);
> > +
> > + return 0;
> > +}
> > +
> > +#ifdef CONFIG_OF
>
> You don't use of_match_ptr() macro to assign of_match_table to the driver
> structure below. This means build with CONFIG_OF disabled is broken now.
> I would just remove that #ifdef and would make the driver depends on
> CONFIG_OF.
OK.
I will add 'depends on OF', then remove '#ifdef CONFIG_OF'.
Thank you for your detailed comment. :)
Best regards,
Jingoo Han
>
> > +static const struct of_device_id exynos_dp_video_phy_of_match[] = {
> > + { .compatible = "samsung,exynos5250-dp-video-phy" },
> > + { },
> > +};
> > +MODULE_DEVICE_TABLE(of, exynos_dp_video_phy_of_match);
> > +#endif
> > +
> > +static struct platform_driver exynos_dp_video_phy_driver = {
> > + .probe = exynos_dp_video_phy_probe,
> > + .driver = {
> > + .name = "exynos-dp-video-phy",
> > + .owner = THIS_MODULE,
> > + .of_match_table = exynos_dp_video_phy_of_match,
> > + }
> > +};
> > +module_platform_driver(exynos_dp_video_phy_driver);
> > +
> > +MODULE_AUTHOR("Jingoo Han<jg1.han@samsung.com>");
> > +MODULE_DESCRIPTION("Samsung EXYNOS SoC DP PHY driver");
> > +MODULE_LICENSE("GPL v2");
>
> Thanks,
> Sylwester
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH V3 2/3] phy: Add driver for Exynos DP PHY
@ 2013-07-02 6:58 ` Jingoo Han
0 siblings, 0 replies; 9+ messages in thread
From: Jingoo Han @ 2013-07-02 6:58 UTC (permalink / raw)
To: linux-arm-kernel
On Tuesday, July 02, 2013 4:48 AM, Sylwester Nawrocki wrote:
> On 07/01/2013 07:24 AM, Jingoo Han wrote:
> > Add a PHY provider driver for the Samsung Exynos SoC DP PHY.
> >
> > Signed-off-by: Jingoo Han<jg1.han@samsung.com>
> > Cc: Sylwester Nawrocki<s.nawrocki@samsung.com>
> > Acked-by: Felipe Balbi<balbi@ti.com>
> > ---
> > .../devicetree/bindings/phy/samsung-phy.txt | 8 ++
> > drivers/phy/Kconfig | 5 +
> > drivers/phy/Makefile | 1 +
> > drivers/phy/phy-exynos-dp-video.c | 118 ++++++++++++++++++++
> > 4 files changed, 132 insertions(+)
> > create mode 100644 drivers/phy/phy-exynos-dp-video.c
> >
> > diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt
> b/Documentation/devicetree/bindings/phy/samsung-phy.txt
> > index 5ff208c..3fb656a 100644
> > --- a/Documentation/devicetree/bindings/phy/samsung-phy.txt
> > +++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt
> > @@ -12,3 +12,11 @@ the PHY specifier identifies the PHY and its meaning is as follows:
> > 1 - MIPI DSIM 0,
> > 2 - MIPI CSIS 1,
> > 3 - MIPI DSIM 1.
> > +
> > +Samsung EXYNOS SoC series DP PHY
>
> I would make it "Samsung EXYNOS SoC series Display Port PHY"
OK.
>
> > +-------------------------------------------------
> > +
> > +Required properties:
> > +- compatible : should be "samsung,exynos5250-dp-video-phy";
> > +- reg : offset and length of the DP PHY register set;
> > +- #phy-cells : from the generic phy bindings, must be 0;
>
> s/phy/PHY ?
OK.
>
> > \ No newline at end of file
>
> Missing new line character.
OK.
>
> > diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> > index 6f446d0..760f55a 100644
> > --- a/drivers/phy/Kconfig
> > +++ b/drivers/phy/Kconfig
> > @@ -19,4 +19,9 @@ config PHY_EXYNOS_MIPI_VIDEO
> > help
> > Support for MIPI CSI-2 and MIPI DSI DPHY found on Samsung
> > S5P and EXYNOS SoCs.
> > +
> > +config PHY_EXYNOS_DP_VIDEO
> > + tristate "EXYNOS SoC series DP PHY driver"
>
> "EXYNOS SoC series Display Port PHY driver" ?
>
> > + help
> > + Support for DP PHY found on Samsung EXYNOS SoCs.
>
> s/DP/Display Port ?
OK.
>
> > endif
> > diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> > index 71d8841..0fd1340 100644
> > --- a/drivers/phy/Makefile
> > +++ b/drivers/phy/Makefile
> > @@ -4,3 +4,4 @@
> >
> > obj-$(CONFIG_GENERIC_PHY) += phy-core.o
> > obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO) += phy-exynos-mipi-video.o
> > +obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO) += phy-exynos-dp-video.o
> > diff --git a/drivers/phy/phy-exynos-dp-video.c b/drivers/phy/phy-exynos-dp-video.c
> > new file mode 100644
> > index 0000000..75e1d11
> > --- /dev/null
> > +++ b/drivers/phy/phy-exynos-dp-video.c
> > @@ -0,0 +1,118 @@
> > +/*
> > + * Samsung EXYNOS SoC series DP PHY driver
>
> s/DP/Display Port ?
OK.
>
> > + *
> > + * Copyright (C) 2013 Samsung Electronics Co., Ltd.
> > + * Author: Jingoo Han<jg1.han@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>
> > +
> > +/* DPTX_PHY_CONTROL register */
> > +#define EXYNOS_DPTX_PHY_ENABLE (1<< 0)
> > +
> > +struct exynos_dp_video_phy {
> > + void __iomem *regs;
> > +};
> > +
> > +static int __set_phy_state(struct exynos_dp_video_phy *state, unsigned int on)
> > +{
> > + void __iomem *addr;
>
> How about just dropping this local variable ?
OK.
I will drop this local variable.
>
> > + u32 reg;
> > +
> > + addr = state->regs;
> > +
> > + reg = readl(addr);
> > + if (on)
> > + reg |= EXYNOS_DPTX_PHY_ENABLE;
> > + else
> > + reg&= ~EXYNOS_DPTX_PHY_ENABLE;
> > + writel(reg, addr);
> > +
> > + return 0;
> > +}
> > +
> > +static int exynos_dp_video_phy_power_on(struct phy *phy)
> > +{
> > + struct exynos_dp_video_phy *state = phy_get_drvdata(phy);
> > +
> > + return __set_phy_state(state, 1);
> > +}
> > +
> > +static int exynos_dp_video_phy_power_off(struct phy *phy)
> > +{
> > + struct exynos_dp_video_phy *state = phy_get_drvdata(phy);
> > +
> > + return __set_phy_state(state, 0);
> > +}
> > +
> > +static struct phy_ops exynos_dp_video_phy_ops = {
> > + .power_on = exynos_dp_video_phy_power_on,
> > + .power_off = exynos_dp_video_phy_power_off,
> > + .owner = THIS_MODULE,
> > +};
> > +
> > +static int exynos_dp_video_phy_probe(struct platform_device *pdev)
> > +{
> > + struct exynos_dp_video_phy *state;
> > + struct device *dev =&pdev->dev;
> > + struct resource *res;
> > + struct phy_provider *phy_provider;
> > + struct phy *phy;
> > +
> > + state = devm_kzalloc(dev, sizeof(*state), GFP_KERNEL);
> > + if (!state)
> > + return -ENOMEM;
> > +
> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +
> > + state->regs = devm_ioremap_resource(dev, res);
> > + if (IS_ERR(state->regs))
> > + return PTR_ERR(state->regs);
> > +
> > + dev_set_drvdata(dev, state);
>
> I can't see any corresponding dev_get_drvdata(), it should be safe
> to remove this assignment.
OK.
I will remove 'dev_set_drvdata(dev, state)'.
>
> > + phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> > + if (IS_ERR(phy_provider))
> > + return PTR_ERR(phy_provider);
> > +
> > + phy = devm_phy_create(dev, 0,&exynos_dp_video_phy_ops, "dp");
>
> The label could be NULL, since there is no need to support non-dt config.
OK.
I will replace "dp" with NULL.
>
> > + if (IS_ERR(phy)) {
> > + dev_err(dev, "failed to create DP PHY\n");
> > + return PTR_ERR(phy);
> > + }
> > + phy_set_drvdata(phy, state);
> > +
> > + return 0;
> > +}
> > +
> > +#ifdef CONFIG_OF
>
> You don't use of_match_ptr() macro to assign of_match_table to the driver
> structure below. This means build with CONFIG_OF disabled is broken now.
> I would just remove that #ifdef and would make the driver depends on
> CONFIG_OF.
OK.
I will add 'depends on OF', then remove '#ifdef CONFIG_OF'.
Thank you for your detailed comment. :)
Best regards,
Jingoo Han
>
> > +static const struct of_device_id exynos_dp_video_phy_of_match[] = {
> > + { .compatible = "samsung,exynos5250-dp-video-phy" },
> > + { },
> > +};
> > +MODULE_DEVICE_TABLE(of, exynos_dp_video_phy_of_match);
> > +#endif
> > +
> > +static struct platform_driver exynos_dp_video_phy_driver = {
> > + .probe = exynos_dp_video_phy_probe,
> > + .driver = {
> > + .name = "exynos-dp-video-phy",
> > + .owner = THIS_MODULE,
> > + .of_match_table = exynos_dp_video_phy_of_match,
> > + }
> > +};
> > +module_platform_driver(exynos_dp_video_phy_driver);
> > +
> > +MODULE_AUTHOR("Jingoo Han<jg1.han@samsung.com>");
> > +MODULE_DESCRIPTION("Samsung EXYNOS SoC DP PHY driver");
> > +MODULE_LICENSE("GPL v2");
>
> Thanks,
> Sylwester
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH V3 2/3] phy: Add driver for Exynos DP PHY
@ 2013-07-02 6:58 ` Jingoo Han
0 siblings, 0 replies; 9+ messages in thread
From: Jingoo Han @ 2013-07-02 6:58 UTC (permalink / raw)
To: linux-arm-kernel
On Tuesday, July 02, 2013 4:48 AM, Sylwester Nawrocki wrote:
> On 07/01/2013 07:24 AM, Jingoo Han wrote:
> > Add a PHY provider driver for the Samsung Exynos SoC DP PHY.
> >
> > Signed-off-by: Jingoo Han<jg1.han@samsung.com>
> > Cc: Sylwester Nawrocki<s.nawrocki@samsung.com>
> > Acked-by: Felipe Balbi<balbi@ti.com>
> > ---
> > .../devicetree/bindings/phy/samsung-phy.txt | 8 ++
> > drivers/phy/Kconfig | 5 +
> > drivers/phy/Makefile | 1 +
> > drivers/phy/phy-exynos-dp-video.c | 118 ++++++++++++++++++++
> > 4 files changed, 132 insertions(+)
> > create mode 100644 drivers/phy/phy-exynos-dp-video.c
> >
> > diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt
> b/Documentation/devicetree/bindings/phy/samsung-phy.txt
> > index 5ff208c..3fb656a 100644
> > --- a/Documentation/devicetree/bindings/phy/samsung-phy.txt
> > +++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt
> > @@ -12,3 +12,11 @@ the PHY specifier identifies the PHY and its meaning is as follows:
> > 1 - MIPI DSIM 0,
> > 2 - MIPI CSIS 1,
> > 3 - MIPI DSIM 1.
> > +
> > +Samsung EXYNOS SoC series DP PHY
>
> I would make it "Samsung EXYNOS SoC series Display Port PHY"
OK.
>
> > +-------------------------------------------------
> > +
> > +Required properties:
> > +- compatible : should be "samsung,exynos5250-dp-video-phy";
> > +- reg : offset and length of the DP PHY register set;
> > +- #phy-cells : from the generic phy bindings, must be 0;
>
> s/phy/PHY ?
OK.
>
> > \ No newline at end of file
>
> Missing new line character.
OK.
>
> > diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> > index 6f446d0..760f55a 100644
> > --- a/drivers/phy/Kconfig
> > +++ b/drivers/phy/Kconfig
> > @@ -19,4 +19,9 @@ config PHY_EXYNOS_MIPI_VIDEO
> > help
> > Support for MIPI CSI-2 and MIPI DSI DPHY found on Samsung
> > S5P and EXYNOS SoCs.
> > +
> > +config PHY_EXYNOS_DP_VIDEO
> > + tristate "EXYNOS SoC series DP PHY driver"
>
> "EXYNOS SoC series Display Port PHY driver" ?
>
> > + help
> > + Support for DP PHY found on Samsung EXYNOS SoCs.
>
> s/DP/Display Port ?
OK.
>
> > endif
> > diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> > index 71d8841..0fd1340 100644
> > --- a/drivers/phy/Makefile
> > +++ b/drivers/phy/Makefile
> > @@ -4,3 +4,4 @@
> >
> > obj-$(CONFIG_GENERIC_PHY) += phy-core.o
> > obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO) += phy-exynos-mipi-video.o
> > +obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO) += phy-exynos-dp-video.o
> > diff --git a/drivers/phy/phy-exynos-dp-video.c b/drivers/phy/phy-exynos-dp-video.c
> > new file mode 100644
> > index 0000000..75e1d11
> > --- /dev/null
> > +++ b/drivers/phy/phy-exynos-dp-video.c
> > @@ -0,0 +1,118 @@
> > +/*
> > + * Samsung EXYNOS SoC series DP PHY driver
>
> s/DP/Display Port ?
OK.
>
> > + *
> > + * Copyright (C) 2013 Samsung Electronics Co., Ltd.
> > + * Author: Jingoo Han<jg1.han@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>
> > +
> > +/* DPTX_PHY_CONTROL register */
> > +#define EXYNOS_DPTX_PHY_ENABLE (1<< 0)
> > +
> > +struct exynos_dp_video_phy {
> > + void __iomem *regs;
> > +};
> > +
> > +static int __set_phy_state(struct exynos_dp_video_phy *state, unsigned int on)
> > +{
> > + void __iomem *addr;
>
> How about just dropping this local variable ?
OK.
I will drop this local variable.
>
> > + u32 reg;
> > +
> > + addr = state->regs;
> > +
> > + reg = readl(addr);
> > + if (on)
> > + reg |= EXYNOS_DPTX_PHY_ENABLE;
> > + else
> > + reg&= ~EXYNOS_DPTX_PHY_ENABLE;
> > + writel(reg, addr);
> > +
> > + return 0;
> > +}
> > +
> > +static int exynos_dp_video_phy_power_on(struct phy *phy)
> > +{
> > + struct exynos_dp_video_phy *state = phy_get_drvdata(phy);
> > +
> > + return __set_phy_state(state, 1);
> > +}
> > +
> > +static int exynos_dp_video_phy_power_off(struct phy *phy)
> > +{
> > + struct exynos_dp_video_phy *state = phy_get_drvdata(phy);
> > +
> > + return __set_phy_state(state, 0);
> > +}
> > +
> > +static struct phy_ops exynos_dp_video_phy_ops = {
> > + .power_on = exynos_dp_video_phy_power_on,
> > + .power_off = exynos_dp_video_phy_power_off,
> > + .owner = THIS_MODULE,
> > +};
> > +
> > +static int exynos_dp_video_phy_probe(struct platform_device *pdev)
> > +{
> > + struct exynos_dp_video_phy *state;
> > + struct device *dev =&pdev->dev;
> > + struct resource *res;
> > + struct phy_provider *phy_provider;
> > + struct phy *phy;
> > +
> > + state = devm_kzalloc(dev, sizeof(*state), GFP_KERNEL);
> > + if (!state)
> > + return -ENOMEM;
> > +
> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +
> > + state->regs = devm_ioremap_resource(dev, res);
> > + if (IS_ERR(state->regs))
> > + return PTR_ERR(state->regs);
> > +
> > + dev_set_drvdata(dev, state);
>
> I can't see any corresponding dev_get_drvdata(), it should be safe
> to remove this assignment.
OK.
I will remove 'dev_set_drvdata(dev, state)'.
>
> > + phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> > + if (IS_ERR(phy_provider))
> > + return PTR_ERR(phy_provider);
> > +
> > + phy = devm_phy_create(dev, 0,&exynos_dp_video_phy_ops, "dp");
>
> The label could be NULL, since there is no need to support non-dt config.
OK.
I will replace "dp" with NULL.
>
> > + if (IS_ERR(phy)) {
> > + dev_err(dev, "failed to create DP PHY\n");
> > + return PTR_ERR(phy);
> > + }
> > + phy_set_drvdata(phy, state);
> > +
> > + return 0;
> > +}
> > +
> > +#ifdef CONFIG_OF
>
> You don't use of_match_ptr() macro to assign of_match_table to the driver
> structure below. This means build with CONFIG_OF disabled is broken now.
> I would just remove that #ifdef and would make the driver depends on
> CONFIG_OF.
OK.
I will add 'depends on OF', then remove '#ifdef CONFIG_OF'.
Thank you for your detailed comment. :)
Best regards,
Jingoo Han
>
> > +static const struct of_device_id exynos_dp_video_phy_of_match[] = {
> > + { .compatible = "samsung,exynos5250-dp-video-phy" },
> > + { },
> > +};
> > +MODULE_DEVICE_TABLE(of, exynos_dp_video_phy_of_match);
> > +#endif
> > +
> > +static struct platform_driver exynos_dp_video_phy_driver = {
> > + .probe = exynos_dp_video_phy_probe,
> > + .driver = {
> > + .name = "exynos-dp-video-phy",
> > + .owner = THIS_MODULE,
> > + .of_match_table = exynos_dp_video_phy_of_match,
> > + }
> > +};
> > +module_platform_driver(exynos_dp_video_phy_driver);
> > +
> > +MODULE_AUTHOR("Jingoo Han<jg1.han@samsung.com>");
> > +MODULE_DESCRIPTION("Samsung EXYNOS SoC DP PHY driver");
> > +MODULE_LICENSE("GPL v2");
>
> Thanks,
> Sylwester
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2013-07-02 6:58 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-07-01 5:24 [PATCH V3 2/3] phy: Add driver for Exynos DP PHY Jingoo Han
2013-07-01 5:24 ` Jingoo Han
2013-07-01 5:24 ` Jingoo Han
2013-07-01 19:48 ` Sylwester Nawrocki
2013-07-01 19:48 ` Sylwester Nawrocki
2013-07-01 19:48 ` Sylwester Nawrocki
2013-07-02 6:58 ` Jingoo Han
2013-07-02 6:58 ` Jingoo Han
2013-07-02 6:58 ` Jingoo Han
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.