devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/9] Armada8k enable per-port SATA interrupts and drop a hack in the IRQ subsystem
@ 2020-11-04 15:52 sven.auhagen
  2020-11-04 15:52 ` [PATCH v2 1/9] ata: ahci: mvebu: Rename a platform data flag sven.auhagen
                   ` (8 more replies)
  0 siblings, 9 replies; 14+ messages in thread
From: sven.auhagen @ 2020-11-04 15:52 UTC (permalink / raw)
  To: axboe, hdegoede, robh+dt, tglx, maz, gregory.clement
  Cc: linux-ide, linux-arm-kernel, devicetree, jason, andrew, rjw,
	viresh.kumar, antoine.tenart, maxime.chevallier,
	thomas.petazzoni, miquel.raynal

From: Sven Auhagen <sven.auhagen@voleatech.de>

Hello,

There were already 4 versions of this series from Miquèl.
I talked to Miquèl and I fixed up the last comments from v4.
I am looking for feedback if this patch series is now ready to be merged
and what should be further changed.

Here is the original cover letter:

Some time ago, when the initial support for Armada CP110 was
contributed, the SATA core was not able to handle per-port
interrupts. Despite the hardware reality, the device tree only
represents one main interrupt for the two ports. Having both SATA
ports enabled at the same time has been achieved by a hack in the ICU
driver(1) that faked the use of the two interrupts, no matter which
SATA port was in use.

Now that the SATA core is ready to handle more than one interrupt,
this series adds support for it in the libahci_platform code. The
CP110 device tree must be updated to reflect the two SATA ports
available and their respective interrupts. To do not break DT backward
compatibility, the ahci_platform driver now embeds a special quirk
which checks if the DT is valid (only for A8k compatible) and, if
needed, creates the two missing sub-nodes, and assign them the
relevant "reg" and "interrupts" properties, before removing the main
SATA node "interrupts" one.

(1) The ICU is an irqchip aggregating the CP110 (south-bridge)
interrupts into MSIs for the AP806 (north-bridge).

Best
Sven

Change from v1:
  * Add a patch to enable custom irq initialization in 
    plattform init host
  * Add multi_irq_host_ack callback for the msi irq handler
  * Rework the ahci mvebu patch to initiate the irq and use
    the new multi_irq_host_ack to handle the custom irq code.
    Remove the custom irq handler and duplicate code.
  * Fix the armada8k backwards compatibility code
  * Rename AHCI_PLATFORM_A8K_QUIRK to AHCI_PLATFORM_ARMADA8K_QUIRK

Miquel Raynal (5):
  ata: ahci: mvebu: Rename a platform data flag
  ata: ahci: mvebu: Support A8k compatible
  irqchip/irq-mvebu-icu: Remove the double SATA ports interrupt hack
  dt-bindings: ata: Update ahci bindings with possible per-port
    interrupts
  dt-bindings: ata: Update ahci_mvebu bindings

Sven Auhagen (4):
  ata: ahci: custom irq init for host init
  ata: ahci: add ack callback to multi irq handler
  ata: ahci: mvebu: Add support for A8k legacy DT bindings
  arm64: dts: marvell: armada-cp110: Switch to per-port SATA interrupts

 .../devicetree/bindings/ata/ahci-platform.txt |   7 +
 arch/arm64/boot/dts/marvell/armada-cp11x.dtsi |   6 +-
 drivers/ata/ahci.h                            |   2 +
 drivers/ata/ahci_mvebu.c                      | 143 ++++++++++++++++--
 drivers/ata/libahci.c                         |   4 +
 drivers/ata/libahci_platform.c                |  19 ++-
 drivers/irqchip/irq-mvebu-icu.c               |  18 ---
 include/linux/ahci_platform.h                 |   1 +
 8 files changed, 160 insertions(+), 40 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2 1/9] ata: ahci: mvebu: Rename a platform data flag
  2020-11-04 15:52 [PATCH v2 0/9] Armada8k enable per-port SATA interrupts and drop a hack in the IRQ subsystem sven.auhagen
@ 2020-11-04 15:52 ` sven.auhagen
  2020-11-04 15:52 ` [PATCH v2 2/9] ata: ahci: mvebu: Support A8k compatible sven.auhagen
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: sven.auhagen @ 2020-11-04 15:52 UTC (permalink / raw)
  To: axboe, hdegoede, robh+dt, tglx, maz, gregory.clement
  Cc: linux-ide, linux-arm-kernel, devicetree, jason, andrew, rjw,
	viresh.kumar, antoine.tenart, maxime.chevallier,
	thomas.petazzoni, miquel.raynal

From: Miquel Raynal <miquel.raynal@bootlin.com>

Before adding more entries in the platform data structure, rename the
flags entry to be more precise and name it host_flags.

Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Sven Auhagen <sven.auhagen@voleatech.de>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/ata/ahci_mvebu.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/ata/ahci_mvebu.c b/drivers/ata/ahci_mvebu.c
index 3ad46d26d9d5..563cc4c64d96 100644
--- a/drivers/ata/ahci_mvebu.c
+++ b/drivers/ata/ahci_mvebu.c
@@ -30,7 +30,7 @@
 
 struct ahci_mvebu_plat_data {
 	int (*plat_config)(struct ahci_host_priv *hpriv);
-	unsigned int flags;
+	unsigned int host_flags;
 };
 
 static void ahci_mvebu_mbus_config(struct ahci_host_priv *hpriv,
@@ -196,7 +196,7 @@ static int ahci_mvebu_probe(struct platform_device *pdev)
 	if (IS_ERR(hpriv))
 		return PTR_ERR(hpriv);
 
-	hpriv->flags |= pdata->flags;
+	hpriv->flags |= pdata->host_flags;
 	hpriv->plat_data = (void *)pdata;
 
 	rc = ahci_platform_enable_resources(hpriv);
@@ -227,7 +227,7 @@ static const struct ahci_mvebu_plat_data ahci_mvebu_armada_380_plat_data = {
 
 static const struct ahci_mvebu_plat_data ahci_mvebu_armada_3700_plat_data = {
 	.plat_config = ahci_mvebu_armada_3700_config,
-	.flags = AHCI_HFLAG_SUSPEND_PHYS | AHCI_HFLAG_IGN_NOTSUPP_POWER_ON,
+	.host_flags = AHCI_HFLAG_SUSPEND_PHYS | AHCI_HFLAG_IGN_NOTSUPP_POWER_ON,
 };
 
 static const struct of_device_id ahci_mvebu_of_match[] = {
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 2/9] ata: ahci: mvebu: Support A8k compatible
  2020-11-04 15:52 [PATCH v2 0/9] Armada8k enable per-port SATA interrupts and drop a hack in the IRQ subsystem sven.auhagen
  2020-11-04 15:52 ` [PATCH v2 1/9] ata: ahci: mvebu: Rename a platform data flag sven.auhagen
@ 2020-11-04 15:52 ` sven.auhagen
  2020-11-04 15:52 ` [PATCH v2 3/9] ata: ahci: custom irq init for host init sven.auhagen
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: sven.auhagen @ 2020-11-04 15:52 UTC (permalink / raw)
  To: axboe, hdegoede, robh+dt, tglx, maz, gregory.clement
  Cc: linux-ide, linux-arm-kernel, devicetree, jason, andrew, rjw,
	viresh.kumar, antoine.tenart, maxime.chevallier,
	thomas.petazzoni, miquel.raynal

From: Miquel Raynal <miquel.raynal@bootlin.com>

The ahci_platform.c driver was historically the one bound to the A8k
AHCI compatible string, but before adding a quirk for this compatible,
it is probably cleaner to put all Marvell EBU code in one place: the
ahci_mvebu.c driver.

Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Sven Auhagen <sven.auhagen@voleatech.de>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/ata/ahci_mvebu.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/ata/ahci_mvebu.c b/drivers/ata/ahci_mvebu.c
index 563cc4c64d96..ed82381dc4a7 100644
--- a/drivers/ata/ahci_mvebu.c
+++ b/drivers/ata/ahci_mvebu.c
@@ -96,6 +96,11 @@ static int ahci_mvebu_armada_3700_config(struct ahci_host_priv *hpriv)
 	return 0;
 }
 
+static int ahci_mvebu_armada_8k_config(struct ahci_host_priv *hpriv)
+{
+	return 0;
+}
+
 /**
  * ahci_mvebu_stop_engine
  *
@@ -230,6 +235,10 @@ static const struct ahci_mvebu_plat_data ahci_mvebu_armada_3700_plat_data = {
 	.host_flags = AHCI_HFLAG_SUSPEND_PHYS | AHCI_HFLAG_IGN_NOTSUPP_POWER_ON,
 };
 
+static const struct ahci_mvebu_plat_data ahci_mvebu_armada_8k_plat_data = {
+	.plat_config = ahci_mvebu_armada_8k_config,
+};
+
 static const struct of_device_id ahci_mvebu_of_match[] = {
 	{
 		.compatible = "marvell,armada-380-ahci",
@@ -239,6 +248,10 @@ static const struct of_device_id ahci_mvebu_of_match[] = {
 		.compatible = "marvell,armada-3700-ahci",
 		.data = &ahci_mvebu_armada_3700_plat_data,
 	},
+	{
+		.compatible = "marvell,armada-8k-ahci",
+		.data = &ahci_mvebu_armada_8k_plat_data,
+	},
 	{ },
 };
 MODULE_DEVICE_TABLE(of, ahci_mvebu_of_match);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 3/9] ata: ahci: custom irq init for host init
  2020-11-04 15:52 [PATCH v2 0/9] Armada8k enable per-port SATA interrupts and drop a hack in the IRQ subsystem sven.auhagen
  2020-11-04 15:52 ` [PATCH v2 1/9] ata: ahci: mvebu: Rename a platform data flag sven.auhagen
  2020-11-04 15:52 ` [PATCH v2 2/9] ata: ahci: mvebu: Support A8k compatible sven.auhagen
@ 2020-11-04 15:52 ` sven.auhagen
  2020-11-09 14:00   ` Hans de Goede
  2020-11-04 15:52 ` [PATCH v2 4/9] ata: ahci: add ack callback to multi irq handler sven.auhagen
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 14+ messages in thread
From: sven.auhagen @ 2020-11-04 15:52 UTC (permalink / raw)
  To: axboe, hdegoede, robh+dt, tglx, maz, gregory.clement
  Cc: linux-ide, linux-arm-kernel, devicetree, jason, andrew, rjw,
	viresh.kumar, antoine.tenart, maxime.chevallier,
	thomas.petazzoni, miquel.raynal

From: Sven Auhagen <sven.auhagen@voleatech.de>

Disable the platform irq init in ahci init platform host
if it was initiated by a custom function.
To check for it I am using the AHCI_HFLAG_MULTI_MSI flag.

Suggested-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Sven Auhagen <sven.auhagen@voleatech.de>
---
 drivers/ata/libahci_platform.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c
index de638dafce21..f6f2a111d226 100644
--- a/drivers/ata/libahci_platform.c
+++ b/drivers/ata/libahci_platform.c
@@ -581,14 +581,16 @@ int ahci_platform_init_host(struct platform_device *pdev,
 	struct ata_host *host;
 	int i, irq, n_ports, rc;
 
-	irq = platform_get_irq(pdev, 0);
-	if (irq <= 0) {
-		if (irq != -EPROBE_DEFER)
-			dev_err(dev, "no irq\n");
-		return irq;
-	}
+	if (!(hpriv->flags & AHCI_HFLAG_MULTI_MSI)) {
+		irq = platform_get_irq(pdev, 0);
+		if (irq <= 0) {
+			if (irq != -EPROBE_DEFER)
+				dev_err(dev, "no irq\n");
+			return irq;
+		}
 
-	hpriv->irq = irq;
+		hpriv->irq = irq;
+	}
 
 	/* prepare host */
 	pi.private_data = (void *)(unsigned long)hpriv->flags;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 4/9] ata: ahci: add ack callback to multi irq handler
  2020-11-04 15:52 [PATCH v2 0/9] Armada8k enable per-port SATA interrupts and drop a hack in the IRQ subsystem sven.auhagen
                   ` (2 preceding siblings ...)
  2020-11-04 15:52 ` [PATCH v2 3/9] ata: ahci: custom irq init for host init sven.auhagen
@ 2020-11-04 15:52 ` sven.auhagen
  2020-11-09 13:59   ` Hans de Goede
  2020-11-04 15:52 ` [PATCH v2 5/9] ata: ahci: mvebu: Add support for A8k legacy DT bindings sven.auhagen
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 14+ messages in thread
From: sven.auhagen @ 2020-11-04 15:52 UTC (permalink / raw)
  To: axboe, hdegoede, robh+dt, tglx, maz, gregory.clement
  Cc: linux-ide, linux-arm-kernel, devicetree, jason, andrew, rjw,
	viresh.kumar, antoine.tenart, maxime.chevallier,
	thomas.petazzoni, miquel.raynal

From: Sven Auhagen <sven.auhagen@voleatech.de>

In order to support custom actions at the end of the irq handler
a multi_irq_host_ack callback is added to the struct ahci_host_priv.

Suggested-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Sven Auhagen <sven.auhagen@voleatech.de>
---
 drivers/ata/ahci.h    | 2 ++
 drivers/ata/libahci.c | 4 ++++
 2 files changed, 6 insertions(+)

diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
index 98b8baa47dc5..d8109e06794c 100644
--- a/drivers/ata/ahci.h
+++ b/drivers/ata/ahci.h
@@ -371,6 +371,8 @@ struct ahci_host_priv {
 	/* only required for per-port MSI(-X) support */
 	int			(*get_irq_vector)(struct ata_host *host,
 						  int port);
+
+	int			(*multi_irq_host_ack)(int irq, struct ata_port *ap);
 };
 
 extern int ahci_ignore_sss;
diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index ea5bf5f4cbed..cf9839135a57 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -1897,6 +1897,7 @@ static void ahci_port_intr(struct ata_port *ap)
 static irqreturn_t ahci_multi_irqs_intr_hard(int irq, void *dev_instance)
 {
 	struct ata_port *ap = dev_instance;
+	struct ahci_host_priv *hpriv = ap->host->private_data;
 	void __iomem *port_mmio = ahci_port_base(ap);
 	u32 status;
 
@@ -1909,6 +1910,9 @@ static irqreturn_t ahci_multi_irqs_intr_hard(int irq, void *dev_instance)
 	ahci_handle_port_interrupt(ap, port_mmio, status);
 	spin_unlock(ap->lock);
 
+	if (hpriv->multi_irq_host_ack)
+		hpriv->multi_irq_host_ack(irq, ap);
+
 	VPRINTK("EXIT\n");
 
 	return IRQ_HANDLED;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 5/9] ata: ahci: mvebu: Add support for A8k legacy DT bindings
  2020-11-04 15:52 [PATCH v2 0/9] Armada8k enable per-port SATA interrupts and drop a hack in the IRQ subsystem sven.auhagen
                   ` (3 preceding siblings ...)
  2020-11-04 15:52 ` [PATCH v2 4/9] ata: ahci: add ack callback to multi irq handler sven.auhagen
@ 2020-11-04 15:52 ` sven.auhagen
  2020-11-09 13:59   ` Hans de Goede
  2020-11-04 15:52 ` [PATCH v2 6/9] irqchip/irq-mvebu-icu: Remove the double SATA ports interrupt hack sven.auhagen
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 14+ messages in thread
From: sven.auhagen @ 2020-11-04 15:52 UTC (permalink / raw)
  To: axboe, hdegoede, robh+dt, tglx, maz, gregory.clement
  Cc: linux-ide, linux-arm-kernel, devicetree, jason, andrew, rjw,
	viresh.kumar, antoine.tenart, maxime.chevallier,
	thomas.petazzoni, miquel.raynal

From: Sven Auhagen <sven.auhagen@voleatech.de>

The CP110 SATA unit has 2 ports, and a dedicated ICU entry per
port. In the past, the AHCI SATA driver only supported one interrupt
per SATA unit. To solve this conflict, the 2 SATA wired interrupts in
the South-Bridge got configured as 1 GIC interrupt in the
North-Bridge, regardless of the number of SATA ports actually
enabled/in use, and the DT bindings only referenced the interrupt of
one port.

Since then, this limitation has been addressed and this patch ensures
backward compatibility with old DTs not describing SATA ports
correctly directly from the AHCI MVEBU driver. This way, we will be
able to drop the hack from the ICU driver. IOW, when the A8k
compatible string is used and there is no sub-nodes in the DT, we
fake the creation and mapping of the second (missing) interrupt.

Signed-off-by: Sven Auhagen <sven.auhagen@voleatech.de>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/ata/ahci_mvebu.c       | 126 ++++++++++++++++++++++++++++++---
 drivers/ata/libahci_platform.c |   3 +
 include/linux/ahci_platform.h  |   1 +
 3 files changed, 120 insertions(+), 10 deletions(-)

diff --git a/drivers/ata/ahci_mvebu.c b/drivers/ata/ahci_mvebu.c
index ed82381dc4a7..22447b1bb23f 100644
--- a/drivers/ata/ahci_mvebu.c
+++ b/drivers/ata/ahci_mvebu.c
@@ -16,6 +16,7 @@
 #include <linux/mbus.h>
 #include <linux/module.h>
 #include <linux/of_device.h>
+#include <linux/of_irq.h>
 #include <linux/platform_device.h>
 #include "ahci.h"
 
@@ -28,9 +29,15 @@
 #define AHCI_WINDOW_BASE(win)	(0x64 + ((win) << 4))
 #define AHCI_WINDOW_SIZE(win)	(0x68 + ((win) << 4))
 
+#define ICU_SATA0_ICU_ID 109
+#define ICU_SATA1_ICU_ID 107
+
 struct ahci_mvebu_plat_data {
-	int (*plat_config)(struct ahci_host_priv *hpriv);
+	int (*plat_config)(struct platform_device *pdev,
+				   struct ahci_host_priv *hpriv);
 	unsigned int host_flags;
+	unsigned int resource_flags;
+	unsigned int port_irq[2];
 };
 
 static void ahci_mvebu_mbus_config(struct ahci_host_priv *hpriv,
@@ -67,7 +74,8 @@ static void ahci_mvebu_regret_option(struct ahci_host_priv *hpriv)
 	writel(0x80, hpriv->mmio + AHCI_VENDOR_SPECIFIC_0_DATA);
 }
 
-static int ahci_mvebu_armada_380_config(struct ahci_host_priv *hpriv)
+static int ahci_mvebu_armada_380_config(struct platform_device *pdev,
+				   struct ahci_host_priv *hpriv)
 {
 	const struct mbus_dram_target_info *dram;
 	int rc = 0;
@@ -83,7 +91,8 @@ static int ahci_mvebu_armada_380_config(struct ahci_host_priv *hpriv)
 	return rc;
 }
 
-static int ahci_mvebu_armada_3700_config(struct ahci_host_priv *hpriv)
+static int ahci_mvebu_armada_3700_config(struct platform_device *pdev,
+				   struct ahci_host_priv *hpriv)
 {
 	u32 reg;
 
@@ -96,8 +105,94 @@ static int ahci_mvebu_armada_3700_config(struct ahci_host_priv *hpriv)
 	return 0;
 }
 
-static int ahci_mvebu_armada_8k_config(struct ahci_host_priv *hpriv)
+static int multi_irq_host_ack_armada8k(int irq, struct ata_port *ap)
 {
+	struct ahci_host_priv *hpriv = ap->host->private_data;
+	void __iomem *mmio = hpriv->mmio;
+
+	writel(BIT(ap->port_no), mmio + HOST_IRQ_STAT);
+
+	return 0;
+}
+
+static int ahci_get_per_port_irq_armada8k(struct ata_host *host, int port)
+{
+	struct ahci_host_priv *hpriv = host->private_data;
+	struct ahci_mvebu_plat_data *pdata = hpriv->plat_data;
+
+	return pdata->port_irq[port];
+}
+
+static int ahci_mvebu_armada_8k_irq_backwards(struct ahci_host_priv *hpriv,
+				       struct device *dev)
+{
+	struct device_node *np = of_irq_find_parent(dev->of_node);
+	struct ahci_mvebu_plat_data *pdata = hpriv->plat_data;
+	struct irq_data *irqd = irq_get_irq_data(pdata->port_irq[0]);
+	int host_irq = irqd ? irqd_to_hwirq(irqd) : 0;
+	int missing_irq = (host_irq == ICU_SATA1_ICU_ID) ?
+		ICU_SATA0_ICU_ID : ICU_SATA1_ICU_ID;
+	struct irq_fwspec fwspec = {
+		.fwnode = of_node_to_fwnode(np),
+		.param_count = 2,
+		.param = {missing_irq, IRQ_TYPE_LEVEL_HIGH},
+	};
+	int irq;
+
+	pdata->port_irq[1] = irq_create_fwspec_mapping(&fwspec);
+	hpriv->mask_port_map = GENMASK(1, 0);
+
+	if (missing_irq == ICU_SATA0_ICU_ID) {
+		irq = pdata->port_irq[0];
+		pdata->port_irq[0] = pdata->port_irq[1];
+		pdata->port_irq[1] = irq;
+	}
+
+	return 0;
+}
+
+static int ahci_mvebu_armada_8k_config(struct platform_device *pdev,
+				   struct ahci_host_priv *hpriv)
+{
+	struct ahci_mvebu_plat_data *pdata = hpriv->plat_data;
+	struct device *dev = &pdev->dev;
+	struct device_node *child;
+	int port_irq, child_nodes, port = 0;
+
+	/* Get IRQs per port */
+	child_nodes = of_get_child_count(dev->of_node);
+	if (child_nodes) {
+		for_each_child_of_node(dev->of_node, child) {
+
+			port_irq = of_irq_get(child, 0);
+			if (!port_irq)
+				port_irq = -EINVAL;
+
+			if (port_irq < 0)
+				goto compat;
+
+			pdata->port_irq[port] = port_irq;
+			port++;
+		}
+
+		goto out;
+	}
+
+compat:
+	/* Backwards Compatibility Check */
+	port_irq = platform_get_irq(pdev, 0);
+	if (port_irq > 0) {
+		pdata->port_irq[0] = port_irq;
+		ahci_mvebu_armada_8k_irq_backwards(hpriv, dev);
+	} else {
+		dev_err(dev, "no irq\n");
+		return port_irq;
+	}
+
+out:
+	hpriv->flags |= AHCI_HFLAG_MULTI_MSI;
+	hpriv->get_irq_vector = ahci_get_per_port_irq_armada8k;
+
 	return 0;
 }
 
@@ -167,7 +262,7 @@ static int ahci_mvebu_resume(struct platform_device *pdev)
 	struct ahci_host_priv *hpriv = host->private_data;
 	const struct ahci_mvebu_plat_data *pdata = hpriv->plat_data;
 
-	pdata->plat_config(hpriv);
+	pdata->plat_config(pdev, hpriv);
 
 	return ahci_platform_resume_host(&pdev->dev);
 }
@@ -189,15 +284,22 @@ static struct scsi_host_template ahci_platform_sht = {
 
 static int ahci_mvebu_probe(struct platform_device *pdev)
 {
-	const struct ahci_mvebu_plat_data *pdata;
+	const struct ahci_mvebu_plat_data *pdata_plat;
+	struct ahci_mvebu_plat_data *pdata;
 	struct ahci_host_priv *hpriv;
 	int rc;
 
-	pdata = of_device_get_match_data(&pdev->dev);
-	if (!pdata)
+	pdata_plat = of_device_get_match_data(&pdev->dev);
+	if (!pdata_plat)
 		return -EINVAL;
 
-	hpriv = ahci_platform_get_resources(pdev, 0);
+	pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return -ENOMEM;
+
+	memcpy(pdata, pdata_plat, sizeof(*pdata));
+
+	hpriv = ahci_platform_get_resources(pdev, pdata->resource_flags);
 	if (IS_ERR(hpriv))
 		return PTR_ERR(hpriv);
 
@@ -210,10 +312,13 @@ static int ahci_mvebu_probe(struct platform_device *pdev)
 
 	hpriv->stop_engine = ahci_mvebu_stop_engine;
 
-	rc = pdata->plat_config(hpriv);
+	rc = pdata->plat_config(pdev, hpriv);
 	if (rc)
 		goto disable_resources;
 
+	if (pdata->resource_flags & AHCI_PLATFORM_ARMADA8K_QUIRK)
+		hpriv->multi_irq_host_ack = multi_irq_host_ack_armada8k;
+
 	rc = ahci_platform_init_host(pdev, hpriv, &ahci_mvebu_port_info,
 				     &ahci_platform_sht);
 	if (rc)
@@ -237,6 +342,7 @@ static const struct ahci_mvebu_plat_data ahci_mvebu_armada_3700_plat_data = {
 
 static const struct ahci_mvebu_plat_data ahci_mvebu_armada_8k_plat_data = {
 	.plat_config = ahci_mvebu_armada_8k_config,
+	.resource_flags = AHCI_PLATFORM_ARMADA8K_QUIRK,
 };
 
 static const struct of_device_id ahci_mvebu_of_match[] = {
diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c
index f6f2a111d226..a5d06565a5c7 100644
--- a/drivers/ata/libahci_platform.c
+++ b/drivers/ata/libahci_platform.c
@@ -464,6 +464,9 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev,
 	if (!child_nodes)
 		hpriv->nports = 1;
 
+	if (!child_nodes && flags & AHCI_PLATFORM_ARMADA8K_QUIRK)
+		hpriv->nports = 2;
+
 	hpriv->phys = devm_kcalloc(dev, hpriv->nports, sizeof(*hpriv->phys), GFP_KERNEL);
 	if (!hpriv->phys) {
 		rc = -ENOMEM;
diff --git a/include/linux/ahci_platform.h b/include/linux/ahci_platform.h
index 49e5383d4222..99ff30b6c37c 100644
--- a/include/linux/ahci_platform.h
+++ b/include/linux/ahci_platform.h
@@ -42,5 +42,6 @@ int ahci_platform_suspend(struct device *dev);
 int ahci_platform_resume(struct device *dev);
 
 #define AHCI_PLATFORM_GET_RESETS	0x01
+#define AHCI_PLATFORM_ARMADA8K_QUIRK	0x02
 
 #endif /* _AHCI_PLATFORM_H */
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 6/9] irqchip/irq-mvebu-icu: Remove the double SATA ports interrupt hack
  2020-11-04 15:52 [PATCH v2 0/9] Armada8k enable per-port SATA interrupts and drop a hack in the IRQ subsystem sven.auhagen
                   ` (4 preceding siblings ...)
  2020-11-04 15:52 ` [PATCH v2 5/9] ata: ahci: mvebu: Add support for A8k legacy DT bindings sven.auhagen
@ 2020-11-04 15:52 ` sven.auhagen
  2020-11-04 15:52 ` [PATCH v2 7/9] dt-bindings: ata: Update ahci bindings with possible per-port interrupts sven.auhagen
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: sven.auhagen @ 2020-11-04 15:52 UTC (permalink / raw)
  To: axboe, hdegoede, robh+dt, tglx, maz, gregory.clement
  Cc: linux-ide, linux-arm-kernel, devicetree, jason, andrew, rjw,
	viresh.kumar, antoine.tenart, maxime.chevallier,
	thomas.petazzoni, miquel.raynal

From: Miquel Raynal <miquel.raynal@bootlin.com>

When writing the driver, a hack was introduced to configure both SATA
interrupts regardless of the port in use to overcome a limitation in
the SATA core. Now that this limitation has been addressed and the
hack moved in the (historically) responsible SATA driver,
ahci_{platform,mvebu}.c, let's clean this driver section.

Acked-by: Marc Zyngier <marc.zyngier@arm.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Sven Auhagen <sven.auhagen@voleatech.de>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/irqchip/irq-mvebu-icu.c | 18 ------------------
 1 file changed, 18 deletions(-)

diff --git a/drivers/irqchip/irq-mvebu-icu.c b/drivers/irqchip/irq-mvebu-icu.c
index 91adf771f185..3e29f8d5b33b 100644
--- a/drivers/irqchip/irq-mvebu-icu.c
+++ b/drivers/irqchip/irq-mvebu-icu.c
@@ -38,8 +38,6 @@
 
 /* ICU definitions */
 #define ICU_MAX_IRQS		207
-#define ICU_SATA0_ICU_ID	109
-#define ICU_SATA1_ICU_ID	107
 
 struct mvebu_icu_subset_data {
 	unsigned int icu_group;
@@ -111,22 +109,6 @@ static void mvebu_icu_write_msg(struct msi_desc *desc, struct msi_msg *msg)
 	}
 
 	writel_relaxed(icu_int, icu->base + ICU_INT_CFG(d->hwirq));
-
-	/*
-	 * The SATA unit has 2 ports, and a dedicated ICU entry per
-	 * port. The ahci sata driver supports only one irq interrupt
-	 * per SATA unit. To solve this conflict, we configure the 2
-	 * SATA wired interrupts in the south bridge into 1 GIC
-	 * interrupt in the north bridge. Even if only a single port
-	 * is enabled, if sata node is enabled, both interrupts are
-	 * configured (regardless of which port is actually in use).
-	 */
-	if (d->hwirq == ICU_SATA0_ICU_ID || d->hwirq == ICU_SATA1_ICU_ID) {
-		writel_relaxed(icu_int,
-			       icu->base + ICU_INT_CFG(ICU_SATA0_ICU_ID));
-		writel_relaxed(icu_int,
-			       icu->base + ICU_INT_CFG(ICU_SATA1_ICU_ID));
-	}
 }
 
 static struct irq_chip mvebu_icu_nsr_chip = {
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 7/9] dt-bindings: ata: Update ahci bindings with possible per-port interrupts
  2020-11-04 15:52 [PATCH v2 0/9] Armada8k enable per-port SATA interrupts and drop a hack in the IRQ subsystem sven.auhagen
                   ` (5 preceding siblings ...)
  2020-11-04 15:52 ` [PATCH v2 6/9] irqchip/irq-mvebu-icu: Remove the double SATA ports interrupt hack sven.auhagen
@ 2020-11-04 15:52 ` sven.auhagen
  2020-11-04 15:52 ` [PATCH v2 8/9] dt-bindings: ata: Update ahci_mvebu bindings sven.auhagen
  2020-11-04 15:52 ` [PATCH v2 9/9] arm64: dts: marvell: armada-cp110: Switch to per-port SATA interrupts sven.auhagen
  8 siblings, 0 replies; 14+ messages in thread
From: sven.auhagen @ 2020-11-04 15:52 UTC (permalink / raw)
  To: axboe, hdegoede, robh+dt, tglx, maz, gregory.clement
  Cc: linux-ide, linux-arm-kernel, devicetree, jason, andrew, rjw,
	viresh.kumar, antoine.tenart, maxime.chevallier,
	thomas.petazzoni, miquel.raynal

From: Miquel Raynal <miquel.raynal@bootlin.com>

Update bindings to reflect the fact that a SATA IP can either have:
- only one interrupt: in this case an 'interrupts' property is
  declared at the root of the node;
or
- each SATA port can have their own interrupt: in this case there is
  one 'interrupts' property per port/sub-node and none at the root.

Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Sven Auhagen <sven.auhagen@voleatech.de>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 Documentation/devicetree/bindings/ata/ahci-platform.txt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/ata/ahci-platform.txt b/Documentation/devicetree/bindings/ata/ahci-platform.txt
index 77091a277642..83d715cbcecd 100644
--- a/Documentation/devicetree/bindings/ata/ahci-platform.txt
+++ b/Documentation/devicetree/bindings/ata/ahci-platform.txt
@@ -49,6 +49,12 @@ And at least one of the following properties:
 - phys		    : reference to the SATA PHY node
 - target-supply     : regulator for SATA target power
 
+Sub-nodes optional properties:
+- interrupts        : <interrupt mapping for SATA ports IRQ>, please
+                      note that either the root SATA node has the
+                      interrupts property, or there is one per SATA
+                      port, but not both at the same time.
+
 Examples:
         sata@ffe08000 {
 		compatible = "snps,spear-ahci";
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 8/9] dt-bindings: ata: Update ahci_mvebu bindings
  2020-11-04 15:52 [PATCH v2 0/9] Armada8k enable per-port SATA interrupts and drop a hack in the IRQ subsystem sven.auhagen
                   ` (6 preceding siblings ...)
  2020-11-04 15:52 ` [PATCH v2 7/9] dt-bindings: ata: Update ahci bindings with possible per-port interrupts sven.auhagen
@ 2020-11-04 15:52 ` sven.auhagen
  2020-11-04 15:52 ` [PATCH v2 9/9] arm64: dts: marvell: armada-cp110: Switch to per-port SATA interrupts sven.auhagen
  8 siblings, 0 replies; 14+ messages in thread
From: sven.auhagen @ 2020-11-04 15:52 UTC (permalink / raw)
  To: axboe, hdegoede, robh+dt, tglx, maz, gregory.clement
  Cc: linux-ide, linux-arm-kernel, devicetree, jason, andrew, rjw,
	viresh.kumar, antoine.tenart, maxime.chevallier,
	thomas.petazzoni, miquel.raynal

From: Miquel Raynal <miquel.raynal@bootlin.com>

Update bindings with the already in use Armada 8k compatible.

Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Sven Auhagen <sven.auhagen@voleatech.de>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 Documentation/devicetree/bindings/ata/ahci-platform.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/ata/ahci-platform.txt b/Documentation/devicetree/bindings/ata/ahci-platform.txt
index 83d715cbcecd..78d9d413a5c6 100644
--- a/Documentation/devicetree/bindings/ata/ahci-platform.txt
+++ b/Documentation/devicetree/bindings/ata/ahci-platform.txt
@@ -15,6 +15,7 @@ Required properties:
   - "ibm,476gtr-ahci"
   - "marvell,armada-380-ahci"
   - "marvell,armada-3700-ahci"
+  - "marvell,armada-8k-ahci"
   - "snps,dwc-ahci"
   - "snps,spear-ahci"
   - "generic-ahci"
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 9/9] arm64: dts: marvell: armada-cp110: Switch to per-port SATA interrupts
  2020-11-04 15:52 [PATCH v2 0/9] Armada8k enable per-port SATA interrupts and drop a hack in the IRQ subsystem sven.auhagen
                   ` (7 preceding siblings ...)
  2020-11-04 15:52 ` [PATCH v2 8/9] dt-bindings: ata: Update ahci_mvebu bindings sven.auhagen
@ 2020-11-04 15:52 ` sven.auhagen
  8 siblings, 0 replies; 14+ messages in thread
From: sven.auhagen @ 2020-11-04 15:52 UTC (permalink / raw)
  To: axboe, hdegoede, robh+dt, tglx, maz, gregory.clement
  Cc: linux-ide, linux-arm-kernel, devicetree, jason, andrew, rjw,
	viresh.kumar, antoine.tenart, maxime.chevallier,
	thomas.petazzoni, miquel.raynal

From: Sven Auhagen <sven.auhagen@voleatech.de>

There are two SATA ports per CP110. Each of them has a dedicated
interrupt. Describe the real hardware by adding two SATA ports to the
CP110 SATA node.

Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Sven Auhagen <sven.auhagen@voleatech.de>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 arch/arm64/boot/dts/marvell/armada-cp11x.dtsi | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi b/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
index 9dcf16beabf5..ec27294f097b 100644
--- a/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
@@ -300,11 +300,9 @@
 		};
 
 		CP11X_LABEL(sata0): sata@540000 {
-			compatible = "marvell,armada-8k-ahci",
-			"generic-ahci";
+			compatible = "marvell,armada-8k-ahci";
 			reg = <0x540000 0x30000>;
 			dma-coherent;
-			interrupts = <107 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&CP11X_LABEL(clk) 1 15>,
 				 <&CP11X_LABEL(clk) 1 16>;
 			#address-cells = <1>;
@@ -312,10 +310,12 @@
 			status = "disabled";
 
 			sata-port@0 {
+				interrupts = <109 IRQ_TYPE_LEVEL_HIGH>;
 				reg = <0>;
 			};
 
 			sata-port@1 {
+				interrupts = <107 IRQ_TYPE_LEVEL_HIGH>;
 				reg = <1>;
 			};
 		};
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 4/9] ata: ahci: add ack callback to multi irq handler
  2020-11-04 15:52 ` [PATCH v2 4/9] ata: ahci: add ack callback to multi irq handler sven.auhagen
@ 2020-11-09 13:59   ` Hans de Goede
  0 siblings, 0 replies; 14+ messages in thread
From: Hans de Goede @ 2020-11-09 13:59 UTC (permalink / raw)
  To: sven.auhagen, axboe, robh+dt, tglx, maz, gregory.clement
  Cc: linux-ide, linux-arm-kernel, devicetree, jason, andrew, rjw,
	viresh.kumar, antoine.tenart, maxime.chevallier,
	thomas.petazzoni, miquel.raynal

Hi,

On 11/4/20 4:52 PM, sven.auhagen@voleatech.de wrote:
> From: Sven Auhagen <sven.auhagen@voleatech.de>
> 
> In order to support custom actions at the end of the irq handler
> a multi_irq_host_ack callback is added to the struct ahci_host_priv.
> 
> Suggested-by: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Sven Auhagen <sven.auhagen@voleatech.de>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans

> ---
>  drivers/ata/ahci.h    | 2 ++
>  drivers/ata/libahci.c | 4 ++++
>  2 files changed, 6 insertions(+)
> 
> diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
> index 98b8baa47dc5..d8109e06794c 100644
> --- a/drivers/ata/ahci.h
> +++ b/drivers/ata/ahci.h
> @@ -371,6 +371,8 @@ struct ahci_host_priv {
>  	/* only required for per-port MSI(-X) support */
>  	int			(*get_irq_vector)(struct ata_host *host,
>  						  int port);
> +
> +	int			(*multi_irq_host_ack)(int irq, struct ata_port *ap);
>  };
>  
>  extern int ahci_ignore_sss;
> diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
> index ea5bf5f4cbed..cf9839135a57 100644
> --- a/drivers/ata/libahci.c
> +++ b/drivers/ata/libahci.c
> @@ -1897,6 +1897,7 @@ static void ahci_port_intr(struct ata_port *ap)
>  static irqreturn_t ahci_multi_irqs_intr_hard(int irq, void *dev_instance)
>  {
>  	struct ata_port *ap = dev_instance;
> +	struct ahci_host_priv *hpriv = ap->host->private_data;
>  	void __iomem *port_mmio = ahci_port_base(ap);
>  	u32 status;
>  
> @@ -1909,6 +1910,9 @@ static irqreturn_t ahci_multi_irqs_intr_hard(int irq, void *dev_instance)
>  	ahci_handle_port_interrupt(ap, port_mmio, status);
>  	spin_unlock(ap->lock);
>  
> +	if (hpriv->multi_irq_host_ack)
> +		hpriv->multi_irq_host_ack(irq, ap);
> +
>  	VPRINTK("EXIT\n");
>  
>  	return IRQ_HANDLED;
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 5/9] ata: ahci: mvebu: Add support for A8k legacy DT bindings
  2020-11-04 15:52 ` [PATCH v2 5/9] ata: ahci: mvebu: Add support for A8k legacy DT bindings sven.auhagen
@ 2020-11-09 13:59   ` Hans de Goede
  0 siblings, 0 replies; 14+ messages in thread
From: Hans de Goede @ 2020-11-09 13:59 UTC (permalink / raw)
  To: sven.auhagen, axboe, robh+dt, tglx, maz, gregory.clement
  Cc: linux-ide, linux-arm-kernel, devicetree, jason, andrew, rjw,
	viresh.kumar, antoine.tenart, maxime.chevallier,
	thomas.petazzoni, miquel.raynal

Hi,

On 11/4/20 4:52 PM, sven.auhagen@voleatech.de wrote:
> From: Sven Auhagen <sven.auhagen@voleatech.de>
> 
> The CP110 SATA unit has 2 ports, and a dedicated ICU entry per
> port. In the past, the AHCI SATA driver only supported one interrupt
> per SATA unit. To solve this conflict, the 2 SATA wired interrupts in
> the South-Bridge got configured as 1 GIC interrupt in the
> North-Bridge, regardless of the number of SATA ports actually
> enabled/in use, and the DT bindings only referenced the interrupt of
> one port.
> 
> Since then, this limitation has been addressed and this patch ensures
> backward compatibility with old DTs not describing SATA ports
> correctly directly from the AHCI MVEBU driver. This way, we will be
> able to drop the hack from the ICU driver. IOW, when the A8k
> compatible string is used and there is no sub-nodes in the DT, we
> fake the creation and mapping of the second (missing) interrupt.
> 
> Signed-off-by: Sven Auhagen <sven.auhagen@voleatech.de>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

Much better, thank you.

The patch looks good to me now:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans

> ---
>  drivers/ata/ahci_mvebu.c       | 126 ++++++++++++++++++++++++++++++---
>  drivers/ata/libahci_platform.c |   3 +
>  include/linux/ahci_platform.h  |   1 +
>  3 files changed, 120 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/ata/ahci_mvebu.c b/drivers/ata/ahci_mvebu.c
> index ed82381dc4a7..22447b1bb23f 100644
> --- a/drivers/ata/ahci_mvebu.c
> +++ b/drivers/ata/ahci_mvebu.c
> @@ -16,6 +16,7 @@
>  #include <linux/mbus.h>
>  #include <linux/module.h>
>  #include <linux/of_device.h>
> +#include <linux/of_irq.h>
>  #include <linux/platform_device.h>
>  #include "ahci.h"
>  
> @@ -28,9 +29,15 @@
>  #define AHCI_WINDOW_BASE(win)	(0x64 + ((win) << 4))
>  #define AHCI_WINDOW_SIZE(win)	(0x68 + ((win) << 4))
>  
> +#define ICU_SATA0_ICU_ID 109
> +#define ICU_SATA1_ICU_ID 107
> +
>  struct ahci_mvebu_plat_data {
> -	int (*plat_config)(struct ahci_host_priv *hpriv);
> +	int (*plat_config)(struct platform_device *pdev,
> +				   struct ahci_host_priv *hpriv);
>  	unsigned int host_flags;
> +	unsigned int resource_flags;
> +	unsigned int port_irq[2];
>  };
>  
>  static void ahci_mvebu_mbus_config(struct ahci_host_priv *hpriv,
> @@ -67,7 +74,8 @@ static void ahci_mvebu_regret_option(struct ahci_host_priv *hpriv)
>  	writel(0x80, hpriv->mmio + AHCI_VENDOR_SPECIFIC_0_DATA);
>  }
>  
> -static int ahci_mvebu_armada_380_config(struct ahci_host_priv *hpriv)
> +static int ahci_mvebu_armada_380_config(struct platform_device *pdev,
> +				   struct ahci_host_priv *hpriv)
>  {
>  	const struct mbus_dram_target_info *dram;
>  	int rc = 0;
> @@ -83,7 +91,8 @@ static int ahci_mvebu_armada_380_config(struct ahci_host_priv *hpriv)
>  	return rc;
>  }
>  
> -static int ahci_mvebu_armada_3700_config(struct ahci_host_priv *hpriv)
> +static int ahci_mvebu_armada_3700_config(struct platform_device *pdev,
> +				   struct ahci_host_priv *hpriv)
>  {
>  	u32 reg;
>  
> @@ -96,8 +105,94 @@ static int ahci_mvebu_armada_3700_config(struct ahci_host_priv *hpriv)
>  	return 0;
>  }
>  
> -static int ahci_mvebu_armada_8k_config(struct ahci_host_priv *hpriv)
> +static int multi_irq_host_ack_armada8k(int irq, struct ata_port *ap)
>  {
> +	struct ahci_host_priv *hpriv = ap->host->private_data;
> +	void __iomem *mmio = hpriv->mmio;
> +
> +	writel(BIT(ap->port_no), mmio + HOST_IRQ_STAT);
> +
> +	return 0;
> +}
> +
> +static int ahci_get_per_port_irq_armada8k(struct ata_host *host, int port)
> +{
> +	struct ahci_host_priv *hpriv = host->private_data;
> +	struct ahci_mvebu_plat_data *pdata = hpriv->plat_data;
> +
> +	return pdata->port_irq[port];
> +}
> +
> +static int ahci_mvebu_armada_8k_irq_backwards(struct ahci_host_priv *hpriv,
> +				       struct device *dev)
> +{
> +	struct device_node *np = of_irq_find_parent(dev->of_node);
> +	struct ahci_mvebu_plat_data *pdata = hpriv->plat_data;
> +	struct irq_data *irqd = irq_get_irq_data(pdata->port_irq[0]);
> +	int host_irq = irqd ? irqd_to_hwirq(irqd) : 0;
> +	int missing_irq = (host_irq == ICU_SATA1_ICU_ID) ?
> +		ICU_SATA0_ICU_ID : ICU_SATA1_ICU_ID;
> +	struct irq_fwspec fwspec = {
> +		.fwnode = of_node_to_fwnode(np),
> +		.param_count = 2,
> +		.param = {missing_irq, IRQ_TYPE_LEVEL_HIGH},
> +	};
> +	int irq;
> +
> +	pdata->port_irq[1] = irq_create_fwspec_mapping(&fwspec);
> +	hpriv->mask_port_map = GENMASK(1, 0);
> +
> +	if (missing_irq == ICU_SATA0_ICU_ID) {
> +		irq = pdata->port_irq[0];
> +		pdata->port_irq[0] = pdata->port_irq[1];
> +		pdata->port_irq[1] = irq;
> +	}
> +
> +	return 0;
> +}
> +
> +static int ahci_mvebu_armada_8k_config(struct platform_device *pdev,
> +				   struct ahci_host_priv *hpriv)
> +{
> +	struct ahci_mvebu_plat_data *pdata = hpriv->plat_data;
> +	struct device *dev = &pdev->dev;
> +	struct device_node *child;
> +	int port_irq, child_nodes, port = 0;
> +
> +	/* Get IRQs per port */
> +	child_nodes = of_get_child_count(dev->of_node);
> +	if (child_nodes) {
> +		for_each_child_of_node(dev->of_node, child) {
> +
> +			port_irq = of_irq_get(child, 0);
> +			if (!port_irq)
> +				port_irq = -EINVAL;
> +
> +			if (port_irq < 0)
> +				goto compat;
> +
> +			pdata->port_irq[port] = port_irq;
> +			port++;
> +		}
> +
> +		goto out;
> +	}
> +
> +compat:
> +	/* Backwards Compatibility Check */
> +	port_irq = platform_get_irq(pdev, 0);
> +	if (port_irq > 0) {
> +		pdata->port_irq[0] = port_irq;
> +		ahci_mvebu_armada_8k_irq_backwards(hpriv, dev);
> +	} else {
> +		dev_err(dev, "no irq\n");
> +		return port_irq;
> +	}
> +
> +out:
> +	hpriv->flags |= AHCI_HFLAG_MULTI_MSI;
> +	hpriv->get_irq_vector = ahci_get_per_port_irq_armada8k;
> +
>  	return 0;
>  }
>  
> @@ -167,7 +262,7 @@ static int ahci_mvebu_resume(struct platform_device *pdev)
>  	struct ahci_host_priv *hpriv = host->private_data;
>  	const struct ahci_mvebu_plat_data *pdata = hpriv->plat_data;
>  
> -	pdata->plat_config(hpriv);
> +	pdata->plat_config(pdev, hpriv);
>  
>  	return ahci_platform_resume_host(&pdev->dev);
>  }
> @@ -189,15 +284,22 @@ static struct scsi_host_template ahci_platform_sht = {
>  
>  static int ahci_mvebu_probe(struct platform_device *pdev)
>  {
> -	const struct ahci_mvebu_plat_data *pdata;
> +	const struct ahci_mvebu_plat_data *pdata_plat;
> +	struct ahci_mvebu_plat_data *pdata;
>  	struct ahci_host_priv *hpriv;
>  	int rc;
>  
> -	pdata = of_device_get_match_data(&pdev->dev);
> -	if (!pdata)
> +	pdata_plat = of_device_get_match_data(&pdev->dev);
> +	if (!pdata_plat)
>  		return -EINVAL;
>  
> -	hpriv = ahci_platform_get_resources(pdev, 0);
> +	pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
> +	if (!pdata)
> +		return -ENOMEM;
> +
> +	memcpy(pdata, pdata_plat, sizeof(*pdata));
> +
> +	hpriv = ahci_platform_get_resources(pdev, pdata->resource_flags);
>  	if (IS_ERR(hpriv))
>  		return PTR_ERR(hpriv);
>  
> @@ -210,10 +312,13 @@ static int ahci_mvebu_probe(struct platform_device *pdev)
>  
>  	hpriv->stop_engine = ahci_mvebu_stop_engine;
>  
> -	rc = pdata->plat_config(hpriv);
> +	rc = pdata->plat_config(pdev, hpriv);
>  	if (rc)
>  		goto disable_resources;
>  
> +	if (pdata->resource_flags & AHCI_PLATFORM_ARMADA8K_QUIRK)
> +		hpriv->multi_irq_host_ack = multi_irq_host_ack_armada8k;
> +
>  	rc = ahci_platform_init_host(pdev, hpriv, &ahci_mvebu_port_info,
>  				     &ahci_platform_sht);
>  	if (rc)
> @@ -237,6 +342,7 @@ static const struct ahci_mvebu_plat_data ahci_mvebu_armada_3700_plat_data = {
>  
>  static const struct ahci_mvebu_plat_data ahci_mvebu_armada_8k_plat_data = {
>  	.plat_config = ahci_mvebu_armada_8k_config,
> +	.resource_flags = AHCI_PLATFORM_ARMADA8K_QUIRK,
>  };
>  
>  static const struct of_device_id ahci_mvebu_of_match[] = {
> diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c
> index f6f2a111d226..a5d06565a5c7 100644
> --- a/drivers/ata/libahci_platform.c
> +++ b/drivers/ata/libahci_platform.c
> @@ -464,6 +464,9 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev,
>  	if (!child_nodes)
>  		hpriv->nports = 1;
>  
> +	if (!child_nodes && flags & AHCI_PLATFORM_ARMADA8K_QUIRK)
> +		hpriv->nports = 2;
> +
>  	hpriv->phys = devm_kcalloc(dev, hpriv->nports, sizeof(*hpriv->phys), GFP_KERNEL);
>  	if (!hpriv->phys) {
>  		rc = -ENOMEM;
> diff --git a/include/linux/ahci_platform.h b/include/linux/ahci_platform.h
> index 49e5383d4222..99ff30b6c37c 100644
> --- a/include/linux/ahci_platform.h
> +++ b/include/linux/ahci_platform.h
> @@ -42,5 +42,6 @@ int ahci_platform_suspend(struct device *dev);
>  int ahci_platform_resume(struct device *dev);
>  
>  #define AHCI_PLATFORM_GET_RESETS	0x01
> +#define AHCI_PLATFORM_ARMADA8K_QUIRK	0x02
>  
>  #endif /* _AHCI_PLATFORM_H */
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 3/9] ata: ahci: custom irq init for host init
  2020-11-04 15:52 ` [PATCH v2 3/9] ata: ahci: custom irq init for host init sven.auhagen
@ 2020-11-09 14:00   ` Hans de Goede
  2020-11-09 17:25     ` Sven Auhagen
  0 siblings, 1 reply; 14+ messages in thread
From: Hans de Goede @ 2020-11-09 14:00 UTC (permalink / raw)
  To: sven.auhagen, axboe, robh+dt, tglx, maz, gregory.clement
  Cc: linux-ide, linux-arm-kernel, devicetree, jason, andrew, rjw,
	viresh.kumar, antoine.tenart, maxime.chevallier,
	thomas.petazzoni, miquel.raynal

Hi,

On 11/4/20 4:52 PM, sven.auhagen@voleatech.de wrote:
> From: Sven Auhagen <sven.auhagen@voleatech.de>
> 
> Disable the platform irq init in ahci init platform host
> if it was initiated by a custom function.
> To check for it I am using the AHCI_HFLAG_MULTI_MSI flag.
> 
> Suggested-by: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Sven Auhagen <sven.auhagen@voleatech.de>

The code is fine here. But I'm not entirely happy with
the commit message how about:

"""
ata: libahci_platform: Do not try to get an IRQ when AHCI_HFLAG_MULTI_MSI is set

When the ahci-host AHCI_HFLAG_MULTI_MSI flag is set then the driver must provide
a get_irq_vector callback and take care of getting the IRQs itself. So in this
case ahci_platform_init_host() should not try to get an IRQ itself.
"""

With the commit message updated to the above (or something similar) you
may add my:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

I've also just given you my Reviewed-by for patch 4 and 5, and the
result already has a Reviewed-by.

So if you can sendout a v3 of this series with the commit message for
this patch fixed, then it is ready for merging from my pov.

Regards,

Hans


> ---
>  drivers/ata/libahci_platform.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c
> index de638dafce21..f6f2a111d226 100644
> --- a/drivers/ata/libahci_platform.c
> +++ b/drivers/ata/libahci_platform.c
> @@ -581,14 +581,16 @@ int ahci_platform_init_host(struct platform_device *pdev,
>  	struct ata_host *host;
>  	int i, irq, n_ports, rc;
>  
> -	irq = platform_get_irq(pdev, 0);
> -	if (irq <= 0) {
> -		if (irq != -EPROBE_DEFER)
> -			dev_err(dev, "no irq\n");
> -		return irq;
> -	}
> +	if (!(hpriv->flags & AHCI_HFLAG_MULTI_MSI)) {
> +		irq = platform_get_irq(pdev, 0);
> +		if (irq <= 0) {
> +			if (irq != -EPROBE_DEFER)
> +				dev_err(dev, "no irq\n");
> +			return irq;
> +		}
>  
> -	hpriv->irq = irq;
> +		hpriv->irq = irq;
> +	}
>  
>  	/* prepare host */
>  	pi.private_data = (void *)(unsigned long)hpriv->flags;
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 3/9] ata: ahci: custom irq init for host init
  2020-11-09 14:00   ` Hans de Goede
@ 2020-11-09 17:25     ` Sven Auhagen
  0 siblings, 0 replies; 14+ messages in thread
From: Sven Auhagen @ 2020-11-09 17:25 UTC (permalink / raw)
  To: Hans de Goede
  Cc: axboe, robh+dt, tglx, maz, gregory.clement, linux-ide,
	linux-arm-kernel, devicetree, jason, andrew, rjw, viresh.kumar,
	antoine.tenart, maxime.chevallier, thomas.petazzoni,
	miquel.raynal

On Mon, Nov 09, 2020 at 03:00:58PM +0100, Hans de Goede wrote:
> Hi,
> 
> On 11/4/20 4:52 PM, sven.auhagen@voleatech.de wrote:
> > From: Sven Auhagen <sven.auhagen@voleatech.de>
> > 
> > Disable the platform irq init in ahci init platform host
> > if it was initiated by a custom function.
> > To check for it I am using the AHCI_HFLAG_MULTI_MSI flag.
> > 
> > Suggested-by: Hans de Goede <hdegoede@redhat.com>
> > Signed-off-by: Sven Auhagen <sven.auhagen@voleatech.de>
> 
> The code is fine here. But I'm not entirely happy with
> the commit message how about:
> 
> """
> ata: libahci_platform: Do not try to get an IRQ when AHCI_HFLAG_MULTI_MSI is set
> 
> When the ahci-host AHCI_HFLAG_MULTI_MSI flag is set then the driver must provide
> a get_irq_vector callback and take care of getting the IRQs itself. So in this
> case ahci_platform_init_host() should not try to get an IRQ itself.
> """
> 
> With the commit message updated to the above (or something similar) you
> may add my:
> 
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> 
> I've also just given you my Reviewed-by for patch 4 and 5, and the
> result already has a Reviewed-by.
> 
> So if you can sendout a v3 of this series with the commit message for
> this patch fixed, then it is ready for merging from my pov.

Thank you, I will change the commit message and send a v3.

Best
Sven

> 
> Regards,
> 
> Hans
> 
> 
> > ---
> >  drivers/ata/libahci_platform.c | 16 +++++++++-------
> >  1 file changed, 9 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c
> > index de638dafce21..f6f2a111d226 100644
> > --- a/drivers/ata/libahci_platform.c
> > +++ b/drivers/ata/libahci_platform.c
> > @@ -581,14 +581,16 @@ int ahci_platform_init_host(struct platform_device *pdev,
> >  	struct ata_host *host;
> >  	int i, irq, n_ports, rc;
> >  
> > -	irq = platform_get_irq(pdev, 0);
> > -	if (irq <= 0) {
> > -		if (irq != -EPROBE_DEFER)
> > -			dev_err(dev, "no irq\n");
> > -		return irq;
> > -	}
> > +	if (!(hpriv->flags & AHCI_HFLAG_MULTI_MSI)) {
> > +		irq = platform_get_irq(pdev, 0);
> > +		if (irq <= 0) {
> > +			if (irq != -EPROBE_DEFER)
> > +				dev_err(dev, "no irq\n");
> > +			return irq;
> > +		}
> >  
> > -	hpriv->irq = irq;
> > +		hpriv->irq = irq;
> > +	}
> >  
> >  	/* prepare host */
> >  	pi.private_data = (void *)(unsigned long)hpriv->flags;
> > 
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2020-11-09 17:25 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-04 15:52 [PATCH v2 0/9] Armada8k enable per-port SATA interrupts and drop a hack in the IRQ subsystem sven.auhagen
2020-11-04 15:52 ` [PATCH v2 1/9] ata: ahci: mvebu: Rename a platform data flag sven.auhagen
2020-11-04 15:52 ` [PATCH v2 2/9] ata: ahci: mvebu: Support A8k compatible sven.auhagen
2020-11-04 15:52 ` [PATCH v2 3/9] ata: ahci: custom irq init for host init sven.auhagen
2020-11-09 14:00   ` Hans de Goede
2020-11-09 17:25     ` Sven Auhagen
2020-11-04 15:52 ` [PATCH v2 4/9] ata: ahci: add ack callback to multi irq handler sven.auhagen
2020-11-09 13:59   ` Hans de Goede
2020-11-04 15:52 ` [PATCH v2 5/9] ata: ahci: mvebu: Add support for A8k legacy DT bindings sven.auhagen
2020-11-09 13:59   ` Hans de Goede
2020-11-04 15:52 ` [PATCH v2 6/9] irqchip/irq-mvebu-icu: Remove the double SATA ports interrupt hack sven.auhagen
2020-11-04 15:52 ` [PATCH v2 7/9] dt-bindings: ata: Update ahci bindings with possible per-port interrupts sven.auhagen
2020-11-04 15:52 ` [PATCH v2 8/9] dt-bindings: ata: Update ahci_mvebu bindings sven.auhagen
2020-11-04 15:52 ` [PATCH v2 9/9] arm64: dts: marvell: armada-cp110: Switch to per-port SATA interrupts sven.auhagen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).