* [PATCH] arm64: kvm: use -fno-jump-tables with clang
From: Nick Desaulniers @ 2018-05-23 17:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAAeHK+wk0hSH6iQcsLzrpjvsGocev-UZ6=z95ysecdFnAcHVSQ@mail.gmail.com>
On Wed, May 23, 2018 at 4:54 AM Andrey Konovalov <andreyknvl@google.com>
wrote:
> On Tue, May 22, 2018 at 8:28 PM, Nick Desaulniers
> <ndesaulniers@google.com> wrote:
> > On Fri, May 18, 2018 at 11:13 AM Marc Zyngier <marc.zyngier@arm.com>
wrote:
> >> > - you have checked that with a released version of the compiler, you
> >
> > On Tue, May 22, 2018 at 10:58 AM Andrey Konovalov <andreyknvl@google.com
> > wrote:
> >> Tested-by: Andrey Konovalov <andreyknvl@google.com>
> >
> > Hi Andrey,
> > Thank you very much for this report. Can you confirm as well the
version
> > of Clang that you were using?
> I'm on 86852a40 ("[InstCombine] Calloc-ed strings optimizations").
> > If it's not a binary release (built from
> > source), would you be able to re-confirm with a released version?
> Sure. Which release should I try and how do I get it?
Maybe clang-6.0 as the latest release (though I suspect you may run into
the recently-fixed-in-clang-7.0 "S" constraint bug that you reported).
I've had luck on debian based distributions installing from:
http://apt.llvm.org/
(These can be added to your /etc/apt/sources.list, then a `sudo apt update`
and `sudo apt install clang-6.0`)
If you're not able to add remote repositories (some employers block this ;)
), then you can find releases for download for a few different platforms:
https://releases.llvm.org/
For example, a quick:
$ mkdir llvm-6.0
$ cd !$
$ wget
https://releases.llvm.org/6.0.0/clang+llvm-6.0.0-x86_64-linux-gnu-debian8.tar.xz
$ tar xvf clang+llvm-6.0.0-x86_64-linux-gnu-debian8.tar.xz
$ ./clang+llvm-6.0.0-x86_64-linux-gnu-debian8/bin/clang-6.0 -v
clang version 6.0.0 (tags/RELEASE_600/final)
Target: x86_64-unknown-linux-gnu
Thread model: posix
InstalledDir: .../llvm-6.0/./clang+llvm-6.0.0-x86_64-linux-gnu-debian8/bin
Found candidate GCC installation: ...
Candidate multilib: .;@m64
Selected multilib: .;@m64
Seems to work.
--
Thanks,
~Nick Desaulniers
^ permalink raw reply
* [PATCH v4 2/2] arm64: signal: Report signal frame size to userspace via auxv
From: Dave Martin @ 2018-05-23 17:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1527097616-25214-1-git-send-email-Dave.Martin@arm.com>
Stateful CPU architecture extensions may require the signal frame
to grow to a size that exceeds the arch's MINSIGSTKSZ #define.
However, changing this #define is an ABI break.
To allow userspace the option of determining the signal frame size
in a more forwards-compatible way, this patch adds a new auxv entry
tagged with AT_MINSIGSTKSZ, which provides the maximum signal frame
size that the process can observe during its lifetime.
If AT_MINSIGSTKSZ is absent from the aux vector, the caller can
assume that the MINSIGSTKSZ #define is sufficient. This allows for
a consistent interface with older kernels that do not provide
AT_MINSIGSTKSZ.
The idea is that libc could expose this via sysconf() or some
similar mechanism.
There is deliberately no AT_SIGSTKSZ. The kernel knows nothing
about userspace's own stack overheads and should not pretend to
know.
For arm64:
The primary motivation for this interface is the Scalable Vector
Extension, which can require at least 4KB or so of extra space
in the signal frame for the largest hardware implementations.
To determine the correct value, a "Christmas tree" mode (via the
add_all argument) is added to setup_sigframe_layout(), to simulate
addition of all possible records to the signal frame at maximum
possible size.
If this procedure goes wrong somehow, resulting in a stupidly large
frame layout and hence failure of sigframe_alloc() to allocate a
record to the frame, then this is indicative of a kernel bug: the
kernel's internal SIGFRAME_MAXSZ is supposed to sanity-check
against generting frames that we consider _impossibly_ large. If
we hit this case, SIGFRAME_MAXSZ is used as our best guess, and we
WARN().
For arm64 SVE:
The SVE context block in the signal frame needs to be considered
too when computing the maximum possible signal frame size.
Because the size of this block depends on the vector length, this
patch computes the size based not on the thread's current vector
length but instead on the maximum possible vector length: this
determines the maximum size of SVE context block that can be
observed in any signal frame for the lifetime of the process.
Signed-off-by: Dave Martin <Dave.Martin@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Alex Benn?e <alex.bennee@linaro.org>
---
Changes since v3:
* Fall back to SIGFRAME_MAXSZ, not SIGSTKSZ in case of
setup_sigframe_layout() failure, since exceeding SIGFRAME_MAXSZ is
the only way this can fail: therefore SIGFRAME_MAXSZ is our best
guess at the required size.
* Commit message updated to reflect the above change.
Requested by Will Deacon:
* Remove superfluous WARN()s from ARCH_DL_INFO and
setup_sigframe_layout(). If something went wrong and we still have
no value for signal_minsigstksz by the time we exec some user
process, AT_MINSIGSTKSZ is omitted from the aux vector. Userspace
should fall back to the MINSIGSTKSZ #define anyway in this case
(i.e., do things the POSIX way).
* Change the type of signal_minsigstksz to unsigned long, to match the
auxv and sigframe_size() types. (sigframe_size uses size_t, but it's
somewhat moot exactly what the types are, providing that
SIGFRAME_MAXSZ fits in all of them).
Also requested by Mark Rutland:
* Merge #ifndef __ASSEMBLY__ block introduced in <asm/elf.h> into
the existing one. The only things above are #defines, which
shouldn't be affected by this.
---
arch/arm64/include/asm/elf.h | 11 ++++++++
arch/arm64/include/asm/processor.h | 5 ++++
arch/arm64/include/uapi/asm/auxvec.h | 3 ++-
arch/arm64/kernel/cpufeature.c | 1 +
arch/arm64/kernel/signal.c | 52 +++++++++++++++++++++++++++++++-----
5 files changed, 64 insertions(+), 8 deletions(-)
diff --git a/arch/arm64/include/asm/elf.h b/arch/arm64/include/asm/elf.h
index fac1c4d..9c18f0e 100644
--- a/arch/arm64/include/asm/elf.h
+++ b/arch/arm64/include/asm/elf.h
@@ -121,6 +121,9 @@
#ifndef __ASSEMBLY__
+#include <linux/bug.h>
+#include <asm/processor.h> /* for signal_minsigstksz, used by ARCH_DLINFO */
+
typedef unsigned long elf_greg_t;
#define ELF_NGREG (sizeof(struct user_pt_regs) / sizeof(elf_greg_t))
@@ -148,6 +151,14 @@ typedef struct user_fpsimd_state elf_fpregset_t;
do { \
NEW_AUX_ENT(AT_SYSINFO_EHDR, \
(elf_addr_t)current->mm->context.vdso); \
+ \
+ /* \
+ * Should always be nonzero unless there's a kernel bug. If \
+ * the we haven't determined a sensible value to give to \
+ * userspace, omit the entry: \
+ */ \
+ if (likely(signal_minsigstksz)) \
+ NEW_AUX_ENT(AT_MINSIGSTKSZ, signal_minsigstksz); \
} while (0)
#define ARCH_HAS_SETUP_ADDITIONAL_PAGES
diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
index 7675989..65ab83e 100644
--- a/arch/arm64/include/asm/processor.h
+++ b/arch/arm64/include/asm/processor.h
@@ -35,6 +35,8 @@
#ifdef __KERNEL__
#include <linux/build_bug.h>
+#include <linux/cache.h>
+#include <linux/init.h>
#include <linux/stddef.h>
#include <linux/string.h>
@@ -244,6 +246,9 @@ void cpu_enable_pan(const struct arm64_cpu_capabilities *__unused);
void cpu_enable_cache_maint_trap(const struct arm64_cpu_capabilities *__unused);
void cpu_clear_disr(const struct arm64_cpu_capabilities *__unused);
+extern unsigned long __ro_after_init signal_minsigstksz; /* sigframe size */
+extern void __init minsigstksz_setup(void);
+
/* Userspace interface for PR_SVE_{SET,GET}_VL prctl()s: */
#define SVE_SET_VL(arg) sve_set_current_vl(arg)
#define SVE_GET_VL() sve_get_current_vl()
diff --git a/arch/arm64/include/uapi/asm/auxvec.h b/arch/arm64/include/uapi/asm/auxvec.h
index ec0a86d..743c0b8 100644
--- a/arch/arm64/include/uapi/asm/auxvec.h
+++ b/arch/arm64/include/uapi/asm/auxvec.h
@@ -19,7 +19,8 @@
/* vDSO location */
#define AT_SYSINFO_EHDR 33
+#define AT_MINSIGSTKSZ 51 /* stack needed for signal delivery */
-#define AT_VECTOR_SIZE_ARCH 1 /* entries in ARCH_DLINFO */
+#define AT_VECTOR_SIZE_ARCH 2 /* entries in ARCH_DLINFO */
#endif
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 9d1b06d..0e0b53d 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -1619,6 +1619,7 @@ void __init setup_cpu_features(void)
pr_info("emulated: Privileged Access Never (PAN) using TTBR0_EL1 switching\n");
sve_setup();
+ minsigstksz_setup();
/* Advertise that we have computed the system capabilities */
set_sys_caps_initialised();
diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
index 154b7d3..00b9990 100644
--- a/arch/arm64/kernel/signal.c
+++ b/arch/arm64/kernel/signal.c
@@ -17,6 +17,7 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
+#include <linux/cache.h>
#include <linux/compat.h>
#include <linux/errno.h>
#include <linux/kernel.h>
@@ -570,8 +571,15 @@ asmlinkage long sys_rt_sigreturn(struct pt_regs *regs)
return 0;
}
-/* Determine the layout of optional records in the signal frame */
-static int setup_sigframe_layout(struct rt_sigframe_user_layout *user)
+/*
+ * Determine the layout of optional records in the signal frame
+ *
+ * add_all: if true, lays out the biggest possible signal frame for
+ * this task; otherwise, generates a layout for the current state
+ * of the task.
+ */
+static int setup_sigframe_layout(struct rt_sigframe_user_layout *user,
+ bool add_all)
{
int err;
@@ -581,7 +589,7 @@ static int setup_sigframe_layout(struct rt_sigframe_user_layout *user)
return err;
/* fault information, if valid */
- if (current->thread.fault_code) {
+ if (add_all || current->thread.fault_code) {
err = sigframe_alloc(user, &user->esr_offset,
sizeof(struct esr_context));
if (err)
@@ -591,8 +599,14 @@ static int setup_sigframe_layout(struct rt_sigframe_user_layout *user)
if (system_supports_sve()) {
unsigned int vq = 0;
- if (test_thread_flag(TIF_SVE))
- vq = sve_vq_from_vl(current->thread.sve_vl);
+ if (add_all || test_thread_flag(TIF_SVE)) {
+ int vl = sve_max_vl;
+
+ if (!add_all)
+ vl = current->thread.sve_vl;
+
+ vq = sve_vq_from_vl(vl);
+ }
err = sigframe_alloc(user, &user->sve_offset,
SVE_SIG_CONTEXT_SIZE(vq));
@@ -603,7 +617,6 @@ static int setup_sigframe_layout(struct rt_sigframe_user_layout *user)
return sigframe_alloc_end(user);
}
-
static int setup_sigframe(struct rt_sigframe_user_layout *user,
struct pt_regs *regs, sigset_t *set)
{
@@ -701,7 +714,7 @@ static int get_sigframe(struct rt_sigframe_user_layout *user,
int err;
init_user_layout(user);
- err = setup_sigframe_layout(user);
+ err = setup_sigframe_layout(user, false);
if (err)
return err;
@@ -936,3 +949,28 @@ asmlinkage void do_notify_resume(struct pt_regs *regs,
thread_flags = READ_ONCE(current_thread_info()->flags);
} while (thread_flags & _TIF_WORK_MASK);
}
+
+unsigned long __ro_after_init signal_minsigstksz;
+
+/*
+ * Determine the stack space required for guaranteed signal devliery.
+ * This function is used to populate AT_MINSIGSTKSZ at process startup.
+ * cpufeatures setup is assumed to be complete.
+ */
+void __init minsigstksz_setup(void)
+{
+ struct rt_sigframe_user_layout user;
+
+ init_user_layout(&user);
+
+ /*
+ * If this fails, SIGFRAME_MAXSZ needs to be enlarged. It won't
+ * be big enough, but it's our best guess:
+ */
+ if (WARN_ON(setup_sigframe_layout(&user, true)))
+ signal_minsigstksz = SIGFRAME_MAXSZ;
+ else
+ signal_minsigstksz = sigframe_size(&user) +
+ round_up(sizeof(struct frame_record), 16) +
+ 16; /* max alignment padding */
+}
--
2.1.4
^ permalink raw reply related
* [PATCH v4 1/2] arm64/sve: Thin out initialisation sanity-checks for sve_max_vl
From: Dave Martin @ 2018-05-23 17:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1527097616-25214-1-git-send-email-Dave.Martin@arm.com>
Now that the kernel SVE support is reasonably mature, it is
excessive to default sve_max_vl to the invalid value -1 and then
sprinkle WARN_ON()s around the place to make sure it has been
initialised before use. The cpufeatures code already runs pretty
early, and will ensure sve_max_vl gets initialised.
This patch initialises sve_max_vl to something sane that will be
supported by every SVE implementation, and removes most of the
sanity checks.
The checks in find_supported_vector_length() are retained for now.
If anything goes horribly wrong, we are likely to trip a check here
sooner or later.
Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---
arch/arm64/kernel/fpsimd.c | 17 ++++-------------
arch/arm64/kernel/ptrace.c | 3 ---
2 files changed, 4 insertions(+), 16 deletions(-)
diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index 87a3536..f9ec640 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -129,7 +129,7 @@ static int sve_default_vl = -1;
#ifdef CONFIG_ARM64_SVE
/* Maximum supported vector length across all CPUs (initially poisoned) */
-int __ro_after_init sve_max_vl = -1;
+int __ro_after_init sve_max_vl = SVE_VL_MIN;
/* Set of available vector lengths, as vq_to_bit(vq): */
static __ro_after_init DECLARE_BITMAP(sve_vq_map, SVE_VQ_MAX);
static void __percpu *efi_sve_state;
@@ -360,22 +360,13 @@ static int sve_proc_do_default_vl(struct ctl_table *table, int write,
return ret;
/* Writing -1 has the special meaning "set to max": */
- if (vl == -1) {
- /* Fail safe if sve_max_vl wasn't initialised */
- if (WARN_ON(!sve_vl_valid(sve_max_vl)))
- vl = SVE_VL_MIN;
- else
- vl = sve_max_vl;
-
- goto chosen;
- }
+ if (vl == -1)
+ vl = sve_max_vl;
if (!sve_vl_valid(vl))
return -EINVAL;
- vl = find_supported_vector_length(vl);
-chosen:
- sve_default_vl = vl;
+ sve_default_vl = find_supported_vector_length(vl);
return 0;
}
diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
index 7ff81fe..577deb0 100644
--- a/arch/arm64/kernel/ptrace.c
+++ b/arch/arm64/kernel/ptrace.c
@@ -766,9 +766,6 @@ static void sve_init_header_from_task(struct user_sve_header *header,
vq = sve_vq_from_vl(header->vl);
header->max_vl = sve_max_vl;
- if (WARN_ON(!sve_vl_valid(sve_max_vl)))
- header->max_vl = header->vl;
-
header->size = SVE_PT_SIZE(vq, header->flags);
header->max_size = SVE_PT_SIZE(sve_vq_from_vl(header->max_vl),
SVE_PT_REGS_SVE);
--
2.1.4
^ permalink raw reply related
* [PATCH v4 0/2] arm64: signal: Report signal frame size to userspace via auxv
From: Dave Martin @ 2018-05-23 17:46 UTC (permalink / raw)
To: linux-arm-kernel
This series adds support for telling userspace the size of the signal
frame via a new AT_MINSIGSTKSZ entry in the aux vector.
This is an update to a previous v3 standalone patch [1], which is
presented here as patch 2 with some updates in response to review.
See the changelog in the patch for details.
Patch 1 is new, and thins out some WARN_ON() checks for timely
initialisation of sve_max_vl that now look excessive, to strengthen
the case for not adding more such checks in patch 2.
[1] [PATCH v3] arm64: signal: Report signal frame size to userspace via auxv
http://lists.infradead.org/pipermail/linux-arm-kernel/2018-May/578585.html
Dave Martin (2):
arm64/sve: Thin out initialisation sanity-checks for sve_max_vl
arm64: signal: Report signal frame size to userspace via auxv
arch/arm64/include/asm/elf.h | 11 ++++++++
arch/arm64/include/asm/processor.h | 5 ++++
arch/arm64/include/uapi/asm/auxvec.h | 3 ++-
arch/arm64/kernel/cpufeature.c | 1 +
arch/arm64/kernel/fpsimd.c | 17 +++---------
arch/arm64/kernel/ptrace.c | 3 ---
arch/arm64/kernel/signal.c | 52 +++++++++++++++++++++++++++++++-----
7 files changed, 68 insertions(+), 24 deletions(-)
--
2.1.4
^ permalink raw reply
* [PATCH 07/11] dts: juno: Add scatter-gather support for all revisions
From: Mathieu Poirier @ 2018-05-23 17:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526661567-4578-8-git-send-email-suzuki.poulose@arm.com>
On 18 May 2018 at 10:39, Suzuki K Poulose <suzuki.poulose@arm.com> wrote:
> Advertise that the scatter-gather is properly integrated on
> all revisions of Juno board.
>
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Cc: Sudeep Holla <sudeep.holla@arm.com>
> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> ---
> arch/arm64/boot/dts/arm/juno-base.dtsi | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm64/boot/dts/arm/juno-base.dtsi b/arch/arm64/boot/dts/arm/juno-base.dtsi
> index eb749c5..6ce9090 100644
> --- a/arch/arm64/boot/dts/arm/juno-base.dtsi
> +++ b/arch/arm64/boot/dts/arm/juno-base.dtsi
> @@ -198,6 +198,7 @@
> clocks = <&soc_smc50mhz>;
> clock-names = "apb_pclk";
> power-domains = <&scpi_devpd 0>;
> + arm,scatter-gather;
> port {
> etr_in_port: endpoint {
> slave-mode;
> --
> 2.7.4
>
^ permalink raw reply
* [PATCH v4 3/3] arm64: Introduce command line parameter to disable CNP
From: Catalin Marinas @ 2018-05-23 17:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526638022-4137-4-git-send-email-vladimir.murzin@arm.com>
On Fri, May 18, 2018 at 11:07:02AM +0100, Vladimir Murzin wrote:
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 11fc28e..8f59d47 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -2636,6 +2636,10 @@
>
> noclflush [BUGS=X86] Don't use the CLFLUSH instruction
>
> + nocnp [ARM64]
> + Disable CNP (Common not Private translations)
> + even if it is supported by processor.
> +
> nodelayacct [KNL] Disable per-task delay accounting
>
> nodsp [SH] Disable hardware DSP at boot time.
Do we actually have a use-case for this command line option? I'm not
considering hardware errata as these are handled separately in the
kernel.
--
Catalin
^ permalink raw reply
* [PATCH 3/5] watchdog: sp805: set WDOG_HW_RUNNING when appropriate
From: Scott Branden @ 2018-05-23 17:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5a996888-d3d3-9ae6-e438-5de4d5e3ea32@broadcom.com>
Raym
On 18-05-23 09:29 AM, Ray Jui wrote:
> Hi Robin,
>
> On 5/23/2018 4:48 AM, Robin Murphy wrote:
>> On 23/05/18 08:52, Scott Branden wrote:
>>>
>>>
>>> On 18-05-22 04:24 PM, Ray Jui wrote:
>>>> Hi Guenter,
>>>>
>>>> On 5/22/2018 1:54 PM, Guenter Roeck wrote:
>>>>> On Tue, May 22, 2018 at 11:47:18AM -0700, Ray Jui wrote:
>>>>>> If the watchdog hardware is already enabled during the boot process,
>>>>>> when the Linux watchdog driver loads, it should reset the
>>>>>> watchdog and
>>>>>> tell the watchdog framework. As a result, ping can be generated from
>>>>>> the watchdog framework, until the userspace watchdog daemon takes
>>>>>> over
>>>>>> control
>>>>>>
>>>>>> Signed-off-by: Ray Jui <ray.jui@broadcom.com>
>>>>>> Reviewed-by: Vladimir Olovyannikov
>>>>>> <vladimir.olovyannikov@broadcom.com>
>>>>>> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
>>>>>> ---
>>>>>> ? drivers/watchdog/sp805_wdt.c | 22 ++++++++++++++++++++++
>>>>>> ? 1 file changed, 22 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/watchdog/sp805_wdt.c
>>>>>> b/drivers/watchdog/sp805_wdt.c
>>>>>> index 1484609..408ffbe 100644
>>>>>> --- a/drivers/watchdog/sp805_wdt.c
>>>>>> +++ b/drivers/watchdog/sp805_wdt.c
>>>>>> @@ -42,6 +42,7 @@
>>>>>> ????? /* control register masks */
>>>>>> ????? #define??? INT_ENABLE??? (1 << 0)
>>>>>> ????? #define??? RESET_ENABLE??? (1 << 1)
>>>>>> +??? #define??? ENABLE_MASK??? (INT_ENABLE | RESET_ENABLE)
>>>>>> ? #define WDTINTCLR??????? 0x00C
>>>>>> ? #define WDTRIS??????????? 0x010
>>>>>> ? #define WDTMIS??????????? 0x014
>>>>>> @@ -74,6 +75,18 @@ module_param(nowayout, bool, 0);
>>>>>> ? MODULE_PARM_DESC(nowayout,
>>>>>> ????????? "Set to 1 to keep watchdog running after device release");
>>>>>> ? +/* returns true if wdt is running; otherwise returns false */
>>>>>> +static bool wdt_is_running(struct watchdog_device *wdd)
>>>>>> +{
>>>>>> +??? struct sp805_wdt *wdt = watchdog_get_drvdata(wdd);
>>>>>> +
>>>>>> +??? if ((readl_relaxed(wdt->base + WDTCONTROL) & ENABLE_MASK) ==
>>>>>> +??????? ENABLE_MASK)
>>>>>> +??????? return true;
>>>>>> +??? else
>>>>>> +??????? return false;
>>>>>
>>>>> ????return !!(readl_relaxed(wdt->base + WDTCONTROL) & ENABLE_MASK));
>>>>>
>>>>
>>>> Note ENABLE_MASK contains two bits (INT_ENABLE and RESET_ENABLE);
>>>> therefore, a simple !!(expression) would not work? That is, the
>>>> masked result needs to be compared against the mask again to ensure
>>>> both bits are set, right?
>>> Ray - your original code looks correct to me.? Easier to read and
>>> less prone to errors as shown in the attempted translation to a
>>> single statement.
>>
>> ?????if (<boolean condition>)
>> ???????? return true;
>> ?????else
>> ???????? return false;
>>
>> still looks really dumb, though, and IMO is actually harder to read
>> than just "return <boolean condition>;" because it forces you to stop
>> and double-check that the logic is, in fact, only doing the obvious
>> thing.
>
> If you can propose a way to modify my original code above to make it
> more readable, I'm fine to make the change.
>
> As I mentioned, I don't think the following change proposed by Guenter
> will work due to the reason I pointed out:
>
> return !!(readl_relaxed(wdt->base + WDTCONTROL) & ENABLE_MASK));
What they are looking for is:
return ((readl_relaxed(wdt->base + WDTCONTROL) & ENABLE_MASK) ==
ENABLE_MASK);
or maybe:
return !!((readl_relaxed(wdt->base + WDTCONTROL) & ENABLE_MASK) ==
ENABLE_MASK);
>
>>
>> Robin.
>>
>>
>>
>> p.s. No thanks for making me remember the mind-boggling horror of
>> briefly maintaining part of this legacy codebase... :P
>>
>> $ grep -r '? true : false' --include=*.cpp . | wc -l
>> 951
^ permalink raw reply
* [PATCH 3/5] watchdog: sp805: set WDOG_HW_RUNNING when appropriate
From: Robin Murphy @ 2018-05-23 17:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5a996888-d3d3-9ae6-e438-5de4d5e3ea32@broadcom.com>
On 23/05/18 17:29, Ray Jui wrote:
> Hi Robin,
>
> On 5/23/2018 4:48 AM, Robin Murphy wrote:
>> On 23/05/18 08:52, Scott Branden wrote:
>>>
>>>
>>> On 18-05-22 04:24 PM, Ray Jui wrote:
>>>> Hi Guenter,
>>>>
>>>> On 5/22/2018 1:54 PM, Guenter Roeck wrote:
>>>>> On Tue, May 22, 2018 at 11:47:18AM -0700, Ray Jui wrote:
>>>>>> If the watchdog hardware is already enabled during the boot process,
>>>>>> when the Linux watchdog driver loads, it should reset the watchdog
>>>>>> and
>>>>>> tell the watchdog framework. As a result, ping can be generated from
>>>>>> the watchdog framework, until the userspace watchdog daemon takes
>>>>>> over
>>>>>> control
>>>>>>
>>>>>> Signed-off-by: Ray Jui <ray.jui@broadcom.com>
>>>>>> Reviewed-by: Vladimir Olovyannikov
>>>>>> <vladimir.olovyannikov@broadcom.com>
>>>>>> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
>>>>>> ---
>>>>>> ? drivers/watchdog/sp805_wdt.c | 22 ++++++++++++++++++++++
>>>>>> ? 1 file changed, 22 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/watchdog/sp805_wdt.c
>>>>>> b/drivers/watchdog/sp805_wdt.c
>>>>>> index 1484609..408ffbe 100644
>>>>>> --- a/drivers/watchdog/sp805_wdt.c
>>>>>> +++ b/drivers/watchdog/sp805_wdt.c
>>>>>> @@ -42,6 +42,7 @@
>>>>>> ????? /* control register masks */
>>>>>> ????? #define??? INT_ENABLE??? (1 << 0)
>>>>>> ????? #define??? RESET_ENABLE??? (1 << 1)
>>>>>> +??? #define??? ENABLE_MASK??? (INT_ENABLE | RESET_ENABLE)
>>>>>> ? #define WDTINTCLR??????? 0x00C
>>>>>> ? #define WDTRIS??????????? 0x010
>>>>>> ? #define WDTMIS??????????? 0x014
>>>>>> @@ -74,6 +75,18 @@ module_param(nowayout, bool, 0);
>>>>>> ? MODULE_PARM_DESC(nowayout,
>>>>>> ????????? "Set to 1 to keep watchdog running after device release");
>>>>>> ? +/* returns true if wdt is running; otherwise returns false */
>>>>>> +static bool wdt_is_running(struct watchdog_device *wdd)
>>>>>> +{
>>>>>> +??? struct sp805_wdt *wdt = watchdog_get_drvdata(wdd);
>>>>>> +
>>>>>> +??? if ((readl_relaxed(wdt->base + WDTCONTROL) & ENABLE_MASK) ==
>>>>>> +??????? ENABLE_MASK)
>>>>>> +??????? return true;
>>>>>> +??? else
>>>>>> +??????? return false;
>>>>>
>>>>> ????return !!(readl_relaxed(wdt->base + WDTCONTROL) & ENABLE_MASK));
>>>>>
>>>>
>>>> Note ENABLE_MASK contains two bits (INT_ENABLE and RESET_ENABLE);
>>>> therefore, a simple !!(expression) would not work? That is, the
>>>> masked result needs to be compared against the mask again to ensure
>>>> both bits are set, right?
>>> Ray - your original code looks correct to me.? Easier to read and
>>> less prone to errors as shown in the attempted translation to a
>>> single statement.
>>
>> ?????if (<boolean condition>)
>> ???????? return true;
>> ?????else
>> ???????? return false;
>>
>> still looks really dumb, though, and IMO is actually harder to read
>> than just "return <boolean condition>;" because it forces you to stop
>> and double-check that the logic is, in fact, only doing the obvious
>> thing.
>
> If you can propose a way to modify my original code above to make it
> more readable, I'm fine to make the change.
Well,
return readl_relaxed(wdt->base + WDTCONTROL) & ENABLE_MASK == ENABLE_MASK;
would probably be reasonable to anyone other than the 80-column zealots,
but removing the silly boolean-to-boolean translation idiom really only
emphasises the fact that it's fundamentally a big complex statement; for
maximum clarity I'd be inclined to separate the two logical operations
(read and comparison), e.g.:
u32 wdtcontrol = readl_relaxed(wdt->base + WDTCONTROL);
return wdtcontrol & ENABLE_MASK == ENABLE_MASK;
which is still -3 lines vs. the original.
> As I mentioned, I don't think the following change proposed by Guenter
> will work due to the reason I pointed out:
>
> return !!(readl_relaxed(wdt->base + WDTCONTROL) & ENABLE_MASK));
FWIW, getting the desired result should only need one logical not
swapping for a bitwise one there:
return !(~readl_relaxed(wdt->base + WDTCONTROL) & ENABLE_MASK);
but that's well into "too clever for its own good" territory ;)
Robin.
^ permalink raw reply
* [PATCH v4 2/3] arm64: KVM: Enable Common Not Private translations
From: Catalin Marinas @ 2018-05-23 17:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526638022-4137-3-git-send-email-vladimir.murzin@arm.com>
On Fri, May 18, 2018 at 11:07:01AM +0100, Vladimir Murzin wrote:
> diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
> index a4c1b76..9a651a2 100644
> --- a/virt/kvm/arm/arm.c
> +++ b/virt/kvm/arm/arm.c
> @@ -472,7 +472,7 @@ static bool need_new_vmid_gen(struct kvm *kvm)
> static void update_vttbr(struct kvm *kvm)
> {
> phys_addr_t pgd_phys;
> - u64 vmid;
> + u64 vmid, cnp = kvm_cpu_has_cnp() ? 1 : 0;
Please define a VTTBR_CNP_BIT here instead of a hard-coded value.
> bool new_gen;
>
> read_lock(&kvm_vmid_lock);
> @@ -522,7 +522,7 @@ static void update_vttbr(struct kvm *kvm)
> pgd_phys = virt_to_phys(kvm->arch.pgd);
> BUG_ON(pgd_phys & ~VTTBR_BADDR_MASK);
> vmid = ((u64)(kvm->arch.vmid) << VTTBR_VMID_SHIFT) & VTTBR_VMID_MASK(kvm_vmid_bits);
> - kvm->arch.vttbr = kvm_phys_to_vttbr(pgd_phys) | vmid;
> + kvm->arch.vttbr = kvm_phys_to_vttbr(pgd_phys) | vmid | cnp;
>
> write_unlock(&kvm_vmid_lock);
> }
--
Catalin
^ permalink raw reply
* [PATCH 0/2] get rid of Kconfig symbol MACH_MESON8B
From: Kevin Hilman @ 2018-05-23 17:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180520172353.19256-1-martin.blumenstingl@googlemail.com>
Martin Blumenstingl <martin.blumenstingl@googlemail.com> writes:
> as noted by Kevin [0] there are two Kconfig symbols which only differ
> in their Kconfig help text and the list of .dtbs that are being built.
> the goal of this small series is to get rid of the unnecessary
> MACH_MESON8B Kconfig symbol by merging it into MACH_MESON8.
Thanks for the cleanup.
Applied to v4.18/dt with Jerome's tag,
Kevin
^ permalink raw reply
* [PATCH v2 0/4] ARM64: dts: meson-axg: i2c updates
From: Kevin Hilman @ 2018-05-23 17:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180517093100.22225-1-jbrunet@baylibre.com>
Jerome Brunet <jbrunet@baylibre.com> writes:
> This patchset fixes a few problems found in the i2c nodes of
> amlogic's meson-axg platform. It also adds the missing pins for
> AO controller so we can use it on the S400
>
> This series has been tested on the S400 board.
>
> Jerome Brunet (4):
> ARM64: dts: meson-axg: clean-up i2c nodes
> ARM64: dts: meson-axg: correct i2c AO clock
> ARM64: dts: meson-axg: add i2c AO pins
> ARM64: dts: meson-axg: enable i2c AO on the S400 board
Applied to v4.18/dt64,
Kevin
^ permalink raw reply
* [PATCH] clk: aspeed: Add 24MHz fixed clock
From: Rob Herring @ 2018-05-23 17:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526633822-17138-1-git-send-email-mine260309@gmail.com>
On Fri, May 18, 2018 at 04:57:02PM +0800, Lei YU wrote:
> Add a 24MHz fixed clock.
> This clock will be used for certain devices, e.g. pwm.
>
> Signed-off-by: Lei YU <mine260309@gmail.com>
> ---
> drivers/clk/clk-aspeed.c | 9 ++++++++-
> include/dt-bindings/clock/aspeed-clock.h | 1 +
> 2 files changed, 9 insertions(+), 1 deletion(-)
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* [PATCH] firmware: arm_scmi: remove some unnecessary checks
From: Sudeep Holla @ 2018-05-23 16:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180519063715.GB4991@mwanda>
On 19/05/18 07:37, Dan Carpenter wrote:
> The "pi->dom_info" buffer is allocated in init() and it can't be NULL
> here. These tests are sort of weird as well because if "pi->dom_info"
> was NULL but "domain" was non-zero then it would lead to an Oops.
>
Indeed. Thanks for the patch.
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Applied.
--
Regards,
Sudeep
^ permalink raw reply
* [PATCH net] net: phy: broadcom: Fix bcm_write_exp()
From: Florian Fainelli @ 2018-05-23 16:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <89a1f8d7-5303-f3c0-aa38-43e64488ec5a@gmail.com>
On 05/22/2018 06:20 PM, Florian Fainelli wrote:
> Hi Andrew,
>
> On 05/22/2018 05:15 PM, Andrew Lunn wrote:
>> On Tue, May 22, 2018 at 05:04:49PM -0700, Florian Fainelli wrote:
>>> On newer PHYs, we need to select the expansion register to write with
>>> setting bits [11:8] to 0xf. This was done correctly by bcm7xxx.c prior
>>> to being migrated to generic code under bcm-phy-lib.c which
>>> unfortunately used the older implementation from the BCM54xx days.
>>
>> Hi Florian
>>
>> Does selecting the expansion register affect access to the standard
>> registers? Does this need locking like the Marvell PHY has when
>> changing pages?
>
> We should probably convert this to the page accessors since the
> expansion, misc and other shadow 0x1c accesses are all indirection
> layers to poke into a different address space of the PHY. That would be
> a separate fix though for a number of reasons.
I realize I did not quite answer your question, the answer to your
question AFAICT is no, setting the expansion register sequence and then
aborting mid-way is not a problem and does not impact the standard MII
registers because of how this is implemented. The registers are accessed
and latched through a specific indirect sequence, but there is no page
switching unlike the Marvell PHYs
--
Florian
^ permalink raw reply
* [PATCH 01/14] memory: ti-emif-sram: Add resume function to recopy sram code
From: Santosh Shilimkar @ 2018-05-23 16:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <739d9bbf-2acc-9c90-db43-cf78f5b184e3@ti.com>
On 5/23/2018 1:47 AM, Keerthy wrote:
>
>
> On Monday 16 April 2018 03:59 PM, Keerthy wrote:
>>
[..]
>>> Instead of this indirect method , why can't just check the previous
>>> deep sleep mode and based on that do copy or not. EMIF power status
>>> register should have something like that ?
>>
>> I will check if we have a register that tells the previous state of sram.
>
> Unfortunately i do not see any such register for knowing SRAM previous
> state in am43 TRM and hence this indirect way of knowing.
>
OK.
>
>>
>>>
>>> Another minor point is even though there is nothing to do in suspend,
>>> might be good to have a callback with comment that nothing to do with
>>> some explanation why not. Don't have strong preference but may for
>>> better readability.
>
> I can add a blank suspend call with comment
>
> "The contents are already present in DDR hence no need to explicitly save"
>
> The comment in resume function pretty much explains the above. So let me
> know if i need to add the suspend callback.
> Please add the empty suspend callback with comment.
Regards,
Santosh
^ permalink raw reply
* [PATCH v10 07/18] arm64: fpsimd: Eliminate task->mm checks
From: Catalin Marinas @ 2018-05-23 16:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180523150337.GO13470@e103592.cambridge.arm.com>
On Wed, May 23, 2018 at 04:03:37PM +0100, Dave P Martin wrote:
> On Wed, May 23, 2018 at 03:56:57PM +0100, Catalin Marinas wrote:
> > On Wed, May 23, 2018 at 02:31:59PM +0100, Dave P Martin wrote:
> > > On Wed, May 23, 2018 at 01:48:12PM +0200, Christoffer Dall wrote:
> > > > On Tue, May 22, 2018 at 05:05:08PM +0100, Dave Martin wrote:
> > > > > This is true by construction however: TIF_FOREIGN_FPSTATE is never
> > > > > cleared except when returning to userspace or returning from a
> > > > > signal: thus, for a true kernel thread no FPSIMD context is ever
> > > > > loaded, TIF_FOREIGN_FPSTATE will remain set and no context will
> > > > > ever be saved.
> > > >
> > > > I don't understand this construction proof; from looking at the patch
> > > > below it is not obvious to me why fpsimd_thread_switch() can never have
> > > > !wrong_task && !wrong_cpu and therefore clear TIF_FOREIGN_FPSTATE for a
> > > > kernel thread?
> > >
> > > Looking at this again, I think it is poorly worded. This patch aims to
> > > make it true by construction, but it isn't prior to the patch.
> > >
> > > I'm tempted to delete the paragraph: the assertion of both untrue and
> > > not the best way to justify that this patch works.
> > >
> > >
> > > How about:
> > >
> > > -8<-
> > >
> > > The context switch logic already isolates user threads from each other.
> > > This, it is sufficient for isolating user threads from the kernel,
> > > since the goal either way is to ensure that code executing in userspace
> > > cannot see any FPSIMD state except its own. Thus, there is no special
> > > property of kernel threads that we care about except that it is
> > > pointless to save or load FPSIMD register state for them.
> > >
> > > At worst, the removal of all the kernel thread special cases by this
> > > patch would thus spuriously load and save state for kernel threads when
> > > unnecessary.
> > >
> > > But the context switch logic is already deliberately optimised to defer
> > > reloads of the regs until ret_to_user (or sigreturn as a special case),
> > > which kernel threads by definition never reach.
> > >
> > > ->8-
> >
> > The "at worst" paragraph makes it look like it could happen (at least
> > until you reach the last paragraph). Maybe you can just say that
> > wrong_task and wrong_cpu (with the fpsimd_cpu = NR_CPUS addition) are
> > always true for kernel threads. You should probably mention this in a
> > comment in the code as well.
>
> What if I just delete the second paragraph, and remove the "But" from
> the start of the third, and append:
>
> "As a result, the wrong_task and wrong_cpu tests in
> fpsimd_thread_switch() will always yield false for kernel threads."
>
> ...with a similar comment in the code?
Sounds fine. With that, feel free to add:
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
^ permalink raw reply
* [PATCH] perf: hisi: fix uncore PMU index ID
From: Will Deacon @ 2018-05-23 16:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <a301d72f-687c-0c4f-1d44-83e5bbad5e60@hisilicon.com>
On Tue, May 22, 2018 at 05:18:51PM +0800, Zhangshaokun wrote:
> On 2018/5/22 1:05, Will Deacon wrote:
> > Whilst I'd normally just accept PMU driver submissions for vendor PMUs,
> > this part rang my alarm bells:
> >
> >> diff --git a/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c b/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c
> >> index 443906e..dcd8e77 100644
> >> --- a/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c
> >> +++ b/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c
> >> @@ -238,19 +238,10 @@ MODULE_DEVICE_TABLE(acpi, hisi_hha_pmu_acpi_match);
> >> static int hisi_hha_pmu_init_data(struct platform_device *pdev,
> >> struct hisi_pmu *hha_pmu)
> >> {
> >> - unsigned long long id;
> >> struct resource *res;
> >> - acpi_status status;
> >> -
> >> - status = acpi_evaluate_integer(ACPI_HANDLE(&pdev->dev),
> >> - "_UID", NULL, &id);
> >> - if (ACPI_FAILURE(status))
> >> - return -EINVAL;
> >> -
> >> - hha_pmu->index_id = id;
> >>
> >> /*
> >> - * Use SCCL_ID and UID to identify the HHA PMU, while
> >> + * Use SCCL_ID and HHA index ID to identify the HHA PMU, while
> >> * SCCL_ID is in MPIDR[aff2].
> >> */
> >> if (device_property_read_u32(&pdev->dev, "hisilicon,scl-id",
> >> @@ -258,6 +249,13 @@ static int hisi_hha_pmu_init_data(struct platform_device *pdev,
> >> dev_err(&pdev->dev, "Can not read hha sccl-id!\n");
> >> return -EINVAL;
> >> }
> >> +
> >> + if (device_property_read_u32(&pdev->dev, "hisilicon,idx-id",
> >> + &hha_pmu->index_id)) {
> >> + dev_err(&pdev->dev, "Can not read hha index-id!\n");
> >> + return -EINVAL;
> >> + }
> >
> > Is this a new DT property? If so, please can you update the binding
> > documentation and get an Ack from a DT maintainer? It's not clear to me
>
> No, it is not a DT property. We don't support DT mode for this platform and
> only support ACPI mode.
Hmm, but by using the firmware-agnostic "device_property_read_u32"
interface, aren't you implicitly supporting it via DT as well? In fact,
don't you now fail the probe if this new property isn't present? Isn't
that a regression?
> > what a "hisilicon,idx-id" is, nor how I would generate on from firmware.
> >
>
> For HiSilicon this platform, it supports multi-sccl. each sccl has more than one uncore
> PMUs. Like HHA uncore PMUs, each sccl has 2-HHA PMUs and idx-id is in _DSD package and
> used to distinguish different HHA PMUs with the same sccl, as follow:
> Name (_DSD, Package () {
> ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> Package () {
> Package () {"hisilicon,scl-id", 0x03},
> Package () {"hisilicon,idx-id", 0x00},
> }
> })
I'm still none the wiser about what this actually is. How is new _DSD crud
supposed to be documented?
Will
^ permalink raw reply
* [PATCH 2/6] dt-bindings: media: rcar-vin: Document data-active
From: Rob Herring @ 2018-05-23 16:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526488352-898-3-git-send-email-jacopo+renesas@jmondi.org>
On Wed, May 16, 2018 at 06:32:28PM +0200, Jacopo Mondi wrote:
> Document 'data-active' property in R-Car VIN device tree bindings.
> The property is optional when running with explicit synchronization
> (eg. BT.601) but mandatory when embedded synchronization is in use (eg.
> BT.656) as specified by the hardware manual.
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
> Documentation/devicetree/bindings/media/rcar_vin.txt | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
> index c53ce4e..17eac8a 100644
> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> @@ -63,6 +63,11 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
> If both HSYNC and VSYNC polarities are not specified, embedded
> synchronization is selected.
>
> + - data-active: active state of data enable signal (CLOCKENB pin).
> + 0/1 for LOW/HIGH respectively. If not specified, use HSYNC as
> + data enable signal. When using embedded synchronization this
> + property is mandatory.
This doesn't match the common property's definition which AIUI is for
the data lines' polarity. You need a new property. Perhaps a common one.
> +
> - port 1 - sub-nodes describing one or more endpoints connected to
> the VIN from local SoC CSI-2 receivers. The endpoint numbers must
> use the following schema.
> --
> 2.7.4
>
^ permalink raw reply
* [PATCH 5/6] ARM: dts: rcar-gen2: Remove unused VIN properties
From: Rob Herring @ 2018-05-23 16:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180516221307.GF17948@bigcity.dyn.berto.se>
On Thu, May 17, 2018 at 12:13:07AM +0200, Niklas S?derlund wrote:
> Hi Jacopo,
>
> Thanks for your work.
>
> On 2018-05-16 18:32:31 +0200, Jacopo Mondi wrote:
> > The 'bus-width' and 'pclk-sample' properties are not parsed by the VIN
> > driver and only confuse users. Remove them in all Gen2 SoC that used
> > them.
> >
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > ---
> > arch/arm/boot/dts/r8a7790-lager.dts | 3 ---
> > arch/arm/boot/dts/r8a7791-koelsch.dts | 3 ---
> > arch/arm/boot/dts/r8a7791-porter.dts | 1 -
> > arch/arm/boot/dts/r8a7793-gose.dts | 3 ---
> > arch/arm/boot/dts/r8a7794-alt.dts | 1 -
> > arch/arm/boot/dts/r8a7794-silk.dts | 1 -
> > 6 files changed, 12 deletions(-)
> >
> > diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
> > index 063fdb6..b56b309 100644
> > --- a/arch/arm/boot/dts/r8a7790-lager.dts
> > +++ b/arch/arm/boot/dts/r8a7790-lager.dts
> > @@ -873,10 +873,8 @@
> > port {
> > vin0ep2: endpoint {
> > remote-endpoint = <&adv7612_out>;
> > - bus-width = <24>;
>
> I can't really make up my mind if this is a good thing or not. Device
> tree describes the hardware and not what the drivers make use of. And
> the fact is that this bus is 24 bits wide. So I'm not sure we should
> remove these properties. But I would love to hear what others think
> about this.
IMO, this property should only be present when all the pins are not
connected. And by "all", I mean the minimum of what each end of the
graph can support.
Rob
^ permalink raw reply
* [PATCH 0/3] irqchip: meson-gpio: add support for Meson-AXG SoCs
From: Marc Zyngier @ 2018-05-23 16:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <0c97c61b-c05b-21cf-c880-80cff04ed6f2@amlogic.com>
On 16/05/18 03:50, Yixun Lan wrote:
> Hi Marc (or anyone else)
>
> On 04/08/18 22:56, Yixun Lan wrote:
>> This series try to add GPIO interrupt controller support for Meson-AXG SoCs.
>> The first patch is a trivial typo fix, I can fold the first two patches
>> together if necessary.
>>
>> Yixun Lan (3):
>> dt-bindings: interrupt-controller: fix the double quotes
>> dt-bindings: interrupt-controller: New binding for Meson-AXG SoC
>> irqchip/meson-gpio: add support for Meson-AXG SoCs
>>
>> .../bindings/interrupt-controller/amlogic,meson-gpio-intc.txt | 11 ++++++-----
>> drivers/irqchip/irq-meson-gpio.c | 5 +++++
>> 2 files changed, 11 insertions(+), 5 deletions(-)
>>
>
> please consider this merely a ping..
> will you take this series, or is there anything holding this?
Sure, I'll queue that for 4.18.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* [PATCH 1/6] dt-bindings: media: rcar-vin: Describe optional ep properties
From: Rob Herring @ 2018-05-23 16:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526488352-898-2-git-send-email-jacopo+renesas@jmondi.org>
On Wed, May 16, 2018 at 06:32:27PM +0200, Jacopo Mondi wrote:
> Describe the optional endpoint properties for endpoint nodes of port at 0
> and port at 1 of the R-Car VIN driver device tree bindings documentation.
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
> Documentation/devicetree/bindings/media/rcar_vin.txt | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
> index a19517e1..c53ce4e 100644
> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> @@ -53,6 +53,16 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
> from external SoC pins described in video-interfaces.txt[1].
> Describing more then one endpoint in port 0 is invalid. Only VIN
> instances that are connected to external pins should have port 0.
> +
> + - Optional properties for endpoint nodes of port at 0:
> + - hsync-active: active state of the HSYNC signal, 0/1 for LOW/HIGH
> + respectively. Default is active high.
> + - vsync-active: active state of the VSYNC signal, 0/1 for LOW/HIGH
> + respectively. Default is active high.
> +
> + If both HSYNC and VSYNC polarities are not specified, embedded
> + synchronization is selected.
No need to copy-n-paste from video-interfaces.txt. Just "see
video-interfaces.txt" for the description is fine.
> +
> - port 1 - sub-nodes describing one or more endpoints connected to
> the VIN from local SoC CSI-2 receivers. The endpoint numbers must
> use the following schema.
> @@ -62,6 +72,8 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
> - Endpoint 2 - sub-node describing the endpoint connected to CSI40
> - Endpoint 3 - sub-node describing the endpoint connected to CSI41
>
> + Endpoint nodes of port at 1 do not support any optional endpoint property.
> +
> Device node example for Gen2 platforms
> --------------------------------------
>
> @@ -112,7 +124,6 @@ Board setup example for Gen2 platforms (vin1 composite video input)
>
> vin1ep0: endpoint {
> remote-endpoint = <&adv7180>;
> - bus-width = <8>;
> };
> };
> };
> --
> 2.7.4
>
^ permalink raw reply
* [PATCH 3/5] watchdog: sp805: set WDOG_HW_RUNNING when appropriate
From: Ray Jui @ 2018-05-23 16:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <76d47e02-7a5f-3fc2-3905-cd4aa03ac69c@arm.com>
Hi Robin,
On 5/23/2018 4:48 AM, Robin Murphy wrote:
> On 23/05/18 08:52, Scott Branden wrote:
>>
>>
>> On 18-05-22 04:24 PM, Ray Jui wrote:
>>> Hi Guenter,
>>>
>>> On 5/22/2018 1:54 PM, Guenter Roeck wrote:
>>>> On Tue, May 22, 2018 at 11:47:18AM -0700, Ray Jui wrote:
>>>>> If the watchdog hardware is already enabled during the boot process,
>>>>> when the Linux watchdog driver loads, it should reset the watchdog and
>>>>> tell the watchdog framework. As a result, ping can be generated from
>>>>> the watchdog framework, until the userspace watchdog daemon takes over
>>>>> control
>>>>>
>>>>> Signed-off-by: Ray Jui <ray.jui@broadcom.com>
>>>>> Reviewed-by: Vladimir Olovyannikov
>>>>> <vladimir.olovyannikov@broadcom.com>
>>>>> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
>>>>> ---
>>>>> ? drivers/watchdog/sp805_wdt.c | 22 ++++++++++++++++++++++
>>>>> ? 1 file changed, 22 insertions(+)
>>>>>
>>>>> diff --git a/drivers/watchdog/sp805_wdt.c
>>>>> b/drivers/watchdog/sp805_wdt.c
>>>>> index 1484609..408ffbe 100644
>>>>> --- a/drivers/watchdog/sp805_wdt.c
>>>>> +++ b/drivers/watchdog/sp805_wdt.c
>>>>> @@ -42,6 +42,7 @@
>>>>> ????? /* control register masks */
>>>>> ????? #define??? INT_ENABLE??? (1 << 0)
>>>>> ????? #define??? RESET_ENABLE??? (1 << 1)
>>>>> +??? #define??? ENABLE_MASK??? (INT_ENABLE | RESET_ENABLE)
>>>>> ? #define WDTINTCLR??????? 0x00C
>>>>> ? #define WDTRIS??????????? 0x010
>>>>> ? #define WDTMIS??????????? 0x014
>>>>> @@ -74,6 +75,18 @@ module_param(nowayout, bool, 0);
>>>>> ? MODULE_PARM_DESC(nowayout,
>>>>> ????????? "Set to 1 to keep watchdog running after device release");
>>>>> ? +/* returns true if wdt is running; otherwise returns false */
>>>>> +static bool wdt_is_running(struct watchdog_device *wdd)
>>>>> +{
>>>>> +??? struct sp805_wdt *wdt = watchdog_get_drvdata(wdd);
>>>>> +
>>>>> +??? if ((readl_relaxed(wdt->base + WDTCONTROL) & ENABLE_MASK) ==
>>>>> +??????? ENABLE_MASK)
>>>>> +??????? return true;
>>>>> +??? else
>>>>> +??????? return false;
>>>>
>>>> ????return !!(readl_relaxed(wdt->base + WDTCONTROL) & ENABLE_MASK));
>>>>
>>>
>>> Note ENABLE_MASK contains two bits (INT_ENABLE and RESET_ENABLE);
>>> therefore, a simple !!(expression) would not work? That is, the
>>> masked result needs to be compared against the mask again to ensure
>>> both bits are set, right?
>> Ray - your original code looks correct to me.? Easier to read and less
>> prone to errors as shown in the attempted translation to a single
>> statement.
>
> ????if (<boolean condition>)
> ??????? return true;
> ????else
> ??????? return false;
>
> still looks really dumb, though, and IMO is actually harder to read than
> just "return <boolean condition>;" because it forces you to stop and
> double-check that the logic is, in fact, only doing the obvious thing.
If you can propose a way to modify my original code above to make it
more readable, I'm fine to make the change.
As I mentioned, I don't think the following change proposed by Guenter
will work due to the reason I pointed out:
return !!(readl_relaxed(wdt->base + WDTCONTROL) & ENABLE_MASK));
>
> Robin.
>
>
>
> p.s. No thanks for making me remember the mind-boggling horror of
> briefly maintaining part of this legacy codebase... :P
>
> $ grep -r '? true : false' --include=*.cpp . | wc -l
> 951
^ permalink raw reply
* [PATCH 1/5] Documentation: DT: Add optional 'timeout-sec' property for sp805
From: Ray Jui @ 2018-05-23 16:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <af81ea74-fb80-11e2-7bdc-d3607bdbd46b@arm.com>
On 5/23/2018 3:57 AM, Robin Murphy wrote:
> On 22/05/18 19:47, Ray Jui wrote:
>> Update the SP805 binding document to add optional 'timeout-sec'
>> devicetree property
>>
>> Signed-off-by: Ray Jui <ray.jui@broadcom.com>
>> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
>> ---
>> ? Documentation/devicetree/bindings/watchdog/sp805-wdt.txt | 2 ++
>> ? 1 file changed, 2 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
>> b/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
>> index edc4f0e..f898a86 100644
>> --- a/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
>> +++ b/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
>> @@ -19,6 +19,8 @@ Required properties:
>> ? Optional properties:
>> ? - interrupts : Should specify WDT interrupt number.
>> +- timeout-sec : Should specify default WDT timeout in seconds. If
>> unset, the
>> +??????????????? default timeout is 30 seconds
>
> According to the SP805 TRM, the default interval is dependent on the
> rate of WDOGCLK, but would typically be a lot longer than that :/
>
> On a related note, anyone have any idea why we seem to have two subtly
> different SP805 bindings defined?
Interesting, I did not even know that until you pointed this out (and
it's funny that I found that I actually reviewed arm,sp805.txt
internally in Broadcom code review).
It looks like one was done by Bhupesh Sharma (sp805-wdt.txt) and the
other was done by Anup Patel (arm,sp805.txt). Both were merged at the
same time around March 20, 2016: 915c56bc01d6. I'd assume both were sent
out at around the same time.
It sounds like we should definitely remove one of them. Given that
sp805-wdt.txt appears to have more detailed descriptions on the use of
the clocks, should we remove arm,sp805.txt?
Thanks,
Ray
>
> Robin.
>
>> ? Examples:
>>
^ permalink raw reply
* [PATCH v3 12/13] dt-bindings: add binding for px30 power domains
From: Rob Herring @ 2018-05-23 16:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1527058371-10706-1-git-send-email-zhangqing@rock-chips.com>
On Wed, May 23, 2018 at 02:52:51PM +0800, Elaine Zhang wrote:
> From: Finley Xiao <finley.xiao@rock-chips.com>
>
> Add binding documentation for the power domains
> found on Rockchip PX30 SoCs.
>
> Signed-off-by: Finley Xiao <finley.xiao@rock-chips.com>
> Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
> ---
> Documentation/devicetree/bindings/soc/rockchip/power_domain.txt | 3 +++
> 1 file changed, 3 insertions(+)
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* [PATCH v3 11/13] dt-bindings: power: add PX30 SoCs header for power-domain
From: Rob Herring @ 2018-05-23 16:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1527058341-10658-1-git-send-email-zhangqing@rock-chips.com>
On Wed, May 23, 2018 at 02:52:21PM +0800, Elaine Zhang wrote:
> From: Finley Xiao <finley.xiao@rock-chips.com>
>
> According to a description from TRM, add all the power domains.
>
> Signed-off-by: Finley Xiao <finley.xiao@rock-chips.com>
> Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
> ---
> include/dt-bindings/power/px30-power.h | 27 +++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
> create mode 100644 include/dt-bindings/power/px30-power.h
Reviewed-by: Rob Herring <robh@kernel.org>
^ 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