Re: [PATCH v2 14/14] Kconfig cleanup (PARPORT_PC dependencies)

2013-10-08 Thread Ralf Baechle
On Tue, Oct 08, 2013 at 01:10:30AM -0400, Mark Salter wrote:

 Remove messy dependencies from PARPORT_PC by having it depend on one
 Kconfig symbol (ARCH_MAY_HAVE_PC_PARPORT) and having architectures
 which need it, select ARCH_MAY_HAVE_PC_PARPORT in arch/*/Kconfig.
 New architectures are unlikely to need PARPORT_PC, so this avoids
 having an ever growing list of architectures to exclude. Those
 architectures which do select ARCH_MAY_HAVE_PC_PARPORT in this
 patch are the ones which have an asm/parport.h (or use the generic
 version).

Acked-by: Ralf Baechle r...@linux-mips.org

  Ralf
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH] powerpc, perf: Configure BHRB filter before enabling PMU interrupts

2013-10-08 Thread Anshuman Khandual
On 10/08/2013 09:51 AM, Michael Ellerman wrote:
 On Mon, Oct 07, 2013 at 10:00:26AM +0530, Anshuman Khandual wrote:
 Right now the `config_bhrb` PMU specific call happens after write_mmcr0
 which actually enables the PMU for event counting and interrupt. So
 there is a small window of time where the PMU and BHRB runs without the
 required HW branch filter (if any) enabled in BHRB. This can cause some
 of the branch samples to be collected through BHRB without any filter
 being applied and hence affecting the correctness of the results. This
 patch moves the BHRB config function call before enabling the interrupts.
 
 Patch looks good.
 
 But it reminds me I have an item in my TODO list:
  - Why can't config_bhrb() be done in compute_mmcr() ?
 

compute_mmcr() function deals with generic MMCR* configs for normal PMU
events. Even if BHRB config touches MMCRA register, it's configuration
does not interfere with the PMU config for general events. So its best
to keep them separate. Besides, we can always look at these code consolidation
issues in future. But this patch solves a problem which is happening right now.

Regards
Anshuman

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH RFC 00/77] Re-design MSI/MSI-X interrupts enablement pattern

2013-10-08 Thread Alexander Gordeev
On Tue, Oct 08, 2013 at 03:33:30PM +1100, Michael Ellerman wrote:
 On Wed, Oct 02, 2013 at 12:29:04PM +0200, Alexander Gordeev wrote:
  This technique proved to be confusing and error-prone. Vast share
  of device drivers simply fail to follow the described guidelines.
 
 To clarify Vast share of device drivers:
 
  - 58 drivers call pci_enable_msix()
  - 24 try a single allocation and then fallback to MSI/LSI
  - 19 use the loop style allocation as above
  - 14 try an allocation, and if it fails retry once
  - 1  incorrectly continues when pci_enable_msix() returns  0
 
 So 33 drivers ( 50%) successfully make use of the confusing and
 error-prone return value.

Ok, you caught me - 'vast share' is incorrect and is a subject to
rewording. But out of 19/58 how many drivers tested fallbacks on the
real hardware? IOW, which drivers are affected by the pSeries quota?

 And yes, one is buggy, so obviously the interface is too complex. Thanks
 drivers/ntb/ntb_hw.c

vmxnet3 would be the best example.

 cheers

-- 
Regards,
Alexander Gordeev
agord...@redhat.com
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


[PATCH] powerpc/powernv: Add a debugfs file to read the firmware console

2013-10-08 Thread Benjamin Herrenschmidt
With OPALv3, the firmware can provide the address of it's internal console
to Linux, which we can then display using debugfs. This is handy for
diagnostics and debugging.

Signed-off-by: Benjamin Herrenschmidt b...@kernel.crashing.org
---

diff --git a/arch/powerpc/platforms/powernv/opal.c 
b/arch/powerpc/platforms/powernv/opal.c
index 2911abe..10d7894 100644
--- a/arch/powerpc/platforms/powernv/opal.c
+++ b/arch/powerpc/platforms/powernv/opal.c
@@ -17,6 +17,8 @@
 #include linux/interrupt.h
 #include linux/notifier.h
 #include linux/slab.h
+#include linux/debugfs.h
+#include linux/uaccess.h
 #include asm/opal.h
 #include asm/firmware.h
 
@@ -27,6 +29,21 @@ struct opal {
u64 entry;
 } opal;
 
+/* OPAL in-memory console */
+struct memcons {
+   uint64_t magic;
+#define MEMCONS_MAGIC  0x6630696567726173
+   uint64_t obuf_phys;
+   uint64_t ibuf_phys;
+   uint32_t obuf_size;
+   uint32_t ibuf_size;
+   uint32_t out_pos;
+#define MEMCONS_OUT_POS_WRAP   0x8000u
+#define MEMCONS_OUT_POS_MASK   0x00ffu
+   uint32_t in_prod;
+   uint32_t in_cons;
+};
+
 static struct device_node *opal_node;
 static DEFINE_SPINLOCK(opal_write_lock);
 extern u64 opal_mc_secondary_handler[];
@@ -369,6 +386,90 @@ static irqreturn_t opal_interrupt(int irq, void *data)
return IRQ_HANDLED;
 }
 
+#ifdef CONFIG_DEBUG_FS
+static ssize_t opal_memcons_read(struct file *file, char __user *to,
+size_t count, loff_t *ppos)
+{
+   struct memcons *mc = file-private_data;
+   size_t available, ret, chunk0, chunk1, lcount;
+   const char *start, *conbuf = __va(mc-obuf_phys);
+   loff_t opos, pos;
+
+   /*
+* Find out how much is in the buffer. If it has wrapped
+* the whole buffer, else just the beginning. It has wrapped
+* if the next character is not \0
+*/
+   if (mc-out_pos  MEMCONS_OUT_POS_WRAP) {
+   available = mc-obuf_size;
+   chunk1 = mc-out_pos  MEMCONS_OUT_POS_MASK;
+   start = conbuf + chunk1;
+   chunk0 = mc-obuf_size - chunk1;
+   } else {
+   available = mc-out_pos;
+   start = conbuf;
+   chunk0 = available;
+   chunk1 = 0;
+   }
+
+   opos = pos = *ppos;
+
+   /* Sanity check arguments */
+   if (pos  0)
+   return -EINVAL;
+   if (pos = available || !count)
+   return 0;
+   if (count  available - pos)
+   count = available - pos;
+
+   /* Handle first chunk */
+   if (pos  chunk0) {
+   lcount = min(count, chunk0 - (size_t)pos);
+   ret = copy_to_user(to, start + pos, lcount);
+   if (ret == lcount)
+   return -EFAULT;
+   lcount -= ret;
+   count -= lcount;
+   *ppos += lcount;
+   to += lcount;
+   pos = 0;
+   } else {
+   *ppos += chunk0;
+   pos -= chunk0;
+   ret = 0;
+   }
+
+   /* Handle second chunk */
+   if (count  chunk1  ret == 0) {
+   lcount = min(count, chunk1 - (size_t)pos);
+   ret = copy_to_user(to, conbuf + pos, lcount);
+   if (ret == lcount)
+   return -EFAULT;
+   lcount -= ret;
+   *ppos += lcount;
+   }
+   return *ppos - opos;
+
+}
+
+static const struct file_operations opal_fops_memcons = {
+   .read = opal_memcons_read,
+   .open = simple_open,
+   .llseek =   default_llseek,
+};
+
+static void opal_init_debugfs(void)
+{
+   u64 mcaddr;
+
+   if (of_property_read_u64(opal_node, ibm,opal-memcons, mcaddr) == 0)
+   debugfs_create_file(opal-log, 0400, powerpc_debugfs_root,
+   __va(mcaddr), opal_fops_memcons);
+}
+#else
+static void opal_init_debugfs(void) { }
+#endif /* CONFIG_DEBUG_FS */
+
 static int __init opal_init(void)
 {
struct device_node *np, *consoles;
@@ -414,6 +515,9 @@ static int __init opal_init(void)
(0x%x)\n, rc, irq, hwirq);
opal_irqs[i] = irq;
}
+
+   opal_init_debugfs();
+
return 0;
 }
 subsys_initcall(opal_init);


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH RFC 07/77] PCI/MSI: Re-design MSI/MSI-X interrupts enablement pattern

2013-10-08 Thread Alexander Gordeev
On Mon, Oct 07, 2013 at 02:17:49PM -0400, Tejun Heo wrote:
 Hello,
 
 On Wed, Oct 02, 2013 at 12:48:23PM +0200, Alexander Gordeev wrote:
  +static int foo_driver_enable_msi(struct foo_adapter *adapter, int nvec)
  +{
  +   rc = pci_get_msi_cap(adapter-pdev);
  +   if (rc  0)
  +   return rc;
  +
  +   nvec = min(nvec, rc);
  +   if (nvec  FOO_DRIVER_MINIMUM_NVEC) {
  +   return -ENOSPC;
  +
  +   rc = pci_enable_msi_block(adapter-pdev, nvec);
  +   return rc;
  +}
 
 If there are many which duplicate the above pattern, it'd probably be
 worthwhile to provide a helper?  It's usually a good idea to reduce
 the amount of boilerplate code in drivers.

I wanted to limit discussion in v1 to as little changes as possible.
I 'planned' those helper(s) for a separate effort if/when the most
important change is accepted and soaked a bit.

  @@ -975,7 +951,7 @@ int pci_enable_msix(struct pci_dev *dev, struct 
  msix_entry *entries, int nvec)
  if (nr_entries  0)
  return nr_entries;
  if (nvec  nr_entries)
  -   return nr_entries;
  +   return -EINVAL;
   
  /* Check for any invalid entries */
  for (i = 0; i  nvec; i++) {
 
 If we do things this way, it breaks all drivers using this interface
 until they're converted, right?

Right. And the rest of the series does it.

 Also, it probably isn't the best idea
 to flip the behavior like this as this can go completely unnoticed (no
 compiler warning or anything, the same function just behaves
 differently).  Maybe it'd be a better idea to introduce a simpler
 interface that most can be converted to?

Well, an *other* interface is a good idea. What do you mean with the
simpler here?

 tejun

-- 
Regards,
Alexander Gordeev
agord...@redhat.com
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH RFC 05/77] PCI/MSI: Convert pci_msix_table_size() to a public interface

2013-10-08 Thread Alexander Gordeev
On Mon, Oct 07, 2013 at 02:10:43PM -0400, Tejun Heo wrote:
 On Wed, Oct 02, 2013 at 12:48:21PM +0200, Alexander Gordeev wrote:
  Make pci_msix_table_size() to return a error code if the device
  does not support MSI-X. This update is needed to facilitate a
  forthcoming re-design MSI/MSI-X interrupts enabling pattern.
  
  Device drivers will use this interface to obtain maximum number
  of MSI-X interrupts the device supports and use that value in
  the following call to pci_enable_msix() interface.
 
 Hmmm... I probably missed something but why is this necessary?  To
 discern between -EINVAL and -ENOSPC?  If so, does that really matter?

pci_msix_table_size() is kind of helper and returns 0 if a device does
not have MSI-X table. If we want drivers to use it we need return -EINVAL
for MSI-X incapable/disabled devices. Nothing about -ENOSPC.

 tejun

-- 
Regards,
Alexander Gordeev
agord...@redhat.com
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH RFC 00/77] Re-design MSI/MSI-X interrupts enablement pattern

