[coreboot] patch: two bugs in the cs5536 ide code
Carl-Daniel Hailfinger
c-d.hailfinger.devel.2006 at gmx.net
Thu May 8 01:22:45 CEST 2008
On 08.05.2008 01:03, Carl-Daniel Hailfinger wrote:
> On 08.05.2008 00:38, Stefan Reinauer wrote:
>
>> 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?
>>
>
> Yes.
>
>
>>> 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.
>>
>
> Will do. I'll also integrate the fixes Mart Raudsepp suggested for dbe61
> and dbe62.
>
Done. Please note that the dbe61 is now back to the preexisting age-old
breakage, but the new breakage has been fixed. The dbe61 code has too
many v2 leftovers to compile.
>> Without breaks, this patch is Acked-by: Stefan Reinauer
>> <stepan at coresystems.de>
>>
>
> Thanks!
>
Committed in r677.
Regards,
Carl-Daniel
> Regards,
> Carl-Daniel
>
>>> 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.
>>
That will come as a separate cleanup, but probably not from me in the
next 3 weeks.
More information about the coreboot
mailing list