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

Abdellatif El Khlifi
2023-07-10T15:03:35Z

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

Cheers
Abdellatif