2013-10-08 Thread Alexander Gordeev
On Mon, Oct 07, 2013 at 02:21:17PM -0400, Tejun Heo wrote:
 Whee that's a lot more than I expected.  I was just scanning
 multiple msi users.  Maybe we can stage the work in more manageable
 steps so that you don't have to go through massive conversion only to
 do it all over again afterwards and likewise people don't get
 bombarded on each iteration?  Maybe we can first update pci / msi code
 proper, msi and then msix?

Multipe MSIs is just a handful of drivers, really. MSI-X impact still
will be huge. But if we opt a different name for the new pci_enable_msix()
then we could first update pci/msi, then drivers (in few stages possibly)
and finally remove the old implementation.

 tejun

-- 
Regards,
Alexander Gordeev
agord...@redhat.com
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH] powerpc/legacy_serial: fix incorrect placement of __initdata tag

2013-10-08 Thread Bartlomiej Zolnierkiewicz
On Tuesday, October 08, 2013 02:56:23 PM Michael Ellerman wrote:
 On Thu, Oct 03, 2013 at 01:51:27PM +0200, Bartlomiej Zolnierkiewicz wrote:
  On Tuesday, October 01, 2013 04:13:25 PM Michael Ellerman wrote:
   On Mon, Sep 30, 2013 at 03:11:42PM +0200, Bartlomiej Zolnierkiewicz wrote:
__initdata tag should be placed between the variable name and equal
sign for the variable to be placed in the intended .init.data section.
   
   I see lots of other occurences of that in arch/powerpc. Why not send a
   single patch to update them all?
  
  The other occurences while not following the preferred kernel coding style
  are (probably) working OK with gcc. This particular occurence just doesn't
  work as it should.
 
 Why would the other occurrences work but not this one?

Because gcc seems to generate the correct code for things like i.e. this one:

struct of_device_id __initdata legacy_serial_parents[]

but not for ones like this:

struct __initdata of_device_id legacy_serial_parents[]

 Regardless, why don't we just do a single patch to clean them all up to
 match coding style and (probably) do what they're intended.

Because:

- fixing this occurence changes runtime, fixing others won't

- there were no such request from powerpc Maintainers

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung RD Institute Poland
Samsung Electronics

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH RFC 00/77] Re-design MSI/MSI-X interrupts enablement pattern

2013-10-08 Thread Alexander Gordeev
On Mon, Oct 07, 2013 at 02:01:11PM -0400, Tejun Heo wrote:
  What about introducing pci_lock_msi() and pci_unlock_msi() and let device
  drivers care about their ranges and specifics in race-safe manner?
  I do not call to introduce it right now (since it appears pSeries has not
  been hitting the race for years) just as a possible alternative to Ben's
  proposal.
 
 I don't think the same race condition would happen with the loop.

We need to distinguish the contexts we're discussing.

If we talk about pSeries quota, then the current pSeries pci_enable_msix()
implementation is racy internally and could fail if the quota went down
*while* pci_enable_msix() is executing. In this case the loop will have to
exit rather than retry with a lower number (what number?).

In this regard the new scheme does not bring anything new and relies on
the fact this race does not hit and therefore does not worry.

If we talk about quota as it has to be, then yes - the loop scheme seems
more preferable.

Overall, looks like we just need to fix the pSeries implementation,
if the guys want it, he-he :)

 The problem case is where multiple msi(x) allocation fails completely
 because the global limit went down before inquiry and allocation.  In
 the loop based interface, it'd retry with the lower number.

I am probably missing something here. If the global limit went down before
inquiry then the inquiry will get what is available and try to allocate with
than number.

-- 
Regards,
Alexander Gordeev
agord...@redhat.com
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Elbc device driver

2013-10-08 Thread Mercier Ivan
Hi,

I'm working on a powerpc qoriq p3041 and trying to communicate with a
device by elbc bus in gpmc mode.

I 've integrated CONFIG_FSL_LBC in Linux which provide the basic functions.

Now I'm wondering how can I do read and write operations on the
bus.Where is mapped my device?

Should I code .read and .write driver functions?How can I start?

How integrates my device in the device tree?



thanks a lot,

Ivan
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH v4 4/4] powerpc/85xx: add sysfs for pw20 state and altivec idle

2013-10-08 Thread Scott Wood
On Mon, 2013-10-07 at 22:58 -0500, Wang Dongsheng-B40534 wrote:
 
  -Original Message-
  From: Wood Scott-B07421
  Sent: Tuesday, October 01, 2013 7:06 AM
  To: Wang Dongsheng-B40534
  Cc: Wood Scott-B07421; Bhushan Bharat-R65777; linuxppc-
  d...@lists.ozlabs.org
  Subject: Re: [PATCH v4 4/4] powerpc/85xx: add sysfs for pw20 state and
  altivec idle
  
  On Sun, 2013-09-29 at 01:57 -0500, Wang Dongsheng-B40534 wrote:
   I think we need to do this:
  
   #define U64_LOW_MASK0xULL
   #define U64_MASK0xULL
  
   u32 tmp_rem;
   u64 ns_u_rem, ns_u, ns_l, ns_l_carry;
   u64 cycle;
  
   ns_u = ns  32;
   ns_l = ns  U64_LOW_MASK;
  
   ns_l *= tb_ticks_per_usec;
   ns_l_carry = ns_l  32;
   ns_u *= tb_ticks_per_usec;
   ns_u += ns_l_carry;
  
   ns_u = div_u64_rem(ns_u, 1000, tmp_rem);
   ns_u_rem = tmp_rem;
   ns_l = (ns_l  U64_LOW_MASK) | ((ns_u_rem)  32);
   ns_l = div_u64(ns_l, 1000);
  
   if (ns_u  32)
   cycle = U64_MASK;
   else
   cycle = (ns_u  32) | (ns_l  U64_LOW_MASK);
  
   I has already tested this code, and works good. :)
  
  Ugh.  I don't think we need to get this complicated (and I'd rather not
  spend the time verifying the correctness of this).
  
  If for some reason we did need something like this in some other context
  (I don't want to see it just for pw20), I'd be more inclined to see
  general 128-bit mult/divide support.
  
 I would like to use my version,:), because it can handle any situation and we 
 do not need to restrict users.

It also would take more time to review than I have to spend on it, not
to mention the impact on anyone in the future that wants to understand
or maintain this code -- all for very unlikely situations (and the
failure in those very unlikely situations is just that we go into PW20
more often than intended).

-Scott



___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 1/7] powerpc: Add interface to get msi region information

2013-10-08 Thread Bjorn Helgaas
On Thu, Oct 3, 2013 at 11:19 PM, Bhushan Bharat-R65777
r65...@freescale.com wrote:

 I don't know enough about VFIO to understand why these new interfaces are
 needed.  Is this the first VFIO IOMMU driver?  I see vfio_iommu_spapr_tce.c 
 and
 vfio_iommu_type1.c but I don't know if they're comparable to the Freescale 
 PAMU.
 Do other VFIO IOMMU implementations support MSI?  If so, do they handle the
 problem of mapping the MSI regions in a different way?

 PAMU is an aperture type of IOMMU while other are paging type, So they are 
 completely different from what PAMU is and handle that differently.

This is not an explanation or a justification for adding new
interfaces.  I still have no idea what an aperture type IOMMU is,
other than that it is different.  But I see that Alex is working on
this issue with you in a different thread, so I'm sure you guys will
sort it out.

Bjorn
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 1/7] powerpc: Add interface to get msi region information

2013-10-08 Thread j...@8bytes.org
On Tue, Oct 08, 2013 at 10:47:49AM -0600, Bjorn Helgaas wrote:
 I still have no idea what an aperture type IOMMU is,
 other than that it is different.

An aperture based IOMMU is basically any GART-like IOMMU which can only
remap a small window (the aperture) of the DMA address space. DMA
outside of that window is either blocked completly or passed through
untranslated.


Joerg


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


RE: [PATCH 1/7] powerpc: Add interface to get msi region information

2013-10-08 Thread Bhushan Bharat-R65777


 -Original Message-
 From: j...@8bytes.org [mailto:j...@8bytes.org]
 Sent: Tuesday, October 08, 2013 10:32 PM
 To: Bjorn Helgaas
 Cc: Bhushan Bharat-R65777; alex.william...@redhat.com; 
 b...@kernel.crashing.org;
 ga...@kernel.crashing.org; linux-ker...@vger.kernel.org; linuxppc-
 d...@lists.ozlabs.org; linux-...@vger.kernel.org; ag...@suse.de; Wood Scott-
 B07421; io...@lists.linux-foundation.org
 Subject: Re: [PATCH 1/7] powerpc: Add interface to get msi region information
 
 On Tue, Oct 08, 2013 at 10:47:49AM -0600, Bjorn Helgaas wrote:
  I still have no idea what an aperture type IOMMU is, other than that
  it is different.
 
 An aperture based IOMMU is basically any GART-like IOMMU which can only remap 
 a
 small window (the aperture) of the DMA address space. DMA outside of that 
 window
 is either blocked completly or passed through untranslated.

It is completely blocked for Freescale PAMU. 
So for this type of iommu what we have to do is to create a MSI mapping just 
after guest physical address, Example: guest have a 512M of memory then we 
create window of 1G (because of power of 2 requirement), then we have to FIT 
MSI just after 512M of guest.
And for that we need
1) to know the physical address of MSI's in interrupt controller (for 
that this patch was all about of).

2) When guest enable MSI interrupt then we write MSI-address and 
MSI-DATA in device. The discussion with Alex Williamson is about that interface.

Thanks
-Bharat

 
 
   Joerg
 
 


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 1/7] powerpc: Add interface to get msi region information

2013-10-08 Thread Scott Wood
On Tue, 2013-10-08 at 10:47 -0600, Bjorn Helgaas wrote:
 On Thu, Oct 3, 2013 at 11:19 PM, Bhushan Bharat-R65777
 r65...@freescale.com wrote:
 
  I don't know enough about VFIO to understand why these new interfaces are
  needed.  Is this the first VFIO IOMMU driver?  I see 
  vfio_iommu_spapr_tce.c and
  vfio_iommu_type1.c but I don't know if they're comparable to the Freescale 
  PAMU.
  Do other VFIO IOMMU implementations support MSI?  If so, do they handle the
  problem of mapping the MSI regions in a different way?
 
  PAMU is an aperture type of IOMMU while other are paging type, So they are 
  completely different from what PAMU is and handle that differently.
 
 This is not an explanation or a justification for adding new
 interfaces.  I still have no idea what an aperture type IOMMU is,
 other than that it is different.  But I see that Alex is working on
 this issue with you in a different thread, so I'm sure you guys will
 sort it out.

