[patch][v2] cs5536 usb port4 configuration

[Marc Jones]

This patch uses byte pointer and the MMIO read and write functions.

Marc

[ron minnich]

neat.

Do you want to do that for v3? I like it.

:-)

ron

[patch][v2] cs5536 usb port4 configuration - new v3 patch

[Marc Jones]

ron minnich wrote:
> neat.
>
> Do you want to do that for v3? I like it.
>
> :-)
>
> ron

As you wish. :)
Builds but not tested on a platform. Can you try it on the dbe62?

Thanks,
Marc

Correctly access MMIO for cs5536 USB port4 configuration.

Signed-off-by: Marc Jones

Index: coreboot-v3/southbridge/amd/cs5536/cs5536.c
===================================================================
--- coreboot-v3.orig/southbridge/amd/cs5536/cs5536.c	2008-06-05 15:37:05.000000000 -0600
+++ coreboot-v3/southbridge/amd/cs5536/cs5536.c	2008-06-05 16:12:54.000000000 -0600
@@ -395,19 +395,18 @@
    }
 }

-/* the /sizeof(u32) is to convert byte offsets into u32 offsets */
-#define HCCPARAMS		(0x08/sizeof(u32))
-#define IPREG04			(0xA0/sizeof(u32))
-#define USB_HCCPW_SET		(1 << 1)
+#define HCCPARAMS		0x08
+#define IPREG04			0xA0
+#define USB_HCCPW_SET	(1 << 1)
 #define UOCCAP			0x00
 #define APU_SET			(1 << 15)
-#define UOCMUX			(0x04/sizeof(u32))
+#define UOCMUX			0x04
 #define PMUX_HOST		0x02
 #define PMUX_DEVICE		0x03
 #define PUEN_SET		(1 << 2)
-#define UDCDEVCTL		(0x404/sizeof(u32))
+#define UDCDEVCTL		0x404
 #define UDC_SD_SET		(1 << 10)
-#define UOCCTL			(0x0C/sizeof(u32))
+#define UOCCTL			0x0C
 #define PADEN_SET		(1 << 7)

/**
@@ -417,7 +416,7 @@
  */
 static void enable_USB_port4(struct southbridge_amd_cs5536_dts_config *sb)
 {
-	u32 *bar;
+	u8 *bar;
    struct msr msr;
    struct device *dev;

@@ -432,41 +431,42 @@
    	/* Write to clear diag register. */
    	wrmsr(USB2_SB_GLD_MSR_DIAG, rdmsr(USB2_SB_GLD_MSR_DIAG));

-		bar = (u32 *) pci_read_config32(dev, PCI_BASE_ADDRESS_0);
+		bar = (u8 *) pci_read_config32(dev, PCI_BASE_ADDRESS_0);

/* Make HCCPARAMS writable. */
-		*(bar + IPREG04) |= USB_HCCPW_SET;
+		writel(readl(bar + IPREG04) | USB_HCCPW_SET, bar + IPREG04);

/* EECP=50h, IST=01h, ASPC=1 */
-		*(bar + HCCPARAMS) = 0x00005012;
+		writel(0x00005012, bar + HCCPARAMS);
    }

dev = dev_find_pci_device(PCI_VENDOR_ID_AMD,
    		      PCI_DEVICE_ID_AMD_CS5536_OTG, 0);
    if (dev) {
-		bar = (u32 *) pci_read_config32(dev, PCI_BASE_ADDRESS_0);
+		bar = (u8 *) pci_read_config32(dev, PCI_BASE_ADDRESS_0);

-		printk(BIOS_DEBUG, "UOCMUX is %x\n", *(bar + UOCMUX));
+		printk(BIOS_DEBUG, "UOCMUX is %x\n", readl(bar + UOCMUX));

-		*(bar + UOCMUX) &= PUEN_SET;
+		writel(readl(bar + UOCMUX) & PUEN_SET, bar + UOCMUX);

/* Host or Device? */
    	if (sb->enable_USBP4_device)
-			*(bar + UOCMUX) |= PMUX_DEVICE;
+			writel(readl(bar + UOCMUX) | PMUX_DEVICE, bar + UOCMUX);
    	else
-			*(bar + UOCMUX) |= PMUX_HOST;
+			writel(readl(bar + UOCMUX) | PMUX_HOST, bar + UOCMUX);

/* Overcurrent configuration */
-		printk(BIOS_DEBUG, "UOCCAP is %x\n", *(bar + UOCCAP));
+		printk(BIOS_DEBUG, "UOCCAP is %x\n", readl(bar + UOCCAP));
    	if (sb->enable_USBP4_overcurrent)
-			*(bar + UOCCAP) |= sb->enable_USBP4_overcurrent;
+			writel(readl(bar + UOCCAP)
+			       | sb->enable_USBP4_overcurrent, bar + UOCCAP);
+
    	/* power control. see comment in the dts for these bits */
    	if (sb->pph) {
-			*(bar + UOCCAP) &= ~0xff;
-			*(bar + UOCCAP) |= sb->pph;
+			writel((readl(bar + UOCCAP) & ~0xff) | sb->pph, bar + UOCCAP);
    	}
-		printk(BIOS_DEBUG, "UOCCAP is %x\n", *(bar + UOCCAP));
-		printk(BIOS_DEBUG, "UOCMUX is %x\n", *(bar + UOCMUX));
+		printk(BIOS_DEBUG, "UOCCAP is %x\n", readl(bar + UOCCAP));
+		printk(BIOS_DEBUG, "UOCMUX is %x\n", readl(bar + UOCMUX));

}

@@ -480,20 +480,20 @@
    	dev = dev_find_pci_device(PCI_VENDOR_ID_AMD,
    			      PCI_DEVICE_ID_AMD_CS5536_UDC, 0);
    	if (dev) {
-			bar = (u32 *)pci_read_config32(dev, PCI_BASE_ADDRESS_0);
-			*(bar + UDCDEVCTL) |= UDC_SD_SET;
+			bar = (u8 *)pci_read_config32(dev, PCI_BASE_ADDRESS_0);
+			writel(readl(bar + UDCDEVCTL) | UDC_SD_SET, bar + UDCDEVCTL);
    	}

dev = dev_find_pci_device(PCI_VENDOR_ID_AMD,
    			      PCI_DEVICE_ID_AMD_CS5536_OTG, 0);
    	if (dev) {
-			bar = (u32 *)pci_read_config32(dev, PCI_BASE_ADDRESS_0);
-			*(bar + UOCCTL) |= PADEN_SET;
-			*(bar + UOCCAP) |= APU_SET;
+			bar = (u8 *)pci_read_config32(dev, PCI_BASE_ADDRESS_0);
+			writel(readl(bar + UOCCTL) | PADEN_SET, bar + UOCCTL);
+			writel(readl(bar + UOCCAP) | APU_SET, bar + UOCCAP);
    	}
    }

-	printk(BIOS_DEBUG, "UOCCTL is %x\n", *(bar + UOCCTL));
+	printk(BIOS_DEBUG, "UOCCTL is %x\n", readl(bar + UOCCTL));

/* Disable virtual PCI UDC and OTG headers.  The kernel never
     * sees a header for this device.  It used to provide an OS

[patch][v2] cs5536 usb port4 configuration - new v3 patch

[Carl-Daniel Hailfinger]

On 06.06.2008 01:32, Marc Jones wrote:
> ron minnich wrote:
> > neat.
> >
> > Do you want to do that for v3? I like it.
> >
> > :-)
> >
> > ron
>
> As you wish. :)
> Builds but not tested on a platform. Can you try it on the dbe62?

Dumb question: Does this break strict aliasing in any way?

Regards,
Carl-Daniel

[patch][v2] cs5536 usb port4 configuration - new v3 patch

[Segher Boessenkool]

> Dumb question: Does this break strict aliasing in any way?

Show the code?  I'll do you an analysis.

Segher

[patch][v2] cs5536 usb port4 configuration - new v3 patch

[Marc Jones]

Segher Boessenkool wrote:
> > Dumb question: Does this break strict aliasing in any way?
>
> Show the code?  I'll do you an analysis.
>
> Segher

Here is the patch.

Marc

[patch][v2] cs5536 usb port4 configuration - new v3 patch

[ron minnich]

I'll try this as soon as I get home from travel.

maybe another week.

ron

[patch][v2] cs5536 usb port4 configuration - new v3 patch

[Marc Jones]

Carl-Daniel Hailfinger wrote:
> Dumb question: Does this break strict aliasing in any way?

I don't think so. There are not two pointers (of different types) that 
refer to the same location in the scope.

Marc

[Peter Stuge]

On Wed, Jun 04, 2008 at 01:21:46PM -0600, Marc Jones wrote:
> This patch uses byte pointer and the MMIO read and write functions.

Nice.
> writel(readl(bar + IPREG04) | USB_HCCPW_SET, bar + IPREG04);

..though I still prefer set and clear functions or macros over this.

//Peter