Hi Leif,
pon., 21 sty 2019 o 12:51 Leif Lindholm <leif.lindh...@linaro.org> napisał(a):
>
> On Mon, Jan 21, 2019 at 11:52:11AM +0100, Marcin Wojtas wrote:
> > From: Grzegorz Jaszczyk <j...@semihalf.com>
> >
> > The memory controller registers are marked as secure in the latest
> > ARM-TF for Armada SoCs. It is available however get the DRAM
> > information via SiP services in the EL3, so use it instead
> > of accessing the registers directly.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Marcin Wojtas <m...@semihalf.com>
> > ---
> > Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLib.inf | 3 ++
> > Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.h | 27
> > ++--------
> > Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c | 55
> > +++++---------------
> > 3 files changed, 21 insertions(+), 64 deletions(-)
> >
> > diff --git
> > a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLib.inf
> > b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLib.inf
> > index e888566..0c7f320 100644
> > --- a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLib.inf
> > +++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLib.inf
> > @@ -41,12 +41,15 @@
> > [Packages]
> > ArmPkg/ArmPkg.dec
> > ArmPlatformPkg/ArmPlatformPkg.dec
> > + EmbeddedPkg/EmbeddedPkg.dec
> > MdeModulePkg/MdeModulePkg.dec
> > MdePkg/MdePkg.dec
> > Silicon/Marvell/Marvell.dec
> >
> > [LibraryClasses]
> > + ArmadaSoCDescLib
> > ArmLib
> > + ArmSmcLib
> > DebugLib
> > MemoryAllocationLib
> > MppLib
> > diff --git
> > a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.h
> > b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.h
> > index cc30e4a..8a46df6 100644
> > --- a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.h
> > +++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.h
> > @@ -47,27 +47,8 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
> > DAMAGE.
> > #define DRAM_REMAP_TARGET \
> > (MmioRead32 (CCU_MC_RTBR_REG) << TARGET_BASE_OFFS)
> >
> > -#define DRAM_CH0_MMAP_LOW_REG(cs) (0xf0020200 + (cs) * 0x8)
> > -#define DRAM_CS_VALID_ENABLED_MASK 0x1
> > -#define DRAM_AREA_LENGTH_OFFS 16
> > -#define DRAM_AREA_LENGTH_MASK (0x1f << DRAM_AREA_LENGTH_OFFS)
> > -#define DRAM_START_ADDRESS_L_OFFS 23
> > -#define DRAM_START_ADDRESS_L_MASK (0x1ff <<
> > DRAM_START_ADDRESS_L_OFFS)
> > -#define DRAM_CH0_MMAP_HIGH_REG(cs) (0xf0020204 + (cs) * 0x8)
> > -#define DRAM_START_ADDR_HTOL_OFFS 32
> > +/* Armada7k8k North Bridge index */
> > +#define ARMADA7K8K_AP806_INDEX 0
> >
> > -#define DRAM_MAX_CS_NUM 8
> > -
> > -#define DRAM_CS_ENABLED(Cs) \
> > - (MmioRead32 (DRAM_CH0_MMAP_LOW_REG (Cs)) &
> > DRAM_CS_VALID_ENABLED_MASK)
> > -#define GET_DRAM_REGION_BASE(Cs) \
> > - ((UINT64)MmioRead32 (DRAM_CH0_MMAP_HIGH_REG ((Cs))) << \
> > - DRAM_START_ADDR_HTOL_OFFS) | \
> > - (MmioRead32 (DRAM_CH0_MMAP_LOW_REG (Cs)) &
> > DRAM_START_ADDRESS_L_MASK);
> > -#define GET_DRAM_REGION_SIZE_CODE(Cs) \
> > - (MmioRead32 (DRAM_CH0_MMAP_LOW_REG ((Cs))) & \
> > - DRAM_AREA_LENGTH_MASK) >> DRAM_AREA_LENGTH_OFFS
> > -#define DRAM_REGION_SIZE_EVEN(C) (((C) >= 7) && ((C) <= 26))
> > -#define GET_DRAM_REGION_SIZE_EVEN(C) ((UINT64)1 << ((C) + 16))
> > -#define DRAM_REGION_SIZE_ODD(C) ((C) <= 4)
> > -#define GET_DRAM_REGION_SIZE_ODD(C) ((UINT64)0x18000000 << (C))
> > +/* Firmware related definition used for SMC calls */
> > +#define MV_SIP_DRAM_SIZE 0x82000010
>
> Hmm...
> This would end us up with having Marvell SMC calls spread across
> multiple files. Could you insert an intermediate patch which breaks out
> the ones from Silicon/Marvell/Library/ComPhyLib/ComPhySipSvc.h into a
> separate (MarvellSmc.h?) include file?
How about MvSmc.h ? I try to use 'Mv' prefix consistently, especially
when adding new code.
>
> If you could further add _SMC_ID to the #defines (like in edk2
> ArmPkg/Include/IndustryStandard/ArmStdSmc.h), that would make me very
> happy. (I'd be happy for you to drop _SIP, or keep it either side of
> the addition, as per your preference - we don't seem to have precedent
> here yet.)
>
How about below:
#define MV_SMC_ID_COMPHY_POWER_ON 0x82000001
#define MV_SMC_ID_COMPHY_POWER_OFF 0x82000002
#define MV_SMC_ID_COMPHY_PLL_LOCK 0x82000003
?
Thanks,
Marcin
> Regards,
>
> Leif
>
> > diff --git
> > a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c
> > b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c
> > index 2a4f5ad..62e8467 100644
> > --- a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c
> > +++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c
> > @@ -33,11 +33,14 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
> > DAMAGE.
> >
> > *******************************************************************************/
> >
> > #include <Base.h>
> > +#include <IndustryStandard/ArmStdSmc.h>
> > #include <Library/ArmPlatformLib.h>
> > +#include <Library/ArmSmcLib.h>
> > #include <Library/DebugLib.h>
> > #include <Library/HobLib.h>
> > #include <Library/IoLib.h>
> > #include <Library/MemoryAllocationLib.h>
> > +#include <Protocol/BoardDesc.h>
> >
> > #include "Armada7k8kLibMem.h"
> >
> > @@ -57,49 +60,19 @@ GetDramSize (
> > IN OUT UINT64 *MemSize
> > )
> > {
> > - UINT64 BaseAddr;
> > - UINT8 RegionCode;
> > - UINT8 Cs;
> > -
> > - *MemSize = 0;
> > -
> > - for (Cs = 0; Cs < DRAM_MAX_CS_NUM; Cs++) {
> > -
> > - /* Exit loop on first disabled DRAM CS */
> > - if (!DRAM_CS_ENABLED (Cs)) {
> > - break;
> > - }
> > -
> > - /*
> > - * Sanity check for base address of next DRAM block.
> > - * Only continuous space will be used.
> > - */
> > - BaseAddr = GET_DRAM_REGION_BASE (Cs);
> > - if (BaseAddr != *MemSize) {
> > - DEBUG ((DEBUG_ERROR,
> > - "%a: DRAM blocks are not contiguous, limit size to 0x%llx\n",
> > - __FUNCTION__,
> > - *MemSize));
> > - return EFI_SUCCESS;
> > - }
> > -
> > - /* Decode area length for current CS from register value */
> > - RegionCode = GET_DRAM_REGION_SIZE_CODE (Cs);
> > -
> > - if (DRAM_REGION_SIZE_EVEN (RegionCode)) {
> > - *MemSize += GET_DRAM_REGION_SIZE_EVEN (RegionCode);
> > - } else if (DRAM_REGION_SIZE_ODD (RegionCode)) {
> > - *MemSize += GET_DRAM_REGION_SIZE_ODD (RegionCode);
> > - } else {
> > - DEBUG ((DEBUG_ERROR,
> > - "%a: Invalid memory region code (0x%x) for CS#%d\n",
> > - __FUNCTION__,
> > - RegionCode,
> > - Cs));
> > - return EFI_INVALID_PARAMETER;
> > - }
> > + ARM_SMC_ARGS SmcRegs = {0};
> > + EFI_STATUS Status;
> > +
> > + SmcRegs.Arg0 = MV_SIP_DRAM_SIZE;
> > + Status = ArmadaSoCAp8xxBaseGet (&SmcRegs.Arg1, ARMADA7K8K_AP806_INDEX);
> > + if (EFI_ERROR (Status)) {
> > + return Status;
> > }
> >
> > + ArmCallSmc (&SmcRegs);
> > +
> > + *MemSize = SmcRegs.Arg0;
> > +
> > return EFI_SUCCESS;
> > }
> >
> > --
> > 2.7.4
> >
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