PAMU is a very constrained IOMMU that cannot do arbitrary page mappings.
Due to these constraints, we cannot map the MSI I/O page at its normal
address while also mapping RAM at the address we want.  The address we
can map it at depends on the addresses of other mappings, so it can't be
hidden in the IOMMU driver -- the user needs to be in control.

Another difference is that (if I understand correctly) PCs handle MSIs
specially, via interrupt remapping, rather than being translated as a
normal memory access through the IOMMU.

-Scott



___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 1/2][v2] pci: fsl: derive the common PCI driver to drivers/pci/host

2013-10-08 Thread Bjorn Helgaas
[+cc Ben, Paul, linuxppc-dev]

On Mon, Sep 30, 2013 at 04:52:54PM +0800, Minghuan Lian wrote:
 The Freescale's Layerscape series processors will use ARM cores.
 The LS1's PCIe controllers is the same as T4240's. So it's better
 the PCIe controller driver can support PowerPC and ARM
 simultaneously. This patch is for this purpose. It derives
 the common functions from arch/powerpc/sysdev/fsl_pci.c to
 drivers/pci/host/pci-fsl-common.c and leaves the architecture
 specific functions which should be implemented in arch related files.

 Signed-off-by: Minghuan Lian minghuan.l...@freescale.com

I cc'd the powerpc maintainers so we can work out which tree this
should go through.

 ---
 change log:
 v1-v2:
 1. rename pci.h to pci-common.h
 2. rename pci-fsl.c to pci-fsl-common.c

 Based on upstream master.
 Based on the discussion of RFC version here
 http://patchwork.ozlabs.org/patch/274487/

  arch/powerpc/sysdev/fsl_pci.c  | 521 +-
  arch/powerpc/sysdev/fsl_pci.h  |  89 
  .../fsl_pci.c = drivers/pci/host/pci-fsl-common.c | 591 
 +
  .../fsl_pci.h = include/linux/fsl/pci-common.h|  45 +-

Is there any way to avoid putting this file in include/linux?  I know
you want to share it beyond PowerPC, and I know there are similar
examples there already, but this is all arch-specific or
chipset-specific stuff that seems like it should be in some
not-so-public place.  It doesn't seem scalable to add an include/linux
subdirectory for every chipset that might be shared across
architectures.

I assume this patch basically just moves code around, so the only
question I really care about is where it ends up.

Bjorn

  4 files changed, 7 insertions(+), 1239 deletions(-)
  copy arch/powerpc/sysdev/fsl_pci.c = drivers/pci/host/pci-fsl-common.c (54%)
  copy arch/powerpc/sysdev/fsl_pci.h = include/linux/fsl/pci-common.h (79%)

 diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c
 index ccfb50d..26039e3 100644
 --- a/arch/powerpc/sysdev/fsl_pci.c
 +++ b/arch/powerpc/sysdev/fsl_pci.c
 @@ -27,6 +27,7 @@
  #include linux/log2.h
  #include linux/slab.h
  #include linux/uaccess.h
 +#include linux/fsl/pci-common.h

  #include asm/io.h
  #include asm/prom.h
 @@ -58,57 +59,8 @@ static void quirk_fsl_pcie_header(struct pci_dev *dev)
   return;
  }

 -static int fsl_indirect_read_config(struct pci_bus *, unsigned int,
 -int, int, u32 *);
 -
 -static int fsl_pcie_check_link(struct pci_controller *hose)
 -{
 - u32 val = 0;
 -
 - if (hose-indirect_type  PPC_INDIRECT_TYPE_FSL_CFG_REG_LINK) {
 - if (hose-ops-read == fsl_indirect_read_config) {
 - struct pci_bus bus;
 - bus.number = hose-first_busno;
 - bus.sysdata = hose;
 - bus.ops = hose-ops;
 - indirect_read_config(bus, 0, PCIE_LTSSM, 4, val);
 - } else
 - early_read_config_dword(hose, 0, 0, PCIE_LTSSM, val);
 - if (val  PCIE_LTSSM_L0)
 - return 1;
 - } else {
 - struct ccsr_pci __iomem *pci = hose-private_data;
 - /* for PCIe IP rev 3.0 or greater use CSR0 for link state */
 - val = (in_be32(pci-pex_csr0)  PEX_CSR0_LTSSM_MASK)
 -  PEX_CSR0_LTSSM_SHIFT;
 - if (val != PEX_CSR0_LTSSM_L0)
 - return 1;
 - }
 -
 - return 0;
 -}
 -
 -static int fsl_indirect_read_config(struct pci_bus *bus, unsigned int devfn,
 -int offset, int len, u32 *val)
 -{
 - struct pci_controller *hose = pci_bus_to_host(bus);
 -
 - if (fsl_pcie_check_link(hose))
 - hose-indirect_type |= PPC_INDIRECT_TYPE_NO_PCIE_LINK;
 - else
 - hose-indirect_type = ~PPC_INDIRECT_TYPE_NO_PCIE_LINK;
 -
 - return indirect_read_config(bus, devfn, offset, len, val);
 -}
 -
  #if defined(CONFIG_FSL_SOC_BOOKE) || defined(CONFIG_PPC_86xx)

 -static struct pci_ops fsl_indirect_pcie_ops =
 -{
 - .read = fsl_indirect_read_config,
 - .write = indirect_write_config,
 -};
 -
  #define MAX_PHYS_ADDR_BITS 40
  static u64 pci64_dma_offset = 1ull  MAX_PHYS_ADDR_BITS;

 @@ -132,291 +84,6 @@ static int fsl_pci_dma_set_mask(struct device *dev, u64 
 dma_mask)
   return 0;
  }

 -static int setup_one_atmu(struct ccsr_pci __iomem *pci,
 - unsigned int index, const struct resource *res,
 - resource_size_t offset)
 -{
 - resource_size_t pci_addr = res-start - offset;
 - resource_size_t phys_addr = res-start;
 - resource_size_t size = resource_size(res);
 - u32 flags = 0x80044000; /* enable  mem R/W */
 - unsigned int i;
 -
 - pr_debug(PCI MEM resource start 0x%016llx, size 0x%016llx.\n,
 - (u64)res-start, (u64)size);
 -
 - if (res-flags  IORESOURCE_PREFETCH)
 - flags |= 0x1000; /* enable relaxed ordering */
 -
 - for (i = 0; size  0; i++) {
 - unsigned int bits = min(ilog2(size),
 - __ffs(pci_addr | phys_addr));
 -
 - if (index + i = 5)
 - return -1;
 -
 - out_be32(pci-pow[index + i].potar, pci_addr  12);
 - out_be32(pci-pow[index + i].potear, (u64)pci_addr  44);
 - out_be32(pci-pow[index + i].powbar, phys_addr  12);
 - out_be32(pci-pow[index + i].powar, flags | (bits - 1));
 -
 - pci_addr += (resource_size_t)1U  bits;
 - phys_addr += 

Re: [PATCH 5/9][v5] powerpc: implement is_instr_load_store().

2013-10-08 Thread Sukadev Bhattiprolu
Michael Ellerman [mich...@ellerman.id.au] wrote:
| bool is_load_store(int ext_opcode)
| {
| upper = ext_opcode  5;
| lower = ext_opcode  0x1f;
| 
| /* Short circuit as many misses as we can */
| if (lower  3 || lower  23)
| return false;

I see some loads/stores like these which are not covered by
the above check. Is it ok to ignore them ?

lower == 29: ldepx, stdepx, eviddepx, evstddepx

lower == 31: lwepx, lbepx, lfdepx, stfdepx,

Looking through the opcode maps, I also see these for primary
op code 4:

evldd, evlddx, evldwx, evldw, evldh, evldhx.

Should we include those also ?

Sukadev

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: Linux 2.6.32 PowerPC MTD partition mounted at boot

2013-10-08 Thread Scott Wood
On Wed, 2013-09-18 at 22:43 -0400, Dorin D wrote:
 I am working on bringing up two Linux systems, both based on Freescale
 PowerPC devices, one is a MPC8349, the other a P1020. I was able to
 build, install and boot the kernel on both cards. The kernel is 2.6.32
 and the  toolchains  are coming from the LTIBs packages from
 Freescale. 
 
 Both cards have a 32 MByte NOR flash memory (AMD) boot flash. I have
 Uboot, kernel, RAM disk image and DTB in the boot flash and I want to
 use the spare space (about 20 MBytes) as flash file system. 
 
 I have the following problem : the P1020 board boots fine using the
 RAM disk with the flash in the device tree , shows the flash device
 partitions (JFFS2) and DOESNT try to mount a flash partition as root.
 The MPC8349 boots fine from the RAM disk but, after identifying the
 flash partitions, the kernel panics because is looking for a flash
 partition to mount as root partition and none of them is usable (not
 formatted). If I remove the flash from the device tree, the card boots
 fine using the RAM disk.
 
 
 I am not too familiar with Linux boot scripts and I didn't figure out
 where I can disable this tentative of mounting the MTD partition. I
 want the boards to boot and mount the RAM disk, as the P1020 board
 does. The flash partition will be initialized and mounted at a later
 time, but not as root partition.

Compare the kernel command line and kernel config for the two boards;
there's probably a relevant difference there (especially with the
root= parameter).

-Scott




___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH v1] powerpc/mpc512x: silence build warning upon disabled DIU

2013-10-08 Thread Anatolij Gustschin
On Fri, 27 Sep 2013 17:28:38 +0200
Gerhard Sittig g...@denx.de wrote:

 a disabled Kconfig option results in a reference to a not implemented
 routine when the IS_ENABLED() macro is used for both conditional
 implementation of the routine as well as a C language source code test
 at the call site -- the if (0) func(); construct only gets eliminated
 later by the optimizer, while the compiler already has emitted its
 warning about func() being undeclared

applied, thanks!
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH] Kind of revert powerpc: 52xx: provide a default in mpc52xx_irqhost_map()

2013-10-08 Thread Anatolij Gustschin
On Fri,  4 Oct 2013 17:37:09 +0200
Wolfram Sang w...@the-dreams.de wrote:

 This more or less reverts commit 6391f697d4892a6f233501beea553e13f7745a23.
 Instead of adding an unneeded 'default', mark the variable to prevent
 the false positive 'uninitialized var'. The other change (fixing the
 printout) needs revert, too. We want to know WHICH critical irq failed,
 not which level it had.
 
 Signed-off-by: Wolfram Sang w...@the-dreams.de
 Cc: Sebastian Andrzej Siewior bige...@linutronix.de
 Cc: Anatolij Gustschin ag...@denx.de

applied, thanks!
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH] powerpc/powernv: Reduce panic timeout from 180s to 10s

2013-10-08 Thread Scott Wood
On Tue, 2013-10-01 at 18:39 +1000, Michael Ellerman wrote:
 On Thu, Sep 26, 2013 at 09:17:19PM +1000, Anton Blanchard wrote:
  
  We made this change to pseries in 2011 and I think it makes
  sense to do the same on powernv.
 
 I'd vote we set it to 10s for all 64-bit machines in
 arch/powerpc/kernel/setup_64.c.

Why is 64-bit relevant?  And wouldn't such a short delay be a problem if
the crash is displayed on a monitor?

-Scott



___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: Gianfar driver crashes in Kernel v3.10

2013-10-08 Thread Scott Wood
On Fri, 2013-10-04 at 12:03 +, Thomas Hühn wrote:
 [code]
 [ 2671.841927] Oops: Exception in kernel mode, sig: 5 [#1]
 [ 2671.847141] Freescale P1014
 [ 2671.849925] Modules linked in: ath9k pppoe ppp_async iptable_nat 
 ath9k_common pppox p
 e xt_tcpudp xt_tcpmss xt_string xt_statistic xt_state xt_recent xt_quota 
 xt_pkttype xt_o
 mark xt_connbytes xt_comment xt_addrtype xt_TCPMSS xt_REDIRECT xt_NETMAP 
 xt_LOG xt_IPMAR
 ms_datafab ums_cypress ums_alauda slhc nf_nat_tftp nf_nat_snmp_basic 
 nf_nat_sip nf_nat_r
 ntrack_sip nf_conntrack_rtsp nf_conntrack_proto_gre nf_conntrack_irc 
 nf_conntrack_h323 n
  compat_xtables compat ath sch_teql sch_tbf sch_sfq sch_red sch_prio sch_htb 
 sch_gred sc
 skbedit act_mirred em_u32 cls_u32 cls_tcindex cls_flow cls_route cls_fw 
 sch_hfsc sch_ing
 r usb_storage leds_gpio ohci_hcd ehci_platform ehci_hcd sd_mod scsi_mod 
 fsl_mph_dr_of gp
 [ 2671.988946] CPU: 0 PID: 5209 Comm: iftop Not tainted 3.10.13 #2
 [ 2671.994859] task: c4b0 ti: c7ff8000 task.ti: c477e000
 [ 2672.000250] NIP: c018c7a0 LR: c018c794 CTR: c000b070
 [ 2672.005206] REGS: c7ff9f10 TRAP: 3202   Not tainted  (3.10.13)

Trap 0x3202 is a watchdog timer.

Did you get a Bad trap at... line before the above dump?  Do you have
any idea why the watchdog would have been armed without CONFIG_BOOKE_WDT
being set?  Is CONFIG_BOOKE_WDT set?

-Scott



___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: Elbc device driver

2013-10-08 Thread Scott Wood
On Tue, 2013-10-08 at 16:06 +0200, Mercier Ivan wrote:
 Hi,
 
 I'm working on a powerpc qoriq p3041 and trying to communicate with a
 device by elbc bus in gpmc mode.
 
 I 've integrated CONFIG_FSL_LBC in Linux which provide the basic functions.
 
 Now I'm wondering how can I do read and write operations on the
 bus.Where is mapped my device?

You'll need to use ioremap() or of_iomap() to map it.

 Should I code .read and .write driver functions?How can I start?
 
 How integrates my device in the device tree?

See Documentation/devicetree/bindings/powerpc/fsl/lbc.txt and examples
such as board-control in various device trees.

-Scott



___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 1/2][v2] pci: fsl: derive the common PCI driver to drivers/pci/host

2013-10-08 Thread Scott Wood
On Tue, 2013-10-08 at 13:13 -0600, Bjorn Helgaas wrote:
 [+cc Ben, Paul, linuxppc-dev]
 
 On Mon, Sep 30, 2013 at 04:52:54PM +0800, Minghuan Lian wrote:
  The Freescale's Layerscape series processors will use ARM cores.
  The LS1's PCIe controllers is the same as T4240's. So it's better
  the PCIe controller driver can support PowerPC and ARM
  simultaneously. This patch is for this purpose. It derives
  the common functions from arch/powerpc/sysdev/fsl_pci.c to
  drivers/pci/host/pci-fsl-common.c and leaves the architecture
  specific functions which should be implemented in arch related files.
 
  Signed-off-by: Minghuan Lian minghuan.l...@freescale.com
 
 I cc'd the powerpc maintainers so we can work out which tree this
 should go through.
 
  ---
  change log:
  v1-v2:
  1. rename pci.h to pci-common.h
  2. rename pci-fsl.c to pci-fsl-common.c
 
  Based on upstream master.
  Based on the discussion of RFC version here
  http://patchwork.ozlabs.org/patch/274487/
 
   arch/powerpc/sysdev/fsl_pci.c  | 521 +-
   arch/powerpc/sysdev/fsl_pci.h  |  89 
   .../fsl_pci.c = drivers/pci/host/pci-fsl-common.c | 591 
  +
   .../fsl_pci.h = include/linux/fsl/pci-common.h|  45 +-
 
 Is there any way to avoid putting this file in include/linux?  I know
 you want to share it beyond PowerPC, and I know there are similar
 examples there already, but this is all arch-specific or
 chipset-specific stuff that seems like it should be in some
 not-so-public place.  It doesn't seem scalable to add an include/linux
 subdirectory for every chipset that might be shared across
 architectures.

What specifically is the problem with it, as long as it's properly
namespaced?

-Scott



___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 1/7] powerpc: Add interface to get msi region information

2013-10-08 Thread Scott Wood
On Thu, 2013-09-19 at 12:59 +0530, Bharat Bhushan wrote:
 @@ -376,6 +405,7 @@ static int fsl_of_msi_probe(struct platform_device *dev)
   int len;
   u32 offset;
   static const u32 all_avail[] = { 0, NR_MSI_IRQS };
 + static int bank_index;
  
   match = of_match_device(fsl_of_msi_ids, dev-dev);
   if (!match)
 @@ -419,8 +449,8 @@ static int fsl_of_msi_probe(struct platform_device *dev)
   dev-dev.of_node-full_name);
   goto error_out;
   }
 - msi-msiir_offset =
 - features-msiir_offset + (res.start  0xf);
 + msi-msiir = res.start + features-msiir_offset;
 + printk(msi-msiir = %llx\n, msi-msiir);

dev_dbg or remove

   }
  
   msi-feature = features-fsl_pic_ip;
 @@ -470,6 +500,7 @@ static int fsl_of_msi_probe(struct platform_device *dev)
   }
   }
  
 + msi-bank_index = bank_index++;

