Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 12 Apr 2019 12:19:33 -0700
From:      Mark Millard <marklmi@yahoo.com>
To:        FreeBSD PowerPC ML <freebsd-ppc@freebsd.org>, Justin Hibbits <chmeeedalf@gmail.com>
Subject:   Patches to allow usefdt mode that works on a 2 socket PowerMac3,6 example too --and makes more work on 2-socket/1-core-each PowerMac11,2
Message-ID:  <988F644F-D5E7-4FB4-AAB3-A72E9DA88CE6@yahoo.com>

next in thread | raw e-mail | index | archive | help
With the following 2 patches for converting
openfirmware to fdt content. . .

PowerMac11,2 example for usefdt mode:
A) bge0 and bge1 are back in their historical places.
B) powerd on the PowerMac11,2 works again.
C) sysctl -a | grep cpufreq lists items for all the cpus
D) probably more.

PowerMac3,6 example for usefdt mode:
E) gem0 is present again and even works.
F) Both CPUs are used again.
F) probably more.
(powerpd and cpufreq's are not operable/present even in
non-usefdt mode.)

This message does not deal with other investigatory
patches for other issues than converting openfirmware
to fdt. But my test environment has all my investigatory
patches in order to avoid other things getting in the
way of my investigations.

The code comments are fairly explicit about
what and why for the changes.

I treat the patches as investigatory, not ready
in form for being official FreeBSD material. There
are likely questions of if the change go in the
right long-term direction --or even if old PowerMacs
will continue to be viewed as worth supporting
(because they compete with time spent on modern
support).

I'll note that I've never had the 2-socket/1-core-each
PowerMac7,2 get any visible behavior after the Kernel
entry point message with any variation of usefdt mode.
Failing so early, I've not figured out any way to
investigate it hanging up. It does boot in non-usefdt
mode with my other investigatory patches in place.

The openfirmware to fdt conversion patches are (white
space details might not have been preserved in the
message):

# svnlite diff /usr/src/stand/powerpc/ofw/ | more
Index: /usr/src/stand/powerpc/ofw/ofwfdt.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- /usr/src/stand/powerpc/ofw/ofwfdt.c (revision 345758)
+++ /usr/src/stand/powerpc/ofw/ofwfdt.c (working copy)
@@ -42,22 +42,54 @@
 }
=20
 static void
