2014-06-16 10:26:39

by Antoine Tenart

[permalink] [raw]
Subject: [PATCH v6 0/7] ARM: berlin: add AHCI support

This series adds the support for Berlin SoC AHCI controller. The
controller allows to use the SATA host interface and, for example, the
eSATA port on the BG2Q.

The series adds a PHY driver to control the two SATA ports available,
and adds a generic compatible to use the existing ahci_platform driver.

Also enable the eSATA interface on the BG2Q DMP.

Changes sinve v5:
- rebased on top of v3.16-rc1
- added the clocks property in the sata node

Changes since v4:
- updated PHY driver as tristate
- handled the case were no SATA port is enabled
- updated the compatible to a generic one
- cosmetic fixups

Changes since v3:
- moved all PHY operations to the PHY driver
- removed PHY sub-nodes
- removed the custom Berlin AHCI driver and switched to
ahci_platform
- added multiple PHYs support to the libahci_platform

Changes since v2:
- modeled each PHY as a sub-node
- cosmetic fixups

Changes since v1:
- added a PHY driver, allowing to enable each port
individually and removed the 'force-port-map' property
- made the drivers a bit less magic :)
- wrote a function to select and configure registers in the
AHCI driver
- removed BG2 / BG2CD nodes

Antoine Ténart (7):
phy: add a driver for the Berlin SATA PHY
Documentation: bindings: add the Berlin SATA PHY
ata: libahci: allow to use multiple PHYs
ata: ahci_platform: add a generic AHCI compatible
Documentation: bindings: document the sub-nodes AHCI bindings
ARM: berlin: add the AHCI node for the BG2Q
ARM: berlin: enable the eSATA interface on the BG2Q DMP

.../devicetree/bindings/ata/ahci-platform.txt | 37 ++++
.../devicetree/bindings/phy/berlin-sata-phy.txt | 14 ++
arch/arm/boot/dts/berlin2q-marvell-dmp.dts | 8 +
arch/arm/boot/dts/berlin2q.dtsi | 28 +++
drivers/ata/ahci.h | 3 +-
drivers/ata/ahci_platform.c | 2 +
drivers/ata/libahci.c | 7 +
drivers/ata/libahci_platform.c | 170 +++++++++++----
drivers/phy/Kconfig | 7 +
drivers/phy/Makefile | 1 +
drivers/phy/phy-berlin-sata.c | 232 +++++++++++++++++++++
11 files changed, 473 insertions(+), 36 deletions(-)
create mode 100644 Documentation/devicetree/bindings/phy/berlin-sata-phy.txt
create mode 100644 drivers/phy/phy-berlin-sata.c

--
1.9.1


2014-06-16 10:26:41

by Antoine Tenart

[permalink] [raw]
Subject: [PATCH v6 1/7] phy: add a driver for the Berlin SATA PHY

The Berlin SoC has a two SATA ports. Add a PHY driver to handle them.

The mode selection can let us think this PHY can be configured to fit
other purposes. But there are reasons to think the SATA mode will be
the only one usable: the PHY registers are only accessible indirectly
through two registers in the SATA range, the PHY seems to be integrated
and no information tells us the contrary. For these reasons, make the
driver a SATA PHY driver.

Signed-off-by: Antoine Ténart <[email protected]>
---
drivers/phy/Kconfig | 7 ++
drivers/phy/Makefile | 1 +
drivers/phy/phy-berlin-sata.c | 232 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 240 insertions(+)
create mode 100644 drivers/phy/phy-berlin-sata.c

diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 16a2f067c242..365ad3651e1c 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -15,6 +15,13 @@ config GENERIC_PHY
phy users can obtain reference to the PHY. All the users of this
framework should select this config.

+config PHY_BERLIN_SATA
+ tristate "Marvell Berlin SATA PHY driver"
+ depends on ARCH_BERLIN && OF
+ select GENERIC_PHY
+ help
+ Enable this to support the SATA PHY on Marvell Berlin SoCs.
+
config PHY_EXYNOS_MIPI_VIDEO
tristate "S5P/EXYNOS SoC series MIPI CSI-2/DSI PHY driver"
depends on HAS_IOMEM
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index b4f1d5770601..a137a2e23218 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -3,6 +3,7 @@
#

obj-$(CONFIG_GENERIC_PHY) += phy-core.o
+obj-$(CONFIG_PHY_BERLIN_SATA) += phy-berlin-sata.o
obj-$(CONFIG_BCM_KONA_USB2_PHY) += phy-bcm-kona-usb2.o
obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO) += phy-exynos-dp-video.o
obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO) += phy-exynos-mipi-video.o
diff --git a/drivers/phy/phy-berlin-sata.c b/drivers/phy/phy-berlin-sata.c
new file mode 100644
index 000000000000..907897a02672
--- /dev/null
+++ b/drivers/phy/phy-berlin-sata.c
@@ -0,0 +1,232 @@
+/*
+ * Marvell Berlin SATA PHY driver
+ *
+ * Copyright (C) 2014 Marvell Technology Group Ltd.
+ *
+ * Antoine Ténart <[email protected]>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/module.h>
+#include <linux/phy/phy.h>
+#include <linux/io.h>
+#include <linux/platform_device.h>
+
+#define HOST_VSA_ADDR 0x0
+#define HOST_VSA_DATA 0x4
+#define PORT_VSR_ADDR 0x78
+#define PORT_VSR_DATA 0x7c
+#define PORT_SCR_CTL 0x2c
+
+#define CONTROL_REGISTER 0x0
+#define MBUS_SIZE_CONTROL 0x4
+
+#define POWER_DOWN_PHY0 BIT(6)
+#define POWER_DOWN_PHY1 BIT(14)
+#define MBUS_WRITE_REQUEST_SIZE_128 (BIT(2) << 16)
+#define MBUS_READ_REQUEST_SIZE_128 (BIT(2) << 19)
+
+#define PHY_BASE 0x200
+
+/* register 0x01 */
+#define REF_FREF_SEL_25 BIT(0)
+#define PHY_MODE_SATA (0x0 << 5)
+
+/* register 0x02 */
+#define USE_MAX_PLL_RATE BIT(12)
+
+/* register 0x23 */
+#define DATA_BIT_WIDTH_10 (0x0 << 10)
+#define DATA_BIT_WIDTH_20 (0x1 << 10)
+#define DATA_BIT_WIDTH_40 (0x2 << 10)
+
+/* register 0x25 */
+#define PHY_GEN_MAX_1_5 (0x0 << 10)
+#define PHY_GEN_MAX_3_0 (0x1 << 10)
+#define PHY_GEN_MAX_6_0 (0x2 << 10)
+
+#define BERLIN_SATA_PHY_NB 2
+
+#define to_berlin_sata_phy_priv(desc) \
+ container_of((desc), struct phy_berlin_priv, phys[(desc)->index])
+
+struct phy_berlin_desc {
+ struct phy *phy;
+ u32 val;
+ unsigned index;
+};
+
+struct phy_berlin_priv {
+ void __iomem *base;
+ spinlock_t lock;
+ struct phy_berlin_desc phys[BERLIN_SATA_PHY_NB];
+};
+
+static inline void phy_berlin_sata_reg_setbits(void __iomem *ctrl_reg, u32 reg,
+ u32 mask, u32 val)
+{
+ u32 regval;
+
+ /* select register */
+ writel(PHY_BASE + reg, ctrl_reg + PORT_VSR_ADDR);
+
+ /* set bits */
+ regval = readl(ctrl_reg + PORT_VSR_DATA);
+ regval &= ~mask;
+ regval |= val;
+ writel(regval, ctrl_reg + PORT_VSR_DATA);
+}
+
+static int phy_berlin_sata_power_on(struct phy *phy)
+{
+ struct phy_berlin_desc *desc = phy_get_drvdata(phy);
+ struct phy_berlin_priv *priv = to_berlin_sata_phy_priv(desc);
+ void __iomem *ctrl_reg = priv->base + 0x60 + (desc->index * 0x80);
+ int ret = 0;
+ u32 regval;
+
+ spin_lock(&priv->lock);
+
+ /* Power on PHY */
+ writel(CONTROL_REGISTER, priv->base + HOST_VSA_ADDR);
+ regval = readl(priv->base + HOST_VSA_DATA);
+ regval &= ~(desc->val);
+ writel(regval, priv->base + HOST_VSA_DATA);
+
+ /* Configure MBus */
+ writel(MBUS_SIZE_CONTROL, priv->base + HOST_VSA_ADDR);
+ regval = readl(priv->base + HOST_VSA_DATA);
+ regval |= MBUS_WRITE_REQUEST_SIZE_128 | MBUS_READ_REQUEST_SIZE_128;
+ writel(regval, priv->base + HOST_VSA_DATA);
+
+ /* set PHY mode and ref freq to 25 MHz */
+ phy_berlin_sata_reg_setbits(ctrl_reg, 0x1, 0xff,
+ REF_FREF_SEL_25 | PHY_MODE_SATA);
+
+ /* set PHY up to 6 Gbps */
+ phy_berlin_sata_reg_setbits(ctrl_reg, 0x25, 0xc00, PHY_GEN_MAX_6_0);
+
+ /* set 40 bits width */
+ phy_berlin_sata_reg_setbits(ctrl_reg, 0x23, 0xc00, DATA_BIT_WIDTH_40);
+
+ /* use max pll rate */
+ phy_berlin_sata_reg_setbits(ctrl_reg, 0x2, 0x0, USE_MAX_PLL_RATE);
+
+ /* set the controller speed */
+ writel(0x31, ctrl_reg + PORT_SCR_CTL);
+
+ spin_unlock(&priv->lock);
+
+ return ret;
+}
+
+static int phy_berlin_sata_power_off(struct phy *phy)
+{
+ struct phy_berlin_desc *desc = phy_get_drvdata(phy);
+ struct phy_berlin_priv *priv = to_berlin_sata_phy_priv(desc);
+ u32 regval;
+
+ spin_lock(&priv->lock);
+
+ /* Power down PHY */
+ writel(CONTROL_REGISTER, priv->base + HOST_VSA_ADDR);
+ regval = readl(priv->base + HOST_VSA_DATA);
+ regval |= desc->val;
+ writel(regval, priv->base + HOST_VSA_DATA);
+
+ spin_unlock(&priv->lock);
+
+ return 0;
+}
+
+static struct phy *phy_berlin_sata_phy_xlate(struct device *dev,
+ struct of_phandle_args *args)
+{
+ struct phy_berlin_priv *priv = dev_get_drvdata(dev);
+
+ if (WARN_ON(args->args[0] >= BERLIN_SATA_PHY_NB))
+ return ERR_PTR(-ENODEV);
+
+ return priv->phys[args->args[0]].phy;
+}
+
+static struct phy_ops phy_berlin_sata_ops = {
+ .power_on = phy_berlin_sata_power_on,
+ .power_off = phy_berlin_sata_power_off,
+ .owner = THIS_MODULE,
+};
+
+static u32 phy_berlin_power_down_bits[] = {
+ POWER_DOWN_PHY0,
+ POWER_DOWN_PHY1,
+};
+
+static int phy_berlin_sata_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct phy *phy;
+ struct phy_provider *phy_provider;
+ struct phy_berlin_priv *priv;
+ struct resource *res;
+ int i;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res)
+ return -EINVAL;
+
+ priv->base = devm_ioremap(dev, res->start, resource_size(res));
+ if (!priv->base)
+ return -ENOMEM;
+
+ dev_set_drvdata(dev, priv);
+ spin_lock_init(&priv->lock);
+
+ for (i = 0; i < BERLIN_SATA_PHY_NB; i++) {
+ phy = devm_phy_create(dev, &phy_berlin_sata_ops, NULL);
+ if (IS_ERR(phy)) {
+ dev_err(dev, "failed to create PHY %d\n", i);
+ return PTR_ERR(phy);
+ }
+
+ priv->phys[i].phy = phy;
+ priv->phys[i].val = phy_berlin_power_down_bits[i];
+ priv->phys[i].index = i;
+ phy_set_drvdata(phy, &priv->phys[i]);
+
+ /* Make sure the PHY is off */
+ phy_berlin_sata_power_off(phy);
+ }
+
+ phy_provider =
+ devm_of_phy_provider_register(dev, phy_berlin_sata_phy_xlate);
+ if (IS_ERR(phy_provider))
+ return PTR_ERR(phy_provider);
+
+ return 0;
+}
+
+static const struct of_device_id phy_berlin_sata_of_match[] = {
+ { .compatible = "marvell,berlin-sata-phy" },
+ { },
+};
+
+static struct platform_driver phy_berlin_sata_driver = {
+ .probe = phy_berlin_sata_probe,
+ .driver = {
+ .name = "phy-berlin-sata",
+ .owner = THIS_MODULE,
+ .of_match_table = phy_berlin_sata_of_match,
+ },
+};
+module_platform_driver(phy_berlin_sata_driver);
+
+MODULE_DESCRIPTION("Marvell Berlin SATA PHY driver");
+MODULE_AUTHOR("Antoine Ténart <[email protected]>");
+MODULE_LICENSE("GPL v2");
--
1.9.1

