[coreboot] patch: two bugs in the cs5536 ide code

Stefan Reinauer stepan at coresystems.de
Thu May 8 00:38:29 CEST 2008


Carl-Daniel Hailfinger wrote:
>
> See below for my take at this.
>
> Move CS5536 IDE configuration into a separate dts and its own PCI device.
>   
Carl-Daniel, is this the patch you mentioned?

> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>
>
> Build-tested on db800, norwich, dbe62, alix.1c, alix.2c3.
> Breaks build for dbe61.
>   

Why is that? Please fix this before committing.

Without breaks, this patch is Acked-by: Stefan Reinauer 
<stepan at coresystems.de>

> Index: LinuxBIOSv3-hierarchical_dts/southbridge/amd/cs5536/cs5536.c
> ===================================================================
> --- LinuxBIOSv3-hierarchical_dts/southbridge/amd/cs5536/cs5536.c	(revision 676)
> +++ LinuxBIOSv3-hierarchical_dts/southbridge/amd/cs5536/cs5536.c	(working copy)
> @@ -590,6 +590,11 @@
>  {
>  	u32 ide_cfg;
>  
> +	struct southbridge_amd_cs5536_ide_config *ide =
> +	    (struct southbridge_amd_cs5536_ide_config *)dev->device_configuration;
> +	if (!ide->enable_ide)
> +		return;
> +
>  	printk(BIOS_DEBUG, "cs5536_ide: %s\n", __func__);
>  	/* GPIO and IRQ setup are handled in the main chipset code. */
>  
> @@ -654,9 +659,6 @@
>  		hide_vpci(sb->unwanted_vpci[i]);
>  	}
>  
> -	if (sb->enable_ide)
> -		ide_init(dev);
> -
>  	cs5536_setup_power_button(sb);
>  
>  	printk(BIOS_SPEW, "cs5536: %s() Exit\n", __FUNCTION__);
> @@ -688,3 +690,17 @@
>  	.phase6_init			= southbridge_init,
>  };
>  
> +struct device_operations cs5536_ide = {
> +	.id = {.type = DEVICE_ID_PCI,
> +		.u = {.pci = {.vendor = PCI_VENDOR_ID_AMD,
> +			      .device = PCI_DEVICE_ID_AMD_CS5536_B0_IDE}}},
> +	.constructor		 = default_device_constructor,
> +#warning FIXME: what has to go in phase3_scan?
> +	.phase3_scan		 = 0,
> +	.phase4_read_resources	 = pci_dev_read_resources,
> +	.phase4_set_resources	 = pci_dev_set_resources,
> +	.phase5_enable_resources = pci_dev_enable_resources,
> +	.phase6_init		 = ide_init,
> +	.ops_pci		 = &pci_dev_ops_pci,
> +};
> +

In v2 I liked that every hardware driver for every pci device was in a 
single, small file. Packing all of it in one large cs5536.c is a bit 
ugly. But that is not within the scope of this patch I guess.


-- 
coresystems GmbH • Brahmsstr. 16 • D-79104 Freiburg i. Br.
      Tel.: +49 761 7668825 • Fax: +49 761 7664613
Email: info at coresystems.dehttp://www.coresystems.de/
Registergericht: Amtsgericht Freiburg • HRB 7656
Geschäftsführer: Stefan Reinauer • Ust-IdNr.: DE245674866


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 249 bytes
Desc: OpenPGP digital signature
URL: <http://www.coreboot.org/pipermail/coreboot/attachments/20080508/500c7447/attachment.sig>


More information about the coreboot mailing list