On Wed, 30 Apr 2025, at 21:21, BALATON Zoltan wrote:
> On Wed, 23 Apr 2025, BALATON Zoltan wrote:
> > The FDT does not normally store name properties but reconstructs it
> > from path but Open Firmware specification says each node should at
> > least have this property. This is correctly handled in getprop but
> > nextprop should also return it even if not present as a property.
> >
> > Explicit name properties are still allowed because they are needed
> > e.g. on the root node that guests expect to have specific names as
> > seen on real machines instead of being empty so sometimes the node
> > name may need to be overriden. For example on pegasos MorphOS checks
> > the name of "/" and expects to find bplan,Pegasos2 which is how it
> > identifies the machine.
> >
> > Signed-off-by: BALATON Zoltan <bala...@eik.bme.hu>
> > ---
> > v3:
> > Keep phandle properties
> > Changed commit message
>
> Ping?
>
sorry for the delay, looks okay to me but (probably was answered but I forgot
because I am slow) I still do not understand what is adding the explicit
property called "name" so vof_nextprop() needs to check if it is the actual
property. Thanks,
> Regards,
> BALATON Zoltan
>
> > v2:
> > Fixed a typo in commit message
> > Simplified loop to get next property name
> >
> > hw/ppc/vof.c | 50 +++++++++++++++++++++++++++++++++-----------------
> > 1 file changed, 33 insertions(+), 17 deletions(-)
> >
> > diff --git a/hw/ppc/vof.c b/hw/ppc/vof.c
> > index 09cb77de93..10bafd011e 100644
> > --- a/hw/ppc/vof.c
> > +++ b/hw/ppc/vof.c
> > @@ -353,34 +353,50 @@ static uint32_t vof_nextprop(const void *fdt,
> > uint32_t phandle,
> > {
> > int offset, nodeoff = fdt_node_offset_by_phandle(fdt, phandle);
> > char prev[OF_PROPNAME_LEN_MAX + 1];
> > - const char *tmp;
> > + const char *tmp = NULL;
> > + bool match = false;
> >
> > if (readstr(prevaddr, prev, sizeof(prev))) {
> > return PROM_ERROR;
> > }
> > -
> > - fdt_for_each_property_offset(offset, fdt, nodeoff) {
> > - if (!fdt_getprop_by_offset(fdt, offset, &tmp, NULL)) {
> > - return 0;
> > + /*
> > + * "name" may or may not be present in fdt but we should still return
> > it.
> > + * Do that first and then skip it if seen later.
> > + */
> > + if (prev[0] == '\0') {
> > + tmp = "name";
> > + } else {
> > + if (strcmp(prev, "name") == 0) {
> > + prev[0] = '\0';
> > }
> > - if (prev[0] == '\0' || strcmp(prev, tmp) == 0) {
> > - if (prev[0] != '\0') {
> > - offset = fdt_next_property_offset(fdt, offset);
> > - if (offset < 0) {
> > - return 0;
> > - }
> > - }
> > + fdt_for_each_property_offset(offset, fdt, nodeoff) {
> > if (!fdt_getprop_by_offset(fdt, offset, &tmp, NULL)) {
> > return 0;
> > }
> > -
> > - if (VOF_MEM_WRITE(nameaddr, tmp, strlen(tmp) + 1) != MEMTX_OK)
> > {
> > - return PROM_ERROR;
> > + if (strcmp(tmp, "name") == 0) {
> > + continue;
> > + }
> > + if (match) {
> > + break;
> > }
> > - return 1;
> > + if (strcmp(prev, tmp) == 0) {
> > + match = true;
> > + continue;
> > + }
> > + if (prev[0] == '\0') {
> > + break;
> > + }
> > + }
> > + if (offset < 0) {
> > + return 0;
> > }
> > }
> > -
> > + if (tmp) {
> > + if (VOF_MEM_WRITE(nameaddr, tmp, strlen(tmp) + 1) != MEMTX_OK) {
> > + return PROM_ERROR;
> > + }
> > + return 1;
> > + }
> > return 0;
> > }
> >
> >
>
