On 07/02/2014 05:07 AM, Kishon Vijay Abraham I wrote:
Hi, On Tuesday 01 July 2014 07:27 PM, Murali Karicheri wrote:This phy driver added to support keystone PCI driver. This is%s/PCI/PCIe/
Ok.
a generic phy driver and currently it supports only SerDes for PCI Controller. The hw vendor that provides the phyThe driver supports only for PCIe or the controller?
I meant PCIe. I will fix it.
hw published only registers and their values. So this driver uses these hard coded values to initialize the phy. Signed-off-by: Murali Karicheri<m-karicheri2@xxxxxx> CC: Rob Herring<robh+dt@xxxxxxxxxx> CC: Pawel Moll<pawel.moll@xxxxxxx> CC: Mark Rutland<mark.rutland@xxxxxxx> CC: Ian Campbell<ijc+devicetree@xxxxxxxxxxxxxx> CC: Kumar Gala<galak@xxxxxxxxxxxxxx> CC: Kishon Vijay Abraham I<kishon@xxxxxx> CC: Grant Likely<grant.likely@xxxxxxxxxx> --- Changelog:- - Resending with changelog moved out of commit message - Added comments to indicate what the register functionality each register blob is doing. - Previous versions were part of the PCI Keystone driver series. This is now send as a separate patch so that this can be reviewed indepedently and merged. .../bindings/phy/phy-serdes-keystone.txt | 25 +++ drivers/phy/Kconfig | 6 + drivers/phy/Makefile | 1 + drivers/phy/phy-serdes-keystone.c | 200 ++++++++++++++++++++ 4 files changed, 232 insertions(+) create mode 100644 Documentation/devicetree/bindings/phy/phy-serdes-keystone.txt create mode 100644 drivers/phy/phy-serdes-keystone.c diff --git a/Documentation/devicetree/bindings/phy/phy-serdes-keystone.txt b/Documentation/devicetree/bindings/phy/phy-serdes-keystone.txtCan we reuse ti-phy.txt for this?
No issues. I will move the content to a section in ti-phy.txt "Keystone SerDes Phy"
new file mode 100644 index 0000000..612c7b5 --- /dev/null +++ b/Documentation/devicetree/bindings/phy/phy-serdes-keystone.txt @@ -0,0 +1,25 @@ +TI Keystone SerDes Phy: DT documentation +======================================= + +Required properties: + - compatible: should be "ti,keystone-serdes-phy"; + - reg: base address and length of the SerDes register set + - reg-names: should be "reg_serdes", name of the reg SerDes register set + - #phy-cells: from the generic phy bindings, must be 0; + +Exampel for Keystone PCIE, + + pcie0_phy: serdes_phy@2320000 { + #phy-cells =<0>; + compatible = "ti,keystone-serdes-phy"; + reg =<0x02320000 0x4000>; + reg-names = "reg_serdes"; + }; + + +Then the PHY can be used in PCIe controller node as + +pcie@21800000 { + phys =<&pcie0_phy>; + phy-names = "pcie-phy"; +}; diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig index 16a2f06..54a8471 100644 --- a/drivers/phy/Kconfig +++ b/drivers/phy/Kconfig @@ -178,4 +178,10 @@ config PHY_XGENE help This option enables support for APM X-Gene SoC multi-purpose PHY. +config PHY_TI_KEYSTONE_SERDES + bool "TI Keystone SerDes PHY support" + depends on ARCH_KEYSTONEdepends on HAS_IOMEM?
Why? Driver works without this. When would one use this config option?
+ select GENERIC_PHY + help + This option enables support for TI Keystone SerDes PHY. endmenu diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile index b4f1d57..a02faaa 100644 --- a/drivers/phy/Makefile +++ b/drivers/phy/Makefile @@ -20,3 +20,4 @@ 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_EXYNOS5_USBDRD) += phy-exynos5-usbdrd.o obj-$(CONFIG_PHY_XGENE) += phy-xgene.o +obj-$(CONFIG_PHY_TI_KEYSTONE_SERDES) += phy-serdes-keystone.o diff --git a/drivers/phy/phy-serdes-keystone.c b/drivers/phy/phy-serdes-keystone.c new file mode 100644 index 0000000..f1a8888 --- /dev/null +++ b/drivers/phy/phy-serdes-keystone.c
Looking at the other phy drivers, feel this should be named phy-keystone-serdes.c?
Make sense. Will rename. Do you think I need to change the config option to PHY_KEYSTONE_SERDES as well?
@@ -0,0 +1,200 @@ +/* + * Keystone SerDes Phy driver + * + * Copyright (C) 2013-2014 Texas Instruments, Inc. + * http://www.ti.com + * + * Author: Murali Karicheri<m-karicheri2@xxxxxx> + * + * 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 {struct ks_serdes_config?
Ok.
+ u32 reg; + u32 val; + u32 mask; +}; + +struct phy_serdes_keystone {struct ks_serdes_phy?
Ok.
+ struct device *dev; + void __iomem *base; +}; + +static struct serdes_config ks_100mhz_5gbps_serdes[] = { + /* SerDes Clock and common configuration */ + {0x0000, 0x00000800, 0x0000ff00}, + {0x0060, 0x00041c5c, 0x00ffffff}, + {0x0064, 0x0343c700, 0xffffff00}, + {0x006c, 0x00000012, 0x000000ff}, + {0x0068, 0x00070000, 0x00ff0000}, + {0x0078, 0x0000c000, 0x0000ff00}, + + /* Lane - A Phy configuration */ + {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}, + + /* Lane - B Phy configuration */ + {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}, + + /* Common Lane configurations */ + {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}, + + /* common clock configuration */ + {0x0000, 0x00000003, 0x000000ff}, + + /* Common Lane configurations */ + {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},I'm not particularly happy about merging this sort of configuration :-/
I know, but this is what best we can do for now. We are working with the IP owner to clarify if we can use more information on this IP. We will update the driver as things changes in the future.
+}; + +static int ks_serdes_phy_init(struct phy *phy) +{ + struct serdes_config *p; + struct phy_serdes_keystone *ks_phy = phy_get_drvdata(phy); + + int i; + + for (i = 0, p =&ks_100mhz_5gbps_serdes[0]; + i< ARRAY_SIZE(ks_100mhz_5gbps_serdes); + i++, p++) { + reg_rmw((ks_phy->base + p->reg), p->val, p->mask); + reg_dump((ks_phy->base + p->reg), p->mask); + } + msleep(20);Why this delay is needed? A small comment would help.
This is to allow for the PLL to lock. I will check if I can really check if PLL is locked and Lane status is Ok if possible.
+ + return 0; +} + +static struct phy_ops ks_serdes_phy_ops = { + .init = ks_serdes_phy_init, + .owner = THIS_MODULE, +}; + +static int ks_serdes_phy_probe(struct platform_device *pdev) +{ + struct phy_provider *phy_provider; + struct device *dev =&pdev->dev; + struct phy_serdes_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_serdes_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 SerDes Phy initialized\n");Lets not make the boot noisy, remove this.
Sure
+ return 0; +} + +static const struct of_device_id ks_serdes_phy_of_match[] = { + { .compatible = "ti,keystone-serdes-phy" }, + { }, +}; +MODULE_DEVICE_TABLE(of, ks_serdes_phy_of_match); + +static struct platform_driver ks_serdes_phy_driver = { + .probe = ks_serdes_phy_probe, + .driver = { + .of_match_table = ks_serdes_phy_of_match, + .name = "ti,keystone-serdes-phy", + .owner = THIS_MODULE, + } +}; +module_platform_driver(ks_serdes_phy_driver); + +MODULE_DESCRIPTION("TI Keystone SerDes PHY driver"); +MODULE_LICENSE("GPL V2");It's *GPL v2*
Ok.
Cheers Kishon
-- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