2014-06-16 10:26:50

by Antoine Tenart

[permalink] [raw]
Subject: [PATCH v6 7/7] ARM: berlin: enable the eSATA interface on the BG2Q DMP

The BG2Q has an AHCI SATA controller with an eSATA interface. Enable it.
Only enable the first port, the BG2Q DMP does not support the second one.

Signed-off-by: Antoine Ténart <[email protected]>
---
arch/arm/boot/dts/berlin2q-marvell-dmp.dts | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/arch/arm/boot/dts/berlin2q-marvell-dmp.dts b/arch/arm/boot/dts/berlin2q-marvell-dmp.dts
index 995150f93795..385f6af64d0c 100644
--- a/arch/arm/boot/dts/berlin2q-marvell-dmp.dts
+++ b/arch/arm/boot/dts/berlin2q-marvell-dmp.dts
@@ -37,3 +37,11 @@
&uart0 {
status = "okay";
};
+
+&sata0 {
+ status = "okay";
+};
+
+&sata_phy {
+ status = "okay";
+};
--
1.9.1

2014-06-16 10:28:48

by Antoine Tenart

[permalink] [raw]
Subject: [PATCH v6 5/7] Documentation: bindings: document the sub-nodes AHCI bindings

The libahci now allows to use multiple PHYs and to represent each port
as a sub-node. Add these bindings to the documentation.

Signed-off-by: Antoine Ténart <[email protected]>
---
.../devicetree/bindings/ata/ahci-platform.txt | 37 ++++++++++++++++++++++
1 file changed, 37 insertions(+)

diff --git a/Documentation/devicetree/bindings/ata/ahci-platform.txt b/Documentation/devicetree/bindings/ata/ahci-platform.txt
index c96d8dcf98fd..24d95fccfb4a 100644
--- a/Documentation/devicetree/bindings/ata/ahci-platform.txt
+++ b/Documentation/devicetree/bindings/ata/ahci-platform.txt
@@ -3,6 +3,10 @@
SATA nodes are defined to describe on-chip Serial ATA controllers.
Each SATA controller should have its own node.

+It is possible, but not required, to represent each port as a sub-node.
+It allows to enable each port independently when dealing with multiple
+PHYs.
+
Required properties:
- compatible : compatible string, one of:
- "allwinner,sun4i-a10-ahci"
@@ -14,6 +18,7 @@ Required properties:
- "snps,dwc-ahci"
- "snps,exynos5440-ahci"
- "snps,spear-ahci"
+ - "generic-ahci"
- interrupts : <interrupt mapping for SATA IRQ>
- reg : <registers mapping>

@@ -21,11 +26,23 @@ Optional properties:
- dma-coherent : Present if dma operations are coherent
- clocks : a list of phandle + clock specifier pairs
- target-supply : regulator for SATA target power
+- phys : reference to the SATA PHY node
+- phy-names : must be "sata-phy"

"fsl,imx53-ahci", "fsl,imx6q-ahci" required properties:
- clocks : must contain the sata, sata_ref and ahb clocks
- clock-names : must contain "ahb" for the ahb clock

+Required properties when using sub-nodes:
+- #address-cells : number of cells to encode an address
+- #size-cells : number of cells representing the size of an address
+
+
+Sub-nodes required properties:
+- reg : the port number
+- phys : reference to the SATA PHY node
+
+
Examples:
sata@ffe08000 {
compatible = "snps,spear-ahci";
@@ -40,3 +57,23 @@ Examples:
clocks = <&pll6 0>, <&ahb_gates 25>;
target-supply = <&reg_ahci_5v>;
};
+
+With sub-nodes:
+ sata@f7e90000 {
+ compatible = "generic-ahci";
+ reg = <0xe90000 0x1000>;
+ interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&chip CLKID_SATA>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ sata0: sata-port@0 {
+ reg = <0>;
+ phys = <&sata_phy 0>;
+ };
+
+ sata1: sata-port@1 {
+ reg = <1>;
+ phys = <&sata_phy 1>;
+ };
+ };
--
1.9.1

2014-06-16 10:28:45

by Antoine Tenart

[permalink] [raw]
Subject: [PATCH v6 6/7] ARM: berlin: add the AHCI node for the BG2Q

The BG2Q has an AHCI SATA controller. Add the corresponding nodes
(AHCI, PHY) into its device tree.

Signed-off-by: Antoine Ténart <[email protected]>
---
arch/arm/boot/dts/berlin2q.dtsi | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)

diff --git a/arch/arm/boot/dts/berlin2q.dtsi b/arch/arm/boot/dts/berlin2q.dtsi
index 635a16a64cb4..3fb0d3935aec 100644
--- a/arch/arm/boot/dts/berlin2q.dtsi
+++ b/arch/arm/boot/dts/berlin2q.dtsi
@@ -303,6 +303,34 @@
clock-names = "refclk";
};

