Re: [PATCH 01/19] target/microblaze: Rename CPU endianness property as 'little-endian'
On Tue, Nov 05, 2024 at 11:18:31PM +0000, Philippe Mathieu-Daudé wrote:
On 5/11/24 23:01, Philippe Mathieu-Daudé wrote:
> Hi Edgar,
>
> On 5/11/24 23:54, Edgar E. Iglesias wrote:
> > On Tue, Nov 05, 2024 at 02:04:13PM +0100, Philippe Mathieu-Daudé wrote:
> > > Rename the 'endian' property as 'little-endian' because
the 'ENDI'
> > > bit is set when the endianness is in little order, and unset in
> > > big order.
> >
> > Hi Phil,
> >
> > Unfortunately, these properties are not only QEMU internal these got
> > named
> > from the bindings Xilinx choose way back in time.
> >
> > This will likely break many of the Xilinx flows with automatic dts to
> > qemu property conversions so I don't think it's a good idea to rename
it.
> > If you like to clarify things perhaps we could keep an alias for the old
> > one?
>
> Adding an alias is the safest way, I'll respin this patch.
>
> Note however I'm worried about this fragile disconnect between Xilinx
> dts conversion which isn't exercised on mainstream (in particular if
> you get busy and can't review).
>
> >
> > For example:
> >
https://github.com/torvalds/linux/blob/master/arch/microblaze/boot/dts/sy...
> >
> > Cheers,
> > Edgar
> >
> >
> > >
> > > Signed-off-by: Philippe Mathieu-Daudé <philmd(a)linaro.org>
> > > ---
> > > hw/microblaze/petalogix_ml605_mmu.c | 2 +-
> > > hw/microblaze/xlnx-zynqmp-pmu.c | 2 +-
> > > target/microblaze/cpu.c | 2 +-
> > > 3 files changed, 3 insertions(+), 3 deletions(-)
> > > diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
> > > index 135947ee800..e9f98806274 100644
> > > --- a/target/microblaze/cpu.c
> > > +++ b/target/microblaze/cpu.c
> > > @@ -368,7 +368,7 @@ static Property mb_properties[] = {
> > > DEFINE_PROP_UINT8("use-non-secure", MicroBlazeCPU,
> > > cfg.use_non_secure, 0),
> > > DEFINE_PROP_BOOL("dcache-writeback", MicroBlazeCPU,
> > > cfg.dcache_writeback,
> > > false),
> > > - DEFINE_PROP_BOOL("endianness", MicroBlazeCPU, cfg.endi,
false),
> > > + DEFINE_PROP_BOOL("little-endian", MicroBlazeCPU, cfg.endi,
false),
> > > /* Enables bus exceptions on failed data accesses
> > > (load/stores). */
> > > DEFINE_PROP_BOOL("dopb-bus-exception", MicroBlazeCPU,
> > > cfg.dopb_bus_exception, false),
> > > --
OK if I squash the following?
Looks good!
Thanks!
Reviewed-by: Edgar E. Iglesias <edgar.iglesias(a)amd.com>
>
> -- >8 --
> diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
> index e9f98806274..b322f060777 100644
> --- a/target/microblaze/cpu.c
> +++ b/target/microblaze/cpu.c
> @@ -328,9 +328,16 @@ static void mb_cpu_initfn(Object *obj)
> qdev_init_gpio_in_named(DEVICE(cpu), mb_cpu_ns_axi_dc, "ns_axi_dc",
1);
> qdev_init_gpio_in_named(DEVICE(cpu), mb_cpu_ns_axi_ic, "ns_axi_ic",
1);
> #endif
> +
> + /* Restricted 'endianness' property is equivalent of
'little-endian' */
> + object_property_add_alias(obj, "little-endian", obj,
"endianness");
> }
>
> static Property mb_properties[] = {
> + /*
> + * Following properties are used by Xilinx DTS conversion tool
> + * do not rename them.
> + */
> DEFINE_PROP_UINT32("base-vectors", MicroBlazeCPU, cfg.base_vectors,
0),
> DEFINE_PROP_BOOL("use-stack-protection", MicroBlazeCPU,
cfg.stackprot,
> false),
> @@ -368,7 +375,7 @@ static Property mb_properties[] = {
> DEFINE_PROP_UINT8("use-non-secure", MicroBlazeCPU,
cfg.use_non_secure,
> 0),
> DEFINE_PROP_BOOL("dcache-writeback", MicroBlazeCPU,
> cfg.dcache_writeback,
> false),
> - DEFINE_PROP_BOOL("little-endian", MicroBlazeCPU, cfg.endi, false),
> + DEFINE_PROP_BOOL("endianness", MicroBlazeCPU, cfg.endi, false),
> /* Enables bus exceptions on failed data accesses (load/stores). */
> DEFINE_PROP_BOOL("dopb-bus-exception", MicroBlazeCPU,
> cfg.dopb_bus_exception, false),
> @@ -387,6 +394,9 @@ static Property mb_properties[] = {
> DEFINE_PROP_UINT8("pvr", MicroBlazeCPU, cfg.pvr, C_PVR_FULL),
> DEFINE_PROP_UINT8("pvr-user1", MicroBlazeCPU, cfg.pvr_user1, 0),
> DEFINE_PROP_UINT32("pvr-user2", MicroBlazeCPU, cfg.pvr_user2, 0),
> + /*
> + * End of properties reserved by Xilinx DTS conversion tool.
> + */
> DEFINE_PROP_END_OF_LIST(),
> };
>
> ---