[Qemu-devel] [PATCH v1 1/1] zynq_slcr: Change the comma to a underscore

[Qemu-devel] [PATCH v1 1/1] zynq_slcr: Change the comma to a underscore

[Alistair Francis]

This patch changes the comma in the "xilinx,zynq_slcr" to an
underscore. This matches every other xilinx* peripheral and
also makes parsing the device via the command line possible.

Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
---

 hw/arm/xilinx_zynq.c |    2 +-
 hw/misc/zynq_slcr.c  |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
index 9ee21e7..f717645 100644
--- a/hw/arm/xilinx_zynq.c
+++ b/hw/arm/xilinx_zynq.c
@@ -167,7 +167,7 @@ static void zynq_init(QEMUMachineInitArgs *args)
                           1, 0x0066, 0x0022, 0x0000, 0x0000, 0x0555, 0x2aa,
                               0);
 
-    dev = qdev_create(NULL, "xilinx,zynq_slcr");
+    dev = qdev_create(NULL, "xilinx_zynq_slcr");
     qdev_init_nofail(dev);
     sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0xF8000000);
 
diff --git a/hw/misc/zynq_slcr.c b/hw/misc/zynq_slcr.c
index d1cc233..8aba715 100644
--- a/hw/misc/zynq_slcr.c
+++ b/hw/misc/zynq_slcr.c
@@ -116,7 +116,7 @@ typedef enum {
   RESET_MAX
 } ResetValues;
 
