[PATCH] ide: Add tx4938ide driver (v2)

[PATCH] ide: Add tx4938ide driver (v2)

[Atsushi Nemoto]

This is the driver for the Toshiba TX4938 SoC EBUS controller ATA mode.
It has custom set_pio_mode and some hacks for big endian.

Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
---
Changes since v1:
* fix wait-cycle calculation.
* get the base address from platform_device resource.
* request and ioremap whole CS0/1 regions.
* some cosmetic changes.

 drivers/ide/Kconfig          |    5 +
 drivers/ide/mips/Makefile    |    1 +
 drivers/ide/mips/tx4938ide.c |  310 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 316 insertions(+), 0 deletions(-)
 create mode 100644 drivers/ide/mips/tx4938ide.c

diff --git a/drivers/ide/Kconfig b/drivers/ide/Kconfig
index 1f0eeba..decafcf 100644
--- a/drivers/ide/Kconfig
+++ b/drivers/ide/Kconfig
@@ -746,6 +746,11 @@ config BLK_DEV_IDE_AU1XXX_SEQTS_PER_RQ
        default "128"
        depends on BLK_DEV_IDE_AU1XXX
 
+config BLK_DEV_IDE_TX4938
+	tristate "TX4938 internal IDE support"
+	depends on SOC_TX4938
+	select IDE_TIMINGS
+
 config BLK_DEV_IDE_TX4939
 	tristate "TX4939 internal IDE support"
 	depends on SOC_TX4939
diff --git a/drivers/ide/mips/Makefile b/drivers/ide/mips/Makefile
index 04e5d86..7b217e6 100644
--- a/drivers/ide/mips/Makefile
+++ b/drivers/ide/mips/Makefile
@@ -1,4 +1,5 @@
 obj-$(CONFIG_BLK_DEV_IDE_AU1XXX)	+= au1xxx-ide.o
 
+obj-$(CONFIG_BLK_DEV_IDE_TX4938)	+= tx4938ide.o
 obj-$(CONFIG_BLK_DEV_IDE_TX4939)	+= tx4939ide.o
 EXTRA_CFLAGS    := -Idrivers/ide
