* [PATCH v2] ARM: davinci: da8xx: Fix sleeping function called from invalid context
From: Alexandre Bailon @ 2016-11-29 16:48 UTC (permalink / raw)
To: linux-arm-kernel
Everytime the usb20 phy is enabled, there is a
"sleeping function called from invalid context" BUG.
clk_enable() from arch/arm/mach-davinci/clock.c uses spin_lock_irqsave()
before to invoke the callback usb20_phy_clk_enable().
usb20_phy_clk_enable() uses clk_get() and clk_enable_prepapre()
which may sleep.
Move clk_get() to da8xx_register_usb20_phy_clk() and
replace clk_prepare_enable() by clk_enable().
Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
---
arch/arm/mach-davinci/usb-da8xx.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/arch/arm/mach-davinci/usb-da8xx.c b/arch/arm/mach-davinci/usb-da8xx.c
index b010e5f..2baa0e1 100644
--- a/arch/arm/mach-davinci/usb-da8xx.c
+++ b/arch/arm/mach-davinci/usb-da8xx.c
@@ -156,23 +156,18 @@ int __init da8xx_register_usb_refclkin(int rate)
return 0;
}
+static struct clk *usb20_clk;
+
static void usb20_phy_clk_enable(struct clk *clk)
{
- struct clk *usb20_clk;
int err;
u32 val;
u32 timeout = 500000; /* 500 msec */
val = readl(DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
- usb20_clk = clk_get(&da8xx_usb20_dev.dev, "usb20");
- if (IS_ERR(usb20_clk)) {
- pr_err("could not get usb20 clk: %ld\n", PTR_ERR(usb20_clk));
- return;
- }
-
/* The USB 2.O PLL requires that the USB 2.O PSC is enabled as well. */
- err = clk_prepare_enable(usb20_clk);
+ err = clk_enable(usb20_clk);
if (err) {
pr_err("failed to enable usb20 clk: %d\n", err);
clk_put(usb20_clk);
@@ -197,8 +192,7 @@ static void usb20_phy_clk_enable(struct clk *clk)
pr_err("Timeout waiting for USB 2.0 PHY clock good\n");
done:
- clk_disable_unprepare(usb20_clk);
- clk_put(usb20_clk);
+ clk_disable(usb20_clk);
}
static void usb20_phy_clk_disable(struct clk *clk)
@@ -287,9 +281,15 @@ int __init da8xx_register_usb20_phy_clk(bool use_usb_refclkin)
struct clk *parent;
int ret = 0;
+ usb20_clk = clk_get(&da8xx_usb20_dev.dev, "usb20");
+ if (IS_ERR(usb20_clk))
+ return PTR_ERR(parent);
+
parent = clk_get(NULL, use_usb_refclkin ? "usb_refclkin" : "pll0_aux");
- if (IS_ERR(parent))
+ if (IS_ERR(parent)) {
+ clk_put(usb20_clk);
return PTR_ERR(parent);
+ }
usb20_phy_clk.parent = parent;
ret = clk_register(&usb20_phy_clk);
--
2.7.3
^ permalink raw reply related
* [PATCH] KVM: arm/arm64: Access CNTHCTL_EL2 bit fields correctly
From: Suzuki K Poulose @ 2016-11-29 16:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <0a475329-6f50-42ae-3244-f0767c56ce09@arm.com>
On 29/11/16 09:36, Marc Zyngier wrote:
> On 29/11/16 03:28, Jintack Lim wrote:
>> On Mon, Nov 28, 2016 at 1:39 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
>>> On 28/11/16 17:43, Marc Zyngier wrote:
>> This looks much cleaner than my patch.
>> While we are at it, is it worth to consider that we just need to set
>> those bits once for VHE case, not for every world switch as an
>> optimization?
>
> Ah! That's a much better idea indeed! And we could stop messing with
> cntvoff_el2 as well, as it doesn't need to be restored to zero on exit.
> Could you try and respin something along those lines?
>
fyi, we have a static_key based cpus_have_const_cap() for Constant cap
checking (like this case) available in linux-next. May be you could make use
of that instead of alternatives.
Cheers
Suzuki
^ permalink raw reply
* [PATCH v7 04/16] drivers: iommu: make of_iommu_set/get_ops() DT agnostic
From: Will Deacon @ 2016-11-29 17:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161129160549.GH2078@8bytes.org>
On Tue, Nov 29, 2016 at 05:05:49PM +0100, Joerg Roedel wrote:
> On Wed, Nov 16, 2016 at 09:56:15AM +0000, Lorenzo Pieralisi wrote:
> > I can easily make the changes Robin suggests above, I need to know
> > what to do with this patch it is the last blocking point for this
> > series and time is running out I can revert to using dev->bus to
> > retrieve iommu_ops (even though I do not think it makes sense given
> > what Robin outlines below) but I need to know please, we can't gate
> > an entire series for this patch that is just syntactic sugar.
>
> Well, I didn't really object to the approach per-se, I just wanted to
> know the rationale behind the need for the iommu-ops pointer. So through
> which tree should this series be merged?
I was just about to send a pull request to you, since it conflicts with
my ARM SMMU patches and the PCI/ACPI bits are acked by Rafael and Bjorn.
My for-joerg/arm-smmu/updates is ready to go; just need to write the
pull request.
Will
^ permalink raw reply
* [RFC v2 PATCH 11/23] ARM: sleep: allow it to be build for R-class
From: Russell King - ARM Linux @ 2016-11-29 17:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480423205-48436-12-git-send-email-vladimir.murzin@arm.com>
On Tue, Nov 29, 2016 at 12:39:53PM +0000, Vladimir Murzin wrote:
> Dependency on MMU is quite strict and prevent R-class from being built -
> relax this condition and guard against M-class only
I thought I'd already commented on some of this change, but it seems
not.
> diff --git a/arch/arm/kernel/sleep.S b/arch/arm/kernel/sleep.S
> index 0f6c100..0e7fddf 100644
> --- a/arch/arm/kernel/sleep.S
> +++ b/arch/arm/kernel/sleep.S
> @@ -119,14 +119,12 @@ ENDPROC(cpu_resume_after_mmu)
> .text
> .align
>
> -#ifdef CONFIG_MMU
> .arm
> ENTRY(cpu_resume_arm)
> THUMB( badr r9, 1f ) @ Kernel is entered in ARM.
> THUMB( bx r9 ) @ If this is a Thumb-2 kernel,
> THUMB( .thumb ) @ switch to Thumb now.
> THUMB(1: )
> -#endif
>
> ENTRY(cpu_resume)
> ARM_BE8(setend be) @ ensure we are in BE mode
> @@ -160,9 +158,7 @@ THUMB( mov sp, r2 )
> THUMB( bx r3 )
> ENDPROC(cpu_resume)
>
> -#ifdef CONFIG_MMU
> ENDPROC(cpu_resume_arm)
> -#endif
These ifdefs were introduced to fix EFM32. The commit description needs
to state why it's safe to remove them now (presumably because of the
dependency on !CPU_V7M). However, it also needs to explain why it's
not going to cause a regression for EFM32 - EFM32 _was_ capable of
building this code.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply
* [RFC v2 PATCH 08/23] ARM: NOMMU: implement secondary_startup_arm
From: Russell King - ARM Linux @ 2016-11-29 17:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480423205-48436-9-git-send-email-vladimir.murzin@arm.com>
On Tue, Nov 29, 2016 at 12:39:50PM +0000, Vladimir Murzin wrote:
> Mediatek's and Qualcomm's platform code has reference to
> secondary_startup_arm and that breaks NOMMU build.
This needs to explain why this is safe in the presence of EFM32.
> Cc: Russell King <linux@armlinux.org.uk>
> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
> ---
> arch/arm/kernel/head-nommu.S | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/arch/arm/kernel/head-nommu.S b/arch/arm/kernel/head-nommu.S
> index 7317554..2ab026f 100644
> --- a/arch/arm/kernel/head-nommu.S
> +++ b/arch/arm/kernel/head-nommu.S
> @@ -89,6 +89,12 @@ ENDPROC(stext)
>
> #ifdef CONFIG_SMP
> .text
> + .arm
> +ENTRY(secondary_startup_arm)
> + THUMB( badr r9, 1f ) @ Kernel is entered in ARM.
> + THUMB( bx r9 ) @ If this is a Thumb-2 kernel,
> + THUMB( .thumb ) @ switch to Thumb now.
> + THUMB(1: )
> ENTRY(secondary_startup)
> /*
> * Common entry point for secondary CPUs.
> @@ -126,6 +132,7 @@ ENTRY(secondary_startup)
> mov fp, #0
> b secondary_start_kernel
> ENDPROC(secondary_startup)
> +ENDPROC(secondary_startup_arm)
>
> .type __secondary_data, %object
> __secondary_data:
> --
> 1.7.9.5
>
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply
* [PATCH 15/22] arm64/cpuinfo: Make hotplug notifier symmetric
From: Suzuki K Poulose @ 2016-11-29 17:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161126231350.10321-16-bigeasy@linutronix.de>
On 26/11/16 23:13, Sebastian Andrzej Siewior wrote:
> From: Anna-Maria Gleixner <anna-maria@linutronix.de>
>
> There is no requirement to keep the sysfs files around until the CPU is
> completely dead. Remove them during the DOWN_PREPARE notification. This is
> a preparatory patch for converting to the hotplug state machine.
>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: linux-arm-kernel at lists.infradead.org
> Signed-off-by: Anna-Maria Gleixner <anna-maria@linutronix.de>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
> arch/arm64/kernel/cpuinfo.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
> index b3d5b3e8fbcb..19aad7041e14 100644
> --- a/arch/arm64/kernel/cpuinfo.c
> +++ b/arch/arm64/kernel/cpuinfo.c
> @@ -272,9 +272,10 @@ static int cpuid_callback(struct notifier_block *nb,
>
> switch (action & ~CPU_TASKS_FROZEN) {
> case CPU_ONLINE:
> + case CPU_DOWN_FAILED:
> rc = cpuid_add_regs(cpu);
> break;
> - case CPU_DEAD:
> + case CPU_DOWN_PREPARE:
> rc = cpuid_remove_regs(cpu);
> break;
> }
Looks reasonable to me:
Reviewed-by : Suzuki K Poulose <suzuki.poulose@arm.com>
^ permalink raw reply
* [PATCH v2] phy: phy-sun4i-usb: Add log when probing
From: Quentin Schulz @ 2016-11-29 17:22 UTC (permalink / raw)
To: linux-arm-kernel
When phy-sun4i-usb's probing fails, it does not print the reason in
kernel log, forcing the developer to edit this driver to add info logs.
This commit makes the kernel print the reason of phy-sun4i-usb's probing
failure or a success message.
Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
---
Sorry for the delay, I completely forgot to send a v2.
v2:
- removing (from v1) debug message when devm_ioremap_resource fails,
- changing dev_info to dev_dbg when driver is successfully loaded,
drivers/phy/phy-sun4i-usb.c | 25 ++++++++++++++++++++-----
1 file changed, 20 insertions(+), 5 deletions(-)
diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
index fec34f5..f1b2848 100644
--- a/drivers/phy/phy-sun4i-usb.c
+++ b/drivers/phy/phy-sun4i-usb.c
@@ -632,19 +632,25 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
data->id_det_gpio = devm_gpiod_get_optional(dev, "usb0_id_det",
GPIOD_IN);
- if (IS_ERR(data->id_det_gpio))
+ if (IS_ERR(data->id_det_gpio)) {
+ dev_err(dev, "Couldn't request ID GPIO\n");
return PTR_ERR(data->id_det_gpio);
+ }
data->vbus_det_gpio = devm_gpiod_get_optional(dev, "usb0_vbus_det",
GPIOD_IN);
- if (IS_ERR(data->vbus_det_gpio))
+ if (IS_ERR(data->vbus_det_gpio)) {
+ dev_err(dev, "Couldn't request VBUS detect GPIO\n");
return PTR_ERR(data->vbus_det_gpio);
+ }
if (of_find_property(np, "usb0_vbus_power-supply", NULL)) {
data->vbus_power_supply = devm_power_supply_get_by_phandle(dev,
"usb0_vbus_power-supply");
- if (IS_ERR(data->vbus_power_supply))
+ if (IS_ERR(data->vbus_power_supply)) {
+ dev_err(dev, "Couldn't get the VBUS power supply\n");
return PTR_ERR(data->vbus_power_supply);
+ }
if (!data->vbus_power_supply)
return -EPROBE_DEFER;
@@ -653,8 +659,10 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
data->dr_mode = of_usb_get_dr_mode_by_phy(np, 0);
data->extcon = devm_extcon_dev_allocate(dev, sun4i_usb_phy0_cable);
- if (IS_ERR(data->extcon))
+ if (IS_ERR(data->extcon)) {
+ dev_err(dev, "Couldn't allocate our extcon device\n");
return PTR_ERR(data->extcon);
+ }
ret = devm_extcon_dev_register(dev, data->extcon);
if (ret) {
@@ -669,8 +677,13 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
snprintf(name, sizeof(name), "usb%d_vbus", i);
phy->vbus = devm_regulator_get_optional(dev, name);
if (IS_ERR(phy->vbus)) {
- if (PTR_ERR(phy->vbus) == -EPROBE_DEFER)
+ if (PTR_ERR(phy->vbus) == -EPROBE_DEFER) {
+ dev_err(dev,
+ "Couldn't get regulator %s... Deferring probe\n",
+ name);
return -EPROBE_DEFER;
+ }
+
phy->vbus = NULL;
}
@@ -754,6 +767,8 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
return PTR_ERR(phy_provider);
}
+ dev_dbg(dev, "successfully loaded\n");
+
return 0;
}
--
2.9.3
^ permalink raw reply related
* [RFC v2 PATCH 23/23] ARM: Allow ARCH_MULTIPLATFORM to be selected for NOMMU
From: Russell King - ARM Linux @ 2016-11-29 17:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480423205-48436-24-git-send-email-vladimir.murzin@arm.com>
On Tue, Nov 29, 2016 at 12:40:05PM +0000, Vladimir Murzin wrote:
> With this patch applied potentially any platform can be built in NOMMU
> configurations if CONFIG_EXPERT is selected. However, there is no
> guaranty that platform can successfully run such Image. So the main
guarantee
> motivation behind of this patch:
> - bring build coverage for NOMMU configurations
> - allow known working NOMMU platforms (like R-class) to be used
> - pave a way to add support for single address space (aka 1:1 mapping)
> for MMU platforms, so they can be usable in NOMMU configurations
>
> Cc: Hartley Sweeten <hsweeten@visionengravers.com>
> Cc: Ryan Mallon <rmallon@gmail.com>
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Cc: Alexander Shiyan <shc_work@mail.ru>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Sascha Hauer <kernel@pengutronix.de>
> Cc: Fabio Estevam <fabio.estevam@nxp.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Russell King <linux@armlinux.org.uk>
> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
> ---
> arch/arm/Kconfig | 21 +++++++++++++++++++--
> 1 file changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index e78c822..bc6f406 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -327,9 +327,9 @@ choice
>
> config ARCH_MULTIPLATFORM
> bool "Allow multiple platforms to be selected"
> - depends on MMU
> + depends on MMU || EXPERT
> select ARM_HAS_SG_CHAIN
> - select ARM_PATCH_PHYS_VIRT
> + select ARM_PATCH_PHYS_VIRT if MMU
> select AUTO_ZRELADDR
> select CLKSRC_OF
> select COMMON_CLK
> @@ -339,6 +339,23 @@ config ARCH_MULTIPLATFORM
> select PCI_DOMAINS if PCI
> select SPARSE_IRQ
> select USE_OF
> + help
> + Please, read carefully if you've selected CONFIG_MMU=n!
> +
> + Multiplatform with !MMU configuration *is not* meant that
> + kernel built to support every platform will boot on them. It
> + is because physical address space layouts (particularly where
> + RAM is located) are different between platforms and there is
> + no MMU to work that around.
> +
> + You must specify where RAM start (via DRAM_BASE config
> + option) and appropriate size of RAM (via DRAM_SIZE config
> + option) which are valid for the platform you are building
> + for.
> +
> + This feature is *EXPERIMENTAL*, please, consider building
> + with CONFIG_MMU=y unless you know what you do or want to
> + help with testing.
Do you actually see this help text anywhere? I don't think multiple-choice
options show help for individual choices.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply
* [PATCH] crypto: arm/aesbs - fix brokenness after skcipher conversion
From: Ard Biesheuvel @ 2016-11-29 17:23 UTC (permalink / raw)
To: linux-arm-kernel
The CBC encryption routine should use the encryption round keys, not
the decryption round keys.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
Another fix for the queued changes, this time for 32-bit ARM.
I must say, I'm not impressed with the level of testing that has been
carried out after applying these changes. If you had cc'd me on these
patches, I would have been happy to test/review.
arch/arm/crypto/aesbs-glue.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/crypto/aesbs-glue.c b/arch/arm/crypto/aesbs-glue.c
index f5eafce10557..d8e06de72ef3 100644
--- a/arch/arm/crypto/aesbs-glue.c
+++ b/arch/arm/crypto/aesbs-glue.c
@@ -113,7 +113,7 @@ static inline void aesbs_encrypt_one(struct crypto_skcipher *tfm,
{
struct aesbs_cbc_ctx *ctx = crypto_skcipher_ctx(tfm);
- AES_encrypt(src, dst, &ctx->dec.rk);
+ AES_encrypt(src, dst, &ctx->enc);
}
static int aesbs_cbc_encrypt(struct skcipher_request *req)
--
2.7.4
^ permalink raw reply related
* [PATCH 16/22] arm64/cpuinfo: Convert to hotplug state machine
From: Suzuki K Poulose @ 2016-11-29 17:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161126231350.10321-17-bigeasy@linutronix.de>
On 26/11/16 23:13, Sebastian Andrzej Siewior wrote:
> From: Anna-Maria Gleixner <anna-maria@linutronix.de>
>
> Install the callbacks via the state machine and let the core invoke
> the callbacks on the already online CPUs.
>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: linux-arm-kernel at lists.infradead.org
> Signed-off-by: Anna-Maria Gleixner <anna-maria@linutronix.de>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
> arch/arm64/kernel/cpuinfo.c | 37 +++++++++----------------------------
> 1 file changed, 9 insertions(+), 28 deletions(-)
>
> diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
> index 19aad7041e14..7b7be71e87bf 100644
> --- a/arch/arm64/kernel/cpuinfo.c
> +++ b/arch/arm64/kernel/cpuinfo.c
> @@ -227,7 +227,7 @@ static struct attribute_group cpuregs_attr_group = {
...
> + ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "arm64/cpuinfo:online",
> + cpuid_cpu_online, cpuid_cpu_offline);
> + if (ret < 0) {
> + pr_err("cpuinfo: failed to register hotplug callbacks.\n");
> + return ret;
> + }
> return 0;
> }
Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
^ permalink raw reply
* [GIT PULL] iommu/arm-smmu: Updates for 4.10
From: Will Deacon @ 2016-11-29 17:28 UTC (permalink / raw)
To: linux-arm-kernel
Hi Joerg,
Please pull these arm-smmu updates for 4.10. The bulk of the changes here
are the IORT support from Lorenzo, which also touches ACPI and PCI core
code. Rafael and Bjorn have acked the relevant changes, but it makes sense
to send it via you to avoid conflicts with the ARM SMMU drivers, which are
modified to work with ACPI-based probing.
The other changes are largely cosmetic (constification and cleanup), but
the MMU-500 configuration change from Nipun results in a hefty performance
boost in bypass mode.
Thanks,
Will
--->8
The following changes since commit a25f0944ba9b1d8a6813fd6f1a86f1bd59ac25a6:
Linux 4.9-rc5 (2016-11-13 10:32:32 -0800)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git for-joerg/arm-smmu/updates
for you to fetch changes up to 643b8e4d86f8b1a62cf5cd9ea221e9bc0d531d18:
ACPI/IORT: Introduce iort_iommu_configure (2016-11-29 15:57:48 +0000)
----------------------------------------------------------------
Bhumika Goyal (3):
iommu/arm-smmu: Constify iommu_gather_ops structures
iommu/arm-smmu: Constify iommu_gather_ops structures
iommu/io-pgtable-arm: Use const and __initconst for iommu_gather_ops structures
Kefeng Wang (1):
iommu/io-pgtable-arm: Use for_each_set_bit to simplify the code
Lorenzo Pieralisi (16):
ACPI: Add FWNODE_ACPI_STATIC fwnode type
ACPI/IORT: Introduce linker section for IORT entries probing
ACPI/IORT: Add support for IOMMU fwnode registration
iommu: Make of_iommu_set/get_ops() DT agnostic
iommu/arm-smmu: Convert struct device of_node to fwnode usage
iommu/arm-smmu-v3: Convert struct device of_node to fwnode usage
ACPI: Implement acpi_dma_configure
ACPI/IORT: Add node match function
ACPI/IORT: Add support for ARM SMMU platform devices creation
iommu/arm-smmu-v3: Split probe functions into DT/generic portions
iommu/arm-smmu-v3: Add IORT configuration
iommu/arm-smmu: Split probe functions into DT/generic portions
iommu/arm-smmu: Add IORT configuration
ACPI/IORT: Replace rid map type with type mask
ACPI/IORT: Add single mapping function
ACPI/IORT: Introduce iort_iommu_configure
Nipun Gupta (1):
iommu/arm-smmu: Set SMTNMB_TLBEN in ACR to enable caching of bypass entries
drivers/acpi/arm64/iort.c | 585 ++++++++++++++++++++++++++++++++++++-
drivers/acpi/glue.c | 4 +-
drivers/acpi/scan.c | 45 +++
drivers/iommu/arm-smmu-v3.c | 104 +++++--
drivers/iommu/arm-smmu.c | 175 ++++++++---
drivers/iommu/io-pgtable-arm-v7s.c | 5 +-
drivers/iommu/io-pgtable-arm.c | 7 +-
drivers/iommu/iommu.c | 40 +++
drivers/iommu/of_iommu.c | 39 ---
drivers/pci/probe.c | 3 +-
include/acpi/acpi_bus.h | 2 +
include/asm-generic/vmlinux.lds.h | 1 +
include/linux/acpi.h | 26 ++
include/linux/acpi_iort.h | 14 +
include/linux/fwnode.h | 3 +-
include/linux/iommu.h | 14 +
include/linux/of_iommu.h | 12 +-
17 files changed, 955 insertions(+), 124 deletions(-)
^ permalink raw reply
* [PATCH V5 02/10] ras: acpi/apei: cper: generic error data entry v3 per ACPI 6.1
From: Baicar, Tyler @ 2016-11-29 17:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <86258A5CC0A3704780874CF6004BA8A62DC87D66@lhreml502-mbb>
On 11/29/2016 4:29 AM, Shiju Jose wrote:
>> @@ -451,12 +484,12 @@ void cper_estatus_print(const char *pfx,
>> printk("%s""event severity: %s\n", pfx,
>> cper_severity_str(severity));
>> data_len = estatus->data_length;
>> gdata = (struct acpi_hest_generic_data *)(estatus + 1);
>> +
>> snprintf(newpfx, sizeof(newpfx), "%s%s", pfx, INDENT_SP);
>> - while (data_len >= sizeof(*gdata)) {
>> - gedata_len = gdata->error_data_length;
>> +
>> + while (data_len >= acpi_hest_generic_data_size(gdata)) {
>> cper_estatus_print_section(newpfx, gdata, sec_no);
>> - data_len -= gedata_len + sizeof(*gdata);
>> - gdata = (void *)(gdata + 1) + gedata_len;
>> + gdata = acpi_hest_generic_data_next(gdata);
>> sec_no++;
>> }
>> }
> Hi Tyler,
> Will the above while loop does not come out because data_len is not getting updated as it did in V4 patch?
> This is the behaviour seen when we tested on our platform. It worked fine when we update the data_len.
Hello Shiju,
Thank you for testing, and you're right...looks like I got a little too excited at this code simplification. :)
I'll add the data_len update in the next patchset.
Thanks,
Tyler
^ permalink raw reply
* [PATCH 2/2] net: dsa: mv88e6xxx: Add 88E6176 device tree support
From: Vivien Didelot @ 2016-11-29 17:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <2c59cc79-b6dc-9920-1725-a7785ff3b6bf@kleine-koenig.org>
Hi,
Uwe Kleine-K?nig <uwe@kleine-koenig.org> writes:
> Also it seems wrong to write "marvell,mv88e6085" (only) if I know the
> hardware is really a "marvell,mv88e6176".
I agree. It might be complex for a user to dig into the driver in order
to figure out how the switch ID is read and which compatible to choose.
I've sent a patch to change this https://lkml.org/lkml/2016/6/8/1198 but
Andrew had a stronger opinion on compatible strings, which makes sense.
>> Linus has said he does not like ARM devices because of all the busses
>> which are not enumerable. Here we have a device which with a little
>> bit of help we can enumerate. So we should.
>
> If you write
>
> compatible = "marvell,mv88e6176", "marvell,mv88e6085";
>
> you can still enumerate in the same way as before.
So we don't break the existing DTS files, I like this.
The driver already prints info about the detected switch. Instead of
failing at probe, which seems against the notion of compatible and
breaks the existing behavior, it could report the eventual mismatch?
We have examples for both usage, still I don't know what the best
practices are. My _preference_ would go with enumerating them all.
Thanks,
Vivien
^ permalink raw reply
* [PATCH 1/2] ARM: mm: fix set_memory_*() bounds checks
From: Russell King - ARM Linux @ 2016-11-29 17:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <24932a5a-4cbf-dd84-2e87-de29b07da5b3@ti.com>
On Tue, Nov 29, 2016 at 09:25:19AM -0600, Dave Gerlach wrote:
> Hi,
> On 11/21/2016 10:08 AM, Russell King wrote:
> >+static bool in_range(unsigned long start, unsigned long size,
> >+ unsigned long range_start, unsigned long range_end)
> >+{
> >+ return start >= range_start && start < range_end &&
> >+ size <= range_end - start;
> >+}
> >+
> > static int change_memory_common(unsigned long addr, int numpages,
> > pgprot_t set_mask, pgprot_t clear_mask)
> > {
> >- unsigned long start = addr;
> >- unsigned long size = PAGE_SIZE*numpages;
> >- unsigned long end = start + size;
> >+ unsigned long start = addr & PAGE_SIZE;
>
> This doesn't work as is, I believe 'start' should be set to
> PAGE_ALIGN(addr), addr & PAGE_SIZE as it is doesn't make sense. If I make
> this change this code works ok.
You're right, but we want to round 'addr' _down_, not up as PAGE_ALIGN()
will do. So that should've been PAGE_MASK.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply
* [LINUX RFC v4 3/4] mtd: spi-nor: add stripe support
From: Cyrille Pitchen @ 2016-11-29 18:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480235616-34038-1-git-send-email-nagasure@xilinx.com>
Hi Naga,
I have not finished to review the whole series yet but here some first
comments:
Le 27/11/2016 ? 09:33, Naga Sureshkumar Relli a ?crit :
> This patch adds stripe support and it is needed for GQSPI parallel
> configuration mode by:
>
> - Adding required parameters like stripe and shift to spi_nor
> structure.
> - Initializing all added parameters in spi_nor_scan()
> - Updating read_sr() and read_fsr() for getting status from both
> flashes
> - Increasing page_size, sector_size, erase_size and toatal flash
> size as and when required.
> - Dividing address by 2
> - Updating spi->master->flags for qspi driver to change CS
>
> Signed-off-by: Naga Sureshkumar Relli <nagasure@xilinx.com>
> ---
> Changes for v4:
> - rename isparallel to stripe
> Changes for v3:
> - No change
> Changes for v2:
> - Splitted to separate MTD layer changes from SPI core changes
> ---
> drivers/mtd/spi-nor/spi-nor.c | 130 ++++++++++++++++++++++++++++++++----------
> include/linux/mtd/spi-nor.h | 2 +
> 2 files changed, 103 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index d0fc165..4252239 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -22,6 +22,7 @@
> #include <linux/of_platform.h>
> #include <linux/spi/flash.h>
> #include <linux/mtd/spi-nor.h>
> +#include <linux/spi/spi.h>
>
> /* Define max times to check status register before we give up. */
>
> @@ -89,15 +90,24 @@ static const struct flash_info *spi_nor_match_id(const char *name);
> static int read_sr(struct spi_nor *nor)
> {
> int ret;
> - u8 val;
> + u8 val[2];
>
> - ret = nor->read_reg(nor, SPINOR_OP_RDSR, &val, 1);
> - if (ret < 0) {
> - pr_err("error %d reading SR\n", (int) ret);
> - return ret;
> + if (nor->stripe) {
> + ret = nor->read_reg(nor, SPINOR_OP_RDSR, &val[0], 2);
> + if (ret < 0) {
> + pr_err("error %d reading SR\n", (int) ret);
> + return ret;
> + }
> + val[0] |= val[1];
Why '|' rather than '&' ?
I guess because of the 'Write In Progress/Busy' bit: when called by
spi_nor_sr_ready(), you want to be sure that this 'BUSY' bit is cleared on
both memories.
But what about when the Status Register is read for purpose other than
checking the state of the 'BUSY' bit?
What about SPI controllers supporting more than 2 memories in parallel?
This solution might fit the ZynqMP controller but doesn't look so generic.
> + } else {
> + ret = nor->read_reg(nor, SPINOR_OP_RDSR, &val[0], 1);
> + if (ret < 0) {
> + pr_err("error %d reading SR\n", (int) ret);
> + return ret;
> + }
> }
>
> - return val;
> + return val[0];
> }
>
> /*
> @@ -108,15 +118,24 @@ static int read_sr(struct spi_nor *nor)
> static int read_fsr(struct spi_nor *nor)
> {
> int ret;
> - u8 val;
> + u8 val[2];
>
> - ret = nor->read_reg(nor, SPINOR_OP_RDFSR, &val, 1);
> - if (ret < 0) {
> - pr_err("error %d reading FSR\n", ret);
> - return ret;
> + if (nor->stripe) {
> + ret = nor->read_reg(nor, SPINOR_OP_RDFSR, &val[0], 2);
> + if (ret < 0) {
> + pr_err("error %d reading FSR\n", ret);
> + return ret;
> + }
> + val[0] &= val[1];
Same comment here: why '&' rather than '|'?
Surely because of the the 'READY' bit which should be set for both memories.
> + } else {
> + ret = nor->read_reg(nor, SPINOR_OP_RDFSR, &val[0], 1);
> + if (ret < 0) {
> + pr_err("error %d reading FSR\n", ret);
> + return ret;
> + }
> }
>
> - return val;
> + return val[0];
> }
>
> /*
> @@ -290,9 +309,16 @@ static int spi_nor_wait_till_ready(struct spi_nor *nor)
> */
> static int erase_chip(struct spi_nor *nor)
> {
> + u32 ret;
> +
> dev_dbg(nor->dev, " %lldKiB\n", (long long)(nor->mtd.size >> 10));
>
> - return nor->write_reg(nor, SPINOR_OP_CHIP_ERASE, NULL, 0);
> + ret = nor->write_reg(nor, SPINOR_OP_CHIP_ERASE, NULL, 0);
> + if (ret)
> + return ret;
> +
> + return ret;
> +
if (ret)
return ret;
else
return ret;
This chunk should be removed, it doesn't ease the patch review ;)
> }
>
> static int spi_nor_lock_and_prep(struct spi_nor *nor, enum spi_nor_ops ops)
> @@ -349,7 +375,7 @@ static int spi_nor_erase_sector(struct spi_nor *nor, u32 addr)
> static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr)
> {
> struct spi_nor *nor = mtd_to_spi_nor(mtd);
> - u32 addr, len;
> + u32 addr, len, offset;
> uint32_t rem;
> int ret;
>
> @@ -399,9 +425,13 @@ static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr)
> /* "sector"-at-a-time erase */
> } else {
> while (len) {
> +
> write_enable(nor);
> + offset = addr;
> + if (nor->stripe)
> + offset /= 2;
I guess this should be /= 4 for controllers supporting 4 memories in parallel.
Shouldn't you use nor->shift and define shift as an unsigned int instead of a
bool?
offset >>= nor->shift;
Anyway, by tuning the address here in spi-nor.c rather than in the SPI
controller driver, you impose a model to support parallel memories that might
not be suited to other controllers.
>
> - ret = spi_nor_erase_sector(nor, addr);
> + ret = spi_nor_erase_sector(nor, offset);
> if (ret)
> goto erase_err;
>
> @@ -525,6 +555,8 @@ static int stm_lock(struct spi_nor *nor, loff_t ofs, uint64_t len)
> bool use_top;
> int ret;
>
> + ofs = ofs >> nor->shift;
> +
> status_old = read_sr(nor);
> if (status_old < 0)
> return status_old;
> @@ -610,6 +642,8 @@ static int stm_unlock(struct spi_nor *nor, loff_t ofs, uint64_t len)
> bool use_top;
> int ret;
>
> + ofs = ofs >> nor->shift;
> +
> status_old = read_sr(nor);
> if (status_old < 0)
> return status_old;
> @@ -709,6 +743,8 @@ static int spi_nor_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
> if (ret)
> return ret;
>
> + ofs = ofs >> nor->shift;
> +
> ret = nor->flash_lock(nor, ofs, len);
>
> spi_nor_unlock_and_unprep(nor, SPI_NOR_OPS_UNLOCK);
> @@ -724,6 +760,8 @@ static int spi_nor_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
> if (ret)
> return ret;
>
> + ofs = ofs >> nor->shift;
> +
> ret = nor->flash_unlock(nor, ofs, len);
>
> spi_nor_unlock_and_unprep(nor, SPI_NOR_OPS_LOCK);
> @@ -1018,6 +1056,9 @@ static const struct flash_info *spi_nor_read_id(struct spi_nor *nor)
> u8 id[SPI_NOR_MAX_ID_LEN];
> const struct flash_info *info;
>
> + nor->spi->master->flags &= ~(SPI_MASTER_BOTH_CS |
> + SPI_MASTER_DATA_STRIPE);
> +
> tmp = nor->read_reg(nor, SPINOR_OP_RDID, id, SPI_NOR_MAX_ID_LEN);
> if (tmp < 0) {
> dev_dbg(nor->dev, "error %d reading JEDEC ID\n", tmp);
> @@ -1041,6 +1082,7 @@ static int spi_nor_read(struct mtd_info *mtd, loff_t from, size_t len,
> {
> struct spi_nor *nor = mtd_to_spi_nor(mtd);
> int ret;
> + u32 offset = from;
>
> dev_dbg(nor->dev, "from 0x%08x, len %zd\n", (u32)from, len);
>
> @@ -1049,7 +1091,13 @@ static int spi_nor_read(struct mtd_info *mtd, loff_t from, size_t len,
> return ret;
>
> while (len) {
> - ret = nor->read(nor, from, len, buf);
> +
> + offset = from;
> +
> + if (nor->stripe)
> + offset /= 2;
> +
> + ret = nor->read(nor, offset, len, buf);
> if (ret == 0) {
> /* We shouldn't see 0-length reads */
> ret = -EIO;
> @@ -1161,6 +1209,7 @@ static int spi_nor_write(struct mtd_info *mtd, loff_t to, size_t len,
> struct spi_nor *nor = mtd_to_spi_nor(mtd);
> size_t page_offset, page_remain, i;
> ssize_t ret;
> + u32 offset;
>
> dev_dbg(nor->dev, "to 0x%08x, len %zd\n", (u32)to, len);
>
> @@ -1178,9 +1227,13 @@ static int spi_nor_write(struct mtd_info *mtd, loff_t to, size_t len,
> /* the size of data remaining on the first page */
> page_remain = min_t(size_t,
> nor->page_size - page_offset, len - i);
> + offset = (to + i);
> +
> + if (nor->stripe)
> + offset /= 2;
>
> write_enable(nor);
> - ret = nor->write(nor, to + i, page_remain, buf + i);
> + ret = nor->write(nor, (offset), page_remain, buf + i);
> if (ret < 0)
> goto write_err;
> written = ret;
> @@ -1302,22 +1355,22 @@ static int spi_nor_check(struct spi_nor *nor)
>
> int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
> {
> - const struct flash_info *info = NULL;
> + struct flash_info *info = NULL;
You should not remove the const and should not try to modify members of *info.
> struct device *dev = nor->dev;
> struct mtd_info *mtd = &nor->mtd;
> struct device_node *np = spi_nor_get_flash_node(nor);
> - int ret;
> - int i;
> + struct device_node *np_spi;
> + int ret, i, xlnx_qspi_mode;
>
> ret = spi_nor_check(nor);
> if (ret)
> return ret;
>
> if (name)
> - info = spi_nor_match_id(name);
> + info = (struct flash_info *)spi_nor_match_id(name);
> /* Try to auto-detect if chip name wasn't specified or not found */
> if (!info)
> - info = spi_nor_read_id(nor);
> + info = (struct flash_info *)spi_nor_read_id(nor);
> if (IS_ERR_OR_NULL(info))
> return -ENOENT;
>
Both spi_nor_match_id() and spi_nor_read_id(), when they succeed, return a
pointer to an entry of the spi_nor_ids[] array, which is located in a
read-only memory area.
Since your patch doesn't remove the const attribute of the spi_nor_ids[],
I wonder whether it has been tested. I expect it not to work on most
architecture.
Anyway spi_nor_ids[] should remain const. Let's take the case of eXecution
In Place (XIP) from an external memory: if spi_nor_ids[] is const, it can be
read directly from this external (read-only) memory and we never need to copy
the array in RAM, so we save some KB of RAM.
This is just an example but I guess we can find other reasons to keep this
array const.
> @@ -1341,7 +1394,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
> */
> dev_warn(dev, "found %s, expected %s\n",
> jinfo->name, info->name);
> - info = jinfo;
> + info = (struct flash_info *)jinfo;
> }
> }
>
> @@ -1370,6 +1423,27 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
> mtd->size = info->sector_size * info->n_sectors;
> mtd->_erase = spi_nor_erase;
> mtd->_read = spi_nor_read;
> +#ifdef CONFIG_OF
> + np_spi = of_get_next_parent(np);
> +
> + if (of_property_read_u32(np_spi, "xlnx,qspi-mode",
> + &xlnx_qspi_mode) < 0) {
This really looks controller specific so should not be placed in the
generic spi-nor.c file.
> + nor->shift = 0;
> + nor->stripe = 0;
> + } else if (xlnx_qspi_mode == 2) {
> + nor->shift = 1;
> + info->sector_size <<= nor->shift;
> + info->page_size <<= nor->shift;
Just don't modify *info members! :)
> + mtd->size <<= nor->shift;
> + nor->stripe = 1;
> + nor->spi->master->flags |= (SPI_MASTER_BOTH_CS |
> + SPI_MASTER_DATA_STRIPE);
> + }
> +#else
> + /* Default to single */
> + nor->shift = 0;
> + nor->stripe = 0;
> +#endif
>
> /* NOR protection support for STmicro/Micron chips and similar */
> if (JEDEC_MFR(info) == SNOR_MFR_MICRON ||
> @@ -1400,10 +1474,10 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
> /* prefer "small sector" erase if possible */
> if (info->flags & SECT_4K) {
> nor->erase_opcode = SPINOR_OP_BE_4K;
> - mtd->erasesize = 4096;
> + mtd->erasesize = 4096 << nor->shift;
> } else if (info->flags & SECT_4K_PMC) {
> nor->erase_opcode = SPINOR_OP_BE_4K_PMC;
> - mtd->erasesize = 4096;
> + mtd->erasesize = 4096 << nor->shift;
> } else
> #endif
> {
> @@ -1508,16 +1582,14 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
> dev_info(dev, "%s (%lld Kbytes)\n", info->name,
> (long long)mtd->size >> 10);
>
> - dev_dbg(dev,
> - "mtd .name = %s, .size = 0x%llx (%lldMiB), "
> + dev_dbg(dev, "mtd .name = %s, .size = 0x%llx (%lldMiB), "
> ".erasesize = 0x%.8x (%uKiB) .numeraseregions = %d\n",
> mtd->name, (long long)mtd->size, (long long)(mtd->size >> 20),
> mtd->erasesize, mtd->erasesize / 1024, mtd->numeraseregions);
>
> if (mtd->numeraseregions)
> for (i = 0; i < mtd->numeraseregions; i++)
> - dev_dbg(dev,
> - "mtd.eraseregions[%d] = { .offset = 0x%llx, "
> + dev_dbg(dev, "mtd.eraseregions[%d] = { .offset = 0x%llx, "
> ".erasesize = 0x%.8x (%uKiB), "
> ".numblocks = %d }\n",
> i, (long long)mtd->eraseregions[i].offset,
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index 84f3ce5..673ec68 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -165,6 +165,8 @@ struct spi_nor {
> u8 read_dummy;
> u8 program_opcode;
> enum read_mode flash_read;
> + bool shift;
> + bool stripe;
> bool sst_write_second;
> u32 flags;
> u8 cmd_buf[SPI_NOR_MAX_CMD_SIZE];
>
Best regards,
Cyrille
^ permalink raw reply
* [PATCH] PCI: Add information about describing PCI in ACPI
From: Bjorn Helgaas @ 2016-11-29 18:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161123150632.GA16033@bhelgaas-glaptop.roam.corp.google.com>
On Wed, Nov 23, 2016 at 09:06:33AM -0600, Bjorn Helgaas wrote:
> On Wed, Nov 23, 2016 at 07:28:12AM +0000, Ard Biesheuvel wrote:
> > On 23 November 2016 at 01:06, Bjorn Helgaas <helgaas@kernel.org> wrote:
> > > On Tue, Nov 22, 2016 at 10:09:50AM +0000, Ard Biesheuvel wrote:
> > >> On 17 November 2016 at 17:59, Bjorn Helgaas <bhelgaas@google.com> wrote:
> > >
> > >> > +PCI host bridges are PNP0A03 or PNP0A08 devices. Their _CRS should
> > >> > +describe all the address space they consume. In principle, this would
> > >> > +be all the windows they forward down to the PCI bus, as well as the
> > >> > +bridge registers themselves. The bridge registers include things like
> > >> > +secondary/subordinate bus registers that determine the bus range below
> > >> > +the bridge, window registers that describe the apertures, etc. These
> > >> > +are all device-specific, non-architected things, so the only way a
> > >> > +PNP0A03/PNP0A08 driver can manage them is via _PRS/_CRS/_SRS, which
> > >> > +contain the device-specific details. These bridge registers also
> > >> > +include ECAM space, since it is consumed by the bridge.
> > >> > +
> > >> > +ACPI defined a Producer/Consumer bit that was intended to distinguish
> > >> > +the bridge apertures from the bridge registers [4, 5]. However,
> > >> > +BIOSes didn't use that bit correctly, and the result is that OSes have
> > >> > +to assume that everything in a PCI host bridge _CRS is a window. That
> > >> > +leaves no way to describe the bridge registers in the PNP0A03/PNP0A08
> > >> > +device itself.
> > >>
> > >> Is that universally true? Or is it still possible to do the right
> > >> thing here on new ACPI architectures such as arm64?
> > >
> > > That's a very good question. I had thought that the ACPI spec had
> > > given up on Consumer/Producer completely, but I was wrong. In the 6.0
> > > spec, the Consumer/Producer bit is still documented in the Extended
> > > Address Space Descriptor (sec 6.4.3.5.4). It is documented as
> > > "ignored" in the QWord, DWord, and Word descriptors (sec 6.4.3.5.1,2,3).
> > >
> > > Linux looks at the producer_consumer bit in acpi_decode_space(), which
> > > I think is used for all these descriptors (QWord, DWord, Word, and
> > > Extended). This doesn't quite follow the spec -- we probably should
> > > ignore it except for Extended. In any event, acpi_decode_space() sets
> > > IORESOURCE_WINDOW for Producer descriptors, but we don't test
> > > IORESOURCE_WINDOW in the PCI host bridge code.
> > >
> > > x86 and ia64 supply their own pci_acpi_root_prepare_resources()
> > > functions that call acpi_pci_probe_root_resources(), which parses _CRS
> > > and looks at producer_consumer. Then they do a little arch-specific
> > > stuff on the result.
> > >
> > > On arm64 we use acpi_pci_probe_root_resources() directly, with no
> > > arch-specific stuff.
> > >
> > > On all three arches, we ignore the Consumer/Producer bit, so all the
> > > resources are treated as Producers, e.g., as bridge windows.
> > >
> > > I think we *could* implement an arm64 version of
> > > pci_acpi_root_prepare_resources() that would pay attention to the
> > > Consumer/Producer bit by checking IORESOURCE_WINDOW. To be spec
> > > compliant, we would have to use Extended descriptors for all bridge
> > > windows, even if they would fit in a DWord or QWord.
> > >
> > > Should we do that? I dunno. I'd like to hear your opinion(s).
> > >
> >
> > Yes, I think we should. If the spec allows for a way for a PNP0A03
> > device to describe all of its resources unambiguously, we should not
> > be relying on workarounds that were designed for another architecture
> > in another decade (for, presumably, another OS)
> >
> > Just for my understanding, we will need to use extended descriptors
> > for all consumed *and* produced regions, even though dword/qword are
> > implicitly produced-only, due to the fact that the bit is ignored?
>
> From an ACPI spec point of view, I would say QWord/DWord/Word
> descriptors are implicitly *consumer*-only because ResourceConsumer
> is the default and they don't have a bit to indicate otherwise.
>
> The current code assumes all PNP0A03 resources are producers. If we
> implement an arm64 pci_acpi_root_prepare_resources() that pays
> attention to the Consumer/Producer bit, we would have to:
>
> - Reserve all producer regions in the iomem/ioport trees. This is
> already done via pci_acpi_root_add_resources(), but we might need
> a new check to handle consumers differently.
>
> - Reserve all consumer regions. This corresponds to what
> pnp/system.c does for PNP0C02 devices. This is similar to the
> producer regions, but I think the consumer ones should be marked
> IORESOURCE_BUSY.
>
> - Use every producer (IORESOURCE_WINDOW) as a host bridge window.
>
> I think it's a bug that acpi_decode_space() looks at producer_consumer
> for QWord/DWord/Word descriptors, but I think QWord/DWord/Word
> descriptors for consumed regions should be safe, as long as they don't
> set the Consumer/Producer bit.
I'm going to post a couple very lightly-tested patches that should
make us ignore the Consumer/Producer bit for QWord/DWord/Word. I'd
appreciate any discussion about whether that's the right approach.
Bjorn
^ permalink raw reply
* [PATCH 2/2] net: dsa: mv88e6xxx: Add 88E6176 device tree support
From: Andrew Lunn @ 2016-11-29 18:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <87oa0yb29b.fsf@ketchup.i-did-not-set--mail-host-address--so-tickle-me>
On Tue, Nov 29, 2016 at 12:54:24PM -0500, Vivien Didelot wrote:
> Hi,
>
> Uwe Kleine-K?nig <uwe@kleine-koenig.org> writes:
>
> > Also it seems wrong to write "marvell,mv88e6085" (only) if I know the
> > hardware is really a "marvell,mv88e6176".
>
> I agree. It might be complex for a user to dig into the driver in order
> to figure out how the switch ID is read and which compatible to choose.
>
> I've sent a patch to change this https://lkml.org/lkml/2016/6/8/1198 but
> Andrew had a stronger opinion on compatible strings, which makes sense.
>
> >> Linus has said he does not like ARM devices because of all the busses
> >> which are not enumerable. Here we have a device which with a little
> >> bit of help we can enumerate. So we should.
> >
> > If you write
> >
> > compatible = "marvell,mv88e6176", "marvell,mv88e6085";
> >
> > you can still enumerate in the same way as before.
>
> So we don't break the existing DTS files, I like this.
>
> The driver already prints info about the detected switch. Instead of
> failing at probe, which seems against the notion of compatible and
> breaks the existing behavior, it could report the eventual mismatch?
I'm still against it.
Say we let the driver probe and warn when the compatible string is
wrong. Is this likely to get fixed? Probably not, it is just a
warning, people ignore them.
A few years later, we have accumulated a number of broken device
trees. And suddenly we really do have a Marvell device with a broken
ID in its port register, or more likely, the revision number was not
incremented but there was incompatible register changes. We suddenly
do have to look at the compatible string. But we know many are actually
wrong. How do we know which to trust? We basically have to say, if the
compatible is "marvell,mv88e6042" we trust it, all the others we don't
trust and ignore.
Isn't that madness?
What we have today is verified correct. If this hypothetical
"marvell,mv88e6042" does happen, then no problems, we add a compatible
string for it, and it works.
We should probably add a comment to the mv88e6xxx_of_match array,
explaining how it currently works.
Andrew
^ permalink raw reply
* [PATCH] PCI: Add information about describing PCI in ACPI
From: Bjorn Helgaas @ 2016-11-29 18:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1AE640813FDE7649BE1B193DEA596E886A2949AF@SHSMSX101.ccr.corp.intel.com>
On Wed, Nov 23, 2016 at 03:23:35AM +0000, Zheng, Lv wrote:
> Hi, Bjorn
>
> Thanks for the documentation.
> It really helps!
>
> However I have a question below.
>
> > From: linux-acpi-owner at vger.kernel.org [mailto:linux-acpi-owner at vger.kernel.org] On Behalf Of Bjorn
> > Helgaas
> > Subject: [PATCH] PCI: Add information about describing PCI in ACPI
> >
> > Add a writeup about how PCI host bridges should be described in ACPI
> > using PNP0A03/PNP0A08 devices, PNP0C02 devices, and the MCFG table.
> >
> > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> > ---
> > Documentation/PCI/00-INDEX | 2 +
> > Documentation/PCI/acpi-info.txt | 136 +++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 138 insertions(+)
> > create mode 100644 Documentation/PCI/acpi-info.txt
> >
> > diff --git a/Documentation/PCI/00-INDEX b/Documentation/PCI/00-INDEX
> > index 147231f..0780280 100644
> > --- a/Documentation/PCI/00-INDEX
> > +++ b/Documentation/PCI/00-INDEX
> > @@ -1,5 +1,7 @@
> > 00-INDEX
> > - this file
> > +acpi-info.txt
> > + - info on how PCI host bridges are represented in ACPI
> > MSI-HOWTO.txt
> > - the Message Signaled Interrupts (MSI) Driver Guide HOWTO and FAQ.
> > PCIEBUS-HOWTO.txt
> > diff --git a/Documentation/PCI/acpi-info.txt b/Documentation/PCI/acpi-info.txt
> > new file mode 100644
> > index 0000000..ccbcfda
> > --- /dev/null
> > +++ b/Documentation/PCI/acpi-info.txt
> > @@ -0,0 +1,136 @@
> > + ACPI considerations for PCI host bridges
> > +
> > +The basic requirement is that the ACPI namespace should describe
> > +*everything* that consumes address space unless there's another
> > +standard way for the OS to find it [1, 2]. ?For example, windows that
> > +are forwarded to PCI by a PCI host bridge should be described via ACPI
> > +devices, since the OS can't locate the host bridge by itself. ?PCI
> > +devices *below* the host bridge do not need to be described via ACPI,
> > +because the resources they consume are inside the host bridge windows,
> > +and the OS can discover them via the standard PCI enumeration
> > +mechanism (using config accesses to read and size the BARs).
> > +
> > +This ACPI resource description is done via _CRS methods of devices in
> > +the ACPI namespace [2]. ? _CRS methods are like generalized PCI BARs:
> > +the OS can read _CRS and figure out what resource is being consumed
> > +even if it doesn't have a driver for the device [3]. ?That's important
> > +because it means an old OS can work correctly even on a system with
> > +new devices unknown to the OS. ?The new devices won't do anything, but
> > +the OS can at least make sure no resources conflict with them.
> > +
> > +Static tables like MCFG, HPET, ECDT, etc., are *not* mechanisms for
> > +reserving address space! The static tables are for things the OS
> > +needs to know early in boot, before it can parse the ACPI namespace.
> > +If a new table is defined, an old OS needs to operate correctly even
> > +though it ignores the table. _CRS allows that because it is generic
> > +and understood by the old OS; a static table does not.
>
> The entire document doesn't talk about the details of _CBA.
> There is only one line below mentioned _CBA as an example.
Yes, that's a good point. I'll add some more details about MCFG
and _CBA.
Bjorn
^ permalink raw reply
* Tearing down DMA transfer setup after DMA client has finished
From: Mason @ 2016-11-29 18:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <092f44ee-4560-be17-25f7-00948dba3cfa@free.fr>
[ Nothing new added below.
Vinod, was the description of my HW's quirks clear enough?
Is there a way to write a driver within the existing framework?
How can I get that HW block supported upstream?
Regards. ]
On 25/11/2016 13:46, Mason wrote:
> On 25/11/2016 05:55, Vinod Koul wrote:
>
>> On Wed, Nov 23, 2016 at 11:25:44AM +0100, Mason wrote:
>>
>>> On my platform, setting up a DMA transfer is a two-step process:
>>>
>>> 1) configure the "switch box" to connect a device to a memory channel
>>> 2) configure the transfer details (address, size, command)
>>>
>>> When the transfer is done, the sbox setup can be torn down,
>>> and the DMA driver can start another transfer.
>>>
>>> The current software architecture for my NFC (NAND Flash controller)
>>> driver is as follows (for one DMA transfer).
>>>
>>> sg_init_one
>>> dma_map_sg
>>> dmaengine_prep_slave_sg
>>> dmaengine_submit
>>> dma_async_issue_pending
>>> configure_NFC_transfer
>>> wait_for_IRQ_from_DMA_engine // via DMA_PREP_INTERRUPT
>>> wait_for_NFC_idle
>>> dma_unmap_sg
>>
>> Looking at thread and discussion now, first thinking would be to ensure
>> the transaction is completed properly and then isr fired. You may need
>> to talk to your HW designers to find a way for that. It is quite common
>> that DMA controllers will fire and complete whereas the transaction is
>> still in flight.
>
> It seems there is a disconnect between what Linux expects - an IRQ
> when the transfer is complete - and the quirks of this HW :-(
>
> On this system, there are MBUS "agents" connected via a "switch box".
> An agent fires an IRQ when it has dealt with its *half* of the transfer.
>
> SOURCE_AGENT <---> SBOX <---> DESTINATION_AGENT
>
> Here are the steps for a transfer, in the general case:
>
> 1) setup the sbox to connect SOURCE TO DEST
> 2) configure source to send N bytes
> 3) configure dest to receive N bytes
>
> When SOURCE_AGENT has sent N bytes, it fires an IRQ
> When DEST_AGENT has received N bytes, it fires an IRQ
> The sbox connection can be torn down only when the destination
> agent has received all bytes.
> (And the twist is that some agents do not have an IRQ line.)
>
> The system provides 3 RAM-to-sbox agents (read channels)
> and 3 sbox-to-RAM agents (write channels).
>
> The NAND Flash controller read and write agents do not have
> IRQ lines.
>
> So for a NAND-to-memory transfer (read from device)
> - nothing happens when the NFC has finished sending N bytes to the sbox
> - the write channel fires an IRQ when it has received N bytes
>
> In that case, one IRQ fires when the transfer is complete,
> like Linux expects.
>
> For a memory-to-NAND transfer (write to device)
> - the read channel fires an IRQ when it has sent N bytes
> - the NFC driver is supposed to poll the NFC to determine
> when the controller has finished writing N bytes
>
> In that case, the IRQ does not indicate that the transfer
> is complete, merely that the sending half has finished
> its part.
>
> For a memory-to-memory transfer (memcpy)
> - the read channel fires an IRQ when it has sent N bytes
> - the write channel fires an IRQ when it has received N bytes
>
> So you actually get two IRQs in that case, which I don't
> think Linux (or the current DMA driver) expects.
>
> I'm not sure how we're supposed to handle this kind of HW
> in Linux? (That's why I started this thread.)
>
>
>> If that is not doable, then since you claim this is custom part which
>> other vendors won't use (hope we are wrong down the line),
>
> I'm not sure how to interpret "you claim this is custom part".
> Do you mean I may be wrong, that it is not custom?
> I don't know if other vendors may have HW with the same
> quirky behavior. What do you mean about being wrong down
> the line?
>
>> then we can have a custom api,
>>
>> foo_sbox_configure(bool enable, ...);
>>
>> This can be invoked from NFC driver when required for configuration and
>> teardown. For very specific cases where people need some specific
>> configuration we do allow custom APIs.
>
> I don't think that would work. The fundamental issue is
> that Linux expects a single IRQ to indicate "transfer
> complete". And the driver (as written) starts a new
> transfer as soon as the IRQ fires.
>
> But the HW may generate 0, 1, or even 2 IRQs for a single
> transfer. And when there is a single IRQ, it may not
> indicate "transfer complete" (as seen above).
>
>> Only problem with that would be it wont be a generic solution
>> and you seem to be fine with that.
>
> I think it is possible to have a generic solution:
> Right now, the callback is called from tasklet context.
> If we can have a new flag to have the callback invoked
> directly from the ISR, then the driver for the client
> device can do what is required.
>
> For example, the NFC driver waits for the IRQ from the
> memory agent, and then polls the controller itself.
>
> I can whip up a proof-of-concept if it's better to
> illustrate with a patch?
^ permalink raw reply
* [PATCH V10 2/6] arm: arm64: Add routine to determine cpuid of other cpus
From: Jeremy Linton @ 2016-11-29 18:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161129104617.GV14217@n2100.armlinux.org.uk>
Hi,
On 11/29/2016 04:46 AM, Russell King - ARM Linux wrote:
> On Tue, Nov 29, 2016 at 10:31:12AM +0000, Will Deacon wrote:
>> [adding Russell]
>>
>> On Wed, Nov 09, 2016 at 05:39:49PM -0600, Jeremy Linton wrote:
>>> It is helpful if we can read the cpuid/midr of other CPUs
>>> in the system independent of arm/arm64.
>>>
>>> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
>>> ---
>>> arch/arm/include/asm/cputype.h | 2 ++
>>> arch/arm64/include/asm/cputype.h | 3 +++
>>> 2 files changed, 5 insertions(+)
>>>
>>> diff --git a/arch/arm/include/asm/cputype.h b/arch/arm/include/asm/cputype.h
>>> index 522b5fe..31fb273 100644
>>> --- a/arch/arm/include/asm/cputype.h
>>> +++ b/arch/arm/include/asm/cputype.h
>>> @@ -235,6 +235,8 @@ static inline unsigned int __attribute_const__ read_cpuid_mpidr(void)
>>> #define cpu_is_sa1100() (read_cpuid_part() == ARM_CPU_PART_SA1100)
>>> #define cpu_is_sa1110() (read_cpuid_part() == ARM_CPU_PART_SA1110)
>>>
>>> +#define read_specific_cpuid(cpu_num) per_cpu_ptr(&cpu_data, cpu_num)->cpuid
>>> +
>>> /*
>>> * Intel's XScale3 core supports some v6 features (supersections, L2)
>>> * but advertises itself as v5 as it does not support the v6 ISA. For
>>
>> Russell -- are you ok with adding this macro to arch/arm/? It will get used
>> by the CPU PMU driver, which needs a portable (i.e. between arm and arm64)
>> way to convert a logical CPU ID into the MIDR register for that CPU.
>
> No, because we don't set the cpuid member in uniprocessor configurations,
> so it's going to be a fragile macro - it'll return zero for kernels
> configured without SMP support.
By itself is easy enough to fix, it could be tweaked to do the same
thing as c_show()
#define read_specific_cpuid(cpu_num) is_smp() ? per_cpu(&cpu_data,
cpu_num)->cpuid : read_cpuid_id()
c_show could then be updated to use read_specific_cpuid().
>
> I'd ideally like cpuid for the boot CPU to be set early, so that we can
> get rid of many ifdefs in this area, but haven't convinced myself that
> it's safe to do so in all configurations with the percpu stuff not being
> up and running in setup_arch().
As the above change is a little ugly... It seems something like
smp_prepare_boot_cpu() call would be perfect if it were available for
!SMP. Although this is getting a little off base for the PMU changes.
The alternative to all this, is yet another #ifdef in the pmu code,
which is IMHO the worse choice.
Suggestions?
^ permalink raw reply
* [PATCH v7 2/8] drm/sun8i: Add DT bindings documentation of Allwinner DE2
From: Laurent Pinchart @ 2016-11-29 18:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <92fc53084274d5ab04fe28f2dc0b16cf5d94481e.1480414715.git.moinejf@free.fr>
Hi Jean-Fran?ois,
Thank you for the patch.
On Monday 28 Nov 2016 19:02:39 Jean-Francois Moine wrote:
> Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
> ---
> .../bindings/display/sunxi/sun8i-de2.txt | 121 ++++++++++++++++++
> 1 file changed, 121 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/display/sunxi/sun8i-de2.txt
>
> diff --git a/Documentation/devicetree/bindings/display/sunxi/sun8i-de2.txt
> b/Documentation/devicetree/bindings/display/sunxi/sun8i-de2.txt new file
> mode 100644
> index 0000000..edf38b8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/sunxi/sun8i-de2.txt
> @@ -0,0 +1,121 @@
> +Allwinner sun8i Display Engine 2 subsystem
> +==========================================
> +
> +The Allwinner DE2 subsystem contains a display controller (DE2),
> +one or two LCD controllers (Timing CONtrollers) and their external
> +interfaces (encoders/connectors).
> +
> + +-----------+
> + | DE2 |
> + | |
> + | +-------+ |
> + plane --->| | | +------+
> + | | mixer |---->| TCON |---> encoder ---> display
> + plane --->| | | +------+ connector device
> + | +-------+ |
> + | |
> + | +-------+ |
> + plane --->| | | +------+
> + | | mixer |---->| TCON |---> encoder ---> display
> + plane --->| | | +------+ connector device
> + | +-------+ |
> + +-----------+
> +
> +The DE2 contains a processor which mixes the input planes and creates
> +the images which are sent to the TCONs.
> +From the software point of vue, there are 2 independent real-time
> +mixers, each one being statically associated to one TCON.
> +
> +The TCONs adjust the image format to the one of the display device.
> +
> +Display controller (DE2)
> +========================
> +
> +Required properties:
> +
> +- compatible: value should be one of the following
> + "allwinner,sun8i-a83t-display-engine"
> + "allwinner,sun8i-h3-display-engine"
> +
> +- reg: base address and size of the I/O memory
> +
> +- clocks: must include clock specifiers corresponding to entries in the
> + clock-names property.
> +
> +- clock-names: must contain
> + "bus": bus gate
> + "clock": clock
> +
> +- resets: phandle to the reset of the device
> +
> +- ports: must contain a list of 2 phandles, indexed by mixer number,
> + and pointing to display interface ports of TCONs
> +
> +LCD controller (TCON)
> +=====================
> +
> +Required properties:
> +
> +- compatible: should be
> + "allwinner,sun8i-a83t-tcon"
> +
> +- reg: base address and size of the I/O memory
> +
> +- clocks: must include clock specifiers corresponding to entries in the
> + clock-names property.
> +
> +- clock-names: must contain
> + "bus": bus gate
> + "clock": pixel clock
> +
> +- resets: phandle to the reset of the device
> +
> +- interrupts: interrupt number for the TCON
> +
> +- port: port node with endpoint definitions as defined in
> + Documentation/devicetree/bindings/media/video-interfaces.txt
> +
> +Example:
> +
> + de: de-controller at 01000000 {
> + compatible = "allwinner,sun8i-h3-display-engine";
> + reg = <0x01000000 0x400000>;
> + clocks = <&ccu CLK_BUS_DE>, <&ccu CLK_DE>;
> + clock-names = "bus", "clock";
> + resets = <&ccu RST_BUS_DE>;
> + ports = <&tcon0_p>, <&tcon1_p>;
This isn't how the OF graph DT bindings are used. You should instead have
ports {
#address-cells = <1>;
#size-cells = <0>;
port at 0 {
de_out_0: endpoint {
remote_endpoint = <&tcon0_hdmi>;
};
};
port at 1 {
/* No endpoint as the port is not connected */
};
};
> + };
> +
> + tcon0: lcd-controller at 01c0c000 {
> + compatible = "allwinner,sun8i-a83t-tcon";
> + reg = <0x01c0c000 0x400>;
> + clocks = <&ccu CLK_BUS_TCON0>, <&ccu CLK_TCON0>;
> + clock-names = "bus", "clock";
> + resets = <&ccu RST_BUS_TCON0>;
> + interrupts = <GIC_SPI 86 IRQ_TYPE_LEVEL_HIGH>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + tcon0_p: port {
> + tcon0_hdmi: endpoint {
> + remote-endpoint = <&hdmi_tcon0>;
> + };
> + };
and here
port {
tcon0_hdmi: endpoint {
remote-endpoint = <&de_out_0>;
};
};
> + };
> +
> + /* not used */
> + tcon1: lcd-controller at 01c0d000 {
> + compatible = "allwinner,sun8i-h3-tcon";
> + reg = <0x01c0d000 0x400>;
> + clocks = <&ccu CLK_BUS_TCON1>,
> + <&ccu CLK_TCON0>; /* no clock */
> + clock-names = "bus", "clock";
> + interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>;
> + status = "disabled";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + tcon1_p: port {
> + endpoint {
> + /* empty */
> + };
> + };
and here
port {
/* No endpoint as the port is not connected */
};
(although I'm not sure why you don't connect it)
> + };
--
Regards,
Laurent Pinchart
^ permalink raw reply
* [PATCH 0/2] ACPI: Ignore Consumer/Producer for QWord/DWord/Word Address Space
From: Bjorn Helgaas @ 2016-11-29 18:43 UTC (permalink / raw)
To: linux-arm-kernel
Per spec, the Consumer/Producer bit is defined only for Extended
Address Space descriptors and should be ignored for QWord/DWord/Word
Address Space descriptors. My understanding is that this is because
x86 BIOSes didn't use the bit consistently, so it couldn't be relied
upon. The Extended descriptors were added later and are presumably
more reliable.
Linux currently looks at Consumer/Producer for all Address Space
descriptors, although we don't use the result very much. The x86 and
ia64 host bridge driver code doesn't use Consumer/Producer to identify
windows; it assumes all descriptors are windows.
We do currently use Consumer/Producer to decide whether to apply _TRA.
The change in these patches is to ignore Consumer/Producer for
QWord/DWord/Word, so we'll apply _TRA from Consumer descriptors where
we didn't before. Per spec, that should be safe because _TRA is
required to be zero for Consumer resources.
If we want to describe host bridge register space and ECAM space
directly in the PNP0A03 device on ARM64 (and I guess potentially even
on x86 & ia64), I think we need changes like this so the arch code can
use IORESOURCE_WINDOW to tell which descriptors are windows and which
are registers.
This is a subtle area, so please take a look and let me know what you
think.
These patches are based on v4.9-rc1. It's getting pretty late in the
cycle, but I think we'd really like to get the ARM64 ACPI PCI story
sorted out for the v4.10 merge window.
---
Bjorn Helgaas (2):
ACPI: Combine acpi_dev_resource_address_space() and acpi_dev_resource_ext_address_space()
ACPI: Ignore Consumer/Producer for QWord/DWord/Word Address Space
drivers/acpi/ioapic.c | 3 -
drivers/acpi/resource.c | 130 ++++++++++++++++++----------------------
drivers/pnp/pnpacpi/rsparser.c | 3 -
include/linux/acpi.h | 2 -
4 files changed, 59 insertions(+), 79 deletions(-)
^ permalink raw reply
* [PATCH 1/2] ACPI: Combine acpi_dev_resource_address_space() and acpi_dev_resource_ext_address_space()
From: Bjorn Helgaas @ 2016-11-29 18:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161129182415.7618.99129.stgit@bhelgaas-glaptop.roam.corp.google.com>
Previously acpi_dev_resource_address_space() and
acpi_dev_resource_ext_address_space() were wrappers that called
acpi_decode_space(). We need to distinguish between Word/DWord/QWord
address descriptors and Extended address descriptors, which was impossible
in acpi_decode_space().
Fold the acpi_dev_resource_address_space() and
acpi_dev_resource_ext_address_space() functionality into
acpi_decode_space() and call the result acpi_dev_resource_address_space().
No functional change intended.
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
drivers/acpi/ioapic.c | 3 -
drivers/acpi/resource.c | 114 ++++++++++++++++------------------------
drivers/pnp/pnpacpi/rsparser.c | 3 -
include/linux/acpi.h | 2 -
4 files changed, 48 insertions(+), 74 deletions(-)
diff --git a/drivers/acpi/ioapic.c b/drivers/acpi/ioapic.c
index 6d7ce6e..6d91417 100644
--- a/drivers/acpi/ioapic.c
+++ b/drivers/acpi/ioapic.c
@@ -50,8 +50,7 @@ static acpi_status setup_res(struct acpi_resource *acpi_res, void *data)
return AE_OK;
if (!acpi_dev_resource_memory(acpi_res, res)) {
- if (acpi_dev_resource_address_space(acpi_res, &win) ||
- acpi_dev_resource_ext_address_space(acpi_res, &win))
+ if (acpi_dev_resource_address_space(acpi_res, &win))
*res = win.res;
}
if ((res->flags & IORESOURCE_PREFETCH) ||
diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
index 56241eb..2732d39e 100644
--- a/drivers/acpi/resource.c
+++ b/drivers/acpi/resource.c
@@ -197,16 +197,54 @@ bool acpi_dev_resource_io(struct acpi_resource *ares, struct resource *res)
}
EXPORT_SYMBOL_GPL(acpi_dev_resource_io);
-static bool acpi_decode_space(struct resource_win *win,
- struct acpi_resource_address *addr,
- struct acpi_address64_attribute *attr)
+/**
+ * acpi_dev_resource_address_space - Extract ACPI address space information.
+ * @ares: Input ACPI resource object.
+ * @win: Output generic resource object.
+ *
+ * Check if the given ACPI resource object represents an address space resource
+ * and if that's the case, use the information in it to populate the generic
+ * resource object pointed to by @win.
+ *
+ * Return:
+ * 1) false with win->res.flags setting to zero: not the expected resource type
+ * 2) false with IORESOURCE_DISABLED in win->res.flags: valid unassigned
+ * resource
+ * 3) true: valid assigned resource
+ XXX check these return values
+ */
+bool acpi_dev_resource_address_space(struct acpi_resource *ares,
+ struct resource_win *win)
{
- u8 iodec = attr->granularity == 0xfff ? ACPI_DECODE_10 : ACPI_DECODE_16;
- bool wp = addr->info.mem.write_protect;
- u64 len = attr->address_length;
- u64 start, end, offset = 0;
+ struct acpi_resource_address *addr;
+ struct acpi_address64_attribute *attr;
+ struct acpi_resource_extended_address64 *ext_addr;
+ struct acpi_resource_address64 addr64;
+ acpi_status status;
+ u8 iodec;
+ bool wp;
+ u64 len, start, end, offset = 0;
struct resource *res = &win->res;
+ win->res.flags = 0;
+
+ if (ares->type == ACPI_RESOURCE_TYPE_EXTENDED_ADDRESS64) {
+ ext_addr = &ares->data.ext_address64;
+ addr = (struct acpi_resource_address *)ext_addr;
+ attr = &ext_addr->address;
+ } else {
+ status = acpi_resource_to_address64(ares, &addr64);
+ if (ACPI_FAILURE(status))
+ return false;
+
+ addr = (struct acpi_resource_address *)&addr64;
+ attr = &addr64.address;
+ }
+
+ iodec = attr->granularity == 0xfff ? ACPI_DECODE_10 : ACPI_DECODE_16;
+ wp = addr->info.mem.write_protect;
+ len = attr->address_length;
+
/*
* Filter out invalid descriptor according to ACPI Spec 5.0, section
* 6.4.3.5 Address Space Resource Descriptors.
@@ -264,68 +302,9 @@ static bool acpi_decode_space(struct resource_win *win,
return !(res->flags & IORESOURCE_DISABLED);
}
-
-/**
- * acpi_dev_resource_address_space - Extract ACPI address space information.
- * @ares: Input ACPI resource object.
- * @win: Output generic resource object.
- *
- * Check if the given ACPI resource object represents an address space resource
- * and if that's the case, use the information in it to populate the generic
- * resource object pointed to by @win.
- *
- * Return:
- * 1) false with win->res.flags setting to zero: not the expected resource type
- * 2) false with IORESOURCE_DISABLED in win->res.flags: valid unassigned
- * resource
- * 3) true: valid assigned resource
- */
-bool acpi_dev_resource_address_space(struct acpi_resource *ares,
- struct resource_win *win)
-{
- struct acpi_resource_address64 addr;
-
- win->res.flags = 0;
- if (ACPI_FAILURE(acpi_resource_to_address64(ares, &addr)))
- return false;
-
- return acpi_decode_space(win, (struct acpi_resource_address *)&addr,
- &addr.address);
-}
EXPORT_SYMBOL_GPL(acpi_dev_resource_address_space);
/**
- * acpi_dev_resource_ext_address_space - Extract ACPI address space information.
- * @ares: Input ACPI resource object.
- * @win: Output generic resource object.
- *
- * Check if the given ACPI resource object represents an extended address space
- * resource and if that's the case, use the information in it to populate the
- * generic resource object pointed to by @win.
- *
- * Return:
- * 1) false with win->res.flags setting to zero: not the expected resource type
- * 2) false with IORESOURCE_DISABLED in win->res.flags: valid unassigned
- * resource
- * 3) true: valid assigned resource
- */
-bool acpi_dev_resource_ext_address_space(struct acpi_resource *ares,
- struct resource_win *win)
-{
- struct acpi_resource_extended_address64 *ext_addr;
-
- win->res.flags = 0;
- if (ares->type != ACPI_RESOURCE_TYPE_EXTENDED_ADDRESS64)
- return false;
-
- ext_addr = &ares->data.ext_address64;
-
- return acpi_decode_space(win, (struct acpi_resource_address *)ext_addr,
- &ext_addr->address);
-}
-EXPORT_SYMBOL_GPL(acpi_dev_resource_ext_address_space);
-
-/**
* acpi_dev_irq_flags - Determine IRQ resource flags.
* @triggering: Triggering type as provided by ACPI.
* @polarity: Interrupt polarity as provided by ACPI.
@@ -542,8 +521,7 @@ static acpi_status acpi_dev_process_resource(struct acpi_resource *ares,
if (acpi_dev_resource_memory(ares, res)
|| acpi_dev_resource_io(ares, res)
- || acpi_dev_resource_address_space(ares, &win)
- || acpi_dev_resource_ext_address_space(ares, &win))
+ || acpi_dev_resource_address_space(ares, &win))
return acpi_dev_new_resource_entry(&win, c);
for (i = 0; acpi_dev_resource_interrupt(ares, i, res); i++) {
diff --git a/drivers/pnp/pnpacpi/rsparser.c b/drivers/pnp/pnpacpi/rsparser.c
index 4b717c6..bc61651 100644
--- a/drivers/pnp/pnpacpi/rsparser.c
+++ b/drivers/pnp/pnpacpi/rsparser.c
@@ -184,8 +184,7 @@ static acpi_status pnpacpi_allocated_resource(struct acpi_resource *res,
struct resource *r = &win.res;
int i, flags;
- if (acpi_dev_resource_address_space(res, &win)
- || acpi_dev_resource_ext_address_space(res, &win)) {
+ if (acpi_dev_resource_address_space(res, &win)) {
pnp_add_resource(dev, &win.res);
return AE_OK;
}
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index ddbeda6..a06a877 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -399,8 +399,6 @@ bool acpi_dev_resource_memory(struct acpi_resource *ares, struct resource *res);
bool acpi_dev_resource_io(struct acpi_resource *ares, struct resource *res);
bool acpi_dev_resource_address_space(struct acpi_resource *ares,
struct resource_win *win);
-bool acpi_dev_resource_ext_address_space(struct acpi_resource *ares,
- struct resource_win *win);
unsigned long acpi_dev_irq_flags(u8 triggering, u8 polarity, u8 shareable);
unsigned int acpi_dev_get_irq_type(int triggering, int polarity);
bool acpi_dev_resource_interrupt(struct acpi_resource *ares, int index,
^ permalink raw reply related
* [PATCH 2/2] ACPI: Ignore Consumer/Producer for QWord/DWord/Word Address Space
From: Bjorn Helgaas @ 2016-11-29 18:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161129182415.7618.99129.stgit@bhelgaas-glaptop.roam.corp.google.com>
Per ACPI spec r6.0, sec 6.4.3.5.1, 2, 3, Bit [0] of General Flags (the
Consumer/Producer bit) should be ignored for QWord/DWord/Word Address Space
descriptors. The Consumer/Producer bit is defined only for the Extended
Address Space descriptor.
Ignore Consumer/Producer except for Extended Address Space descriptors.
Note that for QWord/DWord/Word descriptors, we previously applied the
translation offset (_TRA) only when the Consumer/Producer bit was set.
This patch changes that: for those descriptors, we ignore Consumer/Producer
and always apply the translation offset.
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
drivers/acpi/resource.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
index 2732d39e..b45cd8f 100644
--- a/drivers/acpi/resource.c
+++ b/drivers/acpi/resource.c
@@ -261,11 +261,16 @@ bool acpi_dev_resource_address_space(struct acpi_resource *ares,
* primary side. Non-bridge devices must list 0 for all Address
* Translation offset bits.
*/
- if (addr->producer_consumer == ACPI_PRODUCER)
+ if (ares->type == ACPI_RESOURCE_TYPE_EXTENDED_ADDRESS64) {
+ if (addr->producer_consumer == ACPI_PRODUCER)
+ offset = attr->translation_offset;
+ else if (attr->translation_offset)
+ pr_debug("ACPI: translation_offset(%lld) is invalid for non-bridge device.\n",
+ attr->translation_offset);
+ } else {
offset = attr->translation_offset;
- else if (attr->translation_offset)
- pr_debug("ACPI: translation_offset(%lld) is invalid for non-bridge device.\n",
- attr->translation_offset);
+ }
+
start = attr->minimum + offset;
end = attr->maximum + offset;
@@ -294,7 +299,8 @@ bool acpi_dev_resource_address_space(struct acpi_resource *ares,
return false;
}
- if (addr->producer_consumer == ACPI_PRODUCER)
+ if (ares->type == ACPI_RESOURCE_TYPE_EXTENDED_ADDRESS64 &&
+ addr->producer_consumer == ACPI_PRODUCER)
res->flags |= IORESOURCE_WINDOW;
if (addr->info.mem.caching == ACPI_PREFETCHABLE_MEMORY)
^ permalink raw reply related
* [PATCH v7 2/8] drm/sun8i: Add DT bindings documentation of Allwinner DE2
From: Laurent Pinchart @ 2016-11-29 18:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <36111311.d8ul9hQ2CU@avalon>
Hi Jean-Fran?ois,
A brief update.
On Tuesday 29 Nov 2016 20:41:30 Laurent Pinchart wrote:
> On Monday 28 Nov 2016 19:02:39 Jean-Francois Moine wrote:
> > Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
> > ---
> >
> > .../bindings/display/sunxi/sun8i-de2.txt | 121 ++++++++++++++++
> > 1 file changed, 121 insertions(+)
> > create mode 100644
> >
> > Documentation/devicetree/bindings/display/sunxi/sun8i-de2.txt
> >
> > diff --git a/Documentation/devicetree/bindings/display/sunxi/sun8i-de2.txt
> > b/Documentation/devicetree/bindings/display/sunxi/sun8i-de2.txt new file
> > mode 100644
> > index 0000000..edf38b8
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/sunxi/sun8i-de2.txt
> > @@ -0,0 +1,121 @@
> > +Allwinner sun8i Display Engine 2 subsystem
> > +==========================================
> > +
> > +The Allwinner DE2 subsystem contains a display controller (DE2),
> > +one or two LCD controllers (Timing CONtrollers) and their external
> > +interfaces (encoders/connectors).
> > +
> > + +-----------+
> > + | DE2 |
> > + | |
> > + | +-------+ |
> > + plane --->| | | +------+
> > + | | mixer |---->| TCON |---> encoder ---> display
> > + plane --->| | | +------+ connector device
> > + | +-------+ |
> > + | |
> > + | +-------+ |
> > + plane --->| | | +------+
> > + | | mixer |---->| TCON |---> encoder ---> display
> > + plane --->| | | +------+ connector device
> > + | +-------+ |
> > + +-----------+
> > +
> > +The DE2 contains a processor which mixes the input planes and creates
> > +the images which are sent to the TCONs.
> > +From the software point of vue, there are 2 independent real-time
> > +mixers, each one being statically associated to one TCON.
> > +
> > +The TCONs adjust the image format to the one of the display device.
> > +
> > +Display controller (DE2)
> > +========================
> > +
> > +Required properties:
> > +
> > +- compatible: value should be one of the following
> > + "allwinner,sun8i-a83t-display-engine"
> > + "allwinner,sun8i-h3-display-engine"
> > +
> > +- reg: base address and size of the I/O memory
> > +
> > +- clocks: must include clock specifiers corresponding to entries in the
> > + clock-names property.
> > +
> > +- clock-names: must contain
> > + "bus": bus gate
> > + "clock": clock
> > +
> > +- resets: phandle to the reset of the device
> > +
> > +- ports: must contain a list of 2 phandles, indexed by mixer number,
> > + and pointing to display interface ports of TCONs
> > +
> > +LCD controller (TCON)
> > +=====================
> > +
> > +Required properties:
> > +
> > +- compatible: should be
> > + "allwinner,sun8i-a83t-tcon"
> > +
> > +- reg: base address and size of the I/O memory
> > +
> > +- clocks: must include clock specifiers corresponding to entries in the
> > + clock-names property.
> > +
> > +- clock-names: must contain
> > + "bus": bus gate
> > + "clock": pixel clock
> > +
> > +- resets: phandle to the reset of the device
> > +
> > +- interrupts: interrupt number for the TCON
> > +
> > +- port: port node with endpoint definitions as defined in
> > + Documentation/devicetree/bindings/media/video-interfaces.txt
> > +
> > +Example:
> > +
> > + de: de-controller at 01000000 {
> > + compatible = "allwinner,sun8i-h3-display-engine";
> > + reg = <0x01000000 0x400000>;
> > + clocks = <&ccu CLK_BUS_DE>, <&ccu CLK_DE>;
> > + clock-names = "bus", "clock";
> > + resets = <&ccu RST_BUS_DE>;
> > + ports = <&tcon0_p>, <&tcon1_p>;
>
> This isn't how the OF graph DT bindings are used. You should instead have
>
> ports {
> #address-cells = <1>;
> #size-cells = <0>;
> port at 0 {
I forgot to add reg = <0>; (and similarly for port 1).
> de_out_0: endpoint {
> remote_endpoint = <&tcon0_hdmi>;
> };
> };
> port at 1 {
> /* No endpoint as the port is not connected */
> };
> };
>
> > + };
> > +
> > + tcon0: lcd-controller at 01c0c000 {
> > + compatible = "allwinner,sun8i-a83t-tcon";
> > + reg = <0x01c0c000 0x400>;
> > + clocks = <&ccu CLK_BUS_TCON0>, <&ccu CLK_TCON0>;
> > + clock-names = "bus", "clock";
> > + resets = <&ccu RST_BUS_TCON0>;
> > + interrupts = <GIC_SPI 86 IRQ_TYPE_LEVEL_HIGH>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + tcon0_p: port {
> > + tcon0_hdmi: endpoint {
> > + remote-endpoint = <&hdmi_tcon0>;
> > + };
> > + };
>
> and here
>
> port {
> tcon0_hdmi: endpoint {
> remote-endpoint = <&de_out_0>;
> };
> };
The TCON has an output, so this should instead be
port at 0 {
reg = <0>;
tcon0_in: endpoint {
remote-endpoint = <&de_out_0>;
};
};
port at 1 {
reg = <1>;
tcon1_out: endpoint {
remote-endpoint = <&hdmi_in>;
};
};
(the second port requires adding the HDMI encoder to the example)
> > + };
> > +
> > + /* not used */
> > + tcon1: lcd-controller at 01c0d000 {
> > + compatible = "allwinner,sun8i-h3-tcon";
> > + reg = <0x01c0d000 0x400>;
> > + clocks = <&ccu CLK_BUS_TCON1>,
> > + <&ccu CLK_TCON0>; /* no clock */
> > + clock-names = "bus", "clock";
> > + interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>;
> > + status = "disabled";
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + tcon1_p: port {
> > + endpoint {
> > + /* empty */
> > + };
> > + };
>
> and here
>
> port {
> /* No endpoint as the port is not connected */
> };
>
> (although I'm not sure why you don't connect it)
>
> > + };
--
Regards,
Laurent Pinchart
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox