On Sat, 11 Mar 2023, Mark Cave-Ayland wrote:
According to the IEEE-1275 specification the encode-unit and decode-unit words are static methods which mean they can be called without a current instance.
The PCI bus bindings explicitly mention that the decode-unit methods should insert the bus number into the numerical representation of the bus, a value which is known by the package when the node is created.
In order for this to work when encode-unit and decode-unit are called externally it is necessary to hold a copy of the phandle within the package that can be accessed from C in order to obtain the bus number.
Use the new BIND_NODE_STATIC_METHOD macro to convert encode-unit and decode-unit to static methods, which allows the node to use its own phandle to determine the PCI bus number rather than using the active package.
This allows executing decode-unit from a different package (as occurs in set-args where the active package is the child node of the bus parent, and not the bus node itself) to return the correct unit value for PCI devices.
Signed-off-by: Mark Cave-Ayland mark.cave-ayland@ilande.co.uk
drivers/pci.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/pci.c b/drivers/pci.c index f30e427..6c1ce66 100644 --- a/drivers/pci.c +++ b/drivers/pci.c @@ -190,7 +190,7 @@ ob_pci_close(int *idx) /* ( str len -- phys.lo phys.mid phys.hi ) */
static void -ob_pci_decode_unit(int *idx) +ob_pci_decode_unit(ucell ph) { ucell hi, mid, lo; const char *arg = pop_fstr_copy(); @@ -287,7 +287,7 @@ ob_pci_decode_unit(int *idx) } free((char*)arg);
- bus = get_int_property(get_cur_dev(), "bus-range", &len);
- bus = get_int_property(ph, "bus-range", &len);
I still think this series should not need the first two patches and there must be a simpler way to do this so this is where I start from. I've checked what openfirmware and openboot does and their pcibus encode-unit and decode-unit are the same as these only in forth, except they don't mess with bus-range. That means we don't need to convert these to Forth (although that may still reduce the cultter in pci.c but since we already have these here they can stay) but this line should be removed and be done the same way as openboot does instead which seems to be the following. See:
https://github.com/openbios/openboot/blob/master/obp/dev/pci-bridge/pcinode....
The bridge has a different decode-unit method which calls this method in the parent then adds the bus number to that. So what I think this patch should do instead is to remove this line and copy those methods from openboot to pci.fs such as:
0 value my-bus# defer parent-decode-unit : decode-unit ( adr len -- phys.lo..hi ) parent-decode-unit lwsplit drop my-bus# wljoin ; defer encode-unit ( phys.lo..hi -- adr len ) \ decode-unit and encode-unit must be static methods, so they can't use \ $call-parent at run-time
" decode-unit" my-parent ihandle>phandle find-method drop ( xt ) to parent-decode-unit
" encode-unit" my-parent ihandle>phandle find-method drop ( xt ) to encode-unit
and use these as the bridge methods. I'm still not sure how to install these in the bridge node but maybe the above should be within an is-pci-brodge word which is called to define these when the bridge node is created passing the bus number to it. What do you thing about that?
Regards, BALATON Zoltan
hi = n | p | t | (ss << 24) | (bus << 16) | (dev << 11) | (fn << 8) | reg;
@@ -303,7 +303,7 @@ ob_pci_decode_unit(int *idx) /* ( phys.lo phy.mid phys.hi -- str len ) */
static void -ob_pci_encode_unit(int *idx) +ob_pci_encode_unit(ucell ph) { char buf[28]; cell hi = POP(); @@ -457,8 +457,6 @@ ob_pci_dma_sync(int *idx) NODE_METHODS(ob_pci_bus_node) = { { "open", ob_pci_open }, { "close", ob_pci_close },
- { "decode-unit", ob_pci_decode_unit },
- { "encode-unit", ob_pci_encode_unit }, { "pci-map-in", ob_pci_bus_map_in }, { "dma-alloc", ob_pci_dma_alloc }, { "dma-free", ob_pci_dma_free },
@@ -479,8 +477,6 @@ ob_pci_bridge_map_in(int *idx) NODE_METHODS(ob_pci_bridge_node) = { { "open", ob_pci_open }, { "close", ob_pci_close },
- { "decode-unit", ob_pci_decode_unit },
- { "encode-unit", ob_pci_encode_unit }, { "pci-map-in", ob_pci_bridge_map_in }, { "dma-alloc", ob_pci_dma_alloc }, { "dma-free", ob_pci_dma_free },
@@ -1834,6 +1830,9 @@ static phandle_t ob_configure_pci_device(const char* parent_path, } else { BIND_NODE_METHODS(phandle, ob_pci_bus_node); }
BIND_NODE_STATIC_METHOD(phandle, "encode-unit", ob_pci_encode_unit);BIND_NODE_STATIC_METHOD(phandle, "decode-unit", ob_pci_decode_unit); break;