[PATCH 0/5] Update and rewrite IXP4xx PATA driver

[PATCH 0/5] Update and rewrite IXP4xx PATA driver

[Linus Walleij]

This rewrites the IXP4xx PATA driver to use device tree exclusively.

It is too much trouble to triangulate over using board files and
DT at the same time, let's just replace it with device tree as
we delete all board files at the same time as we merge this.

I'd appreciate an ACK from the PATA maintainer so we can take this
through the ARM SoC tree when we delete the boardfiles at the same
time.

Linus Walleij (5):
  pata: ixp4xx: Use local dev variable
  pata: ixp4xx: Use IS_ENABLED() to determine endianness
  pata: ixp4xx: Refer to cmd and ctl rather than csN
  pata: ixp4xx: Add DT bindings
  pata: ixp4xx: Rewrite to use device tree

 .../ata/intel,ixp4xx-compact-flash.yaml       |  62 ++++
 drivers/ata/pata_ixp4xx_cf.c                  | 264 ++++++++++++------
 include/linux/platform_data/pata_ixp4xx_cf.h  |   4 +-
 3 files changed, 249 insertions(+), 81 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/ata/intel,ixp4xx-compact-flash.yaml

-- 
2.31.1

[PATCH 1/5] pata: ixp4xx: Use local dev variable

[Linus Walleij]

Let's simplify all &pdev->dev references by creating a
local struct device *dev variable.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/ata/pata_ixp4xx_cf.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/ata/pata_ixp4xx_cf.c b/drivers/ata/pata_ixp4xx_cf.c
index 5881d64af943..23f0f7cacd52 100644
--- a/drivers/ata/pata_ixp4xx_cf.c
+++ b/drivers/ata/pata_ixp4xx_cf.c
@@ -139,6 +139,7 @@ static int ixp4xx_pata_probe(struct platform_device *pdev)
 	struct resource *cs0, *cs1;
 	struct ata_host *host;
 	struct ata_port *ap;
+	struct device *dev = &pdev->dev;
 	struct ixp4xx_pata_data *data = dev_get_platdata(&pdev->dev);
 	int ret;
 	int irq;
@@ -150,17 +151,17 @@ static int ixp4xx_pata_probe(struct platform_device *pdev)
 		return -EINVAL;
 
 	/* allocate host */
-	host = ata_host_alloc(&pdev->dev, 1);
+	host = ata_host_alloc(dev, 1);
 	if (!host)
 		return -ENOMEM;
 
 	/* acquire resources and fill host */
-	ret = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32));
+	ret = dma_set_coherent_mask(dev, DMA_BIT_MASK(32));
 	if (ret)
 		return ret;
 
-	data->cs0 = devm_ioremap(&pdev->dev, cs0->start, 0x1000);
-	data->cs1 = devm_ioremap(&pdev->dev, cs1->start, 0x1000);
+	data->cs0 = devm_ioremap(dev, cs0->start, 0x1000);
+	data->cs1 = devm_ioremap(dev, cs1->start, 0x1000);
 
 	if (!data->cs0 || !data->cs1)
 		return -ENOMEM;
@@ -185,7 +186,7 @@ static int ixp4xx_pata_probe(struct platform_device *pdev)
 
 	ixp4xx_setup_port(ap, data, cs0->start, cs1->start);
 
-	ata_print_version_once(&pdev->dev, DRV_VERSION);
+	ata_print_version_once(dev, DRV_VERSION);
 
 	/* activate host */
 	return ata_host_activate(host, irq, ata_sff_interrupt, 0, &ixp4xx_sht);
-- 
2.31.1

[PATCH 2/5] pata: ixp4xx: Use IS_ENABLED() to determine endianness

[Linus Walleij]

Instead of an ARM-specific ifdef, use the global CPU config
and if (IS_ENABLED()).

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/ata/pata_ixp4xx_cf.c | 45 ++++++++++++++++++------------------
 1 file changed, 22 insertions(+), 23 deletions(-)

diff --git a/drivers/ata/pata_ixp4xx_cf.c b/drivers/ata/pata_ixp4xx_cf.c
index 23f0f7cacd52..bc5029d6525d 100644
--- a/drivers/ata/pata_ixp4xx_cf.c
+++ b/drivers/ata/pata_ixp4xx_cf.c
@@ -107,29 +107,28 @@ static void ixp4xx_setup_port(struct ata_port *ap,
 
 	ata_sff_std_ports(ioaddr);
 
-#ifndef __ARMEB__
-
-	/* adjust the addresses to handle the address swizzling of the
-	 * ixp4xx in little endian mode.
-	 */
-
-	*(unsigned long *)&ioaddr->data_addr		^= 0x02;
-	*(unsigned long *)&ioaddr->cmd_addr		^= 0x03;
-	*(unsigned long *)&ioaddr->altstatus_addr	^= 0x03;
-	*(unsigned long *)&ioaddr->ctl_addr		^= 0x03;
-	*(unsigned long *)&ioaddr->error_addr		^= 0x03;
-	*(unsigned long *)&ioaddr->feature_addr		^= 0x03;
-	*(unsigned long *)&ioaddr->nsect_addr		^= 0x03;
-	*(unsigned long *)&ioaddr->lbal_addr		^= 0x03;
-	*(unsigned long *)&ioaddr->lbam_addr		^= 0x03;
-	*(unsigned long *)&ioaddr->lbah_addr		^= 0x03;
-	*(unsigned long *)&ioaddr->device_addr		^= 0x03;
-	*(unsigned long *)&ioaddr->status_addr		^= 0x03;
-	*(unsigned long *)&ioaddr->command_addr		^= 0x03;
-
-	raw_cmd ^= 0x03;
-	raw_ctl ^= 0x03;
-#endif
+	if (!IS_ENABLED(CONFIG_CPU_BIG_ENDIAN)) {
+		/* adjust the addresses to handle the address swizzling of the
+		 * ixp4xx in little endian mode.
+		 */
+
+		*(unsigned long *)&ioaddr->data_addr		^= 0x02;
+		*(unsigned long *)&ioaddr->cmd_addr		^= 0x03;
+		*(unsigned long *)&ioaddr->altstatus_addr	^= 0x03;
+		*(unsigned long *)&ioaddr->ctl_addr		^= 0x03;
+		*(unsigned long *)&ioaddr->error_addr		^= 0x03;
+		*(unsigned long *)&ioaddr->feature_addr		^= 0x03;
+		*(unsigned long *)&ioaddr->nsect_addr		^= 0x03;
+		*(unsigned long *)&ioaddr->lbal_addr		^= 0x03;
+		*(unsigned long *)&ioaddr->lbam_addr		^= 0x03;
+		*(unsigned long *)&ioaddr->lbah_addr		^= 0x03;
+		*(unsigned long *)&ioaddr->device_addr		^= 0x03;
+		*(unsigned long *)&ioaddr->status_addr		^= 0x03;
+		*(unsigned long *)&ioaddr->command_addr		^= 0x03;
+
+		raw_cmd ^= 0x03;
+		raw_ctl ^= 0x03;
+	}
 
 	ata_port_desc(ap, "cmd 0x%lx ctl 0x%lx", raw_cmd, raw_ctl);
 }
-- 
2.31.1

[PATCH 3/5] pata: ixp4xx: Refer to cmd and ctl rather than csN

[Linus Walleij]

The two "cs0" and "cs1" are "chip selects" but on some
platforms such as GW2358 they are actually both in CS3
making this terminology very confusing. Call the
addresses "cmd" and "ctl" after function instead.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/ata/pata_ixp4xx_cf.c                 | 27 ++++++++++----------
 include/linux/platform_data/pata_ixp4xx_cf.h |  4 +--
 2 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/drivers/ata/pata_ixp4xx_cf.c b/drivers/ata/pata_ixp4xx_cf.c
index bc5029d6525d..72d6d6f2ef99 100644
--- a/drivers/ata/pata_ixp4xx_cf.c
+++ b/drivers/ata/pata_ixp4xx_cf.c
@@ -95,15 +95,14 @@ static struct ata_port_operations ixp4xx_port_ops = {
 
 static void ixp4xx_setup_port(struct ata_port *ap,
 			      struct ixp4xx_pata_data *data,
-			      unsigned long raw_cs0, unsigned long raw_cs1)
+			      unsigned long raw_cmd, unsigned long raw_ctl)
 {
 	struct ata_ioports *ioaddr = &ap->ioaddr;
-	unsigned long raw_cmd = raw_cs0;
-	unsigned long raw_ctl = raw_cs1 + 0x06;
 
-	ioaddr->cmd_addr	= data->cs0;
-	ioaddr->altstatus_addr	= data->cs1 + 0x06;
-	ioaddr->ctl_addr	= data->cs1 + 0x06;
+	raw_ctl += 0x06;
+	ioaddr->cmd_addr	= data->cmd;
+	ioaddr->altstatus_addr	= data->ctl + 0x06;
+	ioaddr->ctl_addr	= data->ctl + 0x06;
 
 	ata_sff_std_ports(ioaddr);
 
@@ -135,7 +134,7 @@ static void ixp4xx_setup_port(struct ata_port *ap,
 
 static int ixp4xx_pata_probe(struct platform_device *pdev)
 {
-	struct resource *cs0, *cs1;
+	struct resource *cmd, *ctl;
 	struct ata_host *host;
 	struct ata_port *ap;
 	struct device *dev = &pdev->dev;
@@ -143,10 +142,10 @@ static int ixp4xx_pata_probe(struct platform_device *pdev)
 	int ret;
 	int irq;
 
-	cs0 = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	cs1 = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	cmd = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	ctl = platform_get_resource(pdev, IORESOURCE_MEM, 1);
 
-	if (!cs0 || !cs1)
+	if (!cmd || !ctl)
 		return -EINVAL;
 
 	/* allocate host */
@@ -159,10 +158,10 @@ static int ixp4xx_pata_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	data->cs0 = devm_ioremap(dev, cs0->start, 0x1000);
-	data->cs1 = devm_ioremap(dev, cs1->start, 0x1000);
+	data->cmd = devm_ioremap(dev, cmd->start, 0x1000);
+	data->ctl = devm_ioremap(dev, ctl->start, 0x1000);
 
-	if (!data->cs0 || !data->cs1)
+	if (!data->cmd || !data->ctl)
 		return -ENOMEM;
 
 	irq = platform_get_irq(pdev, 0);
@@ -183,7 +182,7 @@ static int ixp4xx_pata_probe(struct platform_device *pdev)
 	ap->pio_mask = ATA_PIO4;
 	ap->flags |= ATA_FLAG_NO_ATAPI;
 
-	ixp4xx_setup_port(ap, data, cs0->start, cs1->start);
+	ixp4xx_setup_port(ap, data, cmd->start, ctl->start);
 
 	ata_print_version_once(dev, DRV_VERSION);
 
diff --git a/include/linux/platform_data/pata_ixp4xx_cf.h b/include/linux/platform_data/pata_ixp4xx_cf.h
index 601ba97fef57..e60fa41da4a5 100644
--- a/include/linux/platform_data/pata_ixp4xx_cf.h
+++ b/include/linux/platform_data/pata_ixp4xx_cf.h
@@ -14,8 +14,8 @@ struct ixp4xx_pata_data {
 	volatile u32	*cs1_cfg;
 	unsigned long	cs0_bits;
 	unsigned long	cs1_bits;
-	void __iomem	*cs0;
-	void __iomem	*cs1;
+	void __iomem	*cmd;
+	void __iomem	*ctl;
 };
 
 #endif
-- 
2.31.1

[PATCH 4/5] pata: ixp4xx: Add DT bindings

[Linus Walleij]

This adds device tree bindings for the Intel IXP4xx compact flash card
interface.

Cc: devicetree@vger.kernel.org
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 .../ata/intel,ixp4xx-compact-flash.yaml       | 62 +++++++++++++++++++
 1 file changed, 62 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/ata/intel,ixp4xx-compact-flash.yaml

diff --git a/Documentation/devicetree/bindings/ata/intel,ixp4xx-compact-flash.yaml b/Documentation/devicetree/bindings/ata/intel,ixp4xx-compact-flash.yaml
new file mode 100644
index 000000000000..b254be858de6
--- /dev/null
+++ b/Documentation/devicetree/bindings/ata/intel,ixp4xx-compact-flash.yaml
@@ -0,0 +1,62 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/ata/intel,ixp4xx-compact-flash.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Intel IXP4xx CompactFlash Card Controller
+
+maintainers:
+  - Linus Walleij <linus.walleij@linaro.org>
+
+description: |
+  The IXP4xx network processors have a CompactFlash interface that presents
+  a CompactFlash card to the system as a true IDE (parallell ATA) device. The
+  device is always connected to the expansion bus of the IXP4xx SoCs using one
+  or two chip select areas and address translating logic on the board. The
+  node must be placed inside a chip select node on the IXP4xx expansion bus.
+
+properties:
+  compatible:
+    const: intel,ixp4xx-compact-flash
+
+  reg:
+    minItems: 2
+    maxItems: 2
+
+  interrupts:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+allOf:
+  - $ref: pata-common.yaml#
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    bus@c4000000 {
+      compatible = "intel,ixp43x-expansion-bus-controller", "syscon";
+      reg = <0xc4000000 0x1000>;
+      native-endian;
+      #address-cells = <2>;
+      #size-cells = <1>;
+      ranges = <0 0x0 0x50000000 0x01000000>, <1 0x0 0x51000000 0x01000000>;
+      dma-ranges = <0 0x0 0x50000000 0x01000000>, <1 0x0 0x51000000 0x01000000>;
+      ide@1,0 {
+        compatible = "intel,ixp4xx-compact-flash";
+        intel,ixp4xx-eb-t1 = <3>;
+        intel,ixp4xx-eb-t2 = <3>;
+        reg = <1 0x00000000 0x1000>, <1 0x00040000 0x1000>;
+        interrupt-parent = <&gpio0>;
+        interrupts = <12 IRQ_TYPE_EDGE_RISING>;
+      };
+    };
+
+...
-- 
2.31.1

Re: [PATCH 4/5] pata: ixp4xx: Add DT bindings

[Rob Herring]

On Wed, Jul 28, 2021 at 11:02:41AM +0200, Linus Walleij wrote:
> This adds device tree bindings for the Intel IXP4xx compact flash card
> interface.
> 
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  .../ata/intel,ixp4xx-compact-flash.yaml       | 62 +++++++++++++++++++
>  1 file changed, 62 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/ata/intel,ixp4xx-compact-flash.yaml
> 
> diff --git a/Documentation/devicetree/bindings/ata/intel,ixp4xx-compact-flash.yaml b/Documentation/devicetree/bindings/ata/intel,ixp4xx-compact-flash.yaml
> new file mode 100644
> index 000000000000..b254be858de6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/ata/intel,ixp4xx-compact-flash.yaml
> @@ -0,0 +1,62 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/ata/intel,ixp4xx-compact-flash.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Intel IXP4xx CompactFlash Card Controller
> +
> +maintainers:
> +  - Linus Walleij <linus.walleij@linaro.org>
> +
> +description: |
> +  The IXP4xx network processors have a CompactFlash interface that presents
> +  a CompactFlash card to the system as a true IDE (parallell ATA) device. The

typo. (you can setup checkpatch.pl to do spell checking)

> +  device is always connected to the expansion bus of the IXP4xx SoCs using one
> +  or two chip select areas and address translating logic on the board. The
> +  node must be placed inside a chip select node on the IXP4xx expansion bus.
> +
> +properties:
> +  compatible:
> +    const: intel,ixp4xx-compact-flash
> +
> +  reg:
> +    minItems: 2
> +    maxItems: 2

What is each entry?

> +
> +  interrupts:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +
> +allOf:
> +  - $ref: pata-common.yaml#
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    bus@c4000000 {
> +      compatible = "intel,ixp43x-expansion-bus-controller", "syscon";
> +      reg = <0xc4000000 0x1000>;
> +      native-endian;
> +      #address-cells = <2>;
> +      #size-cells = <1>;
> +      ranges = <0 0x0 0x50000000 0x01000000>, <1 0x0 0x51000000 0x01000000>;
> +      dma-ranges = <0 0x0 0x50000000 0x01000000>, <1 0x0 0x51000000 0x01000000>;
> +      ide@1,0 {
> +        compatible = "intel,ixp4xx-compact-flash";

> +        intel,ixp4xx-eb-t1 = <3>;
> +        intel,ixp4xx-eb-t2 = <3>;

Can you omit these? Otherwise, this will start failing soon as 
unevaluatedProperties is implemented (I have a test branch). Or you need 
to handle child node properties as discussed.

> +        reg = <1 0x00000000 0x1000>, <1 0x00040000 0x1000>;
> +        interrupt-parent = <&gpio0>;
> +        interrupts = <12 IRQ_TYPE_EDGE_RISING>;
> +      };
> +    };
> +
> +...
> -- 
> 2.31.1
> 
> 

[PATCH 5/5] pata: ixp4xx: Rewrite to use device tree

[Linus Walleij]

This rewrites the IXP4xx CF (IDE) libata driver to use the
device tree exclusively to look up its resources:

- Probe exclusively from the device tree and look up all
  resources from there.
- Allocate a local state container with devres and pass
  this around in .private_data.
- Initialize with struct ata_port_info.
- Use the .set_piomode() callback instead of the much
  wider .set_mode(), we only support PIO after all.
- Bump driver version number from 0.2 to 1.0 to reflect this
  wider change.
- Get a handle on the expansion bus syscon regmap to alter
  the timings on the chip select.
- Put in the more elaborate timing adjustment code for PIO0
  to PIO4 in 8 and 16bit mode from the downstream OpenWrt
  patch.

The board file initialization path and platform data include
is dropped because the board files will be deleted at the same
time as this patch is merged.

The platform data file is not deleted right now so as not to
conflict with the removal of board files.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/ata/pata_ixp4xx_cf.c | 201 +++++++++++++++++++++++++++--------
 1 file changed, 154 insertions(+), 47 deletions(-)

diff --git a/drivers/ata/pata_ixp4xx_cf.c b/drivers/ata/pata_ixp4xx_cf.c
index 72d6d6f2ef99..99c63087c8ae 100644
--- a/drivers/ata/pata_ixp4xx_cf.c
+++ b/drivers/ata/pata_ixp4xx_cf.c
@@ -13,45 +13,134 @@
  */
 
 #include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
 #include <linux/module.h>
 #include <linux/libata.h>
 #include <linux/irq.h>
 #include <linux/platform_device.h>
-#include <linux/platform_data/pata_ixp4xx_cf.h>
+#include <linux/regmap.h>
 #include <scsi/scsi_host.h>
 
 #define DRV_NAME	"pata_ixp4xx_cf"
-#define DRV_VERSION	"0.2"
+#define DRV_VERSION	"1.0"
 
-static int ixp4xx_set_mode(struct ata_link *link, struct ata_device **error)
+struct ixp4xx_pata {
+	struct ata_host *host;
+	struct regmap *rmap;
+	u32 cmd_csreg;
+	void __iomem *cmd;
+	void __iomem *ctl;
+};
+
+#define IXP4XX_EXP_TIMING_STRIDE	0x04
+/* The timings for the chipselect is in bits 29..16 */
+#define IXP4XX_EXP_T1_T5_MASK	GENMASK(29, 16)
+#define IXP4XX_EXP_PIO_0_8	0x0a470000
+#define IXP4XX_EXP_PIO_1_8	0x06430000
+#define IXP4XX_EXP_PIO_2_8	0x02410000
+#define IXP4XX_EXP_PIO_3_8	0x00820000
+#define IXP4XX_EXP_PIO_4_8	0x00400000
+#define IXP4XX_EXP_PIO_0_16	0x29640000
+#define IXP4XX_EXP_PIO_1_16	0x05030000
+#define IXP4XX_EXP_PIO_2_16	0x00b20000
+#define IXP4XX_EXP_PIO_3_16	0x00820000
+#define IXP4XX_EXP_PIO_4_16	0x00400000
+#define IXP4XX_EXP_BW_MASK	(BIT(6)|BIT(0))
+#define IXP4XX_EXP_BYTE_RD16	BIT(6) /* Byte reads on half-word devices */
+#define IXP4XX_EXP_BYTE_EN	BIT(0) /* Use 8bit data bus if set */
+
+static void ixp4xx_set_8bit_timing(struct ixp4xx_pata *ixpp, u8 pio_mode)
+{
+	switch (pio_mode) {
+	case XFER_PIO_0:
+		regmap_update_bits(ixpp->rmap, ixpp->cmd_csreg,
+				   IXP4XX_EXP_T1_T5_MASK, IXP4XX_EXP_PIO_0_8);
+		break;
+	case XFER_PIO_1:
+		regmap_update_bits(ixpp->rmap, ixpp->cmd_csreg,
+				   IXP4XX_EXP_T1_T5_MASK, IXP4XX_EXP_PIO_1_8);
+		break;
+	case XFER_PIO_2:
+		regmap_update_bits(ixpp->rmap, ixpp->cmd_csreg,
+				   IXP4XX_EXP_T1_T5_MASK, IXP4XX_EXP_PIO_2_8);
+		break;
+	case XFER_PIO_3:
+		regmap_update_bits(ixpp->rmap, ixpp->cmd_csreg,
+				   IXP4XX_EXP_T1_T5_MASK, IXP4XX_EXP_PIO_3_8);
+		break;
+	case XFER_PIO_4:
+		regmap_update_bits(ixpp->rmap, ixpp->cmd_csreg,
+				   IXP4XX_EXP_T1_T5_MASK, IXP4XX_EXP_PIO_4_8);
+		break;
+	default:
+		break;
+	}
+	regmap_update_bits(ixpp->rmap, ixpp->cmd_csreg,
+			   IXP4XX_EXP_BW_MASK, IXP4XX_EXP_BYTE_RD16|IXP4XX_EXP_BYTE_EN);
+}
+
+static void ixp4xx_set_16bit_timing(struct ixp4xx_pata *ixpp, u8 pio_mode)
 {
-	struct ata_device *dev;
-
-	ata_for_each_dev(dev, link, ENABLED) {
-		ata_dev_info(dev, "configured for PIO0\n");
-		dev->pio_mode = XFER_PIO_0;
-		dev->xfer_mode = XFER_PIO_0;
-		dev->xfer_shift = ATA_SHIFT_PIO;
-		dev->flags |= ATA_DFLAG_PIO;
+	switch (pio_mode){
+	case XFER_PIO_0:
+		regmap_update_bits(ixpp->rmap, ixpp->cmd_csreg,
+				   IXP4XX_EXP_T1_T5_MASK, IXP4XX_EXP_PIO_0_16);
+		break;
+	case XFER_PIO_1:
+		regmap_update_bits(ixpp->rmap, ixpp->cmd_csreg,
+				   IXP4XX_EXP_T1_T5_MASK, IXP4XX_EXP_PIO_1_16);
+		break;
+	case XFER_PIO_2:
+		regmap_update_bits(ixpp->rmap, ixpp->cmd_csreg,
+				   IXP4XX_EXP_T1_T5_MASK, IXP4XX_EXP_PIO_2_16);
+		break;
+	case XFER_PIO_3:
+		regmap_update_bits(ixpp->rmap, ixpp->cmd_csreg,
+				   IXP4XX_EXP_T1_T5_MASK, IXP4XX_EXP_PIO_3_16);
+		break;
+	case XFER_PIO_4:
+		regmap_update_bits(ixpp->rmap, ixpp->cmd_csreg,
+				   IXP4XX_EXP_T1_T5_MASK, IXP4XX_EXP_PIO_4_16);
+		break;
+	default:
+		break;
 	}
-	return 0;
+	regmap_update_bits(ixpp->rmap, ixpp->cmd_csreg,
+			   IXP4XX_EXP_BW_MASK, IXP4XX_EXP_BYTE_RD16);
 }
 
+/* This sets up the timing on the chipselect CMD accordingly */
+static void ixp4xx_set_piomode(struct ata_port *ap, struct ata_device *adev)
+{
+	struct ixp4xx_pata *ixpp = ap->host->private_data;
+
+	ata_dev_printk(adev, KERN_INFO, "configured for PIO%d 8bit\n",
+		       adev->pio_mode - XFER_PIO_0);
+	ixp4xx_set_8bit_timing(ixpp, adev->pio_mode);
+}
+
+
 static unsigned int ixp4xx_mmio_data_xfer(struct ata_queued_cmd *qc,
-				unsigned char *buf, unsigned int buflen, int rw)
+					  unsigned char *buf, unsigned int buflen, int rw)
 {
 	unsigned int i;
 	unsigned int words = buflen >> 1;
 	u16 *buf16 = (u16 *) buf;
+	struct ata_device *adev = qc->dev;
 	struct ata_port *ap = qc->dev->link->ap;
 	void __iomem *mmio = ap->ioaddr.data_addr;
-	struct ixp4xx_pata_data *data = dev_get_platdata(ap->host->dev);
+	struct ixp4xx_pata *ixpp = ap->host->private_data;
+	unsigned long flags;
+
+	ata_dev_printk(adev, KERN_DEBUG, "%s %d bytes\n", (rw == READ) ? "READ" : "WRITE",
+		       buflen);
+	spin_lock_irqsave(ap->lock, flags);
 
 	/* set the expansion bus in 16bit mode and restore
 	 * 8 bit mode after the transaction.
 	 */
-	*data->cs0_cfg &= ~(0x01);
-	udelay(100);
+	ixp4xx_set_16bit_timing(ixpp, adev->pio_mode);
+	udelay(5);
 
 	/* Transfer multiple of 2 bytes */
 	if (rw == READ)
@@ -76,8 +165,10 @@ static unsigned int ixp4xx_mmio_data_xfer(struct ata_queued_cmd *qc,
 		words++;
 	}
 
-	udelay(100);
-	*data->cs0_cfg |= 0x01;
+	ixp4xx_set_8bit_timing(ixpp, adev->pio_mode);
+	udelay(5);
+
+	spin_unlock_irqrestore(ap->lock, flags);
 
 	return words << 1;
 }
@@ -90,19 +181,25 @@ static struct ata_port_operations ixp4xx_port_ops = {
 	.inherits		= &ata_sff_port_ops,
 	.sff_data_xfer		= ixp4xx_mmio_data_xfer,
 	.cable_detect		= ata_cable_40wire,
-	.set_mode		= ixp4xx_set_mode,
+	.set_piomode		= ixp4xx_set_piomode,
+};
+
+static struct ata_port_info ixp4xx_port_info = {
+	.flags		= ATA_FLAG_NO_ATAPI,
+	.pio_mask	= ATA_PIO4,
+	.port_ops	= &ixp4xx_port_ops,
 };
 
 static void ixp4xx_setup_port(struct ata_port *ap,
-			      struct ixp4xx_pata_data *data,
+			      struct ixp4xx_pata *ixpp,
 			      unsigned long raw_cmd, unsigned long raw_ctl)
 {
 	struct ata_ioports *ioaddr = &ap->ioaddr;
 
 	raw_ctl += 0x06;
-	ioaddr->cmd_addr	= data->cmd;
-	ioaddr->altstatus_addr	= data->ctl + 0x06;
-	ioaddr->ctl_addr	= data->ctl + 0x06;
+	ioaddr->cmd_addr	= ixpp->cmd;
+	ioaddr->altstatus_addr	= ixpp->ctl + 0x06;
+	ioaddr->ctl_addr	= ixpp->ctl + 0x06;
 
 	ata_sff_std_ports(ioaddr);
 
@@ -135,10 +232,12 @@ static void ixp4xx_setup_port(struct ata_port *ap,
 static int ixp4xx_pata_probe(struct platform_device *pdev)
 {
 	struct resource *cmd, *ctl;
-	struct ata_host *host;
-	struct ata_port *ap;
+	struct ata_port_info pi = ixp4xx_port_info;
+	const struct ata_port_info *ppi[] = { &pi, NULL };
 	struct device *dev = &pdev->dev;
-	struct ixp4xx_pata_data *data = dev_get_platdata(&pdev->dev);
+	struct device_node *np = dev->of_node;
+	struct ixp4xx_pata *ixpp;
+	u32 csindex;
 	int ret;
 	int irq;
 
@@ -148,20 +247,32 @@ static int ixp4xx_pata_probe(struct platform_device *pdev)
 	if (!cmd || !ctl)
 		return -EINVAL;
 
-	/* allocate host */
-	host = ata_host_alloc(dev, 1);
-	if (!host)
+	ixpp = devm_kzalloc(dev, sizeof(*ixpp), GFP_KERNEL);
+	if (!ixpp)
+		return -ENOMEM;
+
+	ixpp->rmap = syscon_node_to_regmap(np->parent);
+	if (IS_ERR(ixpp->rmap))
+		return dev_err_probe(dev, PTR_ERR(ixpp->rmap), "no regmap\n");
+	/* Inspect our address to figure out what chipselect the CMD is on */
+	ret = of_property_read_u32_index(np, "reg", 0, &csindex);
+	if (ret)
+		return dev_err_probe(dev, ret, "can't inspect CMD address\n");
+	dev_info(dev, "using CS%d for PIO timing configuration\n", csindex);
+	ixpp->cmd_csreg = csindex * IXP4XX_EXP_TIMING_STRIDE;
+
+	ixpp->host = ata_host_alloc_pinfo(dev, ppi, 1);
+	if (!ixpp->host)
 		return -ENOMEM;
+	ixpp->host->private_data = ixpp;
 
-	/* acquire resources and fill host */
 	ret = dma_set_coherent_mask(dev, DMA_BIT_MASK(32));
 	if (ret)
 		return ret;
 
-	data->cmd = devm_ioremap(dev, cmd->start, 0x1000);
-	data->ctl = devm_ioremap(dev, ctl->start, 0x1000);
-
-	if (!data->cmd || !data->ctl)
+	ixpp->cmd = devm_ioremap_resource(dev, cmd);
+	ixpp->ctl = devm_ioremap_resource(dev, ctl);
+	if (IS_ERR(ixpp->cmd) || IS_ERR(ixpp->ctl))
 		return -ENOMEM;
 
 	irq = platform_get_irq(pdev, 0);
@@ -172,27 +283,23 @@ static int ixp4xx_pata_probe(struct platform_device *pdev)
 	else
 		return -EINVAL;
 
-	/* Setup expansion bus chip selects */
-	*data->cs0_cfg = data->cs0_bits;
-	*data->cs1_cfg = data->cs1_bits;
-
-	ap = host->ports[0];
-
-	ap->ops	= &ixp4xx_port_ops;
-	ap->pio_mask = ATA_PIO4;
-	ap->flags |= ATA_FLAG_NO_ATAPI;
-
-	ixp4xx_setup_port(ap, data, cmd->start, ctl->start);
+	/* Just one port to set up */
+	ixp4xx_setup_port(ixpp->host->ports[0], ixpp, cmd->start, ctl->start);
 
 	ata_print_version_once(dev, DRV_VERSION);
 
-	/* activate host */
-	return ata_host_activate(host, irq, ata_sff_interrupt, 0, &ixp4xx_sht);
+	return ata_host_activate(ixpp->host, irq, ata_sff_interrupt, 0, &ixp4xx_sht);
 }
 
+static const struct of_device_id ixp4xx_pata_of_match[] = {
+	{ .compatible = "intel,ixp4xx-compact-flash", },
+	{ },
+};
+
 static struct platform_driver ixp4xx_pata_platform_driver = {
 	.driver	 = {
 		.name   = DRV_NAME,
+		.of_match_table = ixp4xx_pata_of_match,
 	},
 	.probe		= ixp4xx_pata_probe,
 	.remove		= ata_platform_remove_one,
-- 
2.31.1