* [arm-soc:to-build 1/3] arch/x86/include/asm/thread_info.h:174:11: warning: calling '__builtin_frame_address' with a nonzero argument is unsafe
From: kbuild test robot @ 2016-10-12 22:06 UTC (permalink / raw)
To: linux-arm-kernel
tree: https://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc.git to-build
head: b0a8f2c6d232f3f8463955e5927c56dd057495b1
commit: 3ddcb978f68b765b3af38bf45bf4a1a4b0a87df1 [1/3] Revert "Disable "maybe-uninitialized" warning globally"
config: i386-randconfig-s1-201641 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
git checkout 3ddcb978f68b765b3af38bf45bf4a1a4b0a87df1
# save the attached .config to linux build tree
make ARCH=i386
All warnings (new ones prefixed by >>):
In file included from include/linux/thread_info.h:69:0,
from arch/x86/include/asm/preempt.h:6,
from include/linux/preempt.h:59,
from include/linux/spinlock.h:50,
from include/linux/mmzone.h:7,
from include/linux/gfp.h:5,
from include/linux/mm.h:9,
from mm/usercopy.c:17:
arch/x86/include/asm/thread_info.h: In function 'check_stack_object':
>> arch/x86/include/asm/thread_info.h:174:11: warning: calling '__builtin_frame_address' with a nonzero argument is unsafe [-Wframe-address]
oldframe = __builtin_frame_address(1);
~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~
arch/x86/include/asm/thread_info.h:176:9: warning: calling '__builtin_frame_address' with a nonzero argument is unsafe [-Wframe-address]
frame = __builtin_frame_address(2);
~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~
vim +/__builtin_frame_address +174 arch/x86/include/asm/thread_info.h
0f60a8ef Kees Cook 2016-07-12 158 * Walks up the stack frames to make sure that the specified object is
0f60a8ef Kees Cook 2016-07-12 159 * entirely contained by a single stack frame.
0f60a8ef Kees Cook 2016-07-12 160 *
0f60a8ef Kees Cook 2016-07-12 161 * Returns:
0f60a8ef Kees Cook 2016-07-12 162 * 1 if within a frame
0f60a8ef Kees Cook 2016-07-12 163 * -1 if placed across a frame boundary (or outside stack)
0f60a8ef Kees Cook 2016-07-12 164 * 0 unable to determine (no frame pointers, etc)
0f60a8ef Kees Cook 2016-07-12 165 */
0f60a8ef Kees Cook 2016-07-12 166 static inline int arch_within_stack_frames(const void * const stack,
0f60a8ef Kees Cook 2016-07-12 167 const void * const stackend,
0f60a8ef Kees Cook 2016-07-12 168 const void *obj, unsigned long len)
0f60a8ef Kees Cook 2016-07-12 169 {
0f60a8ef Kees Cook 2016-07-12 170 #if defined(CONFIG_FRAME_POINTER)
0f60a8ef Kees Cook 2016-07-12 171 const void *frame = NULL;
0f60a8ef Kees Cook 2016-07-12 172 const void *oldframe;
0f60a8ef Kees Cook 2016-07-12 173
0f60a8ef Kees Cook 2016-07-12 @174 oldframe = __builtin_frame_address(1);
0f60a8ef Kees Cook 2016-07-12 175 if (oldframe)
0f60a8ef Kees Cook 2016-07-12 176 frame = __builtin_frame_address(2);
0f60a8ef Kees Cook 2016-07-12 177 /*
0f60a8ef Kees Cook 2016-07-12 178 * low ----------------------------------------------> high
0f60a8ef Kees Cook 2016-07-12 179 * [saved bp][saved ip][args][local vars][saved bp][saved ip]
0f60a8ef Kees Cook 2016-07-12 180 * ^----------------^
0f60a8ef Kees Cook 2016-07-12 181 * allow copies only within here
0f60a8ef Kees Cook 2016-07-12 182 */
:::::: The code at line 174 was first introduced by commit
:::::: 0f60a8efe4005ab5e65ce000724b04d4ca04a199 mm: Implement stack frame object validation
:::::: TO: Kees Cook <keescook@chromium.org>
:::::: CC: Kees Cook <keescook@chromium.org>
---
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: 24527 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161013/8e639a65/attachment-0001.gz>
^ permalink raw reply
* [PATCH V3 09/10] trace, ras: add ARM processor error trace event
From: Baicar, Tyler @ 2016-10-12 21:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161007173959.5ff3fcfa@gandalf.local.home>
Hello Steve,
Thank you for your feedback! Responses below.
On 10/7/2016 3:39 PM, Steven Rostedt wrote:
> On Fri, 7 Oct 2016 15:31:21 -0600
> Tyler Baicar <tbaicar@codeaurora.org> wrote:
>
>> Currently there are trace events for the various RAS
>> errors with the exception of ARM processor type errors.
>> Add a new trace event for such errors so that the user
>> will know when they occur. These trace events are
>> consistent with the ARM processor error section type
>> defined in UEFI 2.6 spec section N.2.4.4.
>>
>> Signed-off-by: Tyler Baicar <tbaicar@codeaurora.org>
>> ---
>> drivers/firmware/efi/cper.c | 9 ++++++
>> drivers/ras/ras.c | 1 +
>> include/ras/ras_event.h | 67 +++++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 77 insertions(+)
>>
>> diff --git a/drivers/firmware/efi/cper.c b/drivers/firmware/efi/cper.c
>> index f9ffba6..21b8a6f 100644
>> --- a/drivers/firmware/efi/cper.c
>> +++ b/drivers/firmware/efi/cper.c
>> @@ -34,6 +34,7 @@
>> #include <linux/aer.h>
>> #include <linux/printk.h>
>> #include <linux/bcd.h>
>> +#include <ras/ras_event.h>
>>
>> #define INDENT_SP " "
>>
>> @@ -256,6 +257,14 @@ static void cper_print_proc_armv8(const char *pfx,
>> CPER_ARMV8_INFO_VALID_PHYSICAL_ADDR)
>> printk("%sphysical fault address: 0x%016llx\n",
>> newpfx, err_info->physical_fault_addr);
>> + trace_arm_event(proc->affinity_level, proc->mpidr, proc->midr,
>> + proc->running_state, proc->psci_state,
>> + err_info->version, err_info->type,
>> + err_info->multiple_error,
>> + err_info->validation_bits,
>> + err_info->error_info,
>> + err_info->virt_fault_addr,
>> + err_info->physical_fault_addr);
> Why waste all the effort into passing each individual field. Why not
> just pass the structure in and sort it out in the TP_fast_assign()?
That should be a lot cleaner, I will make that change in the next patchset.
>> err_info += 1;
>> }
>>
>> diff --git a/drivers/ras/ras.c b/drivers/ras/ras.c
>> index fb2500b..8ba5a94 100644
>> --- a/drivers/ras/ras.c
>> +++ b/drivers/ras/ras.c
>> @@ -28,3 +28,4 @@ EXPORT_TRACEPOINT_SYMBOL_GPL(extlog_mem_event);
>> #endif
>> EXPORT_TRACEPOINT_SYMBOL_GPL(mc_event);
>> EXPORT_TRACEPOINT_SYMBOL_GPL(unknown_sec_event);
>> +EXPORT_TRACEPOINT_SYMBOL_GPL(arm_event);
>> diff --git a/include/ras/ras_event.h b/include/ras/ras_event.h
>> index 5861b6f..eb2719a 100644
>> --- a/include/ras/ras_event.h
>> +++ b/include/ras/ras_event.h
>> @@ -162,6 +162,73 @@ TRACE_EVENT(mc_event,
>> );
>>
>> /*
>> + * ARM Processor Events Report
>> + *
>> + * This event is generated when hardware detects an ARM processor error
>> + * has occurred. UEFI 2.6 spec section N.2.4.4.
>> + */
>> +TRACE_EVENT(arm_event,
>> +
>> + TP_PROTO(const u8 affinity,
>> + const u64 mpidr,
>> + const u64 midr,
>> + const u32 running_state,
>> + const u32 psci_state,
>> + const u8 version,
>> + const u8 type,
>> + const u16 err_count,
>> + const u8 flags,
>> + const u64 info,
>> + const u64 virt_fault_addr,
>> + const u64 phys_fault_addr),
>> +
>> + TP_ARGS(affinity, mpidr, midr, running_state, psci_state,
>> + version, type, err_count, flags, info, virt_fault_addr,
>> + phys_fault_addr),
>> +
>> + TP_STRUCT__entry(
>> + __field(u8, affinity)
>> + __field(u64, mpidr)
>> + __field(u64, midr)
>> + __field(u32, running_state)
>> + __field(u32, psci_state)
>> + __field(u8, version)
>> + __field(u8, type)
>> + __field(u16, err_count)
>> + __field(u8, flags)
>> + __field(u64, info)
>> + __field(u64, virt_fault_addr)
>> + __field(u64, phys_fault_addr)
> The above creates a structure with lots of holes in it. Pack it better.
> You want something like:
>
> __field(u64, mpidr)
> __field(u64, midr)
> __field(u64, info)
> __field(u64, virt_fault_addr)
> __field(u64, phys_fault_addr)
> __field(u32, running_state)
> __field(u32, psci_state)
> __field(u16, err_count)
> __field(u8, affinity)
> __field(u8, version)
> __field(u8, type)
> __field(u8, flags)
>
> The above is a total of 54 bytes. Your original was at a minimum, 64
> bytes.
>
> -- Steve
I will reorder the structure in the next patchset. I originally used
this order because that is the order the entries appear in the spec
(table 260 and 261 of UEFI spec 2.6). It makes more sense to save the
space though.
Thanks,
Tyler
>> + ),
>> +
>> + TP_fast_assign(
>> + __entry->affinity = affinity;
>> + __entry->mpidr = mpidr;
>> + __entry->midr = midr;
>> + __entry->running_state = running_state;
>> + __entry->psci_state = psci_state;
>> + __entry->version = version;
>> + __entry->type = type;
>> + __entry->err_count = err_count;
>> + __entry->flags = flags;
>> + __entry->info = info;
>> + __entry->virt_fault_addr = virt_fault_addr;
>> + __entry->phys_fault_addr = phys_fault_addr;
>> + ),
>> +
>> + TP_printk("affinity level: %d; MPIDR: %016llx; MIDR: %016llx; "
>> + "running state: %d; PSCI state: %d; version: %d; type: %d; "
>> + "error count: 0x%04x; flags: 0x%02x; info: %016llx; "
>> + "virtual fault address: %016llx; "
>> + "physical fault address: %016llx",
>> + __entry->affinity, __entry->mpidr, __entry->midr,
>> + __entry->running_state, __entry->psci_state, __entry->version,
>> + __entry->type, __entry->err_count, __entry->flags,
>> + __entry->info, __entry->virt_fault_addr,
>> + __entry->phys_fault_addr)
>> +);
>> +
>> +/*
>> * Unknown Section Report
>> *
>> * This event is generated when hardware detected a hardware
--
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] drm/sun4i: Check that the plane coordinates are not negative
From: Maxime Ripard @ 2016-10-12 21:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161003181840.0cb369d8@bbrezillon>
On Mon, Oct 03, 2016 at 06:18:40PM +0200, Boris Brezillon wrote:
> On Mon, 3 Oct 2016 14:58:11 +0200
> Maxime Ripard <maxime.ripard@free-electrons.com> wrote:
>
> > Hi Boris,
> >
> > On Fri, Sep 30, 2016 at 06:08:26PM +0200, Boris Brezillon wrote:
> > > On Fri, 30 Sep 2016 16:33:20 +0200
> > > Maxime Ripard <maxime.ripard@free-electrons.com> wrote:
> > >
> > > > Our planes cannot be set at negative coordinates. Make sure we reject such
> > > > configuration.
> > > >
> > > > Reported-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> > > > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> > > > ---
> > > > drivers/gpu/drm/sun4i/sun4i_layer.c | 3 +++
> > > > 1 file changed, 3 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/sun4i/sun4i_layer.c b/drivers/gpu/drm/sun4i/sun4i_layer.c
> > > > index f0035bf5efea..f5463c4c2cde 100644
> > > > --- a/drivers/gpu/drm/sun4i/sun4i_layer.c
> > > > +++ b/drivers/gpu/drm/sun4i/sun4i_layer.c
> > > > @@ -29,6 +29,9 @@ struct sun4i_plane_desc {
> > > > static int sun4i_backend_layer_atomic_check(struct drm_plane *plane,
> > > > struct drm_plane_state *state)
> > > > {
> > > > + if ((state->crtc_x < 0) || (state->crtc_y < 0))
> > > > + return -EINVAL;
> > > > +
> > >
> > > Hm, I think it's a perfectly valid use case from the DRM framework and
> > > DRM user PoV: you may want to place your plane at a negative CRTC
> > > offset (which means part of the plane will be hidden).
> > >
> > > Maybe I'm wrong, but it seems you can support that by adapting the
> > > start address of your framebuffer pointer and the layer size.
> >
> > Indeed, that would probably work. This is even somewhat what we've
> > been using to implement the VGA hack we use on the CHIP.
> >
> > Can you send that patch?
>
> Actually, Ville suggested a slightly different approach: use the
> ->src and ->dst in drm_plane_state.
After more tests, it turns out that it's actually simpler than
that....
The hardware is able to take negative coordinates.
Sorry for the noise,
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: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161012/14a89d77/attachment.sig>
^ permalink raw reply
* [RFC] arm64: Enforce observed order for spinlock and data
From: bdegraaf at codeaurora.org @ 2016-10-12 20:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <a0aa2dba916aafb188f8b1b11aca1599@codeaurora.org>
On 2016-10-05 11:30, bdegraaf at codeaurora.org wrote:
> On 2016-10-05 11:10, Peter Zijlstra wrote:
>> On Wed, Oct 05, 2016 at 10:55:57AM -0400, bdegraaf at codeaurora.org
>> wrote:
>>> On 2016-10-04 15:12, Mark Rutland wrote:
>>> >Hi Brent,
>>> >
>>> >Could you *please* clarify if you are trying to solve:
>>> >
>>> >(a) a correctness issue (e.g. data corruption) seen in practice.
>>> >(b) a correctness issue (e.g. data corruption) found by inspection.
>>> >(c) A performance issue, seen in practice.
>>> >(d) A performance issue, found by inspection.
>>> >
>>> >Any one of these is fine; we just need to know in order to be able to
>>> >help effectively, and so far it hasn't been clear.
>>
>> Brent, you forgot to state which: 'a-d' is the case here.
>>
>>> I found the problem.
>>>
>>> Back in September of 2013, arm64 atomics were broken due to missing
>>> barriers
>>> in certain situations, but the problem at that time was undiscovered.
>>>
>>> Will Deacon's commit d2212b4dce596fee83e5c523400bf084f4cc816c went in
>>> at
>>> that
>>> time and changed the correct cmpxchg64 in lockref.c to
>>> cmpxchg64_relaxed.
>>>
>>> d2212b4 appeared to be OK at that time because the additional barrier
>>> requirements of this specific code sequence were not yet discovered,
>>> and
>>> this change was consistent with the arm64 atomic code of that time.
>>>
>>> Around February of 2014, some discovery led Will to correct the
>>> problem with
>>> the atomic code via commit 8e86f0b409a44193f1587e87b69c5dcf8f65be67,
>>> which
>>> has an excellent explanation of potential ordering problems with the
>>> same
>>> code sequence used by lockref.c.
>>>
>>> With this updated understanding, the earlier commit
>>> (d2212b4dce596fee83e5c523400bf084f4cc816c) should be reverted.
>>>
>>> Because acquire/release semantics are insufficient for the full
>>> ordering,
>>> the single barrier after the store exclusive is the best approach,
>>> similar
>>> to Will's atomic barrier fix.
>>
>> This again does not in fact describe the problem.
>>
>> What is the problem with lockref, and how (refer the earlier a-d
>> multiple choice answer) was this found.
>>
>> Now, I have been looking, and we have some idea what you _might_ be
>> alluding to, but please explain which accesses get reordered how and
>> cause problems.
>
> Sorry for the confusion, this was a "b" item (correctness fix based on
> code
> inspection. I had sent an answer to this yesterday, but didn't realize
> that
> it was in a separate, private email thread.
>
> I'll work out the before/after problem scenarios and send them along
> once
> I've hashed them out (it may take a while for me to paint a clear
> picture).
> In the meantime, however, consider that even without the spinlock code
> in
> the picture, lockref needs to treat the cmpxchg as a full system-level
> atomic,
> because multiple agents could access the value in a variety of timings.
> Since
> atomics similar to this are barriered on arm64 since 8e86f0b, the
> access to
> lockref should be similar.
>
> Brent
I am still working through some additional analyses for mixed accesses,
but I
thought I'd send along some sample commit text for the fix as it
currently stands.
Please feel free to comment if you see something that needs
clarification.
Brent
Text:
All arm64 lockref accesses that occur without taking the spinlock must
behave like
true atomics, ensuring successive operations are all done sequentially.
Currently
the lockref accesses, when decompiled, look like the following sequence:
<Lockref "unlocked" Access [A]>
// Lockref "unlocked" (B)
1: ldxr x0, [B] // Exclusive load
<change lock_count B>
stxr w1, x0, [B]
cbnz w1, 1b
<Lockref "unlocked" Access [C]>
Even though access to the lock_count is protected by exclusives, this is
not enough
to guarantee order: The lock_count must change atomically, in order, so
the only
permitted ordering would be:
A -> B -> C
Unfortunately, this is not the case by the letter of the architecture
and, in fact,
the accesses to A and C are not protected by any sort of barrier, and
hence are
permitted to reorder freely, resulting in orderings such as
Bl -> A -> C -> Bs
In this specific scenario, since "change lock_count" could be an
increment, a decrement
or even a set to a specific value, there could be trouble. With more
agents accessing
the lockref without taking the lock, even scenarios where the cmpxchg
passes falsely
can be encountered, as there is no guarantee that the the "old" value
will not match
exactly a newer value due to out-of-order access by a combination of
agents that
increment and decrement the lock_count by the same amount.
Since multiple agents are accessing this without locking the spinlock,
this access
must have the same protections in place as atomics do in the arch's
atomic.h.
Fortunately, the fix is not complicated: merely removing the errant
_relaxed option
on the cmpxchg64 is enough to introduce exactly the same code sequence
justified
in commit 8e86f0b409a44193f1587e87b69c5dcf8f65be67 to fix arm64 atomics.
1: ldxr x0, [B]
<change lock_count>
stlxr w1, x0, [B]
cbnz w1, 1b
dmb ish
^ permalink raw reply
* [PATCH V2 2/3] ACPI, PCI IRQ: add PCI_USING penalty for ISA interrupts
From: Bjorn Helgaas @ 2016-10-12 19:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475615720-31047-3-git-send-email-okaya@codeaurora.org>
On Tue, Oct 04, 2016 at 05:15:18PM -0400, Sinan Kaya wrote:
> The change introduced in commit 103544d86976 ("ACPI,PCI,IRQ: reduce
> resource requirements") removed PCI_USING penalty from
> acpi_pci_link_allocate function as there is no longer a fixed size penalty
> array for both PCI and IRQ interrupts.
>
> We need to add the PCI_USING penalty for ISA interrupts too if the link is
> in use and matches our ISA IRQ number.
>
> Tested-by: Jonathan Liu <net147@gmail.com>
> Tested-by: Ondrej Zary <linux@rainbow-software.org>
> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
> ---
> drivers/acpi/pci_link.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c
> index f3792f4..06c2a11 100644
> --- a/drivers/acpi/pci_link.c
> +++ b/drivers/acpi/pci_link.c
> @@ -620,6 +620,10 @@ static int acpi_pci_link_allocate(struct acpi_pci_link *link)
> acpi_device_bid(link->device));
> return -ENODEV;
> } else {
> + if (link->irq.active < ACPI_MAX_IRQS)
> + acpi_irq_penalty[link->irq.active] +=
> + PIRQ_PENALTY_PCI_USING;
Nit: elsewhere you use ARRAY_SIZE(acpi_irq_penalty), so it'd be nice
to be consistent, e.g.,
if (link->irq.active < ARRAY_SIZE(acpi_irq_penalty))
acpi_irq_penalty[link->irq.active] += PIRQ_PENALTY_PCI_USING;
There are a couple other similar uses in acpi_irq_get_penalty() and
acpi_irq_penalty_init().
> printk(KERN_WARNING PREFIX "%s [%s] enabled at IRQ %d\n",
> acpi_device_name(link->device),
> acpi_device_bid(link->device), link->irq.active);
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v2] arm64: defconfig: enable EEPROM_AT25 config option
From: Scott Branden @ 2016-10-12 18:51 UTC (permalink / raw)
To: linux-arm-kernel
Enable support for on board SPI EEPROM by turning on
CONFIG_EEPROM_AT25. This needs to be on in order to
boot and test the kernel with a static rootfs image
that is not rebuilt everytime the kernel is rebuilt.
Signed-off-by: Scott Branden <scott.branden@broadcom.com>
---
arch/arm64/configs/defconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index eadf485..9955ee1 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -136,6 +136,7 @@ CONFIG_MTD_SPI_NOR=y
CONFIG_BLK_DEV_LOOP=y
CONFIG_BLK_DEV_NBD=m
CONFIG_VIRTIO_BLK=y
+CONFIG_EEPROM_AT25=y
CONFIG_SRAM=y
# CONFIG_SCSI_PROC_FS is not set
CONFIG_BLK_DEV_SD=y
--
2.5.0
^ permalink raw reply related
* [PATCH 2/2] mmc: sdhci-iproc: support standard byte register accesses
From: Scott Branden @ 2016-10-12 18:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1476297352-7812-1-git-send-email-scott.branden@broadcom.com>
Add bytewise register accesses support for newer versions of IPROC
SDHCI controllers.
Previous sdhci-iproc versions of SDIO controllers
(such as Raspberry Pi and Cygnus) only allowed for 32-bit register
accesses.
Signed-off-by: Srinath Mannam <srinath.mannam@broadcom.com>
Signed-off-by: Scott Branden <scott.branden@broadcom.com>
---
drivers/mmc/host/sdhci-iproc.c | 35 +++++++++++++++++++++++++++++++++--
1 file changed, 33 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/host/sdhci-iproc.c b/drivers/mmc/host/sdhci-iproc.c
index 7262466..d7046d6 100644
--- a/drivers/mmc/host/sdhci-iproc.c
+++ b/drivers/mmc/host/sdhci-iproc.c
@@ -143,6 +143,14 @@ static void sdhci_iproc_writeb(struct sdhci_host *host, u8 val, int reg)
}
static const struct sdhci_ops sdhci_iproc_ops = {
+ .set_clock = sdhci_set_clock,
+ .get_max_clock = sdhci_pltfm_clk_get_max_clock,
+ .set_bus_width = sdhci_set_bus_width,
+ .reset = sdhci_reset,
+ .set_uhs_signaling = sdhci_set_uhs_signaling,
+};
+
+static const struct sdhci_ops sdhci_iproc_32only_ops = {
.read_l = sdhci_iproc_readl,
.read_w = sdhci_iproc_readw,
.read_b = sdhci_iproc_readb,
@@ -156,6 +164,28 @@ static const struct sdhci_ops sdhci_iproc_ops = {
.set_uhs_signaling = sdhci_set_uhs_signaling,
};
+static const struct sdhci_pltfm_data sdhci_iproc_cygnus_pltfm_data = {
+ .quirks = SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK,
+ .quirks2 = SDHCI_QUIRK2_ACMD23_BROKEN,
+ .ops = &sdhci_iproc_32only_ops,
+};
+
+static const struct sdhci_iproc_data iproc_cygnus_data = {
+ .pdata = &sdhci_iproc_cygnus_pltfm_data,
+ .caps = ((0x1 << SDHCI_MAX_BLOCK_SHIFT)
+ & SDHCI_MAX_BLOCK_MASK) |
+ SDHCI_CAN_VDD_330 |
+ SDHCI_CAN_VDD_180 |
+ SDHCI_CAN_DO_SUSPEND |
+ SDHCI_CAN_DO_HISPD |
+ SDHCI_CAN_DO_ADMA2 |
+ SDHCI_CAN_DO_SDMA,
+ .caps1 = SDHCI_DRIVER_TYPE_C |
+ SDHCI_DRIVER_TYPE_D |
+ SDHCI_SUPPORT_DDR50,
+ .mmc_caps = MMC_CAP_1_8V_DDR,
+};
+
static const struct sdhci_pltfm_data sdhci_iproc_pltfm_data = {
.quirks = SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK,
.quirks2 = SDHCI_QUIRK2_ACMD23_BROKEN,
@@ -182,7 +212,7 @@ static const struct sdhci_pltfm_data sdhci_bcm2835_pltfm_data = {
.quirks = SDHCI_QUIRK_BROKEN_CARD_DETECTION |
SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK |
SDHCI_QUIRK_MISSING_CAPS,
- .ops = &sdhci_iproc_ops,
+ .ops = &sdhci_iproc_32only_ops,
};
static const struct sdhci_iproc_data bcm2835_data = {
@@ -194,7 +224,8 @@ static const struct sdhci_iproc_data bcm2835_data = {
static const struct of_device_id sdhci_iproc_of_match[] = {
{ .compatible = "brcm,bcm2835-sdhci", .data = &bcm2835_data },
- { .compatible = "brcm,sdhci-iproc-cygnus", .data = &iproc_data },
+ { .compatible = "brcm,sdhci-iproc-cygnus", .data = &iproc_cygnus_data},
+ { .compatible = "brcm,sdhci-iproc", .data = &iproc_data },
{ }
};
MODULE_DEVICE_TABLE(of, sdhci_iproc_of_match);
--
2.5.0
^ permalink raw reply related
* [PATCH 1/2] mmc: sdhci-iproc: Add brcm, sdhci-iproc compat string in bindings document
From: Scott Branden @ 2016-10-12 18:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1476297352-7812-1-git-send-email-scott.branden@broadcom.com>
Adds brcm,sdhci-iproc compat string to DT bindings document for
the iProc SDHCI driver.
Signed-off-by: Anup Patel <anup.patel@broadcom.com>
Signed-off-by: Scott Branden <scott.branden@broadcom.com>
---
Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt b/Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt
index be56d2b..aa58b94 100644
--- a/Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt
+++ b/Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt
@@ -7,6 +7,7 @@ Required properties:
- compatible : Should be one of the following
"brcm,bcm2835-sdhci"
"brcm,sdhci-iproc-cygnus"
+ "brcm,sdhci-iproc"
- clocks : The clock feeding the SDHCI controller.
--
2.5.0
^ permalink raw reply related
* [PATCH 0/2] mmc: sdhci-iproc: Add byte register access support
From: Scott Branden @ 2016-10-12 18:35 UTC (permalink / raw)
To: linux-arm-kernel
Add brcm,sdhci-iproc compat string and code for support of newer versions of
sdhci-iproc controller that allow byte-wise register accesses.
Scott Branden (2):
mmc: sdhci-iproc: Add brcm,sdhci-iproc compat string in bindings
document
mmc: sdhci-iproc: support standard byte register accesses
.../devicetree/bindings/mmc/brcm,sdhci-iproc.txt | 1 +
drivers/mmc/host/sdhci-iproc.c | 35 ++++++++++++++++++++--
2 files changed, 34 insertions(+), 2 deletions(-)
--
2.5.0
^ permalink raw reply
* [PATCH V3 05/10] acpi: apei: handle SEA notification type for ARMv8
From: Punit Agrawal @ 2016-10-12 18:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475875882-2604-6-git-send-email-tbaicar@codeaurora.org>
Tyler Baicar <tbaicar@codeaurora.org> writes:
> ARM APEI extension proposal added SEA (Synchrounous External
> Abort) notification type for ARMv8.
> Add a new GHES error source handling function for SEA. If an error
> source's notification type is SEA, then this function can be registered
> into the SEA exception handler. That way GHES will parse and report
> SEA exceptions when they occur.
>
> Signed-off-by: Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org>
> Signed-off-by: Tyler Baicar <tbaicar@codeaurora.org>
> Signed-off-by: Naveen Kaje <nkaje@codeaurora.org>
This patch fails to apply for me on v4.8. Is there a different tree this
is based on?
One comment below.
[...]
> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> index c8488f1..28d5a09 100644
> --- a/drivers/acpi/apei/ghes.c
> +++ b/drivers/acpi/apei/ghes.c
> @@ -50,6 +50,10 @@
> #include <acpi/apei.h>
> #include <asm/tlbflush.h>
>
> +#ifdef CONFIG_HAVE_ACPI_APEI_SEA
> +#include <asm/system_misc.h>
> +#endif
> +
> #include "apei-internal.h"
>
> #define GHES_PFX "GHES: "
> @@ -779,6 +783,62 @@ static struct notifier_block ghes_notifier_sci = {
> .notifier_call = ghes_notify_sci,
> };
>
> +#ifdef CONFIG_HAVE_ACPI_APEI_SEA
> +static LIST_HEAD(ghes_sea);
> +
> +static int ghes_notify_sea(struct notifier_block *this,
> + unsigned long event, void *data)
> +{
> + struct ghes *ghes;
> + int ret = NOTIFY_DONE;
> +
> + rcu_read_lock();
> + list_for_each_entry_rcu(ghes, &ghes_sea, list) {
> + if (!ghes_proc(ghes))
> + ret = NOTIFY_OK;
Not something you've introduced but it looks like ghes_proc erroneously
never returns anything other than 0. I plan to post the below fix to
address it.
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index 60746ef..caea575 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -662,7 +662,7 @@ static int ghes_proc(struct ghes *ghes)
ghes_do_proc(ghes, ghes->estatus);
out:
ghes_clear_estatus(ghes);
- return 0;
+ return rc;
}
> + }
> + rcu_read_unlock();
> +
> + return ret;
> +}
> +
[...]
^ permalink raw reply related
* [PATCH V3 04/10] arm64: exception: handle Synchronous External Abort
From: Punit Agrawal @ 2016-10-12 17:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475875882-2604-5-git-send-email-tbaicar@codeaurora.org>
Hi Tyler,
A couple of hopefully not bike shedding comments below.
Tyler Baicar <tbaicar@codeaurora.org> writes:
> SEA exceptions are often caused by an uncorrected hardware
> error, and are handled when data abort and instruction abort
> exception classes have specific values for their Fault Status
> Code.
> When SEA occurs, before killing the process, go through
> the handlers registered in the notification list.
> Update fault_info[] with specific SEA faults so that the
> new SEA handler is used.
>
> Signed-off-by: Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org>
> Signed-off-by: Tyler Baicar <tbaicar@codeaurora.org>
> Signed-off-by: Naveen Kaje <nkaje@codeaurora.org>
> ---
> arch/arm64/include/asm/system_misc.h | 13 ++++++++
> arch/arm64/mm/fault.c | 58 +++++++++++++++++++++++++++++-------
> 2 files changed, 61 insertions(+), 10 deletions(-)
>
> diff --git a/arch/arm64/include/asm/system_misc.h b/arch/arm64/include/asm/system_misc.h
> index 57f110b..90daf4a 100644
> --- a/arch/arm64/include/asm/system_misc.h
> +++ b/arch/arm64/include/asm/system_misc.h
> @@ -64,4 +64,17 @@ extern void (*arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd);
>
> #endif /* __ASSEMBLY__ */
>
> +/*
> + * The functions below are used to register and unregister callbacks
> + * that are to be invoked when a Synchronous External Abort (SEA)
> + * occurs. An SEA is raised by certain fault status codes that have
> + * either data or instruction abort as the exception class, and
> + * callbacks may be registered to parse or handle such hardware errors.
> + *
> + * Registered callbacks are run in an interrupt/atomic context. They
> + * are not allowed to block or sleep.
> + */
> +int sea_register_handler_chain(struct notifier_block *nb);
> +void sea_unregister_handler_chain(struct notifier_block *nb);
> +
> #endif /* __ASM_SYSTEM_MISC_H */
> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
> index 05d2bd7..81cb7ad 100644
> --- a/arch/arm64/mm/fault.c
> +++ b/arch/arm64/mm/fault.c
> @@ -39,6 +39,22 @@
> #include <asm/pgtable.h>
> #include <asm/tlbflush.h>
>
> +/*
> + * GHES SEA handler code may register a notifier call here to
> + * handle HW error record passed from platform.
> + */
> +static ATOMIC_NOTIFIER_HEAD(sea_handler_chain);
> +
> +int sea_register_handler_chain(struct notifier_block *nb)
> +{
> + return atomic_notifier_chain_register(&sea_handler_chain, nb);
> +}
> +
> +void sea_unregister_handler_chain(struct notifier_block *nb)
> +{
> + atomic_notifier_chain_unregister(&sea_handler_chain, nb);
> +}
> +
What do you think of naming the above functions as
[un]register_synchonous_ext_abort_notifier?
For an API, I find "sea" doesn't quite convey the message.
One more comment below.
> static const char *fault_name(unsigned int esr);
>
> #ifdef CONFIG_KPROBES
> @@ -480,6 +496,28 @@ static int do_bad(unsigned long addr, unsigned int esr, struct pt_regs *regs)
> return 1;
> }
>
> +/*
> + * This abort handler deals with Synchronous External Abort.
> + * It calls notifiers, and then returns "fault".
> + */
> +static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs)
> +{
> + struct siginfo info;
> +
> + atomic_notifier_call_chain(&sea_handler_chain, 0, NULL);
> +
> + pr_err("Synchronous External Abort: %s (0x%08x) at 0x%016lx\n",
> + fault_name(esr), esr, addr);
> +
> + info.si_signo = SIGBUS;
> + info.si_errno = 0;
> + info.si_code = 0;
> + info.si_addr = (void __user *)addr;
> + arm64_notify_die("", regs, &info, esr);
> +
> + return 0;
> +}
> +
> static const struct fault_info {
> int (*fn)(unsigned long addr, unsigned int esr, struct pt_regs *regs);
> int sig;
> @@ -502,22 +540,22 @@ static const struct fault_info {
> { do_page_fault, SIGSEGV, SEGV_ACCERR, "level 1 permission fault" },
> { do_page_fault, SIGSEGV, SEGV_ACCERR, "level 2 permission fault" },
> { do_page_fault, SIGSEGV, SEGV_ACCERR, "level 3 permission fault" },
> - { do_bad, SIGBUS, 0, "synchronous external abort" },
> + { do_sea, SIGBUS, 0, "synchronous external abort" },
> { do_bad, SIGBUS, 0, "unknown 17" },
> { do_bad, SIGBUS, 0, "unknown 18" },
> { do_bad, SIGBUS, 0, "unknown 19" },
> - { do_bad, SIGBUS, 0, "synchronous abort (translation table walk)" },
> - { do_bad, SIGBUS, 0, "synchronous abort (translation table walk)" },
> - { do_bad, SIGBUS, 0, "synchronous abort (translation table walk)" },
> - { do_bad, SIGBUS, 0, "synchronous abort (translation table walk)" },
> - { do_bad, SIGBUS, 0, "synchronous parity error" },
> + { do_sea, SIGBUS, 0, "level 0 SEA (trans tbl walk)" },
> + { do_sea, SIGBUS, 0, "level 1 SEA (trans tbl walk)" },
> + { do_sea, SIGBUS, 0, "level 2 SEA (trans tbl walk)" },
> + { do_sea, SIGBUS, 0, "level 3 SEA (trans tbl walk)" },
^^^
The comment about naming applies here as well.
Thanks,
Punit
[...]
^ permalink raw reply
* [PATCH] perf: xgene: Remove bogus IS_ERR() check
From: Tai Nguyen @ 2016-10-12 16:39 UTC (permalink / raw)
To: linux-arm-kernel
This patch fixes the warning issue with static checker.
The bug is reported in [1]
[1] https://www.spinics.net/lists/arm-kernel/msg535957.html
Signed-off-by: Tai Nguyen <ttnguyen@apm.com>
---
drivers/perf/xgene_pmu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/perf/xgene_pmu.c b/drivers/perf/xgene_pmu.c
index c2ac764..a8ac4bc 100644
--- a/drivers/perf/xgene_pmu.c
+++ b/drivers/perf/xgene_pmu.c
@@ -1011,7 +1011,7 @@ xgene_pmu_dev_ctx *acpi_get_pmu_hw_inf(struct xgene_pmu *xgene_pmu,
rc = acpi_dev_get_resources(adev, &resource_list,
acpi_pmu_dev_add_resource, &res);
acpi_dev_free_resource_list(&resource_list);
- if (rc < 0 || IS_ERR(&res)) {
+ if (rc < 0) {
dev_err(dev, "PMU type %d: No resource address found\n", type);
goto err;
}
--
1.9.1
^ permalink raw reply related
* [PATCH v2 0/2] PCI: aardvark: Cleanup
From: Bjorn Helgaas @ 2016-10-12 16:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161012123001.22186.4053.stgit@bhelgaas-glaptop2.roam.corp.google.com>
On Wed, Oct 12, 2016 at 07:36:21AM -0500, Bjorn Helgaas wrote:
> Add local "dev" pointers to reduce repetition of things like "&pdev->dev"
> and remove unused drvdata.
>
> Changes from v1:
> I dropped the following because they added a lot of churn for
> questionable benefit:
> PCI: aardvark: Name private struct pointer "advk" consistently
> PCI: aardvark: Reorder accessor functions
> PCI: aardvark: Swap order of advk_write() reg/val arguments
>
> ---
>
> Bjorn Helgaas (2):
> PCI: aardvark: Add local struct device pointers
> PCI: aardvark: Remove unused platform data
I applied these to pci/host-aardvark for v4.9. I hope to ask Linus to
pull them tomorrow, so if you see any issues, let me know soon.
^ permalink raw reply
* [PATCH V3 01/10] acpi: apei: read ack upon ghes record consumption
From: Punit Agrawal @ 2016-10-12 15:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475875882-2604-2-git-send-email-tbaicar@codeaurora.org>
Hi Tyler,
A few comments below.
Tyler Baicar <tbaicar@codeaurora.org> writes:
> A RAS (Reliability, Availability, Serviceability) controller
> may be a separate processor running in parallel with OS
> execution, and may generate error records for consumption by
> the OS. If the RAS controller produces multiple error records,
> then they may be overwritten before the OS has consumed them.
>
> The Generic Hardware Error Source (GHES) v2 structure
> introduces the capability for the OS to acknowledge the
> consumption of the error record generated by the RAS
> controller. A RAS controller supporting GHESv2 shall wait for
> the acknowledgment before writing a new error record, thus
> eliminating the race condition.
>
> Signed-off-by: Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org>
> Signed-off-by: Richard Ruigrok <rruigrok@codeaurora.org>
> Signed-off-by: Tyler Baicar <tbaicar@codeaurora.org>
> Signed-off-by: Naveen Kaje <nkaje@codeaurora.org>
> ---
> drivers/acpi/apei/ghes.c | 41 +++++++++++++++++++++++++++++++++++++++++
> drivers/acpi/apei/hest.c | 7 +++++--
> include/acpi/ghes.h | 1 +
> 3 files changed, 47 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> index 60746ef..3021f0e 100644
> --- a/drivers/acpi/apei/ghes.c
> +++ b/drivers/acpi/apei/ghes.c
> @@ -45,6 +45,7 @@
> #include <linux/aer.h>
> #include <linux/nmi.h>
>
> +#include <acpi/actbl1.h>
> #include <acpi/ghes.h>
> #include <acpi/apei.h>
> #include <asm/tlbflush.h>
> @@ -244,10 +245,22 @@ static struct ghes *ghes_new(struct acpi_hest_generic *generic)
> struct ghes *ghes;
> unsigned int error_block_length;
> int rc;
> + struct acpi_hest_header *hest_hdr;
>
> ghes = kzalloc(sizeof(*ghes), GFP_KERNEL);
> if (!ghes)
> return ERR_PTR(-ENOMEM);
> +
> + hest_hdr = (struct acpi_hest_header *)generic;
> + if (hest_hdr->type == ACPI_HEST_TYPE_GENERIC_ERROR_V2) {
> + ghes->generic_v2 = (struct acpi_hest_generic_v2 *)generic;
> + rc = apei_map_generic_address(
> + &ghes->generic_v2->read_ack_register);
> + if (rc)
> + goto err_unmap;
> + } else
> + ghes->generic_v2 = NULL;
Since you kzalloc ghes, shouldn't ghes->generic_v2 be NULL already?
> +
> ghes->generic = generic;
> rc = apei_map_generic_address(&generic->error_status_address);
> if (rc)
> @@ -270,6 +283,9 @@ static struct ghes *ghes_new(struct acpi_hest_generic *generic)
>
> err_unmap:
> apei_unmap_generic_address(&generic->error_status_address);
> + if (ghes->generic_v2)
> + apei_unmap_generic_address(
> + &ghes->generic_v2->read_ack_register);
> err_free:
> kfree(ghes);
> return ERR_PTR(rc);
> @@ -279,6 +295,9 @@ static void ghes_fini(struct ghes *ghes)
> {
> kfree(ghes->estatus);
> apei_unmap_generic_address(&ghes->generic->error_status_address);
> + if (ghes->generic_v2)
> + apei_unmap_generic_address(
> + &ghes->generic_v2->read_ack_register);
> }
>
> static inline int ghes_severity(int severity)
> @@ -648,6 +667,22 @@ static void ghes_estatus_cache_add(
> rcu_read_unlock();
> }
>
> +static int ghes_do_read_ack(struct acpi_hest_generic_v2 *generic_v2)
> +{
> + int rc;
> + u64 val = 0;
> +
> + rc = apei_read(&val, &generic_v2->read_ack_register);
> + if (rc)
> + return rc;
> + val &= generic_v2->read_ack_preserve <<
> + generic_v2->read_ack_register.bit_offset;
> + val |= generic_v2->read_ack_write;
Reading the spec, it is not clear whether you need the left shift
above.
Having said that, if you do it for read_ack_preserve, do you also need
to left shift read_ack_write by read_ack_register.bit_offset?
> + rc = apei_write(val, &generic_v2->read_ack_register);
> +
> + return rc;
> +}
> +
> static int ghes_proc(struct ghes *ghes)
> {
> int rc;
> @@ -660,6 +695,12 @@ static int ghes_proc(struct ghes *ghes)
> ghes_estatus_cache_add(ghes->generic, ghes->estatus);
> }
> ghes_do_proc(ghes, ghes->estatus);
> +
> + if (ghes->generic_v2) {
> + rc = ghes_do_read_ack(ghes->generic_v2);
> + if (rc)
> + return rc;
> + }
> out:
> ghes_clear_estatus(ghes);
> return 0;
> diff --git a/drivers/acpi/apei/hest.c b/drivers/acpi/apei/hest.c
> index 792a0d9..ef725a9 100644
> --- a/drivers/acpi/apei/hest.c
> +++ b/drivers/acpi/apei/hest.c
> @@ -52,6 +52,7 @@ static const int hest_esrc_len_tab[ACPI_HEST_TYPE_RESERVED] = {
> [ACPI_HEST_TYPE_AER_ENDPOINT] = sizeof(struct acpi_hest_aer),
> [ACPI_HEST_TYPE_AER_BRIDGE] = sizeof(struct acpi_hest_aer_bridge),
> [ACPI_HEST_TYPE_GENERIC_ERROR] = sizeof(struct acpi_hest_generic),
> + [ACPI_HEST_TYPE_GENERIC_ERROR_V2] = sizeof(struct acpi_hest_generic_v2),
> };
>
> static int hest_esrc_len(struct acpi_hest_header *hest_hdr)
> @@ -146,7 +147,8 @@ static int __init hest_parse_ghes_count(struct acpi_hest_header *hest_hdr, void
> {
> int *count = data;
>
> - if (hest_hdr->type == ACPI_HEST_TYPE_GENERIC_ERROR)
> + if (hest_hdr->type == ACPI_HEST_TYPE_GENERIC_ERROR ||
> + hest_hdr->type == ACPI_HEST_TYPE_GENERIC_ERROR_V2)
> (*count)++;
> return 0;
> }
> @@ -157,7 +159,8 @@ static int __init hest_parse_ghes(struct acpi_hest_header *hest_hdr, void *data)
> struct ghes_arr *ghes_arr = data;
> int rc, i;
>
> - if (hest_hdr->type != ACPI_HEST_TYPE_GENERIC_ERROR)
> + if (hest_hdr->type != ACPI_HEST_TYPE_GENERIC_ERROR &&
> + hest_hdr->type != ACPI_HEST_TYPE_GENERIC_ERROR_V2)
> return 0;
>
> if (!((struct acpi_hest_generic *)hest_hdr)->enabled)
> diff --git a/include/acpi/ghes.h b/include/acpi/ghes.h
> index 720446c..d0108b6 100644
> --- a/include/acpi/ghes.h
> +++ b/include/acpi/ghes.h
> @@ -14,6 +14,7 @@
>
> struct ghes {
> struct acpi_hest_generic *generic;
> + struct acpi_hest_generic_v2 *generic_v2;
You either have a GHES or a GHESv2 structure. Instead of duplication,
could this be represented as a union?
Thanks,
Punit
> struct acpi_hest_generic_status *estatus;
> u64 buffer_paddr;
> unsigned long flags;
^ permalink raw reply
* [bug report] perf: xgene: Add APM X-Gene SoC Performance Monitoring Unit driver
From: Tai Tri Nguyen @ 2016-10-12 15:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161012152326.GB21489@remoulade>
Hi Mark,
Sure I'm working on it.
Regards,
Tai
On Wed, Oct 12, 2016 at 8:23 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> Hi Dan,
>
> Thanks for the report.
>
> On Wed, Oct 12, 2016 at 02:32:29PM +0300, Dan Carpenter wrote:
>> The patch 832c927d119b: "perf: xgene: Add APM X-Gene SoC Performance
>> Monitoring Unit driver" from Jul 15, 2016, leads to the following
>> static checker warning:
>>
>> drivers/perf/xgene_pmu.c:1014 acpi_get_pmu_hw_inf()
>> warn: '&res' isn't an ERR_PTR
>>
>> drivers/perf/xgene_pmu.c
>> 992 static struct
>> 993 xgene_pmu_dev_ctx *acpi_get_pmu_hw_inf(struct xgene_pmu *xgene_pmu,
>> 994 struct acpi_device *adev, u32 type)
>> 995 {
>> 996 struct device *dev = xgene_pmu->dev;
>> 997 struct list_head resource_list;
>> 998 struct xgene_pmu_dev_ctx *ctx;
>> 999 const union acpi_object *obj;
>> 1000 struct hw_pmu_info *inf;
>> 1001 void __iomem *dev_csr;
>> 1002 struct resource res;
>> 1003 int enable_bit;
>> 1004 int rc;
>> 1005
>> 1006 ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
>> 1007 if (!ctx)
>> 1008 return NULL;
>> 1009
>> 1010 INIT_LIST_HEAD(&resource_list);
>> 1011 rc = acpi_dev_get_resources(adev, &resource_list,
>> 1012 acpi_pmu_dev_add_resource, &res);
>> 1013 acpi_dev_free_resource_list(&resource_list);
>> 1014 if (rc < 0 || IS_ERR(&res)) {
>> ^^^^
>> Obviously this is a stack address and not an error pointer. I'm not
>> sure what was intended here.
>
> Urrgh; I should have caught that in review. It's been this way since v1 [1], so
> I can't reverse-engineer the intent. However, I don't believe that it's
> necessary to check anything other than the rc value here.
>
> This should always evaluate as false, and shouldn't result in a functional
> problem, but it is completely bogus.
>
> Tai, can you plase send a patch deleting The IS_ERR() check here?
>
> Thanks,
> Mark.
>
> [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-March/419173.html
--
Tai
^ permalink raw reply
* [bug report] perf: xgene: Add APM X-Gene SoC Performance Monitoring Unit driver
From: Mark Rutland @ 2016-10-12 15:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161012113229.GA16447@mwanda>
Hi Dan,
Thanks for the report.
On Wed, Oct 12, 2016 at 02:32:29PM +0300, Dan Carpenter wrote:
> The patch 832c927d119b: "perf: xgene: Add APM X-Gene SoC Performance
> Monitoring Unit driver" from Jul 15, 2016, leads to the following
> static checker warning:
>
> drivers/perf/xgene_pmu.c:1014 acpi_get_pmu_hw_inf()
> warn: '&res' isn't an ERR_PTR
>
> drivers/perf/xgene_pmu.c
> 992 static struct
> 993 xgene_pmu_dev_ctx *acpi_get_pmu_hw_inf(struct xgene_pmu *xgene_pmu,
> 994 struct acpi_device *adev, u32 type)
> 995 {
> 996 struct device *dev = xgene_pmu->dev;
> 997 struct list_head resource_list;
> 998 struct xgene_pmu_dev_ctx *ctx;
> 999 const union acpi_object *obj;
> 1000 struct hw_pmu_info *inf;
> 1001 void __iomem *dev_csr;
> 1002 struct resource res;
> 1003 int enable_bit;
> 1004 int rc;
> 1005
> 1006 ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> 1007 if (!ctx)
> 1008 return NULL;
> 1009
> 1010 INIT_LIST_HEAD(&resource_list);
> 1011 rc = acpi_dev_get_resources(adev, &resource_list,
> 1012 acpi_pmu_dev_add_resource, &res);
> 1013 acpi_dev_free_resource_list(&resource_list);
> 1014 if (rc < 0 || IS_ERR(&res)) {
> ^^^^
> Obviously this is a stack address and not an error pointer. I'm not
> sure what was intended here.
Urrgh; I should have caught that in review. It's been this way since v1 [1], so
I can't reverse-engineer the intent. However, I don't believe that it's
necessary to check anything other than the rc value here.
This should always evaluate as false, and shouldn't result in a functional
problem, but it is completely bogus.
Tai, can you plase send a patch deleting The IS_ERR() check here?
Thanks,
Mark.
[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-March/419173.html
^ permalink raw reply
* [PATCH v3 2/5] arm64: mm: replace 'block_mappings_allowed' with 'page_mappings_only'
From: Mark Rutland @ 2016-10-12 15:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1476271425-19401-3-git-send-email-ard.biesheuvel@linaro.org>
On Wed, Oct 12, 2016 at 12:23:42PM +0100, Ard Biesheuvel wrote:
> In preparation of adding support for contiguous PTE and PMD mappings,
> let's replace 'block_mappings_allowed' with 'page_mappings_only', which
> will be a more accurate description of the nature of the setting once we
> add such contiguous mappings into the mix.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Regardless of the contiguous bit stuff, I think this makes the code
clearer. As far as I can tell, this is correct. So FWIW:
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Thanks,
Mark.
> ---
> arch/arm64/include/asm/mmu.h | 2 +-
> arch/arm64/kernel/efi.c | 8 ++---
> arch/arm64/mm/mmu.c | 32 ++++++++++----------
> 3 files changed, 21 insertions(+), 21 deletions(-)
>
> diff --git a/arch/arm64/include/asm/mmu.h b/arch/arm64/include/asm/mmu.h
> index 8d9fce037b2f..a81454ad5455 100644
> --- a/arch/arm64/include/asm/mmu.h
> +++ b/arch/arm64/include/asm/mmu.h
> @@ -34,7 +34,7 @@ extern void __iomem *early_io_map(phys_addr_t phys, unsigned long virt);
> extern void init_mem_pgprot(void);
> extern void create_pgd_mapping(struct mm_struct *mm, phys_addr_t phys,
> unsigned long virt, phys_addr_t size,
> - pgprot_t prot, bool allow_block_mappings);
> + pgprot_t prot, bool page_mappings_only);
> extern void *fixmap_remap_fdt(phys_addr_t dt_phys);
>
> #endif
> diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
> index ba9bee389fd5..5d17f377d905 100644
> --- a/arch/arm64/kernel/efi.c
> +++ b/arch/arm64/kernel/efi.c
> @@ -62,8 +62,8 @@ struct screen_info screen_info __section(.data);
> int __init efi_create_mapping(struct mm_struct *mm, efi_memory_desc_t *md)
> {
> pteval_t prot_val = create_mapping_protection(md);
> - bool allow_block_mappings = (md->type != EFI_RUNTIME_SERVICES_CODE &&
> - md->type != EFI_RUNTIME_SERVICES_DATA);
> + bool page_mappings_only = (md->type == EFI_RUNTIME_SERVICES_CODE ||
> + md->type == EFI_RUNTIME_SERVICES_DATA);
>
> if (!PAGE_ALIGNED(md->phys_addr) ||
> !PAGE_ALIGNED(md->num_pages << EFI_PAGE_SHIFT)) {
> @@ -76,12 +76,12 @@ int __init efi_create_mapping(struct mm_struct *mm, efi_memory_desc_t *md)
> * from the MMU routines. So avoid block mappings altogether in
> * that case.
> */
> - allow_block_mappings = false;
> + page_mappings_only = true;
> }
>
> create_pgd_mapping(mm, md->phys_addr, md->virt_addr,
> md->num_pages << EFI_PAGE_SHIFT,
> - __pgprot(prot_val | PTE_NG), allow_block_mappings);
> + __pgprot(prot_val | PTE_NG), page_mappings_only);
> return 0;
> }
>
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index e1c34e5a1d7d..bf1d71b62c4f 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -139,7 +139,7 @@ static void alloc_init_pte(pmd_t *pmd, unsigned long addr,
> static void alloc_init_pmd(pud_t *pud, unsigned long addr, unsigned long end,
> phys_addr_t phys, pgprot_t prot,
> phys_addr_t (*pgtable_alloc)(void),
> - bool allow_block_mappings)
> + bool page_mappings_only)
> {
> pmd_t *pmd;
> unsigned long next;
> @@ -166,7 +166,7 @@ static void alloc_init_pmd(pud_t *pud, unsigned long addr, unsigned long end,
>
> /* try section mapping first */
> if (((addr | next | phys) & ~SECTION_MASK) == 0 &&
> - allow_block_mappings) {
> + !page_mappings_only) {
> pmd_set_huge(pmd, phys, prot);
>
> /*
> @@ -204,7 +204,7 @@ static inline bool use_1G_block(unsigned long addr, unsigned long next,
> static void alloc_init_pud(pgd_t *pgd, unsigned long addr, unsigned long end,
> phys_addr_t phys, pgprot_t prot,
> phys_addr_t (*pgtable_alloc)(void),
> - bool allow_block_mappings)
> + bool page_mappings_only)
> {
> pud_t *pud;
> unsigned long next;
> @@ -226,7 +226,7 @@ static void alloc_init_pud(pgd_t *pgd, unsigned long addr, unsigned long end,
> /*
> * For 4K granule only, attempt to put down a 1GB block
> */
> - if (use_1G_block(addr, next, phys) && allow_block_mappings) {
> + if (use_1G_block(addr, next, phys) && !page_mappings_only) {
> pud_set_huge(pud, phys, prot);
>
> /*
> @@ -238,7 +238,7 @@ static void alloc_init_pud(pgd_t *pgd, unsigned long addr, unsigned long end,
> ~modifiable_attr_mask) != 0);
> } else {
> alloc_init_pmd(pud, addr, next, phys, prot,
> - pgtable_alloc, allow_block_mappings);
> + pgtable_alloc, page_mappings_only);
>
> BUG_ON(pud_val(old_pud) != 0 &&
> pud_val(old_pud) != pud_val(*pud));
> @@ -253,7 +253,7 @@ static void __create_pgd_mapping(pgd_t *pgdir, phys_addr_t phys,
> unsigned long virt, phys_addr_t size,
> pgprot_t prot,
> phys_addr_t (*pgtable_alloc)(void),
> - bool allow_block_mappings)
> + bool page_mappings_only)
> {
> unsigned long addr, length, end, next;
> pgd_t *pgd = pgd_offset_raw(pgdir, virt);
> @@ -273,7 +273,7 @@ static void __create_pgd_mapping(pgd_t *pgdir, phys_addr_t phys,
> do {
> next = pgd_addr_end(addr, end);
> alloc_init_pud(pgd, addr, next, phys, prot, pgtable_alloc,
> - allow_block_mappings);
> + page_mappings_only);
> phys += next - addr;
> } while (pgd++, addr = next, addr != end);
> }
> @@ -302,17 +302,17 @@ static void __init create_mapping_noalloc(phys_addr_t phys, unsigned long virt,
> &phys, virt);
> return;
> }
> - __create_pgd_mapping(init_mm.pgd, phys, virt, size, prot, NULL, true);
> + __create_pgd_mapping(init_mm.pgd, phys, virt, size, prot, NULL, false);
> }
>
> void __init create_pgd_mapping(struct mm_struct *mm, phys_addr_t phys,
> unsigned long virt, phys_addr_t size,
> - pgprot_t prot, bool allow_block_mappings)
> + pgprot_t prot, bool page_mappings_only)
> {
> BUG_ON(mm == &init_mm);
>
> __create_pgd_mapping(mm->pgd, phys, virt, size, prot,
> - pgd_pgtable_alloc, allow_block_mappings);
> + pgd_pgtable_alloc, page_mappings_only);
> }
>
> static void create_mapping_late(phys_addr_t phys, unsigned long virt,
> @@ -325,7 +325,7 @@ static void create_mapping_late(phys_addr_t phys, unsigned long virt,
> }
>
> __create_pgd_mapping(init_mm.pgd, phys, virt, size, prot,
> - NULL, !debug_pagealloc_enabled());
> + NULL, debug_pagealloc_enabled());
> }
>
> static void __init __map_memblock(pgd_t *pgd, phys_addr_t start, phys_addr_t end)
> @@ -343,7 +343,7 @@ static void __init __map_memblock(pgd_t *pgd, phys_addr_t start, phys_addr_t end
> __create_pgd_mapping(pgd, start, __phys_to_virt(start),
> end - start, PAGE_KERNEL,
> early_pgtable_alloc,
> - !debug_pagealloc_enabled());
> + debug_pagealloc_enabled());
> return;
> }
>
> @@ -356,13 +356,13 @@ static void __init __map_memblock(pgd_t *pgd, phys_addr_t start, phys_addr_t end
> __phys_to_virt(start),
> kernel_start - start, PAGE_KERNEL,
> early_pgtable_alloc,
> - !debug_pagealloc_enabled());
> + debug_pagealloc_enabled());
> if (kernel_end < end)
> __create_pgd_mapping(pgd, kernel_end,
> __phys_to_virt(kernel_end),
> end - kernel_end, PAGE_KERNEL,
> early_pgtable_alloc,
> - !debug_pagealloc_enabled());
> + debug_pagealloc_enabled());
>
> /*
> * Map the linear alias of the [_text, __init_begin) interval as
> @@ -372,7 +372,7 @@ static void __init __map_memblock(pgd_t *pgd, phys_addr_t start, phys_addr_t end
> */
> __create_pgd_mapping(pgd, kernel_start, __phys_to_virt(kernel_start),
> kernel_end - kernel_start, PAGE_KERNEL_RO,
> - early_pgtable_alloc, !debug_pagealloc_enabled());
> + early_pgtable_alloc, debug_pagealloc_enabled());
> }
>
> static void __init map_mem(pgd_t *pgd)
> @@ -422,7 +422,7 @@ static void __init map_kernel_segment(pgd_t *pgd, void *va_start, void *va_end,
> BUG_ON(!PAGE_ALIGNED(size));
>
> __create_pgd_mapping(pgd, pa_start, (unsigned long)va_start, size, prot,
> - early_pgtable_alloc, !debug_pagealloc_enabled());
> + early_pgtable_alloc, debug_pagealloc_enabled());
>
> vma->addr = va_start;
> vma->phys_addr = pa_start;
> --
> 2.7.4
>
^ permalink raw reply
* [PATCH v3 1/5] arm64: mm: BUG on unsupported manipulations of live kernel mappings
From: Catalin Marinas @ 2016-10-12 15:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1476271425-19401-2-git-send-email-ard.biesheuvel@linaro.org>
On Wed, Oct 12, 2016 at 12:23:41PM +0100, Ard Biesheuvel wrote:
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -28,8 +28,6 @@
> #include <linux/memblock.h>
> #include <linux/fs.h>
> #include <linux/io.h>
> -#include <linux/slab.h>
> -#include <linux/stop_machine.h>
>
> #include <asm/barrier.h>
> #include <asm/cputype.h>
> @@ -95,6 +93,12 @@ static phys_addr_t __init early_pgtable_alloc(void)
> return phys;
> }
>
> +/*
> + * The following mapping attributes may be updated in live
> + * kernel mappings without the need for break-before-make.
> + */
> +static const pteval_t modifiable_attr_mask = PTE_PXN | PTE_RDONLY | PTE_WRITE;
> +
> static void alloc_init_pte(pmd_t *pmd, unsigned long addr,
> unsigned long end, unsigned long pfn,
> pgprot_t prot,
> @@ -115,8 +119,18 @@ static void alloc_init_pte(pmd_t *pmd, unsigned long addr,
>
> pte = pte_set_fixmap_offset(pmd, addr);
> do {
> + pte_t old_pte = *pte;
> +
> set_pte(pte, pfn_pte(pfn, prot));
> pfn++;
> +
> + /*
> + * After the PTE entry has been populated once, we
> + * only allow updates to the permission attributes.
> + */
> + BUG_ON(pte_val(old_pte) != 0 &&
> + ((pte_val(old_pte) ^ pte_val(*pte)) &
> + ~modifiable_attr_mask) != 0);
Please turn this check into a single macro. You have it in three places
already (though with different types but a macro would do). Something
like below (feel free to come up with a better macro name):
BUG_ON(!safe_pgattr_change(old_pte, *pte));
--
Catalin
^ permalink raw reply
* [PATCH/RFT 07/12] USB: ohci-da8xx: Request gpios and handle interrupt in the driver
From: Axel Haslam @ 2016-10-12 15:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <22f1532a-b651-3330-b2ce-9a64a35c85af@lechnology.com>
Hi David
On Tue, Oct 11, 2016 at 1:18 AM, David Lechner <david@lechnology.com> wrote:
> On 10/07/2016 11:42 AM, ahaslam at baylibre.com wrote:
>>
>> From: Axel Haslam <ahaslam@baylibre.com>
>>
>> Currently requesting the vbus and overcurrent gpio is handled on
>> the board specific file. But this does not play well moving to
>> device tree.
>>
>> In preparation to migrate to a device tree boot, handle requesting
>> gpios and overcurrent interrupt on the usb driver itself, thus avoiding
>> callbacks to arch/mach*
>>
>
> Instead of using gpios, it seems like it would be better to use a regulator
> here. I don't know of any real-life cases, but who is to say someone
> not design a board that uses a regulator controlled by I2C instead of gpios
> or something like that.
>
> Then, boards that don't have gpios can just use a fixed regulator (or you
> can make the regulator optional). Using a regulator would also allow users
> to decide how to respond to overcurrent events (by supplying their own
> regulator driver) instead of the behavior being dictated by the ohci driver.
I agree that we should use a regulator for the vbus power.
i will make that change. However, im not so sure about using the
regulator for the overcurrent handling. There seems to be no other
driver doing this, and as you mention, we would need to change the regulator
framework, which might not be justifiable. I think there is not another way
to handle the over current notification other than powering the port off.
>
> In my particular area of interest (LEGO MINDSTORMS EV3), the 5V (hardware)
> regulator for VBUS does use gpios, but the 5V is also shared with the LEGO
> input and output ports. So what I would ultimately like to be able to do is
> have userspace notified of an overcurrent event and let userspace decided
> when to turn the vbus back on. For example, someone might plug something
> into one of the LEGO input or output ports that causes a short circuit. I
> would like to display a notification to the user and wait for them to
> correct the problem and then press a button to turn the power back on.
>
how about using regulator for vbus, but keeping gpio for overcurrent
notifications?
For the usersapce handling you describe above, maybe we should be able to
listen for an usb overcurrent uevent in userspace? it seems this
question was asked
a couple of years back[1], but im not sure what the conclusion was. In any case,
we could have DT and non-DT based ohci-da8xx working,
And could work on a uevent notification for the scenario you describe
above which
i think is not specific to the ohci-da8xx.
[1]http://linux-usb.vger.kernel.narkive.com/SjcUB5hk/how-best-to-get-over-current-notification-to-user-application
Regards
Axel
> This will require some modifications to the regulator subsystem though. I
> actually started work on this a while back, but haven't had the time to
> pursue it any farther.
>
> Here are my WIP patches in case there is any interest:
> *
> https://github.com/dlech/ev3dev-kernel/commit/541a42b3b8ed639e95bbc835df3292f80190c789
> *
> https://github.com/dlech/ev3dev-kernel/commit/2ba99b1ad6a06c944dd33a073f54044e71b75ae6
> *
> https://github.com/dlech/ev3dev-kernel/commit/cdb03caa50e64931d4f2836c648739aa4385ed3b
> *
> https://github.com/dlech/ev3dev-kernel/commit/9d6b50cde34b51309c74d97c26b1430c7ff6aa0f
^ permalink raw reply
* [linux-sunxi] [PATCH 3/5] Input: add driver for Ilitek ili2139 touch IC
From: Hans de Goede @ 2016-10-12 14:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161011132149.LZ40KToV@smtp1h.mail.yandex.net>
Hi,
On 10/11/2016 12:21 PM, Icenowy Zheng wrote:
>
> 2016?10?11? ??5:37? Hans de Goede <hdegoede@redhat.com>???
>>
>> Hi,
>
> I have a request: could you please test this driver on your E708 Q1? (According to the info you published on github, your E708 has also ili)
I'm afraid the touchscreen on my E708 Q1 is broken (does not even work in android),
so I cannot test this.
>
>>
>> On 10/11/2016 02:33 AM, Icenowy Zheng wrote:
>>> This driver adds support for Ilitek ili2139 touch IC, which is used in
>>> several Colorfly tablets (for example, Colorfly E708 Q1, which is an
>>> Allwinner A31s tablet with mainline kernel support).
>>>
>>> Theortically it may support more Ilitek touch ICs, however, only ili2139
>>> is used in any mainlined device.
>>>
>>> It supports device tree enumeration, with screen resolution and axis
>>> quirks configurable.
>>>
>>> Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
>>> ---
>>> drivers/input/touchscreen/Kconfig | 14 ++
>>> drivers/input/touchscreen/Makefile | 1 +
>>> drivers/input/touchscreen/ili2139.c | 320 ++++++++++++++++++++++++++++++++++++
>>> 3 files changed, 335 insertions(+)
>>> create mode 100644 drivers/input/touchscreen/ili2139.c
>>>
>>> diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
>>> index 5079813..bb4d9d2 100644
>>> --- a/drivers/input/touchscreen/Kconfig
>>> +++ b/drivers/input/touchscreen/Kconfig
>>> @@ -348,6 +348,20 @@ config TOUCHSCREEN_ILI210X
>>> To compile this driver as a module, choose M here: the
>>> module will be called ili210x.
>>>
>>> +config TOUCHSCREEN_ILI2139
>>> + tristate "Ilitek ILI2139 based touchscreen"
>>> + depends on I2C
>>> + depends on OF
>>> + help
>>> + Say Y here if you have a ILI2139 based touchscreen
>>> + controller. Such kind of chipsets can be found in several
>>> + Colorfly tablets.
>>> +
>>> + If unsure, say N.
>>> +
>>> + To compile this driver as a module, choose M here; the
>>> + module will be called ili2139.
>>> +
>>> config TOUCHSCREEN_IPROC
>>> tristate "IPROC touch panel driver support"
>>> depends on ARCH_BCM_IPROC || COMPILE_TEST
>>> diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
>>> index 81b8645..930b5e2 100644
>>> --- a/drivers/input/touchscreen/Makefile
>>> +++ b/drivers/input/touchscreen/Makefile
>>> @@ -40,6 +40,7 @@ obj-$(CONFIG_TOUCHSCREEN_EGALAX_SERIAL) += egalax_ts_serial.o
>>> obj-$(CONFIG_TOUCHSCREEN_FUJITSU) += fujitsu_ts.o
>>> obj-$(CONFIG_TOUCHSCREEN_GOODIX) += goodix.o
>>> obj-$(CONFIG_TOUCHSCREEN_ILI210X) += ili210x.o
>>> +obj-$(CONFIG_TOUCHSCREEN_ILI2139) += ili2139.o
>>> obj-$(CONFIG_TOUCHSCREEN_IMX6UL_TSC) += imx6ul_tsc.o
>>> obj-$(CONFIG_TOUCHSCREEN_INEXIO) += inexio.o
>>> obj-$(CONFIG_TOUCHSCREEN_INTEL_MID) += intel-mid-touch.o
>>> diff --git a/drivers/input/touchscreen/ili2139.c b/drivers/input/touchscreen/ili2139.c
>>> new file mode 100644
>>> index 0000000..65c2dea
>>> --- /dev/null
>>> +++ b/drivers/input/touchscreen/ili2139.c
>>> @@ -0,0 +1,320 @@
>>> +/* -------------------------------------------------------------------------
>>> + * Copyright (C) 2016, Icenowy Zheng <icenowy@aosc.xyz>
>>> + *
>>> + * Derived from:
>>> + * ili210x.c
>>> + * Copyright (C) Olivier Sobrie <olivier@sobrie.be>
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License as published by
>>> + * the Free Software Foundation; either version 2 of the License, or
>>> + * (at your option) any later version.
>>> + *
>>> + * 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.
>>> + * -------------------------------------------------------------------------
>>> + */
>>> +
>>> +#include <linux/module.h>
>>> +#include <linux/i2c.h>
>>> +#include <linux/interrupt.h>
>>> +#include <linux/slab.h>
>>> +#include <linux/input.h>
>>> +#include <linux/input/mt.h>
>>> +#include <linux/input/touchscreen.h>
>>> +#include <linux/delay.h>
>>> +#include <linux/workqueue.h>
>>> +
>>> +#define DEFAULT_POLL_PERIOD 20
>>> +
>>> +#define MAX_TOUCHES 10
>>> +#define COMPATIBLE_TOUCHES 2
>>> +
>>> +/* Touchscreen commands */
>>> +#define REG_TOUCHDATA 0x10
>>> +#define REG_TOUCHSUBDATA 0x11
>>> +#define REG_PANEL_INFO 0x20
>>> +#define REG_FIRMWARE_VERSION 0x40
>>> +#define REG_PROTO_VERSION 0x42
>>> +
>>> +#define SUBDATA_STATUS_TOUCH_POINT 0x80
>>> +#define SUBDATA_STATUS_RELEASE_POINT 0x00
>>> +
>>> +struct finger {
>>> + u8 x_low;
>>> + u8 x_high;
>>> + u8 y_low;
>>> + u8 y_high;
>>> +} __packed;
>>> +
>>> +struct touchdata {
>>> + u8 length;
>>> + struct finger finger[COMPATIBLE_TOUCHES];
>>> +} __packed;
>>> +
>>> +struct touch_subdata {
>>> + u8 status;
>>> + struct finger finger;
>>> +} __packed;
>>> +
>>> +struct panel_info {
>>> + struct finger finger_max;
>>> + u8 xchannel_num;
>>> + u8 ychannel_num;
>>> +} __packed;
>>> +
>>> +struct firmware_version {
>>> + u8 id;
>>> + u8 major;
>>> + u8 minor;
>>> +} __packed;
>>> +
>>> +struct ili2139 {
>>> + struct i2c_client *client;
>>> + struct input_dev *input;
>>> + unsigned int poll_period;
>>> + struct delayed_work dwork;
>>> + struct touchscreen_properties prop;
>>> + int slots[MAX_TOUCHES];
>>> + int ids[MAX_TOUCHES];
>>> + struct input_mt_pos pos[MAX_TOUCHES];
>>> +};
>>> +
>>> +static int ili2139_read_reg(struct i2c_client *client, u8 reg, void *buf,
>>> + size_t len)
>>> +{
>>> + struct i2c_msg msg[2] = {
>>> + {
>>> + .addr = client->addr,
>>> + .flags = 0,
>>> + .len = 1,
>>> + .buf = ®,
>>> + },
>>> + {
>>> + .addr = client->addr,
>>> + .flags = I2C_M_RD,
>>> + .len = len,
>>> + .buf = buf,
>>> + }
>>> + };
>>> +
>>> + if (i2c_transfer(client->adapter, msg, 2) != 2) {
>>> + dev_err(&client->dev, "i2c transfer failed\n");
>>> + return -EIO;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>
>> This just i2c_smbus_read_i2c_block_data, please use that instead.
>>
>>> +static void ili2139_work(struct work_struct *work)
>>> +{
>>> + int id;
>>> + struct ili2139 *priv = container_of(work, struct ili2139,
>>> + dwork.work);
>>> + struct i2c_client *client = priv->client;
>>> + struct touchdata touchdata;
>>> + struct touch_subdata subdata;
>>> + int error;
>>> +
>>> + error = ili2139_read_reg(client, REG_TOUCHDATA,
>>> + &touchdata, sizeof(touchdata));
>>> + if (error) {
>>> + dev_err(&client->dev,
>>> + "Unable to get touchdata, err = %d\n", error);
>>> + return;
>>> + }
>>> +
>>> + for (id = 0; id < touchdata.length; id++) {
>>> + error = ili2139_read_reg(client, REG_TOUCHSUBDATA, &subdata,
>>> + sizeof(subdata));
>>> + if (error) {
>>> + dev_err(&client->dev,
>>> + "Unable to get touch subdata, err = %d\n",
>>> + error);
>>> + return;
>>> + }
>>> +
>>> + priv->ids[id] = subdata.status & 0x3F;
>>> +
>>> + /* The sequence changed in the v2 subdata protocol. */
>>> + touchscreen_set_mt_pos(&priv->pos[id], &priv->prop,
>>> + (subdata.finger.x_high | (subdata.finger.x_low << 8)),
>>> + (subdata.finger.y_high | (subdata.finger.y_low << 8)));
>>> + }
>>> +
>>> + input_mt_assign_slots(priv->input, priv->slots, priv->pos,
>>> + touchdata.length, 0);
>>> +
>>> + for (id = 0; id < touchdata.length; id++) {
>>> + input_mt_slot(priv->input, priv->slots[id]);
>>> + input_mt_report_slot_state(priv->input, MT_TOOL_FINGER,
>>> + subdata.status &
>>> + SUBDATA_STATUS_TOUCH_POINT);
>>> + input_report_abs(priv->input, ABS_MT_POSITION_X,
>>> + priv->pos[id].x);
>>> + input_report_abs(priv->input, ABS_MT_POSITION_Y,
>>> + priv->pos[id].y);
>>> + }
>>> +
>>> + input_mt_sync_frame(priv->input);
>>> + input_sync(priv->input);
>>> +
>>> + schedule_delayed_work(&priv->dwork,
>>> + msecs_to_jiffies(priv->poll_period));
>>
>> If the irq is working properly there should be no need for this,
>> can you try with this schedule call removed ?
>
> Thanks. I'm glad to know this.
> The driver that I modelled after is too old and unmaintained, and even nearly not usable now (as it requires platform data)
>
>>
>>> +}
>>> +
>>> +static irqreturn_t ili2139_irq(int irq, void *irq_data)
>>> +{
>>> + struct ili2139 *priv = irq_data;
>>> +
>>> + schedule_delayed_work(&priv->dwork, 0);
>>> +
>>> + return IRQ_HANDLED;
>>> +}
>>> +
>>> +static int ili2139_i2c_probe(struct i2c_client *client,
>>> + const struct i2c_device_id *id)
>>> +{
>>> + struct device *dev = &client->dev;
>>> + struct ili2139 *priv;
>>> + struct input_dev *input;
>>> + struct panel_info panel;
>>> + struct firmware_version firmware;
>>> + int xmax, ymax;
>>> + int error;
>>> +
>>> + dev_dbg(dev, "Probing for ILI2139 I2C Touschreen driver");
>>> +
>>> + if (client->irq <= 0) {
>>> + dev_err(dev, "No IRQ!\n");
>>> + return -ENODEV;
>>> + }
>>> +
>>> + /* Get firmware version */
>>> + error = ili2139_read_reg(client, REG_FIRMWARE_VERSION,
>>> + &firmware, sizeof(firmware));
>>> + if (error) {
>>> + dev_err(dev, "Failed to get firmware version, err: %d\n",
>>> + error);
>>> + return error;
>>> + }
>>> +
>>> + /* get panel info */
>>> + error = ili2139_read_reg(client, REG_PANEL_INFO, &panel, sizeof(panel));
>>> + if (error) {
>>> + dev_err(dev, "Failed to get panel information, err: %d\n",
>>> + error);
>>> + return error;
>>> + }
>>> +
>>> + xmax = panel.finger_max.x_low | (panel.finger_max.x_high << 8);
>>> + ymax = panel.finger_max.y_low | (panel.finger_max.y_high << 8);
>>> +
>>> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>>> + input = devm_input_allocate_device(dev);
>>> + if (!priv || !input)
>>> + return -ENOMEM;
>>> +
>>> + priv->client = client;
>>> + priv->input = input;
>>> + priv->poll_period = DEFAULT_POLL_PERIOD;
>>> + INIT_DELAYED_WORK(&priv->dwork, ili2139_work);
>>> +
>>> + /* Setup input device */
>>> + input->name = "ILI2139 Touchscreen";
>>> + input->id.bustype = BUS_I2C;
>>> + input->dev.parent = dev;
>>> +
>>> + __set_bit(EV_SYN, input->evbit);
>>> + __set_bit(EV_KEY, input->evbit);
>>> + __set_bit(EV_ABS, input->evbit);
>>> +
>>> + /* Multi touch */
>>> + input_mt_init_slots(input, MAX_TOUCHES, INPUT_MT_DIRECT |
>>> + INPUT_MT_DROP_UNUSED | INPUT_MT_TRACK);
>>> + input_set_abs_params(input, ABS_MT_POSITION_X, 0, xmax, 0, 0);
>>> + input_set_abs_params(input, ABS_MT_POSITION_Y, 0, ymax, 0, 0);
>>> +
>>> + touchscreen_parse_properties(input, true, &priv->prop);
>>> +
>>> + input_set_drvdata(input, priv);
>>> + i2c_set_clientdata(client, priv);
>>> +
>>> + error = devm_request_irq(dev, client->irq, ili2139_irq,
>>> + IRQF_TRIGGER_FALLING, client->name, priv);
>>
>> If things work with the re-scheduleing of the delayed work
>> from the work removed, then you can use request_threaded_irq here,
>> pass in ili2139_irq as the threaded handler (and NULL as the non threaded
>> handler) and do all the i2c reading directly in ili2139_irq without needing
>> to use any work struct at all.
>
> I'll test it, and remember request_threaded_irq function. I've seen the usage of the function in silead.c.
Good :)
Regards,
Hans
>
>>
>> Regards,
>>
>> Hans
>>
>>
>>> + if (error) {
>>> + dev_err(dev, "Unable to request touchscreen IRQ, err: %d\n",
>>> + error);
>>> + return error;
>>> + }
>>> +
>>> + error = input_register_device(priv->input);
>>> + if (error) {
>>> + dev_err(dev, "Cannot register input device, err: %d\n", error);
>>> + return error;
>>> + }
>>> +
>>> + device_init_wakeup(&client->dev, 1);
>>> +
>>> + dev_dbg(dev,
>>> + "ILI2139 initialized (IRQ: %d), firmware version %d.%d.%d",
>>> + client->irq, firmware.id, firmware.major, firmware.minor);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int ili2139_i2c_remove(struct i2c_client *client)
>>> +{
>>> + struct ili2139 *priv = i2c_get_clientdata(client);
>>> +
>>> + cancel_delayed_work_sync(&priv->dwork);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int __maybe_unused ili2139_i2c_suspend(struct device *dev)
>>> +{
>>> + struct i2c_client *client = to_i2c_client(dev);
>>> +
>>> + if (device_may_wakeup(&client->dev))
>>> + enable_irq_wake(client->irq);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int __maybe_unused ili2139_i2c_resume(struct device *dev)
>>> +{
>>> + struct i2c_client *client = to_i2c_client(dev);
>>> +
>>> + if (device_may_wakeup(&client->dev))
>>> + disable_irq_wake(client->irq);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static SIMPLE_DEV_PM_OPS(ili2139_i2c_pm,
>>> + ili2139_i2c_suspend, ili2139_i2c_resume);
>>> +
>>> +static const struct i2c_device_id ili2139_i2c_id[] = {
>>> + { "ili2139", 0 },
>>> + { }
>>> +};
>>> +MODULE_DEVICE_TABLE(i2c, ili2139_i2c_id);
>>> +
>>> +static struct i2c_driver ili2139_ts_driver = {
>>> + .driver = {
>>> + .name = "ili2139_i2c",
>>> + .pm = &ili2139_i2c_pm,
>>> + },
>>> + .id_table = ili2139_i2c_id,
>>> + .probe = ili2139_i2c_probe,
>>> + .remove = ili2139_i2c_remove,
>>> +};
>>> +
>>> +module_i2c_driver(ili2139_ts_driver);
>>> +
>>> +MODULE_AUTHOR("Olivier Sobrie <olivier@sobrie.be>");
>>> +MODULE_DESCRIPTION("ILI2139 I2C Touchscreen Driver");
>>> +MODULE_LICENSE("GPL");
>>>
^ permalink raw reply
* [bug report] perf: xgene: Add APM X-Gene SoC Performance Monitoring Unit driver
From: Mason @ 2016-10-12 14:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161012113229.GA16447@mwanda>
On 12/10/2016 13:32, Dan Carpenter wrote:
> Hello Tai Nguyen,
>
> The patch 832c927d119b: "perf: xgene: Add APM X-Gene SoC Performance
> Monitoring Unit driver" from Jul 15, 2016, leads to the following
> static checker warning:
>
> drivers/perf/xgene_pmu.c:1014 acpi_get_pmu_hw_inf()
> warn: '&res' isn't an ERR_PTR
>
> drivers/perf/xgene_pmu.c
> 992 static struct
> 993 xgene_pmu_dev_ctx *acpi_get_pmu_hw_inf(struct xgene_pmu *xgene_pmu,
> 994 struct acpi_device *adev, u32 type)
> 995 {
> 996 struct device *dev = xgene_pmu->dev;
> 997 struct list_head resource_list;
> 998 struct xgene_pmu_dev_ctx *ctx;
> 999 const union acpi_object *obj;
> 1000 struct hw_pmu_info *inf;
> 1001 void __iomem *dev_csr;
> 1002 struct resource res;
> 1003 int enable_bit;
> 1004 int rc;
> 1005
> 1006 ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> 1007 if (!ctx)
> 1008 return NULL;
> 1009
> 1010 INIT_LIST_HEAD(&resource_list);
> 1011 rc = acpi_dev_get_resources(adev, &resource_list,
> 1012 acpi_pmu_dev_add_resource, &res);
> 1013 acpi_dev_free_resource_list(&resource_list);
> 1014 if (rc < 0 || IS_ERR(&res)) {
> ^^^^
> Obviously this is a stack address and not an error pointer. I'm not
> sure what was intended here.
Reminds me of 287980e49ffc0f6d911601e7e352a812ed27768e
("remove lots of IS_ERR_VALUE abuses")
Regards.
^ permalink raw reply
* Question: Re: [PATCH] drm/bridge: analogix: protect power when get_modes or detect
From: Sean Paul @ 2016-10-12 14:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <57FE0EF5.7010207@rock-chips.com>
On Wed, Oct 12, 2016 at 6:22 AM, Mark yao <mark.yao@rock-chips.com> wrote:
>
> I'm not familiar with the analogix driver, maybe use a power reference count
> would better then direct power on/off analogix_dp.
>
> Does anyone has the idea to protect detect and get_modes context?
>
I'm not sure a reference count is going to help here. The common
pattern is to call detect() followed by get_modes() and then modeset.
However, it's not guaranteed that any one of those functions will be
called after the other. So, if you leave things on after detect or
get_modes, you might be wasting power (or worse).
I recently ran into this exact problem with a panel we're using. Check
out "0b8b059a7: drm/bridge: analogix_dp: Ensure the panel is properly
prepared/unprepared". Perhaps you can piggyback on that function to
add your pm_runtime and plat_data callbacks (since using dpms_mode
might be racey).
Sean
> I found many other connector driver also direct access register on detect or
> get_modes, no problem for it?
>
> On 2016?10?12? 18:00, Mark Yao wrote:
>
> The drm callback ->detect and ->get_modes seems is not power safe,
> they may be called when device is power off, do register access on
> detect or get_modes will cause system die.
>
> Here is the path call ->detect before analogix_dp power on
> [<ffffff800843babc>] analogix_dp_detect+0x44/0xdc
> [<ffffff80083fd840>]
> drm_helper_probe_single_connector_modes_merge_bits+0xe8/0x41c
> [<ffffff80083fdb84>] drm_helper_probe_single_connector_modes+0x10/0x18
> [<ffffff8008418d24>] drm_mode_getconnector+0xf4/0x304
> [<ffffff800840cff0>] drm_ioctl+0x23c/0x390
> [<ffffff80081a8adc>] do_vfs_ioctl+0x4b8/0x58c
> [<ffffff80081a8c10>] SyS_ioctl+0x60/0x88
>
> Cc: Inki Dae <inki.dae@samsung.com>
> Cc: Sean Paul <seanpaul@chromium.org>
> Cc: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> Cc: "Ville Syrj?l?" <ville.syrjala@linux.intel.com>
>
> Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
> ---
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 28
> ++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index efac8ab..09dece2 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -1062,6 +1062,13 @@ int analogix_dp_get_modes(struct drm_connector
> *connector)
> return 0;
> }
>
> + if (dp->dpms_mode != DRM_MODE_DPMS_ON) {
> + pm_runtime_get_sync(dp->dev);
> +
> + if (dp->plat_data->power_on)
> + dp->plat_data->power_on(dp->plat_data);
> + }
> +
> if (analogix_dp_handle_edid(dp) == 0) {
> drm_mode_connector_update_edid_property(&dp->connector, edid);
> num_modes += drm_add_edid_modes(&dp->connector, edid);
> @@ -1073,6 +1080,13 @@ int analogix_dp_get_modes(struct drm_connector
> *connector)
> if (dp->plat_data->get_modes)
> num_modes += dp->plat_data->get_modes(dp->plat_data, connector);
>
> + if (dp->dpms_mode != DRM_MODE_DPMS_ON) {
> + if (dp->plat_data->power_off)
> + dp->plat_data->power_off(dp->plat_data);
> +
> + pm_runtime_put_sync(dp->dev);
> + }
> +
> ret = analogix_dp_prepare_panel(dp, false, false);
> if (ret)
> DRM_ERROR("Failed to unprepare panel (%d)\n", ret);
> @@ -1106,9 +1120,23 @@ analogix_dp_detect(struct drm_connector *connector,
> bool force)
> return connector_status_disconnected;
> }
>
> + if (dp->dpms_mode != DRM_MODE_DPMS_ON) {
> + pm_runtime_get_sync(dp->dev);
> +
> + if (dp->plat_data->power_on)
> + dp->plat_data->power_on(dp->plat_data);
> + }
> +
> if (!analogix_dp_detect_hpd(dp))
> status = connector_status_connected;
>
> + if (dp->dpms_mode != DRM_MODE_DPMS_ON) {
> + if (dp->plat_data->power_off)
> + dp->plat_data->power_off(dp->plat_data);
> +
> + pm_runtime_put_sync(dp->dev);
> + }
> +
> ret = analogix_dp_prepare_panel(dp, false, false);
> if (ret)
> DRM_ERROR("Failed to unprepare panel (%d)\n", ret);
>
>
>
> --
> ?ark Yao
^ permalink raw reply
* [PATCH 2/3] arm64: hw_breakpoint: Handle inexact watchpoint addresses
From: Pavel Labath @ 2016-10-12 13:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAHB_GupR4sJtAGYdRwvCwu7891KM8TEByZCgqMCrYrdt=cJuYw@mail.gmail.com>
Hello Pratyush,
I am sorry about the delay. I have finally had a chance to try out
your changes today. Response inline.
On 8 October 2016 at 06:10, Pratyush Anand <panand@redhat.com> wrote:
> On Fri, Oct 7, 2016 at 10:54 PM, Pavel Labath <labath@google.com> wrote:
>> The thing is, I have observed different behavior here depending on the
>> exact hardware used. I don't have the exact parameters with me now,
>> but I can look it up next week.
>>
>> The thing is that the spec is imprecise about what exact address the
>> hardware can report for the watchpoint hit. I presume that is
>> deliberate to give some leeway to implementers. The spec says the
>> address can be anywhere in the range from the lowest memory address
>> accessed by the instruction to the highest address watched by the
>> watchpoint,
>
> I think, my patches should be able to take care of the above condition.
Unfortunately, the patch does not solve the problem for my hardware,
because of the leeway you give in watchpoint_handler is not big
enough. It does work however, if I change the line
> if (addr + 7 < val + lens || addr > val + lene)
to
> if (addr + 15 < val + lens || addr > val + lene)
I do not think we can assume that address reported by the hardware
will be at most 7 bytes off from the address we put the watchpoint at.
There is nothing in the spec that guarantees that, and it does not
seem to be enough for some hardware. In fact, I am not sure we can
assume 15 is enough either, but maybe it can do for now, until we can
find hardware that does not work with that (I haven't yet tried what
happens it the watchpoint is triggered by cache management
instructions, which can access much larger blocks of memory).
For reference, the hardware in question is:
> Processor : AArch64 Processor rev 0 (aarch64)
> processor : 0
> min_vddcx : 400000
> min_vddmx : 490000
> BogoMIPS : 38.00
> Features : fp asimd evtstrm aes pmull sha1 sha2 crc32
> CPU implementer : 0x51
> CPU architecture: 8
> CPU variant : 0x2
> CPU part : 0x201
> CPU revision : 0
> CPU param : 300 468 468 621 939 299 445 445 621 1077
> Hardware : Qualcomm Technologies, Inc MSM8996pro
And this is how it behaves:
The output of the test app triggering the watchpoint (I have set a
single-byte watchpoint at 555556705f)
>
> Writing to: 555556705f, size: 1
> Writing to: 555556705e, size: 2
> Writing to: 555556705c, size: 4
> Writing to: 5555567058, size: 8
> Writing to: 5555567050, size: 16
> Writing to: 5555567040, size: 32
The addresses received by the kernel:
[ 251.812166] c1 3780 hw-breakpoint: watchpoint_handler: addr:
555556705f, val+lens: 555556705f, val+lene: 555556705f
[ 251.820341] c1 3781 hw-breakpoint: watchpoint_handler: addr:
555556705e, val+lens: 555556705f, val+lene: 555556705f
[ 251.825572] c0 3782 hw-breakpoint: watchpoint_handler: addr:
555556705c, val+lens: 555556705f, val+lene: 555556705f
[ 251.831085] c0 3783 hw-breakpoint: watchpoint_handler: addr:
5555567058, val+lens: 555556705f, val+lene: 555556705f
[ 251.835804] c0 3784 hw-breakpoint: watchpoint_handler: addr:
5555567050, val+lens: 555556705f, val+lene: 555556705f
[ 251.841350] c0 3785 hw-breakpoint: watchpoint_handler: addr:
5555567050, val+lens: 555556705f, val+lene: 555556705f
Note that for the case of 16 and 32-byte access it returns the address
5555567050 -- this is why the "+15" is sufficient for me.
The other thing I am not so sure about in your patch is that it has
potential to mis-attribute the watchpoint hit if we have two
watchpoints close together. For example, if I have first watchpoint on
0x1008-0x100f and a second one on 0x1000-0x1007, *and* the application
writes one byte to 0x1004, then your code will still attribute the hit
to the first watchpoint, even though it was not really triggered. This
is the reason I implemented my fix as a two-stage process, first
looking for exact hits, and then falling back to the nearest one. That
said, I don't know enough about the codebase to say if this is a real
problem.
On the plus side, I like the fact that we can watch arbitrary memory
regions now, and the feature is working perfectly. :)
My proposal would be to combine the two patches - take the byte mask
handling code from yours, and the hit-attribution code from my patch.
What do you think?
regards,
pavel
^ permalink raw reply
* [PATCH v14 16/16] vfio/type1: Introduce MSI_RESV capability
From: Eric Auger @ 2016-10-12 13:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1476278544-3397-1-git-send-email-eric.auger@redhat.com>
This patch allows the user-space to retrieve the MSI reserved region
requirements, if any. The implementation is based on capability chains,
now also added to VFIO_IOMMU_GET_INFO.
The returned info comprises the size and the alignment requirements
In case the userspace must provide the IOVA aperture, we currently report
a size/alignment based on all the doorbells registered by the host kernel.
This may exceed the actual needs.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
v13 -> v14:
- new capability struct
- change the padding in vfio_iommu_type1_info
v11 -> v12:
- msi_doorbell_pages was renamed msi_doorbell_calc_pages
v9 -> v10:
- move cap_offset after iova_pgsizes
- replace __u64 alignment by __u32 order
- introduce __u32 flags in vfio_iommu_type1_info_cap_msi_geometry and
fix alignment
- call msi-doorbell API to compute the size/alignment
v8 -> v9:
- use iommu_msi_supported flag instead of programmable
- replace IOMMU_INFO_REQUIRE_MSI_MAP flag by a more sophisticated
capability chain, reporting the MSI geometry
v7 -> v8:
- use iommu_domain_msi_geometry
v6 -> v7:
- remove the computation of the number of IOVA pages to be provisionned.
This number depends on the domain/group/device topology which can
dynamically change. Let's rely instead rely on an arbitrary max depending
on the system
v4 -> v5:
- move msi_info and ret declaration within the conditional code
v3 -> v4:
- replace former vfio_domains_require_msi_mapping by
more complex computation of MSI mapping requirements, especially the
number of pages to be provided by the user-space.
- reword patch title
RFC v1 -> v1:
- derived from
[RFC PATCH 3/6] vfio: Extend iommu-info to return MSIs automap state
- renamed allow_msi_reconfig into require_msi_mapping
- fixed VFIO_IOMMU_GET_INFO
---
drivers/vfio/vfio_iommu_type1.c | 67 ++++++++++++++++++++++++++++++++++++++++-
include/uapi/linux/vfio.h | 20 +++++++++++-
2 files changed, 85 insertions(+), 2 deletions(-)
diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index c18ba9d..6775da3 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -1147,6 +1147,46 @@ static int vfio_domains_have_iommu_cache(struct vfio_iommu *iommu)
return ret;
}
+static int msi_resv_caps(struct vfio_iommu *iommu, struct vfio_info_cap *caps)
+{
+ struct iommu_domain_msi_resv msi_resv = {.size = 0, .alignment = 0};
+ struct vfio_iommu_type1_info_cap_msi_resv *cap;
+ struct vfio_info_cap_header *header;
+ struct iommu_domain_msi_resv iter;
+ struct vfio_domain *d;
+
+ mutex_lock(&iommu->lock);
+
+ list_for_each_entry(d, &iommu->domain_list, next) {
+ if (iommu_domain_get_attr(d->domain,
+ DOMAIN_ATTR_MSI_RESV, &iter))
+ continue;
+ if (iter.size > msi_resv.size) {
+ msi_resv.size = iter.size;
+ msi_resv.alignment = iter.alignment;
+ }
+ }
+
+ if (!msi_resv.size)
+ return 0;
+
+ mutex_unlock(&iommu->lock);
+
+ header = vfio_info_cap_add(caps, sizeof(*cap),
+ VFIO_IOMMU_TYPE1_INFO_CAP_MSI_RESV, 1);
+
+ if (IS_ERR(header))
+ return PTR_ERR(header);
+
+ cap = container_of(header, struct vfio_iommu_type1_info_cap_msi_resv,
+ header);
+
+ cap->alignment = msi_resv.alignment;
+ cap->size = msi_resv.size;
+
+ return 0;
+}
+
static long vfio_iommu_type1_ioctl(void *iommu_data,
unsigned int cmd, unsigned long arg)
{
@@ -1168,8 +1208,10 @@ static long vfio_iommu_type1_ioctl(void *iommu_data,
}
} else if (cmd == VFIO_IOMMU_GET_INFO) {
struct vfio_iommu_type1_info info;
+ struct vfio_info_cap caps = { .buf = NULL, .size = 0 };
+ int ret;
- minsz = offsetofend(struct vfio_iommu_type1_info, iova_pgsizes);
+ minsz = offsetofend(struct vfio_iommu_type1_info, cap_offset);
if (copy_from_user(&info, (void __user *)arg, minsz))
return -EFAULT;
@@ -1181,6 +1223,29 @@ static long vfio_iommu_type1_ioctl(void *iommu_data,
info.iova_pgsizes = vfio_pgsize_bitmap(iommu);
+ ret = msi_resv_caps(iommu, &caps);
+ if (ret)
+ return ret;
+
+ if (caps.size) {
+ info.flags |= VFIO_IOMMU_INFO_CAPS;
+ if (info.argsz < sizeof(info) + caps.size) {
+ info.argsz = sizeof(info) + caps.size;
+ info.cap_offset = 0;
+ } else {
+ vfio_info_cap_shift(&caps, sizeof(info));
+ if (copy_to_user((void __user *)arg +
+ sizeof(info), caps.buf,
+ caps.size)) {
+ kfree(caps.buf);
+ return -EFAULT;
+ }
+ info.cap_offset = sizeof(info);
+ }
+
+ kfree(caps.buf);
+ }
+
return copy_to_user((void __user *)arg, &info, minsz) ?
-EFAULT : 0;
diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
index 4a9dbc2..e34a9a6 100644
--- a/include/uapi/linux/vfio.h
+++ b/include/uapi/linux/vfio.h
@@ -488,7 +488,23 @@ struct vfio_iommu_type1_info {
__u32 argsz;
__u32 flags;
#define VFIO_IOMMU_INFO_PGSIZES (1 << 0) /* supported page sizes info */
- __u64 iova_pgsizes; /* Bitmap of supported page sizes */
+#define VFIO_IOMMU_INFO_CAPS (1 << 1) /* Info supports caps */
+ __u64 iova_pgsizes; /* Bitmap of supported page sizes */
+ __u32 cap_offset; /* Offset within info struct of first cap */
+ __u32 __resv;
+};
+
+/*
+ * The MSI_RESV capability allows to report the MSI reserved IOVA requirements:
+ * In case this capability is supported, the userspace must provide an IOVA
+ * window characterized by @size and @alignment using VFIO_IOMMU_MAP_DMA with
+ * RESERVED_MSI_IOVA flag.
+ */
+#define VFIO_IOMMU_TYPE1_INFO_CAP_MSI_RESV 1
+struct vfio_iommu_type1_info_cap_msi_resv {
+ struct vfio_info_cap_header header;
+ __u64 size; /* requested IOVA aperture size in bytes */
+ __u64 alignment; /* requested byte alignment of the window */
};
#define VFIO_IOMMU_GET_INFO _IO(VFIO_TYPE, VFIO_BASE + 12)
@@ -503,6 +519,8 @@ struct vfio_iommu_type1_info {
* IOVA region that will be used on some platforms to map the host MSI frames.
* In that specific case, vaddr is ignored. Once registered, an MSI reserved
* IOVA region stays until the container is closed.
+ * The requirement for provisioning such reserved IOVA range can be checked by
+ * checking the VFIO_IOMMU_TYPE1_INFO_CAP_MSI_RESV capability.
*/
struct vfio_iommu_type1_dma_map {
__u32 argsz;
--
1.9.1
^ permalink raw reply related
* [PATCH v14 15/16] iommu/arm-smmu: Do not advertise IOMMU_CAP_INTR_REMAP
From: Eric Auger @ 2016-10-12 13:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1476278544-3397-1-git-send-email-eric.auger@redhat.com>
Do not advertise IOMMU_CAP_INTR_REMAP for arm-smmu(-v3). Indeed the
irq_remapping capability is abstracted on irqchip side for ARM as
opposed to Intel IOMMU featuring IRQ remapping HW.
So to check IRQ remapping capability, the msi domain needs to be
checked instead.
This commit affects platform and PCIe device assignment use cases
on any platform featuring an unsafe MSI controller (currently the
ARM GICv2m). For those platforms the VFIO module must be loaded with
allow_unsafe_interrupts set to 1.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
v9 -> v10:
- reword the commit message (allow_unsafe_interrupts)
---
drivers/iommu/arm-smmu-v3.c | 3 ++-
drivers/iommu/arm-smmu.c | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 572cad8..d71a955 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -1371,7 +1371,8 @@ static bool arm_smmu_capable(enum iommu_cap cap)
case IOMMU_CAP_CACHE_COHERENCY:
return true;
case IOMMU_CAP_INTR_REMAP:
- return true; /* MSIs are just memory writes */
+ /* interrupt translation handled at MSI controller level */
+ return false;
case IOMMU_CAP_NOEXEC:
return true;
default:
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index ae20b9c..becad89 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1361,7 +1361,8 @@ static bool arm_smmu_capable(enum iommu_cap cap)
*/
return true;
case IOMMU_CAP_INTR_REMAP:
- return true; /* MSIs are just memory writes */
+ /* interrupt translation handled at MSI controller level */
+ return false;
case IOMMU_CAP_NOEXEC:
return true;
default:
--
1.9.1
^ permalink raw reply related
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