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@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@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@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@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@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@15,2 { + /config/("southbridge/amd/cs5536/ide"); + enable_ide = "1"; + }; ioport@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@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@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@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@15,2 { + /config/("southbridge/amd/cs5536/ide"); + enable_ide = "1"; + }; ioport@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@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@15,2 { + /config/("southbridge/amd/cs5536/ide"); + enable_ide = "1"; + }; }; };