+ ahci: sata@e90000 {
+ compatible = "generic-ahci";
+ reg = <0xe90000 0x1000>;
+ interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&chip CLKID_SATA>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ sata0: sata-port@0 {
+ reg = <0>;
+ phys = <&sata_phy 0>;
+ status = "disabled";
+ };
+
+ sata1: sata-port@1 {
+ reg = <1>;
+ phys = <&sata_phy 1>;
+ status = "disabled";
+ };
+ };
+
+ sata_phy: phy@e900a0 {
+ compatible = "marvell,berlin-sata-phy";
+ reg = <0xe900a0 0x200>;
+ #phy-cells = <1>;
+ status = "disabled";
+ };
+
apb@fc0000 {
compatible = "simple-bus";
#address-cells = <1>;
--
1.9.1

2014-06-16 10:29:38

by Antoine Tenart

[permalink] [raw]
Subject: [PATCH v6 4/7] ata: ahci_platform: add a generic AHCI compatible

The ahci_platform driver is a generic driver using the libahci_platform
functions. Add a generic compatible to avoid having an endless list of
compatibles with no differences for the same driver.

Signed-off-by: Antoine Ténart <[email protected]>
---
drivers/ata/ahci_platform.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
index ebe505c17763..a30cecd68609 100644
--- a/drivers/ata/ahci_platform.c
+++ b/drivers/ata/ahci_platform.c
@@ -78,6 +78,8 @@ static SIMPLE_DEV_PM_OPS(ahci_pm_ops, ahci_platform_suspend,
ahci_platform_resume);

static const struct of_device_id ahci_of_match[] = {
+ { .compatible = "generic-ahci", },
+ /* Keep the following compatibles for device tree compatibility */
{ .compatible = "snps,spear-ahci", },
{ .compatible = "snps,exynos5440-ahci", },
{ .compatible = "ibm,476gtr-ahci", },
--
1.9.1

2014-06-16 10:29:57

by Antoine Tenart

[permalink] [raw]
Subject: [PATCH v6 3/7] ata: libahci: allow to use multiple PHYs

The current implementation of the libahci does not allow to use multiple
PHYs. This patch adds the support of multiple PHYs by the libahci while
keeping the old bindings valid for device tree compatibility.

This introduce a new way of defining SATA ports in the device tree, with
one port per sub-node. This as the advantage of allowing a per port
configuration. Because some ports may be accessible but disabled in the
device tree, the default port_map is computed automatically when using
this.

Signed-off-by: Antoine Ténart <[email protected]>
---
drivers/ata/ahci.h | 3 +-
drivers/ata/libahci.c | 7 ++
drivers/ata/libahci_platform.c | 170 ++++++++++++++++++++++++++++++++---------
3 files changed, 144 insertions(+), 36 deletions(-)

diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
index 05882e4445a6..e5bdbf827ca8 100644
--- a/drivers/ata/ahci.h
+++ b/drivers/ata/ahci.h
@@ -330,7 +330,8 @@ struct ahci_host_priv {
bool got_runtime_pm; /* Did we do pm_runtime_get? */
struct clk *clks[AHCI_MAX_CLKS]; /* Optional */
struct regulator *target_pwr; /* Optional */
- struct phy *phy; /* If platform uses phy */
+ struct phy **phys; /* If platform uses phys */
+ unsigned nphys; /* Number of phys */
void *plat_data; /* Other platform data */
/*
* Optional ahci_start_engine override, if not set this gets set to the
diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index 40ea583d3610..0bdee8330dcf 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -482,6 +482,13 @@ void ahci_save_initial_config(struct device *dev,
port_map &= mask_port_map;
}

+ /*
+ * If port_map was filled automatically when finding port sub-nodes,
+ * make sure we get the right set here.
+ */
+ if (hpriv->port_map)
+ port_map &= hpriv->port_map;
+
/* cross check port_map and cap.n_ports */
if (port_map) {
int map_ports = 0;
diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c
index 3a5b4ed25a4f..cd2bc50ba913 100644
--- a/drivers/ata/libahci_platform.c
+++ b/drivers/ata/libahci_platform.c
@@ -39,6 +39,61 @@ static struct scsi_host_template ahci_platform_sht = {
};

/**
+ * ahci_platform_enable_phys - Enable PHYs
+ * @hpriv: host private area to store config values
+ *
+ * This function enables all the PHYs found in hpriv->phys, if any.
+ * If a PHY fails to be enabled, it disables all the PHYs already
+ * enabled in reverse order and returns an error.
+ *
+ * RETURNS:
+ * 0 on success otherwise a negative error code
+ */
+int ahci_platform_enable_phys(struct ahci_host_priv *hpriv)
+{
+ int i, rc = 0;
+
+ for (i = 0; i < hpriv->nphys; i++) {
+ rc = phy_init(hpriv->phys[i]);
+ if (rc)
+ goto disable_phys;
+
+ rc = phy_power_on(hpriv->phys[i]);
+ if (rc) {
+ phy_exit(hpriv->phys[i]);
+ goto disable_phys;
+ }
+ }
+
+ return 0;
+
+disable_phys:
+ while (--i >= 0) {
+ phy_power_off(hpriv->phys[i]);
+ phy_exit(hpriv->phys[i]);
+ }
+ return rc;
+}
+EXPORT_SYMBOL_GPL(ahci_platform_enable_phys);
+
+/**
+ * ahci_platform_disable_phys - Enable PHYs
+ * @hpriv: host private area to store config values
+ *
+ * This function disables all PHYs found in hpriv->phys.
+ */
+void ahci_platform_disable_phys(struct ahci_host_priv *hpriv)
+{
+ int i;
+
+ for (i = 0; i < hpriv->nphys; i++) {
+ phy_power_off(hpriv->phys[i]);
+ phy_exit(hpriv->phys[i]);
+ }
+}
+EXPORT_SYMBOL_GPL(ahci_platform_disable_phys);
+
+/**
* ahci_platform_enable_clks - Enable platform clocks
* @hpriv: host private area to store config values
*
@@ -92,7 +147,7 @@ EXPORT_SYMBOL_GPL(ahci_platform_disable_clks);
* following order:
* 1) Regulator
* 2) Clocks (through ahci_platform_enable_clks)
- * 3) Phy
+ * 3) Phys
*
* If resource enabling fails at any point the previous enabled resources
* are disabled in reverse order.
@@ -114,17 +169,9 @@ int ahci_platform_enable_resources(struct ahci_host_priv *hpriv)
if (rc)
goto disable_regulator;

- if (hpriv->phy) {
- rc = phy_init(hpriv->phy);
- if (rc)
- goto disable_clks;
-
- rc = phy_power_on(hpriv->phy);
- if (rc) {
- phy_exit(hpriv->phy);
- goto disable_clks;
- }
- }
+ rc = ahci_platform_enable_phys(hpriv);
+ if (rc)
+ goto disable_clks;

return 0;

@@ -144,16 +191,13 @@ EXPORT_SYMBOL_GPL(ahci_platform_enable_resources);
*
* This function disables all ahci_platform managed resources in the
* following order:
- * 1) Phy
+ * 1) Phys
* 2) Clocks (through ahci_platform_disable_clks)
* 3) Regulator
*/
void ahci_platform_disable_resources(struct ahci_host_priv *hpriv)
{
- if (hpriv->phy) {
- phy_power_off(hpriv->phy);
- phy_exit(hpriv->phy);
- }
+ ahci_platform_disable_phys(hpriv);

ahci_platform_disable_clks(hpriv);

@@ -187,7 +231,7 @@ static void ahci_platform_put_resources(struct device *dev, void *res)
* 2) regulator for controlling the targets power (optional)
* 3) 0 - AHCI_MAX_CLKS clocks, as specified in the devs devicetree node,
* or for non devicetree enabled platforms a single clock
- * 4) phy (optional)
+ * 4) phys (optional)
*
* RETURNS:
* The allocated ahci_host_priv on success, otherwise an ERR_PTR value
@@ -197,7 +241,8 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev)
struct device *dev = &pdev->dev;
struct ahci_host_priv *hpriv;
struct clk *clk;
- int i, rc = -ENOMEM;
+ struct device_node *child;
+ int i, nports, rc = -ENOMEM;

if (!devres_open_group(dev, NULL, GFP_KERNEL))
return ERR_PTR(-ENOMEM);
@@ -246,22 +291,77 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev)
hpriv->clks[i] = clk;
}

