* arm64: unhandled level 0 translation fault
From: Geert Uytterhoeven @ 2017-12-12 20:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171212165728.GC15783@arm.com>
Hi Will,
On Tue, Dec 12, 2017 at 5:57 PM, Will Deacon <will.deacon@arm.com> wrote:
> On Tue, Dec 12, 2017 at 05:00:33PM +0100, Geert Uytterhoeven wrote:
>> On Tue, Dec 12, 2017 at 4:11 PM, Geert Uytterhoeven
>> <geert@linux-m68k.org> wrote:
>> > On Tue, Dec 12, 2017 at 11:36 AM, Will Deacon <will.deacon@arm.com> wrote:
>> >> On Tue, Dec 12, 2017 at 11:20:09AM +0100, Geert Uytterhoeven wrote:
>> >>> During userspace (Debian jessie NFS root) boot on arm64:
>> >>>
>> >>> rpcbind[1083]: unhandled level 0 translation fault (11) at 0x00000008,
>> >>> esr 0x92000004, in dash[aaaaadf77000+1a000]
>> >>> CPU: 0 PID: 1083 Comm: rpcbind Not tainted
>> >>> 4.15.0-rc3-arm64-renesas-02176-g14f9a1826e48e355 #51
>> >>> Hardware name: Renesas Salvator-X 2nd version board based on r8a7795 ES2.0+ (DT)
>> >>> pstate: 80000000 (Nzcv daif -PAN -UAO)
>> >>> pc : 0xaaaaadf8a51c
>> >>> lr : 0xaaaaadf8ac08
>> >>> sp : 0000ffffcffeac00
>> >>> x29: 0000ffffcffeac00 x28: 0000aaaaadfa1000
>> >>> x27: 0000ffffcffebf7c x26: 0000ffffcffead20
>> >>> x25: 0000aaaacea1c5f0 x24: 0000000000000000
>> >>> x23: 0000aaaaadfa1000 x22: 0000aaaaadfa1000
>> >>> x21: 0000000000000000 x20: 0000000000000008
>> >>> x19: 0000000000000000 x18: 0000ffffcffeb500
>> >>> x17: 0000ffffa22babfc x16: 0000aaaaadfa1ae8
>> >>> x15: 0000ffffa2363588 x14: ffffffffffffffff
>> >>> x13: 0000000000000020 x12: 0000000000000010
>> >>> x11: 0101010101010101 x10: 0000aaaaadfa1000
>> >>> x9 : 00000000ffffff81 x8 : 0000aaaaadfa2000
>> >>> x7 : 0000000000000000 x6 : 0000000000000000
>> >>> x5 : 0000aaaaadfa2338 x4 : 0000aaaaadfa2000
>> >>> x3 : 0000aaaaadfa2338 x2 : 0000000000000000
>> >>> x1 : 0000aaaaadfa28b0 x0 : 0000aaaaadfa4c30
>> >>>
>> >>> Sometimes it happens with other processes, but the main address, esr, and
>> >>> pstate values are always the same.
>> >>>
>> >>> I regularly run arm64/for-next/core (through bi-weekly renesas-drivers
>> >>> releases, so the last time was two weeks ago), but never saw the issue
>> >>> before until today, so probably v4.15-rc1 is OK.
>> >>> Unfortunately it doesn't happen during every boot, which makes it
>> >>> cumbersome to bisect.
>> >>>
>> >>> My first guess was UNMAP_KERNEL_AT_EL0, but even after disabling that,
>> >>> and even without today's arm64/for-next/core merged in, I still managed to
>> >>> reproduce the issue, so I believe it was introduced in v4.15-rc2 or
>> >>> v4.15-rc3.
>> >>
>> >> Urgh, this looks nasty. Thanks for the report! A few questions:
>> >>
>> >> - Can you share your .config somewhere please?
>> >
>> > I managed to reproduce it on plain v4.15-rc3 using both arm64_defconfig, and
>> > renesas_defconfig (from Simon's repo).
>>
>> v4.15-rc2 is affected, too.
>
> Do you reckon you can bisect between -rc1 and -rc2? We've been unable to
> reproduce this on any of our systems, unfortunately.
I've tried, but ended up on an unrelated XFS merge commit. Probably I
marked a few commits good due to not seeing this heisenbug.
For reference, here's the bisect log.
Bad commits showed one or both of "unhandled level 0 translation fault" and
"invalid pointer". Good commits didn't show any during 6 tries.
git bisect start
# bad: [ae64f9bd1d3621b5e60d7363bc20afb46aede215] Linux 4.15-rc2
git bisect bad ae64f9bd1d3621b5e60d7363bc20afb46aede215
# good: [4fbd8d194f06c8a3fd2af1ce560ddb31f7ec8323] Linux 4.15-rc1
git bisect good 4fbd8d194f06c8a3fd2af1ce560ddb31f7ec8323
# good: [9e0600f5cf6cecfcab5046d1453a9538c054d8a7] Merge tag
'for-linus' of git://git.kernel.org/pub/scm/virt/kvm/kvm
git bisect good 9e0600f5cf6cecfcab5046d1453a9538c054d8a7
# good: [503505bfea19b7d69e2572297e6defa0f9c2404e] Merge branch
'drm-fixes-4.15' of git://people.freedesktop.org/~agd5f/linux into
drm-fixes
git bisect good 503505bfea19b7d69e2572297e6defa0f9c2404e
# good: [ae753ee2771a1bacade56411bb98037b2545c929] Merge tag
'afs-fixes-20171201' of
git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs
git bisect good ae753ee2771a1bacade56411bb98037b2545c929
# good: [e1ba1c99dad92c5917b22b1047cf36e4426b124a] Merge tag
'riscv-for-linus-4.15-rc2_cleanups' of
git://git.kernel.org/pub/scm/linux/kernel/git/palmer/linux
git bisect good e1ba1c99dad92c5917b22b1047cf36e4426b124a
# bad: [2db767d9889cef087149a5eaa35c1497671fa40f] Merge tag
'nfs-for-4.15-2' of git://git.linux-nfs.org/projects/anna/linux-nfs
git bisect bad 2db767d9889cef087149a5eaa35c1497671fa40f
# good: [22a6c83777ac7c17d6c63891beeeac24cf5da450] xfs: ubsan fixes
git bisect good 22a6c83777ac7c17d6c63891beeeac24cf5da450
# bad: [788c1da05b73aee68ed98f05b577c308351f5619] Merge tag
'xfs-4.15-fixes-4' of git://git.kernel.org/pub/scm/fs/xfs/xfs-linux
git bisect bad 788c1da05b73aee68ed98f05b577c308351f5619
# good: [3b42d385753c22b29d259ccb9d4c3f419e583b30] xfs: scrub inode
mode properly
git bisect good 3b42d385753c22b29d259ccb9d4c3f419e583b30
# good: [373b0589dc8d58bc09c9a28d03611ae4fb216057] xfs: Properly retry
failed dquot items in case of error during buffer writeback
git bisect good 373b0589dc8d58bc09c9a28d03611ae4fb216057
# first bad commit: [788c1da05b73aee68ed98f05b577c308351f5619] Merge
tag 'xfs-4.15-fixes-4' of
git://git.kernel.org/pub/scm/fs/xfs/xfs-linux
Tomorrow there's another day in bisection paradise...
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* [PATCH v3 1/8] SOC: brcmstb: add memory API
From: Jim Quinlan @ 2017-12-12 20:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171205205926.GJ23510@bhelgaas-glaptop.roam.corp.google.com>
On Tue, Dec 5, 2017 at 3:59 PM, Bjorn Helgaas <helgaas@kernel.org> wrote:
> On Tue, Nov 14, 2017 at 05:12:05PM -0500, Jim Quinlan wrote:
>> From: Florian Fainelli <f.fainelli@gmail.com>
>>
>> This commit adds a memory API suitable for ascertaining the sizes of
>> each of the N memory controllers in a Broadcom STB chip. Its first
>> user will be the Broadcom STB PCIe root complex driver, which needs
>> to know these sizes to properly set up DMA mappings for inbound
>> regions.
>>
>> We cannot use memblock here or anything like what Linux provides
>> because it collapses adjacent regions within a larger block, and here
>> we actually need per-memory controller addresses and sizes, which is
>> why we resort to manual DT parsing.
>>
>> Signed-off-by: Jim Quinlan <jim2101024@gmail.com>
>> ---
>> drivers/soc/bcm/brcmstb/Makefile | 2 +-
>> drivers/soc/bcm/brcmstb/memory.c | 172 +++++++++++++++++++++++++++++++++++++++
>> include/soc/brcmstb/memory_api.h | 25 ++++++
>> 3 files changed, 198 insertions(+), 1 deletion(-)
>> create mode 100644 drivers/soc/bcm/brcmstb/memory.c
>> create mode 100644 include/soc/brcmstb/memory_api.h
>>
>> diff --git a/drivers/soc/bcm/brcmstb/Makefile b/drivers/soc/bcm/brcmstb/Makefile
>> index 9120b27..4cea7b6 100644
>> --- a/drivers/soc/bcm/brcmstb/Makefile
>> +++ b/drivers/soc/bcm/brcmstb/Makefile
>> @@ -1 +1 @@
>> -obj-y += common.o biuctrl.o
>> +obj-y += common.o biuctrl.o memory.o
>> diff --git a/drivers/soc/bcm/brcmstb/memory.c b/drivers/soc/bcm/brcmstb/memory.c
>> new file mode 100644
>> index 0000000..eb647ad9
>> --- /dev/null
>> +++ b/drivers/soc/bcm/brcmstb/memory.c
>
> I sort of assume based on [1] that every new file should have an SPDX
> identifier ("The Linux kernel requires the precise SPDX identifier in
> all source files") and that the actual text of the GPL can be omitted.
>
> Only a few files in drivers/pci currently have an SPDX identifier. I
> don't know if that's oversight or work-in-progress or what.
>
> [1] https://lkml.kernel.org/r/20171204212120.484179273 at linutronix.de
>
Bjorn, Did you get a chance to review the other commits of this
submission (V3)? I would like any additional feedback before I send
out a V4 with SPDX fixes. Thanks, JimQ
>> @@ -0,0 +1,172 @@
>> +/*
>> + * Copyright ? 2015-2017 Broadcom
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + * A copy of the GPL is available at
>> + * http://www.broadcom.com/licenses/GPLv2.php or from the Free Software
>> + * Foundation at https://www.gnu.org/licenses/ .
^ permalink raw reply
* [PATCH v4 09/12] clk: qcom: Add Krait clock controller driver
From: Rob Herring @ 2017-12-12 20:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1512726150-7204-10-git-send-email-sricharan@codeaurora.org>
On Fri, Dec 08, 2017 at 03:12:27PM +0530, Sricharan R wrote:
> From: Stephen Boyd <sboyd@codeaurora.org>
>
> The Krait CPU clocks are made up of a primary mux and secondary
> mux for each CPU and the L2, controlled via cp15 accessors. For
> Kraits within KPSSv1 each secondary mux accepts a different aux
> source, but on KPSSv2 each secondary mux accepts the same aux
> source.
>
> Cc: <devicetree@vger.kernel.org>
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
> .../devicetree/bindings/clock/qcom,krait-cc.txt | 22 ++
Please make bindings a separate patch.
> drivers/clk/qcom/Kconfig | 8 +
> drivers/clk/qcom/Makefile | 1 +
> drivers/clk/qcom/krait-cc.c | 350 +++++++++++++++++++++
> 4 files changed, 381 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/clock/qcom,krait-cc.txt
> create mode 100644 drivers/clk/qcom/krait-cc.c
^ permalink raw reply
* [PATCH 3/3] [v5] pinctrl: qcom: qdf2xxx: add support for new ACPI HID QCOM8002
From: Timur Tabi @ 2017-12-12 20:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1513111858-6251-1-git-send-email-timur@codeaurora.org>
Newer versions of the firmware for the Qualcomm Datacenter Technologies
QDF2400 restricts access to a subset of the GPIOs on the TLMM. To
prevent older kernels from accidentally accessing the restricted GPIOs,
we change the ACPI HID for the TLMM block from QCOM8001 to QCOM8002,
and introduce a new property "gpios". This property is an array of
specific GPIOs that are accessible. When an older kernel boots on
newer (restricted) firmware, it will fail to probe.
To implement the sparse GPIO map, we register all of the GPIOs, but set
the pin count for the unavailable GPIOs to zero. The pinctrl-msm
driver will block those unavailable GPIOs from being accessed.
To allow newer kernels to support older firmware, the driver retains
support for QCOM8001.
Signed-off-by: Timur Tabi <timur@codeaurora.org>
---
drivers/pinctrl/qcom/pinctrl-qdf2xxx.c | 143 ++++++++++++++++++++++++---------
1 file changed, 107 insertions(+), 36 deletions(-)
diff --git a/drivers/pinctrl/qcom/pinctrl-qdf2xxx.c b/drivers/pinctrl/qcom/pinctrl-qdf2xxx.c
index bb3ce5c3e18b..90b32f424a28 100644
--- a/drivers/pinctrl/qcom/pinctrl-qdf2xxx.c
+++ b/drivers/pinctrl/qcom/pinctrl-qdf2xxx.c
@@ -38,68 +38,145 @@
/* maximum size of each gpio name (enough room for "gpioXXX" + null) */
#define NAME_SIZE 8
+enum {
+ QDF2XXX_V1,
+ QDF2XXX_V2,
+};
+
+static const struct acpi_device_id qdf2xxx_acpi_ids[] = {
+ {"QCOM8001", QDF2XXX_V1},
+ {"QCOM8002", QDF2XXX_V2},
+ {},
+};
+MODULE_DEVICE_TABLE(acpi, qdf2xxx_acpi_ids);
+
static int qdf2xxx_pinctrl_probe(struct platform_device *pdev)
{
+ const struct acpi_device_id *id =
+ acpi_match_device(qdf2xxx_acpi_ids, &pdev->dev);
struct pinctrl_pin_desc *pins;
struct msm_pingroup *groups;
char (*names)[NAME_SIZE];
unsigned int i;
u32 num_gpios;
+ unsigned int avail_gpios; /* The number of GPIOs we support */
+ u16 *gpios; /* An array of supported GPIOs */
int ret;
/* Query the number of GPIOs from ACPI */
ret = device_property_read_u32(&pdev->dev, "num-gpios", &num_gpios);
if (ret < 0) {
- dev_warn(&pdev->dev, "missing num-gpios property\n");
+ dev_err(&pdev->dev, "missing 'num-gpios' property\n");
return ret;
}
-
if (!num_gpios || num_gpios > MAX_GPIOS) {
- dev_warn(&pdev->dev, "invalid num-gpios property\n");
+ dev_err(&pdev->dev, "invalid 'num-gpios' property\n");
return -ENODEV;
}
+ /*
+ * The QCOM8001 HID contains only the number of GPIOs, and assumes
+ * that all of them are available. avail_gpios is the same as num_gpios.
+ *
+ * The QCOM8002 HID introduces the 'gpios' DSD, which lists
+ * specific GPIOs that the driver is allowed to access.
+ *
+ * The make the common code simpler, in both cases we create an
+ * array of GPIOs that are accessible. So for QCOM8001, that would
+ * be all of the GPIOs.
+ */
+ if (id->driver_data == QDF2XXX_V1) {
+ avail_gpios = num_gpios;
+
+ gpios = devm_kmalloc_array(&pdev->dev, avail_gpios,
+ sizeof(gpios[0]), GFP_KERNEL);
+ if (!gpios)
+ return -ENOMEM;
+
+ for (i = 0; i < avail_gpios; i++)
+ gpios[i] = i;
+ } else {
+ /* The number of GPIOs in the approved list */
+ ret = device_property_read_u16_array(&pdev->dev, "gpios",
+ NULL, 0);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "missing 'gpios' property\n");
+ return ret;
+ }
+ /*
+ * The number of available GPIOs should be non-zero, and no
+ * more than the total number of GPIOS.
+ */
+ if (!ret || ret > num_gpios) {
+ dev_err(&pdev->dev, "invalid 'gpios' property\n");
+ return -ENODEV;
+ }
+ avail_gpios = ret;
+
+ gpios = devm_kmalloc_array(&pdev->dev, avail_gpios,
+ sizeof(gpios[0]), GFP_KERNEL);
+ if (!gpios)
+ return -ENOMEM;
+
+ ret = device_property_read_u16_array(&pdev->dev, "gpios", gpios,
+ avail_gpios);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "could not read list of GPIOs\n");
+ return ret;
+ }
+ }
+
pins = devm_kcalloc(&pdev->dev, num_gpios,
sizeof(struct pinctrl_pin_desc), GFP_KERNEL);
groups = devm_kcalloc(&pdev->dev, num_gpios,
sizeof(struct msm_pingroup), GFP_KERNEL);
- names = devm_kcalloc(&pdev->dev, num_gpios, NAME_SIZE, GFP_KERNEL);
+ names = devm_kcalloc(&pdev->dev, avail_gpios, NAME_SIZE, GFP_KERNEL);
if (!pins || !groups || !names)
return -ENOMEM;
+ /*
+ * Initialize the array. GPIOs not listed in the 'gpios' array
+ * still need a number, but nothing else.
+ */
for (i = 0; i < num_gpios; i++) {
- snprintf(names[i], NAME_SIZE, "gpio%u", i);
-
pins[i].number = i;
- pins[i].name = names[i];
-
- groups[i].npins = 1;
- groups[i].name = names[i];
groups[i].pins = &pins[i].number;
+ }
- groups[i].ctl_reg = 0x10000 * i;
- groups[i].io_reg = 0x04 + 0x10000 * i;
- groups[i].intr_cfg_reg = 0x08 + 0x10000 * i;
- groups[i].intr_status_reg = 0x0c + 0x10000 * i;
- groups[i].intr_target_reg = 0x08 + 0x10000 * i;
-
- groups[i].mux_bit = 2;
- groups[i].pull_bit = 0;
- groups[i].drv_bit = 6;
- groups[i].oe_bit = 9;
- groups[i].in_bit = 0;
- groups[i].out_bit = 1;
- groups[i].intr_enable_bit = 0;
- groups[i].intr_status_bit = 0;
- groups[i].intr_target_bit = 5;
- groups[i].intr_target_kpss_val = 1;
- groups[i].intr_raw_status_bit = 4;
- groups[i].intr_polarity_bit = 1;
- groups[i].intr_detection_bit = 2;
- groups[i].intr_detection_width = 2;
+ /* Populate the entries that are meant to be exposes as GPIOs. */
+ for (i = 0; i < avail_gpios; i++) {
+ unsigned int gpio = gpios[i];
+
+ groups[gpio].npins = 1;
+ snprintf(names[i], NAME_SIZE, "gpio%u", gpio);
+ pins[gpio].name = names[i];
+ groups[gpio].name = names[i];
+
+ groups[gpio].ctl_reg = 0x10000 * gpio;
+ groups[gpio].io_reg = 0x04 + 0x10000 * gpio;
+ groups[gpio].intr_cfg_reg = 0x08 + 0x10000 * gpio;
+ groups[gpio].intr_status_reg = 0x0c + 0x10000 * gpio;
+ groups[gpio].intr_target_reg = 0x08 + 0x10000 * gpio;
+
+ groups[gpio].mux_bit = 2;
+ groups[gpio].pull_bit = 0;
+ groups[gpio].drv_bit = 6;
+ groups[gpio].oe_bit = 9;
+ groups[gpio].in_bit = 0;
+ groups[gpio].out_bit = 1;
+ groups[gpio].intr_enable_bit = 0;
+ groups[gpio].intr_status_bit = 0;
+ groups[gpio].intr_target_bit = 5;
+ groups[gpio].intr_target_kpss_val = 1;
+ groups[gpio].intr_raw_status_bit = 4;
+ groups[gpio].intr_polarity_bit = 1;
+ groups[gpio].intr_detection_bit = 2;
+ groups[gpio].intr_detection_width = 2;
}
+ devm_kfree(&pdev->dev, gpios);
+
qdf2xxx_pinctrl.pins = pins;
qdf2xxx_pinctrl.groups = groups;
qdf2xxx_pinctrl.npins = num_gpios;
@@ -109,12 +186,6 @@ static int qdf2xxx_pinctrl_probe(struct platform_device *pdev)
return msm_pinctrl_probe(pdev, &qdf2xxx_pinctrl);
}
-static const struct acpi_device_id qdf2xxx_acpi_ids[] = {
- {"QCOM8001"},
- {},
-};
-MODULE_DEVICE_TABLE(acpi, qdf2xxx_acpi_ids);
-
static struct platform_driver qdf2xxx_pinctrl_driver = {
.driver = {
.name = "qdf2xxx-pinctrl",
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply related
* [PATCH 2/3] [v8] pinctrl: qcom: disable GPIO groups with no pins
From: Timur Tabi @ 2017-12-12 20:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1513111858-6251-1-git-send-email-timur@codeaurora.org>
pinctrl-msm only accepts an array of GPIOs from 0 to n-1, and it expects
each group to support have only one pin (npins == 1).
We can support "sparse" GPIO maps if we allow for some groups to have zero
pins (npins == 0). These pins are "hidden" from the rest of the driver
and gpiolib.
Access to unavailable GPIOs is blocked via a request callback. If the
requested GPIO is unavailable, -EACCES is returned, which prevents
further access to that GPIO.
Signed-off-by: Timur Tabi <timur@codeaurora.org>
---
drivers/pinctrl/qcom/pinctrl-msm.c | 28 +++++++++++++++++++++++-----
1 file changed, 23 insertions(+), 5 deletions(-)
diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c
index 7a960590ecaa..d45b4c2b5af1 100644
--- a/drivers/pinctrl/qcom/pinctrl-msm.c
+++ b/drivers/pinctrl/qcom/pinctrl-msm.c
@@ -507,6 +507,11 @@ static void msm_gpio_dbg_show_one(struct seq_file *s,
};
g = &pctrl->soc->groups[offset];
+
+ /* If the GPIO group has no pins, then don't show it. */
+ if (!g->npins)
+ return;
+
ctl_reg = readl(pctrl->regs + g->ctl_reg);
is_out = !!(ctl_reg & BIT(g->oe_bit));
@@ -516,7 +521,7 @@ static void msm_gpio_dbg_show_one(struct seq_file *s,
seq_printf(s, " %-8s: %-3s %d", g->name, is_out ? "out" : "in", func);
seq_printf(s, " %dmA", msm_regval_to_drive(drive));
- seq_printf(s, " %s", pulls[pull]);
+ seq_printf(s, " %s\n", pulls[pull]);
}
static void msm_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip)
@@ -524,23 +529,36 @@ static void msm_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip)
unsigned gpio = chip->base;
unsigned i;
- for (i = 0; i < chip->ngpio; i++, gpio++) {
+ for (i = 0; i < chip->ngpio; i++, gpio++)
msm_gpio_dbg_show_one(s, NULL, chip, i, gpio);
- seq_puts(s, "\n");
- }
}
#else
#define msm_gpio_dbg_show NULL
#endif
+/*
+ * If the requested GPIO has no pins, then treat it as unavailable.
+ * Otherwise, call the standard request function.
+ */
+static int msm_gpio_request(struct gpio_chip *chip, unsigned int offset)
+{
+ struct msm_pinctrl *pctrl = gpiochip_get_data(chip);
+ const struct msm_pingroup *g = &pctrl->soc->groups[offset];
+
+ if (!g->npins)
+ return -EACCES;
+
+ return gpiochip_generic_request(chip, offset);
+}
+
static const struct gpio_chip msm_gpio_template = {
.direction_input = msm_gpio_direction_input,
.direction_output = msm_gpio_direction_output,
.get_direction = msm_gpio_get_direction,
.get = msm_gpio_get,
.set = msm_gpio_set,
- .request = gpiochip_generic_request,
+ .request = msm_gpio_request,
.free = gpiochip_generic_free,
.dbg_show = msm_gpio_dbg_show,
};
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply related
* [PATCH 1/3] [v2] Revert "gpio: set up initial state from .get_direction()"
From: Timur Tabi @ 2017-12-12 20:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1513111858-6251-1-git-send-email-timur@codeaurora.org>
This reverts commit 72d3200061776264941be1b5a9bb8e926b3b30a5.
We cannot blindly query the direction of all GPIOs when the pins are
first registered. The get_direction callback normally triggers a
read/write to hardware, but we shouldn't be touching the hardware for
an individual GPIO until after it's been properly claimed.
Signed-off-by: Timur Tabi <timur@codeaurora.org>
---
drivers/gpio/gpiolib.c | 31 +++++++------------------------
1 file changed, 7 insertions(+), 24 deletions(-)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 641a5eb552cb..168dd831551d 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1207,31 +1207,14 @@ int gpiochip_add_data_with_key(struct gpio_chip *chip, void *data,
struct gpio_desc *desc = &gdev->descs[i];
desc->gdev = gdev;
- /*
- * REVISIT: most hardware initializes GPIOs as inputs
- * (often with pullups enabled) so power usage is
- * minimized. Linux code should set the gpio direction
- * first thing; but until it does, and in case
- * chip->get_direction is not set, we may expose the
- * wrong direction in sysfs.
- */
-
- if (chip->get_direction) {
- /*
- * If we have .get_direction, set up the initial
- * direction flag from the hardware.
- */
- int dir = chip->get_direction(chip, i);
- if (!dir)
- set_bit(FLAG_IS_OUT, &desc->flags);
- } else if (!chip->direction_input) {
- /*
- * If the chip lacks the .direction_input callback
- * we logically assume all lines are outputs.
- */
- set_bit(FLAG_IS_OUT, &desc->flags);
- }
+ /* REVISIT: most hardware initializes GPIOs as inputs (often
+ * with pullups enabled) so power usage is minimized. Linux
+ * code should set the gpio direction first thing; but until
+ * it does, and in case chip->get_direction is not set, we may
+ * expose the wrong direction in sysfs.
+ */
+ desc->flags = !chip->direction_input ? (1 << FLAG_IS_OUT) : 0;
}
#ifdef CONFIG_PINCTRL
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply related
* [PATCH 0/3] [v9] pinctrl: qcom: add support for sparse GPIOs
From: Timur Tabi @ 2017-12-12 20:50 UTC (permalink / raw)
To: linux-arm-kernel
A series of patches that add support for GPIO maps that have holes in
them. That is, even though a client driver has N consecutive GPIOs,
some are just unavailable for whatever reason, and the hardware should
not be accessed for those GPIOs.
Patch 1 reverts an old patch that triggers a get_direction of every
pin upon init, without attempting to request the pins first. The
direction is already being queried when the pin is requested.
Patch 2 adds support to pinctrl-msm for "unavailable" GPIOs.
Patch 3 extends that support to pinctrl-qdf2xxx. A recent ACPI change
on QDF2400 platforms blocks access to most pins, so the driver can only
register a subset.
This version drops the availability check in gpiolib, because it's no
necessary. Instead, just having pinctrl-msm return -EACCES is enough
to block all unavailable GPIOs. Patch 1 removes the only instance where
an unrequested GPIO is being accessed.
v9:
Removed "gpiolib: add bitmask for valid GPIO lines"
Timur Tabi (3):
[v2] Revert "gpio: set up initial state from .get_direction()"
[v8] pinctrl: qcom: disable GPIO groups with no pins
[v5] pinctrl: qcom: qdf2xxx: add support for new ACPI HID QCOM8002
drivers/gpio/gpiolib.c | 31 ++-----
drivers/pinctrl/qcom/pinctrl-msm.c | 28 +++++--
drivers/pinctrl/qcom/pinctrl-qdf2xxx.c | 143 ++++++++++++++++++++++++---------
3 files changed, 137 insertions(+), 65 deletions(-)
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply
* [PATCH v7 3/8] KVM: arm/arm64: Don't cache the timer IRQ level
From: Christoffer Dall @ 2017-12-12 20:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1604e8d6-5f9a-f91d-c199-57f323c09374@redhat.com>
On Tue, Dec 12, 2017 at 09:40:10AM +0100, Auger Eric wrote:
>
>
> On 11/12/17 21:51, Auger Eric wrote:
> > Hi Christoffer,
> > On 07/12/17 11:54, Christoffer Dall wrote:
> >> The timer was modeled after a strict idea of modelling an interrupt line
> >> level in software, meaning that only transitions in the level needed to
> >> be reported to the VGIC. This works well for the timer, because the
> >> arch timer code is in complete control of the device and can track the
> >> transitions of the line.
> >>
> >> However, as we are about to support using the HW bit in the VGIC not
> >> just for the timer, but also for VFIO which cannot track transitions of
> >> the interrupt line, we have to decide on an interface for level
> >> triggered mapped interrupts to the GIC, which both the timer and VFIO
> >> can use.
> >>
> >> VFIO only sees an asserting transition of the physical interrupt line,
> >> and tells the VGIC when that happens. That means that part of the
> >> interrupt flow is offloaded to the hardware.
> >>
> >> To use the same interface for VFIO devices and the timer, we therefore
> >> have to change the timer (we cannot change VFIO because it doesn't know
> >> the details of the device it is assigning to a VM).
> >>
> >> Luckily, changing the timer is simple, we just need to stop 'caching'
> >> the line level, but instead let the VGIC know the state of the timer
> >> every time there is a potential change in the line level, and when the
> >> line level should be asserted from the timer ISR. The VGIC can ignore
> >> extra notifications using its validate mechanism.
> >
> > I was confused by the fact we say we stop caching the line level but
> > vtimer->irq.level still exists, is updated in the vtimer host ISR and
> > kvm_timer_update_state() and read in many places.
> >
> > I feel difficult to figure out if each time we use the vtimer->irq.level
> > value it is safe to use it.
> >
> > Also for the validate() to succeed we need the vgic irq->line_level to
> > to be 0. I understand this is properly handled for mapped level irqs in
> > next patch which does that on the populate_lr. However I currently fail
> > to understand why the timer level sensitive mapped IRQ does not require
> > the next patch to work.
> OK reading again "[PATCH v7 7/8] KVM: arm/arm64: Provide a
> get_input_level for the arch timer", I now understand it works because
> we had the
> kvm_timer_sync_hwstate toggling down the line on VM exit. After the
> changes of next patch this can be safely removed.
Yes, but also note that this patch in isolation doesn't break anything,
it just ensures that we notify the GIC of an asserted line more often.
>
> Not related to this patch but I noticed
> Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt now is outdated.
Good point, I have updated the docs and will include that in v8.
Thanks,
-Christoffer
^ permalink raw reply
* [PATCH v4 08/12] clk: qcom: Add KPSS ACC/GCC driver
From: Rob Herring @ 2017-12-12 20:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1512726150-7204-9-git-send-email-sricharan@codeaurora.org>
On Fri, Dec 08, 2017 at 03:12:26PM +0530, Sricharan R wrote:
> From: Stephen Boyd <sboyd@codeaurora.org>
>
> The ACC and GCC regions present in KPSSv1 contain registers to
> control clocks and power to each Krait CPU and L2. For CPUfreq
> purposes probe these devices and expose a mux clock that chooses
> between PXO and PLL8.
>
> Cc: <devicetree@vger.kernel.org>
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
> .../devicetree/bindings/arm/msm/qcom,kpss-acc.txt | 7 ++
> .../devicetree/bindings/arm/msm/qcom,kpss-gcc.txt | 28 +++++++
Please make bindings a separate patch.
> drivers/clk/qcom/Kconfig | 8 ++
> drivers/clk/qcom/Makefile | 1 +
> drivers/clk/qcom/kpss-xcc.c | 96 ++++++++++++++++++++++
> 5 files changed, 140 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/arm/msm/qcom,kpss-gcc.txt
> create mode 100644 drivers/clk/qcom/kpss-xcc.c
>
> diff --git a/Documentation/devicetree/bindings/arm/msm/qcom,kpss-acc.txt b/Documentation/devicetree/bindings/arm/msm/qcom,kpss-acc.txt
> index 1333db9..382a574 100644
> --- a/Documentation/devicetree/bindings/arm/msm/qcom,kpss-acc.txt
> +++ b/Documentation/devicetree/bindings/arm/msm/qcom,kpss-acc.txt
> @@ -21,10 +21,17 @@ PROPERTIES
> the register region. An optional second element specifies
> the base address and size of the alias register region.
>
> +- clock-output-names:
> + Usage: optional
> + Value type: <string>
> + Definition: Name of the output clock. Typically acpuX_aux where X is a
> + CPU number starting at 0.
> +
> Example:
>
> clock-controller at 2088000 {
> compatible = "qcom,kpss-acc-v2";
> reg = <0x02088000 0x1000>,
> <0x02008000 0x1000>;
> + clock-output-names = "acpu0_aux";
> };
> diff --git a/Documentation/devicetree/bindings/arm/msm/qcom,kpss-gcc.txt b/Documentation/devicetree/bindings/arm/msm/qcom,kpss-gcc.txt
> new file mode 100644
> index 0000000..d1e12f1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/msm/qcom,kpss-gcc.txt
> @@ -0,0 +1,28 @@
> +Krait Processor Sub-system (KPSS) Global Clock Controller (GCC)
> +
> +PROPERTIES
> +
> +- compatible:
> + Usage: required
> + Value type: <string>
> + Definition: should be one of:
> + "qcom,kpss-gcc"
Only one implementation?
> +
> +- reg:
> + Usage: required
> + Value type: <prop-encoded-array>
> + Definition: base address and size of the register region
> +
> +- clock-output-names:
> + Usage: required
> + Value type: <string>
> + Definition: Name of the output clock. Typically acpu_l2_aux indicating
> + an L2 cache auxiliary clock.
> +
> +Example:
> +
> + l2cc: clock-controller at 2011000 {
> + compatible = "qcom,kpss-gcc";
> + reg = <0x2011000 0x1000>;
> + clock-output-names = "acpu_l2_aux";
> + };
^ permalink raw reply
* [PATCH v4 05/12] clk: qcom: Add MSM8960/APQ8064's HFPLLs
From: Rob Herring @ 2017-12-12 20:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1512726150-7204-6-git-send-email-sricharan@codeaurora.org>
On Fri, Dec 08, 2017 at 03:12:23PM +0530, Sricharan R wrote:
> From: Stephen Boyd <sboyd@codeaurora.org>
>
> Describe the HFPLLs present on MSM8960 and APQ8064 devices.
>
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
> drivers/clk/qcom/gcc-msm8960.c | 172 +++++++++++++++++++++++++++
> include/dt-bindings/clock/qcom,gcc-msm8960.h | 2 +
For the binding,
Acked-by: Rob Herring <robh@kernel.org>
> 2 files changed, 174 insertions(+)
^ permalink raw reply
* [PATCH v4 04/12] clk: qcom: Add HFPLL driver
From: Rob Herring @ 2017-12-12 20:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1512726150-7204-5-git-send-email-sricharan@codeaurora.org>
On Fri, Dec 08, 2017 at 03:12:22PM +0530, Sricharan R wrote:
> From: Stephen Boyd <sboyd@codeaurora.org>
>
> On some devices (MSM8974 for example), the HFPLLs are
> instantiated within the Krait processor subsystem as separate
> register regions. Add a driver for these PLLs so that we can
> provide HFPLL clocks for use by the system.
>
> Cc: <devicetree@vger.kernel.org>
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
> .../devicetree/bindings/clock/qcom,hfpll.txt | 40 ++++++++
> drivers/clk/qcom/Kconfig | 8 ++
> drivers/clk/qcom/Makefile | 1 +
> drivers/clk/qcom/hfpll.c | 106 +++++++++++++++++++++
> 4 files changed, 155 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/clock/qcom,hfpll.txt
> create mode 100644 drivers/clk/qcom/hfpll.c
>
> diff --git a/Documentation/devicetree/bindings/clock/qcom,hfpll.txt b/Documentation/devicetree/bindings/clock/qcom,hfpll.txt
> new file mode 100644
> index 0000000..fee92bb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/qcom,hfpll.txt
> @@ -0,0 +1,40 @@
> +High-Frequency PLL (HFPLL)
> +
> +PROPERTIES
> +
> +- compatible:
> + Usage: required
> + Value type: <string>
> + Definition: must be "qcom,hfpll"
Fine for a fallback, but please add SoC specific compatibles.
> +
> +- reg:
> + Usage: required
> + Value type: <prop-encoded-array>
> + Definition: address and size of HPLL registers. An optional second
> + element specifies the address and size of the alias
> + register region.
> +
> +- clock-output-names:
> + Usage: required
> + Value type: <string>
> + Definition: Name of the PLL. Typically hfpllX where X is a CPU number
> + starting at 0. Otherwise hfpll_Y where Y is more specific
> + such as "l2".
> +
> +Example:
> +
> +1) An HFPLL for the L2 cache.
> +
> + clock-controller at f9016000 {
> + compatible = "qcom,hfpll";
> + reg = <0xf9016000 0x30>;
> + clock-output-names = "hfpll_l2";
> + };
> +
> +2) An HFPLL for CPU0. This HFPLL has the alias register region.
> +
> + clock-controller at f908a000 {
> + compatible = "qcom,hfpll";
> + reg = <0xf908a000 0x30>, <0xf900a000 0x30>;
> + clock-output-names = "hfpll0";
> + };
> diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
> index 20b5d6f..6c811bd 100644
> --- a/drivers/clk/qcom/Kconfig
> +++ b/drivers/clk/qcom/Kconfig
> @@ -205,3 +205,11 @@ config SPMI_PMIC_CLKDIV
> Technologies, Inc. SPMI PMIC. It configures the frequency of
> clkdiv outputs of the PMIC. These clocks are typically wired
> through alternate functions on GPIO pins.
> +
> +config QCOM_HFPLL
> + tristate "High-Frequency PLL (HFPLL) Clock Controller"
> + depends on COMMON_CLK_QCOM
> + help
> + Support for the high-frequency PLLs present on Qualcomm devices.
> + Say Y if you want to support CPU frequency scaling on devices
> + such as MSM8974, APQ8084, etc.
> diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
> index 4795e21..4a4bf38 100644
> --- a/drivers/clk/qcom/Makefile
> +++ b/drivers/clk/qcom/Makefile
> @@ -36,3 +36,4 @@ obj-$(CONFIG_MSM_MMCC_8996) += mmcc-msm8996.o
> obj-$(CONFIG_QCOM_CLK_RPM) += clk-rpm.o
> obj-$(CONFIG_QCOM_CLK_SMD_RPM) += clk-smd-rpm.o
> obj-$(CONFIG_SPMI_PMIC_CLKDIV) += clk-spmi-pmic-div.o
> +obj-$(CONFIG_QCOM_HFPLL) += hfpll.o
> diff --git a/drivers/clk/qcom/hfpll.c b/drivers/clk/qcom/hfpll.c
> new file mode 100644
> index 0000000..7405bb6
> --- /dev/null
> +++ b/drivers/clk/qcom/hfpll.c
> @@ -0,0 +1,106 @@
> +/*
> + * Copyright (c) 2013-2014, The Linux Foundation. All rights reserved.
It's 2017.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
Use SPDX tags.
Rob
^ permalink raw reply
* [PATCH] ARM: pxa/lubbock: add GPIO driver for LUB_MISC_WR register
From: Robert Jarzmik @ 2017-12-12 20:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <E1eOTFj-0008MB-Fj@rmk-PC.armlinux.org.uk>
Russell King <rmk+kernel@armlinux.org.uk> writes:
> Add a gpio driver for the lubbock miscellaneous write IO register so we
> can take advantage of subsystems modelled around gpiolib, rather than
> having to provide platform specific callbacks.
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
Hi Russell and Linux,
It looks certainly good to me. It's unfortunate in the current status of
linux-next I have, gpio-reg.c seems broken (that one is for Linus in [1] and
[2]), and therefore I can't test it live.
Nonetheless, do you want me to carry it through the pxa tree or do you want to
keep it through your tree ?
Cheers.
--
Robert
[1] For Linus if he wasn't notified before :
---8>---
CC drivers/gpio/gpio-reg.o
drivers/gpio/gpio-reg.c: In function 'gpio_reg_to_irq':
drivers/gpio/gpio-reg.c:106:19: error: 'struct gpio_reg' has no member named 'irq'
if (irq >= 0 && r->irq.domain)
^
drivers/gpio/gpio-reg.c:107:27: error: 'struct gpio_reg' has no member named 'irq'
irq = irq_find_mapping(r->irq.domain, irq);
^
make[2]: *** [drivers/gpio/gpio-reg.o] Erreur 1
---8>---
[2] For Linux, git commit of my linux-next tip
d20787938ddb ("Add linux-next specific files for 20171212")
^ permalink raw reply
* [PATCH v8 10/13] IIO: ADC: add stm32 DFSDM support for PDM microphone
From: Jonathan Cameron @ 2017-12-12 20:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1512987524-2901-11-git-send-email-arnaud.pouliquen@st.com>
On Mon, 11 Dec 2017 11:18:41 +0100
Arnaud Pouliquen <arnaud.pouliquen@st.com> wrote:
> This code offers a way to handle PDM audio microphones in
> ASOC framework. Audio driver should use consumer API.
> A specific management is implemented for DMA, with a
> callback, to allows to handle audio buffers efficiently.
>
> Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@st.com>
Hi Arnaud,
I raise a few queries on v7 of this patch.
https://marc.info/?l=linux-iio&m=151292965915376&w=2
Jonathan
> ---
> .../ABI/testing/sysfs-bus-iio-dfsdm-adc-stm32 | 16 +
> drivers/iio/adc/stm32-dfsdm-adc.c | 508 ++++++++++++++++++++-
> include/linux/iio/adc/stm32-dfsdm-adc.h | 18 +
> 3 files changed, 534 insertions(+), 8 deletions(-)
> create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-dfsdm-adc-stm32
> create mode 100644 include/linux/iio/adc/stm32-dfsdm-adc.h
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-dfsdm-adc-stm32 b/Documentation/ABI/testing/sysfs-bus-iio-dfsdm-adc-stm32
> new file mode 100644
> index 0000000..da98223
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-dfsdm-adc-stm32
> @@ -0,0 +1,16 @@
> +What: /sys/bus/iio/devices/iio:deviceX/in_voltage_spi_clk_freq
> +KernelVersion: 4.14
> +Contact: arnaud.pouliquen at st.com
> +Description:
> + For audio purpose only.
> + Used by audio driver to set/get the spi input frequency.
> + This is mandatory if DFSDM is slave on SPI bus, to
> + provide information on the SPI clock frequency during runtime
> + Notice that the SPI frequency should be a multiple of sample
> + frequency to ensure the precision.
> + if DFSDM input is SPI master
> + Reading SPI clkout frequency,
> + error on writing
> + If DFSDM input is SPI Slave:
> + Reading returns value previously set.
> + Writing value before starting conversions.
> \ No newline at end of file
> diff --git a/drivers/iio/adc/stm32-dfsdm-adc.c b/drivers/iio/adc/stm32-dfsdm-adc.c
> index 68b5920..2d6aed5 100644
> --- a/drivers/iio/adc/stm32-dfsdm-adc.c
> +++ b/drivers/iio/adc/stm32-dfsdm-adc.c
> @@ -6,19 +6,25 @@
> * Author: Arnaud Pouliquen <arnaud.pouliquen@st.com>.
> */
>
> +#include <linux/dmaengine.h>
> +#include <linux/dma-mapping.h>
> #include <linux/interrupt.h>
> #include <linux/iio/buffer.h>
> #include <linux/iio/hw-consumer.h>
> #include <linux/iio/iio.h>
> #include <linux/iio/sysfs.h>
> +#include <linux/iio/trigger_consumer.h>
> +#include <linux/iio/triggered_buffer.h>
> #include <linux/module.h>
> -#include <linux/of.h>
> +#include <linux/of_device.h>
> #include <linux/platform_device.h>
> #include <linux/regmap.h>
> #include <linux/slab.h>
>
> #include "stm32-dfsdm.h"
>
> +#define DFSDM_DMA_BUFFER_SIZE (4 * PAGE_SIZE)
> +
> /* Conversion timeout */
> #define DFSDM_TIMEOUT_US 100000
> #define DFSDM_TIMEOUT (msecs_to_jiffies(DFSDM_TIMEOUT_US / 1000))
> @@ -58,6 +64,18 @@ struct stm32_dfsdm_adc {
> struct completion completion;
> u32 *buffer;
>
> + /* Audio specific */
> + unsigned int spi_freq; /* SPI bus clock frequency */
> + unsigned int sample_freq; /* Sample frequency after filter decimation */
> + int (*cb)(const void *data, size_t size, void *cb_priv);
> + void *cb_priv;
> +
> + /* DMA */
> + u8 *rx_buf;
> + unsigned int bufi; /* Buffer current position */
> + unsigned int buf_sz; /* Buffer size */
> + struct dma_chan *dma_chan;
> + dma_addr_t dma_buf;
> };
>
> struct stm32_dfsdm_str2field {
> @@ -351,10 +369,63 @@ int stm32_dfsdm_channel_parse_of(struct stm32_dfsdm *dfsdm,
> return 0;
> }
>
> +static ssize_t dfsdm_adc_audio_get_spiclk(struct iio_dev *indio_dev,
> + uintptr_t priv,
> + const struct iio_chan_spec *chan,
> + char *buf)
> +{
> + struct stm32_dfsdm_adc *adc = iio_priv(indio_dev);
> +
> + return snprintf(buf, PAGE_SIZE, "%d\n", adc->spi_freq);
> +}
> +
> +static ssize_t dfsdm_adc_audio_set_spiclk(struct iio_dev *indio_dev,
> + uintptr_t priv,
> + const struct iio_chan_spec *chan,
> + const char *buf, size_t len)
> +{
> + struct stm32_dfsdm_adc *adc = iio_priv(indio_dev);
> + struct stm32_dfsdm_filter *fl = &adc->dfsdm->fl_list[adc->fl_id];
> + struct stm32_dfsdm_channel *ch = &adc->dfsdm->ch_list[adc->ch_id];
> + unsigned int sample_freq = adc->sample_freq;
> + unsigned int spi_freq;
> + int ret;
> +
> + dev_err(&indio_dev->dev, "enter %s\n", __func__);
> + /* If DFSDM is master on SPI, SPI freq can not be updated */
> + if (ch->src != DFSDM_CHANNEL_SPI_CLOCK_EXTERNAL)
> + return -EPERM;
> +
> + ret = kstrtoint(buf, 0, &spi_freq);
> + if (ret)
> + return ret;
> +
> + if (!spi_freq)
> + return -EINVAL;
> +
> + if (sample_freq) {
> + if (spi_freq % sample_freq)
> + dev_warn(&indio_dev->dev,
> + "Sampling rate not accurate (%d)\n",
> + spi_freq / (spi_freq / sample_freq));
> +
> + ret = stm32_dfsdm_set_osrs(fl, 0, (spi_freq / sample_freq));
> + if (ret < 0) {
> + dev_err(&indio_dev->dev,
> + "No filter parameters that match!\n");
> + return ret;
> + }
> + }
> + adc->spi_freq = spi_freq;
> +
> + return len;
> +}
> +
> static int stm32_dfsdm_start_conv(struct stm32_dfsdm_adc *adc, bool dma)
> {
> struct regmap *regmap = adc->dfsdm->regmap;
> int ret;
> + unsigned int dma_en = 0, cont_en = 0;
>
> ret = stm32_dfsdm_start_channel(adc->dfsdm, adc->ch_id);
> if (ret < 0)
> @@ -365,6 +436,24 @@ static int stm32_dfsdm_start_conv(struct stm32_dfsdm_adc *adc, bool dma)
> if (ret < 0)
> goto stop_channels;
>
> + if (dma) {
> + /* Enable DMA transfer*/
> + dma_en = DFSDM_CR1_RDMAEN(1);
> + /* Enable conversion triggered by SPI clock*/
> + cont_en = DFSDM_CR1_RCONT(1);
> + }
> + /* Enable DMA transfer*/
> + ret = regmap_update_bits(regmap, DFSDM_CR1(adc->fl_id),
> + DFSDM_CR1_RDMAEN_MASK, dma_en);
> + if (ret < 0)
> + goto stop_channels;
> +
> + /* Enable conversion triggered by SPI clock*/
> + ret = regmap_update_bits(regmap, DFSDM_CR1(adc->fl_id),
> + DFSDM_CR1_RCONT_MASK, cont_en);
> + if (ret < 0)
> + goto stop_channels;
> +
> ret = stm32_dfsdm_start_filter(adc->dfsdm, adc->fl_id);
> if (ret < 0)
> goto stop_channels;
> @@ -398,6 +487,231 @@ static void stm32_dfsdm_stop_conv(struct stm32_dfsdm_adc *adc)
> stm32_dfsdm_stop_channel(adc->dfsdm, adc->ch_id);
> }
>
> +static int stm32_dfsdm_set_watermark(struct iio_dev *indio_dev,
> + unsigned int val)
> +{
> + struct stm32_dfsdm_adc *adc = iio_priv(indio_dev);
> + unsigned int watermark = DFSDM_DMA_BUFFER_SIZE / 2;
> +
> + /*
> + * DMA cyclic transfers are used, buffer is split into two periods.
> + * There should be :
> + * - always one buffer (period) DMA is working on
> + * - one buffer (period) driver pushed to ASoC side.
> + */
> + watermark = min(watermark, val * (unsigned int)(sizeof(u32)));
> + adc->buf_sz = watermark * 2;
> +
> + return 0;
> +}
> +
> +static unsigned int stm32_dfsdm_adc_dma_residue(struct stm32_dfsdm_adc *adc)
> +{
> + struct dma_tx_state state;
> + enum dma_status status;
> +
> + status = dmaengine_tx_status(adc->dma_chan,
> + adc->dma_chan->cookie,
> + &state);
> + if (status == DMA_IN_PROGRESS) {
> + /* Residue is size in bytes from end of buffer */
> + unsigned int i = adc->buf_sz - state.residue;
> + unsigned int size;
> +
> + /* Return available bytes */
> + if (i >= adc->bufi)
> + size = i - adc->bufi;
> + else
> + size = adc->buf_sz + i - adc->bufi;
> +
> + return size;
> + }
> +
> + return 0;
> +}
> +
> +static void stm32_dfsdm_audio_dma_buffer_done(void *data)
> +{
> + struct iio_dev *indio_dev = data;
> + struct stm32_dfsdm_adc *adc = iio_priv(indio_dev);
> + int available = stm32_dfsdm_adc_dma_residue(adc);
> + size_t old_pos;
> +
> + /*
> + * FIXME: In Kernel interface does not support cyclic DMA buffer,and
> + * offers only an interface to push data samples per samples.
> + * For this reason IIO buffer interface is not used and interface is
> + * bypassed using a private callback registered by ASoC.
> + * This should be a temporary solution waiting a cyclic DMA engine
> + * support in IIO.
> + */
> +
> + dev_dbg(&indio_dev->dev, "%s: pos = %d, available = %d\n", __func__,
> + adc->bufi, available);
> + old_pos = adc->bufi;
> +
> + while (available >= indio_dev->scan_bytes) {
> + u32 *buffer = (u32 *)&adc->rx_buf[adc->bufi];
> +
> + /* Mask 8 LSB that contains the channel ID */
> + *buffer = (*buffer & 0xFFFFFF00) << 8;
> + available -= indio_dev->scan_bytes;
> + adc->bufi += indio_dev->scan_bytes;
> + if (adc->bufi >= adc->buf_sz) {
> + if (adc->cb)
> + adc->cb(&adc->rx_buf[old_pos],
> + adc->buf_sz - old_pos, adc->cb_priv);
> + adc->bufi = 0;
> + old_pos = 0;
> + }
> + }
> + if (adc->cb)
> + adc->cb(&adc->rx_buf[old_pos], adc->bufi - old_pos,
> + adc->cb_priv);
> +}
> +
> +static int stm32_dfsdm_adc_dma_start(struct iio_dev *indio_dev)
> +{
> + struct stm32_dfsdm_adc *adc = iio_priv(indio_dev);
> + struct dma_async_tx_descriptor *desc;
> + dma_cookie_t cookie;
> + int ret;
> +
> + if (!adc->dma_chan)
> + return -EINVAL;
> +
> + dev_dbg(&indio_dev->dev, "%s size=%d watermark=%d\n", __func__,
> + adc->buf_sz, adc->buf_sz / 2);
> +
> + /* Prepare a DMA cyclic transaction */
> + desc = dmaengine_prep_dma_cyclic(adc->dma_chan,
> + adc->dma_buf,
> + adc->buf_sz, adc->buf_sz / 2,
> + DMA_DEV_TO_MEM,
> + DMA_PREP_INTERRUPT);
> + if (!desc)
> + return -EBUSY;
> +
> + desc->callback = stm32_dfsdm_audio_dma_buffer_done;
> + desc->callback_param = indio_dev;
> +
> + cookie = dmaengine_submit(desc);
> + ret = dma_submit_error(cookie);
> + if (ret) {
> + dmaengine_terminate_all(adc->dma_chan);
> + return ret;
> + }
> +
> + /* Issue pending DMA requests */
> + dma_async_issue_pending(adc->dma_chan);
> +
> + return 0;
> +}
> +
> +static int stm32_dfsdm_postenable(struct iio_dev *indio_dev)
> +{
> + struct stm32_dfsdm_adc *adc = iio_priv(indio_dev);
> + int ret;
> +
> + /* Reset adc buffer index */
> + adc->bufi = 0;
> +
> + ret = stm32_dfsdm_start_dfsdm(adc->dfsdm);
> + if (ret < 0)
> + return ret;
> +
> + ret = stm32_dfsdm_start_conv(adc, true);
> + if (ret) {
> + dev_err(&indio_dev->dev, "Can't start conversion\n");
> + goto stop_dfsdm;
> + }
> +
> + if (adc->dma_chan) {
> + ret = stm32_dfsdm_adc_dma_start(indio_dev);
> + if (ret) {
> + dev_err(&indio_dev->dev, "Can't start DMA\n");
> + goto err_stop_conv;
> + }
> + }
> +
> + return 0;
> +
> +err_stop_conv:
> + stm32_dfsdm_stop_conv(adc);
> +stop_dfsdm:
> + stm32_dfsdm_stop_dfsdm(adc->dfsdm);
> +
> + return ret;
> +}
> +
> +static int stm32_dfsdm_predisable(struct iio_dev *indio_dev)
> +{
> + struct stm32_dfsdm_adc *adc = iio_priv(indio_dev);
> +
> + if (adc->dma_chan)
> + dmaengine_terminate_all(adc->dma_chan);
> +
> + stm32_dfsdm_stop_conv(adc);
> +
> + stm32_dfsdm_stop_dfsdm(adc->dfsdm);
> +
> + return 0;
> +}
> +
> +static const struct iio_buffer_setup_ops stm32_dfsdm_buffer_setup_ops = {
> + .postenable = &stm32_dfsdm_postenable,
> + .predisable = &stm32_dfsdm_predisable,
> +};
> +
> +/**
> + * stm32_dfsdm_get_buff_cb() - register a callback that will be called when
> + * DMA transfer period is achieved.
> + *
> + * @iio_dev: Handle to IIO device.
> + * @cb: Pointer to callback function:
> + * - data: pointer to data buffer
> + * - size: size in byte of the data buffer
> + * - private: pointer to consumer private structure.
> + * @private: Pointer to consumer private structure.
> + */
> +int stm32_dfsdm_get_buff_cb(struct iio_dev *iio_dev,
> + int (*cb)(const void *data, size_t size,
> + void *private),
> + void *private)
> +{
> + struct stm32_dfsdm_adc *adc;
> +
> + if (!iio_dev)
> + return -EINVAL;
> + adc = iio_priv(iio_dev);
> +
> + adc->cb = cb;
> + adc->cb_priv = private;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(stm32_dfsdm_get_buff_cb);
> +
> +/**
> + * stm32_dfsdm_release_buff_cb - unregister buffer callback
> + *
> + * @iio_dev: Handle to IIO device.
> + */
> +int stm32_dfsdm_release_buff_cb(struct iio_dev *iio_dev)
> +{
> + struct stm32_dfsdm_adc *adc;
> +
> + if (!iio_dev)
> + return -EINVAL;
> + adc = iio_priv(iio_dev);
> +
> + adc->cb = NULL;
> + adc->cb_priv = NULL;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(stm32_dfsdm_release_buff_cb);
> +
> static int stm32_dfsdm_single_conv(struct iio_dev *indio_dev,
> const struct iio_chan_spec *chan, int *res)
> {
> @@ -453,15 +767,41 @@ static int stm32_dfsdm_write_raw(struct iio_dev *indio_dev,
> {
> struct stm32_dfsdm_adc *adc = iio_priv(indio_dev);
> struct stm32_dfsdm_filter *fl = &adc->dfsdm->fl_list[adc->fl_id];
> + struct stm32_dfsdm_channel *ch = &adc->dfsdm->ch_list[adc->ch_id];
> + unsigned int spi_freq = adc->spi_freq;
> int ret = -EINVAL;
>
> - if (mask == IIO_CHAN_INFO_OVERSAMPLING_RATIO) {
> + switch (mask) {
> + case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> ret = stm32_dfsdm_set_osrs(fl, 0, val);
> if (!ret)
> adc->oversamp = val;
> +
> + return ret;
> +
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + if (!val)
> + return -EINVAL;
> + if (ch->src != DFSDM_CHANNEL_SPI_CLOCK_EXTERNAL)
> + spi_freq = adc->dfsdm->spi_master_freq;
> +
> + if (spi_freq % val)
> + dev_warn(&indio_dev->dev,
> + "Sampling rate not accurate (%d)\n",
> + spi_freq / (spi_freq / val));
> +
> + ret = stm32_dfsdm_set_osrs(fl, 0, (spi_freq / val));
> + if (ret < 0) {
> + dev_err(&indio_dev->dev,
> + "Not able to find parameter that match!\n");
> + return ret;
> + }
> + adc->sample_freq = val;
> +
> + return 0;
> }
>
> - return ret;
> + return -EINVAL;
> }
>
> static int stm32_dfsdm_read_raw(struct iio_dev *indio_dev,
> @@ -494,11 +834,22 @@ static int stm32_dfsdm_read_raw(struct iio_dev *indio_dev,
> *val = adc->oversamp;
>
> return IIO_VAL_INT;
> +
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + *val = adc->sample_freq;
> +
> + return IIO_VAL_INT;
> }
>
> return -EINVAL;
> }
>
> +static const struct iio_info stm32_dfsdm_info_audio = {
> + .hwfifo_set_watermark = stm32_dfsdm_set_watermark,
> + .read_raw = stm32_dfsdm_read_raw,
> + .write_raw = stm32_dfsdm_write_raw,
> +};
> +
> static const struct iio_info stm32_dfsdm_info_adc = {
> .read_raw = stm32_dfsdm_read_raw,
> .write_raw = stm32_dfsdm_write_raw,
> @@ -531,6 +882,60 @@ static irqreturn_t stm32_dfsdm_irq(int irq, void *arg)
> return IRQ_HANDLED;
> }
>
> +/*
> + * Define external info for SPI Frequency and audio sampling rate that can be
> + * configured by ASoC driver through consumer.h API
> + */
> +static const struct iio_chan_spec_ext_info dfsdm_adc_audio_ext_info[] = {
> + /* spi_clk_freq : clock freq on SPI/manchester bus used by channel */
> + {
> + .name = "spi_clk_freq",
> + .shared = IIO_SHARED_BY_TYPE,
> + .read = dfsdm_adc_audio_get_spiclk,
> + .write = dfsdm_adc_audio_set_spiclk,
> + },
> + {},
> +};
> +
> +static int stm32_dfsdm_dma_request(struct iio_dev *indio_dev)
> +{
> + struct stm32_dfsdm_adc *adc = iio_priv(indio_dev);
> + struct dma_slave_config config;
> + int ret;
> +
> + adc->dma_chan = dma_request_slave_channel(&indio_dev->dev, "rx");
> + if (!adc->dma_chan)
> + return -EINVAL;
> +
> + adc->rx_buf = dma_alloc_coherent(adc->dma_chan->device->dev,
> + DFSDM_DMA_BUFFER_SIZE,
> + &adc->dma_buf, GFP_KERNEL);
> + if (!adc->rx_buf) {
> + ret = -ENOMEM;
> + goto err_release;
> + }
> +
> + /* Configure DMA channel to read data register */
> + memset(&config, 0, sizeof(config));
> + config.src_addr = (dma_addr_t)adc->dfsdm->phys_base;
> + config.src_addr += DFSDM_RDATAR(adc->fl_id);
> + config.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
> +
> + ret = dmaengine_slave_config(adc->dma_chan, &config);
> + if (ret)
> + goto err_free;
> +
> + return 0;
> +
> +err_free:
> + dma_free_coherent(adc->dma_chan->device->dev, DFSDM_DMA_BUFFER_SIZE,
> + adc->rx_buf, adc->dma_buf);
> +err_release:
> + dma_release_channel(adc->dma_chan);
> +
> + return ret;
> +}
> +
> static int stm32_dfsdm_adc_chan_init_one(struct iio_dev *indio_dev,
> struct iio_chan_spec *ch)
> {
> @@ -551,7 +956,12 @@ static int stm32_dfsdm_adc_chan_init_one(struct iio_dev *indio_dev,
> ch->info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
> ch->info_mask_shared_by_all = BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO);
>
> - ch->scan_type.sign = 'u';
> + if (adc->dev_data->type == DFSDM_AUDIO) {
> + ch->scan_type.sign = 's';
> + ch->ext_info = dfsdm_adc_audio_ext_info;
> + } else {
> + ch->scan_type.sign = 'u';
> + }
> ch->scan_type.realbits = 24;
> ch->scan_type.storagebits = 32;
> adc->ch_id = ch->channel;
> @@ -560,6 +970,64 @@ static int stm32_dfsdm_adc_chan_init_one(struct iio_dev *indio_dev,
> &adc->dfsdm->ch_list[ch->channel]);
> }
>
> +static int stm32_dfsdm_audio_init(struct iio_dev *indio_dev)
> +{
> + struct iio_chan_spec *ch;
> + struct stm32_dfsdm_adc *adc = iio_priv(indio_dev);
> + struct stm32_dfsdm_channel *d_ch;
> + int ret;
> +
> + ret = stm32_dfsdm_dma_request(indio_dev);
> + if (ret) {
> + dev_err(&indio_dev->dev, "DMA request failed\n");
> + return ret;
> + }
> +
> + indio_dev->modes |= INDIO_BUFFER_SOFTWARE;
> +
> + ret = iio_triggered_buffer_setup(indio_dev,
> + &iio_pollfunc_store_time,
> + NULL,
> + &stm32_dfsdm_buffer_setup_ops);
> + if (ret) {
> + dev_err(&indio_dev->dev, "Buffer setup failed\n");
> + goto err_dma_disable;
> + }
> +
> + ch = devm_kzalloc(&indio_dev->dev, sizeof(*ch), GFP_KERNEL);
> + if (!ch)
> + return -ENOMEM;
> +
> + ch->scan_index = 0;
> + ret = stm32_dfsdm_adc_chan_init_one(indio_dev, ch);
> + if (ret < 0) {
> + dev_err(&indio_dev->dev, "channels init failed\n");
> + goto err_buffer_cleanup;
> + }
> + ch->info_mask_separate = BIT(IIO_CHAN_INFO_SAMP_FREQ);
> +
> + d_ch = &adc->dfsdm->ch_list[adc->ch_id];
> + if (d_ch->src != DFSDM_CHANNEL_SPI_CLOCK_EXTERNAL)
> + adc->spi_freq = adc->dfsdm->spi_master_freq;
> +
> + indio_dev->num_channels = 1;
> + indio_dev->channels = ch;
> +
> + return 0;
> +
> +err_buffer_cleanup:
> + iio_triggered_buffer_cleanup(indio_dev);
> +
> +err_dma_disable:
> + if (adc->dma_chan) {
> + dma_free_coherent(adc->dma_chan->device->dev,
> + DFSDM_DMA_BUFFER_SIZE,
> + adc->rx_buf, adc->dma_buf);
> + dma_release_channel(adc->dma_chan);
> + }
> + return ret;
> +}
> +
> static int stm32_dfsdm_adc_init(struct iio_dev *indio_dev)
> {
> struct iio_chan_spec *ch;
> @@ -612,11 +1080,20 @@ static const struct stm32_dfsdm_dev_data stm32h7_dfsdm_adc_data = {
> .init = stm32_dfsdm_adc_init,
> };
>
> +static const struct stm32_dfsdm_dev_data stm32h7_dfsdm_audio_data = {
> + .type = DFSDM_AUDIO,
> + .init = stm32_dfsdm_audio_init,
> +};
> +
> static const struct of_device_id stm32_dfsdm_adc_match[] = {
> {
> .compatible = "st,stm32-dfsdm-adc",
> .data = &stm32h7_dfsdm_adc_data,
> },
> + {
> + .compatible = "st,stm32-dfsdm-dmic",
> + .data = &stm32h7_dfsdm_audio_data,
> + },
> {}
> };
>
> @@ -667,8 +1144,13 @@ static int stm32_dfsdm_adc_probe(struct platform_device *pdev)
> name = devm_kzalloc(dev, sizeof("dfsdm-adc0"), GFP_KERNEL);
> if (!name)
> return -ENOMEM;
> - iio->info = &stm32_dfsdm_info_adc;
> - snprintf(name, sizeof("dfsdm-adc0"), "dfsdm-adc%d", adc->fl_id);
> + if (dev_data->type == DFSDM_AUDIO) {
> + iio->info = &stm32_dfsdm_info_audio;
> + snprintf(name, sizeof("dfsdm-pdm0"), "dfsdm-pdm%d", adc->fl_id);
> + } else {
> + iio->info = &stm32_dfsdm_info_adc;
> + snprintf(name, sizeof("dfsdm-adc0"), "dfsdm-adc%d", adc->fl_id);
> + }
> iio->name = name;
>
> /*
> @@ -700,7 +1182,10 @@ static int stm32_dfsdm_adc_probe(struct platform_device *pdev)
> if (ret < 0)
> return ret;
>
> - return iio_device_register(iio);
> + iio_device_register(iio);
> + if (dev_data->type == DFSDM_AUDIO)
> + return devm_of_platform_populate(&pdev->dev);
> + return 0;
> }
>
> static int stm32_dfsdm_adc_remove(struct platform_device *pdev)
> @@ -709,7 +1194,14 @@ static int stm32_dfsdm_adc_remove(struct platform_device *pdev)
> struct iio_dev *indio_dev = iio_priv_to_dev(adc);
>
> iio_device_unregister(indio_dev);
> -
> + if (indio_dev->pollfunc)
> + iio_triggered_buffer_cleanup(indio_dev);
> + if (adc->dma_chan) {
> + dma_free_coherent(adc->dma_chan->device->dev,
> + DFSDM_DMA_BUFFER_SIZE,
> + adc->rx_buf, adc->dma_buf);
> + dma_release_channel(adc->dma_chan);
> + }
> return 0;
> }
>
> diff --git a/include/linux/iio/adc/stm32-dfsdm-adc.h b/include/linux/iio/adc/stm32-dfsdm-adc.h
> new file mode 100644
> index 0000000..e7dc7a5
> --- /dev/null
> +++ b/include/linux/iio/adc/stm32-dfsdm-adc.h
> @@ -0,0 +1,18 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * This file discribe the STM32 DFSDM IIO driver API for audio part
> + *
> + * Copyright (C) 2017, STMicroelectronics - All Rights Reserved
> + * Author(s): Arnaud Pouliquen <arnaud.pouliquen@st.com>.
> + */
> +
> +#ifndef STM32_DFSDM_ADC_H
> +#define STM32_DFSDM_ADC_H
> +
> +int stm32_dfsdm_get_buff_cb(struct iio_dev *iio_dev,
> + int (*cb)(const void *data, size_t size,
> + void *private),
> + void *private);
> +int stm32_dfsdm_release_buff_cb(struct iio_dev *iio_dev);
> +
> +#endif
^ permalink raw reply
* [PATCH 4/4] [v4] pinctrl: qcom: qdf2xxx: add support for new ACPI HID QCOM8002
From: Timur Tabi @ 2017-12-12 20:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1513076836.25007.641.camel@linux.intel.com>
On 12/12/2017 05:07 AM, Andy Shevchenko wrote:
> Not ACPI standards as of my knowledge. ACPI standard defines a common
> scheme how to define properties, it doesn't tell anything about property
> names or any mappings between names to values or names to "OS
> subsystem").
There was an attempt a while back to standardize this like we do for
device tree, but it fell apart. Device-specific ACPI-only properties
are not standarized. This driver is initialized only on ACPI systems.
It has no device tree binding.
> As for GPIO we just follow *de facto* what DT has right now, i.e. "xxx-
> gpio" or "xxx-gpios" pattern is used to map ACPI standard resource to a
> GPIO name. That's how GPIO ACPI lib is being developed.
GPIOs in device tree are defined completely differently than in ACPI.
On DT, the kernel controls the pin muxing. On ACPI, pins are muxed by
firmware and never re-muxed by the operating system. So all this driver
does is expose a few pins as simple GPIOs.
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply
* [PATCH 4/4] [v4] pinctrl: qcom: qdf2xxx: add support for new ACPI HID QCOM8002
From: Timur Tabi @ 2017-12-12 20:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1513073105.25007.618.camel@linux.intel.com>
On 12/12/2017 04:05 AM, Andy Shevchenko wrote:
>> +static const struct acpi_device_id qdf2xxx_acpi_ids[] = {
>> + {"QCOM8001", QDF2XXX_V1},
>> + {"QCOM8002", QDF2XXX_V2},
>> + {},
>> +};
>> +MODULE_DEVICE_TABLE(acpi, qdf2xxx_acpi_ids);
>>
>> + const struct acpi_device_id *id =
>> + acpi_match_device(qdf2xxx_acpi_ids, &pdev->dev);
> JFYI: there is no need to move IDs like this.
> Use members of struct device_driver wisely.
I have to move it, otherwise I get:
drivers/pinctrl/qcom/pinctrl-qdf2xxx.c:49:21: error: 'qdf2xxx_acpi_ids'
undeclared (first use in this function); did you mean 'qdf2xxx_pinctrl'?
I reference the structure in qdf2xxx_pinctrl_probe().
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply
* [PATCH 2/4] [v2] gpiolib: add bitmask for valid GPIO lines
From: Timur Tabi @ 2017-12-12 20:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1513072725.25007.614.camel@linux.intel.com>
On 12/12/2017 03:58 AM, Andy Shevchenko wrote:
> On Fri, 2017-12-01 at 17:28 -0600, Timur Tabi wrote:
>> Add support for specifying that some GPIOs within a range are
>> unavailable.
>> Some systems have a sparse list of GPIOs, where a range of GPIOs is
>> specified (usually 0 to n-1), but some subset within that range is
>> absent or unavailable for whatever reason.
>>
>> To support this, allow drivers to specify a bitmask of GPIOs that
>> are present or absent. Gpiolib will then block access to those that
>> are absent.
>
>> - status = gpiochip_irqchip_init_valid_mask(chip);
>> + status = gpiochip_init_valid_mask(chip);
>> if (status)
>> goto err_remove_from_list;
>>
>> + status = gpiochip_irqchip_init_valid_mask(chip);
>> + if (status)
>> + goto err_remove_valid_mask;
>
> Yes, this way it looks good!
I've discovered that I can remove all this code. I don't need a valid
mask, all I need to do is block the request properly.
>> +static bool gpiochip_available(const struct gpio_chip *gpiochip,
>> + unsigned int offset)
>> +{
>
>> + pr_info("%s:%u offset=%u\n", __func__, __LINE__, offset);
>
> Debug leftover?
Fixed, thanks.
>
>> +
>> + /* No mask means all valid */
>> + if (likely(!gpiochip->valid_mask))
>> + return true;
>> +
>> + return test_bit(offset, gpiochip->valid_mask);
>
> Not sure which one is better
> return test_bit();
> or
> return !!test_bit();
I've removed this function also.
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply
* [PATCH 5/8] power: supply: axp20x_battery: add support for AXP813
From: Jonathan Cameron @ 2017-12-12 19:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <873410df-046e-bd86-584d-0d35b9d8c993@free-electrons.com>
On Mon, 11 Dec 2017 09:35:43 +0100
Quentin Schulz <quentin.schulz@free-electrons.com> wrote:
> Hi Jonathan,
>
> On 10/12/2017 17:49, Jonathan Cameron wrote:
> > On Mon, 4 Dec 2017 15:12:51 +0100
> > Quentin Schulz <quentin.schulz@free-electrons.com> wrote:
> >
> >> The X-Powers AXP813 PMIC has got some slight differences from
> >> AXP20X/AXP22X PMICs:
> >> - the maximum voltage supplied by the PMIC is 4.35 instead of 4.36/4.24
> >> for AXP20X/AXP22X,
> >> - the constant charge current formula is different,
> >>
> >> It also has a bit to tell whether the battery percentage returned by the
> >> PMIC is valid.
> >>
> >> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
> >
> > I'd use switch statements when matching the IDs as that'll be more elegant
> > as you perhaps add further devices going forward...
> >
> > Other than that, looks good to me.
> >
>
> Well, I was wondering if it shouldn't be better to define a structure
> for each device containing their quirks, functions, etc... like it is
> done for the ADC or the ACIN power supply driver part.
>
Even better.
> struct axp20x_data {
> bool has_valid_fg_reg;
> void constant_charge_current_to_raw(struct axp20x_batt_ps *axp, int *val);
> void raw_to_constant_charge_current(struct axp20x_batt_ps *axp, int *val);
> int get_max_voltage(struct axp20x_batt_ps *axp, int *val);
> [...]
> };
>
> static const struct of_device_id axp20x_battery_ps_id[] = {
> { .compatible = "x-powers,axp209-battery-power-supply", .data = (void
> *)&axp209_data, }, {}
> };
>
> void probe()
> {
> [...]
> axp20x_batt->info = of_device_get_match_data(&pdev->dev);
> [...]
> }
>
> Sebastian, any objection on doing this?
>
> Thanks,
> Quentin
>
> > Jonathan
> >
> >> ---
> >> drivers/power/supply/axp20x_battery.c | 44 +++++++++++++++++++++++++++-
> >> 1 file changed, 43 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/power/supply/axp20x_battery.c b/drivers/power/supply/axp20x_battery.c
> >> index 7494f0f..cb30302 100644
> >> --- a/drivers/power/supply/axp20x_battery.c
> >> +++ b/drivers/power/supply/axp20x_battery.c
> >> @@ -46,6 +46,8 @@
> >> #define AXP20X_CHRG_CTRL1_TGT_4_2V (2 << 5)
> >> #define AXP20X_CHRG_CTRL1_TGT_4_36V (3 << 5)
> >>
> >> +#define AXP813_CHRG_CTRL1_TGT_4_35V (3 << 5)
> >> +
> >> #define AXP22X_CHRG_CTRL1_TGT_4_22V (1 << 5)
> >> #define AXP22X_CHRG_CTRL1_TGT_4_24V (3 << 5)
> >>
> >> @@ -123,10 +125,41 @@ static int axp22x_battery_get_max_voltage(struct axp20x_batt_ps *axp20x_batt,
> >> return 0;
> >> }
> >>
> >> +static int axp813_battery_get_max_voltage(struct axp20x_batt_ps *axp20x_batt,
> >> + int *val)
> >> +{
> >> + int ret, reg;
> >> +
> >> + ret = regmap_read(axp20x_batt->regmap, AXP20X_CHRG_CTRL1, ®);
> >> + if (ret)
> >> + return ret;
> >> +
> >> + switch (reg & AXP20X_CHRG_CTRL1_TGT_VOLT) {
> >
> > You could do a lookup based from a table instead which might
> > be ever so slightly more elegant..
> >
> >> + case AXP20X_CHRG_CTRL1_TGT_4_1V:
> >> + *val = 4100000;
> >> + break;
> >> + case AXP20X_CHRG_CTRL1_TGT_4_15V:
> >> + *val = 4150000;
> >> + break;
> >> + case AXP20X_CHRG_CTRL1_TGT_4_2V:
> >> + *val = 4200000;
> >> + break;
> >> + case AXP813_CHRG_CTRL1_TGT_4_35V:
> >> + *val = 4350000;
> >> + break;
> >> + default:
> >> + return -EINVAL;
> >> + }
> >> +
> >> + return 0;
> >> +}
> >> +
> >> static void raw_to_constant_charge_current(struct axp20x_batt_ps *axp, int *val)
> >> {
> >> if (axp->axp_id == AXP209_ID)
> >> *val = *val * 100000 + 300000;
> >> + else if (axp->axp_id == AXP813_ID)
> >> + *val = *val * 200000 + 200000;
> >> else
> >> *val = *val * 150000 + 300000;
> >
> > Switch?
> >
> >> }
> >> @@ -135,6 +168,8 @@ static void constant_charge_current_to_raw(struct axp20x_batt_ps *axp, int *val)
> >> {
> >> if (axp->axp_id == AXP209_ID)
> >> *val = (*val - 300000) / 100000;
> >> + else if (axp->axp_id == AXP813_ID)
> >> + *val = (*val - 200000) / 200000;
> >> else
> >> *val = (*val - 300000) / 150000;
> >> }
> >> @@ -269,7 +304,8 @@ static int axp20x_battery_get_prop(struct power_supply *psy,
> >> if (ret)
> >> return ret;
> >>
> >> - if (axp20x_batt->axp_id == AXP221_ID &&
> >> + if ((axp20x_batt->axp_id == AXP221_ID ||
> >> + axp20x_batt->axp_id == AXP813_ID) &&
> >> !(reg & AXP22X_FG_VALID))
> >> return -EINVAL;
> >>
> >> @@ -284,6 +320,9 @@ static int axp20x_battery_get_prop(struct power_supply *psy,
> >> if (axp20x_batt->axp_id == AXP209_ID)
> >> return axp20x_battery_get_max_voltage(axp20x_batt,
> >> &val->intval);
> >> + else if (axp20x_batt->axp_id == AXP813_ID)
> >> + return axp813_battery_get_max_voltage(axp20x_batt,
> >> + &val->intval);
> >> return axp22x_battery_get_max_voltage(axp20x_batt,
> >> &val->intval);
> >
> > Worth converting to a switch statement to make it more elegant for future
> > devices?
> >
> >>
> >> @@ -467,6 +506,9 @@ static const struct of_device_id axp20x_battery_ps_id[] = {
> >> }, {
> >> .compatible = "x-powers,axp221-battery-power-supply",
> >> .data = (void *)AXP221_ID,
> >> + }, {
> >> + .compatible = "x-powers,axp813-battery-power-supply",
> >> + .data = (void *)AXP813_ID,
> >> }, { /* sentinel */ },
> >> };
> >> MODULE_DEVICE_TABLE(of, axp20x_battery_ps_id);
> >
>
^ permalink raw reply
* WARNING: suspicious RCU usage
From: Russell King - ARM Linux @ 2017-12-12 19:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAOMZO5Brd7pZtF5e8RLu63MpUo1Sc01vWC1WeccEMTkGBL0Rqw@mail.gmail.com>
On Tue, Dec 12, 2017 at 05:44:07PM -0200, Fabio Estevam wrote:
> Hi Paul,
>
> On Tue, Dec 12, 2017 at 5:36 PM, Paul E. McKenney
> <paulmck@linux.vnet.ibm.com> wrote:
>
> >> Ok, just tested with CONFIG_BL_SWITCHER=y on a imx6q-cubox-i:
> >
> > Just to confirm, your dmesg below is illustrating the hang, correct?
>
> Sorry for not being clear. Let me clarify my tests.
>
> If I run a mainline kernel on a imx6q I do see the exact same RCU
> warning as reported by Peng Fan in this thread.
>
> This is 100% reproducible: the first time I do a suspend/resume after
> boot the RCU warning is present. Subsequent suspend/resume cycles do
> not show the warning.
>
> With your patch applied I don't see the RCU warning anymore.
>
> I originally tested the standard imx_v6_v7_defconfig and also a kernel
> with CONFIG_BL_SWITCHER=y as suggested by Russell.
>
> In my tests even with CONFIG_BL_SWITCHER=y suspend/resume works fine
> and no more RCU warnings were seen.
>
> The dmesg I shared shows the normal output without the RCU warning.
Which is exactly what I would expect with imx6. Just because it
works for imx6 does not mean it works for everyone.
I need to get Will Deacon's permission before I can send an email
containing our discussion on the points here from 2013, and that
probably won't happen until tomorrow - and I'm having to do that
because none of you seem to be listening to what I'm saying wrt
that spinlock.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
^ permalink raw reply
* [PATCH net-next v5 2/2] net: thunderx: add timestamping support
From: Joe Perches @ 2017-12-12 19:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171211233641.cs7zdw34qkngicmj@localhost>
On Mon, 2017-12-11 at 15:36 -0800, Richard Cochran wrote:
> On Mon, Dec 11, 2017 at 05:14:31PM +0300, Aleksey Makarov wrote:
> > @@ -880,6 +889,46 @@ static void nic_pause_frame(struct nicpf *nic, int vf, struct pfc *cfg)
> > }
> > }
> >
> > +/* Enable or disable HW timestamping by BGX for pkts received on a LMAC */
> > +static void nic_config_timestamp(struct nicpf *nic, int vf, struct set_ptp *ptp)
> > +{
> > + struct pkind_cfg *pkind;
> > + u8 lmac, bgx_idx;
> > + u64 pkind_val, pkind_idx;
> > +
> > + if (vf >= nic->num_vf_en)
> > + return;
> > +
> > + bgx_idx = NIC_GET_BGX_FROM_VF_LMAC_MAP(nic->vf_lmac_map[vf]);
> > + lmac = NIC_GET_LMAC_FROM_VF_LMAC_MAP(nic->vf_lmac_map[vf]);
> > +
> > + pkind_idx = lmac + bgx_idx * MAX_LMAC_PER_BGX;
> > + pkind_val = nic_reg_read(nic, NIC_PF_PKIND_0_15_CFG | (pkind_idx << 3));
> > + pkind = (struct pkind_cfg *)&pkind_val;
> > +
> > + if (ptp->enable && !pkind->hdr_sl) {
> > + /* Skiplen to exclude 8byte timestamp while parsing pkt
> > + * If not configured, will result in L2 errors.
> > + */
> > + pkind->hdr_sl = 4;
> > + /* Adjust max packet length allowed */
> > + pkind->maxlen += (pkind->hdr_sl * 2);
Are all compilers smart enough to set this to 8?
I rather doubt a compiler is even allowed to.
^ permalink raw reply
* WARNING: suspicious RCU usage
From: Fabio Estevam @ 2017-12-12 19:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171212193638.GH7829@linux.vnet.ibm.com>
Hi Paul,
On Tue, Dec 12, 2017 at 5:36 PM, Paul E. McKenney
<paulmck@linux.vnet.ibm.com> wrote:
>> Ok, just tested with CONFIG_BL_SWITCHER=y on a imx6q-cubox-i:
>
> Just to confirm, your dmesg below is illustrating the hang, correct?
Sorry for not being clear. Let me clarify my tests.
If I run a mainline kernel on a imx6q I do see the exact same RCU
warning as reported by Peng Fan in this thread.
This is 100% reproducible: the first time I do a suspend/resume after
boot the RCU warning is present. Subsequent suspend/resume cycles do
not show the warning.
With your patch applied I don't see the RCU warning anymore.
I originally tested the standard imx_v6_v7_defconfig and also a kernel
with CONFIG_BL_SWITCHER=y as suggested by Russell.
In my tests even with CONFIG_BL_SWITCHER=y suspend/resume works fine
and no more RCU warnings were seen.
The dmesg I shared shows the normal output without the RCU warning.
Thanks
^ permalink raw reply
* WARNING: suspicious RCU usage
From: Paul E. McKenney @ 2017-12-12 19:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAOMZO5DBMpCHfgmNND6=qJD+sxOLMPFqEsyz+GiBVhMzN3H+Zg@mail.gmail.com>
On Tue, Dec 12, 2017 at 04:11:07PM -0200, Fabio Estevam wrote:
> Hi Russell,
>
> On Tue, Dec 12, 2017 at 3:34 PM, Russell King - ARM Linux
> <linux@armlinux.org.uk> wrote:
>
> > It's fundamentally unsafe.
> >
> > You need to test with CONFIG_BL_SWITCHER enabled - there's spinlocks
> > in smp_call_function_single() path that are conditional on that symbol.
> > If CONFIG_BL_SWITCHER is disabled, then the spinlocks are not present.
>
> Ok, just tested with CONFIG_BL_SWITCHER=y on a imx6q-cubox-i:
Just to confirm, your dmesg below is illustrating the hang, correct?
Thanx, Paul
> # echo enabled > /sys/class/tty/ttymxc0/power/wakeup
> # echo mem > /sys/power/state
> [ 10.503462] PM: suspend entry (deep)
> [ 10.507479] PM: Syncing filesystems ... done.
> [ 10.555024] Freezing user space processes ... (elapsed 0.002 seconds) done.
> [ 10.564511] OOM killer disabled.
> [ 10.567760] Freezing remaining freezable tasks ... (elapsed 0.002 seconds) d.
> [ 10.577420] Suspending console(s) (use no_console_suspend to debug)
> [ 10.657748] PM: suspend devices took 0.080 seconds
> [ 10.669329] Disabling non-boot CPUs ...
> [ 10.717049] IRQ17 no longer affine to CPU1
> [ 10.837141] Enabling non-boot CPUs ...
> [ 10.839386] CPU1 is up
> [ 10.840342] CPU2 is up
> [ 10.841300] CPU3 is up
> [ 11.113735] mmc0: queuing unknown CIS tuple 0x80 (2 bytes)
> [ 11.115676] mmc0: queuing unknown CIS tuple 0x80 (3 bytes)
> [ 11.117595] mmc0: queuing unknown CIS tuple 0x80 (3 bytes)
> [ 11.121014] mmc0: queuing unknown CIS tuple 0x80 (7 bytes)
> [ 11.124454] mmc0: queuing unknown CIS tuple 0x80 (7 bytes)
> [ 11.177299] ata1: SATA link down (SStatus 0 SControl 300)
> [ 11.181930] PM: resume devices took 0.330 seconds
> [ 11.243729] OOM killer enabled.
> [ 11.246886] Restarting tasks ... done.
> [ 11.253012] PM: suspend exit
>
^ permalink raw reply
* [PATCH v5 15/30] arm64/sve: Signal handling support
From: Kees Cook @ 2017-12-12 19:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171212111125.GL22781@e103592.cambridge.arm.com>
On Tue, Dec 12, 2017 at 3:11 AM, Dave Martin <Dave.Martin@arm.com> wrote:
> On Tue, Dec 12, 2017 at 10:40:30AM +0000, Will Deacon wrote:
>> On Mon, Dec 11, 2017 at 11:23:09AM -0800, Kees Cook wrote:
>> > On Mon, Dec 11, 2017 at 6:07 AM, Will Deacon <will.deacon@arm.com> wrote:
>> > > On Thu, Dec 07, 2017 at 10:50:38AM -0800, Kees Cook wrote:
>> > >> My question is mainly: why not just use copy_*() everywhere instead?
>> > >> Having these things so spread out makes it fragile, and there's very
>> > >> little performance benefit from using __copy_*() over copy_*().
>> > >
>> > > I think that's more of a general question. Why not just remove the __
>> > > versions from the kernel entirely if they're not worth the perf?
>> >
>> > That has been something Linus has strongly suggested in the past, so
>> > I've kind of been looking for easy places to drop the __copy_*
>> > versions. :)
>>
>> Tell you what then: I'll Ack the arm64 patch if it's part of a series
>> removing the thing entirely :p
>>
>> I guess we'd still want to the validation of the whole sigframe though,
>> so we don't end up pushing half a signal stack before running into an
>> access_ok failure?
>
> That's an interesting question. In many cases access_ok() might become
> redundant, but for syscalls that you don't want to have side-effects
> on user memory on failure it's still relevant.
>
> In the signal case we'd still an encompassing access_ok() to prevent
> stack guard overruns, because the signal frame can be large and isn't
> written or read contiguously or in a well-defined order.
Yeah, I think bailing early is fine. I think the existing access_ok()
checks are fine; I wouldn't want to drop those.
-Kees
--
Kees Cook
Pixel Security
^ permalink raw reply
* [PATCH] ARM: CPU hotplug: Delegate complete() to surviving CPU
From: Paul E. McKenney @ 2017-12-12 19:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171212173759.GE10595@n2100.armlinux.org.uk>
On Tue, Dec 12, 2017 at 05:37:59PM +0000, Russell King - ARM Linux wrote:
> On Tue, Dec 12, 2017 at 09:20:59AM -0800, Paul E. McKenney wrote:
> > The ARM implementation of arch_cpu_idle_dead() invokes complete(), but
> > does so after RCU has stopped watching the outgoing CPU, which results
> > in lockdep complaints because complete() invokes functions containing RCU
> > readers. This patch therefore uses Thomas Gleixner's trick of delegating
> > the complete() call to a surviving CPU via smp_call_function_single().
> >
> > Reported-by: Peng Fan <van.freenix@gmail.com>
> > Reported-by: Russell King - ARM Linux <linux@armlinux.org.uk>
> > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > Tested-by: Tested-by: Fabio Estevam <fabio.estevam@nxp.com>
> > Cc: Russell King <linux@armlinux.org.uk>
> > Cc: Ingo Molnar <mingo@kernel.org>
> > Cc: "Peter Zijlstra (Intel)" <peterz@infradead.org>
> > Cc: Michal Hocko <mhocko@suse.com>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: <linux-arm-kernel@lists.infradead.org>
>
> As I just described in response to Fabio's testing, this doesn't solve
> anything if CONFIG_BL_SWITCHER is enabled. We could lose the unlock of
> a spinlock in the GIC code for sending the IPI. As I already said
> previously in our discussion (but I guess you just don't believe me):
Sorry, Russell, but most days I don't even believe myself. So it is
nothing personal, just one of the occupational hazards of being me.
> "2. there's some optional locking in the GIC driver that cause problems
> for the cpu dying path.
>
> The concensus last time around was that the IPI solution is a non-
> starter, so the seven year proven-reliable solution (disregarding the
> RCU warning) persists because I don't think anyone came up with a
> better solution."
>
> Using smp_call_function_single() invokes the IPI paths.
OK, another approach is to have the dying CPU simply set an in-memory
flag, which a surviving CPU polls for. There are of course any number
of ways of doing the polling loop.
So what bad thing happens when you use that approach?
Thanx, Paul
^ permalink raw reply
* [PATCH] ARM: CPU hotplug: Delegate complete() to surviving CPU
From: Paul E. McKenney @ 2017-12-12 19:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171212174046.gn4t3qeck3bq7scw@tarshish>
On Tue, Dec 12, 2017 at 07:40:46PM +0200, Baruch Siach wrote:
> Hi Paul,
>
> On Tue, Dec 12, 2017 at 09:20:59AM -0800, Paul E. McKenney wrote:
> > The ARM implementation of arch_cpu_idle_dead() invokes complete(), but
> > does so after RCU has stopped watching the outgoing CPU, which results
> > in lockdep complaints because complete() invokes functions containing RCU
> > readers. This patch therefore uses Thomas Gleixner's trick of delegating
> > the complete() call to a surviving CPU via smp_call_function_single().
> >
> > Reported-by: Peng Fan <van.freenix@gmail.com>
> > Reported-by: Russell King - ARM Linux <linux@armlinux.org.uk>
> > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > Tested-by: Tested-by: Fabio Estevam <fabio.estevam@nxp.com>
>
> Fabio reported only once, though he might have tested twice.
Actually, he did test twice. Apparently this patch has problems
with CONFIG_BL_SWITCHER=y kernels.
But yes, I guess I did get a bit carried away with the Tested-by's,
didn't I? ;-)
Thanx, Paul
> baruch
>
> > Cc: Russell King <linux@armlinux.org.uk>
> > Cc: Ingo Molnar <mingo@kernel.org>
> > Cc: "Peter Zijlstra (Intel)" <peterz@infradead.org>
> > Cc: Michal Hocko <mhocko@suse.com>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: <linux-arm-kernel@lists.infradead.org>
> >
> > diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
> > index b4fbf00ee4ad..75f85e20aafa 100644
> > --- a/arch/arm/kernel/smp.c
> > +++ b/arch/arm/kernel/smp.c
> > @@ -267,6 +267,14 @@ void __cpu_die(unsigned int cpu)
> > }
> >
> > /*
> > + * Invoke complete() on behalf of the outgoing CPU.
> > + */
> > +static void arch_cpu_idle_dead_complete(void *arg)
> > +{
> > + complete(&cpu_died);
> > +}
> > +
> > +/*
> > * Called from the idle thread for the CPU which has been shutdown.
> > *
> > * Note that we disable IRQs here, but do not re-enable them
> > @@ -293,9 +301,11 @@ void arch_cpu_idle_dead(void)
> > /*
> > * Tell __cpu_die() that this CPU is now safe to dispose of. Once
> > * this returns, power and/or clocks can be removed at any point
> > - * from this CPU and its cache by platform_cpu_kill().
> > + * from this CPU and its cache by platform_cpu_kill(). We cannot
> > + * call complete() this late, so we delegate it to an online CPU.
> > */
> > - complete(&cpu_died);
> > + smp_call_function_single(cpumask_first(cpu_online_mask),
> > + arch_cpu_idle_dead_complete, NULL, 0);
> >
> > /*
> > * Ensure that the cache lines associated with that completion are
>
> --
> http://baruch.siach.name/blog/ ~. .~ Tk Open Systems
> =}------------------------------------------------ooO--U--Ooo------------{=
> - baruch at tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -
>
^ permalink raw reply
* [PATCH v2] PCI: keystone: fix interrupt-controller-node lookup
From: Johan Hovold @ 2017-12-12 19:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171212180731.GA22805@red-moon>
On Tue, Dec 12, 2017 at 06:07:31PM +0000, Lorenzo Pieralisi wrote:
> On Tue, Dec 12, 2017 at 11:25:37AM -0600, Bjorn Helgaas wrote:
> > On Mon, Dec 11, 2017 at 10:42:33AM +0000, Lorenzo Pieralisi wrote:
> > > On Mon, Dec 11, 2017 at 11:29:55AM +0100, Johan Hovold wrote:
> > > > On Fri, Nov 17, 2017 at 02:38:31PM +0100, Johan Hovold wrote:
> > > > > Fix child-node lookup during initialisation which was using the wrong
> > > > > OF-helper and ended up searching the whole device tree depth-first
> > > > > starting at the parent rather than just matching on its children.
> > > > >
> > > > > To make things worse, the parent pci node could end up being prematurely
> > > > > freed as of_find_node_by_name() drops a reference to its first argument.
> > > > > Any matching child interrupt-controller node was also leaked.
> > > > >
> > > > > Fixes: 0c4ffcfe1fbc ("PCI: keystone: Add TI Keystone PCIe driver")
> > > > > Cc: stable <stable@vger.kernel.org> # 3.18
> > > > > Acked-by: Murali Karicheri <m-karicheri2@ti.com>
> > > > > Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > > > > Signed-off-by: Johan Hovold <johan@kernel.org>
> > > > > ---
> > > > >
> > > > > v2
> > > > > - amend commit message and mention explicitly that of_find_node_by_name()
> > > > > drops a reference to the start node
> > > > > - add Murali's and Lorenzo's acks
> > > >
> > > > This one hasn't shown up in linux-next, so sending a reminder to make
> > > > sure it doesn't fall between the cracks.
> > >
> > > Hi Johan,
> > >
> > > yes it is in the list of fixes to be sent upstream - I was about to
> > > ask Bjorn to apply it.
> >
> > Is this something that needs to be merged for v4.15? If so, I need to
> > be able to defend it to Linus as being a critical fix. If the issue
> > been around for 3 years (v3.18 was tagged Dec 7 2014), that requires
> > pretty "clear and present danger."
> >
> > From the commit log, I see a sub-optimal search (not critical), a
> > possible use-after-free (could conceivably be critical if people are
> > tripping over this, but would need more specifics about that), and a
> > leak (not critical).
> >
> > Given what I can see now, my inclination would be for Lorenzo to queue
> > it for v4.16, which would still get in linux-next soonish.
>
> It is fine by me and I think, as already mentioned, that the stable
> tag is dubious so I will probably drop it.
The unbalanced put can indeed cause serious problems, for example, after
probe deferrals. Crashes after probe deferrals has been reported for
other drivers with the same type of bug, and I have reproduced it
locally (using yet another driver).
I'm also fine with holding this one off for 4.16 (as we're at -rc3), but
I do think the stable tag is still warranted.
Johan
^ 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