What if multiple MSIs are boing probed in parallel?  bank_index is not
atomic.

 diff --git a/arch/powerpc/sysdev/fsl_msi.h b/arch/powerpc/sysdev/fsl_msi.h
 index 8225f86..6bd5cfc 100644
 --- a/arch/powerpc/sysdev/fsl_msi.h
 +++ b/arch/powerpc/sysdev/fsl_msi.h
 @@ -29,12 +29,19 @@ struct fsl_msi {
   struct irq_domain *irqhost;
  
   unsigned long cascade_irq;
 -
 - u32 msiir_offset; /* Offset of MSIIR, relative to start of CCSR */
 + dma_addr_t msiir; /* MSIIR Address in CCSR */

Are you sure dma_addr_t is right here, versus phys_addr_t?  It implies
that it's the output of the DMA API, but I don't think the DMA API is
used in the MSI driver.  Perhaps it should be, but we still want the raw
physical address to pass on to VFIO.

   void __iomem *msi_regs;
   u32 feature;
   int msi_virqs[NR_MSI_REG];
  
 + /*
 +  * During probe each bank is assigned a index number.
 +  * index number ranges from 0 to 2^32.
 +  * Example  MSI bank 1 = 0
 +  * MSI bank 2 = 1, and so on.
 +  */
 + int bank_index;

2^32 doesn't fit in int (nor does 2^32 - 1).

Just say that indices start at 0.

-Scott



___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


[PATCH] math-emu: fix floating-point to integer unsigned saturation

2013-10-08 Thread Joseph S. Myers
From: Joseph Myers jos...@codesourcery.com

The math-emu macros _FP_TO_INT and _FP_TO_INT_ROUND are supposed to
saturate their results for out-of-range arguments, except in the case
rsigned == 2 (when instead the low bits of the result are taken).
However, in the case rsigned == 0 (converting to unsigned integers),
they mistakenly produce 0 for positive results and the maximum
unsigned integer for negative results, the opposite of correct
unsigned saturation.  This patch fixes the logic.

Signed-off-by: Joseph Myers jos...@codesourcery.com

---

I intend to make the corresponding changes to the glibc/libgcc copy of
this code, given that it would be desirable to resync the Linux and
glibc/libgcc copies (the latter has had many enhancements and bug
fixes since it was copied into Linux), although strictly this
incorrect saturation is only a bug when trying to emulate particular
instruction semantics, not when used in userspace to implement C
operations where the results of out-of-range conversions are
unspecified or undefined.

diff --git a/include/math-emu/op-common.h b/include/math-emu/op-common.h
index 9696a5e..70fe5e9 100644
--- a/include/math-emu/op-common.h
+++ b/include/math-emu/op-common.h
@@ -685,7 +685,7 @@ do {
\
else
\
  { 
\
r = 0;  
\
-   if (X##_s)  
\
+   if (!X##_s) 
\
  r = ~r;   
\
  } 
\
FP_SET_EXCEPTION(FP_EX_INVALID);
\
@@ -762,7 +762,7 @@ do {
\
if (!rsigned)   
\
  { 
\
r = 0;  
\
-   if (X##_s)  
\
+   if (!X##_s) 
\
  r = ~r;   
\
  } 
\
else if (rsigned != 2)  
\

-- 
Joseph S. Myers
jos...@codesourcery.com
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 1/2][v2] pci: fsl: derive the common PCI driver to drivers/pci/host

2013-10-08 Thread Scott Wood
On Tue, 2013-10-08 at 17:09 -0600, Bjorn Helgaas wrote:
 On Tue, Oct 8, 2013 at 4:46 PM, Scott Wood scottw...@freescale.com wrote:
  On Tue, 2013-10-08 at 13:13 -0600, Bjorn Helgaas wrote:
  [+cc Ben, Paul, linuxppc-dev]
 
  On Mon, Sep 30, 2013 at 04:52:54PM +0800, Minghuan Lian wrote:
   The Freescale's Layerscape series processors will use ARM cores.
   The LS1's PCIe controllers is the same as T4240's. So it's better
   the PCIe controller driver can support PowerPC and ARM
   simultaneously. This patch is for this purpose. It derives
   the common functions from arch/powerpc/sysdev/fsl_pci.c to
   drivers/pci/host/pci-fsl-common.c and leaves the architecture
   specific functions which should be implemented in arch related files.
  
   Signed-off-by: Minghuan Lian minghuan.l...@freescale.com
 
  I cc'd the powerpc maintainers so we can work out which tree this
  should go through.
 
   ---
   change log:
   v1-v2:
   1. rename pci.h to pci-common.h
   2. rename pci-fsl.c to pci-fsl-common.c
  
   Based on upstream master.
   Based on the discussion of RFC version here
   http://patchwork.ozlabs.org/patch/274487/
  
arch/powerpc/sysdev/fsl_pci.c  | 521 
   +-
arch/powerpc/sysdev/fsl_pci.h  |  89 
.../fsl_pci.c = drivers/pci/host/pci-fsl-common.c | 591 
   +
.../fsl_pci.h = include/linux/fsl/pci-common.h|  45 +-
 
  Is there any way to avoid putting this file in include/linux?  I know
  you want to share it beyond PowerPC, and I know there are similar
  examples there already, but this is all arch-specific or
  chipset-specific stuff that seems like it should be in some
  not-so-public place.  It doesn't seem scalable to add an include/linux
  subdirectory for every chipset that might be shared across
  architectures.
 
  What specifically is the problem with it, as long as it's properly
  namespaced?
 
 Well, as I said above, it doesn't seem scalable,

I'm not sure what scaling problems you're picturing, assuming proper
namespacing and organization within include/linux/.

  and it doesn't seem to be the common existing practice. 

 Possibly this is just because sharing chipsets across arches isn't very 
 common yet.
 
 I hadn't noticed that include/linux/fsl exists already; I thought you
 were adding it.  Given that it *does* exist already, I guess I'm OK
 with putting more stuff in it.

I see other existing practice as well.  Besides plenty of
include/linux/fsl* that ought to be moved to include/linux/fsl/, I
see things like include/linux/amba/, include/linux/scx200*,
include/linux/clksrc-dbx500-prcmu.h, include/linux/com202020.h, etc.
These are just a few random examples out of many.

 So I'll apply these given an ack from the powerpc folks.

ACK this patch.  The second one I'd like to see broken up into
digestible chunks so I can better review it.

-Scott



___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


[PATCH] math-emu: fix floating-point to integer overflow detection

2013-10-08 Thread Joseph S. Myers
From: Joseph Myers jos...@codesourcery.com

On overflow, the math-emu macro _FP_TO_INT_ROUND tries to saturate its
result (subject to the value of rsigned specifying the desired
overflow semantics).  However, if the rounding step has the effect of
increasing the exponent so as to cause overflow (if the rounded result
is 1 larger than the largest positive value with the given number of
bits, allowing for signedness), the overflow does not get detected,
meaning that for unsigned results 0 is produced instead of the maximum
unsigned integer with the give number of bits, without an exception
being raised for overflow, and that for signed results the minimum
(negative) value is produced instead of the maximum (positive) value,
again without an exception.  This patch makes the code check for
rounding increasing the exponent and adjusts the exponent value as
needed for the overflow check.

Signed-off-by: Joseph Myers jos...@codesourcery.com

---

This macro is not present in the glibc/libgcc version of the code.
This patch is independent of my separate patch
http://lkml.org/lkml/2013/10/8/694 to fix the results for unsigned
saturation, although you need both patches together to get the correct
results for the affected unsigned overflow case.  It remains the case
both before and after this patch that the conversions wrongly treat a
signed result of the most negative integer as an overflow, when
actually only that integer minus 1 or smaller should be an overflow,
although this only means an incorrect exception rather than affecting
the value returned; that was one of the bugs I fixed in the
glibc/libgcc version of this code in 2006 (as part of a major overhaul
of the code including various interface changes, so not trivially
backportable to the kernel version).

diff --git a/include/math-emu/op-common.h b/include/math-emu/op-common.h
index 9696a5e..6bdf8c6 100644
--- a/include/math-emu/op-common.h
+++ b/include/math-emu/op-common.h
@@ -743,12 +743,17 @@ do {  
\
  } 
\
else
\
  { 
\
+   int _lz0, _lz1; 
\
if (X##_e = -_FP_WORKBITS - 1) 
\
  _FP_FRAC_SET_##wc(X, _FP_MINFRAC_##wc);   
\
else
\
  _FP_FRAC_SRS_##wc(X, _FP_FRACBITS_##fs - 1 - X##_e,   
\
_FP_WFRACBITS_##fs);
\
+   _FP_FRAC_CLZ_##wc(_lz0, X); 
\
_FP_ROUND(wc, X);   
\
+   _FP_FRAC_CLZ_##wc(_lz1, X); 
\
+   if (_lz1  _lz0)
\
+ X##_e++; /* For overflow detection.  */   
\
_FP_FRAC_SRL_##wc(X, _FP_WORKBITS); 
\
_FP_FRAC_ASSEMBLE_##wc(r, X, rsize);
\
  } 
\

-- 
Joseph S. Myers
jos...@codesourcery.com
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 1/7] powerpc: Add interface to get msi region information

2013-10-08 Thread Bjorn Helgaas
 - u32 msiir_offset; /* Offset of MSIIR, relative to start of CCSR */
 + dma_addr_t msiir; /* MSIIR Address in CCSR */

 Are you sure dma_addr_t is right here, versus phys_addr_t?  It implies
 that it's the output of the DMA API, but I don't think the DMA API is
 used in the MSI driver.  Perhaps it should be, but we still want the raw
 physical address to pass on to VFIO.

I don't know what msiir is used for, but if it's an address you
program into a PCI device, then it's a dma_addr_t even if you didn't
get it from the DMA API.  Maybe bus_addr_t would have been a more
suggestive name than dma_addr_t.  That said, I have no idea how this
relates to VFIO.

Bjorn
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 1/2][v2] pci: fsl: derive the common PCI driver to drivers/pci/host

2013-10-08 Thread Benjamin Herrenschmidt
On Tue, 2013-10-08 at 18:20 -0500, Scott Wood wrote:
  So I'll apply these given an ack from the powerpc folks.
 
 ACK this patch.  The second one I'd like to see broken up into
 digestible chunks so I can better review it.

Bjorn, for such FSL-only stuff, Scott ack is enough, don't wait for
mine :-)

Cheers,
Ben.


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 1/7] powerpc: Add interface to get msi region information

2013-10-08 Thread Scott Wood
On Tue, 2013-10-08 at 17:25 -0600, Bjorn Helgaas wrote:
  - u32 msiir_offset; /* Offset of MSIIR, relative to start of CCSR */
  + dma_addr_t msiir; /* MSIIR Address in CCSR */
 
  Are you sure dma_addr_t is right here, versus phys_addr_t?  It implies
  that it's the output of the DMA API, but I don't think the DMA API is
  used in the MSI driver.  Perhaps it should be, but we still want the raw
  physical address to pass on to VFIO.
 
 I don't know what msiir is used for, but if it's an address you
 program into a PCI device, then it's a dma_addr_t even if you didn't
 get it from the DMA API.  Maybe bus_addr_t would have been a more
 suggestive name than dma_addr_t.  That said, I have no idea how this
 relates to VFIO.

It's a bit awkward because it gets used both as something to program
into a PCI device (and it's probably a bug that the DMA API doesn't get
used), and also (if I understand the current plans correctly) as a
physical address to give to VFIO to be a destination address in an IOMMU
mapping.  So I think the value we keep here should be a phys_addr_t (it
comes straight from the MMIO address in the device tree), which gets
trivially turned into a dma_addr_t by the non-VFIO code path because
there's currently no translation there.

-Scott



___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


[PATCH] powerpc: fix e500 SPE float to integer and fixed-point conversions

2013-10-08 Thread Joseph S. Myers
From: Joseph Myers jos...@codesourcery.com

The e500 SPE floating-point emulation code has several problems in how
it handles conversions to integer and fixed-point fractional types.

There are the following 20 relevant instructions.  These can convert
to signed or unsigned 32-bit integers, either rounding towards zero
(as correct for C casts from floating-point to integer) or according
to the current rounding mode, or to signed or unsigned 32-bit
fixed-point values (values in the range [-1, 1) or [0, 1)).  For
conversion from double precision there are also instructions to
convert to 64-bit integers, rounding towards zero, although as far as
I know those instructions are completely theoretical (they are only
defined for implementations that support both SPE and classic 64-bit,
and I'm not aware of any such hardware even though the architecture
definition permits that combination).

#define EFSCTUI 0x2d4
#define EFSCTSI 0x2d5
#define EFSCTUF 0x2d6
#define EFSCTSF 0x2d7
#define EFSCTUIZ0x2d8
#define EFSCTSIZ0x2da

#define EVFSCTUI0x294
#define EVFSCTSI0x295
#define EVFSCTUF0x296
#define EVFSCTSF0x297
#define EVFSCTUIZ   0x298
#define EVFSCTSIZ   0x29a

#define EFDCTUIDZ   0x2ea
#define EFDCTSIDZ   0x2eb

#define EFDCTUI 0x2f4
#define EFDCTSI 0x2f5
#define EFDCTUF 0x2f6
#define EFDCTSF 0x2f7
#define EFDCTUIZ0x2f8
#define EFDCTSIZ0x2fa

The emulation code, for the instructions that come in variants
rounding either towards zero or according to the current rounding
direction, uses if (func  0x4) as a condition for using _FP_ROUND
(otherwise _FP_ROUND_ZERO is used).  The condition is correct, but the
code it controls isn't.  Whether _FP_ROUND or _FP_ROUND_ZERO is used
makes no difference, as the effect of those soft-fp macros is to round
an intermediate floating-point result using the low three bits (the
last one sticky) of the working format.  As these operations are
dealing with a freshly unpacked floating-point input, those low bits
are zero and no rounding occurs.  The emulation code then uses the
FP_TO_INT_* macros for the actual integer conversion, with the effect
of always rounding towards zero; for rounding according to the current
rounding direction, it should be using FP_TO_INT_ROUND_*.

The instructions in question have semantics defined (in the Power ISA
documents) for out-of-range values and NaNs: out-of-range values
saturate and NaNs are converted to zero.  The emulation does nothing
to follow those semantics for NaNs (the soft-fp handling is to treat
them as infinities), and messes up the saturation semantics.  For
single-precision conversion to integers, (((func  0x3) != 0) || SB_s)
is the condition used for doing a signed conversion.  The first part
is correct, but the second isn't: negative numbers should result in
saturation to 0 when converted to unsigned.  Double-precision
conversion to 64-bit integers correctly uses ((func  0x1) == 0).
Double-precision conversion to 32-bit integers uses (((func  0x3) !=
0) || DB_s), with correct first part and incorrect second part.  And
vector float conversion to integers uses (((func  0x3) != 0) ||
SB0_s) (and similar for the other vector element), where the sign bit
check is again wrong.

The incorrect handling of negative numbers converted to unsigned was
introduced in commit afc0a07d4a283599ac3a6a31d7454e9baaeccca0.  The
rationale given there was a C testcase with cast from float to
unsigned int.  Conversion of out-of-range floating-point numbers to
integer types in C is undefined behavior in the base standard, defined
in Annex F to produce an unspecified value.  That is, the C testcase
used to justify that patch is incorrect - there is no ISO C
requirement for a particular value resulting from this conversion -
and in any case, the correct semantics for such emulation are the
semantics for the instruction (unsigned saturation, which is what it
does in hardware when the emulation is disabled).

The conversion to fixed-point values has its own problems.  That code
doesn't try to do a full emulation; it relies on the trap handler only
being called for arguments that are infinities, NaNs, subnormal or out
of range.  That's fine, but the logic ((vb.wp[1]  23) == 0xff 
((vb.wp[1]  0x7f)  0)) for NaN detection won't detect negative
NaNs as being NaNs (the same applies for the double-precision case),
and subnormals are mapped to 0 rather than respecting the rounding
mode; the code should also explicitly raise the invalid exception.
The code for vectors works by executing the scalar float instruction
with the trapping disabled, meaning at least subnormals won't be
handled correctly.

As well as all those problems in the main emulation code, the rounding
handler - used to emulate rounding upward and downward when not
supported in hardware and when no higher priority exception occurred -
has its own problems.

* It 

Re: [PATCH] powerpc: fix e500 SPE float to integer and fixed-point conversions

2013-10-08 Thread Joseph S. Myers
On Tue, 8 Oct 2013, Joseph S. Myers wrote:

 I'll send as a followup the testcase I used for verifying that the
 instructions (other than the theoretical conversions to 64-bit
 integers) produce the correct results.  In addition, this has been
 tested with the glibc testsuite (with the e500 port as posted at
 https://sourceware.org/ml/libc-alpha/2013-10/msg00195.html, where it
 improves the libm test results.

Here is that testcase.

#include stdio.h
#include stdlib.h

#define INFF __builtin_inff ()
#define INFD __builtin_inf ()
#define NANF __builtin_nanf ()
#define NAND __builtin_nan ()

/* e500 rounding modes: 0 = nearest, 1 = zero, 2 = up, 3 = down.  */

static inline void
set_rm (unsigned int mode)
{
  unsigned int spefscr;
  asm volatile (mfspefscr %0 : =r (spefscr));
  spefscr = (spefscr  ~3) | mode;
  asm volatile (mtspefscr %0 : : r (spefscr));
}

static int success_count, failure_count;

struct float_test_data
{
  float input;
  unsigned int expected[4];
};

struct double_test_data
{
  double input;
  unsigned int expected[4];
};

typedef float vfloat __attribute__ ((vector_size (8)));
typedef unsigned int vuint __attribute__ ((vector_size (8)));

union vfloat_union
{
  vfloat vf;
  float f[2];
};

union vuint_union
{
  vuint vui;
  unsigned int ui[2];
};

#define T(A, B, C, D, E) { (A), { (B), (C), (D), (E) } }
#define TZ(A, B) T (A, B, B, B, B)

static void
check_result (const char *insn, double input, unsigned int rm,
  unsigned int expected, unsigned int res)
{
  if (res == expected)
success_count++;
  else
{
  failure_count++;
  printf (%s %a mode %u expected 0x%x (%d) got 0x%x (%d)\n,
  insn, input, rm, expected, (int) expected, res, (int) res);
}
}

#define RUN_FLOAT_TESTS(INSN)   \
static void \
test_##INSN (void)  \
{   \
  size_t i; \
  for (i = 0;   \
   i  sizeof (INSN##_test_data) / sizeof (INSN##_test_data[0]);\
   i++) \
{   \
  unsigned int rm;  \
  for (rm = 0; rm = 3; rm++)   \
{   \
  set_rm (rm);  \
  unsigned int res; \
  asm volatile (#INSN  %0, %1 \
: =r (res)   \
: r (INSN##_test_data[i].input)); \
  check_result (#INSN, INSN##_test_data[i].input, rm,   \
INSN##_test_data[i].expected[rm], res); \
}   \
}   \
}

#define RUN_VFLOAT_TESTS(INSN, TINSN)   \
static void \
test_##INSN (void)  \
{   \
  size_t i; \
  for (i = 0;   \
   i  sizeof (TINSN##_test_data) / sizeof (TINSN##_test_data[0]);  \
   i++) \
{   \
  unsigned int rm;  \
  for (rm = 0; rm = 3; rm++)   \
{   \
  set_rm (rm);  \
  union vfloat_union varg;  \
  union vuint_union vres;   \
  varg.f[0] = TINSN##_test_data[i].input;   \
  varg.f[1] = 0;\
  asm volatile (#INSN  %0, %1 \
: =r (vres.vui)  \
: r (varg.vf));   \
  check_result (#INSN  (high), TINSN##_test_data[i].input,\
rm, TINSN##_test_data[i].expected[rm],  \
vres.ui[0]);\
  

Re: [PATCH] powerpc/powernv: Reduce panic timeout from 180s to 10s

2013-10-08 Thread Anton Blanchard

   We made this change to pseries in 2011 and I think it makes
   sense to do the same on powernv.
  
  I'd vote we set it to 10s for all 64-bit machines in
  arch/powerpc/kernel/setup_64.c.
 
 Why is 64-bit relevant?  And wouldn't such a short delay be a problem
 if the crash is displayed on a monitor?

That is why we made it pseries specific in the past. Almost all our
boxes are on a virtual console and the 3 minutes of pausing just hurt
our uptimes.

If other platform maintainers prefer to keep the 3 minute pause, then
we just change the PowerNV platform.

Anton
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 5/9][v5] powerpc: implement is_instr_load_store().

2013-10-08 Thread Michael Ellerman
On Tue, 2013-10-08 at 12:31 -0700, Sukadev Bhattiprolu wrote:
 Michael Ellerman [mich...@ellerman.id.au] wrote:
 | bool is_load_store(int ext_opcode)
 | {
 | upper = ext_opcode  5;
 | lower = ext_opcode  0x1f;
 | 
 | /* Short circuit as many misses as we can */
 | if (lower  3 || lower  23)
 | return false;
 
 I see some loads/stores like these which are not covered by
 the above check. Is it ok to ignore them ?
 
   lower == 29: ldepx, stdepx, eviddepx, evstddepx
 
   lower == 31: lwepx, lbepx, lfdepx, stfdepx,

Those are the external process ID instructions, which I've never heard
of anyone using, I think we can ignore them.

 Looking through the opcode maps, I also see these for primary
 op code 4:
 
   evldd, evlddx, evldwx, evldw, evldh, evldhx.
 
 Should we include those also ?

Yes I think so. I didn't check any of the other opcodes for you.

cheers

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH] powerpc, perf: Configure BHRB filter before enabling PMU interrupts

2013-10-08 Thread Michael Ellerman
On Tue, Oct 08, 2013 at 12:51:18PM +0530, Anshuman Khandual wrote:
 On 10/08/2013 09:51 AM, Michael Ellerman wrote:
  On Mon, Oct 07, 2013 at 10:00:26AM +0530, Anshuman Khandual wrote:
  Right now the `config_bhrb` PMU specific call happens after write_mmcr0
  which actually enables the PMU for event counting and interrupt. So
  there is a small window of time where the PMU and BHRB runs without the
  required HW branch filter (if any) enabled in BHRB. This can cause some
  of the branch samples to be collected through BHRB without any filter
  being applied and hence affecting the correctness of the results. This
  patch moves the BHRB config function call before enabling the interrupts.
  
  Patch looks good.
  
  But it reminds me I have an item in my TODO list:
   - Why can't config_bhrb() be done in compute_mmcr() ?
  
 
 compute_mmcr() function deals with generic MMCR* configs for normal PMU
 events. Even if BHRB config touches MMCRA register, it's configuration
 does not interfere with the PMU config for general events. So its best
 to keep them separate. 

I'm unconvinced. If they'd been together to begin with this bug never
would have happened.

And there's the added overhead of another indirect function call.

 Besides, we can always look at these code consolidation
 issues in future. 

The future is now.

 But this patch solves a problem which is happening right now.

Sure, I'm not saying we shouldn't merge it as a fix. But I think we
should do the cleanup to move it into compute_mmcr() for 3.13.

cheers
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 5/9][v5] powerpc: implement is_instr_load_store().

2013-10-08 Thread Michael Ellerman
On Wed, Oct 09, 2013 at 12:03:19PM +1100, Michael Ellerman wrote:
 On Tue, 2013-10-08 at 12:31 -0700, Sukadev Bhattiprolu wrote:
  Michael Ellerman [mich...@ellerman.id.au] wrote:
  | bool is_load_store(int ext_opcode)
  | {
  | upper = ext_opcode  5;
  | lower = ext_opcode  0x1f;
  | 
  | /* Short circuit as many misses as we can */
  | if (lower  3 || lower  23)
  | return false;
  
  I see some loads/stores like these which are not covered by
  the above check. Is it ok to ignore them ?
  
  lower == 29: ldepx, stdepx, eviddepx, evstddepx
  
  lower == 31: lwepx, lbepx, lfdepx, stfdepx,
 
 Those are the external process ID instructions, which I've never heard
 of anyone using, I think we can ignore them.
 
  Looking through the opcode maps, I also see these for primary
  op code 4:
  
  evldd, evlddx, evldwx, evldw, evldh, evldhx.
  
  Should we include those also ?
 
 Yes I think so. I didn't check any of the other opcodes for you.

Paul points out these are for the SPE extension, which we also don't
care about. So ignore those as well.

cheers
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH RFC 00/77] Re-design MSI/MSI-X interrupts enablement pattern

2013-10-08 Thread Michael Ellerman
On Tue, Oct 08, 2013 at 09:33:02AM +0200, Alexander Gordeev wrote:
 On Tue, Oct 08, 2013 at 03:33:30PM +1100, Michael Ellerman wrote:
  On Wed, Oct 02, 2013 at 12:29:04PM +0200, Alexander Gordeev wrote:
   This technique proved to be confusing and error-prone. Vast share
   of device drivers simply fail to follow the described guidelines.
  
  To clarify Vast share of device drivers:
  
   - 58 drivers call pci_enable_msix()
   - 24 try a single allocation and then fallback to MSI/LSI
   - 19 use the loop style allocation as above
   - 14 try an allocation, and if it fails retry once
   - 1  incorrectly continues when pci_enable_msix() returns  0
  
  So 33 drivers ( 50%) successfully make use of the confusing and
  error-prone return value.
 
 Ok, you caught me - 'vast share' is incorrect and is a subject to
 rewording. But out of 19/58 how many drivers tested fallbacks on the
 real hardware? IOW, which drivers are affected by the pSeries quota?

It's not 19/58, it's 33/58.

As to how many we care about on powerpc I can't say, so you have a point
there. But I still think the interface is not actually that terrible.

cheers
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH RFC 00/77] Re-design MSI/MSI-X interrupts enablement pattern

2013-10-08 Thread Mark Lord
On 13-10-02 06:29 AM, Alexander Gordeev wrote:
..
 This update converts pci_enable_msix() and pci_enable_msi_block()
 interfaces to canonical kernel functions and makes them return a
 error code in case of failure or 0 in case of success.

Rather than silently break dozens of drivers in mysterious ways,
please invent new function names for the replacements to the
existing pci_enable_msix() and pci_enable_msi_block() functions.

That way, both in-tree and out-of-tree drivers will notice the API change,
rather than having it go unseen and just failing for unknown reasons.

Thanks.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH] powerpc/powernv: Add a debugfs file to read the firmware console

2013-10-08 Thread Michael Ellerman
On Tue, Oct 08, 2013 at 06:46:40PM +1100, Benjamin Herrenschmidt wrote:
 With OPALv3, the firmware can provide the address of it's internal console
 to Linux, which we can then display using debugfs. This is handy for
 diagnostics and debugging.
 
 Signed-off-by: Benjamin Herrenschmidt b...@kernel.crashing.org
 ---
 
 diff --git a/arch/powerpc/platforms/powernv/opal.c 
 b/arch/powerpc/platforms/powernv/opal.c
 index 2911abe..10d7894 100644
 --- a/arch/powerpc/platforms/powernv/opal.c
 +++ b/arch/powerpc/platforms/powernv/opal.c
 @@ -17,6 +17,8 @@
  #include linux/interrupt.h
  #include linux/notifier.h
  #include linux/slab.h
 +#include linux/debugfs.h
 +#include linux/uaccess.h
  #include asm/opal.h
  #include asm/firmware.h
  
 @@ -27,6 +29,21 @@ struct opal {
   u64 entry;
  } opal;
  
 +/* OPAL in-memory console */

It might be nice to point out that the format of the struct is defined
by OPAL and must be in sync with what OPAL is using.

 +struct memcons {
 + uint64_t magic;

u64 ?

 +#define MEMCONS_MAGIC0x6630696567726173
 + uint64_t obuf_phys;
 + uint64_t ibuf_phys;
 + uint32_t obuf_size;
 + uint32_t ibuf_size;
 + uint32_t out_pos;
 +#define MEMCONS_OUT_POS_WRAP 0x8000u
 +#define MEMCONS_OUT_POS_MASK 0x00ffu

Where does this come from?

 + uint32_t in_prod;
 + uint32_t in_cons;
 +};

Should it be packed?

 @@ -369,6 +386,90 @@ static irqreturn_t opal_interrupt(int irq, void *data)
   return IRQ_HANDLED;
  }
  
 +#ifdef CONFIG_DEBUG_FS
 +static ssize_t opal_memcons_read(struct file *file, char __user *to,
 +  size_t count, loff_t *ppos)
 +{
 + struct memcons *mc = file-private_data;
 + size_t available, ret, chunk0, chunk1, lcount;
 + const char *start, *conbuf = __va(mc-obuf_phys);
 + loff_t opos, pos;
 +
 + /*
 +  * Find out how much is in the buffer. If it has wrapped
 +  * the whole buffer, else just the beginning. It has wrapped
 +  * if the next character is not \0
 +  */
 + if (mc-out_pos  MEMCONS_OUT_POS_WRAP) {
 + available = mc-obuf_size;
 + chunk1 = mc-out_pos  MEMCONS_OUT_POS_MASK;
 + start = conbuf + chunk1;
 + chunk0 = mc-obuf_size - chunk1;
 + } else {
 + available = mc-out_pos;
 + start = conbuf;
 + chunk0 = available;
 + chunk1 = 0;
 + }

Surely simple_read_from_buffer() could make some of this simpler?


cheers
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH RFC 00/77] Re-design MSI/MSI-X interrupts enablement pattern

2013-10-08 Thread H. Peter Anvin
On 10/02/2013 03:29 AM, Alexander Gordeev wrote:
 
 As result, device drivers will cease to use the overcomplicated
 repeated fallbacks technique and resort to a straightforward
 pattern - determine the number of MSI/MSI-X interrupts required
 before calling pci_enable_msi_block() and pci_enable_msix()
 interfaces:
 
 
   rc = pci_msix_table_size(adapter-pdev);
   if (rc  0)
   return rc;
 
   nvec = min(nvec, rc);
   if (nvec  FOO_DRIVER_MINIMUM_NVEC) {
   return -ENOSPC;
 
   for (i = 0; i  nvec; i++)
   adapter-msix_entries[i].entry = i;
 
   rc = pci_enable_msix(adapter-pdev,
adapter-msix_entries, nvec);
   return rc;
 

Why not add a minimum number to pci_enable_msix(), i.e.:

pci_enable_msix(pdev, msix_entries, nvec, minvec)

... which means nvec is the number of interrupts *requested*, and
minvec is the minimum acceptable number (otherwise fail).

-hpa

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH] powerpc/powernv: Add a debugfs file to read the firmware console

2013-10-08 Thread Benjamin Herrenschmidt
On Wed, 2013-10-09 at 14:23 +1100, Michael Ellerman wrote:
 On Tue, Oct 08, 2013 at 06:46:40PM +1100, Benjamin Herrenschmidt wrote:
  With OPALv3, the firmware can provide the address of it's internal console
  to Linux, which we can then display using debugfs. This is handy for
  diagnostics and debugging.
  
  Signed-off-by: Benjamin Herrenschmidt b...@kernel.crashing.org
  ---
  
  diff --git a/arch/powerpc/platforms/powernv/opal.c 
  b/arch/powerpc/platforms/powernv/opal.c
  index 2911abe..10d7894 100644
  --- a/arch/powerpc/platforms/powernv/opal.c
  +++ b/arch/powerpc/platforms/powernv/opal.c
  @@ -17,6 +17,8 @@
   #include linux/interrupt.h
   #include linux/notifier.h
   #include linux/slab.h
  +#include linux/debugfs.h
  +#include linux/uaccess.h
   #include asm/opal.h
   #include asm/firmware.h
   
  @@ -27,6 +29,21 @@ struct opal {
  u64 entry;
   } opal;
   
  +/* OPAL in-memory console */
 
 It might be nice to point out that the format of the struct is defined
 by OPAL and must be in sync with what OPAL is using.

Yes, we could move the structure definition to opal.h...

  +struct memcons {
  +   uint64_t magic;
 
 u64 ?

Who cares ? Especially if it goes into opal.h it should stick to the
types used in that file.

  +#define MEMCONS_MAGIC  0x6630696567726173
  +   uint64_t obuf_phys;
  +   uint64_t ibuf_phys;
  +   uint32_t obuf_size;
  +   uint32_t ibuf_size;
  +   uint32_t out_pos;
  +#define MEMCONS_OUT_POS_WRAP   0x8000u
  +#define MEMCONS_OUT_POS_MASK   0x00ffu
 
 Where does this come from?

My a** :-) I made it up as I wrote the OPAL side one, why ?

  +   uint32_t in_prod;
  +   uint32_t in_cons;
  +};
 
 Should it be packed?

Nope, no need. It's all nice and naturally aligned.

  @@ -369,6 +386,90 @@ static irqreturn_t opal_interrupt(int irq, void *data)
  return IRQ_HANDLED;
   }
   
  +#ifdef CONFIG_DEBUG_FS
  +static ssize_t opal_memcons_read(struct file *file, char __user *to,
  +size_t count, loff_t *ppos)
  +{
  +   struct memcons *mc = file-private_data;
  +   size_t available, ret, chunk0, chunk1, lcount;
  +   const char *start, *conbuf = __va(mc-obuf_phys);
  +   loff_t opos, pos;
  +
  +   /*
  +* Find out how much is in the buffer. If it has wrapped
  +* the whole buffer, else just the beginning. It has wrapped
  +* if the next character is not \0
  +*/
  +   if (mc-out_pos  MEMCONS_OUT_POS_WRAP) {
  +   available = mc-obuf_size;
  +   chunk1 = mc-out_pos  MEMCONS_OUT_POS_MASK;
  +   start = conbuf + chunk1;
  +   chunk0 = mc-obuf_size - chunk1;
  +   } else {
  +   available = mc-out_pos;
  +   start = conbuf;
  +   chunk0 = available;
  +   chunk1 = 0;
  +   }
 
 Surely simple_read_from_buffer() could make some of this simpler?

If you can find a way to make it deal with a ring buffer...

Cheers,
Ben.


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH RFC 00/77] Re-design MSI/MSI-X interrupts enablement pattern

2013-10-08 Thread Benjamin Herrenschmidt
On Tue, 2013-10-08 at 20:55 -0700, H. Peter Anvin wrote:
 Why not add a minimum number to pci_enable_msix(), i.e.:
 
 pci_enable_msix(pdev, msix_entries, nvec, minvec)
 
 ... which means nvec is the number of interrupts *requested*, and
 minvec is the minimum acceptable number (otherwise fail).

Which is exactly what Ben (the other Ben :-) suggested and that I
supports...

Cheers,
Ben.


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH] powerpc, perf: Configure BHRB filter before enabling PMU interrupts

2013-10-08 Thread Anshuman Khandual
On 10/09/2013 06:51 AM, Michael Ellerman wrote:
 On Tue, Oct 08, 2013 at 12:51:18PM +0530, Anshuman Khandual wrote:
 On 10/08/2013 09:51 AM, Michael Ellerman wrote:
 On Mon, Oct 07, 2013 at 10:00:26AM +0530, Anshuman Khandual wrote:
 Right now the `config_bhrb` PMU specific call happens after write_mmcr0
 which actually enables the PMU for event counting and interrupt. So
 there is a small window of time where the PMU and BHRB runs without the
 required HW branch filter (if any) enabled in BHRB. This can cause some
 of the branch samples to be collected through BHRB without any filter
 being applied and hence affecting the correctness of the results. This
 patch moves the BHRB config function call before enabling the interrupts.

 Patch looks good.

 But it reminds me I have an item in my TODO list:
  - Why can't config_bhrb() be done in compute_mmcr() ?


 compute_mmcr() function deals with generic MMCR* configs for normal PMU
 events. Even if BHRB config touches MMCRA register, it's configuration
 does not interfere with the PMU config for general events. So its best
 to keep them separate. 
 
 I'm unconvinced. If they'd been together to begin with this bug never
 would have happened.

This is an ordering of configuration problem. Putting them together in the
same function does not rule out the chances of this ordering problem. Could
you please kindly explain how this could have been avoided ?

 
 And there's the added overhead of another indirect function call.


This overhead should be minimal given the fact that we already call so
many PMU specific indirect calls. BHRB is a different part of the PMU
hardware, so a separate call for this purpose is not a bad idea. AFAIK,
X86 does that too for LBR. But yes it is debatable.

 Besides, we can always look at these code consolidation
 issues in future. 
 
 The future is now.

What I meant was functional correctness has always more priority than
code consolidation efforts. Yes I will look into this after book3s
software branch filtering code has been merged.

 
 But this patch solves a problem which is happening right now.
 
 Sure, I'm not saying we shouldn't merge it as a fix. But I think we
 should do the cleanup to move it into compute_mmcr() for 3.13.

yeah that sounds reasonable.

Regards
Anshuman

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


RE: [PATCH 1/7] powerpc: Add interface to get msi region information

2013-10-08 Thread Bhushan Bharat-R65777


 -Original Message-
 From: Wood Scott-B07421
 Sent: Wednesday, October 09, 2013 4:27 AM
 To: Bhushan Bharat-R65777
 Cc: alex.william...@redhat.com; j...@8bytes.org; b...@kernel.crashing.org;
 ga...@kernel.crashing.org; linux-ker...@vger.kernel.org; linuxppc-
 d...@lists.ozlabs.org; linux-...@vger.kernel.org; ag...@suse.de;
 io...@lists.linux-foundation.org; Bhushan Bharat-R65777
 Subject: Re: [PATCH 1/7] powerpc: Add interface to get msi region information
 
 On Thu, 2013-09-19 at 12:59 +0530, Bharat Bhushan wrote:
  @@ -376,6 +405,7 @@ static int fsl_of_msi_probe(struct platform_device *dev)
  int len;
  u32 offset;
  static const u32 all_avail[] = { 0, NR_MSI_IRQS };
  +   static int bank_index;
 
  match = of_match_device(fsl_of_msi_ids, dev-dev);
  if (!match)
  @@ -419,8 +449,8 @@ static int fsl_of_msi_probe(struct platform_device *dev)
  dev-dev.of_node-full_name);
  goto error_out;
  }
  -   msi-msiir_offset =
  -   features-msiir_offset + (res.start  0xf);
  +   msi-msiir = res.start + features-msiir_offset;
  +   printk(msi-msiir = %llx\n, msi-msiir);
 
 dev_dbg or remove

Oops, sorry it was leftover of debugging :(

 
  }
 
  msi-feature = features-fsl_pic_ip; @@ -470,6 +500,7 @@ static int
  fsl_of_msi_probe(struct platform_device *dev)
  }
  }
 
  +   msi-bank_index = bank_index++;
 
 What if multiple MSIs are boing probed in parallel?