diff --git a/drivers/ide/mips/tx4938ide.c b/drivers/ide/mips/tx4938ide.c
new file mode 100644
index 0000000..fa660f9
--- /dev/null
+++ b/drivers/ide/mips/tx4938ide.c
@@ -0,0 +1,310 @@
+/*
+ * TX4938 internal IDE driver
+ * Based on tx4939ide.c.
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License.  See the file "COPYING" in the main directory of this archive
+ * for more details.
+ *
+ * (C) Copyright TOSHIBA CORPORATION 2005-2007
+ */
+
+#include <linux/module.h>
+#include <linux/types.h>
+#include <linux/ide.h>
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/io.h>
+#include <asm/txx9/tx4938.h>
+
+static void tx4938ide_tune_ebusc(unsigned int ebus_ch,
+				 unsigned int gbus_clock,
+				 u8 pio)
+{
+	struct ide_timing *t = ide_timing_find_mode(XFER_PIO_0 + pio);
+	u64 cr = __raw_readq(&tx4938_ebuscptr->cr[ebus_ch]);
+	unsigned int sp = (cr >> 4) & 3;
+	unsigned int clock = gbus_clock / (4 - sp);
+	unsigned int cycle = 1000000000 / clock;
+	unsigned int wt, shwt;
+
+	/* Minimum DIOx- active time */
+	wt = DIV_ROUND_UP(t->act8b, cycle) - 2;
+	/* IORDY setup time: 35ns */
+	wt = max(wt, DIV_ROUND_UP(35, cycle));
+	/* actual wait-cycle is max(wt & ~1, 1) */
+	if (wt > 2 && (wt & 1))
+		wt++;
+	wt &= ~1;
+	/* Address-valid to DIOR/DIOW setup */
+	shwt = DIV_ROUND_UP(t->setup, cycle);
+
+	pr_debug("tx4938ide: ebus %d, bus cycle %dns, WT %d, SHWT %d\n",
+		 ebus_ch, cycle, wt, shwt);
+
+	__raw_writeq((cr & ~(0x3f007ull)) | (wt << 12) | shwt,
+		     &tx4938_ebuscptr->cr[ebus_ch]);
+}
+
+static void tx4938ide_set_pio_mode(ide_drive_t *drive, const u8 pio)
+{
+	ide_hwif_t *hwif = drive->hwif;
+	struct tx4938ide_platform_info *pdata = hwif->dev->platform_data;
+	u8 safe = pio;
+	ide_drive_t *pair;
+
+	pair = ide_get_pair_dev(drive);
+	if (pair)
+		safe = min(safe, ide_get_best_pio_mode(pair, 255, 5));
+	tx4938ide_tune_ebusc(pdata->ebus_ch, pdata->gbus_clock, safe);
+}
+
+#ifdef __BIG_ENDIAN
+
+/* custom iops (independent from SWAP_IO_SPACE) */
+static u8 tx4938ide_inb(unsigned long port)
+{
+	return __raw_readb((void __iomem *)port);
+}
+
+static void tx4938ide_outb(u8 value, unsigned long port)
+{
+	__raw_writeb(value, (void __iomem *)port);
+}
+
+static void tx4938ide_tf_load(ide_drive_t *drive, ide_task_t *task)
+{
+	ide_hwif_t *hwif = drive->hwif;
+	struct ide_io_ports *io_ports = &hwif->io_ports;
+	struct ide_taskfile *tf = &task->tf;
+	u8 HIHI = task->tf_flags & IDE_TFLAG_LBA48 ? 0xE0 : 0xEF;
+
+	if (task->tf_flags & IDE_TFLAG_FLAGGED)
+		HIHI = 0xFF;
+
+	if (task->tf_flags & IDE_TFLAG_OUT_DATA) {
+		u16 data = (tf->hob_data << 8) | tf->data;
+
+		/* no endian swap */
+		__raw_writew(data, (void __iomem *)io_ports->data_addr);
+	}
+
+	if (task->tf_flags & IDE_TFLAG_OUT_HOB_FEATURE)
+		tx4938ide_outb(tf->hob_feature, io_ports->feature_addr);
+	if (task->tf_flags & IDE_TFLAG_OUT_HOB_NSECT)
+		tx4938ide_outb(tf->hob_nsect, io_ports->nsect_addr);
+	if (task->tf_flags & IDE_TFLAG_OUT_HOB_LBAL)
+		tx4938ide_outb(tf->hob_lbal, io_ports->lbal_addr);
+	if (task->tf_flags & IDE_TFLAG_OUT_HOB_LBAM)
+		tx4938ide_outb(tf->hob_lbam, io_ports->lbam_addr);
+	if (task->tf_flags & IDE_TFLAG_OUT_HOB_LBAH)
+		tx4938ide_outb(tf->hob_lbah, io_ports->lbah_addr);
+
+	if (task->tf_flags & IDE_TFLAG_OUT_FEATURE)
+		tx4938ide_outb(tf->feature, io_ports->feature_addr);
+	if (task->tf_flags & IDE_TFLAG_OUT_NSECT)
+		tx4938ide_outb(tf->nsect, io_ports->nsect_addr);
+	if (task->tf_flags & IDE_TFLAG_OUT_LBAL)
+		tx4938ide_outb(tf->lbal, io_ports->lbal_addr);
+	if (task->tf_flags & IDE_TFLAG_OUT_LBAM)
+		tx4938ide_outb(tf->lbam, io_ports->lbam_addr);
+	if (task->tf_flags & IDE_TFLAG_OUT_LBAH)
+		tx4938ide_outb(tf->lbah, io_ports->lbah_addr);
+
+	if (task->tf_flags & IDE_TFLAG_OUT_DEVICE)
+		tx4938ide_outb((tf->device & HIHI) | drive->select,
+			       io_ports->device_addr);
+}
+
+static void tx4938ide_tf_read(ide_drive_t *drive, ide_task_t *task)
+{
+	ide_hwif_t *hwif = drive->hwif;
+	struct ide_io_ports *io_ports = &hwif->io_ports;
+	struct ide_taskfile *tf = &task->tf;
+
+	if (task->tf_flags & IDE_TFLAG_IN_DATA) {
+		u16 data;
+
+		/* no endian swap */
+		data = __raw_readw((void __iomem *)io_ports->data_addr);
+		tf->data = data & 0xff;
+		tf->hob_data = (data >> 8) & 0xff;
+	}
+
+	/* be sure we're looking at the low order bits */
+	tx4938ide_outb(ATA_DEVCTL_OBS & ~0x80, io_ports->ctl_addr);
+
+	if (task->tf_flags & IDE_TFLAG_IN_FEATURE)
+		tf->feature = tx4938ide_inb(io_ports->feature_addr);
+	if (task->tf_flags & IDE_TFLAG_IN_NSECT)
+		tf->nsect  = tx4938ide_inb(io_ports->nsect_addr);
+	if (task->tf_flags & IDE_TFLAG_IN_LBAL)
+		tf->lbal   = tx4938ide_inb(io_ports->lbal_addr);
+	if (task->tf_flags & IDE_TFLAG_IN_LBAM)
+		tf->lbam   = tx4938ide_inb(io_ports->lbam_addr);
+	if (task->tf_flags & IDE_TFLAG_IN_LBAH)
+		tf->lbah   = tx4938ide_inb(io_ports->lbah_addr);
+	if (task->tf_flags & IDE_TFLAG_IN_DEVICE)
+		tf->device = tx4938ide_inb(io_ports->device_addr);
+
+	if (task->tf_flags & IDE_TFLAG_LBA48) {
+		tx4938ide_outb(ATA_DEVCTL_OBS | 0x80, io_ports->ctl_addr);
+
+		if (task->tf_flags & IDE_TFLAG_IN_HOB_FEATURE)
+			tf->hob_feature =
+				tx4938ide_inb(io_ports->feature_addr);
+		if (task->tf_flags & IDE_TFLAG_IN_HOB_NSECT)
+			tf->hob_nsect   = tx4938ide_inb(io_ports->nsect_addr);
+		if (task->tf_flags & IDE_TFLAG_IN_HOB_LBAL)
+			tf->hob_lbal    = tx4938ide_inb(io_ports->lbal_addr);
+		if (task->tf_flags & IDE_TFLAG_IN_HOB_LBAM)
+			tf->hob_lbam    = tx4938ide_inb(io_ports->lbam_addr);
+		if (task->tf_flags & IDE_TFLAG_IN_HOB_LBAH)
+			tf->hob_lbah    = tx4938ide_inb(io_ports->lbah_addr);
+	}
+}
+
+static void tx4938ide_input_data_swap(ide_drive_t *drive, struct request *rq,
+				void *buf, unsigned int len)
+{
+	unsigned long port = drive->hwif->io_ports.data_addr;
+	unsigned short *ptr = buf;
+	unsigned int count = (len + 1) / 2;
+
+	while (count--)
+		*ptr++ = cpu_to_le16(__raw_readw((void __iomem *)port));
+	__ide_flush_dcache_range((unsigned long)buf, count * 2);
+}
+
+static void tx4938ide_output_data_swap(ide_drive_t *drive, struct request *rq,
+				void *buf, unsigned int len)
+{
+	unsigned long port = drive->hwif->io_ports.data_addr;
+	unsigned short *ptr = buf;
+	unsigned int count = (len + 1) / 2;
+
+	while (count--) {
+		__raw_writew(le16_to_cpu(*ptr), (void __iomem *)port);
+		ptr++;
+	}
+	__ide_flush_dcache_range((unsigned long)buf, count * 2);
+}
+
+static const struct ide_tp_ops tx4938ide_tp_ops = {
+	.exec_command		= ide_exec_command,
+	.read_status		= ide_read_status,
+	.read_altstatus		= ide_read_altstatus,
+	.read_sff_dma_status	= ide_read_sff_dma_status,
+
+	.set_irq		= ide_set_irq,
+
+	.tf_load		= tx4938ide_tf_load,
+	.tf_read		= tx4938ide_tf_read,
+
+	.input_data		= tx4938ide_input_data_swap,
+	.output_data		= tx4938ide_output_data_swap,
+};
+
+#endif	/* __BIG_ENDIAN */
+
+static const struct ide_port_ops tx4938ide_port_ops = {
+	.set_pio_mode = tx4938ide_set_pio_mode,
+};
+
+static const struct ide_port_info tx4938ide_port_info __initdata = {
+	.port_ops = &tx4938ide_port_ops,
+#ifdef __BIG_ENDIAN
+	.tp_ops = &tx4938ide_tp_ops,
+#endif
+	.host_flags = IDE_HFLAG_MMIO | IDE_HFLAG_NO_DMA,
+	.pio_mask = ATA_PIO5,
+};
+
+static int __init tx4938ide_probe(struct platform_device *pdev)
+{
+	hw_regs_t hw;
+	hw_regs_t *hws[] = { &hw, NULL, NULL, NULL };
+	struct ide_host *host;
+	struct resource *res;
+	struct tx4938ide_platform_info *pdata = pdev->dev.platform_data;
+	int irq, ret, i;
+	unsigned long mapbase;
+	struct ide_port_info d = tx4938ide_port_info;
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0)
+		return -ENODEV;
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res)
+		return -ENODEV;
+
+	if (!devm_request_mem_region(&pdev->dev, res->start,
+				     res->end - res->start + 1, "tx4938ide"))
+		return -EBUSY;
+	mapbase = (unsigned long)devm_ioremap(&pdev->dev, res->start,
+					      res->end - res->start + 1);
+	if (!mapbase)
+		return -EBUSY;
+
+	memset(&hw, 0, sizeof(hw));
+	if (pdata->ioport_shift) {
+		unsigned long port = mapbase;
+
+		hw.io_ports_array[0] = port;
+#ifdef __BIG_ENDIAN
+		port++;
+#endif
+		for (i = 1; i <= 7; i++)
+			hw.io_ports_array[i] =
+				port + (i << pdata->ioport_shift);
+		hw.io_ports.ctl_addr =
+			port + 0x10000 + (6 << pdata->ioport_shift);
+	} else
+		ide_std_init_ports(&hw, mapbase, mapbase + 0x10006);
+	hw.irq = irq;
+	hw.dev = &pdev->dev;
+
+	pr_info("TX4938 IDE interface (base %#lx, irq %d)\n", mapbase, hw.irq);
+	if (pdata->gbus_clock)
+		tx4938ide_tune_ebusc(pdata->ebus_ch, pdata->gbus_clock, 0);
+	else
+		d.port_ops = NULL;
+	ret = ide_host_add(&d, hws, &host);
+	if (ret)
+		return ret;
+	platform_set_drvdata(pdev, host);
+	return 0;
+}
+
+static int __exit tx4938ide_remove(struct platform_device *pdev)
+{
+	struct ide_host *host = platform_get_drvdata(pdev);
+
+	ide_host_remove(host);
+	return 0;
+}
+
+static struct platform_driver tx4938ide_driver = {
+	.driver		= {
+		.name	= "tx4938ide",
+		.owner	= THIS_MODULE,
+	},
+	.remove = __exit_p(tx4938ide_remove),
+};
+
+static int __init tx4938ide_init(void)
+{
+	return platform_driver_probe(&tx4938ide_driver, tx4938ide_probe);
+}
+
+static void __exit tx4938ide_exit(void)
+{
+	platform_driver_unregister(&tx4938ide_driver);
+}
+
+module_init(tx4938ide_init);
+module_exit(tx4938ide_exit);
+
+MODULE_DESCRIPTION("TX4938 internal IDE driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:tx4938ide");
-- 
1.5.6.3

