* Re: [PATCH -V5 06/25] powerpc: Reduce PTE table memory wastage
From: David Gibson @ 2013-04-10 4:46 UTC (permalink / raw)
To: Aneesh Kumar K.V; +Cc: paulus, linuxppc-dev, linux-mm
In-Reply-To: <1365055083-31956-7-git-send-email-aneesh.kumar@linux.vnet.ibm.com>
[-- Attachment #1: Type: text/plain, Size: 15308 bytes --]
On Thu, Apr 04, 2013 at 11:27:44AM +0530, Aneesh Kumar K.V wrote:
> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
>
> We allocate one page for the last level of linux page table. With THP and
> large page size of 16MB, that would mean we are wasting large part
> of that page. To map 16MB area, we only need a PTE space of 2K with 64K
> page size. This patch reduce the space wastage by sharing the page
> allocated for the last level of linux page table with multiple pmd
> entries. We call these smaller chunks PTE page fragments and allocated
> page, PTE page.
>
> In order to support systems which doesn't have 64K HPTE support, we also
> add another 2K to PTE page fragment. The second half of the PTE fragments
> is used for storing slot and secondary bit information of an HPTE. With this
> we now have a 4K PTE fragment.
>
> We use a simple approach to share the PTE page. On allocation, we bump the
> PTE page refcount to 16 and share the PTE page with the next 16 pte alloc
> request. This should help in the node locality of the PTE page fragment,
> assuming that the immediate pte alloc request will mostly come from the
> same NUMA node. We don't try to reuse the freed PTE page fragment. Hence
> we could be waisting some space.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> ---
> arch/powerpc/include/asm/mmu-book3e.h | 4 +
> arch/powerpc/include/asm/mmu-hash64.h | 4 +
> arch/powerpc/include/asm/page.h | 4 +
> arch/powerpc/include/asm/pgalloc-64.h | 72 ++++-------------
> arch/powerpc/kernel/setup_64.c | 4 +-
> arch/powerpc/mm/mmu_context_hash64.c | 35 +++++++++
> arch/powerpc/mm/pgtable_64.c | 137 +++++++++++++++++++++++++++++++++
> 7 files changed, 202 insertions(+), 58 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/mmu-book3e.h b/arch/powerpc/include/asm/mmu-book3e.h
> index 99d43e0..affbd68 100644
> --- a/arch/powerpc/include/asm/mmu-book3e.h
> +++ b/arch/powerpc/include/asm/mmu-book3e.h
> @@ -231,6 +231,10 @@ typedef struct {
> u64 high_slices_psize; /* 4 bits per slice for now */
> u16 user_psize; /* page size index */
> #endif
> +#ifdef CONFIG_PPC_64K_PAGES
> + /* for 4K PTE fragment support */
> + struct page *pgtable_page;
> +#endif
> } mm_context_t;
>
> /* Page size definitions, common between 32 and 64-bit
> diff --git a/arch/powerpc/include/asm/mmu-hash64.h b/arch/powerpc/include/asm/mmu-hash64.h
> index 35bb51e..300ac3c 100644
> --- a/arch/powerpc/include/asm/mmu-hash64.h
> +++ b/arch/powerpc/include/asm/mmu-hash64.h
> @@ -498,6 +498,10 @@ typedef struct {
> unsigned long acop; /* mask of enabled coprocessor types */
> unsigned int cop_pid; /* pid value used with coprocessors */
> #endif /* CONFIG_PPC_ICSWX */
> +#ifdef CONFIG_PPC_64K_PAGES
> + /* for 4K PTE fragment support */
> + struct page *pgtable_page;
> +#endif
> } mm_context_t;
>
>
> diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
> index f072e97..38e7ff6 100644
> --- a/arch/powerpc/include/asm/page.h
> +++ b/arch/powerpc/include/asm/page.h
> @@ -378,7 +378,11 @@ void arch_free_page(struct page *page, int order);
>
> struct vm_area_struct;
>
> +#ifdef CONFIG_PPC_64K_PAGES
> +typedef pte_t *pgtable_t;
> +#else
> typedef struct page *pgtable_t;
> +#endif
Ugh, that's pretty horrible, though I don't see an easy way around it.
> #include <asm-generic/memory_model.h>
> #endif /* __ASSEMBLY__ */
> diff --git a/arch/powerpc/include/asm/pgalloc-64.h b/arch/powerpc/include/asm/pgalloc-64.h
> index cdbf555..3418989 100644
> --- a/arch/powerpc/include/asm/pgalloc-64.h
> +++ b/arch/powerpc/include/asm/pgalloc-64.h
> @@ -150,6 +150,13 @@ static inline void __pte_free_tlb(struct mmu_gather *tlb, pgtable_t table,
>
> #else /* if CONFIG_PPC_64K_PAGES */
>
> +extern pte_t *page_table_alloc(struct mm_struct *, unsigned long, int);
> +extern void page_table_free(struct mm_struct *, unsigned long *, int);
> +extern void pgtable_free_tlb(struct mmu_gather *tlb, void *table, int shift);
> +#ifdef CONFIG_SMP
> +extern void __tlb_remove_table(void *_table);
> +#endif
> +
> #define pud_populate(mm, pud, pmd) pud_set(pud, (unsigned long)pmd)
>
> static inline void pmd_populate_kernel(struct mm_struct *mm, pmd_t *pmd,
> @@ -161,90 +168,42 @@ static inline void pmd_populate_kernel(struct mm_struct *mm, pmd_t *pmd,
> static inline void pmd_populate(struct mm_struct *mm, pmd_t *pmd,
> pgtable_t pte_page)
> {
> - pmd_populate_kernel(mm, pmd, page_address(pte_page));
> + pmd_set(pmd, (unsigned long)pte_page);
> }
>
> static inline pgtable_t pmd_pgtable(pmd_t pmd)
> {
> - return pmd_page(pmd);
> + return (pgtable_t)(pmd_val(pmd) & -sizeof(pte_t)*PTRS_PER_PTE);
> }
>
> static inline pte_t *pte_alloc_one_kernel(struct mm_struct *mm,
> unsigned long address)
> {
> - return (pte_t *)__get_free_page(GFP_KERNEL | __GFP_REPEAT | __GFP_ZERO);
> + return (pte_t *)page_table_alloc(mm, address, 1);
> }
>
> static inline pgtable_t pte_alloc_one(struct mm_struct *mm,
> - unsigned long address)
> + unsigned long address)
> {
> - struct page *page;
> - pte_t *pte;
> -
> - pte = pte_alloc_one_kernel(mm, address);
> - if (!pte)
> - return NULL;
> - page = virt_to_page(pte);
> - pgtable_page_ctor(page);
> - return page;
> + return (pgtable_t)page_table_alloc(mm, address, 0);
> }
>
> static inline void pte_free_kernel(struct mm_struct *mm, pte_t *pte)
> {
> - free_page((unsigned long)pte);
> + page_table_free(mm, (unsigned long *)pte, 1);
> }
>
> static inline void pte_free(struct mm_struct *mm, pgtable_t ptepage)
> {
> - pgtable_page_dtor(ptepage);
> - __free_page(ptepage);
> -}
> -
> -static inline void pgtable_free(void *table, unsigned index_size)
> -{
> - if (!index_size)
> - free_page((unsigned long)table);
> - else {
> - BUG_ON(index_size > MAX_PGTABLE_INDEX_SIZE);
> - kmem_cache_free(PGT_CACHE(index_size), table);
> - }
> + page_table_free(mm, (unsigned long *)ptepage, 0);
> }
>
> -#ifdef CONFIG_SMP
> -static inline void pgtable_free_tlb(struct mmu_gather *tlb,
> - void *table, int shift)
> -{
> - unsigned long pgf = (unsigned long)table;
> - BUG_ON(shift > MAX_PGTABLE_INDEX_SIZE);
> - pgf |= shift;
> - tlb_remove_table(tlb, (void *)pgf);
> -}
> -
> -static inline void __tlb_remove_table(void *_table)
> -{
> - void *table = (void *)((unsigned long)_table & ~MAX_PGTABLE_INDEX_SIZE);
> - unsigned shift = (unsigned long)_table & MAX_PGTABLE_INDEX_SIZE;
> -
> - pgtable_free(table, shift);
> -}
> -#else /* !CONFIG_SMP */
> -static inline void pgtable_free_tlb(struct mmu_gather *tlb,
> - void *table, int shift)
> -{
> - pgtable_free(table, shift);
> -}
> -#endif /* CONFIG_SMP */
> -
> static inline void __pte_free_tlb(struct mmu_gather *tlb, pgtable_t table,
> unsigned long address)
> {
> - struct page *page = page_address(table);
> -
> tlb_flush_pgtable(tlb, address);
> - pgtable_page_dtor(page);
> - pgtable_free_tlb(tlb, page, 0);
> + pgtable_free_tlb(tlb, table, 0);
> }
> -
> #endif /* CONFIG_PPC_64K_PAGES */
>
> static inline pmd_t *pmd_alloc_one(struct mm_struct *mm, unsigned long addr)
> @@ -258,7 +217,6 @@ static inline void pmd_free(struct mm_struct *mm, pmd_t *pmd)
> kmem_cache_free(PGT_CACHE(PMD_INDEX_SIZE), pmd);
> }
>
> -
> #define __pmd_free_tlb(tlb, pmd, addr) \
> pgtable_free_tlb(tlb, pmd, PMD_INDEX_SIZE)
> #ifndef CONFIG_PPC_64K_PAGES
> diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
> index 6da881b..04d833c 100644
> --- a/arch/powerpc/kernel/setup_64.c
> +++ b/arch/powerpc/kernel/setup_64.c
> @@ -575,7 +575,9 @@ void __init setup_arch(char **cmdline_p)
> init_mm.end_code = (unsigned long) _etext;
> init_mm.end_data = (unsigned long) _edata;
> init_mm.brk = klimit;
> -
> +#ifdef CONFIG_PPC_64K_PAGES
> + init_mm.context.pgtable_page = NULL;
> +#endif
> irqstack_early_init();
> exc_lvl_early_init();
> emergency_stack_init();
> diff --git a/arch/powerpc/mm/mmu_context_hash64.c b/arch/powerpc/mm/mmu_context_hash64.c
> index 59cd773..fbfdca2 100644
> --- a/arch/powerpc/mm/mmu_context_hash64.c
> +++ b/arch/powerpc/mm/mmu_context_hash64.c
> @@ -86,6 +86,9 @@ int init_new_context(struct task_struct *tsk, struct mm_struct *mm)
> spin_lock_init(mm->context.cop_lockp);
> #endif /* CONFIG_PPC_ICSWX */
>
> +#ifdef CONFIG_PPC_64K_PAGES
> + mm->context.pgtable_page = NULL;
> +#endif
> return 0;
> }
>
> @@ -97,13 +100,45 @@ void __destroy_context(int context_id)
> }
> EXPORT_SYMBOL_GPL(__destroy_context);
>
> +#ifdef CONFIG_PPC_64K_PAGES
> +static void destroy_pagetable_page(struct mm_struct *mm)
> +{
> + int count;
> + struct page *page;
> +
> + page = mm->context.pgtable_page;
> + if (!page)
> + return;
> +
> + /* drop all the pending references */
> + count = atomic_read(&page->_mapcount) + 1;
> + /* We allow PTE_FRAG_NR(16) fragments from a PTE page */
> + count = atomic_sub_return(16 - count, &page->_count);
You should really move PTE_FRAG_NR to a header so you can actually use
it here rather than hard coding 16.
It took me a fair while to convince myself that there is no race here
with something altering mapcount and count between the atomic_read()
and the atomic_sub_return(). It could do with a comment to explain
why that is safe.
Re-using the mapcount field for your index also seems odd, and it took
me a while to convince myself that that's safe too. Wouldn't it be
simpler to store a pointer to the next sub-page in the mm_context
instead? You can get from that to the struct page easily enough with a
shift and pfn_to_page().
> + if (!count) {
> + pgtable_page_dtor(page);
> + reset_page_mapcount(page);
> + free_hot_cold_page(page, 0);
It would be nice to use put_page() somehow instead of duplicating its
logic, though I realise the sparc code you've based this on does the
same thing.
> + }
> +}
> +
> +#else
> +static inline void destroy_pagetable_page(struct mm_struct *mm)
> +{
> + return;
> +}
> +#endif
> +
> +
> void destroy_context(struct mm_struct *mm)
> {
> +
> #ifdef CONFIG_PPC_ICSWX
> drop_cop(mm->context.acop, mm);
> kfree(mm->context.cop_lockp);
> mm->context.cop_lockp = NULL;
> #endif /* CONFIG_PPC_ICSWX */
> +
> + destroy_pagetable_page(mm);
> __destroy_context(mm->context.id);
> subpage_prot_free(mm);
> mm->context.id = MMU_NO_CONTEXT;
> diff --git a/arch/powerpc/mm/pgtable_64.c b/arch/powerpc/mm/pgtable_64.c
> index e212a27..e79840b 100644
> --- a/arch/powerpc/mm/pgtable_64.c
> +++ b/arch/powerpc/mm/pgtable_64.c
> @@ -337,3 +337,140 @@ EXPORT_SYMBOL(__ioremap_at);
> EXPORT_SYMBOL(iounmap);
> EXPORT_SYMBOL(__iounmap);
> EXPORT_SYMBOL(__iounmap_at);
> +
> +#ifdef CONFIG_PPC_64K_PAGES
> +/*
> + * we support 16 fragments per PTE page. This is limited by how many
> + * bits we can pack in page->_mapcount. We use the first half for
> + * tracking the usage for rcu page table free.
> + */
> +#define PTE_FRAG_NR 16
> +/*
> + * We use a 2K PTE page fragment and another 2K for storing
> + * real_pte_t hash index
> + */
> +#define PTE_FRAG_SIZE (2 * PTRS_PER_PTE * sizeof(pte_t))
> +
> +static pte_t *get_from_cache(struct mm_struct *mm)
> +{
> + int index;
> + pte_t *ret = NULL;
> + struct page *page;
> +
> + spin_lock(&mm->page_table_lock);
> + page = mm->context.pgtable_page;
> + if (page) {
> + void *p = page_address(page);
> + index = atomic_add_return(1, &page->_mapcount);
> + ret = (pte_t *) (p + (index * PTE_FRAG_SIZE));
> + /*
> + * If we have taken up all the fragments mark PTE page NULL
> + */
> + if (index == PTE_FRAG_NR - 1)
> + mm->context.pgtable_page = NULL;
> + }
> + spin_unlock(&mm->page_table_lock);
> + return ret;
> +}
> +
> +static pte_t *__alloc_for_cache(struct mm_struct *mm, int kernel)
> +{
> + pte_t *ret = NULL;
> + struct page *page = alloc_page(GFP_KERNEL | __GFP_NOTRACK |
> + __GFP_REPEAT | __GFP_ZERO);
> + if (!page)
> + return NULL;
> +
> + spin_lock(&mm->page_table_lock);
> + /*
> + * If we find pgtable_page set, we return
> + * the allocated page with single fragement
> + * count.
> + */
> + if (likely(!mm->context.pgtable_page)) {
> + atomic_set(&page->_count, PTE_FRAG_NR);
> + atomic_set(&page->_mapcount, 0);
> + mm->context.pgtable_page = page;
> + }
.. and in the unlikely case where there *is* a pgtable_page already
set, what then? Seems like you should BUG_ON, or at least return NULL
- as it is you will return the first sub-page of that page again,
which is very likely in use.
> + spin_unlock(&mm->page_table_lock);
> +
> + ret = (unsigned long *)page_address(page);
> + if (!kernel)
> + pgtable_page_ctor(page);
> +
> + return ret;
> +}
> +
> +pte_t *page_table_alloc(struct mm_struct *mm, unsigned long vmaddr, int kernel)
> +{
> + pte_t *pte;
> +
> + pte = get_from_cache(mm);
> + if (pte)
> + return pte;
> +
> + return __alloc_for_cache(mm, kernel);
> +}
> +
> +void page_table_free(struct mm_struct *mm, unsigned long *table, int kernel)
> +{
> + struct page *page = virt_to_page(table);
> + if (put_page_testzero(page)) {
> + if (!kernel)
> + pgtable_page_dtor(page);
> + reset_page_mapcount(page);
> + free_hot_cold_page(page, 0);
> + }
> +}
> +
> +#ifdef CONFIG_SMP
> +static void page_table_free_rcu(void *table)
> +{
> + struct page *page = virt_to_page(table);
> + if (put_page_testzero(page)) {
> + pgtable_page_dtor(page);
> + reset_page_mapcount(page);
> + free_hot_cold_page(page, 0);
> + }
> +}
> +
> +void pgtable_free_tlb(struct mmu_gather *tlb, void *table, int shift)
> +{
> + unsigned long pgf = (unsigned long)table;
> +
> + BUG_ON(shift > MAX_PGTABLE_INDEX_SIZE);
> + pgf |= shift;
> + tlb_remove_table(tlb, (void *)pgf);
> +}
> +
> +void __tlb_remove_table(void *_table)
> +{
> + void *table = (void *)((unsigned long)_table & ~MAX_PGTABLE_INDEX_SIZE);
> + unsigned shift = (unsigned long)_table & MAX_PGTABLE_INDEX_SIZE;
> +
> + if (!shift)
> + /* PTE page needs special handling */
> + page_table_free_rcu(table);
> + else {
> + BUG_ON(shift > MAX_PGTABLE_INDEX_SIZE);
> + kmem_cache_free(PGT_CACHE(shift), table);
> + }
> +}
> +#else
> +void pgtable_free_tlb(struct mmu_gather *tlb, void *table, int shift)
> +{
> + if (!shift) {
> + /* PTE page needs special handling */
> + struct page *page = virt_to_page(table);
> + if (put_page_testzero(page)) {
> + pgtable_page_dtor(page);
> + reset_page_mapcount(page);
> + free_hot_cold_page(page, 0);
> + }
> + } else {
> + BUG_ON(shift > MAX_PGTABLE_INDEX_SIZE);
> + kmem_cache_free(PGT_CACHE(shift), table);
> + }
> +}
> +#endif
> +#endif /* CONFIG_PPC_64K_PAGES */
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* [PATCH v2] of/base: release the node correctly in of_parse_phandle_with_args()
From: Yuantian.Tang @ 2013-04-10 3:36 UTC (permalink / raw)
To: grant.likely
Cc: Tang Yuantian, devicetree-discuss, linuxppc-dev, linux-kernel,
rob.herring
From: Tang Yuantian <yuantian.tang@freescale.com>
Call of_node_put() only when the out_args is NULL on success,
or the node's reference count will not be correct because the caller
will call of_node_put() again.
Signed-off-by: Tang Yuantian <Yuantian.Tang@freescale.com>
---
v2:
- modified the title and description. the 1st patch title is:
of: remove the unnecessary of_node_put for of_parse_phandle_with_args()
the 1st patch is not good enough.
drivers/of/base.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 321d3ef..ee94f64 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1158,6 +1158,7 @@ static int __of_parse_phandle_with_args(const struct device_node *np,
if (!phandle)
goto err;
+ /* Found it! return success */
if (out_args) {
int i;
if (WARN_ON(count > MAX_PHANDLE_ARGS))
@@ -1166,11 +1167,10 @@ static int __of_parse_phandle_with_args(const struct device_node *np,
out_args->args_count = count;
for (i = 0; i < count; i++)
out_args->args[i] = be32_to_cpup(list++);
+ } else if (node) {
+ of_node_put(node);
}
- /* Found it! return success */
- if (node)
- of_node_put(node);
return 0;
}
--
1.8.0
^ permalink raw reply related
* [PATCH V5] powerpc/MPIC: Add get_version API both for internal and external use
From: Jia Hongtao @ 2013-04-10 2:52 UTC (permalink / raw)
To: linuxppc-dev, galak; +Cc: B07421, hongtao.jia
MPIC version is useful information for both mpic_alloc() and mpic_init().
The patch provide an API to get MPIC version for reusing the code.
Also, some other IP block may need MPIC version for their own use.
The API for external use is also provided.
Signed-off-by: Jia Hongtao <hongtao.jia@freescale.com>
Signed-off-by: Li Yang <leoli@freescale.com>
---
V5:
* add MPIC_FSL check for fsl_mpic_get_version().
V4:
* change the name of function from mpic_get_version() to
fsl_mpic_get_version().
V3:
* change the name of function from mpic_primary_get_version() to
fsl_mpic_primary_get_version().
* return 0 if mpic_primary is null.
V2:
* Using mpic_get_version() to implement mpic_primary_get_version()
arch/powerpc/include/asm/mpic.h | 3 +++
arch/powerpc/sysdev/mpic.c | 32 +++++++++++++++++++++++++-------
2 files changed, 28 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/include/asm/mpic.h b/arch/powerpc/include/asm/mpic.h
index c0f9ef9..ea6bf72 100644
--- a/arch/powerpc/include/asm/mpic.h
+++ b/arch/powerpc/include/asm/mpic.h
@@ -393,6 +393,9 @@ struct mpic
#define MPIC_REGSET_STANDARD MPIC_REGSET(0) /* Original MPIC */
#define MPIC_REGSET_TSI108 MPIC_REGSET(1) /* Tsi108/109 PIC */
+/* Get the version of primary MPIC */
+extern u32 fsl_mpic_primary_get_version(void);
+
/* Allocate the controller structure and setup the linux irq descs
* for the range if interrupts passed in. No HW initialization is
* actually performed.
diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
index d30e6a6..47ef4ba 100644
--- a/arch/powerpc/sysdev/mpic.c
+++ b/arch/powerpc/sysdev/mpic.c
@@ -1165,10 +1165,33 @@ static struct irq_domain_ops mpic_host_ops = {
.xlate = mpic_host_xlate,
};
+static u32 fsl_mpic_get_version(struct mpic *mpic)
+{
+ u32 brr1;
+
+ if (!(mpic->flags & MPIC_FSL))
+ return 0;
+
+ brr1 = _mpic_read(mpic->reg_type, &mpic->thiscpuregs,
+ MPIC_FSL_BRR1);
+
+ return brr1 & MPIC_FSL_BRR1_VER;
+}
+
/*
* Exported functions
*/
+u32 fsl_mpic_primary_get_version(void)
+{
+ struct mpic *mpic = mpic_primary;
+
+ if (mpic)
+ return fsl_mpic_get_version(mpic);
+
+ return 0;
+}
+
struct mpic * __init mpic_alloc(struct device_node *node,
phys_addr_t phys_addr,
unsigned int flags,
@@ -1315,7 +1338,6 @@ struct mpic * __init mpic_alloc(struct device_node *node,
mpic_map(mpic, mpic->paddr, &mpic->tmregs, MPIC_INFO(TIMER_BASE), 0x1000);
if (mpic->flags & MPIC_FSL) {
- u32 brr1;
int ret;
/*
@@ -1326,9 +1348,7 @@ struct mpic * __init mpic_alloc(struct device_node *node,
mpic_map(mpic, mpic->paddr, &mpic->thiscpuregs,
MPIC_CPU_THISBASE, 0x1000);
- brr1 = _mpic_read(mpic->reg_type, &mpic->thiscpuregs,
- MPIC_FSL_BRR1);
- fsl_version = brr1 & MPIC_FSL_BRR1_VER;
+ fsl_version = fsl_mpic_get_version(mpic);
/* Error interrupt mask register (EIMR) is required for
* handling individual device error interrupts. EIMR
@@ -1518,9 +1538,7 @@ void __init mpic_init(struct mpic *mpic)
mpic_cpu_write(MPIC_INFO(CPU_CURRENT_TASK_PRI), 0xf);
if (mpic->flags & MPIC_FSL) {
- u32 brr1 = _mpic_read(mpic->reg_type, &mpic->thiscpuregs,
- MPIC_FSL_BRR1);
- u32 version = brr1 & MPIC_FSL_BRR1_VER;
+ u32 version = fsl_mpic_get_version(mpic);
/*
* Timer group B is present at the latest in MPIC 3.1 (e.g.
--
1.8.0
^ permalink raw reply related
* RE: [PATCH V4] powerpc/MPIC: Add get_version API both for internal and external use
From: Jia Hongtao-B38951 @ 2013-04-10 3:26 UTC (permalink / raw)
To: Wood Scott-B07421; +Cc: linuxppc-dev@lists.ozlabs.org, Li Yang-R58472
In-Reply-To: <1365564008.29365.3@snotra>
> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Wednesday, April 10, 2013 11:20 AM
> To: Jia Hongtao-B38951
> Cc: Wood Scott-B07421; linuxppc-dev@lists.ozlabs.org;
> galak@kernel.crashing.org; Li Yang-R58472
> Subject: Re: [PATCH V4] powerpc/MPIC: Add get_version API both for
> internal and external use
>=20
> On 04/09/2013 10:14:06 PM, Jia Hongtao-B38951 wrote:
> >
> >
> > > -----Original Message-----
> > > From: Wood Scott-B07421
> > > Sent: Wednesday, April 10, 2013 11:12 AM
> > > To: Jia Hongtao-B38951
> > > Cc: Wood Scott-B07421; linuxppc-dev@lists.ozlabs.org;
> > > galak@kernel.crashing.org; Li Yang-R58472
> > > Subject: Re: [PATCH V4] powerpc/MPIC: Add get_version API both for
> > > internal and external use
> > >
> > > On 04/09/2013 10:10:37 PM, Jia Hongtao-B38951 wrote:
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Wood Scott-B07421
> > > > > Sent: Wednesday, April 10, 2013 11:08 AM
> > > > > To: Jia Hongtao-B38951
> > > > > Cc: Wood Scott-B07421; linuxppc-dev@lists.ozlabs.org;
> > > > > galak@kernel.crashing.org; Li Yang-R58472
> > > > > Subject: Re: [PATCH V4] powerpc/MPIC: Add get_version API both
> > for
> > > > > internal and external use
> > > > >
> > > > > On 04/09/2013 10:04:44 PM, Jia Hongtao-B38951 wrote:
> > > > > > Since all the functions including mpic_alloc() and
> > mpic_init() do
> > > > the
> > > > > > check for MPIC_FSL before using fsl_mpic_get_version() I'd
> > like
> > > > to add
> > > > > > check just for fsl_mpic_primary_get_version().
> > > > > >
> > > > > > It will be like this:
> > > > > > u32 fsl_mpic_primary_get_version(void)
> > > > > > {
> > > > > > struct mpic *mpic =3D mpic_primary;
> > > > > >
> > > > > > if (mpic && (mpic->flags & MPIC_FSL))
> > > > > > return fsl_mpic_get_version(mpic);
> > > > > >
> > > > > > return 0;
> > > > > > }
> > > > > >
> > > > > > Could we reach an agreement here?
> > > > >
> > > > > Is there any particular reason? It would be more robust and
> > more
> > > > > consistent if the check were done in fsl_mpic_get_version().
> > > > >
> > > > > -Scott
> > > >
> > > > I found out that all the functions using fsl_mpic_get_version()
> > have
> > > > already done the check. Adding the check in fsl_mpic_get_version()
> > > > will cause duplicate check there. This is my consideration.
> > >
> > > Does that duplicate check cause any harm?
> > >
> > > -Scott
> >
> > No harm at all just not necessary.
>=20
> Not *necessary*, but makes it more robust and more consistent.
>=20
> > I wonder if I could add check in fsl_mpic_get_version() and remove
> > all the
> > check from functions in which using fsl_mpic_get_version()?
>=20
> One of the two places that calls it is the place that maps thiscpuregs
> in the first place, so no. :-)
Reasonable enough.
I will just add check in fsl_mpic_get_version().
Thanks.
-Hongtao.
>=20
> The check in mpic_init() for the number of timers could perhaps have
> the check removed if we're comfortable equating a version of zero with
> a non-FSL MPIC. This really isn't something that's worth worrying
> about, though.
>=20
> -Scott
^ permalink raw reply
* Re: [PATCH V4] powerpc/MPIC: Add get_version API both for internal and external use
From: Scott Wood @ 2013-04-10 3:20 UTC (permalink / raw)
To: Jia Hongtao-B38951
Cc: Wood Scott-B07421, linuxppc-dev@lists.ozlabs.org, Li Yang-R58472
In-Reply-To: <412C8208B4A0464FA894C5F0C278CD5D01C355B5@039-SN1MPN1-003.039d.mgd.msft.net>
On 04/09/2013 10:14:06 PM, Jia Hongtao-B38951 wrote:
>=20
>=20
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Wednesday, April 10, 2013 11:12 AM
> > To: Jia Hongtao-B38951
> > Cc: Wood Scott-B07421; linuxppc-dev@lists.ozlabs.org;
> > galak@kernel.crashing.org; Li Yang-R58472
> > Subject: Re: [PATCH V4] powerpc/MPIC: Add get_version API both for
> > internal and external use
> >
> > On 04/09/2013 10:10:37 PM, Jia Hongtao-B38951 wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Wood Scott-B07421
> > > > Sent: Wednesday, April 10, 2013 11:08 AM
> > > > To: Jia Hongtao-B38951
> > > > Cc: Wood Scott-B07421; linuxppc-dev@lists.ozlabs.org;
> > > > galak@kernel.crashing.org; Li Yang-R58472
> > > > Subject: Re: [PATCH V4] powerpc/MPIC: Add get_version API both =20
> for
> > > > internal and external use
> > > >
> > > > On 04/09/2013 10:04:44 PM, Jia Hongtao-B38951 wrote:
> > > > > Since all the functions including mpic_alloc() and =20
> mpic_init() do
> > > the
> > > > > check for MPIC_FSL before using fsl_mpic_get_version() I'd =20
> like
> > > to add
> > > > > check just for fsl_mpic_primary_get_version().
> > > > >
> > > > > It will be like this:
> > > > > u32 fsl_mpic_primary_get_version(void)
> > > > > {
> > > > > struct mpic *mpic =3D mpic_primary;
> > > > >
> > > > > if (mpic && (mpic->flags & MPIC_FSL))
> > > > > return fsl_mpic_get_version(mpic);
> > > > >
> > > > > return 0;
> > > > > }
> > > > >
> > > > > Could we reach an agreement here?
> > > >
> > > > Is there any particular reason? It would be more robust and =20
> more
> > > > consistent if the check were done in fsl_mpic_get_version().
> > > >
> > > > -Scott
> > >
> > > I found out that all the functions using fsl_mpic_get_version() =20
> have
> > > already done the check. Adding the check in fsl_mpic_get_version()
> > > will cause duplicate check there. This is my consideration.
> >
> > Does that duplicate check cause any harm?
> >
> > -Scott
>=20
> No harm at all just not necessary.
Not *necessary*, but makes it more robust and more consistent.
> I wonder if I could add check in fsl_mpic_get_version() and remove =20
> all the
> check from functions in which using fsl_mpic_get_version()?
One of the two places that calls it is the place that maps thiscpuregs =20
in the first place, so no. :-)
The check in mpic_init() for the number of timers could perhaps have =20
the check removed if we're comfortable equating a version of zero with =20
a non-FSL MPIC. This really isn't something that's worth worrying =20
about, though.
-Scott=
^ permalink raw reply
* RE: [PATCH V4] powerpc/MPIC: Add get_version API both for internal and external use
From: Jia Hongtao-B38951 @ 2013-04-10 3:14 UTC (permalink / raw)
To: Wood Scott-B07421; +Cc: linuxppc-dev@lists.ozlabs.org, Li Yang-R58472
In-Reply-To: <1365563520.29365.2@snotra>
> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Wednesday, April 10, 2013 11:12 AM
> To: Jia Hongtao-B38951
> Cc: Wood Scott-B07421; linuxppc-dev@lists.ozlabs.org;
> galak@kernel.crashing.org; Li Yang-R58472
> Subject: Re: [PATCH V4] powerpc/MPIC: Add get_version API both for
> internal and external use
>=20
> On 04/09/2013 10:10:37 PM, Jia Hongtao-B38951 wrote:
> >
> >
> > > -----Original Message-----
> > > From: Wood Scott-B07421
> > > Sent: Wednesday, April 10, 2013 11:08 AM
> > > To: Jia Hongtao-B38951
> > > Cc: Wood Scott-B07421; linuxppc-dev@lists.ozlabs.org;
> > > galak@kernel.crashing.org; Li Yang-R58472
> > > Subject: Re: [PATCH V4] powerpc/MPIC: Add get_version API both for
> > > internal and external use
> > >
> > > On 04/09/2013 10:04:44 PM, Jia Hongtao-B38951 wrote:
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Wood Scott-B07421
> > > > > Sent: Wednesday, April 10, 2013 10:32 AM
> > > > > To: Jia Hongtao-B38951
> > > > > Cc: linuxppc-dev@lists.ozlabs.org; galak@kernel.crashing.org;
> > Wood
> > > > Scott-
> > > > > B07421; Li Yang-R58472; Jia Hongtao-B38951
> > > > > Subject: Re: [PATCH V4] powerpc/MPIC: Add get_version API both
> > for
> > > > > internal and external use
> > > > >
> > > > > On 04/07/2013 09:01:54 PM, Jia Hongtao wrote:
> > > > > > diff --git a/arch/powerpc/sysdev/mpic.c
> > > > b/arch/powerpc/sysdev/mpic.c
> > > > > > index d30e6a6..48c8fae 100644
> > > > > > --- a/arch/powerpc/sysdev/mpic.c
> > > > > > +++ b/arch/powerpc/sysdev/mpic.c
> > > > > > @@ -1165,10 +1165,30 @@ static struct irq_domain_ops
> > > > mpic_host_ops =3D {
> > > > > > .xlate =3D mpic_host_xlate,
> > > > > > };
> > > > > >
> > > > > > +static u32 fsl_mpic_get_version(struct mpic *mpic) {
> > > > > > + u32 brr1;
> > > > > > +
> > > > > > + brr1 =3D _mpic_read(mpic->reg_type, &mpic->thiscpuregs,
> > > > > > + MPIC_FSL_BRR1);
> > > > > > +
> > > > > > + return brr1 & MPIC_FSL_BRR1_VER; }
> > > > >
> > > > > If it's not an FSL mpic, thiscpuregs->base will be NULL. Please
> > > > check
> > > > > mpic->flags for MPIC_FSL.
> > > > >
> > > > > > +
> > > > > > /*
> > > > > > * Exported functions
> > > > > > */
> > > > > >
> > > > > > +u32 fsl_mpic_primary_get_version(void)
> > > > > > +{
> > > > > > + struct mpic *mpic =3D mpic_primary;
> > > > > > +
> > > > > > + if (mpic)
> > > > > > + return fsl_mpic_get_version(mpic);
> > > > > > +
> > > > > > + return 0;
> > > > > > +}
> > > > >
> > > > > ...especially since the external version doesn't check for it
> > > > either.
> > > > >
> > > > > Otherwise, this and the MSI-X patch look OK to me.
> > > > >
> > > > > -Scott
> > > >
> > > >
> > > > Since all the functions including mpic_alloc() and mpic_init() do
> > the
> > > > check for MPIC_FSL before using fsl_mpic_get_version() I'd like
> > to add
> > > > check just for fsl_mpic_primary_get_version().
> > > >
> > > > It will be like this:
> > > > u32 fsl_mpic_primary_get_version(void)
> > > > {
> > > > struct mpic *mpic =3D mpic_primary;
> > > >
> > > > if (mpic && (mpic->flags & MPIC_FSL))
> > > > return fsl_mpic_get_version(mpic);
> > > >
> > > > return 0;
> > > > }
> > > >
> > > > Could we reach an agreement here?
> > >
> > > Is there any particular reason? It would be more robust and more
> > > consistent if the check were done in fsl_mpic_get_version().
> > >
> > > -Scott
> >
> > I found out that all the functions using fsl_mpic_get_version() have
> > already done the check. Adding the check in fsl_mpic_get_version()
> > will cause duplicate check there. This is my consideration.
>=20
> Does that duplicate check cause any harm?
>=20
> -Scott
No harm at all just not necessary.
I wonder if I could add check in fsl_mpic_get_version() and remove all the
check from functions in which using fsl_mpic_get_version()?
-Hongtao.
^ permalink raw reply
* Re: [PATCH V4] powerpc/MPIC: Add get_version API both for internal and external use
From: Scott Wood @ 2013-04-10 3:12 UTC (permalink / raw)
To: Jia Hongtao-B38951
Cc: Wood Scott-B07421, linuxppc-dev@lists.ozlabs.org, Li Yang-R58472
In-Reply-To: <412C8208B4A0464FA894C5F0C278CD5D01C35599@039-SN1MPN1-003.039d.mgd.msft.net>
On 04/09/2013 10:10:37 PM, Jia Hongtao-B38951 wrote:
>=20
>=20
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Wednesday, April 10, 2013 11:08 AM
> > To: Jia Hongtao-B38951
> > Cc: Wood Scott-B07421; linuxppc-dev@lists.ozlabs.org;
> > galak@kernel.crashing.org; Li Yang-R58472
> > Subject: Re: [PATCH V4] powerpc/MPIC: Add get_version API both for
> > internal and external use
> >
> > On 04/09/2013 10:04:44 PM, Jia Hongtao-B38951 wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Wood Scott-B07421
> > > > Sent: Wednesday, April 10, 2013 10:32 AM
> > > > To: Jia Hongtao-B38951
> > > > Cc: linuxppc-dev@lists.ozlabs.org; galak@kernel.crashing.org; =20
> Wood
> > > Scott-
> > > > B07421; Li Yang-R58472; Jia Hongtao-B38951
> > > > Subject: Re: [PATCH V4] powerpc/MPIC: Add get_version API both =20
> for
> > > > internal and external use
> > > >
> > > > On 04/07/2013 09:01:54 PM, Jia Hongtao wrote:
> > > > > diff --git a/arch/powerpc/sysdev/mpic.c
> > > b/arch/powerpc/sysdev/mpic.c
> > > > > index d30e6a6..48c8fae 100644
> > > > > --- a/arch/powerpc/sysdev/mpic.c
> > > > > +++ b/arch/powerpc/sysdev/mpic.c
> > > > > @@ -1165,10 +1165,30 @@ static struct irq_domain_ops
> > > mpic_host_ops =3D {
> > > > > .xlate =3D mpic_host_xlate,
> > > > > };
> > > > >
> > > > > +static u32 fsl_mpic_get_version(struct mpic *mpic) {
> > > > > + u32 brr1;
> > > > > +
> > > > > + brr1 =3D _mpic_read(mpic->reg_type, &mpic->thiscpuregs,
> > > > > + MPIC_FSL_BRR1);
> > > > > +
> > > > > + return brr1 & MPIC_FSL_BRR1_VER;
> > > > > +}
> > > >
> > > > If it's not an FSL mpic, thiscpuregs->base will be NULL. Please
> > > check
> > > > mpic->flags for MPIC_FSL.
> > > >
> > > > > +
> > > > > /*
> > > > > * Exported functions
> > > > > */
> > > > >
> > > > > +u32 fsl_mpic_primary_get_version(void)
> > > > > +{
> > > > > + struct mpic *mpic =3D mpic_primary;
> > > > > +
> > > > > + if (mpic)
> > > > > + return fsl_mpic_get_version(mpic);
> > > > > +
> > > > > + return 0;
> > > > > +}
> > > >
> > > > ...especially since the external version doesn't check for it
> > > either.
> > > >
> > > > Otherwise, this and the MSI-X patch look OK to me.
> > > >
> > > > -Scott
> > >
> > >
> > > Since all the functions including mpic_alloc() and mpic_init() do =20
> the
> > > check for MPIC_FSL before using fsl_mpic_get_version() I'd like =20
> to add
> > > check just for fsl_mpic_primary_get_version().
> > >
> > > It will be like this:
> > > u32 fsl_mpic_primary_get_version(void)
> > > {
> > > struct mpic *mpic =3D mpic_primary;
> > >
> > > if (mpic && (mpic->flags & MPIC_FSL))
> > > return fsl_mpic_get_version(mpic);
> > >
> > > return 0;
> > > }
> > >
> > > Could we reach an agreement here?
> >
> > Is there any particular reason? It would be more robust and more
> > consistent if the check were done in fsl_mpic_get_version().
> >
> > -Scott
>=20
> I found out that all the functions using fsl_mpic_get_version() have
> already done the check. Adding the check in fsl_mpic_get_version() =20
> will
> cause duplicate check there. This is my consideration.
Does that duplicate check cause any harm?
-Scott=
^ permalink raw reply
* RE: [PATCH V4] powerpc/MPIC: Add get_version API both for internal and external use
From: Jia Hongtao-B38951 @ 2013-04-10 3:10 UTC (permalink / raw)
To: Wood Scott-B07421; +Cc: linuxppc-dev@lists.ozlabs.org, Li Yang-R58472
In-Reply-To: <1365563255.29365.1@snotra>
> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Wednesday, April 10, 2013 11:08 AM
> To: Jia Hongtao-B38951
> Cc: Wood Scott-B07421; linuxppc-dev@lists.ozlabs.org;
> galak@kernel.crashing.org; Li Yang-R58472
> Subject: Re: [PATCH V4] powerpc/MPIC: Add get_version API both for
> internal and external use
>=20
> On 04/09/2013 10:04:44 PM, Jia Hongtao-B38951 wrote:
> >
> >
> > > -----Original Message-----
> > > From: Wood Scott-B07421
> > > Sent: Wednesday, April 10, 2013 10:32 AM
> > > To: Jia Hongtao-B38951
> > > Cc: linuxppc-dev@lists.ozlabs.org; galak@kernel.crashing.org; Wood
> > Scott-
> > > B07421; Li Yang-R58472; Jia Hongtao-B38951
> > > Subject: Re: [PATCH V4] powerpc/MPIC: Add get_version API both for
> > > internal and external use
> > >
> > > On 04/07/2013 09:01:54 PM, Jia Hongtao wrote:
> > > > diff --git a/arch/powerpc/sysdev/mpic.c
> > b/arch/powerpc/sysdev/mpic.c
> > > > index d30e6a6..48c8fae 100644
> > > > --- a/arch/powerpc/sysdev/mpic.c
> > > > +++ b/arch/powerpc/sysdev/mpic.c
> > > > @@ -1165,10 +1165,30 @@ static struct irq_domain_ops
> > mpic_host_ops =3D {
> > > > .xlate =3D mpic_host_xlate,
> > > > };
> > > >
> > > > +static u32 fsl_mpic_get_version(struct mpic *mpic) {
> > > > + u32 brr1;
> > > > +
> > > > + brr1 =3D _mpic_read(mpic->reg_type, &mpic->thiscpuregs,
> > > > + MPIC_FSL_BRR1);
> > > > +
> > > > + return brr1 & MPIC_FSL_BRR1_VER;
> > > > +}
> > >
> > > If it's not an FSL mpic, thiscpuregs->base will be NULL. Please
> > check
> > > mpic->flags for MPIC_FSL.
> > >
> > > > +
> > > > /*
> > > > * Exported functions
> > > > */
> > > >
> > > > +u32 fsl_mpic_primary_get_version(void)
> > > > +{
> > > > + struct mpic *mpic =3D mpic_primary;
> > > > +
> > > > + if (mpic)
> > > > + return fsl_mpic_get_version(mpic);
> > > > +
> > > > + return 0;
> > > > +}
> > >
> > > ...especially since the external version doesn't check for it
> > either.
> > >
> > > Otherwise, this and the MSI-X patch look OK to me.
> > >
> > > -Scott
> >
> >
> > Since all the functions including mpic_alloc() and mpic_init() do the
> > check for MPIC_FSL before using fsl_mpic_get_version() I'd like to add
> > check just for fsl_mpic_primary_get_version().
> >
> > It will be like this:
> > u32 fsl_mpic_primary_get_version(void)
> > {
> > struct mpic *mpic =3D mpic_primary;
> >
> > if (mpic && (mpic->flags & MPIC_FSL))
> > return fsl_mpic_get_version(mpic);
> >
> > return 0;
> > }
> >
> > Could we reach an agreement here?
>=20
> Is there any particular reason? It would be more robust and more
> consistent if the check were done in fsl_mpic_get_version().
>=20
> -Scott
I found out that all the functions using fsl_mpic_get_version() have
already done the check. Adding the check in fsl_mpic_get_version() will
cause duplicate check there. This is my consideration.
-Hongtao.
^ permalink raw reply
* Re: [PATCH V4] powerpc/MPIC: Add get_version API both for internal and external use
From: Scott Wood @ 2013-04-10 3:07 UTC (permalink / raw)
To: Jia Hongtao-B38951
Cc: Wood Scott-B07421, linuxppc-dev@lists.ozlabs.org, Li Yang-R58472
In-Reply-To: <412C8208B4A0464FA894C5F0C278CD5D01C3557B@039-SN1MPN1-003.039d.mgd.msft.net>
On 04/09/2013 10:04:44 PM, Jia Hongtao-B38951 wrote:
>=20
>=20
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Wednesday, April 10, 2013 10:32 AM
> > To: Jia Hongtao-B38951
> > Cc: linuxppc-dev@lists.ozlabs.org; galak@kernel.crashing.org; Wood =20
> Scott-
> > B07421; Li Yang-R58472; Jia Hongtao-B38951
> > Subject: Re: [PATCH V4] powerpc/MPIC: Add get_version API both for
> > internal and external use
> >
> > On 04/07/2013 09:01:54 PM, Jia Hongtao wrote:
> > > diff --git a/arch/powerpc/sysdev/mpic.c =20
> b/arch/powerpc/sysdev/mpic.c
> > > index d30e6a6..48c8fae 100644
> > > --- a/arch/powerpc/sysdev/mpic.c
> > > +++ b/arch/powerpc/sysdev/mpic.c
> > > @@ -1165,10 +1165,30 @@ static struct irq_domain_ops =20
> mpic_host_ops =3D {
> > > .xlate =3D mpic_host_xlate,
> > > };
> > >
> > > +static u32 fsl_mpic_get_version(struct mpic *mpic) {
> > > + u32 brr1;
> > > +
> > > + brr1 =3D _mpic_read(mpic->reg_type, &mpic->thiscpuregs,
> > > + MPIC_FSL_BRR1);
> > > +
> > > + return brr1 & MPIC_FSL_BRR1_VER;
> > > +}
> >
> > If it's not an FSL mpic, thiscpuregs->base will be NULL. Please =20
> check
> > mpic->flags for MPIC_FSL.
> >
> > > +
> > > /*
> > > * Exported functions
> > > */
> > >
> > > +u32 fsl_mpic_primary_get_version(void)
> > > +{
> > > + struct mpic *mpic =3D mpic_primary;
> > > +
> > > + if (mpic)
> > > + return fsl_mpic_get_version(mpic);
> > > +
> > > + return 0;
> > > +}
> >
> > ...especially since the external version doesn't check for it =20
> either.
> >
> > Otherwise, this and the MSI-X patch look OK to me.
> >
> > -Scott
>=20
>=20
> Since all the functions including mpic_alloc() and mpic_init() do the
> check for MPIC_FSL before using fsl_mpic_get_version() I'd like to add
> check just for fsl_mpic_primary_get_version().
>=20
> It will be like this:
> u32 fsl_mpic_primary_get_version(void)
> {
> struct mpic *mpic =3D mpic_primary;
>=20
> if (mpic && (mpic->flags & MPIC_FSL))
> return fsl_mpic_get_version(mpic);
>=20
> return 0;
> }
>=20
> Could we reach an agreement here?
Is there any particular reason? It would be more robust and more =20
consistent if the check were done in fsl_mpic_get_version().
-Scott=
^ permalink raw reply
* RE: [PATCH V4] powerpc/MPIC: Add get_version API both for internal and external use
From: Jia Hongtao-B38951 @ 2013-04-10 3:04 UTC (permalink / raw)
To: Wood Scott-B07421; +Cc: linuxppc-dev@lists.ozlabs.org, Li Yang-R58472
In-Reply-To: <1365561148.29365.0@snotra>
> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Wednesday, April 10, 2013 10:32 AM
> To: Jia Hongtao-B38951
> Cc: linuxppc-dev@lists.ozlabs.org; galak@kernel.crashing.org; Wood Scott-
> B07421; Li Yang-R58472; Jia Hongtao-B38951
> Subject: Re: [PATCH V4] powerpc/MPIC: Add get_version API both for
> internal and external use
>=20
> On 04/07/2013 09:01:54 PM, Jia Hongtao wrote:
> > diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
> > index d30e6a6..48c8fae 100644
> > --- a/arch/powerpc/sysdev/mpic.c
> > +++ b/arch/powerpc/sysdev/mpic.c
> > @@ -1165,10 +1165,30 @@ static struct irq_domain_ops mpic_host_ops =3D =
{
> > .xlate =3D mpic_host_xlate,
> > };
> >
> > +static u32 fsl_mpic_get_version(struct mpic *mpic) {
> > + u32 brr1;
> > +
> > + brr1 =3D _mpic_read(mpic->reg_type, &mpic->thiscpuregs,
> > + MPIC_FSL_BRR1);
> > +
> > + return brr1 & MPIC_FSL_BRR1_VER;
> > +}
>=20
> If it's not an FSL mpic, thiscpuregs->base will be NULL. Please check
> mpic->flags for MPIC_FSL.
>=20
> > +
> > /*
> > * Exported functions
> > */
> >
> > +u32 fsl_mpic_primary_get_version(void)
> > +{
> > + struct mpic *mpic =3D mpic_primary;
> > +
> > + if (mpic)
> > + return fsl_mpic_get_version(mpic);
> > +
> > + return 0;
> > +}
>=20
> ...especially since the external version doesn't check for it either.
>=20
> Otherwise, this and the MSI-X patch look OK to me.
>=20
> -Scott
Since all the functions including mpic_alloc() and mpic_init() do the
check for MPIC_FSL before using fsl_mpic_get_version() I'd like to add
check just for fsl_mpic_primary_get_version().
It will be like this:
u32 fsl_mpic_primary_get_version(void)
{
struct mpic *mpic =3D mpic_primary;
if (mpic && (mpic->flags & MPIC_FSL))
return fsl_mpic_get_version(mpic);
return 0;
}
Could we reach an agreement here?
Thanks.
-Hongtao.
^ permalink raw reply
* RE: [PATCH V4] powerpc/MPIC: Add get_version API both for internal and external use
From: Jia Hongtao-B38951 @ 2013-04-10 2:35 UTC (permalink / raw)
To: Wood Scott-B07421; +Cc: linuxppc-dev@lists.ozlabs.org, Li Yang-R58472
In-Reply-To: <1365561148.29365.0@snotra>
> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Wednesday, April 10, 2013 10:32 AM
> To: Jia Hongtao-B38951
> Cc: linuxppc-dev@lists.ozlabs.org; galak@kernel.crashing.org; Wood Scott-
> B07421; Li Yang-R58472; Jia Hongtao-B38951
> Subject: Re: [PATCH V4] powerpc/MPIC: Add get_version API both for
> internal and external use
>=20
> On 04/07/2013 09:01:54 PM, Jia Hongtao wrote:
> > diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
> > index d30e6a6..48c8fae 100644
> > --- a/arch/powerpc/sysdev/mpic.c
> > +++ b/arch/powerpc/sysdev/mpic.c
> > @@ -1165,10 +1165,30 @@ static struct irq_domain_ops mpic_host_ops =3D =
{
> > .xlate =3D mpic_host_xlate,
> > };
> >
> > +static u32 fsl_mpic_get_version(struct mpic *mpic) {
> > + u32 brr1;
> > +
> > + brr1 =3D _mpic_read(mpic->reg_type, &mpic->thiscpuregs,
> > + MPIC_FSL_BRR1);
> > +
> > + return brr1 & MPIC_FSL_BRR1_VER;
> > +}
>=20
> If it's not an FSL mpic, thiscpuregs->base will be NULL. Please check
> mpic->flags for MPIC_FSL.
I will add the check soon.
>=20
> > +
> > /*
> > * Exported functions
> > */
> >
> > +u32 fsl_mpic_primary_get_version(void)
> > +{
> > + struct mpic *mpic =3D mpic_primary;
> > +
> > + if (mpic)
> > + return fsl_mpic_get_version(mpic);
> > +
> > + return 0;
> > +}
>=20
> ...especially since the external version doesn't check for it either.
>=20
> Otherwise, this and the MSI-X patch look OK to me.
>=20
> -Scott
Thanks.
-Hongtao.
^ permalink raw reply
* Re: [PATCH V4] powerpc/MPIC: Add get_version API both for internal and external use
From: Scott Wood @ 2013-04-10 2:32 UTC (permalink / raw)
To: Jia Hongtao; +Cc: hongtao.jia, B07421, linuxppc-dev
In-Reply-To: <1365386514-14647-1-git-send-email-hongtao.jia@freescale.com>
On 04/07/2013 09:01:54 PM, Jia Hongtao wrote:
> diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
> index d30e6a6..48c8fae 100644
> --- a/arch/powerpc/sysdev/mpic.c
> +++ b/arch/powerpc/sysdev/mpic.c
> @@ -1165,10 +1165,30 @@ static struct irq_domain_ops mpic_host_ops =3D {
> .xlate =3D mpic_host_xlate,
> };
>=20
> +static u32 fsl_mpic_get_version(struct mpic *mpic)
> +{
> + u32 brr1;
> +
> + brr1 =3D _mpic_read(mpic->reg_type, &mpic->thiscpuregs,
> + MPIC_FSL_BRR1);
> +
> + return brr1 & MPIC_FSL_BRR1_VER;
> +}
If it's not an FSL mpic, thiscpuregs->base will be NULL. Please check =20
mpic->flags for MPIC_FSL.
> +
> /*
> * Exported functions
> */
>=20
> +u32 fsl_mpic_primary_get_version(void)
> +{
> + struct mpic *mpic =3D mpic_primary;
> +
> + if (mpic)
> + return fsl_mpic_get_version(mpic);
> +
> + return 0;
> +}
...especially since the external version doesn't check for it either.
Otherwise, this and the MSI-X patch look OK to me.
-Scott=
^ permalink raw reply
* RE: [PATCH V5] powerpc/85xx: Add machine check handler to fix PCIe erratum on mpc85xx
From: Jia Hongtao-B38951 @ 2013-04-10 2:27 UTC (permalink / raw)
To: Wood Scott-B07421
Cc: Jia Hongtao-B38951, linuxppc-dev@lists.ozlabs.org, Li Yang-R58472
In-Reply-To: <1365409614-2634-1-git-send-email-hongtao.jia@freescale.com>
Hi Scott,
I added load instruction handler for the skipped instruction.
For now most common load instructions are handled in this patch.
Any advice for this?
Thanks.
-Hongtao.
> -----Original Message-----
> From: Jia Hongtao-B38951
> Sent: Monday, April 08, 2013 4:27 PM
> To: linuxppc-dev@lists.ozlabs.org; galak@kernel.crashing.org
> Cc: Wood Scott-B07421; Li Yang-R58472; Jia Hongtao-B38951
> Subject: [PATCH V5] powerpc/85xx: Add machine check handler to fix PCIe
> erratum on mpc85xx
>=20
> A PCIe erratum of mpc85xx may causes a core hang when a link of PCIe goes
> down. when the link goes down, Non-posted transactions issued via the
> ATMU requiring completion result in an instruction stall.
> At the same time a machine-check exception is generated to the core to
> allow further processing by the handler. We implements the handler which
> skips the instruction caused the stall.
>=20
> This patch depends on patch:
> powerpc/85xx: Add platform_device declaration to fsl_pci.h
>=20
> Signed-off-by: Zhao Chenhui <b35336@freescale.com>
> Signed-off-by: Li Yang <leoli@freescale.com>
> Signed-off-by: Liu Shuo <soniccat.liu@gmail.com>
> Signed-off-by: Jia Hongtao <hongtao.jia@freescale.com>
> ---
> Changes for V4:
> * Fill rd with all-Fs if the skipped instruction is load and emulate the
> instruction.
> * Let KVM/QEMU deal with the exception if the machine check comes from
> KVM.
>=20
> arch/powerpc/kernel/cpu_setup_fsl_booke.S | 2 +-
> arch/powerpc/kernel/traps.c | 3 +
> arch/powerpc/sysdev/fsl_pci.c | 121
> ++++++++++++++++++++++++++++++
> arch/powerpc/sysdev/fsl_pci.h | 6 ++
> 4 files changed, 131 insertions(+), 1 deletion(-)
>=20
> diff --git a/arch/powerpc/kernel/cpu_setup_fsl_booke.S
> b/arch/powerpc/kernel/cpu_setup_fsl_booke.S
> index dcd8819..f1bde90 100644
> --- a/arch/powerpc/kernel/cpu_setup_fsl_booke.S
> +++ b/arch/powerpc/kernel/cpu_setup_fsl_booke.S
> @@ -66,7 +66,7 @@ _GLOBAL(__setup_cpu_e500v2)
> bl __e500_icache_setup
> bl __e500_dcache_setup
> bl __setup_e500_ivors
> -#ifdef CONFIG_FSL_RIO
> +#if defined(CONFIG_FSL_RIO) || defined(CONFIG_FSL_PCI)
> /* Ensure that RFXE is set */
> mfspr r3,SPRN_HID1
> oris r3,r3,HID1_RFXE@h
> diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
> index a008cf5..dd275a4 100644
> --- a/arch/powerpc/kernel/traps.c
> +++ b/arch/powerpc/kernel/traps.c
> @@ -59,6 +59,7 @@
> #include <asm/fadump.h>
> #include <asm/switch_to.h>
> #include <asm/debug.h>
> +#include <sysdev/fsl_pci.h>
>=20
> #if defined(CONFIG_DEBUGGER) || defined(CONFIG_KEXEC) int
> (*__debugger)(struct pt_regs *regs) __read_mostly; @@ -556,6 +557,8 @@
> int machine_check_e500(struct pt_regs *regs)
> if (reason & MCSR_BUS_RBERR) {
> if (fsl_rio_mcheck_exception(regs))
> return 1;
> + if (fsl_pci_mcheck_exception(regs))
> + return 1;
> }
>=20
> printk("Machine check in kernel mode.\n"); diff --git
> a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c index
> 682084d..48326cd 100644
> --- a/arch/powerpc/sysdev/fsl_pci.c
> +++ b/arch/powerpc/sysdev/fsl_pci.c
> @@ -26,11 +26,14 @@
> #include <linux/memblock.h>
> #include <linux/log2.h>
> #include <linux/slab.h>
> +#include <linux/uaccess.h>
>=20
> #include <asm/io.h>
> #include <asm/prom.h>
> #include <asm/pci-bridge.h>
> +#include <asm/ppc-pci.h>
> #include <asm/machdep.h>
> +#include <asm/disassemble.h>
> #include <sysdev/fsl_soc.h>
> #include <sysdev/fsl_pci.h>
>=20
> @@ -826,6 +829,124 @@ u64 fsl_pci_immrbar_base(struct pci_controller
> *hose)
> return 0;
> }
>=20
> +#ifdef CONFIG_E500
> +
> +#define OP_LWZ 32
> +#define OP_LWZU 33
> +#define OP_LBZ 34
> +#define OP_LBZU 35
> +#define OP_LHZ 40
> +#define OP_LHZU 41
> +#define OP_LHA 42
> +#define OP_LHAU 43
> +
> +static int mcheck_handle_load(struct pt_regs *regs, u32 inst) {
> + unsigned int rd, ra, d;
> +
> + rd =3D get_rt(inst);
> + ra =3D get_ra(inst);
> + d =3D get_d(inst);
> +
> + switch (get_op(inst)) {
> + case OP_LWZ:
> + regs->gpr[rd] =3D 0xffffffff;
> + break;
> +
> + case OP_LWZU:
> + regs->gpr[rd] =3D 0xffffffff;
> + regs->gpr[ra] +=3D (s16)d;
> + break;
> +
> + case OP_LBZ:
> + regs->gpr[rd] =3D 0xff;
> + break;
> +
> + case OP_LBZU:
> + regs->gpr[rd] =3D 0xff;
> + regs->gpr[ra] +=3D (s16)d;
> + break;
> +
> + case OP_LHZ:
> + regs->gpr[rd] =3D 0xffff;
> + break;
> +
> + case OP_LHZU:
> + regs->gpr[rd] =3D 0xffff;
> + regs->gpr[ra] +=3D (s16)d;
> + break;
> +
> + case OP_LHA:
> + regs->gpr[rd] =3D 0xffff;
> + break;
> +
> + case OP_LHAU:
> + regs->gpr[rd] =3D 0xffff;
> + regs->gpr[ra] +=3D (s16)d;
> + break;
> +
> + default:
> + return 0;
> + }
> +
> + return 1;
> +}
> +
> +static int is_in_pci_mem_space(phys_addr_t addr) {
> + struct pci_controller *hose;
> + struct resource *res;
> + int i;
> +
> + list_for_each_entry(hose, &hose_list, list_node) {
> + if (!early_find_capability(hose, 0, 0, PCI_CAP_ID_EXP))
> + continue;
> +
> + for (i =3D 0; i < 3; i++) {
> + res =3D &hose->mem_resources[i];
> + if ((res->flags & IORESOURCE_MEM) &&
> + addr >=3D res->start && addr <=3D res->end)
> + return 1;
> + }
> + }
> + return 0;
> +}
> +
> +int fsl_pci_mcheck_exception(struct pt_regs *regs) {
> + u32 inst;
> + int ret;
> + phys_addr_t addr =3D 0;
> +
> + /* Let KVM/QEMU deal with the exception */
> + if (regs->msr & MSR_GS)
> + return 0;
> +
> +#ifdef CONFIG_PHYS_64BIT
> + addr =3D mfspr(SPRN_MCARU);
> + addr <<=3D 32;
> +#endif
> + addr +=3D mfspr(SPRN_MCAR);
> +
> + if (is_in_pci_mem_space(addr)) {
> + if (user_mode(regs)) {
> + pagefault_disable();
> + ret =3D get_user(regs->nip, &inst);
> + pagefault_enable();
> + } else {
> + ret =3D probe_kernel_address(regs->nip, inst);
> + }
> +
> + if (mcheck_handle_load(regs, inst)) {
> + regs->nip +=3D 4;
> + return 1;
> + }
> + }
> +
> + return 0;
> +}
> +#endif
> +
> #if defined(CONFIG_FSL_SOC_BOOKE) || defined(CONFIG_PPC_86xx) static
> const struct of_device_id pci_ids[] =3D {
> { .compatible =3D "fsl,mpc8540-pci", },
> diff --git a/arch/powerpc/sysdev/fsl_pci.h
> b/arch/powerpc/sysdev/fsl_pci.h index 851dd56..b0d01ea 100644
> --- a/arch/powerpc/sysdev/fsl_pci.h
> +++ b/arch/powerpc/sysdev/fsl_pci.h
> @@ -115,5 +115,11 @@ static inline int mpc85xx_pci_err_probe(struct
> platform_device *op) } #endif
>=20
> +#ifdef CONFIG_FSL_PCI
> +extern int fsl_pci_mcheck_exception(struct pt_regs *); #else static
> +inline int fsl_pci_mcheck_exception(struct pt_regs *regs) {return 0; }
> +#endif
> +
> #endif /* __POWERPC_FSL_PCI_H */
> #endif /* __KERNEL__ */
> --
> 1.8.0
^ permalink raw reply
* RE: [PATCH V4] powerpc/MPIC: Add get_version API both for internal and external use
From: Jia Hongtao-B38951 @ 2013-04-10 2:22 UTC (permalink / raw)
To: Wood Scott-B07421, galak@kernel.crashing.org
Cc: linuxppc-dev@lists.ozlabs.org, Li Yang-R58472, Jia Hongtao-B38951
In-Reply-To: <1365386514-14647-1-git-send-email-hongtao.jia@freescale.com>
Hi Kumar and Scott,
Any more comments for this patch and MSI-X erratum patch?
Thanks.
-Hongtao.
> -----Original Message-----
> From: Jia Hongtao-B38951
> Sent: Monday, April 08, 2013 10:02 AM
> To: linuxppc-dev@lists.ozlabs.org; galak@kernel.crashing.org
> Cc: Wood Scott-B07421; Li Yang-R58472; Jia Hongtao-B38951
> Subject: [PATCH V4] powerpc/MPIC: Add get_version API both for internal
> and external use
>=20
> MPIC version is useful information for both mpic_alloc() and mpic_init().
> The patch provide an API to get MPIC version for reusing the code.
> Also, some other IP block may need MPIC version for their own use.
> The API for external use is also provided.
>=20
> Signed-off-by: Jia Hongtao <hongtao.jia@freescale.com>
> Signed-off-by: Li Yang <leoli@freescale.com>
> ---
> Changes for V4:
> * change the name of function from mpic_get_version() to
> fsl_mpic_get_version().
>=20
> Changes for V3:
> * change the name of function from mpic_primary_get_version() to
> fsl_mpic_primary_get_version().
> * return 0 if mpic_primary is null.
>=20
> Changes for V2:
> * Using mpic_get_version() to implement mpic_primary_get_version()
>=20
> arch/powerpc/include/asm/mpic.h | 3 +++
> arch/powerpc/sysdev/mpic.c | 29 ++++++++++++++++++++++-------
> 2 files changed, 25 insertions(+), 7 deletions(-)
>=20
> diff --git a/arch/powerpc/include/asm/mpic.h
> b/arch/powerpc/include/asm/mpic.h index c0f9ef9..ea6bf72 100644
> --- a/arch/powerpc/include/asm/mpic.h
> +++ b/arch/powerpc/include/asm/mpic.h
> @@ -393,6 +393,9 @@ struct mpic
> #define MPIC_REGSET_STANDARD MPIC_REGSET(0) /* Original
> MPIC */
> #define MPIC_REGSET_TSI108 MPIC_REGSET(1) /* Tsi108/109
> PIC */
>=20
> +/* Get the version of primary MPIC */
> +extern u32 fsl_mpic_primary_get_version(void);
> +
> /* Allocate the controller structure and setup the linux irq descs
> * for the range if interrupts passed in. No HW initialization is
> * actually performed.
> diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
> index d30e6a6..48c8fae 100644
> --- a/arch/powerpc/sysdev/mpic.c
> +++ b/arch/powerpc/sysdev/mpic.c
> @@ -1165,10 +1165,30 @@ static struct irq_domain_ops mpic_host_ops =3D {
> .xlate =3D mpic_host_xlate,
> };
>=20
> +static u32 fsl_mpic_get_version(struct mpic *mpic) {
> + u32 brr1;
> +
> + brr1 =3D _mpic_read(mpic->reg_type, &mpic->thiscpuregs,
> + MPIC_FSL_BRR1);
> +
> + return brr1 & MPIC_FSL_BRR1_VER;
> +}
> +
> /*
> * Exported functions
> */
>=20
> +u32 fsl_mpic_primary_get_version(void)
> +{
> + struct mpic *mpic =3D mpic_primary;
> +
> + if (mpic)
> + return fsl_mpic_get_version(mpic);
> +
> + return 0;
> +}
> +
> struct mpic * __init mpic_alloc(struct device_node *node,
> phys_addr_t phys_addr,
> unsigned int flags,
> @@ -1315,7 +1335,6 @@ struct mpic * __init mpic_alloc(struct device_node
> *node,
> mpic_map(mpic, mpic->paddr, &mpic->tmregs, MPIC_INFO(TIMER_BASE),
> 0x1000);
>=20
> if (mpic->flags & MPIC_FSL) {
> - u32 brr1;
> int ret;
>=20
> /*
> @@ -1326,9 +1345,7 @@ struct mpic * __init mpic_alloc(struct device_node
> *node,
> mpic_map(mpic, mpic->paddr, &mpic->thiscpuregs,
> MPIC_CPU_THISBASE, 0x1000);
>=20
> - brr1 =3D _mpic_read(mpic->reg_type, &mpic->thiscpuregs,
> - MPIC_FSL_BRR1);
> - fsl_version =3D brr1 & MPIC_FSL_BRR1_VER;
> + fsl_version =3D fsl_mpic_get_version(mpic);
>=20
> /* Error interrupt mask register (EIMR) is required for
> * handling individual device error interrupts. EIMR @@ -
> 1518,9 +1535,7 @@ void __init mpic_init(struct mpic *mpic)
> mpic_cpu_write(MPIC_INFO(CPU_CURRENT_TASK_PRI), 0xf);
>=20
> if (mpic->flags & MPIC_FSL) {
> - u32 brr1 =3D _mpic_read(mpic->reg_type, &mpic->thiscpuregs,
> - MPIC_FSL_BRR1);
> - u32 version =3D brr1 & MPIC_FSL_BRR1_VER;
> + u32 version =3D fsl_mpic_get_version(mpic);
>=20
> /*
> * Timer group B is present at the latest in MPIC 3.1 (e.g.
> --
> 1.8.0
^ permalink raw reply
* Re: [RFC PATCH powerpc] try secondary hash before BUG in kernel_map_linear_page()
From: Michael Ellerman @ 2013-04-10 2:21 UTC (permalink / raw)
To: Li Zhong; +Cc: Paul Mackerras, PowerPC email list
In-Reply-To: <1361784575.3001.5.camel@ThinkPad-T5421.cn.ibm.com>
On Mon, Feb 25, 2013 at 05:29:35PM +0800, Li Zhong wrote:
> This patch tries to fix following issue when CONFIG_DEBUG_PAGEALLOC
> is enabled:
>
> [ 543.075675] ------------[ cut here ]------------
> [ 543.075701] kernel BUG at arch/powerpc/mm/hash_utils_64.c:1239!
> [ 543.075714] Oops: Exception in kernel mode, sig: 5 [#1]
So the issue is that kernel_map_linear_page() doesn't try the secondary
hash slot.
> The code is borrowed from that in __hash_page_huge().
It is, and in fact there is another copy in hash_low_64.S - in assembler.
So I think we should at least try and keep ourselves to two
implementations, one in asm and one in C. So can you split it out into a
helper routine called by both kernel_map_linear_page() and
__hash_page_huge() ?
cheers
^ permalink raw reply
* Re: [PATCH] fadump: allow duplicate assignment to /sys/kernel/fadump_registered when it's assigned the desired value already
From: Mahesh Jagannath Salgaonkar @ 2013-04-10 1:43 UTC (permalink / raw)
To: Wang Sheng-Hui; +Cc: Paul Mackerras, linuxppc-dev, Suzuki K. Poulose
In-Reply-To: <5164B664.6070306@gmail.com>
On 04/10/2013 06:16 AM, Wang Sheng-Hui wrote:
> When the fadump is enabled, we have /sys/kernel/fadump_enabled assigned 1.
> But sometimes we need to restart the fadump service by 'service kdump
> restart',
> in case the kdump script has added the fadump detect/support already.
> In current implementation, we cannot re-assign 1 to
> /sys/kernel/fadump_enabled
I assume you meant /sys/kernel/fadump_registered here. Ideally service
kdump restart should first echo 0 to /sys/kernel/fadump_registered to
un-register fadump before echo 1 to /sys/kernel/fadump_registered for
re-registration. I would fix the user space tool than changing the
kernel code.
> if 1 is already set and we have added more logic check in the user space
> script.
>
> I think we can enable the duplicate assignment to ease the user space
> tools,
> as long as the value is the right 1 or 0.
>
> Signed-off-by: Wang Sheng-Hui <shhuiw@gmail.com>
> ---
> arch/powerpc/kernel/fadump.c | 8 ++------
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
> index 06c8202..e1347e5 100644
> --- a/arch/powerpc/kernel/fadump.c
> +++ b/arch/powerpc/kernel/fadump.c
> @@ -1140,18 +1140,14 @@ static ssize_t fadump_register_store(struct
> kobject *kobj,
>
> switch (buf[0]) {
> case '0':
> - if (fw_dump.dump_registered == 0) {
> - ret = -EINVAL;
> + if (fw_dump.dump_registered == 0)
> goto unlock_out;
> - }
> /* Un-register Firmware-assisted dump */
> fadump_unregister_dump(&fdm);
> break;
> case '1':
> - if (fw_dump.dump_registered == 1) {
> - ret = -EINVAL;
> + if (fw_dump.dump_registered == 1)
> goto unlock_out;
> - }
> /* Register Firmware-assisted dump */
> register_fadump();
> break;
^ permalink raw reply
* [PATCH] fadump: allow duplicate assignment to /sys/kernel/fadump_registered when it's assigned the desired value already
From: Wang Sheng-Hui @ 2013-04-10 0:46 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Suzuki K. Poulose,
linuxppc-dev
When the fadump is enabled, we have /sys/kernel/fadump_enabled assigned 1.
But sometimes we need to restart the fadump service by 'service kdump restart',
in case the kdump script has added the fadump detect/support already.
In current implementation, we cannot re-assign 1 to /sys/kernel/fadump_enabled
if 1 is already set and we have added more logic check in the user space script.
I think we can enable the duplicate assignment to ease the user space tools,
as long as the value is the right 1 or 0.
Signed-off-by: Wang Sheng-Hui <shhuiw@gmail.com>
---
arch/powerpc/kernel/fadump.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
index 06c8202..e1347e5 100644
--- a/arch/powerpc/kernel/fadump.c
+++ b/arch/powerpc/kernel/fadump.c
@@ -1140,18 +1140,14 @@ static ssize_t fadump_register_store(struct kobject *kobj,
switch (buf[0]) {
case '0':
- if (fw_dump.dump_registered == 0) {
- ret = -EINVAL;
+ if (fw_dump.dump_registered == 0)
goto unlock_out;
- }
/* Un-register Firmware-assisted dump */
fadump_unregister_dump(&fdm);
break;
case '1':
- if (fw_dump.dump_registered == 1) {
- ret = -EINVAL;
+ if (fw_dump.dump_registered == 1)
goto unlock_out;
- }
/* Register Firmware-assisted dump */
register_fadump();
break;
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH] powerpc/crypto: Add property for 'era' in SEC dts crypto node
From: Kim Phillips @ 2013-04-09 22:31 UTC (permalink / raw)
To: Vakul Garg
Cc: Shaveta Leekha, linux-kernel, Andy Fleming, Paul Mackerras,
linuxppc-dev
In-Reply-To: <1365508233-26292-1-git-send-email-vakul@freescale.com>
On Tue, 9 Apr 2013 17:20:33 +0530
Vakul Garg <vakul@freescale.com> wrote:
> The crypto node now contains a new property 'fsl,sec-era'.
> This is required so that applications can retrieve era info without
> having to be able to read SEC's register space.
>
> Signed-off-by: Vakul Garg <vakul@freescale.com>
> ---
> arch/powerpc/boot/dts/fsl/pq3-sec4.4-0.dtsi | 1 +
> arch/powerpc/boot/dts/fsl/qoriq-sec4.0-0.dtsi | 1 +
> arch/powerpc/boot/dts/fsl/qoriq-sec4.2-0.dtsi | 1 +
> arch/powerpc/boot/dts/fsl/qoriq-sec5.0-0.dtsi | 1 +
> arch/powerpc/boot/dts/fsl/qoriq-sec5.2-0.dtsi | 1 +
> arch/powerpc/boot/dts/fsl/qoriq-sec5.3-0.dtsi | 1 +
> 6 files changed, 6 insertions(+), 0 deletions(-)
missing p1023si-post.dtsi and qoriq-sec4.1-0.dtsi files.
Kim
^ permalink raw reply
* Re: Build regressions/improvements in v3.9-rc6
From: Geert Uytterhoeven @ 2013-04-09 21:07 UTC (permalink / raw)
To: Linux Kernel Development; +Cc: Linux/PPC Development, linux-sh
In-Reply-To: <alpine.DEB.2.00.1304092304220.23715@ayla.of.borg>
On Tue, 9 Apr 2013, Geert Uytterhoeven wrote:
> JFYI, when comparing v3.9-rc6 to v3.9-rc5[3], the summaries are:
> - build errors: +8/-13
Ignoring R_PPC64_REL24 truncated relocations, as usual:
+ error: binder.c: undefined reference to `get_vm_area': => .text+0x2fe8b8)
+ error: binder.c: undefined reference to `map_vm_area': => .text+0x2fa11c)
+ error: binder.c: undefined reference to `zap_page_range': => .text+0x2fa354)
sh4/sh-randconfig
+ error: cuboot-pq2.c: undefined reference to `fsl_get_immr': => .text+0x314)
powerpc-randconfig
> [1] http://kisskb.ellerman.id.au/kisskb/head/6063/ (all 118 configs)
> [3] http://kisskb.ellerman.id.au/kisskb/head/6041/ (all 118 configs)
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* [PATCH] powerpc/fsl-booke: Minor fixes to T4240 Si device tree
From: Kumar Gala @ 2013-04-09 14:53 UTC (permalink / raw)
To: linuxppc-dev
* Fix cpu unit address to match reg
* Update compatible for rcpm & clockgen to be 2.0 instead of 2
Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
---
arch/powerpc/boot/dts/fsl/t4240si-post.dtsi | 4 ++--
arch/powerpc/boot/dts/fsl/t4240si-pre.dtsi | 22 +++++++++++-----------
2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/arch/powerpc/boot/dts/fsl/t4240si-post.dtsi b/arch/powerpc/boot/dts/fsl/t4240si-post.dtsi
index 1d72926..e77e6ad 100644
--- a/arch/powerpc/boot/dts/fsl/t4240si-post.dtsi
+++ b/arch/powerpc/boot/dts/fsl/t4240si-post.dtsi
@@ -364,12 +364,12 @@
};
clockgen: global-utilities@e1000 {
- compatible = "fsl,t4240-clockgen", "fsl,qoriq-clockgen-2";
+ compatible = "fsl,t4240-clockgen", "fsl,qoriq-clockgen-2.0";
reg = <0xe1000 0x1000>;
};
rcpm: global-utilities@e2000 {
- compatible = "fsl,t4240-rcpm", "fsl,qoriq-rcpm-2";
+ compatible = "fsl,t4240-rcpm", "fsl,qoriq-rcpm-2.0";
reg = <0xe2000 0x1000>;
};
diff --git a/arch/powerpc/boot/dts/fsl/t4240si-pre.dtsi b/arch/powerpc/boot/dts/fsl/t4240si-pre.dtsi
index 9b39a43..a93c55a 100644
--- a/arch/powerpc/boot/dts/fsl/t4240si-pre.dtsi
+++ b/arch/powerpc/boot/dts/fsl/t4240si-pre.dtsi
@@ -69,57 +69,57 @@
reg = <0 1>;
next-level-cache = <&L2_1>;
};
- cpu1: PowerPC,e6500@1 {
+ cpu1: PowerPC,e6500@2 {
device_type = "cpu";
reg = <2 3>;
next-level-cache = <&L2_1>;
};
- cpu2: PowerPC,e6500@2 {
+ cpu2: PowerPC,e6500@4 {
device_type = "cpu";
reg = <4 5>;
next-level-cache = <&L2_1>;
};
- cpu3: PowerPC,e6500@3 {
+ cpu3: PowerPC,e6500@6 {
device_type = "cpu";
reg = <6 7>;
next-level-cache = <&L2_1>;
};
- cpu4: PowerPC,e6500@4 {
+ cpu4: PowerPC,e6500@8 {
device_type = "cpu";
reg = <8 9>;
next-level-cache = <&L2_2>;
};
- cpu5: PowerPC,e6500@5 {
+ cpu5: PowerPC,e6500@10 {
device_type = "cpu";
reg = <10 11>;
next-level-cache = <&L2_2>;
};
- cpu6: PowerPC,e6500@6 {
+ cpu6: PowerPC,e6500@12 {
device_type = "cpu";
reg = <12 13>;
next-level-cache = <&L2_2>;
};
- cpu7: PowerPC,e6500@7 {
+ cpu7: PowerPC,e6500@14 {
device_type = "cpu";
reg = <14 15>;
next-level-cache = <&L2_2>;
};
- cpu8: PowerPC,e6500@8 {
+ cpu8: PowerPC,e6500@16 {
device_type = "cpu";
reg = <16 17>;
next-level-cache = <&L2_3>;
};
- cpu9: PowerPC,e6500@9 {
+ cpu9: PowerPC,e6500@18 {
device_type = "cpu";
reg = <18 19>;
next-level-cache = <&L2_3>;
};
- cpu10: PowerPC,e6500@10 {
+ cpu10: PowerPC,e6500@20 {
device_type = "cpu";
reg = <20 21>;
next-level-cache = <&L2_3>;
};
- cpu11: PowerPC,e6500@11 {
+ cpu11: PowerPC,e6500@22 {
device_type = "cpu";
reg = <22 23>;
next-level-cache = <&L2_3>;
--
1.7.9.7
^ permalink raw reply related
* Re: powerpc userspace address space layout information
From: Chris Friesen @ 2013-04-09 14:30 UTC (permalink / raw)
To: David Gibson; +Cc: linuxppc-dev, Paul Mackerras
In-Reply-To: <20130407055844.GB17787@truffula.fritz.box>
On 04/06/2013 11:58 PM, David Gibson wrote:
> On Thu, Apr 04, 2013 at 10:53:58PM -0600, Chris Friesen wrote:
>> Third, what's the most reliable way to ensure a block of addresses around
>> 0xf6000000 don't get used for shared libraries? (We want to preserve
>> those addresses for emulating hardware in a virtual machine.) We have
>> this working on an older system but after upgrading to new software the
>> libraries now extend further down the address space.
>
> The only reliable method I can think of would be to use a custom
> linker script to give your binary an extra program header specifying
> that virtual region to map.
Thanks for your help. We had started working on the custom linker
script while waiting to see if anyone would respond, so it's good to get
some validation that we picked the right solution.
Chris
^ permalink raw reply
* [PATCH] powerpc/perf: Add an explict flag indicating presence of SLOT field
From: Michael Ellerman @ 2013-04-09 13:41 UTC (permalink / raw)
To: linuxppc-dev; +Cc: sukadev, Paul Mackerras, Anton Blanchard
In perf_ip_adjust() we potentially use the MMCRA[SLOT] field to adjust
the reported IP of a sampled instruction.
Currently the logic is written so that if the backend does NOT have
the PPMU_ALT_SIPR flag set then we assume MMCRA[SLOT] exists.
This is wrong on power7, where we have SIPR in the "alternate" location,
but also have the MMCRA[SLOT] field. Furthermore on power8 we do not
want to set ALT_SIPR (it's in a third location), and we also do not have
MMCRA[SLOT].
So add a new flag which only indicates whether MMCRA[SLOT] exists.
Naively we'd set it on everything except power6/7, because they set
ALT_SIPR, and we've reversed the polarity of the flag. But it's more
complicated than that.
mpc7450 is 32-bit, and uses its own version of perf_ip_adjust()
which doesn't use MMCRA[SLOT], so it doesn't need the new flag set and
the behaviour is unchanged.
PPC970 (and I assume power4) don't have MMCRA[SLOT], so shouldn't have
the new flag set. This is a behaviour change on those cpus, though we
were probably getting lucky and the bits in question were 0.
power5 and power5+ set the new flag, behaviour unchanged.
power6 does not set the new flag, behaviour unchanged.
power7 sets the new flag, which is a behaviour change.
Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
I went mildly insane working out all the different cases in this patch, so
any review appreciated.
arch/powerpc/include/asm/perf_event_server.h | 1 +
arch/powerpc/perf/core-book3s.c | 3 ++-
arch/powerpc/perf/power5+-pmu.c | 2 +-
arch/powerpc/perf/power5-pmu.c | 1 +
arch/powerpc/perf/power7-pmu.c | 2 +-
5 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/include/asm/perf_event_server.h b/arch/powerpc/include/asm/perf_event_server.h
index d0aec72..7074aec 100644
--- a/arch/powerpc/include/asm/perf_event_server.h
+++ b/arch/powerpc/include/asm/perf_event_server.h
@@ -52,6 +52,7 @@ struct power_pmu {
#define PPMU_NO_SIPR 0x00000004 /* no SIPR/HV in MMCRA at all */
#define PPMU_NO_CONT_SAMPLING 0x00000008 /* no continuous sampling */
#define PPMU_SIAR_VALID 0x00000010 /* Processor has SIAR Valid bit */
+#define PPMU_HAS_SSLOT 0x00000020 /* Has sampled slot in MMCRA */
/*
* Values for flags to get_alternatives()
diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index 65362e9..eb64480 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -98,11 +98,12 @@ static inline unsigned long perf_ip_adjust(struct pt_regs *regs)
{
unsigned long mmcra = regs->dsisr;
- if ((mmcra & MMCRA_SAMPLE_ENABLE) && !(ppmu->flags & PPMU_ALT_SIPR)) {
+ if ((ppmu->flags & PPMU_HAS_SSLOT) && (mmcra & MMCRA_SAMPLE_ENABLE)) {
unsigned long slot = (mmcra & MMCRA_SLOT) >> MMCRA_SLOT_SHIFT;
if (slot > 1)
return 4 * (slot - 1);
}
+
return 0;
}
diff --git a/arch/powerpc/perf/power5+-pmu.c b/arch/powerpc/perf/power5+-pmu.c
index a8757ba..b03b6dc 100644
--- a/arch/powerpc/perf/power5+-pmu.c
+++ b/arch/powerpc/perf/power5+-pmu.c
@@ -671,7 +671,7 @@ static struct power_pmu power5p_pmu = {
.get_alternatives = power5p_get_alternatives,
.disable_pmc = power5p_disable_pmc,
.limited_pmc_event = power5p_limited_pmc_event,
- .flags = PPMU_LIMITED_PMC5_6,
+ .flags = PPMU_LIMITED_PMC5_6 | PPMU_HAS_SSLOT,
.n_generic = ARRAY_SIZE(power5p_generic_events),
.generic_events = power5p_generic_events,
.cache_events = &power5p_cache_events,
diff --git a/arch/powerpc/perf/power5-pmu.c b/arch/powerpc/perf/power5-pmu.c
index e7f06eb..1e8ce42 100644
--- a/arch/powerpc/perf/power5-pmu.c
+++ b/arch/powerpc/perf/power5-pmu.c
@@ -615,6 +615,7 @@ static struct power_pmu power5_pmu = {
.n_generic = ARRAY_SIZE(power5_generic_events),
.generic_events = power5_generic_events,
.cache_events = &power5_cache_events,
+ .flags = PPMU_HAS_SSLOT,
};
static int __init init_power5_pmu(void)
diff --git a/arch/powerpc/perf/power7-pmu.c b/arch/powerpc/perf/power7-pmu.c
index 3c475d6..744a5cf 100644
--- a/arch/powerpc/perf/power7-pmu.c
+++ b/arch/powerpc/perf/power7-pmu.c
@@ -448,7 +448,7 @@ static struct power_pmu power7_pmu = {
.get_constraint = power7_get_constraint,
.get_alternatives = power7_get_alternatives,
.disable_pmc = power7_disable_pmc,
- .flags = PPMU_ALT_SIPR,
+ .flags = PPMU_ALT_SIPR | PPMU_HAS_SSLOT,
.attr_groups = power7_pmu_attr_groups,
.n_generic = ARRAY_SIZE(power7_generic_events),
.generic_events = power7_generic_events,
--
1.7.10.4
^ permalink raw reply related
* [PATCH] powerpc/crypto: Add property for 'era' in SEC dts crypto node
From: Vakul Garg @ 2013-04-09 11:50 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Shaveta Leekha, linux-kernel, Paul Mackerras, Andy Fleming
The crypto node now contains a new property 'fsl,sec-era'.
This is required so that applications can retrieve era info without
having to be able to read SEC's register space.
Signed-off-by: Vakul Garg <vakul@freescale.com>
---
arch/powerpc/boot/dts/fsl/pq3-sec4.4-0.dtsi | 1 +
arch/powerpc/boot/dts/fsl/qoriq-sec4.0-0.dtsi | 1 +
arch/powerpc/boot/dts/fsl/qoriq-sec4.2-0.dtsi | 1 +
arch/powerpc/boot/dts/fsl/qoriq-sec5.0-0.dtsi | 1 +
arch/powerpc/boot/dts/fsl/qoriq-sec5.2-0.dtsi | 1 +
arch/powerpc/boot/dts/fsl/qoriq-sec5.3-0.dtsi | 1 +
6 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/arch/powerpc/boot/dts/fsl/pq3-sec4.4-0.dtsi b/arch/powerpc/boot/dts/fsl/pq3-sec4.4-0.dtsi
index ffadcb5..bb3d826 100644
--- a/arch/powerpc/boot/dts/fsl/pq3-sec4.4-0.dtsi
+++ b/arch/powerpc/boot/dts/fsl/pq3-sec4.4-0.dtsi
@@ -34,6 +34,7 @@
crypto@30000 {
compatible = "fsl,sec-v4.4", "fsl,sec-v4.0";
+ fsl,sec-era = <3>;
#address-cells = <1>;
#size-cells = <1>;
ranges = <0x0 0x30000 0x10000>;
diff --git a/arch/powerpc/boot/dts/fsl/qoriq-sec4.0-0.dtsi b/arch/powerpc/boot/dts/fsl/qoriq-sec4.0-0.dtsi
index 0cbbac3..02bee5f 100644
--- a/arch/powerpc/boot/dts/fsl/qoriq-sec4.0-0.dtsi
+++ b/arch/powerpc/boot/dts/fsl/qoriq-sec4.0-0.dtsi
@@ -34,6 +34,7 @@
crypto: crypto@300000 {
compatible = "fsl,sec-v4.0";
+ fsl,sec-era = <1>;
#address-cells = <1>;
#size-cells = <1>;
reg = <0x300000 0x10000>;
diff --git a/arch/powerpc/boot/dts/fsl/qoriq-sec4.2-0.dtsi b/arch/powerpc/boot/dts/fsl/qoriq-sec4.2-0.dtsi
index 7990e0d..7f7574e 100644
--- a/arch/powerpc/boot/dts/fsl/qoriq-sec4.2-0.dtsi
+++ b/arch/powerpc/boot/dts/fsl/qoriq-sec4.2-0.dtsi
@@ -34,6 +34,7 @@
crypto: crypto@300000 {
compatible = "fsl,sec-v4.2", "fsl,sec-v4.0";
+ fsl,sec-era = <3>;
#address-cells = <1>;
#size-cells = <1>;
reg = <0x300000 0x10000>;
diff --git a/arch/powerpc/boot/dts/fsl/qoriq-sec5.0-0.dtsi b/arch/powerpc/boot/dts/fsl/qoriq-sec5.0-0.dtsi
index ffd458f..e298efb 100644
--- a/arch/powerpc/boot/dts/fsl/qoriq-sec5.0-0.dtsi
+++ b/arch/powerpc/boot/dts/fsl/qoriq-sec5.0-0.dtsi
@@ -34,6 +34,7 @@
crypto: crypto@300000 {
compatible = "fsl,sec-v5.0", "fsl,sec-v4.0";
+ fsl,sec-era = <5>;
#address-cells = <1>;
#size-cells = <1>;
reg = <0x300000 0x10000>;
diff --git a/arch/powerpc/boot/dts/fsl/qoriq-sec5.2-0.dtsi b/arch/powerpc/boot/dts/fsl/qoriq-sec5.2-0.dtsi
index 7b2ab8a..33ff09d 100644
--- a/arch/powerpc/boot/dts/fsl/qoriq-sec5.2-0.dtsi
+++ b/arch/powerpc/boot/dts/fsl/qoriq-sec5.2-0.dtsi
@@ -34,6 +34,7 @@
crypto: crypto@300000 {
compatible = "fsl,sec-v5.2", "fsl,sec-v5.0", "fsl,sec-v4.0";
+ fsl,sec-era = <5>;
#address-cells = <1>;
#size-cells = <1>;
reg = <0x300000 0x10000>;
diff --git a/arch/powerpc/boot/dts/fsl/qoriq-sec5.3-0.dtsi b/arch/powerpc/boot/dts/fsl/qoriq-sec5.3-0.dtsi
index 0339825..0877822 100644
--- a/arch/powerpc/boot/dts/fsl/qoriq-sec5.3-0.dtsi
+++ b/arch/powerpc/boot/dts/fsl/qoriq-sec5.3-0.dtsi
@@ -34,6 +34,7 @@
crypto: crypto@300000 {
compatible = "fsl,sec-v5.3", "fsl,sec-v5.0", "fsl,sec-v4.0";
+ fsl,sec-era = <4>;
#address-cells = <1>;
#size-cells = <1>;
reg = <0x300000 0x10000>;
--
1.7.7.6
^ permalink raw reply related
* RE: [PATCH v4] cpufreq: powerpc: Add cpufreq driver for Freescale e500mc SoCs
From: Tang Yuantian-B29983 @ 2013-04-09 10:04 UTC (permalink / raw)
To: Viresh Kumar
Cc: Li Yang-R58472, linux-pm@vger.kernel.org,
linux-kernel@vger.kernel.org, cpufreq@vger.kernel.org,
rjw@sisk.pl, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <CAKohpokVf0hCokDL62WeaUdYEBycf+Kh_cRgWxTR--_12xjEBA@mail.gmail.com>
VGhhbmtzLCB5b3UgbWFrZSBteSBjb2RlIGxvb2sgYmV0dGVyIGVhY2ggcmV2aWV3Lg0KDQpUaGFu
a3MsDQpZdWFudGlhbg0KDQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTog
VmlyZXNoIEt1bWFyIFttYWlsdG86dmlyZXNoLmt1bWFyQGxpbmFyby5vcmddDQo+IFNlbnQ6IDIw
MTPE6jTUwjnI1SAxNzo0Nw0KPiBUbzogVGFuZyBZdWFudGlhbi1CMjk5ODMNCj4gQ2M6IHJqd0Bz
aXNrLnBsOyBjcHVmcmVxQHZnZXIua2VybmVsLm9yZzsgbGludXgtcG1Admdlci5rZXJuZWwub3Jn
Ow0KPiBsaW51eC1rZXJuZWxAdmdlci5rZXJuZWwub3JnOyBsaW51eHBwYy1kZXZAbGlzdHMub3ps
YWJzLm9yZzsgTGkgWWFuZy0NCj4gUjU4NDcyDQo+IFN1YmplY3Q6IFJlOiBbUEFUQ0ggdjRdIGNw
dWZyZXE6IHBvd2VycGM6IEFkZCBjcHVmcmVxIGRyaXZlciBmb3INCj4gRnJlZXNjYWxlIGU1MDBt
YyBTb0NzDQo+IA0KPiBNb3N0bHkgZ29vZCBub3csIFY1IHNob3VsZCBiZSB0aGUgZmluYWwgb25l
Lg0KPiANCj4gT24gOSBBcHJpbCAyMDEzIDE0OjAyLCAgPFl1YW50aWFuLlRhbmdAZnJlZXNjYWxl
LmNvbT4gd3JvdGU6DQo+ID4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvY3B1ZnJlcS9wcGMtY29yZW5l
dC1jcHVmcmVxLmMNCj4gPiBiL2RyaXZlcnMvY3B1ZnJlcS9wcGMtY29yZW5ldC1jcHVmcmVxLmMN
Cj4gDQo+ID4gK3N0YXRpYyBpbnQgY29yZW5ldF9jcHVmcmVxX3RhcmdldChzdHJ1Y3QgY3B1ZnJl
cV9wb2xpY3kgKnBvbGljeSwNCj4gPiArICAgICAgICAgICAgICAgdW5zaWduZWQgaW50IHRhcmdl
dF9mcmVxLCB1bnNpZ25lZCBpbnQgcmVsYXRpb24pIHsNCj4gPiArICAgICAgIHN0cnVjdCBjcHVm
cmVxX2ZyZXFzIGZyZXFzOw0KPiA+ICsgICAgICAgdW5zaWduZWQgaW50IG5ldzsNCj4gPiArICAg
ICAgIHN0cnVjdCBjbGsgKnBhcmVudDsNCj4gPiArICAgICAgIGludCByZXQ7DQo+ID4gKyAgICAg
ICBzdHJ1Y3QgY3B1X2RhdGEgKmRhdGEgPSBwZXJfY3B1KGNwdV9kYXRhLCBwb2xpY3ktPmNwdSk7
DQo+ID4gKw0KPiA+ICsgICAgICAgY3B1ZnJlcV9mcmVxdWVuY3lfdGFibGVfdGFyZ2V0KHBvbGlj
eSwgZGF0YS0+dGFibGUsDQo+ID4gKyAgICAgICAgICAgICAgICAgICAgICAgdGFyZ2V0X2ZyZXEs
IHJlbGF0aW9uLCAmbmV3KTsNCj4gPiArDQo+ID4gKyAgICAgICBpZiAocG9saWN5LT5jdXIgPT0g
ZGF0YS0+dGFibGVbbmV3XS5mcmVxdWVuY3kpDQo+ID4gKyAgICAgICAgICAgICAgIHJldHVybiAw
Ow0KPiA+ICsNCj4gPiArICAgICAgIGZyZXFzLm9sZCA9IHBvbGljeS0+Y3VyOw0KPiA+ICsgICAg
ICAgZnJlcXMubmV3ID0gZGF0YS0+dGFibGVbbmV3XS5mcmVxdWVuY3k7DQo+ID4gKyAgICAgICBm
cmVxcy5jcHUgPSBwb2xpY3ktPmNwdTsNCj4gDQo+IFlvdSBkb24ndCBuZWVkIHRvIHNldCBmcmVx
cy5jcHUuLg0KPiANCj4gPiArICAgICAgIG11dGV4X2xvY2soJmNwdWZyZXFfbG9jayk7DQo+ID4g
KyAgICAgICBjcHVmcmVxX25vdGlmeV90cmFuc2l0aW9uKHBvbGljeSwgJmZyZXFzLCBDUFVGUkVR
X1BSRUNIQU5HRSk7DQo+ID4gKw0KPiA+ICsgICAgICAgcGFyZW50ID0gb2ZfY2xrX2dldChkYXRh
LT5wYXJlbnQsIG5ldyk7DQo+ID4gKyAgICAgICByZXQgPSBjbGtfc2V0X3BhcmVudChkYXRhLT5j
bGssIHBhcmVudCk7DQo+ID4gKyAgICAgICBpZiAocmV0KSB7DQo+ID4gKyAgICAgICAgICAgICAg
IGZyZXFzLm5ldyA9IGZyZXFzLm9sZDsNCj4gPiArICAgICAgICAgICAgICAgY3B1ZnJlcV9ub3Rp
ZnlfdHJhbnNpdGlvbihwb2xpY3ksICZmcmVxcywNCj4gQ1BVRlJFUV9QT1NUQ0hBTkdFKTsNCj4g
PiArICAgICAgICAgICAgICAgbXV0ZXhfdW5sb2NrKCZjcHVmcmVxX2xvY2spOw0KPiA+ICsgICAg
ICAgICAgICAgICByZXR1cm4gcmV0Ow0KPiA+ICsgICAgICAgfQ0KPiA+ICsNCj4gPiArICAgICAg
IGNwdWZyZXFfbm90aWZ5X3RyYW5zaXRpb24ocG9saWN5LCAmZnJlcXMsIENQVUZSRVFfUE9TVENI
QU5HRSk7DQo+ID4gKyAgICAgICBtdXRleF91bmxvY2soJmNwdWZyZXFfbG9jayk7DQo+IA0KPiBX
aGF0IGFib3V0IHdyaXRpbmcgaXQgYXM6DQo+IA0KPiArICAgICAgIHJldCA9IGNsa19zZXRfcGFy
ZW50KGRhdGEtPmNsaywgcGFyZW50KTsNCj4gKyAgICAgICBpZiAocmV0KQ0KPiArICAgICAgICAg
ICAgICAgZnJlcXMubmV3ID0gZnJlcXMub2xkOw0KPiArDQo+ICsgICAgICAgY3B1ZnJlcV9ub3Rp
ZnlfdHJhbnNpdGlvbihwb2xpY3ksICZmcmVxcywgQ1BVRlJFUV9QT1NUQ0hBTkdFKTsNCj4gKyAg
ICAgICBtdXRleF91bmxvY2soJmNwdWZyZXFfbG9jayk7DQo+IA0KPiByZXR1cm4gcmV0Ow0KPiAN
Cj4gPiArICAgICAgIHJldHVybiAwOw0KPiA+ICt9DQoNCg==
^ permalink raw reply
* Re: [PATCH v4] cpufreq: powerpc: Add cpufreq driver for Freescale e500mc SoCs
From: Viresh Kumar @ 2013-04-09 9:47 UTC (permalink / raw)
To: Yuantian.Tang; +Cc: linux-pm, linux-kernel, cpufreq, rjw, linuxppc-dev
In-Reply-To: <1365496365-28450-1-git-send-email-Yuantian.Tang@freescale.com>
Mostly good now, V5 should be the final one.
On 9 April 2013 14:02, <Yuantian.Tang@freescale.com> wrote:
> diff --git a/drivers/cpufreq/ppc-corenet-cpufreq.c b/drivers/cpufreq/ppc-corenet-cpufreq.c
> +static int corenet_cpufreq_target(struct cpufreq_policy *policy,
> + unsigned int target_freq, unsigned int relation)
> +{
> + struct cpufreq_freqs freqs;
> + unsigned int new;
> + struct clk *parent;
> + int ret;
> + struct cpu_data *data = per_cpu(cpu_data, policy->cpu);
> +
> + cpufreq_frequency_table_target(policy, data->table,
> + target_freq, relation, &new);
> +
> + if (policy->cur == data->table[new].frequency)
> + return 0;
> +
> + freqs.old = policy->cur;
> + freqs.new = data->table[new].frequency;
> + freqs.cpu = policy->cpu;
You don't need to set freqs.cpu..
> + mutex_lock(&cpufreq_lock);
> + cpufreq_notify_transition(policy, &freqs, CPUFREQ_PRECHANGE);
> +
> + parent = of_clk_get(data->parent, new);
> + ret = clk_set_parent(data->clk, parent);
> + if (ret) {
> + freqs.new = freqs.old;
> + cpufreq_notify_transition(policy, &freqs, CPUFREQ_POSTCHANGE);
> + mutex_unlock(&cpufreq_lock);
> + return ret;
> + }
> +
> + cpufreq_notify_transition(policy, &freqs, CPUFREQ_POSTCHANGE);
> + mutex_unlock(&cpufreq_lock);
What about writing it as:
+ ret = clk_set_parent(data->clk, parent);
+ if (ret)
+ freqs.new = freqs.old;
+
+ cpufreq_notify_transition(policy, &freqs, CPUFREQ_POSTCHANGE);
+ mutex_unlock(&cpufreq_lock);
return ret;
> + return 0;
> +}
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox