* Re: [PATCH 2/3] ARM: dts: Add DP PHY node to exynos5250.dtsi
From: Kishon Vijay Abraham I @ 2013-06-28 5:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <001601ce73bf$9f2e9120$dd8bb360$@samsung.com>
Hi,
On Friday 28 June 2013 10:53 AM, Jingoo Han wrote:
> Add PHY provider node for the DP PHY.
>
> Signed-off-by: Jingoo Han <jg1.han@samsung.com>
> ---
> arch/arm/boot/dts/exynos5250.dtsi | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
> index 41cd625..d1d6e14 100644
> --- a/arch/arm/boot/dts/exynos5250.dtsi
> +++ b/arch/arm/boot/dts/exynos5250.dtsi
> @@ -614,6 +614,12 @@
> interrupts = <0 94 0>;
> };
>
> + dp_phy: video-phy@10040720 {
> + compatible = "samsung,exynos5250-dp-video-phy";
> + reg = <0x10040720 4>;
> + #phy-cells = <1>;
phy-cells can be '0' here since this phy_provider implements only one PHY.
Thanks
Kishon
^ permalink raw reply
* Re: [PATCH 1/3] phy: Add driver for Exynos DP PHY
From: Jingoo Han @ 2013-06-28 5:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <51CD1FA7.2010608@ti.com>
On Friday, June 28, 2013 2:31 PM, Kishon Vijay Abraham I wrote:
>
> Hi,
>
> On Friday 28 June 2013 10:52 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>
> > ---
> > .../phy/samsung,exynos5250-dp-video-phy.txt | 7 ++
> > drivers/phy/Kconfig | 8 ++
> > drivers/phy/Makefile | 3 +-
> > drivers/phy/phy-exynos-dp-video.c | 130 ++++++++++++++++++++
> > 4 files changed, 147 insertions(+), 1 deletion(-)
> > create mode 100644 Documentation/devicetree/bindings/phy/samsung,exynos5250-dp-video-phy.txt
> > create mode 100644 drivers/phy/phy-exynos-dp-video.c
> >
> > diff --git a/Documentation/devicetree/bindings/phy/samsung,exynos5250-dp-video-phy.txt
> > b/Documentation/devicetree/bindings/phy/samsung,exynos5250-dp-video-phy.txt
> > new file mode 100644
> > index 0000000..8b6fa79
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/phy/samsung,exynos5250-dp-video-phy.txt
> > @@ -0,0 +1,7 @@
> > +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 1;
> > diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> > index 5f85909..6d10e3b 100644
> > --- a/drivers/phy/Kconfig
> > +++ b/drivers/phy/Kconfig
> > @@ -11,3 +11,11 @@ menuconfig GENERIC_PHY
> > devices present in the kernel. This layer will have the generic
> > API by which phy drivers can create PHY using the phy framework and
> > phy users can obtain reference to the PHY.
> > +
> > +if GENERIC_PHY
> > +
> > +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 9e9560f..d8d861c 100644
> > --- a/drivers/phy/Makefile
> > +++ b/drivers/phy/Makefile
> > @@ -2,4 +2,5 @@
> > # Makefile for the phy drivers.
> > #
> >
> > -obj-$(CONFIG_GENERIC_PHY) += phy-core.o
> > +obj-$(CONFIG_GENERIC_PHY) += phy-core.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..376b3bc2
> > --- /dev/null
> > +++ b/drivers/phy/phy-exynos-dp-video.c
> > @@ -0,0 +1,130 @@
> > +/*
> > + * 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/delay.h>
>
> this header file is not needed here.
OK, I will remove it.
>
> > +#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>
> > +
> > +/* DPTX_PHY_CONTROL register */
> > +#define EXYNOS_DPTX_PHY_ENABLE (1 << 0)
> > +
> > +struct exynos_dp_video_phy {
> > + spinlock_t slock;
> > + struct phy *phys;
> > + void __iomem *regs;
> > +};
> > +
> > +static int __set_phy_state(struct exynos_dp_video_phy *state, unsigned int on)
> > +{
> > + void __iomem *addr;
> > + unsigned long flags;
> > + u32 reg;
> > +
> > + addr = state->regs;
> > +
> > + spin_lock_irqsave(&state->slock, flags);
> > + reg = readl(addr);
> > + if (on)
> > + reg |= EXYNOS_DPTX_PHY_ENABLE;
> > + else
> > + reg &= ~EXYNOS_DPTX_PHY_ENABLE;
> > + writel(reg, addr);
> > + spin_unlock_irqrestore(&state->slock, flags);
> > + 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 *exynos_dp_video_phy_xlate(struct device *dev,
> > + struct of_phandle_args *args)
> > +{
> > + struct exynos_dp_video_phy *state = dev_get_drvdata(dev);
> > +
> > + return state->phys;
>
> you can instead use of_phy_simple_xlate for such simple cases.
OK, I will use of_phy_simple_xlate().
> > +}
> > +
> > +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;
> > +
> > + 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,
> > + exynos_dp_video_phy_xlate);
> > + if (IS_ERR(phy_provider))
> > + return PTR_ERR(phy_provider);
> > +
> > + state->phys = devm_phy_create(dev, 0, &exynos_dp_video_phy_ops, "dp");
> > + if (IS_ERR(state->phys)) {
> > + dev_err(dev, "failed to create DP PHY\n");
> > + return PTR_ERR(state->phys);
> > + }
> > + phy_set_drvdata(state->phys, state);
> > +
> > + return 0;
> > +}
> > +
> > +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);
>
> This above should come inside #ifdef CONFIG_OF.
OK, I will add '#ifdef CONFIG_OF'.
> > +
> > +static struct platform_driver exynos_dp_video_phy_driver = {
> > + .probe = exynos_dp_video_phy_probe,
>
> missing .remove?
No, it is intentional.
In the exynos_dp_video_phy_probe(), only devm_*() are called as below.
devm_kzalloc(),
devm_ioremap_resource(),
devm_of_phy_provider_register(),
devm_phy_create(),
Also, dev_set_drvdata(dev, NULL), phy_set_drvdata(state->phys, NULL)
are not necessary in remove(), because driver core clears automatically
after device_release.
Thus, there is no functions in the remove().
Thank you for your comment. :)
It is very helpful.
Best regards,
Jingoo Han
> > + .driver = {
> > + .of_match_table = exynos_dp_video_phy_of_match,
> > + .name = "exynos-dp-video-phy",
> > + .owner = THIS_MODULE,
> > + }
> > +};
> > +module_platform_driver(exynos_dp_video_phy_driver);
> > +
> > +MODULE_DESCRIPTION("Samsung EXYNOS SoC DP PHY driver");
> > +MODULE_AUTHOR("Jingoo Han <jg1.han@samsung.com>");
> > +MODULE_LICENSE("GPL v2");
>
> Thanks
> Kishon
^ permalink raw reply
* Re: [PATCH 2/3] ARM: dts: Add DP PHY node to exynos5250.dtsi
From: Jingoo Han @ 2013-06-28 5:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <51CD2214.10506@ti.com>
On Friday, June 28, 2013 2:42 PM, Kishon Vijay Abraham I wrote:
>
> Hi,
>
> On Friday 28 June 2013 10:53 AM, Jingoo Han wrote:
> > Add PHY provider node for the DP PHY.
> >
> > Signed-off-by: Jingoo Han <jg1.han@samsung.com>
> > ---
> > arch/arm/boot/dts/exynos5250.dtsi | 13 ++++++++-----
> > 1 file changed, 8 insertions(+), 5 deletions(-)
> >
> > diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
> > index 41cd625..d1d6e14 100644
> > --- a/arch/arm/boot/dts/exynos5250.dtsi
> > +++ b/arch/arm/boot/dts/exynos5250.dtsi
> > @@ -614,6 +614,12 @@
> > interrupts = <0 94 0>;
> > };
> >
> > + dp_phy: video-phy@10040720 {
> > + compatible = "samsung,exynos5250-dp-video-phy";
> > + reg = <0x10040720 4>;
> > + #phy-cells = <1>;
>
> phy-cells can be '0' here since this phy_provider implements only one PHY.
Oh, thank you.
I will fix it.
Best regards,
Jingoo Han
>
> Thanks
> Kishon
^ permalink raw reply
* Re: [PATCH 3/3] video: exynos_dp: Use the generic PHY driver
From: Kishon Vijay Abraham I @ 2013-06-28 5:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <001701ce73bf$bebf9f20$3c3edd60$@samsung.com>
Hi,
On Friday 28 June 2013 10:54 AM, Jingoo Han wrote:
> Use the generic PHY API instead of the platform callback to control
> the DP PHY. The 'phy_label' field is added to the platform data
> structure to allow PHY lookup on non-dt platforms.
>
> Signed-off-by: Jingoo Han <jg1.han@samsung.com>
> ---
> .../devicetree/bindings/video/exynos_dp.txt | 17 ---
> drivers/video/exynos/exynos_dp_core.c | 118 ++------------------
> drivers/video/exynos/exynos_dp_core.h | 2 +
> include/video/exynos_dp.h | 6 +-
> 4 files changed, 15 insertions(+), 128 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/video/exynos_dp.txt b/Documentation/devicetree/bindings/video/exynos_dp.txt
> index 84f10c1..a8320e3 100644
> --- a/Documentation/devicetree/bindings/video/exynos_dp.txt
> +++ b/Documentation/devicetree/bindings/video/exynos_dp.txt
> @@ -1,17 +1,6 @@
> The Exynos display port interface should be configured based on
> the type of panel connected to it.
>
> -We use two nodes:
> - -dp-controller node
> - -dptx-phy node(defined inside dp-controller node)
> -
> -For the DP-PHY initialization, we use the dptx-phy node.
> -Required properties for dptx-phy:
> - -reg:
> - Base address of DP PHY register.
> - -samsung,enable-mask:
> - The bit-mask used to enable/disable DP PHY.
> -
> For the Panel initialization, we read data from dp-controller node.
> Required properties for dp-controller:
> -compatible:
> @@ -67,12 +56,6 @@ SOC specific portion:
> interrupt-parent = <&combiner>;
> clocks = <&clock 342>;
> clock-names = "dp";
> -
> - dptx-phy {
> - reg = <0x10040720>;
> - samsung,enable-mask = <1>;
> - };
> -
> };
>
> Board Specific portion:
> diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
> index 12bbede..bac515b 100644
> --- a/drivers/video/exynos/exynos_dp_core.c
> +++ b/drivers/video/exynos/exynos_dp_core.c
> @@ -19,6 +19,7 @@
> #include <linux/interrupt.h>
> #include <linux/delay.h>
> #include <linux/of.h>
> +#include <linux/phy/phy.h>
>
> #include <video/exynos_dp.h>
>
> @@ -960,84 +961,15 @@ static struct exynos_dp_platdata *exynos_dp_dt_parse_pdata(struct device *dev)
> return ERR_PTR(-EINVAL);
> }
>
> - return pd;
> -}
> -
> -static int exynos_dp_dt_parse_phydata(struct exynos_dp_device *dp)
> -{
> - struct device_node *dp_phy_node = of_node_get(dp->dev->of_node);
> - u32 phy_base;
> - int ret = 0;
> -
> - dp_phy_node = of_find_node_by_name(dp_phy_node, "dptx-phy");
> - if (!dp_phy_node) {
> - dev_err(dp->dev, "could not find dptx-phy node\n");
> - return -ENODEV;
> - }
> -
> - if (of_property_read_u32(dp_phy_node, "reg", &phy_base)) {
> - dev_err(dp->dev, "failed to get reg for dptx-phy\n");
> - ret = -EINVAL;
> - goto err;
> - }
> -
> - if (of_property_read_u32(dp_phy_node, "samsung,enable-mask",
> - &dp->enable_mask)) {
> - dev_err(dp->dev, "failed to get enable-mask for dptx-phy\n");
> - ret = -EINVAL;
> - goto err;
> - }
> -
> - dp->phy_addr = ioremap(phy_base, SZ_4);
> - if (!dp->phy_addr) {
> - dev_err(dp->dev, "failed to ioremap dp-phy\n");
> - ret = -ENOMEM;
> - goto err;
> - }
> -
> -err:
> - of_node_put(dp_phy_node);
> -
> - return ret;
> -}
> -
> -static void exynos_dp_phy_init(struct exynos_dp_device *dp)
> -{
> - u32 reg;
> -
> - reg = __raw_readl(dp->phy_addr);
> - reg |= dp->enable_mask;
> - __raw_writel(reg, dp->phy_addr);
> -}
> -
> -static void exynos_dp_phy_exit(struct exynos_dp_device *dp)
> -{
> - u32 reg;
> + pd->phy_label = "dp";
In the case of non-dt boot, this phy_label should have ideally come from
platform code.
Thanks
Kishon
^ permalink raw reply
* Re: [PATCH 3/3] video: exynos_dp: Use the generic PHY driver
From: Jingoo Han @ 2013-06-28 6:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <51CD25F2.5010206@ti.com>
On Friday, June 28, 2013 2:58 PM, Kishon Vijay Abraham I wrote:
>
> Hi,
>
> On Friday 28 June 2013 10:54 AM, Jingoo Han wrote:
> > Use the generic PHY API instead of the platform callback to control
> > the DP PHY. The 'phy_label' field is added to the platform data
> > structure to allow PHY lookup on non-dt platforms.
> >
> > Signed-off-by: Jingoo Han <jg1.han@samsung.com>
> > ---
> > .../devicetree/bindings/video/exynos_dp.txt | 17 ---
> > drivers/video/exynos/exynos_dp_core.c | 118 ++------------------
> > drivers/video/exynos/exynos_dp_core.h | 2 +
> > include/video/exynos_dp.h | 6 +-
> > 4 files changed, 15 insertions(+), 128 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/video/exynos_dp.txt
> b/Documentation/devicetree/bindings/video/exynos_dp.txt
> > index 84f10c1..a8320e3 100644
> > --- a/Documentation/devicetree/bindings/video/exynos_dp.txt
> > +++ b/Documentation/devicetree/bindings/video/exynos_dp.txt
> > @@ -1,17 +1,6 @@
> > The Exynos display port interface should be configured based on
> > the type of panel connected to it.
> >
> > -We use two nodes:
> > - -dp-controller node
> > - -dptx-phy node(defined inside dp-controller node)
> > -
> > -For the DP-PHY initialization, we use the dptx-phy node.
> > -Required properties for dptx-phy:
> > - -reg:
> > - Base address of DP PHY register.
> > - -samsung,enable-mask:
> > - The bit-mask used to enable/disable DP PHY.
> > -
> > For the Panel initialization, we read data from dp-controller node.
> > Required properties for dp-controller:
> > -compatible:
> > @@ -67,12 +56,6 @@ SOC specific portion:
> > interrupt-parent = <&combiner>;
> > clocks = <&clock 342>;
> > clock-names = "dp";
> > -
> > - dptx-phy {
> > - reg = <0x10040720>;
> > - samsung,enable-mask = <1>;
> > - };
> > -
> > };
> >
> > Board Specific portion:
> > diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
> > index 12bbede..bac515b 100644
> > --- a/drivers/video/exynos/exynos_dp_core.c
> > +++ b/drivers/video/exynos/exynos_dp_core.c
> > @@ -19,6 +19,7 @@
> > #include <linux/interrupt.h>
> > #include <linux/delay.h>
> > #include <linux/of.h>
> > +#include <linux/phy/phy.h>
> >
> > #include <video/exynos_dp.h>
> >
> > @@ -960,84 +961,15 @@ static struct exynos_dp_platdata *exynos_dp_dt_parse_pdata(struct device *dev)
> > return ERR_PTR(-EINVAL);
> > }
> >
> > - return pd;
> > -}
> > -
> > -static int exynos_dp_dt_parse_phydata(struct exynos_dp_device *dp)
> > -{
> > - struct device_node *dp_phy_node = of_node_get(dp->dev->of_node);
> > - u32 phy_base;
> > - int ret = 0;
> > -
> > - dp_phy_node = of_find_node_by_name(dp_phy_node, "dptx-phy");
> > - if (!dp_phy_node) {
> > - dev_err(dp->dev, "could not find dptx-phy node\n");
> > - return -ENODEV;
> > - }
> > -
> > - if (of_property_read_u32(dp_phy_node, "reg", &phy_base)) {
> > - dev_err(dp->dev, "failed to get reg for dptx-phy\n");
> > - ret = -EINVAL;
> > - goto err;
> > - }
> > -
> > - if (of_property_read_u32(dp_phy_node, "samsung,enable-mask",
> > - &dp->enable_mask)) {
> > - dev_err(dp->dev, "failed to get enable-mask for dptx-phy\n");
> > - ret = -EINVAL;
> > - goto err;
> > - }
> > -
> > - dp->phy_addr = ioremap(phy_base, SZ_4);
> > - if (!dp->phy_addr) {
> > - dev_err(dp->dev, "failed to ioremap dp-phy\n");
> > - ret = -ENOMEM;
> > - goto err;
> > - }
> > -
> > -err:
> > - of_node_put(dp_phy_node);
> > -
> > - return ret;
> > -}
> > -
> > -static void exynos_dp_phy_init(struct exynos_dp_device *dp)
> > -{
> > - u32 reg;
> > -
> > - reg = __raw_readl(dp->phy_addr);
> > - reg |= dp->enable_mask;
> > - __raw_writel(reg, dp->phy_addr);
> > -}
> > -
> > -static void exynos_dp_phy_exit(struct exynos_dp_device *dp)
> > -{
> > - u32 reg;
> > + pd->phy_label = "dp";
>
> In the case of non-dt boot, this phy_label should have ideally come from
> platform code.
No, this is NOT the case of non-dt.
'pd->phy_label = "dp";' is included in exynos_dp_dt_parse_pdata(),
not exynos_dp_phy_exit().
Also, exynos_dp_dt_parse_pdata() is called in the case of dt.
But, diff is a little bit confusing. :(
Best regards,
Jingoo Han
>
> Thanks
> Kishon
^ permalink raw reply
* Re: [PATCH 2/3] ARM: dts: Add DP PHY node to exynos5250.dtsi
From: Hui Wang @ 2013-06-28 6:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <001b01ce73c4$88f45020$9adcf060$@samsung.com>
On 06/28/2013 01:58 PM, Jingoo Han wrote:
> On Friday, June 28, 2013 2:42 PM, Kishon Vijay Abraham I wrote:
>> Hi,
>>
>> On Friday 28 June 2013 10:53 AM, Jingoo Han wrote:
>>> Add PHY provider node for the DP PHY.
>>>
>>> Signed-off-by: Jingoo Han <jg1.han@samsung.com>
>>> ---
>>> arch/arm/boot/dts/exynos5250.dtsi | 13 ++++++++-----
>>> 1 file changed, 8 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
>>> index 41cd625..d1d6e14 100644
>>> --- a/arch/arm/boot/dts/exynos5250.dtsi
>>> +++ b/arch/arm/boot/dts/exynos5250.dtsi
>>> @@ -614,6 +614,12 @@
>>> interrupts = <0 94 0>;
>>> };
>>>
>>> + dp_phy: video-phy@10040720 {
>>> + compatible = "samsung,exynos5250-dp-video-phy";
>>> + reg = <0x10040720 4>;
>>> + #phy-cells = <1>;
>> phy-cells can be '0' here since this phy_provider implements only one PHY.
> Oh, thank you.
> I will fix it.
Don't forget to fix the corresponding description in the
samsung,exynos5250-dp-video-phy.txt as well. :-)
> Best regards,
> Jingoo Han
>
>> Thanks
>> Kishon
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
^ permalink raw reply
* Re: [PATCH 2/3] ARM: dts: Add DP PHY node to exynos5250.dtsi
From: Jingoo Han @ 2013-06-28 6:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <51CD27FF.1060706@gmail.com>
On Friday, June 28, 2013 3:07 PM, Hui Wang wrote:
> On 06/28/2013 01:58 PM, Jingoo Han wrote:
> > On Friday, June 28, 2013 2:42 PM, Kishon Vijay Abraham I wrote:
> >> Hi,
> >>
> >> On Friday 28 June 2013 10:53 AM, Jingoo Han wrote:
> >>> Add PHY provider node for the DP PHY.
> >>>
> >>> Signed-off-by: Jingoo Han <jg1.han@samsung.com>
> >>> ---
> >>> arch/arm/boot/dts/exynos5250.dtsi | 13 ++++++++-----
> >>> 1 file changed, 8 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
> >>> index 41cd625..d1d6e14 100644
> >>> --- a/arch/arm/boot/dts/exynos5250.dtsi
> >>> +++ b/arch/arm/boot/dts/exynos5250.dtsi
> >>> @@ -614,6 +614,12 @@
> >>> interrupts = <0 94 0>;
> >>> };
> >>>
> >>> + dp_phy: video-phy@10040720 {
> >>> + compatible = "samsung,exynos5250-dp-video-phy";
> >>> + reg = <0x10040720 4>;
> >>> + #phy-cells = <1>;
> >> phy-cells can be '0' here since this phy_provider implements only one PHY.
> > Oh, thank you.
> > I will fix it.
> Don't forget to fix the corresponding description in the
> samsung,exynos5250-dp-video-phy.txt as well. :-)
Hi
OK, I already fixed it. :)
Thank you for reminding me.
Best regards,
Jignoo Han
> > Best regards,
> > Jingoo Han
> >
> >> Thanks
> >> Kishon
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-media" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
> >
^ permalink raw reply
* Re: [PATCH 1/3] phy: Add driver for Exynos DP PHY
From: Kishon Vijay Abraham I @ 2013-06-28 6:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <001801ce73c3$e6838900$b38a9b00$@samsung.com>
Hi,
On Friday 28 June 2013 11:24 AM, Jingoo Han wrote:
> On Friday, June 28, 2013 2:31 PM, Kishon Vijay Abraham I wrote:
>>
>> Hi,
>>
>> On Friday 28 June 2013 10:52 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>
>>> ---
>>> .../phy/samsung,exynos5250-dp-video-phy.txt | 7 ++
>>> drivers/phy/Kconfig | 8 ++
>>> drivers/phy/Makefile | 3 +-
>>> drivers/phy/phy-exynos-dp-video.c | 130 ++++++++++++++++++++
>>> 4 files changed, 147 insertions(+), 1 deletion(-)
>>> create mode 100644 Documentation/devicetree/bindings/phy/samsung,exynos5250-dp-video-phy.txt
>>> create mode 100644 drivers/phy/phy-exynos-dp-video.c
>>>
>>> diff --git a/Documentation/devicetree/bindings/phy/samsung,exynos5250-dp-video-phy.txt
>>> b/Documentation/devicetree/bindings/phy/samsung,exynos5250-dp-video-phy.txt
>>> new file mode 100644
>>> index 0000000..8b6fa79
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/phy/samsung,exynos5250-dp-video-phy.txt
>>> @@ -0,0 +1,7 @@
>>> +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 1;
>>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>>> index 5f85909..6d10e3b 100644
>>> --- a/drivers/phy/Kconfig
>>> +++ b/drivers/phy/Kconfig
>>> @@ -11,3 +11,11 @@ menuconfig GENERIC_PHY
>>> devices present in the kernel. This layer will have the generic
>>> API by which phy drivers can create PHY using the phy framework and
>>> phy users can obtain reference to the PHY.
>>> +
>>> +if GENERIC_PHY
>>> +
>>> +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 9e9560f..d8d861c 100644
>>> --- a/drivers/phy/Makefile
>>> +++ b/drivers/phy/Makefile
>>> @@ -2,4 +2,5 @@
>>> # Makefile for the phy drivers.
>>> #
>>>
>>> -obj-$(CONFIG_GENERIC_PHY) += phy-core.o
>>> +obj-$(CONFIG_GENERIC_PHY) += phy-core.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..376b3bc2
>>> --- /dev/null
>>> +++ b/drivers/phy/phy-exynos-dp-video.c
>>> @@ -0,0 +1,130 @@
>>> +/*
>>> + * 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/delay.h>
>>
>> this header file is not needed here.
>
> OK, I will remove it.
>
>>
>>> +#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>
>>> +
>>> +/* DPTX_PHY_CONTROL register */
>>> +#define EXYNOS_DPTX_PHY_ENABLE (1 << 0)
>>> +
>>> +struct exynos_dp_video_phy {
>>> + spinlock_t slock;
>>> + struct phy *phys;
>>> + void __iomem *regs;
>>> +};
>>> +
>>> +static int __set_phy_state(struct exynos_dp_video_phy *state, unsigned int on)
>>> +{
>>> + void __iomem *addr;
>>> + unsigned long flags;
>>> + u32 reg;
>>> +
>>> + addr = state->regs;
>>> +
>>> + spin_lock_irqsave(&state->slock, flags);
>>> + reg = readl(addr);
>>> + if (on)
>>> + reg |= EXYNOS_DPTX_PHY_ENABLE;
>>> + else
>>> + reg &= ~EXYNOS_DPTX_PHY_ENABLE;
>>> + writel(reg, addr);
>>> + spin_unlock_irqrestore(&state->slock, flags);
>>> + 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 *exynos_dp_video_phy_xlate(struct device *dev,
>>> + struct of_phandle_args *args)
>>> +{
>>> + struct exynos_dp_video_phy *state = dev_get_drvdata(dev);
>>> +
>>> + return state->phys;
>>
>> you can instead use of_phy_simple_xlate for such simple cases.
>
> OK, I will use of_phy_simple_xlate().
>
>>> +}
>>> +
>>> +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;
>>> +
>>> + 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,
>>> + exynos_dp_video_phy_xlate);
>>> + if (IS_ERR(phy_provider))
>>> + return PTR_ERR(phy_provider);
>>> +
>>> + state->phys = devm_phy_create(dev, 0, &exynos_dp_video_phy_ops, "dp");
>>> + if (IS_ERR(state->phys)) {
>>> + dev_err(dev, "failed to create DP PHY\n");
>>> + return PTR_ERR(state->phys);
>>> + }
>>> + phy_set_drvdata(state->phys, state);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +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);
>>
>> This above should come inside #ifdef CONFIG_OF.
>
> OK, I will add '#ifdef CONFIG_OF'.
>
>>> +
>>> +static struct platform_driver exynos_dp_video_phy_driver = {
>>> + .probe = exynos_dp_video_phy_probe,
>>
>> missing .remove?
>
> No, it is intentional.
>
> In the exynos_dp_video_phy_probe(), only devm_*() are called as below.
> devm_kzalloc(),
> devm_ioremap_resource(),
> devm_of_phy_provider_register(),
> devm_phy_create(),
>
> Also, dev_set_drvdata(dev, NULL), phy_set_drvdata(state->phys, NULL)
> are not necessary in remove(), because driver core clears automatically
> after device_release.
>
> Thus, there is no functions in the remove().
Looks correct. Alright then.
Thanks
Kishon
^ permalink raw reply
* Re: [PATCH v3 3/5] video: exynos_mipi_dsim: Use the generic PHY driver
From: Donghwa Lee @ 2013-06-28 6:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1372258946-15607-4-git-send-email-s.nawrocki@samsung.com>
On 06/28/2013 00:02, Sylwester Nawrocki wrote:
> Use the generic PHY API instead of the platform callback to control
> the MIPI DSIM DPHY. The 'phy_label' field is added to the platform
> data structure to allow PHY lookup on non-dt platforms.
>
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> Acked-by: Felipe Balbi <balbi@ti.com>
> ---
> drivers/video/exynos/exynos_mipi_dsi.c | 18 +++++++++---------
> include/video/exynos_mipi_dsim.h | 6 ++++--
> 2 files changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/video/exynos/exynos_mipi_dsi.c b/drivers/video/exynos/exynos_mipi_dsi.c
> index 32e5406..1f96004 100644
> --- a/drivers/video/exynos/exynos_mipi_dsi.c
> +++ b/drivers/video/exynos/exynos_mipi_dsi.c
> @@ -156,8 +156,7 @@ static int exynos_mipi_dsi_blank_mode(struct mipi_dsim_device *dsim, int power)
> exynos_mipi_regulator_enable(dsim);
>
> /* enable MIPI-DSI PHY. */
> - if (dsim->pd->phy_enable)
> - dsim->pd->phy_enable(pdev, true);
> + phy_power_on(dsim->phy);
>
> clk_enable(dsim->clock);
>
> @@ -373,6 +372,10 @@ static int exynos_mipi_dsi_probe(struct platform_device *pdev)
> return ret;
> }
>
> + dsim->phy = devm_phy_get(&pdev->dev, dsim_pd->phy_label);
> + if (IS_ERR(dsim->phy))
> + return PTR_ERR(dsim->phy);
> +
> dsim->clock = devm_clk_get(&pdev->dev, "dsim0");
> if (IS_ERR(dsim->clock)) {
> dev_err(&pdev->dev, "failed to get dsim clock source\n");
> @@ -439,8 +442,7 @@ static int exynos_mipi_dsi_probe(struct platform_device *pdev)
> exynos_mipi_regulator_enable(dsim);
>
> /* enable MIPI-DSI PHY. */
> - if (dsim->pd->phy_enable)
> - dsim->pd->phy_enable(pdev, true);
> + phy_power_on(dsim->phy);
>
> exynos_mipi_update_cfg(dsim);
>
> @@ -504,9 +506,8 @@ static int exynos_mipi_dsi_suspend(struct device *dev)
> if (client_drv && client_drv->suspend)
> client_drv->suspend(client_dev);
>
> - /* enable MIPI-DSI PHY. */
> - if (dsim->pd->phy_enable)
> - dsim->pd->phy_enable(pdev, false);
> + /* disable MIPI-DSI PHY. */
> + phy_power_off(dsim->phy);
>
> clk_disable(dsim->clock);
>
> @@ -536,8 +537,7 @@ static int exynos_mipi_dsi_resume(struct device *dev)
> exynos_mipi_regulator_enable(dsim);
>
> /* enable MIPI-DSI PHY. */
> - if (dsim->pd->phy_enable)
> - dsim->pd->phy_enable(pdev, true);
> + phy_power_on(dsim->phy);
>
> clk_enable(dsim->clock);
>
> diff --git a/include/video/exynos_mipi_dsim.h b/include/video/exynos_mipi_dsim.h
> index 89dc88a..fd69beb 100644
> --- a/include/video/exynos_mipi_dsim.h
> +++ b/include/video/exynos_mipi_dsim.h
> @@ -216,6 +216,7 @@ struct mipi_dsim_config {
> * automatically.
> * @e_clk_src: select byte clock source.
> * @pd: pointer to MIPI-DSI driver platform data.
> + * @phy: pointer to the generic PHY
> */
> struct mipi_dsim_device {
> struct device *dev;
> @@ -236,6 +237,7 @@ struct mipi_dsim_device {
> bool suspended;
>
> struct mipi_dsim_platform_data *pd;
> + struct phy *phy;
> };
>
> /*
> @@ -248,7 +250,7 @@ struct mipi_dsim_device {
> * @enabled: indicate whether mipi controller got enabled or not.
> * @lcd_panel_info: pointer for lcd panel specific structure.
> * this structure specifies width, height, timing and polarity and so on.
> - * @phy_enable: pointer to a callback controlling D-PHY enable/reset
> + * @phy_label: the generic PHY label
> */
> struct mipi_dsim_platform_data {
> char lcd_panel_name[PANEL_NAME_SIZE];
> @@ -257,7 +259,7 @@ struct mipi_dsim_platform_data {
> unsigned int enabled;
> void *lcd_panel_info;
>
> - int (*phy_enable)(struct platform_device *pdev, bool on);
> + const char *phy_label;
> };
>
> /*
I confirmed that this patch operates well. It looks good to me.
Acked-by: Donghwa Lee <dh09.lee@samsung.com>
Thank you,
Donghwa Lee
^ permalink raw reply
* [PATCH V2 0/3] Generic PHY driver for the Exynos SoC DP PHY
From: Jingoo Han @ 2013-06-28 7:14 UTC (permalink / raw)
To: linux-arm-kernel
This patch series adds a simple driver for the Samsung Exynos SoC
series DP transmitter PHY, using the generic PHY framework [1].
Previously the DP PHY used a platform callback or internal DT node
to control the PHY power enable bit.
The platform callback and internal DT node can be dropped and this
driver does not need any calls back to the platform code.
These patches was tested on Exynos5250.
This PATCH v2 follows:
* PATCH v1, sent on June, 28th 2013
Changes between v1 and v2:
* Replaced exynos_dp_video_phy_xlate() with of_phy_simple_xlate(),
as Kishon Vijay Abraham I guided.
* Set the value of phy-cells as 0, because the phy_provider implements
only one PHY.
* Removed unnecessary header include.
* Added '#ifdef CONFIG_OF' and of_match_ptr macro.
This series depends on the generic PHY framework [1]. These patches
refer to Sylwester Nawrocki's patches about Exynos MIPI [2].
[1] https://lkml.org/lkml/2013/6/26/259
[2] http://www.spinics.net/lists/linux-samsung-soc/msg20034.html
Jingoo Han (3):
phy: Add driver for Exynos DP PHY
ARM: dts: Add DP PHY node to exynos5250.dtsi
video: exynos_dp: Use the generic PHY driver
.../phy/samsung,exynos5250-dp-video-phy.txt | 7 ++
.../devicetree/bindings/video/exynos_dp.txt | 17 ---
arch/arm/boot/dts/exynos5250.dtsi | 13 ++++++++-----
drivers/phy/Kconfig | 8 ++
drivers/phy/Makefile | 3 +-
drivers/phy/phy-exynos-dp-video.c | 122 ++++++++++++++++++++
drivers/video/exynos/exynos_dp_core.c | 118 ++------------------
drivers/video/exynos/exynos_dp_core.h | 2 +
include/video/exynos_dp.h | 6 +-
9 files changed, 162 insertions(+), 134 deletions(-)
create mode 100644 Documentation/devicetree/bindings/phy/samsung,exynos5250-dp-video-phy.txt
create mode 100644 drivers/phy/phy-exynos-dp-video.c
--
1.7.10.4
^ permalink raw reply
* [PATCH V2 1/3] phy: Add driver for Exynos DP PHY
From: Jingoo Han @ 2013-06-28 7:15 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>
---
.../phy/samsung,exynos5250-dp-video-phy.txt | 7 ++
drivers/phy/Kconfig | 8 ++
drivers/phy/Makefile | 3 +-
drivers/phy/phy-exynos-dp-video.c | 122 ++++++++++++++++++++
4 files changed, 139 insertions(+), 1 deletion(-)
create mode 100644 Documentation/devicetree/bindings/phy/samsung,exynos5250-dp-video-phy.txt
create mode 100644 drivers/phy/phy-exynos-dp-video.c
diff --git a/Documentation/devicetree/bindings/phy/samsung,exynos5250-dp-video-phy.txt
b/Documentation/devicetree/bindings/phy/samsung,exynos5250-dp-video-phy.txt
new file mode 100644
index 0000000..d1771ef
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/samsung,exynos5250-dp-video-phy.txt
@@ -0,0 +1,7 @@
+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;
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 5f85909..6d10e3b 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -11,3 +11,11 @@ menuconfig GENERIC_PHY
devices present in the kernel. This layer will have the generic
API by which phy drivers can create PHY using the phy framework and
phy users can obtain reference to the PHY.
+
+if GENERIC_PHY
+
+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 9e9560f..d8d861c 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -2,4 +2,5 @@
# Makefile for the phy drivers.
#
-obj-$(CONFIG_GENERIC_PHY) += phy-core.o
+obj-$(CONFIG_GENERIC_PHY) += phy-core.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..9a3d6f1
--- /dev/null
+++ b/drivers/phy/phy-exynos-dp-video.c
@@ -0,0 +1,122 @@
+/*
+ * 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>
+#include <linux/spinlock.h>
+
+/* DPTX_PHY_CONTROL register */
+#define EXYNOS_DPTX_PHY_ENABLE (1 << 0)
+
+struct exynos_dp_video_phy {
+ spinlock_t slock;
+ struct phy *phys;
+ void __iomem *regs;
+};
+
+static int __set_phy_state(struct exynos_dp_video_phy *state, unsigned int on)
+{
+ void __iomem *addr;
+ unsigned long flags;
+ u32 reg;
+
+ addr = state->regs;
+
+ spin_lock_irqsave(&state->slock, flags);
+ reg = readl(addr);
+ if (on)
+ reg |= EXYNOS_DPTX_PHY_ENABLE;
+ else
+ reg &= ~EXYNOS_DPTX_PHY_ENABLE;
+ writel(reg, addr);
+ spin_unlock_irqrestore(&state->slock, flags);
+ 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;
+
+ 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);
+
+ state->phys = devm_phy_create(dev, 0, &exynos_dp_video_phy_ops, "dp");
+ if (IS_ERR(state->phys)) {
+ dev_err(dev, "failed to create DP PHY\n");
+ return PTR_ERR(state->phys);
+ }
+ phy_set_drvdata(state->phys, 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_DESCRIPTION("Samsung EXYNOS SoC DP PHY driver");
+MODULE_AUTHOR("Jingoo Han <jg1.han@samsung.com>");
+MODULE_LICENSE("GPL v2");
--
1.7.10.4
^ permalink raw reply related
* [PATCH V2 2/3] ARM: dts: Add DP PHY node to exynos5250.dtsi
From: Jingoo Han @ 2013-06-28 7:16 UTC (permalink / raw)
To: linux-arm-kernel
Add PHY provider node for the DP PHY.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
arch/arm/boot/dts/exynos5250.dtsi | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
index 41cd625..f7bac75 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi
@@ -614,6 +614,12 @@
interrupts = <0 94 0>;
};
+ dp_phy: video-phy@10040720 {
+ compatible = "samsung,exynos5250-dp-video-phy";
+ reg = <0x10040720 4>;
+ #phy-cells = <0>;
+ };
+
dp-controller {
compatible = "samsung,exynos5-dp";
reg = <0x145b0000 0x1000>;
@@ -623,11 +629,8 @@
clock-names = "dp";
#address-cells = <1>;
#size-cells = <0>;
-
- dptx-phy {
- reg = <0x10040720>;
- samsung,enable-mask = <1>;
- };
+ phys = <&dp_phy 0>;
+ phy-names = "dp";
};
fimd {
--
1.7.10.4
^ permalink raw reply related
* [PATCH V2 3/3] video: exynos_dp: Use the generic PHY driver
From: Jingoo Han @ 2013-06-28 7:18 UTC (permalink / raw)
To: linux-arm-kernel
Use the generic PHY API instead of the platform callback to control
the DP PHY. The 'phy_label' field is added to the platform data
structure to allow PHY lookup on non-dt platforms.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
.../devicetree/bindings/video/exynos_dp.txt | 17 ---
drivers/video/exynos/exynos_dp_core.c | 118 ++------------------
drivers/video/exynos/exynos_dp_core.h | 2 +
include/video/exynos_dp.h | 6 +-
4 files changed, 15 insertions(+), 128 deletions(-)
diff --git a/Documentation/devicetree/bindings/video/exynos_dp.txt b/Documentation/devicetree/bindings/video/exynos_dp.txt
index 84f10c1..a8320e3 100644
--- a/Documentation/devicetree/bindings/video/exynos_dp.txt
+++ b/Documentation/devicetree/bindings/video/exynos_dp.txt
@@ -1,17 +1,6 @@
The Exynos display port interface should be configured based on
the type of panel connected to it.
-We use two nodes:
- -dp-controller node
- -dptx-phy node(defined inside dp-controller node)
-
-For the DP-PHY initialization, we use the dptx-phy node.
-Required properties for dptx-phy:
- -reg:
- Base address of DP PHY register.
- -samsung,enable-mask:
- The bit-mask used to enable/disable DP PHY.
-
For the Panel initialization, we read data from dp-controller node.
Required properties for dp-controller:
-compatible:
@@ -67,12 +56,6 @@ SOC specific portion:
interrupt-parent = <&combiner>;
clocks = <&clock 342>;
clock-names = "dp";
-
- dptx-phy {
- reg = <0x10040720>;
- samsung,enable-mask = <1>;
- };
-
};
Board Specific portion:
diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
index 12bbede..bac515b 100644
--- a/drivers/video/exynos/exynos_dp_core.c
+++ b/drivers/video/exynos/exynos_dp_core.c
@@ -19,6 +19,7 @@
#include <linux/interrupt.h>
#include <linux/delay.h>
#include <linux/of.h>
+#include <linux/phy/phy.h>
#include <video/exynos_dp.h>
@@ -960,84 +961,15 @@ static struct exynos_dp_platdata *exynos_dp_dt_parse_pdata(struct device *dev)
return ERR_PTR(-EINVAL);
}
- return pd;
-}
-
-static int exynos_dp_dt_parse_phydata(struct exynos_dp_device *dp)
-{
- struct device_node *dp_phy_node = of_node_get(dp->dev->of_node);
- u32 phy_base;
- int ret = 0;
-
- dp_phy_node = of_find_node_by_name(dp_phy_node, "dptx-phy");
- if (!dp_phy_node) {
- dev_err(dp->dev, "could not find dptx-phy node\n");
- return -ENODEV;
- }
-
- if (of_property_read_u32(dp_phy_node, "reg", &phy_base)) {
- dev_err(dp->dev, "failed to get reg for dptx-phy\n");
- ret = -EINVAL;
- goto err;
- }
-
- if (of_property_read_u32(dp_phy_node, "samsung,enable-mask",
- &dp->enable_mask)) {
- dev_err(dp->dev, "failed to get enable-mask for dptx-phy\n");
- ret = -EINVAL;
- goto err;
- }
-
- dp->phy_addr = ioremap(phy_base, SZ_4);
- if (!dp->phy_addr) {
- dev_err(dp->dev, "failed to ioremap dp-phy\n");
- ret = -ENOMEM;
- goto err;
- }
-
-err:
- of_node_put(dp_phy_node);
-
- return ret;
-}
-
-static void exynos_dp_phy_init(struct exynos_dp_device *dp)
-{
- u32 reg;
-
- reg = __raw_readl(dp->phy_addr);
- reg |= dp->enable_mask;
- __raw_writel(reg, dp->phy_addr);
-}
-
-static void exynos_dp_phy_exit(struct exynos_dp_device *dp)
-{
- u32 reg;
+ pd->phy_label = "dp";
- reg = __raw_readl(dp->phy_addr);
- reg &= ~(dp->enable_mask);
- __raw_writel(reg, dp->phy_addr);
+ return pd;
}
#else
static struct exynos_dp_platdata *exynos_dp_dt_parse_pdata(struct device *dev)
{
return NULL;
}
-
-static int exynos_dp_dt_parse_phydata(struct exynos_dp_device *dp)
-{
- return -EINVAL;
-}
-
-static void exynos_dp_phy_init(struct exynos_dp_device *dp)
-{
- return;
-}
-
-static void exynos_dp_phy_exit(struct exynos_dp_device *dp)
-{
- return;
-}
#endif /* CONFIG_OF */
static int exynos_dp_probe(struct platform_device *pdev)
@@ -1061,10 +993,6 @@ static int exynos_dp_probe(struct platform_device *pdev)
pdata = exynos_dp_dt_parse_pdata(&pdev->dev);
if (IS_ERR(pdata))
return PTR_ERR(pdata);
-
- ret = exynos_dp_dt_parse_phydata(dp);
- if (ret)
- return ret;
} else {
pdata = pdev->dev.platform_data;
if (!pdata) {
@@ -1073,6 +1001,10 @@ static int exynos_dp_probe(struct platform_device *pdev)
}
}
+ dp->phy = devm_phy_get(&pdev->dev, pdata->phy_label);
+ if (IS_ERR(dp->phy))
+ return PTR_ERR(dp->phy);
+
dp->clock = devm_clk_get(&pdev->dev, "dp");
if (IS_ERR(dp->clock)) {
dev_err(&pdev->dev, "failed to get clock\n");
@@ -1097,13 +1029,7 @@ static int exynos_dp_probe(struct platform_device *pdev)
dp->video_info = pdata->video_info;
- if (pdev->dev.of_node) {
- if (dp->phy_addr)
- exynos_dp_phy_init(dp);
- } else {
- if (pdata->phy_init)
- pdata->phy_init();
- }
+ phy_power_on(dp->phy);
exynos_dp_init_dp(dp);
@@ -1121,42 +1047,27 @@ static int exynos_dp_probe(struct platform_device *pdev)
static int exynos_dp_remove(struct platform_device *pdev)
{
- struct exynos_dp_platdata *pdata = pdev->dev.platform_data;
struct exynos_dp_device *dp = platform_get_drvdata(pdev);
flush_work(&dp->hotplug_work);
- if (pdev->dev.of_node) {
- if (dp->phy_addr)
- exynos_dp_phy_exit(dp);
- } else {
- if (pdata->phy_exit)
- pdata->phy_exit();
- }
+ phy_power_off(dp->phy);
clk_disable_unprepare(dp->clock);
-
return 0;
}
#ifdef CONFIG_PM_SLEEP
static int exynos_dp_suspend(struct device *dev)
{
- struct exynos_dp_platdata *pdata = dev->platform_data;
struct exynos_dp_device *dp = dev_get_drvdata(dev);
disable_irq(dp->irq);
flush_work(&dp->hotplug_work);
- if (dev->of_node) {
- if (dp->phy_addr)
- exynos_dp_phy_exit(dp);
- } else {
- if (pdata->phy_exit)
- pdata->phy_exit();
- }
+ phy_power_off(dp->phy);
clk_disable_unprepare(dp->clock);
@@ -1165,16 +1076,9 @@ static int exynos_dp_suspend(struct device *dev)
static int exynos_dp_resume(struct device *dev)
{
- struct exynos_dp_platdata *pdata = dev->platform_data;
struct exynos_dp_device *dp = dev_get_drvdata(dev);
- if (dev->of_node) {
- if (dp->phy_addr)
- exynos_dp_phy_init(dp);
- } else {
- if (pdata->phy_init)
- pdata->phy_init();
- }
+ phy_power_on(dp->phy);
clk_prepare_enable(dp->clock);
diff --git a/drivers/video/exynos/exynos_dp_core.h b/drivers/video/exynos/exynos_dp_core.h
index 6c567bbf..b3d0328 100644
--- a/drivers/video/exynos/exynos_dp_core.h
+++ b/drivers/video/exynos/exynos_dp_core.h
@@ -42,6 +42,8 @@ struct exynos_dp_device {
struct video_info *video_info;
struct link_train link_train;
struct work_struct hotplug_work;
+
+ struct phy *phy;
};
/* exynos_dp_reg.c */
diff --git a/include/video/exynos_dp.h b/include/video/exynos_dp.h
index bd8cabd..f38c9af 100644
--- a/include/video/exynos_dp.h
+++ b/include/video/exynos_dp.h
@@ -122,10 +122,8 @@ struct video_info {
};
struct exynos_dp_platdata {
- struct video_info *video_info;
-
- void (*phy_init)(void);
- void (*phy_exit)(void);
+ struct video_info *video_info;
+ const char *phy_label;
};
#endif /* _EXYNOS_DP_H */
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH v3 1/5] phy: Add driver for Exynos MIPI CSIS/DSIM DPHYs
From: Hui Wang @ 2013-06-28 8:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1372258946-15607-2-git-send-email-s.nawrocki@samsung.com>
On 06/26/2013 11:02 PM, Sylwester Nawrocki wrote:
> Add a PHY provider driver for the Samsung S5P/Exynos SoC MIPI CSI-2
> receiver and MIPI DSI transmitter DPHYs.
>
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
> Changes since v2:
> - adapted to the generic PHY API v9: use phy_set/get_drvdata(),
> - fixed of_xlate callback to return ERR_PTR() instead of NULL,
> - namespace cleanup, put "GPL v2" as MODULE_LICENSE, removed pr_debug,
> - removed phy id check in __set_phy_state().
> ---
[...]
> +
> + if (IS_EXYNOS_MIPI_DSIM_PHY_ID(id))
> + reset = EXYNOS_MIPI_PHY_MRESETN;
> + else
> + reset = EXYNOS_MIPI_PHY_SRESETN;
> +
> + spin_lock_irqsave(&state->slock, flags);
Sorry for one stupid question here, why do you use spin_lock_irqsave()
rather than spin_lock(),
I don't see the irq handler will use this spinlock anywhere in this c file.
Regards,
Hui.
> + reg = readl(addr);
> + if (on)
> + reg |= reset;
> + else
> + reg &= ~reset;
> + writel(reg, addr);
> +
> + /* Clear ENABLE bit only if MRESETN, SRESETN bits are not set. */
> + if (on)
> + reg |= EXYNOS_MIPI_PHY_ENABLE;
> + else if (!(reg & EXYNOS_MIPI_PHY_RESET_MASK))
> + reg &= ~EXYNOS_MIPI_PHY_ENABLE;
> +
> + writel(reg, addr);
> + spin_unlock_irqrestore(&state->slock, flags);
> + return 0;
> +}
>
^ permalink raw reply
* Re: [PATCH V2 1/3] phy: Add driver for Exynos DP PHY
From: Felipe Balbi @ 2013-06-28 9:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <001f01ce73cf$46d8c940$d48a5bc0$@samsung.com>
[-- Attachment #1: Type: text/plain, Size: 300 bytes --]
On Fri, Jun 28, 2013 at 04:15:32PM +0900, Jingoo Han wrote:
> Add a PHY provider driver for the Samsung Exynos SoC DP PHY.
>
> Signed-off-by: Jingoo Han <jg1.han@samsung.com>
Now that you fixed Kishon's concerns, this looks pretty good:
Acked-by: Felipe Balbi <balbi@ti.com>
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH V2 2/3] ARM: dts: Add DP PHY node to exynos5250.dtsi
From: Felipe Balbi @ 2013-06-28 9:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <002001ce73cf$721b9d80$5652d880$@samsung.com>
[-- Attachment #1: Type: text/plain, Size: 1277 bytes --]
On Fri, Jun 28, 2013 at 04:16:44PM +0900, Jingoo Han wrote:
> Add PHY provider node for the DP PHY.
>
> Signed-off-by: Jingoo Han <jg1.han@samsung.com>
> ---
> arch/arm/boot/dts/exynos5250.dtsi | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
> index 41cd625..f7bac75 100644
> --- a/arch/arm/boot/dts/exynos5250.dtsi
> +++ b/arch/arm/boot/dts/exynos5250.dtsi
> @@ -614,6 +614,12 @@
> interrupts = <0 94 0>;
> };
>
> + dp_phy: video-phy@10040720 {
> + compatible = "samsung,exynos5250-dp-video-phy";
> + reg = <0x10040720 4>;
> + #phy-cells = <0>;
> + };
> +
> dp-controller {
> compatible = "samsung,exynos5-dp";
> reg = <0x145b0000 0x1000>;
> @@ -623,11 +629,8 @@
> clock-names = "dp";
> #address-cells = <1>;
> #size-cells = <0>;
> -
> - dptx-phy {
> - reg = <0x10040720>;
> - samsung,enable-mask = <1>;
> - };
> + phys = <&dp_phy 0>;
phy-cells being 0, means that this would become:
phys = <&dp_phy>;
> + phy-names = "dp";
for the label, I would use something more descriptive such as
'display-port'.
other than that:
Acked-by: Felipe Balbi <balbi@ti.com>
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH V2 3/3] video: exynos_dp: Use the generic PHY driver
From: Felipe Balbi @ 2013-06-28 9:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <002101ce73cf$ac989b70$05c9d250$@samsung.com>
[-- Attachment #1: Type: text/plain, Size: 3839 bytes --]
On Fri, Jun 28, 2013 at 04:18:23PM +0900, Jingoo Han wrote:
> Use the generic PHY API instead of the platform callback to control
> the DP PHY. The 'phy_label' field is added to the platform data
> structure to allow PHY lookup on non-dt platforms.
>
> Signed-off-by: Jingoo Han <jg1.han@samsung.com>
> ---
> .../devicetree/bindings/video/exynos_dp.txt | 17 ---
> drivers/video/exynos/exynos_dp_core.c | 118 ++------------------
> drivers/video/exynos/exynos_dp_core.h | 2 +
> include/video/exynos_dp.h | 6 +-
> 4 files changed, 15 insertions(+), 128 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/video/exynos_dp.txt b/Documentation/devicetree/bindings/video/exynos_dp.txt
> index 84f10c1..a8320e3 100644
> --- a/Documentation/devicetree/bindings/video/exynos_dp.txt
> +++ b/Documentation/devicetree/bindings/video/exynos_dp.txt
> @@ -1,17 +1,6 @@
> The Exynos display port interface should be configured based on
> the type of panel connected to it.
>
> -We use two nodes:
> - -dp-controller node
> - -dptx-phy node(defined inside dp-controller node)
> -
> -For the DP-PHY initialization, we use the dptx-phy node.
> -Required properties for dptx-phy:
> - -reg:
> - Base address of DP PHY register.
> - -samsung,enable-mask:
> - The bit-mask used to enable/disable DP PHY.
> -
> For the Panel initialization, we read data from dp-controller node.
> Required properties for dp-controller:
> -compatible:
> @@ -67,12 +56,6 @@ SOC specific portion:
> interrupt-parent = <&combiner>;
> clocks = <&clock 342>;
> clock-names = "dp";
> -
> - dptx-phy {
> - reg = <0x10040720>;
> - samsung,enable-mask = <1>;
> - };
> -
> };
>
> Board Specific portion:
> diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
> index 12bbede..bac515b 100644
> --- a/drivers/video/exynos/exynos_dp_core.c
> +++ b/drivers/video/exynos/exynos_dp_core.c
> @@ -19,6 +19,7 @@
> #include <linux/interrupt.h>
> #include <linux/delay.h>
> #include <linux/of.h>
> +#include <linux/phy/phy.h>
>
> #include <video/exynos_dp.h>
>
> @@ -960,84 +961,15 @@ static struct exynos_dp_platdata *exynos_dp_dt_parse_pdata(struct device *dev)
> return ERR_PTR(-EINVAL);
> }
>
> - return pd;
> -}
> -
> -static int exynos_dp_dt_parse_phydata(struct exynos_dp_device *dp)
> -{
> - struct device_node *dp_phy_node = of_node_get(dp->dev->of_node);
> - u32 phy_base;
> - int ret = 0;
> -
> - dp_phy_node = of_find_node_by_name(dp_phy_node, "dptx-phy");
> - if (!dp_phy_node) {
> - dev_err(dp->dev, "could not find dptx-phy node\n");
> - return -ENODEV;
> - }
> -
> - if (of_property_read_u32(dp_phy_node, "reg", &phy_base)) {
> - dev_err(dp->dev, "failed to get reg for dptx-phy\n");
> - ret = -EINVAL;
> - goto err;
> - }
> -
> - if (of_property_read_u32(dp_phy_node, "samsung,enable-mask",
> - &dp->enable_mask)) {
> - dev_err(dp->dev, "failed to get enable-mask for dptx-phy\n");
> - ret = -EINVAL;
> - goto err;
> - }
> -
> - dp->phy_addr = ioremap(phy_base, SZ_4);
> - if (!dp->phy_addr) {
> - dev_err(dp->dev, "failed to ioremap dp-phy\n");
> - ret = -ENOMEM;
> - goto err;
> - }
> -
> -err:
> - of_node_put(dp_phy_node);
> -
> - return ret;
> -}
> -
> -static void exynos_dp_phy_init(struct exynos_dp_device *dp)
> -{
> - u32 reg;
> -
> - reg = __raw_readl(dp->phy_addr);
> - reg |= dp->enable_mask;
> - __raw_writel(reg, dp->phy_addr);
> -}
> -
> -static void exynos_dp_phy_exit(struct exynos_dp_device *dp)
> -{
> - u32 reg;
> + pd->phy_label = "dp";
only the label, which I would use 'display-port'. Other than that:
Acked-by: Felipe Balbi <balbi@ti.com>
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH 3/3] video: exynos_dp: Use the generic PHY driver
From: Kishon Vijay Abraham I @ 2013-06-28 9:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <001c01ce73c5$552e1cc0$ff8a5640$@samsung.com>
Hi,
On Friday 28 June 2013 11:34 AM, Jingoo Han wrote:
> On Friday, June 28, 2013 2:58 PM, Kishon Vijay Abraham I wrote:
>>
>> Hi,
>>
>> On Friday 28 June 2013 10:54 AM, Jingoo Han wrote:
>>> Use the generic PHY API instead of the platform callback to control
>>> the DP PHY. The 'phy_label' field is added to the platform data
>>> structure to allow PHY lookup on non-dt platforms.
>>>
>>> Signed-off-by: Jingoo Han <jg1.han@samsung.com>
>>> ---
>>> .../devicetree/bindings/video/exynos_dp.txt | 17 ---
>>> drivers/video/exynos/exynos_dp_core.c | 118 ++------------------
>>> drivers/video/exynos/exynos_dp_core.h | 2 +
>>> include/video/exynos_dp.h | 6 +-
>>> 4 files changed, 15 insertions(+), 128 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/video/exynos_dp.txt
>> b/Documentation/devicetree/bindings/video/exynos_dp.txt
>>> index 84f10c1..a8320e3 100644
>>> --- a/Documentation/devicetree/bindings/video/exynos_dp.txt
>>> +++ b/Documentation/devicetree/bindings/video/exynos_dp.txt
>>> @@ -1,17 +1,6 @@
>>> The Exynos display port interface should be configured based on
>>> the type of panel connected to it.
>>>
>>> -We use two nodes:
>>> - -dp-controller node
>>> - -dptx-phy node(defined inside dp-controller node)
>>> -
>>> -For the DP-PHY initialization, we use the dptx-phy node.
>>> -Required properties for dptx-phy:
>>> - -reg:
>>> - Base address of DP PHY register.
>>> - -samsung,enable-mask:
>>> - The bit-mask used to enable/disable DP PHY.
>>> -
>>> For the Panel initialization, we read data from dp-controller node.
>>> Required properties for dp-controller:
>>> -compatible:
>>> @@ -67,12 +56,6 @@ SOC specific portion:
>>> interrupt-parent = <&combiner>;
>>> clocks = <&clock 342>;
>>> clock-names = "dp";
>>> -
>>> - dptx-phy {
>>> - reg = <0x10040720>;
>>> - samsung,enable-mask = <1>;
>>> - };
>>> -
>>> };
>>>
>>> Board Specific portion:
>>> diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
>>> index 12bbede..bac515b 100644
>>> --- a/drivers/video/exynos/exynos_dp_core.c
>>> +++ b/drivers/video/exynos/exynos_dp_core.c
>>> @@ -19,6 +19,7 @@
>>> #include <linux/interrupt.h>
>>> #include <linux/delay.h>
>>> #include <linux/of.h>
>>> +#include <linux/phy/phy.h>
>>>
>>> #include <video/exynos_dp.h>
>>>
>>> @@ -960,84 +961,15 @@ static struct exynos_dp_platdata *exynos_dp_dt_parse_pdata(struct device *dev)
>>> return ERR_PTR(-EINVAL);
>>> }
>>>
>>> - return pd;
>>> -}
>>> -
>>> -static int exynos_dp_dt_parse_phydata(struct exynos_dp_device *dp)
>>> -{
>>> - struct device_node *dp_phy_node = of_node_get(dp->dev->of_node);
>>> - u32 phy_base;
>>> - int ret = 0;
>>> -
>>> - dp_phy_node = of_find_node_by_name(dp_phy_node, "dptx-phy");
>>> - if (!dp_phy_node) {
>>> - dev_err(dp->dev, "could not find dptx-phy node\n");
>>> - return -ENODEV;
>>> - }
>>> -
>>> - if (of_property_read_u32(dp_phy_node, "reg", &phy_base)) {
>>> - dev_err(dp->dev, "failed to get reg for dptx-phy\n");
>>> - ret = -EINVAL;
>>> - goto err;
>>> - }
>>> -
>>> - if (of_property_read_u32(dp_phy_node, "samsung,enable-mask",
>>> - &dp->enable_mask)) {
>>> - dev_err(dp->dev, "failed to get enable-mask for dptx-phy\n");
>>> - ret = -EINVAL;
>>> - goto err;
>>> - }
>>> -
>>> - dp->phy_addr = ioremap(phy_base, SZ_4);
>>> - if (!dp->phy_addr) {
>>> - dev_err(dp->dev, "failed to ioremap dp-phy\n");
>>> - ret = -ENOMEM;
>>> - goto err;
>>> - }
>>> -
>>> -err:
>>> - of_node_put(dp_phy_node);
>>> -
>>> - return ret;
>>> -}
>>> -
>>> -static void exynos_dp_phy_init(struct exynos_dp_device *dp)
>>> -{
>>> - u32 reg;
>>> -
>>> - reg = __raw_readl(dp->phy_addr);
>>> - reg |= dp->enable_mask;
>>> - __raw_writel(reg, dp->phy_addr);
>>> -}
>>> -
>>> -static void exynos_dp_phy_exit(struct exynos_dp_device *dp)
>>> -{
>>> - u32 reg;
>>> + pd->phy_label = "dp";
>>
>> In the case of non-dt boot, this phy_label should have ideally come from
>> platform code.
>
> No, this is NOT the case of non-dt.
>
> 'pd->phy_label = "dp";' is included in exynos_dp_dt_parse_pdata(),
> not exynos_dp_phy_exit().
> Also, exynos_dp_dt_parse_pdata() is called in the case of dt.
ah.. right. Do you support non-dt boot. I dont see any modifications in
the platform code?
Thanks
Kishon
^ permalink raw reply
* Re: [PATCH 1/3] phy: Add driver for Exynos DP PHY
From: Kishon Vijay Abraham I @ 2013-06-28 9:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <001501ce73bf$87c49c00$974dd400$@samsung.com>
Hi,
On Friday 28 June 2013 10:52 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>
> ---
> .../phy/samsung,exynos5250-dp-video-phy.txt | 7 ++
> drivers/phy/Kconfig | 8 ++
> drivers/phy/Makefile | 3 +-
> drivers/phy/phy-exynos-dp-video.c | 130 ++++++++++++++++++++
> 4 files changed, 147 insertions(+), 1 deletion(-)
> create mode 100644 Documentation/devicetree/bindings/phy/samsung,exynos5250-dp-video-phy.txt
> create mode 100644 drivers/phy/phy-exynos-dp-video.c
>
> diff --git a/Documentation/devicetree/bindings/phy/samsung,exynos5250-dp-video-phy.txt
> b/Documentation/devicetree/bindings/phy/samsung,exynos5250-dp-video-phy.txt
> new file mode 100644
> index 0000000..8b6fa79
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/samsung,exynos5250-dp-video-phy.txt
How about creating a single Documentation file for all samsung video phys?
Sylwester?
Thanks
Kishon
^ permalink raw reply
* Re: [RFC 3/6] drm: add SimpleDRM driver
From: David Herrmann @ 2013-06-28 9:59 UTC (permalink / raw)
To: Andy Lutomirski
Cc: dri-devel@lists.freedesktop.org, linux-kernel, Dave Airlie,
linux-fbdev, Stephen Warren, Olof Johansson
In-Reply-To: <51C8ECC7.4030404@mit.edu>
Hi
On Tue, Jun 25, 2013 at 3:05 AM, Andy Lutomirski <luto@amacapital.net> wrote:
> On 06/24/2013 03:27 PM, David Herrmann wrote:
>> + sdrm->fb_map = ioremap(sdrm->fb_base, sdrm->fb_size);
>
> This should probably be ioremap_wc. Otherwise it will be *really* slow
> if used in legacy mode and it may cause conflicts with the
> pgprot_writecombine mode for mmap.
Whoops, yepp, I fixed that.
Thanks
David
^ permalink raw reply
* Re: [RFC 3/6] drm: add SimpleDRM driver
From: David Herrmann @ 2013-06-28 10:01 UTC (permalink / raw)
To: Stephen Warren
Cc: dri-devel@lists.freedesktop.org, linux-kernel, Dave Airlie,
linux-fbdev, Stephen Warren, Olof Johansson
In-Reply-To: <51CB55DB.6040306@wwwdotorg.org>
Hi
On Wed, Jun 26, 2013 at 10:58 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 06/24/2013 04:27 PM, David Herrmann wrote:
>> The SimpleDRM driver binds to simple-framebuffer devices and provides a
>> DRM/KMS API. It provides only a single CRTC+encoder+connector combination
>> plus one initial mode.
>>
>> Userspace can create one dumb-buffer and attach it to the CRTC. Only if
>> the buffer is destroyed, a new buffer can be created. The buffer is
>> directly mapped into user-space, so we have only resources for a single
>> buffer. Otherwise, shadow buffers plus damage-request would be needed.
>
>> diff --git a/drivers/gpu/drm/simpledrm/Kconfig b/drivers/gpu/drm/simpledrm/Kconfig
>
>> +config DRM_SIMPLEDRM
>> + tristate "Simple firmware framebuffer DRM driver"
>> + depends on DRM && !FB_SIMPLE
>> + help
>> + SimpleDRM can run on all systems with pre-initialized graphics
>> + hardware. It uses a framebuffer that was initialized during
>> + firmware boot. No page-flipping, modesetting or other advanced
>> + features are available. However, other DRM drivers can be loaded
>> + later and take over from SimpleDRM if they provide real hardware
>> + support.
>> +
>> + SimpleDRM supports: "simple-framebuffer" DeviceTree objects, x86 VESA
>> + BIOS Extensions (VBE), EFI framebuffers
>
> DT objects, yes. I'm not sure it's quite true to say it actually
> directly supports VBE or EFI FBs; it's more the code in patch 2/6 that
> supports those. I guess this is a bit nit-picky of a distinction though.
I initially intended to add support for "platform-framebuffer"
objects, too. This would make vesafb/... obsolete but I dropped that
idea for now. I will fix the Kconfig description.
>> diff --git a/drivers/gpu/drm/simpledrm/simpledrm_drv.c b/drivers/gpu/drm/simpledrm/simpledrm_drv.c
>
>> +static int parse_dt(struct platform_device *pdev,
>> + struct simplefb_platform_data *mode)
>
>> + strlcpy(mode->format, format, sizeof(mode->format));
>
> Even here, I believe the DT data sticks around so just copying the
> pointer should be safe. It'd be worth validating that for sure though.
Yep, indeed, I fixed that.
Thanks!
David
> I didn't review the DRM stuff here since I'm not at all familiar with DRM.
^ permalink raw reply
* Re: [PATCH 1/3] phy: Add driver for Exynos DP PHY
From: Sylwester Nawrocki @ 2013-06-28 10:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <51CD57EF.5010808@ti.com>
Hi,
On 06/28/2013 11:31 AM, Kishon Vijay Abraham I wrote:
>> diff --git a/Documentation/devicetree/bindings/phy/samsung,exynos5250-dp-video-phy.txt
>> b/Documentation/devicetree/bindings/phy/samsung,exynos5250-dp-video-phy.txt
>> new file mode 100644
>> index 0000000..8b6fa79
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/phy/samsung,exynos5250-dp-video-phy.txt
>
> How about creating a single Documentation file for all samsung video phys?
> Sylwester?
Yes, makes sense. There are quite a few various PHYs on the Exynos SoC.
Let me resend my series with the binding description file name changed
to samsung-phy.txt. I need to add couple fixes to that series anyway.
Regards,
Sylwester
^ permalink raw reply
* Re: [RFC 1/6] fbdev: simplefb: add init through platform_data
From: David Herrmann @ 2013-06-28 10:03 UTC (permalink / raw)
To: Stephen Warren
Cc: dri-devel@lists.freedesktop.org, linux-kernel, Dave Airlie,
linux-fbdev, Stephen Warren, Olof Johansson
In-Reply-To: <51CB5176.5000404@wwwdotorg.org>
Hi
On Wed, Jun 26, 2013 at 10:39 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 06/24/2013 04:27 PM, David Herrmann wrote:
>> If we create proper platform-devices in x86 boot-code, we can use simplefb
>> for VBE or EFI framebuffers, too. However, there is normally no OF support
>> so we introduce a platform_data object so x86 boot-code can pass the
>> paramaters via plain old platform-data.
>>
>> This also removes the OF dependency as it is not needed. The headers
>> provide proper dummies for the case OF is disabled.
>>
>> Furthermore, we move the FORMAT-definitions to the common platform header
>> so initialization code can use it to transform "struct screen_info" to
>> the right format-name.
>
>> diff --git a/include/linux/platform_data/simplefb.h b/include/linux/platform_data/simplefb.h
>
>> +/* the framebuffer size and location is available as IORESOURCE_MEM */
>> +struct simplefb_platform_data {
>> + u32 width;
>> + u32 height;
>> + u32 stride;
>> + char format[64];
>> +};
>
> Any reason not to make format:
>
> const char *format;
>
> You should be able to initialize that just as easily in platform code,
> either as static data or at runtime, I think.
That makes sense. I fixed it up.
Thanks
David
^ permalink raw reply
* Re: [PATCH v3 1/5] phy: Add driver for Exynos MIPI CSIS/DSIM DPHYs
From: Sylwester Nawrocki @ 2013-06-28 10:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <51CD4698.3070409@gmail.com>
On 06/28/2013 10:17 AM, Hui Wang wrote:
> On 06/26/2013 11:02 PM, Sylwester Nawrocki wrote:
>> Add a PHY provider driver for the Samsung S5P/Exynos SoC MIPI CSI-2
>> receiver and MIPI DSI transmitter DPHYs.
>>
>> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>> ---
>> Changes since v2:
>> - adapted to the generic PHY API v9: use phy_set/get_drvdata(),
>> - fixed of_xlate callback to return ERR_PTR() instead of NULL,
>> - namespace cleanup, put "GPL v2" as MODULE_LICENSE, removed pr_debug,
>> - removed phy id check in __set_phy_state().
>> ---
> [...]
>> +
>> + if (IS_EXYNOS_MIPI_DSIM_PHY_ID(id))
>> + reset = EXYNOS_MIPI_PHY_MRESETN;
>> + else
>> + reset = EXYNOS_MIPI_PHY_SRESETN;
>> +
>> + spin_lock_irqsave(&state->slock, flags);
>
> Sorry for one stupid question here, why do you use spin_lock_irqsave()
> rather than spin_lock(),
> I don't see the irq handler will use this spinlock anywhere in this c file.
Yes, there is no chance the PHY users could call the phy ops from within
an interrupt context. Especially now when there is a per phy object
mutex used in the PHY operation helpers. So I'll replace it with plain
spin_lock/unlock. Thank you for the review.
Regards,
Sylwester
^ permalink raw reply
* Re: [RFC 2/6] x86: provide platform-devices for boot-framebuffers
From: David Herrmann @ 2013-06-28 10:11 UTC (permalink / raw)
To: Stephen Warren
Cc: dri-devel@lists.freedesktop.org, linux-kernel, Dave Airlie,
linux-fbdev, Stephen Warren, Olof Johansson
In-Reply-To: <51CB53D7.7030602@wwwdotorg.org>
Hi
On Wed, Jun 26, 2013 at 10:49 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 06/24/2013 04:27 PM, David Herrmann wrote:
>> The current situation regarding boot-framebuffers (VGA, VESA/VBE, EFI) on
>> x86 causes troubles when loading multiple fbdev drivers. The global
>> "struct screen_info" does not provide any state-tracking about which
>> drivers use the FBs. request_mem_region() theoretically works, but
>> unfortunately vesafb/efifb ignore it due to quirks for broken boards.
>>
>> Avoid this by creating a "platform-framebuffer" device with a pointer
>> to the "struct screen_info" as platform-data. Drivers can now create
>> platform-drivers and the driver-core will refuse multiple drivers being
>> active simultaneously.
>>
>> We keep the screen_info available for backwards-compatibility. Drivers
>> can be converted in follow-up patches.
>>
>> Apart from "platform-framebuffer" devices, this also introduces a
>> compatibility option for "simple-framebuffer" drivers which recently got
>> introduced for OF based systems. If CONFIG_X86_SYSFB is selected, we
>> try to match the screen_info against a simple-framebuffer supported
>> format. If we succeed, we create a "simple-framebuffer" device instead
>> of a platform-framebuffer.
>> This allows to reuse the simplefb.c driver across architectures and also
>> to introduce a SimpleDRM driver. There is no need to have vesafb.c,
>> efifb.c, simplefb.c and more just to have architecture specific quirks
>> in their setup-routines.
>>
>> Instead, we now move the architecture specific quirks into x86-setup and
>> provide a generic simple-framebuffer. For backwards-compatibility (if
>> strange formats are used), we still allow vesafb/efifb to be loaded
>> simultaneously and pick up all remaining devices.
>
>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>
>> +config X86_SYSFB
>> + bool "Mark VGA/VBE/EFI FB as generic system framebuffer"
>> + help
>> + Firmwares often provide initial graphics framebuffers so the BIOS,
>> + bootloader or kernel can show basic video-output during boot for
>> + user-guidance and debugging. Historically, x86 used the VESA BIOS
>> + Extensions and EFI-framebuffers for this, which are mostly limited
>> + to x86. However, a generic system-framebuffer initialization emerged
>> + recently on some non-x86 architectures.
>
> After this patch has been in the kernel a while, that very last won't
> really be true; simplefb won't have been introduced recently. Perhaps
> just delete that one sentence?
It rather belongs in the commit message, right. I will rephrase that.
>> + This option, if enabled, marks VGA/VBE/EFI framebuffers as generic
>> + framebuffers so the new generic system-framebuffer drivers can be
>> + used on x86.
>> +
>> + This breaks any x86-only driver like efifb, vesafb, uvesafb, which
>> + will not work if this is selected.
>
> Doesn't that imply that some form of conflicts or depends ! statement
> should be added here?
There is no real conflict here. You still can use vesafb/... with this
option, but they will not pick up the device. I intend to fix these up
to use "platform-framebuffer" devices instead of globally binding to
"struct screen_info". This way, framebuffers either end up as
simple-framebuffers or platform-framebuffers. This option selects
which device they end up as.
As all non-compatible framebuffers (with incompatible pixel-formats)
always end up as "platform-framebuffer", it still makes sense to use
vesafb as fallback. Hence, I'd not introduce any "conflicts"
dependency here.
Maybe I should rephrase the warning to something that makes clear that
if this option is selected, you need simplefb.c or simpledrm to make
use of these devices.
>> diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
>
>> +obj-y += sysfb.o
>
> I suspect that should be obj-$(CONFIG_X86_SYSFB) += sysfb.o.
No. This patch tries to solve two things: First of all, every
system-framebuffer now gets a "platform-framebuffer" platform-device.
Iff X86_SYSFB is selected, it additionally tries to parse the
framebuffer information as "simple-framebuffer". If it succeeds, it
created a "simple-framebuffer" object, if it doesn't, a fallback
"platform-framebuffer" is provided.
This series is missing vesafb/efifb/.. patches, which should now bind
to "platform-framebuffer" devices instead of using "struct
screen_info" directly. I intend to add these in the next round.
>> diff --git a/arch/x86/kernel/sysfb.c b/arch/x86/kernel/sysfb.c
>
>> +#ifdef CONFIG_X86_SYSFB
>
> Rather than ifdef'ing the body of this file, why not create a dummy
> static inline version of add_sysfb() and put that into a header file
> that users include. There should be a header file to prototype the
> function anyway. That way, you avoid all of the ifdefs and static inline
> functions in this file.
>
>> +static bool parse_mode(const struct screen_info *si,
>> + struct simplefb_platform_data *mode)
>
>> + strlcpy(mode->format, f->name, sizeof(mode->format));
>
> Per my comments about the type of mode->format, I think that could just be:
>
> mode->format = f->name;
>
> ... since formats[] (i.e. f) isn't initdata.
>
>> +#else /* CONFIG_X86_SYSFB */
>> +
>> +static bool parse_mode(const struct screen_info *si,
>> + struct simplefb_platform_data *mode)
>> +{
>> + return false;
>> +}
>> +
>> +static int create_simplefb(const struct screen_info *si,
>> + const struct simplefb_platform_data *mode)
>> +{
>> + return -EINVAL;
>> +}
>> +
>> +#endif /* CONFIG_X86_SYSFB */
>
> Following on from my ifdef comment above, I believe those versions of
> those functions will always cause add_sysfb() to return -ENODEV, so you
> may as well provide a static inline for add_sysfb() instead.
No. add_sysfb() is supposed to always succeed. However, if
parse_mode/create_simplefb fail, it creates a "platform-framebuffer"
as fallback. I don't see any way to avoid these ifdefs. Considering
the explanation above, could you elaborate how you think this should
work?
Thanks
David
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox