* [PATCH RFC v3 5/7] x86: Hook up epoll_ctl_batch syscall
From: Fam Zheng @ 2015-02-13 9:04 UTC (permalink / raw)
To: linux-kernel
Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Alexander Viro,
Andrew Morton, Kees Cook, Andy Lutomirski, David Herrmann,
Alexei Starovoitov, Miklos Szeredi, David Drysdale, Oleg Nesterov,
David S. Miller, Vivek Goyal, Mike Frysinger, Theodore Ts'o,
Heiko Carstens, Rasmus Villemoes, Rashika Kheria, Hugh Dickins,
Mathieu Desnoyers, Fam Zheng, Peter Zijlstra <peter>
In-Reply-To: <1423818243-15410-1-git-send-email-famz@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
---
arch/x86/syscalls/syscall_32.tbl | 1 +
arch/x86/syscalls/syscall_64.tbl | 1 +
2 files changed, 2 insertions(+)
diff --git a/arch/x86/syscalls/syscall_32.tbl b/arch/x86/syscalls/syscall_32.tbl
index b3560ec..fe809f6 100644
--- a/arch/x86/syscalls/syscall_32.tbl
+++ b/arch/x86/syscalls/syscall_32.tbl
@@ -365,3 +365,4 @@
356 i386 memfd_create sys_memfd_create
357 i386 bpf sys_bpf
358 i386 execveat sys_execveat stub32_execveat
+359 i386 epoll_ctl_batch sys_epoll_ctl_batch
diff --git a/arch/x86/syscalls/syscall_64.tbl b/arch/x86/syscalls/syscall_64.tbl
index 8d656fb..67b2ea4 100644
--- a/arch/x86/syscalls/syscall_64.tbl
+++ b/arch/x86/syscalls/syscall_64.tbl
@@ -329,6 +329,7 @@
320 common kexec_file_load sys_kexec_file_load
321 common bpf sys_bpf
322 64 execveat stub_execveat
+323 64 epoll_ctl_batch sys_epoll_ctl_batch
#
# x32-specific system call numbers start at 512 to avoid cache impact
--
1.9.3
^ permalink raw reply related
* [PATCH RFC v3 6/7] epoll: Add implementation for epoll_pwait1
From: Fam Zheng @ 2015-02-13 9:04 UTC (permalink / raw)
To: linux-kernel
Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Alexander Viro,
Andrew Morton, Kees Cook, Andy Lutomirski, David Herrmann,
Alexei Starovoitov, Miklos Szeredi, David Drysdale, Oleg Nesterov,
David S. Miller, Vivek Goyal, Mike Frysinger, Theodore Ts'o,
Heiko Carstens, Rasmus Villemoes, Rashika Kheria, Hugh Dickins,
Mathieu Desnoyers, Fam Zheng, Peter Zijlstra <peter>
In-Reply-To: <1423818243-15410-1-git-send-email-famz@redhat.com>
This is the new implementation for poll which has a flags parameter and
packs a number of parameters into a structure.
The main advantage of it over existing epoll_pwait is about timeout:
epoll_pwait expects a relative millisecond value, while epoll_pwait1
accepts 1) a timespec which is in nanosecond granularity; 2) a clockid
to allow using a clock other than CLOCK_MONOTONIC.
The 'flags' field in params is reserved for now and must be zero. The
next step would be allowing absolute timeout value.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
fs/eventpoll.c | 27 +++++++++++++++++++++++++++
include/linux/syscalls.h | 5 +++++
include/uapi/linux/eventpoll.h | 7 +++++++
3 files changed, 39 insertions(+)
diff --git a/fs/eventpoll.c b/fs/eventpoll.c
index 12e2e63..cc51e8c 100644
--- a/fs/eventpoll.c
+++ b/fs/eventpoll.c
@@ -2117,6 +2117,33 @@ out:
return ret;
}
+SYSCALL_DEFINE5(epoll_pwait1, int, epfd, int, flags,
+ struct epoll_event __user *, events,
+ int, maxevents,
+ struct epoll_wait_params __user *, params)
+{
+ struct epoll_wait_params p;
+ ktime_t kt = { 0 };
+ sigset_t sigmask;
+
+ if (flags)
+ return -EINVAL;
+ if (!params)
+ return -EINVAL;
+ if (copy_from_user(&p, params, sizeof(p)))
+ return -EFAULT;
+ if (p.sigmask) {
+ if (copy_from_user(&sigmask, p.sigmask, sizeof(sigmask)))
+ return -EFAULT;
+ if (p.sigsetsize != sizeof(p.sigmask))
+ return -EINVAL;
+ }
+ kt = timespec_to_ktime(p.timeout);
+
+ return epoll_pwait_do(epfd, events, maxevents, p.clockid,
+ kt, p.sigmask ? &sigmask : NULL);
+}
+
#ifdef CONFIG_COMPAT
COMPAT_SYSCALL_DEFINE6(epoll_pwait, int, epfd,
struct epoll_event __user *, events,
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 7d784e3..a4823d9 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -13,6 +13,7 @@
struct epoll_event;
struct epoll_ctl_cmd;
+struct epoll_wait_params;
struct iattr;
struct inode;
struct iocb;
@@ -635,6 +636,10 @@ asmlinkage long sys_epoll_pwait(int epfd, struct epoll_event __user *events,
int maxevents, int timeout,
const sigset_t __user *sigmask,
size_t sigsetsize);
+asmlinkage long sys_epoll_pwait1(int epfd, int flags,
+ struct epoll_event __user *events,
+ int maxevents,
+ struct epoll_wait_params __user *params);
asmlinkage long sys_epoll_ctl_batch(int epfd, int flags,
int ncmds,
struct epoll_ctl_cmd __user *cmds);
diff --git a/include/uapi/linux/eventpoll.h b/include/uapi/linux/eventpoll.h
index 4e18b17..d35c591 100644
--- a/include/uapi/linux/eventpoll.h
+++ b/include/uapi/linux/eventpoll.h
@@ -72,6 +72,13 @@ struct epoll_ctl_cmd {
int result;
} EPOLL_PACKED;
+struct epoll_wait_params {
+ int clockid;
+ struct timespec timeout;
+ sigset_t *sigmask;
+ size_t sigsetsize;
+} EPOLL_PACKED;
+
#ifdef CONFIG_PM_SLEEP
static inline void ep_take_care_of_epollwakeup(struct epoll_event *epev)
{
--
1.9.3
^ permalink raw reply related
* [PATCH RFC v3 7/7] x86: Hook up epoll_pwait1 syscall
From: Fam Zheng @ 2015-02-13 9:04 UTC (permalink / raw)
To: linux-kernel
Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Alexander Viro,
Andrew Morton, Kees Cook, Andy Lutomirski, David Herrmann,
Alexei Starovoitov, Miklos Szeredi, David Drysdale, Oleg Nesterov,
David S. Miller, Vivek Goyal, Mike Frysinger, Theodore Ts'o,
Heiko Carstens, Rasmus Villemoes, Rashika Kheria, Hugh Dickins,
Mathieu Desnoyers, Fam Zheng, Peter Zijlstra <peter>
In-Reply-To: <1423818243-15410-1-git-send-email-famz@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
---
arch/x86/syscalls/syscall_32.tbl | 1 +
arch/x86/syscalls/syscall_64.tbl | 1 +
2 files changed, 2 insertions(+)
diff --git a/arch/x86/syscalls/syscall_32.tbl b/arch/x86/syscalls/syscall_32.tbl
index fe809f6..bf912d8 100644
--- a/arch/x86/syscalls/syscall_32.tbl
+++ b/arch/x86/syscalls/syscall_32.tbl
@@ -366,3 +366,4 @@
357 i386 bpf sys_bpf
358 i386 execveat sys_execveat stub32_execveat
359 i386 epoll_ctl_batch sys_epoll_ctl_batch
+360 i386 epoll_pwait1 sys_epoll_pwait1
diff --git a/arch/x86/syscalls/syscall_64.tbl b/arch/x86/syscalls/syscall_64.tbl
index 67b2ea4..9246ad5 100644
--- a/arch/x86/syscalls/syscall_64.tbl
+++ b/arch/x86/syscalls/syscall_64.tbl
@@ -330,6 +330,7 @@
321 common bpf sys_bpf
322 64 execveat stub_execveat
323 64 epoll_ctl_batch sys_epoll_ctl_batch
+324 64 epoll_pwait1 sys_epoll_pwait1
#
# x32-specific system call numbers start at 512 to avoid cache impact
--
1.9.3
^ permalink raw reply related
* Re: [PATCH RFC v3 0/7] epoll: Introduce new syscalls, epoll_ctl_batch and epoll_pwait1
From: Omar Sandoval @ 2015-02-13 9:53 UTC (permalink / raw)
To: Fam Zheng
Cc: linux-kernel, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86,
Alexander Viro, Andrew Morton, Kees Cook, Andy Lutomirski,
David Herrmann, Alexei Starovoitov, Miklos Szeredi,
David Drysdale, Oleg Nesterov, David S. Miller, Vivek Goyal,
Mike Frysinger, Theodore Ts'o, Heiko Carstens,
Rasmus Villemoes, Rashika Kheria, Hugh Dickins, Mathieu Desnoyers,
Peter Zijlstra <peterz@
In-Reply-To: <1423818243-15410-1-git-send-email-famz@redhat.com>
On Fri, Feb 13, 2015 at 05:03:56PM +0800, Fam Zheng wrote:
> Hi all,
>
> This is the updated series for the new epoll system calls, with the cover
> letter rewritten which includes some more explanation. Comments are very
> welcome!
>
> Original Motivation
> ===================
>
> QEMU, and probably many more select/poll based applications, will consider
> epoll as an alternative, when its event loop needs to handle a big number of
> fds. However, there are currently two concerns with epoll which prevents the
> switching from ppoll to epoll.
>
> The major one is the timeout precision.
>
> For example in QEMU, the main loop takes care of calling callbacks at a
> specific timeout - the QEMU timer API. The timeout value in ppoll depends on
> the next firing timer. epoll_pwait's millisecond timeout is so coarse that
> rounding up the timeout will hurt performance badly.
>
> The minor one is the number of system call to update fd set.
>
> While epoll can handle a large number of fds quickly, it still requires one
> epoll_ctl per fd update, compared to the one-shot call to select/poll with an
> fd array. This may as well make epoll inferior to ppoll in the cases where a
> small, but frequently changing set of fds are polled by the event loop.
>
> This series introduces two new epoll APIs to address them respectively. The
> idea of epoll_ctl_batch is suggested by Andy Lutomirski in [1], who also
> suggested clockid as a parameter in epoll_pwait1.
>
> Discussion
> ==========
>
> [Note: This is the question part regarding the interface contract of
> epoll_ctl_batch. If you don't have the context of what is epoll_ctl_batch yet,
> please skip this part and probably start with the man page style documentation.
> You can resume to this section later.]
>
> [Thanks to Omar Sandoval <osandov@osandov.com>, who pointed out this in
> reviewing v1]
>
> We try to report status for each command in epoll_ctl_batch, by writting to
> user provided command array (pointed to cmds). The tricky thing in the
> implementation is that, copying the results back to userspace comes last, after
> the commands are executed. At this point, if the copy_to_user fails, the
> effects are done and no return - or if we add some mechanism to revert it, the
> code will be too complicated and slow.
>
> In above corner case, the return value of epoll_ctl_batch is smaller than
> ncmds, which assures our caller that last N commands failed, where N = ncmds -
> ret. But they'll also find that cmd.result is not changed to error code.
>
> I suppose this is not a big deal, because 1) it should happen very rarely. 2)
> user does know the actual number of commands that succeed.
>
> So, do we leave it as is? Or is there any way to improve?
>
> One tiny improvement (not a complete fix) in my mind is a testing copy_to_user
> before we execute the commands. If it succeeds, it's even less likely the last
> copy_to_user could fail, so that we can even probably assert it won't. The
> testing 'copy_to_user(cmds, &kcmds, ...)' will not hurt functionality if do it
> right after 'copy_from_user(&kcmds, cmds, ...)'. But I'm not sure about the
> performance impact, especially when @ncmds is big.
>
I don't think this is the right thing to do, since, for example, another
thread could unmap the memory region holding buffer between the "check"
copy_to_user and the actual one.
The two alternatives that I see are:
1. If copy_to_user fails, ignore it and return the number of commands
that succeeded (i.e., what the code in your patch does).
2. If copy_to_user fails, return -EFAULT, regardless of how many
commands succeeded.
The problem with 1 is that it could potentially mask bugs in a user
program. You could imagine a buggy program that passes a read-only
buffer to epoll_ctl_batch and never finds out about it because they
don't get the error. (Then, when there's a real error in one of the
epoll_ctl_cmds, but .result is 0, someone will be very confused.)
So I think 2 is the better option. Sure, the user will have no idea how
many commands were executed, but when would EFAULT on this system call
be part of normal operation, anyways? You'll find the memory bug, fix
it, and rest easy knowing that nothing is silently failing behind your
back.
> Links
> =====
>
> [1]: http://lists.openwall.net/linux-kernel/2015/01/08/542
>
> Changelog
> =========
>
> Changes from v2 (https://lkml.org/lkml/2015/2/4/105)
> ----------------------------------------------------
>
> - Rename epoll_ctl_cmd.error_hint to "result". [Michael]
>
> - Add background introduction in cover letter. [Michael]
>
> - Expand the last struct of epoll_pwait1, add clockid and timespec.
>
> - Update man page in cover letter accordingly:
>
> * "error_hint" -> "result".
> * The result field's caveat in "RETURN VALUE" secion of epoll_ctl_batch.
>
> Please review!
>
> Changes from v1 (https://lkml.org/lkml/2015/1/20/189)
> -----------------------------------------------------
>
> - As discussed in previous thread [1], split the call to epoll_ctl_batch and
> epoll_pwait. [Michael]
>
> - Fix memory leaks. [Omar]
>
> - Add a short comment about the ignored copy_to_user failure. [Omar]
>
> - Cover letter rewritten.
>
[snip]
--
Omar
^ permalink raw reply
* Re: [PATCH 02/14] ARM: ARMv7M: Enlarge vector table to 256 entries
From: Uwe Kleine-König @ 2015-02-13 10:00 UTC (permalink / raw)
To: Maxime Coquelin
Cc: Geert Uytterhoeven, Jonathan Corbet, Rob Herring, Pawel Moll,
Mark Rutland, Ian Campbell, Kumar Gala, Philipp Zabel,
Russell King, Daniel Lezcano, Thomas Gleixner, Linus Walleij,
Greg Kroah-Hartman, Jiri Slaby, Arnd Bergmann, Andrew Morton,
David S. Miller, Mauro Carvalho Chehab, Joe Perches,
Antti Palosaari, Tejun Heo, Will Deacon, Nikolay Borisov
In-Reply-To: <CALszF6BDa9pUb534YN2z9DbYA+hPCnG8XYy5YbjJwSiseKz4xg@mail.gmail.com>
On Fri, Feb 13, 2015 at 09:42:46AM +0100, Maxime Coquelin wrote:
> Hi Geert,
>
> 2015-02-12 21:34 GMT+01:00 Geert Uytterhoeven <geert@linux-m68k.org>:
> > On Thu, Feb 12, 2015 at 6:45 PM, Maxime Coquelin
> > <mcoquelin.stm32@gmail.com> wrote:
> >> From Cortex-M4 and M7 reference manuals, the nvic supports up to 240
> >> interrupts. So the number of entries in vectors table is 256.
> >>
> >> This patch adds the missing entries, and change the alignement, so that
> >> vector_table remains naturally aligned.
> >
> > Shouldn't this depend on ARCH_STM32, or some other M4 or M7 specific
> > Kconfig option, to avoid wasting the space on other CPUs?
>
> Actually, the STM32F429 has 90 interrupts, so it would need 106
> entries in the vector table.
> The maximum of supported interrupts is not only for Cortex-M4 and M7,
> this is also true for Cortex-M3.
>
> I see two possibilities:
> 1 - We declare the vector table for the maximum supported number of
> IRQs, as this patch does.
> - Pro: it will be functionnal with all Cortex-M MCUs
> - Con: Waste of less than 1KB for memory
> 2 - We introduce a config flag that provides the number of interrupts
> - Pro: No more memory waste
> - Con: Need to declare a per MCU model config flag.
I'd vote for 2, something like:
config CPUV7M_NUM_IRQ
int
default 90 if STM32F429
default 38 if EFM32GG
default 240
then there is a working default and platforms being short on memory can
configure as appropriate. (The only down side is that if we create
multi-platfrom images at some time in the future either all or none of
the supported platforms must provide a value here.)
> Then, regarding the natural alignment, is there a way to ensure it
> depending on the value of a config flag?
The exact wording in ARMARMv7-M is:
The Vector table must be naturally aligned to a power of two
whose alignment value is greater than or equal
to (Number of Exceptions supported x 4), with a minimum
alignment of 128 bytes.
> Or we should keep it at the maximum value possible?
So we need:
.align x
with x being max(7, ceil(log((CPUV7M_NUM_IRQ + 16) * 4, 2))). So the
alignment needed is between 7 and 10.
If the assembler supports an expression here I'd use that. But before
adding strange hacks to generate the right value there better go for a
static value like:
/* The vector table must be naturally aligned */
#if CONFIG_CPUV7M_NUM_IRQ <= 112
.align 9 /* log2((112 + 16) * 4) */
#else
.align 10
#endif
Further steps would be:
CONFIG_CPUV7M_NUM_IRQ <= 48 -> .align 8
CONFIG_CPUV7M_NUM_IRQ <= 16 -> .align 7
Probably it's not worth to add the respective #ifdefs here.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 12/14] ARM: dts: Introduce STM32F429 MCU
From: Philipp Zabel @ 2015-02-13 11:47 UTC (permalink / raw)
To: Maxime Coquelin
Cc: Jonathan Corbet, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Russell King, Daniel Lezcano,
Thomas Gleixner, Linus Walleij, Greg Kroah-Hartman, Jiri Slaby,
Arnd Bergmann, Andrew Morton, David S. Miller,
Mauro Carvalho Chehab, Joe Perches, Antti Palosaari, Tejun Heo,
Will Deacon, Nikolay Borisov, Rusty Russell, Kees Cook, Michal
In-Reply-To: <1423763164-5606-13-git-send-email-mcoquelin.stm32@gmail.com>
Hi Maxime,
Am Donnerstag, den 12.02.2015, 18:46 +0100 schrieb Maxime Coquelin:
[...]
> + soc {
> + reset_ahb1: reset@40023810 {
> + #reset-cells = <1>;
> + compatible = "st,stm32-reset";
> + reg = <0x40023810 0x4>;
> + };
> +
> + reset_ahb2: reset@40023814 {
> + #reset-cells = <1>;
> + compatible = "st,stm32-reset";
> + reg = <0x40023814 0x4>;
> + };
> +
> + reset_ahb3: reset@40023818 {
> + #reset-cells = <1>;
> + compatible = "st,stm32-reset";
> + reg = <0x40023818 0x4>;
> + };
> +
> + reset_apb1: reset@40023820 {
> + #reset-cells = <1>;
> + compatible = "st,stm32-reset";
> + reg = <0x40023820 0x4>;
> + };
> +
> + reset_apb2: reset@40023824 {
> + #reset-cells = <1>;
> + compatible = "st,stm32-reset";
> + reg = <0x40023824 0x4>;
> + };
These are mostly consecutive, single registers. I wonder if these are
part of the same IP block and thus should be grouped together into the
same reset controller node?
regards
Philipp
^ permalink raw reply
* Re: [PATCH 04/14] reset: Add reset_controller_of_init() function
From: Philipp Zabel @ 2015-02-13 11:49 UTC (permalink / raw)
To: Maxime Coquelin
Cc: Jonathan Corbet, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Russell King, Daniel Lezcano,
Thomas Gleixner, Linus Walleij, Greg Kroah-Hartman, Jiri Slaby,
Arnd Bergmann, Andrew Morton, David S. Miller,
Mauro Carvalho Chehab, Joe Perches, Antti Palosaari, Tejun Heo,
Will Deacon, Nikolay Borisov, Rusty Russell, Kees Cook, Michal
In-Reply-To: <1423763164-5606-5-git-send-email-mcoquelin.stm32@gmail.com>
Hi Maxime,
Am Donnerstag, den 12.02.2015, 18:45 +0100 schrieb Maxime Coquelin:
> Some platforms need to initialize the reset controller before the timers.
>
> This patch introduces a reset_controller_of_init() function that can be
> called before the timers intialization.
>
> Signed-off-by: Maxime Coquelin <mcoquelin.stm32@gmail.com>
> ---
> drivers/reset/core.c | 20 ++++++++++++++++++++
> include/asm-generic/vmlinux.lds.h | 4 +++-
> include/linux/reset-controller.h | 6 ++++++
> 3 files changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/reset/core.c b/drivers/reset/core.c
> index 7955e00..18ee579 100644
> --- a/drivers/reset/core.c
> +++ b/drivers/reset/core.c
> @@ -86,6 +86,26 @@ void reset_controller_unregister(struct reset_controller_dev *rcdev)
> }
> EXPORT_SYMBOL_GPL(reset_controller_unregister);
>
> +extern struct of_device_id __reset_ctrl_of_table[];
> +
> +static const struct of_device_id __reset_ctrl_of_table_sentinel
> + __used __section(__reset_ctrl_of_table_end);
> +
> +void __init reset_controller_of_init(void)
The patch looks fine to me, but this function is missing a kerneldoc
comment.
> +{
> + struct device_node *np;
> + const struct of_device_id *match;
> + of_init_fn_1 init_func;
> +
> + for_each_matching_node_and_match(np, __reset_ctrl_of_table, &match) {
> + if (!of_device_is_available(np))
> + continue;
> +
> + init_func = match->data;
> + init_func(np);
> + }
> +}
> +
> /**
> * reset_control_reset - reset the controlled device
> * @rstc: reset controller
regards
Philipp
^ permalink raw reply
* Re: [PATCH 12/14] ARM: dts: Introduce STM32F429 MCU
From: Maxime Coquelin @ 2015-02-13 15:59 UTC (permalink / raw)
To: Philipp Zabel
Cc: Jonathan Corbet, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Russell King, Daniel Lezcano,
Thomas Gleixner, Linus Walleij, Greg Kroah-Hartman, Jiri Slaby,
Arnd Bergmann, Andrew Morton, David S. Miller,
Mauro Carvalho Chehab, Joe Perches, Antti Palosaari, Tejun Heo,
Will Deacon, Nikolay Borisov, Rusty Russell, Kees Cook, Michal
In-Reply-To: <1423828078.4182.17.camel@pengutronix.de>
Hi Philipp,
2015-02-13 12:47 GMT+01:00 Philipp Zabel <p.zabel@pengutronix.de>:
> Hi Maxime,
>
> Am Donnerstag, den 12.02.2015, 18:46 +0100 schrieb Maxime Coquelin:
> [...]
>> + soc {
>> + reset_ahb1: reset@40023810 {
>> + #reset-cells = <1>;
>> + compatible = "st,stm32-reset";
>> + reg = <0x40023810 0x4>;
>> + };
>> +
>> + reset_ahb2: reset@40023814 {
>> + #reset-cells = <1>;
>> + compatible = "st,stm32-reset";
>> + reg = <0x40023814 0x4>;
>> + };
>> +
>> + reset_ahb3: reset@40023818 {
>> + #reset-cells = <1>;
>> + compatible = "st,stm32-reset";
>> + reg = <0x40023818 0x4>;
>> + };
>> +
>> + reset_apb1: reset@40023820 {
>> + #reset-cells = <1>;
>> + compatible = "st,stm32-reset";
>> + reg = <0x40023820 0x4>;
>> + };
>> +
>> + reset_apb2: reset@40023824 {
>> + #reset-cells = <1>;
>> + compatible = "st,stm32-reset";
>> + reg = <0x40023824 0x4>;
>> + };
>
> These are mostly consecutive, single registers. I wonder if these are
> part of the same IP block and thus should be grouped together into the
> same reset controller node?
What I could to is to have two instances. One for AHB and one for APB domain.
Doing this, I will have one instance per domain, and only consecutive registers.
Is it fine for you?
Thanks,
Maxime
>
> regards
> Philipp
>
^ permalink raw reply
* Re: [PATCH 04/14] reset: Add reset_controller_of_init() function
From: Maxime Coquelin @ 2015-02-13 16:00 UTC (permalink / raw)
To: Philipp Zabel
Cc: Jonathan Corbet, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Russell King, Daniel Lezcano,
Thomas Gleixner, Linus Walleij, Greg Kroah-Hartman, Jiri Slaby,
Arnd Bergmann, Andrew Morton, David S. Miller,
Mauro Carvalho Chehab, Joe Perches, Antti Palosaari, Tejun Heo,
Will Deacon, Nikolay Borisov, Rusty Russell, Kees Cook, Michal
In-Reply-To: <1423828144.4182.18.camel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Hi Philipp,
2015-02-13 12:49 GMT+01:00 Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>:
> Hi Maxime,
>
> Am Donnerstag, den 12.02.2015, 18:45 +0100 schrieb Maxime Coquelin:
>> Some platforms need to initialize the reset controller before the timers.
>>
>> This patch introduces a reset_controller_of_init() function that can be
>> called before the timers intialization.
>>
>> Signed-off-by: Maxime Coquelin <mcoquelin.stm32-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>> drivers/reset/core.c | 20 ++++++++++++++++++++
>> include/asm-generic/vmlinux.lds.h | 4 +++-
>> include/linux/reset-controller.h | 6 ++++++
>> 3 files changed, 29 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/reset/core.c b/drivers/reset/core.c
>> index 7955e00..18ee579 100644
>> --- a/drivers/reset/core.c
>> +++ b/drivers/reset/core.c
>> @@ -86,6 +86,26 @@ void reset_controller_unregister(struct reset_controller_dev *rcdev)
>> }
>> EXPORT_SYMBOL_GPL(reset_controller_unregister);
>>
>> +extern struct of_device_id __reset_ctrl_of_table[];
>> +
>> +static const struct of_device_id __reset_ctrl_of_table_sentinel
>> + __used __section(__reset_ctrl_of_table_end);
>> +
>> +void __init reset_controller_of_init(void)
>
> The patch looks fine to me, but this function is missing a kerneldoc
> comment.
Right! It will be fixed in the v2.
Thanks,
Maxime
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 12/14] ARM: dts: Introduce STM32F429 MCU
From: Philipp Zabel @ 2015-02-13 16:25 UTC (permalink / raw)
To: Maxime Coquelin
Cc: Jonathan Corbet, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Russell King, Daniel Lezcano,
Thomas Gleixner, Linus Walleij, Greg Kroah-Hartman, Jiri Slaby,
Arnd Bergmann, Andrew Morton, David S. Miller,
Mauro Carvalho Chehab, Joe Perches, Antti Palosaari, Tejun Heo,
Will Deacon, Nikolay Borisov, Rusty Russell, Kees Cook, Michal
In-Reply-To: <CALszF6D+5mXajUWarwq8M4yL_Nc+x1Bmu4D7_HxDRy2kGRXNVQ@mail.gmail.com>
Hi Maxime,
Am Freitag, den 13.02.2015, 16:59 +0100 schrieb Maxime Coquelin:
> Hi Philipp,
>
> 2015-02-13 12:47 GMT+01:00 Philipp Zabel <p.zabel@pengutronix.de>:
> > Hi Maxime,
> >
> > Am Donnerstag, den 12.02.2015, 18:46 +0100 schrieb Maxime Coquelin:
> > [...]
> >> + soc {
> >> + reset_ahb1: reset@40023810 {
> >> + #reset-cells = <1>;
> >> + compatible = "st,stm32-reset";
> >> + reg = <0x40023810 0x4>;
> >> + };
> >> +
> >> + reset_ahb2: reset@40023814 {
> >> + #reset-cells = <1>;
> >> + compatible = "st,stm32-reset";
> >> + reg = <0x40023814 0x4>;
> >> + };
> >> +
> >> + reset_ahb3: reset@40023818 {
> >> + #reset-cells = <1>;
> >> + compatible = "st,stm32-reset";
> >> + reg = <0x40023818 0x4>;
> >> + };
> >> +
> >> + reset_apb1: reset@40023820 {
> >> + #reset-cells = <1>;
> >> + compatible = "st,stm32-reset";
> >> + reg = <0x40023820 0x4>;
> >> + };
> >> +
> >> + reset_apb2: reset@40023824 {
> >> + #reset-cells = <1>;
> >> + compatible = "st,stm32-reset";
> >> + reg = <0x40023824 0x4>;
> >> + };
> >
> > These are mostly consecutive, single registers. I wonder if these are
> > part of the same IP block and thus should be grouped together into the
> > same reset controller node?
>
> What I could to is to have two instances. One for AHB and one for APB domain.
> Doing this, I will have one instance per domain, and only consecutive registers.
> Is it fine for you?
Looking at
http://www.st.com/web/en/resource/technical/document/reference_manual/DM00031020.pdf
Table 34 (RCC register map and reset values), I'd say there is a single
"Reset and Clock Control" device at 0x40023800 - 0x40023884:
soc {
rcc: rcc@40023800 {
#clock-cells = <1>;
#reset-cells = <1>;
compatible = "st,stm32-rcc";
reg = <0x40023800 0x84>;
};
...
If you really want to describe the reset controller parts (offsets +0x10
to +0x24) in a separate node, I won't argue against it too long,
although this is a somewhat arbitrary decision.
In any case, the whole register at offset +0x1c is reserved, so there is
no reason to split the reset controller. It is ok to have unused ranges
as is already the case with reserved bits inside the used registers.
regards
Philipp
^ permalink raw reply
* Re: [PATCH 12/14] ARM: dts: Introduce STM32F429 MCU
From: Maxime Coquelin @ 2015-02-13 16:41 UTC (permalink / raw)
To: Philipp Zabel
Cc: Mark Rutland, linux-doc@vger.kernel.org, Linus Walleij,
Will Deacon, Nikolay Borisov, linux-api@vger.kernel.org,
Jiri Slaby, Mauro Carvalho Chehab, Linux-Arch, Russell King,
Pawel Moll, Jonathan Corbet, Daniel Lezcano, Antti Palosaari,
linux-serial@vger.kernel.org, devicetree@vger.kernel.org,
Kees Cook, Arnd Bergmann, Ian Campbell, Rusty Russell,
Joe Perches, Rob
In-Reply-To: <1423844735.4182.52.camel@pengutronix.de>
2015-02-13 17:25 GMT+01:00 Philipp Zabel <p.zabel@pengutronix.de>:
> Hi Maxime,
>
> Am Freitag, den 13.02.2015, 16:59 +0100 schrieb Maxime Coquelin:
>> Hi Philipp,
>>
>> 2015-02-13 12:47 GMT+01:00 Philipp Zabel <p.zabel@pengutronix.de>:
>> > Hi Maxime,
>> >
>> > Am Donnerstag, den 12.02.2015, 18:46 +0100 schrieb Maxime Coquelin:
>> > [...]
>> >> + soc {
>> >> + reset_ahb1: reset@40023810 {
>> >> + #reset-cells = <1>;
>> >> + compatible = "st,stm32-reset";
>> >> + reg = <0x40023810 0x4>;
>> >> + };
>> >> +
>> >> + reset_ahb2: reset@40023814 {
>> >> + #reset-cells = <1>;
>> >> + compatible = "st,stm32-reset";
>> >> + reg = <0x40023814 0x4>;
>> >> + };
>> >> +
>> >> + reset_ahb3: reset@40023818 {
>> >> + #reset-cells = <1>;
>> >> + compatible = "st,stm32-reset";
>> >> + reg = <0x40023818 0x4>;
>> >> + };
>> >> +
>> >> + reset_apb1: reset@40023820 {
>> >> + #reset-cells = <1>;
>> >> + compatible = "st,stm32-reset";
>> >> + reg = <0x40023820 0x4>;
>> >> + };
>> >> +
>> >> + reset_apb2: reset@40023824 {
>> >> + #reset-cells = <1>;
>> >> + compatible = "st,stm32-reset";
>> >> + reg = <0x40023824 0x4>;
>> >> + };
>> >
>> > These are mostly consecutive, single registers. I wonder if these are
>> > part of the same IP block and thus should be grouped together into the
>> > same reset controller node?
>>
>> What I could to is to have two instances. One for AHB and one for APB domain.
>> Doing this, I will have one instance per domain, and only consecutive registers.
>> Is it fine for you?
>
> Looking at
> http://www.st.com/web/en/resource/technical/document/reference_manual/DM00031020.pdf
> Table 34 (RCC register map and reset values), I'd say there is a single
> "Reset and Clock Control" device at 0x40023800 - 0x40023884:
>
> soc {
> rcc: rcc@40023800 {
> #clock-cells = <1>;
> #reset-cells = <1>;
> compatible = "st,stm32-rcc";
> reg = <0x40023800 0x84>;
> };
>
> ...
>
> If you really want to describe the reset controller parts (offsets +0x10
> to +0x24) in a separate node, I won't argue against it too long,
> although this is a somewhat arbitrary decision.
>
> In any case, the whole register at offset +0x1c is reserved, so there is
> no reason to split the reset controller. It is ok to have unused ranges
> as is already the case with reserved bits inside the used registers.
Ok. I understand your point.
But it will be more difficult at usage, because the node referencing
the fourth reset bit of apb2 register will have to pass 164 as
parameter.
It is error prone IMHO.
Other solution would be to add some defines for each reset line in the
DT-Bindings, as we do today for STi platform.
But it is giving an unneeded constraint between DT and reset trees.
Br,
Maxime
>
> regards
> Philipp
>
^ permalink raw reply
* Re: [PATCH 12/14] ARM: dts: Introduce STM32F429 MCU
From: Philipp Zabel @ 2015-02-13 19:18 UTC (permalink / raw)
To: Maxime Coquelin
Cc: Jonathan Corbet, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Russell King, Daniel Lezcano,
Thomas Gleixner, Linus Walleij, Greg Kroah-Hartman, Jiri Slaby,
Arnd Bergmann, Andrew Morton, David S. Miller,
Mauro Carvalho Chehab, Joe Perches, Antti Palosaari, Tejun Heo,
Will Deacon, Nikolay Borisov, Rusty Russell, Kees Cook, Michal
In-Reply-To: <CALszF6Ad93BgT4UsJudVKpfhhL2dbtKTTRno1XqQ5=eANkwXzA@mail.gmail.com>
Am Freitag, den 13.02.2015, 17:41 +0100 schrieb Maxime Coquelin:
> 2015-02-13 17:25 GMT+01:00 Philipp Zabel <p.zabel@pengutronix.de>:
> > Hi Maxime,
> >
> > Am Freitag, den 13.02.2015, 16:59 +0100 schrieb Maxime Coquelin:
> >> Hi Philipp,
> >>
> >> 2015-02-13 12:47 GMT+01:00 Philipp Zabel <p.zabel@pengutronix.de>:
> >> > Hi Maxime,
> >> >
> >> > Am Donnerstag, den 12.02.2015, 18:46 +0100 schrieb Maxime Coquelin:
> >> > [...]
> >> >> + soc {
> >> >> + reset_ahb1: reset@40023810 {
> >> >> + #reset-cells = <1>;
> >> >> + compatible = "st,stm32-reset";
> >> >> + reg = <0x40023810 0x4>;
> >> >> + };
> >> >> +
> >> >> + reset_ahb2: reset@40023814 {
> >> >> + #reset-cells = <1>;
> >> >> + compatible = "st,stm32-reset";
> >> >> + reg = <0x40023814 0x4>;
> >> >> + };
> >> >> +
> >> >> + reset_ahb3: reset@40023818 {
> >> >> + #reset-cells = <1>;
> >> >> + compatible = "st,stm32-reset";
> >> >> + reg = <0x40023818 0x4>;
> >> >> + };
> >> >> +
> >> >> + reset_apb1: reset@40023820 {
> >> >> + #reset-cells = <1>;
> >> >> + compatible = "st,stm32-reset";
> >> >> + reg = <0x40023820 0x4>;
> >> >> + };
> >> >> +
> >> >> + reset_apb2: reset@40023824 {
> >> >> + #reset-cells = <1>;
> >> >> + compatible = "st,stm32-reset";
> >> >> + reg = <0x40023824 0x4>;
> >> >> + };
> >> >
> >> > These are mostly consecutive, single registers. I wonder if these are
> >> > part of the same IP block and thus should be grouped together into the
> >> > same reset controller node?
> >>
> >> What I could to is to have two instances. One for AHB and one for APB domain.
> >> Doing this, I will have one instance per domain, and only consecutive registers.
> >> Is it fine for you?
> >
> > Looking at
> > http://www.st.com/web/en/resource/technical/document/reference_manual/DM00031020.pdf
> > Table 34 (RCC register map and reset values), I'd say there is a single
> > "Reset and Clock Control" device at 0x40023800 - 0x40023884:
> >
> > soc {
> > rcc: rcc@40023800 {
> > #clock-cells = <1>;
> > #reset-cells = <1>;
> > compatible = "st,stm32-rcc";
> > reg = <0x40023800 0x84>;
> > };
> >
> > ...
> >
> > If you really want to describe the reset controller parts (offsets +0x10
> > to +0x24) in a separate node, I won't argue against it too long,
> > although this is a somewhat arbitrary decision.
> >
> > In any case, the whole register at offset +0x1c is reserved, so there is
> > no reason to split the reset controller. It is ok to have unused ranges
> > as is already the case with reserved bits inside the used registers.
>
> Ok. I understand your point.
> But it will be more difficult at usage, because the node referencing
> the fourth reset bit of apb2 register will have to pass 164 as
> parameter.
> It is error prone IMHO.
>
> Other solution would be to add some defines for each reset line in the
> DT-Bindings, as we do today for STi platform.
> But it is giving an unneeded constraint between DT and reset trees.
That is a bit unfortunate, but providing the named constants in
include/dt-bindings/reset/ makes for a much better readable device tree,
so I'd prefer that solution, even if it means having to coordinate pull
requests.
regards
Philipp
^ permalink raw reply
* [PATCHv4 02/25] [media] media: Fix DVB devnode representation at media controller
From: Mauro Carvalho Chehab @ 2015-02-13 22:57 UTC (permalink / raw)
To: Linux Media Mailing List
Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab, Hans Verkuil,
Antti Palosaari, Sakari Ailus, Sylwester Nawrocki,
Ramakrishnan Muthukrishnan, Laurent Pinchart, linux-api
In-Reply-To: <cover.1423867976.git.mchehab@osg.samsung.com>
The previous provision for DVB media controller support were to
define an ID (likely meaning the adapter number) for the DVB
devnodes.
This is just plain wrong. Just like V4L, DVB devices (and any other
device node)) are uniquely identified via a (major, minor) tuple.
This is enough to uniquely identify a devnode, no matter what
API it implements.
So, before we go too far, let's mark the old v4l, fb, dvb and alsa
"devnode" info as deprecated, and just call it as "dev".
We can latter add fields specific to each API if needed.
As we don't want to break compilation on already existing apps,
let's just keep the old definitions as-is, adding a note that
those are deprecated at media-entity.h.
Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
index 86bb93fd7db8..d89d5cb465d9 100644
--- a/drivers/media/v4l2-core/v4l2-dev.c
+++ b/drivers/media/v4l2-core/v4l2-dev.c
@@ -943,8 +943,8 @@ int __video_register_device(struct video_device *vdev, int type, int nr,
vdev->vfl_type != VFL_TYPE_SUBDEV) {
vdev->entity.type = MEDIA_ENT_T_DEVNODE_V4L;
vdev->entity.name = vdev->name;
- vdev->entity.info.v4l.major = VIDEO_MAJOR;
- vdev->entity.info.v4l.minor = vdev->minor;
+ vdev->entity.info.dev.major = VIDEO_MAJOR;
+ vdev->entity.info.dev.minor = vdev->minor;
ret = media_device_register_entity(vdev->v4l2_dev->mdev,
&vdev->entity);
if (ret < 0)
diff --git a/drivers/media/v4l2-core/v4l2-device.c b/drivers/media/v4l2-core/v4l2-device.c
index 015f92aab44a..204cc67c84e8 100644
--- a/drivers/media/v4l2-core/v4l2-device.c
+++ b/drivers/media/v4l2-core/v4l2-device.c
@@ -248,8 +248,8 @@ int v4l2_device_register_subdev_nodes(struct v4l2_device *v4l2_dev)
goto clean_up;
}
#if defined(CONFIG_MEDIA_CONTROLLER)
- sd->entity.info.v4l.major = VIDEO_MAJOR;
- sd->entity.info.v4l.minor = vdev->minor;
+ sd->entity.info.dev.major = VIDEO_MAJOR;
+ sd->entity.info.dev.minor = vdev->minor;
#endif
sd->devnode = vdev;
}
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index e00459185d20..d6d74bcfe183 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -87,17 +87,7 @@ struct media_entity {
struct {
u32 major;
u32 minor;
- } v4l;
- struct {
- u32 major;
- u32 minor;
- } fb;
- struct {
- u32 card;
- u32 device;
- u32 subdevice;
- } alsa;
- int dvb;
+ } dev;
/* Sub-device specifications */
/* Nothing needed yet */
diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index d847c760e8f0..418f4fec391a 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h
@@ -78,6 +78,20 @@ struct media_entity_desc {
struct {
__u32 major;
__u32 minor;
+ } dev;
+
+#if 1
+ /*
+ * DEPRECATED: previous node specifications. Kept just to
+ * avoid breaking compilation, but media_entity_desc.dev
+ * should be used instead. In particular, alsa and dvb
+ * fields below are wrong: for all devnodes, there should
+ * be just major/minor inside the struct, as this is enough
+ * to represent any devnode, no matter what type.
+ */
+ struct {
+ __u32 major;
+ __u32 minor;
} v4l;
struct {
__u32 major;
@@ -89,6 +103,7 @@ struct media_entity_desc {
__u32 subdevice;
} alsa;
int dvb;
+#endif
/* Sub-device specifications */
/* Nothing needed yet */
--
2.1.0
^ permalink raw reply related
* [PATCHv4 04/25] [media] media: add new types for DVB devnodes
From: Mauro Carvalho Chehab @ 2015-02-13 22:57 UTC (permalink / raw)
To: Linux Media Mailing List
Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab,
linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <cover.1423867976.git.mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
Most of the DVB subdevs have already their own devnode.
Add support for them at the media controller API.
Signed-off-by: Mauro Carvalho Chehab <mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index 418f4fec391a..4c8f26243252 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h
@@ -50,7 +50,14 @@ struct media_device_info {
#define MEDIA_ENT_T_DEVNODE_V4L (MEDIA_ENT_T_DEVNODE + 1)
#define MEDIA_ENT_T_DEVNODE_FB (MEDIA_ENT_T_DEVNODE + 2)
#define MEDIA_ENT_T_DEVNODE_ALSA (MEDIA_ENT_T_DEVNODE + 3)
-#define MEDIA_ENT_T_DEVNODE_DVB (MEDIA_ENT_T_DEVNODE + 4)
+#define MEDIA_ENT_T_DEVNODE_DVB_FE (MEDIA_ENT_T_DEVNODE + 4)
+#define MEDIA_ENT_T_DEVNODE_DVB_DEMUX (MEDIA_ENT_T_DEVNODE + 5)
+#define MEDIA_ENT_T_DEVNODE_DVB_DVR (MEDIA_ENT_T_DEVNODE + 6)
+#define MEDIA_ENT_T_DEVNODE_DVB_CA (MEDIA_ENT_T_DEVNODE + 7)
+#define MEDIA_ENT_T_DEVNODE_DVB_NET (MEDIA_ENT_T_DEVNODE + 8)
+
+/* Legacy symbol. Use it to avoid userspace compilation breakages */
+#define MEDIA_ENT_T_DEVNODE_DVB MEDIA_ENT_T_DEVNODE_DVB_FE
#define MEDIA_ENT_T_V4L2_SUBDEV (2 << MEDIA_ENT_TYPE_SHIFT)
#define MEDIA_ENT_T_V4L2_SUBDEV_SENSOR (MEDIA_ENT_T_V4L2_SUBDEV + 1)
--
2.1.0
^ permalink raw reply related
* [PATCHv4 06/25] [media] media: add a subdev type for tuner
From: Mauro Carvalho Chehab @ 2015-02-13 22:57 UTC (permalink / raw)
To: Linux Media Mailing List
Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab, linux-api
In-Reply-To: <cover.1423867976.git.mchehab@osg.samsung.com>
Add MEDIA_ENT_T_V4L2_SUBDEV_TUNER to represent the V4L2
(and dvb) tuner subdevices.
Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index 4c8f26243252..52cc2a6b19b7 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h
@@ -66,6 +66,8 @@ struct media_device_info {
/* A converter of analogue video to its digital representation. */
#define MEDIA_ENT_T_V4L2_SUBDEV_DECODER (MEDIA_ENT_T_V4L2_SUBDEV + 4)
+#define MEDIA_ENT_T_V4L2_SUBDEV_TUNER (MEDIA_ENT_T_V4L2_SUBDEV + 5)
+
#define MEDIA_ENT_FL_DEFAULT (1 << 0)
struct media_entity_desc {
--
2.1.0
^ permalink raw reply related
* Re: [PATCH v2] iio: Export userspace IIO headers
From: Jonathan Cameron @ 2015-02-14 11:53 UTC (permalink / raw)
To: Daniel Baluta, knaack.h-Mmb7MZpHnFY, lars-Qo5EllUWu/uELgA04lAiVw,
pmeerw-jW+XmwGofnusTnJN9+BGXg
Cc: irina.tirdea-ral2JQCrhuEAvxtiuMwx3w,
roberta.dobrescu-Re5JQEeQqe8AvxtiuMwx3w,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1423586031-25199-1-git-send-email-daniel.baluta-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
On 10/02/15 16:33, Daniel Baluta wrote:
> After UAPI header file split [1] all user-kernel interfaces were
> placed under include/uapi/.
>
> This patch moves IIO user specific API from:
> * include/linux/iio/events.h => include/uapi/linux/iio/events.h
> * include/linux/types.h => include/uapi/linux/types.h
>
> Now there is no need for nasty tricks to compile userspace programs
> (e.g iio_event_monitor). Just installing the kernel headers with
> make headers_install command does the job.
>
> [1] http://lwn.net/Articles/507794/
>
> Signed-off-by: Daniel Baluta <daniel.baluta-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Looks good to me. A bonus blank line and EOF that I've removed.
Applied to the togreg branch of iio.git
Thanks,
Jonathan
> ---
> Changes since v1:
> * keep enum iio_event_info and IIO_VAL_* constants in
> include/linux/iio/types.h since they aren't part of the
> ABI.
>
> include/linux/iio/events.h | 30 +-------------
> include/linux/iio/types.h | 78 +----------------------------------
> include/uapi/linux/Kbuild | 1 +
> include/uapi/linux/iio/Kbuild | 3 ++
> include/uapi/linux/iio/events.h | 43 +++++++++++++++++++
> include/uapi/linux/iio/types.h | 91 +++++++++++++++++++++++++++++++++++++++++
> 6 files changed, 140 insertions(+), 106 deletions(-)
> create mode 100644 include/uapi/linux/iio/Kbuild
> create mode 100644 include/uapi/linux/iio/events.h
> create mode 100644 include/uapi/linux/iio/types.h
>
> diff --git a/include/linux/iio/events.h b/include/linux/iio/events.h
> index 03fa332..8ad87d1 100644
> --- a/include/linux/iio/events.h
> +++ b/include/linux/iio/events.h
> @@ -9,22 +9,8 @@
> #ifndef _IIO_EVENTS_H_
> #define _IIO_EVENTS_H_
>
> -#include <linux/ioctl.h>
> -#include <linux/types.h>
> #include <linux/iio/types.h>
> -
> -/**
> - * struct iio_event_data - The actual event being pushed to userspace
> - * @id: event identifier
> - * @timestamp: best estimate of time of event occurrence (often from
> - * the interrupt handler)
> - */
> -struct iio_event_data {
> - __u64 id;
> - __s64 timestamp;
> -};
> -
> -#define IIO_GET_EVENT_FD_IOCTL _IOR('i', 0x90, int)
> +#include <uapi/linux/iio/events.h>
>
> /**
> * IIO_EVENT_CODE() - create event identifier
> @@ -70,18 +56,4 @@ struct iio_event_data {
> #define IIO_UNMOD_EVENT_CODE(chan_type, number, type, direction) \
> IIO_EVENT_CODE(chan_type, 0, 0, direction, type, number, 0, 0)
>
> -#define IIO_EVENT_CODE_EXTRACT_TYPE(mask) ((mask >> 56) & 0xFF)
> -
> -#define IIO_EVENT_CODE_EXTRACT_DIR(mask) ((mask >> 48) & 0x7F)
> -
> -#define IIO_EVENT_CODE_EXTRACT_CHAN_TYPE(mask) ((mask >> 32) & 0xFF)
> -
> -/* Event code number extraction depends on which type of event we have.
> - * Perhaps review this function in the future*/
> -#define IIO_EVENT_CODE_EXTRACT_CHAN(mask) ((__s16)(mask & 0xFFFF))
> -#define IIO_EVENT_CODE_EXTRACT_CHAN2(mask) ((__s16)(((mask) >> 16) & 0xFFFF))
> -
> -#define IIO_EVENT_CODE_EXTRACT_MODIFIER(mask) ((mask >> 40) & 0xFF)
> -#define IIO_EVENT_CODE_EXTRACT_DIFF(mask) (((mask) >> 55) & 0x1)
> -
> #endif
> diff --git a/include/linux/iio/types.h b/include/linux/iio/types.h
> index 580ed5b..942b6de 100644
> --- a/include/linux/iio/types.h
> +++ b/include/linux/iio/types.h
> @@ -10,76 +10,7 @@
> #ifndef _IIO_TYPES_H_
> #define _IIO_TYPES_H_
>
> -enum iio_chan_type {
> - IIO_VOLTAGE,
> - IIO_CURRENT,
> - IIO_POWER,
> - IIO_ACCEL,
> - IIO_ANGL_VEL,
> - IIO_MAGN,
> - IIO_LIGHT,
> - IIO_INTENSITY,
> - IIO_PROXIMITY,
> - IIO_TEMP,
> - IIO_INCLI,
> - IIO_ROT,
> - IIO_ANGL,
> - IIO_TIMESTAMP,
> - IIO_CAPACITANCE,
> - IIO_ALTVOLTAGE,
> - IIO_CCT,
> - IIO_PRESSURE,
> - IIO_HUMIDITYRELATIVE,
> - IIO_ACTIVITY,
> - IIO_STEPS,
> - IIO_ENERGY,
> - IIO_DISTANCE,
> - IIO_VELOCITY,
> -};
> -
> -enum iio_modifier {
> - IIO_NO_MOD,
> - IIO_MOD_X,
> - IIO_MOD_Y,
> - IIO_MOD_Z,
> - IIO_MOD_X_AND_Y,
> - IIO_MOD_X_AND_Z,
> - IIO_MOD_Y_AND_Z,
> - IIO_MOD_X_AND_Y_AND_Z,
> - IIO_MOD_X_OR_Y,
> - IIO_MOD_X_OR_Z,
> - IIO_MOD_Y_OR_Z,
> - IIO_MOD_X_OR_Y_OR_Z,
> - IIO_MOD_LIGHT_BOTH,
> - IIO_MOD_LIGHT_IR,
> - IIO_MOD_ROOT_SUM_SQUARED_X_Y,
> - IIO_MOD_SUM_SQUARED_X_Y_Z,
> - IIO_MOD_LIGHT_CLEAR,
> - IIO_MOD_LIGHT_RED,
> - IIO_MOD_LIGHT_GREEN,
> - IIO_MOD_LIGHT_BLUE,
> - IIO_MOD_QUATERNION,
> - IIO_MOD_TEMP_AMBIENT,
> - IIO_MOD_TEMP_OBJECT,
> - IIO_MOD_NORTH_MAGN,
> - IIO_MOD_NORTH_TRUE,
> - IIO_MOD_NORTH_MAGN_TILT_COMP,
> - IIO_MOD_NORTH_TRUE_TILT_COMP,
> - IIO_MOD_RUNNING,
> - IIO_MOD_JOGGING,
> - IIO_MOD_WALKING,
> - IIO_MOD_STILL,
> - IIO_MOD_ROOT_SUM_SQUARED_X_Y_Z,
> -};
> -
> -enum iio_event_type {
> - IIO_EV_TYPE_THRESH,
> - IIO_EV_TYPE_MAG,
> - IIO_EV_TYPE_ROC,
> - IIO_EV_TYPE_THRESH_ADAPTIVE,
> - IIO_EV_TYPE_MAG_ADAPTIVE,
> - IIO_EV_TYPE_CHANGE,
> -};
> +#include <uapi/linux/iio/types.h>
>
> enum iio_event_info {
> IIO_EV_INFO_ENABLE,
> @@ -88,13 +19,6 @@ enum iio_event_info {
> IIO_EV_INFO_PERIOD,
> };
>
> -enum iio_event_direction {
> - IIO_EV_DIR_EITHER,
> - IIO_EV_DIR_RISING,
> - IIO_EV_DIR_FALLING,
> - IIO_EV_DIR_NONE,
> -};
> -
> #define IIO_VAL_INT 1
> #define IIO_VAL_INT_PLUS_MICRO 2
> #define IIO_VAL_INT_PLUS_NANO 3
> diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
> index 00b10002..5bfc5bd 100644
> --- a/include/uapi/linux/Kbuild
> +++ b/include/uapi/linux/Kbuild
> @@ -6,6 +6,7 @@ header-y += caif/
> header-y += dvb/
> header-y += hdlc/
> header-y += hsi/
> +header-y += iio/
> header-y += isdn/
> header-y += mmc/
> header-y += nfsd/
> diff --git a/include/uapi/linux/iio/Kbuild b/include/uapi/linux/iio/Kbuild
> new file mode 100644
> index 0000000..86f76d8
> --- /dev/null
> +++ b/include/uapi/linux/iio/Kbuild
> @@ -0,0 +1,3 @@
> +# UAPI Header export list
> +header-y += events.h
> +header-y += types.h
> diff --git a/include/uapi/linux/iio/events.h b/include/uapi/linux/iio/events.h
> new file mode 100644
> index 0000000..4b06477
> --- /dev/null
> +++ b/include/uapi/linux/iio/events.h
> @@ -0,0 +1,43 @@
> +/* The industrial I/O - event passing to userspace
> + *
> + * Copyright (c) 2008-2011 Jonathan Cameron
> + *
> + * 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.
> + */
> +#ifndef _UAPI_IIO_EVENTS_H_
> +#define _UAPI_IIO_EVENTS_H_
> +
> +#include <linux/ioctl.h>
> +#include <linux/types.h>
> +
> +/**
> + * struct iio_event_data - The actual event being pushed to userspace
> + * @id: event identifier
> + * @timestamp: best estimate of time of event occurrence (often from
> + * the interrupt handler)
> + */
> +struct iio_event_data {
> + __u64 id;
> + __s64 timestamp;
> +};
> +
> +#define IIO_GET_EVENT_FD_IOCTL _IOR('i', 0x90, int)
> +
> +#define IIO_EVENT_CODE_EXTRACT_TYPE(mask) ((mask >> 56) & 0xFF)
> +
> +#define IIO_EVENT_CODE_EXTRACT_DIR(mask) ((mask >> 48) & 0x7F)
> +
> +#define IIO_EVENT_CODE_EXTRACT_CHAN_TYPE(mask) ((mask >> 32) & 0xFF)
> +
> +/* Event code number extraction depends on which type of event we have.
> + * Perhaps review this function in the future*/
> +#define IIO_EVENT_CODE_EXTRACT_CHAN(mask) ((__s16)(mask & 0xFFFF))
> +#define IIO_EVENT_CODE_EXTRACT_CHAN2(mask) ((__s16)(((mask) >> 16) & 0xFFFF))
> +
> +#define IIO_EVENT_CODE_EXTRACT_MODIFIER(mask) ((mask >> 40) & 0xFF)
> +#define IIO_EVENT_CODE_EXTRACT_DIFF(mask) (((mask) >> 55) & 0x1)
> +
> +#endif /* _UAPI_IIO_EVENTS_H_ */
> +
> diff --git a/include/uapi/linux/iio/types.h b/include/uapi/linux/iio/types.h
> new file mode 100644
> index 0000000..56a1529
> --- /dev/null
> +++ b/include/uapi/linux/iio/types.h
> @@ -0,0 +1,91 @@
> +/* industrial I/O data types needed both in and out of kernel
> + *
> + * Copyright (c) 2008 Jonathan Cameron
> + *
> + * 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.
> + */
> +
> +#ifndef _UAPI_IIO_TYPES_H_
> +#define _UAPI_IIO_TYPES_H_
> +
> +enum iio_chan_type {
> + IIO_VOLTAGE,
> + IIO_CURRENT,
> + IIO_POWER,
> + IIO_ACCEL,
> + IIO_ANGL_VEL,
> + IIO_MAGN,
> + IIO_LIGHT,
> + IIO_INTENSITY,
> + IIO_PROXIMITY,
> + IIO_TEMP,
> + IIO_INCLI,
> + IIO_ROT,
> + IIO_ANGL,
> + IIO_TIMESTAMP,
> + IIO_CAPACITANCE,
> + IIO_ALTVOLTAGE,
> + IIO_CCT,
> + IIO_PRESSURE,
> + IIO_HUMIDITYRELATIVE,
> + IIO_ACTIVITY,
> + IIO_STEPS,
> + IIO_ENERGY,
> + IIO_DISTANCE,
> + IIO_VELOCITY,
> +};
> +
> +enum iio_modifier {
> + IIO_NO_MOD,
> + IIO_MOD_X,
> + IIO_MOD_Y,
> + IIO_MOD_Z,
> + IIO_MOD_X_AND_Y,
> + IIO_MOD_X_AND_Z,
> + IIO_MOD_Y_AND_Z,
> + IIO_MOD_X_AND_Y_AND_Z,
> + IIO_MOD_X_OR_Y,
> + IIO_MOD_X_OR_Z,
> + IIO_MOD_Y_OR_Z,
> + IIO_MOD_X_OR_Y_OR_Z,
> + IIO_MOD_LIGHT_BOTH,
> + IIO_MOD_LIGHT_IR,
> + IIO_MOD_ROOT_SUM_SQUARED_X_Y,
> + IIO_MOD_SUM_SQUARED_X_Y_Z,
> + IIO_MOD_LIGHT_CLEAR,
> + IIO_MOD_LIGHT_RED,
> + IIO_MOD_LIGHT_GREEN,
> + IIO_MOD_LIGHT_BLUE,
> + IIO_MOD_QUATERNION,
> + IIO_MOD_TEMP_AMBIENT,
> + IIO_MOD_TEMP_OBJECT,
> + IIO_MOD_NORTH_MAGN,
> + IIO_MOD_NORTH_TRUE,
> + IIO_MOD_NORTH_MAGN_TILT_COMP,
> + IIO_MOD_NORTH_TRUE_TILT_COMP,
> + IIO_MOD_RUNNING,
> + IIO_MOD_JOGGING,
> + IIO_MOD_WALKING,
> + IIO_MOD_STILL,
> + IIO_MOD_ROOT_SUM_SQUARED_X_Y_Z,
> +};
> +
> +enum iio_event_type {
> + IIO_EV_TYPE_THRESH,
> + IIO_EV_TYPE_MAG,
> + IIO_EV_TYPE_ROC,
> + IIO_EV_TYPE_THRESH_ADAPTIVE,
> + IIO_EV_TYPE_MAG_ADAPTIVE,
> + IIO_EV_TYPE_CHANGE,
> +};
> +
> +enum iio_event_direction {
> + IIO_EV_DIR_EITHER,
> + IIO_EV_DIR_RISING,
> + IIO_EV_DIR_FALLING,
> + IIO_EV_DIR_NONE,
> +};
> +
> +#endif /* _UAPI_IIO_TYPES_H_ */
>
^ permalink raw reply
* Re: [PATCH v3 linux-trace 1/8] tracing: attach eBPF programs to tracepoints and syscalls
From: Alexei Starovoitov @ 2015-02-14 22:48 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Steven Rostedt, Ingo Molnar, Namhyung Kim,
Arnaldo Carvalho de Melo, Jiri Olsa, Masami Hiramatsu, Linux API,
Network Development, LKML, Linus Torvalds, Eric W. Biederman
On Wed, Feb 11, 2015 at 5:28 AM, Peter Zijlstra <peterz-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> wrote:
>
> We're compiling the BPF stuff against the 'current' kernel headers
> right?
the tracex1 example is pulling kernel headers to demonstrate
how bpf_fetch*() helpers can be used to walk kernel structures
without debug info.
The other examples don't need any internal headers.
> So would enforcing module versioning not be sufficient?
I'm going to redo the ex1 to use kprobe and some form of
version check. Indeed module-like versioning should
be enough.
^ permalink raw reply
* Re: [PATCH v3 linux-trace 1/8] tracing: attach eBPF programs to tracepoints and syscalls
From: Alexei Starovoitov @ 2015-02-14 22:54 UTC (permalink / raw)
To: Steven Rostedt
Cc: Ingo Molnar, Namhyung Kim, Arnaldo Carvalho de Melo, Jiri Olsa,
Masami Hiramatsu, Linux API, Network Development, LKML,
Linus Torvalds, Peter Zijlstra, Eric W. Biederman
On Wed, Feb 11, 2015 at 7:51 AM, Steven Rostedt <rostedt-nx8X9YLhiw1AfugRpC6u6w@public.gmane.org> wrote:
> On Tue, 10 Feb 2015 22:33:05 -0800
> Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org> wrote:
>
>
>> fair enough.
>> Something like TRACE_MARKER(arg1, arg2) that prints
>> it was hit without accessing the args would be enough.
>> Without any args it is indeed a 'fast kprobe' only.
>> Debug info would still be needed to access
>> function arguments.
>> On x64 function entry point and x64 abi make it easy
>> to access args, but i386 or kprobe in the middle
>> lose visibility when debug info is not available.
>> TRACE_MARKER (with few key args that function
>> is operating on) is enough to achieve roughly the same
>> as kprobe without debug info.
>
> Actually, what about a TRACE_EVENT_DEBUG(), that has a few args and
> possibly a full trace event layout.
>
> The difference would be that the trace events do not show up unless you
> have "trace_debug" on the command line. This should prevent
> applications from depending on them.
>
> I could even do the nasty dmesg output like I do with trace_printk()s,
> that would definitely keep a production kernel from adding it by
> default.
>
> When trace_debug is not there, the trace points could still be accessed
> but perhaps only via bpf, or act like a simple trace marker.
I think that is a great idea!
Makes it clear that all prints are for debugging and
no abi guarantees.
> Note, if you need ids, I rather have them in another directory than
> tracefs. Make a eventfs perhaps that holds these. I rather keep tracefs
> simple.
indeed. makes sense. no reason to burn fs memory just
to get an id from name. may be perf_event api can be
extended to lookup id from name. I think perf will benefit as well.
^ permalink raw reply
* Re: [PATCH v3 linux-trace 1/8] tracing: attach eBPF programs to tracepoints and syscalls
From: Alexei Starovoitov @ 2015-02-14 23:02 UTC (permalink / raw)
To: Hekuang
Cc: Steven Rostedt, Ingo Molnar, Namhyung Kim,
Arnaldo Carvalho de Melo, Jiri Olsa, Masami Hiramatsu, Linux API,
Network Development, LKML, Linus Torvalds, Peter Zijlstra,
Eric W. Biederman, wangnan0-hv44wF8Li93QT0dZR+AlfA
On Wed, Feb 11, 2015 at 11:58 PM, Hekuang <hekuang-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> wrote:
>
>>> eBPF is very flexible, which means it is bound to have someone use it
>>> in a way you never dreamed of, and that will be what bites you in the
>>> end (pun intended).
>>
>> understood :)
>> let's start slow then with bpf+syscall and bpf+kprobe only.
>
>
> I think BPF + system calls/kprobes can meet our use case
> (https://lkml.org/lkml/2015/2/6/44), but there're some issues to be
> improved.
>
> I suggest that you can improve bpf+kprobes when attached to function
> headers(or TRACE_MARKERS), make it converts pt-regs to bpf_ctx->arg1,
> arg2.., then top models and architectures can be separated by bpf.
>
> BPF bytecode is cross-platform, but what we can get by using bpf+kprobes
> is a 'regs->rdx' kind of information, such information is both
> architecture and kernel version related.
for kprobes in the middle of the function, kernel cannot
convert pt_regs into argN. Placement was decided by compiler
and can only be found in debug info.
I think bpf+kprobe will be using it when it is available.
When there is no debug info, kprobes will be limited
to function entry and mapping of regs/stack into
argN can be done by user space depending on architecture.
So user tracing scripts in some higher level language
can be kernel/arch independent when 'perf probe+bpf'
is loading them on the fly on the given machine.
> We hope to establish some models for describing kernel procedures such
> as IO and network, which requires that it does not rely on architecture
> and does not rely to a specific kernel version as much as possible.
That's obviously a goal, but it requires a new approach to tracepoints.
I think a lot of great ideas were discussed in this thread, so I'm
hopeful that we'll come up with solution that will satisfy even
strictest Peter's requirements :)
^ permalink raw reply
* Re: [PATCH 02/14] ARM: ARMv7M: Enlarge vector table to 256 entries
From: Maxime Coquelin @ 2015-02-15 14:34 UTC (permalink / raw)
To: Uwe Kleine-König, Russell King
Cc: Geert Uytterhoeven, Jonathan Corbet, Rob Herring, Pawel Moll,
Mark Rutland, Ian Campbell, Kumar Gala, Philipp Zabel,
Daniel Lezcano, Thomas Gleixner, Linus Walleij,
Greg Kroah-Hartman, Jiri Slaby, Arnd Bergmann, Andrew Morton,
David S. Miller, Mauro Carvalho Chehab, Joe Perches,
Antti Palosaari, Tejun Heo, Will Deacon, Nikolay Borisov,
Rusty Russell
In-Reply-To: <20150213100050.GM10842@pengutronix.de>
2015-02-13 11:00 GMT+01:00 Uwe Kleine-König <u.kleine-koenig@pengutronix.de>:
> On Fri, Feb 13, 2015 at 09:42:46AM +0100, Maxime Coquelin wrote:
>> Hi Geert,
>>
>> 2015-02-12 21:34 GMT+01:00 Geert Uytterhoeven <geert@linux-m68k.org>:
>> > On Thu, Feb 12, 2015 at 6:45 PM, Maxime Coquelin
>> > <mcoquelin.stm32@gmail.com> wrote:
>> >> From Cortex-M4 and M7 reference manuals, the nvic supports up to 240
>> >> interrupts. So the number of entries in vectors table is 256.
>> >>
>> >> This patch adds the missing entries, and change the alignement, so that
>> >> vector_table remains naturally aligned.
>> >
>> > Shouldn't this depend on ARCH_STM32, or some other M4 or M7 specific
>> > Kconfig option, to avoid wasting the space on other CPUs?
>>
>> Actually, the STM32F429 has 90 interrupts, so it would need 106
>> entries in the vector table.
>> The maximum of supported interrupts is not only for Cortex-M4 and M7,
>> this is also true for Cortex-M3.
>>
>> I see two possibilities:
>> 1 - We declare the vector table for the maximum supported number of
>> IRQs, as this patch does.
>> - Pro: it will be functionnal with all Cortex-M MCUs
>> - Con: Waste of less than 1KB for memory
>> 2 - We introduce a config flag that provides the number of interrupts
>> - Pro: No more memory waste
>> - Con: Need to declare a per MCU model config flag.
> I'd vote for 2, something like:
>
> config CPUV7M_NUM_IRQ
> int
> default 90 if STM32F429
> default 38 if EFM32GG
> default 240
>
> then there is a working default and platforms being short on memory can
> configure as appropriate. (The only down side is that if we create
> multi-platfrom images at some time in the future either all or none of
> the supported platforms must provide a value here.)
Ok, I'm fine doing this way.
I will implement this in the v2 if Russel is fine with the proposal too.
>
>> Then, regarding the natural alignment, is there a way to ensure it
>> depending on the value of a config flag?
> The exact wording in ARMARMv7-M is:
>
> The Vector table must be naturally aligned to a power of two
> whose alignment value is greater than or equal
> to (Number of Exceptions supported x 4), with a minimum
> alignment of 128 bytes.
>
>> Or we should keep it at the maximum value possible?
> So we need:
>
> .align x
>
> with x being max(7, ceil(log((CPUV7M_NUM_IRQ + 16) * 4, 2))). So the
> alignment needed is between 7 and 10.
>
> If the assembler supports an expression here I'd use that. But before
> adding strange hacks to generate the right value there better go for a
> static value like:
>
> /* The vector table must be naturally aligned */
> #if CONFIG_CPUV7M_NUM_IRQ <= 112
> .align 9 /* log2((112 + 16) * 4) */
> #else
> .align 10
> #endif
>
> Further steps would be:
>
> CONFIG_CPUV7M_NUM_IRQ <= 48 -> .align 8
> CONFIG_CPUV7M_NUM_IRQ <= 16 -> .align 7
>
> Probably it's not worth to add the respective #ifdefs here.
I will go for the #ifdefs.
Thanks,
Maxime
>
> Best regards
> Uwe
>
> --
> Pengutronix e.K. | Uwe Kleine-König |
> Industrial Linux Solutions | http://www.pengutronix.de/ |
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 12/14] ARM: dts: Introduce STM32F429 MCU
From: Maxime Coquelin @ 2015-02-15 14:36 UTC (permalink / raw)
To: Philipp Zabel
Cc: Jonathan Corbet, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Russell King, Daniel Lezcano,
Thomas Gleixner, Linus Walleij, Greg Kroah-Hartman, Jiri Slaby,
Arnd Bergmann, Andrew Morton, David S. Miller,
Mauro Carvalho Chehab, Joe Perches, Antti Palosaari, Tejun Heo,
Will Deacon, Nikolay Borisov, Rusty Russell, Kees Cook, Michal
In-Reply-To: <1423855102.4182.63.camel@pengutronix.de>
Hi Philipp,
2015-02-13 20:18 GMT+01:00 Philipp Zabel <p.zabel@pengutronix.de>:
> Am Freitag, den 13.02.2015, 17:41 +0100 schrieb Maxime Coquelin:
>> 2015-02-13 17:25 GMT+01:00 Philipp Zabel <p.zabel@pengutronix.de>:
>> > Hi Maxime,
>> >
>> > Am Freitag, den 13.02.2015, 16:59 +0100 schrieb Maxime Coquelin:
>> >> Hi Philipp,
>> >>
>> >> 2015-02-13 12:47 GMT+01:00 Philipp Zabel <p.zabel@pengutronix.de>:
>> >> > Hi Maxime,
>> >> >
>> >> > Am Donnerstag, den 12.02.2015, 18:46 +0100 schrieb Maxime Coquelin:
>> >> > [...]
>> >> >> + soc {
>> >> >> + reset_ahb1: reset@40023810 {
>> >> >> + #reset-cells = <1>;
>> >> >> + compatible = "st,stm32-reset";
>> >> >> + reg = <0x40023810 0x4>;
>> >> >> + };
>> >> >> +
>> >> >> + reset_ahb2: reset@40023814 {
>> >> >> + #reset-cells = <1>;
>> >> >> + compatible = "st,stm32-reset";
>> >> >> + reg = <0x40023814 0x4>;
>> >> >> + };
>> >> >> +
>> >> >> + reset_ahb3: reset@40023818 {
>> >> >> + #reset-cells = <1>;
>> >> >> + compatible = "st,stm32-reset";
>> >> >> + reg = <0x40023818 0x4>;
>> >> >> + };
>> >> >> +
>> >> >> + reset_apb1: reset@40023820 {
>> >> >> + #reset-cells = <1>;
>> >> >> + compatible = "st,stm32-reset";
>> >> >> + reg = <0x40023820 0x4>;
>> >> >> + };
>> >> >> +
>> >> >> + reset_apb2: reset@40023824 {
>> >> >> + #reset-cells = <1>;
>> >> >> + compatible = "st,stm32-reset";
>> >> >> + reg = <0x40023824 0x4>;
>> >> >> + };
>> >> >
>> >> > These are mostly consecutive, single registers. I wonder if these are
>> >> > part of the same IP block and thus should be grouped together into the
>> >> > same reset controller node?
>> >>
>> >> What I could to is to have two instances. One for AHB and one for APB domain.
>> >> Doing this, I will have one instance per domain, and only consecutive registers.
>> >> Is it fine for you?
>> >
>> > Looking at
>> > http://www.st.com/web/en/resource/technical/document/reference_manual/DM00031020.pdf
>> > Table 34 (RCC register map and reset values), I'd say there is a single
>> > "Reset and Clock Control" device at 0x40023800 - 0x40023884:
>> >
>> > soc {
>> > rcc: rcc@40023800 {
>> > #clock-cells = <1>;
>> > #reset-cells = <1>;
>> > compatible = "st,stm32-rcc";
>> > reg = <0x40023800 0x84>;
>> > };
>> >
>> > ...
>> >
>> > If you really want to describe the reset controller parts (offsets +0x10
>> > to +0x24) in a separate node, I won't argue against it too long,
>> > although this is a somewhat arbitrary decision.
>> >
>> > In any case, the whole register at offset +0x1c is reserved, so there is
>> > no reason to split the reset controller. It is ok to have unused ranges
>> > as is already the case with reserved bits inside the used registers.
>>
>> Ok. I understand your point.
>> But it will be more difficult at usage, because the node referencing
>> the fourth reset bit of apb2 register will have to pass 164 as
>> parameter.
>> It is error prone IMHO.
>>
>> Other solution would be to add some defines for each reset line in the
>> DT-Bindings, as we do today for STi platform.
>> But it is giving an unneeded constraint between DT and reset trees.
>
> That is a bit unfortunate, but providing the named constants in
> include/dt-bindings/reset/ makes for a much better readable device tree,
> so I'd prefer that solution, even if it means having to coordinate pull
> requests.
Ok, I will add constants in include/dt-bindings/reset/
Thanks,
Maxime
>
> regards
> Philipp
>
^ permalink raw reply
* Re: [PATCH 00/14] Add support to STMicroelectronics STM32 family
From: Andreas Färber @ 2015-02-15 15:14 UTC (permalink / raw)
To: Maxime Coquelin
Cc: Jonathan Corbet, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Philipp Zabel, Russell King,
Daniel Lezcano, Thomas Gleixner, Linus Walleij,
Greg Kroah-Hartman, Jiri Slaby, Arnd Bergmann, Andrew Morton,
David S. Miller, Mauro Carvalho Chehab, Joe Perches,
Antti Palosaari, Tejun Heo, Will Deacon, Nikolay Borisov,
Rusty Russell, Kees
In-Reply-To: <1423763164-5606-1-git-send-email-mcoquelin.stm32@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 5960 bytes --]
Hi Maxime,
Am 12.02.2015 um 18:45 schrieb Maxime Coquelin:
> This patchset adds basic support for STMicroelectronics STM32 series MCUs.
>
> STM32 MCUs are Cortex-M CPU, used in various applications (consumer
> electronics, industrial applications, hobbyists...).
> Datasheets, user and programming manuals are publicly available on
> STMicroelectronics website.
>
> With this series applied, the STM32F419 Discovery can boot succesfully.
>
> Once this series accepted, next steps will be to add DMA support, as USART,
> I2C and SPI IPs don't have any FIFO. Then will come the clock driver, as today
> the bootloader has to be patched to enable the needed clocks.
This is somewhat unfortunate, as I have been working on the same thing
and have demonstrated the STM32F429 Discovery Kit at ARM TechSymposium
Europe in December and submitted a talk for LinuxCon Japan.
https://github.com/afaerber/afboot-stm32
https://github.com/afaerber/linux/commits/stm32
On a brief look, it seems you are further along in terms of code quality
and documenting. Do you spot anything that's missing in your series and
could be added from my branch? The clk controller maybe? Also I already
started looking into gpio and usb drivers. Me too, I skipped DMA support
though.
Regards,
Andreas
>
> Maxime Coquelin (14):
> scripts: link-vmlinux: Don't pass page offset to kallsyms if XIP
> Kernel
> ARM: ARMv7M: Enlarge vector table to 256 entries
> clocksource: Add ARM System timer driver
> reset: Add reset_controller_of_init() function
> ARM: call reset_controller_of_init from default time_init handler
> drivers: reset: Add STM32 reset driver
> clockevent: Add STM32 Timer driver
> pinctrl: Add pinctrl driver for STM32 MCUs
> serial: stm32-usart: Add STM32 USART Driver
> ARM: Add STM32 family machine
> ARM: dts: Add ARM System timer as clockevent in armv7m
> ARM: dts: Introduce STM32F429 MCU
> ARM: configs: Add STM32 defconfig
> MAINTAINERS: Add entry for STM32 MCUs
>
> Documentation/arm/stm32/overview.txt | 32 +
> Documentation/arm/stm32/stm32f429-overview.txt | 22 +
> .../devicetree/bindings/arm/system_timer.txt | 15 +
> .../devicetree/bindings/pinctrl/pinctrl-stm32.txt | 99 +++
> .../devicetree/bindings/reset/st,stm32-reset.txt | 19 +
> .../devicetree/bindings/serial/st,stm32-usart.txt | 18 +
> .../devicetree/bindings/timer/st,stm32-timer.txt | 19 +
> MAINTAINERS | 7 +
> arch/arm/Kconfig | 22 +
> arch/arm/Makefile | 1 +
> arch/arm/boot/dts/Makefile | 1 +
> arch/arm/boot/dts/armv7-m.dtsi | 7 +
> arch/arm/boot/dts/stm32f429-disco.dts | 41 ++
> arch/arm/boot/dts/stm32f429.dtsi | 279 ++++++++
> arch/arm/configs/stm32_defconfig | 72 ++
> arch/arm/kernel/entry-v7m.S | 8 +-
> arch/arm/kernel/time.c | 4 +
> arch/arm/mach-stm32/Makefile | 1 +
> arch/arm/mach-stm32/Makefile.boot | 0
> arch/arm/mach-stm32/board-dt.c | 19 +
> drivers/clocksource/Kconfig | 16 +
> drivers/clocksource/Makefile | 2 +
> drivers/clocksource/arm_system_timer.c | 74 ++
> drivers/clocksource/timer-stm32.c | 187 +++++
> drivers/pinctrl/Kconfig | 9 +
> drivers/pinctrl/Makefile | 1 +
> drivers/pinctrl/pinctrl-stm32.c | 779 +++++++++++++++++++++
> drivers/reset/Makefile | 1 +
> drivers/reset/core.c | 20 +
> drivers/reset/reset-stm32.c | 124 ++++
> drivers/tty/serial/Kconfig | 17 +
> drivers/tty/serial/Makefile | 1 +
> drivers/tty/serial/stm32-usart.c | 695 ++++++++++++++++++
> include/asm-generic/vmlinux.lds.h | 4 +-
> include/dt-bindings/pinctrl/pinctrl-stm32.h | 43 ++
> include/linux/reset-controller.h | 6 +
> include/uapi/linux/serial_core.h | 3 +
> scripts/link-vmlinux.sh | 2 +-
> 38 files changed, 2664 insertions(+), 6 deletions(-)
> create mode 100644 Documentation/arm/stm32/overview.txt
> create mode 100644 Documentation/arm/stm32/stm32f429-overview.txt
> create mode 100644 Documentation/devicetree/bindings/arm/system_timer.txt
> create mode 100644 Documentation/devicetree/bindings/pinctrl/pinctrl-stm32.txt
> create mode 100644 Documentation/devicetree/bindings/reset/st,stm32-reset.txt
> create mode 100644 Documentation/devicetree/bindings/serial/st,stm32-usart.txt
> create mode 100644 Documentation/devicetree/bindings/timer/st,stm32-timer.txt
> create mode 100644 arch/arm/boot/dts/stm32f429-disco.dts
> create mode 100644 arch/arm/boot/dts/stm32f429.dtsi
> create mode 100644 arch/arm/configs/stm32_defconfig
> create mode 100644 arch/arm/mach-stm32/Makefile
> create mode 100644 arch/arm/mach-stm32/Makefile.boot
> create mode 100644 arch/arm/mach-stm32/board-dt.c
> create mode 100644 drivers/clocksource/arm_system_timer.c
> create mode 100644 drivers/clocksource/timer-stm32.c
> create mode 100644 drivers/pinctrl/pinctrl-stm32.c
> create mode 100644 drivers/reset/reset-stm32.c
> create mode 100644 drivers/tty/serial/stm32-usart.c
> create mode 100644 include/dt-bindings/pinctrl/pinctrl-stm32.h
>
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH RFC v3 0/7] epoll: Introduce new syscalls, epoll_ctl_batch and epoll_pwait1
From: Michael Kerrisk (man-pages) @ 2015-02-15 15:16 UTC (permalink / raw)
To: Omar Sandoval, Fam Zheng
Cc: mtk.manpages, linux-kernel, Thomas Gleixner, Ingo Molnar,
H. Peter Anvin, x86, Alexander Viro, Andrew Morton, Kees Cook,
Andy Lutomirski, David Herrmann, Alexei Starovoitov,
Miklos Szeredi, David Drysdale, Oleg Nesterov, David S. Miller,
Vivek Goyal, Mike Frysinger, Theodore Ts'o, Heiko Carstens,
Rasmus Villemoes, Rashika Kheria, Hugh Dickins, Mathieu Desnoyers
In-Reply-To: <20150213095357.GA20668@mew>
On 02/13/2015 10:53 AM, Omar Sandoval wrote:
> On Fri, Feb 13, 2015 at 05:03:56PM +0800, Fam Zheng wrote:
>> Hi all,
>>
>> This is the updated series for the new epoll system calls, with the cover
>> letter rewritten which includes some more explanation. Comments are very
>> welcome!
>>
>> Original Motivation
>> ===================
>>
>> QEMU, and probably many more select/poll based applications, will consider
>> epoll as an alternative, when its event loop needs to handle a big number of
>> fds. However, there are currently two concerns with epoll which prevents the
>> switching from ppoll to epoll.
>>
>> The major one is the timeout precision.
>>
>> For example in QEMU, the main loop takes care of calling callbacks at a
>> specific timeout - the QEMU timer API. The timeout value in ppoll depends on
>> the next firing timer. epoll_pwait's millisecond timeout is so coarse that
>> rounding up the timeout will hurt performance badly.
>>
>> The minor one is the number of system call to update fd set.
>>
>> While epoll can handle a large number of fds quickly, it still requires one
>> epoll_ctl per fd update, compared to the one-shot call to select/poll with an
>> fd array. This may as well make epoll inferior to ppoll in the cases where a
>> small, but frequently changing set of fds are polled by the event loop.
>>
>> This series introduces two new epoll APIs to address them respectively. The
>> idea of epoll_ctl_batch is suggested by Andy Lutomirski in [1], who also
>> suggested clockid as a parameter in epoll_pwait1.
>>
>> Discussion
>> ==========
>>
>> [Note: This is the question part regarding the interface contract of
>> epoll_ctl_batch. If you don't have the context of what is epoll_ctl_batch yet,
>> please skip this part and probably start with the man page style documentation.
>> You can resume to this section later.]
>>
>> [Thanks to Omar Sandoval <osandov@osandov.com>, who pointed out this in
>> reviewing v1]
>>
>> We try to report status for each command in epoll_ctl_batch, by writting to
>> user provided command array (pointed to cmds). The tricky thing in the
>> implementation is that, copying the results back to userspace comes last, after
>> the commands are executed. At this point, if the copy_to_user fails, the
>> effects are done and no return - or if we add some mechanism to revert it, the
>> code will be too complicated and slow.
>>
>> In above corner case, the return value of epoll_ctl_batch is smaller than
>> ncmds, which assures our caller that last N commands failed, where N = ncmds -
>> ret. But they'll also find that cmd.result is not changed to error code.
>>
>> I suppose this is not a big deal, because 1) it should happen very rarely. 2)
>> user does know the actual number of commands that succeed.
>>
>> So, do we leave it as is? Or is there any way to improve?
>>
>> One tiny improvement (not a complete fix) in my mind is a testing copy_to_user
>> before we execute the commands. If it succeeds, it's even less likely the last
>> copy_to_user could fail, so that we can even probably assert it won't. The
>> testing 'copy_to_user(cmds, &kcmds, ...)' will not hurt functionality if do it
>> right after 'copy_from_user(&kcmds, cmds, ...)'. But I'm not sure about the
>> performance impact, especially when @ncmds is big.
>>
> I don't think this is the right thing to do, since, for example, another
> thread could unmap the memory region holding buffer between the "check"
> copy_to_user and the actual one.
>
> The two alternatives that I see are:
>
> 1. If copy_to_user fails, ignore it and return the number of commands
> that succeeded (i.e., what the code in your patch does).
> 2. If copy_to_user fails, return -EFAULT, regardless of how many
> commands succeeded.
>
> The problem with 1 is that it could potentially mask bugs in a user
> program. You could imagine a buggy program that passes a read-only
> buffer to epoll_ctl_batch and never finds out about it because they
> don't get the error. (Then, when there's a real error in one of the
> epoll_ctl_cmds, but .result is 0, someone will be very confused.)
>
> So I think 2 is the better option. Sure, the user will have no idea how
> many commands were executed, but when would EFAULT on this system call
> be part of normal operation, anyways? You'll find the memory bug, fix
> it, and rest easy knowing that nothing is silently failing behind your
> back.
What Omar says makes sense to me too. Best to have the user get a clear
error indication for this case.
Cheers,
Michael
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
^ permalink raw reply
* Re: [PATCH RFC v3 0/7] epoll: Introduce new syscalls, epoll_ctl_batch and epoll_pwait1
From: Jonathan Corbet @ 2015-02-15 22:00 UTC (permalink / raw)
To: Fam Zheng
Cc: linux-kernel, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86,
Alexander Viro, Andrew Morton, Kees Cook, Andy Lutomirski,
David Herrmann, Alexei Starovoitov, Miklos Szeredi,
David Drysdale, Oleg Nesterov, David S. Miller, Vivek Goyal,
Mike Frysinger, Theodore Ts'o, Heiko Carstens,
Rasmus Villemoes, Rashika Kheria, Hugh Dickins, Mathieu Desnoyers,
Peter Zijlstra <peter>
In-Reply-To: <1423818243-15410-1-git-send-email-famz@redhat.com>
On Fri, 13 Feb 2015 17:03:56 +0800
Fam Zheng <famz@redhat.com> wrote:
> SYNOPSIS
>
> #include <sys/epoll.h>
>
> int epoll_pwait1(int epfd, int flags,
> struct epoll_event *events,
> int maxevents,
> struct epoll_wait_params *params);
Quick, possibly dumb question: might it make sense to also pass in
sizeof(struct epoll_wait_params)? That way, when somebody wants to add
another parameter in the future, the kernel can tell which version is in
use and they won't have to do an epoll_pwait2()?
Thanks,
jon
^ permalink raw reply
* Re: [PATCH 05/14] ARM: call reset_controller_of_init from default time_init handler
From: Rob Herring @ 2015-02-15 22:17 UTC (permalink / raw)
To: Maxime Coquelin
Cc: Jonathan Corbet, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Philipp Zabel, Russell King,
Daniel Lezcano, Thomas Gleixner, Linus Walleij,
Greg Kroah-Hartman, Jiri Slaby, Arnd Bergmann, Andrew Morton,
David S. Miller, Mauro Carvalho Chehab, Joe Perches,
Antti Palosaari, Tejun Heo, Will Deacon, Nikolay Borisov,
Rusty Russell, Kees
In-Reply-To: <1423763164-5606-6-git-send-email-mcoquelin.stm32@gmail.com>
On Thu, Feb 12, 2015 at 11:45 AM, Maxime Coquelin
<mcoquelin.stm32@gmail.com> wrote:
> Some DT ARM platforms need the reset controllers to be initialized before
> the timers.
> This is the case of the stm32 and sunxi platforms.
I would say this is the exception, not the rule and therefore should
be handled in a machine desc function. Or it could be part of your
timer setup. Or is the bootloader's problem (like arch timer setup).
We just want to limit how much this mechanism gets used.
Rob
>
> This patch adds a call to reset_controller_of_init() to the default
> .init_time callback when RESET_CONTROLLER is used by the platform.
>
> Signed-off-by: Maxime Coquelin <mcoquelin.stm32@gmail.com>
> ---
> arch/arm/kernel/time.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/arch/arm/kernel/time.c b/arch/arm/kernel/time.c
> index 0cc7e58..4601b1e 100644
> --- a/arch/arm/kernel/time.c
> +++ b/arch/arm/kernel/time.c
> @@ -20,6 +20,7 @@
> #include <linux/irq.h>
> #include <linux/kernel.h>
> #include <linux/profile.h>
> +#include <linux/reset-controller.h>
> #include <linux/sched.h>
> #include <linux/sched_clock.h>
> #include <linux/smp.h>
> @@ -117,6 +118,9 @@ void __init time_init(void)
> if (machine_desc->init_time) {
> machine_desc->init_time();
> } else {
> +#ifdef CONFIG_RESET_CONTROLLER
> + reset_controller_of_init();
> +#endif
> #ifdef CONFIG_COMMON_CLK
> of_clk_init(NULL);
> #endif
> --
> 1.9.1
>
^ 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