[PATCH 0/5] macio: switch over to use 2 ata-3 controllers

[Mark Cave-Ayland]

Further work on Darwin/OS X shows that IOKit expects both the hard disk and 
cdrom to be attached to the ata-3 controller for QEMU's mac99 machine.

Currently OpenBIOS generates an ata-3 and an ata-4 controller (and by default 
attaches the cdrom to the ata-4 controller) meaning that IOKit would fail to
locate and mount the cdrom on boot. Resolve this by instantiating 2 ata-3
controller nodes instead which also is a better match for a real device tree.

The relevant changes to the device tree are shown below:

Before:

fff542a8 /pci@80000000/mac-io@3/ata-3@20000 (ata)
fff54540 /pci@80000000/mac-io@3/ata-4@21000 (ata)
fff547d8 /pci@80000000/mac-io@3/ata-4@21000/cdrom@0 (block)

After:

fff542a8 /pci@80000000/mac-io@3/ata-3@20000 (ata)
fff54540 /pci@80000000/mac-io@3/ata-3@21000 (ata)
fff547d8 /pci@80000000/mac-io@3/ata-3@21000/cdrom@0 (block)

The OpenBIOS IDE code currently works by using the device name internally to
reference the controller node, which of course breaks when 2 nodes are given
the same name property. Patches 1-4 switch the OpenBIOS IDE code over to
reference controller nodes by phandle rather than name, while patch 5 does
the actual switch from 1 ata-3 and 1 ata-4 controller to 2 ata-3 controllers.

Signed-off-by: Mark Cave-Ayland

Mark Cave-Ayland (5):
  ide: use REGISTER_NAMED_NODE_PHANDLE rather than REGISTER_NAMED_NODE
  libopenbios: introduce new get_path_from_ph() helper function
  ide: switch IDE init functions over to use new get_path_from_ph()
    helper
  ide: reference IDE channels by phandle, not device name
  macio: switch over to use 2 ata-3 ide controllers

openbios-devel/drivers/ide.c                  |   46 +++++++++----------------
 openbios-devel/drivers/ide.h                  |    2 +-
 openbios-devel/drivers/pci.c                  |    8 ++---
 openbios-devel/include/libopenbios/bindings.h |    1 +
 openbios-devel/libopenbios/bindings.c         |    8 +++++
 5 files changed, 31 insertions(+), 34 deletions(-)

[PATCH 1/5] ide: use REGISTER_NAMED_NODE_PHANDLE rather than REGISTER_NAMED_NODE

[Mark Cave-Ayland]

There already exists a macro to create the device node and return its phandle,
so use this directly rather than having to create the node and then navigate
to its full path separately.

Signed-off-by: Mark Cave-Ayland 
---
 openbios-devel/drivers/ide.c |   14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/openbios-devel/drivers/ide.c b/openbios-devel/drivers/ide.c
