Author: rminnich
Date: 2008-01-29 18:48:10 +0100 (Tue, 29 Jan 2008)
New Revision: 566

Modified:
   coreboot-v3/southbridge/amd/cs5536/cs5536.c
   coreboot-v3/util/dtc/dtc-parser.y
   coreboot-v3/util/dtc/dtc.c
   coreboot-v3/util/dtc/dtc.h
Log:
In the current version of dtc, if one has the line:
/config/ = "northbridge/amd/geodelx";

Then the file northbridge/amd/geodelx/dts is read in and processed. 
Magic(TM) appends the name "/dts" to the path. 

This hack is fine with chips that only do one thing. 
But some (all) northbridge parts play several roles: APIC cluster, PCI domain
device, and PCI device. The result is a need for more than one dts, since
there are three possible devices, with three types of IDs, and so on. 

To keep things sane, I am proposing to enable multiple dts files in a
directory, names (e.g., nothing required here):
domaindts
pcidts
apicdts

(of course these names can be anything, this is just an example).
This change will require a change to the dtc, since we can no longer
assume just one dts file, and hence need a way to name these different 
files. 

The proposed change is very simple. We now require the full path name 
for the file, and eliminate the Magic(TM).

So, 
/config/ = "northbridge/amd/geodelx/pcidts";

will open the pcidts file. 
/config/ = "northbridge/amd/geodelx/domaindts";
will open the domain dts. 

Maybe we should just call it domain and pci and apic? works for me.
/config/ = "northbridge/amd/geodelx/domain";
/config/ = "northbridge/amd/geodelx/pcibridge";
/config/ = "northbridge/amd/geodelx/apic";

Changes: 
dtc.c: create a new function, fopenfile, that will only open a path if it 
really is a file. Modify dtc_open_file to use this function. fopenfile
assumes "-" means stdin; should it, or should I move that assumption back
to dtc_open_file?
dtc.h: add prototypes
dtc-parser.y: Given a config path, open the path.
southbridge/amd/cs5536/cs5536.c: example of how C code changes


Signed-off-by: Ronald G. Minnich <[EMAIL PROTECTED]>

Acked-by: Carl-Daniel Hailfinger <[EMAIL PROTECTED]>

Please see the comments below, but they do not have to be addressed for
this commit, just keep them in mind for future commits in that area.



Modified: coreboot-v3/southbridge/amd/cs5536/cs5536.c
===================================================================
--- coreboot-v3/southbridge/amd/cs5536/cs5536.c 2008-01-28 17:50:56 UTC (rev 
565)
+++ coreboot-v3/southbridge/amd/cs5536/cs5536.c 2008-01-29 17:48:10 UTC (rev 
566)
@@ -172,7 +172,7 @@
  *
  * @param sb Southbridge config structure.
  */