Re: [PATCH] ide: Add tx4938ide driver (v2)

[Sergei Shtylyov]

Hello.

Atsushi Nemoto wrote:

> This is the driver for the Toshiba TX4938 SoC EBUS controller ATA mode.
> It has custom set_pio_mode and some hacks for big endian.

> Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>

> diff --git a/drivers/ide/mips/tx4938ide.c b/drivers/ide/mips/tx4938ide.c
> new file mode 100644
> index 0000000..fa660f9
> --- /dev/null
> +++ b/drivers/ide/mips/tx4938ide.c
> @@ -0,0 +1,310 @@
> +/*
> + * TX4938 internal IDE driver
> + * Based on tx4939ide.c.
> + *
> + * This file is subject to the terms and conditions of the GNU General Public
> + * License.  See the file "COPYING" in the main directory of this archive
> + * for more details.
> + *
> + * (C) Copyright TOSHIBA CORPORATION 2005-2007
> + */
> +
> +#include <linux/module.h>
> +#include <linux/types.h>
> +#include <linux/ide.h>
> +#include <linux/init.h>
> +#include <linux/platform_device.h>
> +#include <linux/io.h>
> +#include <asm/txx9/tx4938.h>
> +
> +static void tx4938ide_tune_ebusc(unsigned int ebus_ch,
> +				 unsigned int gbus_clock,
> +				 u8 pio)
> +{
> +	struct ide_timing *t = ide_timing_find_mode(XFER_PIO_0 + pio);
> +	u64 cr = __raw_readq(&tx4938_ebuscptr->cr[ebus_ch]);
> +	unsigned int sp = (cr >> 4) & 3;
> +	unsigned int clock = gbus_clock / (4 - sp);
> +	unsigned int cycle = 1000000000 / clock;

    Hm, couldn't all these values be calculated only once?

> +	unsigned int wt, shwt;
> +
> +	/* Minimum DIOx- active time */
> +	wt = DIV_ROUND_UP(t->act8b, cycle) - 2;
> +	/* IORDY setup time: 35ns */
> +	wt = max(wt, DIV_ROUND_UP(35, cycle));

    Same comment about calculating only once...

> +	/* actual wait-cycle is max(wt & ~1, 1) */
> +	if (wt > 2 && (wt & 1))
> +		wt++;
> +	wt &= ~1;
> +	/* Address-valid to DIOR/DIOW setup */

    It's really address valid to -CSx assertion and -CSx to -DIOx assertion
setup time, and contrarywise, -DIOx to -CSx release and -CSx release to 
address invalid hold time, so it actualy applies 4 times and so constitutes 
-DIOx recovery time. It's worth to check if the minimum cycle time is reached 
with the setup time -- for PIO mode 0, minimum setup is 70 ns, multiplying 
that by 4 gives 280 ns recovery and adding 290 ns active time gives 570 ns 
cycle while the minimum is 600 ns.  Luckily, PIO0 seems the only problematic 
mode as I doubt that EBUS controller can do back-to-back IDE reads/writes 
keeping address and/or -CSx asserted in-between amounting to recovery time 
being only 2x/3x setup times -- in the worst, 2x case PIO mode 3 would also 
have too little cycle/recovery time...

> +	shwt = DIV_ROUND_UP(t->setup, cycle);
> +
> +	pr_debug("tx4938ide: ebus %d, bus cycle %dns, WT %d, SHWT %d\n",
> +		 ebus_ch, cycle, wt, shwt);
> +
> +	__raw_writeq((cr & ~(0x3f007ull)) | (wt << 12) | shwt,
> +		     &tx4938_ebuscptr->cr[ebus_ch]);

    Unneeded parens around the numeric constant.

> +static int __init tx4938ide_probe(struct platform_device *pdev)
> +{
[...]
> +	mapbase = (unsigned long)devm_ioremap(&pdev->dev, res->start,
> +					      res->end - res->start + 1);

    Calling devm_ioremap() on the whole 128 KB region wasn't such a great idea 
perhaps...

> +	ret = ide_host_add(&d, hws, &host);
> +	if (ret)
> +		return ret;
> +	platform_set_drvdata(pdev, host);
> +	return 0;

    I'd suggest shorter:

	if (!ret)
		platform_set_drvdata(pdev, host);
	return ret;

MBR, Sergei

Re: [PATCH] ide: Add tx4938ide driver (v2)

[Atsushi Nemoto]

On Thu, 23 Oct 2008 21:38:48 +0400, Sergei Shtylyov <sshtylyov@ru.mvista.com> wrote:
> > +	unsigned int sp = (cr >> 4) & 3;
> > +	unsigned int clock = gbus_clock / (4 - sp);
> > +	unsigned int cycle = 1000000000 / clock;
> 
>     Hm, couldn't all these values be calculated only once?
> 
> > +	unsigned int wt, shwt;
> > +
> > +	/* Minimum DIOx- active time */
> > +	wt = DIV_ROUND_UP(t->act8b, cycle) - 2;
> > +	/* IORDY setup time: 35ns */
> > +	wt = max(wt, DIV_ROUND_UP(35, cycle));
> 
>     Same comment about calculating only once...

Yes.  But are there any good place to hold calculated values?  I don't
think it is not worth to allocate a local structure just for holding
such values.  And this is not a critical path anyway :-)

> > +	/* actual wait-cycle is max(wt & ~1, 1) */
> > +	if (wt > 2 && (wt & 1))
> > +		wt++;
> > +	wt &= ~1;
> > +	/* Address-valid to DIOR/DIOW setup */
> 
>     It's really address valid to -CSx assertion and -CSx to -DIOx assertion
> setup time, and contrarywise, -DIOx to -CSx release and -CSx release to 
> address invalid hold time, so it actualy applies 4 times and so constitutes 
> -DIOx recovery time. It's worth to check if the minimum cycle time is reached 
> with the setup time -- for PIO mode 0, minimum setup is 70 ns, multiplying 
> that by 4 gives 280 ns recovery and adding 290 ns active time gives 570 ns 
> cycle while the minimum is 600 ns.  Luckily, PIO0 seems the only problematic 
> mode as I doubt that EBUS controller can do back-to-back IDE reads/writes 
> keeping address and/or -CSx asserted in-between amounting to recovery time 
> being only 2x/3x setup times -- in the worst, 2x case PIO mode 3 would also 
> have too little cycle/recovery time...

Oh I had not considered the total cycle time at all...  I agree with
your analysis that PIO0 is the only problematic case.

And I also noticed that the calculated shwt value can be begger than 7
(which is max SHWT value) on fast GBUS and slow drive.  I will add
some check for it.


I'll address all other issues too (and send incremental patch).
Thanks!

---
Atsushi Nemoto

Re: [PATCH] ide: Add tx4938ide driver (v2)

[Atsushi Nemoto]

On Sat, 25 Oct 2008 00:55:44 +0900 (JST), Atsushi Nemoto <anemo@mba.ocn.ne.jp> wrote:
> >     Same comment about calculating only once...
> 
> Yes.  But are there any good place to hold calculated values?  I don't
> think it is not worth to allocate a local structure just for holding
> such values.  And this is not a critical path anyway :-)

Oh, I mean "I don't think it is worth to ...".
---
Atsushi Nemoto

Re: [PATCH] ide: Add tx4938ide driver (v2)

[Bartlomiej Zolnierkiewicz]

On Wednesday 22 October 2008, Atsushi Nemoto wrote:
> This is the driver for the Toshiba TX4938 SoC EBUS controller ATA mode.
> It has custom set_pio_mode and some hacks for big endian.
> 
> Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>

I applied it so please address issues left (if any) with incremental patches.

Thanks for all your work on TX493x drivers.

Re: [PATCH] ide: Add tx4938ide driver (v2)

[Sergei Shtylyov]

Hello.

Bartlomiej Zolnierkiewicz wrote:

>> This is the driver for the Toshiba TX4938 SoC EBUS controller ATA mode.
>> It has custom set_pio_mode and some hacks for big endian.
>>
>> Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
>>     
>
> I applied it so please address issues left (if any) with incremental patches.
>   

   I'm not sure there was really a need to haste it into -rc1 (if you 
could push it to Linus later anyway)...
   I've realized that I have a question to Atsushi on why he chose the 
same way of implemnting the register accesses as on TX4939 despite 
TX4938 IDE is not really a SoC but a board level device (so probable 
should be using the normal, not the "raw" I/O accessors)...

