Re: [PATCH v14 11/11] arm_ffa: efi: corstone1000: enable MM communication

Tom Rini
2023-07-10T16:18:37Z

[-- Attachment #1: Type: text/plain, Size: 1574 bytes --]

On Mon, Jul 10, 2023 at 04:03:22PM +0100, Abdellatif El Khlifi wrote:
> Hi Simon, Tom,
>
> > > > > #include <linux/sizes.h>
> > > > >
> > > > > +#define FFA_SHARED_MM_BUFFER_SIZE SZ_4K /* 4 KB */
> > > > > +
> > > > > +/*
> > > > > + * shared buffer physical address used for communication between
> > > > > + * u-boot and the MM SP
> > > > > + */
> > > > > +#define FFA_SHARED_MM_BUFFER_ADDR 0x02000000UL
> > > > > +#define FFA_SHARED_MM_BUFFER_OFFSET 0
> > > > > +
> > > >
> > > > These should be in devicetree properties, shouldn't they? We don't
> > > > want things in board config.h files anymore.
> > >
> > > Wasn't one of the earlier debates on if the whole thing needs to be in
> > > device tree, or not, because it's a "discoverable bus" and so Linux
> > > wasn't needing one? I don't want to de-rail this series, which I think
> > > we're just about otherwise ready to merge, over that debate again.
> > >
> > > As these values have to match up (I assume) with the other side, are
> > > these truly per-board, or SoC dependent?
> >
> > Yes let's not derail the series...it has gone on too long already.
> >
> > But we do need to think about this. Addresses in #defines are not a
> > great look. It isn't even a CFG.
> >
>
> I think replacing the FFA_SHARED_MM_BUFFER_* defines with
> a config makes sense.
>
> In v15 I'll add these as configs if you think guys it's appropriate:
>
> FFA_SHARED_MM_BUFFER_SIZE
> FFA_SHARED_MM_BUFFER_ADDR
> FFA_SHARED_MM_BUFFER_OFFSET

Sounds good, thanks!

--
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]