Re: [PATCH 01/19] target/microblaze: Rename CPU endianness property as 'little-endian'

["Edgar E. Iglesias" <edgar.iglesias@gmail.com>]

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/system.dts#L73
> > > 
> > > Cheers,
> > > Edgar
> > > 
> > > 
> > > > 
> > > > Signed-off-by: Philippe Mathieu-Daudé <philmd@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@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(),
>  };
> 
> ---