-static void lpc_init(struct southbridge_amd_cs5536_config *sb)
+static void lpc_init(struct southbridge_amd_cs5536_dts_config *sb)
 {
        struct msr msr;
 
@@ -220,7 +220,7 @@
  *
  * @param sb Southbridge config structure.
  */
-static void uarts_init(struct southbridge_amd_cs5536_config *sb)
+static void uarts_init(struct southbridge_amd_cs5536_dts_config *sb)
 {
        struct msr msr;
        u16 addr = 0;
@@ -383,7 +383,7 @@
  *
  * @param sb Southbridge config structure.
  */
-static void enable_USB_port4(struct southbridge_amd_cs5536_config *sb)
+static void enable_USB_port4(struct southbridge_amd_cs5536_dts_config *sb)
 {
        u32 *bar;
        struct msr msr;
@@ -475,7 +475,7 @@
 {
        struct device *dev;
        struct msr msr;
-       struct southbridge_amd_cs5536_config *sb;
+       struct southbridge_amd_cs5536_dts_config *sb;
        const struct msrinit *csi;
 
        post_code(P80_CHIPSET_INIT);
@@ -486,7 +486,7 @@
                       __FUNCTION__);
                return;
        }
-       sb = (struct southbridge_amd_cs5536_config *)dev->device_configuration;
+       sb = (struct southbridge_amd_cs5536_dts_config 
*)dev->device_configuration;
 
 #if 0
        if (!IsS3Resume())
@@ -548,8 +548,8 @@
  */
 static void southbridge_init(struct device *dev)
 {
-       struct southbridge_amd_cs5536_config *sb =
-           (struct southbridge_amd_cs5536_config *)dev->device_configuration;
+       struct southbridge_amd_cs5536_dts_config *sb =
+           (struct southbridge_amd_cs5536_dts_config 
*)dev->device_configuration;
 
        /*
         * struct device *gpiodev;

Modified: coreboot-v3/util/dtc/dtc-parser.y
===================================================================
--- coreboot-v3/util/dtc/dtc-parser.y   2008-01-28 17:50:56 UTC (rev 565)
+++ coreboot-v3/util/dtc/dtc-parser.y   2008-01-29 17:48:10 UTC (rev 566)
@@ -121,17 +121,16 @@
 config:        DT_CONFIG '(' 
                includepath {
                        void switchin(FILE *f);
-
-                       /* switch ... */
-                       char path[1024];
                        FILE *f;
+                       /* The need for a cast here is silly */
+                       char *name = (char *)$3.val;
+
                        /* TODO: keep track of which of these we have read in. 
If we have already done it, then 
                          * don't do it twice. 
                          */
-                       sprintf(path, "%s/dts", $3.val);
-                       f  = fopen(path, "r");
+                       f  = fopenfile(name);
                        if (! f){
-                               perror(path);
+                               perror(name);
                                exit(1);
                        }
                        switchin(f);

Modified: coreboot-v3/util/dtc/dtc.c
===================================================================
--- coreboot-v3/util/dtc/dtc.c  2008-01-28 17:50:56 UTC (rev 565)
+++ coreboot-v3/util/dtc/dtc.c  2008-01-29 17:48:10 UTC (rev 566)
@@ -61,21 +61,47 @@
                fill_fullpaths(child, tree->fullpath);
 }
 
-static FILE *dtc_open_file(char *fname)
+/* How stupid is POSIX? This stupid: you can fopen a directory if mode
+ * is "r", but then there is very little you can do with it except get errors. 
+ * This function now makes sure that the thing you open is a file. 
+ */
+FILE *fopenfile(char *fname)
 {
-       FILE *f;
+       FILE *f = NULL;
 
-       if (streq(fname, "-"))
+       if (streq(fname, "-")){
                f = stdin;
-       else
+       } else {
+               struct stat buf;
+               if (stat(fname, &buf) < 0) {
+                       errno = EISDIR;
+                       goto no;
+               }
+
+               if (! S_ISREG(buf.st_mode)){
+                       errno = EISDIR;
+                       goto no;
+               }
+
                f = fopen(fname, "r");
+       }
 
+no:
+
+       return f;
+}
+
+FILE *dtc_open_file(char *fname)
+{
+       FILE *f = fopenfile(fname);
+
        if (! f)
                die("Couldn't open \"%s\": %s\n", fname, strerror(errno));
 
        return f;
 }
 
+
 static void usage(void)
 {
        fprintf(stderr, "Usage:\n");

Modified: coreboot-v3/util/dtc/dtc.h
===================================================================
--- coreboot-v3/util/dtc/dtc.h  2008-01-28 17:50:56 UTC (rev 565)
+++ coreboot-v3/util/dtc/dtc.h  2008-01-29 17:48:10 UTC (rev 566)
@@ -31,6 +31,8 @@
 #include <errno.h>
 #include <unistd.h>
 #include <netinet/in.h>
+#include <sys/types.h>
+#include <sys/stat.h>
 
 /* also covers (part of?) linux's byteswap.h functionality */
 #include "endian.h"
@@ -236,5 +238,6 @@
 
 char *join_path(char *path, char *name);
 void fill_fullpaths(struct node *tree, char *prefix);
-
+FILE *fopenfile(char *fname);
+FILE *dtc_open_file(char *fname);
 #endif /* _DTC_H */


-- 
coreboot mailing list
[email protected]
http://www.coreboot.org/mailman/listinfo/coreboot

Reply via email to