-#define TYPE_ZYNQ_SLCR "xilinx,zynq_slcr"
+#define TYPE_ZYNQ_SLCR "xilinx_zynq_slcr"
 #define ZYNQ_SLCR(obj) OBJECT_CHECK(ZynqSLCRState, (obj), TYPE_ZYNQ_SLCR)
 
 typedef struct ZynqSLCRState {
-- 
1.7.1

Re: [Qemu-devel] [PATCH v1 1/1] zynq_slcr: Change the comma to a underscore

[Peter Crosthwaite]

On Wed, Mar 26, 2014 at 1:05 PM, Alistair Francis
<alistair.francis@xilinx.com> wrote:
> This patch changes the comma in the "xilinx,zynq_slcr" to an
> underscore. This matches every other xilinx* peripheral and
> also makes parsing the device via the command line possible.
>

I think its actually a case of this being slightly ahead and everyone
else being behind. The comma is probably ultimately wrong and I'm
guessing its awkward for your command-line work due to command line
character escaping. I am in favor of the "xlnx.foo" styling that is
more widely adopted:

[qemu]$ git grep -c "xlnx\."
hw/arm/xilinx_zynq.c:1
hw/char/xilinx_uartlite.c:2
hw/dma/xilinx_axidma.c:2
hw/intc/xilinx_intc.c:2
hw/microblaze/petalogix_ml605_mmu.c:6
hw/microblaze/petalogix_s3adsp1800_mmu.c:5
hw/net/xilinx_axienet.c:3
hw/net/xilinx_ethlite.c:2
hw/ppc/virtex_ml507.c:2
hw/ssi/xilinx_spi.c:1
hw/ssi/xilinx_spips.c:2
hw/timer/xilinx_timer.c:2
target-microblaze/cpu.c:1

will xlnx.zynq-slcr work? (fix the underscore while at it).

Regards,
Peter

> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
> ---
>
>  hw/arm/xilinx_zynq.c |    2 +-
>  hw/misc/zynq_slcr.c  |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
> index 9ee21e7..f717645 100644
> --- a/hw/arm/xilinx_zynq.c
> +++ b/hw/arm/xilinx_zynq.c
> @@ -167,7 +167,7 @@ static void zynq_init(QEMUMachineInitArgs *args)
>                            1, 0x0066, 0x0022, 0x0000, 0x0000, 0x0555, 0x2aa,
>                                0);
>
> -    dev = qdev_create(NULL, "xilinx,zynq_slcr");
> +    dev = qdev_create(NULL, "xilinx_zynq_slcr");
>      qdev_init_nofail(dev);
>      sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0xF8000000);
>
> diff --git a/hw/misc/zynq_slcr.c b/hw/misc/zynq_slcr.c
> index d1cc233..8aba715 100644
> --- a/hw/misc/zynq_slcr.c
> +++ b/hw/misc/zynq_slcr.c
> @@ -116,7 +116,7 @@ typedef enum {
>    RESET_MAX
>  } ResetValues;
>
> -#define TYPE_ZYNQ_SLCR "xilinx,zynq_slcr"
> +#define TYPE_ZYNQ_SLCR "xilinx_zynq_slcr"
>  #define ZYNQ_SLCR(obj) OBJECT_CHECK(ZynqSLCRState, (obj), TYPE_ZYNQ_SLCR)
>
>  typedef struct ZynqSLCRState {
> --
> 1.7.1
>
>

Re: [Qemu-devel] [PATCH v1 1/1] zynq_slcr: Change the comma to a underscore

[Alistair Francis]

On Wed, Apr 9, 2014 at 11:14 AM, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> On Wed, Mar 26, 2014 at 1:05 PM, Alistair Francis
> <alistair.francis@xilinx.com> wrote:
>> This patch changes the comma in the "xilinx,zynq_slcr" to an
>> underscore. This matches every other xilinx* peripheral and
>> also makes parsing the device via the command line possible.
>>
>
> I think its actually a case of this being slightly ahead and everyone
> else being behind. The comma is probably ultimately wrong and I'm
> guessing its awkward for your command-line work due to command line
> character escaping. I am in favor of the "xlnx.foo" styling that is
> more widely adopted:
>
> [qemu]$ git grep -c "xlnx\."
> hw/arm/xilinx_zynq.c:1
> hw/char/xilinx_uartlite.c:2
> hw/dma/xilinx_axidma.c:2
> hw/intc/xilinx_intc.c:2
> hw/microblaze/petalogix_ml605_mmu.c:6
> hw/microblaze/petalogix_s3adsp1800_mmu.c:5
> hw/net/xilinx_axienet.c:3
> hw/net/xilinx_ethlite.c:2
> hw/ppc/virtex_ml507.c:2
> hw/ssi/xilinx_spi.c:1
> hw/ssi/xilinx_spips.c:2
> hw/timer/xilinx_timer.c:2
> target-microblaze/cpu.c:1
>
> will xlnx.zynq-slcr work? (fix the underscore while at it).

Full stops are fine, just as long as it is not a comma

>
> Regards,
> Peter
>
>> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
>> ---
>>
>>  hw/arm/xilinx_zynq.c |    2 +-
>>  hw/misc/zynq_slcr.c  |    2 +-
>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
>> index 9ee21e7..f717645 100644
>> --- a/hw/arm/xilinx_zynq.c
>> +++ b/hw/arm/xilinx_zynq.c
>> @@ -167,7 +167,7 @@ static void zynq_init(QEMUMachineInitArgs *args)
>>                            1, 0x0066, 0x0022, 0x0000, 0x0000, 0x0555, 0x2aa,
>>                                0);
>>
>> -    dev = qdev_create(NULL, "xilinx,zynq_slcr");
>> +    dev = qdev_create(NULL, "xilinx_zynq_slcr");
>>      qdev_init_nofail(dev);
>>      sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0xF8000000);
>>
>> diff --git a/hw/misc/zynq_slcr.c b/hw/misc/zynq_slcr.c
>> index d1cc233..8aba715 100644
>> --- a/hw/misc/zynq_slcr.c
>> +++ b/hw/misc/zynq_slcr.c
>> @@ -116,7 +116,7 @@ typedef enum {
>>    RESET_MAX
>>  } ResetValues;
>>
>> -#define TYPE_ZYNQ_SLCR "xilinx,zynq_slcr"
>> +#define TYPE_ZYNQ_SLCR "xilinx_zynq_slcr"
>>  #define ZYNQ_SLCR(obj) OBJECT_CHECK(ZynqSLCRState, (obj), TYPE_ZYNQ_SLCR)
>>
>>  typedef struct ZynqSLCRState {
>> --
>> 1.7.1
>>
>>
>

Re: [Qemu-devel] [PATCH v1 1/1] zynq_slcr: Change the comma to a underscore

[Markus Armbruster]

Alistair Francis <alistair.francis@xilinx.com> writes:

> On Wed, Apr 9, 2014 at 11:14 AM, Peter Crosthwaite
> <peter.crosthwaite@xilinx.com> wrote:
>> On Wed, Mar 26, 2014 at 1:05 PM, Alistair Francis
>> <alistair.francis@xilinx.com> wrote:
>>> This patch changes the comma in the "xilinx,zynq_slcr" to an
>>> underscore. This matches every other xilinx* peripheral and
>>> also makes parsing the device via the command line possible.
>>>
>>
>> I think its actually a case of this being slightly ahead and everyone
>> else being behind. The comma is probably ultimately wrong and I'm
>> guessing its awkward for your command-line work due to command line
>> character escaping. I am in favor of the "xlnx.foo" styling that is
>> more widely adopted:
>>
>> [qemu]$ git grep -c "xlnx\."
>> hw/arm/xilinx_zynq.c:1
>> hw/char/xilinx_uartlite.c:2
>> hw/dma/xilinx_axidma.c:2
>> hw/intc/xilinx_intc.c:2
>> hw/microblaze/petalogix_ml605_mmu.c:6
>> hw/microblaze/petalogix_s3adsp1800_mmu.c:5
>> hw/net/xilinx_axienet.c:3
>> hw/net/xilinx_ethlite.c:2
>> hw/ppc/virtex_ml507.c:2
>> hw/ssi/xilinx_spi.c:1
>> hw/ssi/xilinx_spips.c:2
>> hw/timer/xilinx_timer.c:2
>> target-microblaze/cpu.c:1
>>
>> will xlnx.zynq-slcr work? (fix the underscore while at it).
>
> Full stops are fine, just as long as it is not a comma

The most common separator in device model names is '-'.  There's a fair
number of '.', some '_', and a few ','.

The comma are probably rooted in device tree usage.  I doubt that buys
us anything but confusing command line trouble.

I suspect period breaks -global.

I very much recommend picking '-' whenever practical.

Re: [Qemu-devel] [PATCH v1 1/1] zynq_slcr: Change the comma to a underscore

[Markus Armbruster]

Markus Armbruster <armbru@redhat.com> writes:

> Alistair Francis <alistair.francis@xilinx.com> writes:
>
>> On Wed, Apr 9, 2014 at 11:14 AM, Peter Crosthwaite
>> <peter.crosthwaite@xilinx.com> wrote:
>>> On Wed, Mar 26, 2014 at 1:05 PM, Alistair Francis
>>> <alistair.francis@xilinx.com> wrote:
>>>> This patch changes the comma in the "xilinx,zynq_slcr" to an
>>>> underscore. This matches every other xilinx* peripheral and
>>>> also makes parsing the device via the command line possible.
>>>>
>>>
>>> I think its actually a case of this being slightly ahead and everyone
>>> else being behind. The comma is probably ultimately wrong and I'm
>>> guessing its awkward for your command-line work due to command line
>>> character escaping. I am in favor of the "xlnx.foo" styling that is
>>> more widely adopted:
>>>
>>> [qemu]$ git grep -c "xlnx\."
>>> hw/arm/xilinx_zynq.c:1
>>> hw/char/xilinx_uartlite.c:2
>>> hw/dma/xilinx_axidma.c:2
>>> hw/intc/xilinx_intc.c:2
>>> hw/microblaze/petalogix_ml605_mmu.c:6
>>> hw/microblaze/petalogix_s3adsp1800_mmu.c:5
>>> hw/net/xilinx_axienet.c:3
>>> hw/net/xilinx_ethlite.c:2
>>> hw/ppc/virtex_ml507.c:2
>>> hw/ssi/xilinx_spi.c:1
>>> hw/ssi/xilinx_spips.c:2
>>> hw/timer/xilinx_timer.c:2
>>> target-microblaze/cpu.c:1
>>>
>>> will xlnx.zynq-slcr work? (fix the underscore while at it).
>>
>> Full stops are fine, just as long as it is not a comma
>
> The most common separator in device model names is '-'.  There's a fair
> number of '.', some '_', and a few ','.
>
> The comma are probably rooted in device tree usage.  I doubt that buys
> us anything but confusing command line trouble.
>
> I suspect period breaks -global.

Yes, it does: "-global cfi.pflash01.name=foo" gets parsed as driver =
"cfi", property = "pflash01.name", value = "foo".  I'm going to start a
new thread for this issue.

> I very much recommend picking '-' whenever practical.

Still do :)