diff mbox

[v1,3/5] phy: pci serdes phy driver for keystone

Message ID 1400169692-9677-4-git-send-email-m-karicheri2@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Murali Karicheri May 15, 2014, 4:01 p.m. UTC
This phy driver is used by keystone PCI driver. The hw vendor that
provides the phy hw published only registers and their values. So
this driver uses these hard coded values to initialize the phy.

CC: Grant Likely <grant.likely@linaro.org>
CC: Rob Herring <robh+dt@kernel.org>
CC: Mohit Kumar <mohit.kumar@st.com>
CC: Jingoo Han <jg1.han@samsung.com>
CC: Bjorn Helgaas <bhelgaas@google.com>

Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
---
 drivers/phy/Kconfig        |    6 ++
 drivers/phy/Makefile       |    1 +
 drivers/phy/phy-keystone.c |  230 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 237 insertions(+)
 create mode 100644 drivers/phy/phy-keystone.c

Comments

Arnd Bergmann May 15, 2014, 4:14 p.m. UTC | #1
On Thursday 15 May 2014 12:01:30 Murali Karicheri wrote:
> +static struct serdes_config ks_100mhz_pcie_5gbps_serdes[] = {
> +       {0x0000, 0x00000800, 0x0000ff00},
> +       {0x0060, 0x00041c5c, 0x00ffffff},
> +       {0x0064, 0x0343c700, 0xffffff00},
> +       {0x006c, 0x00000012, 0x000000ff},
> +       {0x0068, 0x00070000, 0x00ff0000},
> +       {0x0078, 0x0000c000, 0x0000ff00},

It looks like the PHY is generic, but the configuration above is
PCI specific. If this is true, you should have #phy-cells=<1>
and document the possible modes, adding a lookup table here to
pick the configuration based on the argument. It's fine to just
implement pcie-5ghz initially, but the binding should list all
the modes that the PHY can support.

Also, please list the exact name of the phy if you can find it
out. You mention that you don't know the register descriptions,
but you should at least be able to let us know what phy this
is, in case some other SoC reuses the same thing.

> +static int ks_phy_init(struct phy *phy)
> +{
> +       struct serdes_config *p;
> +       struct phy_keystone *ks_phy = phy_get_drvdata(phy);
> +
> +       int i;
> +
> +       for (i = 0, p = &ks_100mhz_pcie_5gbps_serdes[0];
> +               i < ARRAY_SIZE(ks_100mhz_pcie_5gbps_serdes);
> +               i++, p++) {
> +               reg_rmw((ks_phy->base + p->reg), p->val, p->mask);
> +               reg_dump((ks_phy->base + p->reg), p->mask);
> +       }
> +       udelay(2000);

This should probably be msleep(2);

	Arnd
Murali Karicheri May 23, 2014, 5:14 p.m. UTC | #2
On 5/15/2014 12:14 PM, Arnd Bergmann wrote:
> On Thursday 15 May 2014 12:01:30 Murali Karicheri wrote:
>> +static struct serdes_config ks_100mhz_pcie_5gbps_serdes[] = {
>> +       {0x0000, 0x00000800, 0x0000ff00},
>> +       {0x0060, 0x00041c5c, 0x00ffffff},
>> +       {0x0064, 0x0343c700, 0xffffff00},
>> +       {0x006c, 0x00000012, 0x000000ff},
>> +       {0x0068, 0x00070000, 0x00ff0000},
>> +       {0x0078, 0x0000c000, 0x0000ff00},
> It looks like the PHY is generic, but the configuration above is
> PCI specific. If this is true, you should have #phy-cells=<1>
> and document the possible modes, adding a lookup table here to
> pick the configuration based on the argument. It's fine to just
> implement pcie-5ghz initially, but the binding should list all
> the modes that the PHY can support.

The PCIE name needs to be dropped and as you correctly guessed, this SERDES
is generic.
>
> Also, please list the exact name of the phy if you can find it
> out. You mention that you don't know the register descriptions,
> but you should at least be able to let us know what phy this
> is, in case some other SoC reuses the same thing.
Unfortunately there is a NDA restriction that prevents us from using the
actual Phy name and keystone phy is the name what we can usel at the 
moment. I am checking
this with our IP team and if original name can  be used, I will update 
the driver to reflect the same.
>> +static int ks_phy_init(struct phy *phy)
>> +{
>> +       struct serdes_config *p;
>> +       struct phy_keystone *ks_phy = phy_get_drvdata(phy);
>> +
>> +       int i;
>> +
>> +       for (i = 0, p = &ks_100mhz_pcie_5gbps_serdes[0];
>> +               i < ARRAY_SIZE(ks_100mhz_pcie_5gbps_serdes);
>> +               i++, p++) {
>> +               reg_rmw((ks_phy->base + p->reg), p->val, p->mask);
>> +               reg_dump((ks_phy->base + p->reg), p->mask);
>> +       }
>> +       udelay(2000);
> This should probably be msleep(2);
Fine.

Murali
>
> 	Arnd
Arnd Bergmann May 23, 2014, 7:23 p.m. UTC | #3
On Friday 23 May 2014 13:14:00 Murali Karicheri wrote:
> On 5/15/2014 12:14 PM, Arnd Bergmann wrote:
> > On Thursday 15 May 2014 12:01:30 Murali Karicheri wrote:
> >> +static struct serdes_config ks_100mhz_pcie_5gbps_serdes[] = {
> >> +       {0x0000, 0x00000800, 0x0000ff00},
> >> +       {0x0060, 0x00041c5c, 0x00ffffff},
> >> +       {0x0064, 0x0343c700, 0xffffff00},
> >> +       {0x006c, 0x00000012, 0x000000ff},
> >> +       {0x0068, 0x00070000, 0x00ff0000},
> >> +       {0x0078, 0x0000c000, 0x0000ff00},
> > It looks like the PHY is generic, but the configuration above is
> > PCI specific. If this is true, you should have #phy-cells=<1>
> > and document the possible modes, adding a lookup table here to
> > pick the configuration based on the argument. It's fine to just
> > implement pcie-5ghz initially, but the binding should list all
> > the modes that the PHY can support.
> 
> The PCIE name needs to be dropped and as you correctly guessed, this SERDES
> is generic.

Ok, good.

> > Also, please list the exact name of the phy if you can find it
> > out. You mention that you don't know the register descriptions,
> > but you should at least be able to let us know what phy this
> > is, in case some other SoC reuses the same thing.
>
> Unfortunately there is a NDA restriction that prevents us from using the
> actual Phy name and keystone phy is the name what we can usel at the 
> moment. I am checking
> this with our IP team and if original name can  be used, I will update 
> the driver to reflect the same.

Can you please check the phy drivers that we already have in linux-next
to see if any of those are for the same hardware then?

I guess it's our best hope to catch duplications by inspecting all
other phy drivers as they get merged then.

	Arnd
Murali Karicheri May 27, 2014, 4:46 p.m. UTC | #4
On 5/23/2014 3:23 PM, Arnd Bergmann wrote:
> On Friday 23 May 2014 13:14:00 Murali Karicheri wrote:
>> On 5/15/2014 12:14 PM, Arnd Bergmann wrote:
>>> On Thursday 15 May 2014 12:01:30 Murali Karicheri wrote:
>>>> +static struct serdes_config ks_100mhz_pcie_5gbps_serdes[] = {
>>>> +       {0x0000, 0x00000800, 0x0000ff00},
>>>> +       {0x0060, 0x00041c5c, 0x00ffffff},
>>>> +       {0x0064, 0x0343c700, 0xffffff00},
>>>> +       {0x006c, 0x00000012, 0x000000ff},
>>>> +       {0x0068, 0x00070000, 0x00ff0000},
>>>> +       {0x0078, 0x0000c000, 0x0000ff00},
>>> It looks like the PHY is generic, but the configuration above is
>>> PCI specific. If this is true, you should have #phy-cells=<1>
>>> and document the possible modes, adding a lookup table here to
>>> pick the configuration based on the argument. It's fine to just
>>> implement pcie-5ghz initially, but the binding should list all
>>> the modes that the PHY can support.
>> The PCIE name needs to be dropped and as you correctly guessed, this SERDES
>> is generic.
> Ok, good.
>
>>> Also, please list the exact name of the phy if you can find it
>>> out. You mention that you don't know the register descriptions,
>>> but you should at least be able to let us know what phy this
>>> is, in case some other SoC reuses the same thing.
>> Unfortunately there is a NDA restriction that prevents us from using the
>> actual Phy name and keystone phy is the name what we can usel at the
>> moment. I am checking
>> this with our IP team and if original name can  be used, I will update
>> the driver to reflect the same.
> Can you please check the phy drivers that we already have in linux-next
> to see if any of those are for the same hardware then?
>
> I guess it's our best hope to catch duplications by inspecting all
> other phy drivers as they get merged then.
>
> 	Arnd
Arnd,

I have checked the register set of the following drivers under 
drivers/phy from the
master branch (v3.15-rc6) using the registers set available with us 
internally and
I can't find a match.

phy-exynos5250-sata.c  phy-exynos-mipi-video.c  phy-omap-usb2.c 
phy-sun4i-usb.c
phy-xgene.c phy-exynos4210-usb2.c  phy-exynos5250-usb2.c phy-mvebu-sata.c
phy-samsung-usb2.c  phy-ti-pipe3.c phy-bcm-kona-usb2.c phy-exynos4x12-usb2.c
  phy-exynos-dp-video.c  phy-omap-control.c  phy-twl4030-usb.c

So there is no duplication.

Murali
Arnd Bergmann May 27, 2014, 6:36 p.m. UTC | #5
On Tuesday 27 May 2014 12:46:54 Murali Karicheri wrote:
> I have checked the register set of the following drivers under 
> drivers/phy from the
> master branch (v3.15-rc6) using the registers set available with us 
> internally and
> I can't find a match.
> 
> phy-exynos5250-sata.c  phy-exynos-mipi-video.c  phy-omap-usb2.c 
> phy-sun4i-usb.c
> phy-xgene.c phy-exynos4210-usb2.c  phy-exynos5250-usb2.c phy-mvebu-sata.c
> phy-samsung-usb2.c  phy-ti-pipe3.c phy-bcm-kona-usb2.c phy-exynos4x12-usb2.c
>   phy-exynos-dp-video.c  phy-omap-control.c  phy-twl4030-usb.c
> 
> So there is no duplication.

Ok, thanks a lot for checking!

	Arnd
Kishon Vijay Abraham I June 2, 2014, 6:16 a.m. UTC | #6
Hi,

On Thursday 15 May 2014 09:31 PM, Murali Karicheri wrote:
> This phy driver is used by keystone PCI driver. The hw vendor that
> provides the phy hw published only registers and their values. So
> this driver uses these hard coded values to initialize the phy.
> 
> CC: Grant Likely <grant.likely@linaro.org>
> CC: Rob Herring <robh+dt@kernel.org>
> CC: Mohit Kumar <mohit.kumar@st.com>
> CC: Jingoo Han <jg1.han@samsung.com>
> CC: Bjorn Helgaas <bhelgaas@google.com>
> 
> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
> ---
>  drivers/phy/Kconfig        |    6 ++
>  drivers/phy/Makefile       |    1 +
>  drivers/phy/phy-keystone.c |  230 ++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 237 insertions(+)
>  create mode 100644 drivers/phy/phy-keystone.c
> 
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index 4906c27..e5f4b5a 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -167,4 +167,10 @@ config PHY_XGENE
>  	help
>  	  This option enables support for APM X-Gene SoC multi-purpose PHY.
>  
> +config PHY_TI_KEYSTONE
> +	bool "TI Keystone PHY support"
> +	depends on ARCH_KEYSTONE
> +	select GENERIC_PHY
> +	help
> +	  This option enables support for TI Keystone PHY (serdes).
>  endmenu
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index 7728518..bd306a7 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -19,3 +19,4 @@ phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4210_USB2)	+= phy-exynos4210-usb2.o
>  phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4X12_USB2)	+= phy-exynos4x12-usb2.o
>  phy-exynos-usb2-$(CONFIG_PHY_EXYNOS5250_USB2)	+= phy-exynos5250-usb2.o
>  obj-$(CONFIG_PHY_XGENE)			+= phy-xgene.o
> +obj-$(CONFIG_PHY_TI_KEYSTONE)		+= phy-keystone.o
> diff --git a/drivers/phy/phy-keystone.c b/drivers/phy/phy-keystone.c
> new file mode 100644
> index 0000000..ba1b9fa
> --- /dev/null
> +++ b/drivers/phy/phy-keystone.c
> @@ -0,0 +1,230 @@
> +/*
> + * PCIe Keystone platform specific driver code
> + *
> + * Copyright (C) 2013-2014 Texas Instruments, Inc.
> + *		http://www.ti.com
> + *
> + * Author: Murali Karicheri <m-karicheri2@ti.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/clk.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_platform.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +
> +#define reg_dump(addr, mask) \
> +		pr_debug("reg %p has value %x\n", (void *)addr, \
> +			(readl(addr) & ~mask))
> +
> +/* mask bits point to bits being modified */
> +#define reg_rmw(addr, value, mask) \
> +		writel(((readl(addr) & (~(mask))) | \
> +			(value & (mask))), (addr))
> +struct serdes_config {
> +	u32 reg;
> +	u32 val;
> +	u32 mask;
> +};
> +
> +struct phy_keystone {
> +	struct device *dev;
> +	void __iomem *base;
> +};
> +
> +static struct serdes_config ks_100mhz_pcie_5gbps_serdes[] = {
> +	{0x0000, 0x00000800, 0x0000ff00},
> +	{0x0060, 0x00041c5c, 0x00ffffff},
> +	{0x0064, 0x0343c700, 0xffffff00},
> +	{0x006c, 0x00000012, 0x000000ff},
> +	{0x0068, 0x00070000, 0x00ff0000},
> +	{0x0078, 0x0000c000, 0x0000ff00},
> +
> +	{0x0200, 0x00000000, 0x000000ff},
> +	{0x0204, 0x5e000080, 0xff0000ff},
> +	{0x0208, 0x00000006, 0x000000ff},
> +	{0x0210, 0x00000023, 0x000000ff},
> +	{0x0214, 0x2e003060, 0xff00ffff},
> +	{0x0218, 0x76000000, 0xff000000},
> +	{0x022c, 0x00200002, 0x00ff00ff},
> +	{0x02a0, 0xffee0000, 0xffff0000},
> +	{0x02a4, 0x0000000f, 0x000000ff},
> +	{0x0204, 0x5e000000, 0xff000000},
> +	{0x0208, 0x00000006, 0x000000ff},
> +	{0x0278, 0x00002000, 0x0000ff00},
> +	{0x0280, 0x00280028, 0x00ff00ff},
> +	{0x0284, 0x2d0f0385, 0xffffffff},
> +	{0x0250, 0xd0000000, 0xff000000},
> +	{0x0284, 0x00000085, 0x000000ff},
> +	{0x0294, 0x20000000, 0xff000000},
> +
> +	{0x0400, 0x00000000, 0x000000ff},
> +	{0x0404, 0x5e000080, 0xff0000ff},
> +	{0x0408, 0x00000006, 0x000000ff},
> +	{0x0410, 0x00000023, 0x000000ff},
> +	{0x0414, 0x2e003060, 0xff00ffff},
> +	{0x0418, 0x76000000, 0xff000000},
> +	{0x042c, 0x00200002, 0x00ff00ff},
> +	{0x04a0, 0xffee0000, 0xffff0000},
> +	{0x04a4, 0x0000000f, 0x000000ff},
> +	{0x0404, 0x5e000000, 0xff000000},
> +	{0x0408, 0x00000006, 0x000000ff},
> +	{0x0478, 0x00002000, 0x0000ff00},
> +	{0x0480, 0x00280028, 0x00ff00ff},
> +	{0x0484, 0x2d0f0385, 0xffffffff},
> +	{0x0450, 0xd0000000, 0xff000000},
> +	{0x0494, 0x20000000, 0xff000000},
> +
> +	{0x0604, 0x00000080, 0x000000ff},
> +	{0x0600, 0x00000000, 0x000000ff},
> +	{0x0604, 0x5e000000, 0xff000000},
> +	{0x0608, 0x00000006, 0x000000ff},
> +	{0x0610, 0x00000023, 0x000000ff},
> +	{0x0614, 0x2e003060, 0xff00ffff},
> +	{0x0618, 0x76000000, 0xff000000},
> +	{0x062c, 0x00200002, 0x00ff00ff},
> +	{0x06a0, 0xffee0000, 0xffff0000},
> +	{0x06a4, 0x0000000f, 0x000000ff},
> +	{0x0604, 0x5e000000, 0xff000000},
> +	{0x0608, 0x00000006, 0x000000ff},
> +	{0x0678, 0x00002000, 0x0000ff00},
> +	{0x0680, 0x00280028, 0x00ff00ff},
> +	{0x0684, 0x2d0f0385, 0xffffffff},
> +	{0x0650, 0xd0000000, 0xff000000},
> +	{0x0694, 0x20000000, 0xff000000},
> +
> +	{0x0800, 0x00000000, 0x000000ff},
> +	{0x0804, 0x5e000080, 0xff0000ff},
> +	{0x0808, 0x00000006, 0x000000ff},
> +	{0x0810, 0x00000023, 0x000000ff},
> +	{0x0814, 0x2e003060, 0xff00ffff},
> +	{0x0818, 0x76000000, 0xff000000},
> +	{0x082c, 0x00200002, 0x00ff00ff},
> +	{0x08a0, 0xffee0000, 0xffff0000},
> +	{0x08a4, 0x0000000f, 0x000000ff},
> +	{0x0804, 0x5e000000, 0xff000000},
> +	{0x0808, 0x00000006, 0x000000ff},
> +	{0x0878, 0x00002000, 0x0000ff00},
> +	{0x0880, 0x00280028, 0x00ff00ff},
> +	{0x0884, 0x2d0f0385, 0xffffffff},
> +	{0x0850, 0xd0000000, 0xff000000},
> +	{0x0894, 0x20000000, 0xff000000},
> +
> +	{0x0a00, 0x00000100, 0x0000ff00},
> +	{0x0a08, 0x00e12c08, 0x00ffffff},
> +	{0x0a0c, 0x00000081, 0x000000ff},
> +	{0x0a18, 0x00e80000, 0x00ff0000},
> +	{0x0a30, 0x002f2f00, 0x00ffff00},
> +	{0x0a4c, 0xac820000, 0xffff0000},
> +	{0x0a54, 0xc0000000, 0xff000000},
> +	{0x0a58, 0x00001441, 0x0000ffff},
> +	{0x0a84, 0x00000301, 0x0000ffff},
> +
> +	{0x0a8c, 0x81030000, 0xffff0000},
> +	{0x0a90, 0x00006001, 0x0000ffff},
> +	{0x0a94, 0x01000000, 0xff000000},
> +	{0x0aa0, 0x81000000, 0xff000000},
> +	{0x0abc, 0xff000000, 0xff000000},
> +	{0x0ac0, 0x0000008b, 0x000000ff},
> +
> +	{0x0000, 0x00000003, 0x000000ff},
> +	{0x0a00, 0x0000009f, 0x000000ff},
> +
> +	{0x0a44, 0x5f733d00, 0xffffff00},
> +	{0x0a48, 0x00fdca00, 0x00ffff00},
> +	{0x0a5c, 0x00000000, 0xffff0000},
> +	{0x0a60, 0x00008000, 0xffffffff},
> +	{0x0a64, 0x0c581220, 0xffffffff},
> +	{0x0a68, 0xe13b0602, 0xffffffff},
> +	{0x0a6c, 0xb8074cc1, 0xffffffff},
> +	{0x0a70, 0x3f02e989, 0xffffffff},
> +	{0x0a74, 0x00000001, 0x000000ff},
> +	{0x0b14, 0x00370000, 0x00ff0000},
> +	{0x0b10, 0x37000000, 0xff000000},
> +	{0x0b14, 0x0000005d, 0x000000ff},

Which of these is powering on/off the PHY? Which of these is for clocks? Which
of these is for TX or RX? Which of these is for lane configuration?...

Imagining how would it be if all peripherals are programmed this way in linux..

-Kishon

> +};
> +
> +static int ks_phy_init(struct phy *phy)
> +{
> +	struct serdes_config *p;
> +	struct phy_keystone *ks_phy = phy_get_drvdata(phy);
> +
> +	int i;
> +
> +	for (i = 0, p = &ks_100mhz_pcie_5gbps_serdes[0];
> +		i < ARRAY_SIZE(ks_100mhz_pcie_5gbps_serdes);
> +		i++, p++) {
> +		reg_rmw((ks_phy->base + p->reg), p->val, p->mask);
> +		reg_dump((ks_phy->base + p->reg), p->mask);
> +	}
> +	udelay(2000);
> +
> +	return 0;
> +}
> +
> +static struct phy_ops ks_phy_ops = {
> +	.init		= ks_phy_init,
> +	.owner		= THIS_MODULE,
> +};
> +
> +static int ks_phy_probe(struct platform_device *pdev)
> +{
> +	struct phy_provider *phy_provider;
> +	struct device *dev = &pdev->dev;
> +	struct phy_keystone *ks_phy;
> +	struct phy *phy;
> +	struct resource *res;
> +
> +	ks_phy = devm_kzalloc(dev, sizeof(*ks_phy), GFP_KERNEL);
> +	if (!ks_phy)
> +		return -ENOMEM;
> +
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "reg_serdes");
> +	ks_phy->base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(ks_phy->base))
> +		return PTR_ERR(ks_phy->base);
> +
> +	ks_phy->dev = dev;
> +	phy = devm_phy_create(dev, &ks_phy_ops, NULL);
> +	if (IS_ERR(phy))
> +		return PTR_ERR(phy);
> +
> +	phy_set_drvdata(phy, ks_phy);
> +	phy_provider = devm_of_phy_provider_register(ks_phy->dev,
> +				of_phy_simple_xlate);
> +
> +	if (IS_ERR(phy_provider))
> +		return PTR_ERR(phy_provider);
> +
> +	dev_info(dev, "keystone phy initialized\n");
> +	return 0;
> +}
> +
> +static const struct of_device_id ks_phy_of_match[] = {
> +	{ .compatible = "ti,keystone-phy" },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, ks_phy_of_match);
> +
> +static struct platform_driver ks_phy_driver = {
> +	.probe	= ks_phy_probe,
> +	.driver = {
> +		.of_match_table	= ks_phy_of_match,
> +		.name  = "ti,keystone-phy",
> +		.owner = THIS_MODULE,
> +	}
> +};
> +module_platform_driver(ks_phy_driver);
> +
> +MODULE_DESCRIPTION("TI Keystone SerDes PHY driver");
> +MODULE_LICENSE("GPL V2");
> +MODULE_AUTHOR("Murali Karicheri <m-karicheri2@ti.com>");
>
Jingoo Han June 2, 2014, 6:45 a.m. UTC | #7
On Monday, June 02, 2014 3:17 PM, Kishon Vijay Abraham I wrote:
> On Thursday 15 May 2014 09:31 PM, Murali Karicheri wrote:
> > This phy driver is used by keystone PCI driver. The hw vendor that
> > provides the phy hw published only registers and their values. So
> > this driver uses these hard coded values to initialize the phy.
> >
> > CC: Grant Likely <grant.likely@linaro.org>
> > CC: Rob Herring <robh+dt@kernel.org>
> > CC: Mohit Kumar <mohit.kumar@st.com>
> > CC: Jingoo Han <jg1.han@samsung.com>
> > CC: Bjorn Helgaas <bhelgaas@google.com>
> >
> > Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
> > ---
> >  drivers/phy/Kconfig        |    6 ++
> >  drivers/phy/Makefile       |    1 +
> >  drivers/phy/phy-keystone.c |  230 ++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 237 insertions(+)
> >  create mode 100644 drivers/phy/phy-keystone.c
> >
> > diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> > index 4906c27..e5f4b5a 100644
> > --- a/drivers/phy/Kconfig
> > +++ b/drivers/phy/Kconfig
> > @@ -167,4 +167,10 @@ config PHY_XGENE
> >  	help
> >  	  This option enables support for APM X-Gene SoC multi-purpose PHY.
> >
> > +config PHY_TI_KEYSTONE
> > +	bool "TI Keystone PHY support"
> > +	depends on ARCH_KEYSTONE
> > +	select GENERIC_PHY
> > +	help
> > +	  This option enables support for TI Keystone PHY (serdes).
> >  endmenu
> > diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> > index 7728518..bd306a7 100644
> > --- a/drivers/phy/Makefile
> > +++ b/drivers/phy/Makefile
> > @@ -19,3 +19,4 @@ phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4210_USB2)	+= phy-exynos4210-usb2.o
> >  phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4X12_USB2)	+= phy-exynos4x12-usb2.o
> >  phy-exynos-usb2-$(CONFIG_PHY_EXYNOS5250_USB2)	+= phy-exynos5250-usb2.o
> >  obj-$(CONFIG_PHY_XGENE)			+= phy-xgene.o
> > +obj-$(CONFIG_PHY_TI_KEYSTONE)		+= phy-keystone.o
> > diff --git a/drivers/phy/phy-keystone.c b/drivers/phy/phy-keystone.c
> > new file mode 100644
> > index 0000000..ba1b9fa
> > --- /dev/null
> > +++ b/drivers/phy/phy-keystone.c
> > @@ -0,0 +1,230 @@
> > +/*
> > + * PCIe Keystone platform specific driver code
> > + *
> > + * Copyright (C) 2013-2014 Texas Instruments, Inc.
> > + *		http://www.ti.com
> > + *
> > + * Author: Murali Karicheri <m-karicheri2@ti.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/clk.h>
> > +#include <linux/delay.h>
> > +#include <linux/err.h>
> > +#include <linux/io.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/of_address.h>
> > +#include <linux/of_platform.h>
> > +#include <linux/phy/phy.h>
> > +#include <linux/platform_device.h>
> > +
> > +#define reg_dump(addr, mask) \
> > +		pr_debug("reg %p has value %x\n", (void *)addr, \
> > +			(readl(addr) & ~mask))
> > +
> > +/* mask bits point to bits being modified */
> > +#define reg_rmw(addr, value, mask) \
> > +		writel(((readl(addr) & (~(mask))) | \
> > +			(value & (mask))), (addr))
> > +struct serdes_config {
> > +	u32 reg;
> > +	u32 val;
> > +	u32 mask;
> > +};
> > +
> > +struct phy_keystone {
> > +	struct device *dev;
> > +	void __iomem *base;
> > +};
> > +
> > +static struct serdes_config ks_100mhz_pcie_5gbps_serdes[] = {
> > +	{0x0000, 0x00000800, 0x0000ff00},
> > +	{0x0060, 0x00041c5c, 0x00ffffff},
> > +	{0x0064, 0x0343c700, 0xffffff00},
> > +	{0x006c, 0x00000012, 0x000000ff},
> > +	{0x0068, 0x00070000, 0x00ff0000},
> > +	{0x0078, 0x0000c000, 0x0000ff00},
> > +
> > +	{0x0200, 0x00000000, 0x000000ff},
> > +	{0x0204, 0x5e000080, 0xff0000ff},
> > +	{0x0208, 0x00000006, 0x000000ff},
> > +	{0x0210, 0x00000023, 0x000000ff},
> > +	{0x0214, 0x2e003060, 0xff00ffff},
> > +	{0x0218, 0x76000000, 0xff000000},
> > +	{0x022c, 0x00200002, 0x00ff00ff},
> > +	{0x02a0, 0xffee0000, 0xffff0000},
> > +	{0x02a4, 0x0000000f, 0x000000ff},
> > +	{0x0204, 0x5e000000, 0xff000000},
> > +	{0x0208, 0x00000006, 0x000000ff},
> > +	{0x0278, 0x00002000, 0x0000ff00},
> > +	{0x0280, 0x00280028, 0x00ff00ff},
> > +	{0x0284, 0x2d0f0385, 0xffffffff},
> > +	{0x0250, 0xd0000000, 0xff000000},
> > +	{0x0284, 0x00000085, 0x000000ff},
> > +	{0x0294, 0x20000000, 0xff000000},
> > +
> > +	{0x0400, 0x00000000, 0x000000ff},
> > +	{0x0404, 0x5e000080, 0xff0000ff},
> > +	{0x0408, 0x00000006, 0x000000ff},
> > +	{0x0410, 0x00000023, 0x000000ff},
> > +	{0x0414, 0x2e003060, 0xff00ffff},
> > +	{0x0418, 0x76000000, 0xff000000},
> > +	{0x042c, 0x00200002, 0x00ff00ff},
> > +	{0x04a0, 0xffee0000, 0xffff0000},
> > +	{0x04a4, 0x0000000f, 0x000000ff},
> > +	{0x0404, 0x5e000000, 0xff000000},
> > +	{0x0408, 0x00000006, 0x000000ff},
> > +	{0x0478, 0x00002000, 0x0000ff00},
> > +	{0x0480, 0x00280028, 0x00ff00ff},
> > +	{0x0484, 0x2d0f0385, 0xffffffff},
> > +	{0x0450, 0xd0000000, 0xff000000},
> > +	{0x0494, 0x20000000, 0xff000000},
> > +
> > +	{0x0604, 0x00000080, 0x000000ff},
> > +	{0x0600, 0x00000000, 0x000000ff},
> > +	{0x0604, 0x5e000000, 0xff000000},
> > +	{0x0608, 0x00000006, 0x000000ff},
> > +	{0x0610, 0x00000023, 0x000000ff},
> > +	{0x0614, 0x2e003060, 0xff00ffff},
> > +	{0x0618, 0x76000000, 0xff000000},
> > +	{0x062c, 0x00200002, 0x00ff00ff},
> > +	{0x06a0, 0xffee0000, 0xffff0000},
> > +	{0x06a4, 0x0000000f, 0x000000ff},
> > +	{0x0604, 0x5e000000, 0xff000000},
> > +	{0x0608, 0x00000006, 0x000000ff},
> > +	{0x0678, 0x00002000, 0x0000ff00},
> > +	{0x0680, 0x00280028, 0x00ff00ff},
> > +	{0x0684, 0x2d0f0385, 0xffffffff},
> > +	{0x0650, 0xd0000000, 0xff000000},
> > +	{0x0694, 0x20000000, 0xff000000},
> > +
> > +	{0x0800, 0x00000000, 0x000000ff},
> > +	{0x0804, 0x5e000080, 0xff0000ff},
> > +	{0x0808, 0x00000006, 0x000000ff},
> > +	{0x0810, 0x00000023, 0x000000ff},
> > +	{0x0814, 0x2e003060, 0xff00ffff},
> > +	{0x0818, 0x76000000, 0xff000000},
> > +	{0x082c, 0x00200002, 0x00ff00ff},
> > +	{0x08a0, 0xffee0000, 0xffff0000},
> > +	{0x08a4, 0x0000000f, 0x000000ff},
> > +	{0x0804, 0x5e000000, 0xff000000},
> > +	{0x0808, 0x00000006, 0x000000ff},
> > +	{0x0878, 0x00002000, 0x0000ff00},
> > +	{0x0880, 0x00280028, 0x00ff00ff},
> > +	{0x0884, 0x2d0f0385, 0xffffffff},
> > +	{0x0850, 0xd0000000, 0xff000000},
> > +	{0x0894, 0x20000000, 0xff000000},
> > +
> > +	{0x0a00, 0x00000100, 0x0000ff00},
> > +	{0x0a08, 0x00e12c08, 0x00ffffff},
> > +	{0x0a0c, 0x00000081, 0x000000ff},
> > +	{0x0a18, 0x00e80000, 0x00ff0000},
> > +	{0x0a30, 0x002f2f00, 0x00ffff00},
> > +	{0x0a4c, 0xac820000, 0xffff0000},
> > +	{0x0a54, 0xc0000000, 0xff000000},
> > +	{0x0a58, 0x00001441, 0x0000ffff},
> > +	{0x0a84, 0x00000301, 0x0000ffff},
> > +
> > +	{0x0a8c, 0x81030000, 0xffff0000},
> > +	{0x0a90, 0x00006001, 0x0000ffff},
> > +	{0x0a94, 0x01000000, 0xff000000},
> > +	{0x0aa0, 0x81000000, 0xff000000},
> > +	{0x0abc, 0xff000000, 0xff000000},
> > +	{0x0ac0, 0x0000008b, 0x000000ff},
> > +
> > +	{0x0000, 0x00000003, 0x000000ff},
> > +	{0x0a00, 0x0000009f, 0x000000ff},
> > +
> > +	{0x0a44, 0x5f733d00, 0xffffff00},
> > +	{0x0a48, 0x00fdca00, 0x00ffff00},
> > +	{0x0a5c, 0x00000000, 0xffff0000},
> > +	{0x0a60, 0x00008000, 0xffffffff},
> > +	{0x0a64, 0x0c581220, 0xffffffff},
> > +	{0x0a68, 0xe13b0602, 0xffffffff},
> > +	{0x0a6c, 0xb8074cc1, 0xffffffff},
> > +	{0x0a70, 0x3f02e989, 0xffffffff},
> > +	{0x0a74, 0x00000001, 0x000000ff},
> > +	{0x0b14, 0x00370000, 0x00ff0000},
> > +	{0x0b10, 0x37000000, 0xff000000},
> > +	{0x0b14, 0x0000005d, 0x000000ff},
> 
> Which of these is powering on/off the PHY? Which of these is for clocks? Which
> of these is for TX or RX? Which of these is for lane configuration?...
> 
> Imagining how would it be if all peripherals are programmed this way in linux..

+111

I agree with Kishon's comment. The mainline linux kernel is
the Open source that can be shared with a lot of people. So,
readability and manageability are important.

Please split this HUGE chunk 'ks_100mhz_pcie_5gbps_serdes[]'
into several chunks, according to what they do.

For example,
static struct serdes_config init_phy_clocks[]
static struct serdes_config power_on_phy[]
static struct serdes_config reset_phy_core[]
....

Then, if possible, covert hardcoded 'reg' values into macro
names.

For example,
#define 0x0000	SERDES_PLL_CONTROL
#define 0x0004	SERDES_PLL_STATUS
....

Best regards,
Jingoo Han

> 
> -Kishon
> 
> > +};
> > +
> > +static int ks_phy_init(struct phy *phy)
> > +{
> > +	struct serdes_config *p;
> > +	struct phy_keystone *ks_phy = phy_get_drvdata(phy);
> > +
> > +	int i;
> > +
> > +	for (i = 0, p = &ks_100mhz_pcie_5gbps_serdes[0];
> > +		i < ARRAY_SIZE(ks_100mhz_pcie_5gbps_serdes);
> > +		i++, p++) {
> > +		reg_rmw((ks_phy->base + p->reg), p->val, p->mask);
> > +		reg_dump((ks_phy->base + p->reg), p->mask);
> > +	}
> > +	udelay(2000);
> > +
> > +	return 0;
> > +}
> > +
> > +static struct phy_ops ks_phy_ops = {
> > +	.init		= ks_phy_init,
> > +	.owner		= THIS_MODULE,
> > +};
> > +
> > +static int ks_phy_probe(struct platform_device *pdev)
> > +{
> > +	struct phy_provider *phy_provider;
> > +	struct device *dev = &pdev->dev;
> > +	struct phy_keystone *ks_phy;
> > +	struct phy *phy;
> > +	struct resource *res;
> > +
> > +	ks_phy = devm_kzalloc(dev, sizeof(*ks_phy), GFP_KERNEL);
> > +	if (!ks_phy)
> > +		return -ENOMEM;
> > +
> > +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "reg_serdes");
> > +	ks_phy->base = devm_ioremap_resource(dev, res);
> > +	if (IS_ERR(ks_phy->base))
> > +		return PTR_ERR(ks_phy->base);
> > +
> > +	ks_phy->dev = dev;
> > +	phy = devm_phy_create(dev, &ks_phy_ops, NULL);
> > +	if (IS_ERR(phy))
> > +		return PTR_ERR(phy);
> > +
> > +	phy_set_drvdata(phy, ks_phy);
> > +	phy_provider = devm_of_phy_provider_register(ks_phy->dev,
> > +				of_phy_simple_xlate);
> > +
> > +	if (IS_ERR(phy_provider))
> > +		return PTR_ERR(phy_provider);
> > +
> > +	dev_info(dev, "keystone phy initialized\n");
> > +	return 0;
> > +}
> > +
> > +static const struct of_device_id ks_phy_of_match[] = {
> > +	{ .compatible = "ti,keystone-phy" },
> > +	{ },
> > +};
> > +MODULE_DEVICE_TABLE(of, ks_phy_of_match);
> > +
> > +static struct platform_driver ks_phy_driver = {
> > +	.probe	= ks_phy_probe,
> > +	.driver = {
> > +		.of_match_table	= ks_phy_of_match,
> > +		.name  = "ti,keystone-phy",
> > +		.owner = THIS_MODULE,
> > +	}
> > +};
> > +module_platform_driver(ks_phy_driver);
> > +
> > +MODULE_DESCRIPTION("TI Keystone SerDes PHY driver");
> > +MODULE_LICENSE("GPL V2");
> > +MODULE_AUTHOR("Murali Karicheri <m-karicheri2@ti.com>");
> >
Murali Karicheri June 2, 2014, 2:28 p.m. UTC | #8
On 6/2/2014 2:16 AM, ABRAHAM, KISHON VIJAY wrote:
> Hi,
>
> On Thursday 15 May 2014 09:31 PM, Murali Karicheri wrote:
>> This phy driver is used by keystone PCI driver. The hw vendor that
>> provides the phy hw published only registers and their values. So
>> this driver uses these hard coded values to initialize the phy.
>>
>> CC: Grant Likely <grant.likely@linaro.org>
>> CC: Rob Herring <robh+dt@kernel.org>
>> CC: Mohit Kumar <mohit.kumar@st.com>
>> CC: Jingoo Han <jg1.han@samsung.com>
>> CC: Bjorn Helgaas <bhelgaas@google.com>
>>
>> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
>> ---
>>   drivers/phy/Kconfig        |    6 ++
>>   drivers/phy/Makefile       |    1 +
>>   drivers/phy/phy-keystone.c |  230 ++++++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 237 insertions(+)
>>   create mode 100644 drivers/phy/phy-keystone.c
>>
>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>> index 4906c27..e5f4b5a 100644
>> --- a/drivers/phy/Kconfig
>> +++ b/drivers/phy/Kconfig
>> @@ -167,4 +167,10 @@ config PHY_XGENE
>>   	help
>>   	  This option enables support for APM X-Gene SoC multi-purpose PHY.
>>   
>> +config PHY_TI_KEYSTONE
>> +	bool "TI Keystone PHY support"
>> +	depends on ARCH_KEYSTONE
>> +	select GENERIC_PHY
>> +	help
>> +	  This option enables support for TI Keystone PHY (serdes).
>>   endmenu
>> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
>> index 7728518..bd306a7 100644
>> --- a/drivers/phy/Makefile
>> +++ b/drivers/phy/Makefile
>> @@ -19,3 +19,4 @@ phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4210_USB2)	+= phy-exynos4210-usb2.o
>>   phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4X12_USB2)	+= phy-exynos4x12-usb2.o
>>   phy-exynos-usb2-$(CONFIG_PHY_EXYNOS5250_USB2)	+= phy-exynos5250-usb2.o
>>   obj-$(CONFIG_PHY_XGENE)			+= phy-xgene.o
>> +obj-$(CONFIG_PHY_TI_KEYSTONE)		+= phy-keystone.o
>> diff --git a/drivers/phy/phy-keystone.c b/drivers/phy/phy-keystone.c
>> new file mode 100644
>> index 0000000..ba1b9fa
>> --- /dev/null
>> +++ b/drivers/phy/phy-keystone.c
>> @@ -0,0 +1,230 @@
>> +/*
>> + * PCIe Keystone platform specific driver code
>> + *
>> + * Copyright (C) 2013-2014 Texas Instruments, Inc.
>> + *		http://www.ti.com
>> + *
>> + * Author: Murali Karicheri <m-karicheri2@ti.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/clk.h>
>> +#include <linux/delay.h>
>> +#include <linux/err.h>
>> +#include <linux/io.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/phy/phy.h>
>> +#include <linux/platform_device.h>
>> +
>> +#define reg_dump(addr, mask) \
>> +		pr_debug("reg %p has value %x\n", (void *)addr, \
>> +			(readl(addr) & ~mask))
>> +
>> +/* mask bits point to bits being modified */
>> +#define reg_rmw(addr, value, mask) \
>> +		writel(((readl(addr) & (~(mask))) | \
>> +			(value & (mask))), (addr))
>> +struct serdes_config {
>> +	u32 reg;
>> +	u32 val;
>> +	u32 mask;
>> +};
>> +
>> +struct phy_keystone {
>> +	struct device *dev;
>> +	void __iomem *base;
>> +};
>> +
>> +static struct serdes_config ks_100mhz_pcie_5gbps_serdes[] = {
>> +	{0x0000, 0x00000800, 0x0000ff00},
>> +	{0x0060, 0x00041c5c, 0x00ffffff},
>> +	{0x0064, 0x0343c700, 0xffffff00},
>> +	{0x006c, 0x00000012, 0x000000ff},
>> +	{0x0068, 0x00070000, 0x00ff0000},
>> +	{0x0078, 0x0000c000, 0x0000ff00},
>> +
>> +	{0x0200, 0x00000000, 0x000000ff},
>> +	{0x0204, 0x5e000080, 0xff0000ff},
>> +	{0x0208, 0x00000006, 0x000000ff},
>> +	{0x0210, 0x00000023, 0x000000ff},
>> +	{0x0214, 0x2e003060, 0xff00ffff},
>> +	{0x0218, 0x76000000, 0xff000000},
>> +	{0x022c, 0x00200002, 0x00ff00ff},
>> +	{0x02a0, 0xffee0000, 0xffff0000},
>> +	{0x02a4, 0x0000000f, 0x000000ff},
>> +	{0x0204, 0x5e000000, 0xff000000},
>> +	{0x0208, 0x00000006, 0x000000ff},
>> +	{0x0278, 0x00002000, 0x0000ff00},
>> +	{0x0280, 0x00280028, 0x00ff00ff},
>> +	{0x0284, 0x2d0f0385, 0xffffffff},
>> +	{0x0250, 0xd0000000, 0xff000000},
>> +	{0x0284, 0x00000085, 0x000000ff},
>> +	{0x0294, 0x20000000, 0xff000000},
>> +
>> +	{0x0400, 0x00000000, 0x000000ff},
>> +	{0x0404, 0x5e000080, 0xff0000ff},
>> +	{0x0408, 0x00000006, 0x000000ff},
>> +	{0x0410, 0x00000023, 0x000000ff},
>> +	{0x0414, 0x2e003060, 0xff00ffff},
>> +	{0x0418, 0x76000000, 0xff000000},
>> +	{0x042c, 0x00200002, 0x00ff00ff},
>> +	{0x04a0, 0xffee0000, 0xffff0000},
>> +	{0x04a4, 0x0000000f, 0x000000ff},
>> +	{0x0404, 0x5e000000, 0xff000000},
>> +	{0x0408, 0x00000006, 0x000000ff},
>> +	{0x0478, 0x00002000, 0x0000ff00},
>> +	{0x0480, 0x00280028, 0x00ff00ff},
>> +	{0x0484, 0x2d0f0385, 0xffffffff},
>> +	{0x0450, 0xd0000000, 0xff000000},
>> +	{0x0494, 0x20000000, 0xff000000},
>> +
>> +	{0x0604, 0x00000080, 0x000000ff},
>> +	{0x0600, 0x00000000, 0x000000ff},
>> +	{0x0604, 0x5e000000, 0xff000000},
>> +	{0x0608, 0x00000006, 0x000000ff},
>> +	{0x0610, 0x00000023, 0x000000ff},
>> +	{0x0614, 0x2e003060, 0xff00ffff},
>> +	{0x0618, 0x76000000, 0xff000000},
>> +	{0x062c, 0x00200002, 0x00ff00ff},
>> +	{0x06a0, 0xffee0000, 0xffff0000},
>> +	{0x06a4, 0x0000000f, 0x000000ff},
>> +	{0x0604, 0x5e000000, 0xff000000},
>> +	{0x0608, 0x00000006, 0x000000ff},
>> +	{0x0678, 0x00002000, 0x0000ff00},
>> +	{0x0680, 0x00280028, 0x00ff00ff},
>> +	{0x0684, 0x2d0f0385, 0xffffffff},
>> +	{0x0650, 0xd0000000, 0xff000000},
>> +	{0x0694, 0x20000000, 0xff000000},
>> +
>> +	{0x0800, 0x00000000, 0x000000ff},
>> +	{0x0804, 0x5e000080, 0xff0000ff},
>> +	{0x0808, 0x00000006, 0x000000ff},
>> +	{0x0810, 0x00000023, 0x000000ff},
>> +	{0x0814, 0x2e003060, 0xff00ffff},
>> +	{0x0818, 0x76000000, 0xff000000},
>> +	{0x082c, 0x00200002, 0x00ff00ff},
>> +	{0x08a0, 0xffee0000, 0xffff0000},
>> +	{0x08a4, 0x0000000f, 0x000000ff},
>> +	{0x0804, 0x5e000000, 0xff000000},
>> +	{0x0808, 0x00000006, 0x000000ff},
>> +	{0x0878, 0x00002000, 0x0000ff00},
>> +	{0x0880, 0x00280028, 0x00ff00ff},
>> +	{0x0884, 0x2d0f0385, 0xffffffff},
>> +	{0x0850, 0xd0000000, 0xff000000},
>> +	{0x0894, 0x20000000, 0xff000000},
>> +
>> +	{0x0a00, 0x00000100, 0x0000ff00},
>> +	{0x0a08, 0x00e12c08, 0x00ffffff},
>> +	{0x0a0c, 0x00000081, 0x000000ff},
>> +	{0x0a18, 0x00e80000, 0x00ff0000},
>> +	{0x0a30, 0x002f2f00, 0x00ffff00},
>> +	{0x0a4c, 0xac820000, 0xffff0000},
>> +	{0x0a54, 0xc0000000, 0xff000000},
>> +	{0x0a58, 0x00001441, 0x0000ffff},
>> +	{0x0a84, 0x00000301, 0x0000ffff},
>> +
>> +	{0x0a8c, 0x81030000, 0xffff0000},
>> +	{0x0a90, 0x00006001, 0x0000ffff},
>> +	{0x0a94, 0x01000000, 0xff000000},
>> +	{0x0aa0, 0x81000000, 0xff000000},
>> +	{0x0abc, 0xff000000, 0xff000000},
>> +	{0x0ac0, 0x0000008b, 0x000000ff},
>> +
>> +	{0x0000, 0x00000003, 0x000000ff},
>> +	{0x0a00, 0x0000009f, 0x000000ff},
>> +
>> +	{0x0a44, 0x5f733d00, 0xffffff00},
>> +	{0x0a48, 0x00fdca00, 0x00ffff00},
>> +	{0x0a5c, 0x00000000, 0xffff0000},
>> +	{0x0a60, 0x00008000, 0xffffffff},
>> +	{0x0a64, 0x0c581220, 0xffffffff},
>> +	{0x0a68, 0xe13b0602, 0xffffffff},
>> +	{0x0a6c, 0xb8074cc1, 0xffffffff},
>> +	{0x0a70, 0x3f02e989, 0xffffffff},
>> +	{0x0a74, 0x00000001, 0x000000ff},
>> +	{0x0b14, 0x00370000, 0x00ff0000},
>> +	{0x0b10, 0x37000000, 0xff000000},
>> +	{0x0b14, 0x0000005d, 0x000000ff},
> Which of these is powering on/off the PHY? Which of these is for clocks? Which
> of these is for TX or RX? Which of these is for lane configuration?...
>
> Imagining how would it be if all peripherals are programmed this way in linux..
>
> -Kishon
Kishon, Jingoo,

You are right and I also agree with you. This was originally discussed 
with Arnd on the list.
There is a NDA between TI and owner of the IP not to disclose the 
register details. So we
have to keep the driver the way it is. Once we have permission to 
disclose the details, this
driver will be updated. As per Arnd's request I have also checked the 
existing drivers for
similar register map and couldn't find any. So there is no duplication.

Murali
>> +};
>> +
>> +static int ks_phy_init(struct phy *phy)
>> +{
>> +	struct serdes_config *p;
>> +	struct phy_keystone *ks_phy = phy_get_drvdata(phy);
>> +
>> +	int i;
>> +
>> +	for (i = 0, p = &ks_100mhz_pcie_5gbps_serdes[0];
>> +		i < ARRAY_SIZE(ks_100mhz_pcie_5gbps_serdes);
>> +		i++, p++) {
>> +		reg_rmw((ks_phy->base + p->reg), p->val, p->mask);
>> +		reg_dump((ks_phy->base + p->reg), p->mask);
>> +	}
>> +	udelay(2000);
>> +
>> +	return 0;
>> +}
>> +
>> +static struct phy_ops ks_phy_ops = {
>> +	.init		= ks_phy_init,
>> +	.owner		= THIS_MODULE,
>> +};
>> +
>> +static int ks_phy_probe(struct platform_device *pdev)
>> +{
>> +	struct phy_provider *phy_provider;
>> +	struct device *dev = &pdev->dev;
>> +	struct phy_keystone *ks_phy;
>> +	struct phy *phy;
>> +	struct resource *res;
>> +
>> +	ks_phy = devm_kzalloc(dev, sizeof(*ks_phy), GFP_KERNEL);
>> +	if (!ks_phy)
>> +		return -ENOMEM;
>> +
>> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "reg_serdes");
>> +	ks_phy->base = devm_ioremap_resource(dev, res);
>> +	if (IS_ERR(ks_phy->base))
>> +		return PTR_ERR(ks_phy->base);
>> +
>> +	ks_phy->dev = dev;
>> +	phy = devm_phy_create(dev, &ks_phy_ops, NULL);
>> +	if (IS_ERR(phy))
>> +		return PTR_ERR(phy);
>> +
>> +	phy_set_drvdata(phy, ks_phy);
>> +	phy_provider = devm_of_phy_provider_register(ks_phy->dev,
>> +				of_phy_simple_xlate);
>> +
>> +	if (IS_ERR(phy_provider))
>> +		return PTR_ERR(phy_provider);
>> +
>> +	dev_info(dev, "keystone phy initialized\n");
>> +	return 0;
>> +}
>> +
>> +static const struct of_device_id ks_phy_of_match[] = {
>> +	{ .compatible = "ti,keystone-phy" },
>> +	{ },
>> +};
>> +MODULE_DEVICE_TABLE(of, ks_phy_of_match);
>> +
>> +static struct platform_driver ks_phy_driver = {
>> +	.probe	= ks_phy_probe,
>> +	.driver = {
>> +		.of_match_table	= ks_phy_of_match,
>> +		.name  = "ti,keystone-phy",
>> +		.owner = THIS_MODULE,
>> +	}
>> +};
>> +module_platform_driver(ks_phy_driver);
>> +
>> +MODULE_DESCRIPTION("TI Keystone SerDes PHY driver");
>> +MODULE_LICENSE("GPL V2");
>> +MODULE_AUTHOR("Murali Karicheri <m-karicheri2@ti.com>");
>>
diff mbox

