Bug in generated paths for CIF's package-to-path?

[Mark Cave-Ayland]

Hi all,

One of the reasons the previous posted patch for setting the keyboard 
devalias has to work with string concatenation is because of this oddity 
in get-package-path (the underlying word used by CIF's package-to-path):

0 > keyboard-phandle @ get-package-path  ok
2 > type /pci@80000000/mac-io@4/via-cuda@16000/adb/keyboard@8 ok
0 > s" /pci@80000000/mac-io@4/via-cuda@16000/adb/keyboard@8" open-dev u. 
0  ok

Hmmmm. So the path we're returning is not valid. Some experimentation 
shows that the problem is due to the trailing @8 on the keyboard node:

0 > s" /pci@80000000/mac-io@4/via-cuda@16000/adb/keyboard" open-dev u. 
7c5b33c  ok

0 > keyboard-phandle @ push-package  ok
0 > .properties
name                      "keyboard"
device_type               "keyboard"
reg                       00000008
  ok

Now AFAICT one of two things is wrong here:

1) get-package-path should be fixed so that if a phandle belongs to a 
leaf node, the unit address is not included as part of the node name

2) open-dev should happily accept the trailing @8 on the keyboard node

Does anyone know which one of these is the correct solution?

Many thanks,

Mark.

[Tarl Neustaedter]

On 2012-Nov-26 05:58 , Mark Cave-Ayland wrote:
> One of the reasons the previous posted patch for setting the keyboard 
> devalias has to work with string concatenation is because of this 
> oddity in get-package-path (the underlying word used by CIF's 
> package-to-path):
>
> 0 > keyboard-phandle @ get-package-path  ok
> 2 > type /pci@80000000/mac-io@4/via-cuda@16000/adb/keyboard@8 ok
> 0 > s" /pci@80000000/mac-io@4/via-cuda@16000/adb/keyboard@8" open-dev 
> u. 0  ok
>
> Hmmmm. So the path we're returning is not valid. Some experimentation 
> shows that the problem is due to the trailing @8 on the keyboard node:
>
> 0 > s" /pci@80000000/mac-io@4/via-cuda@16000/adb/keyboard" open-dev u. 
> 7c5b33c  ok
>
> 0 > keyboard-phandle @ push-package  ok
> 0 > .properties
> name                      "keyboard"
> device_type               "keyboard"
> reg                       00000008
>  ok
>
> Now AFAICT one of two things is wrong here:
> get-package-path should be fixed so that if a phandle belongs to a
>
> leaf node, the unit address is not included as part of the node name
> open-dev should happily accept the trailing @8 on the keyboard node
>
> Does anyone know which one of these is the correct solution?

What does "adb" have for encode-unit and decode-unit? The fact that you 
can open "keyboard" but not "keyboard@8" indicates that the 
encode-unit/decode-unit aren't properly translating between reg property 
and string unit-address.

[Mark Cave-Ayland]

On 26/11/12 16:58, Tarl Neustaedter wrote:
> > Now AFAICT one of two things is wrong here:
> > get-package-path should be fixed so that if a phandle belongs to a
> >
> > leaf node, the unit address is not included as part of the node name
> > open-dev should happily accept the trailing @8 on the keyboard node
> >
> > Does anyone know which one of these is the correct solution?
>
> What does "adb" have for encode-unit and decode-unit? The fact that you
> can open "keyboard" but not "keyboard@8" indicates that the
> encode-unit/decode-unit aren't properly translating between reg property
> and string unit-address.

Hi Tarl,

I've finally worked out what was going on here. The adb device doesn't 
have explicit encode-unit/decode-unit words, and currently if the 
path-resolution algorithm in OpenBIOS can't find one then it switches 
over to a default version based upon a single unit cell.

The reason that opening the keyboard with the trailing @8 didn't work 
was because there was no symmetry; i.e. if decode-unit didn't exist, the 
process terminated without matching rather than translating the single 
unit cell back into a string.

I now have a working patch to implement the opposite for decode-unit 
which doesn't cause regressions on any of my test images, and allows 
direct opening of the keyboard device obtained from get-package-path 
without any extra string manipulation. So unless I find any other 
issues, I'll apply this along with a correction for the previous patch 
over the next couple of days.

ATB,

Mark.