MBR, Sergei

Re: [PATCH] ide: Add tx4938ide driver (v2)

[Atsushi Nemoto]

On Fri, 24 Oct 2008 15:12:15 +0400, Sergei Shtylyov <sshtylyov@ru.mvista.com> wrote:
>    I've realized that I have a question to Atsushi on why he chose the 
> same way of implemnting the register accesses as on TX4939 despite 
> TX4938 IDE is not really a SoC but a board level device (so probable 
> should be using the normal, not the "raw" I/O accessors)...

Well, because it behaves like SoC registers, not as other PCI IDE
registers (which are swapped on big endian).

Maybe new board designer can chose another option, so it might be
worth to add 'do_swap' member in tx4938ide_platform_info.  But anyway
current code just works with both endian on RBTX4938 board :-)

---
Atsushi Nemoto

Re: [PATCH] ide: Add tx4938ide driver (v2)

[Sergei Shtylyov]

Hello.

Atsushi Nemoto wrote:

> This is the driver for the Toshiba TX4938 SoC EBUS controller ATA mode.
> It has custom set_pio_mode and some hacks for big endian.
>
> Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
>   
[...]
> +static void tx4938ide_input_data_swap(ide_drive_t *drive, struct request *rq,
> +				void *buf, unsigned int len)
> +{
> +	unsigned long port = drive->hwif->io_ports.data_addr;
> +	unsigned short *ptr = buf;
> +	unsigned int count = (len + 1) / 2;
> +
> +	while (count--)
> +		*ptr++ = cpu_to_le16(__raw_readw((void __iomem *)port));
> +	__ide_flush_dcache_range((unsigned long)buf, count * 2);
> +}
> +
> +static void tx4938ide_output_data_swap(ide_drive_t *drive, struct request *rq,
> +				void *buf, unsigned int len)
> +{
> +	unsigned long port = drive->hwif->io_ports.data_addr;
> +	unsigned short *ptr = buf;
> +	unsigned int count = (len + 1) / 2;
> +
> +	while (count--) {
> +		__raw_writew(le16_to_cpu(*ptr), (void __iomem *)port);
> +		ptr++;
> +	}
> +	__ide_flush_dcache_range((unsigned long)buf, count * 2);
> +}

   Atsushi, does TX49 really suffer from the issue that these flushes 
