[PATCH v3 0/5] AHCI and SATA PHY support for Broadcom STB SoCs

[PATCH v3 0/5] AHCI and SATA PHY support for Broadcom STB SoCs

[Brian Norris]

Few changes since v2, and the changes are *only* to the SATA driver, not to any
of the DT bindings or the PHY driver. I'm resending the whole series for
completeness.

I'll list the changelog in each patch, but in summary:

v2 -> v3:
 - straighten out endianness for big endian MIPS and ARM in AHCI driver
 - rename sata_brcmstb.c to ahci_brcmstb.c
 - bind ahci_brcmstb.c against the specific string, "brcm,bcm7445-ahci" instead
   of the generic one

Brian

Brian Norris (5):
  Documentation: devicetree: add Broadcom SATA binding
  Documentation: devicetree: add Broadcom SATA PHY binding
  ata: add Broadcom AHCI SATA3 driver for STB chips
  phy: add Broadcom SATA3 PHY driver for Broadcom STB SoCs
  ARM: dts: brcmstb: add nodes for SATA controller and PHY

 .../devicetree/bindings/ata/brcm,sata-brcmstb.txt  |  35 +++
 .../bindings/phy/brcm,brcmstb-sata-phy.txt         |  40 +++
 arch/arm/boot/dts/bcm7445.dtsi                     |  37 +++
 drivers/ata/Kconfig                                |   9 +
 drivers/ata/Makefile                               |   1 +
 drivers/ata/ahci_brcmstb.c                         | 322 +++++++++++++++++++++
 drivers/phy/Kconfig                                |   9 +
 drivers/phy/Makefile                               |   1 +
 drivers/phy/phy-brcmstb-sata.c                     | 216 ++++++++++++++
 9 files changed, 670 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/ata/brcm,sata-brcmstb.txt
 create mode 100644 Documentation/devicetree/bindings/phy/brcm,brcmstb-sata-phy.txt
 create mode 100644 drivers/ata/ahci_brcmstb.c
 create mode 100644 drivers/phy/phy-brcmstb-sata.c

-- 
1.9.1

[PATCH v3 1/5] Documentation: devicetree: add Broadcom SATA binding

[Brian Norris]

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
v3: no change
v2: no change

 .../devicetree/bindings/ata/brcm,sata-brcmstb.txt  | 35 ++++++++++++++++++++++
 1 file changed, 35 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/ata/brcm,sata-brcmstb.txt

diff --git a/Documentation/devicetree/bindings/ata/brcm,sata-brcmstb.txt b/Documentation/devicetree/bindings/ata/brcm,sata-brcmstb.txt
new file mode 100644
index 000000000000..afeede13e195
--- /dev/null
+++ b/Documentation/devicetree/bindings/ata/brcm,sata-brcmstb.txt
@@ -0,0 +1,35 @@
+* Broadcom SATA3 AHCI Controller for STB
+
+SATA nodes are defined to describe on-chip Serial ATA controllers.
+Each SATA controller should have its own node.
+
+Required properties:
+- compatible         : compatible list, may contain "brcm,bcm7445-ahci" and/or
+                       "brcm,sata3-ahci"
+- reg                : register mappings for AHCI and SATA_TOP_CTRL
+- reg-names          : "ahci" and "top-ctrl"
+- interrupts         : interrupt mapping for SATA IRQ
+
+Also see ahci-platform.txt.
+
+Example:
+
+	sata@f045a000 {
+		compatible = "brcm,bcm7445-ahci", "brcm,sata3-ahci";
+		reg = <0xf045a000 0xa9c>, <0xf0458040 0x24>;
+		reg-names = "ahci", "top-ctrl";
+		interrupts = <0 30 0>;
+		#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

Re: [PATCH v3 1/5] Documentation: devicetree: add Broadcom SATA binding

[Tejun Heo]

On Tue, May 12, 2015 at 04:28:19PM -0700, Brian Norris wrote:
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>

Applied to libata/for-4.2.

Thanks.

-- 
tejun

[PATCH v3 2/5] Documentation: devicetree: add Broadcom SATA PHY binding

[Brian Norris]

For 28nm STB chips, based on BCM7445.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
v3: no change

v2:
  - make each subnode into a provider, so we can use direct phandle references
    to them
  - drop the 'port-ctrl' register range, since this was shared with the SATA
    node 

 .../bindings/phy/brcm,brcmstb-sata-phy.txt         | 40 ++++++++++++++++++++++
 1 file changed, 40 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/brcm,brcmstb-sata-phy.txt

diff --git a/Documentation/devicetree/bindings/phy/brcm,brcmstb-sata-phy.txt b/Documentation/devicetree/bindings/phy/brcm,brcmstb-sata-phy.txt
new file mode 100644
index 000000000000..7f81ef90146a
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/brcm,brcmstb-sata-phy.txt
@@ -0,0 +1,40 @@
+* Broadcom SATA3 PHY for STB
+
+Required properties:
+- compatible: should be one or more of
+     "brcm,bcm7445-sata-phy"
+     "brcm,phy-sata3"
+- address-cells: should be 1
+- size-cells: should be 0
+- reg: register range for the PHY PCB interface
+- reg-names: should be "phy"
+
+Sub-nodes:
+  Each port's PHY should be represented as a sub-node.
+
+Sub-nodes required properties:
+- reg: the PHY number
+- phy-cells: generic PHY binding; must be 0
+Optional:
+- brcm,enable-ssc: use spread spectrum clocking (SSC) on this port
+
+
+Example:
+
+	sata-phy@f0458100 {
+		compatible = "brcm,bcm7445-sata-phy", "brcm,phy-sata3";
+		reg = <0xf0458100 0x1e00>, <0xf045804c 0x10>;
+		reg-names = "phy";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		sata-phy@0 {
+			reg = <0>;
+			#phy-cells = <0>;
+		};
+
+		sata-phy@1 {
+			reg = <1>;
+			#phy-cells = <0>;
+		};
+	};
-- 
1.9.1

[PATCH v3 3/5] ata: add Broadcom AHCI SATA3 driver for STB chips

[Brian Norris]

Pretty straightforward driver, using the nice library-ization of the
generic ahci_platform driver.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
v3:
 - straighten out endianness for big endian MIPS and ARM
 - rename from sata_brcmstb.c to ahci_brcmstb.c
 - bind against the specific string, "brcm,bcm7445-ahci" instead of the generic
   one

v2:
  - move port enabling into this driver, since the affected registers are in
    the SATA_TOP_CTRL block. This means we need to check for the implemented
    port(s) here.
    
  - fix up layering issues with using drvdata (libata expects to use drvdata),
    similar to this issue:
 
      http://marc.info/?l=linux-ide&m=142851961920009&w=2

  - trivial fixups

 drivers/ata/Kconfig        |   9 ++
 drivers/ata/Makefile       |   1 +
 drivers/ata/ahci_brcmstb.c | 322 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 332 insertions(+)
 create mode 100644 drivers/ata/ahci_brcmstb.c

diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index 5f601553b9b0..dfb273a1194b 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -98,6 +98,15 @@ config SATA_AHCI_PLATFORM
 
 	  If unsure, say N.
 
+config AHCI_BRCMSTB
+	tristate "Broadcom STB AHCI SATA support"
+	depends on ARCH_BRCMSTB
+	help
+	  This option enables support for the AHCI SATA3 controller found on
+	  STB SoC's.
+
+	  If unsure, say N.
+
 config AHCI_DA850
 	tristate "DaVinci DA850 AHCI SATA support"
 	depends on ARCH_DAVINCI_DA850
diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
index b67e995179a9..b95449184ae1 100644
--- a/drivers/ata/Makefile
+++ b/drivers/ata/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_SATA_INIC162X)	+= sata_inic162x.o
 obj-$(CONFIG_SATA_SIL24)	+= sata_sil24.o
 obj-$(CONFIG_SATA_DWC)		+= sata_dwc_460ex.o
 obj-$(CONFIG_SATA_HIGHBANK)	+= sata_highbank.o libahci.o
+obj-$(CONFIG_AHCI_BRCMSTB)	+= ahci_brcmstb.o libahci.o libahci_platform.o
 obj-$(CONFIG_AHCI_DA850)	+= ahci_da850.o libahci.o libahci_platform.o
 obj-$(CONFIG_AHCI_IMX)		+= ahci_imx.o libahci.o libahci_platform.o
 obj-$(CONFIG_AHCI_MVEBU)	+= ahci_mvebu.o libahci.o libahci_platform.o