index 5125b78..51f1696 100644
--- a/openbios-devel/drivers/ide.c
+++ b/openbios-devel/drivers/ide.c
@@ -1424,9 +1424,7 @@ int ob_ide_init(const char *path, uint32_t io_port0, uint32_t ctl_port0,

snprintf(nodebuff, sizeof(nodebuff), "%s/" DEV_NAME, path,
                          current_channel);
-		REGISTER_NAMED_NODE(ob_ide_ctrl, nodebuff);
-
-		dnode = find_dev(nodebuff);
+		REGISTER_NAMED_NODE_PHANDLE(ob_ide_ctrl, nodebuff, dnode);

#if !defined(CONFIG_PPC) && !defined(CONFIG_SPARC64)
    	props[0]=14; props[1]=0;
@@ -1471,8 +1469,7 @@ int ob_ide_init(const char *path, uint32_t io_port0, uint32_t ctl_port0,
                         snprintf(nodebuff, sizeof(nodebuff),
                                  "%s/" DEV_NAME "/%s", path, current_channel,
                                  media);
-			REGISTER_NAMED_NODE(ob_ide, nodebuff);
-			dnode=find_dev(nodebuff);
+			REGISTER_NAMED_NODE_PHANDLE(ob_ide, nodebuff, dnode);
    		set_int_property(dnode, "reg", j);

/* create aliases */
@@ -1596,9 +1593,7 @@ int macio_ide_init(const char *path, uint32_t addr, int nb_channels)

snprintf(nodebuff, sizeof(nodebuff), "%s/" DEV_NAME, path,
                          current_channel);
-		REGISTER_NAMED_NODE(ob_ide_ctrl, nodebuff);
-
-		dnode = find_dev(nodebuff);
+		REGISTER_NAMED_NODE_PHANDLE(ob_ide_ctrl, nodebuff, dnode);

set_property(dnode, "compatible", (is_oldworld() ?
    		     "heathrow-ata" : "keylargo-ata"), 13);
@@ -1694,8 +1689,7 @@ int macio_ide_init(const char *path, uint32_t addr, int nb_channels)
                         snprintf(nodebuff, sizeof(nodebuff),
                                  "%s/" DEV_NAME "/%s", path, current_channel,
                                  media);
-			REGISTER_NAMED_NODE(ob_ide, nodebuff);
-			dnode = find_dev(nodebuff);
+			REGISTER_NAMED_NODE_PHANDLE(ob_ide, nodebuff, dnode);
    		set_int_property(dnode, "reg", j);

/* create aliases */

[PATCH 2/5] libopenbios: introduce new get_path_from_ph() helper function

[Mark Cave-Ayland]

This helper function returns the full device path for the device with the
given phandle as a string.

Signed-off-by: Mark Cave-Ayland 
---
 openbios-devel/include/libopenbios/bindings.h |    1 +
 openbios-devel/libopenbios/bindings.c         |    8 ++++++++
 2 files changed, 9 insertions(+)

diff --git a/openbios-devel/include/libopenbios/bindings.h b/openbios-devel/include/libopenbios/bindings.h
index de9c775..4ec9789 100644
--- a/openbios-devel/include/libopenbios/bindings.h
+++ b/openbios-devel/include/libopenbios/bindings.h
@@ -56,6 +56,7 @@ extern ihandle_t	open_package( const char *argstr, phandle_t ph );
 extern ihandle_t	open_dev( const char *spec );
 extern void		close_package( ihandle_t ih );
 extern void		close_dev( ihandle_t ih );
+extern char		*get_path_from_ph( phandle_t ph );

/* property access */
 extern void		set_property( phandle_t ph, const char *name,
diff --git a/openbios-devel/libopenbios/bindings.c b/openbios-devel/libopenbios/bindings.c
index 5323421..4f7a993 100644
--- a/openbios-devel/libopenbios/bindings.c
+++ b/openbios-devel/libopenbios/bindings.c
@@ -366,6 +366,14 @@ find_dev( const char *path )
    return ret;
 }

+char *
+get_path_from_ph( phandle_t ph )
+{
+	PUSH(ph);
+	fword("get-package-path");
+	return pop_fstr_copy();
+}
+
 phandle_t
 dt_iter_begin( void )
 {

[PATCH 3/5] ide: switch IDE init functions over to use new get_path_from_ph() helper

[Mark Cave-Ayland]

Signed-off-by: Mark Cave-Ayland 
---
 openbios-devel/drivers/ide.c |   10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/openbios-devel/drivers/ide.c b/openbios-devel/drivers/ide.c
index 51f1696..c63ddd8 100644
--- a/openbios-devel/drivers/ide.c
+++ b/openbios-devel/drivers/ide.c
@@ -1466,9 +1466,8 @@ int ob_ide_init(const char *path, uint32_t io_port0, uint32_t ctl_port0,
    				break;
    		}
    		IDE_DPRINTF("%s]: %s\n", media, drive->model);
-                        snprintf(nodebuff, sizeof(nodebuff),
-                                 "%s/" DEV_NAME "/%s", path, current_channel,
-                                 media);
+			snprintf(nodebuff, sizeof(nodebuff), "%s/%s",
+				 get_path_from_ph(dnode), media);
    		REGISTER_NAMED_NODE_PHANDLE(ob_ide, nodebuff, dnode);
    		set_int_property(dnode, "reg", j);

@@ -1686,9 +1685,8 @@ int macio_ide_init(const char *path, uint32_t addr, int nb_channels)
    				break;
    		}
    		IDE_DPRINTF("%s]: %s\n", media, drive->model);
-                        snprintf(nodebuff, sizeof(nodebuff),
-                                 "%s/" DEV_NAME "/%s", path, current_channel,
-                                 media);
+			snprintf(nodebuff, sizeof(nodebuff), "%s/%s",
+				 get_path_from_ph(dnode), media);
    		REGISTER_NAMED_NODE_PHANDLE(ob_ide, nodebuff, dnode);
    		set_int_property(dnode, "reg", j);

[PATCH 4/5] ide: reference IDE channels by phandle, not device name

[Mark Cave-Ayland]

Otherwise when two device nodes have the same name property we are unable
to distinguish between them.

Signed-off-by: Mark Cave-Ayland 
---
 openbios-devel/drivers/ide.c |   20 ++++++++------------
 openbios-devel/drivers/ide.h |    2 +-
 2 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/openbios-devel/drivers/ide.c b/openbios-devel/drivers/ide.c
index c63ddd8..5ebaf5a 100644
--- a/openbios-devel/drivers/ide.c
+++ b/openbios-devel/drivers/ide.c
@@ -73,13 +73,13 @@ static inline void ide_add_channel(struct ide_channel *chan)
    channels = chan;
 }

-static struct ide_channel *ide_seek_channel(const char *name)
+static struct ide_channel *ide_seek_channel(phandle_t ph)
 {
    struct ide_channel *current;

current = channels;
    while (current) {
-		if (!strcmp(current->name, name))
+		if (current->ph == ph)
    		return current;
    	current = current->next;
    }
@@ -1247,11 +1247,10 @@ ob_ide_initialize(int *idx)
 static void
 ob_ide_open(int *idx)
 {
-	int ret=1, len;
+	int ret=1;
    phandle_t ph;
    struct ide_drive *drive;
    struct ide_channel *chan;
-	char *idename;
    int unit;

fword("my-unit");
@@ -1260,9 +1259,8 @@ ob_ide_open(int *idx)
    fword("my-parent");
    fword("ihandle>phandle");
    ph=(phandle_t)POP();
-	idename=get_property(ph, "name", &len);

-	chan = ide_seek_channel(idename);
+	chan = ide_seek_channel(ph);
    drive = &chan->drives[unit];
    *(struct ide_drive **)idx = drive;

@@ -1380,9 +1378,6 @@ int ob_ide_init(const char *path, uint32_t io_port0, uint32_t ctl_port0,

chan = malloc(sizeof(struct ide_channel));

-		snprintf(chan->name, sizeof(chan->name),
-			 DEV_NAME, current_channel);
-
    	chan->mmio = 0;

for (j = 0; j < 8; j++)
@@ -1426,6 +1421,8 @@ int ob_ide_init(const char *path, uint32_t io_port0, uint32_t ctl_port0,
                          current_channel);
    	REGISTER_NAMED_NODE_PHANDLE(ob_ide_ctrl, nodebuff, dnode);

+		chan->ph = dnode;
+
 #if !defined(CONFIG_PPC) && !defined(CONFIG_SPARC64)
    	props[0]=14; props[1]=0;
    	set_property(dnode, "interrupts",
@@ -1552,9 +1549,6 @@ int macio_ide_init(const char *path, uint32_t addr, int nb_channels)

chan = malloc(sizeof(struct ide_channel));

-		snprintf(chan->name, sizeof(chan->name),
-			 DEV_NAME, current_channel);
-
    	chan->mmio = addr + MACIO_IDE_OFFSET + i * MACIO_IDE_SIZE;

chan->obide_inb = macio_ide_inb;
@@ -1594,6 +1588,8 @@ int macio_ide_init(const char *path, uint32_t addr, int nb_channels)
                          current_channel);
    	REGISTER_NAMED_NODE_PHANDLE(ob_ide_ctrl, nodebuff, dnode);

+		chan->ph = dnode;
+
    	set_property(dnode, "compatible", (is_oldworld() ?
    		     "heathrow-ata" : "keylargo-ata"), 13);

diff --git a/openbios-devel/drivers/ide.h b/openbios-devel/drivers/ide.h
index d6c4b9f..8983c8e 100644
--- a/openbios-devel/drivers/ide.h
+++ b/openbios-devel/drivers/ide.h
@@ -167,7 +167,7 @@ struct ide_drive {

struct ide_channel {

-	char name[32];
+	phandle_t ph;
    struct ide_channel *next;

/*

[PATCH 5/5] macio: switch over to use 2 ata-3 ide controllers

[Mark Cave-Ayland]

This better matches real device trees and fixes Darwin/OS X boot from CDROM
under the QEMU mac99 machine (it seems that IOKit will only detect macio IDE
devices connected to an ata-3 ide controller rather than the previous ata-4
ide controller).

Signed-off-by: Mark Cave-Ayland 
---
 openbios-devel/drivers/ide.c |    6 +++---
 openbios-devel/drivers/pci.c |    8 ++++----
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/openbios-devel/drivers/ide.c b/openbios-devel/drivers/ide.c
index 5ebaf5a..1da60c8 100644
--- a/openbios-devel/drivers/ide.c
+++ b/openbios-devel/drivers/ide.c
@@ -1542,10 +1542,10 @@ int macio_ide_init(const char *path, uint32_t addr, int nb_channels)
    struct ide_channel *chan;

/* IDE ports on Macs are numbered from 3.
-	 * Also see comments in macio.c:openpic_init() */
+	 * Also see comments in pci.c:ob_pci_host_set_interrupt_map() */
    current_channel = 3;

-	for (i = 0; i < nb_channels; i++, current_channel++) {
+	for (i = 0; i < nb_channels; i++) {

chan = malloc(sizeof(struct ide_channel));

@@ -1651,7 +1651,7 @@ int macio_ide_init(const char *path, uint32_t addr, int nb_channels)
    	OLDWORLD(set_property(dnode, "AAPL,address",
    			      (char *)&props, 2*sizeof(props[0])));

-		props[0] = 0;
+		props[0] = i;
    	set_property(dnode, "AAPL,bus-id", (char*)props,
    		 1 * sizeof(props[0]));
    	IDE_DPRINTF(DEV_NAME": [io ports 0x%lx]\n",
diff --git a/openbios-devel/drivers/pci.c b/openbios-devel/drivers/pci.c
index 935ecb8..1cb25e6 100644
--- a/openbios-devel/drivers/pci.c
+++ b/openbios-devel/drivers/pci.c
@@ -1430,12 +1430,12 @@ static void ob_pci_host_set_interrupt_map(phandle_t host)
         /* On a new world Mac these are not numbered but named by the
          * ATA version they support. Thus we have: ata-3, ata-3, ata-4
          * On g3beige they all called just ide.
-         * We take ata-3 and ata-4 which seems to work for both
-         * at least for clients we care about */
-        target_node = find_dev("/pci/mac-io/ata-3");
+         * We take 2 x ata-3 buses which seems to work for
+         * at least the clients we care about */
+        target_node = find_dev("/pci/mac-io/ata-3@20000");
         set_int_property(target_node, "interrupt-parent", dnode);

-        target_node = find_dev("/pci/mac-io/ata-4");
+        target_node = find_dev("/pci/mac-io/ata-3@21000");
         set_int_property(target_node, "interrupt-parent", dnode);

target_node = find_dev("/pci/mac-io/via-cuda");

[PATCH 0/5] macio: switch over to use 2 ata-3 controllers

[Programmingkid]

On Dec 24, 2015, at 2:53 PM, Mark Cave-Ayland wrote:
> Further work on Darwin/OS X shows that IOKit expects both the hard disk and 
> cdrom to be attached to the ata-3 controller for QEMU's mac99 machine.
>
> Currently OpenBIOS generates an ata-3 and an ata-4 controller (and by default 
> attaches the cdrom to the ata-4 controller) meaning that IOKit would fail to
> locate and mount the cdrom on boot. Resolve this by instantiating 2 ata-3
> controller nodes instead which also is a better match for a real device tree.
>
> The relevant changes to the device tree are shown below:
>
> Before:
>
> fff542a8 /pci@80000000/mac-io@3/ata-3@20000 (ata)
> fff54540 /pci@80000000/mac-io@3/ata-4@21000 (ata)
> fff547d8 /pci@80000000/mac-io@3/ata-4@21000/cdrom@0 (block)
>
> After:
>
> fff542a8 /pci@80000000/mac-io@3/ata-3@20000 (ata)
> fff54540 /pci@80000000/mac-io@3/ata-3@21000 (ata)
> fff547d8 /pci@80000000/mac-io@3/ata-3@21000/cdrom@0 (block)
>
> The OpenBIOS IDE code currently works by using the device name internally to
> reference the controller node, which of course breaks when 2 nodes are given
> the same name property. Patches 1-4 switch the OpenBIOS IDE code over to
> reference controller nodes by phandle rather than name, while patch 5 does
> the actual switch from 1 ata-3 and 1 ata-4 controller to 2 ata-3 controllers.
>
> Signed-off-by: Mark Cave-Ayland
>
> Mark Cave-Ayland (5):
>  ide: use REGISTER_NAMED_NODE_PHANDLE rather than REGISTER_NAMED_NODE
>  libopenbios: introduce new get_path_from_ph() helper function
>  ide: switch IDE init functions over to use new get_path_from_ph()
>    helper
>  ide: reference IDE channels by phandle, not device name
>  macio: switch over to use 2 ata-3 ide controllers
>
> openbios-devel/drivers/ide.c                  |   46 +++++++++----------------
> openbios-devel/drivers/ide.h                  |    2 +-
> openbios-devel/drivers/pci.c                  |    8 ++---
> openbios-devel/include/libopenbios/bindings.h |    1 +
> openbios-devel/libopenbios/bindings.c         |    8 +++++
> 5 files changed, 31 insertions(+), 34 deletions(-)

This patch set did allow my Darwin iso to boot to the installer screen. It also stops my HD image file from booting if it is set to use the -hdd slot. hda to hdc still work. I guess I could live with the adjustment.

[PATCH 0/5] macio: switch over to use 2 ata-3 controllers

[Alexander Graf]

On 24.12.15 20:53, Mark Cave-Ayland wrote:
> Further work on Darwin/OS X shows that IOKit expects both the hard disk and 
> cdrom to be attached to the ata-3 controller for QEMU's mac99 machine.
>
> Currently OpenBIOS generates an ata-3 and an ata-4 controller (and by default 
> attaches the cdrom to the ata-4 controller) meaning that IOKit would fail to
> locate and mount the cdrom on boot. Resolve this by instantiating 2 ata-3
> controller nodes instead which also is a better match for a real device tree.
>
> The relevant changes to the device tree are shown below:
>
> Before:
>
> fff542a8 /pci@80000000/mac-io@3/ata-3@20000 (ata)
> fff54540 /pci@80000000/mac-io@3/ata-4@21000 (ata)
> fff547d8 /pci@80000000/mac-io@3/ata-4@21000/cdrom@0 (block)
>
> After:
>
> fff542a8 /pci@80000000/mac-io@3/ata-3@20000 (ata)
> fff54540 /pci@80000000/mac-io@3/ata-3@21000 (ata)
> fff547d8 /pci@80000000/mac-io@3/ata-3@21000/cdrom@0 (block)
>
> The OpenBIOS IDE code currently works by using the device name internally to
> reference the controller node, which of course breaks when 2 nodes are given
> the same name property. Patches 1-4 switch the OpenBIOS IDE code over to
> reference controller nodes by phandle rather than name, while patch 5 does
> the actual switch from 1 ata-3 and 1 ata-4 controller to 2 ata-3 controllers.
>
> Signed-off-by: Mark Cave-Ayland

Reviewed-by: Alexander Graf

Alex