Patch

diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 4906c27..e5f4b5a 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -167,4 +167,10 @@  config PHY_XGENE
 	help
 	  This option enables support for APM X-Gene SoC multi-purpose PHY.
 
+config PHY_TI_KEYSTONE
+	bool "TI Keystone PHY support"
+	depends on ARCH_KEYSTONE
+	select GENERIC_PHY
+	help
+	  This option enables support for TI Keystone PHY (serdes).
 endmenu
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index 7728518..bd306a7 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -19,3 +19,4 @@  phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4210_USB2)	+= phy-exynos4210-usb2.o
 phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4X12_USB2)	+= phy-exynos4x12-usb2.o
 phy-exynos-usb2-$(CONFIG_PHY_EXYNOS5250_USB2)	+= phy-exynos5250-usb2.o
 obj-$(CONFIG_PHY_XGENE)			+= phy-xgene.o
+obj-$(CONFIG_PHY_TI_KEYSTONE)		+= phy-keystone.o
diff --git a/drivers/phy/phy-keystone.c b/drivers/phy/phy-keystone.c
new file mode 100644
index 0000000..ba1b9fa
--- /dev/null
+++ b/drivers/phy/phy-keystone.c
@@ -0,0 +1,230 @@ 
+/*
+ * PCIe Keystone platform specific driver code
+ *
+ * Copyright (C) 2013-2014 Texas Instruments, Inc.
+ *		http://www.ti.com
+ *
+ * Author: Murali Karicheri <m-karicheri2@ti.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/clk.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_platform.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+
+#define reg_dump(addr, mask) \
+		pr_debug("reg %p has value %x\n", (void *)addr, \
+			(readl(addr) & ~mask))
+
+/* mask bits point to bits being modified */
+#define reg_rmw(addr, value, mask) \
+		writel(((readl(addr) & (~(mask))) | \
+			(value & (mask))), (addr))
+struct serdes_config {
+	u32 reg;
+	u32 val;
+	u32 mask;
+};
+
+struct phy_keystone {
+	struct device *dev;
+	void __iomem *base;
+};
+
+static struct serdes_config ks_100mhz_pcie_5gbps_serdes[] = {
+	{0x0000, 0x00000800, 0x0000ff00},
+	{0x0060, 0x00041c5c, 0x00ffffff},
+	{0x0064, 0x0343c700, 0xffffff00},
+	{0x006c, 0x00000012, 0x000000ff},
+	{0x0068, 0x00070000, 0x00ff0000},
+	{0x0078, 0x0000c000, 0x0000ff00},
+
+	{0x0200, 0x00000000, 0x000000ff},
+	{0x0204, 0x5e000080, 0xff0000ff},
+	{0x0208, 0x00000006, 0x000000ff},
+	{0x0210, 0x00000023, 0x000000ff},
+	{0x0214, 0x2e003060, 0xff00ffff},
+	{0x0218, 0x76000000, 0xff000000},
+	{0x022c, 0x00200002, 0x00ff00ff},
+	{0x02a0, 0xffee0000, 0xffff0000},
+	{0x02a4, 0x0000000f, 0x000000ff},
+	{0x0204, 0x5e000000, 0xff000000},
+	{0x0208, 0x00000006, 0x000000ff},
+	{0x0278, 0x00002000, 0x0000ff00},
+	{0x0280, 0x00280028, 0x00ff00ff},
+	{0x0284, 0x2d0f0385, 0xffffffff},
+	{0x0250, 0xd0000000, 0xff000000},
+	{0x0284, 0x00000085, 0x000000ff},
+	{0x0294, 0x20000000, 0xff000000},
+
+	{0x0400, 0x00000000, 0x000000ff},
+	{0x0404, 0x5e000080, 0xff0000ff},
+	{0x0408, 0x00000006, 0x000000ff},
+	{0x0410, 0x00000023, 0x000000ff},
+	{0x0414, 0x2e003060, 0xff00ffff},
+	{0x0418, 0x76000000, 0xff000000},
+	{0x042c, 0x00200002, 0x00ff00ff},
+	{0x04a0, 0xffee0000, 0xffff0000},
+	{0x04a4, 0x0000000f, 0x000000ff},
+	{0x0404, 0x5e000000, 0xff000000},
+	{0x0408, 0x00000006, 0x000000ff},
+	{0x0478, 0x00002000, 0x0000ff00},
+	{0x0480, 0x00280028, 0x00ff00ff},
+	{0x0484, 0x2d0f0385, 0xffffffff},
+	{0x0450, 0xd0000000, 0xff000000},
+	{0x0494, 0x20000000, 0xff000000},
+
+	{0x0604, 0x00000080, 0x000000ff},
+	{0x0600, 0x00000000, 0x000000ff},
+	{0x0604, 0x5e000000, 0xff000000},
+	{0x0608, 0x00000006, 0x000000ff},
+	{0x0610, 0x00000023, 0x000000ff},
+	{0x0614, 0x2e003060, 0xff00ffff},
+	{0x0618, 0x76000000, 0xff000000},
+	{0x062c, 0x00200002, 0x00ff00ff},
+	{0x06a0, 0xffee0000, 0xffff0000},
+	{0x06a4, 0x0000000f, 0x000000ff},
+	{0x0604, 0x5e000000, 0xff000000},
+	{0x0608, 0x00000006, 0x000000ff},
+	{0x0678, 0x00002000, 0x0000ff00},
+	{0x0680, 0x00280028, 0x00ff00ff},
+	{0x0684, 0x2d0f0385, 0xffffffff},
+	{0x0650, 0xd0000000, 0xff000000},
+	{0x0694, 0x20000000, 0xff000000},
+
+	{0x0800, 0x00000000, 0x000000ff},
+	{0x0804, 0x5e000080, 0xff0000ff},
+	{0x0808, 0x00000006, 0x000000ff},
+	{0x0810, 0x00000023, 0x000000ff},
+	{0x0814, 0x2e003060, 0xff00ffff},
+	{0x0818, 0x76000000, 0xff000000},
+	{0x082c, 0x00200002, 0x00ff00ff},
+	{0x08a0, 0xffee0000, 0xffff0000},
+	{0x08a4, 0x0000000f, 0x000000ff},
+	{0x0804, 0x5e000000, 0xff000000},
+	{0x0808, 0x00000006, 0x000000ff},
+	{0x0878, 0x00002000, 0x0000ff00},
+	{0x0880, 0x00280028, 0x00ff00ff},
+	{0x0884, 0x2d0f0385, 0xffffffff},
+	{0x0850, 0xd0000000, 0xff000000},
+	{0x0894, 0x20000000, 0xff000000},
+
+	{0x0a00, 0x00000100, 0x0000ff00},
+	{0x0a08, 0x00e12c08, 0x00ffffff},
+	{0x0a0c, 0x00000081, 0x000000ff},
+	{0x0a18, 0x00e80000, 0x00ff0000},
+	{0x0a30, 0x002f2f00, 0x00ffff00},
+	{0x0a4c, 0xac820000, 0xffff0000},
+	{0x0a54, 0xc0000000, 0xff000000},
+	{0x0a58, 0x00001441, 0x0000ffff},
+	{0x0a84, 0x00000301, 0x0000ffff},
+
+	{0x0a8c, 0x81030000, 0xffff0000},
+	{0x0a90, 0x00006001, 0x0000ffff},
+	{0x0a94, 0x01000000, 0xff000000},
+	{0x0aa0, 0x81000000, 0xff000000},
+	{0x0abc, 0xff000000, 0xff000000},
+	{0x0ac0, 0x0000008b, 0x000000ff},
+
+	{0x0000, 0x00000003, 0x000000ff},
+	{0x0a00, 0x0000009f, 0x000000ff},
+
+	{0x0a44, 0x5f733d00, 0xffffff00},
+	{0x0a48, 0x00fdca00, 0x00ffff00},
+	{0x0a5c, 0x00000000, 0xffff0000},
+	{0x0a60, 0x00008000, 0xffffffff},
+	{0x0a64, 0x0c581220, 0xffffffff},
+	{0x0a68, 0xe13b0602, 0xffffffff},
+	{0x0a6c, 0xb8074cc1, 0xffffffff},
+	{0x0a70, 0x3f02e989, 0xffffffff},
+	{0x0a74, 0x00000001, 0x000000ff},
+	{0x0b14, 0x00370000, 0x00ff0000},
+	{0x0b10, 0x37000000, 0xff000000},
+	{0x0b14, 0x0000005d, 0x000000ff},
+};
+
+static int ks_phy_init(struct phy *phy)
+{
+	struct serdes_config *p;
+	struct phy_keystone *ks_phy = phy_get_drvdata(phy);
+
+	int i;
+
+	for (i = 0, p = &ks_100mhz_pcie_5gbps_serdes[0];
+		i < ARRAY_SIZE(ks_100mhz_pcie_5gbps_serdes);
+		i++, p++) {
+		reg_rmw((ks_phy->base + p->reg), p->val, p->mask);
+		reg_dump((ks_phy->base + p->reg), p->mask);
+	}
+	udelay(2000);
+
+	return 0;
+}
+
+static struct phy_ops ks_phy_ops = {
+	.init		= ks_phy_init,
+	.owner		= THIS_MODULE,
+};
+
+static int ks_phy_probe(struct platform_device *pdev)
+{
+	struct phy_provider *phy_provider;
+	struct device *dev = &pdev->dev;
+	struct phy_keystone *ks_phy;
+	struct phy *phy;
+	struct resource *res;
+
+	ks_phy = devm_kzalloc(dev, sizeof(*ks_phy), GFP_KERNEL);
+	if (!ks_phy)
+		return -ENOMEM;
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "reg_serdes");
+	ks_phy->base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(ks_phy->base))
+		return PTR_ERR(ks_phy->base);
+
+	ks_phy->dev = dev;
+	phy = devm_phy_create(dev, &ks_phy_ops, NULL);
+	if (IS_ERR(phy))
+		return PTR_ERR(phy);
+
+	phy_set_drvdata(phy, ks_phy);
+	phy_provider = devm_of_phy_provider_register(ks_phy->dev,
+				of_phy_simple_xlate);
+
+	if (IS_ERR(phy_provider))
+		return PTR_ERR(phy_provider);
+
+	dev_info(dev, "keystone phy initialized\n");
+	return 0;
+}
+
+static const struct of_device_id ks_phy_of_match[] = {
+	{ .compatible = "ti,keystone-phy" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, ks_phy_of_match);
+
+static struct platform_driver ks_phy_driver = {
+	.probe	= ks_phy_probe,
+	.driver = {
+		.of_match_table	= ks_phy_of_match,
+		.name  = "ti,keystone-phy",
+		.owner = THIS_MODULE,
+	}
+};
+module_platform_driver(ks_phy_driver);
+
+MODULE_DESCRIPTION("TI Keystone SerDes PHY driver");
+MODULE_LICENSE("GPL V2");
+MODULE_AUTHOR("Murali Karicheri <m-karicheri2@ti.com>");