diff --git a/drivers/ata/ahci_brcmstb.c b/drivers/ata/ahci_brcmstb.c
new file mode 100644
index 000000000000..ce1e3a885981
--- /dev/null
+++ b/drivers/ata/ahci_brcmstb.c
@@ -0,0 +1,322 @@
+/*
+ * Broadcom SATA3 AHCI Controller Driver
+ *
+ * Copyright © 2009-2015 Broadcom Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2, or (at your option)
+ * any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/ahci_platform.h>
+#include <linux/compiler.h>
+#include <linux/device.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/libata.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/string.h>
+
+#include "ahci.h"
+
+#define DRV_NAME					"brcm-ahci"
+
+#define SATA_TOP_CTRL_VERSION				0x0
+#define SATA_TOP_CTRL_BUS_CTRL				0x4
+ #define MMIO_ENDIAN_SHIFT				0 /* CPU->AHCI */
+ #define DMADESC_ENDIAN_SHIFT				2 /* AHCI->DDR */
+ #define DMADATA_ENDIAN_SHIFT				4 /* AHCI->DDR */
+ #define PIODATA_ENDIAN_SHIFT				6
+  #define ENDIAN_SWAP_NONE				0
+  #define ENDIAN_SWAP_FULL				2
+ #define OVERRIDE_HWINIT				BIT(16)
+#define SATA_TOP_CTRL_TP_CTRL				0x8
+#define SATA_TOP_CTRL_PHY_CTRL				0xc
+ #define SATA_TOP_CTRL_PHY_CTRL_1			0x0
+  #define SATA_TOP_CTRL_1_PHY_DEFAULT_POWER_STATE	BIT(14)
+ #define SATA_TOP_CTRL_PHY_CTRL_2			0x4
+  #define SATA_TOP_CTRL_2_SW_RST_MDIOREG		BIT(0)
+  #define SATA_TOP_CTRL_2_SW_RST_OOB			BIT(1)
+  #define SATA_TOP_CTRL_2_SW_RST_RX			BIT(2)
+  #define SATA_TOP_CTRL_2_SW_RST_TX			BIT(3)
+  #define SATA_TOP_CTRL_2_PHY_GLOBAL_RESET		BIT(14)
+ #define SATA_TOP_CTRL_PHY_OFFS				0x8
+ #define SATA_TOP_MAX_PHYS				2
+#define SATA_TOP_CTRL_SATA_TP_OUT			0x1c
+#define SATA_TOP_CTRL_CLIENT_INIT_CTRL			0x20
+
+/* On big-endian MIPS, buses are reversed to big endian, so switch them back */
+#if defined(CONFIG_MIPS) && defined(__BIG_ENDIAN)
+#define DATA_ENDIAN			 2 /* AHCI->DDR inbound accesses */
+#define MMIO_ENDIAN			 2 /* CPU->AHCI outbound accesses */
+#else
+#define DATA_ENDIAN			 0
+#define MMIO_ENDIAN			 0
+#endif
+
+#define BUS_CTRL_ENDIAN_CONF				\
+	((DATA_ENDIAN << DMADATA_ENDIAN_SHIFT) |	\
+	(DATA_ENDIAN << DMADESC_ENDIAN_SHIFT) |		\
+	(MMIO_ENDIAN << MMIO_ENDIAN_SHIFT))
+
+struct brcm_ahci_priv {
+	struct device *dev;
+	void __iomem *top_ctrl;
+	u32 port_mask;
+};
+
+static const struct ata_port_info ahci_brcm_port_info = {
+	.flags		= AHCI_FLAG_COMMON,
+	.pio_mask	= ATA_PIO4,
+	.udma_mask	= ATA_UDMA6,
+	.port_ops	= &ahci_platform_ops,
+};
+
+static inline u32 brcm_sata_readreg(void __iomem *addr)
+{
+	/*
+	 * MIPS endianness is configured by boot strap, which also reverses all
+	 * bus endianness (i.e., big-endian CPU + big endian bus ==> native
+	 * endian I/O).
+	 *
+	 * Other architectures (e.g., ARM) either do not support big endian, or
+	 * else leave I/O in little endian mode.
+	 */
+	if (IS_ENABLED(CONFIG_MIPS) && IS_ENABLED(__BIG_ENDIAN))
+		return __raw_readl(addr);
+	else
+		return readl_relaxed(addr);
+}
+
+static inline void brcm_sata_writereg(u32 val, void __iomem *addr)
+{
+	/* See brcm_sata_readreg() comments */
+	if (IS_ENABLED(CONFIG_MIPS) && IS_ENABLED(__BIG_ENDIAN))
+		__raw_writel(val, addr);
+	else
+		writel_relaxed(val, addr);
+}
+
+static void brcm_sata_phy_enable(struct brcm_ahci_priv *priv, int port)
+{
+	void __iomem *phyctrl = priv->top_ctrl + SATA_TOP_CTRL_PHY_CTRL +
+				(port * SATA_TOP_CTRL_PHY_OFFS);
+	void __iomem *p;
+	u32 reg;
+
+	/* clear PHY_DEFAULT_POWER_STATE */
+	p = phyctrl + SATA_TOP_CTRL_PHY_CTRL_1;
+	reg = brcm_sata_readreg(p);
+	reg &= ~SATA_TOP_CTRL_1_PHY_DEFAULT_POWER_STATE;
+	brcm_sata_writereg(reg, p);
+
+	/* reset the PHY digital logic */
+	p = phyctrl + SATA_TOP_CTRL_PHY_CTRL_2;
+	reg = brcm_sata_readreg(p);
+	reg &= ~(SATA_TOP_CTRL_2_SW_RST_MDIOREG | SATA_TOP_CTRL_2_SW_RST_OOB |
+		 SATA_TOP_CTRL_2_SW_RST_RX);
+	reg |= SATA_TOP_CTRL_2_SW_RST_TX;
+	brcm_sata_writereg(reg, p);
+	reg = brcm_sata_readreg(p);
+	reg |= SATA_TOP_CTRL_2_PHY_GLOBAL_RESET;
+	brcm_sata_writereg(reg, p);
+	reg = brcm_sata_readreg(p);
+	reg &= ~SATA_TOP_CTRL_2_PHY_GLOBAL_RESET;
+	brcm_sata_writereg(reg, p);
+	(void)brcm_sata_readreg(p);
+}
+
+static void brcm_sata_phy_disable(struct brcm_ahci_priv *priv, int port)
+{
+	void __iomem *phyctrl = priv->top_ctrl + SATA_TOP_CTRL_PHY_CTRL +
+				(port * SATA_TOP_CTRL_PHY_OFFS);
+	void __iomem *p;
+	u32 reg;
+
+	/* power-off the PHY digital logic */
+	p = phyctrl + SATA_TOP_CTRL_PHY_CTRL_2;
+	reg = brcm_sata_readreg(p);
+	reg |= (SATA_TOP_CTRL_2_SW_RST_MDIOREG | SATA_TOP_CTRL_2_SW_RST_OOB |
+		SATA_TOP_CTRL_2_SW_RST_RX | SATA_TOP_CTRL_2_SW_RST_TX |
+		SATA_TOP_CTRL_2_PHY_GLOBAL_RESET);
+	brcm_sata_writereg(reg, p);
+
+	/* set PHY_DEFAULT_POWER_STATE */
+	p = phyctrl + SATA_TOP_CTRL_PHY_CTRL_1;
+	reg = brcm_sata_readreg(p);
+	reg |= SATA_TOP_CTRL_1_PHY_DEFAULT_POWER_STATE;
+	brcm_sata_writereg(reg, p);
+}
+
+static void brcm_sata_phys_enable(struct brcm_ahci_priv *priv)
+{
+	int i;
+
+	for (i = 0; i < SATA_TOP_MAX_PHYS; i++)
+		if (priv->port_mask & BIT(i))
+			brcm_sata_phy_enable(priv, i);
+}
+
+static void brcm_sata_phys_disable(struct brcm_ahci_priv *priv)
+{
+	int i;
+
+	for (i = 0; i < SATA_TOP_MAX_PHYS; i++)
+		if (priv->port_mask & BIT(i))
+			brcm_sata_phy_disable(priv, i);
+}
+
+static u32 brcm_ahci_get_portmask(struct platform_device *pdev,
+				  struct brcm_ahci_priv *priv)
+{
+	void __iomem *ahci;
+	struct resource *res;
+	u32 impl;
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ahci");
+	ahci = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(ahci))
+		return 0;
+
+	impl = readl(ahci + HOST_PORTS_IMPL);
+
+	if (fls(impl) > SATA_TOP_MAX_PHYS)
+		dev_warn(priv->dev, "warning: more ports than PHYs (%#x)\n",
+			 impl);
+	else if (!impl)
+		dev_info(priv->dev, "no ports found\n");
+
+	devm_iounmap(&pdev->dev, ahci);
+	devm_release_mem_region(&pdev->dev, res->start, resource_size(res));
+
+	return impl;
+}
+
+static void brcm_sata_init(struct brcm_ahci_priv *priv)
+{
+	/* Configure endianness */
+	brcm_sata_writereg(BUS_CTRL_ENDIAN_CONF,
+			   priv->top_ctrl + SATA_TOP_CTRL_BUS_CTRL);
+}
+
+static int brcm_ahci_suspend(struct device *dev)
+{
+	struct ata_host *host = dev_get_drvdata(dev);
+	struct ahci_host_priv *hpriv = host->private_data;
+	struct brcm_ahci_priv *priv = hpriv->plat_data;
+	int ret;
+
+	ret = ahci_platform_suspend(dev);
+	brcm_sata_phys_disable(priv);
+	return ret;
+}
+
+static int brcm_ahci_resume(struct device *dev)
+{
+	struct ata_host *host = dev_get_drvdata(dev);
+	struct ahci_host_priv *hpriv = host->private_data;
+	struct brcm_ahci_priv *priv = hpriv->plat_data;
+
+	brcm_sata_init(priv);
+	brcm_sata_phys_enable(priv);
+	return ahci_platform_resume(dev);
+}
+
+static struct scsi_host_template ahci_platform_sht = {
+	AHCI_SHT(DRV_NAME),
+};
+
+static int brcm_ahci_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct brcm_ahci_priv *priv;
+	struct ahci_host_priv *hpriv;
+	struct resource *res;
+	int ret;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+	priv->dev = dev;
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "top-ctrl");
+	priv->top_ctrl = devm_ioremap_resource(dev, res);
+	if (IS_ERR(priv->top_ctrl))
+		return PTR_ERR(priv->top_ctrl);
+
+	brcm_sata_init(priv);
+
+	priv->port_mask = brcm_ahci_get_portmask(pdev, priv);
+	if (!priv->port_mask)
+		return -ENODEV;
+
+	brcm_sata_phys_enable(priv);
+
+	hpriv = ahci_platform_get_resources(pdev);
+	if (IS_ERR(hpriv))
+		return PTR_ERR(hpriv);
+	hpriv->plat_data = priv;
+
+	ret = ahci_platform_enable_resources(hpriv);
+	if (ret)
+		return ret;
+
+	ret = ahci_platform_init_host(pdev, hpriv, &ahci_brcm_port_info,
+				      &ahci_platform_sht);
+	if (ret)
+		return ret;
+
+	dev_info(dev, "Broadcom AHCI SATA3 registered\n");
+
+	return 0;
+}
+
+static int brcm_ahci_remove(struct platform_device *pdev)
+{
+	struct ata_host *host = dev_get_drvdata(&pdev->dev);
+	struct ahci_host_priv *hpriv = host->private_data;
+	struct brcm_ahci_priv *priv = hpriv->plat_data;
+	int ret;
+
+	ret = ata_platform_remove_one(pdev);
+	if (ret)
+		return ret;
+
+	brcm_sata_phys_disable(priv);
+
+	return 0;
+}
+
+static const struct of_device_id ahci_of_match[] = {
+	{.compatible = "brcm,bcm7445-ahci"},
+	{},
+};
+MODULE_DEVICE_TABLE(of, ahci_of_match);
+
+static SIMPLE_DEV_PM_OPS(ahci_brcm_pm_ops, brcm_ahci_suspend, brcm_ahci_resume);
+
+static struct platform_driver brcm_ahci_driver = {
+	.probe = brcm_ahci_probe,
+	.remove = brcm_ahci_remove,
+	.driver = {
+		.name = DRV_NAME,
+		.of_match_table = ahci_of_match,
+		.pm = &ahci_brcm_pm_ops,
+	},
+};
+module_platform_driver(brcm_ahci_driver);
+
+MODULE_DESCRIPTION("Broadcom SATA3 AHCI Controller Driver");
+MODULE_AUTHOR("Brian Norris");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:sata-brcmstb");
-- 
1.9.1

Re: [PATCH v3 3/5] ata: add Broadcom AHCI SATA3 driver for STB chips

[Tejun Heo]

On Tue, May 12, 2015 at 04:28:21PM -0700, Brian Norris wrote:
> Pretty straightforward driver, using the nice library-ization of the
> generic ahci_platform driver.
> 
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>

Applied to libata/for-4.2.

Hans, if you see any issues, please let me know.

Thanks.

-- 
tejun

Re: [PATCH v3 3/5] ata: add Broadcom AHCI SATA3 driver for STB chips

[Fabio Estevam]

On Tue, May 12, 2015 at 8:28 PM, Brian Norris
<computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> +       res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ahci");
> +       ahci = devm_ioremap_resource(&pdev->dev, res);
> +       if (IS_ERR(ahci))
> +               return 0;

You should propagate 'return PTR_ERR(ahci)' instead.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 3/5] ata: add Broadcom AHCI SATA3 driver for STB chips

[Tejun Heo]

On Thu, May 21, 2015 at 07:16:07PM -0300, Fabio Estevam wrote:
> On Tue, May 12, 2015 at 8:28 PM, Brian Norris
> <computersforpeace@gmail.com> wrote:
> 
> > +       res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ahci");
> > +       ahci = devm_ioremap_resource(&pdev->dev, res);
> > +       if (IS_ERR(ahci))
> > +               return 0;
> 
> You should propagate 'return PTR_ERR(ahci)' instead.

Brian, can you please update the patch?

Thanks.

-- 
tejun

Re: [PATCH v3 3/5] ata: add Broadcom AHCI SATA3 driver for STB chips

[Brian Norris]

