[Fwd: Re: [patch][v2] cs5536 usb port4 configuration - new v3 patch]

[Marc Jones]

And the v3 version.

-------- Original Message --------
Subject: Re: [coreboot] [patch][v2] cs5536 usb port4 configuration - new 
v3 patch
Date: Thu, 05 Jun 2008 17:32:12 -0600
From: Marc Jones 
To: ron minnich 
CC: Coreboot 
References:  

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

[Carl-Daniel Hailfinger]

Hi Marc,

please ignore my other mail regarding your equivalent v2 patch. I had
not seen this mail yet and assumed divergence.
Overall, I like the current style a little bit more, but the current
style also has these ugly divisions by sizeof(u32) which I dislike.

If anybody else acks this patch, feel free to add
Acked-by: Carl-Daniel Hailfinger 
to your v2 and v3 patch.

The only nitpick I have is a slight whitespace issue below.

On 12.07.2008 01:27, Marc Jones wrote:
> 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)

A few of the #define above look a bit unaligned.
> #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

Regards,
Carl-Daniel

[Marc Jones]

Carl-Daniel Hailfinger wrote:
> Hi Marc,
>
> please ignore my other mail regarding your equivalent v2 patch. I had
> not seen this mail yet and assumed divergence.
> Overall, I like the current style a little bit more, but the current
> style also has these ugly divisions by sizeof(u32) which I dislike.
>
> If anybody else acks this patch, feel free to add
> Acked-by: Carl-Daniel Hailfinger 
> to your v2 and v3 patch.
>
> The only nitpick I have is a slight whitespace issue below.

Thanks,I'll doulbe check the whitespace before I check it in.
Marc

[Marc Jones]

Carl-Daniel Hailfinger wrote:
> Hi Marc,
>
> please ignore my other mail regarding your equivalent v2 patch. I had
> not seen this mail yet and assumed divergence.
> Overall, I like the current style a little bit more, but the current
> style also has these ugly divisions by sizeof(u32) which I dislike.
>
> If anybody else acks this patch, feel free to add
> Acked-by: Carl-Daniel Hailfinger

v2 r3421

[Marc Jones]

Marc Jones wrote:
> Carl-Daniel Hailfinger wrote:
> > Hi Marc,
> >
> > please ignore my other mail regarding your equivalent v2 patch. I had
> > not seen this mail yet and assumed divergence.
> > Overall, I like the current style a little bit more, but the current
> > style also has these ugly divisions by sizeof(u32) which I dislike.
> >
> > If anybody else acks this patch, feel free to add
> > Acked-by: Carl-Daniel Hailfinger
>
> v2 r3421

Doh! I misread you  "If anybody else acks this patch". Just saw the ack. 
I will hold off on the v3 one until ron acks. (I think that the v2 one 
is required.)

Marc

[Carl-Daniel Hailfinger]

On 12.07.2008 02:11, Marc Jones wrote:
> Marc Jones wrote:
> > Carl-Daniel Hailfinger wrote:
> > > Hi Marc,
> > >
> > > please ignore my other mail regarding your equivalent v2 patch. I had
> > > not seen this mail yet and assumed divergence.
> > > Overall, I like the current style a little bit more, but the current
> > > style also has these ugly divisions by sizeof(u32) which I dislike.
> > >
> > > If anybody else acks this patch, feel free to add
> > > Acked-by: Carl-Daniel Hailfinger
> >
> > v2 r3421
>
> Doh! I misread you  "If anybody else acks this patch". Just saw the
> ack. I will hold off on the v3 one until ron acks. (I think that the
> v2 one is required.)

No problem with me. Since Ron said he liked your variant, I am not going
to protest.
If Ron doesn't ack in the next few days, feel free to use my ack standalone.

And thanks a lot for keeping track of this stuff.

Regards,
Carl-Daniel

[Stefan Reinauer]

Marc Jones wrote:
> Marc Jones wrote:
> > Carl-Daniel Hailfinger wrote:
> > > Hi Marc,
> > >
> > > please ignore my other mail regarding your equivalent v2 patch. I had
> > > not seen this mail yet and assumed divergence.
> > > Overall, I like the current style a little bit more, but the current
> > > style also has these ugly divisions by sizeof(u32) which I dislike.
> > >
> > > If anybody else acks this patch, feel free to add
> > > Acked-by: Carl-Daniel Hailfinger
> >
> > v2 r3421
>
> Doh! I misread you  "If anybody else acks this patch". Just saw the ack. 
> I will hold off on the v3 one until ron acks. (I think that the v2 one 
> is required.)

Acked-by: Stefan Reinauer

for the v3 patch.

[Marc Jones]

Stefan Reinauer wrote:
> Marc Jones wrote:
> > Marc Jones wrote:
> > > Carl-Daniel Hailfinger wrote:
> > > > Hi Marc,
> > > >
> > > > please ignore my other mail regarding your equivalent v2 patch. I had
> > > > not seen this mail yet and assumed divergence.
> > > > Overall, I like the current style a little bit more, but the current
> > > > style also has these ugly divisions by sizeof(u32) which I dislike.
> > > >
> > > > If anybody else acks this patch, feel free to add
> > > > Acked-by: Carl-Daniel Hailfinger
> > >
> > > v2 r3421
> >
> > Doh! I misread you  "If anybody else acks this patch". Just saw the ack. 
> > I will hold off on the v3 one until ron acks. (I think that the v2 one 
> > is required.)
>
> Acked-by: Stefan Reinauer
>
> for the v3 patch.

v3 r696

Thanks guys!
Marc