Re: [DistroKit] [PATCH 1/1] v8a: Add support for tq-ma93xxca

[Fabian Pflug <f.pflug@pengutronix.de>]

Hello Ahmad,

> Nitpick: chosen, random

Fixed

> 
> > +CONFIG_HAB=y
> > +CONFIG_AHAB=y
> 
> I think we should leave this disabled, until we start actually doing
> verified boot in DistroKit.

CONFIG_IMX93 sets CONFIG_AHAB, which in turn sets CONFIG_HAB
Not really possible to disable.

> 
> > +CONFIG_USB_EHCI=y
> 
> Interesting. I thought the i.MX93 would use a USB 3.0 capable
> controller
> like the i.MX8MP, but apparently it's not so.

Nope, USB2.0 only


> 
> > diff --git a/configs/platform-v8a/config/images/imx93.config
> > b/configs/platform-v8a/config/images/imx93.config
> > new file mode 100644
> > index 0000000..bbdb232
> > --- /dev/null
> > +++ b/configs/platform-v8a/config/images/imx93.config
> > @@ -0,0 +1,21 @@
> > +image @IMAGE@ {
> > +       hdimage {
> > +               align = 1M
> > +               partition-table-type = gpt
> > +       }
> > +       partition barebox {
> > +               image = "@BAREBOX_IMAGE@"
> > +               offset = 32k
> > +               in-partition-table = false
> 
> Why not make it a proper GPT partition? I know we are not consistent
> with that in existing boards, but for new board support, we should
> put
> everything into a GPT partition from the get-go.

I have no opinion about it. It was just copy & paste from all the other
boards. Will do what the majority agrees upon.

> 
> > +       }
> > +       partition barebox-state {
> 
> Please add a barebox-environment partition as well with a comment
> that
> it's currently unused as the environment comes from the SPI flash.
> 
> That way, we'll be able to just switch over to using environment on
> the
> boot medium in future without changing the partition layout again.
> 
> > +               offset = 3M
> > +               size = 1M
> > +               partition-type-uuid = barebox-state
> > +       }
> > +       partition root-A {
> > +               offset = 4M
> 
> If everything has a size and is described in the partition table, do
> we still need the offset (except of course for the barebox
> partition).
> 
> > +Boot Media
> > +----------
> > +
> > +Booting is available via multiple sources (Compare Table 6: Boot
> > configuration i.MX 93 from .
> 
> from? missing rest of line?

Good catch. Yeah missing.

> 
> 
> > +Write the bootloader ``platform-v8a/images/barebox-tqma93xx.img``
> > to USB:
> 
> s/Write to/Upload via/
> 
> Maybe even add the full command:
> 
>   imx-usb-loader platform-v8a/images/barebox-tqma93xx.img

See next line on the RST:

> 
> > +
> > +.. code-block:: shell
> > +
> > +    platform-v8a/sysroot-host/bin/imx-usb-loader platform-
> > v8a/images/barebox-tqma93xx.img
> > +
> > +In Barebox, set the default boot location to nfs:
> > +
> > +    nv boot.default:
> > nfs://dude06//ptx/work/user/fpg/DistroKit/platform-v8a/root
> 
> Drop the colon, so it's an executable command.

Done

> 
> Thanks for including docs.

You're welcome :)

Will send a new patch later, with your and other remarks.

Fabian

> 
> Cheers,
> Ahmad
>