Re: [PATCH v2 14/14] Kconfig cleanup (PARPORT_PC dependencies)
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
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
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
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
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
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
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
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
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
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
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
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
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
-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
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
[+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().
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
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
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()
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
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
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
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
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
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
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
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
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
- 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
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
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
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
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
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().
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
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().
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
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
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
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
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
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
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
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
-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
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
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
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