- hpriv->phy = devm_phy_get(dev, "sata-phy");
- if (IS_ERR(hpriv->phy)) {
- rc = PTR_ERR(hpriv->phy);
- switch (rc) {
- case -ENODEV:
- case -ENOSYS:
- /* continue normally */
- hpriv->phy = NULL;
- break;
+ nports = of_get_child_count(dev->of_node);

- case -EPROBE_DEFER:
+ if (nports) {
+ hpriv->phys = devm_kzalloc(dev, nports * sizeof(*hpriv->phys),
+ GFP_KERNEL);
+ if (!hpriv->phys) {
+ rc = -ENOMEM;
goto err_out;
+ }

- default:
- dev_err(dev, "couldn't get sata-phy\n");
- goto err_out;
+ for_each_child_of_node(dev->of_node, child) {
+ u32 port;
+
+ if (!of_device_is_available(child))
+ continue;
+
+ if (of_property_read_u32(child, "reg", &port)) {
+ rc = -EINVAL;
+ goto err_out;
+ }
+
+ hpriv->port_map |= BIT(port);
+
+ hpriv->phys[hpriv->nphys] = devm_of_phy_get(dev, child,
+ NULL);
+ if (IS_ERR(hpriv->phys[hpriv->nphys])) {
+ rc = PTR_ERR(hpriv->phys[hpriv->nphys]);
+ dev_err(dev,
+ "couldn't get PHY in node %s: %d\n",
+ child->name, rc);
+ goto err_out;
+ }
+
+ hpriv->nphys++;
+ }
+
+ if (!hpriv->nphys) {
+ dev_warn(dev, "No port enabled\n");
+ return ERR_PTR(-ENODEV);
+ }
+ } else {
+ /*
+ * If no sub-node was found, keep this for device tree
+ * compatibility
+ */
+ struct phy *phy = devm_phy_get(dev, "sata-phy");
+ if (!IS_ERR(phy)) {
+ hpriv->phys = devm_kzalloc(dev, sizeof(*hpriv->phys),
+ GFP_KERNEL);
+ if (!hpriv->phys) {
+ rc = -ENOMEM;
+ goto err_out;
+ }
+
+ hpriv->phys[0] = phy;
+ hpriv->nphys = 1;
+ } else {
+ rc = PTR_ERR(phy);
+ switch (rc) {
+ case -ENODEV:
+ case -ENOSYS:
+ /* continue normally */
+ break;
+
+ case -EPROBE_DEFER:
+ goto err_out;
+
+ default:
+ dev_err(dev, "couldn't get sata-phy\n");
+ goto err_out;
+ }
}
}

@@ -288,7 +388,7 @@ EXPORT_SYMBOL_GPL(ahci_platform_get_resources);
* @mask_port_map: param passed to ahci_save_initial_config
*
* This function does all the usual steps needed to bring up an
- * ahci-platform host, note any necessary resources (ie clks, phy, etc.)
+ * ahci-platform host, note any necessary resources (ie clks, phys, etc.)
* must be initialized / enabled before calling this.
*
* RETURNS:
@@ -394,7 +494,7 @@ static void ahci_host_stop(struct ata_host *host)
* @dev: device pointer for the host
*
* This function does all the usual steps needed to suspend an
- * ahci-platform host, note any necessary resources (ie clks, phy, etc.)
+ * ahci-platform host, note any necessary resources (ie clks, phys, etc.)
* must be disabled after calling this.
*
* RETURNS:
@@ -431,7 +531,7 @@ EXPORT_SYMBOL_GPL(ahci_platform_suspend_host);
* @dev: device pointer for the host
*
* This function does all the usual steps needed to resume an ahci-platform
- * host, note any necessary resources (ie clks, phy, etc.) must be
+ * host, note any necessary resources (ie clks, phys, etc.) must be
* initialized / enabled before calling this.
*
* RETURNS:
--
1.9.1

2014-06-16 10:31:01

by Antoine Tenart

[permalink] [raw]
Subject: [PATCH v6 2/7] Documentation: bindings: add the Berlin SATA PHY

The Berlin SATA PHY drives the PHY related to the SATA interface. Add
the corresponding documentation.

Signed-off-by: Antoine Ténart <[email protected]>
---
Documentation/devicetree/bindings/phy/berlin-sata-phy.txt | 14 ++++++++++++++
1 file changed, 14 insertions(+)
create mode 100644 Documentation/devicetree/bindings/phy/berlin-sata-phy.txt

diff --git a/Documentation/devicetree/bindings/phy/berlin-sata-phy.txt b/Documentation/devicetree/bindings/phy/berlin-sata-phy.txt
new file mode 100644
index 000000000000..72646db2f5fc
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/berlin-sata-phy.txt
@@ -0,0 +1,14 @@
+Berlin SATA PHY
+---------------
+
+Required properties:
+- compatible: "marvell,berlin-sata-phy"
+- phy-cells: from the generic PHY bindings, must be 1
+- reg: address and length of the register
+
+Example:
+ sata_phy: phy@f7e900a0 {
+ compatible = "marvell,berlin-sata-phy";
+ reg = <0xf7e900a0 0x200>;
+ #phy-cells = <1>;
+ };
--
1.9.1

2014-06-16 10:44:36

by Sebastian Hesselbarth

[permalink] [raw]
Subject: Re: [PATCH v6 6/7] ARM: berlin: add the AHCI node for the BG2Q

On 06/16/2014 12:26 PM, Antoine Ténart wrote:
> The BG2Q has an AHCI SATA controller. Add the corresponding nodes
> (AHCI, PHY) into its device tree.
>
> Signed-off-by: Antoine Ténart <[email protected]>
> ---
> arch/arm/boot/dts/berlin2q.dtsi | 28 ++++++++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
> diff --git a/arch/arm/boot/dts/berlin2q.dtsi b/arch/arm/boot/dts/berlin2q.dtsi
> index 635a16a64cb4..3fb0d3935aec 100644
> --- a/arch/arm/boot/dts/berlin2q.dtsi
> +++ b/arch/arm/boot/dts/berlin2q.dtsi
> @@ -303,6 +303,34 @@
> clock-names = "refclk";
> };
>
> + ahci: sata@e90000 {
> + compatible = "generic-ahci";
> + reg = <0xe90000 0x1000>;
> + interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&chip CLKID_SATA>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + sata0: sata-port@0 {
> + reg = <0>;
> + phys = <&sata_phy 0>;
> + status = "disabled";
> + };
> +
> + sata1: sata-port@1 {
> + reg = <1>;
> + phys = <&sata_phy 1>;
> + status = "disabled";
> + };
> + };
> +
> + sata_phy: phy@e900a0 {
> + compatible = "marvell,berlin-sata-phy";
> + reg = <0xe900a0 0x200>;

Antoine,

I guess you'll also need
clocks = <&chip CLKID_SATA>;
here and corresponding code in the PHY driver.

If SATA PHY is accessing SATA registers, disabling the clock will
most likely lock-up the SoC.

Sebastian

> + #phy-cells = <1>;
> + status = "disabled";
> + };
> +
> apb@fc0000 {
> compatible = "simple-bus";
> #address-cells = <1>;
>

2014-06-16 10:46:29

by Sebastian Hesselbarth

[permalink] [raw]
Subject: Re: [PATCH v6 0/7] ARM: berlin: add AHCI support

On 06/16/2014 12:26 PM, Antoine Ténart wrote:
> This series adds the support for Berlin SoC AHCI controller. The
> controller allows to use the SATA host interface and, for example, the
> eSATA port on the BG2Q.
>
> The series adds a PHY driver to control the two SATA ports available,
> and adds a generic compatible to use the existing ahci_platform driver.

Besides the comment about adding SATA clock to SATA PHY node and driver,

Acked-by: Sebastian Hesselbarth <[email protected]>

for patches 1-5.

> Also enable the eSATA interface on the BG2Q DMP.
>
> Changes sinve v5:
> - rebased on top of v3.16-rc1
> - added the clocks property in the sata node
>
> Changes since v4:
> - updated PHY driver as tristate
> - handled the case were no SATA port is enabled
> - updated the compatible to a generic one
> - cosmetic fixups
>
> Changes since v3:
> - moved all PHY operations to the PHY driver
> - removed PHY sub-nodes
> - removed the custom Berlin AHCI driver and switched to
> ahci_platform
> - added multiple PHYs support to the libahci_platform
>
> Changes since v2:
> - modeled each PHY as a sub-node
> - cosmetic fixups
>
> Changes since v1:
> - added a PHY driver, allowing to enable each port
> individually and removed the 'force-port-map' property
> - made the drivers a bit less magic :)
> - wrote a function to select and configure registers in the
> AHCI driver
> - removed BG2 / BG2CD nodes
>
> Antoine Ténart (7):
> phy: add a driver for the Berlin SATA PHY
> Documentation: bindings: add the Berlin SATA PHY
> ata: libahci: allow to use multiple PHYs
> ata: ahci_platform: add a generic AHCI compatible
> Documentation: bindings: document the sub-nodes AHCI bindings
> ARM: berlin: add the AHCI node for the BG2Q
> ARM: berlin: enable the eSATA interface on the BG2Q DMP
>
> .../devicetree/bindings/ata/ahci-platform.txt | 37 ++++
> .../devicetree/bindings/phy/berlin-sata-phy.txt | 14 ++
> arch/arm/boot/dts/berlin2q-marvell-dmp.dts | 8 +
> arch/arm/boot/dts/berlin2q.dtsi | 28 +++
> drivers/ata/ahci.h | 3 +-
> drivers/ata/ahci_platform.c | 2 +
> drivers/ata/libahci.c | 7 +
> drivers/ata/libahci_platform.c | 170 +++++++++++----
> drivers/phy/Kconfig | 7 +
> drivers/phy/Makefile | 1 +
> drivers/phy/phy-berlin-sata.c | 232 +++++++++++++++++++++
> 11 files changed, 473 insertions(+), 36 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/phy/berlin-sata-phy.txt
> create mode 100644 drivers/phy/phy-berlin-sata.c
>

2014-06-17 18:17:09

by Sebastian Hesselbarth

[permalink] [raw]
Subject: Re: [PATCH v6 1/7] phy: add a driver for the Berlin SATA PHY

On 06/16/2014 12:26 PM, Antoine Ténart wrote:
> The Berlin SoC has a two SATA ports. Add a PHY driver to handle them.
>
> The mode selection can let us think this PHY can be configured to fit
> other purposes. But there are reasons to think the SATA mode will be
> the only one usable: the PHY registers are only accessible indirectly
> through two registers in the SATA range, the PHY seems to be integrated
> and no information tells us the contrary. For these reasons, make the
> driver a SATA PHY driver.
>
> Signed-off-by: Antoine Ténart <[email protected]>
> ---
[...]
> diff --git a/drivers/phy/phy-berlin-sata.c b/drivers/phy/phy-berlin-sata.c
> new file mode 100644
> index 000000000000..907897a02672
> --- /dev/null
> +++ b/drivers/phy/phy-berlin-sata.c
> @@ -0,0 +1,232 @@
> +/*
> + * Marvell Berlin SATA PHY driver
> + *
> + * Copyright (C) 2014 Marvell Technology Group Ltd.
> + *
> + * Antoine Ténart <[email protected]>
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2. This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/phy/phy.h>
> +#include <linux/io.h>
> +#include <linux/platform_device.h>
> +
> +#define HOST_VSA_ADDR 0x0
> +#define HOST_VSA_DATA 0x4
> +#define PORT_VSR_ADDR 0x78
> +#define PORT_VSR_DATA 0x7c
> +#define PORT_SCR_CTL 0x2c
> +
> +#define CONTROL_REGISTER 0x0
> +#define MBUS_SIZE_CONTROL 0x4
> +
> +#define POWER_DOWN_PHY0 BIT(6)
> +#define POWER_DOWN_PHY1 BIT(14)
> +#define MBUS_WRITE_REQUEST_SIZE_128 (BIT(2) << 16)
> +#define MBUS_READ_REQUEST_SIZE_128 (BIT(2) << 19)
> +
> +#define PHY_BASE 0x200

Antoine,

I gave your Berlin AHCI patches a try on BG2. I finally got it working
but BG2 has a different PHY_BASE and need some register fixups.

Please update this patch and the DT bindings to reflect the difference
of BG2Q with respect to BG2 as below.

[...]
> +
> +static const struct of_device_id phy_berlin_sata_of_match[] = {
> + { .compatible = "marvell,berlin-sata-phy" },

s/marvell,berlin-sata-phy/marvell,berlin2-sata-phy/

and add

marvell,berlin2q-sata-phy

That way it can be applied now without proper support for BG2
and I can send patches later.

Sebastian

> + { },
> +};
> +
> +static struct platform_driver phy_berlin_sata_driver = {
> + .probe = phy_berlin_sata_probe,
> + .driver = {
> + .name = "phy-berlin-sata",
> + .owner = THIS_MODULE,
> + .of_match_table = phy_berlin_sata_of_match,
> + },
> +};
> +module_platform_driver(phy_berlin_sata_driver);
> +
> +MODULE_DESCRIPTION("Marvell Berlin SATA PHY driver");
> +MODULE_AUTHOR("Antoine Ténart <[email protected]>");
> +MODULE_LICENSE("GPL v2");
>

2014-06-23 13:07:48

by Antoine Tenart

[permalink] [raw]
Subject: Re: [PATCH v6 1/7] phy: add a driver for the Berlin SATA PHY

Sebastian,

On Tue, Jun 17, 2014 at 08:17:02PM +0200, Sebastian Hesselbarth wrote:
> On 06/16/2014 12:26 PM, Antoine T?nart wrote:
> > +
> > +#define PHY_BASE 0x200
>
> Antoine,
>
> I gave your Berlin AHCI patches a try on BG2. I finally got it working
> but BG2 has a different PHY_BASE and need some register fixups.
>
> Please update this patch and the DT bindings to reflect the difference
> of BG2Q with respect to BG2 as below.
>
> [...]
> > +
> > +static const struct of_device_id phy_berlin_sata_of_match[] = {
> > + { .compatible = "marvell,berlin-sata-phy" },
>
> s/marvell,berlin-sata-phy/marvell,berlin2-sata-phy/
>
> and add
>
> marvell,berlin2q-sata-phy
>
> That way it can be applied now without proper support for BG2
> and I can send patches later.

Sure. I'll just s/marvell,berlin-sata-phy/marvell,berlin2q-sata-phy/ so
we don't have a non working compatible.

Antoine

--
Antoine T?nart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

2014-06-24 12:01:26

by Kishon Vijay Abraham I

[permalink] [raw]
Subject: Re: [PATCH v6 1/7] phy: add a driver for the Berlin SATA PHY

Hi,

On Monday 16 June 2014 03:56 PM, Antoine Ténart wrote:
> The Berlin SoC has a two SATA ports. Add a PHY driver to handle them.
>
> The mode selection can let us think this PHY can be configured to fit
> other purposes. But there are reasons to think the SATA mode will be
> the only one usable: the PHY registers are only accessible indirectly
> through two registers in the SATA range, the PHY seems to be integrated
> and no information tells us the contrary. For these reasons, make the
> driver a SATA PHY driver.
>
> Signed-off-by: Antoine Ténart <[email protected]>
> ---
> drivers/phy/Kconfig | 7 ++
> drivers/phy/Makefile | 1 +
> drivers/phy/phy-berlin-sata.c | 232 ++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 240 insertions(+)
> create mode 100644 drivers/phy/phy-berlin-sata.c
>
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index 16a2f067c242..365ad3651e1c 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -15,6 +15,13 @@ config GENERIC_PHY
> phy users can obtain reference to the PHY. All the users of this
> framework should select this config.
>
> +config PHY_BERLIN_SATA
> + tristate "Marvell Berlin SATA PHY driver"
> + depends on ARCH_BERLIN && OF
> + select GENERIC_PHY
> + help
> + Enable this to support the SATA PHY on Marvell Berlin SoCs.
> +
> config PHY_EXYNOS_MIPI_VIDEO
> tristate "S5P/EXYNOS SoC series MIPI CSI-2/DSI PHY driver"
> depends on HAS_IOMEM
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index b4f1d5770601..a137a2e23218 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -3,6 +3,7 @@
> #
>
> obj-$(CONFIG_GENERIC_PHY) += phy-core.o
> +obj-$(CONFIG_PHY_BERLIN_SATA) += phy-berlin-sata.o
> obj-$(CONFIG_BCM_KONA_USB2_PHY) += phy-bcm-kona-usb2.o
> obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO) += phy-exynos-dp-video.o
> obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO) += phy-exynos-mipi-video.o
> diff --git a/drivers/phy/phy-berlin-sata.c b/drivers/phy/phy-berlin-sata.c
> new file mode 100644
> index 000000000000..907897a02672
> --- /dev/null
> +++ b/drivers/phy/phy-berlin-sata.c
> @@ -0,0 +1,232 @@
> +/*
> + * Marvell Berlin SATA PHY driver
> + *
> + * Copyright (C) 2014 Marvell Technology Group Ltd.
> + *
> + * Antoine Ténart <[email protected]>
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2. This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/phy/phy.h>
> +#include <linux/io.h>
> +#include <linux/platform_device.h>
> +
> +#define HOST_VSA_ADDR 0x0
> +#define HOST_VSA_DATA 0x4
> +#define PORT_VSR_ADDR 0x78
> +#define PORT_VSR_DATA 0x7c
> +#define PORT_SCR_CTL 0x2c
> +
> +#define CONTROL_REGISTER 0x0
> +#define MBUS_SIZE_CONTROL 0x4
> +
> +#define POWER_DOWN_PHY0 BIT(6)
> +#define POWER_DOWN_PHY1 BIT(14)
> +#define MBUS_WRITE_REQUEST_SIZE_128 (BIT(2) << 16)
> +#define MBUS_READ_REQUEST_SIZE_128 (BIT(2) << 19)
> +
> +#define PHY_BASE 0x200
> +
> +/* register 0x01 */
> +#define REF_FREF_SEL_25 BIT(0)
> +#define PHY_MODE_SATA (0x0 << 5)
> +
> +/* register 0x02 */
> +#define USE_MAX_PLL_RATE BIT(12)
> +
> +/* register 0x23 */
> +#define DATA_BIT_WIDTH_10 (0x0 << 10)
> +#define DATA_BIT_WIDTH_20 (0x1 << 10)
> +#define DATA_BIT_WIDTH_40 (0x2 << 10)
> +
> +/* register 0x25 */
> +#define PHY_GEN_MAX_1_5 (0x0 << 10)
> +#define PHY_GEN_MAX_3_0 (0x1 << 10)
> +#define PHY_GEN_MAX_6_0 (0x2 << 10)
> +
> +#define BERLIN_SATA_PHY_NB 2

multi-phy PHY providers should be modelled so that each individual
PHY is made as sub-node of the PHY provider. Please refer [1] for the
discussion on this.

[1] -> https://lkml.org/lkml/2014/6/24/131

Thanks
Kishon
> +
> +#define to_berlin_sata_phy_priv(desc) \
> + container_of((desc), struct phy_berlin_priv, phys[(desc)->index])
> +
> +struct phy_berlin_desc {
> + struct phy *phy;
> + u32 val;
> + unsigned index;
> +};
> +
> +struct phy_berlin_priv {
> + void __iomem *base;
> + spinlock_t lock;
> + struct phy_berlin_desc phys[BERLIN_SATA_PHY_NB];
> +};
> +
> +static inline void phy_berlin_sata_reg_setbits(void __iomem *ctrl_reg, u32 reg,
> + u32 mask, u32 val)
> +{
> + u32 regval;
> +
> + /* select register */
> + writel(PHY_BASE + reg, ctrl_reg + PORT_VSR_ADDR);
> +
> + /* set bits */
> + regval = readl(ctrl_reg + PORT_VSR_DATA);
> + regval &= ~mask;
> + regval |= val;
> + writel(regval, ctrl_reg + PORT_VSR_DATA);
> +}
> +
> +static int phy_berlin_sata_power_on(struct phy *phy)
> +{
> + struct phy_berlin_desc *desc = phy_get_drvdata(phy);
> + struct phy_berlin_priv *priv = to_berlin_sata_phy_priv(desc);
> + void __iomem *ctrl_reg = priv->base + 0x60 + (desc->index * 0x80);
> + int ret = 0;
> + u32 regval;
> +
> + spin_lock(&priv->lock);
> +
> + /* Power on PHY */
> + writel(CONTROL_REGISTER, priv->base + HOST_VSA_ADDR);
> + regval = readl(priv->base + HOST_VSA_DATA);
> + regval &= ~(desc->val);
> + writel(regval, priv->base + HOST_VSA_DATA);
> +
> + /* Configure MBus */
> + writel(MBUS_SIZE_CONTROL, priv->base + HOST_VSA_ADDR);
> + regval = readl(priv->base + HOST_VSA_DATA);
> + regval |= MBUS_WRITE_REQUEST_SIZE_128 | MBUS_READ_REQUEST_SIZE_128;
> + writel(regval, priv->base + HOST_VSA_DATA);
> +
> + /* set PHY mode and ref freq to 25 MHz */
> + phy_berlin_sata_reg_setbits(ctrl_reg, 0x1, 0xff,
> + REF_FREF_SEL_25 | PHY_MODE_SATA);
> +
> + /* set PHY up to 6 Gbps */
> + phy_berlin_sata_reg_setbits(ctrl_reg, 0x25, 0xc00, PHY_GEN_MAX_6_0);
> +
> + /* set 40 bits width */
> + phy_berlin_sata_reg_setbits(ctrl_reg, 0x23, 0xc00, DATA_BIT_WIDTH_40);
> +
> + /* use max pll rate */
> + phy_berlin_sata_reg_setbits(ctrl_reg, 0x2, 0x0, USE_MAX_PLL_RATE);
> +
> + /* set the controller speed */
> + writel(0x31, ctrl_reg + PORT_SCR_CTL);
> +
> + spin_unlock(&priv->lock);
> +
> + return ret;
> +}
> +
> +static int phy_berlin_sata_power_off(struct phy *phy)
> +{
> + struct phy_berlin_desc *desc = phy_get_drvdata(phy);
> + struct phy_berlin_priv *priv = to_berlin_sata_phy_priv(desc);
> + u32 regval;
> +
> + spin_lock(&priv->lock);
> +
> + /* Power down PHY */
> + writel(CONTROL_REGISTER, priv->base + HOST_VSA_ADDR);
> + regval = readl(priv->base + HOST_VSA_DATA);
> + regval |= desc->val;
> + writel(regval, priv->base + HOST_VSA_DATA);
> +
> + spin_unlock(&priv->lock);
> +
> + return 0;
> +}
> +
> +static struct phy *phy_berlin_sata_phy_xlate(struct device *dev,
> + struct of_phandle_args *args)
> +{
> + struct phy_berlin_priv *priv = dev_get_drvdata(dev);
> +
> + if (WARN_ON(args->args[0] >= BERLIN_SATA_PHY_NB))
> + return ERR_PTR(-ENODEV);
> +
> + return priv->phys[args->args[0]].phy;
> +}
> +
> +static struct phy_ops phy_berlin_sata_ops = {
> + .power_on = phy_berlin_sata_power_on,
> + .power_off = phy_berlin_sata_power_off,
> + .owner = THIS_MODULE,
> +};
> +
> +static u32 phy_berlin_power_down_bits[] = {
> + POWER_DOWN_PHY0,
> + POWER_DOWN_PHY1,
> +};
> +
> +static int phy_berlin_sata_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct phy *phy;
> + struct phy_provider *phy_provider;
> + struct phy_berlin_priv *priv;
> + struct resource *res;
> + int i;
> +
> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res)
> + return -EINVAL;
> +
> + priv->base = devm_ioremap(dev, res->start, resource_size(res));
> + if (!priv->base)
> + return -ENOMEM;
> +
> + dev_set_drvdata(dev, priv);
> + spin_lock_init(&priv->lock);
> +
> + for (i = 0; i < BERLIN_SATA_PHY_NB; i++) {
> + phy = devm_phy_create(dev, &phy_berlin_sata_ops, NULL);
> + if (IS_ERR(phy)) {
> + dev_err(dev, "failed to create PHY %d\n", i);
> + return PTR_ERR(phy);
> + }
> +
> + priv->phys[i].phy = phy;
> + priv->phys[i].val = phy_berlin_power_down_bits[i];
> + priv->phys[i].index = i;
> + phy_set_drvdata(phy, &priv->phys[i]);
> +
> + /* Make sure the PHY is off */
> + phy_berlin_sata_power_off(phy);
> + }
> +
> + phy_provider =
> + devm_of_phy_provider_register(dev, phy_berlin_sata_phy_xlate);
> + if (IS_ERR(phy_provider))
> + return PTR_ERR(phy_provider);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id phy_berlin_sata_of_match[] = {
> + { .compatible = "marvell,berlin-sata-phy" },
> + { },
> +};
> +
> +static struct platform_driver phy_berlin_sata_driver = {
> + .probe = phy_berlin_sata_probe,
> + .driver = {
> + .name = "phy-berlin-sata",
> + .owner = THIS_MODULE,
> + .of_match_table = phy_berlin_sata_of_match,
> + },
> +};
> +module_platform_driver(phy_berlin_sata_driver);
> +
> +MODULE_DESCRIPTION("Marvell Berlin SATA PHY driver");
> +MODULE_AUTHOR("Antoine Ténart <[email protected]>");
> +MODULE_LICENSE("GPL v2");
>