are trying to address?

MBR, Sergei

Re: [PATCH] ide: Add tx4938ide driver (v2)

[Sergei Shtylyov]

Hello, I wrote:

>> This is the driver for the Toshiba TX4938 SoC EBUS controller ATA mode.
>> It has custom set_pio_mode and some hacks for big endian.
>>
>> Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
>>   
> [...]
>> +static void tx4938ide_input_data_swap(ide_drive_t *drive, struct 
>> request *rq,
>> +                void *buf, unsigned int len)
>> +{
>> +    unsigned long port = drive->hwif->io_ports.data_addr;
>> +    unsigned short *ptr = buf;
>> +    unsigned int count = (len + 1) / 2;
>> +
>> +    while (count--)
>> +        *ptr++ = cpu_to_le16(__raw_readw((void __iomem *)port));
>> +    __ide_flush_dcache_range((unsigned long)buf, count * 2);
>> +}
>> +
>> +static void tx4938ide_output_data_swap(ide_drive_t *drive, struct 
>> request *rq,
>> +                void *buf, unsigned int len)
>> +{
>> +    unsigned long port = drive->hwif->io_ports.data_addr;
>> +    unsigned short *ptr = buf;
>> +    unsigned int count = (len + 1) / 2;
>> +
>> +    while (count--) {
>> +        __raw_writew(le16_to_cpu(*ptr), (void __iomem *)port);
>> +        ptr++;
>> +    }
>> +    __ide_flush_dcache_range((unsigned long)buf, count * 2);
>> +}
>
>   Atsushi, does TX49 really suffer from the issue that these flushes 
> are trying to address?

   Well, looking thru the TX4939 thread, it appears that I've asked this 
question already. Isn't this related to VIVT caches?

MBR, Sergei

Re: [PATCH] ide: Add tx4938ide driver (v2)

[Atsushi Nemoto]

On Sun, 08 Feb 2009 14:52:48 +0300, Sergei Shtylyov <sshtylyov@ru.mvista.com> wrote:
> >> +    __ide_flush_dcache_range((unsigned long)buf, count * 2);
> >> +}
> >
> >   Atsushi, does TX49 really suffer from the issue that these flushes 
> > are trying to address?
> 
>    Well, looking thru the TX4939 thread, it appears that I've asked this 
> question already. Isn't this related to VIVT caches?

No, TX49 has VIPT cache.  It is related to D-cache aliasing on PIO.
My first attempt to fix this issue goes back to 2004:

http://www.linux-mips.org/archives/linux-mips/2004-03/msg00185.html

And more generic lengthy discussion can be found on (as mentioned in
the previous tx4939 thread):

http://lkml.org/lkml/2006/1/13/156

---
Atsushi Nemoto