[PATCH] powerpc/kvm/xive: Don't access TIMA using byte accesses

2017-09-05 Thread Benjamin Herrenschmidt 
The TIMA only supports byte stores to the CPPR, for everything else,
we need to do a 32-bit or 64-bit load.

Signed-off-by: Benjamin Herrenschmidt 
Fixes: 2c4fb78f78b6e420604ee1b05bdfb5c1d637869f
---
 arch/powerpc/kvm/book3s_hv_rm_xive.c| 1 -
 arch/powerpc/kvm/book3s_xive.c  | 1 -
 arch/powerpc/kvm/book3s_xive_template.c | 7 ---
 3 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/kvm/book3s_hv_rm_xive.c 
b/arch/powerpc/kvm/book3s_hv_rm_xive.c
index abf5f01b6eb1..5b81a807d742 100644
--- a/arch/powerpc/kvm/book3s_hv_rm_xive.c
+++ b/arch/powerpc/kvm/book3s_hv_rm_xive.c
@@ -38,7 +38,6 @@ static inline void __iomem *get_tima_phys(void)
 #define __x_tima   get_tima_phys()
 #define __x_eoi_page(xd)   ((void __iomem *)((xd)->eoi_page))
 #define __x_trig_page(xd)  ((void __iomem *)((xd)->trig_page))
-#define __x_readb  __raw_rm_readb
 #define __x_writeb __raw_rm_writeb
 #define __x_readw  __raw_rm_readw
 #define __x_readq  __raw_rm_readq
diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xive.c
index 08b200a0bbce..13304622ab1c 100644
--- a/arch/powerpc/kvm/book3s_xive.c
+++ b/arch/powerpc/kvm/book3s_xive.c
@@ -48,7 +48,6 @@
 #define __x_tima   xive_tima
 #define __x_eoi_page(xd)   ((void __iomem *)((xd)->eoi_mmio))
 #define __x_trig_page(xd)  ((void __iomem *)((xd)->trig_mmio))
-#define __x_readb  __raw_readb
 #define __x_writeb __raw_writeb
 #define __x_readw  __raw_readw
 #define __x_readq  __raw_readq
diff --git a/arch/powerpc/kvm/book3s_xive_template.c 
b/arch/powerpc/kvm/book3s_xive_template.c
index d1ed2c41b5d2..c7a5deadd1cc 100644
--- a/arch/powerpc/kvm/book3s_xive_template.c
+++ b/arch/powerpc/kvm/book3s_xive_template.c
@@ -28,7 +28,8 @@ static void GLUE(X_PFX,ack_pending)(struct kvmppc_xive_vcpu 
*xc)
 * bit.
 */
if (cpu_has_feature(CPU_FTR_POWER9_DD1)) {
-   u8 pipr = __x_readb(__x_tima + TM_QW1_OS + TM_PIPR);
+   __be64 qw1 = __x_readq(__x_tima + TM_QW1_OS);
+   u8 pipr = be64_to_cpu(qw1) & 0xff;
if (pipr >= xc->hw_cppr)
return;
}
@@ -336,7 +337,6 @@ X_STATIC unsigned long GLUE(X_PFX,h_ipoll)(struct kvm_vcpu 
*vcpu, unsigned long
struct kvmppc_xive_vcpu *xc = vcpu->arch.xive_vcpu;
u8 pending = xc->pending;
u32 hirq;
-   u8 pipr;
 
pr_devel("H_IPOLL(server=%ld)\n", server);
 
@@ -353,7 +353,8 @@ X_STATIC unsigned long GLUE(X_PFX,h_ipoll)(struct kvm_vcpu 
*vcpu, unsigned long
pending = 0xff;
} else {
/* Grab pending interrupt if any */
-   pipr = __x_readb(__x_tima + TM_QW1_OS + TM_PIPR);
+   __be64 qw1 = __x_readq(__x_tima + TM_QW1_OS);
+   u8 pipr = be64_to_cpu(qw1) & 0xff;
if (pipr < 8)
pending |= 1 << pipr;
}

Re: [rfc 2/3] powerpc/mce: Extract physical_address for UE errors

2017-09-05 Thread Balbir Singh 
On Wed, Sep 6, 2017 at 10:36 AM, Nicholas Piggin  wrote:
> On Tue,  5 Sep 2017 14:15:54 +1000
> Balbir Singh  wrote:
>
>> Walk the page table for NIP and extract the instruction. Then
>> use the instruction to find the effective address via analyse_instr().
>>
>> We might have page table walking races, but we expect them to
>> be rare, the physical address extraction is best effort. The idea
>> is to then hook up this infrastructure to memory failure eventually.
>
> Cool. Too bad hardware doesn't give us the RA.
>
>>
>> Signed-off-by: Balbir Singh 
>> ---
>>  arch/powerpc/include/asm/mce.h  |  2 +-
>>  arch/powerpc/kernel/mce.c   |  6 -
>>  arch/powerpc/kernel/mce_power.c | 60 
>> +
>>  3 files changed, 61 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/mce.h b/arch/powerpc/include/asm/mce.h
>> index 75292c7..3a1226e 100644
>> --- a/arch/powerpc/include/asm/mce.h
>> +++ b/arch/powerpc/include/asm/mce.h
>> @@ -204,7 +204,7 @@ struct mce_error_info {
>>
>>  extern void save_mce_event(struct pt_regs *regs, long handled,
>>  struct mce_error_info *mce_err, uint64_t nip,
>> -uint64_t addr);
>> +uint64_t addr, uint64_t phys_addr);
>>  extern int get_mce_event(struct machine_check_event *mce, bool release);
>>  extern void release_mce_event(void);
>>  extern void machine_check_queue_event(void);
>> diff --git a/arch/powerpc/kernel/mce.c b/arch/powerpc/kernel/mce.c
>> index e254399..f41a75d 100644
>> --- a/arch/powerpc/kernel/mce.c
>> +++ b/arch/powerpc/kernel/mce.c
>> @@ -82,7 +82,7 @@ static void mce_set_error_info(struct machine_check_event 
>> *mce,
>>   */
>>  void save_mce_event(struct pt_regs *regs, long handled,
>>   struct mce_error_info *mce_err,
>> - uint64_t nip, uint64_t addr)
>> + uint64_t nip, uint64_t addr, uint64_t phys_addr)
>>  {
>>   int index = __this_cpu_inc_return(mce_nest_count) - 1;
>>   struct machine_check_event *mce = this_cpu_ptr(_event[index]);
>> @@ -140,6 +140,10 @@ void save_mce_event(struct pt_regs *regs, long handled,
>>   } else if (mce->error_type == MCE_ERROR_TYPE_UE) {
>>   mce->u.ue_error.effective_address_provided = true;
>>   mce->u.ue_error.effective_address = addr;
>> + if (phys_addr != ULONG_MAX) {
>> + mce->u.ue_error.physical_address_provided = true;
>> + mce->u.ue_error.physical_address = phys_addr;
>> + }
>>   }
>>   return;
>>  }
>> diff --git a/arch/powerpc/kernel/mce_power.c 
>> b/arch/powerpc/kernel/mce_power.c
>> index b76ca19..b77a698 100644
>> --- a/arch/powerpc/kernel/mce_power.c
>> +++ b/arch/powerpc/kernel/mce_power.c
>> @@ -27,6 +27,25 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +static unsigned long addr_to_pfn(struct mm_struct *mm, unsigned long addr)
>> +{
>> + pte_t *ptep;
>> + unsigned long flags;
>> +
>> + local_irq_save(flags);
>> + if (mm == current->mm)
>> + ptep = find_current_mm_pte(mm->pgd, addr, NULL, NULL);
>> + else
>> + ptep = find_init_mm_pte(addr, NULL);
>> + local_irq_restore(flags);
>> + if (!ptep)
>> + return ULONG_MAX;
>> + return pte_pfn(*ptep);
>
> I think you need to check that it's still cacheable memory here?
> !pte_speical && pfn <= highest_memmap_pfn?
>

find_*pte will return a NULL PTE, so we do have a check there. !pte_special is a
good check to have, I'll add it


>
>> +}
>>
>>  static void flush_tlb_206(unsigned int num_sets, unsigned int action)
>>  {
>> @@ -489,7 +508,8 @@ static int mce_handle_ierror(struct pt_regs *regs,
>>
>>  static int mce_handle_derror(struct pt_regs *regs,
>>   const struct mce_derror_table table[],
>> - struct mce_error_info *mce_err, uint64_t *addr)
>> + struct mce_error_info *mce_err, uint64_t *addr,
>> + uint64_t *phys_addr)
>>  {
>>   uint64_t dsisr = regs->dsisr;
>>   int handled = 0;
>> @@ -555,7 +575,37 @@ static int mce_handle_derror(struct pt_regs *regs,
>>   mce_err->initiator = table[i].initiator;
>>   if (table[i].dar_valid)
>>   *addr = regs->dar;
>> -
>> + else if (mce_err->severity == MCE_SEV_ERROR_SYNC &&
>> + table[i].error_type == MCE_ERROR_TYPE_UE) {
>> + /*
>> +  * Carefully look at the NIP to determine
>> +  * the instruction to analyse. Reading the NIP
>> +  * in real-mode is tricky and can lead to recursive
>> +  * faults
>> +  */
>
> What recursive faults? If you ensure NIP is cacheable memory, I guess you
> can get a recursive machine

Re: [PATCH 1/2] PCI: Remove reset argument from pci_iov_{add,remove}_virtfn()

2017-09-05 Thread Russell Currey 
On Wed, 2017-09-06 at 01:21 +0200, Jan H. Schönherr wrote:
> The reset argument passed to pci_iov_add_virtfn() and
> pci_iov_remove_virtfn() is always zero since commit 46cb7b1bd86fc227a
> ("PCI: Remove unused SR-IOV VF Migration support").
> 
> Remove the argument together with the associated code.
> 
> Signed-off-by: Jan H. Schönherr 

Acked-by: Russell Currey

[PATCH v2 2/2] powerpc/powernv/npu: Don't explicitly flush nmmu tlb

2017-09-05 Thread Alistair Popple 
The nest mmu required an explicit flush as a tlbi would not flush it in the
same way as the core. However an alternate firmware fix exists which should
eliminate the need for this flush, so instead add a device-tree property
(ibm,nmmu-flush) on the NVLink2 PHB to enable it only if required.

Signed-off-by: Alistair Popple 
---

Changes for v2:
 - Use mm_context_add_copro()/mm_context_remove_copro() instead
   of inc_mm_active_cpus()/dec_mm_active_cpus()

 arch/powerpc/platforms/powernv/npu-dma.c | 28 +++-
 arch/powerpc/platforms/powernv/pci.h |  3 +++
 2 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/npu-dma.c 
b/arch/powerpc/platforms/powernv/npu-dma.c
index 2fff9a65..f6cbc1a 100644
--- a/arch/powerpc/platforms/powernv/npu-dma.c
+++ b/arch/powerpc/platforms/powernv/npu-dma.c
@@ -395,6 +395,7 @@ struct npu_context {
struct pci_dev *npdev[NV_MAX_NPUS][NV_MAX_LINKS];
struct mmu_notifier mn;
struct kref kref;
+   bool nmmu_flush;
 
/* Callback to stop translation requests on a given GPU */
struct npu_context *(*release_cb)(struct npu_context *, void *);
@@ -545,11 +546,13 @@ static void mmio_invalidate(struct npu_context 
*npu_context, int va,
struct mmio_atsd_reg mmio_atsd_reg[NV_MAX_NPUS];
unsigned long pid = npu_context->mm->context.id;
 
-   /*
-* Unfortunately the nest mmu does not support flushing specific
-* addresses so we have to flush the whole mm.
-*/
-   flush_all_mm(npu_context->mm);
+   if (npu_context->nmmu_flush)
+   /*
+* Unfortunately the nest mmu does not support flushing specific
+* addresses so we have to flush the whole mm once before
+* shooting down the GPU translation.
+*/
+   flush_all_mm(npu_context->mm);
 
/*
 * Loop over all the NPUs this process is active on and launch
@@ -722,6 +725,16 @@ struct npu_context *pnv_npu2_init_context(struct pci_dev 
*gpdev,
return ERR_PTR(-ENODEV);
npu_context->npdev[npu->index][nvlink_index] = npdev;
 
+   if (!nphb->npu.nmmu_flush) {
+   /*
+* If we're not explicitly flushing ourselves we need to mark
+* the thread for global flushes
+*/
+   npu_context->nmmu_flush = false;
+   mm_context_add_copro(mm);
+   } else
+   npu_context->nmmu_flush = true;
+
return npu_context;
 }
 EXPORT_SYMBOL(pnv_npu2_init_context);
@@ -731,6 +744,9 @@ static void pnv_npu2_release_context(struct kref *kref)
struct npu_context *npu_context =
container_of(kref, struct npu_context, kref);
 
+   if (!npu_context->nmmu_flush)
+   mm_context_remove_copro(npu_context->mm);
+
npu_context->mm->context.npu_context = NULL;
mmu_notifier_unregister(_context->mn,
npu_context->mm);
@@ -819,6 +835,8 @@ int pnv_npu2_init(struct pnv_phb *phb)
static int npu_index;
uint64_t rc = 0;
 
+   phb->npu.nmmu_flush =
+   of_property_read_bool(phb->hose->dn, "ibm,nmmu-flush");
for_each_child_of_node(phb->hose->dn, dn) {
gpdev = pnv_pci_get_gpu_dev(get_pci_dev(dn));
if (gpdev) {
diff --git a/arch/powerpc/platforms/powernv/pci.h 
b/arch/powerpc/platforms/powernv/pci.h
index a95273c..22025c6 100644
--- a/arch/powerpc/platforms/powernv/pci.h
+++ b/arch/powerpc/platforms/powernv/pci.h
@@ -187,6 +187,9 @@ struct pnv_phb {
 
/* Bitmask for MMIO register usage */
unsigned long mmio_atsd_usage;
+
+   /* Do we need to explicitly flush the nest mmu? */
+   bool nmmu_flush;
} npu;
 
 #ifdef CONFIG_CXL_BASE
-- 
2.1.4

[PATCH v2 1/2] powerpc/npu: Use flush_all_mm() instead of flush_tlb_mm()

2017-09-05 Thread Alistair Popple 
With the optimisations introduced by commit a46cc7a908 ("powerpc/mm/radix:
Improve TLB/PWC flushes"), flush_tlb_mm() no longer flushes the page walk
cache with radix. Switch to using flush_all_mm() to ensure the pwc and tlb
are properly flushed on the nmmu.

Signed-off-by: Alistair Popple 
---
 arch/powerpc/platforms/powernv/npu-dma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/powernv/npu-dma.c 
b/arch/powerpc/platforms/powernv/npu-dma.c
index 2cb6cbe..2fff9a65 100644
--- a/arch/powerpc/platforms/powernv/npu-dma.c
+++ b/arch/powerpc/platforms/powernv/npu-dma.c
@@ -549,7 +549,7 @@ static void mmio_invalidate(struct npu_context 
*npu_context, int va,
 * Unfortunately the nest mmu does not support flushing specific
 * addresses so we have to flush the whole mm.
 */
-   flush_tlb_mm(npu_context->mm);
+   flush_all_mm(npu_context->mm);
 
/*
 * Loop over all the NPUs this process is active on and launch
-- 
2.1.4

Re: [rfc 2/3] powerpc/mce: Extract physical_address for UE errors

2017-09-05 Thread Nicholas Piggin 
On Tue,  5 Sep 2017 14:15:54 +1000
Balbir Singh  wrote:

> Walk the page table for NIP and extract the instruction. Then
> use the instruction to find the effective address via analyse_instr().
> 
> We might have page table walking races, but we expect them to
> be rare, the physical address extraction is best effort. The idea
> is to then hook up this infrastructure to memory failure eventually.

Cool. Too bad hardware doesn't give us the RA.

> 
> Signed-off-by: Balbir Singh 
> ---
>  arch/powerpc/include/asm/mce.h  |  2 +-
>  arch/powerpc/kernel/mce.c   |  6 -
>  arch/powerpc/kernel/mce_power.c | 60 
> +
>  3 files changed, 61 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/mce.h b/arch/powerpc/include/asm/mce.h
> index 75292c7..3a1226e 100644
> --- a/arch/powerpc/include/asm/mce.h
> +++ b/arch/powerpc/include/asm/mce.h
> @@ -204,7 +204,7 @@ struct mce_error_info {
>  
>  extern void save_mce_event(struct pt_regs *regs, long handled,
>  struct mce_error_info *mce_err, uint64_t nip,
> -uint64_t addr);
> +uint64_t addr, uint64_t phys_addr);
>  extern int get_mce_event(struct machine_check_event *mce, bool release);
>  extern void release_mce_event(void);
>  extern void machine_check_queue_event(void);
> diff --git a/arch/powerpc/kernel/mce.c b/arch/powerpc/kernel/mce.c
> index e254399..f41a75d 100644
> --- a/arch/powerpc/kernel/mce.c
> +++ b/arch/powerpc/kernel/mce.c
> @@ -82,7 +82,7 @@ static void mce_set_error_info(struct machine_check_event 
> *mce,
>   */
>  void save_mce_event(struct pt_regs *regs, long handled,
>   struct mce_error_info *mce_err,
> - uint64_t nip, uint64_t addr)
> + uint64_t nip, uint64_t addr, uint64_t phys_addr)
>  {
>   int index = __this_cpu_inc_return(mce_nest_count) - 1;
>   struct machine_check_event *mce = this_cpu_ptr(_event[index]);
> @@ -140,6 +140,10 @@ void save_mce_event(struct pt_regs *regs, long handled,
>   } else if (mce->error_type == MCE_ERROR_TYPE_UE) {
>   mce->u.ue_error.effective_address_provided = true;
>   mce->u.ue_error.effective_address = addr;
> + if (phys_addr != ULONG_MAX) {
> + mce->u.ue_error.physical_address_provided = true;
> + mce->u.ue_error.physical_address = phys_addr;
> + }
>   }
>   return;
>  }
> diff --git a/arch/powerpc/kernel/mce_power.c b/arch/powerpc/kernel/mce_power.c
> index b76ca19..b77a698 100644
> --- a/arch/powerpc/kernel/mce_power.c
> +++ b/arch/powerpc/kernel/mce_power.c
> @@ -27,6 +27,25 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
> +#include 
> +
> +static unsigned long addr_to_pfn(struct mm_struct *mm, unsigned long addr)
> +{
> + pte_t *ptep;
> + unsigned long flags;
> +
> + local_irq_save(flags);
> + if (mm == current->mm)
> + ptep = find_current_mm_pte(mm->pgd, addr, NULL, NULL);
> + else
> + ptep = find_init_mm_pte(addr, NULL);
> + local_irq_restore(flags);
> + if (!ptep)
> + return ULONG_MAX;
> + return pte_pfn(*ptep);

I think you need to check that it's still cacheable memory here?
!pte_speical && pfn <= highest_memmap_pfn?


> +}
>  
>  static void flush_tlb_206(unsigned int num_sets, unsigned int action)
>  {
> @@ -489,7 +508,8 @@ static int mce_handle_ierror(struct pt_regs *regs,
>  
>  static int mce_handle_derror(struct pt_regs *regs,
>   const struct mce_derror_table table[],
> - struct mce_error_info *mce_err, uint64_t *addr)
> + struct mce_error_info *mce_err, uint64_t *addr,
> + uint64_t *phys_addr)
>  {
>   uint64_t dsisr = regs->dsisr;
>   int handled = 0;
> @@ -555,7 +575,37 @@ static int mce_handle_derror(struct pt_regs *regs,
>   mce_err->initiator = table[i].initiator;
>   if (table[i].dar_valid)
>   *addr = regs->dar;
> -
> + else if (mce_err->severity == MCE_SEV_ERROR_SYNC &&
> + table[i].error_type == MCE_ERROR_TYPE_UE) {
> + /*
> +  * Carefully look at the NIP to determine
> +  * the instruction to analyse. Reading the NIP
> +  * in real-mode is tricky and can lead to recursive
> +  * faults
> +  */

What recursive faults? If you ensure NIP is cacheable memory, I guess you
can get a recursive machine check from reading it, but that's probably
tolerable.

> + int instr;
> + struct mm_struct *mm;
> + unsigned long nip = regs->nip;
> + unsigned long pfn = 0, instr_addr;
> + struct instruction_op op;
> +

[PATCH 1/2] PCI: Remove reset argument from pci_iov_{add, remove}_virtfn()

2017-09-05 Thread Jan H . Schönherr 
The reset argument passed to pci_iov_add_virtfn() and
pci_iov_remove_virtfn() is always zero since commit 46cb7b1bd86fc227a
("PCI: Remove unused SR-IOV VF Migration support").

Remove the argument together with the associated code.

Signed-off-by: Jan H. Schönherr 
---
 arch/powerpc/kernel/eeh_driver.c |  4 ++--
 drivers/pci/iov.c| 18 +-
 include/linux/pci.h  |  8 
 3 files changed, 11 insertions(+), 19 deletions(-)

diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index c405c79..23ea86f 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -441,7 +441,7 @@ static void *eeh_add_virt_device(void *data, void *userdata)
}
 
 #ifdef CONFIG_PPC_POWERNV
-   pci_iov_add_virtfn(edev->physfn, pdn->vf_index, 0);
+   pci_iov_add_virtfn(edev->physfn, pdn->vf_index);
 #endif
return NULL;
 }
@@ -499,7 +499,7 @@ static void *eeh_rmv_device(void *data, void *userdata)
 #ifdef CONFIG_PPC_POWERNV
struct pci_dn *pdn = eeh_dev_to_pdn(edev);
 
-   pci_iov_remove_virtfn(edev->physfn, pdn->vf_index, 0);
+   pci_iov_remove_virtfn(edev->physfn, pdn->vf_index);
edev->pdev = NULL;
 
/*
diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index 120485d..21b55ca 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -113,7 +113,7 @@ resource_size_t pci_iov_resource_size(struct pci_dev *dev, 
int resno)
return dev->sriov->barsz[resno - PCI_IOV_RESOURCES];
 }
 
-int pci_iov_add_virtfn(struct pci_dev *dev, int id, int reset)
+int pci_iov_add_virtfn(struct pci_dev *dev, int id)
 {
int i;
int rc = -ENOMEM;
@@ -157,9 +157,6 @@ int pci_iov_add_virtfn(struct pci_dev *dev, int id, int 
reset)
BUG_ON(rc);
}
 
-   if (reset)
-   __pci_reset_function(virtfn);
-
pci_device_add(virtfn, virtfn->bus);
 
pci_bus_add_device(virtfn);
@@ -187,7 +184,7 @@ int pci_iov_add_virtfn(struct pci_dev *dev, int id, int 
reset)
return rc;
 }
 
-void pci_iov_remove_virtfn(struct pci_dev *dev, int id, int reset)
+void pci_iov_remove_virtfn(struct pci_dev *dev, int id)
 {
char buf[VIRTFN_ID_LEN];
struct pci_dev *virtfn;
@@ -198,11 +195,6 @@ void pci_iov_remove_virtfn(struct pci_dev *dev, int id, 
int reset)
if (!virtfn)
return;
 
-   if (reset) {
-   device_release_driver(>dev);
-   __pci_reset_function(virtfn);
-   }
-
sprintf(buf, "virtfn%u", id);
sysfs_remove_link(>dev.kobj, buf);
/*
@@ -317,7 +309,7 @@ static int sriov_enable(struct pci_dev *dev, int nr_virtfn)
pci_cfg_access_unlock(dev);
 
for (i = 0; i < initial; i++) {
-   rc = pci_iov_add_virtfn(dev, i, 0);
+   rc = pci_iov_add_virtfn(dev, i);
if (rc)
goto failed;
}
@@ -329,7 +321,7 @@ static int sriov_enable(struct pci_dev *dev, int nr_virtfn)
 
 failed:
while (i--)
-   pci_iov_remove_virtfn(dev, i, 0);
+   pci_iov_remove_virtfn(dev, i);
 
pcibios_sriov_disable(dev);
 err_pcibios:
@@ -355,7 +347,7 @@ static void sriov_disable(struct pci_dev *dev)
return;
 
for (i = 0; i < iov->num_VFs; i++)
-   pci_iov_remove_virtfn(dev, i, 0);
+   pci_iov_remove_virtfn(dev, i);
 
pcibios_sriov_disable(dev);
 
diff --git a/include/linux/pci.h b/include/linux/pci.h
index f958d07..4acf321 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1903,8 +1903,8 @@ int pci_iov_virtfn_devfn(struct pci_dev *dev, int id);
 
 int pci_enable_sriov(struct pci_dev *dev, int nr_virtfn);
 void pci_disable_sriov(struct pci_dev *dev);
-int pci_iov_add_virtfn(struct pci_dev *dev, int id, int reset);
-void pci_iov_remove_virtfn(struct pci_dev *dev, int id, int reset);
+int pci_iov_add_virtfn(struct pci_dev *dev, int id);
+void pci_iov_remove_virtfn(struct pci_dev *dev, int id);
 int pci_num_vf(struct pci_dev *dev);
 int pci_vfs_assigned(struct pci_dev *dev);
 int pci_sriov_set_totalvfs(struct pci_dev *dev, u16 numvfs);
@@ -1921,12 +1921,12 @@ static inline int pci_iov_virtfn_devfn(struct pci_dev 
*dev, int id)
 }
 static inline int pci_enable_sriov(struct pci_dev *dev, int nr_virtfn)
 { return -ENODEV; }
-static inline int pci_iov_add_virtfn(struct pci_dev *dev, int id, int reset)
+static inline int pci_iov_add_virtfn(struct pci_dev *dev, int id)
 {
return -ENOSYS;
 }
 static inline void pci_iov_remove_virtfn(struct pci_dev *dev,
-int id, int reset) { }
+int id) { }
 static inline void pci_disable_sriov(struct pci_dev *dev) { }
 static inline int pci_num_vf(struct pci_dev *dev) { return 0; }
 static inline int pci_vfs_assigned(struct pci_dev *dev)
--

[PATCH 2/2] PCI: Remove unused function __pci_reset_function()

2017-09-05 Thread Jan H . Schönherr 
The last caller of __pci_reset_function() has been removed. Remove
the function as well.

Signed-off-by: Jan H. Schönherr 
---
 drivers/pci/pci.c   | 35 +++
 include/linux/pci.h |  1 -
 2 files changed, 3 insertions(+), 33 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index fdf65a6..dabcf5a 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -4115,35 +4115,6 @@ static void pci_dev_restore(struct pci_dev *dev)
 }
 
 /**
- * __pci_reset_function - reset a PCI device function
- * @dev: PCI device to reset
- *
- * Some devices allow an individual function to be reset without affecting
- * other functions in the same device.  The PCI device must be responsive
- * to PCI config space in order to use this function.
- *
- * The device function is presumed to be unused when this function is called.
- * Resetting the device will make the contents of PCI configuration space
- * random, so any caller of this must be prepared to reinitialise the
- * device including MSI, bus mastering, BARs, decoding IO and memory spaces,
- * etc.
- *
- * Returns 0 if the device function was successfully reset or negative if the
- * device doesn't support resetting a single function.
- */
-int __pci_reset_function(struct pci_dev *dev)
-{
-   int ret;
-
-   pci_dev_lock(dev);
-   ret = __pci_reset_function_locked(dev);
-   pci_dev_unlock(dev);
-
-   return ret;
-}
-EXPORT_SYMBOL_GPL(__pci_reset_function);
-
-/**
  * __pci_reset_function_locked - reset a PCI device function while holding
  * the @dev mutex lock.
  * @dev: PCI device to reset
@@ -4233,8 +4204,8 @@ int pci_probe_reset_function(struct pci_dev *dev)
  *
  * This function does not just reset the PCI portion of a device, but
  * clears all the state associated with the device.  This function differs
- * from __pci_reset_function in that it saves and restores device state
- * over the reset.
+ * from __pci_reset_function_locked() in that it saves and restores device 
state
+ * over the reset and takes the PCI device lock.
  *
  * Returns 0 if the device function was successfully reset or negative if the
  * device doesn't support resetting a single function.
@@ -4269,7 +4240,7 @@ EXPORT_SYMBOL_GPL(pci_reset_function);
  *
  * This function does not just reset the PCI portion of a device, but
  * clears all the state associated with the device.  This function differs
- * from __pci_reset_function() in that it saves and restores device state
+ * from __pci_reset_function_locked() in that it saves and restores device 
state
  * over the reset.  It also differs from pci_reset_function() in that it
  * requires the PCI device lock to be held.
  *
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 4acf321..b55e0cf 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1066,7 +1066,6 @@ int pcie_set_mps(struct pci_dev *dev, int mps);
 int pcie_get_minimum_link(struct pci_dev *dev, enum pci_bus_speed *speed,
  enum pcie_link_width *width);
 void pcie_flr(struct pci_dev *dev);
-int __pci_reset_function(struct pci_dev *dev);
 int __pci_reset_function_locked(struct pci_dev *dev);
 int pci_reset_function(struct pci_dev *dev);
 int pci_reset_function_locked(struct pci_dev *dev);
-- 
2.10.0.1.g70cd14e

Re: [PATCH] sound: soc: fsl: Do not set DAI sysclk when it is equal to system freq

2017-09-05 Thread Fabio Estevam 
On Tue, Sep 5, 2017 at 6:13 PM, Łukasz Majewski  wrote:

>  {
> clock-frequency = <40>;
> pinctrl-names = "default";
> pinctrl-0 = <_i2c1>;
> status = "okay";
>
> codec: tfa9879@6C {
> #sound-dai-cells = <0>;
> compatible = "tfa9879";

This codec seems to miss device tree support. Don't you need something
like this?

--- a/sound/soc/codecs/tfa9879.c
+++ b/sound/soc/codecs/tfa9879.c
@@ -312,9 +312,15 @@ static const struct i2c_device_id tfa9879_i2c_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, tfa9879_i2c_id);

+static const struct of_device_id tfa9879_of_match[] = {
+   { .compatible = "nxp,tfa9879", },
+   { }
+};
+
 static struct i2c_driver tfa9879_i2c_driver = {
.driver = {
.name = "tfa9879",
+   .of_match_table = tfa9879_of_match,
},
.probe = tfa9879_i2c_probe,
.remove = tfa9879_i2c_remove,

Re: [PATCH] sound: soc: fsl: Do not set DAI sysclk when it is equal to system freq

2017-09-05 Thread Nicolin Chen 
On Tue, Sep 05, 2017 at 11:13:40PM +0200, Łukasz Majewski wrote:

> They key point here is the asoc_simple_card_parse_clk() function
> from simple-card-utils.c
> 
> Please look how the clock is assigned; It first checks for cpu
> clock, then for "system-clock-frequency" DTS node and _finally_
> looks for another "child" clock [1], which is the codec attached to
> I2C.

Here is the routine that I understood from the code:
1) asoc_simple_card_parse_clk_cpu(dev, cpu, dai_link, cpu_dai);
   => asoc_simple_card_parse_clk(dev, cpu,// cpu node in sound{} [1]
 dai_link->cpu_of_node, // node ssi2 [2]
 cpu_dai, dai_link->cpu_dai_name);
   ==> 1.1) devm_get_clk_from_child(dev, node, NULL); // [1]
   ==> 1.2) of_property_read_u32(node, "system-clock-frequency", )// [1]
   ==> 1.3) devm_get_clk_from_child(dev, dai_of_node, NULL);  // [2]

2) asoc_simple_card_parse_clk_codec(dev, codec, dai_link, codec_dai);
   => asoc_simple_card_parse_clk(dev, codec,// codec node in sound{} [3]
 dai_link->codec_of_node,// node tfa9879 [4]
 codec_dai, dai_link->codec_dai_name);
   ==> 2.1) devm_get_clk_from_child(dev, node, NULL); // [3]
   ==> 2.2) of_property_read_u32(node, "system-clock-frequency", )// [3]
   ==> 2.3) devm_get_clk_from_child(dev, dai_of_node, NULL);  // [4]

For the cpu routine, it first checks for clock property under cpu
node of simple-card, then for "system-clock-frequency" in the cpu
node of simple-card, and finally looks for clock property in ssi2
node.

For codec routine, it first checks for clock property under codec
node of simple-card, then for "system-clock-frequency" in codec
node of simple-card, and finally looks for clock property in the
tfa9879 node.

The cpu and codec are totally separate, aren't they? And I don't
think it's right if not.

> >>which has clock from I2C (66 MHz).
> >
> >You mean I2C scl or I2S sclk?
> 
> I2C scl.

A couple of things here.
1) I don't think I2C scl can run at 66MHz...The 66MHz is probably
   the ipg clock of I2C controller.
2) Even if the scl does run at 66MHz, there is no point in passing
   the clock of I2C scl into an I2S dai-link.
   
So I feel something must be wrong. I suggest you to add some printk
to check the names of those nodes and x_of_node in the simple card
driver.

> My DTS [1] (it is different than other in-tree supported codecs - at
> least I did not find similar setup in DTSes):
> 
>   sound {
>   compatible = "simple-audio-card";
>   label = "tfa9879-mono";
> 
>   simple-audio-card,dai-link {
>   /* DAC */
>   format = "i2s";
>   bitclock-master = <_master>;
>   frame-master = <_master>;
> 
>   dailink_master: cpu {
>   sound-dai = <>;
>   };
>   codec {
>   sound-dai = <>;
>   };
>   };

I remember there is another style for a single dai-link. Could you
please try that one? There is an example in:
Documentation/devicetree/bindings/sound/simple-card.txt

Re: [PATCH] devicetree: Remove remaining references/tests for "chosen@0"

2017-09-05 Thread Robert P. J. Day 
On Tue, 5 Sep 2017, Rob Herring wrote:

> On Sun, Sep 3, 2017 at 5:43 AM, Robert P. J. Day  
> wrote:
> > On Sun, 3 Sep 2017, Benjamin Herrenschmidt wrote:
> >
> >> On Sat, 2017-09-02 at 04:43 -0400, Robert P. J. Day wrote:
> >> > Since, according to a recent devicetree ML posting by Rob Herring,
> >> > the node "/chosen@0" is most likely for real Open Firmware and
> >> > does not apply to DTSpec, remove all remaining tests and
> >> > references for that node, of which there are very few left:
> >>
> >> Technically that would break Open Firmware systems where the node is
> >> really called chosen@0
> >>
> >> Now I'm not sure such a thing actually exist however.
> >>
> >> My collection of DTs don't seem to have one, except in the ancient
> >> html variants that were extracted by the pengionppc folks for the
> >> original PowerMac 8600 but I wonder if that's a bug in the
> >> extraction script since they also have @0 on /packages etc...
> >
> >   obviously, this isn't a priority issue, i was just working off a
> > comment by rob herring that "chosen@0" is not defined by the current
> > DTSpec 0.1, so it seemed appropriate to toss it. if there's a reason
> > to hang onto it, that's fine with me.
> >
> >   however, given the diff stat of the change to remove every single
> > reference to that node name in the current kernel source:
> >
> >  arch/microblaze/kernel/prom.c | 3 +--
> >  arch/mips/generic/yamon-dt.c  | 4 
> >  arch/powerpc/boot/oflib.c | 7 ++-
> >  drivers/of/base.c | 2 --
> >  drivers/of/fdt.c  | 5 +
> >  5 files changed, 4 insertions(+), 17 deletions(-)
> >
> > it seems inconsistent that three architectures would be testing for
> > that node, but none of the rest. consistency suggests that every
> > architecture should take it into account, or none should.
>
> I generally agree and have moved various things from arch to
> drivers/of/ to ensure that. But for legacy things, we have to allow
> for exceptions. I agree with Ben and think that microblaze (they
> generally just copied PPC), MIPS, and the FDT code in drivers/of/ can
> be changed.

  i'm not quite sure what people have agreed on, so i'll just leave it
in the hands of others here to submit patches for what they think is
appropriate.

rday

-- 


Robert P. J. Day Ottawa, Ontario, CANADA
http://crashcourse.ca

Twitter:   http://twitter.com/rpjday
LinkedIn:   http://ca.linkedin.com/in/rpjday

Re: [PATCH] sound: soc: fsl: Do not set DAI sysclk when it is equal to system freq

2017-09-05 Thread Łukasz Majewski 

Hi Fabio,


Hi Lukasz,

On Tue, Sep 5, 2017 at 5:35 AM, Łukasz Majewski  wrote:


Note:
[*] - I could workaround this problem by setting:

system-clock-frequency = <0> in

 dailink_master: cpu {
 sound-dai = <>;
 };



Which mx6 type are you using? Can you post the complete audio related
bindings of your dts?

Still trying to understand why we do not see this error on other imx6 boards.



I've sent my DTS in the other e-mail.

--
Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de

Re: [PATCH] sound: soc: fsl: Do not set DAI sysclk when it is equal to system freq

2017-09-05 Thread Łukasz Majewski 

Hi Nicolin,


On Tue, Sep 05, 2017 at 10:35:34AM +0200, Łukasz Majewski wrote:


And apparently, we shouldn't set bitclk to 66MHz either. Can
you help to find where this 66MHz comes from?



2. int asoc_simple_card_init_dai() @ simple-card-utils.c


Oh, I just searched in the simple-card.c but missed this file.


In this function (point 2.) the
simple_dai->sysclk is set and:
snd_soc_dai_set_sysclk(dai, 0, simple_dai->sysclk, 0)
which sets frequency to 66 MHz [*].

The asoc_simple_card_init_dai() is called in
asoc_simple_card_dai_init() @ simple-card.c
which is assigned to dai_link->init
dai_link->init   = asoc_simple_card_dai_init; @ simple_card.c

And the sysclk itself is defined at:
-
dai_props->codec_dai->sysclk, which is used at:a


Why codec_dai? Why not dai_props->cpu_dai->sysclk since we are talking
about SSI?


This is how the simple-card (simple-sound-card) is written.




asoc_simple_card_startup(), asoc_simple_card_shutdown() and others
functions at simple-card.c
It is setup at:
asoc_simple_card_parse_clk() @ simple-card-utils.c from macro:
#define asoc_simple_card_parse_clk_cpu()
And the problem is:
---

At the
asoc_simple_card_parse_clk()
we finally go to dts node:
/soc/aips-bus@0210/i2c@021a/tfa9879@6C


This tfa9879 should be the CODEC right?


Yes. The tfa9879 is a codec (very simple -> I2S + I2C, mono).

They key point here is the asoc_simple_card_parse_clk() function from 
simple-card-utils.c


Please look how the clock is assigned; It first checks for cpu clock, 
then for "system-clock-frequency" DTS node and _finally_ looks for 
another "child" clock [1], which is the codec attached to I2C.


And from there it takes the 66 MHz CLK:
/soc/aips-bus@0210/i2c@021a/tfa9879@6C






which has clock from I2C (66 MHz).


You mean I2C scl or I2S sclk?


I2C scl.



-

But anyway, I feel very confused here as you have 66MHz clock rate
(regardless of it purpose) for a codec dai but it's been passed to
a cpu dai (SSI).


Please look into asoc_simple_card_parse_clk().


My DTS [1] (it is different than other in-tree supported codecs - at 
least I did not find similar setup in DTSes):


sound {
compatible = "simple-audio-card";
label = "tfa9879-mono";

simple-audio-card,dai-link {
/* DAC */
format = "i2s";
bitclock-master = <_master>;
frame-master = <_master>;

dailink_master: cpu {
sound-dai = <>;
};
codec {
sound-dai = <>;
};
};
};


 {
clock-frequency = <40>;
pinctrl-names = "default";
pinctrl-0 = <_i2c1>;
status = "okay";

codec: tfa9879@6C {
#sound-dai-cells = <0>;
compatible = "tfa9879";
reg = <0x6C>;
};
};

 {
fsl,mode = "i2s-master";
status = "okay";
};

the ssi2 node is defined in imx6qdl.dtsi file (no changes).

The SOC is IMX6Q.

The TFA9879 is a slave for I2S transmission.





[*] - I could workaround this problem by setting:

system-clock-frequency = <0> in

dailink_master: cpu {
sound-dai = <>;
};

but this is IMHO even worse hack than this patch.


I haven't used simple-card for a while so I forgot how to define
its DT bindings specifically. But you should assign ssi2 as the
CPU dai and assign tfa9879 as a CODEC dai.




--
Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de

Re: [PATCH] sound: soc: fsl: Do not set DAI sysclk when it is equal to system freq

2017-09-05 Thread Fabio Estevam 
Hi Lukasz,

On Tue, Sep 5, 2017 at 5:35 AM, Łukasz Majewski  wrote:

> Note:
> [*] - I could workaround this problem by setting:
>
> system-clock-frequency = <0> in
>
> dailink_master: cpu {
> sound-dai = <>;
> };
>

Which mx6 type are you using? Can you post the complete audio related
bindings of your dts?

Still trying to understand why we do not see this error on other imx6 boards.

Re: [PATCH] sound: soc: fsl: Do not set DAI sysclk when it is equal to system freq

2017-09-05 Thread Nicolin Chen 
On Tue, Sep 05, 2017 at 10:35:34AM +0200, Łukasz Majewski wrote:

> >And apparently, we shouldn't set bitclk to 66MHz either. Can
> >you help to find where this 66MHz comes from?

> 2. int asoc_simple_card_init_dai() @ simple-card-utils.c

Oh, I just searched in the simple-card.c but missed this file.

> In this function (point 2.) the
> simple_dai->sysclk is set and:
> snd_soc_dai_set_sysclk(dai, 0, simple_dai->sysclk, 0)
> which sets frequency to 66 MHz [*].
> 
> The asoc_simple_card_init_dai() is called in
> asoc_simple_card_dai_init() @ simple-card.c
> which is assigned to dai_link->init
> dai_link->init= asoc_simple_card_dai_init; @ simple_card.c
> 
> And the sysclk itself is defined at:
> -
> dai_props->codec_dai->sysclk, which is used at:a

Why codec_dai? Why not dai_props->cpu_dai->sysclk since we are talking
about SSI?

> asoc_simple_card_startup(), asoc_simple_card_shutdown() and others
> functions at simple-card.c
> It is setup at:
> asoc_simple_card_parse_clk() @ simple-card-utils.c from macro:
> #define asoc_simple_card_parse_clk_cpu()
> And the problem is:
> ---
> 
> At the
> asoc_simple_card_parse_clk()
> we finally go to dts node:
> /soc/aips-bus@0210/i2c@021a/tfa9879@6C

This tfa9879 should be the CODEC right?

> which has clock from I2C (66 MHz).

You mean I2C scl or I2S sclk?

-

But anyway, I feel very confused here as you have 66MHz clock rate
(regardless of it purpose) for a codec dai but it's been passed to
a cpu dai (SSI).

> [*] - I could workaround this problem by setting:
> 
> system-clock-frequency = <0> in
> 
>   dailink_master: cpu {
>   sound-dai = <>;
>   };
> 
> but this is IMHO even worse hack than this patch.

I haven't used simple-card for a while so I forgot how to define
its DT bindings specifically. But you should assign ssi2 as the
CPU dai and assign tfa9879 as a CODEC dai.

Re: [PATCH] sound: soc: fsl: Do not set DAI sysclk when it is equal to system freq

2017-09-05 Thread Nicolin Chen 
On Tue, Sep 05, 2017 at 04:15:50PM +0100, Mark Brown wrote:
 
> > Just to be clear:
> 
> > What clock shall be set with:
> 
> > struct snd_soc_dai_ops {
> > int (*set_sysclk)(struct snd_soc_dai *dai,
> > int clk_id, unsigned int freq, int dir);
> > }
> 
> > callback?
> 
> > The SSI IP block or BCLK ?
> 
> Not the BCLK, probably the IP clock but perhaps nothing.  The bclk is
> usually derived from the sample rate and width, the system clock is the
> clock rate going into the device.  It doesn't *have* to be configured
> (particuarly if it's discoverable by the IP and managable via the clock
> API).

Hmm...to clarify the things, some background here:

SSI has mainly two different clock inputs internally (not external
pads such as bclk or lrclk): IP block clock (ipg_clk in Reference
Manual) and sys clock (ccm_ssi_clk in Reference Manual). According
to RM, ccm_ssi_clk: "module/system clock for bit clock generation".

The ipg clock is merely used to access registers, and has nothing
(directly) to do with external clock outputs. The driver shall not
change the ipg clock as the system ipg clock (its parent clock)
might be messed and even system time would get weird -- happened
once when the fsl_spdif driver used to call clk_set_rate() on its
ipg clock. Although the clock controller should have some kind of
protection in my opinion, we just avoid IP clock rate change in all
audio drivers as well.

On the other hand, the sys clock (baudclk in the driver) should be
configured whenever it's related to external clock outputs. When I
implemented this set_sysclk() for fsl_ssi.c, I used it to set this
sys clock (baudclk) by a machine driver, in order to set bit clock.
Then someone patched the driver by moving all the code to set_bclk()
to make machine drivers simpler. Now the set_sysclk() is remained
to give machine drivers a chance to override clock configurations
in the hw_params(). This could be used in TDM or some other special
cases (It could also have a purpose for backwards compatibility).

So here, we should set baudclk (BCLK generator).

[PATCH v2] axonram: Delete an unnecessary variable initialisation in axon_ram_probe()

2017-09-05 Thread SF Markus Elfring 
From: Markus Elfring 
Date: Tue, 5 Sep 2017 18:47:02 +0200

The local variable "rc" will eventually be set only to an error code.
Thus omit the explicit initialisation at the beginning.

Signed-off-by: Markus Elfring 
---

v2:
Three update steps were integrated for this software module on 2017-09-01.
Thus improve another source code place.

 arch/powerpc/sysdev/axonram.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/sysdev/axonram.c b/arch/powerpc/sysdev/axonram.c
index c60e84e4558d..1b307c80b401 100644
--- a/arch/powerpc/sysdev/axonram.c
+++ b/arch/powerpc/sysdev/axonram.c
@@ -184,7 +184,7 @@ static int axon_ram_probe(struct platform_device *device)
static int axon_ram_bank_id = -1;
struct axon_ram_bank *bank;
struct resource resource;
-   int rc = 0;
+   int rc;
 
axon_ram_bank_id++;
 
-- 
2.14.1

Re: [PATCH] sound: soc: fsl: Do not set DAI sysclk when it is equal to system freq

2017-09-05 Thread Mark Brown 
On Tue, Sep 05, 2017 at 10:19:05AM +0200, Łukasz Majewski wrote:
> On 09/05/2017 09:52 AM, Nicolin Chen wrote:

> > Can you elaborate why you set ipg clock as bclk? I don't remember SSI could
> > derive bitclock from ipg clock.

> Just to be clear:

> What clock shall be set with:

> struct snd_soc_dai_ops {
>   int (*set_sysclk)(struct snd_soc_dai *dai,
>   int clk_id, unsigned int freq, int dir);
> }

> callback?

> The SSI IP block or BCLK ?

Not the BCLK, probably the IP clock but perhaps nothing.  The bclk is
usually derived from the sample rate and width, the system clock is the
clock rate going into the device.  It doesn't *have* to be configured
(particuarly if it's discoverable by the IP and managable via the clock
API).


signature.asc
Description: PGP signature

Re: [PATCH] devicetree: Remove remaining references/tests for "chosen@0"

2017-09-05 Thread Rob Herring 
On Sat, Sep 2, 2017 at 3:43 AM, Robert P. J. Day  wrote:
>
> Since, according to a recent devicetree ML posting by Rob Herring,
> the node "/chosen@0" is most likely for real Open Firmware and does
> not apply to DTSpec, remove all remaining tests and references for
> that node, of which there are very few left:
>
>  arch/microblaze/kernel/prom.c | 3 +--
>  arch/mips/generic/yamon-dt.c  | 4 
>  arch/powerpc/boot/oflib.c | 7 ++-
>  drivers/of/base.c | 2 --
>  drivers/of/fdt.c  | 5 +
>  5 files changed, 4 insertions(+), 17 deletions(-)
>
> This should be innocuous as, in all of the three arch/ files above,
> there is a test for "chosen" immediately before the test for
> "chosen@0", so nothing should change.
>
> Signed-off-by: Robert P. J. Day 
>
> ---
>
>   if this patch is premature, then just ignore it, thanks.
>
> diff --git a/arch/microblaze/kernel/prom.c b/arch/microblaze/kernel/prom.c
> index 68f0999..c81bfd7 100644
> --- a/arch/microblaze/kernel/prom.c
> +++ b/arch/microblaze/kernel/prom.c
> @@ -53,8 +53,7 @@ static int __init early_init_dt_scan_chosen_serial(unsigned 
> long node,
>
> pr_debug("%s: depth: %d, uname: %s\n", __func__, depth, uname);
>
> -   if (depth == 1 && (strcmp(uname, "chosen") == 0 ||
> -   strcmp(uname, "chosen@0") == 0)) {
> +   if (depth == 1 && (strcmp(uname, "chosen") == 0)) {

I'd really hoped to remove early_init_dt_scan_chosen_serial()
altogether. It may now be just a matter of adding the compatible
strings to the uartlite earlycon.

Rob

Re: [PATCH] devicetree: Remove remaining references/tests for "chosen@0"

2017-09-05 Thread Rob Herring 
On Sun, Sep 3, 2017 at 5:43 AM, Robert P. J. Day  wrote:
> On Sun, 3 Sep 2017, Benjamin Herrenschmidt wrote:
>
>> On Sat, 2017-09-02 at 04:43 -0400, Robert P. J. Day wrote:
>> > Since, according to a recent devicetree ML posting by Rob Herring,
>> > the node "/chosen@0" is most likely for real Open Firmware and
>> > does not apply to DTSpec, remove all remaining tests and
>> > references for that node, of which there are very few left:
>>
>> Technically that would break Open Firmware systems where the node is
>> really called chosen@0
>>
>> Now I'm not sure such a thing actually exist however.
>>
>> My collection of DTs don't seem to have one, except in the ancient
>> html variants that were extracted by the pengionppc folks for the
>> original PowerMac 8600 but I wonder if that's a bug in the
>> extraction script since they also have @0 on /packages etc...
>
>   obviously, this isn't a priority issue, i was just working off a
> comment by rob herring that "chosen@0" is not defined by the current
> DTSpec 0.1, so it seemed appropriate to toss it. if there's a reason
> to hang onto it, that's fine with me.
>
>   however, given the diff stat of the change to remove every single
> reference to that node name in the current kernel source:
>
>  arch/microblaze/kernel/prom.c | 3 +--
>  arch/mips/generic/yamon-dt.c  | 4 
>  arch/powerpc/boot/oflib.c | 7 ++-
>  drivers/of/base.c | 2 --
>  drivers/of/fdt.c  | 5 +
>  5 files changed, 4 insertions(+), 17 deletions(-)
>
> it seems inconsistent that three architectures would be testing for
> that node, but none of the rest. consistency suggests that every
> architecture should take it into account, or none should.

I generally agree and have moved various things from arch to
drivers/of/ to ensure that. But for legacy things, we have to allow
for exceptions. I agree with Ben and think that microblaze (they
generally just copied PPC), MIPS, and the FDT code in drivers/of/ can
be changed.

Rob

[PATCH] fsl_pci: Correct fsl_pci_mcheck_exception

2017-09-05 Thread Joakim Tjernlund 
get_user() had it args reversed causing NIP to be NULL:ed instead
of fixing up the PCI access.

Note: This still hangs my P1020 Freescale CPU hard, but at least
I get a NIP now.

Signed-off-by: Joakim Tjernlund 
---
 arch/powerpc/sysdev/fsl_pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c
index 7c8b779c329a..9e64c12dff6a 100644
--- a/arch/powerpc/sysdev/fsl_pci.c
+++ b/arch/powerpc/sysdev/fsl_pci.c
@@ -996,7 +996,7 @@ int fsl_pci_mcheck_exception(struct pt_regs *regs)
if (is_in_pci_mem_space(addr)) {
if (user_mode(regs)) {
pagefault_disable();
-   ret = get_user(regs->nip, );
+   ret = get_user(inst, (__u32 __user *)regs->nip);
pagefault_enable();
} else {
ret = probe_kernel_address(regs->nip, inst);
-- 
2.13.5

Re: powerpc/eeh: Delete an error message for a failed memory allocation in two functions

2017-09-05 Thread SF Markus Elfring 
> Applied to powerpc next, thanks.
> 
> https://git.kernel.org/powerpc/c/6ab41161b44a3b4d504ac29c9dd997

Thanks that you picked another update suggestion up.

* It might matter to mention that only one patch hunk was accepted in this case.

* Can it be that the adjusted commit subject does contain a typo at the moment?
  Would the wording “powerpc/eeh: Delete a message for an "out of memory error"
  (at init time)” be more appropriate there?

Regards,
Markus

Re: [PATCH 2/2] powerpc/powernv/npu: Don't explicitly flush nmmu tlb

2017-09-05 Thread Alistair Popple 
Hi,

On Tue, 5 Sep 2017 10:10:03 AM Frederic Barrat wrote:
> > 
> > +   if (!nphb->npu.nmmu_flush) {
> > +   /*
> > +* If we're not explicitly flushing ourselves we need to mark
> > +* the thread for global flushes
> > +*/
> > +   npu_context->nmmu_flush = false;
> > +   inc_mm_active_cpus(mm);
> 
> 
> I can see that you use the inc_mm_active_cpus()/dec_mm_active_cpus() and 
> not the new mm_context_add_copro()/mm_context_remove_copro(). The only 
> difference is that mm_context_remove_copro() adds an extra flush of the 
> full mm (pwc and tlb) before decrementing the active cpu count.

Thanks for pointing that out, I'd overlooked the difference between the two.

> But don't you need the flush when releasing the context? As soon as the 
> active cpu count is decremented, the next TLBI may be local and not 
> visible to the nMMU, so the nMMU could keep obsolete/wrong entries in 
> its cache.

Yes, I think you're right. In practice I doubt we'd ever hit it as the driver
never calls pnv_npu2_destroy_context() prior to the process being torn down
(which should trigger the right global tlbies). But obviously something that
needs fixing so I will post a v2 ... thanks for reviewing!

Regards,

Alistair

>Fred
> 
> 
> 
> > +   } else
> > +   npu_context->nmmu_flush = true;
> > +
> > return npu_context;
> >   }
> >   EXPORT_SYMBOL(pnv_npu2_init_context);
> > @@ -731,6 +744,9 @@ static void pnv_npu2_release_context(struct kref *kref)
> > struct npu_context *npu_context =
> > container_of(kref, struct npu_context, kref);
> > 
> > +   if (!npu_context->nmmu_flush)
> > +   dec_mm_active_cpus(npu_context->mm);
> > +
> > npu_context->mm->context.npu_context = NULL;
> > mmu_notifier_unregister(_context->mn,
> > npu_context->mm);
> > @@ -819,6 +835,8 @@ int pnv_npu2_init(struct pnv_phb *phb)
> > static int npu_index;
> > uint64_t rc = 0;
> > 
> > +   phb->npu.nmmu_flush =
> > +   of_property_read_bool(phb->hose->dn, "ibm,nmmu-flush");
> > for_each_child_of_node(phb->hose->dn, dn) {
> > gpdev = pnv_pci_get_gpu_dev(get_pci_dev(dn));
> > if (gpdev) {
> > diff --git a/arch/powerpc/platforms/powernv/pci.h 
> > b/arch/powerpc/platforms/powernv/pci.h
> > index a95273c..22025c6 100644
> > --- a/arch/powerpc/platforms/powernv/pci.h
> > +++ b/arch/powerpc/platforms/powernv/pci.h
> > @@ -187,6 +187,9 @@ struct pnv_phb {
> > 
> > /* Bitmask for MMIO register usage */
> > unsigned long mmio_atsd_usage;
> > +
> > +   /* Do we need to explicitly flush the nest mmu? */
> > +   bool nmmu_flush;
> > } npu;
> > 
> >   #ifdef CONFIG_CXL_BASE
> > 
>

Re: [PATCH V2] cxl: Add support for POWER9 DD2

2017-09-05 Thread Frederic Barrat 



Le 04/09/2017 à 17:29, Christophe Lombard a écrit :

The PSL initialization sequence has been updated to DD2.
This patch adapts to the changes, retaining compatibility with DD1.
The patch includes some changes to DD1 fix-ups as well.

Tests performed on some of the old/new hardware.

The function is_page_fault(), for POWER9, lists the Translation Checkout
Responses where the page fault will be handled by copro_handle_mm_fault().
This list is too restrictive and not necessary.

This patches removes this restriction and all page faults, whatever the
reason, will be handled. In this case, the interruption is always
acknowledged.

Signed-off-by: Christophe Lombard 

---


I'm ok with those changes.
We should add to the commit message that we'll have more changes coming, 
at least to add the phb reset when switching to capi mode (if we still 
can't get rid of it). The other item is capp recovery, which is still 
being worked on, but any change required there would hopefully be 
limited to skiboot.


With the above:
Acked-by: Frederic Barrat 

  Fred



Changelog[v2]
  - Rebase to latest upstream.
  - Update the function is_page_fault()
---
  drivers/misc/cxl/cxl.h   |  2 ++
  drivers/misc/cxl/fault.c | 15 ++-
  drivers/misc/cxl/pci.c   | 46 +++---
  3 files changed, 27 insertions(+), 36 deletions(-)

diff --git a/drivers/misc/cxl/cxl.h b/drivers/misc/cxl/cxl.h
index b1afecc..0167df8 100644
--- a/drivers/misc/cxl/cxl.h
+++ b/drivers/misc/cxl/cxl.h
@@ -100,6 +100,8 @@ static const cxl_p1_reg_t CXL_XSL_FEC   = {0x0158};
  static const cxl_p1_reg_t CXL_XSL_DSNCTL= {0x0168};
  /* PSL registers - CAIA 2 */
  static const cxl_p1_reg_t CXL_PSL9_CONTROL  = {0x0020};
+static const cxl_p1_reg_t CXL_XSL9_INV  = {0x0110};
+static const cxl_p1_reg_t CXL_XSL9_DEF  = {0x0140};
  static const cxl_p1_reg_t CXL_XSL9_DSNCTL   = {0x0168};
  static const cxl_p1_reg_t CXL_PSL9_FIR1 = {0x0300};
  static const cxl_p1_reg_t CXL_PSL9_FIR2 = {0x0308};
diff --git a/drivers/misc/cxl/fault.c b/drivers/misc/cxl/fault.c
index 6eed7d0..0cf7f4a 100644
--- a/drivers/misc/cxl/fault.c
+++ b/drivers/misc/cxl/fault.c
@@ -204,22 +204,11 @@ static bool cxl_is_segment_miss(struct cxl_context *ctx, 
u64 dsisr)

  static bool cxl_is_page_fault(struct cxl_context *ctx, u64 dsisr)
  {
-   u64 crs; /* Translation Checkout Response Status */
-
if ((cxl_is_power8()) && (dsisr & CXL_PSL_DSISR_An_DM))
return true;

-   if (cxl_is_power9()) {
-   crs = (dsisr & CXL_PSL9_DSISR_An_CO_MASK);
-   if ((crs == CXL_PSL9_DSISR_An_PF_SLR) ||
-   (crs == CXL_PSL9_DSISR_An_PF_RGC) ||
-   (crs == CXL_PSL9_DSISR_An_PF_RGP) ||
-   (crs == CXL_PSL9_DSISR_An_PF_HRH) ||
-   (crs == CXL_PSL9_DSISR_An_PF_STEG) ||
-   (crs == CXL_PSL9_DSISR_An_URTCH)) {
-   return true;
-   }
-   }
+   if (cxl_is_power9())
+   return true;

return false;
  }
diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
index d18b3d9..3edc991 100644
--- a/drivers/misc/cxl/pci.c
+++ b/drivers/misc/cxl/pci.c
@@ -401,7 +401,8 @@ int cxl_calc_capp_routing(struct pci_dev *dev, u64 *chipid,
*capp_unit_id = get_capp_unit_id(np, *phb_index);
of_node_put(np);
if (!*capp_unit_id) {
-   pr_err("cxl: invalid capp unit id\n");
+   pr_err("cxl: invalid capp unit id (phb_index: %d)\n",
+  *phb_index);
return -ENODEV;
}

@@ -475,37 +476,36 @@ static int init_implementation_adapter_regs_psl9(struct 
cxl *adapter,
psl_fircntl |= 0x1ULL; /* ce_thresh */
cxl_p1_write(adapter, CXL_PSL9_FIR_CNTL, psl_fircntl);

-   /* vccredits=0x1  pcklat=0x4 */
-   cxl_p1_write(adapter, CXL_PSL9_DSNDCTL, 0x1810ULL);
-
-   /*
-* For debugging with trace arrays.
-* Configure RX trace 0 segmented mode.
-* Configure CT trace 0 segmented mode.
-* Configure LA0 trace 0 segmented mode.
-* Configure LA1 trace 0 segmented mode.
+   /* Setup the PSL to transmit packets on the PCIe before the
+* CAPP is enabled
 */
-   cxl_p1_write(adapter, CXL_PSL9_TRACECFG, 0x804080008000ULL);
-   cxl_p1_write(adapter, CXL_PSL9_TRACECFG, 0x804080008003ULL);
-   cxl_p1_write(adapter, CXL_PSL9_TRACECFG, 0x804080008005ULL);
-   cxl_p1_write(adapter, CXL_PSL9_TRACECFG, 0x804080008006ULL);
+   cxl_p1_write(adapter, CXL_PSL9_DSNDCTL, 0x000100102A10ULL);

/*
 * A response to an ASB_Notify request is returned by the
 * system as an MMIO write to the address defined in
-* the PSL_TNR_ADDR register
+* the PSL_TNR_ADDR register.
+* keep the Reset Value: 0x0002E000

Re: Machine Check in P2010(e500v2)

2017-09-05 Thread Joakim Tjernlund 
So after some debugging I found this bug:
@@ -996,7 +998,7 @@ int fsl_pci_mcheck_exception(struct pt_regs *regs)
if (is_in_pci_mem_space(addr)) {
if (user_mode(regs)) {
pagefault_disable();
-   ret = get_user(regs->nip, );
+   ret = get_user(inst, (__u32 __user *)regs->nip);
pagefault_enable();
} else {
ret = probe_kernel_address(regs->nip, inst);

However, the kernel still locked up after fixing that.
Now I wonder why this fixup is there in the first place? The routine
will not really fixup the insn, just return 0x for the failing
read and then advance the process NIP.

Removing the fixup does not help either, kernel still locks up:
[   28.170532] Machine check in kernel mode.
[   28.174538] Caused by (from MCSR=10008):
[   28.182804] Bus - Read Data Bus Error: DAR:b7013000
[   28.197079] Oops: Machine check, sig: 7 [#1]
[   28.201343] P1010 RDB
[   28.203608] Modules linked in: linux_bcm_knet(PO) linux_user_bde(PO) 
linux_kernel_bde(PO)
[   28.211796] CPU: 0 PID: 470 Comm: emxp2_hw_bl Tainted: P   O
4.1.38+ #201
[   28.219540] task: db16ed10 ti: df122000 task.ti: df122000
[   28.224935] NIP: 10a4e2f4 LR: 10a4e404 CTR: 10046c38
[   28.229896] REGS: df123f10 TRAP: 0204   Tainted: P   O (4.1.38+)
[   28.236942] MSR: 0002d000   CR: 44002428  XER: 
[   28.243306] DEAR: b7013000 ESR: 
GPR00: 10a4e404 bfab2730 b7b354a0 132f9fa8 07006000 0700  132f9fd8
GPR08: b6fd5000 b6fe5000 0003e000 bfab2720 24004424 11d6cf7c  
GPR16: 10f6e29c 10f6c872 10f6db01 b541 b541 11d92fcc 0011 0001
GPR24: 01a5bd3e 132ffbf0 11d6  07006000  132f9fa8 
[   28.275547] NIP [10a4e2f4] 0x10a4e2f4
[   28.279204] LR [10a4e404] 0x10a4e404
[   28.282772] Call Trace:
[   28.285213] ---[ end trace 9f8b64ab1e83f449 ]---
[   28.289825]


 Jocke 

On Fri, 2017-09-01 at 13:32 +0200, Joakim Tjernlund wrote:
> I am trying to debug a Machine Check for a P2010 (e500v2) CPU:
> 
> [   28.111816] Caused by (from MCSR=10008): Bus - Read Data Bus Error
> [   28.117998] Oops: Machine check, sig: 7 [#1]
> [   28.122263] P1010 RDB
> [   28.124529] Modules linked in: linux_bcm_knet(PO) linux_user_bde(PO) 
> linux_kernel_bde(PO)
> [   28.132718] CPU: 0 PID: 470 Comm: emxp2_hw_bl Tainted: P   O
> 4.1.38+ #49
> [   28.140376] task: db16cd10 ti: df128000 task.ti: df128000
> [   28.145770] NIP:  LR: 10a4e404 CTR: 10046c38
> [   28.150730] REGS: df129f10 TRAP: 0204   Tainted: P   O 
> (4.1.38+)
> [   28.157776] MSR: 0002d000   CR: 44002428  XER: 
> [   28.164140] DEAR: b7187000 ESR: 
> GPR00: 10a4e404 bf86ea30 b7ca94a0 132f9fa8 07006000 0700  132f9fd8
> GPR08: b7149000 b7159000 0003e000 bf86ea20 24004424 11d6cf7c  
> GPR16: 10f6e29c 10f6c872 10f6db01 b541 b541 11d92fcc 0011 0001
> GPR24: 01a4d12d 132ffbf0 11d6  07006000  132f9fa8 
> [   28.196375] NIP []   (null)
> [   28.199859] LR [10a4e404] 0x10a4e404
> [   28.203426] Call Trace:
> [   28.205866] ---[ end trace f456255ddf9bee83 ]---
> 
> I cannot figure out why NIP is NULL ? It LOOKs like NIP is set to
> MCSRR0 early on but maybe it is lost somehow?
> 
> Anyhow, looking at entry_32.S:
>   .globl  mcheck_transfer_to_handler
> mcheck_transfer_to_handler:
>   mfspr   r0,SPRN_DSRR0
>   stw r0,_DSRR0(r11)
>   mfspr   r0,SPRN_DSRR1
>   stw r0,_DSRR1(r11)
>   /* fall through */
> 
>   .globl  debug_transfer_to_handler
> debug_transfer_to_handler:
>   mfspr   r0,SPRN_CSRR0
>   stw r0,_CSRR0(r11)
>   mfspr   r0,SPRN_CSRR1
>   stw r0,_CSRR1(r11)
>   /* fall through */
> 
>   .globl  crit_transfer_to_handler
> crit_transfer_to_handler:
> 
> It looks odd that DSRRx is assigned in mcheck and CSRRx in debug and
> crit has none. Should not this assigment be shifted down one level?
> 
>   Jocke

Re: [PATCH] sound: soc: fsl: Do not set DAI sysclk when it is equal to system freq

2017-09-05 Thread Łukasz Majewski 

On 09/05/2017 07:20 AM, Nicolin Chen wrote:

On Sun, Sep 03, 2017 at 04:40:21PM +0200, Łukasz Majewski wrote:

/*
* Hardware limitation: The bclk rate must be
* never greater than 1/5 IPG clock rate
*/
if (freq * 5 > clk_get_rate(ssi_private->clk)) {
dev_err(cpu_dai->dev, "bitclk > ipgclk/5\n");
return -EINVAL;
}



Unfortunately not.

This is the part of fsl_ssi_set_bclk() function which is called after
fsl_ssi_set_dai_sysclk() (which sets ssi_private->bitclk_freq = freq;).

Before the aforementioned check we do have:

if (ssi_private->bitclk_freq)
freq = ssi_private->bitclk_freq;
else
freq = params_channels(hw_params) * 32 *
params_rate(hw_params);


Which assigns freq = bitclk_freq (66 MHz)


[...]

And then we break on this particular check:
66MHz * 5 > 66 MHz.

[...]

Does the check fail and return an error here, or not?


Yes, this check fails and return error (-EINVAL).




The culprit IMHO is the  ssi_private->bitclk_freq = freq; in the
fsl_ssi_set_dai_sysclk(), since we _should_ set SSI's IP block clock
(ssi_private->clk), not the bit clock (BCLK).


No. We should not set the IP block clock. That's from IPG bus
on certain IMX SoCs. Setting it might change IPG bus clocks
which might break the system.

And apparently, we shouldn't set bitclk to 66MHz either. Can
you help to find where this 66MHz comes from?


OK.

The fsi_ssi.c file defines:

ops->set_sysclk() routine:

.set_sysclk = fsl_ssi_set_dai_sysclk,

This routine is called in:
int snd_soc_dai_set_sysclk() @ soc-core.c

which is called in two places for my setup:

1. asoc_simple_card_hw_params() @ simple-card.c

and

2. int asoc_simple_card_init_dai() @ simple-card-utils.c

In this function (point 2.) the

simple_dai->sysclk is set and:

snd_soc_dai_set_sysclk(dai, 0, simple_dai->sysclk, 0)

which sets frequency to 66 MHz [*].



The asoc_simple_card_init_dai() is called in

asoc_simple_card_dai_init() @ simple-card.c

which is assigned to dai_link->init

dai_link->init   = asoc_simple_card_dai_init; @ simple_card.c



And the sysclk itself is defined at:
-

dai_props->codec_dai->sysclk, which is used at:

asoc_simple_card_startup(), asoc_simple_card_shutdown() and others 
functions at simple-card.c



It is setup at:

asoc_simple_card_parse_clk() @ simple-card-utils.c from macro:
#define asoc_simple_card_parse_clk_cpu()


And the problem is:
---

At the
asoc_simple_card_parse_clk()
we finally go to dts node:

/soc/aips-bus@0210/i2c@021a/tfa9879@6C

which has clock from I2C (66 MHz).

(So the [*] hack may help here, since it is checked before checking the 
i2c node).



Note:
[*] - I could workaround this problem by setting:

system-clock-frequency = <0> in

dailink_master: cpu {
sound-dai = <>;
};

but this is IMHO even worse hack than this patch.




This patch just quits early if it detects change, which don't need to be
done.


I kinda see your point is to error out before hw_params(). So
I feel it would be better to have a similar 5-times-check in
the set_sysclk() too, if it's really necessary.

Btw, I don't see simple card calling set_sysclk() function in
any earlier stage than hw_params(). I am wondering how did you
encounter the problem?

Thanks
Nicolin




--
Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de

Re: [PATCH] sound: soc: fsl: Do not set DAI sysclk when it is equal to system freq

2017-09-05 Thread Łukasz Majewski 

On 09/05/2017 09:52 AM, Nicolin Chen wrote:

On Tue, Sep 05, 2017 at 09:37:43AM +0200, Łukasz Majewski wrote:


The last call is changing the bit clock (BCLK) frequency to SSI's IP
block clock (ipg = 66 MHz) [1].


I think a bigger question here is why the routine sets BCLK to 66MHz.


Yes, exactly.

In my case the bclk is set to ipg clock, which is the SSI IP block clock
(ipg).


Can you elaborate why you set ipg clock as bclk? I don't remember SSI could
derive bitclock from ipg clock.


Just to be clear:

What clock shall be set with:

struct snd_soc_dai_ops {

int (*set_sysclk)(struct snd_soc_dai *dai,
int clk_id, unsigned int freq, int dir);
}

callback?

The SSI IP block or BCLK ?





This is wrong, since IMX SSI block requires the I2S BCLK to be less
than 1/5 of [1].

As a result the driver initialization passes without any errors, but the
speaker-test test case breaks.

This commit checks if the fsl_ssi_set_dai_sysclk() frequency passed is
not equal to [1].


I don't feel it's quite comprehensive...what if it's being set to 67MHz.


I think that this clock is not changing for the SoC. It should be 66 MHz
fixed.


What I mean is that we cannot just look at this SoC. Today is 66MHz for this
SoC. Tomorrow could be 133MHz for another one. We should put a check that none
of these shall pass -- the 1/5 limit.



Ok. Good point.


--
Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de

Re: [PATCH 2/2] powerpc/powernv/npu: Don't explicitly flush nmmu tlb

2017-09-05 Thread Frederic Barrat 



Le 05/09/2017 à 05:57, Alistair Popple a écrit :

The nest mmu required an explicit flush as a tlbi would not flush it in the
same way as the core. However an alternate firmware fix exists which should
eliminate the need for this flush, so instead add a device-tree property
(ibm,nmmu-flush) on the NVLink2 PHB to enable it only if required.

Signed-off-by: Alistair Popple 
---
  arch/powerpc/platforms/powernv/npu-dma.c | 28 +++-
  arch/powerpc/platforms/powernv/pci.h |  3 +++
  2 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/npu-dma.c 
b/arch/powerpc/platforms/powernv/npu-dma.c
index 2fff9a65..4b4fcac 100644
--- a/arch/powerpc/platforms/powernv/npu-dma.c
+++ b/arch/powerpc/platforms/powernv/npu-dma.c
@@ -395,6 +395,7 @@ struct npu_context {
struct pci_dev *npdev[NV_MAX_NPUS][NV_MAX_LINKS];
struct mmu_notifier mn;
struct kref kref;
+   bool nmmu_flush;

/* Callback to stop translation requests on a given GPU */
struct npu_context *(*release_cb)(struct npu_context *, void *);
@@ -545,11 +546,13 @@ static void mmio_invalidate(struct npu_context 
*npu_context, int va,
struct mmio_atsd_reg mmio_atsd_reg[NV_MAX_NPUS];
unsigned long pid = npu_context->mm->context.id;

-   /*
-* Unfortunately the nest mmu does not support flushing specific
-* addresses so we have to flush the whole mm.
-*/
-   flush_all_mm(npu_context->mm);
+   if (npu_context->nmmu_flush)
+   /*
+* Unfortunately the nest mmu does not support flushing specific
+* addresses so we have to flush the whole mm once before
+* shooting down the GPU translation.
+*/
+   flush_all_mm(npu_context->mm);

/*
 * Loop over all the NPUs this process is active on and launch
@@ -722,6 +725,16 @@ struct npu_context *pnv_npu2_init_context(struct pci_dev 
*gpdev,
return ERR_PTR(-ENODEV);
npu_context->npdev[npu->index][nvlink_index] = npdev;

+   if (!nphb->npu.nmmu_flush) {
+   /*
+* If we're not explicitly flushing ourselves we need to mark
+* the thread for global flushes
+*/
+   npu_context->nmmu_flush = false;
+   inc_mm_active_cpus(mm);



I can see that you use the inc_mm_active_cpus()/dec_mm_active_cpus() and 
not the new mm_context_add_copro()/mm_context_remove_copro(). The only 
difference is that mm_context_remove_copro() adds an extra flush of the 
full mm (pwc and tlb) before decrementing the active cpu count.


But don't you need the flush when releasing the context? As soon as the 
active cpu count is decremented, the next TLBI may be local and not 
visible to the nMMU, so the nMMU could keep obsolete/wrong entries in 
its cache.


  Fred




+   } else
+   npu_context->nmmu_flush = true;
+
return npu_context;
  }
  EXPORT_SYMBOL(pnv_npu2_init_context);
@@ -731,6 +744,9 @@ static void pnv_npu2_release_context(struct kref *kref)
struct npu_context *npu_context =
container_of(kref, struct npu_context, kref);

+   if (!npu_context->nmmu_flush)
+   dec_mm_active_cpus(npu_context->mm);
+
npu_context->mm->context.npu_context = NULL;
mmu_notifier_unregister(_context->mn,
npu_context->mm);
@@ -819,6 +835,8 @@ int pnv_npu2_init(struct pnv_phb *phb)
static int npu_index;
uint64_t rc = 0;

+   phb->npu.nmmu_flush =
+   of_property_read_bool(phb->hose->dn, "ibm,nmmu-flush");
for_each_child_of_node(phb->hose->dn, dn) {
gpdev = pnv_pci_get_gpu_dev(get_pci_dev(dn));
if (gpdev) {
diff --git a/arch/powerpc/platforms/powernv/pci.h 
b/arch/powerpc/platforms/powernv/pci.h
index a95273c..22025c6 100644
--- a/arch/powerpc/platforms/powernv/pci.h
+++ b/arch/powerpc/platforms/powernv/pci.h
@@ -187,6 +187,9 @@ struct pnv_phb {

/* Bitmask for MMIO register usage */
unsigned long mmio_atsd_usage;
+
+   /* Do we need to explicitly flush the nest mmu? */
+   bool nmmu_flush;
} npu;

  #ifdef CONFIG_CXL_BASE

Re: [PATCH] sound: soc: fsl: Do not set DAI sysclk when it is equal to system freq

2017-09-05 Thread Nicolin Chen 
On Tue, Sep 05, 2017 at 09:37:43AM +0200, Łukasz Majewski wrote:

> >>The last call is changing the bit clock (BCLK) frequency to SSI's IP
> >>block clock (ipg = 66 MHz) [1].
> >
> >I think a bigger question here is why the routine sets BCLK to 66MHz.
> 
> Yes, exactly.
> 
> In my case the bclk is set to ipg clock, which is the SSI IP block clock
> (ipg).

Can you elaborate why you set ipg clock as bclk? I don't remember SSI could
derive bitclock from ipg clock.

> >>This is wrong, since IMX SSI block requires the I2S BCLK to be less
> >>than 1/5 of [1].
> >>
> >>As a result the driver initialization passes without any errors, but the
> >>speaker-test test case breaks.
> >>
> >>This commit checks if the fsl_ssi_set_dai_sysclk() frequency passed is
> >>not equal to [1].
> >
> >I don't feel it's quite comprehensive...what if it's being set to 67MHz.
> 
> I think that this clock is not changing for the SoC. It should be 66 MHz
> fixed.

What I mean is that we cannot just look at this SoC. Today is 66MHz for this
SoC. Tomorrow could be 133MHz for another one. We should put a check that none
of these shall pass -- the 1/5 limit.

Re: [PATCH] sound: soc: fsl: Do not set DAI sysclk when it is equal to system freq

2017-09-05 Thread Łukasz Majewski 

On 09/05/2017 07:06 AM, Nicolin Chen wrote:

On Sun, Sep 03, 2017 at 01:05:01PM +0200, Lukasz Majewski wrote:

The problem is visible in the following setup (on the imx6q):
"simple-audio-card" -> ssi2 -> I2S + I2C -> codec

The function call log (simple-card probe -> CONFIG_SND_SIMPLE_CARD):

asoc_simple_card_init_dai() @ sound/soc/generic/simple-card-utils.c
snd_soc_dai_set_sysclk()
fsl_ssi_set_dai_sysclk() @ sound/soc/fsl/fsl_ssi.c

The last call is changing the bit clock (BCLK) frequency to SSI's IP
block clock (ipg = 66 MHz) [1].


I think a bigger question here is why the routine sets BCLK to 66MHz.


Yes, exactly.

In my case the bclk is set to ipg clock, which is the SSI IP block clock 
(ipg).





This is wrong, since IMX SSI block requires the I2S BCLK to be less
than 1/5 of [1].

As a result the driver initialization passes without any errors, but the
speaker-test test case breaks.

This commit checks if the fsl_ssi_set_dai_sysclk() frequency passed is
not equal to [1].


I don't feel it's quite comprehensive...what if it's being set to 67MHz.


I think that this clock is not changing for the SoC. It should be 66 MHz 
fixed.




Thanks
Nicolin


Signed-off-by: Lukasz Majewski 
---
  sound/soc/fsl/fsl_ssi.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index 173cb84..1186fa9 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -809,6 +809,8 @@ static int fsl_ssi_set_dai_sysclk(struct snd_soc_dai 
*cpu_dai,
int clk_id, unsigned int freq, int dir)
  {
struct fsl_ssi_private *ssi_private = snd_soc_dai_get_drvdata(cpu_dai);
+   if (clk_get_rate(ssi_private->clk) == freq)
+   return 0;
  
  	ssi_private->bitclk_freq = freq;
  
--

2.1.4






--
Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de

oops: arch_update_cpu_topology -> lockdep_assert_cpus_held

2017-09-05 Thread Daniel Black 

4.13-rc3 mainline at b1b6f83ac938d176742c85757960dec2cf10e468

Backtrace occurred for every CPU during boot.

its a kvm host machine.


[ 254.131688] new mount options do not match the existing superblock,
will be ignored
[ 255.654622] [ cut here ]
[ 255.654658] WARNING: CPU: 1 PID: 14 at kernel/cpu.c:240
lockdep_assert_cpus_held+0x54/0x80
[ 255.654661] Modules linked in: bridge stp llc binfmt_misc kvm_hv kvm
leds_powernv vmx_crypto powernv_rng rng_core led_class powernv_op_panel
autofs4 xfs btrfs lzo_compress raid456 async_raid6_recov async_memcpy
async_pq async_xor async_tx xor raid6_pq libcrc32c multipath mlx4_en
raid10 crc32c_vpmsum lpfc be2net crc_t10dif crct10dif_generic
crct10dif_common mlx4_core
[ 255.654734] CPU: 1 PID: 14 Comm: cpuhp/1 Tainted: G W 4.13.0 #1
[ 255.654737] task: c01ff25fa100 task.stack: c01ff2578000
[ 255.654740] NIP: c00f8624 LR: c00f8618 CTR:
c01763e0
[ 255.654742] REGS: c01ff257b780 TRAP: 0700 Tainted: G W (4.13.0)
[ 255.654744] MSR: 90029033 
[ 255.654764] CR: 24200422 XER: 
[ 255.654766] CFAR: c01783d0 SOFTE: 1
GPR00: c00f8618 c01ff257ba00 c1042100 
GPR04:  0001  
GPR08: 001ffe49   c07fee782f50
GPR12:  cfd80500 c012c878 c01ff6209180
GPR16:   c0ef2728 
GPR20:   0001 
GPR24:  c1087d50 c0fe0e7d c01448c0
GPR28: 001ffe49  0002 c1088050
[ 255.654842] NIP [c00f8624] lockdep_assert_cpus_held+0x54/0x80
[ 255.654845] LR [c00f8618] lockdep_assert_cpus_held+0x48/0x80
[ 255.654847] Call Trace:
[ 255.654851] [c01ff257ba00] [c00f8618]
lockdep_assert_cpus_held+0x48/0x80 (unreliable)
[ 255.654858] [c01ff257ba20] [c007a848]
arch_update_cpu_topology+0x18/0x30
[ 255.654864] [c01ff257ba40] [c016bd0c]
partition_sched_domains+0x8c/0x4e0
[ 255.654870] [c01ff257baf0] [c020a914]
cpuset_update_active_cpus+0x24/0x60
[ 255.654876] [c01ff257bb10] [c01449bc]
sched_cpu_deactivate+0xfc/0x1a0
[ 255.654882] [c01ff257bc20] [c00f5a8c]
cpuhp_invoke_callback+0x19c/0xe00
[ 255.654888] [c01ff257bcb0] [c00f6868]
cpuhp_down_callbacks+0x78/0xf0
[ 255.654893] [c01ff257bd00] [c00f6c1c]
cpuhp_thread_fun+0x1fc/0x210
[ 255.654899] [c01ff257bd50] [c0132f4c]
smpboot_thread_fn+0x2fc/0x370
[ 255.654905] [c01ff257bdc0] [c012ca24] kthread+0x1b4/0x1c0
[ 255.654911] [c01ff257be30] [c000bcec]
ret_from_kernel_thread+0x5c/0x70
[ 255.654914] Instruction dump:
[ 255.654919] 4e800020 6000 6042 7c0802a6 3c62ffeb 3880
38639280 f8010030
[ 255.654934] 4807fd45 6000 2fa3 409e0020 <0fe0> e8010030
38210020 7c0803a6
[ 255.654950] ---[ end trace 06efa323f571f14b ]---
[ 255.894356] [ cut here ]
[ 255.894367] WARNING: CPU: 2 PID: 20 at kernel/cpu.c:240
lockdep_assert_cpus_held+0x54/0x80
[ 255.894370] Modules linked in: bridge stp llc binfmt_misc kvm_hv kvm
leds_powernv vmx_crypto powernv_rng rng_core led_class powernv_op_panel
autofs4 xfs btrfs lzo_compress raid456 async_raid6_recov async_memcpy
async_pq async_xor async_tx xor raid6_pq libcrc32c multipath mlx4_en
raid10 crc32c_vpmsum lpfc be2net crc_t10dif crct10dif_generic
crct10dif_common mlx4_core
[ 255.894438] CPU: 2 PID: 20 Comm: cpuhp/2 Tainted: G W 4.13.0 #1
[ 255.894441] task: c01ff25e8a00 task.stack: c01ff270
[ 255.89] NIP: c00f8624 LR: c00f8618 CTR:
c0146650
[ 255.894447] REGS: c01ff2703780 TRAP: 0700 Tainted: G W (4.13.0)
[ 255.894448] MSR: 90029033 
[ 255.894468] CR: 24200422 XER: 
[ 255.894470] CFAR: c01783d0 SOFTE: 1
GPR00: c00f8618 c01ff2703a00 c1042100 
GPR04:  0006 c01fe60ec000 0001
GPR08:    c07fe9acf550
GPR12:  cfd80a00 c012c878 c01ff6209180
GPR16:   c0ef2728 
GPR20:   0001 
GPR24:  c1087d50 c0fe0e7d c01448c0
GPR28: 001ffe4d  0004 c1088050
[ 255.894543] NIP [c00f8624] lockdep_assert_cpus_held+0x54/0x80
[ 255.894546] LR [c00f8618] lockdep_assert_cpus_held+0x48/0x80
[ 255.894549] Call Trace:
[ 255.894552] [c01ff2703a00] [c00f8618]
lockdep_assert_cpus_held+0x48/0x80 (unreliable)
[ 255.894560] [c01ff2703a20] [c007a848]

4.13.0-next-20170904 WARNING: possible circular locking dependency detected

2017-09-05 Thread Daniel Black 

found on booting.

kvm bare metal host machine.

==
WARNING: possible circular locking dependency detected
4.13.0-next-20170904 #1 Tainted: GW
--
ppc64_cpu/4940 is trying to acquire lock:
 ((complete)>done){+.+.}, at: []
takedown_cpu+0xe8/0x1c0

but task is already holding lock:
 (sparse_irq_lock){+.+.}, at: [] irq_lock_sparse+0x2c/0x40

which lock already depends on the new lock.


the existing dependency chain (in reverse order) is:

-> #2 (sparse_irq_lock){+.+.}:
   lock_acquire+0xdc/0x2f0
   __mutex_lock+0xac/0xb60
   irq_lock_sparse+0x2c/0x40
   irq_affinity_online_cpu+0x2c/0x1f0
   cpuhp_invoke_callback+0x1b4/0xd90
   cpuhp_up_callbacks+0x58/0x150
   cpuhp_thread_fun+0x1cc/0x210
   smpboot_thread_fn+0x2fc/0x370
   kthread+0x200/0x210
   ret_from_kernel_thread+0x5c/0x70

-> #1 (cpuhp_state){+.+.}:
   smpboot_thread_fn+0x2fc/0x370
   kthread+0x200/0x210
   ret_from_kernel_thread+0x5c/0x70

-> #0 ((complete)>done){+.+.}:
   __lock_acquire+0x16a4/0x17f0
   lock_acquire+0xdc/0x2f0
   wait_for_common+0x8c/0x250
   takedown_cpu+0xe8/0x1c0
   cpuhp_invoke_callback+0x1b4/0xd90
   cpuhp_down_callbacks+0x78/0xf0
   _cpu_down+0x150/0x1b0
   do_cpu_down+0x68/0xb0
   cpu_subsys_offline+0x2c/0x50
   device_offline+0xcc/0x150
   online_store+0x6c/0xc0
   dev_attr_store+0x68/0xa0
   sysfs_kf_write+0x74/0xc0
   kernfs_fop_write+0x194/0x270
   __vfs_write+0x6c/0x1d0
   vfs_write+0xd8/0x240
   SyS_write+0x6c/0x110
   system_call+0x58/0x6c

other info that might help us debug this:

Chain exists of:
  (complete)>done --> cpuhp_state --> sparse_irq_lock

 Possible unsafe locking scenario:

   CPU0CPU1
   
  lock(sparse_irq_lock);
   lock(cpuhp_state);
   lock(sparse_irq_lock);
  lock((complete)>done);

 *** DEADLOCK ***

8 locks held by ppc64_cpu/4940:
 #0:  (sb_writers#6){.+.+}, at: [] vfs_write+0x210/0x240
 #1:  (>mutex){+.+.}, at: []
kernfs_fop_write+0x148/0x270
 #2:  (kn->count#130){.+.+}, at: []
kernfs_fop_write+0x154/0x270
 #3:  (device_hotplug_lock){+.+.}, at: []
lock_device_hotplug_sysfs+0x28/0x80
 #4:  (>mutex){}, at: []
device_offline+0x84/0x150
 #5:  (cpu_add_remove_lock){+.+.}, at: []
do_cpu_down+0x44/0xb0
 #6:  (cpu_hotplug_lock.rw_sem){}, at: []
percpu_down_write+0x4c/0x160
 #7:  (sparse_irq_lock){+.+.}, at: []
irq_lock_sparse+0x2c/0x40

stack backtrace:
CPU: 2 PID: 4940 Comm: ppc64_cpu Tainted: GW
4.13.0-next-20170904 #1
Call Trace:
[c01fb5d83510] [c0bcb7ec] dump_stack+0xe8/0x15c (unreliable)
[c01fb5d83550] [c0199478] print_circular_bug+0x2e8/0x4f0
[c01fb5d83600] [c019ab58] check_prev_add+0x468/0xa90
[c01fb5d83700] [c019e684] __lock_acquire+0x16a4/0x17f0
[c01fb5d83830] [c019f47c] lock_acquire+0xdc/0x2f0
[c01fb5d83900] [c0be7dcc] wait_for_common+0x8c/0x250
[c01fb5d83980] [c010e198] takedown_cpu+0xe8/0x1c0
[c01fb5d839e0] [c010e5b4] cpuhp_invoke_callback+0x1b4/0xd90
[c01fb5d83a70] [c010f318] cpuhp_down_callbacks+0x78/0xf0
[c01fb5d83ac0] [c0112c30] _cpu_down+0x150/0x1b0
[c01fb5d83b20] [c0110598] do_cpu_down+0x68/0xb0
[c01fb5d83b60] [c07a54ac] cpu_subsys_offline+0x2c/0x50
[c01fb5d83b90] [c079c37c] device_offline+0xcc/0x150
[c01fb5d83bd0] [c079c58c] online_store+0x6c/0xc0
[c01fb5d83c20] [c0796aa8] dev_attr_store+0x68/0xa0
[c01fb5d83c60] [c04bd954] sysfs_kf_write+0x74/0xc0
[c01fb5d83ca0] [c04bc1d4] kernfs_fop_write+0x194/0x270
[c01fb5d83cf0] [c03ec6bc] __vfs_write+0x6c/0x1d0
[c01fb5d83d90] [c03ee718] vfs_write+0xd8/0x240
[c01fb5d83de0] [c03f06ac] SyS_write+0x6c/0x110
[c01fb5d83e30] [c000b96c] system_call+0x58/0x6c
[  OK  ] Started User Manager for UID 1004.
nf_conntrack version 0.5.0 (65536 buckets, 262144 max)
ip_tables: (C) 2000-2006 Netfilter Core Team

Ubuntu 16.04.2 LTS p87 hvc0

p87 login: Initializing XFRM netlink socket
[ cut here ]
WARNING: CPU: 48 PID: 5464 at net/netfilter/core.c:218
__nf_hook_entries_try_shrink+0xd4/0x270
Modules linked in: ipt_MASQUERADE nf_nat_masquerade_ipv4 xfrm_user
xfrm_algo iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4
xt_addrtype iptable_filter ip_tables xt_conntrack x_tables nf_nat
nf_conntrack overlay bridge stp llc binfmt_misc vmx_crypto leds_powernv
led_class powernv_op_panel powernv_rng rng_core kvm_hv kvm autofs4 xfs
btrfs zstd_decompress zstd_compress xxhash lzo_compress raid456
async_raid6_recov async_memcpy async_pq async_xor async_tx xor raid6_pq
libcrc32c multipath mlx4_en crc32c_vpmsum raid10 lpfc be2net crc_t10dif