Re: [coreboot] patch: fix USB ports on DBE62, and other cs5536-based platforms

[Carl-Daniel Hailfinger]

On 04.06.2008 06:28, ron minnich wrote:
> This change adds some debug prints, and a comment warning to dts on cs5536.
>
> Most importantly it fixes a simple programming error which made it so most of
> the sets on the USB were not doing anything. The bug is also in V2.
>
> With this fix, the DBE62 USB ports all work!
>
> If someone clones the fix to V2, it will also fix V2. Or, we can just convince
> you to move forward to V3 :-)
>
> Signed-off-by: Ronald G. Minnich

If you fix the problems outlined below, this is
Acked-by: Carl-Daniel Hailfinger
> Also, please note, my remaining issue is to fix X11 on DBE62, then I
> believe we are done. Has someone tested audio however?
> Index: southbridge/amd/cs5536/cs5536.c
>
> --- southbridge/amd/cs5536/cs5536.c	(revision 688)
> +++ southbridge/amd/cs5536/cs5536.c	(working copy)
> @@ -395,18 +395,19 @@
>    }
>  }
>
> -#define HCCPARAMS		0x08
> -#define IPREG04			0xA0
> +/* the /sizeof(unsigned long) is to convert byte offsets into u32 offsets */

sizeof(unsigned long) is u64 for 64bit architectures. I suggest either
sizeof(u32) or sizeof(int).
> +#define HCCPARAMS		(0x08/sizeof(unsigned long))
> +#define IPREG04			(0xA0/sizeof(unsigned long))

same here. The indentation looks strange as well.
> #define USB_HCCPW_SET		(1 << 1)
>  #define UOCCAP			0x00
>  #define APU_SET			(1 << 15)
> -#define UOCMUX			0x04
> +#define UOCMUX			(0x04/sizeof(unsigned long))

same here
> #define PMUX_HOST		0x02
>  #define PMUX_DEVICE		0x03
>  #define PUEN_SET		(1 << 2)
> -#define UDCDEVCTL		0x404
> +#define UDCDEVCTL		(0x404/sizeof(unsigned long))

same here
> #define UDC_SD_SET		(1 << 10)
> -#define UOCCTL			0x0C
> +#define UOCCTL			(0x0C/sizeof(unsigned long))

same here.
> #define PADEN_SET		(1 << 7)
>
> /**
> @@ -445,6 +446,8 @@
>    if (dev) {
>    	bar = (u32 *) pci_read_config32(dev, PCI_BASE_ADDRESS_0);
> printk(BIOS_DEBUG, "UOCMUX is %x\n", *(bar + UOCMUX));
>
> *(bar + UOCMUX) &= PUEN_SET;
>
> /* Host or Device? */
>
> @@ -463,6 +466,7 @@
>    		*(bar + UOCCAP) |= sb->pph;
>    	}
>    	printk(BIOS_DEBUG, "UOCCAP is %x\n", *(bar + UOCCAP));
> printk(BIOS_DEBUG, "UOCMUX is %x\n", *(bar + UOCMUX));
>
> }
>
> @@ -489,6 +493,8 @@
>    	}
>    }
> printk(BIOS_DEBUG, "UOCCTL is %x\n", *(bar + UOCCTL));
> /* Disable virtual PCI UDC and OTG headers.  The kernel never
> sees a header for this device.  It used to provide an OS
> visible device, but that was defeatured.  There are still
> Index: southbridge/amd/cs5536/dts
>
> --- southbridge/amd/cs5536/dts	(revision 688)
> +++ southbridge/amd/cs5536/dts	(working copy)
> @@ -66,6 +66,10 @@
>    pph = "0";
>    /* 0:off, xxxx:overcurrent setting, e.g. 0x3FEA.
>     * See CS5536 - Data Book (pages 380-381).
> * And don't just set this to "1". You have to set it 
> * to values that make sense for the register. Do not set this
> * for your mainboard unless you have made sure of the register
> * settings!
> */
>  enable_USBP4_overcurrent = "0";

Regards,
Carl-Daniel