Patchwork [U-Boot,1/5] arch/powerpc/cpu/mpc8xxx: PAMU driver support

login
register
mail settings
Submitter Andy Fleming
Date May 22, 2013, 7:47 p.m.
Message ID <CAKWjMd5suhRkhGnWrrkdjvoDe98wTv19qPAVCMzvPVCOPk7eOg@mail.gmail.com>
Download mbox | patch
Permalink /patch/245703/
State Not Applicable, archived
Delegated to: Andy Fleming
Headers show

Comments

Andy Fleming - May 22, 2013, 7:47 p.m.
On Thu, Mar 28, 2013 at 5:46 AM, Ruchika Gupta
<ruchika.gupta@freescale.com>wrote:

> PAMU driver basic support for usage in Secure Boot.
> In secure boot PAMU is not in bypass mode. Hence to use
> any peripheral (SEC Job ring in our case), PAMU has to be
> configured.
>
> The Header file fsl_pamu.h and few functions in driver have been derived
> from Freescale Libos.
>
> Signed-off-by: Kuldip Giroh <kuldip.giroh@freescale.com>
> Signed-off-by: Ruchika Gupta <ruchika.gupta@freescale.com>
> ---
> Based upon git://git.denx.de/u-boot.git branch master
>

You don't really have to say this as a) It's assumed, b) It becomes quickly
obsolete (as it is now). You probably only need to mention it if you're
based on an unusual git repo.


diff --git a/arch/powerpc/cpu/mpc8xxx/fsl_pamu.c
b/arch/powerpc/cpu/mpc8xxx/fsl_pamu.c
new file mode 100644
index 0000000..bdbec07
--- /dev/null
+++ b/arch/powerpc/cpu/mpc8xxx/fsl_pamu.c

[...]

+
+static inline int __ilog2_roundup_64(uint64_t val)
+{
+
+       if ((val & (val - 1)) == 0)
+               return __ilog2_u64(val);
+       else
+               return  __ilog2_u64(val) + 1;
+}


These sorts of functions seem like they belong
in arch/powerpc/include/asm/bitops.h



> +
> +
> +static inline int count_lsb_zeroes(unsigned long val)
> +{
> +       return ffs(val) - 1;
> +}
> +
>

[...]


> +static int pamu_config_ppaace(uint32_t liodn, uint64_t win_addr,
> +       uint64_t win_size, uint32_t omi,
> +       uint32_t snoopid, uint32_t stashid,
> +       uint32_t subwin_cnt)
> +{
> +       unsigned long fspi;
> +       paace_t *ppaace;
> +
> +       if ((win_size & (win_size - 1)) || win_size < PAMU_PAGE_SIZE)
> +               return -1;
> +
> +       if (win_addr & (win_size - 1))
> +               return -2;
> +
> +       if (liodn > NUM_PPAACT_ENTRIES) {
> +               printf("Entries in PPACT not sufficient\n");
> +               return -3;
> +       }
>


These return values should really have names to define, up-front, what they
mean.



> +
> +       ppaace = &ppaact[liodn];
> +
> +       /* window size is 2^(WSE+1) bytes */
> +       set_bf(ppaace->addr_bitfields, PPAACE_AF_WSE,
> +               map_addrspace_size_to_wse(win_size));
> +
> +       pamu_setup_default_xfer_to_host_ppaace(ppaace);
> +
> +       if (sizeof(phys_addr_t) > 4)
> +               ppaace->wbah = (u64)win_addr >> (PAMU_PAGE_SHIFT + 20);
> +       else
> +               ppaace->wbah = 0;
> +
> +       set_bf(ppaace->addr_bitfields, PPAACE_AF_WBAL,
> +              (win_addr >> PAMU_PAGE_SHIFT));
> +
> +       /* set up operation mapping if it's configured */
> +       if (omi < OME_NUMBER_ENTRIES) {
> +               set_bf(ppaace->impl_attr, PAACE_IA_OTM, PAACE_OTM_INDEXED);
> +               ppaace->op_encode.index_ot.omi = omi;
> +       } else if (~omi != 0)
> +               return -3;
>

ditto here



> +static int pamu_config_spaace(uint32_t liodn,
> +       uint64_t subwin_size, uint64_t subwin_addr, uint64_t size,
> +       uint32_t omi, uint32_t snoopid, uint32_t stashid)
> +{
> +       paace_t *paace;
> +       unsigned long fspi;
> +       /* Align start addr of subwin to subwindoe size */
> +       uint64_t sec_addr = subwin_addr & ~(subwin_size - 1);
> +       uint64_t end_addr = subwin_addr + size;
> +       int size_shift = __ilog2_u64(subwin_size);
> +       uint64_t win_size = 0;
> +       uint32_t index, swse;
> +
> +       /* Recalculate the size */
> +       size = end_addr - sec_addr;
> +
> +       if (!subwin_size)
> +               return -1;
>

Also need a named constant here



> +
> +       if (liodn > NUM_PPAACT_ENTRIES) {
> +               printf("LIODN Number to be programmed %d"
> +                       "greater than number of PPAACT entries %d\n",
> +                               liodn, NUM_PPAACT_ENTRIES);
> +               return -1;
> +       }
>

And here



> +
> +       while (sec_addr < end_addr) {
> +#ifdef DEBUG
> +               printf("sec_addr < end_addr is %llx < %llx\n", sec_addr,
> +                       end_addr);
> +#endif
> +               paace = &ppaact[liodn];
> +               if (!paace)
> +                       return -1;
>

And here



+
+int pamu_init(void)
+{
+       u32 base_addr = CONFIG_SYS_PAMU_ADDR;

void *base_addr = (void *)CONFIG_SYS_PAMU_ADDR;


+       ccsr_pamu_t *regs;
+       u32 i = 0;
+       u64 ppaact_phys, ppaact_lim, ppaact_size;
+       u64 spaact_phys, spaact_lim, spaact_size;
+
+       ppaact_size = sizeof(paace_t) * NUM_PPAACT_ENTRIES;
+       spaact_size = sizeof(paace_t) * NUM_SPAACT_ENTRIES;
+
+       /* Allocate space for Primary PAACT Table */
+       ppaact = memalign(PAMU_TABLE_ALIGNMENT, ppaact_size);
+       if (!ppaact)
+               return -1;

Named constant.



> +       memset(ppaact, 0, ppaact_size);
> +
> +       /* Allocate space for Secondary PAACT Table */
> +       sec = memalign(PAMU_TABLE_ALIGNMENT, spaact_size);
> +       if (!sec)
> +               return -1;
>

And here



> +       memset(sec, 0, spaact_size);
> +
> +       ppaact_phys = virt_to_phys((void *)ppaact);
> +       ppaact_lim = ppaact_phys + ppaact_size;
> +
> +       spaact_phys = (uint64_t)virt_to_phys((void *)sec);
> +       spaact_lim = spaact_phys + spaact_size;
> +
> +       /* Configure all PAMU's */
> +       for (i = 0; i < CONFIG_NUM_PAMU; i++) {
> +               regs = (ccsr_pamu_t *)base_addr;
>

No need for cast once base_addr is more sane.



> +
> +               out_be32(&regs->ppbah, ppaact_phys >> 32);
> +               out_be32(&regs->ppbal, (uint32_t)ppaact_phys);
> +
> +               out_be32(&regs->pplah, (ppaact_lim) >> 32);
> +               out_be32(&regs->pplal, (uint32_t)ppaact_lim);
> +
> +               if (sec != NULL) {
> +                       out_be32(&regs->spbah, spaact_phys >> 32);
> +                       out_be32(&regs->spbal, (uint32_t)spaact_phys);
> +                       out_be32(&regs->splah, spaact_lim >> 32);
> +                       out_be32(&regs->splal, (uint32_t)spaact_lim);
> +               }
> +               asm volatile("sync" : : : "memory");
> +
> +               base_addr += PAMU_OFFSET;
> +       }
> +
> +       return 0;
> +}
> +
> +void pamu_enable(void)
> +{
> +       u32 i = 0;
> +       u32 base_addr = CONFIG_SYS_PAMU_ADDR;
>

void *base_addr = (void *)CONFIG_SYS_PAMU_ADDR;



> +       for (i = 0; i < CONFIG_NUM_PAMU; i++) {
> +               setbits_be32((void *)base_addr + PAMU_PCR_OFFSET,
> +                       PAMU_PCR_PE);
>

And then eliminate the "(void *)" here



> +               asm volatile("sync" : : : "memory");
> +               base_addr += PAMU_OFFSET;
> +       }
> +}
> +
> +void pamu_reset(void)
> +{
> +       u32 i  = 0;
> +       u32 base_addr = CONFIG_SYS_PAMU_ADDR;
>

void *base_addr = (void *)CONFIG_SYS_PAMU_ADDR;

In general, why would you declare an address as u32? We have 32 and 64-bit
processors that use PAMU. Does this work at all on e5500 or e6500?


+       ccsr_pamu_t *regs;
> +
> +       for (i = 0; i < CONFIG_NUM_PAMU; i++) {
> +               regs = (ccsr_pamu_t *)base_addr;
>

Get rid of cast by following above recommendation.



> +       /* Clear PPAACT Base register */
> +               out_be32(&regs->ppbah, 0);
> +               out_be32(&regs->ppbal, 0);
> +               out_be32(&regs->pplah, 0);
> +               out_be32(&regs->pplal, 0);
> +               out_be32(&regs->spbah, 0);
> +               out_be32(&regs->spbal, 0);
> +               out_be32(&regs->splah, 0);
> +               out_be32(&regs->splal, 0);
> +
> +               clrbits_be32((void *)regs + PAMU_PCR_OFFSET, PAMU_PCR_PE);
> +               asm volatile("sync" : : : "memory");
> +               base_addr += PAMU_OFFSET;
> +       }
> +}
> +
> +void pamu_disable(void)
> +{
> +       u32 i  = 0;
> +       u32 base_addr = CONFIG_SYS_PAMU_ADDR;
>

ditto here.



> +
> +
> +       for (i = 0; i < CONFIG_NUM_PAMU; i++) {
> +               clrbits_be32((void *)base_addr + PAMU_PCR_OFFSET,
> PAMU_PCR_PE);
>

And this cast



> +               asm volatile("sync" : : : "memory");
> +               base_addr += PAMU_OFFSET;
> +       }
> +}
> +
> +
>

[...]


> +int config_pamu(struct pamu_addr_tbl *tbl, int num_entries, uint32_t
> liodn)
> +{
> +       int i = 0;
> +       int ret = 0;
> +       uint32_t num_sec_windows = 0;
> +       uint32_t num_windows = 0;
> +       uint64_t min_addr, max_addr;
> +       uint64_t size;
> +       uint64_t subwin_size;
> +       int sizebit;
> +
> +       min_addr = find_min(tbl->start_addr, num_entries);
> +       max_addr = find_max(tbl->end_addr, num_entries);
> +       size = max_addr - min_addr + 1;
> +
> +       if (!size)
> +               return -1;
>

Named constant



> +
> +       sizebit = __ilog2_roundup_64(size);
> +       size = 1 << sizebit;
> +#ifdef DEBUG
> +       printf("min start_addr is %llx\n", min_addr);
> +       printf("max end_addr is %llx\n", max_addr);
> +       printf("size found is  %llx\n", size);
> +#endif
> +
> +       if (size < PAMU_PAGE_SIZE)
> +               size = PAMU_PAGE_SIZE;
> +
> +       while (1) {
> +               min_addr = min_addr & ~(size - 1);
> +               if (min_addr + size > max_addr)
> +                       break;
> +               size <<= 1;
> +               if (!size)
> +                       return -1;
>

Named constant

Patch

diff --git a/arch/powerpc/include/asm/fsl_pamu.h
b/arch/powerpc/include/asm/fsl_pamu.h
new file mode 100644
index 0000000..e2cfe97
--- /dev/null
+++ b/arch/powerpc/include/asm/fsl_pamu.h
@@ -0,0 +1,193 @@ 


> +
> +#define CONFIG_NUM_PAMU                16
> +#define NUM_PPAACT_ENTRIES     512
> +#define NUM_SPAACT_ENTRIES     128
>


Aren't these values variable, depending on the platform?



> +
> +/* PAMU_OFFSET to the next pamu space in ccsr */
> +#define PAMU_OFFSET 0x1000
> +
> +#define PAMU_TABLE_ALIGNMENT 0x00001000
> +
> +#define PAMU_PAGE_SHIFT 12
> +#define PAMU_PAGE_SIZE  4096U
> +
> +#define PAACE_M_COHERENCE_REQ   0x01
> +
> +#define PAACE_DA_HOST_CR                0x80
> +#define PAACE_DA_HOST_CR_SHIFT          7
> +
> +#define PAACE_AF_PT                     0x00000002
> +#define PAACE_AF_PT_SHIFT               1
> +
> +#define PAACE_PT_PRIMARY       0x0
> +#define PAACE_PT_SECONDARY     0x1
> +
> +#define PPAACE_AF_WBAL                 0xfffff000
> +#define PPAACE_AF_WBAL_SHIFT           12
> +
> +#define        OME_NUMBER_ENTRIES      16   /* based on P4080 2.0 silicon
> plan */
>


...Is this P4080-specific?

 Andy