Message ID | 1396967856-27470-2-git-send-email-t.stanislaws@samsung.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Hi Tomasz, On 04/08/2014 04:37 PM, Tomasz Stanislawski wrote: > Add exynos-simple-phy driver to support a single register > PHY interfaces present on Exynos4 SoC. > > Signed-off-by: Tomasz Stanislawski <t.stanislaws@samsung.com> > --- > .../devicetree/bindings/phy/samsung-phy.txt | 24 +++ > drivers/phy/Kconfig | 5 + > drivers/phy/Makefile | 1 + > drivers/phy/exynos-simple-phy.c | 154 ++++++++++++++++++++ > 4 files changed, 184 insertions(+) > create mode 100644 drivers/phy/exynos-simple-phy.c > > diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt b/Documentation/devicetree/bindings/phy/samsung-phy.txt > index b422e38..f97c4c3 100644 > --- a/Documentation/devicetree/bindings/phy/samsung-phy.txt > +++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt > @@ -114,3 +114,27 @@ Example: > compatible = "samsung,exynos-sataphy-i2c"; > reg = <0x38>; > }; > + > +Samsung S5P/EXYNOS SoC series SIMPLE PHY > +------------------------------------------------- > + > +Required properties: > +- compatible : should be one of the listed compatibles: > + - "samsung,exynos4210-simple-phy" > + - "samsung,exynos4412-simple-phy" > +- reg : offset and length of the register set; > +- #phy-cells : from the generic phy bindings, must be 1; > + > +For "samsung,exynos4210-simple-phy" compatible PHYs the second cell in > +the PHY specifier identifies the PHY and its meaning is as follows: > + 0 - HDMI PHY, > + 1 - DAC PHY, > + 2 - ADC PHY, > + 3 - PCIE PHY. > + 4 - SATA PHY. > + > +For "samsung,exynos4412-simple-phy" compatible PHYs the second cell in > +the PHY specifier identifies the PHY and its meaning is as follows: > + 0 - HDMI PHY, > + 1 - ADC PHY, What about using preprocessor macros? > + > diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig > index 3bb05f1..65ab783 100644 > --- a/drivers/phy/Kconfig > +++ b/drivers/phy/Kconfig > @@ -166,4 +166,9 @@ config PHY_XGENE > help > This option enables support for APM X-Gene SoC multi-purpose PHY. > > +config EXYNOS_SIMPLE_PHY > + tristate "Exynos Simple PHY driver" > + help > + Support for 1-bit PHY controllers on SoCs from Exynos family. > + > endmenu > diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile > index 2faf78e..88c5b60 100644 > --- a/drivers/phy/Makefile > +++ b/drivers/phy/Makefile > @@ -18,3 +18,4 @@ obj-$(CONFIG_PHY_EXYNOS4210_USB2) += phy-exynos4210-usb2.o > obj-$(CONFIG_PHY_EXYNOS4X12_USB2) += phy-exynos4x12-usb2.o > obj-$(CONFIG_PHY_EXYNOS5250_USB2) += phy-exynos5250-usb2.o > obj-$(CONFIG_PHY_XGENE) += phy-xgene.o > +obj-$(CONFIG_EXYNOS_SIMPLE_PHY) += exynos-simple-phy.o > diff --git a/drivers/phy/exynos-simple-phy.c b/drivers/phy/exynos-simple-phy.c > new file mode 100644 > index 0000000..57ad338 > --- /dev/null > +++ b/drivers/phy/exynos-simple-phy.c > @@ -0,0 +1,154 @@ > +/* > + * Exynos Simple PHY driver > + * > + * Copyright (C) 2013 Samsung Electronics Co., Ltd. > + * Author: Tomasz Stanislawski <t.stanislaws@samsung.com> > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + */ > + > +#include <linux/io.h> > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/of_address.h> > +#include <linux/of_device.h> > +#include <linux/phy/phy.h> > + > +#define EXYNOS_PHY_ENABLE (1 << 0) > + > +static int exynos_phy_power_on(struct phy *phy) > +{ > + void __iomem *reg = phy_get_drvdata(phy); > + u32 val; > + > + val = readl(reg); > + val |= EXYNOS_PHY_ENABLE; > + writel(val, reg); > + > + return 0; > +} > + > +static int exynos_phy_power_off(struct phy *phy) > +{ > + void __iomem *reg = phy_get_drvdata(phy); > + u32 val; > + > + val = readl(reg); > + val &= ~EXYNOS_PHY_ENABLE; > + writel(val, reg); > + > + return 0; > +} > + > +static struct phy_ops exynos_phy_ops = { > + .power_on = exynos_phy_power_on, > + .power_off = exynos_phy_power_off, > + .owner = THIS_MODULE, > +}; > + > +static const u32 exynos4210_offsets[] = { > + 0x0700, /* HDMI_PHY */ > + 0x070C, /* DAC_PHY */ > + 0x0718, /* ADC_PHY */ > + 0x071C, /* PCIE_PHY */ > + 0x0720, /* SATA_PHY */ > + ~0, /* end mark */ > +}; > + > +static const u32 exynos4412_offsets[] = { > + 0x0700, /* HDMI_PHY */ > + 0x0718, /* ADC_PHY */ > + ~0, /* end mark */ > +}; Why have you selected only these registers? According to specs Exynos 4210 has 9 and 4412 has 7 control registers with 'phy-enable' functionality. I guess MIPI would require little more work as it has also reset bits, but it will be still better than separate driver. > + > +static const struct of_device_id exynos_phy_of_match[] = { > + { .compatible = "samsung,exynos4210-simple-phy", > + .data = exynos4210_offsets}, > + { .compatible = "samsung,exynos4412-simple-phy", > + .data = exynos4412_offsets}, > + { }, > +}; > +MODULE_DEVICE_TABLE(of, exynos_phy_of_match); > + > +static struct phy *exynos_phy_xlate(struct device *dev, > + struct of_phandle_args *args) > +{ > + struct phy **phys = dev_get_drvdata(dev); > + int index = args->args[0]; > + int i; > + > + /* verify if index is valid */ > + for (i = 0; i <= index; ++i) > + if (!phys[i]) > + return ERR_PTR(-ENODEV); > + > + return phys[index]; > +} > + > +static int exynos_phy_probe(struct platform_device *pdev) > +{ > + const struct of_device_id *of_id = of_match_device( > + of_match_ptr(exynos_phy_of_match), &pdev->dev); > + const u32 *offsets = of_id->data; > + int count; > + struct device *dev = &pdev->dev; > + struct phy **phys; > + struct resource *res; > + void __iomem *regs; > + int i; > + struct phy_provider *phy_provider; > + > + /* count number of phys to create */ > + for (count = 0; offsets[count] != ~0; ++count) > + ; count = ARRAY_SIZE(offsets) - 1; > + > + phys = devm_kzalloc(dev, (count + 1) * sizeof(phys[0]), GFP_KERNEL); > + if (!phys) > + return -ENOMEM; > + > + dev_set_drvdata(dev, phys); > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + > + regs = devm_ioremap(dev, res->start, res->end - res->start); > + if (!regs) { > + dev_err(dev, "failed to ioremap registers\n"); > + return -EFAULT; > + } Why not devm_ioremap_resource? If not, resource_size function calculates length of resource correctly. Anyway I like the idea of implementing multiple phys in one driver. The only drawback I see is that some phys will be created even there are no consumers for them. To avoid such situation you can try to use lazy approach - create phy only if there is request for it, exynos_phy_xlate callback should allow this. Regards Andrzej > + > + /* NOTE: last entry in phys[] is NULL */ > + for (i = 0; i < count; ++i) { > + phys[i] = devm_phy_create(dev, &exynos_phy_ops, NULL); > + if (IS_ERR(phys[i])) { > + dev_err(dev, "failed to create PHY %d\n", i); > + return PTR_ERR(phys[i]); > + } > + phy_set_drvdata(phys[i], regs + offsets[i]); > + } > + > + phy_provider = devm_of_phy_provider_register(dev, exynos_phy_xlate); > + if (IS_ERR(phy_provider)) { > + dev_err(dev, "failed to register PHY provider\n"); > + return PTR_ERR(phy_provider); > + } > + > + dev_info(dev, "added %d phys\n", count); > + > + return 0; > +} > + > +static struct platform_driver exynos_phy_driver = { > + .probe = exynos_phy_probe, > + .driver = { > + .of_match_table = exynos_phy_of_match, > + .name = "exynos-simple-phy", > + .owner = THIS_MODULE, > + } > +}; > +module_platform_driver(exynos_phy_driver); > + > +MODULE_DESCRIPTION("Exynos Simple PHY driver"); > +MODULE_AUTHOR("Tomasz Stanislawski <t.stanislaws@samsung.com>"); > +MODULE_LICENSE("GPL v2"); > -- 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
Hi Tomasz, On 9 April 2014 14:07, Andrzej Hajda <a.hajda@samsung.com> wrote: > Hi Tomasz, > > On 04/08/2014 04:37 PM, Tomasz Stanislawski wrote: >> Add exynos-simple-phy driver to support a single register >> PHY interfaces present on Exynos4 SoC. >> >> Signed-off-by: Tomasz Stanislawski <t.stanislaws@samsung.com> >> --- >> .../devicetree/bindings/phy/samsung-phy.txt | 24 +++ >> drivers/phy/Kconfig | 5 + >> drivers/phy/Makefile | 1 + >> drivers/phy/exynos-simple-phy.c | 154 ++++++++++++++++++++ >> 4 files changed, 184 insertions(+) >> create mode 100644 drivers/phy/exynos-simple-phy.c >> >> diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt b/Documentation/devicetree/bindings/phy/samsung-phy.txt >> index b422e38..f97c4c3 100644 >> --- a/Documentation/devicetree/bindings/phy/samsung-phy.txt >> +++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt >> @@ -114,3 +114,27 @@ Example: >> compatible = "samsung,exynos-sataphy-i2c"; >> reg = <0x38>; >> }; >> + >> +Samsung S5P/EXYNOS SoC series SIMPLE PHY >> +------------------------------------------------- >> + >> +Required properties: >> +- compatible : should be one of the listed compatibles: >> + - "samsung,exynos4210-simple-phy" >> + - "samsung,exynos4412-simple-phy" >> +- reg : offset and length of the register set; >> +- #phy-cells : from the generic phy bindings, must be 1; >> + >> +For "samsung,exynos4210-simple-phy" compatible PHYs the second cell in >> +the PHY specifier identifies the PHY and its meaning is as follows: >> + 0 - HDMI PHY, >> + 1 - DAC PHY, >> + 2 - ADC PHY, >> + 3 - PCIE PHY. >> + 4 - SATA PHY. >> + >> +For "samsung,exynos4412-simple-phy" compatible PHYs the second cell in >> +the PHY specifier identifies the PHY and its meaning is as follows: >> + 0 - HDMI PHY, >> + 1 - ADC PHY, > > What about using preprocessor macros? > >> + >> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig >> index 3bb05f1..65ab783 100644 >> --- a/drivers/phy/Kconfig >> +++ b/drivers/phy/Kconfig >> @@ -166,4 +166,9 @@ config PHY_XGENE >> help >> This option enables support for APM X-Gene SoC multi-purpose PHY. >> >> +config EXYNOS_SIMPLE_PHY >> + tristate "Exynos Simple PHY driver" >> + help >> + Support for 1-bit PHY controllers on SoCs from Exynos family. >> + >> endmenu >> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile >> index 2faf78e..88c5b60 100644 >> --- a/drivers/phy/Makefile >> +++ b/drivers/phy/Makefile >> @@ -18,3 +18,4 @@ obj-$(CONFIG_PHY_EXYNOS4210_USB2) += phy-exynos4210-usb2.o >> obj-$(CONFIG_PHY_EXYNOS4X12_USB2) += phy-exynos4x12-usb2.o >> obj-$(CONFIG_PHY_EXYNOS5250_USB2) += phy-exynos5250-usb2.o >> obj-$(CONFIG_PHY_XGENE) += phy-xgene.o >> +obj-$(CONFIG_EXYNOS_SIMPLE_PHY) += exynos-simple-phy.o >> diff --git a/drivers/phy/exynos-simple-phy.c b/drivers/phy/exynos-simple-phy.c >> new file mode 100644 >> index 0000000..57ad338 >> --- /dev/null >> +++ b/drivers/phy/exynos-simple-phy.c >> @@ -0,0 +1,154 @@ >> +/* >> + * Exynos Simple PHY driver >> + * >> + * Copyright (C) 2013 Samsung Electronics Co., Ltd. >> + * Author: Tomasz Stanislawski <t.stanislaws@samsung.com> >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License version 2 as >> + * published by the Free Software Foundation. >> + */ >> + >> +#include <linux/io.h> >> +#include <linux/kernel.h> >> +#include <linux/module.h> >> +#include <linux/of.h> >> +#include <linux/of_address.h> >> +#include <linux/of_device.h> >> +#include <linux/phy/phy.h> >> + >> +#define EXYNOS_PHY_ENABLE (1 << 0) >> + >> +static int exynos_phy_power_on(struct phy *phy) >> +{ >> + void __iomem *reg = phy_get_drvdata(phy); >> + u32 val; >> + >> + val = readl(reg); >> + val |= EXYNOS_PHY_ENABLE; >> + writel(val, reg); >> + >> + return 0; >> +} >> + >> +static int exynos_phy_power_off(struct phy *phy) >> +{ >> + void __iomem *reg = phy_get_drvdata(phy); >> + u32 val; >> + >> + val = readl(reg); >> + val &= ~EXYNOS_PHY_ENABLE; >> + writel(val, reg); >> + >> + return 0; >> +} >> + >> +static struct phy_ops exynos_phy_ops = { >> + .power_on = exynos_phy_power_on, >> + .power_off = exynos_phy_power_off, >> + .owner = THIS_MODULE, >> +}; >> + >> +static const u32 exynos4210_offsets[] = { >> + 0x0700, /* HDMI_PHY */ >> + 0x070C, /* DAC_PHY */ >> + 0x0718, /* ADC_PHY */ >> + 0x071C, /* PCIE_PHY */ >> + 0x0720, /* SATA_PHY */ >> + ~0, /* end mark */ >> +}; >> + >> +static const u32 exynos4412_offsets[] = { >> + 0x0700, /* HDMI_PHY */ >> + 0x0718, /* ADC_PHY */ >> + ~0, /* end mark */ >> +}; > > Why have you selected only these registers? > According to specs Exynos 4210 has 9 and 4412 has 7 control registers > with 'phy-enable' functionality. > I guess MIPI would require little more work as it has also reset bits, > but it will be still better than separate driver. > >> + >> +static const struct of_device_id exynos_phy_of_match[] = { >> + { .compatible = "samsung,exynos4210-simple-phy", >> + .data = exynos4210_offsets}, >> + { .compatible = "samsung,exynos4412-simple-phy", >> + .data = exynos4412_offsets}, >> + { }, >> +}; >> +MODULE_DEVICE_TABLE(of, exynos_phy_of_match); >> + >> +static struct phy *exynos_phy_xlate(struct device *dev, >> + struct of_phandle_args *args) >> +{ >> + struct phy **phys = dev_get_drvdata(dev); >> + int index = args->args[0]; >> + int i; >> + >> + /* verify if index is valid */ >> + for (i = 0; i <= index; ++i) >> + if (!phys[i]) >> + return ERR_PTR(-ENODEV); >> + >> + return phys[index]; >> +} >> + >> +static int exynos_phy_probe(struct platform_device *pdev) >> +{ >> + const struct of_device_id *of_id = of_match_device( >> + of_match_ptr(exynos_phy_of_match), &pdev->dev); >> + const u32 *offsets = of_id->data; >> + int count; >> + struct device *dev = &pdev->dev; >> + struct phy **phys; >> + struct resource *res; >> + void __iomem *regs; >> + int i; >> + struct phy_provider *phy_provider; >> + >> + /* count number of phys to create */ >> + for (count = 0; offsets[count] != ~0; ++count) >> + ; > > count = ARRAY_SIZE(offsets) - 1; > >> + >> + phys = devm_kzalloc(dev, (count + 1) * sizeof(phys[0]), GFP_KERNEL); >> + if (!phys) >> + return -ENOMEM; >> + >> + dev_set_drvdata(dev, phys); >> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> + >> + regs = devm_ioremap(dev, res->start, res->end - res->start); >> + if (!regs) { >> + dev_err(dev, "failed to ioremap registers\n"); >> + return -EFAULT; >> + } > > Why not devm_ioremap_resource? If not, resource_size function calculates > length of resource correctly. > > Anyway I like the idea of implementing multiple phys in one driver. > The only drawback I see is that some phys will be created even there are > no consumers for them. To avoid such situation you can try to use > lazy approach - create phy only if there is request for it, > exynos_phy_xlate callback should allow this. > > Regards > Andrzej > Idea looks good. How about keeping compatible which is independent of SoC, something like "samsung,exynos-simple-phy" and provide Reg and Bit through phy provider node. This way we can avoid SoC specific hardcoding in phy driver and don't need to look into dt bindings for each new SoC. We can use syscon interface to access PMU bits like USB phy. PMU is already registered as system controller Regards, Rahul Sharma. >> + >> + /* NOTE: last entry in phys[] is NULL */ >> + for (i = 0; i < count; ++i) { >> + phys[i] = devm_phy_create(dev, &exynos_phy_ops, NULL); >> + if (IS_ERR(phys[i])) { >> + dev_err(dev, "failed to create PHY %d\n", i); >> + return PTR_ERR(phys[i]); >> + } >> + phy_set_drvdata(phys[i], regs + offsets[i]); >> + } >> + >> + phy_provider = devm_of_phy_provider_register(dev, exynos_phy_xlate); >> + if (IS_ERR(phy_provider)) { >> + dev_err(dev, "failed to register PHY provider\n"); >> + return PTR_ERR(phy_provider); >> + } >> + >> + dev_info(dev, "added %d phys\n", count); >> + >> + return 0; >> +} >> + >> +static struct platform_driver exynos_phy_driver = { >> + .probe = exynos_phy_probe, >> + .driver = { >> + .of_match_table = exynos_phy_of_match, >> + .name = "exynos-simple-phy", >> + .owner = THIS_MODULE, >> + } >> +}; >> +module_platform_driver(exynos_phy_driver); >> + >> +MODULE_DESCRIPTION("Exynos Simple PHY driver"); >> +MODULE_AUTHOR("Tomasz Stanislawski <t.stanislaws@samsung.com>"); >> +MODULE_LICENSE("GPL v2"); >> > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- 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
Hi, On 09/04/14 11:12, Rahul Sharma wrote: > Idea looks good. How about keeping compatible which is independent > of SoC, something like "samsung,exynos-simple-phy" and provide Reg > and Bit through phy provider node. This way we can avoid SoC specific > hardcoding in phy driver and don't need to look into dt bindings for > each new SoC. I believe it is a not recommended approach. > We can use syscon interface to access PMU bits like USB phy. > PMU is already registered as system controller Yes, that sounds good. This way we could avoid overlapping memory mapped register regions specified in 'reg' properties in the device tree.
Hi Rahul, On 04/09/2014 11:12 AM, Rahul Sharma wrote: > Hi Tomasz, > > On 9 April 2014 14:07, Andrzej Hajda <a.hajda@samsung.com> wrote: >> Hi Tomasz, >> >> On 04/08/2014 04:37 PM, Tomasz Stanislawski wrote: >>> Add exynos-simple-phy driver to support a single register >>> PHY interfaces present on Exynos4 SoC. >>> >>> Signed-off-by: Tomasz Stanislawski <t.stanislaws@samsung.com> [snip] >>> + >>> + regs = devm_ioremap(dev, res->start, res->end - res->start); >>> + if (!regs) { >>> + dev_err(dev, "failed to ioremap registers\n"); >>> + return -EFAULT; >>> + } >> >> Why not devm_ioremap_resource? If not, resource_size function calculates >> length of resource correctly. >> >> Anyway I like the idea of implementing multiple phys in one driver. >> The only drawback I see is that some phys will be created even there are >> no consumers for them. To avoid such situation you can try to use >> lazy approach - create phy only if there is request for it, >> exynos_phy_xlate callback should allow this. >> >> Regards >> Andrzej >> > > Idea looks good. How about keeping compatible which is independent > of SoC, something like "samsung,exynos-simple-phy" and provide Reg > and Bit through phy provider node. This way we can avoid SoC specific > hardcoding in phy driver and don't need to look into dt bindings for > each new SoC. A very nice idea BUT there is a very strong pressure from DT guys to avoid adding any bit fields/offsets/masks in DT nodes. Hopefully, as long as driver name starts with "exynos-" prefix one can hide SoCs specific tricks deep inside driver code. The idea behind this driver was not to create a generic phy for 1-bit devices but rather to hide SoC-specific issues from client drivers like DRM-HDMI. > > We can use syscon interface to access PMU bits like USB phy. > PMU is already registered as system controller > Ok. I will try to use it in PATCHv3. > Regards, > Rahul Sharma. > Regards, Tomasz Stanislawski -- 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
Hello Andrzej, On 09.04.2014 10:37, Andrzej Hajda wrote: >> +static int exynos_phy_probe(struct platform_device *pdev) >> +{ >> + const struct of_device_id *of_id = of_match_device( >> + of_match_ptr(exynos_phy_of_match), &pdev->dev); >> + const u32 *offsets = of_id->data; >> + int count; >> + struct device *dev = &pdev->dev; >> + struct phy **phys; >> + struct resource *res; >> + void __iomem *regs; >> + int i; >> + struct phy_provider *phy_provider; >> + >> + /* count number of phys to create */ >> + for (count = 0; offsets[count] != ~0; ++count) >> + ; > > count = ARRAY_SIZE(offsets) - 1; u32 *offsets is not an array. Regards, Andreas -- 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
Hi Tomasz, I have tested your patches for exynos5250 and 5420. Works fine. Are you planning to post v3? If you want I can share hand with you for v3. Regards, Rahul Sharma On 9 April 2014 17:17, Andreas Oberritter <obi@saftware.de> wrote: > Hello Andrzej, > > On 09.04.2014 10:37, Andrzej Hajda wrote: >>> +static int exynos_phy_probe(struct platform_device *pdev) >>> +{ >>> + const struct of_device_id *of_id = of_match_device( >>> + of_match_ptr(exynos_phy_of_match), &pdev->dev); >>> + const u32 *offsets = of_id->data; >>> + int count; >>> + struct device *dev = &pdev->dev; >>> + struct phy **phys; >>> + struct resource *res; >>> + void __iomem *regs; >>> + int i; >>> + struct phy_provider *phy_provider; >>> + >>> + /* count number of phys to create */ >>> + for (count = 0; offsets[count] != ~0; ++count) >>> + ; >> >> count = ARRAY_SIZE(offsets) - 1; > > u32 *offsets is not an array. > > Regards, > Andreas > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- 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
Hi Rahul, I will prepare we v3 version. Do you want me to add your patches for exynos5?50 to the patchset? Regards, Tomasz Stanislawski On 04/30/2014 08:37 AM, Rahul Sharma wrote: > Hi Tomasz, > > I have tested your patches for exynos5250 and 5420. Works fine. Are > you planning to post v3? If you want I can share hand with you for v3. > > Regards, > Rahul Sharma > > On 9 April 2014 17:17, Andreas Oberritter <obi@saftware.de> wrote: >> Hello Andrzej, >> >> On 09.04.2014 10:37, Andrzej Hajda wrote: >>>> +static int exynos_phy_probe(struct platform_device *pdev) >>>> +{ >>>> + const struct of_device_id *of_id = of_match_device( >>>> + of_match_ptr(exynos_phy_of_match), &pdev->dev); >>>> + const u32 *offsets = of_id->data; >>>> + int count; >>>> + struct device *dev = &pdev->dev; >>>> + struct phy **phys; >>>> + struct resource *res; >>>> + void __iomem *regs; >>>> + int i; >>>> + struct phy_provider *phy_provider; >>>> + >>>> + /* count number of phys to create */ >>>> + for (count = 0; offsets[count] != ~0; ++count) >>>> + ; >>> >>> count = ARRAY_SIZE(offsets) - 1; >> >> u32 *offsets is not an array. >> >> Regards, >> Andreas >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html > -- 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
Sure (5250, 5420). I will wait for the same to update DT patches, if any. Regards, Rahul Sharma. On 30 April 2014 14:02, Tomasz Stanislawski <t.stanislaws@samsung.com> wrote: > Hi Rahul, > I will prepare we v3 version. > Do you want me to add your patches for exynos5?50 to the patchset? > Regards, > Tomasz Stanislawski > > On 04/30/2014 08:37 AM, Rahul Sharma wrote: >> Hi Tomasz, >> >> I have tested your patches for exynos5250 and 5420. Works fine. Are >> you planning to post v3? If you want I can share hand with you for v3. >> >> Regards, >> Rahul Sharma >> >> On 9 April 2014 17:17, Andreas Oberritter <obi@saftware.de> wrote: >>> Hello Andrzej, >>> >>> On 09.04.2014 10:37, Andrzej Hajda wrote: >>>>> +static int exynos_phy_probe(struct platform_device *pdev) >>>>> +{ >>>>> + const struct of_device_id *of_id = of_match_device( >>>>> + of_match_ptr(exynos_phy_of_match), &pdev->dev); >>>>> + const u32 *offsets = of_id->data; >>>>> + int count; >>>>> + struct device *dev = &pdev->dev; >>>>> + struct phy **phys; >>>>> + struct resource *res; >>>>> + void __iomem *regs; >>>>> + int i; >>>>> + struct phy_provider *phy_provider; >>>>> + >>>>> + /* count number of phys to create */ >>>>> + for (count = 0; offsets[count] != ~0; ++count) >>>>> + ; >>>> >>>> count = ARRAY_SIZE(offsets) - 1; >>> >>> u32 *offsets is not an array. >>> >>> Regards, >>> Andreas >>> -- >>> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in >>> the body of a message to majordomo@vger.kernel.org >>> More majordomo info at http://vger.kernel.org/majordomo-info.html >> > -- 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
Hi, On Wednesday 09 April 2014 03:31 PM, Sylwester Nawrocki wrote: > Hi, > > On 09/04/14 11:12, Rahul Sharma wrote: >> Idea looks good. How about keeping compatible which is independent >> of SoC, something like "samsung,exynos-simple-phy" and provide Reg >> and Bit through phy provider node. This way we can avoid SoC specific >> hardcoding in phy driver and don't need to look into dt bindings for >> each new SoC. > > I believe it is a not recommended approach. Why not? We should try to avoid hard coding in the driver code. Moreover by avoiding hardcoding we can make it a generic driver for single bit PHYs. Cheers 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
On 5 May 2014 15:14, Kishon Vijay Abraham I <kishon@ti.com> wrote: > Hi, > > On Wednesday 09 April 2014 03:31 PM, Sylwester Nawrocki wrote: >> Hi, >> >> On 09/04/14 11:12, Rahul Sharma wrote: >>> Idea looks good. How about keeping compatible which is independent >>> of SoC, something like "samsung,exynos-simple-phy" and provide Reg >>> and Bit through phy provider node. This way we can avoid SoC specific >>> hardcoding in phy driver and don't need to look into dt bindings for >>> each new SoC. >> >> I believe it is a not recommended approach. > > Why not? We should try to avoid hard coding in the driver code. Moreover by > avoiding hardcoding we can make it a generic driver for single bit PHYs. > +1. @Tomasz, any plans to consider this approach for simple phy driver? Regards, Rahul Sharma. > Cheers > 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
On 05/07/2014 12:38 PM, Rahul Sharma wrote: > On 5 May 2014 15:14, Kishon Vijay Abraham I <kishon@ti.com> wrote: >> Hi, >> >> On Wednesday 09 April 2014 03:31 PM, Sylwester Nawrocki wrote: >>> Hi, >>> >>> On 09/04/14 11:12, Rahul Sharma wrote: >>>> Idea looks good. How about keeping compatible which is independent >>>> of SoC, something like "samsung,exynos-simple-phy" and provide Reg >>>> and Bit through phy provider node. This way we can avoid SoC specific >>>> hardcoding in phy driver and don't need to look into dt bindings for >>>> each new SoC. >>> >>> I believe it is a not recommended approach. >> >> Why not? We should try to avoid hard coding in the driver code. Moreover by >> avoiding hardcoding we can make it a generic driver for single bit PHYs. >> > > +1. > > @Tomasz, any plans to consider this approach for simple phy driver? > > Regards, > Rahul Sharma. > Hi Rahul, Initially, I wanted to make a very generic driver and to add bit and register (or its offset) attribute to the PHY node. However, there was a very strong opposition from DT maintainers to adding any bit related configuration to DT. The current solution was designed to be a trade-off between being generic and being accepted :). Regards, Tomasz Stanislawski >> Cheers >> 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
On 7 May 2014 19:06, Tomasz Stanislawski <t.stanislaws@samsung.com> wrote: > On 05/07/2014 12:38 PM, Rahul Sharma wrote: >> On 5 May 2014 15:14, Kishon Vijay Abraham I <kishon@ti.com> wrote: >>> Hi, >>> >>> On Wednesday 09 April 2014 03:31 PM, Sylwester Nawrocki wrote: >>>> Hi, >>>> >>>> On 09/04/14 11:12, Rahul Sharma wrote: >>>>> Idea looks good. How about keeping compatible which is independent >>>>> of SoC, something like "samsung,exynos-simple-phy" and provide Reg >>>>> and Bit through phy provider node. This way we can avoid SoC specific >>>>> hardcoding in phy driver and don't need to look into dt bindings for >>>>> each new SoC. >>>> >>>> I believe it is a not recommended approach. >>> >>> Why not? We should try to avoid hard coding in the driver code. Moreover by >>> avoiding hardcoding we can make it a generic driver for single bit PHYs. >>> >> >> +1. >> >> @Tomasz, any plans to consider this approach for simple phy driver? >> >> Regards, >> Rahul Sharma. >> > > Hi Rahul, > Initially, I wanted to make a very generic driver and to add bit and > register (or its offset) attribute to the PHY node. > However, there was a very strong opposition from DT maintainers > to adding any bit related configuration to DT. > The current solution was designed to be a trade-off between > being generic and being accepted :). > Thanks Tomasz, Ok got it. lets discuss it again and conclude it. @Kishon, DT-folks, The original RFC patch from Tomasz (at https://lkml.org/lkml/2013/10/21/313) added simple phy driver as "Generic-simple-phy" with these properties: + of_property_read_u32(dev->of_node, "mask", &sphy->mask); + of_property_read_u32(dev->of_node, "on-value", &sphy->on_value); + of_property_read_u32(dev->of_node, "off-value", &sphy->off_value); Shall we consider the same solution again for generic simple phy driver which just expose on/off control through register bit. Regards, Rahul Sharma > Regards, > Tomasz Stanislawski > > > >>> Cheers >>> Kishon >> > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- 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
[CCing more DT-folks :)] On 07.05.2014 16:19, Rahul Sharma wrote: > On 7 May 2014 19:06, Tomasz Stanislawski <t.stanislaws@samsung.com> wrote: >> On 05/07/2014 12:38 PM, Rahul Sharma wrote: >>> On 5 May 2014 15:14, Kishon Vijay Abraham I <kishon@ti.com> wrote: >>>> Hi, >>>> >>>> On Wednesday 09 April 2014 03:31 PM, Sylwester Nawrocki wrote: >>>>> Hi, >>>>> >>>>> On 09/04/14 11:12, Rahul Sharma wrote: >>>>>> Idea looks good. How about keeping compatible which is independent >>>>>> of SoC, something like "samsung,exynos-simple-phy" and provide Reg >>>>>> and Bit through phy provider node. This way we can avoid SoC specific >>>>>> hardcoding in phy driver and don't need to look into dt bindings for >>>>>> each new SoC. >>>>> >>>>> I believe it is a not recommended approach. >>>> >>>> Why not? We should try to avoid hard coding in the driver code. Moreover by >>>> avoiding hardcoding we can make it a generic driver for single bit PHYs. >>>> >>> >>> +1. >>> >>> @Tomasz, any plans to consider this approach for simple phy driver? >>> >>> Regards, >>> Rahul Sharma. >>> >> >> Hi Rahul, >> Initially, I wanted to make a very generic driver and to add bit and >> register (or its offset) attribute to the PHY node. >> However, there was a very strong opposition from DT maintainers >> to adding any bit related configuration to DT. >> The current solution was designed to be a trade-off between >> being generic and being accepted :). >> > > Thanks Tomasz, > Ok got it. lets discuss it again and conclude it. > > @Kishon, DT-folks, > > The original RFC patch from Tomasz (at https://lkml.org/lkml/2013/10/21/313) > added simple phy driver as "Generic-simple-phy" with these properties: > > + of_property_read_u32(dev->of_node, "mask", &sphy->mask); > + of_property_read_u32(dev->of_node, "on-value", &sphy->on_value); > + of_property_read_u32(dev->of_node, "off-value", &sphy->off_value); > > Shall we consider the same solution again for generic simple phy > driver which just expose on/off control through register bit. > > Regards, > Rahul Sharma > >> Regards, >> Tomasz Stanislawski >> >> >> >>>> Cheers >>>> Kishon >>> >> >> _______________________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/dri-devel > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Best regards, Tomasz -- 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
<Gentle PING> On 7 May 2014 21:03, Tomasz Figa <t.figa@samsung.com> wrote: > [CCing more DT-folks :)] > > On 07.05.2014 16:19, Rahul Sharma wrote: >> On 7 May 2014 19:06, Tomasz Stanislawski <t.stanislaws@samsung.com> wrote: >>> On 05/07/2014 12:38 PM, Rahul Sharma wrote: >>>> On 5 May 2014 15:14, Kishon Vijay Abraham I <kishon@ti.com> wrote: >>>>> Hi, >>>>> >>>>> On Wednesday 09 April 2014 03:31 PM, Sylwester Nawrocki wrote: >>>>>> Hi, >>>>>> >>>>>> On 09/04/14 11:12, Rahul Sharma wrote: >>>>>>> Idea looks good. How about keeping compatible which is independent >>>>>>> of SoC, something like "samsung,exynos-simple-phy" and provide Reg >>>>>>> and Bit through phy provider node. This way we can avoid SoC specific >>>>>>> hardcoding in phy driver and don't need to look into dt bindings for >>>>>>> each new SoC. >>>>>> >>>>>> I believe it is a not recommended approach. >>>>> >>>>> Why not? We should try to avoid hard coding in the driver code. Moreover by >>>>> avoiding hardcoding we can make it a generic driver for single bit PHYs. >>>>> >>>> >>>> +1. >>>> >>>> @Tomasz, any plans to consider this approach for simple phy driver? >>>> >>>> Regards, >>>> Rahul Sharma. >>>> >>> >>> Hi Rahul, >>> Initially, I wanted to make a very generic driver and to add bit and >>> register (or its offset) attribute to the PHY node. >>> However, there was a very strong opposition from DT maintainers >>> to adding any bit related configuration to DT. >>> The current solution was designed to be a trade-off between >>> being generic and being accepted :). >>> >> >> Thanks Tomasz, >> Ok got it. lets discuss it again and conclude it. >> >> @Kishon, DT-folks, >> >> The original RFC patch from Tomasz (at https://lkml.org/lkml/2013/10/21/313) >> added simple phy driver as "Generic-simple-phy" with these properties: >> >> + of_property_read_u32(dev->of_node, "mask", &sphy->mask); >> + of_property_read_u32(dev->of_node, "on-value", &sphy->on_value); >> + of_property_read_u32(dev->of_node, "off-value", &sphy->off_value); >> >> Shall we consider the same solution again for generic simple phy >> driver which just expose on/off control through register bit. >> >> Regards, >> Rahul Sharma >> >>> Regards, >>> Tomasz Stanislawski >>> >>> >>> >>>>> Cheers >>>>> Kishon >>>> >>> >>> _______________________________________________ >>> dri-devel mailing list >>> dri-devel@lists.freedesktop.org >>> http://lists.freedesktop.org/mailman/listinfo/dri-devel >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> > > Best regards, > Tomasz > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- 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
diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt b/Documentation/devicetree/bindings/phy/samsung-phy.txt index b422e38..f97c4c3 100644 --- a/Documentation/devicetree/bindings/phy/samsung-phy.txt +++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt @@ -114,3 +114,27 @@ Example: compatible = "samsung,exynos-sataphy-i2c"; reg = <0x38>; }; + +Samsung S5P/EXYNOS SoC series SIMPLE PHY +------------------------------------------------- + +Required properties: +- compatible : should be one of the listed compatibles: + - "samsung,exynos4210-simple-phy" + - "samsung,exynos4412-simple-phy" +- reg : offset and length of the register set; +- #phy-cells : from the generic phy bindings, must be 1; + +For "samsung,exynos4210-simple-phy" compatible PHYs the second cell in +the PHY specifier identifies the PHY and its meaning is as follows: + 0 - HDMI PHY, + 1 - DAC PHY, + 2 - ADC PHY, + 3 - PCIE PHY. + 4 - SATA PHY. + +For "samsung,exynos4412-simple-phy" compatible PHYs the second cell in +the PHY specifier identifies the PHY and its meaning is as follows: + 0 - HDMI PHY, + 1 - ADC PHY, + diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig index 3bb05f1..65ab783 100644 --- a/drivers/phy/Kconfig +++ b/drivers/phy/Kconfig @@ -166,4 +166,9 @@ config PHY_XGENE help This option enables support for APM X-Gene SoC multi-purpose PHY. +config EXYNOS_SIMPLE_PHY + tristate "Exynos Simple PHY driver" + help + Support for 1-bit PHY controllers on SoCs from Exynos family. + endmenu diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile index 2faf78e..88c5b60 100644 --- a/drivers/phy/Makefile +++ b/drivers/phy/Makefile @@ -18,3 +18,4 @@ obj-$(CONFIG_PHY_EXYNOS4210_USB2) += phy-exynos4210-usb2.o obj-$(CONFIG_PHY_EXYNOS4X12_USB2) += phy-exynos4x12-usb2.o obj-$(CONFIG_PHY_EXYNOS5250_USB2) += phy-exynos5250-usb2.o obj-$(CONFIG_PHY_XGENE) += phy-xgene.o +obj-$(CONFIG_EXYNOS_SIMPLE_PHY) += exynos-simple-phy.o diff --git a/drivers/phy/exynos-simple-phy.c b/drivers/phy/exynos-simple-phy.c new file mode 100644 index 0000000..57ad338 --- /dev/null +++ b/drivers/phy/exynos-simple-phy.c @@ -0,0 +1,154 @@ +/* + * Exynos Simple PHY driver + * + * Copyright (C) 2013 Samsung Electronics Co., Ltd. + * Author: Tomasz Stanislawski <t.stanislaws@samsung.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include <linux/io.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/of_address.h> +#include <linux/of_device.h> +#include <linux/phy/phy.h> + +#define EXYNOS_PHY_ENABLE (1 << 0) + +static int exynos_phy_power_on(struct phy *phy) +{ + void __iomem *reg = phy_get_drvdata(phy); + u32 val; + + val = readl(reg); + val |= EXYNOS_PHY_ENABLE; + writel(val, reg); + + return 0; +} + +static int exynos_phy_power_off(struct phy *phy) +{ + void __iomem *reg = phy_get_drvdata(phy); + u32 val; + + val = readl(reg); + val &= ~EXYNOS_PHY_ENABLE; + writel(val, reg); + + return 0; +} + +static struct phy_ops exynos_phy_ops = { + .power_on = exynos_phy_power_on, + .power_off = exynos_phy_power_off, + .owner = THIS_MODULE, +}; + +static const u32 exynos4210_offsets[] = { + 0x0700, /* HDMI_PHY */ + 0x070C, /* DAC_PHY */ + 0x0718, /* ADC_PHY */ + 0x071C, /* PCIE_PHY */ + 0x0720, /* SATA_PHY */ + ~0, /* end mark */ +}; + +static const u32 exynos4412_offsets[] = { + 0x0700, /* HDMI_PHY */ + 0x0718, /* ADC_PHY */ + ~0, /* end mark */ +}; + +static const struct of_device_id exynos_phy_of_match[] = { + { .compatible = "samsung,exynos4210-simple-phy", + .data = exynos4210_offsets}, + { .compatible = "samsung,exynos4412-simple-phy", + .data = exynos4412_offsets}, + { }, +}; +MODULE_DEVICE_TABLE(of, exynos_phy_of_match); + +static struct phy *exynos_phy_xlate(struct device *dev, + struct of_phandle_args *args) +{ + struct phy **phys = dev_get_drvdata(dev); + int index = args->args[0]; + int i; + + /* verify if index is valid */ + for (i = 0; i <= index; ++i) + if (!phys[i]) + return ERR_PTR(-ENODEV); + + return phys[index]; +} + +static int exynos_phy_probe(struct platform_device *pdev) +{ + const struct of_device_id *of_id = of_match_device( + of_match_ptr(exynos_phy_of_match), &pdev->dev); + const u32 *offsets = of_id->data; + int count; + struct device *dev = &pdev->dev; + struct phy **phys; + struct resource *res; + void __iomem *regs; + int i; + struct phy_provider *phy_provider; + + /* count number of phys to create */ + for (count = 0; offsets[count] != ~0; ++count) + ; + + phys = devm_kzalloc(dev, (count + 1) * sizeof(phys[0]), GFP_KERNEL); + if (!phys) + return -ENOMEM; + + dev_set_drvdata(dev, phys); + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + + regs = devm_ioremap(dev, res->start, res->end - res->start); + if (!regs) { + dev_err(dev, "failed to ioremap registers\n"); + return -EFAULT; + } + + /* NOTE: last entry in phys[] is NULL */ + for (i = 0; i < count; ++i) { + phys[i] = devm_phy_create(dev, &exynos_phy_ops, NULL); + if (IS_ERR(phys[i])) { + dev_err(dev, "failed to create PHY %d\n", i); + return PTR_ERR(phys[i]); + } + phy_set_drvdata(phys[i], regs + offsets[i]); + } + + phy_provider = devm_of_phy_provider_register(dev, exynos_phy_xlate); + if (IS_ERR(phy_provider)) { + dev_err(dev, "failed to register PHY provider\n"); + return PTR_ERR(phy_provider); + } + + dev_info(dev, "added %d phys\n", count); + + return 0; +} + +static struct platform_driver exynos_phy_driver = { + .probe = exynos_phy_probe, + .driver = { + .of_match_table = exynos_phy_of_match, + .name = "exynos-simple-phy", + .owner = THIS_MODULE, + } +}; +module_platform_driver(exynos_phy_driver); + +MODULE_DESCRIPTION("Exynos Simple PHY driver"); +MODULE_AUTHOR("Tomasz Stanislawski <t.stanislaws@samsung.com>"); +MODULE_LICENSE("GPL v2");
Add exynos-simple-phy driver to support a single register PHY interfaces present on Exynos4 SoC. Signed-off-by: Tomasz Stanislawski <t.stanislaws@samsung.com> --- .../devicetree/bindings/phy/samsung-phy.txt | 24 +++ drivers/phy/Kconfig | 5 + drivers/phy/Makefile | 1 + drivers/phy/exynos-simple-phy.c | 154 ++++++++++++++++++++ 4 files changed, 184 insertions(+) create mode 100644 drivers/phy/exynos-simple-phy.c