From patchwork Wed May 22 19:47:18 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andy Fleming X-Patchwork-Id: 245703 X-Patchwork-Delegate: afleming@freescale.com Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from theia.denx.de (theia.denx.de [85.214.87.163]) by ozlabs.org (Postfix) with ESMTP id 7106F2C00A1 for ; Thu, 23 May 2013 05:47:39 +1000 (EST) Received: from localhost (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id BE2024A02A; Wed, 22 May 2013 21:47:37 +0200 (CEST) X-Virus-Scanned: Debian amavisd-new at theia.denx.de Received: from theia.denx.de ([127.0.0.1]) by localhost (theia.denx.de [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 5j6kwq5fQEej; Wed, 22 May 2013 21:47:37 +0200 (CEST) Received: from theia.denx.de (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id B22984A021; Wed, 22 May 2013 21:47:33 +0200 (CEST) Received: from localhost (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id 6DC294A023 for ; Wed, 22 May 2013 21:47:28 +0200 (CEST) X-Virus-Scanned: Debian amavisd-new at theia.denx.de Received: from theia.denx.de ([127.0.0.1]) by localhost (theia.denx.de [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id WZx2eje81lqq for ; Wed, 22 May 2013 21:47:27 +0200 (CEST) X-policyd-weight: NOT_IN_SBL_XBL_SPAMHAUS=-1.5 NOT_IN_SPAMCOP=-1.5 NOT_IN_BL_NJABL=-1.5 (only DNSBL check requested) Received: from mail-we0-f175.google.com (mail-we0-f175.google.com [74.125.82.175]) by theia.denx.de (Postfix) with ESMTPS id 04D604A020 for ; Wed, 22 May 2013 21:47:19 +0200 (CEST) Received: by mail-we0-f175.google.com with SMTP id p60so1623880wes.34 for ; Wed, 22 May 2013 12:47:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=uf0CFcPKP/GhkeUiBmAQC7OKa3jWkWcVFHRdCpPVFSk=; b=SDMF3uPeC6yo8PB7H+MofnqOLorBE0UJr4dIcvVOI0527t/FMNPXNuUJmkkdBbvhLc yeyYDlUj3utHMUwQ6IFwrICMiF8DKZ1ODarbCssJsNZWIZp3v0ZsYH8B48DpRX5HpjBF jje3b0CIdAmrHCqVtrB+1Q7I0LkEoOSggKGewAYW9/pBx34aRLshWjPxbd89dS6G4ZsR ty/dvssYfeP0EyM7c/Q03u77qFSrFPBthOUzi8hfYtrMgBG+hlGrBbRRgkSDob0AJKsv 1fNDmv/Bcpe787y/FQMMgEFqeyvRPD7xk13lz5jxLz4c0zBUlmqo8U3ShKawIFbRrpXu KI0w== MIME-Version: 1.0 X-Received: by 10.180.79.40 with SMTP id g8mr17693384wix.3.1369252038660; Wed, 22 May 2013 12:47:18 -0700 (PDT) Received: by 10.194.216.194 with HTTP; Wed, 22 May 2013 12:47:18 -0700 (PDT) In-Reply-To: <1364467595-15539-2-git-send-email-ruchika.gupta@freescale.com> References: <1364467595-15539-1-git-send-email-ruchika.gupta@freescale.com> <1364467595-15539-2-git-send-email-ruchika.gupta@freescale.com> Date: Wed, 22 May 2013 14:47:18 -0500 Message-ID: From: Andy Fleming To: Ruchika Gupta X-Content-Filtered-By: Mailman/MimeDel 2.1.11 Cc: U-Boot list , Andy Fleming , Kuldip Giroh Subject: Re: [U-Boot] [PATCH 1/5] arch/powerpc/cpu/mpc8xxx: PAMU driver support X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.11 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: u-boot-bounces@lists.denx.de Errors-To: u-boot-bounces@lists.denx.de On Thu, Mar 28, 2013 at 5:46 AM, Ruchika Gupta 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 > Signed-off-by: Ruchika Gupta > --- > 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(®s->ppbah, ppaact_phys >> 32); > + out_be32(®s->ppbal, (uint32_t)ppaact_phys); > + > + out_be32(®s->pplah, (ppaact_lim) >> 32); > + out_be32(®s->pplal, (uint32_t)ppaact_lim); > + > + if (sec != NULL) { > + out_be32(®s->spbah, spaact_phys >> 32); > + out_be32(®s->spbal, (uint32_t)spaact_phys); > + out_be32(®s->splah, spaact_lim >> 32); > + out_be32(®s->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(®s->ppbah, 0); > + out_be32(®s->ppbal, 0); > + out_be32(®s->pplah, 0); > + out_be32(®s->pplal, 0); > + out_be32(®s->spbah, 0); > + out_be32(®s->spbal, 0); > + out_be32(®s->splah, 0); > + out_be32(®s->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 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