Ohh, I have not thought that it can be called in parallel

  bank_index is not atomic.

Will declare bank_intex as atomic_t and use atomic_inc_return(bank_index)

 
  diff --git a/arch/powerpc/sysdev/fsl_msi.h
  b/arch/powerpc/sysdev/fsl_msi.h index 8225f86..6bd5cfc 100644
  --- a/arch/powerpc/sysdev/fsl_msi.h
  +++ b/arch/powerpc/sysdev/fsl_msi.h
  @@ -29,12 +29,19 @@ struct fsl_msi {
  struct irq_domain *irqhost;
 
  unsigned long cascade_irq;
  -
  -   u32 msiir_offset; /* Offset of MSIIR, relative to start of CCSR */
  +   dma_addr_t msiir; /* MSIIR Address in CCSR */
 
 Are you sure dma_addr_t is right here, versus phys_addr_t?  It implies that 
 it's
 the output of the DMA API, but I don't think the DMA API is used in the MSI
 driver.  Perhaps it should be, but we still want the raw physical address to
 pass on to VFIO.

Looking through the conversation I will make this phys_addr_t

 
  void __iomem *msi_regs;
  u32 feature;
  int msi_virqs[NR_MSI_REG];
 
  +   /*
  +* During probe each bank is assigned a index number.
  +* index number ranges from 0 to 2^32.
  +* Example  MSI bank 1 = 0
  +* MSI bank 2 = 1, and so on.
  +*/
  +   int bank_index;
 
 2^32 doesn't fit in int (nor does 2^32 - 1).

Right :(

 
 Just say that indices start at 0.

Will correct this

Thanks
-Bharat

 
 -Scott
 

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


[PATCH 1/4] powerpc: Added __cmpdi2 for signed 64bit comparision

2013-10-08 Thread Bharat Bhushan
This was missing on powerpc and I am getting compilation error
drivers/vfio/pci/vfio_pci_rdwr.c:193: undefined reference to `__cmpdi2'
drivers/vfio/pci/vfio_pci_rdwr.c:193: undefined reference to `__cmpdi2'

Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
---
 arch/powerpc/kernel/misc_32.S   |   14 ++
 arch/powerpc/kernel/ppc_ksyms.c |2 ++
 2 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/kernel/misc_32.S b/arch/powerpc/kernel/misc_32.S
index 777d999..7c0eec2 100644
--- a/arch/powerpc/kernel/misc_32.S
+++ b/arch/powerpc/kernel/misc_32.S
@@ -644,6 +644,20 @@ _GLOBAL(__lshrdi3)
blr
 
 /*
+ * 64-bit comparison: __cmpdi2(s64 a, s64 b)
+ * Returns 0 if a  b, 1 if a == b, 2 if a  b.
+ */
+_GLOBAL(__cmpdi2)
+   cmpwr3,r5
+   li  r3,1
+   bne 1f
+   cmplw   r4,r6
+   beqlr
+1: li  r3,0
+   bltlr
+   li  r3,2
+   blr
+/*
  * 64-bit comparison: __ucmpdi2(u64 a, u64 b)
  * Returns 0 if a  b, 1 if a == b, 2 if a  b.
  */
diff --git a/arch/powerpc/kernel/ppc_ksyms.c b/arch/powerpc/kernel/ppc_ksyms.c
index 21646db..5674c00 100644
--- a/arch/powerpc/kernel/ppc_ksyms.c
+++ b/arch/powerpc/kernel/ppc_ksyms.c
@@ -143,6 +143,8 @@ EXPORT_SYMBOL(__ashldi3);
 EXPORT_SYMBOL(__lshrdi3);
 int __ucmpdi2(unsigned long long, unsigned long long);
 EXPORT_SYMBOL(__ucmpdi2);
+int __cmpdi2(long long, long long);
+EXPORT_SYMBOL(__cmpdi2);
 #endif
 long long __bswapdi2(long long);
 EXPORT_SYMBOL(__bswapdi2);
-- 
1.7.0.4


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


RE: [PATCH 1/4] powerpc: Added __cmpdi2 for signed 64bit comparision

2013-10-08 Thread Bhushan Bharat-R65777
Oops it came as 1/4,
I am sorry, please ignore this

Thanks
-Bharat

 -Original Message-
 From: Bhushan Bharat-R65777
 Sent: Wednesday, October 09, 2013 10:39 AM
 To: Wood Scott-B07421; linuxppc-dev@lists.ozlabs.org; b...@kernel.crashing.org
 Cc: Bhushan Bharat-R65777; Bhushan Bharat-R65777
 Subject: [PATCH 1/4] powerpc: Added __cmpdi2 for signed 64bit comparision
 
 This was missing on powerpc and I am getting compilation error
 drivers/vfio/pci/vfio_pci_rdwr.c:193: undefined reference to `__cmpdi2'
 drivers/vfio/pci/vfio_pci_rdwr.c:193: undefined reference to `__cmpdi2'
 
 Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
 ---
  arch/powerpc/kernel/misc_32.S   |   14 ++
  arch/powerpc/kernel/ppc_ksyms.c |2 ++
  2 files changed, 16 insertions(+), 0 deletions(-)
 
 diff --git a/arch/powerpc/kernel/misc_32.S b/arch/powerpc/kernel/misc_32.S 
 index
 777d999..7c0eec2 100644
 --- a/arch/powerpc/kernel/misc_32.S
 +++ b/arch/powerpc/kernel/misc_32.S
 @@ -644,6 +644,20 @@ _GLOBAL(__lshrdi3)
   blr
 
  /*
 + * 64-bit comparison: __cmpdi2(s64 a, s64 b)
 + * Returns 0 if a  b, 1 if a == b, 2 if a  b.
 + */
 +_GLOBAL(__cmpdi2)
 + cmpwr3,r5
 + li  r3,1
 + bne 1f
 + cmplw   r4,r6
 + beqlr
 +1:   li  r3,0
 + bltlr
 + li  r3,2
 + blr
 +/*
   * 64-bit comparison: __ucmpdi2(u64 a, u64 b)
   * Returns 0 if a  b, 1 if a == b, 2 if a  b.
   */
 diff --git a/arch/powerpc/kernel/ppc_ksyms.c b/arch/powerpc/kernel/ppc_ksyms.c
 index 21646db..5674c00 100644
 --- a/arch/powerpc/kernel/ppc_ksyms.c
 +++ b/arch/powerpc/kernel/ppc_ksyms.c
 @@ -143,6 +143,8 @@ EXPORT_SYMBOL(__ashldi3);  EXPORT_SYMBOL(__lshrdi3);  int
 __ucmpdi2(unsigned long long, unsigned long long);  EXPORT_SYMBOL(__ucmpdi2);
 +int __cmpdi2(long long, long long);
 +EXPORT_SYMBOL(__cmpdi2);
  #endif
  long long __bswapdi2(long long);
  EXPORT_SYMBOL(__bswapdi2);
 --
 1.7.0.4


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


[PATCH] powerpc: Added __cmpdi2 for signed 64bit comparision

2013-10-08 Thread Bharat Bhushan
This was missing on powerpc and I am getting compilation error
drivers/vfio/pci/vfio_pci_rdwr.c:193: undefined reference to `__cmpdi2'
drivers/vfio/pci/vfio_pci_rdwr.c:193: undefined reference to `__cmpdi2'

Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
---
 arch/powerpc/kernel/misc_32.S   |   14 ++
 arch/powerpc/kernel/ppc_ksyms.c |2 ++
 2 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/kernel/misc_32.S b/arch/powerpc/kernel/misc_32.S
index 777d999..7c0eec2 100644
--- a/arch/powerpc/kernel/misc_32.S
+++ b/arch/powerpc/kernel/misc_32.S
@@ -644,6 +644,20 @@ _GLOBAL(__lshrdi3)
blr
 
 /*
+ * 64-bit comparison: __cmpdi2(s64 a, s64 b)
+ * Returns 0 if a  b, 1 if a == b, 2 if a  b.
+ */
+_GLOBAL(__cmpdi2)
+   cmpwr3,r5
+   li  r3,1
+   bne 1f
+   cmplw   r4,r6
+   beqlr
+1: li  r3,0
+   bltlr
+   li  r3,2
+   blr
+/*
  * 64-bit comparison: __ucmpdi2(u64 a, u64 b)
  * Returns 0 if a  b, 1 if a == b, 2 if a  b.
  */
diff --git a/arch/powerpc/kernel/ppc_ksyms.c b/arch/powerpc/kernel/ppc_ksyms.c
index 21646db..5674c00 100644
--- a/arch/powerpc/kernel/ppc_ksyms.c
+++ b/arch/powerpc/kernel/ppc_ksyms.c
@@ -143,6 +143,8 @@ EXPORT_SYMBOL(__ashldi3);
 EXPORT_SYMBOL(__lshrdi3);
 int __ucmpdi2(unsigned long long, unsigned long long);
 EXPORT_SYMBOL(__ucmpdi2);
+int __cmpdi2(long long, long long);
+EXPORT_SYMBOL(__cmpdi2);
 #endif
 long long __bswapdi2(long long);
 EXPORT_SYMBOL(__bswapdi2);
-- 
1.7.0.4


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev