Re: [PATCH 1/2] docs/system/arm: Fix for rename of type "xlnx.bbram-ctrl"

[Markus Armbruster]

Thomas Huth <thuth@redhat.com> writes:

> On 13/11/2023 14.43, Markus Armbruster wrote:
>> Fixes: b65b4b7ae3c8 (xlnx-bbram: hw/nvram: Use dot in device type name)
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>   docs/system/arm/xlnx-versal-virt.rst | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>> diff --git a/docs/system/arm/xlnx-versal-virt.rst 
>> b/docs/system/arm/xlnx-versal-virt.rst
>> index d2d1b26692..a6a77b3799 100644
>> --- a/docs/system/arm/xlnx-versal-virt.rst
>> +++ b/docs/system/arm/xlnx-versal-virt.rst
>> @@ -194,7 +194,7 @@ To use a different index value, N, from default of 0, 
>> add:
>>     .. code-block:: bash
>>   -  -global xlnx,bbram-ctrl.drive-index=N
>> +  -global xlnx.bbram-ctrl.drive-index=N
>
> Ouch, that's now ugly, too. Imagine that we have a device called "xlnx" one 
> day, how's the reader supposed to distinguish between the "xlnx" and the 
> "xlnx.bbram-ctrl" device here?

Hmm, it's actually worse than ugly: it doesn't work.

    $ qemu-system-aarch64 -nodefaults -S -display none -M none -global 
xlnx.bbram-ctrl.drive-index=2
    qemu-system-aarch64: warning: global xlnx.bbram-ctrl.drive-index has 
invalid class name

That's because xlnx.bbram-ctrl.drive-index=N is syntactically ambiguous: it
could be sugar for

    driver=xlnx.bbram-ctrl,property=drive-index,value=N

or for

    driver=xlnx,property=bbram-ctrl.drive-index,value=N

Our parser picks the latter.

I'll respin the patch to use longhand syntax.

> It feels like we should forbid both, "," and "." in device names...

Yes, '.' is a bad idea, too.  But there are many names with them, and
I'm not ready to tackle them.

> Anyway, for the current state:
> Reviewed-by: Thomas Huth <thuth@redhat.com>

Thanks!