* [PATCH v10 1/8] phy: add a driver for the Berlin SATA PHY
2014-07-18 12:29 [PATCH v10 0/8] ARM: berlin: add AHCI support Antoine Ténart
@ 2014-07-18 12:30 ` Antoine Ténart
2014-07-18 12:54 ` Varka Bhadram
2014-07-18 12:30 ` [PATCH v10 2/8] Documentation: bindings: add " Antoine Ténart
` (7 subsequent siblings)
8 siblings, 1 reply; 22+ messages in thread
From: Antoine Ténart @ 2014-07-18 12:30 UTC (permalink / raw)
To: linux-arm-kernel
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 <antoine.tenart@free-electrons.com>
---
drivers/phy/Kconfig | 7 ++
drivers/phy/Makefile | 1 +
drivers/phy/phy-berlin-sata.c | 284 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 292 insertions(+)
create mode 100644 drivers/phy/phy-berlin-sata.c
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 64b98d242ea6..458b15ae2e81 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 && HAS_IOMEM && 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..c5e688b0899f
--- /dev/null
+++ b/drivers/phy/phy-berlin-sata.c
@@ -0,0 +1,284 @@
+/*
+ * Marvell Berlin SATA PHY driver
+ *
+ * Copyright (C) 2014 Marvell Technology Group Ltd.
+ *
+ * Antoine T?nart <antoine.tenart@free-electrons.com>
+ *
+ * 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/clk.h>
+#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_SCR_CTL 0x2c
+#define PORT_VSR_ADDR 0x78
+#define PORT_VSR_DATA 0x7c
+
+#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)
+
+struct phy_berlin_desc {
+ struct phy *phy;
+ u32 power_bit;
+ unsigned index;
+};
+
+struct phy_berlin_priv {
+ void __iomem *base;
+ spinlock_t lock;
+ struct clk *clk;
+ struct phy_berlin_desc **phys;
+ unsigned nphys;
+};
+
+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 = dev_get_drvdata(phy->dev.parent);
+ void __iomem *ctrl_reg = priv->base + 0x60 + (desc->index * 0x80);
+ int ret = 0;
+ u32 regval;
+
+ clk_prepare_enable(priv->clk);
+
+ spin_lock(&priv->lock);
+
+ /* Power on PHY */
+ writel(CONTROL_REGISTER, priv->base + HOST_VSA_ADDR);
+ regval = readl(priv->base + HOST_VSA_DATA);
+ regval &= ~desc->power_bit;
+ 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 Gen3 controller speed */
+ regval = readl(ctrl_reg + PORT_SCR_CTL);
+ regval &= ~GENMASK(7, 4);
+ regval |= 0x30;
+ writel(regval, ctrl_reg + PORT_SCR_CTL);
+
+ spin_unlock(&priv->lock);
+
+ clk_disable_unprepare(priv->clk);
+
+ 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 = dev_get_drvdata(phy->dev.parent);
+ u32 regval;
+
+ clk_prepare_enable(priv->clk);
+
+ spin_lock(&priv->lock);
+
+ /* Power down PHY */
+ writel(CONTROL_REGISTER, priv->base + HOST_VSA_ADDR);
+ regval = readl(priv->base + HOST_VSA_DATA);
+ regval |= desc->power_bit;
+ writel(regval, priv->base + HOST_VSA_DATA);
+
+ spin_unlock(&priv->lock);
+
+ clk_disable_unprepare(priv->clk);
+
+ 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);
+ int i;
+
+ if (WARN_ON(args->args[0] >= priv->nphys))
+ return ERR_PTR(-ENODEV);
+
+ for (i = 0; i < priv->nphys; i++) {
+ if (priv->phys[i]->index == args->args[0])
+ break;
+ }
+
+ if (i == priv->nphys)
+ return ERR_PTR(-ENODEV);
+
+ return priv->phys[i]->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 device_node *child;
+ struct phy *phy;
+ struct phy_provider *phy_provider;
+ struct phy_berlin_priv *priv;
+ struct resource *res;
+ int i = 0;
+ u32 phy_id;
+
+ 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;
+
+ priv->clk = devm_clk_get(dev, NULL);
+ if (IS_ERR(priv->clk))
+ return PTR_ERR(priv->clk);
+
+ priv->nphys = of_get_child_count(dev->of_node);
+ if (priv->nphys == 0)
+ return -ENODEV;
+
+ priv->phys = devm_kzalloc(dev, priv->nphys * sizeof(*priv->phys),
+ GFP_KERNEL);
+ if (!priv->phys)
+ return -ENOMEM;
+
+ dev_set_drvdata(dev, priv);
+ spin_lock_init(&priv->lock);
+
+ for_each_available_child_of_node(dev->of_node, child) {
+ struct phy_berlin_desc *phy_desc;
+
+ if (of_property_read_u32(child, "reg", &phy_id)) {
+ dev_err(dev, "missing reg property in node %s\n",
+ child->name);
+ return -EINVAL;
+ }
+
+ if (phy_id >= ARRAY_SIZE(phy_berlin_power_down_bits)) {
+ dev_err(dev, "invalid reg in node %s\n", child->name);
+ return -EINVAL;
+ }
+
+ phy_desc = devm_kzalloc(dev, sizeof(*phy_desc), GFP_KERNEL);
+ if (!phy_desc)
+ return -ENOMEM;
+
+ phy = devm_phy_create(dev, &phy_berlin_sata_ops, NULL);
+ if (IS_ERR(phy)) {
+ dev_err(dev, "failed to create PHY %d\n", phy_id);
+ return PTR_ERR(phy);
+ }
+
+ phy_desc->phy = phy;
+ phy_desc->power_bit = phy_berlin_power_down_bits[phy_id];
+ phy_desc->index = phy_id;
+ phy_set_drvdata(phy, phy_desc);
+
+ priv->phys[i++] = phy_desc;
+
+ /* 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,berlin2q-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 <antoine.tenart@free-electrons.com>");
+MODULE_LICENSE("GPL v2");
--
1.9.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v10 1/8] phy: add a driver for the Berlin SATA PHY
2014-07-18 12:30 ` [PATCH v10 1/8] phy: add a driver for the Berlin SATA PHY Antoine Ténart
@ 2014-07-18 12:54 ` Varka Bhadram
0 siblings, 0 replies; 22+ messages in thread
From: Varka Bhadram @ 2014-07-18 12:54 UTC (permalink / raw)
To: linux-arm-kernel
On 07/18/2014 06:00 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.
>
(...)
> +
> +static const struct of_device_id phy_berlin_sata_of_match[] = {
> + { .compatible = "marvell,berlin2q-sata-phy" },
> + { },
> +};
> +
> +static struct platform_driver phy_berlin_sata_driver = {
> + .probe = phy_berlin_sata_probe,
> + .driver = {
> + .name = "phy-berlin-sata",
> + .owner = THIS_MODULE,
owner filed automatically update with module_platform_driver().
see:http://lxr.free-electrons.com/source/include/linux/platform_device.h#L190
--
Regards,
Varka Bhadram.
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v10 2/8] Documentation: bindings: add the Berlin SATA PHY
2014-07-18 12:29 [PATCH v10 0/8] ARM: berlin: add AHCI support Antoine Ténart
2014-07-18 12:30 ` [PATCH v10 1/8] phy: add a driver for the Berlin SATA PHY Antoine Ténart
@ 2014-07-18 12:30 ` Antoine Ténart
2014-07-18 17:27 ` Sergei Shtylyov
2014-07-18 12:30 ` [PATCH v10 3/8] ata: libahci_platform: move port_map parameters into the AHCI structure Antoine Ténart
` (6 subsequent siblings)
8 siblings, 1 reply; 22+ messages in thread
From: Antoine Ténart @ 2014-07-18 12:30 UTC (permalink / raw)
To: linux-arm-kernel
The Berlin SATA PHY drives the PHY related to the SATA interface. Add
the corresponding documentation.
Signed-off-by: Antoine T?nart <antoine.tenart@free-electrons.com>
---
.../devicetree/bindings/phy/berlin-sata-phy.txt | 34 ++++++++++++++++++++++
1 file changed, 34 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..88f8c23384c0
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/berlin-sata-phy.txt
@@ -0,0 +1,34 @@
+Berlin SATA PHY
+---------------
+
+Required properties:
+- compatible: should be "marvell,berlin2q-sata-phy"
+- address-cells: should be 1
+- size-cells: should be 0
+- phy-cells: from the generic PHY bindings, must be 1
+- reg: address and length of the register
+- clocks: reference to the clock entry
+
+Sub-nodes:
+Each PHY should be represented as a sub-node.
+
+Sub-nodes required properties:
+- reg: the PHY number
+
+Example:
+ sata_phy: phy at f7e900a0 {
+ compatible = "marvell,berlin2q-sata-phy";
+ reg = <0xf7e900a0 0x200>;
+ clocks = <&chip CLKID_SATA>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ #phy-cells = <1>;
+
+ sata-phy at 0 {
+ reg = <0>;
+ };
+
+ sata-phy at 1 {
+ reg = <1>;
+ };
+ };
--
1.9.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v10 2/8] Documentation: bindings: add the Berlin SATA PHY
2014-07-18 12:30 ` [PATCH v10 2/8] Documentation: bindings: add " Antoine Ténart
@ 2014-07-18 17:27 ` Sergei Shtylyov
2014-07-21 9:04 ` Antoine Ténart
0 siblings, 1 reply; 22+ messages in thread
From: Sergei Shtylyov @ 2014-07-18 17:27 UTC (permalink / raw)
To: linux-arm-kernel
Hello.
On 07/18/2014 04:30 PM, Antoine T?nart wrote:
> The Berlin SATA PHY drives the PHY related to the SATA interface. Add
> the corresponding documentation.
> Signed-off-by: Antoine T?nart <antoine.tenart@free-electrons.com>
> ---
> .../devicetree/bindings/phy/berlin-sata-phy.txt | 34 ++++++++++++++++++++++
> 1 file changed, 34 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..88f8c23384c0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/berlin-sata-phy.txt
> @@ -0,0 +1,34 @@
> +Berlin SATA PHY
> +---------------
> +
> +Required properties:
> +- compatible: should be "marvell,berlin2q-sata-phy"
> +- address-cells: should be 1
> +- size-cells: should be 0
> +- phy-cells: from the generic PHY bindings, must be 1
It's "#address-cells", "#size-cells", and "#phy-cells".
> +- reg: address and length of the register
> +- clocks: reference to the clock entry
> +
> +Sub-nodes:
> +Each PHY should be represented as a sub-node.
Then "#phy-cells" should also belong to the sub-nodes.
> +
> +Sub-nodes required properties:
> +- reg: the PHY number
WBR, Sergei
^ permalink raw reply [flat|nested] 22+ messages in thread* [PATCH v10 2/8] Documentation: bindings: add the Berlin SATA PHY
2014-07-18 17:27 ` Sergei Shtylyov
@ 2014-07-21 9:04 ` Antoine Ténart
0 siblings, 0 replies; 22+ messages in thread
From: Antoine Ténart @ 2014-07-21 9:04 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On Fri, Jul 18, 2014 at 09:27:28PM +0400, Sergei Shtylyov wrote:
> On 07/18/2014 04:30 PM, Antoine T?nart wrote:
>
> >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..88f8c23384c0
> >--- /dev/null
> >+++ b/Documentation/devicetree/bindings/phy/berlin-sata-phy.txt
> >@@ -0,0 +1,34 @@
> >+Berlin SATA PHY
> >+---------------
> >+
> >+Required properties:
> >+- compatible: should be "marvell,berlin2q-sata-phy"
> >+- address-cells: should be 1
> >+- size-cells: should be 0
> >+- phy-cells: from the generic PHY bindings, must be 1
>
> It's "#address-cells", "#size-cells", and "#phy-cells".
Sure.
>
> >+- reg: address and length of the register
> >+- clocks: reference to the clock entry
> >+
> >+Sub-nodes:
> >+Each PHY should be represented as a sub-node.
>
> Then "#phy-cells" should also belong to the sub-nodes.
No, because the PHY provider is still the parent.
Antoine
--
Antoine T?nart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v10 3/8] ata: libahci_platform: move port_map parameters into the AHCI structure
2014-07-18 12:29 [PATCH v10 0/8] ARM: berlin: add AHCI support Antoine Ténart
2014-07-18 12:30 ` [PATCH v10 1/8] phy: add a driver for the Berlin SATA PHY Antoine Ténart
2014-07-18 12:30 ` [PATCH v10 2/8] Documentation: bindings: add " Antoine Ténart
@ 2014-07-18 12:30 ` Antoine Ténart
2014-07-18 13:47 ` Tejun Heo
2014-07-18 12:30 ` [PATCH v10 4/8] ata: libahci: allow to use multiple PHYs Antoine Ténart
` (5 subsequent siblings)
8 siblings, 1 reply; 22+ messages in thread
From: Antoine Ténart @ 2014-07-18 12:30 UTC (permalink / raw)
To: linux-arm-kernel
This patch moves force_port_map and mask_port_map into the
ahci_host_priv structure. This allows to modify them into the AHCI
framework. This is needed by the new dt bindings representing ports as
the port_map mask is computed automatically.
Signed-off-by: Antoine T?nart <antoine.tenart@free-electrons.com>
---
drivers/ata/ahci.h | 6 +++---
drivers/ata/ahci_da850.c | 3 +--
drivers/ata/ahci_imx.c | 3 +--
drivers/ata/ahci_mvebu.c | 3 +--
drivers/ata/ahci_platform.c | 3 +--
drivers/ata/ahci_st.c | 2 +-
drivers/ata/ahci_sunxi.c | 2 +-
drivers/ata/ahci_xgene.c | 2 +-
drivers/ata/libahci.c | 17 +++++++----------
drivers/ata/libahci_platform.c | 8 ++------
include/linux/ahci_platform.h | 4 +---
11 files changed, 20 insertions(+), 33 deletions(-)
diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
index 5513296e5e2e..cb8d58926851 100644
--- a/drivers/ata/ahci.h
+++ b/drivers/ata/ahci.h
@@ -321,6 +321,8 @@ struct ahci_host_priv {
u32 cap; /* cap to use */
u32 cap2; /* cap2 to use */
u32 port_map; /* port map to use */
+ u32 force_port_map; /* force port map */
+ u32 mask_port_map; /* mask out particular bits */
u32 saved_cap; /* saved initial cap */
u32 saved_cap2; /* saved initial cap2 */
u32 saved_port_map; /* saved initial port_map */
@@ -361,9 +363,7 @@ unsigned int ahci_dev_classify(struct ata_port *ap);
void ahci_fill_cmd_slot(struct ahci_port_priv *pp, unsigned int tag,
u32 opts);
void ahci_save_initial_config(struct device *dev,
- struct ahci_host_priv *hpriv,
- unsigned int force_port_map,
- unsigned int mask_port_map);
+ struct ahci_host_priv *hpriv);
void ahci_init_controller(struct ata_host *host);
int ahci_reset_controller(struct ata_host *host);
diff --git a/drivers/ata/ahci_da850.c b/drivers/ata/ahci_da850.c
index 2b77d53bccf8..33b862b2c5db 100644
--- a/drivers/ata/ahci_da850.c
+++ b/drivers/ata/ahci_da850.c
@@ -85,8 +85,7 @@ static int ahci_da850_probe(struct platform_device *pdev)
da850_sata_init(dev, pwrdn_reg, hpriv->mmio);
- rc = ahci_platform_init_host(pdev, hpriv, &ahci_da850_port_info,
- 0, 0, 0);
+ rc = ahci_platform_init_host(pdev, hpriv, &ahci_da850_port_info, 0);
if (rc)
goto disable_resources;
diff --git a/drivers/ata/ahci_imx.c b/drivers/ata/ahci_imx.c
index cac4360f272a..c8eef2d745d8 100644
--- a/drivers/ata/ahci_imx.c
+++ b/drivers/ata/ahci_imx.c
@@ -454,8 +454,7 @@ static int imx_ahci_probe(struct platform_device *pdev)
reg_val = clk_get_rate(imxpriv->ahb_clk) / 1000;
writel(reg_val, hpriv->mmio + IMX_TIMER1MS);
- ret = ahci_platform_init_host(pdev, hpriv, &ahci_imx_port_info,
- 0, 0, 0);
+ ret = ahci_platform_init_host(pdev, hpriv, &ahci_imx_port_info, 0);
if (ret)
goto disable_sata;
diff --git a/drivers/ata/ahci_mvebu.c b/drivers/ata/ahci_mvebu.c
index fd3dfd733b84..0ba0cf7a9ba9 100644
--- a/drivers/ata/ahci_mvebu.c
+++ b/drivers/ata/ahci_mvebu.c
@@ -88,8 +88,7 @@ static int ahci_mvebu_probe(struct platform_device *pdev)
ahci_mvebu_mbus_config(hpriv, dram);
ahci_mvebu_regret_option(hpriv);
- rc = ahci_platform_init_host(pdev, hpriv, &ahci_mvebu_port_info,
- 0, 0, 0);
+ rc = ahci_platform_init_host(pdev, hpriv, &ahci_mvebu_port_info, 0);
if (rc)
goto disable_resources;
diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
index b10d81ddb528..95f9ca82082a 100644
--- a/drivers/ata/ahci_platform.c
+++ b/drivers/ata/ahci_platform.c
@@ -60,8 +60,7 @@ static int ahci_probe(struct platform_device *pdev)
if (of_device_is_compatible(dev->of_node, "hisilicon,hisi-ahci"))
hflags |= AHCI_HFLAG_NO_FBS | AHCI_HFLAG_NO_NCQ;
- rc = ahci_platform_init_host(pdev, hpriv, &ahci_port_info,
- hflags, 0, 0);
+ rc = ahci_platform_init_host(pdev, hpriv, &ahci_port_info, hflags);
if (rc)
goto pdata_exit;
diff --git a/drivers/ata/ahci_st.c b/drivers/ata/ahci_st.c
index 2595598df9ce..6767c7790b4a 100644
--- a/drivers/ata/ahci_st.c
+++ b/drivers/ata/ahci_st.c
@@ -166,7 +166,7 @@ static int st_ahci_probe(struct platform_device *pdev)
if (err)
return err;
- err = ahci_platform_init_host(pdev, hpriv, &st_ahci_port_info, 0, 0, 0);
+ err = ahci_platform_init_host(pdev, hpriv, &st_ahci_port_info, 0);
if (err) {
ahci_platform_disable_resources(hpriv);
return err;
diff --git a/drivers/ata/ahci_sunxi.c b/drivers/ata/ahci_sunxi.c
index 02002f125bd4..9f8ff1f9e912 100644
--- a/drivers/ata/ahci_sunxi.c
+++ b/drivers/ata/ahci_sunxi.c
@@ -188,7 +188,7 @@ static int ahci_sunxi_probe(struct platform_device *pdev)
AHCI_HFLAG_NO_PMP | AHCI_HFLAG_YES_NCQ;
rc = ahci_platform_init_host(pdev, hpriv, &ahci_sunxi_port_info,
- hflags, 0, 0);
+ hflags);
if (rc)
goto disable_resources;
diff --git a/drivers/ata/ahci_xgene.c b/drivers/ata/ahci_xgene.c
index ee3a3659bd9e..87d39fbe616b 100644
--- a/drivers/ata/ahci_xgene.c
+++ b/drivers/ata/ahci_xgene.c
@@ -487,7 +487,7 @@ static int xgene_ahci_probe(struct platform_device *pdev)
hflags = AHCI_HFLAG_NO_PMP | AHCI_HFLAG_YES_NCQ;
rc = ahci_platform_init_host(pdev, hpriv, &xgene_ahci_port_info,
- hflags, 0, 0);
+ hflags);
if (rc)
goto disable_resources;
diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index d72ce0470309..3c0b623747bd 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -398,10 +398,7 @@ static ssize_t ahci_show_em_supported(struct device *dev,
* LOCKING:
* None.
*/
-void ahci_save_initial_config(struct device *dev,
- struct ahci_host_priv *hpriv,
- unsigned int force_port_map,
- unsigned int mask_port_map)
+void ahci_save_initial_config(struct device *dev, struct ahci_host_priv *hpriv)
{
void __iomem *mmio = hpriv->mmio;
u32 cap, cap2, vers, port_map;
@@ -468,17 +465,17 @@ void ahci_save_initial_config(struct device *dev,
cap &= ~HOST_CAP_FBS;
}
- if (force_port_map && port_map != force_port_map) {
+ if (hpriv->force_port_map && port_map != hpriv->force_port_map) {
dev_info(dev, "forcing port_map 0x%x -> 0x%x\n",
- port_map, force_port_map);
- port_map = force_port_map;
+ port_map, hpriv->force_port_map);
+ port_map = hpriv->force_port_map;
}
- if (mask_port_map) {
+ if (hpriv->mask_port_map) {
dev_warn(dev, "masking port_map 0x%x -> 0x%x\n",
port_map,
- port_map & mask_port_map);
- port_map &= mask_port_map;
+ port_map & hpriv->mask_port_map);
+ port_map &= hpriv->mask_port_map;
}
/* cross check port_map and cap.n_ports */
diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c
index b0077589f065..db9b90d876dd 100644
--- a/drivers/ata/libahci_platform.c
+++ b/drivers/ata/libahci_platform.c
@@ -289,8 +289,6 @@ EXPORT_SYMBOL_GPL(ahci_platform_get_resources);
* @hpriv: ahci-host private data for the host
* @pi_template: template for the ata_port_info to use
* @host_flags: ahci host flags used in ahci_host_priv
- * @force_port_map: param passed to ahci_save_initial_config
- * @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.)
@@ -302,9 +300,7 @@ EXPORT_SYMBOL_GPL(ahci_platform_get_resources);
int ahci_platform_init_host(struct platform_device *pdev,
struct ahci_host_priv *hpriv,
const struct ata_port_info *pi_template,
- unsigned long host_flags,
- unsigned int force_port_map,
- unsigned int mask_port_map)
+ unsigned long host_flags)
{
struct device *dev = &pdev->dev;
struct ata_port_info pi = *pi_template;
@@ -322,7 +318,7 @@ int ahci_platform_init_host(struct platform_device *pdev,
pi.private_data = (void *)host_flags;
hpriv->flags |= host_flags;
- ahci_save_initial_config(dev, hpriv, force_port_map, mask_port_map);
+ ahci_save_initial_config(dev, hpriv);
if (hpriv->cap & HOST_CAP_NCQ)
pi.flags |= ATA_FLAG_NCQ;
diff --git a/include/linux/ahci_platform.h b/include/linux/ahci_platform.h
index 6dfd51a04d77..554bcafa7eb4 100644
--- a/include/linux/ahci_platform.h
+++ b/include/linux/ahci_platform.h
@@ -44,9 +44,7 @@ struct ahci_host_priv *ahci_platform_get_resources(
int ahci_platform_init_host(struct platform_device *pdev,
struct ahci_host_priv *hpriv,
const struct ata_port_info *pi_template,
- unsigned long host_flags,
- unsigned int force_port_map,
- unsigned int mask_port_map);
+ unsigned long host_flags);
int ahci_platform_suspend_host(struct device *dev);
int ahci_platform_resume_host(struct device *dev);
--
1.9.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v10 3/8] ata: libahci_platform: move port_map parameters into the AHCI structure
2014-07-18 12:30 ` [PATCH v10 3/8] ata: libahci_platform: move port_map parameters into the AHCI structure Antoine Ténart
@ 2014-07-18 13:47 ` Tejun Heo
2014-07-21 9:02 ` Antoine Ténart
0 siblings, 1 reply; 22+ messages in thread
From: Tejun Heo @ 2014-07-18 13:47 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Jul 18, 2014 at 02:30:02PM +0200, Antoine T?nart wrote:
> @@ -321,6 +321,8 @@ struct ahci_host_priv {
> u32 cap; /* cap to use */
> u32 cap2; /* cap2 to use */
> u32 port_map; /* port map to use */
> + u32 force_port_map; /* force port map */
> + u32 mask_port_map; /* mask out particular bits */
So, ->flags and ->force/mask_port_map are the only input ones, right?
Can we collect them to one spot and label them as such?
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 22+ messages in thread* [PATCH v10 3/8] ata: libahci_platform: move port_map parameters into the AHCI structure
2014-07-18 13:47 ` Tejun Heo
@ 2014-07-21 9:02 ` Antoine Ténart
0 siblings, 0 replies; 22+ messages in thread
From: Antoine Ténart @ 2014-07-21 9:02 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On Fri, Jul 18, 2014 at 09:47:30AM -0400, Tejun Heo wrote:
> On Fri, Jul 18, 2014 at 02:30:02PM +0200, Antoine T?nart wrote:
> > @@ -321,6 +321,8 @@ struct ahci_host_priv {
> > u32 cap; /* cap to use */
> > u32 cap2; /* cap2 to use */
> > u32 port_map; /* port map to use */
> > + u32 force_port_map; /* force port map */
> > + u32 mask_port_map; /* mask out particular bits */
>
> So, ->flags and ->force/mask_port_map are the only input ones, right?
> Can we collect them to one spot and label them as such?
Yes. So you want a comment about this in the header?
Antoine
--
Antoine T?nart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v10 4/8] ata: libahci: allow to use multiple PHYs
2014-07-18 12:29 [PATCH v10 0/8] ARM: berlin: add AHCI support Antoine Ténart
` (2 preceding siblings ...)
2014-07-18 12:30 ` [PATCH v10 3/8] ata: libahci_platform: move port_map parameters into the AHCI structure Antoine Ténart
@ 2014-07-18 12:30 ` Antoine Ténart
2014-07-18 13:17 ` Lothar Waßmann
2014-07-18 12:30 ` [PATCH v10 5/8] ata: ahci_platform: add a generic AHCI compatible Antoine Ténart
` (4 subsequent siblings)
8 siblings, 1 reply; 22+ messages in thread
From: Antoine Ténart @ 2014-07-18 12:30 UTC (permalink / raw)
To: linux-arm-kernel
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 port_map mask is computed automatically when using
this.
Signed-off-by: Antoine T?nart <antoine.tenart@free-electrons.com>
---
drivers/ata/ahci.h | 3 +-
drivers/ata/libahci_platform.c | 179 ++++++++++++++++++++++++++++++++---------
2 files changed, 142 insertions(+), 40 deletions(-)
diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
index cb8d58926851..b0ea7b077d6e 100644
--- a/drivers/ata/ahci.h
+++ b/drivers/ata/ahci.h
@@ -332,7 +332,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_platform.c b/drivers/ata/libahci_platform.c
index db9b90d876dd..2c2439b4101d 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,9 @@ 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;
+ u32 mask_port_map = 0;
if (!devres_open_group(dev, NULL, GFP_KERNEL))
return ERR_PTR(-ENOMEM);
@@ -246,27 +292,82 @@ 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 -ENOSYS:
- /* No PHY support. Check if PHY is required. */
- if (of_find_property(dev->of_node, "phys", NULL)) {
- dev_err(dev, "couldn't get sata-phy: ENOSYS\n");
+ nports = of_get_child_count(dev->of_node);
+
+ if (nports) {
+ hpriv->phys = devm_kzalloc(dev, nports * sizeof(*hpriv->phys),
+ GFP_KERNEL);
+ if (!hpriv->phys) {
+ rc = -ENOMEM;
+ 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;
}
- case -ENODEV:
- /* continue normally */
- hpriv->phy = NULL;
- break;
- case -EPROBE_DEFER:
- goto err_out;
+ mask_port_map |= BIT(port);
- default:
- dev_err(dev, "couldn't get sata-phy\n");
- goto err_out;
+ 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);
+ }
+
+ if (!hpriv->mask_port_map)
+ hpriv->mask_port_map = mask_port_map;
+ } 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 -ENOSYS:
+ /* No PHY support. Check if PHY is required. */
+ if (of_find_property(dev->of_node, "phys", NULL)) {
+ dev_err(dev, "couldn't get sata-phy: ENOSYS\n");
+ goto err_out;
+ }
+ case -ENODEV:
+ /* continue normally */
+ hpriv->phys = NULL;
+ break;
+
+ default:
+ goto err_out;
+
+ }
}
}
@@ -291,7 +392,7 @@ EXPORT_SYMBOL_GPL(ahci_platform_get_resources);
* @host_flags: ahci host flags used in ahci_host_priv
*
* 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:
@@ -395,7 +496,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:
@@ -432,7 +533,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
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v10 4/8] ata: libahci: allow to use multiple PHYs
2014-07-18 12:30 ` [PATCH v10 4/8] ata: libahci: allow to use multiple PHYs Antoine Ténart
@ 2014-07-18 13:17 ` Lothar Waßmann
2014-07-21 9:01 ` Antoine Ténart
0 siblings, 1 reply; 22+ messages in thread
From: Lothar Waßmann @ 2014-07-18 13:17 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
Antoine T?nart wrote:
> 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 port_map mask is computed automatically when using
> this.
>
> Signed-off-by: Antoine T?nart <antoine.tenart@free-electrons.com>
> ---
> drivers/ata/ahci.h | 3 +-
> drivers/ata/libahci_platform.c | 179 ++++++++++++++++++++++++++++++++---------
> 2 files changed, 142 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
> index cb8d58926851..b0ea7b077d6e 100644
> --- a/drivers/ata/ahci.h
> +++ b/drivers/ata/ahci.h
> @@ -332,7 +332,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_platform.c b/drivers/ata/libahci_platform.c
> index db9b90d876dd..2c2439b4101d 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;
> +
You shouldn't have to initialize rc here, or does your gcc falsely
complain about an uninitialized variable if you don't?
> + 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;
> +}
Lothar Wa?mann
--
___________________________________________________________
Ka-Ro electronics GmbH | Pascalstra?e 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Gesch?ftsf?hrer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996
www.karo-electronics.de | info at karo-electronics.de
___________________________________________________________
^ permalink raw reply [flat|nested] 22+ messages in thread* [PATCH v10 4/8] ata: libahci: allow to use multiple PHYs
2014-07-18 13:17 ` Lothar Waßmann
@ 2014-07-21 9:01 ` Antoine Ténart
0 siblings, 0 replies; 22+ messages in thread
From: Antoine Ténart @ 2014-07-21 9:01 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On Fri, Jul 18, 2014 at 03:17:45PM +0200, Lothar Wa?mann wrote:
> Antoine T?nart wrote:
> > diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c
> > index db9b90d876dd..2c2439b4101d 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;
> > +
> You shouldn't have to initialize rc here, or does your gcc falsely
> complain about an uninitialized variable if you don't?
You're right, this is not needed.
Antoine
--
Antoine T?nart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v10 5/8] ata: ahci_platform: add a generic AHCI compatible
2014-07-18 12:29 [PATCH v10 0/8] ARM: berlin: add AHCI support Antoine Ténart
` (3 preceding siblings ...)
2014-07-18 12:30 ` [PATCH v10 4/8] ata: libahci: allow to use multiple PHYs Antoine Ténart
@ 2014-07-18 12:30 ` Antoine Ténart
2014-07-18 14:30 ` Hans de Goede
2014-07-18 12:30 ` [PATCH v10 6/8] Documentation: bindings: document the sub-nodes AHCI bindings Antoine Ténart
` (3 subsequent siblings)
8 siblings, 1 reply; 22+ messages in thread
From: Antoine Ténart @ 2014-07-18 12:30 UTC (permalink / raw)
To: linux-arm-kernel
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 <antoine.tenart@free-electrons.com>
---
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 95f9ca82082a..61b15e784020 100644
--- a/drivers/ata/ahci_platform.c
+++ b/drivers/ata/ahci_platform.c
@@ -77,6 +77,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
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v10 5/8] ata: ahci_platform: add a generic AHCI compatible
2014-07-18 12:30 ` [PATCH v10 5/8] ata: ahci_platform: add a generic AHCI compatible Antoine Ténart
@ 2014-07-18 14:30 ` Hans de Goede
0 siblings, 0 replies; 22+ messages in thread
From: Hans de Goede @ 2014-07-18 14:30 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On 07/18/2014 02:30 PM, Antoine T?nart wrote:
> 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 <antoine.tenart@free-electrons.com>
> ---
> 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 95f9ca82082a..61b15e784020 100644
> --- a/drivers/ata/ahci_platform.c
> +++ b/drivers/ata/ahci_platform.c
> @@ -77,6 +77,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", },
>
I'm not against this, I've introduced the same for the ehci / uhci platform
driver after all, but this should be documented in Documentation/devicetree,
and please make sure to mention in the docs that the correct way to use this
inside a dts file is:
compatible = "manufacturer,soc-model-ahci", "generic-ahci";
So that we can later add quirks for the specific soc if necessary without
needing to change the dts.
Regards,
Hans
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v10 6/8] Documentation: bindings: document the sub-nodes AHCI bindings
2014-07-18 12:29 [PATCH v10 0/8] ARM: berlin: add AHCI support Antoine Ténart
` (4 preceding siblings ...)
2014-07-18 12:30 ` [PATCH v10 5/8] ata: ahci_platform: add a generic AHCI compatible Antoine Ténart
@ 2014-07-18 12:30 ` Antoine Ténart
2014-07-18 12:30 ` [PATCH v10 7/8] ARM: berlin: add the AHCI node for the BG2Q Antoine Ténart
` (2 subsequent siblings)
8 siblings, 0 replies; 22+ messages in thread
From: Antoine Ténart @ 2014-07-18 12:30 UTC (permalink / raw)
To: linux-arm-kernel
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 <antoine.tenart@free-electrons.com>
---
.../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 at ffe08000 {
compatible = "snps,spear-ahci";
@@ -40,3 +57,23 @@ Examples:
clocks = <&pll6 0>, <&ahb_gates 25>;
target-supply = <®_ahci_5v>;
};
+
+With sub-nodes:
+ sata at 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 at 0 {
+ reg = <0>;
+ phys = <&sata_phy 0>;
+ };
+
+ sata1: sata-port at 1 {
+ reg = <1>;
+ phys = <&sata_phy 1>;
+ };
+ };
--
1.9.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v10 7/8] ARM: berlin: add the AHCI node for the BG2Q
2014-07-18 12:29 [PATCH v10 0/8] ARM: berlin: add AHCI support Antoine Ténart
` (5 preceding siblings ...)
2014-07-18 12:30 ` [PATCH v10 6/8] Documentation: bindings: document the sub-nodes AHCI bindings Antoine Ténart
@ 2014-07-18 12:30 ` Antoine Ténart
2014-07-18 12:30 ` [PATCH v10 8/8] ARM: berlin: enable the eSATA interface on the BG2Q DMP Antoine Ténart
2014-07-18 13:57 ` [PATCH v10 0/8] ARM: berlin: add AHCI support Tejun Heo
8 siblings, 0 replies; 22+ messages in thread
From: Antoine Ténart @ 2014-07-18 12:30 UTC (permalink / raw)
To: linux-arm-kernel
The BG2Q has an AHCI SATA controller. Add the corresponding nodes
(AHCI, PHY) into its device tree.
Signed-off-by: Antoine T?nart <antoine.tenart@free-electrons.com>
---
arch/arm/boot/dts/berlin2q.dtsi | 39 +++++++++++++++++++++++++++++++++++++++
1 file changed, 39 insertions(+)
diff --git a/arch/arm/boot/dts/berlin2q.dtsi b/arch/arm/boot/dts/berlin2q.dtsi
index 635a16a64cb4..fa34ae6657ac 100644
--- a/arch/arm/boot/dts/berlin2q.dtsi
+++ b/arch/arm/boot/dts/berlin2q.dtsi
@@ -303,6 +303,45 @@
clock-names = "refclk";
};
+ ahci: sata at 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 at 0 {
+ reg = <0>;
+ phys = <&sata_phy 0>;
+ status = "disabled";
+ };
+
+ sata1: sata-port at 1 {
+ reg = <1>;
+ phys = <&sata_phy 1>;
+ status = "disabled";
+ };
+ };
+
+ sata_phy: phy at e900a0 {
+ compatible = "marvell,berlin2q-sata-phy";
+ reg = <0xe900a0 0x200>;
+ clocks = <&chip CLKID_SATA>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ #phy-cells = <1>;
+ status = "disabled";
+
+ sata-phy at 0 {
+ reg = <0>;
+ };
+
+ sata-phy at 1 {
+ reg = <1>;
+ };
+ };
+
apb at fc0000 {
compatible = "simple-bus";
#address-cells = <1>;
--
1.9.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v10 8/8] ARM: berlin: enable the eSATA interface on the BG2Q DMP
2014-07-18 12:29 [PATCH v10 0/8] ARM: berlin: add AHCI support Antoine Ténart
` (6 preceding siblings ...)
2014-07-18 12:30 ` [PATCH v10 7/8] ARM: berlin: add the AHCI node for the BG2Q Antoine Ténart
@ 2014-07-18 12:30 ` Antoine Ténart
2014-07-18 13:57 ` [PATCH v10 0/8] ARM: berlin: add AHCI support Tejun Heo
8 siblings, 0 replies; 22+ messages in thread
From: Antoine Ténart @ 2014-07-18 12:30 UTC (permalink / raw)
To: linux-arm-kernel
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 <antoine.tenart@free-electrons.com>
---
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
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v10 0/8] ARM: berlin: add AHCI support
2014-07-18 12:29 [PATCH v10 0/8] ARM: berlin: add AHCI support Antoine Ténart
` (7 preceding siblings ...)
2014-07-18 12:30 ` [PATCH v10 8/8] ARM: berlin: enable the eSATA interface on the BG2Q DMP Antoine Ténart
@ 2014-07-18 13:57 ` Tejun Heo
2014-07-19 9:03 ` Sebastian Hesselbarth
2014-07-19 10:18 ` Hans de Goede
8 siblings, 2 replies; 22+ messages in thread
From: Tejun Heo @ 2014-07-18 13:57 UTC (permalink / raw)
To: linux-arm-kernel
(cc'ing Hans who's now maintaining libahci-platform.)
On Fri, Jul 18, 2014 at 02:29:59PM +0200, Antoine T?nart wrote:
> Tejun, Kishon, Sebastian,
>
> I looked into the AHCI framework to see how to map PHYs and ports
> information. I see two ways of doing this:
> - We can attach the ahci_port_priv to the ahci_host_priv structure,
> but that would require quite a lot of changes since the
> ahci_port_priv is initialized at the very end (in port_start()) and
> because ahci_port_priv is currently retrieved from the ata_port
> structure in libahci functions. We do want to parse the dt ports
> early in the AHCI initialization to be able to generate the right
> port_map mask. Tests would be needed to ensure nothing is broken.
> - We can move the PHY handling to where the ports are handled, moving
> PHYs from ahci_host_priv to ahci_port_priv. This also would require
> to perform some tests as PHY operations would be moved from
> libahci_platform to libahci.
I don't get the last part. Why would it have to be moved from
libahci_platform to libahci? Can't we break up the init steps so that
PHY handling can be put inbetween? The last time I suggested that,
Hans seemed to agree.
> In both cases we do not have time to do this for the next release, as
> the request popped up quite late.
>
> So as of now:
> - Either the series is merged as is and changes to the AHCI framework
> can be made for 3.18, as it's not particularly linked to this
> series.
> - Or you really do not want it. Then that would be great if patches
> 1-2 and 7-8 could be merged so that we do not end up with this big
> series going for yet another cycle... I think Kishon already took
> patches 1-2.
I don't wanna take in code which isn't in the shape that it should be.
Things like this accumulate to become a large maintenance burden over
time. Sure, urgent things can slip in and then later be fixed up but
who are gonna do that here? You guys already seem to be under time
pressure as it is.
If you guys can figure something out with Hans regarding how to
proceed on this, I'll be happy take the code as is.
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 22+ messages in thread* [PATCH v10 0/8] ARM: berlin: add AHCI support
2014-07-18 13:57 ` [PATCH v10 0/8] ARM: berlin: add AHCI support Tejun Heo
@ 2014-07-19 9:03 ` Sebastian Hesselbarth
2014-07-19 10:18 ` Hans de Goede
1 sibling, 0 replies; 22+ messages in thread
From: Sebastian Hesselbarth @ 2014-07-19 9:03 UTC (permalink / raw)
To: linux-arm-kernel
On 07/18/2014 03:57 PM, Tejun Heo wrote:
> On Fri, Jul 18, 2014 at 02:29:59PM +0200, Antoine T?nart wrote:
>> In both cases we do not have time to do this for the next release, as
>> the request popped up quite late.
>>
>> So as of now:
>> - Either the series is merged as is and changes to the AHCI framework
>> can be made for 3.18, as it's not particularly linked to this
>> series.
>> - Or you really do not want it. Then that would be great if patches
>> 1-2 and 7-8 could be merged so that we do not end up with this big
>> series going for yet another cycle... I think Kishon already took
>> patches 1-2.
>
> I don't wanna take in code which isn't in the shape that it should be.
> Things like this accumulate to become a large maintenance burden over
> time. Sure, urgent things can slip in and then later be fixed up but
> who are gonna do that here? You guys already seem to be under time
> pressure as it is.
>
> If you guys can figure something out with Hans regarding how to
> proceed on this, I'll be happy take the code as is.
I cannot say much about AHCI, but if the PHY driver is fine I can
take the DT patches adding it to BG2Q and BG2Q-DMP at any time.
Sebastian
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v10 0/8] ARM: berlin: add AHCI support
2014-07-18 13:57 ` [PATCH v10 0/8] ARM: berlin: add AHCI support Tejun Heo
2014-07-19 9:03 ` Sebastian Hesselbarth
@ 2014-07-19 10:18 ` Hans de Goede
2014-07-19 10:31 ` Hans de Goede
1 sibling, 1 reply; 22+ messages in thread
From: Hans de Goede @ 2014-07-19 10:18 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On 07/18/2014 03:57 PM, Tejun Heo wrote:
> (cc'ing Hans who's now maintaining libahci-platform.)
Note I was already following this thread as I'm subscribed to
linux-ide now.
>
> On Fri, Jul 18, 2014 at 02:29:59PM +0200, Antoine T?nart wrote:
>> Tejun, Kishon, Sebastian,
>>
>> I looked into the AHCI framework to see how to map PHYs and ports
>> information. I see two ways of doing this:
>> - We can attach the ahci_port_priv to the ahci_host_priv structure,
>> but that would require quite a lot of changes since the
>> ahci_port_priv is initialized at the very end (in port_start()) and
>> because ahci_port_priv is currently retrieved from the ata_port
>> structure in libahci functions. We do want to parse the dt ports
>> early in the AHCI initialization to be able to generate the right
>> port_map mask. Tests would be needed to ensure nothing is broken.
>> - We can move the PHY handling to where the ports are handled, moving
>> PHYs from ahci_host_priv to ahci_port_priv. This also would require
>> to perform some tests as PHY operations would be moved from
>> libahci_platform to libahci.
>
> I don't get the last part. Why would it have to be moved from
> libahci_platform to libahci? Can't we break up the init steps so that
> PHY handling can be put inbetween? The last time I suggested that,
> Hans seemed to agree.
Yes as it sounded good, but I did not look at the code to closely,
looking closer at the code I can see the problem.
ahci_port_priv gets allocated from ahci_port_start in
libahci.c and that same function also starts the port.
If we want to store the phy in ahci_port_priv then we need to do
some work between ahci_port_priv getting allocated and the port getting
started.
ahci_port_start gets allocated from ata_host_start, with the relevant
bit being:
for (i = 0; i < host->n_ports; i++) {
struct ata_port *ap = host->ports[i];
if (ap->ops->port_start) {
rc = ap->ops->port_start(ap);
We could move the allocating of ahci_port_priv to libahci.c:
ahci_init_controller() which has a similar loop. But then the
ahci_port_priv info still is allocated after we call
ahci_platform_enable_resources()
The problem is that:
1) We need to enable resources before we can do ahci_save_initial_config()
2) We must do ahci_save_initial_config() before we can do ata_host_alloc_pinfo()
3) Therefor we don't have port_info at enable_resources time, which is when we
want to enable the phys (and we cannot just enable the phys elsewhere as
enable_resouces gets used on e.g. resume too).
So I think it is best to just make the phy pointers an array inside
ahci_host_priv, with a comment that the array indexes must match port
indexes. Which brings us back to square one, sorry for long dance to
get there.
I know I initially agreed that it would be good to store the phy
pointer inside ahci_port_priv, but in practice this just does not
work well as we get and enable resources before we have ahci_port_priv.
Which I see is exactly what you've done in patch 4 of v10 of this
series :)
>> In both cases we do not have time to do this for the next release, as
>> the request popped up quite late.
>>
>> So as of now:
>> - Either the series is merged as is and changes to the AHCI framework
>> can be made for 3.18, as it's not particularly linked to this
>> series.
>> - Or you really do not want it. Then that would be great if patches
>> 1-2 and 7-8 could be merged so that we do not end up with this big
>> series going for yet another cycle... I think Kishon already took
>> patches 1-2.
>
> I don't wanna take in code which isn't in the shape that it should be.
> Things like this accumulate to become a large maintenance burden over
> time. Sure, urgent things can slip in and then later be fixed up but
> who are gonna do that here? You guys already seem to be under time
> pressure as it is.
>
> If you guys can figure something out with Hans regarding how to
> proceed on this, I'll be happy take the code as is.
I think the way to proceed with this is just leaving things as they are,
see above.
As for taking the ahci parts of this series, except for the minor comments
from you to patch 3 and from me to patch 5 that is fine with me.
Regards,
Hans
>
> Thanks.
>
^ permalink raw reply [flat|nested] 22+ messages in thread* [PATCH v10 0/8] ARM: berlin: add AHCI support
2014-07-19 10:18 ` Hans de Goede
@ 2014-07-19 10:31 ` Hans de Goede
2014-07-21 9:09 ` Antoine Ténart
0 siblings, 1 reply; 22+ messages in thread
From: Hans de Goede @ 2014-07-19 10:31 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On 07/19/2014 12:18 PM, Hans de Goede wrote:
<snip>
> The problem is that:
>
> 1) We need to enable resources before we can do ahci_save_initial_config()
> 2) We must do ahci_save_initial_config() before we can do ata_host_alloc_pinfo()
> 3) Therefor we don't have port_info at enable_resources time, which is when we
> want to enable the phys (and we cannot just enable the phys elsewhere as
> enable_resouces gets used on e.g. resume too).
>
> So I think it is best to just make the phy pointers an array inside
> ahci_host_priv, with a comment that the array indexes must match port
> indexes.
So looking at "[PATCH v10 4/8] ata: libahci: allow to use multiple PHYs"
I see that currently the phy array indexes do not necessarily match the
port indexes. Since you already allocate the phys array at nports size,
I suggest simply making the array sparse, leaving in NULL entries for
unused ports, and adjusting enable / disable_phys to check for NULL
pointers. This way we still have a 1:1 way to map ports <-> phys if
we want to do something with phys on a per port basis in the future.
Note please also add a check that reg < nports so that we don't use
the array out of bounds if there is an error in the dts.
<snip>
Regards,
Hans
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v10 0/8] ARM: berlin: add AHCI support
2014-07-19 10:31 ` Hans de Goede
@ 2014-07-21 9:09 ` Antoine Ténart
0 siblings, 0 replies; 22+ messages in thread
From: Antoine Ténart @ 2014-07-21 9:09 UTC (permalink / raw)
To: linux-arm-kernel
Hi Hans,
On Sat, Jul 19, 2014 at 12:31:19PM +0200, Hans de Goede wrote:
> On 07/19/2014 12:18 PM, Hans de Goede wrote:
>
> >The problem is that:
> >
> >1) We need to enable resources before we can do ahci_save_initial_config()
> >2) We must do ahci_save_initial_config() before we can do ata_host_alloc_pinfo()
> >3) Therefor we don't have port_info at enable_resources time, which is when we
> >want to enable the phys (and we cannot just enable the phys elsewhere as
> >enable_resouces gets used on e.g. resume too).
> >
> >So I think it is best to just make the phy pointers an array inside
> >ahci_host_priv, with a comment that the array indexes must match port
> >indexes.
>
> So looking at "[PATCH v10 4/8] ata: libahci: allow to use multiple PHYs"
> I see that currently the phy array indexes do not necessarily match the
> port indexes. Since you already allocate the phys array at nports size,
> I suggest simply making the array sparse, leaving in NULL entries for
> unused ports, and adjusting enable / disable_phys to check for NULL
> pointers. This way we still have a 1:1 way to map ports <-> phys if
> we want to do something with phys on a per port basis in the future.
>
> Note please also add a check that reg < nports so that we don't use
> the array out of bounds if there is an error in the dts.
Ok. I'll rework patch 4 along with the other modifications requested,
and I'll send a new version early this week.
Antoine
--
Antoine T?nart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 22+ messages in thread