2014-06-24 12:08:57

by Varka Bhadram

[permalink] [raw]
Subject: Re: [PATCH v6 1/7] phy: add a driver for the Berlin SATA PHY

On 06/24/2014 05:30 PM, Kishon Vijay Abraham I wrote:
> Hi,
>
> On Monday 16 June 2014 03:56 PM, Antoine Ténart wrote:
>> The Berlin SoC has a two SATA ports. Add a PHY driver to handle them.
>>
>> The mode selection can let us think this PHY can be configured to fit
>> other purposes. But there are reasons to think the SATA mode will be
>> the only one usable: the PHY registers are only accessible indirectly
>> through two registers in the SATA range, the PHY seems to be integrated
>> and no information tells us the contrary. For these reasons, make the
>> driver a SATA PHY driver.
>>
>> Signed-off-by: Antoine Ténart <[email protected]>
>> ---
>> drivers/phy/Kconfig | 7 ++
>> drivers/phy/Makefile | 1 +
>> drivers/phy/phy-berlin-sata.c | 232 ++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 240 insertions(+)
>> create mode 100644 drivers/phy/phy-berlin-sata.c
>>
>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>> index 16a2f067c242..365ad3651e1c 100644
>> --- a/drivers/phy/Kconfig
>> +++ b/drivers/phy/Kconfig
>> @@ -15,6 +15,13 @@ config GENERIC_PHY
>> phy users can obtain reference to the PHY. All the users of this
>> framework should select this config.
>>
>> +config PHY_BERLIN_SATA
>> + tristate "Marvell Berlin SATA PHY driver"
>> + depends on ARCH_BERLIN && OF
>> + select GENERIC_PHY
>> + help
>> + Enable this to support the SATA PHY on Marvell Berlin SoCs.
>> +
>> config PHY_EXYNOS_MIPI_VIDEO
>> tristate "S5P/EXYNOS SoC series MIPI CSI-2/DSI PHY driver"
>> depends on HAS_IOMEM
>> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
>> index b4f1d5770601..a137a2e23218 100644
>> --- a/drivers/phy/Makefile
>> +++ b/drivers/phy/Makefile
>> @@ -3,6 +3,7 @@
>> #
>>
>> obj-$(CONFIG_GENERIC_PHY) += phy-core.o
>> +obj-$(CONFIG_PHY_BERLIN_SATA) += phy-berlin-sata.o
>> obj-$(CONFIG_BCM_KONA_USB2_PHY) += phy-bcm-kona-usb2.o
>> obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO) += phy-exynos-dp-video.o
>> obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO) += phy-exynos-mipi-video.o
>> diff --git a/drivers/phy/phy-berlin-sata.c b/drivers/phy/phy-berlin-sata.c
>> new file mode 100644
>> index 000000000000..907897a02672
>> --- /dev/null
>> +++ b/drivers/phy/phy-berlin-sata.c
>> @@ -0,0 +1,232 @@
>> +/*
>> + * Marvell Berlin SATA PHY driver
>> + *
>> + * Copyright (C) 2014 Marvell Technology Group Ltd.
>> + *
>> + * Antoine Ténart <[email protected]>
>> + *
>> + * This file is licensed under the terms of the GNU General Public
>> + * License version 2. This program is licensed "as is" without any
>> + * warranty of any kind, whether express or implied.
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/phy/phy.h>
>> +#include <linux/io.h>
>> +#include <linux/platform_device.h>
>> +
>> +#define HOST_VSA_ADDR 0x0
>> +#define HOST_VSA_DATA 0x4
>> +#define PORT_VSR_ADDR 0x78
>> +#define PORT_VSR_DATA 0x7c
>> +#define PORT_SCR_CTL 0x2c
>> +
>> +#define CONTROL_REGISTER 0x0
>> +#define MBUS_SIZE_CONTROL 0x4
>> +
>> +#define POWER_DOWN_PHY0 BIT(6)
>> +#define POWER_DOWN_PHY1 BIT(14)
>> +#define MBUS_WRITE_REQUEST_SIZE_128 (BIT(2) << 16)
>> +#define MBUS_READ_REQUEST_SIZE_128 (BIT(2) << 19)
>> +
>> +#define PHY_BASE 0x200
>> +
>> +/* register 0x01 */
>> +#define REF_FREF_SEL_25 BIT(0)
>> +#define PHY_MODE_SATA (0x0 << 5)
>> +
>> +/* register 0x02 */
>> +#define USE_MAX_PLL_RATE BIT(12)
>> +
>> +/* register 0x23 */
>> +#define DATA_BIT_WIDTH_10 (0x0 << 10)
>> +#define DATA_BIT_WIDTH_20 (0x1 << 10)
>> +#define DATA_BIT_WIDTH_40 (0x2 << 10)
>> +
>> +/* register 0x25 */
>> +#define PHY_GEN_MAX_1_5 (0x0 << 10)
>> +#define PHY_GEN_MAX_3_0 (0x1 << 10)
>> +#define PHY_GEN_MAX_6_0 (0x2 << 10)
>> +
>> +#define BERLIN_SATA_PHY_NB 2
> multi-phy PHY providers should be modelled so that each individual
> PHY is made as sub-node of the PHY provider. Please refer [1] for the
> discussion on this.
>
> [1] -> https://lkml.org/lkml/2014/6/24/131
>
> Thanks
> Kishon
>> +
>> +#define to_berlin_sata_phy_priv(desc) \
>> + container_of((desc), struct phy_berlin_priv, phys[(desc)->index])
>> +
>> +struct phy_berlin_desc {
>> + struct phy *phy;
>> + u32 val;
>> + unsigned index;
>> +};
>> +
>> +struct phy_berlin_priv {
>> + void __iomem *base;
>> + spinlock_t lock;
>> + struct phy_berlin_desc phys[BERLIN_SATA_PHY_NB];
>> +};
>> +
>> +static inline void phy_berlin_sata_reg_setbits(void __iomem *ctrl_reg, u32 reg,
>> + u32 mask, u32 val)
>> +{
>> + u32 regval;
>> +
>> + /* select register */
>> + writel(PHY_BASE + reg, ctrl_reg + PORT_VSR_ADDR);
>> +
>> + /* set bits */
>> + regval = readl(ctrl_reg + PORT_VSR_DATA);
>> + regval &= ~mask;
>> + regval |= val;
>> + writel(regval, ctrl_reg + PORT_VSR_DATA);
>> +}
>> +
>> +static int phy_berlin_sata_power_on(struct phy *phy)
>> +{
>> + struct phy_berlin_desc *desc = phy_get_drvdata(phy);
>> + struct phy_berlin_priv *priv = to_berlin_sata_phy_priv(desc);
>> + void __iomem *ctrl_reg = priv->base + 0x60 + (desc->index * 0x80);
>> + int ret = 0;
>> + u32 regval;
>> +
>> + spin_lock(&priv->lock);
>> +
>> + /* Power on PHY */
>> + writel(CONTROL_REGISTER, priv->base + HOST_VSA_ADDR);
>> + regval = readl(priv->base + HOST_VSA_DATA);
>> + regval &= ~(desc->val);
>> + writel(regval, priv->base + HOST_VSA_DATA);
>> +
>> + /* Configure MBus */
>> + writel(MBUS_SIZE_CONTROL, priv->base + HOST_VSA_ADDR);
>> + regval = readl(priv->base + HOST_VSA_DATA);
>> + regval |= MBUS_WRITE_REQUEST_SIZE_128 | MBUS_READ_REQUEST_SIZE_128;
>> + writel(regval, priv->base + HOST_VSA_DATA);
>> +
>> + /* set PHY mode and ref freq to 25 MHz */
>> + phy_berlin_sata_reg_setbits(ctrl_reg, 0x1, 0xff,
>> + REF_FREF_SEL_25 | PHY_MODE_SATA);
>> +
>> + /* set PHY up to 6 Gbps */
>> + phy_berlin_sata_reg_setbits(ctrl_reg, 0x25, 0xc00, PHY_GEN_MAX_6_0);
>> +
>> + /* set 40 bits width */
>> + phy_berlin_sata_reg_setbits(ctrl_reg, 0x23, 0xc00, DATA_BIT_WIDTH_40);
>> +
>> + /* use max pll rate */
>> + phy_berlin_sata_reg_setbits(ctrl_reg, 0x2, 0x0, USE_MAX_PLL_RATE);
>> +
>> + /* set the controller speed */
>> + writel(0x31, ctrl_reg + PORT_SCR_CTL);
>> +
>> + spin_unlock(&priv->lock);
>> +
>> + return ret;
>> +}
>> +
>> +static int phy_berlin_sata_power_off(struct phy *phy)
>> +{
>> + struct phy_berlin_desc *desc = phy_get_drvdata(phy);
>> + struct phy_berlin_priv *priv = to_berlin_sata_phy_priv(desc);
>> + u32 regval;
>> +
>> + spin_lock(&priv->lock);
>> +
>> + /* Power down PHY */
>> + writel(CONTROL_REGISTER, priv->base + HOST_VSA_ADDR);
>> + regval = readl(priv->base + HOST_VSA_DATA);
>> + regval |= desc->val;
>> + writel(regval, priv->base + HOST_VSA_DATA);
>> +
>> + spin_unlock(&priv->lock);
>> +
>> + return 0;
>> +}
>> +
>> +static struct phy *phy_berlin_sata_phy_xlate(struct device *dev,
>> + struct of_phandle_args *args)

Indent the function properly like this:
static struct phy *phy_berlin_sata_phy_xlate(struct device *dev,
struct of_phandle_args *args)

check with checkpatch.pl script

>> +{
>> + struct phy_berlin_priv *priv = dev_get_drvdata(dev);
>> +
>> + if (WARN_ON(args->args[0] >= BERLIN_SATA_PHY_NB))
>> + return ERR_PTR(-ENODEV);
>> +
>> + return priv->phys[args->args[0]].phy;
>> +}
>> +
>> +static struct phy_ops phy_berlin_sata_ops = {
>> + .power_on = phy_berlin_sata_power_on,
>> + .power_off = phy_berlin_sata_power_off,
>> + .owner = THIS_MODULE,
>> +};
>> +
>> +static u32 phy_berlin_power_down_bits[] = {
>> + POWER_DOWN_PHY0,
>> + POWER_DOWN_PHY1,
>> +};
>> +
>> +static int phy_berlin_sata_probe(struct platform_device *pdev)
>> +{
>> + struct device *dev = &pdev->dev;
>> + struct phy *phy;
>> + struct phy_provider *phy_provider;
>> + struct phy_berlin_priv *priv;
>> + struct resource *res;
>> + int i;
>> +
>> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>> + if (!priv)
>> + return -ENOMEM;
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + if (!res)
>> + return -EINVAL;
>> +
>> + priv->base = devm_ioremap(dev, res->start, resource_size(res));
>> + if (!priv->base)
>> + return -ENOMEM;
>> +
>> + dev_set_drvdata(dev, priv);
>> + spin_lock_init(&priv->lock);
>> +
>> + for (i = 0; i < BERLIN_SATA_PHY_NB; i++) {
>> + phy = devm_phy_create(dev, &phy_berlin_sata_ops, NULL);
>> + if (IS_ERR(phy)) {
>> + dev_err(dev, "failed to create PHY %d\n", i);
>> + return PTR_ERR(phy);
>> + }
>> +
>> + priv->phys[i].phy = phy;
>> + priv->phys[i].val = phy_berlin_power_down_bits[i];
>> + priv->phys[i].index = i;
>> + phy_set_drvdata(phy, &priv->phys[i]);
>> +
>> + /* Make sure the PHY is off */
>> + phy_berlin_sata_power_off(phy);
>> + }
>> +
>> + phy_provider =
>> + devm_of_phy_provider_register(dev, phy_berlin_sata_phy_xlate);
>> + if (IS_ERR(phy_provider))
>> + return PTR_ERR(phy_provider);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct of_device_id phy_berlin_sata_of_match[] = {
>> + { .compatible = "marvell,berlin-sata-phy" },
>> + { },
>> +};
>> +
>> +static struct platform_driver phy_berlin_sata_driver = {
>> + .probe = phy_berlin_sata_probe,
>> + .driver = {
>> + .name = "phy-berlin-sata",
>> + .owner = THIS_MODULE,
>> + .of_match_table = phy_berlin_sata_of_match,

use of_match_ptr for of_match_table

>> + },
>> +};
>> +module_platform_driver(phy_berlin_sata_driver);
>> +
>> +MODULE_DESCRIPTION("Marvell Berlin SATA PHY driver");
>> +MODULE_AUTHOR("Antoine Ténart <[email protected]>");
>> +MODULE_LICENSE("GPL v2");
>>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2014-06-24 12:15:34

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v6 1/7] phy: add a driver for the Berlin SATA PHY

On Tue, 24 Jun 2014, Varka Bhadram wrote:
> On 06/24/2014 05:30 PM, Kishon Vijay Abraham I wrote:
> >On Monday 16 June 2014 03:56 PM, Antoine Ténart wrote:
> >>The Berlin SoC has a two SATA ports. Add a PHY driver to handle them.
> >>
> >>The mode selection can let us think this PHY can be configured to fit
> >>other purposes. But there are reasons to think the SATA mode will be
> >>the only one usable: the PHY registers are only accessible indirectly
> >>through two registers in the SATA range, the PHY seems to be integrated
> >>and no information tells us the contrary. For these reasons, make the
> >>driver a SATA PHY driver.
> >>
> >>Signed-off-by: Antoine Ténart <[email protected]>
> >>---
> >> drivers/phy/Kconfig | 7 ++
> >> drivers/phy/Makefile | 1 +
> >> drivers/phy/phy-berlin-sata.c | 232 ++++++++++++++++++++++++++++++++++++++++++
> >> 3 files changed, 240 insertions(+)
> >> create mode 100644 drivers/phy/phy-berlin-sata.c

Please snip all unseccersary code when replying to patches.

[...]

> >>+static struct phy *phy_berlin_sata_phy_xlate(struct device *dev,
> >>+ struct of_phandle_args *args)
>
> Indent the function properly like this:
> static struct phy *phy_berlin_sata_phy_xlate(struct device *dev,
> struct of_phandle_args *args)

You can also indent with TABs, especially if it means the following
line(s) would wrap.

> check with checkpatch.pl script

Snippy, snippy.

[...]

> >>+static struct platform_driver phy_berlin_sata_driver = {
> >>+ .probe = phy_berlin_sata_probe,
> >>+ .driver = {
> >>+ .name = "phy-berlin-sata",
> >>+ .owner = THIS_MODULE,
> >>+ .of_match_table = phy_berlin_sata_of_match,
>
> use of_match_ptr for of_match_table

What use is this?

[...]

--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2014-06-24 12:22:59

by Varka Bhadram

[permalink] [raw]
Subject: Re: [PATCH v6 1/7] phy: add a driver for the Berlin SATA PHY

On 06/24/2014 05:45 PM, Lee Jones wrote:
> On Tue, 24 Jun 2014, Varka Bhadram wrote:
>> On 06/24/2014 05:30 PM, Kishon Vijay Abraham I wrote:
>>> On Monday 16 June 2014 03:56 PM, Antoine Ténart wrote:
>>>> The Berlin SoC has a two SATA ports. Add a PHY driver to handle them.
>>>>
>>>> The mode selection can let us think this PHY can be configured to fit
>>>> other purposes. But there are reasons to think the SATA mode will be
>>>> the only one usable: the PHY registers are only accessible indirectly
>>>> through two registers in the SATA range, the PHY seems to be integrated
>>>> and no information tells us the contrary. For these reasons, make the
>>>> driver a SATA PHY driver.
>>>>
>>>> Signed-off-by: Antoine Ténart <[email protected]>
>>>> ---
>>>> drivers/phy/Kconfig | 7 ++
>>>> drivers/phy/Makefile | 1 +
>>>> drivers/phy/phy-berlin-sata.c | 232 ++++++++++++++++++++++++++++++++++++++++++
>>>> 3 files changed, 240 insertions(+)
>>>> create mode 100644 drivers/phy/phy-berlin-sata.c

[...]

>>>> +static struct platform_driver phy_berlin_sata_driver = {
>>>> + .probe = phy_berlin_sata_probe,
>>>> + .driver = {
>>>> + .name = "phy-berlin-sata",
>>>> + .owner = THIS_MODULE,
>>>> + .of_match_table = phy_berlin_sata_of_match,
>> use of_match_ptr for of_match_table
> What use is this?
>
> [...]
>
of_match_table is NULL for Non-DT based. Better to use the of_match_ptr().
If driver is DT based 'of_match_table = phy_berlin_sata_of_match'
else 'of_match_table = NULL'

This is take care by of_match_ptr() macro.

Thanks,
Varka Bhadram

2014-06-24 12:39:56

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v6 1/7] phy: add a driver for the Berlin SATA PHY

On Tue, 24 Jun 2014, Varka Bhadram wrote:

> On 06/24/2014 05:45 PM, Lee Jones wrote:
> >On Tue, 24 Jun 2014, Varka Bhadram wrote:
> >>On 06/24/2014 05:30 PM, Kishon Vijay Abraham I wrote:
> >>>On Monday 16 June 2014 03:56 PM, Antoine Ténart wrote:
> >>>>The Berlin SoC has a two SATA ports. Add a PHY driver to handle them.
> >>>>
> >>>>The mode selection can let us think this PHY can be configured to fit
> >>>>other purposes. But there are reasons to think the SATA mode will be
> >>>>the only one usable: the PHY registers are only accessible indirectly
> >>>>through two registers in the SATA range, the PHY seems to be integrated
> >>>>and no information tells us the contrary. For these reasons, make the
> >>>>driver a SATA PHY driver.
> >>>>
> >>>>Signed-off-by: Antoine Ténart <[email protected]>
> >>>>---
> >>>> drivers/phy/Kconfig | 7 ++
> >>>> drivers/phy/Makefile | 1 +
> >>>> drivers/phy/phy-berlin-sata.c | 232 ++++++++++++++++++++++++++++++++++++++++++
> >>>> 3 files changed, 240 insertions(+)
> >>>> create mode 100644 drivers/phy/phy-berlin-sata.c
>
> [...]
>
> >>>>+static struct platform_driver phy_berlin_sata_driver = {
> >>>>+ .probe = phy_berlin_sata_probe,
> >>>>+ .driver = {
> >>>>+ .name = "phy-berlin-sata",
> >>>>+ .owner = THIS_MODULE,
> >>>>+ .of_match_table = phy_berlin_sata_of_match,
> >>use of_match_ptr for of_match_table
> >What use is this?
> >
> >[...]
> >
> of_match_table is NULL for Non-DT based. Better to use the of_match_ptr().
> If driver is DT based 'of_match_table = phy_berlin_sata_of_match'
> else 'of_match_table = NULL'
>
> This is take care by of_match_ptr() macro.

This driver 'depends on OF', so it's okay to always populate
.of_match_table.

--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2014-06-30 10:25:29

by Antoine Tenart

[permalink] [raw]
Subject: Re: [PATCH v6 1/7] phy: add a driver for the Berlin SATA PHY

Hello,

On Tue, Jun 24, 2014 at 05:37:56PM +0530, Varka Bhadram wrote:
> On 06/24/2014 05:30 PM, Kishon Vijay Abraham I wrote:
> >On Monday 16 June 2014 03:56 PM, Antoine T?nart wrote:
> >>+
> >>+static struct phy *phy_berlin_sata_phy_xlate(struct device *dev,
> >>+ struct of_phandle_args *args)
>
> Indent the function properly like this:
> static struct phy *phy_berlin_sata_phy_xlate(struct device *dev,
> struct of_phandle_args *args)
>
> check with checkpatch.pl script

Already fixed in v7.


Antoine

--
Antoine T?nart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com