[coreboot] patch: two bugs in the cs5536 ide code
Carl-Daniel Hailfinger
c-d.hailfinger.devel.2006 at gmx.net
Wed May 7 02:42:24 CEST 2008
On 06.05.2008 17:37, ron minnich wrote:
> On Tue, May 6, 2008 at 5:15 AM, Carl-Daniel Hailfinger
>
>> Indeed. Since we don't use the struct device passed in to ide_init, why
>> don't we give Geode IDE its own struct device_operations and store the
>> IDE config there?
>>
>
> That's a great question. The reason is that the cs5536 is one chip. On
> v2, we treated it as more than one. For my goal of easing non-experts
> into the code, I decided to try making it one chip and one dts. This
> is an experiment that may, in the end, prove wrong.
>
> I still think one dts per chip is the right way to go, but do we need
> more than one level of dts? I really think this all should be in one
> file, it got much simpler that way.
>
My dream is to have templates for such chips so we can refer to the
template unless we need some very special handling. That would mean
multi-level includes in the dts AFAICS, but I'm not totally happy about
that.
>> I have problems parsing the sentence above: "... I don't think it was
>> ever IDE."
>>
>
> I should not write when that tired.
>
> Let's try again.
>
> Works on 2.6.24. hangs on 2.6.25, I thought for a second it was NOT
> ide, but it is clear that 2.6.25 is hanging on probing ide1 -- or it
> sure looks that way. And, again, the post code of 00 is weird; I keep
> thinking there are vsa issues here. It just doesn't act like Linux
> hangs I see on other boards.
>
The phenomenon is weird, the text is well-written.
>> You know my preference for killing dev_find_pci_device, but that can
>> come later if it is too difficult.
>>
>
> I have no disagreement as long as, in so doing, we reduce code overall
> and increase comprehensability.
>
See below for my take at this.
Move CS5536 IDE configuration into a separate dts and its own PCI device.
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.
Index: LinuxBIOSv3-hierarchical_dts/southbridge/amd/cs5536/ide
===================================================================
--- LinuxBIOSv3-hierarchical_dts/southbridge/amd/cs5536/ide (revision 0)
+++ LinuxBIOSv3-hierarchical_dts/southbridge/amd/cs5536/ide (revision 0)
@@ -0,0 +1,26 @@
+/*
+ * This file is part of the coreboot project.
+ *
+ * Copyright (C) 2007 Ronald G. Minnich <rminnich at gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+{
+ device_operations = "cs5536_ide";
+
+ /* IDE: enable CS5536 IDE. There may be a different IDE controller on board */
+ enable_ide = "0";
+};
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,
+};
+
Index: LinuxBIOSv3-hierarchical_dts/southbridge/amd/cs5536/dts
===================================================================
--- LinuxBIOSv3-hierarchical_dts/southbridge/amd/cs5536/dts (revision 676)
+++ LinuxBIOSv3-hierarchical_dts/southbridge/amd/cs5536/dts (working copy)
@@ -36,9 +36,6 @@
/* 0:IDE 1:FLASH, if you are using NAND flash instead of IDE drive. */
enable_ide_nand_flash = "0";
- /* IDE: enable CS5536 IDE. There may be a different IDE controller on board */
- enable_ide = "0";
-
/* Enable USB Port 4 (0:host 1:device). */
enable_USBP4_device = "0";
Index: LinuxBIOSv3-hierarchical_dts/mainboard/amd/norwich/dts
===================================================================
--- LinuxBIOSv3-hierarchical_dts/mainboard/amd/norwich/dts (revision 676)
+++ LinuxBIOSv3-hierarchical_dts/mainboard/amd/norwich/dts (working copy)
@@ -34,7 +34,6 @@
};
pci at 15,0 {
/config/("southbridge/amd/cs5536/dts");
- enable_ide = "1";
/* Interrupt enables for LPC bus.
* Each bit is an IRQ 0-15. */
lpc_serirq_enable = "0x00001002";
@@ -50,5 +49,9 @@
com1_address = "0x3f8";
com1_irq = "4";
};
+ pci at 15,2 {
+ /config/("southbridge/amd/cs5536/ide");
+ enable_ide = "1";
+ };
};
};
Index: LinuxBIOSv3-hierarchical_dts/mainboard/amd/db800/dts
===================================================================
--- LinuxBIOSv3-hierarchical_dts/mainboard/amd/db800/dts (revision 676)
+++ LinuxBIOSv3-hierarchical_dts/mainboard/amd/db800/dts (working copy)
@@ -34,7 +34,6 @@
};
pci at 15,0 {
/config/("southbridge/amd/cs5536/dts");
- enable_ide = "1";
/* Interrupt enables for LPC bus.
* Each bit is an IRQ 0-15. */
lpc_serirq_enable = "0x000010da";
@@ -47,6 +46,10 @@
enable_gpio_int_route = "0x0D0C0700";
enable_USBP4_device = "1";
};
+ pci at 15,2 {
+ /config/("southbridge/amd/cs5536/ide");
+ enable_ide = "1";
+ };
ioport at 46 {
/config/("superio/winbond/w83627hf/dts");
com1enable = "1";
Index: LinuxBIOSv3-hierarchical_dts/mainboard/artecgroup/dbe62/dts
===================================================================
--- LinuxBIOSv3-hierarchical_dts/mainboard/artecgroup/dbe62/dts (revision 676)
+++ LinuxBIOSv3-hierarchical_dts/mainboard/artecgroup/dbe62/dts (working copy)
@@ -34,7 +34,6 @@
};
pci at 15,0 {
/config/("southbridge/amd/cs5536/dts");
- enable_ide = "1";
/* Interrupt enables for LPC bus.
* Each bit is an IRQ 0-15. */
lpc_serirq_enable = "0x00001002";
@@ -54,5 +53,9 @@
/* Set com2 IRQ to be what is usually COM1 */
com2_irq = "4";
};
+ pci at 15,2 {
+ /config/("southbridge/amd/cs5536/ide");
+ enable_ide = "1";
+ };
};
};
Index: LinuxBIOSv3-hierarchical_dts/mainboard/pcengines/alix1c/dts
===================================================================
--- LinuxBIOSv3-hierarchical_dts/mainboard/pcengines/alix1c/dts (revision 676)
+++ LinuxBIOSv3-hierarchical_dts/mainboard/pcengines/alix1c/dts (working copy)
@@ -34,7 +34,6 @@
};
pci at 15,0 {
/config/("southbridge/amd/cs5536/dts");
- enable_ide = "1";
/* Interrupt enables for LPC bus.
* Each bit is an IRQ 0-15. */
lpc_serirq_enable = "0x000010da";
@@ -46,6 +45,10 @@
* See virtual PIC spec. */
enable_gpio_int_route = "0x0D0C0700";
};
+ pci at 15,2 {
+ /config/("southbridge/amd/cs5536/ide");
+ enable_ide = "1";
+ };
ioport at 46 {
/config/("superio/winbond/w83627hf/dts");
com1enable = "1";
Index: LinuxBIOSv3-hierarchical_dts/mainboard/pcengines/alix2c3/dts
===================================================================
--- LinuxBIOSv3-hierarchical_dts/mainboard/pcengines/alix2c3/dts (revision 676)
+++ LinuxBIOSv3-hierarchical_dts/mainboard/pcengines/alix2c3/dts (working copy)
@@ -32,7 +32,6 @@
};
pci at 15,0 {
/config/("southbridge/amd/cs5536/dts");
- enable_ide = "1";
/* Interrupt enables for LPC bus.
* Each bit is an IRQ 0-15. */
lpc_serirq_enable = "0x000010da";
@@ -50,5 +49,9 @@
/* this board does not really have vga; disable it (pci device 00:01.1) */
unwanted_vpci = < 80000900 0 >;
};
+ pci at 15,2 {
+ /config/("southbridge/amd/cs5536/ide");
+ enable_ide = "1";
+ };
};
};
More information about the coreboot
mailing list