On Thu, May 21, 2015 at 3:20 PM, Tejun Heo <tj@kernel.org> wrote:
> On Thu, May 21, 2015 at 07:16:07PM -0300, Fabio Estevam wrote:
>> On Tue, May 12, 2015 at 8:28 PM, Brian Norris
>> <computersforpeace@gmail.com> wrote:
>>
>> > +       res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ahci");
>> > +       ahci = devm_ioremap_resource(&pdev->dev, res);
>> > +       if (IS_ERR(ahci))
>> > +               return 0;
>>
>> You should propagate 'return PTR_ERR(ahci)' instead.
>
> Brian, can you please update the patch?

Note how the function brcm_ahci_get_portmask() is used. In brcm_ahci_probe():

        priv->port_mask = brcm_ahci_get_portmask(pdev, priv);
        if (!priv->port_mask)
                return -ENODEV;

I could try to change the return semantics if really needed. Or I
could add some comments on brcm_ahci_get_portmask() to note its return
semantics. Or I could just leave the driver as-is. Your call.

Brian

Re: [PATCH v3 3/5] ata: add Broadcom AHCI SATA3 driver for STB chips

[Tejun Heo]

On Thu, May 21, 2015 at 03:26:10PM -0700, Brian Norris wrote:
> On Thu, May 21, 2015 at 3:20 PM, Tejun Heo <tj@kernel.org> wrote:
> > On Thu, May 21, 2015 at 07:16:07PM -0300, Fabio Estevam wrote:
> >> On Tue, May 12, 2015 at 8:28 PM, Brian Norris
> >> <computersforpeace@gmail.com> wrote:
> >>
> >> > +       res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ahci");
> >> > +       ahci = devm_ioremap_resource(&pdev->dev, res);
> >> > +       if (IS_ERR(ahci))
> >> > +               return 0;
> >>
> >> You should propagate 'return PTR_ERR(ahci)' instead.
> >
> > Brian, can you please update the patch?
> 
> Note how the function brcm_ahci_get_portmask() is used. In brcm_ahci_probe():
> 
>         priv->port_mask = brcm_ahci_get_portmask(pdev, priv);
>         if (!priv->port_mask)
>                 return -ENODEV;
> 
> I could try to change the return semantics if really needed. Or I
> could add some comments on brcm_ahci_get_portmask() to note its return
> semantics. Or I could just leave the driver as-is. Your call.

Ah, you're right.  It's fine.  Leave it alone.

Thanks.

-- 
tejun

[PATCH v3 4/5] phy: add Broadcom SATA3 PHY driver for Broadcom STB SoCs

[Brian Norris]

Supports up to two ports which can each be powered on/off and configured
independently.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
v3: no change

v2: 
  - stop sharing SATA_TOP_CTRL registers with SATA driver
  - kill custom xlate function

 drivers/phy/Kconfig            |   9 ++
 drivers/phy/Makefile           |   1 +
 drivers/phy/phy-brcmstb-sata.c | 216 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 226 insertions(+)
 create mode 100644 drivers/phy/phy-brcmstb-sata.c

diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index a53bd5b52df9..36788b6f0220 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -309,4 +309,13 @@ config PHY_QCOM_UFS
 	help
 	  Support for UFS PHY on QCOM chipsets.
 
+config PHY_BRCMSTB_SATA
+	tristate "Broadcom STB SATA PHY driver"
+	depends on ARCH_BRCMSTB
+	depends on OF
+	select GENERIC_PHY
+	help
+	  Enable this to support the SATA3 PHY on 28nm Broadcom STB SoCs.
+	  Likely useful only with CONFIG_SATA_BRCMSTB enabled.
+
 endmenu
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index f12625178780..c61f3fdd191e 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -40,3 +40,4 @@ obj-$(CONFIG_PHY_STIH41X_USB)		+= phy-stih41x-usb.o
 obj-$(CONFIG_PHY_QCOM_UFS) 	+= phy-qcom-ufs.o
 obj-$(CONFIG_PHY_QCOM_UFS) 	+= phy-qcom-ufs-qmp-20nm.o
 obj-$(CONFIG_PHY_QCOM_UFS) 	+= phy-qcom-ufs-qmp-14nm.o
+obj-$(CONFIG_PHY_BRCMSTB_SATA)		+= phy-brcmstb-sata.o
diff --git a/drivers/phy/phy-brcmstb-sata.c b/drivers/phy/phy-brcmstb-sata.c
new file mode 100644
index 000000000000..8387c8cbea8c
--- /dev/null
+++ b/drivers/phy/phy-brcmstb-sata.c
@@ -0,0 +1,216 @@
+/*
+ * Broadcom SATA3 AHCI Controller PHY Driver
+ *
+ * Copyright © 2009-2015 Broadcom Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2, or (at your option)
+ * any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/device.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+
+#define SATA_MDIO_BANK_OFFSET				0x23c
+#define SATA_MDIO_REG_OFFSET(ofs)			((ofs) * 4)
+#define SATA_MDIO_REG_SPACE_SIZE			0x1000
+#define SATA_MDIO_REG_LENGTH				0x1f00
+
+#define MAX_PORTS					2
+
+/* Register offset between PHYs in PCB space */
+#define SATA_MDIO_REG_SPACE_SIZE			0x1000
+
+struct brcm_sata_port {
+	int portnum;
+	struct phy *phy;
+	struct brcm_sata_phy *phy_priv;
+	bool ssc_en;
+};
+
+struct brcm_sata_phy {
+	struct device *dev;
+	void __iomem *phy_base;
+
+	struct brcm_sata_port phys[MAX_PORTS];
+};
+
+enum sata_mdio_phy_regs_28nm {
+	PLL_REG_BANK_0				= 0x50,
+	PLL_REG_BANK_0_PLLCONTROL_0		= 0x81,
+
+	TXPMD_REG_BANK				= 0x1a0,
+	TXPMD_CONTROL1				= 0x81,
+	TXPMD_CONTROL1_TX_SSC_EN_FRC		= BIT(0),
+	TXPMD_CONTROL1_TX_SSC_EN_FRC_VAL	= BIT(1),
+	TXPMD_TX_FREQ_CTRL_CONTROL1		= 0x82,
+	TXPMD_TX_FREQ_CTRL_CONTROL2		= 0x83,
+	TXPMD_TX_FREQ_CTRL_CONTROL2_FMIN_MASK	= 0x3ff,
+	TXPMD_TX_FREQ_CTRL_CONTROL3		= 0x84,
+	TXPMD_TX_FREQ_CTRL_CONTROL3_FMAX_MASK	= 0x3ff,
+};
+
+static inline void __iomem *brcm_sata_phy_base(struct brcm_sata_port *port)
+{
+	struct brcm_sata_phy *priv = port->phy_priv;
+
+	return priv->phy_base + (port->portnum * SATA_MDIO_REG_SPACE_SIZE);
+}
+
+static void brcm_sata_mdio_wr(void __iomem *addr, u32 bank, u32 ofs,
+			      u32 msk, u32 value)
+{
+	u32 tmp;
+
+	writel(bank, addr + SATA_MDIO_BANK_OFFSET);
+	tmp = readl(addr + SATA_MDIO_REG_OFFSET(ofs));
+	tmp = (tmp & msk) | value;
+	writel(tmp, addr + SATA_MDIO_REG_OFFSET(ofs));
+}
+
+/* These defaults were characterized by H/W group */
+#define FMIN_VAL_DEFAULT	0x3df
+#define FMAX_VAL_DEFAULT	0x3df
+#define FMAX_VAL_SSC		0x83
+
+static void cfg_ssc_28nm(struct brcm_sata_port *port)
+{
+	void __iomem *base = brcm_sata_phy_base(port);
+	struct brcm_sata_phy *priv = port->phy_priv;
+	u32 tmp;
+
+	/* override the TX spread spectrum setting */
+	tmp = TXPMD_CONTROL1_TX_SSC_EN_FRC_VAL | TXPMD_CONTROL1_TX_SSC_EN_FRC;
+	brcm_sata_mdio_wr(base, TXPMD_REG_BANK, TXPMD_CONTROL1, ~tmp, tmp);
+
+	/* set fixed min freq */
+	brcm_sata_mdio_wr(base, TXPMD_REG_BANK, TXPMD_TX_FREQ_CTRL_CONTROL2,
+			  ~TXPMD_TX_FREQ_CTRL_CONTROL2_FMIN_MASK,
+			  FMIN_VAL_DEFAULT);
+
+	/* set fixed max freq depending on SSC config */
+	if (port->ssc_en) {
+		dev_info(priv->dev, "enabling SSC on port %d\n", port->portnum);
+		tmp = FMAX_VAL_SSC;
+	} else {
+		tmp = FMAX_VAL_DEFAULT;
+	}
+
+	brcm_sata_mdio_wr(base, TXPMD_REG_BANK, TXPMD_TX_FREQ_CTRL_CONTROL3,
+			  ~TXPMD_TX_FREQ_CTRL_CONTROL3_FMAX_MASK, tmp);
+}
+
+static int brcm_sata_phy_init(struct phy *phy)
+{
+	struct brcm_sata_port *port = phy_get_drvdata(phy);
+
+	cfg_ssc_28nm(port);
+
+	return 0;
+}
+
+static struct phy_ops phy_ops_28nm = {
+	.init		= brcm_sata_phy_init,
+	.owner		= THIS_MODULE,
+};
+
+static const struct of_device_id brcm_sata_phy_of_match[] = {
+	{ .compatible	= "brcm,bcm7445-sata-phy" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, brcm_sata_phy_of_match);
+
+static int brcm_sata_phy_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *dn = dev->of_node, *child;
+	struct brcm_sata_phy *priv;
+	struct resource *res;
+	struct phy_provider *provider;
+	int count = 0;
+
+	if (of_get_child_count(dn) == 0)
+		return -ENODEV;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+	dev_set_drvdata(dev, priv);
+	priv->dev = dev;
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "phy");
+	priv->phy_base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(priv->phy_base))
+		return PTR_ERR(priv->phy_base);
+
+	for_each_available_child_of_node(dn, child) {
+		unsigned int id;
+		struct brcm_sata_port *port;
+
+		if (of_property_read_u32(child, "reg", &id)) {
+			dev_err(dev, "missing reg property in node %s\n",
+					child->name);
+			return -EINVAL;
+		}
+
+		if (id >= MAX_PORTS) {
+			dev_err(dev, "invalid reg: %u\n", id);
+			return -EINVAL;
+		}
+		if (priv->phys[id].phy) {
+			dev_err(dev, "already registered port %u\n", id);
+			return -EINVAL;
+		}
+
+		port = &priv->phys[id];
+		port->portnum = id;
+		port->phy_priv = priv;
+		port->phy = devm_phy_create(dev, child, &phy_ops_28nm);
+		port->ssc_en = of_property_read_bool(child, "brcm,enable-ssc");
+		if (IS_ERR(port->phy)) {
+			dev_err(dev, "failed to create PHY\n");
+			return PTR_ERR(port->phy);
+		}
+
+		phy_set_drvdata(port->phy, port);
+		count++;
+	}
+
+	provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+	if (IS_ERR(provider)) {
+		dev_err(dev, "could not register PHY provider\n");
+		return PTR_ERR(provider);
+	}
+
+	dev_info(dev, "registered %d port(s)\n", count);
+
+	return 0;
+}
+
+static struct platform_driver brcm_sata_phy_driver = {
+	.probe	= brcm_sata_phy_probe,
+	.driver	= {
+		.of_match_table	= brcm_sata_phy_of_match,
+		.name		= "brcmstb-sata-phy",
+	}
+};
+module_platform_driver(brcm_sata_phy_driver);
+
+MODULE_DESCRIPTION("Broadcom STB SATA PHY driver");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Marc Carino");
+MODULE_AUTHOR("Brian Norris");
+MODULE_ALIAS("platform:phy-brcmstb-sata");
-- 
1.9.1

Re: [PATCH v3 4/5] phy: add Broadcom SATA3 PHY driver for Broadcom STB SoCs

[Kishon Vijay Abraham I]

Hi,

On Wednesday 13 May 2015 04:58 AM, Brian Norris wrote:
> Supports up to two ports which can each be powered on/off and configured
> independently.
>
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>

couple of minor comments below
> ---
> v3: no change
>
> v2:
>    - stop sharing SATA_TOP_CTRL registers with SATA driver
>    - kill custom xlate function
>
>   drivers/phy/Kconfig            |   9 ++
>   drivers/phy/Makefile           |   1 +
>   drivers/phy/phy-brcmstb-sata.c | 216 +++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 226 insertions(+)
>   create mode 100644 drivers/phy/phy-brcmstb-sata.c
>
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index a53bd5b52df9..36788b6f0220 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -309,4 +309,13 @@ config PHY_QCOM_UFS
>   	help
>   	  Support for UFS PHY on QCOM chipsets.
>
> +config PHY_BRCMSTB_SATA
> +	tristate "Broadcom STB SATA PHY driver"
> +	depends on ARCH_BRCMSTB
> +	depends on OF
> +	select GENERIC_PHY
> +	help
> +	  Enable this to support the SATA3 PHY on 28nm Broadcom STB SoCs.
> +	  Likely useful only with CONFIG_SATA_BRCMSTB enabled.
> +
>   endmenu
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index f12625178780..c61f3fdd191e 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -40,3 +40,4 @@ obj-$(CONFIG_PHY_STIH41X_USB)		+= phy-stih41x-usb.o
>   obj-$(CONFIG_PHY_QCOM_UFS) 	+= phy-qcom-ufs.o
>   obj-$(CONFIG_PHY_QCOM_UFS) 	+= phy-qcom-ufs-qmp-20nm.o
>   obj-$(CONFIG_PHY_QCOM_UFS) 	+= phy-qcom-ufs-qmp-14nm.o
> +obj-$(CONFIG_PHY_BRCMSTB_SATA)		+= phy-brcmstb-sata.o
> diff --git a/drivers/phy/phy-brcmstb-sata.c b/drivers/phy/phy-brcmstb-sata.c
> new file mode 100644
> index 000000000000..8387c8cbea8c
> --- /dev/null
> +++ b/drivers/phy/phy-brcmstb-sata.c
> @@ -0,0 +1,216 @@
> +/*
> + * Broadcom SATA3 AHCI Controller PHY Driver
> + *
> + * Copyright © 2009-2015 Broadcom Corporation
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2, or (at your option)
> + * any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +
> +#define SATA_MDIO_BANK_OFFSET				0x23c
> +#define SATA_MDIO_REG_OFFSET(ofs)			((ofs) * 4)
> +#define SATA_MDIO_REG_SPACE_SIZE			0x1000
> +#define SATA_MDIO_REG_LENGTH				0x1f00
> +
> +#define MAX_PORTS					2
> +
> +/* Register offset between PHYs in PCB space */
> +#define SATA_MDIO_REG_SPACE_SIZE			0x1000
> +
> +struct brcm_sata_port {
> +	int portnum;
> +	struct phy *phy;
> +	struct brcm_sata_phy *phy_priv;
> +	bool ssc_en;
> +};
> +
> +struct brcm_sata_phy {
> +	struct device *dev;
> +	void __iomem *phy_base;
> +
> +	struct brcm_sata_port phys[MAX_PORTS];
> +};
> +
> +enum sata_mdio_phy_regs_28nm {

Why should these defines be in enum?


> +	PLL_REG_BANK_0				= 0x50,
> +	PLL_REG_BANK_0_PLLCONTROL_0		= 0x81,
> +
> +	TXPMD_REG_BANK				= 0x1a0,
> +	TXPMD_CONTROL1				= 0x81,
> +	TXPMD_CONTROL1_TX_SSC_EN_FRC		= BIT(0),
> +	TXPMD_CONTROL1_TX_SSC_EN_FRC_VAL	= BIT(1),
> +	TXPMD_TX_FREQ_CTRL_CONTROL1		= 0x82,
> +	TXPMD_TX_FREQ_CTRL_CONTROL2		= 0x83,
> +	TXPMD_TX_FREQ_CTRL_CONTROL2_FMIN_MASK	= 0x3ff,
> +	TXPMD_TX_FREQ_CTRL_CONTROL3		= 0x84,
> +	TXPMD_TX_FREQ_CTRL_CONTROL3_FMAX_MASK	= 0x3ff,
> +};
> +
> +static inline void __iomem *brcm_sata_phy_base(struct brcm_sata_port *port)
> +{
> +	struct brcm_sata_phy *priv = port->phy_priv;
> +
> +	return priv->phy_base + (port->portnum * SATA_MDIO_REG_SPACE_SIZE);
> +}
> +
> +static void brcm_sata_mdio_wr(void __iomem *addr, u32 bank, u32 ofs,
> +			      u32 msk, u32 value)
> +{
> +	u32 tmp;
> +
> +	writel(bank, addr + SATA_MDIO_BANK_OFFSET);
> +	tmp = readl(addr + SATA_MDIO_REG_OFFSET(ofs));
> +	tmp = (tmp & msk) | value;
> +	writel(tmp, addr + SATA_MDIO_REG_OFFSET(ofs));
> +}
> +
> +/* These defaults were characterized by H/W group */
> +#define FMIN_VAL_DEFAULT	0x3df
> +#define FMAX_VAL_DEFAULT	0x3df
> +#define FMAX_VAL_SSC		0x83
> +
> +static void cfg_ssc_28nm(struct brcm_sata_port *port)

brcm_sata_cfg_ssc_28nm to make it similar to other functions.
> +{
> +	void __iomem *base = brcm_sata_phy_base(port);
> +	struct brcm_sata_phy *priv = port->phy_priv;
> +	u32 tmp;
> +
> +	/* override the TX spread spectrum setting */
> +	tmp = TXPMD_CONTROL1_TX_SSC_EN_FRC_VAL | TXPMD_CONTROL1_TX_SSC_EN_FRC;
> +	brcm_sata_mdio_wr(base, TXPMD_REG_BANK, TXPMD_CONTROL1, ~tmp, tmp);
> +
> +	/* set fixed min freq */
> +	brcm_sata_mdio_wr(base, TXPMD_REG_BANK, TXPMD_TX_FREQ_CTRL_CONTROL2,
> +			  ~TXPMD_TX_FREQ_CTRL_CONTROL2_FMIN_MASK,
> +			  FMIN_VAL_DEFAULT);
> +
> +	/* set fixed max freq depending on SSC config */
> +	if (port->ssc_en) {
> +		dev_info(priv->dev, "enabling SSC on port %d\n", port->portnum);
> +		tmp = FMAX_VAL_SSC;
> +	} else {
> +		tmp = FMAX_VAL_DEFAULT;
> +	}
> +
> +	brcm_sata_mdio_wr(base, TXPMD_REG_BANK, TXPMD_TX_FREQ_CTRL_CONTROL3,
> +			  ~TXPMD_TX_FREQ_CTRL_CONTROL3_FMAX_MASK, tmp);
> +}
> +
> +static int brcm_sata_phy_init(struct phy *phy)
> +{
> +	struct brcm_sata_port *port = phy_get_drvdata(phy);
> +
> +	cfg_ssc_28nm(port);
> +
> +	return 0;
> +}
> +
> +static struct phy_ops phy_ops_28nm = {
> +	.init		= brcm_sata_phy_init,
> +	.owner		= THIS_MODULE,
> +};
> +
> +static const struct of_device_id brcm_sata_phy_of_match[] = {
> +	{ .compatible	= "brcm,bcm7445-sata-phy" },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, brcm_sata_phy_of_match);
> +
> +static int brcm_sata_phy_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct device_node *dn = dev->of_node, *child;
> +	struct brcm_sata_phy *priv;
> +	struct resource *res;
> +	struct phy_provider *provider;
> +	int count = 0;
> +
> +	if (of_get_child_count(dn) == 0)
> +		return -ENODEV;
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +	dev_set_drvdata(dev, priv);
> +	priv->dev = dev;
> +
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "phy");
> +	priv->phy_base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(priv->phy_base))
> +		return PTR_ERR(priv->phy_base);
> +
> +	for_each_available_child_of_node(dn, child) {
> +		unsigned int id;
> +		struct brcm_sata_port *port;
> +
> +		if (of_property_read_u32(child, "reg", &id)) {
> +			dev_err(dev, "missing reg property in node %s\n",
> +					child->name);
> +			return -EINVAL;
> +		}
> +
> +		if (id >= MAX_PORTS) {
> +			dev_err(dev, "invalid reg: %u\n", id);
> +			return -EINVAL;
> +		}
> +		if (priv->phys[id].phy) {
> +			dev_err(dev, "already registered port %u\n", id);
> +			return -EINVAL;
> +		}
> +
> +		port = &priv->phys[id];
> +		port->portnum = id;
> +		port->phy_priv = priv;
> +		port->phy = devm_phy_create(dev, child, &phy_ops_28nm);
> +		port->ssc_en = of_property_read_bool(child, "brcm,enable-ssc");
> +		if (IS_ERR(port->phy)) {
> +			dev_err(dev, "failed to create PHY\n");
> +			return PTR_ERR(port->phy);
> +		}
> +
> +		phy_set_drvdata(port->phy, port);
> +		count++;
> +	}
> +
> +	provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> +	if (IS_ERR(provider)) {
> +		dev_err(dev, "could not register PHY provider\n");
> +		return PTR_ERR(provider);
> +	}
> +
> +	dev_info(dev, "registered %d port(s)\n", count);

lets not make the boot noisy. Change to dev_dbg?

Thanks
Kishon

Re: [PATCH v3 4/5] phy: add Broadcom SATA3 PHY driver for Broadcom STB SoCs

[Brian Norris]

On Wed, May 13, 2015 at 04:37:05PM +0530, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On Wednesday 13 May 2015 04:58 AM, Brian Norris wrote:
> >Supports up to two ports which can each be powered on/off and configured
> >independently.
> >
> >Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> 
> couple of minor comments below
> >---
> >v3: no change
> >
> >v2:
> >   - stop sharing SATA_TOP_CTRL registers with SATA driver
> >   - kill custom xlate function
> >
> >  drivers/phy/Kconfig            |   9 ++
> >  drivers/phy/Makefile           |   1 +
> >  drivers/phy/phy-brcmstb-sata.c | 216 +++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 226 insertions(+)
> >  create mode 100644 drivers/phy/phy-brcmstb-sata.c
> >
> >diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> >index a53bd5b52df9..36788b6f0220 100644
> >--- a/drivers/phy/Kconfig
> >+++ b/drivers/phy/Kconfig
> >@@ -309,4 +309,13 @@ config PHY_QCOM_UFS
> >  	help
> >  	  Support for UFS PHY on QCOM chipsets.
> >
> >+config PHY_BRCMSTB_SATA
> >+	tristate "Broadcom STB SATA PHY driver"
> >+	depends on ARCH_BRCMSTB
> >+	depends on OF
> >+	select GENERIC_PHY
> >+	help
> >+	  Enable this to support the SATA3 PHY on 28nm Broadcom STB SoCs.
> >+	  Likely useful only with CONFIG_SATA_BRCMSTB enabled.
> >+
> >  endmenu
> >diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> >index f12625178780..c61f3fdd191e 100644
> >--- a/drivers/phy/Makefile
> >+++ b/drivers/phy/Makefile
> >@@ -40,3 +40,4 @@ obj-$(CONFIG_PHY_STIH41X_USB)		+= phy-stih41x-usb.o
> >  obj-$(CONFIG_PHY_QCOM_UFS) 	+= phy-qcom-ufs.o
> >  obj-$(CONFIG_PHY_QCOM_UFS) 	+= phy-qcom-ufs-qmp-20nm.o
> >  obj-$(CONFIG_PHY_QCOM_UFS) 	+= phy-qcom-ufs-qmp-14nm.o
> >+obj-$(CONFIG_PHY_BRCMSTB_SATA)		+= phy-brcmstb-sata.o
> >diff --git a/drivers/phy/phy-brcmstb-sata.c b/drivers/phy/phy-brcmstb-sata.c
> >new file mode 100644
> >index 000000000000..8387c8cbea8c
> >--- /dev/null
> >+++ b/drivers/phy/phy-brcmstb-sata.c
> >@@ -0,0 +1,216 @@
> >+/*
> >+ * Broadcom SATA3 AHCI Controller PHY Driver
> >+ *
> >+ * Copyright © 2009-2015 Broadcom Corporation
> >+ *
> >+ * This program is free software; you can redistribute it and/or modify
> >+ * it under the terms of the GNU General Public License as published by
> >+ * the Free Software Foundation; either version 2, or (at your option)
> >+ * any later version.
> >+ *
> >+ * This program is distributed in the hope that it will be useful,
> >+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> >+ * GNU General Public License for more details.
> >+ */
> >+
> >+#include <linux/device.h>
> >+#include <linux/init.h>
> >+#include <linux/interrupt.h>
> >+#include <linux/io.h>
> >+#include <linux/kernel.h>
> >+#include <linux/module.h>
> >+#include <linux/of.h>
> >+#include <linux/phy/phy.h>
> >+#include <linux/platform_device.h>
> >+
> >+#define SATA_MDIO_BANK_OFFSET				0x23c
> >+#define SATA_MDIO_REG_OFFSET(ofs)			((ofs) * 4)
> >+#define SATA_MDIO_REG_SPACE_SIZE			0x1000
> >+#define SATA_MDIO_REG_LENGTH				0x1f00
> >+
> >+#define MAX_PORTS					2
> >+
> >+/* Register offset between PHYs in PCB space */
> >+#define SATA_MDIO_REG_SPACE_SIZE			0x1000
> >+
> >+struct brcm_sata_port {
> >+	int portnum;
> >+	struct phy *phy;
> >+	struct brcm_sata_phy *phy_priv;
> >+	bool ssc_en;
> >+};
> >+
> >+struct brcm_sata_phy {
> >+	struct device *dev;
> >+	void __iomem *phy_base;
> >+
> >+	struct brcm_sata_port phys[MAX_PORTS];
> >+};
> >+
> >+enum sata_mdio_phy_regs_28nm {
> 
> Why should these defines be in enum?

Why not? They're logically grouped this way, and IMO, they look nicer.
You can see drivers/ata/ahci.h for a similar example.

> >+	PLL_REG_BANK_0				= 0x50,
> >+	PLL_REG_BANK_0_PLLCONTROL_0		= 0x81,
> >+
> >+	TXPMD_REG_BANK				= 0x1a0,
> >+	TXPMD_CONTROL1				= 0x81,
> >+	TXPMD_CONTROL1_TX_SSC_EN_FRC		= BIT(0),
> >+	TXPMD_CONTROL1_TX_SSC_EN_FRC_VAL	= BIT(1),
> >+	TXPMD_TX_FREQ_CTRL_CONTROL1		= 0x82,
> >+	TXPMD_TX_FREQ_CTRL_CONTROL2		= 0x83,
> >+	TXPMD_TX_FREQ_CTRL_CONTROL2_FMIN_MASK	= 0x3ff,
> >+	TXPMD_TX_FREQ_CTRL_CONTROL3		= 0x84,
> >+	TXPMD_TX_FREQ_CTRL_CONTROL3_FMAX_MASK	= 0x3ff,
> >+};
> >+
> >+static inline void __iomem *brcm_sata_phy_base(struct brcm_sata_port *port)
> >+{
> >+	struct brcm_sata_phy *priv = port->phy_priv;
> >+
> >+	return priv->phy_base + (port->portnum * SATA_MDIO_REG_SPACE_SIZE);
> >+}
> >+
> >+static void brcm_sata_mdio_wr(void __iomem *addr, u32 bank, u32 ofs,
> >+			      u32 msk, u32 value)
> >+{
> >+	u32 tmp;
> >+
> >+	writel(bank, addr + SATA_MDIO_BANK_OFFSET);
> >+	tmp = readl(addr + SATA_MDIO_REG_OFFSET(ofs));
> >+	tmp = (tmp & msk) | value;
> >+	writel(tmp, addr + SATA_MDIO_REG_OFFSET(ofs));
> >+}
> >+
> >+/* These defaults were characterized by H/W group */
> >+#define FMIN_VAL_DEFAULT	0x3df
> >+#define FMAX_VAL_DEFAULT	0x3df
> >+#define FMAX_VAL_SSC		0x83
> >+
> >+static void cfg_ssc_28nm(struct brcm_sata_port *port)
> 
> brcm_sata_cfg_ssc_28nm to make it similar to other functions.

OK.

> >+{
> >+	void __iomem *base = brcm_sata_phy_base(port);
> >+	struct brcm_sata_phy *priv = port->phy_priv;
> >+	u32 tmp;
> >+
> >+	/* override the TX spread spectrum setting */
> >+	tmp = TXPMD_CONTROL1_TX_SSC_EN_FRC_VAL | TXPMD_CONTROL1_TX_SSC_EN_FRC;
> >+	brcm_sata_mdio_wr(base, TXPMD_REG_BANK, TXPMD_CONTROL1, ~tmp, tmp);
> >+
> >+	/* set fixed min freq */
> >+	brcm_sata_mdio_wr(base, TXPMD_REG_BANK, TXPMD_TX_FREQ_CTRL_CONTROL2,
> >+			  ~TXPMD_TX_FREQ_CTRL_CONTROL2_FMIN_MASK,
> >+			  FMIN_VAL_DEFAULT);
> >+
> >+	/* set fixed max freq depending on SSC config */
> >+	if (port->ssc_en) {
> >+		dev_info(priv->dev, "enabling SSC on port %d\n", port->portnum);
> >+		tmp = FMAX_VAL_SSC;
> >+	} else {
> >+		tmp = FMAX_VAL_DEFAULT;
> >+	}
> >+
> >+	brcm_sata_mdio_wr(base, TXPMD_REG_BANK, TXPMD_TX_FREQ_CTRL_CONTROL3,
> >+			  ~TXPMD_TX_FREQ_CTRL_CONTROL3_FMAX_MASK, tmp);
> >+}
> >+
> >+static int brcm_sata_phy_init(struct phy *phy)
> >+{
> >+	struct brcm_sata_port *port = phy_get_drvdata(phy);
> >+
> >+	cfg_ssc_28nm(port);
> >+
> >+	return 0;
> >+}
> >+
> >+static struct phy_ops phy_ops_28nm = {
> >+	.init		= brcm_sata_phy_init,
> >+	.owner		= THIS_MODULE,
> >+};
> >+
> >+static const struct of_device_id brcm_sata_phy_of_match[] = {
> >+	{ .compatible	= "brcm,bcm7445-sata-phy" },
> >+	{},
> >+};
> >+MODULE_DEVICE_TABLE(of, brcm_sata_phy_of_match);
> >+
> >+static int brcm_sata_phy_probe(struct platform_device *pdev)
> >+{
> >+	struct device *dev = &pdev->dev;
> >+	struct device_node *dn = dev->of_node, *child;
> >+	struct brcm_sata_phy *priv;
> >+	struct resource *res;
> >+	struct phy_provider *provider;
> >+	int count = 0;
> >+
> >+	if (of_get_child_count(dn) == 0)
> >+		return -ENODEV;
> >+
> >+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> >+	if (!priv)
> >+		return -ENOMEM;
> >+	dev_set_drvdata(dev, priv);
> >+	priv->dev = dev;
> >+
> >+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "phy");
> >+	priv->phy_base = devm_ioremap_resource(dev, res);
> >+	if (IS_ERR(priv->phy_base))
> >+		return PTR_ERR(priv->phy_base);
> >+
> >+	for_each_available_child_of_node(dn, child) {
> >+		unsigned int id;
> >+		struct brcm_sata_port *port;
> >+
> >+		if (of_property_read_u32(child, "reg", &id)) {
> >+			dev_err(dev, "missing reg property in node %s\n",
> >+					child->name);
> >+			return -EINVAL;
> >+		}
> >+
> >+		if (id >= MAX_PORTS) {
> >+			dev_err(dev, "invalid reg: %u\n", id);
> >+			return -EINVAL;
> >+		}
> >+		if (priv->phys[id].phy) {
> >+			dev_err(dev, "already registered port %u\n", id);
> >+			return -EINVAL;
> >+		}
> >+
> >+		port = &priv->phys[id];
> >+		port->portnum = id;
> >+		port->phy_priv = priv;
> >+		port->phy = devm_phy_create(dev, child, &phy_ops_28nm);
> >+		port->ssc_en = of_property_read_bool(child, "brcm,enable-ssc");
> >+		if (IS_ERR(port->phy)) {
> >+			dev_err(dev, "failed to create PHY\n");
> >+			return PTR_ERR(port->phy);
> >+		}
> >+
> >+		phy_set_drvdata(port->phy, port);
> >+		count++;
> >+	}
> >+
> >+	provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> >+	if (IS_ERR(provider)) {
> >+		dev_err(dev, "could not register PHY provider\n");
> >+		return PTR_ERR(provider);
> >+	}
> >+
> >+	dev_info(dev, "registered %d port(s)\n", count);
> 
> lets not make the boot noisy. Change to dev_dbg?

Why? What's the harm? And I thought we discussed this already. "Noisy"
depends on your point of view; IMO, this is important information. If
for some reason this driver wasn't loaded or probed, we'd want to know.
Besides, dev_dbg() requires you to fiddle with both the log level and
the dynamic debug boot parameters just to get these informational
messages.

Brian

Re: [PATCH v3 4/5] phy: add Broadcom SATA3 PHY driver for Broadcom STB SoCs

[Kishon Vijay Abraham I]

Hi,

On Thursday 14 May 2015 12:19 AM, Brian Norris wrote:
> On Wed, May 13, 2015 at 04:37:05PM +0530, Kishon Vijay Abraham I wrote:
>> Hi,
>>
>> On Wednesday 13 May 2015 04:58 AM, Brian Norris wrote:
>>> Supports up to two ports which can each be powered on/off and configured
>>> independently.
>>>
>>> Signed-off-by: Brian Norris <computersforpeace@gmail.com>
>>
>> couple of minor comments below
>>> ---
>>> v3: no change
>>>
>>> v2:
>>>    - stop sharing SATA_TOP_CTRL registers with SATA driver
>>>    - kill custom xlate function
>>>
>>>   drivers/phy/Kconfig            |   9 ++
>>>   drivers/phy/Makefile           |   1 +
>>>   drivers/phy/phy-brcmstb-sata.c | 216 +++++++++++++++++++++++++++++++++++++++++
>>>   3 files changed, 226 insertions(+)
>>>   create mode 100644 drivers/phy/phy-brcmstb-sata.c
>>>
>>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>>> index a53bd5b52df9..36788b6f0220 100644
>>> --- a/drivers/phy/Kconfig
>>> +++ b/drivers/phy/Kconfig
>>> @@ -309,4 +309,13 @@ config PHY_QCOM_UFS
>>>   	help
>>>   	  Support for UFS PHY on QCOM chipsets.
>>>
>>> +config PHY_BRCMSTB_SATA
>>> +	tristate "Broadcom STB SATA PHY driver"
>>> +	depends on ARCH_BRCMSTB
>>> +	depends on OF
>>> +	select GENERIC_PHY
>>> +	help
>>> +	  Enable this to support the SATA3 PHY on 28nm Broadcom STB SoCs.
>>> +	  Likely useful only with CONFIG_SATA_BRCMSTB enabled.
>>> +
>>>   endmenu
>>> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
>>> index f12625178780..c61f3fdd191e 100644
>>> --- a/drivers/phy/Makefile
>>> +++ b/drivers/phy/Makefile
>>> @@ -40,3 +40,4 @@ obj-$(CONFIG_PHY_STIH41X_USB)		+= phy-stih41x-usb.o
>>>   obj-$(CONFIG_PHY_QCOM_UFS) 	+= phy-qcom-ufs.o
>>>   obj-$(CONFIG_PHY_QCOM_UFS) 	+= phy-qcom-ufs-qmp-20nm.o
>>>   obj-$(CONFIG_PHY_QCOM_UFS) 	+= phy-qcom-ufs-qmp-14nm.o
>>> +obj-$(CONFIG_PHY_BRCMSTB_SATA)		+= phy-brcmstb-sata.o
>>> diff --git a/drivers/phy/phy-brcmstb-sata.c b/drivers/phy/phy-brcmstb-sata.c
>>> new file mode 100644
>>> index 000000000000..8387c8cbea8c
>>> --- /dev/null
>>> +++ b/drivers/phy/phy-brcmstb-sata.c
>>> @@ -0,0 +1,216 @@
>>> +/*
>>> + * Broadcom SATA3 AHCI Controller PHY Driver
>>> + *
>>> + * Copyright © 2009-2015 Broadcom Corporation
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License as published by
>>> + * the Free Software Foundation; either version 2, or (at your option)
>>> + * any later version.
>>> + *
>>> + * This program is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>> + * GNU General Public License for more details.
>>> + */
>>> +
>>> +#include <linux/device.h>
>>> +#include <linux/init.h>
>>> +#include <linux/interrupt.h>
>>> +#include <linux/io.h>
>>> +#include <linux/kernel.h>
>>> +#include <linux/module.h>
>>> +#include <linux/of.h>
>>> +#include <linux/phy/phy.h>
>>> +#include <linux/platform_device.h>
>>> +
>>> +#define SATA_MDIO_BANK_OFFSET				0x23c
>>> +#define SATA_MDIO_REG_OFFSET(ofs)			((ofs) * 4)
>>> +#define SATA_MDIO_REG_SPACE_SIZE			0x1000
>>> +#define SATA_MDIO_REG_LENGTH				0x1f00
>>> +
>>> +#define MAX_PORTS					2
>>> +
>>> +/* Register offset between PHYs in PCB space */
>>> +#define SATA_MDIO_REG_SPACE_SIZE			0x1000
>>> +
>>> +struct brcm_sata_port {
>>> +	int portnum;
>>> +	struct phy *phy;
>>> +	struct brcm_sata_phy *phy_priv;
>>> +	bool ssc_en;
>>> +};
>>> +
>>> +struct brcm_sata_phy {
>>> +	struct device *dev;
>>> +	void __iomem *phy_base;
>>> +
>>> +	struct brcm_sata_port phys[MAX_PORTS];
>>> +};
>>> +
>>> +enum sata_mdio_phy_regs_28nm {
>>
>> Why should these defines be in enum?
>
> Why not? They're logically grouped this way, and IMO, they look nicer.
> You can see drivers/ata/ahci.h for a similar example.

fair enough.
>
>>> +	PLL_REG_BANK_0				= 0x50,
>>> +	PLL_REG_BANK_0_PLLCONTROL_0		= 0x81,
>>> +
>>> +	TXPMD_REG_BANK				= 0x1a0,
>>> +	TXPMD_CONTROL1				= 0x81,
>>> +	TXPMD_CONTROL1_TX_SSC_EN_FRC		= BIT(0),
>>> +	TXPMD_CONTROL1_TX_SSC_EN_FRC_VAL	= BIT(1),
>>> +	TXPMD_TX_FREQ_CTRL_CONTROL1		= 0x82,
>>> +	TXPMD_TX_FREQ_CTRL_CONTROL2		= 0x83,
>>> +	TXPMD_TX_FREQ_CTRL_CONTROL2_FMIN_MASK	= 0x3ff,
>>> +	TXPMD_TX_FREQ_CTRL_CONTROL3		= 0x84,
>>> +	TXPMD_TX_FREQ_CTRL_CONTROL3_FMAX_MASK	= 0x3ff,
>>> +};
>>> +
>>> +static inline void __iomem *brcm_sata_phy_base(struct brcm_sata_port *port)
>>> +{
>>> +	struct brcm_sata_phy *priv = port->phy_priv;
>>> +
>>> +	return priv->phy_base + (port->portnum * SATA_MDIO_REG_SPACE_SIZE);
>>> +}
>>> +
>>> +static void brcm_sata_mdio_wr(void __iomem *addr, u32 bank, u32 ofs,
>>> +			      u32 msk, u32 value)
>>> +{
>>> +	u32 tmp;
>>> +
>>> +	writel(bank, addr + SATA_MDIO_BANK_OFFSET);
>>> +	tmp = readl(addr + SATA_MDIO_REG_OFFSET(ofs));
>>> +	tmp = (tmp & msk) | value;
>>> +	writel(tmp, addr + SATA_MDIO_REG_OFFSET(ofs));
>>> +}
>>> +
>>> +/* These defaults were characterized by H/W group */
>>> +#define FMIN_VAL_DEFAULT	0x3df
>>> +#define FMAX_VAL_DEFAULT	0x3df
>>> +#define FMAX_VAL_SSC		0x83
>>> +
>>> +static void cfg_ssc_28nm(struct brcm_sata_port *port)
>>
>> brcm_sata_cfg_ssc_28nm to make it similar to other functions.
>
> OK.
>
>>> +{
>>> +	void __iomem *base = brcm_sata_phy_base(port);
>>> +	struct brcm_sata_phy *priv = port->phy_priv;
>>> +	u32 tmp;
>>> +
>>> +	/* override the TX spread spectrum setting */
>>> +	tmp = TXPMD_CONTROL1_TX_SSC_EN_FRC_VAL | TXPMD_CONTROL1_TX_SSC_EN_FRC;
>>> +	brcm_sata_mdio_wr(base, TXPMD_REG_BANK, TXPMD_CONTROL1, ~tmp, tmp);
>>> +
>>> +	/* set fixed min freq */
>>> +	brcm_sata_mdio_wr(base, TXPMD_REG_BANK, TXPMD_TX_FREQ_CTRL_CONTROL2,
>>> +			  ~TXPMD_TX_FREQ_CTRL_CONTROL2_FMIN_MASK,
>>> +			  FMIN_VAL_DEFAULT);
>>> +
>>> +	/* set fixed max freq depending on SSC config */
>>> +	if (port->ssc_en) {
>>> +		dev_info(priv->dev, "enabling SSC on port %d\n", port->portnum);
>>> +		tmp = FMAX_VAL_SSC;
>>> +	} else {
>>> +		tmp = FMAX_VAL_DEFAULT;
>>> +	}
>>> +
>>> +	brcm_sata_mdio_wr(base, TXPMD_REG_BANK, TXPMD_TX_FREQ_CTRL_CONTROL3,
>>> +			  ~TXPMD_TX_FREQ_CTRL_CONTROL3_FMAX_MASK, tmp);
>>> +}
>>> +
>>> +static int brcm_sata_phy_init(struct phy *phy)
>>> +{
>>> +	struct brcm_sata_port *port = phy_get_drvdata(phy);
>>> +
>>> +	cfg_ssc_28nm(port);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static struct phy_ops phy_ops_28nm = {
>>> +	.init		= brcm_sata_phy_init,
>>> +	.owner		= THIS_MODULE,
>>> +};
>>> +
>>> +static const struct of_device_id brcm_sata_phy_of_match[] = {
>>> +	{ .compatible	= "brcm,bcm7445-sata-phy" },
>>> +	{},
>>> +};
>>> +MODULE_DEVICE_TABLE(of, brcm_sata_phy_of_match);
>>> +
>>> +static int brcm_sata_phy_probe(struct platform_device *pdev)
>>> +{
>>> +	struct device *dev = &pdev->dev;
>>> +	struct device_node *dn = dev->of_node, *child;
>>> +	struct brcm_sata_phy *priv;
>>> +	struct resource *res;
>>> +	struct phy_provider *provider;
>>> +	int count = 0;
>>> +
>>> +	if (of_get_child_count(dn) == 0)
>>> +		return -ENODEV;
>>> +
>>> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>>> +	if (!priv)
>>> +		return -ENOMEM;
>>> +	dev_set_drvdata(dev, priv);
>>> +	priv->dev = dev;
>>> +
>>> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "phy");
>>> +	priv->phy_base = devm_ioremap_resource(dev, res);
>>> +	if (IS_ERR(priv->phy_base))
>>> +		return PTR_ERR(priv->phy_base);
>>> +
>>> +	for_each_available_child_of_node(dn, child) {
>>> +		unsigned int id;
>>> +		struct brcm_sata_port *port;
>>> +
>>> +		if (of_property_read_u32(child, "reg", &id)) {
>>> +			dev_err(dev, "missing reg property in node %s\n",
>>> +					child->name);
>>> +			return -EINVAL;
>>> +		}
>>> +
>>> +		if (id >= MAX_PORTS) {
>>> +			dev_err(dev, "invalid reg: %u\n", id);
>>> +			return -EINVAL;
>>> +		}
>>> +		if (priv->phys[id].phy) {
>>> +			dev_err(dev, "already registered port %u\n", id);
>>> +			return -EINVAL;
>>> +		}
>>> +
>>> +		port = &priv->phys[id];
>>> +		port->portnum = id;
>>> +		port->phy_priv = priv;
>>> +		port->phy = devm_phy_create(dev, child, &phy_ops_28nm);
>>> +		port->ssc_en = of_property_read_bool(child, "brcm,enable-ssc");
>>> +		if (IS_ERR(port->phy)) {
>>> +			dev_err(dev, "failed to create PHY\n");
>>> +			return PTR_ERR(port->phy);
>>> +		}
>>> +
>>> +		phy_set_drvdata(port->phy, port);
>>> +		count++;
>>> +	}
>>> +
>>> +	provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
>>> +	if (IS_ERR(provider)) {
>>> +		dev_err(dev, "could not register PHY provider\n");
>>> +		return PTR_ERR(provider);
>>> +	}
>>> +
>>> +	dev_info(dev, "registered %d port(s)\n", count);
>>
>> lets not make the boot noisy. Change to dev_dbg?
>
> Why? What's the harm? And I thought we discussed this already. "Noisy"
> depends on your point of view; IMO, this is important information. If
> for some reason this driver wasn't loaded or probed, we'd want to know.
> Besides, dev_dbg() requires you to fiddle with both the log level and
> the dynamic debug boot parameters just to get these informational
> messages.

so be it. While sending the next version please also add a MAINTAINERs entry 
for this driver.

Cheers
Kishon

Re: [PATCH v3 4/5] phy: add Broadcom SATA3 PHY driver for Broadcom STB SoCs

[Brian Norris]

On Thu, May 14, 2015 at 11:22:55AM +0530, Kishon Vijay Abraham I wrote:
> While sending the next version please also add a
> MAINTAINERs entry for this driver.

I think this has it covered, right?

https://lkml.org/lkml/2015/3/18/932

Brian

Re: [PATCH v3 4/5] phy: add Broadcom SATA3 PHY driver for Broadcom STB SoCs

[Florian Fainelli]

On 14/05/15 10:39, Brian Norris wrote:
> On Thu, May 14, 2015 at 11:22:55AM +0530, Kishon Vijay Abraham I wrote:
>> While sending the next version please also add a
>> MAINTAINERs entry for this driver.
> 
> I think this has it covered, right?
> 
> https://lkml.org/lkml/2015/3/18/932

Kishon, this patch is on its way to arm-soc via a pull request:
http://www.spinics.net/lists/arm-kernel/msg418276.html

this should address your concern. Thanks!
-- 
Florian

Re: [PATCH v3 4/5] phy: add Broadcom SATA3 PHY driver for Broadcom STB SoCs

[Kishon Vijay Abraham I]

On Thursday 14 May 2015 11:12 PM, Florian Fainelli wrote:
> On 14/05/15 10:39, Brian Norris wrote:
>> On Thu, May 14, 2015 at 11:22:55AM +0530, Kishon Vijay Abraham I wrote:
>>> While sending the next version please also add a
>>> MAINTAINERs entry for this driver.
>>
>> I think this has it covered, right?
>>
>> https://lkml.org/lkml/2015/3/18/932
>
> Kishon, this patch is on its way to arm-soc via a pull request:
> http://www.spinics.net/lists/arm-kernel/msg418276.html
>
> this should address your concern. Thanks!

Indeed!

Thanks
Kishon

[PATCH v4] phy: add Broadcom SATA3 PHY driver for Broadcom STB SoCs

[Brian Norris]

Supports up to two ports which can each be powered on/off and configured
independently.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
Note this was part of a 5-patch series (this patch is 'In-Reply-To' version 3
of the series). Because there were no comments on the other patches, and this
driver only had a trivial modification, I'm sending just this single driver.
Please consider merging v3 of all the other patches.

v4: 
  - rename cfg_ssc_28nm() to brcm_sata_cfg_ssc_28nm()
  - MAINTAINERS entry is queued up in arm-soc (regex for brcmstb)

v3: no change

v2:
  - stop sharing SATA_TOP_CTRL registers with SATA driver
  - kill custom xlate function

 drivers/phy/Kconfig            |   9 ++
 drivers/phy/Makefile           |   1 +
 drivers/phy/phy-brcmstb-sata.c | 216 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 226 insertions(+)
 create mode 100644 drivers/phy/phy-brcmstb-sata.c

diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index a53bd5b52df9..36788b6f0220 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -309,4 +309,13 @@ config PHY_QCOM_UFS
 	help
 	  Support for UFS PHY on QCOM chipsets.
 
+config PHY_BRCMSTB_SATA
+	tristate "Broadcom STB SATA PHY driver"
+	depends on ARCH_BRCMSTB
+	depends on OF
+	select GENERIC_PHY
+	help
+	  Enable this to support the SATA3 PHY on 28nm Broadcom STB SoCs.
+	  Likely useful only with CONFIG_SATA_BRCMSTB enabled.
+
 endmenu
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index f12625178780..c61f3fdd191e 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -40,3 +40,4 @@ obj-$(CONFIG_PHY_STIH41X_USB)		+= phy-stih41x-usb.o
 obj-$(CONFIG_PHY_QCOM_UFS) 	+= phy-qcom-ufs.o
 obj-$(CONFIG_PHY_QCOM_UFS) 	+= phy-qcom-ufs-qmp-20nm.o
 obj-$(CONFIG_PHY_QCOM_UFS) 	+= phy-qcom-ufs-qmp-14nm.o
+obj-$(CONFIG_PHY_BRCMSTB_SATA)		+= phy-brcmstb-sata.o
diff --git a/drivers/phy/phy-brcmstb-sata.c b/drivers/phy/phy-brcmstb-sata.c
new file mode 100644
index 000000000000..b7e303d28caf
--- /dev/null
+++ b/drivers/phy/phy-brcmstb-sata.c
@@ -0,0 +1,216 @@
+/*
+ * Broadcom SATA3 AHCI Controller PHY Driver
+ *
+ * Copyright © 2009-2015 Broadcom Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2, or (at your option)
+ * any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/device.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+
+#define SATA_MDIO_BANK_OFFSET				0x23c
+#define SATA_MDIO_REG_OFFSET(ofs)			((ofs) * 4)
+#define SATA_MDIO_REG_SPACE_SIZE			0x1000
+#define SATA_MDIO_REG_LENGTH				0x1f00
+
+#define MAX_PORTS					2
+
+/* Register offset between PHYs in PCB space */
+#define SATA_MDIO_REG_SPACE_SIZE			0x1000
+
+struct brcm_sata_port {
+	int portnum;
+	struct phy *phy;
+	struct brcm_sata_phy *phy_priv;
+	bool ssc_en;
+};
+
+struct brcm_sata_phy {
+	struct device *dev;
+	void __iomem *phy_base;
+
+	struct brcm_sata_port phys[MAX_PORTS];
+};
+
+enum sata_mdio_phy_regs_28nm {
+	PLL_REG_BANK_0				= 0x50,
+	PLL_REG_BANK_0_PLLCONTROL_0		= 0x81,
+
+	TXPMD_REG_BANK				= 0x1a0,
+	TXPMD_CONTROL1				= 0x81,
+	TXPMD_CONTROL1_TX_SSC_EN_FRC		= BIT(0),
+	TXPMD_CONTROL1_TX_SSC_EN_FRC_VAL	= BIT(1),
+	TXPMD_TX_FREQ_CTRL_CONTROL1		= 0x82,
+	TXPMD_TX_FREQ_CTRL_CONTROL2		= 0x83,
+	TXPMD_TX_FREQ_CTRL_CONTROL2_FMIN_MASK	= 0x3ff,
+	TXPMD_TX_FREQ_CTRL_CONTROL3		= 0x84,
+	TXPMD_TX_FREQ_CTRL_CONTROL3_FMAX_MASK	= 0x3ff,
+};
+
+static inline void __iomem *brcm_sata_phy_base(struct brcm_sata_port *port)
+{
+	struct brcm_sata_phy *priv = port->phy_priv;
+
+	return priv->phy_base + (port->portnum * SATA_MDIO_REG_SPACE_SIZE);
+}
+
+static void brcm_sata_mdio_wr(void __iomem *addr, u32 bank, u32 ofs,
+			      u32 msk, u32 value)
+{
+	u32 tmp;
+
+	writel(bank, addr + SATA_MDIO_BANK_OFFSET);
+	tmp = readl(addr + SATA_MDIO_REG_OFFSET(ofs));
+	tmp = (tmp & msk) | value;
+	writel(tmp, addr + SATA_MDIO_REG_OFFSET(ofs));
+}
+
+/* These defaults were characterized by H/W group */
+#define FMIN_VAL_DEFAULT	0x3df
+#define FMAX_VAL_DEFAULT	0x3df
+#define FMAX_VAL_SSC		0x83
+
+static void brcm_sata_cfg_ssc_28nm(struct brcm_sata_port *port)
+{
+	void __iomem *base = brcm_sata_phy_base(port);
+	struct brcm_sata_phy *priv = port->phy_priv;
+	u32 tmp;
+
+	/* override the TX spread spectrum setting */
+	tmp = TXPMD_CONTROL1_TX_SSC_EN_FRC_VAL | TXPMD_CONTROL1_TX_SSC_EN_FRC;
+	brcm_sata_mdio_wr(base, TXPMD_REG_BANK, TXPMD_CONTROL1, ~tmp, tmp);
+
+	/* set fixed min freq */
+	brcm_sata_mdio_wr(base, TXPMD_REG_BANK, TXPMD_TX_FREQ_CTRL_CONTROL2,
+			  ~TXPMD_TX_FREQ_CTRL_CONTROL2_FMIN_MASK,
+			  FMIN_VAL_DEFAULT);
+
+	/* set fixed max freq depending on SSC config */
+	if (port->ssc_en) {
+		dev_info(priv->dev, "enabling SSC on port %d\n", port->portnum);
+		tmp = FMAX_VAL_SSC;
+	} else {
+		tmp = FMAX_VAL_DEFAULT;
+	}
+
+	brcm_sata_mdio_wr(base, TXPMD_REG_BANK, TXPMD_TX_FREQ_CTRL_CONTROL3,
+			  ~TXPMD_TX_FREQ_CTRL_CONTROL3_FMAX_MASK, tmp);
+}
+
+static int brcm_sata_phy_init(struct phy *phy)
+{
+	struct brcm_sata_port *port = phy_get_drvdata(phy);
+
+	brcm_sata_cfg_ssc_28nm(port);
+
+	return 0;
+}
+
+static struct phy_ops phy_ops_28nm = {
+	.init		= brcm_sata_phy_init,
+	.owner		= THIS_MODULE,
+};
+
+static const struct of_device_id brcm_sata_phy_of_match[] = {
+	{ .compatible	= "brcm,bcm7445-sata-phy" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, brcm_sata_phy_of_match);
+
+static int brcm_sata_phy_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *dn = dev->of_node, *child;
+	struct brcm_sata_phy *priv;
+	struct resource *res;
+	struct phy_provider *provider;
+	int count = 0;
+
+	if (of_get_child_count(dn) == 0)
+		return -ENODEV;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+	dev_set_drvdata(dev, priv);
+	priv->dev = dev;
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "phy");
+	priv->phy_base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(priv->phy_base))
+		return PTR_ERR(priv->phy_base);
+
+	for_each_available_child_of_node(dn, child) {
+		unsigned int id;
+		struct brcm_sata_port *port;
+
+		if (of_property_read_u32(child, "reg", &id)) {
+			dev_err(dev, "missing reg property in node %s\n",
+					child->name);
+			return -EINVAL;
+		}
+
+		if (id >= MAX_PORTS) {
+			dev_err(dev, "invalid reg: %u\n", id);
+			return -EINVAL;
+		}
+		if (priv->phys[id].phy) {
+			dev_err(dev, "already registered port %u\n", id);
+			return -EINVAL;
+		}
+
+		port = &priv->phys[id];
+		port->portnum = id;
+		port->phy_priv = priv;
+		port->phy = devm_phy_create(dev, child, &phy_ops_28nm);
+		port->ssc_en = of_property_read_bool(child, "brcm,enable-ssc");
+		if (IS_ERR(port->phy)) {
+			dev_err(dev, "failed to create PHY\n");
+			return PTR_ERR(port->phy);
+		}
+
+		phy_set_drvdata(port->phy, port);
+		count++;
+	}
+
+	provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+	if (IS_ERR(provider)) {
+		dev_err(dev, "could not register PHY provider\n");
+		return PTR_ERR(provider);
+	}
+
+	dev_info(dev, "registered %d port(s)\n", count);
+
+	return 0;
+}
+
+static struct platform_driver brcm_sata_phy_driver = {
+	.probe	= brcm_sata_phy_probe,
+	.driver	= {
+		.of_match_table	= brcm_sata_phy_of_match,
+		.name		= "brcmstb-sata-phy",
+	}
+};
+module_platform_driver(brcm_sata_phy_driver);
+
+MODULE_DESCRIPTION("Broadcom STB SATA PHY driver");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Marc Carino");
+MODULE_AUTHOR("Brian Norris");
+MODULE_ALIAS("platform:phy-brcmstb-sata");
-- 
1.9.1

[PATCH v3 5/5] ARM: dts: brcmstb: add nodes for SATA controller and PHY

[Brian Norris]

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
v3: no change

v2:
  - fix up some typos
  - account for binding changes in previous patches

 arch/arm/boot/dts/bcm7445.dtsi | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/arch/arm/boot/dts/bcm7445.dtsi b/arch/arm/boot/dts/bcm7445.dtsi
index 39ac7840d7ee..ce44a211b05c 100644
--- a/arch/arm/boot/dts/bcm7445.dtsi
+++ b/arch/arm/boot/dts/bcm7445.dtsi
@@ -108,6 +108,43 @@
 			brcm,int-map-mask = <0x25c>, <0x7000000>;
 			brcm,int-fwd-mask = <0x70000>;
 		};
+
+		sata@45a000 {
+			compatible = "brcm,bcm7445-ahci", "brcm,sata3-ahci";
+			reg-names = "ahci", "top-ctrl";
+			reg = <0x45a000 0xa9c>, <0x458040 0x24>;
+			interrupts = <GIC_SPI 30 0>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			sata0: sata-port@0 {
+				reg = <0>;
+				phys = <&sata_phy0>;
+			};
+
+			sata1: sata-port@1 {
+				reg = <1>;
+				phys = <&sata_phy1>;
+			};
+		};
+
+		sata_phy: sata-phy@458100 {
+			compatible = "brcm,bcm7445-sata-phy", "brcm,phy-sata3";
+			reg = <0x458100 0x1f00>;
+			reg-names = "phy";
+			#address-cells = <0x1>;
+			#size-cells = <0x0>;
+
+			sata_phy0: sata-phy@0 {
+				reg = <0>;
+				#phy-cells = <0>;
+			};
+
+			sata_phy1: sata-phy@1 {
+				reg = <1>;
+				#phy-cells = <0>;
+			};
+		};
 	};
 
 	smpboot {
-- 
1.9.1

Re: [PATCH v3 5/5] ARM: dts: brcmstb: add nodes for SATA controller and PHY

[Florian Fainelli]

On 12/05/15 16:28, Brian Norris wrote:
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>

Applied to devicetree/next, thanks!
-- 
Florian

Re: [PATCH v3 0/5] AHCI and SATA PHY support for Broadcom STB SoCs

[Kishon Vijay Abraham I]

Hi Tejun,

On Wednesday 13 May 2015 04:58 AM, Brian Norris wrote:
> Few changes since v2, and the changes are *only* to the SATA driver, not to any
> of the DT bindings or the PHY driver. I'm resending the whole series for
> completeness.
>
> I'll list the changelog in each patch, but in summary:
>
> v2 -> v3:
>   - straighten out endianness for big endian MIPS and ARM in AHCI driver
>   - rename sata_brcmstb.c to ahci_brcmstb.c
>   - bind ahci_brcmstb.c against the specific string, "brcm,bcm7445-ahci" instead
>     of the generic one
>
> Brian
>
> Brian Norris (5):
>    Documentation: devicetree: add Broadcom SATA binding
>    Documentation: devicetree: add Broadcom SATA PHY binding
>    ata: add Broadcom AHCI SATA3 driver for STB chips
>    phy: add Broadcom SATA3 PHY driver for Broadcom STB SoCs
>    ARM: dts: brcmstb: add nodes for SATA controller and PHY

I'll pick "[PATCH v3 2/5] Documentation: devicetree: add Broadcom SATA PHY 
binding" and
"[PATCH v4] phy: add Broadcom SATA3 PHY driver for Broadcom STB SoCs"
in the linux-phy tree.

Thanks
Kishon

Re: [PATCH v3 0/5] AHCI and SATA PHY support for Broadcom STB SoCs

[Tejun Heo]

Hello,

On Thu, May 21, 2015 at 06:52:51PM +0530, Kishon Vijay Abraham I wrote:
> I'll pick "[PATCH v3 2/5] Documentation: devicetree: add Broadcom SATA PHY
> binding" and
> "[PATCH v4] phy: add Broadcom SATA3 PHY driver for Broadcom STB SoCs"
> in the linux-phy tree.

Applying patch 5 to libata/for-4.2.  If more need to go through
libata, please let me know.

Thanks.

-- 
tejun

Re: [PATCH v3 0/5] AHCI and SATA PHY support for Broadcom STB SoCs

[Tejun Heo]

On Thu, May 21, 2015 at 06:00:50PM -0400, Tejun Heo wrote:
> Hello,
> 
> On Thu, May 21, 2015 at 06:52:51PM +0530, Kishon Vijay Abraham I wrote:
> > I'll pick "[PATCH v3 2/5] Documentation: devicetree: add Broadcom SATA PHY
> > binding" and
> > "[PATCH v4] phy: add Broadcom SATA3 PHY driver for Broadcom STB SoCs"
> > in the linux-phy tree.
> 
> Applying patch 5 to libata/for-4.2.  If more need to go through
> libata, please let me know.

lol not patch 5, the following one.  Sorry.

[PATCH v3 3/5] ata: add Broadcom AHCI SATA3 driver for STB chips

-- 
tejun

Re: [PATCH v3 0/5] AHCI and SATA PHY support for Broadcom STB SoCs

[Brian Norris]

On Thu, May 21, 2015 at 06:01:38PM -0400, Tejun Heo wrote:
> On Thu, May 21, 2015 at 06:00:50PM -0400, Tejun Heo wrote:
> > On Thu, May 21, 2015 at 06:52:51PM +0530, Kishon Vijay Abraham I wrote:
> > > I'll pick "[PATCH v3 2/5] Documentation: devicetree: add Broadcom SATA PHY
> > > binding" and

Thanks Kishon!

> > > "[PATCH v4] phy: add Broadcom SATA3 PHY driver for Broadcom STB SoCs"
> > > in the linux-phy tree.
> > 
> > Applying patch 5 to libata/for-4.2.  If more need to go through
> > libata, please let me know.
> 
> lol not patch 5, the following one.  Sorry.
> 
> [PATCH v3 3/5] ata: add Broadcom AHCI SATA3 driver for STB chips

Thanks. Also, this one?

[PATCH v3 1/5] Documentation: devicetree: add Broadcom SATA binding

Regards,
Brian

Re: [PATCH v3 0/5] AHCI and SATA PHY support for Broadcom STB SoCs

[Tejun Heo]

On Thu, May 21, 2015 at 03:03:28PM -0700, Brian Norris wrote:
> > [PATCH v3 3/5] ata: add Broadcom AHCI SATA3 driver for STB chips
> 
> Thanks. Also, this one?
> 
> [PATCH v3 1/5] Documentation: devicetree: add Broadcom SATA binding

devicetree patches usually haven't gone through libata tho.  Is this
an exception?

Thanks.

-- 
tejun
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 0/5] AHCI and SATA PHY support for Broadcom STB SoCs

[Brian Norris]

On Thu, May 21, 2015 at 06:04:30PM -0400, Tejun Heo wrote:
> On Thu, May 21, 2015 at 03:03:28PM -0700, Brian Norris wrote:
> > > [PATCH v3 3/5] ata: add Broadcom AHCI SATA3 driver for STB chips
> > 
> > Thanks. Also, this one?
> > 
> > [PATCH v3 1/5] Documentation: devicetree: add Broadcom SATA binding
> 
> devicetree patches usually haven't gone through libata tho.

Most subsystems take device tree binding patches for their constituent
drivers. And I see you've been merging others:

e35b98849f25 ata: sata_rcar: Add r8a7793 device support
8340bfeb03 ahci: st: Update the ahci_st DT documentation
af64dce4cb3a ahci: st: Provide DT bindings for ST's SATA implementation
a1a205df6ee2 ahci: add support for Hisilicon sata

All have your sign-off / commit stamp.

> Is this
> an exception?

Not that I know of. Where would you like it to go instead?

Brian

Re: [PATCH v3 0/5] AHCI and SATA PHY support for Broadcom STB SoCs

[Tejun Heo]

Hello,

On Thu, May 21, 2015 at 03:13:06PM -0700, Brian Norris wrote:
> Most subsystems take device tree binding patches for their constituent
> drivers. And I see you've been merging others:
> 
> e35b98849f25 ata: sata_rcar: Add r8a7793 device support
> 8340bfeb03 ahci: st: Update the ahci_st DT documentation
> af64dce4cb3a ahci: st: Provide DT bindings for ST's SATA implementation
> a1a205df6ee2 ahci: add support for Hisilicon sata
> 
> All have your sign-off / commit stamp.

Yeap, I've been routing some of them.

> > Is this
> > an exception?
> 
> Not that I know of. Where would you like it to go instead?

But the rules have never been clear to me.  If the subsystem
maintainer is okay with it, I'm happy to take the patches.  I'm just
kinda curious why this doesn't go through devicetree tree while some
other devicetree patches go through there.  Can somebody explain the
overall policy to me?  I'm not looking for some absolute rules and
exceptions are fine but I do wanna have a general direction.

Thanks.

-- 
tejun

Re: [PATCH v3 0/5] AHCI and SATA PHY support for Broadcom STB SoCs

[Brian Norris]

I can explain part of this, but I'm curious if anyone else has different
info.

On Thu, May 21, 2015 at 06:23:50PM -0400, Tejun Heo wrote:
> But the rules have never been clear to me.  If the subsystem
> maintainer is okay with it, I'm happy to take the patches.  I'm just
> kinda curious why this doesn't go through devicetree tree while some
> other devicetree patches go through there.

AFAIK, there is no official tree for device tree bindings. There's just
a mailing list and several reviewers, who usually try to help on the big
picture binding review. Note that there's no tree listed in MAINTAINERS
under:

  OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

But you will see several MAINTAINERS entries for different subdirs of
Documentation/devicetree/bindings/. Maybe you should add one for .../ata
if you're going to continue taking patches?

It's possible you're confusing binding documentation with .dts source
files? The DTS files (arch/*/boot/dts/) go through arch trees. For
instance, the arm-soc maintainers have a structured process by which
sub-architecture maintainers track .dts(i) file updates for their
boards/chips and filter them up to Arnd, Olof, etc., via their separate
'dts' branches. That's why Florian took patch 5 to his tree.

> Can somebody explain the
> overall policy to me?  I'm not looking for some absolute rules and
> exceptions are fine but I do wanna have a general direction.

Brian

Re: [PATCH v3 0/5] AHCI and SATA PHY support for Broadcom STB SoCs

[Tejun Heo]

Hello, Brian.

On Thu, May 21, 2015 at 03:38:25PM -0700, Brian Norris wrote:
> It's possible you're confusing binding documentation with .dts source
> files? The DTS files (arch/*/boot/dts/) go through arch trees. For
> instance, the arm-soc maintainers have a structured process by which
> sub-architecture maintainers track .dts(i) file updates for their
> boards/chips and filter them up to Arnd, Olof, etc., via their separate
> 'dts' branches. That's why Florian took patch 5 to his tree.

Yeah, I think that's why I'm constantly getting confused, so the
binding documentation and actual libata changes through libata and the
rest through arch trees.  Got it.

Thanks for the explanation.

-- 
tejun