Re: [PART2 RFC v2 00/10] iommu/AMD: Introduce IOMMU AVIC support
On 6/21/2016 9:46 AM, Joerg Roedel wrote: On Tue, Jun 21, 2016 at 09:27:27AM -0500, Suthikulpanit, Suravee wrote: On 6/21/2016 8:50 AM, Joerg Roedel wrote: The code has a few style issues (thing I'd implemented differently), but it looks functional. Anything in particular that you think I should improve or/and change? One thing I noticed were all the if (small IRTE) ... else (big IRTE) that would better be hidden behind function pointers. Also a few magic numbers are in there that could be replaced by proper defines. Joerg Ok, I'll clean up some more and send out the V3. Thanks, Suravee
Re: [PART2 RFC v2 00/10] iommu/AMD: Introduce IOMMU AVIC support
On 6/21/2016 8:50 AM, Joerg Roedel wrote: The code has a few style issues (thing I'd implemented differently), but it looks functional. Anything in particular that you think I should improve or/and change? Thanks, Suravee
Re: [PATCH] KVM: SVM: compile out AVIC if !CONFIG_X86_LOCAL_APIC
On 6/14/2016 5:01 PM, Paolo Bonzini wrote: On 14/06/2016 23:44, Suravee Suthikulanit wrote: > On 6/14/2016 4:22 PM, Paolo Bonzini wrote: >> - Original Message - >>> From: "Suravee Suthikulanit" >>> To: "Paolo Bonzini" , >>> linux-kernel@vger.kernel.org, k...@vger.kernel.org >>> Cc: rkrc...@redhat.com >>> Sent: Tuesday, June 14, 2016 8:20:30 PM >>> Subject: Re: [PATCH] KVM: SVM: compile out AVIC if >>> !CONFIG_X86_LOCAL_APIC >>> >>> Hi Paolo, >>> >>> On 6/14/2016 11:40 AM, Paolo Bonzini wrote: >>>> AVIC needs __default_cpu_present_to_apicid. Stub out all functions >>>> that use it, and disable the module parameter, if Linux is >>>> compiled without local APIC support. >>> >>> I think you are right that we should disable AVIC #ifndef >>> CONFIG_X86_LOCAL_APIC. However, do you think we should just use >>> default_cpu_present_to_apicid() instead of the >>> __default_cpu_present_to_apicid()? >> >> I'm not sure why that would help? default_cpu_present_to_apicid >> is also declared only if CONFIG_X86_LOCAL_APIC is defined. > > Actually, I also meant to include the change that I sent out > (https://lkml.org/lkml/2016/6/13/898), which declares a dummy for the > case #ifndef CONFIG_X86_LOCAL_APIC. That should help with the issue here. I think it is by design that default_cpu_present_to_apicid() is absent. Perhaps you could create a kvm_cpu_get_apicid() function that is either __default_cpu_present_to_apicid() or WARN_ON_ONCE(1)+return 0? Paolo Ok, I'll do that and send out V2. Thanks, Suravee
Re: [PATCH] KVM: SVM: compile out AVIC if !CONFIG_X86_LOCAL_APIC
On 6/14/2016 4:22 PM, Paolo Bonzini wrote: - Original Message - From: "Suravee Suthikulanit" To: "Paolo Bonzini" , linux-kernel@vger.kernel.org, k...@vger.kernel.org Cc: rkrc...@redhat.com Sent: Tuesday, June 14, 2016 8:20:30 PM Subject: Re: [PATCH] KVM: SVM: compile out AVIC if !CONFIG_X86_LOCAL_APIC Hi Paolo, On 6/14/2016 11:40 AM, Paolo Bonzini wrote: AVIC needs __default_cpu_present_to_apicid. Stub out all functions that use it, and disable the module parameter, if Linux is compiled without local APIC support. I think you are right that we should disable AVIC #ifndef CONFIG_X86_LOCAL_APIC. However, do you think we should just use default_cpu_present_to_apicid() instead of the __default_cpu_present_to_apicid()? I'm not sure why that would help? default_cpu_present_to_apicid is also declared only if CONFIG_X86_LOCAL_APIC is defined. Actually, I also meant to include the change that I sent out (https://lkml.org/lkml/2016/6/13/898), which declares a dummy for the case #ifndef CONFIG_X86_LOCAL_APIC. That should help with the issue here. As for disabling AVIC, I think we can also do: if (!IS_ENABLED(CONFIG_X86_LOCAL_APIC)) avic = false; Yes, we'll need to do that once AVIC is enabled by default; or static bool avic = IS_ENABLED(CONFIG_X86_LOCAL_APIC); In any case the module parameter must be hidden if there's no support in the kernel for the local APIC. Paolo Agree. If you okay with using default_cpu_present_to_apicid(), I can send out the v2 of the "[PATCH] x86/SVM: Fix implicit declaration issue for __default_cpu_present_to_apicid()" with changes that you suggested here. Thanks, Suravee
Re: [PATCH] KVM: SVM: compile out AVIC if !CONFIG_X86_LOCAL_APIC
Hi Paolo, On 6/14/2016 11:40 AM, Paolo Bonzini wrote: AVIC needs __default_cpu_present_to_apicid. Stub out all functions that use it, and disable the module parameter, if Linux is compiled without local APIC support. I think you are right that we should disable AVIC #ifndef CONFIG_X86_LOCAL_APIC. However, do you think we should just use default_cpu_present_to_apicid() instead of the __default_cpu_present_to_apicid()? This way, we can avoid having #ifdef CONFIG_X86_LOCAL_APIC in multiple places. As for disabling AVIC, I think we can also do: if (!IS_ENABLED(CONFIG_X86_LOCAL_APIC)) avic = false; What do you think? Thanks, Suravee
Re: [PATCH V8 0/9] Support for ARM64 ACPI based PCI host controller
On 5/30/2016 10:14 AM, Tomasz Nowicki wrote:
From the functionality point of view this series may be split into the
following logic parts:
1. Export ECAM API and add parent device to pci_config_window
2. Add IO resources handling to PCI core code
3. Support for generic domain assignment based on ACPI
4. New MCFG driver
5. Implement ARM64 ACPI based PCI host controller driver under arch/arm64/
Patches has been built on top of 4.7-rc1 and can be found here:
g...@github.com:semihalf-nowicki-tomasz/linux.git (pci-acpi-v8)
This has been tested on Cavium ThunderX server. Any help in reviewing and
testing is very appreciated.
v7 -> v8
- move code from drivers/acpi/pci_root_generic.c to arch/arm64/kernel/pci.c
- minor changes around domain assignment
- pci_mcfg.c improvements for parsing MCFG tables and lookup its entries
v6 -> v7
- drop quirks handling
- changes for ACPI companion and domain number assignment approach
- implement arch pcibios_{add|remove}_bus and call acpi_pci_{add|remove}_bus
from there
- cleanups around nomenclature
- use resources oriented API for ECAM
- fix for based address calculation before mapping ECAM region
- remove useless lock for MCFG lookup
- move MCFG stuff to separated file pci_mcfg.c
- drop MCFG entries caching
- rebase against 4.6-rc7
v5 -> v6
- drop idea of x86 MMCONFIG code refactoring
- integrate JC's patches which introduce new ECAM API:
https://lkml.org/lkml/2016/4/11/907
git: https://github.com/jchandra-brcm/linux/ (arm64-acpi-pci-v3)
- integrate Sinan's fix for releasing IO resources, see patch [06/13]
- added ACPI support for ThunderX ECAM and PEM drivers
- rebase against 4.6-rc2
v4 -> v5
- drop MCFG refactoring group patches 1-6 from series v4 and integrate
Jayachandran's patch
https://patchwork.ozlabs.org/patch/575525/
- rewrite PCI legacy IRQs allocation
- squash two patches 11 and 12 from series v4, fixed bisection issue
- changelog improvements
- rebase against 4.5-rc3
v3 -> v4
- drop Jiang's fix http://lkml.iu.edu/hypermail/linux/kernel/1601.1/04318.html
- add Lorenzo's fix patch 19/24
- ACPI PCI bus domain number assigning cleanup
- change resource management, we now claim and reassign resources
- improvements for applying quirks
- drop Matthew's http://www.spinics.net/lists/linux-pci/msg45950.html dependency
- rebase against 4.5-rc1
v2 -> v3
- fix legacy IRQ assigning and IO ports registration
- remove reference to arch specific companion device for ia64
- move ACPI PCI host controller driver to pci_root.c
- drop generic domain assignment for x86 and ia64 as I am not
able to run all necessary test variants
- drop patch which cleaned legacy IRQ assignment since it belongs to
Mathew's series:
https://patchwork.ozlabs.org/patch/557504/
- extend MCFG quirk code
- rebase against 4.4
v1 -> v2
- move non-arch specific piece of code to dirver/acpi/ directory
- fix IO resource handling
- introduce PCI config accessors quirks matching
- moved ACPI_COMPANION_SET to generic code
v1 - https://lkml.org/lkml/2015/10/27/504
v2 - https://lkml.org/lkml/2015/12/16/246
v3 - http://lkml.iu.edu/hypermail/linux/kernel/1601.1/04308.html
v4 - https://lkml.org/lkml/2016/2/4/646
v5 - https://lkml.org/lkml/2016/2/16/426
v6 - https://lkml.org/lkml/2016/4/15/594
Jayachandran C (2):
PCI: ecam: move ecam.h to linux/include/pci-ecam.h
PCI: ecam: Add parent device field to pci_config_window
Tomasz Nowicki (7):
pci: Add new function to unmap IO resources.
acpi, pci: Support IO resources when parsing PCI host bridge
resources.
pci, acpi: add acpi hook to assign domain number.
arm64, pci, acpi: ACPI support for legacy IRQs parsing and
consolidation with DT code.
acpi: Add generic MCFG table handling
arm64, pci, acpi: Provide ACPI-specific prerequisites for PCI bus
enumeration.
pci, acpi: ARM64 support for ACPI based generic PCI host controller
arch/arm64/Kconfig | 2 +
arch/arm64/kernel/pci.c | 143 ++--
drivers/acpi/Kconfig| 3 +
drivers/acpi/Makefile | 1 +
drivers/acpi/pci_mcfg.c | 94
drivers/acpi/pci_root.c | 39 ++
drivers/pci/ecam.c | 6 +-
drivers/pci/ecam.h | 67 -
drivers/pci/host/pci-host-common.c | 3 +-
drivers/pci/host/pci-host-generic.c | 3 +-
drivers/pci/host/pci-thunder-ecam.c | 3 +-
drivers/pci/host/pci-thunder-pem.c | 6 +-
drivers/pci/pci.c | 29 +++-
include/linux/pci-acpi.h| 2 +
include/linux/pci-ecam.h| 67 +
include/linux/pci.h | 9 ++-
16 files changed, 387 insertions(+), 90 deletions(-)
create mode 100644 drivers/acpi/pci_mcfg.c
delete mode 100644 drivers/pci/ecam.h
create mode 100644 include/linux/pci-ecam.h
For the series,
Tested-by: Suravee Suthikulpanit
Thanks,
Suravee
Re: [PART2 RFC v1 1/9] iommu/amd: Detect and enable guest vAPIC support
On 5/9/2016 6:49 AM, Joerg Roedel wrote: On Fri, Apr 08, 2016 at 07:49:22AM -0500, Suthikulpanit, Suravee wrote: From: Suravee Suthikulpanit This patch introduces a new IOMMU driver parameter, amd_iommu_guest_ir, which can be used to specify different interrupt remapping mode for passthrough devices to VM guest: * legacy: Legacy interrupt remapping mode (w/ 32-bit IRTE) * ga: Guest vAPIC interrupt remapping mode (w/ 128-bit IRTE) Note that the GA mode also supports legacy interrupt remapping for non-passthrough devices with the 128-bit IRTE. Does this need to be under user control? The code can just check what the hardware supports and use the 128bit IRTEs if supported, no? Joerg It does not need to be signified by user. Currently, if the MMIO Offset 30h[GASup] bit (of IOMMU Extended Feature Register) is set, the driver should default to using the 128bit IRTE by setting MMIO Offset 0018h[GAEn] bit (of IOMMU Control Register). The default is also enabling GA mode (by setting MMIO 0018h[GAEn] if MMIO 0030h[GASup] is set). However, if SVM AVIC is not enabled, or if the AVIC HW cannot support the type of interrupt (e.g. multicast/broadcast), it falls back to use legacy interrupt remapping mode w/ 128-bit IRTE. This option is intended for the case when we want to force IOMMU to use legacy interrupt remapping (hence no need for 128-bit IRTE). I will improve on the documentation in the next patch series. Thanks, Suravee
Re: [PART1 RFC v4 08/11] svm: Add VMEXIT handlers for AVIC
Hi Radim / Paolo,
Sorry for delay in response.
On 4/12/2016 11:22 AM, Radim Krčmář wrote:
2016-04-07 03:20-0500, Suravee Suthikulpanit:
From: Suravee Suthikulpanit
This patch introduces VMEXIT handlers, avic_incomplete_ipi_interception()
and avic_unaccelerated_access_interception() along with two trace points
(trace_kvm_avic_incomplete_ipi and trace_kvm_avic_unaccelerated_access).
Signed-off-by: Suravee Suthikulpanit
---
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
@@ -3515,6 +3515,250 @@ static int mwait_interception(struct vcpu_svm *svm)
+static u32 *avic_get_logical_id_entry(struct kvm_vcpu *vcpu, u8 mda, bool flat)
+{
+ struct kvm_arch *vm_data = &vcpu->kvm->arch;
+ int index;
+ u32 *logical_apic_id_table;
+
+ if (flat) { /* flat */
+ if (mda > 7)
Don't you want to check that just one bit it set?
+ return NULL;
+ index = mda;
+ } else { /* cluster */
+ int apic_id = mda & 0xf;
+ int cluster_id = (mda & 0xf0) >> 8;
">> 4".
+
+ if (apic_id > 4 || cluster_id >= 0xf)
+ return NULL;
+ index = (cluster_id << 2) + apic_id;
ffs(apic_id), because 'apic_id' must be compacted into 2 bits.
+ }
+ logical_apic_id_table = (u32 *)
page_address(vm_data->avic_logical_id_table_page);
+
+ return &logical_apic_id_table[index];
+}
I have quite messed up in the logic here for handling the logical
cluster ID. Sorry for not catching this earlier. I'm rewriting this
function altogether to simplify it in the V5.
[...]
+ lid = ffs(dlid) - 1;
+ ret = avic_handle_ldr_write(&svm->vcpu, svm->vcpu.vcpu_id, lid);
+ if (ret)
+ return 0;
OS can actually change LDR, so the old one should be invalidated.
(No OS does, but that is not an important factor for the hypervisor.)
By validating the old one, are you suggesting that we should disable the
logical APIC table entry previously used by this vcpu? If so, that means
we would need to cached the previous LDR value since the one in vAPIC
backing page would already be updated.
[...]
+ if (vm_data->ldr_mode != mod) {
+
clear_page(page_address(vm_data->avic_logical_id_table_page));
+ vm_data->ldr_mode = mod;
+ }
+ break;
+ }
All these cases need to be called on KVM_SET_LAPIC -- the userspace can
provide completely new set of APIC registers and AVIC should build its
maps with them. Test with save/restore or migration.
Hm.. This means we might need to introduce a new hook which is called
from the arch/x86/kvm/lapic.c: kvm_apic_post_state_restore(). Probably
something like kvm_x86_ops->apic_post_state_restore(), which sets up the
new physical and logical APIC id tables for AVIC.
If this works, I'll add support to handle this and test with the
migration stuff in the V5.
+ if (offset >= 0x400) {
+ WARN(1, "Unsupported APIC offset %#x\n", offset);
"printk_ratelimited(KERN_INFO " is the most severe message you could
give. I think that not printing anything is best,
+ return ret;
because we should not return, but continue to emulate the access.
Then, this would continue as if we are handling the normal fault access.
+ }
+
+ if (trap) {
+ /* Handling Trap */
+ if (!write) /* Trap read should never happens */
+ BUG();
(BUG_ON(!write) is shorter, though I would avoid BUG -- only guests are
going to fail, so we don't need to kill the host.)
Ok. What about just WARN_ONCE(!write, "svm: Handling trap read.\n");
Thanks,
Suravee
Re: [PATCH 00/13] dtb: amd: Miscelleneous Updates for AMD Seattle DTS
On 1/28/2016 8:43 PM, Olof Johansson wrote: On Thu, Jan 28, 2016 at 2:20 PM, Suravee Suthikulanit wrote: Hi Olof, On 1/28/2016 3:39 PM, Olof Johansson wrote: Hi Suravee, On Wed, Jan 27, 2016 at 1:11 PM, Suravee Suthikulpanit wrote: From: Suravee Suthikulpanit This patch series contains several updates for the AMD Seattle SOC DTS files. It also adds new board files for newer Overdrive and Linaro 96boards (Husky) platforms. My Overdrive comes with DT provided by firmware, so there's no need to have a in-kernel-tree DT source. You are correct that the FW comes with DT, and in typical case, you wouldn't need this. Are you aware of other reasons to have it here? I just foresee divergence and conflicts between the two. It was quite obvious before this update when the FW-provided DT was a lot more complete than what we had in the kernel tree. However, there are still new/updated drivers being developed, and sometimes requires new/changes in DT binding. So, the DT that comes with the FW can get out of date, and will lack the support for new drivers. Note that it's expected that the driver will cope with the old DT contents, i.e. it needs to go with defaults that made sense before the binding was updated. It, however, doesn't have to enable new features. In other words, booting with an old DT needs to continue working. You can't require a user to update DT to avoid getting driver breakage. (The opposite is not enforced: Booting with a DT that is newer than the kernel isn't guaranteed to always work). Ok. I understand your point that driver will not break the existing DT. :) Certain version of the FW allows overriding the DT that comes with the FW. So, we are providing the in-kernel DT to allow developers to provide the updated device tree for newer kernels. This patch series is bringing the in-kernel DT closer to what the latest FW is providing to avoid potential conflicts. I do appreciate keeping the kernel one up to date with what firmware provides if it's truly needed, but I'd even more prefer that it wasn't. After all, it's how the ACPI-based booting works (no overriding table provided with the kernel), so it's a model you should already be somewhat familiar with. :) Agree. This is true in the x86 world where things are mostly stable. However, in the ARM64 cases, there are still newer supports being added. Often that I have been asked by folks to provide a base DT that they can extend (e.g. to add support for platform device pass-through, PCI pass-through, SBSA GWDT, etc). Eventually, this in-kernel DT would not be needed as the more stable DT would have already been in the later version of the FW. But in the meantime, it seems useful to have this in one accessible place. Thanks, Suravee I'm not doing a hard NAK on this, but I would like to get a bit more understanding of why it's considered needed. -Olof
Re: [PATCH 00/13] dtb: amd: Miscelleneous Updates for AMD Seattle DTS
Hi Olof, On 1/28/2016 3:39 PM, Olof Johansson wrote: Hi Suravee, On Wed, Jan 27, 2016 at 1:11 PM, Suravee Suthikulpanit wrote: From: Suravee Suthikulpanit This patch series contains several updates for the AMD Seattle SOC DTS files. It also adds new board files for newer Overdrive and Linaro 96boards (Husky) platforms. My Overdrive comes with DT provided by firmware, so there's no need to have a in-kernel-tree DT source. You are correct that the FW comes with DT, and in typical case, you wouldn't need this. Are you aware of other reasons to have it here? I just foresee divergence and conflicts between the two. It was quite obvious before this update when the FW-provided DT was a lot more complete than what we had in the kernel tree. However, there are still new/updated drivers being developed, and sometimes requires new/changes in DT binding. So, the DT that comes with the FW can get out of date, and will lack the support for new drivers. Certain version of the FW allows overriding the DT that comes with the FW. So, we are providing the in-kernel DT to allow developers to provide the updated device tree for newer kernels. This patch series is bringing the in-kernel DT closer to what the latest FW is providing to avoid potential conflicts. Regards, Suravee -Olof
Re: [PATCH v7 3/4] gicv2m: Refactor to prepare for ACPI support
On 12/17/2015 10:57 AM, Bjorn Helgaas wrote:
On Wed, Dec 16, 2015 at 06:23:49PM -0600, Suravee Suthikulanit wrote:
Hi Bjorn,
Thanks for your review. Please see my comments below.
On 12/16/2015 4:12 PM, Bjorn Helgaas wrote:
On Thu, Dec 10, 2015 at 08:55:29AM -0800, Suravee Suthikulpanit wrote:
This patch replaces the struct device_node with struct fwnode_handle
since this structure is common between DT and ACPI.
It also refactors gicv2m_init_one() to prepare for ACPI support.
The only functional change is removing the node name from pr_info.
Reviewed-by: Marc Zyngier
Signed-off-by: Suravee Suthikulpanit
@@ -359,10 +355,10 @@ static int __init gicv2m_init_one(struct device_node
*node,
}
list_add_tail(&v2m->entry, &v2m_nodes);
- pr_info("Node %s: range[%#lx:%#lx], SPI[%d:%d]\n", node->name,
- (unsigned long)v2m->res.start, (unsigned long)v2m->res.end,
- v2m->spi_start, (v2m->spi_start + v2m->nr_spis));
+ pr_info("range[%#lx:%#lx], SPI[%d:%d]\n",
+ (unsigned long)res->start, (unsigned long)res->end,
+ v2m->spi_start, (v2m->spi_start + v2m->nr_spis));
You didn't change this, but I don't think this message has enough
context. It's pretty cryptic all by itself. It'd be nice if it could
at least include a device name, e.g., if you could use dev_info().
Here is the example of the information printed:
[0.00] GICv2m: range[0xe118:0xe1181000], SPI[64:320]
Basically, the v2m is just an extension of the GIC. Here, we are
printing the memory range that it is covering, which can be used to
identify different V2m frame and the associate interrupt range
(SPI). The node name is not really providing any values. So, we are
removing it.
I noticed the pr_fmt definition later; that adds some useful context I
didn't know about. I guess there's no struct device for the GIC? I
don't see one in struct device_node. Seems like this piece of
hardware that apparently responds to a memory range *could* have a
struct device,but I'm a little fuzzy on how we handle ACPI and OF
device descriptions in that regard.
For DT, v2m is advertised as a sub-node inside GIC. So, both of them has
the struct device_node references. IIUC, GIC node is match as irqchip,
and not as a traditional platform bus device.
Similarly, for ACPI, v2m is advertised as a sub-table inside MADT, and
we are using the fwnode_handle to reference to.
I hadn't noticed the memory range part; maybe you could use %pR there?
I guess we could have :) I can send a separate patch to clean this up.
Just to double-check, there's no off-by-one error in the SPI range, is
there? The pattern I usually expect is "start, start + nr_items - 1".
In that case, this should have been [64:319]. I'll send a small patch to
clean this up.
I'm just kibbitzing here; this isn't PCI code, and you don't need my
ack, so just consider these as random observations.
Bjorn
Thanks for sharing your observation. It's always been good ones :)
Suravee
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] i2c: designware: Do not require clock when SSCN and FFCN are provided
Hi Mika,
On 12/18/2015 4:13 AM, Mika Westerberg wrote:
[]
So instead of this, what if we do not assign dev->get_clk_rate_khz at
all and then do something like below in the core driver?
I like the changes below since it is clear to see within the core file
how things are handled when get_clk_rate_khz is not assigned (i.e.
input_clock_hz = 0), and not necessary relying on the platform driver to
return 0 in this case.
So, at this point, I can re-submit the V3 and combine these changes, and
we both can sign-off. How does that sound?
Thanks,
Suravee
Of course we still need the other changes you did in this patch to cope
with the missing clock.
diff --git a/drivers/i2c/busses/i2c-designware-core.c
b/drivers/i2c/busses/i2c-designware-core.c
index 8c48b27ba059..25dccd8df772 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -271,6 +271,17 @@ static void __i2c_dw_enable(struct dw_i2c_dev *dev, bool
enable)
enable ? "en" : "dis");
}
+static unsigned long i2c_dw_clk_rate(struct dw_i2c_dev *dev)
+{
+ /*
+* Clock is not necessary if we got LCNT/HCNT values directly from
+* the platform code.
+*/
+ if (WARN_ON_ONCE(!dev->get_clk_rate_khz))
+ return 0;
+ return dev->get_clk_rate_khz(dev);
+}
+
/**
* i2c_dw_init() - initialize the designware i2c master hardware
* @dev: device private data
@@ -281,7 +292,6 @@ static void __i2c_dw_enable(struct dw_i2c_dev *dev, bool
enable)
*/
int i2c_dw_init(struct dw_i2c_dev *dev)
{
- u32 input_clock_khz;
u32 hcnt, lcnt;
u32 reg;
u32 sda_falling_time, scl_falling_time;
@@ -295,8 +305,6 @@ int i2c_dw_init(struct dw_i2c_dev *dev)
}
}
- input_clock_khz = dev->get_clk_rate_khz(dev);
-
reg = dw_readl(dev, DW_IC_COMP_TYPE);
if (reg == ___constant_swab32(DW_IC_COMP_TYPE_VALUE)) {
/* Configure register endianess access */
@@ -325,12 +333,12 @@ int i2c_dw_init(struct dw_i2c_dev *dev)
hcnt = dev->ss_hcnt;
lcnt = dev->ss_lcnt;
} else {
- hcnt = i2c_dw_scl_hcnt(input_clock_khz,
+ hcnt = i2c_dw_scl_hcnt(i2c_dw_clk_rate(dev),
4000, /* tHD;STA = tHIGH = 4.0 us */
sda_falling_time,
0, /* 0: DW default, 1: Ideal */
0); /* No offset */
- lcnt = i2c_dw_scl_lcnt(input_clock_khz,
+ lcnt = i2c_dw_scl_lcnt(i2c_dw_clk_rate(dev),
4700, /* tLOW = 4.7 us */
scl_falling_time,
0); /* No offset */
@@ -344,12 +352,12 @@ int i2c_dw_init(struct dw_i2c_dev *dev)
hcnt = dev->fs_hcnt;
lcnt = dev->fs_lcnt;
} else {
- hcnt = i2c_dw_scl_hcnt(input_clock_khz,
+ hcnt = i2c_dw_scl_hcnt(i2c_dw_clk_rate(dev),
600,/* tHD;STA = tHIGH = 0.6 us */
sda_falling_time,
0, /* 0: DW default, 1: Ideal */
0); /* No offset */
- lcnt = i2c_dw_scl_lcnt(input_clock_khz,
+ lcnt = i2c_dw_scl_lcnt(i2c_dw_clk_rate(dev),
1300, /* tLOW = 1.3 us */
scl_falling_time,
0); /* No offset */
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] iommu/amd: Assign default IOMMU when there is only one IOMMU
On 12/14/2015 9:08 AM, Joerg Roedel wrote: On Fri, Dec 11, 2015 at 04:54:38PM -0600, Suravee Suthikulpanit wrote: Current driver makes assumption that device with devid zero is always included in the range of devices to be managed by IOMMU. However, certain FW does not include devid zero in IVRS table. This has caused IOMMU perf driver to fail to initialize. Hmm, this is a firmware bug. Is this bug seen in any systems that are for sale? This patch implements a workaround for this case by always assign devid zero to be handled by the first IOMMU. Otherwise its better to fix the firmware than to add workarounds. Joerg Please ignore this patch. There are more stuff that I am planning to fix, and I am reworking them into V2. I'll send this out soon. Suravee -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] i2c: designware: Do not require clock when SSCN and FFCN are provided
On 12/16/2015 8:56 PM, Loc Ho wrote: Hi, The current driver uses input clock source frequency to calculate values for [SS|FS]_[HC|LC] registers. However, when booting ACPI, we do not currently have a good way to provide the frequency information. Instead, we can leverage the SSCN and FFCN ACPI methods, which can be used to directly provide these values. So, the clock information should no longer be required during probing. However, since clk can be invalid, additional checks must be done where we are making use of it. Signed-off-by: Suravee Suthikulpanit --- Note: This has been tested on AMD Seattle RevB for both DT and ACPI. Tested on X-Gene hardware also. -Loc Thanks for quick response. Suravee -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] i2c: designware: Add support for AMD Seattle I2C
Mika,
On 12/16/2015 8:54 AM, Mika Westerberg wrote:
On Wed, Dec 16, 2015 at 08:29:38AM -0600, Suravee Suthikulpanit wrote:
>
>
>On 12/16/2015 03:16 AM, Mika Westerberg wrote:
> >On Tue, Dec 15, 2015 at 08:14:34PM -0600, Suravee Suthikulpanit wrote:
> >>Hi Mika,
> >>
> >>On 12/15/15 15:55, Suravee Suthikulpanit wrote:
> >>>Add device HID AMDI0510 to match the I2C controlers on AMD Seattle platform
> >>>
> >>>Signed-off-by: Suravee Suthikulpanit
> >>>---
> >>> drivers/i2c/busses/i2c-designware-platdrv.c | 1 +
> >>> 1 file changed, 1 insertion(+)
> >>>
> >>>diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c
b/drivers/i2c/busses/i2c-designware-platdrv.c
> >>>index 57f623b..a027154 100644
> >>>--- a/drivers/i2c/busses/i2c-designware-platdrv.c
> >>>+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> >>>@@ -117,6 +117,7 @@ static const struct acpi_device_id dw_i2c_acpi_match[]
= {
> >>> { "80860F41", 0 },
> >>> { "808622C1", 0 },
> >>> { "AMD0010", 0 },
> >>>+ { "AMDI0510", 0 },
> >>> { }
> >>
> >>Since this driver seems to be used by several SOCs, and we have been adding
> >>the HID from various SOC vendors. Do you think it would be better to assign
> >>a CID so that each SOC vendor can specify in their ACPI DSDT and we can
> >>match them here?
> >
> >Sure _CID would work here.
>
>Do you know if Synopsys has already provided a CID that we can use for this?
No.
>If not, who do you think should provide this?
Why can't you make _CID for AMD part only? For Intel we are going to get
new IDs for every major SoC release no matter what.
Actually, after discussed with the team. We have decided to go with the
AMDI0510 at this point, and we will reuse this as CID in future SOC if
it contains compatible I2C controller.
Thanks,
Suravee
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/4] acpi: pci: Setup MSI domain for ACPI based pci devices
On 12/16/2015 4:15 PM, Bjorn Helgaas wrote: On Thu, Dec 10, 2015 at 08:55:27AM -0800, Suravee Suthikulpanit wrote: >This patch introduces pci_msi_register_fwnode_provider() for irqchip >to register a callback, to provide a way to determine appropriate MSI >domain for a pci device. > >It also introduces pci_host_bridge_acpi_msi_domain(), which returns >the MSI domain of the specified PCI host bridge with DOMAIN_BUS_PCI_MSI >bus token. Then, it is assigned to pci device. > >Reviewed-by: Marc Zyngier >Cc: Bjorn Helgaas >Cc: Rafael J. Wysocki >Signed-off-by: Suravee Suthikulpanit Acked-by: Bjorn Helgaas I assume the whole series will be queued via a non-PCI tree. Thank you, Bjorn. I think Marc is planning to pull this in once all parties have acked the patch series. Suravee -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 3/4] gicv2m: Refactor to prepare for ACPI support
Hi Bjorn,
Thanks for your review. Please see my comments below.
On 12/16/2015 4:12 PM, Bjorn Helgaas wrote:
On Thu, Dec 10, 2015 at 08:55:29AM -0800, Suravee Suthikulpanit wrote:
This patch replaces the struct device_node with struct fwnode_handle
since this structure is common between DT and ACPI.
It also refactors gicv2m_init_one() to prepare for ACPI support.
The only functional change is removing the node name from pr_info.
Reviewed-by: Marc Zyngier
Signed-off-by: Suravee Suthikulpanit
@@ -359,10 +355,10 @@ static int __init gicv2m_init_one(struct device_node
*node,
}
list_add_tail(&v2m->entry, &v2m_nodes);
- pr_info("Node %s: range[%#lx:%#lx], SPI[%d:%d]\n", node->name,
- (unsigned long)v2m->res.start, (unsigned long)v2m->res.end,
- v2m->spi_start, (v2m->spi_start + v2m->nr_spis));
+ pr_info("range[%#lx:%#lx], SPI[%d:%d]\n",
+ (unsigned long)res->start, (unsigned long)res->end,
+ v2m->spi_start, (v2m->spi_start + v2m->nr_spis));
You didn't change this, but I don't think this message has enough
context. It's pretty cryptic all by itself. It'd be nice if it could
at least include a device name, e.g., if you could use dev_info().
Here is the example of the information printed:
[0.00] GICv2m: range[0xe118:0xe1181000], SPI[64:320]
Basically, the v2m is just an extension of the GIC. Here, we are
printing the memory range that it is covering, which can be used to
identify different V2m frame and the associate interrupt range (SPI).
The node name is not really providing any values. So, we are removing it.
Thanks,
Suravee
If that's possible, I guess a separate patch would be appropriate,
since there are other similar things in this file.
j
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 4/4] gicv2m: acpi: Introducing GICv2m ACPI support
On 12/10/2015 3:14 AM, Marc Zyngier wrote:
+int __init gicv2m_init(struct fwnode_handle *parent_handle,
>+ struct irq_domain *parent)
>+{
>+ int ret = gicv2m_of_init(parent_handle, parent);
>+
>+ if (ret)
>+ ret = gicv2m_acpi_init(parent);
>+ return ret;
This should really read:
if (is_of_node(parent_handle))
return gicv2m_of_init(parent_handle, parent);
return gicv2m_acpi_init(parent);
and you can loose the test for NULL in gicv2m_of_init().
Right... Your style of returning which is cleaner ;)
I'll update in V7 and send it out shortly.
Thanks,
Suravee
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 0/4] gicv2m: acpi: Add ACPI support for GICv2m MSI
On 12/9/2015 8:20 PM, Duc Dang wrote: This has been tested on AMD Seattle (Overdrive) RevB system. > >NOTE: I have not tested ACPI GICv2m multiframe support since >I don't have access to such system. Any helps are appreciated. Hi Suravee, I tested your v2m-multiframe-v6 branch with APM X-Gene 2 that has multiple GICv2m frames and it worked. Please feel free to add: Tested-by: Duc Dang Thanks Duc. Marc, if you decide to add take V6 would you please also add the Tested-by? Thanks, Suravee -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 5/5] gicv2m: acpi: Introducing GICv2m ACPI support
On 12/9/2015 12:16 PM, Marc Zyngier wrote: diff --git a/drivers/irqchip/irq-gic-v2m.c b/drivers/irqchip/irq-gic-v2m.c >>>[...] >>>@@ -359,6 +368,8 @@ static int __init gicv2m_init_one(struct fwnode_handle *fwnode, >>> >>>if (to_of_node(fwnode)) >>>name = to_of_node(fwnode)->name; >>>+ else >>>+ name = irq_domain_get_irqchip_fwnode_name(fwnode); >> >>Don't bother with that, the name associated with the domain is >>absolutely meaningless. You are already printing the frame address, >>which is enough to identify it, should someone need to debug this. >> >>Drop the name from the previous patch as well, and that will make one >>less difference to care about. Patch #3 can die as well. >> > >Ok. I'll just leave them blank (i.e. const char *name ="") No, just remove name altogether. Nobody reads that anyway, and if they want to find out, there is the address that's clear enough. Thanks, M. Ok. Suravee -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 5/5] gicv2m: acpi: Introducing GICv2m ACPI support
Hi Marc, On 12/9/2015 4:38 AM, Marc Zyngier wrote: On Tue, 8 Dec 2015 17:48:06 -0800 Suravee Suthikulpanit wrote: This patch introduces gicv2m_acpi_init(), which uses information in MADT GIC MSI frames structure to initialize GICv2m driver. Signed-off-by: Suravee Suthikulpanit Signed-off-by: Hanjun Guo --- drivers/irqchip/irq-gic-v2m.c | 95 + drivers/irqchip/irq-gic.c | 3 ++ include/linux/irqchip/arm-gic.h | 4 ++ 3 files changed, 102 insertions(+) diff --git a/drivers/irqchip/irq-gic-v2m.c b/drivers/irqchip/irq-gic-v2m.c [...] @@ -359,6 +368,8 @@ static int __init gicv2m_init_one(struct fwnode_handle *fwnode, if (to_of_node(fwnode)) name = to_of_node(fwnode)->name; + else + name = irq_domain_get_irqchip_fwnode_name(fwnode); Don't bother with that, the name associated with the domain is absolutely meaningless. You are already printing the frame address, which is enough to identify it, should someone need to debug this. Drop the name from the previous patch as well, and that will make one less difference to care about. Patch #3 can die as well. Ok. I'll just leave them blank (i.e. const char *name ="") [...] diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h index bae69e5..30b2ccb 100644 --- a/include/linux/irqchip/arm-gic.h +++ b/include/linux/irqchip/arm-gic.h @@ -108,6 +108,10 @@ void gic_init(unsigned int nr, int start, int gicv2m_of_init(struct device_node *node, struct irq_domain *parent); +#ifdef CONFIG_ACPI +int gicv2m_acpi_init(struct irq_domain *parent); +#endif + How about having a single: int gicv2m_init(struct fwnode_handle *parent_handle, struct irq_domain *parent_domain); which in turn calls either gicv2m_of_init or gicv2m_acpi_init? Saves some #ifdef, and avoids another entry point. Sounds good. I'll take care of this. void gic_send_sgi(unsigned int cpu_id, unsigned int irq); int gic_get_cpu_id(unsigned int cpu); void gic_migrate_target(unsigned int new_cpu_id); Otherwise, this looks good. Thanks, M. Thanks. Sending out v6 in a little bit. Suravee -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 1/5] acpi: pci: Setup MSI domain for ACPI based pci devices
On 12/9/2015 4:13 AM, Marc Zyngier wrote: On Tue, 8 Dec 2015 17:48:02 -0800 Suravee Suthikulpanit wrote: This patch introduces pci_msi_register_fwnode_provider() for irqchip to register a callback, to provide a way to determine appropriate MSI domain for a pci device. It also introduces pci_host_bridge_acpi_msi_domain(), which returns the MSI domain of the specified PCI host bridge with DOMAIN_BUS_PCI_MSI bus token. Then, it is assigned to pci device. Signed-off-by: Suravee Suthikulpanit --- drivers/pci/pci-acpi.c| 32 drivers/pci/probe.c | 2 ++ include/linux/irqdomain.h | 5 + include/linux/pci.h | 10 ++ 4 files changed, 49 insertions(+) [...] Other than the couple of nits above: Reviewed-by: Marc Zyngier M. Thanks. I'll take care of them and send out V6. Suravee -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] PCI: Fix logic OF logic in pci_dma_configure()
Arg... sorry for the typo in the subject. It should say:
[PATCH] PCI: Fix OF logic in pci_dma_configure()
Suravee
On 11/18/2015 6:49 PM, Suravee Suthikulpanit wrote:
This patch fixes a bug introduced by previous commit,
which incorrectly checkes the of_node of the end-point device.
Instead, it should check the of_node of the host bridge.
Fixes: 50230713b639 ("PCI: OF: Move of_pci_dma_configure() to
pci_dma_configure()")
Reported-by: Robin Murphy
Cc: Rafael J. Wysocki
Cc: Bjorn Helgaas
Cc: Arnd Bergmann
Signed-off-by: Suravee Suthikulpanit
---
drivers/pci/probe.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index e735c72..edb1984 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1685,8 +1685,8 @@ static void pci_dma_configure(struct pci_dev *dev)
{
struct device *bridge = pci_get_host_bridge_device(dev);
- if (IS_ENABLED(CONFIG_OF) && dev->dev.of_node) {
- if (bridge->parent)
+ if (IS_ENABLED(CONFIG_OF) &&
+ bridge->parent && bridge->parent->of_node) {
of_dma_configure(&dev->dev, bridge->parent->of_node);
} else if (has_acpi_companion(bridge)) {
struct acpi_device *adev = to_acpi_device_node(bridge->fwnode);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V5 8/9] PCI: OF: Move of_pci_dma_configure() to pci_dma_configure()
Hi All,
On 11/18/2015 6:41 AM, Arnd Bergmann wrote:
On Wednesday 18 November 2015 12:00:32 Robin Murphy wrote:
On 17/11/15 15:00, Robin Murphy wrote:
On 28/10/15 22:50, Suravee Suthikulpanit wrote:
Signed-off-by: Suravee Suthikulpanit
Acked-by: Rob Herring
Acked-by: Bjorn Helgaas
Reviewed-by: Hanjun Guo
CC: Rafael J. Wysocki
[...]
+/**
+ * pci_dma_configure - Setup DMA configuration
+ * @dev: ptr to pci_dev struct of the PCI device
+ *
+ * Function to update PCI devices's DMA configuration using the same
+ * info from the OF node of host bridge's parent (if any).
+ */
+static void pci_dma_configure(struct pci_dev *dev)
+{
+struct device *bridge = pci_get_host_bridge_device(dev);
+
+if (IS_ENABLED(CONFIG_OF) && dev->dev.of_node) {
Previously I was seeing of_dma_configure, and thus of_iommu_configure,
called for every PCI device on Juno. The check above now prevents this
happening, since the PCI devices are probed directly from the bus and
don't have OF nodes of their own. They now get left in some
half-configured state where arch_setup_dma_ops isn't called either.
Just to follow up on that, Arnd's patch to tidy up dma_get_ops (now
queued[1]) makes this even worse, since preventing arch_setup_dma_ops
being called means the PCI devices now get the dummy DMA ops which leave
the drivers failing to probe at all, IOMMU hacks or not
Ok, glad we found that with my patch then. We really have to
configure the DMA (offset/size/coherency/iommu) for all devices that might
be masters, otherwise things can randomly go wrong.
ARnd
Robin is correct. Thanks for catching the bug. Rafael, do you want me to
submit just the fixed-up patch on top of what we had earlier. Or do you
want a new revision (V6)?
Thanks,
Suravee
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V5 8/9] PCI: OF: Move of_pci_dma_configure() to pci_dma_configure()
On 11/18/2015 6:41 AM, Arnd Bergmann wrote:
On Wednesday 18 November 2015 12:00:32 Robin Murphy wrote:
On 17/11/15 15:00, Robin Murphy wrote:
On 28/10/15 22:50, Suravee Suthikulpanit wrote:
Signed-off-by: Suravee Suthikulpanit
Acked-by: Rob Herring
Acked-by: Bjorn Helgaas
Reviewed-by: Hanjun Guo
CC: Rafael J. Wysocki
[...]
+/**
+ * pci_dma_configure - Setup DMA configuration
+ * @dev: ptr to pci_dev struct of the PCI device
+ *
+ * Function to update PCI devices's DMA configuration using the same
+ * info from the OF node of host bridge's parent (if any).
+ */
+static void pci_dma_configure(struct pci_dev *dev)
+{
+struct device *bridge = pci_get_host_bridge_device(dev);
+
+if (IS_ENABLED(CONFIG_OF) && dev->dev.of_node) {
Previously I was seeing of_dma_configure, and thus of_iommu_configure,
called for every PCI device on Juno. The check above now prevents this
happening, since the PCI devices are probed directly from the bus and
don't have OF nodes of their own. They now get left in some
half-configured state where arch_setup_dma_ops isn't called either.
Just to follow up on that, Arnd's patch to tidy up dma_get_ops (now
queued[1]) makes this even worse, since preventing arch_setup_dma_ops
being called means the PCI devices now get the dummy DMA ops which leave
the drivers failing to probe at all, IOMMU hacks or not
Ok, glad we found that with my patch then. We really have to
configure the DMA (offset/size/coherency/iommu) for all devices that might
be masters, otherwise things can randomly go wrong.
ARnd
I'm double checking this and will get back ASAP.
Suravee
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V5 0/9] PCI: ACPI: Setting up DMA coherency for PCI device from _CCA attribute
On 11/1/2015 7:01 PM, Rafael J. Wysocki wrote: Queued up for v4.4, but I'll include it into my second pull request in the second half of the merge window. Thanks, Rafael Thank you, Suravee -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V5 1/9] ACPI: Honor ACPI _CCA attribute setting
Hi Dennis / Hanjun,
On 11/2/2015 5:58 AM, Hanjun Guo wrote:
Hi Dennis,
On 11/02/2015 12:02 PM, Dennis Chen wrote:
On Thu, Oct 29, 2015 at 6:50 AM, Suravee Suthikulpanit
wrote:
From: Jeremy Linton
ACPI configurations can now mark devices as noncoherent,
support that choice.
NOTE: This is required to support USB on ARM Juno Development Board.
Signed-off-by: Jeremy Linton
Signed-off-by: Suravee Suthikulpanit
CC: Bjorn Helgaas
CC: Catalin Marinas
CC: Rob Herring
CC: Will Deacon
CC: Rafael J. Wysocki
---
include/acpi/acpi_bus.h | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index d11eff8..0f131d2 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -407,7 +407,7 @@ static inline bool acpi_check_dma(struct
acpi_device *adev, bool *coherent)
* case 1. Do not support and disable DMA.
* case 2. Support but rely on arch-specific cache maintenance for
* non-coherence DMA operations.
- * Currently, we implement case 1 above.
+ * Currently, we implement case 2 above.
*
* For the case when _CCA is missing (i.e. cca_seen=0) and
* platform specifies ACPI_CCA_REQUIRED, we do not support DMA,
@@ -415,7 +415,8 @@ static inline bool acpi_check_dma(struct
acpi_device *adev, bool *coherent)
*
* See acpi_init_coherency() for more info.
*/
-if (adev->flags.coherent_dma) {
+if (adev->flags.coherent_dma ||
+(adev->flags.cca_seen && IS_ENABLED(CONFIG_ARM64))) {
ret = true;
if (coherent)
*coherent = adev->flags.coherent_dma;
Hi Suravee,
The acpi_check_dma function has been removed in patch 6 of this patch
set, why it is still be used
here, am I missing something? If the acpi_check_dma will be used in
the future, personally I'd like
I think this patch just to let people know that there is
case that arch-specific cache maintenance is still needed
for ACPI (such as Juno board), and in the later patches will
cover this case.
acpi_check_dma() will be replaced by acpi_get_dma_attr(),
and in acpi_get_dma_attr() will cover that case and will
be easily understood. (Suravee, correct me if I'm wrong :) )
Thanks Hanjun for filling in the info.
Yes, this is mainly to document the logic changes required by Juno,
which would be more clear than just merging this change in the later patch.
to use IS_ENABLED(CONFIG_ACPI_CCA_REQUIRED) while not CONFIG_ARM64
macro here,
We could have used CONFIG_ACPI_CCA_REQUIRED here, but this will be
replaced by logic in patch 5, and removed in patch 6 anyways. So, I
think it is okay. Eventually, the rest of the logic will be using
CONFIG_ACPI_CCA_REQUIRED.
or since _CCA attribute
is arch-specific, it's reasonable to leave the _CCA handling policy to
the arch-specific code. For example,
with a link weak function like acpi_arch_check_dma() as a default
handling if no arch-specific code
provided, the actual _CCA handling will be implemented in the ARM,
Intel or other Arch if required.
Actually Intel platform don't need _CCA and it's coherent
in default, since _CCA is in ACPI spec, I would like it's
handled in ACPI core.
Thanks
Hanjun
I also agree with Hanjun that the CCA parsing should be handled by the
ACPI core driver. Since we are using the CONFIG_ACPI_CCA_REQUIRED, we
should not need to have arch-specific code. If the ACPI spec gets more
complicate in the future, we can revisit this. :)
Thanks,
Suravee
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 6/6] gicv2m: acpi: Introducing GICv2m ACPI support
Hi Tomasz,
On 10/15/2015 9:03 AM, Suravee Suthikulanit wrote:
+if (!data)
+return NULL;
+
+return data->fwnode;
+}
+
+static int __init
+acpi_parse_madt_msi(struct acpi_subtable_header *header,
+const unsigned long end)
+{
+int ret;
+struct resource res;
+u32 spi_start = 0, nr_spis = 0;
+struct acpi_madt_generic_msi_frame *m;
+struct fwnode_handle *fwnode = NULL;
+
+m = (struct acpi_madt_generic_msi_frame *)header;
+if (BAD_MADT_ENTRY(m, end))
+return -EINVAL;
+
+res.start = m->base_address;
+res.end = m->base_address + 0x1000;
+
+if (m->flags & ACPI_MADT_OVERRIDE_SPI_VALUES) {
+spi_start = m->spi_base;
+nr_spis = m->spi_count;
+
+pr_info("ACPI overriding V2M MSI_TYPER (base:%u, num:%u)\n",
+spi_start, nr_spis);
+}
+
+fwnode = irq_domain_alloc_fwnode((void *)m->base_address);
+if (!fwnode) {
+pr_err("Unable to allocate GICv2m domain token\n");
+return -EINVAL;
+}
+
+ret = gicv2m_init_one(fwnode, spi_start, nr_spis, &res);
I case of error, we should call here:
irq_domain_free_fwnode(fwnode);
This should have already been handled when returning from the
acpi_parse_madt_msi() in gicv2m_teardown() since we would need to
iterate through all existing MSI frame to clean up.
Actually, you are correct since the fwnode allocated here might not get
assigned to the v2m_data.fwnode and added to the v2m_nodes list yet. So,
we would need to call irq_domain_alloc_fwnode() here in case of error.
Thanks,
Suravee
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 6/6] gicv2m: acpi: Introducing GICv2m ACPI support
Hi Tomasz,
Thanks for the feedback.
On 10/15/2015 1:15 AM, Tomasz Nowicki wrote:
[..]
@@ -138,6 +140,11 @@ static int gicv2m_irq_gic_domain_alloc(struct
irq_domain *domain,
fwspec.param[0] = 0;
fwspec.param[1] = hwirq - 32;
fwspec.param[2] = IRQ_TYPE_EDGE_RISING;
+} else if (is_fwnode_irqchip(domain->parent->fwnode)) {
+fwspec.fwnode = domain->parent->fwnode;
+fwspec.param_count = 2;
+fwspec.param[0] = hwirq;
+fwspec.param[1] = IRQ_TYPE_EDGE_RISING & IRQ_TYPE_SENSE_MASK;
How about just:
fwspec.param[1] = IRQ_TYPE_EDGE_RISING;
Right.
} else {
return -EINVAL;
}
@@ -255,6 +262,8 @@ static void gicv2m_teardown(void)
kfree(v2m->bm);
iounmap(v2m->base);
of_node_put(to_of_node(v2m->fwnode));
+if (is_fwnode_irqchip(v2m->fwnode))
+irq_domain_free_fwnode(v2m->fwnode);
kfree(v2m);
}
}
@@ -359,6 +368,8 @@ static int __init gicv2m_init_one(struct
fwnode_handle *fwnode,
if (to_of_node(fwnode))
name = to_of_node(fwnode)->name;
+else
+name = irq_domain_get_irqchip_fwnode_name(fwnode);
pr_info("Frame %s: range[%#lx:%#lx], SPI[%d:%d]\n", name,
(unsigned long)res->start, (unsigned long)res->end,
@@ -415,3 +426,86 @@ int __init gicv2m_of_init(struct device_node
*node, struct irq_domain *parent)
gicv2m_teardown();
return ret;
}
+
+#ifdef CONFIG_ACPI
+static int acpi_num_msi;
+
+static struct fwnode_handle *gicv2m_get_fwnode(struct device *dev)
+{
+struct v2m_data *data;
+
+if (WARN_ON(acpi_num_msi <= 0))
+return NULL;
+
+/* We only return the fwnode of the first MSI frame. */
+data = list_first_entry_or_null(&v2m_nodes,
+struct v2m_data, entry);
This can be one line and still fits within 80 characters.
Ok.
+if (!data)
+return NULL;
+
+return data->fwnode;
+}
+
+static int __init
+acpi_parse_madt_msi(struct acpi_subtable_header *header,
+const unsigned long end)
+{
+int ret;
+struct resource res;
+u32 spi_start = 0, nr_spis = 0;
+struct acpi_madt_generic_msi_frame *m;
+struct fwnode_handle *fwnode = NULL;
+
+m = (struct acpi_madt_generic_msi_frame *)header;
+if (BAD_MADT_ENTRY(m, end))
+return -EINVAL;
+
+res.start = m->base_address;
+res.end = m->base_address + 0x1000;
+
+if (m->flags & ACPI_MADT_OVERRIDE_SPI_VALUES) {
+spi_start = m->spi_base;
+nr_spis = m->spi_count;
+
+pr_info("ACPI overriding V2M MSI_TYPER (base:%u, num:%u)\n",
+spi_start, nr_spis);
+}
+
+fwnode = irq_domain_alloc_fwnode((void *)m->base_address);
+if (!fwnode) {
+pr_err("Unable to allocate GICv2m domain token\n");
+return -EINVAL;
+}
+
+ret = gicv2m_init_one(fwnode, spi_start, nr_spis, &res);
I case of error, we should call here:
irq_domain_free_fwnode(fwnode);
This should have already been handled when returning from the
acpi_parse_madt_msi() in gicv2m_teardown() since we would need to
iterate through all existing MSI frame to clean up.
+
+return ret;
+}
+
+int __init gicv2m_acpi_init(struct irq_domain *parent)
+{
+int ret;
+
+if (acpi_num_msi > 0)
+return 0;
+
+acpi_num_msi =
acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_MSI_FRAME,
+ acpi_parse_madt_msi, 0);
+
+if (acpi_num_msi <= 0)
+goto err_out;
If acpi_table_parse_madt return 0, then we don't need to call
gicv2m_teardown(). Instead we can simply return, optionally add some
pr_info. Well, gicv2m_teardown would do nothing, so this is just
cosmetic and up to you.
I'd be hesitate to add pr_info here since V2m is optional, we already
print information for each frame found. I think I would just leave this
one the way it is.
[..]
diff --git a/include/linux/irqchip/arm-gic.h
b/include/linux/irqchip/arm-gic.h
index bae69e5..7398538 100644
--- a/include/linux/irqchip/arm-gic.h
+++ b/include/linux/irqchip/arm-gic.h
@@ -108,6 +108,12 @@ void gic_init(unsigned int nr, int start,
int gicv2m_of_init(struct device_node *node, struct irq_domain
*parent);
+#ifdef CONFIG_ACPI
+#include
I think, we don't need this include here. You already added it to itq-gic.c
Right.
Thanks,
Suravee
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 2/2] irqchip/gic-v2m: Add support for multiple MSI frames
On 10/14/2015 10:28 AM, Marc Zyngier wrote:
On 14/10/15 15:13, Suravee Suthikulanit wrote:
Hi Marc,
On 10/14/2015 6:27 AM, Marc Zyngier wrote:
The GICv2m driver is so far limited to a single MSI frame, but
nothing prevents an implementation from having several of them.
This patch expands the driver to enumerate all frames, keeping
the first one as the canonical identifier for the MSI domains.
Tested-by: Duc Dang
Signed-off-by: Marc Zyngier
---
drivers/irqchip/irq-gic-v2m.c | 124
++
1 file changed, 78 insertions(+), 46 deletions(-)
diff --git a/drivers/irqchip/irq-gic-v2m.c b/drivers/irqchip/irq-gic-v2m.c
index bf9b3c0..87f8d10 100644
--- a/drivers/irqchip/irq-gic-v2m.c
+++ b/drivers/irqchip/irq-gic-v2m.c
@@ -50,8 +50,12 @@
/* List of flags for specific v2m implementation */
#define GICV2M_NEEDS_SPI_OFFSET 0x0001
+static LIST_HEAD(v2m_nodes);
+static DEFINE_SPINLOCK(v2m_lock);
+
struct v2m_data {
- spinlock_t msi_cnt_lock;
+ struct list_head entry;
+ struct device_node *node;
Would it be better if we use struct fwnode_handle * here instead. I
noticed that later on, this is also used as of_node_to_fwnode(v2m->node)
in several places. Also, this would need to change anyways when we
introducing ACPI support (see here https://lkml.org/lkml/2015/10/13/846).
I was thinking that it would be part of your series adapting it to ACPI.
I don't mind either way...
Thanks,
M.
Ok, I'll rebase the GICv2m ACPI support on top of this multi-frame
change and send out V2 If this won't be changing again any time soon.
Thanks,
Suravee
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 2/2] irqchip/gic-v2m: Add support for multiple MSI frames
Hi Marc,
On 10/14/2015 6:27 AM, Marc Zyngier wrote:
The GICv2m driver is so far limited to a single MSI frame, but
nothing prevents an implementation from having several of them.
This patch expands the driver to enumerate all frames, keeping
the first one as the canonical identifier for the MSI domains.
Tested-by: Duc Dang
Signed-off-by: Marc Zyngier
---
drivers/irqchip/irq-gic-v2m.c | 124 ++
1 file changed, 78 insertions(+), 46 deletions(-)
diff --git a/drivers/irqchip/irq-gic-v2m.c b/drivers/irqchip/irq-gic-v2m.c
index bf9b3c0..87f8d10 100644
--- a/drivers/irqchip/irq-gic-v2m.c
+++ b/drivers/irqchip/irq-gic-v2m.c
@@ -50,8 +50,12 @@
/* List of flags for specific v2m implementation */
#define GICV2M_NEEDS_SPI_OFFSET 0x0001
+static LIST_HEAD(v2m_nodes);
+static DEFINE_SPINLOCK(v2m_lock);
+
struct v2m_data {
- spinlock_t msi_cnt_lock;
+ struct list_head entry;
+ struct device_node *node;
Would it be better if we use struct fwnode_handle * here instead. I
noticed that later on, this is also used as of_node_to_fwnode(v2m->node)
in several places. Also, this would need to change anyways when we
introducing ACPI support (see here https://lkml.org/lkml/2015/10/13/846).
Thanks,
Suravee
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V3 2/4] ACPI/scan: Clean up acpi_check_dma
Bjorn / Rafael,
On 10/13/2015 10:52 AM, Suravee Suthikulpanit wrote:
On 09/14/2015 09:34 AM, Bjorn Helgaas wrote:
[..]
I think acpi_check_dma_coherency() is better, but only slightly. It
still doesn't give a hint about the *sense* of the return value. I
think it'd be easier to read if there were two functions, e.g.,
I have been going back-and-forth between the current version, and the
two-function-approach in the past. I can definitely go with this route
if you would prefer. Although, if acpi_dma_is_coherent() == 0, it would
be ambiguous whether DMA is not supported or non-coherent DMA is
supported. Then, we would need to call acpi_dma_is_supported() to find
out. So, that's okay with you?
Thinking about this again, I still think having one API (which can tell
whether DMA is supported or not, and if so whether it is coherent or
non-coherent) would be the least confusing and least error prone.
What if we would just have:
enum dev_dma_type acpi_get_dev_dma_type(struct acpi_device *adev);
where:
enum dev_dma_type {
DEV_DMA_NOT_SUPPORTED,
DEV_DMA_NON_COHERENT,
DEV_DMA_COHERENT,
};
This would probably mean that we should modify drivers/base/property.c
to replace:
bool device_dma_is_coherent()
to:
enum dev_dma_type device_get_dma_type()
We used to discuss the enum approach in the past
(https://lkml.org/lkml/2015/8/25/868). But we only considered at the
ACPI level at the time. Actually, this should also reflect in the
property.c.
At this point, only drivers/crypto/ccp/ccp-platform.c and
drivers/net/ethernet/amd/xgbe/xgbe-main.c are calling the
device_dma_is_coherent(). So, it should be easy to change this API.
Please let me know your opinions, or if you have other suggestions.
Thanks again,
Suravee
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC/RFT PATCH v2] PCI: move pci_read_bridge_bases to the generic PCI layer
For ARM64 PROBE_ONLY and non-PROBE_ONLY modes:
Reviewed and Tested-by: Suravee Suthikulpanit
Please see minor comments below.
On 6/9/2015 4:01 AM, Lorenzo Pieralisi wrote:
When a PCI bus is scanned, upon PCI bridge detection the kernel
has to read the bridge registers to set-up its resources so that
the PCI resource hierarchy can be validated properly.
Most if not all architectures read PCI bridge registers in the
pcibios_fixup_bus hook, that is called by the PCI generic layer
whenever a PCI bus is scanned.
Since pci_read_bridge_bases is an arch agnostic operation (and it
is carried out on all architectures) it can be moved to the generic
PCI layer in order to consolidate code and remove the respective
calls from the architectures back-ends.
The PCI_PROBE_ONLY flag is not checked before calling
pci_read_bridge_buses in the generic layer since reading the bridge
bases is not related to resources assignment; this implies that it
can be carried out safely on PCI_PROBE_ONLY systems too and should
not affect architectures (alpha, mips) that check the PCI_PROBE_ONLY
flag before reading the bridge bases.
In order to validate the resource hierarchy as soon as the resources
themselves are probed (ie read from the bridge), this patch also adds
code to pci_read_bridge_bases that claims the bridge resources, so that
they are validated and inserted in the resource hierarchy as soon as
the bridge bases are probed.
Signed-off-by: Lorenzo Pieralisi
Cc: Ralf Baechle
Cc: James E.J. Bottomley
Cc: Michael Ellerman
Cc: Bjorn Helgaas
Cc: Richard Henderson
Cc: Benjamin Herrenschmidt
Cc: David Howells
Cc: Russell King
Cc: Tony Luck
Cc: David S. Miller
Cc: Ingo Molnar
Cc: Guenter Roeck
Cc: Michal Simek
Cc: Chris Zankel
---
v1->v2:
- Moved pci_read_bridge_bases call to pci_scan_bridge to read bases before
scanning devices
- Added bridge resources claiming
v1: https://lkml.org/lkml/2015/5/21/359
arch/alpha/kernel/pci.c | 7 +--
arch/frv/mb93090-mb00/pci-vdk.c | 2 --
arch/ia64/pci/pci.c | 1 -
arch/microblaze/pci/pci-common.c | 9 +
arch/mips/pci/pci.c | 6 --
arch/mn10300/unit-asb2305/pci.c | 1 -
arch/powerpc/kernel/pci-common.c | 8 +---
arch/x86/pci/common.c| 1 -
arch/xtensa/kernel/pci.c | 4
drivers/parisc/dino.c| 3 ---
drivers/parisc/lba_pci.c | 1 -
drivers/pci/probe.c | 26 ++
12 files changed, 29 insertions(+), 40 deletions(-)
diff --git a/arch/alpha/kernel/pci.c b/arch/alpha/kernel/pci.c
index 82f738e..cded02c 100644
--- a/arch/alpha/kernel/pci.c
+++ b/arch/alpha/kernel/pci.c
@@ -242,12 +242,7 @@ pci_restore_srm_config(void)
void pcibios_fixup_bus(struct pci_bus *bus)
{
- struct pci_dev *dev = bus->self;
-
- if (pci_has_flag(PCI_PROBE_ONLY) && dev &&
- (dev->class >> 8) == PCI_CLASS_BRIDGE_PCI) {
- pci_read_bridge_bases(bus);
- }
+ struct pci_dev *dev;
list_for_each_entry(dev, &bus->devices, bus_list) {
pdev_save_srm_config(dev);
diff --git a/arch/frv/mb93090-mb00/pci-vdk.c b/arch/frv/mb93090-mb00/pci-vdk.c
index f211839..f9c86c4 100644
--- a/arch/frv/mb93090-mb00/pci-vdk.c
+++ b/arch/frv/mb93090-mb00/pci-vdk.c
@@ -294,8 +294,6 @@ void pcibios_fixup_bus(struct pci_bus *bus)
printk("### PCIBIOS_FIXUP_BUS(%d)\n",bus->number);
#endif
- pci_read_bridge_bases(bus);
-
if (bus->number == 0) {
struct pci_dev *dev;
list_for_each_entry(dev, &bus->devices, bus_list) {
diff --git a/arch/ia64/pci/pci.c b/arch/ia64/pci/pci.c
index 7cc3be9..563228b 100644
--- a/arch/ia64/pci/pci.c
+++ b/arch/ia64/pci/pci.c
@@ -534,7 +534,6 @@ void pcibios_fixup_bus(struct pci_bus *b)
struct pci_dev *dev;
if (b->self) {
- pci_read_bridge_bases(b);
pcibios_fixup_bridge_resources(b->self);
}
list_for_each_entry(dev, &b->devices, bus_list)
diff --git a/arch/microblaze/pci/pci-common.c b/arch/microblaze/pci/pci-common.c
index ae838ed..6b8b752 100644
--- a/arch/microblaze/pci/pci-common.c
+++ b/arch/microblaze/pci/pci-common.c
@@ -863,14 +863,7 @@ void pcibios_setup_bus_devices(struct pci_bus *bus)
void pcibios_fixup_bus(struct pci_bus *bus)
{
- /* When called from the generic PCI probe, read PCI<->PCI bridge
-* bases. This is -not- called when generating the PCI tree from
-* the OF device-tree.
-*/
- if (bus->self != NULL)
- pci_read_bridge_bases(bus);
-
- /* Now fixup the bus bus */
+ /* Fixup the bus */
pcibios_setup_bus_self(bus);
/* Now fixup devices on that bus */
diff --git a/arch/mips/pci/pci.c b/arch/mips/pci/pci.c
index b8a0bf5..c6996cf 100644
--- a/arch/mips/pci/pci.c
+++ b/arch/mips/pci/pci.c
@@ -311,12 +311,6 @@ int pcibios_enable_device(struct pci_dev *dev, int mask
Re: [V5 PATCH 2/5] arm64 : Introduce support for ACPI _CCA object
On 5/28/2015 9:38 PM, Mark Salter wrote: On Wed, 2015-05-20 at 17:09 -0500, Suravee Suthikulpanit wrote: >Fromhttp://www.uefi.org/sites/default/files/resources/ACPI_6.0.pdf, >section 6.2.17 _CCA states that ARM platforms require ACPI _CCA >object to be specified for DMA-cabpable devices. Therefore, this patch >specifies ACPI_CCA_REQUIRED in arm64 Kconfig. > >In addition, to handle the case when _CCA is missing, arm64 would assign >dummy_dma_ops to disable DMA capability of the device. > >Acked-by: Catalin Marinas >Signed-off-by: Mark Salter >Signed-off-by: Suravee Suthikulpanit >--- > arch/arm64/Kconfig | 1 + > arch/arm64/include/asm/dma-mapping.h | 18 ++- > arch/arm64/mm/dma-mapping.c | 92 > 3 files changed, 109 insertions(+), 2 deletions(-) > >diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig >index 4269dba..95307b4 100644 >--- a/arch/arm64/Kconfig >+++ b/arch/arm64/Kconfig >@@ -1,5 +1,6 @@ > config ARM64 >def_bool y >+ select ACPI_CCA_REQUIRED if ACPI >select ACPI_GENERIC_GSI if ACPI >select ACPI_REDUCED_HARDWARE_ONLY if ACPI >select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE >diff --git a/arch/arm64/include/asm/dma-mapping.h b/arch/arm64/include/asm/dma-mapping.h >index 9437e3d..f0d6d0b 100644 >--- a/arch/arm64/include/asm/dma-mapping.h >+++ b/arch/arm64/include/asm/dma-mapping.h >@@ -18,6 +18,7 @@ > > #ifdef __KERNEL__ > >+#include > #include > #include > ^^^ This hunk causes build issues with a couple of drivers: drivers/scsi/megaraid/megaraid_sas_fp.c:69:0: warning: "FALSE" redefined [enabled by default] #define FALSE 0 ^ In file included from include/acpi/acpi.h:58:0, from include/linux/acpi.h:37, from ./arch/arm64/include/asm/dma-mapping.h:21, from include/linux/dma-mapping.h:86, from ./arch/arm64/include/asm/pci.h:7, from include/linux/pci.h:1460, from drivers/scsi/megaraid/megaraid_sas_fp.c:37: include/acpi/actypes.h:433:0: note: this is the location of the previous definition #define FALSE (1 == 0) ^ In file included from include/acpi/acpi.h:58:0, from include/linux/acpi.h:37, from ./arch/arm64/include/asm/dma-mapping.h:21, from include/linux/dma-mapping.h:86, from include/scsi/scsi_cmnd.h:4, from drivers/scsi/ufs/ufshcd.h:60, from drivers/scsi/ufs/ufshcd.c:43: include/acpi/actypes.h:433:41: error: expected identifier before ‘(’ token #define FALSE (1 == 0) ^ drivers/scsi/ufs/unipro.h:203:2: note: in expansion of macro ‘FALSE’ FALSE = 0, ^ This happens because the ACPI definitions of TRUE and FALSE conflict with local definitions in megaraid and enum declaration in ufs. Mark, Thanks for pointing this out. Although, I would think that the megaraid_sas_fp.c should have had the #ifndef to check before defining the TRUE and FALSE as following. #ifndef TRUE #define TRUE 1 #endif #ifndef FALSE #define FALSE 0 #endif This seems to be what other drivers are also doing. If this is okay, I can send out a fix-up patch for the megaraid driver. Thanks, Suravee -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC/RFT PATCH] PCI: move pci_read_bridge_bases to the generic PCI layer
On 5/27/2015 12:54 PM, Lorenzo Pieralisi wrote:
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> >index 062fee6..335d9f2 100644
> >--- a/drivers/pci/probe.c
> >+++ b/drivers/pci/probe.c
> >@@ -453,7 +453,11 @@ void pci_read_bridge_bases(struct pci_bus *child)
> > struct resource *res;
> > int i;
> >
> >- if (pci_is_root_bus(child)) /* It's a host bus, nothing to read */
> >+ /*
> >+ * If it is not a PCI bridge there is nothing to read
> >+ */
> >+ if (pci_is_root_bus(child) || !dev ||
> >+ !((dev->class >> 8) == PCI_CLASS_BRIDGE_PCI))
> > return;
> >
> > dev_info(&dev->dev, "PCI bridge to %pR%s\n",
> >@@ -1878,6 +1882,11 @@ unsigned int pci_scan_child_bus(struct pci_bus *bus)
> >* all PCI-to-PCI bridges on this bus.
> >*/
> > if (!bus->is_added) {
> >+ /*
> >+ * Read and initialize bridge resources.
> >+ */
> >+ pci_read_bridge_bases(bus);
> >+
> > dev_dbg(&bus->dev, "fixups for bus\n");
> > pcibios_fixup_bus(bus);
> > bus->is_added = 1;
> >
>
>So, I have tested the patch on ARM64 system w/ PROBE_ONLY mode, and
>noticed that we are calling pci_read_bridge_bases() after adding the
>devices on the slots. This is not soon enough since the downstream
>devices still failing to claim resources.
>
>However, do you think we can move pci_read_bridge_bases() before the
>pci_scan_slot() loop?
Right, how about moving it to pci_scan_bridge() before calling the
respective pci_scan_child_bus() ? I think it belongs there anyway.
Thanks,
That seems reasonable. I test putting it here in pci_scan_bridge(),
and it works.
child = pci_find_bus(pci_domain_nr(bus), secondary);
if (!child) {
child = pci_add_new_bus(bus, dev, secondary);
if (!child)
goto out;
child->primary = primary;
pci_bus_insert_busn_res(child, secondary, subordinate);
child->bridge_ctl = bctl;
pci_read_bridge_bases(child);
}
cmax = pci_scan_child_bus(child);
.
Thanks,
Suravee
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC/RFT PATCH] PCI: move pci_read_bridge_bases to the generic PCI layer
Hi Lorenzo,
Sorry for late reply.
On 5/21/2015 8:14 AM, Lorenzo Pieralisi wrote:
When a PCI bus is scanned, upon PCI bridge detection the kernel
has to read the bridge registers to set-up its resources so that
the PCI resource hierarchy can be validated properly.
Most if not all architectures read PCI bridge registers in the
pcibios_fixup_bus hook, that is called by the PCI generic layer
whenever a PCI bus is scanned.
Since pci_read_bridge_bases is an arch agnostic operation (and it
is carried out on all architectures) it can be moved to the generic
PCI layer in order to consolidate code and remove the respective
calls from the architectures back-ends.
The PCI_PROBE_ONLY flag is not checked before calling
pci_read_bridge_buses in the generic layer since reading the bridge
bases is not related to resources assignment; this implies that it
can be carried out safely on PCI_PROBE_ONLY systems too and should
not affect architectures (alpha, mips) that check the PCI_PROBE_ONLY
flag before reading the bridge bases.
Signed-off-by: Lorenzo Pieralisi
Cc: Ralf Baechle
Cc: James E.J. Bottomley
Cc: Michael Ellerman
Cc: Bjorn Helgaas
Cc: Richard Henderson
Cc: Benjamin Herrenschmidt
Cc: David Howells
Cc: Russell King
Cc: Tony Luck
Cc: David S. Miller
Cc: Ingo Molnar
Cc: Michal Simek
Cc: Koichi Yasutake
Cc: Chris Zankel
---
arch/alpha/kernel/pci.c | 7 +--
arch/frv/mb93090-mb00/pci-vdk.c | 2 --
arch/ia64/pci/pci.c | 1 -
arch/microblaze/pci/pci-common.c | 9 +
arch/mips/pci/pci.c | 6 --
arch/mn10300/unit-asb2305/pci.c | 1 -
arch/powerpc/kernel/pci-common.c | 8 +---
arch/x86/pci/common.c| 1 -
arch/xtensa/kernel/pci.c | 4
drivers/parisc/dino.c| 3 ---
drivers/parisc/lba_pci.c | 1 -
drivers/pci/probe.c | 11 ++-
12 files changed, 13 insertions(+), 41 deletions(-)
[.]
>
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 062fee6..335d9f2 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -453,7 +453,11 @@ void pci_read_bridge_bases(struct pci_bus *child)
struct resource *res;
int i;
- if (pci_is_root_bus(child)) /* It's a host bus, nothing to read */
+ /*
+* If it is not a PCI bridge there is nothing to read
+*/
+ if (pci_is_root_bus(child) || !dev ||
+ !((dev->class >> 8) == PCI_CLASS_BRIDGE_PCI))
return;
dev_info(&dev->dev, "PCI bridge to %pR%s\n",
@@ -1878,6 +1882,11 @@ unsigned int pci_scan_child_bus(struct pci_bus *bus)
* all PCI-to-PCI bridges on this bus.
*/
if (!bus->is_added) {
+ /*
+* Read and initialize bridge resources.
+*/
+ pci_read_bridge_bases(bus);
+
dev_dbg(&bus->dev, "fixups for bus\n");
pcibios_fixup_bus(bus);
bus->is_added = 1;
So, I have tested the patch on ARM64 system w/ PROBE_ONLY mode, and
noticed that we are calling pci_read_bridge_bases() after adding the
devices on the slots. This is not soon enough since the downstream
devices still failing to claim resources.
However, do you think we can move pci_read_bridge_bases() before the
pci_scan_slot() loop?
Thanks,
Suravee
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [V5 PATCH 1/5] ACPI / scan: Parse _CCA and setup device coherency
On 5/22/2015 8:25 PM, Rafael J. Wysocki wrote:
On Friday, May 22, 2015 07:15:17 PM Suravee Suthikulanit wrote:
On 5/22/2015 6:05 PM, Rafael J. Wysocki wrote:
On Friday, May 22, 2015 05:24:15 PM Suravee Suthikulanit wrote:
Not sure if this went out earlier. So I am resending.
On 5/22/15 16:56, Rafael J. Wysocki wrote:
diff --git a/drivers/acpi/glue.c b/drivers/acpi/glue.c
index 39c485b..b9657af 100644
--- a/drivers/acpi/glue.c
+++ b/drivers/acpi/glue.c
@@ -13,6 +13,7 @@
#include
#include
#include
+#include
#include "internal.h"
@@ -167,6 +168,7 @@ int acpi_bind_one(struct device *dev, struct acpi_device
*acpi_dev)
struct list_head *physnode_list;
unsigned int node_id;
int retval = -EINVAL;
+ bool coherent;
if (has_acpi_companion(dev)) {
if (acpi_dev) {
@@ -223,6 +225,9 @@ int acpi_bind_one(struct device *dev, struct acpi_device
*acpi_dev)
if (!has_acpi_companion(dev))
ACPI_COMPANION_SET(dev, acpi_dev);
+ if (acpi_check_dma(acpi_dev, &coherent))
+ arch_setup_dma_ops(dev, 0, 0, NULL, coherent);
+
Well, so is this going to work for PCI too after all?
No, as Bjorn suggested, PCI changes for setting DMA coherent from _CCA
(patch 3/6 in V4) will be submitted separately. We are working on
cleaning up and up-streaming the PCI ACPI support for ARM64.
OK, but acpi_bind_one() is called for PCI devices too. Won't that be a problem?
>
In this case, we would be going through the following call path:
--> pci_device_add()
|--> pci_dma_configure() ** 1 **
|--> device_add()
|--> platform_notify()
|--> acpi_platform_notify()
|--> acpi_bind_one() ** 2 **
At (1), we would be calling arch_setup_dma_ops() with the PCI host
bridge _CCA information. So, it should have already taken care of
setting up device coherency here.
At (2), if there is no acpi_dev for endpoint devices (which I believe
this is normally the case), it would return early and skip
arch_setup_dma_ops().
That's not correct. There may be ACPI companions for endpoint devices too.
Ok. Duly noted :)
At (2), if there is an acpi_dev, the coherent_dma flag should have
already been setup by the acpi_init_device_object during ACPI scan.
That one sets the flag for the *ACPI* *companion* of the device, which
I'm still thinking is pointless, isn't it?
When you say pointless, are you referring to the part where we are end
up calling arch_setup_dma_coherent() twice in this case? I am not quite
following your point.
However, I am not certain about this case since I don't have the DSDT
containing PCI endpoint devices to test with.
Every x86 PC has them (as far as I can say), but in that case there's no
_CCA and they are all coherent.
Ok.
Thanks,
Suravee
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [V5 PATCH 1/5] ACPI / scan: Parse _CCA and setup device coherency
On 5/22/2015 6:05 PM, Rafael J. Wysocki wrote:
On Friday, May 22, 2015 05:24:15 PM Suravee Suthikulanit wrote:
Not sure if this went out earlier. So I am resending.
On 5/22/15 16:56, Rafael J. Wysocki wrote:
diff --git a/drivers/acpi/glue.c b/drivers/acpi/glue.c
index 39c485b..b9657af 100644
--- a/drivers/acpi/glue.c
+++ b/drivers/acpi/glue.c
@@ -13,6 +13,7 @@
#include
#include
#include
+#include
#include "internal.h"
@@ -167,6 +168,7 @@ int acpi_bind_one(struct device *dev, struct acpi_device
*acpi_dev)
struct list_head *physnode_list;
unsigned int node_id;
int retval = -EINVAL;
+ bool coherent;
if (has_acpi_companion(dev)) {
if (acpi_dev) {
@@ -223,6 +225,9 @@ int acpi_bind_one(struct device *dev, struct acpi_device
*acpi_dev)
if (!has_acpi_companion(dev))
ACPI_COMPANION_SET(dev, acpi_dev);
+ if (acpi_check_dma(acpi_dev, &coherent))
+ arch_setup_dma_ops(dev, 0, 0, NULL, coherent);
+
Well, so is this going to work for PCI too after all?
No, as Bjorn suggested, PCI changes for setting DMA coherent from _CCA
(patch 3/6 in V4) will be submitted separately. We are working on
cleaning up and up-streaming the PCI ACPI support for ARM64.
OK, but acpi_bind_one() is called for PCI devices too. Won't that be a problem?
>
In this case, we would be going through the following call path:
--> pci_device_add()
|--> pci_dma_configure() ** 1 **
|--> device_add()
|--> platform_notify()
|--> acpi_platform_notify()
|--> acpi_bind_one() ** 2 **
At (1), we would be calling arch_setup_dma_ops() with the PCI host
bridge _CCA information. So, it should have already taken care of
setting up device coherency here.
At (2), if there is no acpi_dev for endpoint devices (which I believe
this is normally the case), it would return early and skip
arch_setup_dma_ops().
At (2), if there is an acpi_dev, the coherent_dma flag should have
already been setup by the acpi_init_device_object during ACPI scan.
However, I am not certain about this case since I don't have the DSDT
containing PCI endpoint devices to test with.
Thanks,
Suravee
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [V5 PATCH 1/5] ACPI / scan: Parse _CCA and setup device coherency
Not sure if this went out earlier. So I am resending.
On 5/22/15 16:56, Rafael J. Wysocki wrote:
diff --git a/drivers/acpi/glue.c b/drivers/acpi/glue.c
>index 39c485b..b9657af 100644
>--- a/drivers/acpi/glue.c
>+++ b/drivers/acpi/glue.c
>@@ -13,6 +13,7 @@
> #include
> #include
> #include
>+#include
>
> #include "internal.h"
>
>@@ -167,6 +168,7 @@ int acpi_bind_one(struct device *dev, struct acpi_device
*acpi_dev)
>struct list_head *physnode_list;
>unsigned int node_id;
>int retval = -EINVAL;
>+ bool coherent;
>
>if (has_acpi_companion(dev)) {
>if (acpi_dev) {
>@@ -223,6 +225,9 @@ int acpi_bind_one(struct device *dev, struct acpi_device
*acpi_dev)
>if (!has_acpi_companion(dev))
>ACPI_COMPANION_SET(dev, acpi_dev);
>
>+ if (acpi_check_dma(acpi_dev, &coherent))
>+ arch_setup_dma_ops(dev, 0, 0, NULL, coherent);
>+
Well, so is this going to work for PCI too after all?
No, as Bjorn suggested, PCI changes for setting DMA coherent from _CCA
(patch 3/6 in V4) will be submitted separately. We are working on
cleaning up and up-streaming the PCI ACPI support for ARM64.
Thanks,
Suravee
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [V4 PATCH 4/6] device property: Introduces device_dma_is_coherent()
On 5/20/2015 5:28 AM, Will Deacon wrote:
On Fri, May 15, 2015 at 10:23:12PM +0100, Suravee Suthikulpanit wrote:
Currently, device drivers, which support both OF and ACPI,
need to call two separate APIs, of_dma_is_coherent() and
acpi_dma_is_coherent()) to determine device coherency attribute.
This patch simplifies this process by introducing a new device
property API, device_dma_is_coherent(), which calls the appropriate
interface based on the booting architecture.
Signed-off-by: Suravee Suthikulpanit
CC: Rafael J. Wysocki
---
drivers/base/property.c | 12
include/linux/property.h | 2 ++
2 files changed, 14 insertions(+)
diff --git a/drivers/base/property.c b/drivers/base/property.c
index 1d0b116..8123c6e 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -14,6 +14,7 @@
#include
#include
#include
+#include
#include
/**
@@ -519,3 +520,14 @@ unsigned int device_get_child_node_count(struct device
*dev)
return count;
}
EXPORT_SYMBOL_GPL(device_get_child_node_count);
+
+bool device_dma_is_coherent(struct device *dev)
+{
+ if (IS_ENABLED(CONFIG_OF) && dev->of_node)
+ return of_dma_is_coherent(dev->of_node);
+ else if (has_acpi_companion(dev))
+ return acpi_dma_is_coherent(acpi_node(dev->fwnode));
I don't think you need the has_acpi_companion check, as acpi_node handles
this and acpi_dma_is_coherent(NULL) returns false.
Will
You are right.
Thanks,
Suravee
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [V4 PATCH 3/6] pci: Generic function for setting up PCI device DMA coherency
On 5/20/2015 4:34 AM, Catalin Marinas wrote:
On Wed, May 20, 2015 at 11:27:54AM +0200, Arnd Bergmann wrote:
On Wednesday 20 May 2015 10:24:15 Catalin Marinas wrote:
On Sat, May 16, 2015 at 01:59:00AM +0200, Rafael J. Wysocki wrote:
On Friday, May 15, 2015 04:23:11 PM Suravee Suthikulpanit wrote:
+/**
+ * pci_dma_configure - Setup DMA configuration
+ * @pci_dev: ptr to pci_dev struct of the PCI device
+ *
+ * Function to update PCI devices's DMA configuration using the same
+ * info from the OF node or ACPI node of host bridge's parent (if any).
+ */
+static void pci_dma_configure(struct pci_dev *pci_dev)
+{
+ struct device *dev = &pci_dev->dev;
+ struct device *bridge = pci_get_host_bridge_device(pci_dev);
+ struct device *host = bridge->parent;
+ struct acpi_device *adev;
+
+ if (!host)
+ return;
+
+ if (acpi_disabled) {
+ of_dma_configure(dev, host->of_node);
I'd rather do
if (IS_ENABLED(CONFIG_OF) && host->of_node) {
of_dma_configure(dev, host->of_node);
Nitpick: do we need the CONFIG_OF check? If disabled, I don't think
anyone would set host->of_node.
If of_dma_configure() is defined in a file that is built conditionally
based on CONFIG_OF, you need it.
We have a dummy of_dma_configure() already when !CONFIG_OF, otherwise
we would need #ifndef here. I already replied, I think for other
architectures we need this check to avoid a useless host->of_node test.
It seems that there are several places that have similar check. Would it
be good to convert this into a macro? Something like:
#define OF_NODE_ENABLED(dev)(IS_ENABLED(CONFIG_OF) && dev->of_node)
Thanks all for the review feedback.
Suravee
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [V4 PATCH 1/6] ACPI / scan: Parse _CCA and setup device coherency
On 5/20/2015 5:01 AM, Catalin Marinas wrote:
On Fri, May 15, 2015 at 04:23:09PM -0500, Suravee Suthikulpanit wrote:
+static inline bool acpi_dma_is_supported(struct acpi_device *adev)
+{
+ /**
+* Currently, we mainly support _CCA=1 (i.e. is_coherent=1)
+* This should be equivalent to specifyig dma-coherent for
+* a device in OF.
+*
+* For the case when _CCA=0 (i.e. is_coherent=0 && cca_seen=1),
+* There are two approaches:
+* 1. Do not support and disable DMA.
+* 2. Support but rely on arch-specific cache maintenance for
+* non-coherence DMA operations. ARM64 is one example.
+*
+* For the case when _CCA is missing (i.e. cca_seen=0) but
+* platform specifies ACPI_CCA_REQUIRED, we do not support DMA,
+* and fallback to arch-specific default handling.
+*
+* See acpi_init_coherency() for more info.
+*/
+ return adev && (adev->flags.is_coherent ||
+ (adev->flags.cca_seen && IS_ENABLED(CONFIG_ARM64)));
+}
I don't particularly like the check for CONFIG_ARM64 here but I
understand why it was added (I had the wrong impression that x86 can
cope with _CCA = 0).
Alternatively, we could leave it out (together with cca_seen) until
someone comes forward with a real use-case for _CCA = 0 on arm64. One
platform I'm aware of is Juno but even though it boot with ACPI, I
wouldn't call it a server platform.
Ok. That seems to be what Arnd would prefer as well. Let's just leave
the support for _CCA=0 out until it is needed then.
Thanks,
Suravee
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [V4 PATCH 2/6] arm64 : Introduce support for ACPI _CCA object
On 5/20/2015 5:03 AM, Catalin Marinas wrote: On Fri, May 15, 2015 at 04:23:10PM -0500, Suravee Suthikulpanit wrote: From http://www.uefi.org/sites/default/files/resources/ACPI_6.0.pdf, section 6.2.17 _CCA states that ARM platforms require ACPI _CCA object to be specified for DMA-cabpable devices. This patch introduces ACPI_MUST_HAVE_CCA in arm64 Kconfig to specify such requirement. In case _CCA is missing, arm64 would assign dummy_dma_ops to disable DMA capability of the device. Signed-off-by: Mark Salter Signed-off-by: Suravee Suthikulpanit CC: Catalin Marinas CC: Will Deacon CC: Arnd Bergmann Acked-by: Catalin Marinas Thanks, Suravee -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [V4 PATCH 1/6] ACPI / scan: Parse _CCA and setup device coherency
Hi Rafael,
On 5/15/2015 6:53 PM, Rafael J. Wysocki wrote:
On Friday, May 15, 2015 04:23:09 PM Suravee Suthikulpanit wrote:
[...]
diff --git a/drivers/acpi/acpi_platform.c b/drivers/acpi/acpi_platform.c
index 4bf7559..f6bc438 100644
--- a/drivers/acpi/acpi_platform.c
+++ b/drivers/acpi/acpi_platform.c
@@ -103,14 +103,18 @@ struct platform_device
*acpi_create_platform_device(struct acpi_device *adev)
pdevinfo.res = resources;
pdevinfo.num_res = count;
pdevinfo.fwnode = acpi_fwnode_handle(adev);
- pdevinfo.dma_mask = DMA_BIT_MASK(32);
+ pdevinfo.dma_mask = acpi_dma_is_supported(adev) ? DMA_BIT_MASK(32) : 0;
pdev = platform_device_register_full(&pdevinfo);
- if (IS_ERR(pdev))
+ if (IS_ERR(pdev)) {
dev_err(&adev->dev, "platform device creation failed: %ld\n",
PTR_ERR(pdev));
- else
+ } else {
+ if (acpi_dma_is_supported(adev))
+ arch_setup_dma_ops(&pdev->dev, 0, 0, NULL,
+ acpi_dma_is_coherent(adev));
Shouldn't we generally do that in acpi_bind_one() for all bus types
that don't have specific handling rather than here?
I think that would also work, and makes sense. However, I'm not sure if
this would help in the case when we are creating PCI end-point devices,
since the CCA is specified at the host bridge node, and there is no ACPI
companion for the end-point devices. It seems that patch 3/6 of this
series is still needed.
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 849b699..c56e66a 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -11,6 +11,7 @@
#include
#include
#include
+#include
#include
@@ -2137,6 +2138,44 @@ void acpi_free_pnp_ids(struct acpi_device_pnp *pnp)
kfree(pnp->unique_id);
}
+static void acpi_init_coherency(struct acpi_device *adev)
+{
+ unsigned long long cca = 0;
+ acpi_status status;
+ struct acpi_device *parent = adev->parent;
+ struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
+
+ if (parent && parent->flags.cca_seen) {
+ /*
+* From ACPI spec, OSPM will ignore _CCA if an ancestor
+* already saw one.
+*/
+ adev->flags.cca_seen = 1;
+ cca = acpi_dma_is_coherent(parent);
Shouldn't the device's own _CCA take precedence?
According to the ACPI specification, the parent's _CCA take precedence.
+ } else {
+ status = acpi_evaluate_integer(adev->handle, "_CCA",
+ NULL, &cca);
+ if (ACPI_SUCCESS(status)) {
+ adev->flags.cca_seen = 1;
+ } else if (!IS_ENABLED(CONFIG_ACPI_CCA_REQUIRED)) {
+ /*
+* If architecture does not specify that _CCA is
+* required for DMA-able devices (e.g. x86),
+* we default to _CCA=1.
+*/
+ cca = 1;
+ } else {
What about using acpi_handle_debug() here?
Ok I can do that.
[...]
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 8de4fa9..2a05ffb 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -208,7 +208,9 @@ struct acpi_device_flags {
u32 visited:1;
u32 hotplug_notify:1;
u32 is_dock_station:1;
- u32 reserved:23;
+ u32 is_coherent:1;
I'd prefer to call this 'coherent_dma'.
OK.
Thanks,
Suravee
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [V3 PATCH 1/5] ACPI / scan: Parse _CCA and setup device coherency
On 5/11/2015 8:20 PM, Rafael J. Wysocki wrote:
On Monday, May 11, 2015 05:16:27 PM Catalin Marinas wrote:
On Fri, May 08, 2015 at 10:53:59PM +0200, Rafael J. Wysocki wrote:
On Thursday, May 07, 2015 07:37:12 PM Suravee Suthikulpanit wrote:
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index ab2cbb5..7822149 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -54,6 +54,12 @@ config ACPI_GENERIC_GSI
config ACPI_SYSTEM_POWER_STATES_SUPPORT
bool
+config ACPI_CCA_REQUIRED
+ bool
+
+config ARM64_SUPPORT_ACPI_CCA_ZERO
Hmm. I guess the Arnd's idea what to simply use CONFIG_ARM64 directly instead
of adding this new option.
I agree.
+static inline bool acpi_dma_is_supported(struct acpi_device *adev)
+{
+ /**
+* Currently, we mainly support _CCA=1 (i.e. is_coherent=1)
+* This should be equivalent to specifyig dma-coherent for
+* a device in OF.
+*
+* For the case when _CCA=0 (i.e. is_coherent=0 && cca_seen=1),
+* we would rely on arch-specific cache maintenance for
+* non-coherence DMA operations if architecture specifies
+* _XXX_SUPPORT_CCA_ZERO. Otherwise, we do not support
+* DMA on this device and fallback to arch-specific default
+* handling.
+*
+* For the case when _CCA is missing (i.e. cca_seen=0) but
+* platform specifies ACPI_CCA_REQUIRED, we do not support DMA,
+* and fallback to arch-specific default handling.
+*/
+ return adev && (adev->flags.is_coherent ||
+ (adev->flags.cca_seen &&
+IS_ENABLED(CONFIG_ARM64_SUPPORT_ACPI_CCA_ZERO)));
So what exactly would be wrong with using IS_ENABLED(CONFIG_ARM64) here?
I'm not sure I follow why we need to check for ARM64 here at all. Can we
not just have something like:
return adev && (!IS_ENABLED(CONFIG_ACPI_CCA_REQUIRED) ||
adev->flags.cca_seen)
If _CCA returns 0 on non-ARM64, DMA is not supported for this device, so
in that case the function should return 'false' while the above check will
make it return 'true'.
The main idea is basically to allow architecture to decide if it wants
to specify if it wants to support _CCA=0. Currently, there are two
approaches.
1. Do not support and disable DMA
2. Support and default to what architecture would normally do for
non-coherent DMA.
Since, ARM64 being the only platform, which supports ACPI and would
support _CCA=0. I can just put CONFIG_ARM64 then as Arnd and Rafael
mentioned.
Thanks,
Suravee
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [V2 PATCH 3/5] device property: Introduces device_dma_is_coherent()
Rafael,
Any comments on this patch?
Thanks,
Suravee
On 5/5/2015 10:12 AM, Suravee Suthikulpanit wrote:
Currently, device drivers, which support both OF and ACPI,
need to call two separate APIs, of_dma_is_coherent() and
acpi_dma_is_coherent()) to determine device coherency attribute.
This patch simplifies this process by introducing a new device
property API, device_dma_is_coherent(), which calls the appropriate
interface based on the booting architecture.
Signed-off-by: Suravee Suthikulpanit
---
drivers/base/property.c | 12
include/linux/property.h | 2 ++
2 files changed, 14 insertions(+)
diff --git a/drivers/base/property.c b/drivers/base/property.c
index 1d0b116..8123c6e 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -14,6 +14,7 @@
#include
#include
#include
+#include
#include
/**
@@ -519,3 +520,14 @@ unsigned int device_get_child_node_count(struct device
*dev)
return count;
}
EXPORT_SYMBOL_GPL(device_get_child_node_count);
+
+bool device_dma_is_coherent(struct device *dev)
+{
+ if (IS_ENABLED(CONFIG_OF) && dev->of_node)
+ return of_dma_is_coherent(dev->of_node);
+ else if (has_acpi_companion(dev))
+ return acpi_dma_is_coherent(acpi_node(dev->fwnode));
+
+ return false;
+}
+EXPORT_SYMBOL_GPL(device_dma_is_coherent);
diff --git a/include/linux/property.h b/include/linux/property.h
index de8bdf4..76ebde9 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -164,4 +164,6 @@ struct property_set {
void device_add_property_set(struct device *dev, struct property_set *pset);
+bool device_dma_is_coherent(struct device *dev);
+
#endif /* _LINUX_PROPERTY_H_ */
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [V2 PATCH 1/5] ACPI / scan: Parse _CCA and setup device coherency
On 5/6/2015 5:21 PM, Rafael J. Wysocki wrote:
> >>+ bool
> >>+
> >>+config ACPI_SUPPORT_CCA_ZERO
> >
> >I guess this means "we support devices that can DMA, but are not coherent".
> >right?
>
>Yes, basically when _CCA=0.
So what about
ARCH_SUPPORT_CACHE_INCOHERENT_DMA
Since this is specific to ACPI _CCA, I just want to be clear with the
naming.
or something similar?
> >>+ bool
> >>+
> >> config ACPI_SLEEP
> >> bool
> >> depends on SUSPEND || HIBERNATION
> >>diff --git a/drivers/acpi/acpi_platform.c b/drivers/acpi/acpi_platform.c
> >>index 4bf7559..a6feca4 100644
> >>--- a/drivers/acpi/acpi_platform.c
> >>+++ b/drivers/acpi/acpi_platform.c
> >>@@ -108,9 +108,11 @@ struct platform_device
*acpi_create_platform_device(struct acpi_device *adev)
> >> if (IS_ERR(pdev))
> >> dev_err(&adev->dev, "platform device creation failed:
%ld\n",
> >> PTR_ERR(pdev));
> >>- else
> >>+ else {
> >
> >Please add braces to both branches when making such changes (as per
CodingStyle).
> >
>
>OK.
>
> >>+ acpi_setup_device_dma(adev, &pdev->dev);
> >
> >Why do we need to do that here (for the second time)?
>
>Because we are calling:
>acpi_create_platform_device()
> |--> platform_device_register_device_full()
>|-->platform_device_alloc()
>
>This creates platform_device, which allocate a new platform_device->dev.
>This is not the same as the original acpi_device->dev that was created
>during acpi_add_single_object(). So, we have to set up the device
>coherency again.
Ah, so the second arg is different now.
Well, in that case, why do we need to set it up for the adev's dev member?
Just for sanity, since I don't know if adev->dev will be referenced
anywhere else. This way, it's consistent for all copied of struct device
generated.
Lemme know if you think that is unnecessary.
Thanks,
Suravee
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [V2 PATCH 2/5] arm64 : Introduce support for ACPI _CCA object
On 5/6/2015 5:08 AM, Robin Murphy wrote:
[...]
+static void __dummy_sync_single_for_cpu(struct device *dev,
+dma_addr_t dev_addr, size_t size,
+enum dma_data_direction dir)
+{
+}
+
+static void __dummy_sync_single_for_device(struct device *dev,
+ dma_addr_t dev_addr, size_t size,
+ enum dma_data_direction dir)
+{
+}
Minor point, but I don't see the need to have multiple dummy functions
with identical signatures - just have a generic dummy_sync_single and
assign it to both ops.
+static void __dummy_sync_sg_for_cpu(struct device *dev,
+struct scatterlist *sgl, int nelems,
+enum dma_data_direction dir)
+{
+}
+
+static void __dummy_sync_sg_for_device(struct device *dev,
+ struct scatterlist *sgl, int nelems,
+ enum dma_data_direction dir)
+{
+}
Ditto here with dummy_sync_sg.
Hi Robin,
Good point. I'll take care of that in V3.
I wonder if there's any argument for putting the dummy DMA ops somewhere
common, like drivers/base/dma-mapping.c?
Robin.
Hm.. If this approach will be adopted by other architectures, then it
would make sense. Currently, this is only used by arm64. So, I think it
is okay to leave this here for now.
Thanks,
Suravee
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [V2 PATCH 2/5] arm64 : Introduce support for ACPI _CCA object
On 5/5/2015 10:44 AM, Arnd Bergmann wrote:
On Tuesday 05 May 2015 10:12:06 Suravee Suthikulpanit wrote:
+struct dma_map_ops dummy_dma_ops = {
+ .alloc = __dummy_alloc,
+ .free = __dummy_free,
+ .mmap = __dummy_mmap,
+ .map_page = __dummy_map_page,
+ .unmap_page = __dummy_unmap_page,
+ .map_sg = __dummy_map_sg,
+ .unmap_sg = __dummy_unmap_sg,
+ .sync_single_for_cpu= __dummy_sync_single_for_cpu,
+ .sync_single_for_device = __dummy_sync_single_for_device,
+ .sync_sg_for_cpu= __dummy_sync_sg_for_cpu,
+ .sync_sg_for_device = __dummy_sync_sg_for_device,
+ .mapping_error = __dummy_mapping_error,
+ .dma_supported = __dummy_dma_supported,
+};
+EXPORT_SYMBOL(dummy_dma_ops);
+
This will clearly work, but I think it's easier to just leave
the dma_mask pointer as NULL when creating the platform device,
which should let the normal dma ops fail all the callbacks.
Arnd
However, codes in several places are making use of dma_map_ops without
checking if the ops are NULL (i.e.
include/asm-generic/dma-mapping-common.h and in arch-specific
implementation). If setting it to NULL is what we are planning to
support, we would need to scrub the current code to put NULL check.
Also, would you consider if that is safe to do going forward?
Thanks,
Suravee
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Linaro-acpi] [V2 PATCH 2/5] arm64 : Introduce support for ACPI _CCA object
On 5/5/2015 11:12 AM, Arnd Bergmann wrote: On Tuesday 05 May 2015 11:09:38 Suravee Suthikulanit wrote: However, codes in several places are making use of dma_map_ops without checking if the ops are NULL (i.e. include/asm-generic/dma-mapping-common.h and in arch-specific implementation). If setting it to NULL is what we are planning to support, we would need to scrub the current code to put NULL check. Also, would you consider if that is safe to do going forward? I mean the dma_mask pointer, not dma_map_ops. Arnd Ah, got it. Sorry for confusion. Suravee -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Linaro-acpi] [PATCH 2/2] ACPI / scan: Parse _CCA and setup device coherency
On 4/30/2015 3:23 AM, Arnd Bergmann wrote:
On Wednesday 29 April 2015 16:53:10 Suravee Suthikulpanit wrote:
On 4/29/15 11:25, Arnd Bergmann wrote:
On Wednesday 29 April 2015 08:44:09 Suravee Suthikulpanit wrote:
[...]
As for the case where _CCA=0, I think the ACPI driver should essentially
communicate the information as HW is non-coherent as described in the
spec, and should be calling arch_setup_dma_ops(dev, false). It is true
that this in probably less-likely for the ARM64 server platforms.
However, I would think that the ACPI driver should not be making such
assumption.
Can you add a description to the ACPI spec then to describe in detail what
"non-coherent" is supposed to mean, and which action the OS is supposed to
take when accessing data from device or CPU?
I believe Will has already provided this, and we have already discussed
this on separate emails in this thread.
[...]
On a related note, I'm not sure how to handle different DMA masks here.
arch_setup_dma_ops() gets passed a size (and offset) argument, which should
match the DMA mask, but I don't know if there is a way to find out the
size from ACPI. Should we assume it's always 64-bit DMA capable?
Looking at the ACPI spec, it does have the _DMA object. IIUC, this can
be used to describe DMA properties of a particular bus.
Method(_DMA, ResourceTemplate()
{
QWORDMemory(
ResourceConsumer,
PosDecode, // _DEC
MinFixed, // _MIF
MaxFixed, // _MAF
Prefetchable, // _MEM
ReadWrite, // _RW
0, // _GRA
0, // _MIN
0x1fff, // _MAX
0x2, // _TRA
0x2000, // _LEN
, , ,
)
}
I am not sure if this is an appropriate use for this object, but this
seems to be similar to the dma-ranges property for OF, and probably can
be used to specify baseaddr and size information when calling
arch_setup_dma_ops().
Yes, that seems like a good idea. What is the expected behavior when that
object is absent? Do we assume that the parent device is not DMA capable?
From the spec:
If the _DMA object is not present for a bus device, the OS assumes that
any address placed on a bus by a child device will be decoded either by
a device on the bus or by the bus itself, (in other words, all address
ranges can be used for DMA).
The issue is, since this is optional, I don't know which FW often
providing this info.
Is this sufficient to describe the case where a device can only do DMA
to a specific address range that is not at bus address zero but that maps
to the beginning of physical RAM?
I believe that's the _MIN (Minimum Base Address) is for.
For legacy reasons, the default mask is probably best left at 32-bit,
but drivers are expected to call dma_set_mask() if they can do 64-bit DMA,
and that should fail based on the information provided by the platform
if the bus is not capable of doing that.
However, on ARM64 the dma_base and size parameter for
arch_setup_dma_ops() is currently not used, and only coherent flag is
used.
We can hope that we won't need the dma_base setting here, but it's
good to have the option to pass it down if we need it.
Not passing the size is a bug that needs to be fixed ASAP, I believe
a number of folks have run into this, most recently the APM X-Gene
MMC controller
Ok. I'll look at this separately.
We probably should look at this separately. For the moment, we can
probably say that if _CCA object is missing when needed, the ACPI driver
won't set up dma_mask when creating platform_device, which should be
equivalent to saying DMA is not supported.
Please let me know if this is acceptable, and I'll make change in V2
accordingly.
I would still ask that you treat non-coherent to mean "no DMA" until
we have come up with a way to sufficiently describe the kind of
non-coherency in ACPI.
Arnd
Ok. In V2, when _CCA=0, since we are not aware of ARM64 systems that is
working with such assumption with ACPI. I will also default to not
calling arch_setup_dma_ops() and fallback to arch-specific default. We
can revisit this later once we need to support such case.
Thanks,
Suravee
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [V8 PATCH 0/3] Introduce ACPI support for ahci_platform driver
Ping. Are there any other concerns for this patch series? Thanks, Suravee On 3/30/2015 4:56 PM, Suravee Suthikulpanit wrote: This patch series introduce ACPI support for AHCI platform driver. Existing ACPI support for AHCI assumes the device controller is a PCI device. Since there is no ACPI _CID for generic AHCI controller, the driver could not use it for matching devices. Therefore, this patch introduces a mechanism for drivers to match devices using ACPI _CLS method. _CLS contains PCI-defined class-code. This patch series also modifies ACPI modalias to add class-code to the exisiting format, which currently only uses _HID and _CIDs. This is required to support loadable modules w/ _CLS. This is rebased from and tested with: http://git.linaro.org/leg/acpi/acpi.git acpi-5.1-v11 This topic was discussed earlier here (as part of introducing support for AMD Seattle SATA controller): http://marc.info/?l=linux-arm-kernel&m=141083492521584&w=2 Changes from V7 (https://lkml.org/lkml/2015/3/26/592) * [1/3] Return AE_AML_OPERAND_TYPE when _CLS package containing invalid type (per Robert Moore suggestion). * [2/3] Fixed build error due missing ACPI_DEVICE_CLASS definition when disabling ACPI. Changes from V6 (https://lkml.org/lkml/2015/3/25/797) * Adding Acked-by Mika, and Reviewed-by Hanjun * Minor clen up to use lower case 0xff for cls_msk (per Mika suggestions). * Modify the ACPI_DEVICE_CLASS macro to use designated initializer (per Mika suggestions). Changes from V5 (https://lkml.org/lkml/2015/3/6/24) * Rebased and tested with acpi-5.1-v11 * Splitting up the ACPICA changes into a separate patch [1/3]. (per Mika suggestion) * Adding class-code mask support (per Mika suggestion) * Use macro to define struct acpi_device_id entry (per Mika suggestion) * Note: Mika also recommend reordering the member of struct acpi_device_id and define a macro to be used for declaring each table entry. This is a large amount of changes, and will be done separtely from this patch series. Changes from V4 (https://lkml.org/lkml/2015/3/2/56) * [1/2] Bug fixed: Reorder the declaration of struct acpi_pnp_device_id cls in the struct acpi_device_info (include/acpi/actypes.h) since compatible_id_list must be last one. * [2/2] Added Acked-by: Tejun Heo Changes from V3 (https://lkml.org/lkml/2015/2/8/106) * Instead of introducing new structure acpi_device_cls, add cls into the acpi_device_id, and modify the __acpi_match_device to also match for cls. (per Mika suggestion.) * Add loadable module support, which requires changes in ACPI modalias. (per Mika suggestion.) * Rebased and tested with acpi-5.1-v9 Changes from V2 (https://lkml.org/lkml/2015/1/5/662) * Update with review comment from Rafael in patch 1/2 * Rebased and tested with acpi-5.1-v8 Changes from V1 (https://lkml.org/lkml/2014/12/19/345) * Rebased to 3.19.0-rc2 * Change from acpi_cls in device_driver to acpi_match_cls (Hanjun comment) * Change the matching logic in acpi_driver_match_device() due to the new special PRP0001 _HID. * Simplify the return type of acpi_match_device_cls() to boolean. Changes from RFC (https://lkml.org/lkml/2014/12/17/446) * Remove #ifdef and make non-ACPI version of the acpi_match_device_cls as inline. (per Arnd) * Simplify logic to retrieve and evaluate _CLS handle. (per Hanjun) Suravee Suthikulpanit (3): ACPICA: Add ACPI _CLS processing ACPI / scan: Add support for ACPI _CLS device matching ata: ahci_platform: Add ACPI _CLS matching drivers/acpi/acpica/acutils.h | 3 ++ drivers/acpi/acpica/nsxfname.c| 21 +-- drivers/acpi/acpica/utids.c | 73 +++ drivers/acpi/scan.c | 36 --- drivers/ata/Kconfig | 2 +- drivers/ata/ahci_platform.c | 9 + include/acpi/acnames.h| 1 + include/acpi/actypes.h| 4 ++- include/linux/acpi.h | 14 include/linux/mod_devicetable.h | 2 ++ scripts/mod/devicetable-offsets.c | 2 ++ scripts/mod/file2alias.c | 32 +++-- 12 files changed, 189 insertions(+), 10 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/9] AMD IOMMU Updates for v4.1
On 4/1/2015 7:58 AM, Joerg Roedel wrote: Hi, here are a few fixes and enhancements for the AMD IOMMU driver for the next merge window. They are tested on different versions of AMD IOMMUs. Please review. Thanks, Joerg Joerg Roedel (9): iommu/amd: Use BUS_NOTIFY_REMOVED_DEVICE iommu/amd: Ignore BUS_NOTIFY_UNBOUND_DRIVER event iommu/amd: Don't allocate with __GFP_ZERO in alloc_coherent iommu/amd: Add support for contiguous dma allocator iommu/amd: Return the pte page-size in fetch_pte iommu/amd: Optimize iommu_unmap_page for new fetch_pte interface iommu/amd: Optimize alloc_new_range for new fetch_pte interface iommu/amd: Optimize amd_iommu_iova_to_phys for new fetch_pte interface iommu/amd: Correctly encode huge pages in iommu page tables drivers/iommu/amd_iommu.c | 166 +++- drivers/iommu/amd_iommu_types.h | 6 ++ 2 files changed, 83 insertions(+), 89 deletions(-) Tested-by: Suravee Suthikulpanit Thanks, Suravee -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [V7 PATCH 1/3] ACPICA: Add ACPI _CLS processing
On 3/27/2015 12:51 PM, Moore, Robert wrote:
+ cls_objects = obj_desc->package.elements;
+
+ if (obj_desc->common.type == ACPI_TYPE_PACKAGE &&
+ obj_desc->package.count == 3 &&
+ cls_objects[0]->common.type == ACPI_TYPE_INTEGER &&
+ cls_objects[1]->common.type == ACPI_TYPE_INTEGER &&
+ cls_objects[2]->common.type == ACPI_TYPE_INTEGER) {
+
+ /* Allocate a buffer for the CLS */
+ cls = ACPI_ALLOCATE_ZEROED(sizeof(struct acpi_pnp_device_id) +
+(acpi_size) 7);
I would like to see an error returned if an object or subobject is of the
incorrect type.
Then, the caller knows not to attempt to look at it.
Ok. I will return AE_TYPE if the condition is false here.
Thanks,
Suravee
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [V4 PATCH 1/2] ACPI / scan: Add support for ACPI _CLS device matching
On 3/2/2015 2:27 AM, Suravee Suthikulpanit wrote:
Device drivers typically use ACPI _HIDs/_CIDs listed in struct device_driver
acpi_match_table to match devices. However, for generic drivers, we do not
want to list _HID for all supported devices. Also, certain classes of devices
do not have _CID (e.g. SATA, USB). Instead, we can leverage ACPI _CLS,
which specifies PCI-defined class code (i.e. base-class, subclass and
programming interface). This patch adds support for matching ACPI devices using
the _CLS method.
To support loadable module, current design uses _HID or _CID to match device's
modalias. With the new way of matching with _CLS this would requires
modification
to the current ACPI modalias key to include _CLS. This patch appends PCI-defined
class-code to the existing ACPI modalias as following.
acpi..:::
E.g:
# cat /sys/devices/platform/AMDI0600:00/modalias
acpi:AMDI0600:010601:
where bb is th base-class code, ss is te sub-class code, and pp is the
programming interface code
Since there would not be _HID/_CID in the ACPI matching table of the driver,
this patch adds a field to acpi_device_id to specify the matching _CLS.
static const struct acpi_device_id ahci_acpi_match[] = {
{ "", 0, PCI_CLASS_STORAGE_SATA_AHCI },
{},
};
In this case, the corresponded entry in modules.alias file would be:
alias acpi*:010601:* ahci_platform
Signed-off-by: Suravee Suthikulpanit
---
drivers/acpi/acpica/acutils.h | 3 ++
drivers/acpi/acpica/nsxfname.c| 20 +--
drivers/acpi/acpica/utids.c | 71 +++
drivers/acpi/scan.c | 17 --
include/acpi/acnames.h| 1 +
include/acpi/actypes.h| 4 ++-
include/linux/mod_devicetable.h | 1 +
scripts/mod/devicetable-offsets.c | 1 +
scripts/mod/file2alias.c | 13 +--
9 files changed, 123 insertions(+), 8 deletions(-)
[]
>
diff --git a/include/acpi/actypes.h b/include/acpi/actypes.h
index b034f10..50d8019 100644
--- a/include/acpi/actypes.h
+++ b/include/acpi/actypes.h
@@ -1148,7 +1148,7 @@ struct acpi_device_info {
u32 name; /* ACPI object Name */
acpi_object_type type; /* ACPI object Type */
u8 param_count; /* If a method, required parameter count */
- u8 valid; /* Indicates which optional fields are valid */
+ u16 valid; /* Indicates which optional fields are valid */
u8 flags; /* Miscellaneous info */
u8 highest_dstates[4]; /* _sx_d values: 0xFF indicates not valid */
u8 lowest_dstates[5]; /* _sx_w values: 0xFF indicates not valid */
@@ -1158,6 +1158,7 @@ struct acpi_device_info {
struct acpi_pnp_device_id unique_id;/* _UID value */
struct acpi_pnp_device_id subsystem_id; /* _SUB value */
struct acpi_pnp_device_id_list compatible_id_list; /* _CID list */
+ struct acpi_pnp_device_id cls; /* _CLS value */
};
Please disregard this patch. I found out a mistake on my part here. I
have sent out V5 here (https://lkml.org/lkml/2015/3/6/24)
Thank you,
Suravee
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 00/17] Introduce ACPI for ARM64 based on ACPI 5.1
On 1/14/2015 9:04 AM, Hanjun Guo wrote: Hi, This is the v7 of ACPI core patches for ARM64 based on ACPI 5.1 updates from v6: - Rebased on top of 3.19-rc4, add Mack Salter's patch to use the early_ioremap after paging_init() for ACPI table mappings; - Two patches about converting apic_id to phys_id to make it arch agnostic were already merged into RC4 by Rafael. - Split patch "Parse FADT table to get PSCI flags for PSCI init" into two as Lorenzo's suggestion, also fix typo and lack of __init for psci_0_2_set_functions() which is spotted by Lorenzo. - Add Tested-by from Yijing Wang. previous version is here: v6: https://lkml.org/lkml/2015/1/4/40 1. Why we need ACPI on ARM64? - Grant already posted a blog about this, and stated clearly why we need ACPI on ARM64: http://www.secretlab.ca/archives/151 2. What we need to do before the arm64 ACPI core patches could be merged into the kernel? - Al Stone posted a TODO list and updates v2 for the progress we made: http://www.spinics.net/lists/arm-kernel/msg390069.html - so from the progress we can see that we already finished most of the items, and _OSI we got a plan to fix it, RFC patch is on the way. This patch set was tested on FVP by Fuwei, and booted ok as expected. (No functional change since last version) Thanks Hanjun The V7 patch series has also been re-tested on AMD Seattle server platform along with the "Introduce ACPI support for ahci_platform driver" patch series here (https://lkml.org/lkml/2015/1/5/662). Tested-by: Suravee Suthikulpanit Thanks, Suravee -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [V2 PATCH 0/2] Introduce ACPI support for ahci_platform driver
On 1/5/2015 5:24 PM, Rafael J. Wysocki wrote: On Monday, January 05, 2015 03:11:13 PM Suravee Suthikulpanit wrote: This patch series introduce ACPI support for non-PCI AHCI platform driver. Existing ACPI support for AHCI assumes the device controller is a PCI device. Also, since there is no ACPI _HID/_CID for generic AHCI controller, the driver could not use them for matching devices. Therefore, this patch introduces a mechanism for drivers to match devices using ACPI _CLS method. This patch series is rebased from and tested with: http://git.linaro.org/leg/acpi/acpi.git acpi-5.1-v7 This topic was discussed earlier here (as part of introducing support for AMD Seattle SATA controller): http://marc.info/?l=linux-arm-kernel&m=141083492521584&w=2 NOTE: * PATCH 2/2 has already been Acked-by Tejun Heo in V1. I only made a minor renaming of the acpi_cls to acpi_match_cls for clarity in V2. It probably should be routed together with the PATCH 1/2 (once acked) since it defines the new member in the struct. Changes V1 (https://lkml.org/lkml/2014/12/19/345) * Rebased to 3.19.0-rc2 * Change from acpi_cls in device_driver to acpi_match_cls (Hanjun comment) * Change the matching logic in acpi_driver_match_device() due to the new special PRP0001 _HID. * Simplify the return type of acpi_match_device_cls() to boolean. Changes from RFC (https://lkml.org/lkml/2014/12/17/446) * Remove #ifdef and make non-ACPI version of the acpi_match_device_cls as inline. (per Arnd) * Simplify logic to retrieve and evaluate _CLS handle. (per Hanjun) Suravee Suthikulpanit (2): ACPI / scan: Add support for ACPI _CLS device matching ata: ahci_platform: Add ACPI _CLS matching drivers/acpi/scan.c | 79 +++-- drivers/ata/Kconfig | 2 +- drivers/ata/ahci_platform.c | 3 ++ include/acpi/acnames.h | 1 + include/linux/acpi.h| 10 ++ include/linux/device.h | 1 + include/linux/mod_devicetable.h | 6 7 files changed, 98 insertions(+), 4 deletions(-) I'll take care of this when I'm back from travels later this month. Thanks! Thanks Rafael. Suravee -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC 2/4] PCI: generic: Add support for ARM64 and MSI(x)
On 1/2/2015 5:55 AM, Lorenzo Pieralisi wrote: Hi Suravee, On Mon, Dec 29, 2014 at 07:32:44PM +, Suravee Suthikulpanit wrote: >Hi, > >I am not sure if this thread is still alive. I'm trying to see what I >can do to help clean up/convert to make the PCI GHC also works for arm64 >w/ zero or minimal ifdefs. > >Please let me know if someone is already working on this. I noticed that >Lorenzo's patches has already been in 3.19-rc1, and in Bjorn's >pci/domain branch. Otherwise, I'll try to continue the work based on the >sample patch from Arnd here. If I am not mistaken, the only bit missing to remove pci_sys_data (and so having a generic host controller driver that works on ARM32/64) is generic MSI management. Lorenzo, Do you mean to remove pci_sys_data from pci-host-generic.c or removing it completely? I assume the former case. So, looking at the current code in the pci-host-generic.c, my understanding is that the: *gen_pci = pci_bus->sysdata->private_data will be changed to: *gen_pci = pci_bus->sysdata Then, we can simply just call pci_scan_root_bus() directly since we no longer need to declare hw_pci for calling pci_common_init_dev(). I know for certain Marc is working on it, and the solution is WIP, I think we should prevent adding more churn to pci_sys_data, since I managed to remove most of the dependencies (domain, mem_offset). Thanks for cleaning up the domain and mem_offset. I saw Marc's irq/msi_domain patch series (http://git.kernel.org/cgit/linux/kernel/git/maz/arm-platforms.git/log/?h=irq/msi_domain). My understanding is that deals with associating the newly introduced msi_domain to each device, which replaces the need for pci_bus->msi and hw_pci->msi_ctrl when configure with CONFIG_PCI_MSI_IRQ_DOMAIN (not sure if this would be the plan for all arm32). For ARM32, if not define CONFIG_PCI_MSI_IRQ_DOMAIN, it would still fall back to using the [pci_sys_data|hw_pci]->msi_ctrl. However, I noticed that the hw_pci->msi_controller is not even used for the pci-host-generic. So, this should not be blocking the work to free pci-host-generic from pci_sys_data and hw_pci, as the MSI stuff can go in separately. Am I missing something? So to sum it up, to have a generic host controller driver for ARM32/64 we just need to work out how to handle the MSI data, patches will be on the lists shortly to handle that, please review. Thanks, Lorenzo Thanks for the update and the summary. I'll help review/test the MSI patch once posted. Thanks, Suravee -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 1/2] ACPI / scan: Add support for ACPI _CLS device matching
On 12/19/2014 12:47 PM, Suravee Suthikulanit wrote:
On 12/19/2014 8:56 AM, Hanjun Guo wrote:
Hi Suravee,
On 2014年12月18日 07:16, Suravee Suthikulpanit wrote:
From: Suravee Suthikulpanit
Device drivers typically use ACPI _HIDs/_CIDs listed in struct
device_driver
acpi_match_table to match devices. However, for generic drivers, we do
not want to list _HID for all supported devices, and some device classes
do not have _CID (e.g. SATA, USB). Instead, we can leverage ACPI _CLS,
which specifies PCI-defined class code (i.e. base-class, subclass and
programming interface).
This patch adds support for matching ACPI devices using the _CLS method.
Signed-off-by: Suravee Suthikulpanit
---
drivers/acpi/scan.c | 73
+
include/acpi/acnames.h | 1 +
include/linux/acpi.h| 12 ++-
include/linux/device.h | 1 +
include/linux/mod_devicetable.h | 6
5 files changed, 92 insertions(+), 1 deletion(-)
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index d670158..6406648 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -864,6 +864,79 @@ int acpi_match_device_ids(struct acpi_device
*device,
}
EXPORT_SYMBOL(acpi_match_device_ids);
+/**
+ * acpi_match_device_cls - Match a struct device against a ACPI _CLS
method
+ * @dev_cls: A pointer to struct acpi_device_cls object to match
against.
+ * @dev: The ACPI device structure to match.
+ *
+ * Check if @dev has a valid ACPI and _CLS handle. If there is a
+ * struct acpi_device_cls object for that handle, use that object to
match
+ * against the given struct acpi_device_cls object.
+ *
+ * Return 0 on success or error code on failure.
+ */
+int acpi_match_device_cls(const struct acpi_device_cls *dev_cls,
+ const struct device *dev)
+{
+int ret = -EINVAL;
+acpi_status status;
+struct acpi_device *adev;
+union acpi_object *pkg;
+struct acpi_device_cls cls;
+struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
+struct acpi_buffer format = { sizeof("NNN"), "NNN" };
+struct acpi_buffer state = { 0, NULL };
+acpi_handle handle = ACPI_HANDLE(dev);
...
+acpi_handle cls_handle;
+
+if (!handle || acpi_bus_get_device(handle, &adev))
if handle is not NULL, adev will not NULL too :)
because you get the handle from adev, ACPI_HANDLE() is defined as:
acpi_device_handle(ACPI_COMPANION(dev)), and adev = ACPI_COMPANION(dev);
you may use adev = ACPI_COMPANION(dev) to simplify the code.
Thanks for the pointer.
+return ret;
+
+if (!adev->status.present || !dev_cls)
+return ret;
+
+status = acpi_get_handle(adev->handle, METHOD_NAME__CLS,
&cls_handle);
do we need this function called here? _CLS is the method under ACPI
device object in DSDT/SSDT, and you will get adev->handle == cls_handle
if I'm not wrong :)
You are right. It is not needed, and we can just evaluate right from the
handle.
+if (ACPI_FAILURE(status))
+return ret;
+
+status = acpi_evaluate_object(cls_handle, "_CLS", NULL, &buffer);
+if (ACPI_FAILURE(status)) {
+ACPI_EXCEPTION((AE_INFO, status, "Failed to Evaluat _CLS"));
+return ret;
+}
I think you can evaluate _CLS directly with handle here.
Thanks
Hanjun
Yep. I will send out the new patch in a bit.
Thanks,
Suravee
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Linaro-acpi] [RFC PATCH 2/2] ata: ahci_platform: Add ACPI _CLS matching
On 12/18/2014 3:17 AM, Arnd Bergmann wrote:
On Wednesday 17 December 2014 17:16:35 Suravee Suthikulpanit wrote:
+#ifdef CONFIG_ATA_ACPI
+#include
+#endif
#include "ahci.h"
static const struct ata_port_info ahci_port_info = {
@@ -71,6 +74,10 @@ static const struct of_device_id ahci_of_match[] = {
};
MODULE_DEVICE_TABLE(of, ahci_of_match);
+#ifdef CONFIG_ATA_ACPI
+static const struct acpi_device_cls ahci_cls = {0x01, 0x06, 0x01};
+#endif
+
static struct platform_driver ahci_driver = {
.probe = ahci_probe,
.remove = ata_platform_remove_one,
@@ -78,6 +85,9 @@ static struct platform_driver ahci_driver = {
.name = "ahci",
.owner = THIS_MODULE,
.of_match_table = ahci_of_match,
+#ifdef CONFIG_ATA_ACPI
+ .acpi_cls = &ahci_cls,
+#endif
.pm = &ahci_pm_ops,
It would be better to leave out the various #ifdef here, in particular the
one around the header file inclusion.
Arnd
Right. I'll get rid of them. Also, I noticed that I should have declared
acpi_match_device_cls() in include/linux/acpi.h as "inline" for when
building w/o ACPI to avoid warning messages.
Thanks,
Suravee
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 18/18] Documentation: ACPI for ARM64
On 10/17/2014 8:37 AM, Hanjun Guo wrote: From: Graeme Gregory Add documentation for the guidelines of how to use ACPI on ARM64. Signed-off-by: Graeme Gregory Signed-off-by: Al Stone Signed-off-by: Hanjun Guo --- Documentation/arm64/arm-acpi.txt | 323 ++ 1 file changed, 323 insertions(+) create mode 100644 Documentation/arm64/arm-acpi.txt [...] AMD has reviewed this document, and currently implements ACPI table in the firmware for AMD Seattle platform based on the documentation published here: http://amd-dev.wpengine.netdna-cdn.com/wordpress/media/2012/10/Seattle_ACPI_Guide.pdf Reviewed-by: Suravee Suthikulpanit Thank you, Suravee -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [V10 PATCH 2/2] irqchip: gicv2m: Add supports for ARM GICv2m MSI(-X)
On 11/5/2014 6:05 PM, Suravee Suthikulanit wrote: - Overall, it seems that msi_domain_alloc() could be quite different across architectures. Would it be possible to declare this function as weak, and allow arch to override (similar to arch_setup_msi_irq)? Actually, declaring "msi_domain_ops" as non-static, and allow other code to override the .alloc and .free? Thanks, Suravee -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [V10 PATCH 2/2] irqchip: gicv2m: Add supports for ARM GICv2m MSI(-X)
On 11/4/2014 7:01 AM, Jiang Liu wrote: Hi Suravee, You may build a two level hierarchy irqdomains. Use the utilities in this thread http://www.spinics.net/lists/arm-kernel/msg374722.html to build an MSI irqdomain to manage MSI controllers in PCI devices. And build another irqdomain to manage SPI allocation in GICv2. That is: MSI irqdomain (program MSI registers) --> GIV irqdomain (manage SPIs in GICv2 controller) Regards! Gerry Gerry, I try out your patch from the link above, and I have a couple questions/issues. 1. In the drivers/pci/msi.c: msi_irq_domain_alloc_irqs(), it seems that the hwirq comes from msi_get_hwirq(dev, msidesc). In GICv2m, hwirq for MSI is fixed over a specific range. This might require arch-specific callback. 2. In msi_domain_activate, why "if (!irq_data->chip_data)"? 3. In, msi_domain_alloc(): - There should be a way to specify other types of irq handler besides the "handle_edge_irq". In case of GIC, it needs handle_fasteoi_irq. - When calling irq_domain_set_hwirq_and_chip(), you are passing "(void *)(long)i" for the "void *chip_data" parameter. What is this used for, and where? Shouldn't this be pointing to arch-specific data structure? - The code is calling irq_domain_alloc_irqs_parent before the loop, which calls irq_domain_set_hwirq_and_chip() and __irq_set_handler. Shouldn't the order be switched? - Overall, it seems that msi_domain_alloc() could be quite different across architectures. Would it be possible to declare this function as weak, and allow arch to override (similar to arch_setup_msi_irq)? Thanks, Suravee -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [V10 PATCH 2/2] irqchip: gicv2m: Add supports for ARM GICv2m MSI(-X)
On 11/3/2014 4:51 PM, Thomas Gleixner wrote:
On Mon, 3 Nov 2014, suravee.suthikulpa...@amd.com wrote:
+static void gicv2m_teardown_msi_irq(struct msi_chip *chip, unsigned int irq)
+{
+ int pos;
+ struct v2m_data *v2m = container_of(chip, struct v2m_data, msi_chip);
+
+ spin_lock(&v2m->msi_cnt_lock);
Why do you need an extra lock here? Is that stuff not serialized from
the msi_chip layer already?
If not, why don't we have the serialization there instead of forcing
every callback to implement its own?
From the following call paths:
|--> pci_enable_msi_range
|--> msi_capability_init
|--> arch_setup_msi_irqs
|--> arch_setup_msi_irq
and
|--> pci_enable_msix
|--> msix_capability_init
|--> arch_setup_msi_irqs
|--> arch_setup_msi_irq
It serialize when a PCI device driver tries to allocate multiple
interrupts. However, AFAICT, it would not serialize the allocation when
multiple drivers trying to setup MSI irqs at the same time. I needed
that to protect the bitmap structure. I also noticed the same in other
drivers as well.
I can look into this more to see where would be a good point.
+ pos = irq - v2m->spi_start;
So this assumes that @irq is the hwirq number, right? How does the
calling function know about that? It should only have knowledge about
the virq number if I'm not missing something.
And if I'm missing something, then that msi_chip stuff is seriously
broken.
It works this way because of the direct mapping (as you noticed). But I
am planning to change that. See below.
+ if (pos >= 0 && pos < v2m->nr_spis)
So you simply avoid the clear bitmap instead of yelling loudly about
being called with completely wrong data?
I'll provide appropriate warnings.
I would not be surprised if that is related to my question above.
Not quite sure which of the above questions.
+ spin_lock(&v2m->msi_cnt_lock);
+ offset = bitmap_find_free_region(v2m->bm, v2m->nr_spis, 0);
+ spin_unlock(&v2m->msi_cnt_lock);
+ if (offset < 0)
+ return offset;
+
+ hwirq = v2m->spi_start + offset;
+ virq = __irq_domain_alloc_irqs(v2m->domain, hwirq,
+ 1, NUMA_NO_NODE, v2m, true);
+ if (virq < 0) {
+ gicv2m_teardown_msi_irq(chip, hwirq);
+ return virq;
+ }
+
+ irq_domain_set_hwirq_and_chip(v2m->domain, virq, hwirq,
+ &v2m_chip, v2m);
+
+ irq_set_msi_desc(hwirq, desc);
+ irq_set_irq_type(hwirq, IRQ_TYPE_EDGE_RISING);
Sure both calls work perfectly fine as long as virq == hwirq, right?
I was running into an issue when calling the
irq_domain_alloc_irq_parent(), it requires of_phandle_args pointer to be
passed in. However, this does not work for GICv2m since it does not have
interrupt information in the device tree. So, I decided at first to use
direct (virq == hwirq) mapping, which simplifies the code a bit, but
might not be ideal solution, as you pointed out.
An alternative would be to create a temporary struct of_phandle_args,
and populate it with the interrupt information for the requested MSI.
Then pass it to:
--> irq_domain_alloc_irq_parent
|--> gic_irq_domain_alloc
|--> gic_irq_domain_xlate
|--> gic_irq_domain_map
However, this would still not be ideal if we want to support ACPI.
Another alternative would be coming up with a dedicate structure to be
used here. I noticed on X86, it uses struct irq_alloc_info. May be
that's what we also need here.
[...]
I do not care at all how YOU waste your time. But I care very much
about the fact that YOU are wasting MY precious time by exposing me to
your patch trainwrecks.
I don't intend to waste yours or anybody's precious time. Sorry if it
takes a couple iterations to work out the issues. Also, I will try to
put more comment in my code to make it more clear. Let me know what
works best for you to work out the issues.
Thanks,
Suravee
Thanks,
tglx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [V9 PATCH 2/2] irqchip: gicv2m: Add supports for ARM GICv2m MSI(-X)
On 11/3/2014 8:10 AM, Marc Zyngier wrote:
On 03/11/14 09:50, Marc Zyngier wrote:
@@ -843,10 +847,14 @@ static int gic_irq_domain_alloc(struct irq_domain
*domain, unsigned int virq,
unsigned int type = IRQ_TYPE_NONE;
struct of_phandle_args *irq_data = arg;
- ret = gic_irq_domain_xlate(domain, irq_data->np, irq_data->args,
- irq_data->args_count, &hwirq, &type);
- if (ret)
- return ret;
+ if (irq_data) {
+ ret = gic_irq_domain_xlate(domain, irq_data->np, irq_data->args,
+ irq_data->args_count, &hwirq, &type);
+ if (ret)
+ return ret;
+ } else {
+ hwirq = virq;
+ }
I'm slightly puzzled here. What's the purpose of this? The whole goal of
the domain hierarchy is to avoid that kind of thing. Also, you should
never have to call xlate on an MSI, because it should never be described
in the device tree the first place.
Thinking of it some more:
The actual reason why this is required is because the MSI domain calls
into this via irq_domain_alloc_irqs_parent(). But because MSIs are not
described in DT, they do not have a of_phandle to pass down to the xlate
helper. In this case, the v2m widget has the knowledge of what are the
valid SPI numbers, and the core GIC code must blindly accept it.
This definitely requires a fat comment, because this is far from obvious.
Thanks,
M.
I'll put in proper comments here.
Suravee
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [V9 PATCH 2/2] irqchip: gicv2m: Add supports for ARM GICv2m MSI(-X)
On 11/3/2014 3:50 AM, Marc Zyngier wrote:
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
>index a99c211..4069eb3 100644
>--- a/drivers/irqchip/irq-gic.c
>+++ b/drivers/irqchip/irq-gic.c
>@@ -46,6 +46,7 @@
> #include
>
> #include "irq-gic-common.h"
>+#include "irq-gic-v2m.h"
> #include "irqchip.h"
>
> union gic_base {
>@@ -68,6 +69,9 @@ struct gic_chip_data {
> #ifdef CONFIG_GIC_NON_BANKED
> void __iomem *(*get_base)(union gic_base *);
> #endif
>+#ifdef CONFIG_ARM_GIC_V2M
>+ struct list_head v2m_list;
>+#endif
Can't that be something private to the v2m widget driver? I don't think
it brings anything to the main GIC driver.
Looking at this again, now that we use the hierarchy irqdomain, GIC no
longer needs to be handling with children v2m. I'll remove this altogether.
Suravee
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [V9 PATCH 2/2] irqchip: gicv2m: Add supports for ARM GICv2m MSI(-X)
On 10/31/2014 4:40 AM, Thomas Gleixner wrote: On Fri, 31 Oct 2014, suravee.suthikulpa...@amd.com wrote: +/* + * alloc_msi_irq - Allocate MSIs from available MSI bitmap. + * @data: Pointer to v2m_data + * @nvec: Number of interrupts to allocate + * @irq: Pointer to the allocated irq + * + * Allocates interrupts only if the contiguous range of MSIs + * with specified nvec are available. Otherwise return the number + * of available interrupts. If none are available, then returns -ENOENT. And the exact purpose of returning the number of available interrupts is? Initially, I intended to use this function to allocate irqs for both MSI and multi-MSI case. But I'll simplify this and revisit it again when adding the multi-MSI. + virq = __irq_domain_alloc_irqs(v2m->domain, hwirq, + 1, NUMA_NO_NODE, v2m, true); And surely the ability of alloc_msi_irq() to allocate a contiguous vector space is required to satisfy an hardcoded allocation of ONE interrupt. What is guaranteeing that the caller only requests a single interrupt? Since this is calling from gicv2m_setup_msi_irq(), it should only setup 1 MSI interrupt. >[...] +err_out: Single error exit which undoes the stuff in the same order it got initialized is just plain wrong. Ever looked at proper error exits in other kernel files? I'll clean this up in V10. Thanks for pointing this out. Suravee -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] arm64: amd-seattle: Adding device tree for AMD Seattle platform
On 10/27/2014 6:25 PM, Alexander Graf wrote: On 27.10.14 15:29, Suravee Suthikulanit wrote: On 10/26/2014 9:08 AM, Alexander Graf wrote: This option doesn't exist in upstream kernels, does it? Why not just make it dtb-y? CONFIG_ARCH_SEATTLE is being added one hunk above.:) Oops:). I'm not convinced we need a config option just for the sake of compiling a device tree though. Alex Eventually, we would add other device driver selections when CONFIG_ARCH_SEATTLE=y. At this point, those drivers are still not ready. Could you please give me some examples of drivers that would depend on CONFIG_ARCH_SEATTLE? I like the current way things work without the need for such an option, where everything's implemented purely as drivers you can opt in our out of. You don't have a CONFIG_ARCH_SB7XX on x86 either, right? ;) Alex I am not saying that device drivers need to depend on CONFIG_ARCH_SEATTLE. I am thinking along the line of an easy way to enable SOC without having to manually select each of the required drivers to support the SOC. An example is the "ARCH_VEXPRESS". Suravee -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] arm64: amd-seattle: Adding device tree for AMD Seattle platform
On 10/27/2014 2:07 PM, Arnd Bergmann wrote:
On Monday 27 October 2014 13:54:25 suravee.suthikulpa...@amd.com wrote:
+ sata0: sata@0030 {
+ compatible = "snps,spear-ahci";
+ reg = <0 0x30 0 0x800>;
+ interrupts = <0 355 4>;
+ clocks = <&sataclk_333mhz>;
+ clock-names = "apb_pclk";
+ dma-coherent;
+ };
Please use "snps,dwc-ahci" as the compatible string for a designware
ahci controller, not "snps,spear-ahci", which refers to the implementation
of that in the ST microelectronics SPEAr consumer SoC.
It would be good to also add a string that is specific for your SoC if
we ever need to identify it directly.
Arnd
Thanks. I will update this in V3.
Suravee
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] arm64: amd-seattle: Adding device tree for AMD Seattle platform
On 10/27/2014 8:50 AM, Mark Rutland wrote:
Hi Suravee,
[...]
+ chosen {
+ linux,stdout-path = "console=ttyAMA0,115200
earlycon=pl011,0xe101";
The stdout-path property should just be a path to the UART node. It's
not a direct replacement for /chosen/bootargs.
This should be (assuming you fix up the label above):
stdout-path = &serial0;
That will give us earlycon if "earlycon" (with no arguments) is provided
on the command line, and should set up that UART as the console. There's
no need for the "linux," prefix now either.
Unfortuantely, I believe that the UART rate will get changed when the
real PL011 driver registers, unless the rate is explicitly provided on
the command line. It might be worth looking into retaining the
configured rate somehow indepentent of bootargs (unless overriden).
Thanks for the explanation. I think I should be able to get rid of the
"console=ttyAMA0,115200 earlycon=pl011,0xe101" altogether. I'll send
out V2 soon.
Thanks,
Suravee
Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] arm64: amd-seattle: Adding device tree for AMD Seattle platform
On 10/26/2014 9:09 AM, Andreas Färber wrote: Hi, Am 24.10.2014 um 14:20 schrieb suravee.suthikulpa...@amd.com: diff --git a/arch/arm64/boot/dts/Makefile b/arch/arm64/boot/dts/Makefile index f8001a6..6c27047 100644 --- a/arch/arm64/boot/dts/Makefile +++ b/arch/arm64/boot/dts/Makefile @@ -1,6 +1,7 @@ dtb-$(CONFIG_ARCH_THUNDER) += thunder-88xx.dtb dtb-$(CONFIG_ARCH_VEXPRESS) += rtsm_ve-aemv8a.dtb foundation-v8.dtb dtb-$(CONFIG_ARCH_XGENE) += apm-mustang.dtb +dtb-$(CONFIG_ARCH_SEATTLE) += amd-seattle.dtb It seems these lines are sorted alphabetically, so Seattle should go somewhere before Thunder. targets += dtbs targets += $(dtb-y) Regards, Andreas Ah.. right. I'll fix this and send out V2. Suravee -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] arm64: amd-seattle: Adding device tree for AMD Seattle platform
On 10/26/2014 9:08 AM, Alexander Graf wrote: This option doesn't exist in upstream kernels, does it? Why not just >>make it dtb-y? > >CONFIG_ARCH_SEATTLE is being added one hunk above.:) Oops:). I'm not convinced we need a config option just for the sake of compiling a device tree though. Alex Eventually, we would add other device driver selections when CONFIG_ARCH_SEATTLE=y. At this point, those drivers are still not ready. Suravee -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] ata: ahci_platform: Add ACPI support for AMD Seattle SATA controller
On 9/16/2014 8:26 PM, Matthew Garrett wrote:
On Mon, Sep 15, 2014 at 07:47:23PM -0500, suravee.suthikulpa...@amd.com wrote:
From: Suravee Suthikulpanit
This patch adds ACPI match table in ahci_platform. The table includes
the acpi_device_id to match AMD Seattle SATA controller with following
asl structure in DSDT:
Device (SATA0)
{
Name(_HID, "AMDI0600") // Seattle AHSATA
There really ought to be a well-defined PNPID for AHCI, so you can _HID
to AMD and _CID to something generic. That way we won't have:
+#ifdef CONFIG_ATA_ACPI
+static const struct acpi_device_id ahci_acpi_match[] = {
+ { "AMDI0600", 0 }, /* AMD Seattle AHCI */
+ { },
+};
utter sadness here. Really, please don't end up in a situation where we
need to add device-specific IDs to a generic driver.
Matthew,
Currently, there is no _CID defined for generic AHCI. We will work on
proposing one, and provide update patches for including the new ID.
Thanks,
Suravee
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2 V4] irqchip: gicv2m: Add support for multiple MSI for ARM64 GICv2m
On 8/15/2014 8:31 AM, Marc Zyngier wrote:
Hi Suravee,
+/*
+ * ARM64 function for seting up MSI irqs.
+ * Copied from driver/pci/msi.c: arch_setup_msi_irqs().
+ */
+int arm64_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
+{
+ struct msi_desc *entry;
+ int ret;
+
+ if (type == PCI_CAP_ID_MSI && nvec > 1)
+ return 1;
+
+ list_for_each_entry(entry, &dev->msi_list, list) {
+ ret = arch_setup_msi_irq(dev, entry);
+ if (ret < 0)
+ return ret;
+ if (ret > 0)
+ return -ENOSPC;
+ }
+
+ return 0;
+}
I'm going to reiterate what I said last time: Why do we need this?
[Suravee] Marc, I understand what you described last time but I think
there is one point that missing here. See below.
So far, we have two MSI-capable controllers on their way upstream:
GICv2m and GICv3. Both are perfectly capable of handling more than a
single MSI per device.
[Suravee] I am aware of this.
So why should we cater for this? My gut feeling is that we should just
have:
int arch_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
{
struct msi_desc *entry;
int ret;
/*
* So far, all our MSI controllers are capable of handling more
* than a single MSI per device. Should we encounter less
* capable devices, we'll consider doing something special for
* them.
*/
list_for_each_entry(entry, &dev->msi_list, list) {
ret = arch_setup_msi_irq(dev, entry);
if (ret < 0)
return ret;
if (ret > 0)
return -ENOSPC;
}
return 0;
}
and nothing else. Your driver should be able to retrieve the number of
MSI needed by the device, and allocate them. GICv3 manages it, and so
should GICv2m.
[Suravee] Multi-MSI and MSI-x are not the same. For MSI-X, you can treat
each of the MSI separately since it MSI-X capability structure has a
table specific for each one of them. For Multi-MSI, there is only one
MSI capability structure which control all of them, and you need to
program the "multiple-message enable" field with the encoding for
"power-of-two", and therefore must be in contiguous range.
Your logic above is what the standard MSI-x setup code is using. It is
not handling of how many it can allocate all at once.
As for sharing the logic b/w GICv2m and GICv3, unless they are sharing
the same common data structure (e.g. the struct v2m which contans
msi_chip), and the allocation function (e.g. generic
gic_alloc_msi_irqs()), you pretty much need to do this separately since
we need to walk the msi_chip back to its container structure.
I am not saying this cannot be done, but we need to work out the detail
together b/w GICv2m and GICv3.
Thanks,
Suravee
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/4] perf tools: allow user to specify hardware breakpoint bp_len
Frederic, 3.17 windows is now open. Do you think you could get this one in? Thanks Suravee On 6/6/2014 11:30 AM, Suravee Suthikulanit wrote: This message has been archived. View the original item <http://ausev2.amd.com/EnterpriseVault/ViewMessage.asp?VaultId=1C7248B8A7CDE234D884C352F0CB2021B111amdvault.amd.com&SavesetId=201407115122720~20140606163016~Z~F134EBF86540766B73DA6ADB45060FE1> On 6/3/2014 6:55 AM, Jiri Olsa wrote: > On Tue, Jun 03, 2014 at 10:36:22AM +0900, Namhyung Kim wrote: >> Hi Jiri, >> >> On Fri, 30 May 2014 15:39:06 +0200, Jiri Olsa wrote: >>> On Thu, May 29, 2014 at 05:26:51PM +0200, Frederic Weisbecker wrote: >>>> From: Jacob Shin >>>> >>>> Currently bp_len is given a default value of 4. Allow user to override it: >>>> >>>>$ perf stat -e mem:0x1000/8 >>>> ^ >>>> bp_len >>>> >>>> If no value is given, it will default to 4 as it did before. >>> >>> Namhyung, >>> both perf tols patches from this patchset mess up with hists >>> tests.. I havent found any connection yet.. any idea? ;-) >> >> So you already found the problem in the hpp->elide change and that's the >> reason of the failure, right? :) > > I haven't got a chance to test it yet.. but it looks like > thats the case.. I'll retest soon ;-) > > jirka > Anything I can do to help here? Thanks, Suravee -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/4 V3] irqchip: gic: Add supports for ARM GICv2m MSI(-X)
On 8/1/2014 10:42 AM, Suravee Suthikulanit wrote:
+#ifdef CONFIG_SMP
+ .irq_set_affinity = gic_set_affinity,
+#endif
+#ifdef CONFIG_PM
+ .irq_set_wake = gic_set_wake,
+#endif
+};
+
+#ifdef CONFIG_OF
+static int __init
+gicv2m_of_init(struct device_node *node, struct device_node *parent)
+{
+ struct gic_chip_data *gic;
+ int ret;
+
+ ret = _gic_of_init(node, parent, &gicv2m_chip, &gic);
+ if (ret) {
+ pr_err("GICv2m: Failed to initialize GIC\n");
+ return ret;
+ }
+
+ gic->msi_chip.owner = THIS_MODULE;
+ gic->msi_chip.of_node = node;
+ gic->msi_chip.setup_irq = gicv2m_setup_msi_irq;
+ gic->msi_chip.teardown_irq = gicv2m_teardown_msi_irq;
+ ret = of_pci_msi_chip_add(&gic->msi_chip);
+ if (ret) {
+ /* MSI is optional and not supported here */
+ pr_info("GICv2m: MSI is not supported.\n");
+ return 0;
+ }
+
+ ret = gicv2m_msi_init(node, &gic->v2m_data);
+ if (ret)
+ return ret;
+ return ret;
+}
+
+IRQCHIP_DECLARE(arm_gic_400_v2m, "arm,gic-400-v2m", gicv2m_of_init);
So if you follow my advise of reversing your probing and call into the
v2m init from the main GIC driver, you could take a irq_chip as a
parameter, and use it to populate the v2m irq_chip, only overriding the
two methods that actually differ.
This would have the net effect of completely dropping patch #2, which
becomes effectively useless.
[Suravee] Ok, lemme look into this.
So, in previous revision, you mentioned that we should have a separate
irq_chip for gicv2m stuff, is that is still the case here?
Thanks,
Suravee
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/4 V3] irqchip: gicv2m: Add support for multiple MSI for ARM64 GICv2m
On 8/1/2014 9:51 AM, Marc Zyngier wrote: Hi Suravee, On 01/08/14 15:36, Suravee Suthikulanit wrote: On 7/30/2014 10:16 AM, Marc Zyngier wrote: Why do we need this complexity at all? Is there any case where we'd want to limit ourselves to a single vector for MSI? I think the ARM64 GICv2m should not be the limitation for the devices multiple MSI if there is no real hardware/design limitation. arm64 is a new enough architecture so that we can expect all interrupt controllers to cope with that. I am not sure if I understand this comment. We are not forcing all interrupt controllers for ARM64 to handle multi-MSI. They have the option to support if multi-MSI if they want to. I just think that we should not put the architectural limit here. Let me be clearer: I think we should put the burden of *not* handling multi-MSI on interrupt controllers. Here, you're making the architectural default to be "I don't support multi-MSI", hence having to override global vectors and such for well behaved MSI controllers like GICv2m and GICv3 ITS. Let's only support multi-MSI for the time being. If someone comes up with a silly old MSI controller that can't deal with it, we'll address the issue at that problem. Thanks, M. Ok, I'm fine with that. Thanks for clarification. Suravee -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/4 V3] irqchip: gic: Add supports for ARM GICv2m MSI(-X)
On 7/30/2014 9:57 AM, Marc Zyngier wrote:
On Thu, Jul 10 2014 at 12:05:03 am BST, "suravee.suthikulpa...@amd.com"
wrote:
Hi Suravee,
From: Suravee Suthikulpanit
>> ..
>>
- first region is the GIC distributor register base and size. The 2nd region is
- the GIC cpu interface register base and size.
+- reg : Specifies base physical address(s) and size of the GIC register frames.
+
+ Region | Description
+ Index |
+ ---
+ 0 | GIC distributor register base and size
+ 1 | GIC cpu interface register base and size
+ 2 | VGIC interface control register base and size (Optional)
+ 3 | VGIC CPU interface register base and size (Optional)
+ 4 | GICv2m MSI interface register base and size (Optional)
Given that the v2m block is somehow bolted on the side of a standard
GICv2, I'd rather see it as a subnode of the main GIC.
Then you could directly call into the v2m layer to initialize it,
instead of the odd "reverse probing" you're using here...
[Suravee] IIUC, you don't think we should introduce the "gic-400-v2m"
compatible ID. Instead, you want to see something like:
gic @ 0x00f0 {
.
v2m {
msi-controller;
reg = < >; /* v2m reg frame */
}
}
If so, I can change this.
+
+static int __init
+gicv2m_msi_init(struct device_node *node, struct v2m_data *v2m)
+{
+ unsigned int val;
+
+ if (of_address_to_resource(node, GIC_OF_MSIV2M_RANGE_INDEX,
+ &v2m->res)) {
+ pr_err("GICv2m: Failed locate GICv2m MSI register frame\n");
+ return -EINVAL;
+ }
+
+ v2m->base = of_iomap(node, GIC_OF_MSIV2M_RANGE_INDEX);
+ if (!v2m->base) {
+ pr_err("GICv2m: Failed to map GIC MSI registers\n");
+ return -EINVAL;
+ }
+
+ val = readl_relaxed(v2m->base + V2M_MSI_TYPER);
+ if (!val) {
+ pr_warn("GICv2m: Failed to read V2M_MSI_TYPER register\n");
+ return -EINVAL;
+ }
+
+ v2m->spi_start = (val >> V2M_MSI_TYPER_BASE_SHIFT) &
+ V2M_MSI_TYPER_BASE_MASK;
+ v2m->nr_spis = val & V2M_MSI_TYPER_NUM_MASK;
+ if ((v2m->spi_start < V2M_MIN_SPI) || (v2m->nr_spis >= V2M_MAX_SPI)) {
+ pr_err("GICv2m: Invalid MSI_TYPER (%#x)\n", val);
+ return -EINVAL;
+ }
+
+ v2m->bm = kzalloc(sizeof(long) * BITS_TO_LONGS(v2m->nr_spis),
+ GFP_KERNEL);
+ if (!v2m->bm) {
+ pr_err("GICv2m: Failed to allocate MSI bitmap\n");
+ return -ENOMEM;
+ }
+
+ spin_lock_init(&v2m->msi_cnt_lock);
+
+ pr_info("GICv2m: SPI range [%d:%d]\n",
+ v2m->spi_start, (v2m->spi_start + v2m->nr_spis));
+
+ return 0;
+}
This function is assuming that you will only see one single MSI frame
here. Is there any chance that there would be more than one in a system?
[Suravee] If I would imagine such SOC, where there are multiple MSI
frames (hence multiple msi-controllers), can we currently support this
with the current msichip interface? Currently, each PCI RC connects to
an "interrupt-parrent", which is also an MSI controller. We would need
to have a way for PCI RC to specify which of the msichips within an
interrupt-controller it wants to use.
Currently, I am not aware if there is a GIC w/ multiple MSI frames.
Could you check if there is such product for ARM GICs?
+
+static void gicv2m_mask_irq(struct irq_data *d)
+{
+ gic_mask_irq(d);
+ if (d->msi_desc)
+ mask_msi_irq(d);
+}
+
+static void gicv2m_unmask_irq(struct irq_data *d)
+{
+ gic_unmask_irq(d);
+ if (d->msi_desc)
+ unmask_msi_irq(d);
+}
+
+static struct irq_chip gicv2m_chip = {
+ .name = "GICv2m",
+ .irq_mask = gicv2m_mask_irq,
+ .irq_unmask = gicv2m_unmask_irq,
+ .irq_eoi= gic_eoi_irq,
+ .irq_set_type = gic_set_type,
+ .irq_retrigger = gic_retrigger,
I think you can loose the retrigger here.
OK.
+#ifdef CONFIG_SMP
+ .irq_set_affinity = gic_set_affinity,
+#endif
+#ifdef CONFIG_PM
+ .irq_set_wake = gic_set_wake,
+#endif
+};
+
+#ifdef CONFIG_OF
+static int __init
+gicv2m_of_init(struct device_node *node, struct device_node *parent)
+{
+ struct gic_chip_data *gic;
+ int ret;
+
+ ret = _gic_of_init(node, parent, &gicv2m_chip, &gic);
+ if (ret) {
+ pr_err("GICv2m: Failed to initialize GIC\n");
+ return ret;
+ }
+
+ gic->msi_chip.owner = THIS_MODULE;
+ gic->msi_chip.of_node = node;
+ gic->msi_chip.setup_irq = gicv2m_setup_msi_irq;
+ gic->msi_chip.teardown_irq = gicv2m_teardown_msi_irq;
+ ret = of_pci_msi_chip_ad
Re: [PATCH 4/4 V3] irqchip: gicv2m: Add support for multiple MSI for ARM64 GICv2m
On 7/30/2014 10:16 AM, Marc Zyngier wrote: Why do we need this complexity at all? Is there any case where we'd want to limit ourselves to a single vector for MSI? I think the ARM64 GICv2m should not be the limitation for the devices multiple MSI if there is no real hardware/design limitation. arm64 is a new enough architecture so that we can expect all interrupt controllers to cope with that. I am not sure if I understand this comment. We are not forcing all interrupt controllers for ARM64 to handle multi-MSI. They have the option to support if multi-MSI if they want to. I just think that we should not put the architectural limit here. Thanks, Suravee -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/4 V3] irqchip: gic: Introduce ARM GICv2m MSI(-X) support
On 7/17/2014 8:55 AM, Mark Rutland wrote:
Hi Jason,
On Thu, Jul 17, 2014 at 02:18:54PM +0100, Jason Cooper wrote:
On Wed, Jul 09, 2014 at 06:05:00PM -0500, suravee.suthikulpa...@amd.com wrote:
From: Suravee Suthikulpanit
This patch set introduces support for MSI(-X) in GICv2m specification,
which is implemented in some variation of GIC400.
This depends on and has been tested with the V7 of"Add support for PCI in
AArch64"
(https://lkml.org/lkml/2014/3/14/320).
Changes in V3:
* Rebase to git://git.infradead.org/users/jcooper/linux.git irqchip/gic
(per Jason Cooper request)
* Misc fix/clean up per Mark Rutland comments
* Minor Clean up in the driver/irqchip/irq-gic-v2m.c: alloc_msi_irqs()
* Patch 4 is new to the series:
* Add ARM64-specific version arch_setup_msi_irqs() to allow support
for Multiple MSI.
* Add support for Multiple MSI for GICv2m.
Suravee Suthikulpanit (4):
irqchip: gic: Add binding probe for ARM GIC400
irqchip: gic: Restructuring ARM GIC code
irqchip: gic: Add supports for ARM GICv2m MSI(-X)
irqchip: gicv2m: Add support for multiple MSI for ARM64 GICv2m
Ok, patch #1 applied to irqchip/urgent. Patches 2 and 3 applied to
irqchip/gic with irqchip/urgent merged in. To facilitate
testing/merging, I've prepared an unsigned tag for you on the
irqchip/gic branch:
I'm a little concerned that this is all going through for v3.17 without
a {Reviewed,Acked}-by from Marc or anyone working with GIC{,v2m}.
While his comments on v1 have been addressed, he has not had a chance to
acknowledge the solutions. I appreciate Marc's holiday is unfortunately
timed.
I also have an open concern with the binding with regard to the
orthogonality of GICV GICH and the MSI registers.
The MSI part is normally enabled from the optional "msi-controller"
keyword. It should not really matter which compatible ID it uses.
Ooops. I noticed that was accidentally dropped the check for
"msi-controller" in the gicv2m_of_init() function. I'll send a follow
up patch to fix this.
Suravee, do you need this urgently for v3.17? I was under the impression
that we wouldn't have full PCIe support by then.
PCI is the dependency for this patch to function. So, it should be
aligned with upstreaming of PCI patches.
Suravee
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] irqchip: gic: Add binding probe for ARM GIC400
On 7/17/2014 7:48 AM, Jason Cooper wrote: On Tue, Jul 15, 2014 at 12:03:03AM +0200, Heiko Stübner wrote: From: Suravee Suthikulpanit Commit 3ab72f9156bb "dt-bindings: add GIC-400 binding" added the "arm,gic-400" compatible string, but the corresponding IRQCHIP_DECLARE was never added to the gic driver. Therefore add the missing irqchip declaration for it. Signed-off-by: Suravee Suthikulpanit Removed additional empty line and adapted commit message to mark it as fixing an issue. Signed-off-by: Heiko Stuebner --- As I really need this, I took the liberty of adapting the patch accordingly to make it apply on top of the current irqchip/for-next (or urgent) and explicitly state the fixed issue. Hope that is ok drivers/irqchip/irq-gic.c | 1 + 1 file changed, 1 insertion(+) Applied to irqchip/urgent with Will's Ack and Cc'd to stable for v3.14+ thx, Jason. Thank you, Suravee -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/4 V3] irqchip: gic: Introduce ARM GICv2m MSI(-X) support
On 7/13/2014 6:14 PM, Jason Cooper wrote: Suravee, On Wed, Jul 09, 2014 at 06:05:00PM -0500, suravee.suthikulpa...@amd.com wrote: From: Suravee Suthikulpanit This patch set introduces support for MSI(-X) in GICv2m specification, which is implemented in some variation of GIC400. This depends on and has been tested with the V7 of"Add support for PCI in AArch64" (https://lkml.org/lkml/2014/3/14/320). Grrr. I mis-spoke against your v1 of this series. There are more changes to irq-gic.c than I originally thought in this series. I am not quite sure what your are referring to. Additionally, we have a lot of other significant changes to that driver as well this cycle. It would be really helpful if I could take patches 1-3 through irqchip/gic. I can Ack #4 with the Subject change, and the branch it lands in can depend on irqchip/gic, no problem there. Patch 1-3 should be able to go through the irqchip/gic along with the gicv3 from Marc, which I have rebased this patch against. Patch 4 is arch64 architectural changes. Therefore, it might need to be going through a different branch. Marc/Mark/Will/Catalin, do you have any suggestions on which branch this should go to? My main concern is your statement above and your answer to my inquiry against v1. Right now, I'm only concerned about breaking the build. Can I take 1-3? Or, do we need to wait until aarch64 PCI lands in mainline? 1 and 2 should be trivial since there is no change functionally. 3 mostly adding new files which should not get built if ARCH64 PCI is not supported based on the arch/arm64/Kconfig below. +++ b/arch/arm64/Kconfig @@ -9,6 +9,7 @@ config ARM64 select ARM_AMBA select ARM_ARCH_TIMER select ARM_GIC + select ARM_GIC_V2M if (PCI && PCI_MSI) select ARM_GIC_V3 select BUILDTIME_EXTABLE_SORT select CLONE_BACKWARDS The only thing is the change related to MSI in the irq-gic.c which should not affect with the non-PCI system. Thanks, Suravee -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3 V2] irqchip: gic: Add supports for ARM GICv2m MSI(-X)
On 7/3/2014 3:46 AM, Mark Rutland wrote: On Wed, Jul 02, 2014 at 09:04:09PM +0100, Suravee Suthikulanit wrote: Thanks again for the review. Please see my comments below. On 7/2/2014 11:39 AM, Mark Rutland wrote: On Wed, Jul 02, 2014 at 04:22:23PM +0100, suravee.suthikulpa...@amd.com wrote: From: Suravee Suthikulpanit diff --git a/Documentation/devicetree/bindings/arm/gic.txt b/Documentation/devicetree/bindings/arm/gic.txt index 5573c08..9e46f7f 100644 --- a/Documentation/devicetree/bindings/arm/gic.txt +++ b/Documentation/devicetree/bindings/arm/gic.txt @@ -12,11 +12,13 @@ Main node required properties: - compatible : should be one of: "arm,gic-400" + "arm,gic-400-plus" I am not keen on this name. Ok, I'll change it. Any suggestion on name? I'm not sure what is the "official" product name. I've seen this in some slides from ARM. What about "gic-400-v2m"??. I'll query this internally. Thanks. @@ -37,9 +39,16 @@ Main node required properties: the 8 possible cpus attached to the GIC. A bit set to '1' indicated the interrupt is wired to that CPU. Only valid for PPI interrupts. -- reg : Specifies base physical address(s) and size of the GIC registers. The - first region is the GIC distributor register base and size. The 2nd region is - the GIC cpu interface register base and size. +- reg : Specifies base physical address(s) and size of the GIC register frames. + + Region | Description + Index | + --- + 0 | GIC distributor register base and size + 1 | GIC cpu interface register base and size + 2 | VGIC interface control register base and size (Optional) + 3 | VGIC CPU interface register base and size (Optional) + 4 | GICv2m MSI interface register base and size (Optional) As far as I am aware, the MSI interface is completely orthogonal to having a GICH and GICV. Agree. I'm not doing anything with it. I'm just listing them here since they are also mentioned in the gic.txt We should figure out how we expect to handle that (zero-sized reg entries? reg-names?). I'm not sure how VGIC stuff handles reg/size = 0. Neither am I. However, what I said was that we should figure out how we _expect_ to handle that case. If we have to make changes to handle it, that's fine given we already have to make changes to support GICv2m. Looking through the code in virt/kvm/arm/vgic.c, I believe this would ended up returning -EINVAL or -ENXIO from kvm_vgic_hyp_init(). Optional - interrupts : Interrupt source of the parent interrupt controller on @@ -55,6 +64,10 @@ Optional by a crossbar/multiplexer preceding the GIC. The GIC irq input line is assigned dynamically when the corresponding peripheral's crossbar line is mapped. + +- msi-controller : Identifies the node as an MSI controller. This requires + the register region index 4. That last clarifying comment is more confusing than helpful. If you are referring to the table, I added that since it was easier to see than scanning the text. I was referring to "This requires the register region index 4". How about: - msi-controller : Identifies the node as an MSI controller. Requried for GICv2m. OK [...] +#define GIC_V2M_MIN_SPI32 +#define GIC_V2M_MAX_SPI1024 GIC interrupt IDs 1020 and above are reserved, no? Surely the max ID an SPI can take is 1019? Right, thanks for catching. But the spec says that the SPI ID value must be in the range of 32 to 1020. I guess it was a bit unclear, but definitely not 1024 :) Which spec? This was in the "Server Base System Architecture v1.0". In the GICv2 spec I see: * "Interrupt numbers ID32-ID1019 are used for SPIs." in 2.2.1 Interrupt IDs. * "The GIC architecture reserves interrupt ID numbers 1020-1023 for special purposes." in 3.2.5 Special interrupt numbers. +#define GIC_OF_MSIV2M_RANGE_INDEX 4 + +/** + * alloc_msi_irq - Allocate MSIs from avaialbe MSI bitmap. + * @data: Pointer to v2m_data + * @nvec: Number of interrupts to allocate + * @irq: Pointer to the allocated irq + * + * Allocates interrupts only if the contiguous range of MSIs + * with specified nvec are available. Otherwise return the number + * of available interrupts. If none are available, then returns -ENOENT. + */ This function is overly complicated, and pointlessly so. It doesn't achieve anything useful as it returns the size of the _last_ contiguous block rather than the _largest_ contiguous block, and the only caller (gicv2m_setup_msi_irq) doesn't even care. Simplify this to just return an error code if allocation is impossible. Actually, I made another mistake in the gicv2m_setup_msi_irq when r
Re: [PATCH 3/3 V2] irqchip: gic: Add supports for ARM GICv2m MSI(-X)
Thanks again for the review. Please see my comments below. On 7/2/2014 11:39 AM, Mark Rutland wrote: On Wed, Jul 02, 2014 at 04:22:23PM +0100, suravee.suthikulpa...@amd.com wrote: From: Suravee Suthikulpanit diff --git a/Documentation/devicetree/bindings/arm/gic.txt b/Documentation/devicetree/bindings/arm/gic.txt index 5573c08..9e46f7f 100644 --- a/Documentation/devicetree/bindings/arm/gic.txt +++ b/Documentation/devicetree/bindings/arm/gic.txt @@ -12,11 +12,13 @@ Main node required properties: - compatible : should be one of: "arm,gic-400" + "arm,gic-400-plus" I am not keen on this name. Ok, I'll change it. Any suggestion on name? I'm not sure what is the "official" product name. I've seen this in some slides from ARM. What about "gic-400-v2m"??. "arm,cortex-a15-gic" "arm,cortex-a9-gic" "arm,cortex-a7-gic" "arm,arm11mp-gic" - interrupt-controller : Identifies the node as an interrupt controller + - #interrupt-cells : Specifies the number of cells needed to encode an interrupt source. The type shall be a and the value shall be 3. Random (inconsistent) whitespace change? It looks to me like there should have been a space here to keep the consistent look, and make it easy to read @@ -37,9 +39,16 @@ Main node required properties: the 8 possible cpus attached to the GIC. A bit set to '1' indicated the interrupt is wired to that CPU. Only valid for PPI interrupts. -- reg : Specifies base physical address(s) and size of the GIC registers. The - first region is the GIC distributor register base and size. The 2nd region is - the GIC cpu interface register base and size. +- reg : Specifies base physical address(s) and size of the GIC register frames. + + Region | Description + Index | + --- + 0 | GIC distributor register base and size + 1 | GIC cpu interface register base and size + 2 | VGIC interface control register base and size (Optional) + 3 | VGIC CPU interface register base and size (Optional) + 4 | GICv2m MSI interface register base and size (Optional) As far as I am aware, the MSI interface is completely orthogonal to having a GICH and GICV. Agree. I'm not doing anything with it. I'm just listing them here since they are also mentioned in the gic.txt We should figure out how we expect to handle that (zero-sized reg entries? reg-names?). I'm not sure how VGIC stuff handles reg/size = 0. Optional - interrupts : Interrupt source of the parent interrupt controller on @@ -55,6 +64,10 @@ Optional by a crossbar/multiplexer preceding the GIC. The GIC irq input line is assigned dynamically when the corresponding peripheral's crossbar line is mapped. + +- msi-controller : Identifies the node as an MSI controller. This requires + the register region index 4. That last clarifying comment is more confusing than helpful. If you are referring to the table, I added that since it was easier to see than scanning the text. [...] +#define GIC_V2M_MIN_SPI32 +#define GIC_V2M_MAX_SPI1024 GIC interrupt IDs 1020 and above are reserved, no? Surely the max ID an SPI can take is 1019? Right, thanks for catching. But the spec says that the SPI ID value must be in the range of 32 to 1020. I guess it was a bit unclear, but definitely not 1024 :) +#define GIC_OF_MSIV2M_RANGE_INDEX 4 + +/** + * alloc_msi_irq - Allocate MSIs from avaialbe MSI bitmap. + * @data: Pointer to v2m_data + * @nvec: Number of interrupts to allocate + * @irq: Pointer to the allocated irq + * + * Allocates interrupts only if the contiguous range of MSIs + * with specified nvec are available. Otherwise return the number + * of available interrupts. If none are available, then returns -ENOENT. + */ This function is overly complicated, and pointlessly so. It doesn't achieve anything useful as it returns the size of the _last_ contiguous block rather than the _largest_ contiguous block, and the only caller (gicv2m_setup_msi_irq) doesn't even care. Simplify this to just return an error code if allocation is impossible. Actually, I made another mistake in the gicv2m_setup_msi_irq when returning from the alloc_msi_irq(). My understanding is the arch_setup_msi_irqs() is supposed to return the number of available vectors if the requested amount could not be allocated. I notice that the current "drivers/pci/msi.c: arch_setup_msi_irqs()" does not do so, which is okay. However, We are also working on adding support for multi-MSI support since some of the devices we have are using it, which means we will need to provide a different version of the "arch_setup_msi_irqs()" as the current one does not allow (PCI_CAP_ID_MSI && nvec > 1). Therefore, I implemented the "alloc_msi_irq" this way to account for futu
Re: [PATCH 2/2] arm/gic: Add supports for GICv2m MSI(-X)
On 6/24/2014 4:52 AM, Marc Zyngier wrote: Overall, this requires to be re-architected. If you want to have a look at the way I did the GICv3 ITS support: git://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git gicv3/its Thanks, Thanks for the review comments. I'll take a look at the GICv3 and re-architect for the V2 patch set. Suravee -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] arm/gic: Add supports for GICv2m MSI(-X)
Mark,
Thank you for all your comments. Please see my reply below. I have
omitted the minor ones.
On 6/24/2014 5:11 AM, Mark Rutland wrote:
On Tue, Jun 24, 2014 at 01:33:00AM +0100, suravee.suthikulpa...@amd.com wrote:
From: Suravee Suthikulpanit
+static int alloc_msi_irq(struct gicv2m_msi_data *data, int nvec, int *irq)
+{
+ int size = sizeof(unsigned long) * data->bm_sz;
+ int next = size, ret, num;
+
+ spin_lock(&data->msi_cnt_lock);
+
+ for (num = nvec; num > 0; num--) {
+ next = bitmap_find_next_zero_area(data->bm,
+ size, 0, num, 0);
+ if (next < size)
+ break;
+ }
If max_spi_cnt is not a multiple of BITS_PER_LONG, this can allocate a
number greater or equal to max_spi_cnt, but below size. We should never
allocate max_spi_cnt or above.
Thanks for the catch. I also need to clean up this logic for V2.
+ spin_unlock(&data->msi_cnt_lock);
+
+ return ret;
+}
+
+static void free_msi_irq(struct gicv2m_msi_data *data, unsigned int irq)
+{
+ int pos;
+
+ spin_lock(&data->msi_cnt_lock);
+
+ pos = irq - data->spi_start;
+ if (pos >= 0 && pos < data->max_spi_cnt)
Should either of these cases ever happen?
This is to check if the irq provided is within the MSI range.
+static int gicv2m_setup_msi_irq(struct msi_chip *chip, struct pci_dev *pdev,
+ struct msi_desc *desc)
+{
+ int avail, irq = 0;
+ struct msi_msg msg;
+ phys_addr_t addr;
+ struct gicv2m_msi_data *data = to_gicv2m_msi_data(chip);
+
+ if (data == NULL) {
If container_of returns NULL, you have bigger problems.
It was just sanity check. But, if you think this is obvious, I'll remove
this check then.
+int __init gicv2m_msi_init(struct device_node *node,
+ struct gicv2m_msi_data *msi)
+{
+ unsigned int val;
+ const __be32 *msi_be;
Every time I see a __be32* in a DT probe function I weep. There is no
need to deal with the internal details of the DTB.
+
+ msi_be = of_get_address(node, GIC_OF_MSIV2M_RANGE_INDEX, NULL, NULL);
+ if (!msi_be) {
+ pr_err("GICv2m failed. Fail to locate MSI base.\n");
+ return -EINVAL;
+ }
+
+ msi->paddr64 = of_translate_address(node, msi_be);
+ msi->base = of_iomap(node, GIC_OF_MSIV2M_RANGE_INDEX);
You can instead use of_address_to_resource to query the address from the
DTB once, without having to muck about with endianness conversion
manually. Take a look at what of_iomap does.
Thanks for this suggestion. I was not quite familiar with the "of_"
interface. I am cleaning up this whole section now.
I'm surprised we don't have an ioremap_resource given we have a devm_
variant.
+
+ /*
+ * MSI_TYPER:
+ * [31:26] Reserved
+ * [25:16] lowest SPI assigned to MSI
+ * [15:10] Reserved
+ * [9:0] Numer of SPIs assigned to MSI
+ */
+ val = __raw_readl(msi->base + MSI_TYPER);
Are you sure you want to use __raw_readl?
Probably not. I am replacing this with ioread32(msi->base + MSI_TYPER).
+ if (!val) {
+ pr_warn("GICv2m: Failed to read MSI_TYPER register\n");
+ return -EINVAL;
+ }
+
+ msi->spi_start = (val >> 16) & 0x3ff;
+ msi->max_spi_cnt = val & 0x3ff;
+
+ pr_debug("GICv2m: SPI = %u, cnt = %u\n",
+ msi->spi_start, msi->max_spi_cnt);
+
+ if (msi->spi_start < GIC_V2M_MIN_SPI ||
+ msi->max_spi_cnt >= GIC_V2M_MAX_SPI) {
+ pr_err("GICv2m: Init failed\n");
+ return -EINVAL;
+ }
+
+ msi->bm_sz = BITS_TO_LONGS(msi->max_spi_cnt);
So msi->bm_sz is msi->max_spi_cnt in _longs_ (rounded up)...
+ msi->bm = kzalloc(msi->bm_sz, GFP_KERNEL);
...yet we allocate that many _bytes_?
Sorry, I got a bit confused here. I'll fix this.
+
+ gic_arch_extn.irq_mask = gicv2m_mask_msi;
+ gic_arch_extn.irq_unmask = gicv2m_unmask_msi;
I'll leave others to comment on the validity of this...
Marc has commented this part in the other patch.
+ void __iomem *base; /* GICv2m virt address */
+ unsigned int spi_start; /* The SPI number that MSIs start */
+ unsigned int max_spi_cnt; /* The number of SPIs for MSIs */
+ unsigned long *bm;/* MSI vector bitmap */
+ unsigned long bm_sz; /* MSI bitmap size */
It's a bit odd in my mind that this is in longs. Why not just use
max_spi_cnt, which you can trivially use to determine bytes or longs?
That's true. I'm cleaning this up.
@@ -246,7 +254,8 @@ static int gic_set_affinity(struct irq_data *d, const
struct cpumask *mask_val,
bool force)
{
void __iomem *reg = gic_dist_base(d) + GIC_DIST_TARGET + (gic_irq(d) &
~3);
- unsigned int cpu, shift = (gic_irq(d)
Re: [PATCH 0/2] Introduce ARM GICv2m MSI(-X) support
On 6/24/2014 7:26 AM, Jason Cooper wrote: On Mon, Jun 23, 2014 at 07:32:58PM -0500, suravee.suthikulpa...@amd.com wrote: This patch set introduces support for MSI(-X) in GICv2m specification, which is implemented in some variation of GIC400. This depends on and has been tested with the V7 of "Add support for PCI in AArch64" (https://lkml.org/lkml/2014/3/14/320). Is this a build, boot, or runtime dependency? thx, Jason. For all of the above. Suravee -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] ahci: Check and set 64-bit DMA mask for platform AHCI driver
On 6/12/2014 12:47 PM, Sergei Shtylyov wrote:
diff --git a/drivers/ata/libahci_platform.c
b/drivers/ata/libahci_platform.c
index 3a5b4ed..a958a2b 100644
--- a/drivers/ata/libahci_platform.c
+++ b/drivers/ata/libahci_platform.c
@@ -364,6 +364,19 @@ int ahci_platform_init_host(struct
platform_device *pdev,
ap->ops = &ata_dummy_port_ops;
}
+if (hpriv->cap & HOST_CAP_64) {
+rc = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(64));
+if (rc) {
+rc = dma_coerce_mask_and_coherent(dev,
+ DMA_BIT_MASK(32));
+if (rc) {
+dev_err(dev, "Failed to enable 64-bit DMA.\n");
Not 32-bit?
Actually, I intended to say 64 since this is supposed to be setting up
64-bit DMA mask. Or we could just say failed to set up DMA mask.
Suravee
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/4] perf tools: allow user to specify hardware breakpoint bp_len
On 6/3/2014 6:55 AM, Jiri Olsa wrote: On Tue, Jun 03, 2014 at 10:36:22AM +0900, Namhyung Kim wrote: Hi Jiri, On Fri, 30 May 2014 15:39:06 +0200, Jiri Olsa wrote: On Thu, May 29, 2014 at 05:26:51PM +0200, Frederic Weisbecker wrote: From: Jacob Shin Currently bp_len is given a default value of 4. Allow user to override it: $ perf stat -e mem:0x1000/8 ^ bp_len If no value is given, it will default to 4 as it did before. Namhyung, both perf tols patches from this patchset mess up with hists tests.. I havent found any connection yet.. any idea? ;-) So you already found the problem in the hpp->elide change and that's the reason of the failure, right? :) I haven't got a chance to test it yet.. but it looks like thats the case.. I'll retest soon ;-) jirka Anything I can do to help here? Thanks, Suravee -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] ata: Check and set 64-bit DMA mask for platform AHCI driver
Hans/Bartlomiej,
Do you guys have any questions about this patch?
Thank you,
Suravee
On 6/3/2014 12:58 PM, Tejun Heo wrote:
Hans, Bartlomiej, can you guys please review this patch?
Thanks.
On Fri, May 23, 2014 at 12:35:10PM -0500, suravee.suthikulpa...@amd.com wrote:
From: Suravee Suthikulpanit
The current platform AHCI drier does not set the dma_mask correctly
for 64-bit DMA capable AHCI controller. This patch checks the AHCI
capability bit and set the dma_mask and coherent_dma_mask accordingly.
Signed-off-by: Suravee Suthikulpanit
---
drivers/ata/libahci_platform.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c
index 7cb3a85..85049ef 100644
--- a/drivers/ata/libahci_platform.c
+++ b/drivers/ata/libahci_platform.c
@@ -368,6 +368,15 @@ int ahci_platform_init_host(struct platform_device *pdev,
ahci_init_controller(host);
ahci_print_info(host, "platform");
+ if (hpriv->cap & HOST_CAP_64) {
+ if (!dev->dma_mask)
+ dev->dma_mask = &dev->coherent_dma_mask;
+
+ rc = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
+ if (rc)
+ return rc;
+ }
+
return ata_host_activate(host, irq, ahci_interrupt, IRQF_SHARED,
&ahci_platform_sht);
}
--
1.9.0
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/4] perf tools: add hardware breakpoint bp_len test cases
On 5/29/2014 10:26 AM, Frederic Weisbecker wrote:
@@ -1389,6 +1432,21 @@ static struct evlist_test test__events[] = {
.check = test__pinned_group,
.id= 41,
},
+ {
+ .name = "mem:0/1",
+ .check = test__checkevent_breakpoint_len,
+ .id= 42,
+ },
+ {
+ .name = "mem:0/2:w",
+ .check = test__checkevent_breakpoint_len_w,
+ .id= 43,
+ },
+ {
+ .name = "mem:0/4:rw:u",
+ .check = test__checkevent_breakpoint_len_rw_modifier,
+ .id= 44
+ },
#if defined(__s390x__)
{
.name = "kvm-s390:kvm_s390_create_vm",
Frederic,
This hunk seems to fail to apply on the main tree. Which tree are you
working on?
Suravee
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V5 3/4] x86/PCI: Stop enabling ECS for AMD CPUs after Fam16h
On 5/22/2014 9:54 PM, Bjorn Helgaas wrote: I've been poking around for recent dmesg logs that contain "PCI: Using configuration type 1 for extended access", and there are quite a few. In most cases there*is* an MCFG table, but apparently we decide not to use it for some reason (unfortunately we don't print the specific reason). One example is at https://bugzilla.kernel.org/show_bug.cgi?id=68591 . I'm going to go out on a limb and guess that Windows does not enable ECS, so it probably uses ECAM. Therefore, I suspect Linux's parsing of MCFG is broken in some way, and we probably*could* use ECAM in all these cases I'm seeing. It would probably be prudent to figure out why Linux is rejecting these MCFG tables. We'll probably see similar tables on Fam17h systems, and if we continue rejecting them, and we don't turn on ECS, we won't be able to access extended config space. I opened a bugzilla for this issue: https://bugzilla.kernel.org/show_bug.cgi?id=76771 I'm wavering on whether it's a good idea to put this patch in before understanding the issue. As much as I'd like to stop fiddling with ECS, we'd likely end up with a v3.15 where extended config space doesn't work on some Fam17h systems. So, I have located a system which presents issue with MMCONFIG. Here is my investigation: DEBUG: pci_io_ecs_init: pci_probe = 4000f ACPI: bus type PCI registered DEBUG: -> pci_mmcfg_early_init DEBUG: pci_parse_mcfg PCI: MMCONFIG for domain [bus 00-01] at [mem 0xe000-0xe01f] (base 0xe000) DEBUG: pci_mmcfg_check_reserved DEBUG: is_mmconf_reserved: method = E820 PCI: not using MMCONFIG DEBUG: pci_direct_init PCI: Using configuration type 1 for base access PCI: Using configuration type 1 for extended access ACPI: Added _OSI(Module Device) ACPI: Added _OSI(Processor Device) ACPI: Added _OSI(3.0 _SCP Extensions) ACPI: Added _OSI(Processor Aggregator Device) [Firmware Bug]: ACPI: BIOS _OSI(Linux) query ignored \_SB_:_OSC invalid UUID _OSC request data:1 1f ACPI: Interpreter enabled ACPI Exception: AE_NOT_FOUND, While evaluating Sleep State [\_S1_] (20140214/hwxface-580) ACPI Exception: AE_NOT_FOUND, While evaluating Sleep State [\_S2_] (20140214/hwxface-580) ACPI Exception: AE_NOT_FOUND, While evaluating Sleep State [\_S4_] (20140214/hwxface-580) ACPI: (supports S0 S3 S5) ACPI: Using IOAPIC for interrupt routing DEBUG: > pci_mmcfg_late_init DEBUG: pci_parse_mcfg PCI: MMCONFIG for domain [bus 00-01] at [mem 0xe000-0xe01f] (base 0xe000) DEBUG: pci_mmcfg_check_reserved DEBUG: is_mmconf_reserved: method = ACPI motherboard resources PCI: MMCONFIG at [mem 0xe000-0xe01f] reserved in ACPI motherboard resources During pci_mmcfg_early_init(), the MMCONFIG failed because the range 0xe000 is not showing as reserved in the E820 mapping. Here is the snippet of E820 mapping from the system: BIOS-e820: [mem 0xc7eb-0xc7ec0fff] ACPI data BIOS-e820: [mem 0xc7ec1000-0xc7ec2fff] ACPI NVS BIOS-e820: [mem 0xc7ec3000-0xc7efefff] reserved BIOS-e820: [mem 0xc7f0-0xc7ff] reserved BIOS-e820: [mem 0xfec0-0xfec0] reserved However, during pci_mmcfg_late_init(), the area is reserved in the "ACPI motherboard resources", and the pci_mmcfg_check_reserved() does not fail here. But this is too late since we already setup the "raw_pci_ext_ops" in the "arch/x86/pci/direct.c: pci_direct_init()" (during to use the IO_ECS. Suravee -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V5 3/4] x86/PCI: Stop enabling ECS for AMD CPUs after Fam16h
On 5/23/2014 6:56 AM, Robert Richter wrote: On 22.05.14 20:54:54, Bjorn Helgaas wrote: I'm going to go out on a limb and guess that Windows does not enable ECS, so it probably uses ECAM. Therefore, I suspect Linux's parsing of MCFG is broken in some way, and we probably *could* use ECAM in all these cases I'm seeing. Even if ECS is not enabled the system should be fine anyway, as ECS is only used to enable certain features. For family 10h this was originally the IBS EILVT (extended interrupt local vector table, needed for hw profiling) setup which need to be set by the OS which the BIOS didn't right. This should be fixed now and properly set by the BIOS on 15h+ systems. I don't remember what was added to 16h where ECS was needed, I think there was one (Suravee?). Not sure if this is essential. I am not aware of anything specific in the family16h which require IO_ECS to be enabled. Suravee -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V5 0/4] x86/pci Fix numa_node info for AMD hostbridge and misc clean up
[Resending due to mail client crashing] On 5/21/2014 6:17 PM, Bjorn Helgaas wrote: [resending because I forgot to copy the lists, sorry guys] Hi Suravee, Sorry it took me so long to get to these patches. Here's my proposal. I reordered them and added some comments in the code and changelogs, but I think your patches look fine as-is. So I just need comments on these two significant changes: 1) I added a patch to stop enabling ECS after Fam16h, because that's another case of CPU-dependent code that we should not need to keep carrying. I don't think there are any post-Fam16h CPUs yet, but I certainly don't want to do anything that will keep them from working when they do arrive. It would be useful if somebody could test this on current platforms by tweaking the patch so we don't enable ECS on Fam15h. I'm okay with this. In my V4 patch, I also tested disabling ECS on family15h already and that seems to work fine. But I would not mind deprecate this for post fam16h processors. 2) I dropped the quirk_amd_nb_node() removal. I could be convinced otherwise, but I don't really object to the quirk because it is already explicitly limited to specific devices, and removing it will change things in sysfs. I think the changes would be harmless as far as the kernel is concerned, since there are no drivers for these devices. But Andreas added the quirk because of complaints, so apparently somebody is looking at what's in sysfs, and I don't want to get the same complaints again by removing it. However, I will certainly ask questions if I see the quirk being extended to more devices. I'm okay with keeping this. The AMD BKDG does say the BIOS should provide an MCFG table (sec 2.8 of 42301), so I think it provides guidance matching the intent of my "stop enabling ECS" patch. But the BKDG doesn't mention _PXM at all. Is there any chance you could squeeze in a mention of that, so BIOS writers know that they *should* provide it? I want to avoid more fire-drills in the future. Bjorn The ship for family15h stuff have sailed and we probably would not be able to get them to change. We are going to have to keep an eye on the future platforms to make sure that the _PXM info is documented for future platforms. I have tested this patch set and they seems to be ok now. Suravee --- Bjorn Helgaas (1): x86/PCI: Stop enabling ECS for AMD CPUs after Fam16h Myron Stowe (1): x86/PCI: Warn if we have to "guess" host bridge node information Suravee Suthikulpanit (2): x86/PCI: Work around AMD Fam15h BIOSes that fail to provide _PXM x86/PCI: Clean up and mark early_root_info_init() as deprecated arch/x86/pci/acpi.c|6 +++ arch/x86/pci/amd_bus.c | 87 +++- 2 files changed, 62 insertions(+), 31 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V5 0/4] x86/pci Fix numa_node info for AMD hostbridge and misc clean up
On 5/21/2014 6:17 PM, Bjorn Helgaas wrote: [resending because I forgot to copy the lists, sorry guys] Hi Suravee, Sorry it took me so long to get to these patches. Here's my proposal. I reordered them and added some comments in the code and changelogs, but I think your patches look fine as-is. So I just need comments on these two significant changes: 1) I added a patch to stop enabling ECS after Fam16h, because that's another case of CPU-dependent code that we should not need to keep carrying. I don't think there are any post-Fam16h CPUs yet, but I certainly don't want to do anything that will keep them from working when they do arrive. It would be useful if somebody could test this on current platforms by tweaking the patch so we don't enable ECS on Fam15h. I'm okay with this. In my V4 patch, I also tested disabling ECS on family15h already and that seems to work fine. But I would not mind deprecate this for post fam16h processors. 2) I dropped the quirk_amd_nb_node() removal. I could be convinced otherwise, but I don't really object to the quirk because it is already explicitly limited to specific devices, and removing it will change things in sysfs. I think the changes would be harmless as far as the kernel is concerned, since there are no drivers for these devices. But Andreas added the quirk because of complaints, so apparently somebody is looking at what's in sysfs, and I don't want to get the same complaints again by removing it. However, I will certainly ask questions if I see the quirk being extended to more devices. I'm okay with keeping this. The AMD BKDG does say the BIOS should provide an MCFG table (sec 2.8 of 42301), so I think it provides guidance matching the intent of my "stop enabling ECS" patch. But the BKDG doesn't mention _PXM at all. Is there any chance you could squeeze in a mention of that, so BIOS writers know that they *should* provide it? I want to avoid more fire-drills in the future. The ship for family15h stuff have sailed and we probably would not be able to get them to change. We are going to have to keep an eye on the future platforms to make sure that the _PXM info is documented for future platforms. I have tested this patch set and they seems to be ok now. Suravee Bjorn --- Bjorn Helgaas (1): x86/PCI: Stop enabling ECS for AMD CPUs after Fam16h Myron Stowe (1): x86/PCI: Warn if we have to "guess" host bridge node information Suravee Suthikulpanit (2): x86/PCI: Work around AMD Fam15h BIOSes that fail to provide _PXM x86/PCI: Clean up and mark early_root_info_init() as deprecated arch/x86/pci/acpi.c|6 +++ arch/x86/pci/amd_bus.c | 87 +++- 2 files changed, 62 insertions(+), 31 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V5 3/4] x86/PCI: Stop enabling ECS for AMD CPUs after Fam16h
On 5/22/2014 3:20 PM, Bjorn Helgaas wrote: On Thu, May 22, 2014 at 1:17 PM, Borislav Petkov wrote: On Thu, May 22, 2014 at 11:56:03AM -0600, Bjorn Helgaas wrote: I chose Fam16h (0x16) because it looks like that's the newest stuff that's in the field. I suspect things would probably work if we changed this patch to leave ECS disabled on some Fam16h, Fam15h, etc., but that would change behavior on existing systems, which obviously adds some risk. I didn't think there was much benefit that makes the risk worthwhile. My goal is to stop needing CPU-specific changes in the future, not necessarily to remove the CPU-specific code we already have. Does that make sense? I'm not sure whether I understood your real question. No, you got it right. I'm just wondering why only the newest stuff. MMCONFIG is supposed to work just fine on everything from Fam10h onwards, I'm not sure all Fam10h supported it though. Maybe Suravee can verify that... Even if MMCONFIG does work fine on everything from Fam10h onwards, we still depend on the BIOS to provide a correct MCFG table. I don't think we can guarantee that changing from ECS to MMCONFIG on a Fam16h box in the field is safe, because we'd then be using a feature we've never used before. Bjorn At this point, family11h and later (upto 16h which is our most current processor) should already have supports for the MCFG. However, we can't guarantee that all the systems currently out there would not use the ECS. So, I think it is ok to say we won't support it post 16h as Bjorn suggests. Suravee -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V3 1/3] x86/PCI: Fix PCI root numa_node info on AMD family15h
On 5/8/2014 10:14 AM, Robert Richter wrote:
On 08.05.14 09:39:55, Suravee Suthikulanit wrote:
On 5/8/2014 4:01 AM, Robert Richter wrote:
On 08.05.14 10:59:05, Robert Richter wrote:
On 07.05.14 13:58:45, suravee.suthikulpa...@amd.com wrote:
@@ -113,10 +122,17 @@ static int __init early_fill_mp_bus_info(void)
info = alloc_pci_root_info(min_bus, max_bus, node, link);
}
+ /*
+* The following code is only supported until Fam11h.
+* Newer processors will depend on ACPI MCFG table instead.
+*/
+ if (boot_cpu_data.x86 > 0x11)
+ return 0;
+
/* get the default node and link for left over res */
As this is the only substantial change of your patch, I would better
drop ther rest or at least split it in two patches. Should this change
also be for stable?
Of course adding the hostbridge must be also part of the patch, didn't
note this due to the other noise. See why the split would be good?
-Robert
Robert,
I have already added the hostbridge for family15h in this patch.
+static struct amd_hostbridge hb_probes[] __initdata = {
+ { 0, 0x18, 0x1100 }, /* K8 */
+ { 0, 0x18, 0x1200 }, /* Family10h */
+ { 0xff, 0, 0x1200 }, /* Family10h */
+ { 0, 0x18, 0x1300 }, /* Family11h */
+ { 0, 0x18, 0x1600 }, /* Family15h */ <--- HERE
Yes, I noticed that, but later, thus my 2nd mail.
};
The rest of the changes are mostly comments, some minor renaming of
variables for clarity, and replace hardcode values with preprocessor macro.
If needed, I can split them.
I just would drop it, you just need the fam15h device and the cpu mode
check.
-Robert
The reason I put it all these comments here is because it took us a
while to discuss what to do with this file going forward. There were
some confusions. Therefore, I just want to document it here.
Also, the check for (boot_cpu_data.x86 > 0x11) was needed because it
should not be done for family15h.
Suravee
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V3 1/3] x86/PCI: Fix PCI root numa_node info on AMD family15h
On 5/8/2014 4:01 AM, Robert Richter wrote:
On 08.05.14 10:59:05, Robert Richter wrote:
On 07.05.14 13:58:45, suravee.suthikulpa...@amd.com wrote:
@@ -113,10 +122,17 @@ static int __init early_fill_mp_bus_info(void)
info = alloc_pci_root_info(min_bus, max_bus, node, link);
}
+ /*
+* The following code is only supported until Fam11h.
+* Newer processors will depend on ACPI MCFG table instead.
+*/
+ if (boot_cpu_data.x86 > 0x11)
+ return 0;
+
/* get the default node and link for left over res */
As this is the only substantial change of your patch, I would better
drop ther rest or at least split it in two patches. Should this change
also be for stable?
Of course adding the hostbridge must be also part of the patch, didn't
note this due to the other noise. See why the split would be good?
-Robert
Robert,
I have already added the hostbridge for family15h in this patch.
+static struct amd_hostbridge hb_probes[] __initdata = {
+ { 0, 0x18, 0x1100 }, /* K8 */
+ { 0, 0x18, 0x1200 }, /* Family10h */
+ { 0xff, 0, 0x1200 }, /* Family10h */
+ { 0, 0x18, 0x1300 }, /* Family11h */
+ { 0, 0x18, 0x1600 }, /* Family15h */ <--- HERE
};
The rest of the changes are mostly comments, some minor renaming of
variables for clarity, and replace hardcode values with preprocessor
macro. If needed, I can split them.
Suravee
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] iommu/amd: Take mmap_sem when calling get_user_pages
On 5/6/2014 2:11 PM, Suravee Suthikulanit wrote: On 5/6/2014 1:14 PM, Joerg Roedel wrote: On Mon, Apr 28, 2014 at 05:27:46PM -0500, Suthikulpanit, Suravee wrote: From: Jay Cornwall get_user_pages requires caller to hold a read lock on mmap_sem. Right, but can't we just switch to get_user_pages_fast instead? Joerg You're right. I think it can. Let me spin out another patch. Suravee Actually, we would have to provide the pointer to task_struct for the corresponding PASID which is not necessary "current". So I take it back. I don't think we can use _fast. Suravee -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] iommu/amd: Take mmap_sem when calling get_user_pages
On 5/6/2014 1:14 PM, Joerg Roedel wrote: On Mon, Apr 28, 2014 at 05:27:46PM -0500, Suthikulpanit, Suravee wrote: From: Jay Cornwall get_user_pages requires caller to hold a read lock on mmap_sem. Right, but can't we just switch to get_user_pages_fast instead? Joerg You're right. I think it can. Let me spin out another patch. Suravee -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