-add_node_to_fdt(void *buffer, phandle_t node, int fdt_offset)
+add_node_to_fdt(void *buffer, phandle_t parent_node, int fdt_offset)
 {
        int i, child_offset, error;
-       char name[255], *lastprop, *subname;
+       char name[255+1], *lastprop, *subname; // +1 added for always =
having a trailing '\0' position.
        void *propbuf;
        ssize_t proplen;
=20
-       lastprop =3D NULL;
-       while (OF_nextprop(node, lastprop, name) > 0) {
-               proplen =3D OF_getproplen(node, name);
+       // WARNING: fdt_setprop adds to the beginning of the property =
sequence. So,
+       // to avoid reversing the sequence, use it last to first for the =
originals.
=20
+       int prop_cnt=3D 0;
+       lastprop=3D NULL;
+       while (0<OF_nextprop(parent_node, lastprop, name)) {
+               prop_cnt++;
+               lastprop=3D name;
+       }
+       int prop_want=3D prop_cnt;
+       for (; 0!=3Dprop_want; prop_want--) {
+               lastprop=3D NULL;
+               int prop_at=3D 0;
+               for(; prop_at!=3Dprop_want; prop_at++) {
+                       OF_nextprop(parent_node, lastprop, name);
+                       lastprop=3D name;
+               }
+
+               proplen =3D OF_getproplen(parent_node, name);
+
                /* Detect and correct for errors and strangeness */
-               if (proplen < 0)
+               if (proplen < 0) {
+                       printf("proplen was negative: %jd while adding =
property %s to "
+                           "parent_node %d\n", (intmax_t)proplen, name, =
fdt_offset);
                        proplen =3D 0;
-               if (proplen > 1024)
+               }
+               if (proplen > 1024) {
+                       printf("proplen was large: %jd while adding =
property %s to "
+                           "parent_node %d\n", (intmax_t)proplen, name, =
fdt_offset);
+#if 0
+// WARNING: Some Macintoshes end up with video-card driver code as =
properties.
+//          An example PowerMac7,2 configuration had 2 such properties, =
each
+//          being 96306 bytes in size. If these were ever used in a =
truncated
+//          from things would be messed up. So do not force =
truncations. There
+//          are a few other, smaller properties that bigger than 1 =
KBytes. I
+//          checked: fdt_platform_load_dtb's 409600 buflen is more than
+//          sufficient for the example context.
+
                        proplen =3D 1024;
+#endif
+               }
=20
                propbuf =3D malloc(proplen);
                if (propbuf =3D=3D NULL) {
@@ -64,25 +96,93 @@
                        printf("Cannot allocate memory for prop %s\n", =
name);
                        return;
                }
-               OF_getprop(node, name, propbuf, proplen);
+               OF_getprop(parent_node, name, propbuf, proplen);
                error =3D fdt_setprop(buffer, fdt_offset, name, propbuf, =
proplen);
                free(propbuf);
-               lastprop =3D name;
                if (error)
                        printf("Error %d adding property %s to "
-                           "node %d\n", error, name, fdt_offset);
+                           "parent_node %d\n", error, name, =
fdt_offset);
        }
=20
-       if (!OF_hasprop(node, "phandle") && !OF_hasprop(node, =
"linux,phandle")
-           && !OF_hasprop(node, "ibm,phandle"))
-               fdt_setprop(buffer, fdt_offset, "phandle", &node, =
sizeof(node));
+       if (!OF_hasprop(parent_node, "phandle") && =
!OF_hasprop(parent_node, "linux,phandle")
+           && !OF_hasprop(parent_node, "ibm,phandle"))
+               fdt_setprop(buffer, fdt_offset, "phandle", &parent_node, =
sizeof(parent_node));
=20
-       for (node =3D OF_child(node); node > 0; node =3D OF_peer(node)) =
{
-               OF_package_to_path(node, name, sizeof(name));
-               subname =3D strrchr(name, '/');
+       // WARNING: fdt_add_subnode adds to the beginning of the node =
sequence (after
+       // the properties). So, to avoid reversing the sequence, use it =
last to first
+       // for the originals.
+
+       // WARNING: openfirmware's package-to-path(nd,nm,len) does not =
place a trailing '\0'
+       //          character in nm when it returns a full_str_len with =
len<=3Dfull_str_len .
+       //          For full_str_len<len, a trailing '\0' is placed at =
nm[full_str_len].
+
+       int child_cnt=3D 0;
+       phandle_t node=3D OF_child(parent_node);
+       for (; 0<node; node=3D OF_peer(node)) {
+               child_cnt++;
+       }
+       name[255]=3D '\0'; // Avoid OF_package_to_path leaving no '\0' =
at end of long path.
+       int node_want=3D child_cnt;
+       for (; 0!=3Dnode_want; node_want--) {
+               node=3D OF_child(parent_node);
+               int node_at=3D 1;
+               for (; node_at!=3Dnode_want; node_at++) {
+                       node =3D OF_peer(node);
+               }
+
+               int full_str_len=3D OF_package_to_path(node, name, =
sizeof(name)-1); // Avoids having trailing '\0' missing.
+               if (-1=3D=3Dfull_str_len) { // Highly unlikely.
+                       printf("add_node_to_fdt got -1 return from =
OF_packakge_to_path\n");
+                       continue;
+               }
+
+               // WARNING: For some Macintoshes, name can sometimes =
contain '\0' characters
+               //          in the middle! So there is avoidance-code =
because the fdt code is
+               //          not designed for such.
+
+               // WARNING: For some Macintoshes, multiple nodes under a =
parent can have the
+               //          same full-path-text (name here). I've =
adjusted fdt_add_subnode to
+               //          allow such to be recorded so that such =
Macintosh information is
+               //          not lost.
+
+               // full_str_len omits the offical trailing '\0' =
position.
+               // full_str_len is *not* limited by the sizeof(name)-1 =
value above: it reports
+               // the space needed to get all the text (ignoring the =
official trailing '\0').
+               if (0=3D=3Dfull_str_len) { // Highly unlikely.
+                       printf("Error: Node name has no bytes before =
trailing null byte\n");
+                       continue;
+               }
+               if (255<full_str_len) {
+                       printf("Error: Node path %s (truncated), needs =
more than 255+1 bytes\n", name);
+                       continue;
+               }
+
+               // Eliminate any internal '\0' full path characters: =
makes saved results more standard
+               // and, so, fdt-supported. The extra Macintosh '\0' =
characters are non-essential.
+               char *from_pos=3D name;
+               char *to_pos=3D name;
+               char *original_end=3D name+full_str_len;
+               while (from_pos<original_end) {
+                       if ('\0'!=3D*from_pos)
+                               *to_pos++=3D *from_pos;
+                       else
+                               full_str_len--;
+                       from_pos++;
+               }
+               *to_pos=3D '\0';
+               if (to_pos=3D=3Dname) { // Highly unlikely.
+                       printf("Error: Adjusted node name has no bytes =
before trailing null byte\n");
+                       continue;
+               }
+
+               subname=3D strrchr(name,'/');
+               if (NULL=3D=3Dsubname) { // Highly unlikely.
+                       printf("Error: Node name %s has no '/'\n", =
name);
+                       continue;
+               }
                subname++;
                child_offset =3D fdt_add_subnode(buffer, fdt_offset, =
subname);
-               if (child_offset < 0) {
+               if (child_offset < 0) { // Note: fdt_add_subnode was =
modified to allow duplicate paths.
                        printf("Error %d adding node %s (%s), =
skipping\n",
                            child_offset, name, subname);
                        continue;


# svnlite diff /usr/src/sys/contrib/ | more
Index: /usr/src/sys/contrib/libfdt/fdt_rw.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- /usr/src/sys/contrib/libfdt/fdt_rw.c        (revision 345758)
+++ /usr/src/sys/contrib/libfdt/fdt_rw.c        (working copy)
@@ -357,9 +357,15 @@
        FDT_RW_CHECK_HEADER(fdt);
=20
        offset =3D fdt_subnode_offset_namelen(fdt, parentoffset, name, =
namelen);
+#if 0
+// Some Macintoshes have identical package-to-pathname results for
+// multiple nodes of the same type and unit under the parent node.
+// Avoid blocking this for fdt.
        if (offset >=3D 0)
                return -FDT_ERR_EXISTS;
-       else if (offset !=3D -FDT_ERR_NOTFOUND)
+       else
+#endif
+       if (offset !=3D -FDT_ERR_NOTFOUND)
                return offset;
=20
        /* Try to place the new node after the parent's properties */



=3D=3D=3D
Mark Millard
marklmi at yahoo.com
( dsl-only.net went
away in early 2018-Mar)




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?988F644F-D5E7-4FB4-AAB3-A72E9DA88CE6>