* [PATCH 3/3] ARM: dts: da850: add usb device node
From: Axel Haslam @ 2016-11-08 18:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108185831.17683-1-ahaslam@baylibre.com>
This adds the ohci device node for the da850 soc.
It also enables it for the omapl138 hawk board.
Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
---
arch/arm/boot/dts/da850-lcdk.dts | 8 ++++++++
arch/arm/boot/dts/da850.dtsi | 8 ++++++++
2 files changed, 16 insertions(+)
diff --git a/arch/arm/boot/dts/da850-lcdk.dts b/arch/arm/boot/dts/da850-lcdk.dts
index 7b8ab21..aaf533e 100644
--- a/arch/arm/boot/dts/da850-lcdk.dts
+++ b/arch/arm/boot/dts/da850-lcdk.dts
@@ -86,6 +86,14 @@
};
};
+&usb_phy {
+ status = "okay";
+};
+
+&ohci {
+ status = "okay";
+};
+
&serial2 {
pinctrl-names = "default";
pinctrl-0 = <&serial2_rxtx_pins>;
diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
index 2534aab..c74f1a6 100644
--- a/arch/arm/boot/dts/da850.dtsi
+++ b/arch/arm/boot/dts/da850.dtsi
@@ -405,6 +405,14 @@
>;
status = "disabled";
};
+ ohci: usb at 225000 {
+ compatible = "ti,da830-ohci";
+ reg = <0x225000 0x1000>;
+ interrupts = <59>;
+ phys = <&usb_phy 1>;
+ phy-names = "usb-phy";
+ status = "disabled";
+ };
gpio: gpio at 226000 {
compatible = "ti,dm6441-gpio";
gpio-controller;
--
2.10.1
^ permalink raw reply related
* [PATCH 3/4] fpga mgr: zynq: Add support for encrypted bitstreams
From: Moritz Fischer @ 2016-11-08 18:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108183217.GV14444@xsjsorenbubuntu>
Hi S?ren,
On Tue, Nov 8, 2016 at 10:32 AM, S?ren Brinkmann
<soren.brinkmann@xilinx.com> wrote:
> On Sun, 2016-11-06 at 17:13:25 -0700, Moritz Fischer wrote:
>> Add new flag FPGA_MGR_DECRYPT_BISTREAM as well as a matching
>> capability FPGA_MGR_CAP_DECRYPT to allow for on-the-fly
>> decryption of an encrypted bitstream.
>>
>> If the system is not booted in secure mode AES & HMAC units
>> are disabled by the boot ROM, therefore the capability
>> is not available.
>>
>> Signed-off-by: Moritz Fischer <moritz.fischer@ettus.com>
>> Cc: Alan Tull <atull@opensource.altera.com>
>> Cc: Michal Simek <michal.simek@xilinx.com>
>> Cc: S?ren Brinkmann <soren.brinkmann@xilinx.com>
>> Cc: linux-kernel at vger.kernel.org
>> Cc: linux-arm-kernel at lists.infradead.org
>> ---
>> drivers/fpga/fpga-mgr.c | 7 +++++++
>> drivers/fpga/zynq-fpga.c | 21 +++++++++++++++++++--
>> include/linux/fpga/fpga-mgr.h | 2 ++
>> 3 files changed, 28 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c
>> index 98230b7..e4d08e1 100644
>> --- a/drivers/fpga/fpga-mgr.c
>> +++ b/drivers/fpga/fpga-mgr.c
>> @@ -61,6 +61,12 @@ int fpga_mgr_buf_load(struct fpga_manager *mgr, u32 flags, const char *buf,
>> return -ENOTSUPP;
>> }
>>
>> + if (flags & FPGA_MGR_DECRYPT_BITSTREAM &&
>> + !fpga_mgr_has_cap(FPGA_MGR_CAP_DECRYPT, mgr->caps)) {
>> + dev_err(dev, "Bitstream decryption not supported\n");
>> + return -ENOTSUPP;
>> + }
>> +
>> /*
>> * Call the low level driver's write_init function. This will do the
>> * device-specific things to get the FPGA into the state where it is
>> @@ -170,6 +176,7 @@ static const char * const state_str[] = {
>> static const char * const cap_str[] = {
>> [FPGA_MGR_CAP_FULL_RECONF] = "Full reconfiguration",
>> [FPGA_MGR_CAP_PARTIAL_RECONF] = "Partial reconfiguration",
>> + [FPGA_MGR_CAP_DECRYPT] = "Decrypt bitstream on the fly",
>> };
>>
>> static ssize_t name_show(struct device *dev,
>> diff --git a/drivers/fpga/zynq-fpga.c b/drivers/fpga/zynq-fpga.c
>> index 1d37ff0..0aa4705 100644
>> --- a/drivers/fpga/zynq-fpga.c
>> +++ b/drivers/fpga/zynq-fpga.c
>> @@ -71,6 +71,10 @@
>> #define CTRL_PCAP_PR_MASK BIT(27)
>> /* Enable PCAP */
>> #define CTRL_PCAP_MODE_MASK BIT(26)
>> +/* Needed to reduce clock rate for secure config */
>> +#define CTRL_PCAP_RATE_EN_MASK BIT(25)
>> +/* System booted in secure mode */
>> +#define CTRL_SEC_EN_MASK BIT(7)
>>
>> /* Miscellaneous Control Register bit definitions */
>> /* Internal PCAP loopback */
>> @@ -252,12 +256,20 @@ static int zynq_fpga_ops_write_init(struct fpga_manager *mgr, u32 flags,
>>
>> /* set configuration register with following options:
>> * - enable PCAP interface
>> - * - set throughput for maximum speed
>> + * - set throughput for maximum speed (if we're not decrypting)
>> * - set CPU in user mode
>> */
>> ctrl = zynq_fpga_read(priv, CTRL_OFFSET);
>> - zynq_fpga_write(priv, CTRL_OFFSET,
>> + if (flags & FPGA_MGR_DECRYPT_BITSTREAM) {
>> + zynq_fpga_write(priv, CTRL_OFFSET,
>> + (CTRL_PCAP_PR_MASK | CTRL_PCAP_MODE_MASK |
>> + CTRL_PCAP_RATE_EN_MASK | ctrl));
>> +
>> + } else {
>> + ctrl &= ~CTRL_PCAP_RATE_EN_MASK;
>> + zynq_fpga_write(priv, CTRL_OFFSET,
>> (CTRL_PCAP_PR_MASK | CTRL_PCAP_MODE_MASK | ctrl));
>> + }
>
> Minor nit:
> Assuming that there may be more caps to check to come, wouldn't it be
> slightly easier to write this in a way like?:
> if (flags & SOME_FLAG)
> ctrl |= FOO;
> if (flags & SOME_OTHER_FLAG)
> ctrl |= BAR;
> zynq_fpga_write(priv, CTRL_OFFSET, ctrl);
>
> i.e. moving the fpga_write outside of the conditionals.
Yeah, will do. Definitely better that way.
Thanks for the review,
Moritz
^ permalink raw reply
* [PATCH 1/1 v7] ARM: imx: Added perf functionality to mmdc driver
From: Paul Gortmaker @ 2016-11-08 19:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1474307267-3081-1-git-send-email-Frank.Li@nxp.com>
On Mon, Sep 19, 2016 at 1:47 PM, Frank Li <Frank.Li@nxp.com> wrote:
> From: Zhengyu Shen <zhengyu.shen@nxp.com>
>
> MMDC is a multi-mode DDR controller that supports DDR3/DDR3L x16/x32/x64
> and LPDDR2 two channel x16/x32 memory types. MMDC is configurable, high
> performance, and optimized. MMDC is present on i.MX6 Quad and i.MX6
> QuadPlus devices, but this driver only supports i.MX6 Quad at the moment.
> MMDC provides registers for performance counters which read via this
> driver to help debug memory throughput and similar issues.
>
> $ perf stat -a -e mmdc/busy-cycles/,mmdc/read-accesses/,mmdc/read-bytes/,mmdc/total-cycles/,mmdc/write-accesses/,mmdc/write-bytes/ dd if=/dev/zero of=/dev/null bs=1M count=5000
> Performance counter stats for 'dd if=/dev/zero of=/dev/null bs=1M count=5000':
>
> 898021787 mmdc/busy-cycles/
> 14819600 mmdc/read-accesses/
> 471.30 MB mmdc/read-bytes/
> 2815419216 mmdc/total-cycles/
> 13367354 mmdc/write-accesses/
> 427.76 MB mmdc/write-bytes/
>
> 5.334757334 seconds time elapsed
>
> Signed-off-by: Zhengyu Shen <zhengyu.shen@nxp.com>
> Signed-off-by: Frank Li <frank.li@nxp.com>
> ---
> Changes from v6 to v7
> use mmdc_pmu prefix
> remove unnecessary check
> improve group event check according to mark's feedback.
> check pmu_mmdc->mmdc_events[cfg] at event_add
> only check == 0 at event_del
>
> Changes from v5 to v6
> Improve group event error handle
>
> Changes from v4 to v5
> Remove mmdc_pmu:irq
> remove static variable cpuhp_mmdc_pmu
> remove spin_lock
> check is_sampling_event(event)
> remove unnecessary cast
> use hw_perf_event::prev_count
>
> Changes from v3 to v4:
> Tested and fixed crash relating to removing events with perf fuzzer
> Adjusted formatting
> Moved all perf event code under CONFIG_PERF_EVENTS
> Switched cpuhp_setup_state to cpuhp_setup_state_nocalls
>
> Changes from v2 to v3:
> Use WARN_ONCE instead of returning generic error values
> Replace CPU Notifiers with newer state machine hotplug
> Added additional checks on event_init for grouping and sampling
> Remove useless mmdc_enable_profiling function
> Added comments
> Moved start index of events from 0x01 to 0x00
> Added a counter to pmu_mmdc to only stop hrtimer after all events are finished
> Replace readl_relaxed and writel_relaxed with readl and writel
> Removed duplicate update function
> Used devm_kasprintf when naming mmdcs probed
>
> Changes from v1 to v2:
> Added cpumask and migration handling support to driver
> Validated event during event_init
> Added code to properly stop counters
> Used perf_invalid_context instead of perf_sw_context
> Added hrtimer to poll for overflow
> Added better description
> Added support for multiple mmdcs
>
> arch/arm/mach-imx/mmdc.c | 459 ++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 457 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mach-imx/mmdc.c b/arch/arm/mach-imx/mmdc.c
> index db9621c..1f70376 100644
> --- a/arch/arm/mach-imx/mmdc.c
> +++ b/arch/arm/mach-imx/mmdc.c
> @@ -1,5 +1,5 @@
> /*
> - * Copyright 2011 Freescale Semiconductor, Inc.
> + * Copyright 2011,2016 Freescale Semiconductor, Inc.
> * Copyright 2011 Linaro Ltd.
> *
> * The code contained herein is licensed under the GNU General Public
> @@ -10,12 +10,16 @@
> * http://www.gnu.org/copyleft/gpl.html
> */
>
> +#include <linux/hrtimer.h>
> #include <linux/init.h>
> +#include <linux/interrupt.h>
> #include <linux/io.h>
> #include <linux/module.h>
> #include <linux/of.h>
> #include <linux/of_address.h>
> #include <linux/of_device.h>
> +#include <linux/perf_event.h>
> +#include <linux/slab.h>
>
> #include "common.h"
>
> @@ -27,8 +31,458 @@
> #define BM_MMDC_MDMISC_DDR_TYPE 0x18
> #define BP_MMDC_MDMISC_DDR_TYPE 0x3
>
> +#define TOTAL_CYCLES 0x0
> +#define BUSY_CYCLES 0x1
> +#define READ_ACCESSES 0x2
> +#define WRITE_ACCESSES 0x3
> +#define READ_BYTES 0x4
> +#define WRITE_BYTES 0x5
> +
> +/* Enables, resets, freezes, overflow profiling*/
> +#define DBG_DIS 0x0
> +#define DBG_EN 0x1
> +#define DBG_RST 0x2
> +#define PRF_FRZ 0x4
> +#define CYC_OVF 0x8
> +
> +#define MMDC_MADPCR0 0x410
> +#define MMDC_MADPSR0 0x418
> +#define MMDC_MADPSR1 0x41C
> +#define MMDC_MADPSR2 0x420
> +#define MMDC_MADPSR3 0x424
> +#define MMDC_MADPSR4 0x428
> +#define MMDC_MADPSR5 0x42C
> +
> +#define MMDC_NUM_COUNTERS 6
> +
> +#define to_mmdc_pmu(p) container_of(p, struct mmdc_pmu, pmu)
> +
> static int ddr_type;
>
> +#ifdef CONFIG_PERF_EVENTS
> +
> +static DEFINE_IDA(mmdc_ida);
> +
> +PMU_EVENT_ATTR_STRING(total-cycles, mmdc_pmu_total_cycles, "event=0x00")
> +PMU_EVENT_ATTR_STRING(busy-cycles, mmdc_pmu_busy_cycles, "event=0x01")
> +PMU_EVENT_ATTR_STRING(read-accesses, mmdc_pmu_read_accesses, "event=0x02")
> +PMU_EVENT_ATTR_STRING(write-accesses, mmdc_pmu_write_accesses, "config=0x03")
> +PMU_EVENT_ATTR_STRING(read-bytes, mmdc_pmu_read_bytes, "event=0x04")
> +PMU_EVENT_ATTR_STRING(read-bytes.unit, mmdc_pmu_read_bytes_unit, "MB");
> +PMU_EVENT_ATTR_STRING(read-bytes.scale, mmdc_pmu_read_bytes_scale, "0.000001");
> +PMU_EVENT_ATTR_STRING(write-bytes, mmdc_pmu_write_bytes, "event=0x05")
> +PMU_EVENT_ATTR_STRING(write-bytes.unit, mmdc_pmu_write_bytes_unit, "MB");
> +PMU_EVENT_ATTR_STRING(write-bytes.scale, mmdc_pmu_write_bytes_scale, "0.000001");
> +
> +struct mmdc_pmu {
> + struct pmu pmu;
> + void __iomem *mmdc_base;
> + cpumask_t cpu;
> + struct hrtimer hrtimer;
> + unsigned int active_events;
> + struct device *dev;
> + struct perf_event *mmdc_events[MMDC_NUM_COUNTERS];
> + struct hlist_node node;
> +};
> +
> +/*
> + * Polling period is set to one second, overflow of total-cycles (the fastest
> + * increasing counter) takes ten seconds so one second is safe
> + */
> +static unsigned int mmdc_pmu_poll_period_us = 1000000;
> +
> +module_param_named(pmu_pmu_poll_period_us, mmdc_pmu_poll_period_us, uint,
> + S_IRUGO | S_IWUSR);
I just noticed this commit now that linux-next is back after the week off.
This should probably use core_param or setup_param since the Kconfig
for this is bool and not tristate. Similarly, that means that the .remove
function you've added is dead code -- unless you envision a case where
the user needs to forcibly unbind the driver...
Do you want to redo the existing commit or do you want me to submit
a follow-up fixup patch?
Thanks
Paul.
--
> +
> +static ktime_t mmdc_pmu_timer_period(void)
> +{
> + return ns_to_ktime((u64)mmdc_pmu_poll_period_us * 1000);
> +}
> +
^ permalink raw reply
* Summary of LPC guest MSI discussion in Santa Fe
From: Don Dutile @ 2016-11-08 19:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108175457.GK20591@arm.com>
On 11/08/2016 12:54 PM, Will Deacon wrote:
> On Tue, Nov 08, 2016 at 03:27:23PM +0100, Auger Eric wrote:
>> On 08/11/2016 03:45, Will Deacon wrote:
>>> Rather than treat these as separate problems, a better interface is to
>>> tell userspace about a set of reserved regions, and have this include
>>> the MSI doorbell, irrespective of whether or not it can be remapped.
>>> Don suggested that we statically pick an address for the doorbell in a
>>> similar way to x86, and have the kernel map it there. We could even pick
>>> 0xfee00000. If it conflicts with a reserved region on the platform (due
>>> to (4)), then we'd obviously have to (deterministically?) allocate it
>>> somewhere else, but probably within the bottom 4G.
>> This is tentatively achieved now with
>> [1] [RFC v2 0/8] KVM PCIe/MSI passthrough on ARM/ARM64 - Alt II
>> (http://www.mail-archive.com/linux-kernel at vger.kernel.org/msg1264506.html)
> Yup, I saw that fly by. Hopefully some of the internals can be reused
> with the current thinking on user ABI.
>
>>> The next question is how to tell userspace about all of the reserved
>>> regions. Initially, the idea was to extend VFIO, however Alex pointed
>>> out a horrible scenario:
>>>
>>> 1. QEMU spawns a VM on system 0
>>> 2. VM is migrated to system 1
>>> 3. QEMU attempts to passthrough a device using PCI hotplug
>>>
>>> In this scenario, the guest memory map is chosen at step (1), yet there
>>> is no VFIO fd available to determine the reserved regions. Furthermore,
>>> the reserved regions may vary between system 0 and system 1. This pretty
>>> much rules out using VFIO to determine the reserved regions.Alex suggested
>>> that the SMMU driver can advertise the regions via /sys/class/iommu/. This
>>> would solve part of the problem, but migration between systems with
>>> different memory maps can still cause problems if the reserved regions
>>> of the new system conflict with the guest memory map chosen by QEMU.
>>
>> OK so I understand we do not want anymore the VFIO chain capability API
>> (patch 5 of above series) but we prefer a sysfs approach instead.
> Right.
>
>> I understand the sysfs approach which allows the userspace to get the
>> info earlier and independently on VFIO. Keeping in mind current QEMU
>> virt - which is not the only userspace - will not do much from this info
>> until we bring upheavals in virt address space management. So if I am
>> not wrong, at the moment the main action to be undertaken is the
>> rejection of the PCI hotplug in case we detect a collision.
> I don't think so; it should be up to userspace to reject the hotplug.
> If userspace doesn't have support for the regions, then that's fine --
> you just end up in a situation where the CPU page table maps memory
> somewhere that the device can't see. In other words, you'll end up with
> spurious DMA failures, but that's exactly what happens with current systems
> if you passthrough an overlapping region (Robin demonstrated this on Juno).
>
> Additionally, you can imagine some future support where you can tell the
> guest not to use certain regions of its memory for DMA. In this case, you
> wouldn't want to refuse the hotplug in the case of overlapping regions.
>
> Really, I think the kernel side just needs to enumerate the fixed reserved
> regions, place the doorbell at a fixed address and then advertise these
> via sysfs.
>
>> I can respin [1]
>> - studying and taking into account Robin's comments about dm_regions
>> similarities
>> - removing the VFIO capability chain and replacing this by a sysfs API
> Ideally, this would be reusable between different SMMU drivers so the sysfs
> entries have the same format etc.
>
>> Would that be OK?
> Sounds good to me. Are you in a position to prototype something on the qemu
> side once we've got kernel-side agreement?
>
>> What about Alex comments who wanted to report the usable memory ranges
>> instead of unusable memory ranges?
>>
>> Also did you have a chance to discuss the following items:
>> 1) the VFIO irq safety assessment
> The discussion really focussed on system topology, as opposed to properties
> of the doorbell. Regardless of how the device talks to the doorbell, if
> the doorbell can't protect against things like MSI spoofing, then it's
> unsafe. My opinion is that we shouldn't allow passthrough by default on
> systems with unsafe doorbells (we could piggyback on allow_unsafe_interrupts
> cmdline option to VFIO).
>
> A first step would be making all this opt-in, and only supporting GICv3
> ITS for now.
You're trying to support a config that is < GICv3 and no ITS ? ...
That would be the equiv. of x86 pre-intr-remap, and that's why allow_unsafe_interrupts
hook was created ... to enable devel/kick-the-tires.
>> 2) the MSI reserved size computation (is an arbitrary size OK?)
> If we fix the base address, we could fix a size too. However, we'd still
> need to enumerate the doorbells to check that they fit in the region we
> have. If not, then we can warn during boot and treat it the same way as
> a resource conflict (that is, reallocate the region in some deterministic
> way).
>
> Will
^ permalink raw reply
* KASAN & the vmalloc area
From: Mark Rutland @ 2016-11-08 19:03 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
I see a while back [1] there was a discussion of what to do about KASAN
and vmapped stacks, but it doesn't look like that was solved, judging by
the vmapped stacks pull [2] for v4.9.
I wondered whether anyone had looked at that since?
I have an additional reason to want to dynamically allocate the vmalloc
area shadow: it turns out that KASAN currently interacts rather poorly
with the arm64 ptdump code.
When KASAN is selected, we allocate shadow for the whole vmalloc area,
using common zero pte, pmd, pud tables. Walking over these in the ptdump
code takes a *very* long time (I've seen up to 15 minutes with
KASAN_OUTLINE enabled). For DEBUG_WX [3], this means boot hangs for that
long, too.
If I don't allocate vmalloc shadow (and remove the apparently pointlesss
shadow of the shadow area), and only allocate shadow for the image,
fixmap, vmemmap and so on, that delay gets cut to a few seconds, which
is tolerable for a debug configuration...
... however, things blow up when the kernel touches vmalloc'd memory for
the first time, as we don't install shadow for that dynamically.
Thanks,
Mark.
[1] https://lkml.kernel.org/r/CALCETrWucrYp+yq8RHSDqf93xtg793duByirurzJbLRhrz=tcA at mail.gmail.com
[2] https://lkml.kernel.org/r/20161003092940.GA691 at gmail.com
[3] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-October/464191.html
^ permalink raw reply
* [RESPIN-PATCH v2 1/2] ARM: EXYNOS: Remove static mapping of SCU SFR
From: Krzysztof Kozlowski @ 2016-11-08 19:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478603984-504-2-git-send-email-pankaj.dubey@samsung.com>
On Tue, Nov 08, 2016 at 04:49:43PM +0530, Pankaj Dubey wrote:
> Lets remove static mapping of SCU SFR mainly used in CORTEX-A9 SoC based boards.
> Instead use mapping from device tree node of SCU.
>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> Reviewed-by: Alim Akhtar <alim.akhtar@samsung.com>
> ---
> arch/arm/mach-exynos/common.h | 1 +
> arch/arm/mach-exynos/exynos.c | 22 ------------------
> arch/arm/mach-exynos/include/mach/map.h | 2 --
> arch/arm/mach-exynos/platsmp.c | 34 +++++++++++++++++++++-------
> arch/arm/mach-exynos/pm.c | 5 ++--
> arch/arm/mach-exynos/suspend.c | 13 ++++-------
> arch/arm/plat-samsung/include/plat/map-s5p.h | 4 ----
> 7 files changed, 34 insertions(+), 47 deletions(-)
>
> diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
> index 9424a8a..dd5d8e8 100644
> --- a/arch/arm/mach-exynos/common.h
> +++ b/arch/arm/mach-exynos/common.h
> @@ -161,6 +161,7 @@ extern void exynos_cpu_restore_register(void);
> extern void exynos_pm_central_suspend(void);
> extern int exynos_pm_central_resume(void);
> extern void exynos_enter_aftr(void);
> +extern int exynos_scu_enable(void);
>
> extern struct cpuidle_exynos_data cpuidle_coupled_exynos_data;
>
> diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
> index 757fc11..fa08ef9 100644
> --- a/arch/arm/mach-exynos/exynos.c
> +++ b/arch/arm/mach-exynos/exynos.c
> @@ -28,15 +28,6 @@
>
> #include "common.h"
>
> -static struct map_desc exynos4_iodesc[] __initdata = {
> - {
> - .virtual = (unsigned long)S5P_VA_COREPERI_BASE,
> - .pfn = __phys_to_pfn(EXYNOS4_PA_COREPERI),
> - .length = SZ_8K,
> - .type = MT_DEVICE,
> - },
> -};
> -
> static struct platform_device exynos_cpuidle = {
> .name = "exynos_cpuidle",
> #ifdef CONFIG_ARM_EXYNOS_CPUIDLE
> @@ -99,17 +90,6 @@ static int __init exynos_fdt_map_chipid(unsigned long node, const char *uname,
> return 1;
> }
>
> -/*
> - * exynos_map_io
> - *
> - * register the standard cpu IO areas
> - */
> -static void __init exynos_map_io(void)
> -{
> - if (soc_is_exynos4())
> - iotable_init(exynos4_iodesc, ARRAY_SIZE(exynos4_iodesc));
> -}
> -
> static void __init exynos_init_io(void)
> {
> debug_ll_io_init();
> @@ -118,8 +98,6 @@ static void __init exynos_init_io(void)
>
> /* detect cpu id and rev. */
> s5p_init_cpu(S5P_VA_CHIPID);
> -
> - exynos_map_io();
> }
>
> /*
> diff --git a/arch/arm/mach-exynos/include/mach/map.h b/arch/arm/mach-exynos/include/mach/map.h
> index 5fb0040..0eef407 100644
> --- a/arch/arm/mach-exynos/include/mach/map.h
> +++ b/arch/arm/mach-exynos/include/mach/map.h
> @@ -18,6 +18,4 @@
>
> #define EXYNOS_PA_CHIPID 0x10000000
>
> -#define EXYNOS4_PA_COREPERI 0x10500000
> -
> #endif /* __ASM_ARCH_MAP_H */
> diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c
> index a5d6841..94405c7 100644
> --- a/arch/arm/mach-exynos/platsmp.c
> +++ b/arch/arm/mach-exynos/platsmp.c
> @@ -168,6 +168,27 @@ int exynos_cluster_power_state(int cluster)
> S5P_CORE_LOCAL_PWR_EN);
> }
>
> +/**
> + * exynos_scu_enable : enables SCU for Cortex-A9 based system
> + * returns 0 on success else non-zero error code
> + */
> +int exynos_scu_enable(void)
> +{
> + struct device_node *np;
> + void __iomem *scu_base;
> +
> + np = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-scu");
> + scu_base = of_iomap(np, 0);
> + of_node_put(np);
> + if (!scu_base) {
> + pr_err("%s failed to map scu_base\n", __func__);
> + return -ENOMEM;
> + }
> + scu_enable(scu_base);
> + iounmap(scu_base);
> + return 0;
> +}
> +
> static void __iomem *cpu_boot_reg_base(void)
> {
> if (soc_is_exynos4210() && samsung_rev() == EXYNOS4210_REV_1_1)
> @@ -224,11 +245,6 @@ static void write_pen_release(int val)
> sync_cache_w(&pen_release);
> }
>
> -static void __iomem *scu_base_addr(void)
> -{
> - return (void __iomem *)(S5P_VA_SCU);
> -}
> -
> static DEFINE_SPINLOCK(boot_lock);
>
> static void exynos_secondary_init(unsigned int cpu)
> @@ -393,9 +409,11 @@ static void __init exynos_smp_prepare_cpus(unsigned int max_cpus)
>
> exynos_set_delayed_reset_assertion(true);
>
> - if (read_cpuid_part() == ARM_CPU_PART_CORTEX_A9)
> - scu_enable(scu_base_addr());
> -
> + if (read_cpuid_part() == ARM_CPU_PART_CORTEX_A9) {
> + /* if exynos_scu_enable fails, return */
> + if (exynos_scu_enable())
> + return;
Ohhh, someone (e.g. out-of-tree DTS) might be surprised with this.
Please mention such change of behaviour in the commit log (describe the
possible impact of this commit).
> + }
> /*
> * Write the address of secondary startup into the
> * system-wide flags register. The boot monitor waits
> diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c
> index 487295f..23db2af 100644
> --- a/arch/arm/mach-exynos/pm.c
> +++ b/arch/arm/mach-exynos/pm.c
> @@ -18,6 +18,7 @@
> #include <linux/cpu_pm.h>
> #include <linux/io.h>
> #include <linux/of.h>
> +#include <linux/of_address.h>
Why do you need this include? Was it coming from mach/map.h?
> #include <linux/soc/samsung/exynos-regs-pmu.h>
> #include <linux/soc/samsung/exynos-pmu.h>
>
> @@ -26,8 +27,6 @@
> #include <asm/suspend.h>
> #include <asm/cacheflush.h>
>
> -#include <mach/map.h>
> -
> #include "common.h"
>
> static inline void __iomem *exynos_boot_vector_addr(void)
> @@ -177,7 +176,7 @@ void exynos_enter_aftr(void)
> cpu_suspend(0, exynos_aftr_finisher);
>
> if (read_cpuid_part() == ARM_CPU_PART_CORTEX_A9) {
> - scu_enable(S5P_VA_SCU);
> + exynos_scu_enable();
> if (call_firmware_op(resume) == -ENOSYS)
> exynos_cpu_restore_register();
> }
> diff --git a/arch/arm/mach-exynos/suspend.c b/arch/arm/mach-exynos/suspend.c
> index 06332f6..c73c857 100644
> --- a/arch/arm/mach-exynos/suspend.c
> +++ b/arch/arm/mach-exynos/suspend.c
> @@ -34,8 +34,6 @@
> #include <asm/smp_scu.h>
> #include <asm/suspend.h>
>
> -#include <mach/map.h>
> -
> #include <plat/pm-common.h>
>
> #include "common.h"
> @@ -461,12 +459,11 @@ static void exynos_pm_resume(void)
> /* For release retention */
> exynos_pm_release_retention();
>
> - if (cpuid == ARM_CPU_PART_CORTEX_A9)
> - scu_enable(S5P_VA_SCU);
> -
> - if (call_firmware_op(resume) == -ENOSYS
> - && cpuid == ARM_CPU_PART_CORTEX_A9)
> - exynos_cpu_restore_register();
> + if (cpuid == ARM_CPU_PART_CORTEX_A9) {
> + exynos_scu_enable();
> + if (call_firmware_op(resume) == -ENOSYS)
> + exynos_cpu_restore_register();
It does not look right. I think you changed the logic here. Previously
if CPU != A9, then call_firmware_op() was executed. Now it won't be.
BTW, don't respin patchset with the same version number. This is
basically v3. To me, increasing version number is always welcomed. It
makes dealign with patches easier.
Best regards,
Krzysztof
> + }
>
> early_wakeup:
>
> diff --git a/arch/arm/plat-samsung/include/plat/map-s5p.h b/arch/arm/plat-samsung/include/plat/map-s5p.h
> index 0fe2828..512ed1f 100644
> --- a/arch/arm/plat-samsung/include/plat/map-s5p.h
> +++ b/arch/arm/plat-samsung/include/plat/map-s5p.h
> @@ -15,10 +15,6 @@
>
> #define S5P_VA_CHIPID S3C_ADDR(0x02000000)
>
> -#define S5P_VA_COREPERI_BASE S3C_ADDR(0x02800000)
> -#define S5P_VA_COREPERI(x) (S5P_VA_COREPERI_BASE + (x))
> -#define S5P_VA_SCU S5P_VA_COREPERI(0x0)
> -
> #define VA_VIC(x) (S3C_VA_IRQ + ((x) * 0x10000))
> #define VA_VIC0 VA_VIC(0)
> #define VA_VIC1 VA_VIC(1)
> --
> 2.7.4
>
^ permalink raw reply
* [PATCH 1/2] mfd: pm8921: add support to pm8821
From: Bjorn Andersson @ 2016-11-08 19:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478622577-20699-1-git-send-email-srinivas.kandagatla@linaro.org>
On Tue 08 Nov 08:29 PST 2016, Srinivas Kandagatla wrote:
> This patch adds support to PM8821 PMIC and interrupt support.
> PM8821 is companion device that supplements primary PMIC PM8921 IC.
>
Linus Walleij has a patch out for renaming a lot of things in this file,
so we should probably make sure that lands and then rebase this ontop.
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
> Tested this patch for MPP and IRQ functionality on IFC6410 and SD600 EVAL
> board with mpps PM8821 and PM8921.
>
> .../devicetree/bindings/mfd/qcom-pm8xxx.txt | 1 +
> drivers/mfd/pm8921-core.c | 368 +++++++++++++++++++--
> 2 files changed, 340 insertions(+), 29 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mfd/qcom-pm8xxx.txt b/Documentation/devicetree/bindings/mfd/qcom-pm8xxx.txt
> index 37a088f..8f1b4ec 100644
> --- a/Documentation/devicetree/bindings/mfd/qcom-pm8xxx.txt
> +++ b/Documentation/devicetree/bindings/mfd/qcom-pm8xxx.txt
> @@ -11,6 +11,7 @@ voltages and other various functionality to Qualcomm SoCs.
> Definition: must be one of:
> "qcom,pm8058"
> "qcom,pm8921"
> + "qcom,pm8821"
>
> - #address-cells:
> Usage: required
> diff --git a/drivers/mfd/pm8921-core.c b/drivers/mfd/pm8921-core.c
> index 0e3a2ea..28c2470 100644
> --- a/drivers/mfd/pm8921-core.c
> +++ b/drivers/mfd/pm8921-core.c
> @@ -28,16 +28,26 @@
> #include <linux/mfd/core.h>
>
> #define SSBI_REG_ADDR_IRQ_BASE 0x1BB
> -
> -#define SSBI_REG_ADDR_IRQ_ROOT (SSBI_REG_ADDR_IRQ_BASE + 0)
> -#define SSBI_REG_ADDR_IRQ_M_STATUS1 (SSBI_REG_ADDR_IRQ_BASE + 1)
> -#define SSBI_REG_ADDR_IRQ_M_STATUS2 (SSBI_REG_ADDR_IRQ_BASE + 2)
> -#define SSBI_REG_ADDR_IRQ_M_STATUS3 (SSBI_REG_ADDR_IRQ_BASE + 3)
> -#define SSBI_REG_ADDR_IRQ_M_STATUS4 (SSBI_REG_ADDR_IRQ_BASE + 4)
> -#define SSBI_REG_ADDR_IRQ_BLK_SEL (SSBI_REG_ADDR_IRQ_BASE + 5)
> -#define SSBI_REG_ADDR_IRQ_IT_STATUS (SSBI_REG_ADDR_IRQ_BASE + 6)
> -#define SSBI_REG_ADDR_IRQ_CONFIG (SSBI_REG_ADDR_IRQ_BASE + 7)
> -#define SSBI_REG_ADDR_IRQ_RT_STATUS (SSBI_REG_ADDR_IRQ_BASE + 8)
Keep these (per argumentation that follows), but try to name them
appropriately.
> +#define SSBI_PM8821_REG_ADDR_IRQ_BASE 0x100
> +
> +#define SSBI_REG_ADDR_IRQ_ROOT (0)
> +#define SSBI_REG_ADDR_IRQ_M_STATUS1 (1)
> +#define SSBI_REG_ADDR_IRQ_M_STATUS2 (2)
> +#define SSBI_REG_ADDR_IRQ_M_STATUS3 (3)
> +#define SSBI_REG_ADDR_IRQ_M_STATUS4 (4)
> +#define SSBI_REG_ADDR_IRQ_BLK_SEL (5)
> +#define SSBI_REG_ADDR_IRQ_IT_STATUS (6)
> +#define SSBI_REG_ADDR_IRQ_CONFIG (7)
> +#define SSBI_REG_ADDR_IRQ_RT_STATUS (8)
Unnecessary parenthesis.
> +
> +#define PM8821_TOTAL_IRQ_MASTERS 2
Unused.
> +#define PM8821_BLOCKS_PER_MASTER 7
> +#define PM8821_IRQ_MASTER1_SET 0x01
BIT(0), but I would prefer that you just inline this with a comment.
> +#define PM8821_IRQ_CLEAR_OFFSET 0x01
Rather than having a single define for this and add in the base and
block numbers I think you should split it into a master0 and master1
define. (And it's not a offset as much as a register)
> +#define PM8821_IRQ_RT_STATUS_OFFSET 0x0f
Dito
> +#define PM8821_IRQ_MASK_REG_OFFSET 0x08
Dito
> +#define SSBI_REG_ADDR_IRQ_MASTER0 0x30
> +#define SSBI_REG_ADDR_IRQ_MASTER1 0xb0
Fold these two into the registers above.
>
> #define PM_IRQF_LVL_SEL 0x01 /* level select */
> #define PM_IRQF_MASK_FE 0x02 /* mask falling edge */
> @@ -54,30 +64,41 @@
> #define REG_HWREV_2 0x0E8 /* PMIC4 revision 2 */
>
> #define PM8921_NR_IRQS 256
> +#define PM8821_NR_IRQS 112
>
> struct pm_irq_chip {
> struct regmap *regmap;
> spinlock_t pm_irq_lock;
> struct irq_domain *irqdomain;
> + unsigned int irq_reg_base;
> unsigned int num_irqs;
> unsigned int num_blocks;
> unsigned int num_masters;
> u8 config[0];
> };
>
> +struct pm8xxx_data {
> + int num_irqs;
> + unsigned int irq_reg_base;
As far as I can see this is always SSBI_PM8821_REG_ADDR_IRQ_BASE in the
8821 functions and SSBI_REG_ADDR_IRQ_BASE in the pm8xxx functions. If
you have disjunct code paths I think it's better to not obscure this
with a variable.
Try renaming the constants appropriately instead. This also has the
benefit of reducing the size of the patch slightly.
> + const struct irq_domain_ops *irq_domain_ops;
> + void (*irq_handler)(struct irq_desc *desc);
> +};
> +
> static int pm8xxx_read_block_irq(struct pm_irq_chip *chip, unsigned int bp,
> unsigned int *ip)
> {
> int rc;
>
> spin_lock(&chip->pm_irq_lock);
> - rc = regmap_write(chip->regmap, SSBI_REG_ADDR_IRQ_BLK_SEL, bp);
> + rc = regmap_write(chip->regmap,
> + chip->irq_reg_base + SSBI_REG_ADDR_IRQ_BLK_SEL, bp);
> if (rc) {
> pr_err("Failed Selecting Block %d rc=%d\n", bp, rc);
> goto bail;
> }
>
> - rc = regmap_read(chip->regmap, SSBI_REG_ADDR_IRQ_IT_STATUS, ip);
> + rc = regmap_read(chip->regmap,
> + chip->irq_reg_base + SSBI_REG_ADDR_IRQ_IT_STATUS, ip);
> if (rc)
> pr_err("Failed Reading Status rc=%d\n", rc);
> bail:
> @@ -91,14 +112,16 @@ pm8xxx_config_irq(struct pm_irq_chip *chip, unsigned int bp, unsigned int cp)
> int rc;
>
> spin_lock(&chip->pm_irq_lock);
> - rc = regmap_write(chip->regmap, SSBI_REG_ADDR_IRQ_BLK_SEL, bp);
> + rc = regmap_write(chip->regmap,
> + chip->irq_reg_base + SSBI_REG_ADDR_IRQ_BLK_SEL, bp);
> if (rc) {
> pr_err("Failed Selecting Block %d rc=%d\n", bp, rc);
> goto bail;
> }
>
> cp |= PM_IRQF_WRITE;
> - rc = regmap_write(chip->regmap, SSBI_REG_ADDR_IRQ_CONFIG, cp);
> + rc = regmap_write(chip->regmap,
> + chip->irq_reg_base + SSBI_REG_ADDR_IRQ_CONFIG, cp);
> if (rc)
> pr_err("Failed Configuring IRQ rc=%d\n", rc);
> bail:
> @@ -137,8 +160,8 @@ static int pm8xxx_irq_master_handler(struct pm_irq_chip *chip, int master)
> unsigned int blockbits;
> int block_number, i, ret = 0;
>
> - ret = regmap_read(chip->regmap, SSBI_REG_ADDR_IRQ_M_STATUS1 + master,
> - &blockbits);
> + ret = regmap_read(chip->regmap, chip->irq_reg_base +
> + SSBI_REG_ADDR_IRQ_M_STATUS1 + master, &blockbits);
> if (ret) {
> pr_err("Failed to read master %d ret=%d\n", master, ret);
> return ret;
> @@ -165,7 +188,8 @@ static void pm8xxx_irq_handler(struct irq_desc *desc)
>
> chained_irq_enter(irq_chip, desc);
>
> - ret = regmap_read(chip->regmap, SSBI_REG_ADDR_IRQ_ROOT, &root);
> + ret = regmap_read(chip->regmap,
> + chip->irq_reg_base + SSBI_REG_ADDR_IRQ_ROOT, &root);
> if (ret) {
> pr_err("Can't read root status ret=%d\n", ret);
> return;
> @@ -182,6 +206,122 @@ static void pm8xxx_irq_handler(struct irq_desc *desc)
> chained_irq_exit(irq_chip, desc);
> }
>
> +static int pm8821_read_master_irq(const struct pm_irq_chip *chip,
> + int m, unsigned int *master)
> +{
I think you should inline this, as you already have the calls unrolled
in pm8821_irq_handler().
> + unsigned int base;
> +
> + if (!m)
> + base = chip->irq_reg_base + SSBI_REG_ADDR_IRQ_MASTER0;
> + else
> + base = chip->irq_reg_base + SSBI_REG_ADDR_IRQ_MASTER1;
> +
> + return regmap_read(chip->regmap, base, master);
> +}
> +
> +static int pm8821_read_block_irq(struct pm_irq_chip *chip, int master,
> + u8 block, unsigned int *bits)
> +{
> + int rc;
> +
> + unsigned int base;
Odd empty line between rc and base. (And btw, sorting your local
variables in descending length make things pretty).
> +
> + if (!master)
> + base = chip->irq_reg_base + SSBI_REG_ADDR_IRQ_MASTER0;
> + else
> + base = chip->irq_reg_base + SSBI_REG_ADDR_IRQ_MASTER1;
> +
> + spin_lock(&chip->pm_irq_lock);
The reason why this is done under a lock in the other case is because
the status register is paged, so you shouldn't need it here.
With this updated I think you can favorably inline this into
pm8821_irq_block_handler().
> +
> + rc = regmap_read(chip->regmap, base + block, bits);
> + if (rc)
> + pr_err("Failed Reading Status rc=%d\n", rc);
> +
> + spin_unlock(&chip->pm_irq_lock);
> + return rc;
> +}
> +
> +static int pm8821_irq_block_handler(struct pm_irq_chip *chip,
> + int master_number, int block)
> +{
> + int pmirq, irq, i, ret;
> + unsigned int bits;
> +
> + ret = pm8821_read_block_irq(chip, master_number, block, &bits);
> + if (ret) {
> + pr_err("Failed reading %d block ret=%d", block, ret);
> + return ret;
> + }
> + if (!bits) {
> + pr_err("block bit set in master but no irqs: %d", block);
> + return 0;
> + }
> +
> + /* Convert block offset to global block number */
> + block += (master_number * PM8821_BLOCKS_PER_MASTER) - 1;
So this is block -= 1 for master 0 and block += 6 for master 1, is the
latter correct?
> +
> + /* Check IRQ bits */
> + for (i = 0; i < 8; i++) {
> + if (bits & BIT(i)) {
> + pmirq = block * 8 + i;
> + irq = irq_find_mapping(chip->irqdomain, pmirq);
> + generic_handle_irq(irq);
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int pm8821_irq_read_master(struct pm_irq_chip *chip,
> + int master_number, u8 master_val)
This isn't so much a matter of "reading master X" as "handle master X".
Also, you don't care about the return value, so no need to return one...
> +{
> + int ret = 0;
> + int block;
> +
> + for (block = 1; block < 8; block++) {
> + if (master_val & BIT(block)) {
> + ret |= pm8821_irq_block_handler(chip,
> + master_number, block);
> + }
> + }
> +
> + return ret;
> +}
> +
> +static void pm8821_irq_handler(struct irq_desc *desc)
> +{
> + struct pm_irq_chip *chip = irq_desc_get_handler_data(desc);
> + struct irq_chip *irq_chip = irq_desc_get_chip(desc);
> + int ret;
> + unsigned int master;
> +
> + chained_irq_enter(irq_chip, desc);
> + /* check master 0 */
> + ret = pm8821_read_master_irq(chip, 0, &master);
> + if (ret) {
> + pr_err("Failed to re:Qad master 0 ret=%d\n", ret);
> + return;
> + }
> +
> + if (master & ~PM8821_IRQ_MASTER1_SET)
Rather than having a define for MASTER1_SET use BIT(0) here and write a
comment like:
"bits 1 through 7 marks the first 7 blocks"
> + pm8821_irq_read_master(chip, 0, master);
> +
and then
"bit 0 is set if second master contains any bits"
Or just skip this optimization and check the two masters unconditionally
in a loop.
> + /* check master 1 */
> + if (!(master & PM8821_IRQ_MASTER1_SET))
> + goto done;
> +
> + ret = pm8821_read_master_irq(chip, 1, &master);
> + if (ret) {
> + pr_err("Failed to read master 1 ret=%d\n", ret);
> + return;
> + }
> +
> + pm8821_irq_read_master(chip, 1, master);
> +
> +done:
> + chained_irq_exit(irq_chip, desc);
> +}
> +
> static void pm8xxx_irq_mask_ack(struct irq_data *d)
> {
> struct pm_irq_chip *chip = irq_data_get_irq_chip_data(d);
> @@ -254,13 +394,15 @@ static int pm8xxx_irq_get_irqchip_state(struct irq_data *d,
> irq_bit = pmirq % 8;
>
> spin_lock(&chip->pm_irq_lock);
> - rc = regmap_write(chip->regmap, SSBI_REG_ADDR_IRQ_BLK_SEL, block);
> + rc = regmap_write(chip->regmap, chip->irq_reg_base +
> + SSBI_REG_ADDR_IRQ_BLK_SEL, block);
> if (rc) {
> pr_err("Failed Selecting Block %d rc=%d\n", block, rc);
> goto bail;
> }
>
> - rc = regmap_read(chip->regmap, SSBI_REG_ADDR_IRQ_RT_STATUS, &bits);
> + rc = regmap_read(chip->regmap, chip->irq_reg_base +
> + SSBI_REG_ADDR_IRQ_RT_STATUS, &bits);
> if (rc) {
> pr_err("Failed Reading Status rc=%d\n", rc);
> goto bail;
> @@ -299,6 +441,151 @@ static const struct irq_domain_ops pm8xxx_irq_domain_ops = {
> .map = pm8xxx_irq_domain_map,
> };
>
> +static void pm8821_irq_mask_ack(struct irq_data *d)
> +{
> + struct pm_irq_chip *chip = irq_data_get_irq_chip_data(d);
> + unsigned int base, pmirq = irqd_to_hwirq(d);
> + u8 block, master;
> + int irq_bit, rc;
> +
> + block = pmirq / 8;
> + master = block / PM8821_BLOCKS_PER_MASTER;
> + irq_bit = pmirq % 8;
> + block %= PM8821_BLOCKS_PER_MASTER;
You can deobfuscate this somewhat by instead of testing for !master
below you just do:
if (block < PM8821_BLOCKS_PER_MASTER) {
base =
} else {
base =
block -= PM8821_BLOCKS_PER_MASTER;
}
> +
> + if (!master)
> + base = chip->irq_reg_base + SSBI_REG_ADDR_IRQ_MASTER0;
> + else
> + base = chip->irq_reg_base + SSBI_REG_ADDR_IRQ_MASTER1;
> +
> + spin_lock(&chip->pm_irq_lock);
The irqchip code grabs a lock on the irq_desc, so this can't race with
unmask - and the regmap_update_bits() is internally protecting the
read/write cycle.
So you shouldn't need to lock around this section.
> + rc = regmap_update_bits(chip->regmap,
> + base + PM8821_IRQ_MASK_REG_OFFSET + block,
> + BIT(irq_bit), BIT(irq_bit));
> +
> + if (rc) {
> + pr_err("Failed to read/write mask IRQ:%d rc=%d\n", pmirq, rc);
> + goto fail;
> + }
> +
> + rc = regmap_update_bits(chip->regmap,
> + base + PM8821_IRQ_CLEAR_OFFSET + block,
> + BIT(irq_bit), BIT(irq_bit));
> +
> + if (rc) {
> + pr_err("Failed to read/write IT_CLEAR IRQ:%d rc=%d\n",
> + pmirq, rc);
> + }
> +
> +fail:
> + spin_unlock(&chip->pm_irq_lock);
> +}
> +
> +static void pm8821_irq_unmask(struct irq_data *d)
> +{
> + struct pm_irq_chip *chip = irq_data_get_irq_chip_data(d);
> + unsigned int base, pmirq = irqd_to_hwirq(d);
> + int irq_bit, rc;
> + u8 block, master;
> +
> + block = pmirq / 8;
> + master = block / PM8821_BLOCKS_PER_MASTER;
> + irq_bit = pmirq % 8;
> + block %= PM8821_BLOCKS_PER_MASTER;
As mask().
> +
> + if (!master)
> + base = chip->irq_reg_base + SSBI_REG_ADDR_IRQ_MASTER0;
> + else
> + base = chip->irq_reg_base + SSBI_REG_ADDR_IRQ_MASTER1;
> +
> + spin_lock(&chip->pm_irq_lock);
As mask().
> +
> + rc = regmap_update_bits(chip->regmap,
> + base + PM8821_IRQ_MASK_REG_OFFSET + block,
> + BIT(irq_bit), ~BIT(irq_bit));
> +
> + if (rc)
> + pr_err("Failed to read/write unmask IRQ:%d rc=%d\n", pmirq, rc);
> +
> + spin_unlock(&chip->pm_irq_lock);
> +}
> +
> +static int pm8821_irq_set_type(struct irq_data *d, unsigned int flow_type)
> +{
> +
> + /*
> + * PM8821 IRQ controller does not have explicit software support for
> + * IRQ flow type.
> + */
Is returning "success" here the right thing to do? Shouldn't we just
omit the function? Or did you perhaps hit some clients that wouldn't
deal with that?
> + return 0;
> +}
> +
> +static int pm8821_irq_get_irqchip_state(struct irq_data *d,
> + enum irqchip_irq_state which,
> + bool *state)
> +{
> + struct pm_irq_chip *chip = irq_data_get_irq_chip_data(d);
> + int pmirq, rc;
> + u8 block, irq_bit, master;
> + unsigned int bits;
> + unsigned int base;
> + unsigned long flags;
> +
> + pmirq = irqd_to_hwirq(d);
> +
> + block = pmirq / 8;
> + master = block / PM8821_BLOCKS_PER_MASTER;
> + irq_bit = pmirq % 8;
> + block %= PM8821_BLOCKS_PER_MASTER;
> +
Simplify as in mask().
> + if (!master)
> + base = chip->irq_reg_base + SSBI_REG_ADDR_IRQ_MASTER0;
> + else
> + base = chip->irq_reg_base + SSBI_REG_ADDR_IRQ_MASTER1;
> +
> + spin_lock_irqsave(&chip->pm_irq_lock, flags);
No need to lock here as we're just reading a single register.
> +
> + rc = regmap_read(chip->regmap,
> + base + PM8821_IRQ_RT_STATUS_OFFSET + block, &bits);
> + if (rc) {
> + pr_err("Failed Reading Status rc=%d\n", rc);
> + goto bail_out;
> + }
> +
> + *state = !!(bits & BIT(irq_bit));
> +
> +bail_out:
> + spin_unlock_irqrestore(&chip->pm_irq_lock, flags);
> +
> + return rc;
> +}
> +
> +static struct irq_chip pm8821_irq_chip = {
> + .name = "pm8821",
> + .irq_mask_ack = pm8821_irq_mask_ack,
> + .irq_unmask = pm8821_irq_unmask,
> + .irq_set_type = pm8821_irq_set_type,
> + .irq_get_irqchip_state = pm8821_irq_get_irqchip_state,
> + .flags = IRQCHIP_MASK_ON_SUSPEND | IRQCHIP_SKIP_SET_WAKE,
> +};
> +
Regards,
Bjorn
^ permalink raw reply
* Summary of LPC guest MSI discussion in Santa Fe
From: Will Deacon @ 2016-11-08 19:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5822214F.2070500@redhat.com>
On Tue, Nov 08, 2016 at 02:02:39PM -0500, Don Dutile wrote:
> On 11/08/2016 12:54 PM, Will Deacon wrote:
> >A first step would be making all this opt-in, and only supporting GICv3
> >ITS for now.
> You're trying to support a config that is < GICv3 and no ITS ? ...
> That would be the equiv. of x86 pre-intr-remap, and that's why allow_unsafe_interrupts
> hook was created ... to enable devel/kick-the-tires.
Yup, that's exactly what I was envisaging. For systems that can't do
passthrough safely, we'll always have to throw a devel switch.
Will
^ permalink raw reply
* [PATCH 2/2] ARM: dts: apq8064: add support to pm8821
From: Bjorn Andersson @ 2016-11-08 19:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478622577-20699-2-git-send-email-srinivas.kandagatla@linaro.org>
On Tue 08 Nov 08:29 PST 2016, Srinivas Kandagatla wrote:
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
> arch/arm/boot/dts/qcom-apq8064.dtsi | 28 ++++++++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
> diff --git a/arch/arm/boot/dts/qcom-apq8064.dtsi b/arch/arm/boot/dts/qcom-apq8064.dtsi
> index 1dbe697..fde006c 100644
> --- a/arch/arm/boot/dts/qcom-apq8064.dtsi
> +++ b/arch/arm/boot/dts/qcom-apq8064.dtsi
> @@ -627,6 +627,34 @@
> clock-names = "core";
> };
>
> + qcom,ssbi at c00000 {
No "qcom," in the node name.
> + compatible = "qcom,ssbi";
> + reg = <0x00c00000 0x1000>;
> + qcom,controller-type = "pmic-arbiter";
> +
> + pmicintc2: pmic at 1 {
I think we should follow Linus' lead and label this "pm8821".
> + compatible = "qcom,pm8821";
> + interrupt-parent = <&tlmm_pinmux>;
> + interrupts = <76 8>;
Please spell out IRQ_TYPE_LEVEL_LOW.
And interrupts-extended = <&tlmm_pinmux 76 IRQ_TYPE_LEVEL_LOW> combines
the two lines nicely.
> + #interrupt-cells = <2>;
> + interrupt-controller;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + pm8821_mpps: mpps at 50 {
> +
Extra newline.
> + compatible = "qcom,pm8821-mpp", "qcom,ssbi-mpp";
> + reg = <0x50>;
> +
> + interrupts = <24 1>, <25 1>, <26 1>,
> + <27 1>;
I think these should be IRQ_TYPE_NONE per the discussion on how to share
interrupts between the gpio/mpp driver and other clients.
On the other hand, per the pm8821 driver we only support LEVEL_LOW
(high?).
> +
> + gpio-controller;
> + #gpio-cells = <2>;
> + };
> + };
> + };
> +
Regards,
Bjorn
^ permalink raw reply
* [PATCH 1/1 v7] ARM: imx: Added perf functionality to mmdc driver
From: Zhi Li @ 2016-11-08 19:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAP=VYLo_p3ZmOFcyM9om_31-=5O3ZiRLCBkJDDtj84JxAA7h8Q@mail.gmail.com>
On Tue, Nov 8, 2016 at 1:00 PM, Paul Gortmaker
<paul.gortmaker@windriver.com> wrote:
> On Mon, Sep 19, 2016 at 1:47 PM, Frank Li <Frank.Li@nxp.com> wrote:
>> From: Zhengyu Shen <zhengyu.shen@nxp.com>
>>
>> MMDC is a multi-mode DDR controller that supports DDR3/DDR3L x16/x32/x64
>> and LPDDR2 two channel x16/x32 memory types. MMDC is configurable, high
>> performance, and optimized. MMDC is present on i.MX6 Quad and i.MX6
>> QuadPlus devices, but this driver only supports i.MX6 Quad at the moment.
>> MMDC provides registers for performance counters which read via this
>> driver to help debug memory throughput and similar issues.
>>
>> $ perf stat -a -e mmdc/busy-cycles/,mmdc/read-accesses/,mmdc/read-bytes/,mmdc/total-cycles/,mmdc/write-accesses/,mmdc/write-bytes/ dd if=/dev/zero of=/dev/null bs=1M count=5000
>> Performance counter stats for 'dd if=/dev/zero of=/dev/null bs=1M count=5000':
>>
>> 898021787 mmdc/busy-cycles/
>> 14819600 mmdc/read-accesses/
>> 471.30 MB mmdc/read-bytes/
>> 2815419216 mmdc/total-cycles/
>> 13367354 mmdc/write-accesses/
>> 427.76 MB mmdc/write-bytes/
>>
>> 5.334757334 seconds time elapsed
>>
>> Signed-off-by: Zhengyu Shen <zhengyu.shen@nxp.com>
>> Signed-off-by: Frank Li <frank.li@nxp.com>
>> ---
>> Changes from v6 to v7
>> use mmdc_pmu prefix
>> remove unnecessary check
>> improve group event check according to mark's feedback.
>> check pmu_mmdc->mmdc_events[cfg] at event_add
>> only check == 0 at event_del
>>
>> Changes from v5 to v6
>> Improve group event error handle
>>
>> Changes from v4 to v5
>> Remove mmdc_pmu:irq
>> remove static variable cpuhp_mmdc_pmu
>> remove spin_lock
>> check is_sampling_event(event)
>> remove unnecessary cast
>> use hw_perf_event::prev_count
>>
>> Changes from v3 to v4:
>> Tested and fixed crash relating to removing events with perf fuzzer
>> Adjusted formatting
>> Moved all perf event code under CONFIG_PERF_EVENTS
>> Switched cpuhp_setup_state to cpuhp_setup_state_nocalls
>>
>> Changes from v2 to v3:
>> Use WARN_ONCE instead of returning generic error values
>> Replace CPU Notifiers with newer state machine hotplug
>> Added additional checks on event_init for grouping and sampling
>> Remove useless mmdc_enable_profiling function
>> Added comments
>> Moved start index of events from 0x01 to 0x00
>> Added a counter to pmu_mmdc to only stop hrtimer after all events are finished
>> Replace readl_relaxed and writel_relaxed with readl and writel
>> Removed duplicate update function
>> Used devm_kasprintf when naming mmdcs probed
>>
>> Changes from v1 to v2:
>> Added cpumask and migration handling support to driver
>> Validated event during event_init
>> Added code to properly stop counters
>> Used perf_invalid_context instead of perf_sw_context
>> Added hrtimer to poll for overflow
>> Added better description
>> Added support for multiple mmdcs
>>
>> arch/arm/mach-imx/mmdc.c | 459 ++++++++++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 457 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/mach-imx/mmdc.c b/arch/arm/mach-imx/mmdc.c
>> index db9621c..1f70376 100644
>> --- a/arch/arm/mach-imx/mmdc.c
>> +++ b/arch/arm/mach-imx/mmdc.c
>> @@ -1,5 +1,5 @@
>> /*
>> - * Copyright 2011 Freescale Semiconductor, Inc.
>> + * Copyright 2011,2016 Freescale Semiconductor, Inc.
>> * Copyright 2011 Linaro Ltd.
>> *
>> * The code contained herein is licensed under the GNU General Public
>> @@ -10,12 +10,16 @@
>> * http://www.gnu.org/copyleft/gpl.html
>> */
>>
>> +#include <linux/hrtimer.h>
>> #include <linux/init.h>
>> +#include <linux/interrupt.h>
>> #include <linux/io.h>
>> #include <linux/module.h>
>> #include <linux/of.h>
>> #include <linux/of_address.h>
>> #include <linux/of_device.h>
>> +#include <linux/perf_event.h>
>> +#include <linux/slab.h>
>>
>> #include "common.h"
>>
>> @@ -27,8 +31,458 @@
>> #define BM_MMDC_MDMISC_DDR_TYPE 0x18
>> #define BP_MMDC_MDMISC_DDR_TYPE 0x3
>>
>> +#define TOTAL_CYCLES 0x0
>> +#define BUSY_CYCLES 0x1
>> +#define READ_ACCESSES 0x2
>> +#define WRITE_ACCESSES 0x3
>> +#define READ_BYTES 0x4
>> +#define WRITE_BYTES 0x5
>> +
>> +/* Enables, resets, freezes, overflow profiling*/
>> +#define DBG_DIS 0x0
>> +#define DBG_EN 0x1
>> +#define DBG_RST 0x2
>> +#define PRF_FRZ 0x4
>> +#define CYC_OVF 0x8
>> +
>> +#define MMDC_MADPCR0 0x410
>> +#define MMDC_MADPSR0 0x418
>> +#define MMDC_MADPSR1 0x41C
>> +#define MMDC_MADPSR2 0x420
>> +#define MMDC_MADPSR3 0x424
>> +#define MMDC_MADPSR4 0x428
>> +#define MMDC_MADPSR5 0x42C
>> +
>> +#define MMDC_NUM_COUNTERS 6
>> +
>> +#define to_mmdc_pmu(p) container_of(p, struct mmdc_pmu, pmu)
>> +
>> static int ddr_type;
>>
>> +#ifdef CONFIG_PERF_EVENTS
>> +
>> +static DEFINE_IDA(mmdc_ida);
>> +
>> +PMU_EVENT_ATTR_STRING(total-cycles, mmdc_pmu_total_cycles, "event=0x00")
>> +PMU_EVENT_ATTR_STRING(busy-cycles, mmdc_pmu_busy_cycles, "event=0x01")
>> +PMU_EVENT_ATTR_STRING(read-accesses, mmdc_pmu_read_accesses, "event=0x02")
>> +PMU_EVENT_ATTR_STRING(write-accesses, mmdc_pmu_write_accesses, "config=0x03")
>> +PMU_EVENT_ATTR_STRING(read-bytes, mmdc_pmu_read_bytes, "event=0x04")
>> +PMU_EVENT_ATTR_STRING(read-bytes.unit, mmdc_pmu_read_bytes_unit, "MB");
>> +PMU_EVENT_ATTR_STRING(read-bytes.scale, mmdc_pmu_read_bytes_scale, "0.000001");
>> +PMU_EVENT_ATTR_STRING(write-bytes, mmdc_pmu_write_bytes, "event=0x05")
>> +PMU_EVENT_ATTR_STRING(write-bytes.unit, mmdc_pmu_write_bytes_unit, "MB");
>> +PMU_EVENT_ATTR_STRING(write-bytes.scale, mmdc_pmu_write_bytes_scale, "0.000001");
>> +
>> +struct mmdc_pmu {
>> + struct pmu pmu;
>> + void __iomem *mmdc_base;
>> + cpumask_t cpu;
>> + struct hrtimer hrtimer;
>> + unsigned int active_events;
>> + struct device *dev;
>> + struct perf_event *mmdc_events[MMDC_NUM_COUNTERS];
>> + struct hlist_node node;
>> +};
>> +
>> +/*
>> + * Polling period is set to one second, overflow of total-cycles (the fastest
>> + * increasing counter) takes ten seconds so one second is safe
>> + */
>> +static unsigned int mmdc_pmu_poll_period_us = 1000000;
>> +
>> +module_param_named(pmu_pmu_poll_period_us, mmdc_pmu_poll_period_us, uint,
>> + S_IRUGO | S_IWUSR);
>
> I just noticed this commit now that linux-next is back after the week off.
>
> This should probably use core_param or setup_param since the Kconfig
> for this is bool and not tristate. Similarly, that means that the .remove
> function you've added is dead code -- unless you envision a case where
> the user needs to forcibly unbind the driver...
>
> Do you want to redo the existing commit or do you want me to submit
> a follow-up fixup patch?
I will do follow-up fixup patch.
I think pmu_pmu_poll_period_us should be removed because no benefit to
change it.
I can delete .remove
best regards
Frank Li
>
> Thanks
> Paul.
> --
>
>> +
>> +static ktime_t mmdc_pmu_timer_period(void)
>> +{
>> + return ns_to_ktime((u64)mmdc_pmu_poll_period_us * 1000);
>> +}
>> +
^ permalink raw reply
* [PATCH 2/2] mm: hugetlb: support gigantic surplus pages
From: Gerald Schaefer @ 2016-11-08 19:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108091725.GA18678@sha-win-210.asiapac.arm.com>
On Tue, 8 Nov 2016 17:17:28 +0800
Huang Shijie <shijie.huang@arm.com> wrote:
> > I will look at the lockdep issue.
> I tested the new patch (will be sent out later) on the arm64 platform,
> and I did not meet the lockdep issue when I enabled the lockdep.
> The following is my config:
>
> CONFIG_LOCKD=y
> CONFIG_LOCKD_V4=y
> CONFIG_LOCKUP_DETECTOR=y
> # CONFIG_BOOTPARAM_SOFTLOCKUP_PANIC is not set
> CONFIG_BOOTPARAM_SOFTLOCKUP_PANIC_VALUE=0
> CONFIG_DEBUG_SPINLOCK=y
> CONFIG_DEBUG_LOCK_ALLOC=y
> CONFIG_PROVE_LOCKING=y
> CONFIG_LOCKDEP=y
> CONFIG_LOCK_STAT=y
> CONFIG_DEBUG_LOCKDEP=y
> CONFIG_DEBUG_LOCKING_API_SELFTESTS=y
>
> So do I miss something?
Those options should be OK. Meanwhile I looked into this a little more,
and the problematic line/lock is spin_lock_irqsave(&z->lock, flags) at
the top of alloc_gigantic_page(). From the lockdep trace we see that
it is triggered by an mmap(), and then hugetlb_acct_memory() ->
__alloc_huge_page() -> alloc_gigantic_page().
However, in between those functions (inside gather_surplus_pages())
a NUMA_NO_NODE node id comes into play. And this finally results in
alloc_gigantic_page() being called with NUMA_NO_NODE as nid (which is
-1), and NODE_DATA(nid)->node_zones will then reach into Nirvana.
So, I guess the problem is a missing NUMA_NO_NODE check in
alloc_gigantic_page(), similar to the one in
__hugetlb_alloc_buddy_huge_page(). And somehow this was not a problem
before the gigantic surplus change.
^ permalink raw reply
* [PATCH 1/1 v7] ARM: imx: Added perf functionality to mmdc driver
From: Peter Zijlstra @ 2016-11-08 20:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAHrpEqQGRLY7ZB6moP9FZ0w-t63QkyLE1fUb3heRZJfRd2Y9sw@mail.gmail.com>
On Tue, Nov 08, 2016 at 01:21:47PM -0600, Zhi Li wrote:
> On Tue, Nov 8, 2016 at 1:00 PM, Paul Gortmaker
> > I just noticed this commit now that linux-next is back after the week off.
> >
> > This should probably use core_param or setup_param since the Kconfig
> > for this is bool and not tristate.
> I think pmu_pmu_poll_period_us should be removed because no benefit to
> change it.
> I can delete .remove
Why not make it tristate ?
^ permalink raw reply
* [PATCH 1/1 v7] ARM: imx: Added perf functionality to mmdc driver
From: Zhi Li @ 2016-11-08 20:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108201114.GQ3117@twins.programming.kicks-ass.net>
On Tue, Nov 8, 2016 at 2:11 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Tue, Nov 08, 2016 at 01:21:47PM -0600, Zhi Li wrote:
>> On Tue, Nov 8, 2016 at 1:00 PM, Paul Gortmaker
>> > I just noticed this commit now that linux-next is back after the week off.
>> >
>> > This should probably use core_param or setup_param since the Kconfig
>> > for this is bool and not tristate.
>
>
>> I think pmu_pmu_poll_period_us should be removed because no benefit to
>> change it.
>> I can delete .remove
>
> Why not make it tristate ?
The based code provided a function, which need by suspend and resume.
best regards
Frank Li
^ permalink raw reply
* [GIT PULL 1/4] DaVinci cleanup for v4.10
From: Olof Johansson @ 2016-11-08 20:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161104062535.9593-1-nsekhar@ti.com>
On Fri, Nov 04, 2016 at 11:55:32AM +0530, Sekhar Nori wrote:
> The following changes since commit 1001354ca34179f3db924eb66672442a173147dc:
>
> Linux 4.9-rc1 (2016-10-15 12:17:50 -0700)
>
> are available in the git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/nsekhar/linux-davinci.git tags/davinci-for-v4.10/cleanup
>
> for you to fetch changes up to 766763dbdc1dca11deabdb00077a1c19e2803f0a:
>
> ARM: davinci: da8xx: Remove duplicated defines (2016-10-31 16:51:56 +0530)
>
> ----------------------------------------------------------------
> Clean-up some unnecessary code from mach-davinci.
>
> - Remove now unneeded dma resources where drivers
> are already converted to use the dma_slave_map[]
> structure.
>
> - Remove some duplicated defines related to USB support.
We don't have a cleanup branch in our tree any more, but thanks for breaking
this out for us anyway! I've merged it into next/soc.
-Olof
^ permalink raw reply
* [GIT PULL 2/4] DaVinci SoC updates for v4.10
From: Olof Johansson @ 2016-11-08 20:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161104062535.9593-2-nsekhar@ti.com>
On Fri, Nov 04, 2016 at 11:55:33AM +0530, Sekhar Nori wrote:
> The following changes since commit 766763dbdc1dca11deabdb00077a1c19e2803f0a:
>
> ARM: davinci: da8xx: Remove duplicated defines (2016-10-31 16:51:56 +0530)
>
> are available in the git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/nsekhar/linux-davinci.git tags/davinci-for-v4.10/soc
>
> for you to fetch changes up to ced95ac0815501f47a6041548d70d8900400912d:
>
> ARM: davinci: da8xx: register USB PHY clocks in the DT file (2016-11-01 15:24:24 +0530)
>
> ----------------------------------------------------------------
> DaVinci SoC support improvements mainly towards an effort to
> get to working USB support.
>
> - use CFGCHIP syscon device to access common registers
>
> - define platform data and device tree nodes for newly
> introduced USB phy driver
>
> - clock lookup and auxdata lookup for USB phy and also
> for LCDC (LCD controller)
Merged, thanks.
-Olof
^ permalink raw reply
* [GIT PULL 3/4] DaVinci device-tree updates for v4.10
From: Olof Johansson @ 2016-11-08 20:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161104062535.9593-3-nsekhar@ti.com>
On Fri, Nov 04, 2016 at 11:55:34AM +0530, Sekhar Nori wrote:
> The following changes since commit 1001354ca34179f3db924eb66672442a173147dc:
>
> Linux 4.9-rc1 (2016-10-15 12:17:50 -0700)
>
> are available in the git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/nsekhar/linux-davinci.git tags/davinci-for-v4.10/dt
>
> for you to fetch changes up to 1b499f255589204466d8f8ab26e6b577d7b5c88f:
>
> ARM: dts: da850: Add cfgchip syscon node (2016-11-01 15:11:10 +0530)
>
> ----------------------------------------------------------------
> DaVinci device-tree source additions for
> LCD, SPI and cfgchip syscon.
Merged, thanks!
-Olof
^ permalink raw reply
* [GIT PULL 4/4] DaVinci defconfig updates for v4.10
From: Olof Johansson @ 2016-11-08 20:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161104062535.9593-4-nsekhar@ti.com>
On Fri, Nov 04, 2016 at 11:55:35AM +0530, Sekhar Nori wrote:
> The following changes since commit 1001354ca34179f3db924eb66672442a173147dc:
>
> Linux 4.9-rc1 (2016-10-15 12:17:50 -0700)
>
> are available in the git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/nsekhar/linux-davinci.git tags/davinci-for-v4.10/defconfig
>
> for you to fetch changes up to 6e9be8608771192851a3adf59f0ba9240e3f802c:
>
> ARM: davinci_all_defconfig: enable LED default-on trigger (2016-11-01 11:42:54 +0530)
>
> ----------------------------------------------------------------
> Some updates to davinci_all_defconfig for MMC,
> LCDC and GPIO/LEDs.
>
> ----------------------------------------------------------------
> Bartosz Golaszewski (1):
> ARM: davinci_all_defconfig: enable LCDC DRM driver
>
> David Lechner (3):
> ARM: davinci_all_defconfig: enable gpio poweroff driver
> ARM: davinci_all_defconfig: build MMC into kernel
> ARM: davinci_all_defconfig: enable LED default-on trigger
Merged, thanks.
-Olof
^ permalink raw reply
* Summary of LPC guest MSI discussion in Santa Fe (was: Re: [RFC 0/8] KVM PCIe/MSI passthrough on ARM/ARM64 (Alt II))
From: Christoffer Dall @ 2016-11-08 20:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108024559.GA20591@arm.com>
Hi Will,
On Tue, Nov 08, 2016 at 02:45:59AM +0000, Will Deacon wrote:
> Hi all,
>
> I figured this was a reasonable post to piggy-back on for the LPC minutes
> relating to guest MSIs on arm64.
>
> On Thu, Nov 03, 2016 at 10:02:05PM -0600, Alex Williamson wrote:
> > We can always have QEMU reject hot-adding the device if the reserved
> > region overlaps existing guest RAM, but I don't even really see how we
> > advise users to give them a reasonable chance of avoiding that
> > possibility. Apparently there are also ARM platforms where MSI pages
> > cannot be remapped to support the previous programmable user/VM
> > address, is it even worthwhile to support those platforms? Does that
> > decision influence whether user programmable MSI reserved regions are
> > really a second class citizen to fixed reserved regions? I expect
> > we'll be talking about this tomorrow morning, but I certainly haven't
> > come up with any viable solutions to this. Thanks,
>
> At LPC last week, we discussed guest MSIs on arm64 as part of the PCI
> microconference. I presented some slides to illustrate some of the issues
> we're trying to solve:
>
> http://www.willdeacon.ukfsn.org/bitbucket/lpc-16/msi-in-guest-arm64.pdf
>
> Punit took some notes (thanks!) on the etherpad here:
>
> https://etherpad.openstack.org/p/LPC2016_PCI
>
> although the discussion was pretty lively and jumped about, so I've had
> to go from memory where the notes didn't capture everything that was
> said.
>
> To summarise, arm64 platforms differ in their handling of MSIs when compared
> to x86:
>
> 1. The physical memory map is not standardised (Jon pointed out that
> this is something that was realised late on)
> 2. MSIs are usually treated the same as DMA writes, in that they must be
> mapped by the SMMU page tables so that they target a physical MSI
> doorbell
> 3. On some platforms, MSIs bypass the SMMU entirely (e.g. due to an MSI
> doorbell built into the PCI RC)
> 4. Platforms typically have some set of addresses that abort before
> reaching the SMMU (e.g. because the PCI identifies them as P2P).
>
> All of this means that userspace (QEMU) needs to identify the memory
> regions corresponding to points (3) and (4) and ensure that they are
> not allocated in the guest physical (IPA) space. For platforms that can
> remap the MSI doorbell as in (2), then some space also needs to be
> allocated for that.
>
> Rather than treat these as separate problems, a better interface is to
> tell userspace about a set of reserved regions, and have this include
> the MSI doorbell, irrespective of whether or not it can be remapped.
Is my understanding correct, that you need to tell userspace about the
location of the doorbell (in the IOVA space) in case (2), because even
though the configuration of the device is handled by the (host) kernel
through trapping of the BARs, we have to avoid the VFIO user programming
the device to create other DMA transactions to this particular address,
since that will obviously conflict and either not produce the desired
DMA transactions or result in unintended weird interrupts?
Thanks,
Christoffer
^ permalink raw reply
* [PATCH 3/3] ARM: dts: da850: add usb device node
From: kbuild test robot @ 2016-11-08 20:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108185831.17683-4-ahaslam@baylibre.com>
Hi Axel,
[auto build test ERROR on usb/usb-testing]
[also build test ERROR on next-20161108]
[cannot apply to robh/for-next v4.9-rc4]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Axel-Haslam/USB-ohci-da8xx-Add-devicetree-bindings/20161109-031338
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
config: arm-at91_dt_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm
All errors (new ones prefixed by >>):
>> Error: arch/arm/boot/dts/da850-lcdk.dts:89.1-9 Label or path usb_phy not found
FATAL ERROR: Syntax error parsing input tree
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/gzip
Size: 21781 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161109/5e5ea637/attachment-0001.gz>
^ permalink raw reply
* [PATCH] drm/sun4i: Fix error handling
From: Maxime Ripard @ 2016-11-08 20:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1d4a8fc9-6b62-4af7-19bc-565b15cdc413@wanadoo.fr>
Salut,
On Sat, Nov 05, 2016 at 07:15:45AM +0100, Christophe JAILLET wrote:
> Le 02/11/2016 ? 19:14, Maxime Ripard a ?crit :
> > Hi,
> >
> > On Sun, Oct 30, 2016 at 12:53:02PM +0100, Christophe JAILLET wrote:
> > > BTW, memory allocation in 'sun4i_layers_init()' looks spurious, especially
> > > the use of 'layer' in the for loop.
> > > Just my 2 cents.
> > What do you mean by it's spurious?
> Hi Maxime,
>
> what I mean is:
>
> > struct sun4i_layer **sun4i_layers_init(struct drm_device *drm)
> > {
> > struct sun4i_drv *drv = drm->dev_private;
> > struct sun4i_layer **layers;
> > int i;
> >
> > layers = devm_kcalloc(drm->dev, ARRAY_SIZE(sun4i_backend_planes),
> > sizeof(**layers), GFP_KERNEL);
> Here, we allocate some memory for ARRAY_SIZE(sun4i_backend_planes) (i.e. 2)
> 'struct sun4i_layer'.
> We do not allocate some space for some pointers but for some structure.
>
> Also, these structures are zeroed and seem to never be initialized by
> anything else.
>
> > if (!layers)
> > return ERR_PTR(-ENOMEM);
> >
> > /*
> > * The hardware is a bit unusual here.
> > *
> > * Even though it supports 4 layers, it does the composition
> > * in two separate steps.
> > *
> > * The first one is assigning a layer to one of its two
> > * pipes. If more that 1 layer is assigned to the same pipe,
> > * and if pixels overlaps, the pipe will take the pixel from
> > * the layer with the highest priority.
> > *
> > * The second step is the actual alpha blending, that takes
> > * the two pipes as input, and uses the eventual alpha
> > * component to do the transparency between the two.
> > *
> > * This two steps scenario makes us unable to guarantee a
> > * robust alpha blending between the 4 layers in all
> > * situations. So we just expose two layers, one per pipe. On
> > * SoCs that support it, sprites could fill the need for more
> > * layers.
> > */
> The comment make me think that this driver (and this function) only handles
> 2 layers ("So we just expose two layers"), which is consistent with
> ARRAY_SIZE(sun4i_backend_planes) (i.e. 2)
> So I would expect that only 2 'struct sun4i_layer' to be allcoated
>
> > for (i = 0; i < ARRAY_SIZE(sun4i_backend_planes); i++) {
> > const struct sun4i_plane_desc *plane = &sun4i_backend_planes[i];
> > struct sun4i_layer *layer = layers[i];
> Here, we take the address of one of the 2 structure allocated above.
> This is overridden, just the line after.
>
> >
> > layer = sun4i_layer_init_one(drm, plane);
> 'sun4i_layer_init_one()' looks() like:
>
> struct sun4i_layer *layer;
> layer = devm_kzalloc(drm->dev, sizeof(*layer), GFP_KERNEL);
> ...
> return layer;
>
> So we once more allocate some 'struct sun4i_layer'
>
> BUT, the corresponding address is stored into the 'layer' variable, and
> finally seems to get lost and no reference is kept of this. (i.e. 'layers'
> (with an s) is left unchanged)
>
> > if (IS_ERR(layer)) {
> > dev_err(drm->dev, "Couldn't initialize %s plane\n",
> > i ? "overlay" : "primary");
> > return ERR_CAST(layer);
> > };
> >
> > DRM_DEBUG_DRIVER("Assigning %s plane to pipe %d\n",
> > i ? "overlay" : "primary", plane->pipe);
> > regmap_update_bits(drv->backend->regs,
> SUN4I_BACKEND_ATTCTL_REG0(i),
> > SUN4I_BACKEND_ATTCTL_REG0_LAY_PIPESEL_MASK,
> > SUN4I_BACKEND_ATTCTL_REG0_LAY_PIPESEL(plane->pipe));
> >
> > layer->id = i;
> > };
> >
> > return layers;
> > }
>
>
> So, 4 'struct sun4i_layer' have been allocated. 2 in 'sun4i_layers_init()'
> and 2 in 'sun4i_layer_init_one()' (once at a time, but called twice)
>
> What looks spurious to me is either:
> - 'struct sun4i_layer *layer = layers[i];' which should just be 'struct
> sun4i_layer *layer;'
> or
> - 'layers' (with an s) should be an array of pointers and the addresses
> returned by 'sun4i_layer_init_one()' should be saved there.
>
>
> I don't know at all this driver, so I'm maybe completely wrong.
> What I would have expected would be something like: (un-tested, just to give
> an idea)
>
>
> ==============8<================================================
>
> @@ -133,9 +133,9 @@ struct sun4i_layer **sun4i_layers_init(struct drm_device
> *drm)
> struct sun4i_layer **layers;
> int i;
>
> layers = devm_kcalloc(drm->dev, ARRAY_SIZE(sun4i_backend_planes),
> - sizeof(**layers), GFP_KERNEL);
> + sizeof(*layers), GFP_KERNEL);
> if (!layers)
> return ERR_PTR(-ENOMEM);
>
> /*
> @@ -160,16 +160,17 @@ struct sun4i_layer **sun4i_layers_init(struct
> drm_device *drm)
> * layers.
> */
> for (i = 0; i < ARRAY_SIZE(sun4i_backend_planes); i++) {
> const struct sun4i_plane_desc *plane = &sun4i_backend_planes[i];
> - struct sun4i_layer *layer = layers[i];
> + struct sun4i_layer *layer;
>
> layer = sun4i_layer_init_one(drm, plane);
> if (IS_ERR(layer)) {
> dev_err(drm->dev, "Couldn't initialize %s plane\n",
> i ? "overlay" : "primary");
> return ERR_CAST(layer);
> };
> + layers[i] = layer;
>
> DRM_DEBUG_DRIVER("Assigning %s plane to pipe %d\n",
> i ? "overlay" : "primary", plane->pipe);
> regmap_update_bits(drv->backend->regs,
> SUN4I_BACKEND_ATTCTL_REG0(i),
You're totally right. Can you send this as a formal patch?
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161108/2ed78f33/attachment.sig>
^ permalink raw reply
* [PATCH 3/3] ARM: gr8: evb: Add i2s codec
From: Maxime Ripard @ 2016-11-08 20:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAGb2v6557B5dooERbKc709Hbrzb3vP8eoSj3nh6rKek7UwrgEQ@mail.gmail.com>
On Tue, Nov 08, 2016 at 03:57:48PM +0800, Chen-Yu Tsai wrote:
> On Tue, Nov 8, 2016 at 3:44 PM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > On Mon, Nov 07, 2016 at 10:11:45PM +0800, Chen-Yu Tsai wrote:
> >> On Mon, Nov 7, 2016 at 9:08 PM, Maxime Ripard
> >> <maxime.ripard@free-electrons.com> wrote:
> >> > The GR8-EVB comes with a wm8978 codec connected to the i2s bus.
> >> >
> >> > Add a card in order to have it working
> >> >
> >> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> >> > ---
> >> > arch/arm/boot/dts/ntc-gr8-evb.dts | 14 ++++++++++++++
> >> > 1 file changed, 14 insertions(+), 0 deletions(-)
> >> >
> >> > diff --git a/arch/arm/boot/dts/ntc-gr8-evb.dts b/arch/arm/boot/dts/ntc-gr8-evb.dts
> >> > index 12b4317a4383..5291e425caf9 100644
> >> > --- a/arch/arm/boot/dts/ntc-gr8-evb.dts
> >> > +++ b/arch/arm/boot/dts/ntc-gr8-evb.dts
> >> > @@ -76,6 +76,20 @@
> >> > default-brightness-level = <8>;
> >> > };
> >> >
> >> > + i2s {
> >>
> >> "sound" might be a better node name? The I2S controllers are also
> >> named "i2s".
> >
> > I know, but we also had the codec and SPDIF on this board, so sound
> > was too generic to be meaningful I guess. I don't really care about
> > the name though, if you have any suggestion...
>
> Well people seem to use "sound" for the sound card nodes...
>
> What about "sound-analog" for this one, and "sound-spdif" for the
> SPDIF simple card? Or "analog-sound" and "spdif-sound" if that looks
> better.
sound-analog works for me. I'll either fix it in the v2, or while
applying.
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161108/68ade2b6/attachment.sig>
^ permalink raw reply
* [PATCHv2] ARM: dts: socfpga: add specific compatible strings for boards
From: Dinh Nguyen @ 2016-11-08 20:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161101222352.GA6328@localhost>
On 11/01/2016 05:23 PM, Olof Johansson wrote:
> On Tue, Nov 01, 2016 at 03:56:52PM -0500, Dinh Nguyen wrote:
>> Add a more specific board compatible entry for all of the SOCFPGA
>> Cyclone 5 based boards.
>>
[snip]
>> / {
>> model = "Altera SOCFPGA Cyclone V SoC Development Kit";
>> - compatible = "altr,socfpga-cyclone5", "altr,socfpga";
>> + compatible = "altr,socdk", "altr,socfpga-cyclone5", "altr,socfpga";
>
> This looks a little too generic, what if there's another dk with another
> SoC down the road?
>
Right...I'll change it to "altr,socfpga-cyclone5-socdk",
>> chosen {
>> bootargs = "earlyprintk";
>> diff --git a/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts b/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts
>> index 02e22f5..c5623a7 100644
>> --- a/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts
>> +++ b/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts
>> @@ -19,7 +19,7 @@
>>
>> / {
>> model = "Terasic SoCkit";
>> - compatible = "altr,socfpga-cyclone5", "altr,socfpga";
>> + compatible = "terasic,sockit", "altr,socfpga-cyclone5", "altr,socfpga";
>
> Same thing here, this seems a bit on the generic side.
>
perhaps "terasic,socfpga-cyclone5-sockit" ?
Thanks for reviewing.
Dinh
^ permalink raw reply
* [alsa-devel] [PATCH 2/9] ALSA: ac97: add an ac97 bus
From: Robert Jarzmik @ 2016-11-08 21:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <d57cdfba-da22-652d-3d75-6d1b754cf1d5@metafoo.de>
Lars-Peter Clausen <lars@metafoo.de> writes:
> On 10/26/2016 09:41 PM, Robert Jarzmik wrote:
>> +#define to_ac97_device(d) container_of(d, struct ac97_codec_device, dev)
>> +#define to_ac97_driver(d) container_of(d, struct ac97_codec_driver, driver)
>
> In my opinion these should be inline functions rather than macros as that
> generates much more legible compiler errors e.g. in case there is a type
> mismatch.
Sure, why not.
>> +struct ac97_codec_driver {
>> + struct device_driver driver;
>> + int (*probe)(struct ac97_codec_device *);
>> + int (*remove)(struct ac97_codec_device *);
>> + int (*suspend)(struct ac97_codec_device *);
>> + int (*resume)(struct ac97_codec_device *);
>> + void (*shutdown)(struct ac97_codec_device *);
>
> The suspend, resume and shutdown callbacks are never used. Which is good,
> since all new frameworks should use dev_pm_ops. Just drop the from the struct.
Ok.
>> +/**
>> + * struct ac97_controller - The AC97 controller of the AC-Link
>> + * @ops: the AC97 operations.
>> + * @controllers: linked list of all existing controllers.
>> + * @dev: the device providing the AC97 controller.
>> + * @slots_available: the mask of accessible/scanable codecs.
>> + * @codecs: the 4 possible AC97 codecs (NULL if none found).
>> + * @codecs_pdata: platform_data for each codec (NULL if no pdata).
>> + *
>> + * This structure is internal to AC97 bus, and should not be used by the
>> + * controllers themselves, excepting for using @dev.
>> + */
>> +struct ac97_controller {
>> + const struct ac97_controller_ops *ops;
>> + struct list_head controllers;
>> + struct device *dev;
>
> I'd make the controller itself a struct dev, rather than just having the
> pointer to the parent. This is more idiomatic and matches what other
> subsystems do. It has several advantages, you get proper refcounting of your
> controller structure, the controller gets its own sysfs directory where the
> CODECs appear as children, you don't need the manual sysfs attribute
> creation and removal in ac97_controler_{un,}register().
If you mean having "struct device dev" instead of "struct device *dev", it has
also a drawback : all the legacy platforms have already a probing method, be
that platform data, device-tree or something else.
I'm a bit converned about the conversion toll. Maybe I've not understood your
point fully, so please feel free to explain, with an actual example even better.
>> + void (*wait)(struct ac97_controller *adrv, int slot);
>> + void (*init)(struct ac97_controller *adrv, int slot);
>
> Neither wait nor init are ever used.
This is because I've not begun to porting sound/pci/ac97_codec.c into
sound/ac97.
>> + if ((codec_num < 0) || (codec_num >= AC97_BUS_MAX_CODECS))
>> + return ERR_PTR(-ERANGE);
>
> I'd make this EINVAL.
Ok.
>> + adev = container_of(dev, struct ac97_codec_device, dev);
>
> to_ac97_device()
Sure.
>> + codec_name = kasprintf(GFP_KERNEL, "%s:%d", dev_name(ac97_ctrl->dev),
>> + idx);
>> + codec->dev.init_name = codec_name;
>
> init_name is only for statically allocated devices. Use dev_set_name(dev,
> ...). No need for kasprintf() either as dev_set_name() takes a format string.
I'll try again, I seem to remember having tried that and it failed, but I can't
remember why.
> For this you need to split device_register into device_initialize() and
> device_add(). But usually that is what you want anyway.
Let me try again.
>> + ret = sysfs_create_link(&codec->dev.kobj, &ac97_ctrl->dev->kobj,
>> + "ac97_controller");
>
> Since the CODEC is a child of the controller this should not be necessary as
> this just points one directory up. It's like `ln -s .. parent`
This creates :
/sys/bus/ac97/devices/pxa2xx-ac97\:0/ac97_controller
Of course as you pointed out, it's a 'ln -s .. parent' like link, but it seems
to me very unfriendly to have :
- /sys/bus/ac97/devices/pxa2xx-ac97\:0/../ac97_operations
- while /sys/bus/ac97/devices/ac97_operations doesn't exist (obviously)
Mmm, I don't know for this one, my mind is not set, it's a bit hard to tell if
it's only an unecessary link bringing "comfort", or something usefull.
>
>> + if (ret)
>> + goto err_unregister_device;
>> +
>> + return 0;
>> +err_unregister_device:
>> + put_device(&codec->dev);
>> +err_free_codec:
>> + kfree(codec);
>
> Since the struct is reference counted, the freeing is done in the release
> callback and this leads to a double free.
A yes in the "goto err_unregister_device" case right, while it's necessary in
the "goto err_free_codec" case ... I need to rework that a bit.
>> +int snd_ac97_codec_driver_register(struct ac97_codec_driver *drv)
>> +{
>> + int ret;
>> +
>> + drv->driver.bus = &ac97_bus_type;
>> +
>> + ret = driver_register(&drv->driver);
>> + if (!ret)
>> + ac97_rescan_all_controllers();
>
> Rescanning the bus when a new codec driver is registered should not be
> neccessary. The bus is scanned once when the controller is registered, this
> creates the device. The device driver core will take care of binding the
> device to the driver, if the driver is registered after thed evice.
That's because you suppose the initial scanning found all the ac97 codecs.
But that's an incomplete vision as a codec might be powered off at that time and
not seen by the first scanning, while the new scanning will discover it.
>> +static int ac97_ctrl_codecs_unregister(struct ac97_controller *ac97_ctrl)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < AC97_BUS_MAX_CODECS; i++)
>> + if (ac97_ctrl->codecs[i])
>> + put_device(&ac97_ctrl->codecs[i]->dev);
>
> This should be device_unregister() to match the device_register() in
> ac97_codec_add().
Right.
>> +
>> + return 0;
>> +}
>> +
>> +static ssize_t cold_reset_store(struct device *dev,
>> + struct device_attribute *attr, const char *buf,
>> + size_t len)
>> +{
>> + struct ac97_controller *ac97_ctrl = ac97_ctrl_find(dev);
>> +
>> + if (!dev)
>> + return -ENODEV;
>
> dev is never NULL here.
Ok.
> And for the ac97_ctrl there is a race condition. It could be unregistered and
> freed after ac97_ctrl_find() returned sucessfully, but before ac97_ctrl->ops
> is used.
A good catch, the ac97_controllers_mutex is missing.
> Same here.
Indeed.
>> +
>> + ac97_ctrl->ops->warm_reset(ac97_ctrl);
>> + return len;
>> +}
>> +static DEVICE_ATTR_WO(warm_reset);
>> +
>> +static struct attribute *ac97_controller_device_attrs[] = {
>> + &dev_attr_cold_reset.attr,
>> + &dev_attr_warm_reset.attr,
>> + NULL
>> +};
>
> This adds new userspace ABI that is not documented at the moment.
Very true. And as all userspace interface, it needs to be discussed. If nobody
complains, I'll add the documentation for next patch round.
>
>> +int snd_ac97_controller_register(const struct ac97_controller_ops *ops,
>> + struct device *dev,
>> + unsigned short slots_available,
>> + void **codecs_pdata)
>
> In my opinion this should return a handle to a ac97 controller which can
> then be passed to snd_ac97_controller_unregister(). This is in my opinion
> the better approach rather than looking up the controller by parent device.
This is another "legacy drivers" issue.
The legacy driver (the one probed by platform_data or devicetree) doesn't
necessarily have a private structure to store this ac97_controller pointer. This
enables an "easier" porting of existing drivers.
>> +/**
>> + * snd_ac97_controller_unregister - unregister an ac97 controller
>> + * @dev: the device previously provided to ac97_controller_register()
>> + *
>> + * Returns 0 on success, negative upon error
>
> Unregister must not be able to fail. Hotunplug is one of the core concepts
> of the device driver model and there is really nothing that can be done to
> prevent a device from disappearing, so there is no sensible way of handling
> the error (and your pxa driver modifications simply ignore it as well).
>
> This also means the framework needs to cope with the case where the
> controller is removed and the CODEC devices are still present. All future
> operations should return -ENODEV in that case.
Ah ... that will require a serious modification, and I see your point, so I'll
prepare this for next patchset.
>> +static struct bus_type ac97_bus_type = {
>> + .name = "ac97",
>> + .dev_attrs = ac97_dev_attrs,
>
> dev_attrs is deprecated in favor of dev_groups (See commit 880ffb5c6).
Ok.
>> index 000000000000..a835f03744bf
>> --- /dev/null
>> +++ b/sound/ac97/codec.c
>> @@ -0,0 +1,15 @@
>> +/*
>> + * Copyright (C) 2016 Robert Jarzmik <robert.jarzmik@free.fr>
>> + *
>> + * 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.
>> + */
>> +
>> +#include <sound/ac97_codec.h>
>> +#include <sound/ac97/codec.h>
>> +#include <sound/ac97/controller.h>
>> +#include <linux/device.h>
>> +#include <linux/slab.h>
>> +#include <sound/soc.h> /* For compat_ac97_* */
>> +
>
> I'm not sure I understand what this file does.
Ah yes, I'll remove it.
It's the future conversion of sound/pci/ac97_codec.c, but it's ... empty so far.
Thanks very much for the very detailed review.
--
Robert
^ permalink raw reply
* [PATCHv3] ARM: dts: socfpga: add specific compatible strings for boards
From: Dinh Nguyen @ 2016-11-08 21:50 UTC (permalink / raw)
To: linux-arm-kernel
Add a more specific board compatible entry for all of the SOCFPGA
Cyclone 5 based boards.
Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
---
v3: Be a bit more specific with the c5 dk and sockit, use
"altr,socfpga-cyclone5-socdk" and "terasic,socfpga-cyclone5-sockit"
v2: remove extra space and add a comma between compatible entries
---
arch/arm/boot/dts/socfpga_cyclone5_de0_sockit.dts | 2 +-
arch/arm/boot/dts/socfpga_cyclone5_mcvevk.dts | 2 +-
arch/arm/boot/dts/socfpga_cyclone5_socdk.dts | 2 +-
arch/arm/boot/dts/socfpga_cyclone5_sockit.dts | 2 +-
arch/arm/boot/dts/socfpga_cyclone5_sodia.dts | 2 +-
arch/arm/boot/dts/socfpga_cyclone5_vining_fpga.dts | 2 +-
6 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/arm/boot/dts/socfpga_cyclone5_de0_sockit.dts b/arch/arm/boot/dts/socfpga_cyclone5_de0_sockit.dts
index afea364..5ecd2ef 100644
--- a/arch/arm/boot/dts/socfpga_cyclone5_de0_sockit.dts
+++ b/arch/arm/boot/dts/socfpga_cyclone5_de0_sockit.dts
@@ -18,7 +18,7 @@
/ {
model = "Terasic DE-0(Atlas)";
- compatible = "altr,socfpga-cyclone5", "altr,socfpga";
+ compatible = "terasic,de0-atlas", "altr,socfpga-cyclone5", "altr,socfpga";
chosen {
bootargs = "earlyprintk";
diff --git a/arch/arm/boot/dts/socfpga_cyclone5_mcvevk.dts b/arch/arm/boot/dts/socfpga_cyclone5_mcvevk.dts
index 424523b..e5a98e5 100644
--- a/arch/arm/boot/dts/socfpga_cyclone5_mcvevk.dts
+++ b/arch/arm/boot/dts/socfpga_cyclone5_mcvevk.dts
@@ -19,7 +19,7 @@
/ {
model = "Aries/DENX MCV EVK";
- compatible = "altr,socfpga-cyclone5", "altr,socfpga";
+ compatible = "denx,mcvevk", "altr,socfpga-cyclone5", "altr,socfpga";
aliases {
ethernet0 = &gmac0;
diff --git a/arch/arm/boot/dts/socfpga_cyclone5_socdk.dts b/arch/arm/boot/dts/socfpga_cyclone5_socdk.dts
index 15e43f4..7a5f42d 100644
--- a/arch/arm/boot/dts/socfpga_cyclone5_socdk.dts
+++ b/arch/arm/boot/dts/socfpga_cyclone5_socdk.dts
@@ -19,7 +19,7 @@
/ {
model = "Altera SOCFPGA Cyclone V SoC Development Kit";
- compatible = "altr,socfpga-cyclone5", "altr,socfpga";
+ compatible = "altr,socfpga-cyclone5-socdk", "altr,socfpga-cyclone5", "altr,socfpga";
chosen {
bootargs = "earlyprintk";
diff --git a/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts b/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts
index 02e22f5..fcacaf7b 100644
--- a/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts
+++ b/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts
@@ -19,7 +19,7 @@
/ {
model = "Terasic SoCkit";
- compatible = "altr,socfpga-cyclone5", "altr,socfpga";
+ compatible = "terasic,socfpga-cyclone5-sockit", "altr,socfpga-cyclone5", "altr,socfpga";
chosen {
bootargs = "earlyprintk";
diff --git a/arch/arm/boot/dts/socfpga_cyclone5_sodia.dts b/arch/arm/boot/dts/socfpga_cyclone5_sodia.dts
index 9aaf413..5b7e3c2 100644
--- a/arch/arm/boot/dts/socfpga_cyclone5_sodia.dts
+++ b/arch/arm/boot/dts/socfpga_cyclone5_sodia.dts
@@ -21,7 +21,7 @@
/ {
model = "Altera SOCFPGA Cyclone V SoC Macnica Sodia board";
- compatible = "altr,socfpga-cyclone5", "altr,socfpga";
+ compatible = "macnica,sodia", "altr,socfpga-cyclone5", "altr,socfpga";
chosen {
bootargs = "earlyprintk";
diff --git a/arch/arm/boot/dts/socfpga_cyclone5_vining_fpga.dts b/arch/arm/boot/dts/socfpga_cyclone5_vining_fpga.dts
index b844473..363ee62 100644
--- a/arch/arm/boot/dts/socfpga_cyclone5_vining_fpga.dts
+++ b/arch/arm/boot/dts/socfpga_cyclone5_vining_fpga.dts
@@ -51,7 +51,7 @@
/ {
model = "samtec VIN|ING FPGA";
- compatible = "altr,socfpga-cyclone5", "altr,socfpga";
+ compatible = "samtec,vining", "altr,socfpga-cyclone5", "altr,socfpga";
chosen {
bootargs = "console=ttyS0,115200";
--
2.8.3
^ permalink raw reply related
* KASAN & the vmalloc area
From: Dmitry Vyukov @ 2016-11-08 22:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108190302.GH15297@leverpostej>
On Tue, Nov 8, 2016 at 11:03 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> Hi,
>
> I see a while back [1] there was a discussion of what to do about KASAN
> and vmapped stacks, but it doesn't look like that was solved, judging by
> the vmapped stacks pull [2] for v4.9.
>
> I wondered whether anyone had looked at that since?
>
> I have an additional reason to want to dynamically allocate the vmalloc
> area shadow: it turns out that KASAN currently interacts rather poorly
> with the arm64 ptdump code.
>
> When KASAN is selected, we allocate shadow for the whole vmalloc area,
> using common zero pte, pmd, pud tables. Walking over these in the ptdump
> code takes a *very* long time (I've seen up to 15 minutes with
> KASAN_OUTLINE enabled). For DEBUG_WX [3], this means boot hangs for that
> long, too.
>
> If I don't allocate vmalloc shadow (and remove the apparently pointlesss
> shadow of the shadow area), and only allocate shadow for the image,
> fixmap, vmemmap and so on, that delay gets cut to a few seconds, which
> is tolerable for a debug configuration...
>
> ... however, things blow up when the kernel touches vmalloc'd memory for
> the first time, as we don't install shadow for that dynamically.
I've seen the same iteration slowness problem on x86 with
CONFIG_DEBUG_RODATA which walks all pages. The is about 1 minute, but
it is enough to trigger rcu stall warning.
The zero pud and vmalloc-ed stacks looks like different problems.
To overcome the slowness we could map zero shadow for vmalloc area lazily.
However for vmalloc-ed stacks we need to map actual memory, because
stack instrumentation will read/write into the shadow. One downside
here is that vmalloc shadow can be as large as 1:1 (if we allocate 1
page in vmalloc area we need to allocate 1 page for shadow).
Re slowness: could we just skip the KASAN zero puds (the top level)
while walking? Can they be interesting for anybody? We can just
pretend that they are not there. Looks like a trivial solution for the
problem at hand.
^ 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