Re: [PATCH v2 1/7] usb/isp1760: Move to native-endian ptds

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Arvid Brodin wrote:
This helps users with platform-bus-connected isp176xs with endianness problems

could you please define endianness problems? This driver works on BE and
LE machines. However, if the chip is wired wrong you need additional swaps
in read/write routines.

and fixes a bug with non-32-bit io transfers on big endian hardware with
straight data bus, where payload has to be byteswapped instead of ptds.

diff --git a/drivers/usb/host/isp1760-hcd.c b/drivers/usb/host/isp1760-hcd.c
index c470cc8..428a255 100644
--- a/drivers/usb/host/isp1760-hcd.c
+++ b/drivers/usb/host/isp1760-hcd.c
@@ -114,73 +114,136 @@ struct isp1760_qh {
#define ehci_port_speed(priv, portsc) USB_PORT_STAT_HIGH_SPEED -static unsigned int isp1760_readl(__u32 __iomem *regs)
+static u32 isp176x_reg_read32(void __iomem *base, u32 reg)
 {
-	return readl(regs);
+	BUG_ON(reg >= 0x0400);
+	return readl(base + reg);

Why do you push base address into read/write routines?

 }
/*
  * The next two copy via MMIO data to/from the device. memcpy_{to|from}io()
  * doesn't quite work because some people have to enforce 32-bit access
  */
-static void priv_read_copy(struct isp1760_hcd *priv, u32 *src,
-		__u32 __iomem *dst, u32 len)
+static void isp176x_bank_reads8(void __iomem *src_base, u32 src_offset, u32 bank_addr, __u32 *dst, u32 bytes)
 {
+	__u32 __iomem *src;
 	u32 val;
-	u8 *buff8;
+	__u8 *src_byteptr;
+	__u8 *dst_byteptr;
- if (!src) {
-		printk(KERN_ERR "ERROR: buffer: %p len: %d\n", src, len);
-		return;
-	}
+	BUG_ON(src_offset & 0x03);
+	BUG_ON(src_offset < PTD_OFFSET);
- while (len >= 4) {
-		*src = __raw_readl(dst);
-		len -= 4;
-		src++;
-		dst++;
+	src = src_base + (bank_addr | src_offset);
+
+	if (src_offset < PAYLOAD_OFFSET) {
+		while (bytes >= 4) {
+			*dst = le32_to_cpu(__raw_readl(src));

Not sure if this does not break the other BE machines and fixes yours.
Don't have any HW around for a quick test.

As far as I can see you do three things here:
- split the register read/write function into base and reg. Why? The only
  "benefit I see is that you check if one tries to reach beyond the
  register space. You should only map the register space so any access
  after the register space should be catched by the VM.

- you push bank address the read/write functions. You lose the cool trick
  where we implicit delay 90ns.

- you change the PTD struct from LE to native. Later you do an endian
  swap. How does it help?

Please split this patch in three since you have three logical changes as
far as I can see. I saw some missing spaces and long lines. Please make checkpatch happy.

Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux