* Re: [PATCH v5 10/10] arm64: atomics: Use K constraint when toolchain appears to support it
From: Nick Desaulniers @ 2019-08-29 17:45 UTC (permalink / raw)
To: Will Deacon
Cc: Mark Rutland, Peter Zijlstra, Catalin Marinas, Ard.Biesheuvel,
andrew.murray, Nathan Chancellor, Robin Murphy, Linux ARM
In-Reply-To: <20190829165457.grindfmgpdpsbt4i@willie-the-truck>
On Thu, Aug 29, 2019 at 9:55 AM Will Deacon <will@kernel.org> wrote:
>
> On Thu, Aug 29, 2019 at 04:48:34PM +0100, Will Deacon wrote:
> > diff --git a/arch/arm64/include/asm/atomic_ll_sc.h b/arch/arm64/include/asm/atomic_ll_sc.h
> > index 95091f72228b..7fa042f5444e 100644
> > --- a/arch/arm64/include/asm/atomic_ll_sc.h
> > +++ b/arch/arm64/include/asm/atomic_ll_sc.h
> > @@ -23,6 +23,10 @@ asm_ops "\n" \
> > #define __LL_SC_FALLBACK(asm_ops) asm_ops
> > #endif
> >
> > +#ifndef CONFIG_CC_HAS_K_CONSTRAINT
> > +#define K
> > +#endif
>
> Bah, I need to use something like __stringify when the constraint is used
> in order for this to get expanded properly. Updated diff below.
>
> Will
Hi Will, thanks for cc'ing me on the patch set. I'd be happy to help
test w/ Clang. Would you mind pushing this set with the below diff to
a publicly available tree+branch I can pull from? (I haven't yet
figured out how to download multiple diff's from gmail rather than 1
by 1, and TBH I'd rather just use git).
>
> --->8
>
> diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
> index 61de992bbea3..0cef056b5fb1 100644
> --- a/arch/arm64/Makefile
> +++ b/arch/arm64/Makefile
> @@ -39,6 +39,12 @@ $(warning LSE atomics not supported by binutils)
> endif
> endif
>
> +cc_has_k_constraint := $(call try-run,echo \
> + 'int main(void) { \
> + asm volatile("and w0, w0, %w0" :: "K" (4294967295)); \
> + return 0; \
> + }' | $(CC) -S -x c -o "$$TMP" -,,-DCONFIG_CC_HAS_K_CONSTRAINT=1)
> +
> ifeq ($(CONFIG_ARM64), y)
> brokengasinst := $(call as-instr,1:\n.inst 0\n.rept . - 1b\n\nnop\n.endr\n,,-DCONFIG_BROKEN_GAS_INST=1)
>
> @@ -63,7 +69,8 @@ ifeq ($(CONFIG_GENERIC_COMPAT_VDSO), y)
> endif
> endif
>
> -KBUILD_CFLAGS += -mgeneral-regs-only $(lseinstr) $(brokengasinst) $(compat_vdso)
> +KBUILD_CFLAGS += -mgeneral-regs-only $(lseinstr) $(brokengasinst) \
> + $(compat_vdso) $(cc_has_k_constraint)
> KBUILD_CFLAGS += -fno-asynchronous-unwind-tables
> KBUILD_CFLAGS += $(call cc-disable-warning, psabi)
> KBUILD_AFLAGS += $(lseinstr) $(brokengasinst) $(compat_vdso)
> diff --git a/arch/arm64/include/asm/atomic_ll_sc.h b/arch/arm64/include/asm/atomic_ll_sc.h
> index 95091f72228b..7b012148bfd6 100644
> --- a/arch/arm64/include/asm/atomic_ll_sc.h
> +++ b/arch/arm64/include/asm/atomic_ll_sc.h
> @@ -10,6 +10,8 @@
> #ifndef __ASM_ATOMIC_LL_SC_H
> #define __ASM_ATOMIC_LL_SC_H
>
> +#include <linux/stringify.h>
> +
> #if IS_ENABLED(CONFIG_ARM64_LSE_ATOMICS) && IS_ENABLED(CONFIG_AS_LSE)
> #define __LL_SC_FALLBACK(asm_ops) \
> " b 3f\n" \
> @@ -23,6 +25,10 @@ asm_ops "\n" \
> #define __LL_SC_FALLBACK(asm_ops) asm_ops
> #endif
>
> +#ifndef CONFIG_CC_HAS_K_CONSTRAINT
> +#define K
> +#endif
> +
> /*
> * AArch64 UP and SMP safe atomic ops. We use load exclusive and
> * store exclusive to ensure that these are atomic. We may loop
> @@ -44,7 +50,7 @@ __ll_sc_atomic_##op(int i, atomic_t *v) \
> " stxr %w1, %w0, %2\n" \
> " cbnz %w1, 1b\n") \
> : "=&r" (result), "=&r" (tmp), "+Q" (v->counter) \
> - : #constraint "r" (i)); \
> + : __stringify(constraint) "r" (i)); \
> }
>
> #define ATOMIC_OP_RETURN(name, mb, acq, rel, cl, op, asm_op, constraint)\
> @@ -63,7 +69,7 @@ __ll_sc_atomic_##op##_return##name(int i, atomic_t *v) \
> " cbnz %w1, 1b\n" \
> " " #mb ) \
> : "=&r" (result), "=&r" (tmp), "+Q" (v->counter) \
> - : #constraint "r" (i) \
> + : __stringify(constraint) "r" (i) \
> : cl); \
> \
> return result; \
> @@ -85,7 +91,7 @@ __ll_sc_atomic_fetch_##op##name(int i, atomic_t *v) \
> " cbnz %w2, 1b\n" \
> " " #mb ) \
> : "=&r" (result), "=&r" (val), "=&r" (tmp), "+Q" (v->counter) \
> - : #constraint "r" (i) \
> + : __stringify(constraint) "r" (i) \
> : cl); \
> \
> return result; \
> @@ -113,10 +119,15 @@ ATOMIC_OPS(sub, sub, J)
> ATOMIC_FETCH_OP (_acquire, , a, , "memory", __VA_ARGS__)\
> ATOMIC_FETCH_OP (_release, , , l, "memory", __VA_ARGS__)
>
> -ATOMIC_OPS(and, and, )
> +ATOMIC_OPS(and, and, K)
> +ATOMIC_OPS(or, orr, K)
> +ATOMIC_OPS(xor, eor, K)
> +/*
> + * GAS converts the mysterious and undocumented BIC (immediate) alias to
> + * an AND (immediate) instruction with the immediate inverted. We don't
> + * have a constraint for this, so fall back to register.
> + */
> ATOMIC_OPS(andnot, bic, )
> -ATOMIC_OPS(or, orr, )
> -ATOMIC_OPS(xor, eor, )
>
> #undef ATOMIC_OPS
> #undef ATOMIC_FETCH_OP
> @@ -138,7 +149,7 @@ __ll_sc_atomic64_##op(s64 i, atomic64_t *v) \
> " stxr %w1, %0, %2\n" \
> " cbnz %w1, 1b") \
> : "=&r" (result), "=&r" (tmp), "+Q" (v->counter) \
> - : #constraint "r" (i)); \
> + : __stringify(constraint) "r" (i)); \
> }
>
> #define ATOMIC64_OP_RETURN(name, mb, acq, rel, cl, op, asm_op, constraint)\
> @@ -157,7 +168,7 @@ __ll_sc_atomic64_##op##_return##name(s64 i, atomic64_t *v) \
> " cbnz %w1, 1b\n" \
> " " #mb ) \
> : "=&r" (result), "=&r" (tmp), "+Q" (v->counter) \
> - : #constraint "r" (i) \
> + : __stringify(constraint) "r" (i) \
> : cl); \
> \
> return result; \
> @@ -179,7 +190,7 @@ __ll_sc_atomic64_fetch_##op##name(s64 i, atomic64_t *v) \
> " cbnz %w2, 1b\n" \
> " " #mb ) \
> : "=&r" (result), "=&r" (val), "=&r" (tmp), "+Q" (v->counter) \
> - : #constraint "r" (i) \
> + : __stringify(constraint) "r" (i) \
> : cl); \
> \
> return result; \
> @@ -208,9 +219,14 @@ ATOMIC64_OPS(sub, sub, J)
> ATOMIC64_FETCH_OP (_release,, , l, "memory", __VA_ARGS__)
>
> ATOMIC64_OPS(and, and, L)
> -ATOMIC64_OPS(andnot, bic, )
> ATOMIC64_OPS(or, orr, L)
> ATOMIC64_OPS(xor, eor, L)
> +/*
> + * GAS converts the mysterious and undocumented BIC (immediate) alias to
> + * an AND (immediate) instruction with the immediate inverted. We don't
> + * have a constraint for this, so fall back to register.
> + */
> +ATOMIC64_OPS(andnot, bic, )
>
> #undef ATOMIC64_OPS
> #undef ATOMIC64_FETCH_OP
> @@ -269,7 +285,7 @@ __ll_sc__cmpxchg_case_##name##sz(volatile void *ptr, \
> "2:") \
> : [tmp] "=&r" (tmp), [oldval] "=&r" (oldval), \
> [v] "+Q" (*(u##sz *)ptr) \
> - : [old] #constraint "r" (old), [new] "r" (new) \
> + : [old] __stringify(constraint) "r" (old), [new] "r" (new) \
> : cl); \
> \
> return oldval; \
> @@ -280,21 +296,21 @@ __ll_sc__cmpxchg_case_##name##sz(volatile void *ptr, \
> * handle the 'K' constraint for the value 4294967295 - thus we use no
> * constraint for 32 bit operations.
> */
> -__CMPXCHG_CASE(w, b, , 8, , , , , )
> -__CMPXCHG_CASE(w, h, , 16, , , , , )
> -__CMPXCHG_CASE(w, , , 32, , , , , )
> +__CMPXCHG_CASE(w, b, , 8, , , , , K)
> +__CMPXCHG_CASE(w, h, , 16, , , , , K)
> +__CMPXCHG_CASE(w, , , 32, , , , , K)
> __CMPXCHG_CASE( , , , 64, , , , , L)
> -__CMPXCHG_CASE(w, b, acq_, 8, , a, , "memory", )
> -__CMPXCHG_CASE(w, h, acq_, 16, , a, , "memory", )
> -__CMPXCHG_CASE(w, , acq_, 32, , a, , "memory", )
> +__CMPXCHG_CASE(w, b, acq_, 8, , a, , "memory", K)
> +__CMPXCHG_CASE(w, h, acq_, 16, , a, , "memory", K)
> +__CMPXCHG_CASE(w, , acq_, 32, , a, , "memory", K)
> __CMPXCHG_CASE( , , acq_, 64, , a, , "memory", L)
> -__CMPXCHG_CASE(w, b, rel_, 8, , , l, "memory", )
> -__CMPXCHG_CASE(w, h, rel_, 16, , , l, "memory", )
> -__CMPXCHG_CASE(w, , rel_, 32, , , l, "memory", )
> +__CMPXCHG_CASE(w, b, rel_, 8, , , l, "memory", K)
> +__CMPXCHG_CASE(w, h, rel_, 16, , , l, "memory", K)
> +__CMPXCHG_CASE(w, , rel_, 32, , , l, "memory", K)
> __CMPXCHG_CASE( , , rel_, 64, , , l, "memory", L)
> -__CMPXCHG_CASE(w, b, mb_, 8, dmb ish, , l, "memory", )
> -__CMPXCHG_CASE(w, h, mb_, 16, dmb ish, , l, "memory", )
> -__CMPXCHG_CASE(w, , mb_, 32, dmb ish, , l, "memory", )
> +__CMPXCHG_CASE(w, b, mb_, 8, dmb ish, , l, "memory", K)
> +__CMPXCHG_CASE(w, h, mb_, 16, dmb ish, , l, "memory", K)
> +__CMPXCHG_CASE(w, , mb_, 32, dmb ish, , l, "memory", K)
> __CMPXCHG_CASE( , , mb_, 64, dmb ish, , l, "memory", L)
>
> #undef __CMPXCHG_CASE
> @@ -332,5 +348,6 @@ __CMPXCHG_DBL( , , , )
> __CMPXCHG_DBL(_mb, dmb ish, l, "memory")
>
> #undef __CMPXCHG_DBL
> +#undef K
>
> #endif /* __ASM_ATOMIC_LL_SC_H */
--
Thanks,
~Nick Desaulniers
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] [RFC] tty/serial: imx: make use of format specifier %dE
From: Uwe Kleine-König @ 2019-08-29 17:19 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Jani Nikula, Petr Mladek, open list:SERIAL DRIVERS,
Jonathan Corbet, Greg Kroah-Hartman, Linux Documentation List,
Linux Kernel Mailing List, Steven Rostedt, Enrico Weigelt,
NXP Linux Team, Sascha Hauer, Jiri Slaby, Shawn Guo,
Andrew Morton, Fabio Estevam, linux-arm Mailing List,
Sergey Senozhatsky
In-Reply-To: <CAHp75VeV8jDP1uP3HtkJ+j7+SbkB50cs4V9tJ+j9tS6icO95FQ@mail.gmail.com>
[-- Attachment #1.1.1: Type: text/plain, Size: 1386 bytes --]
On 8/29/19 3:43 PM, Andy Shevchenko wrote:
> On Thu, Aug 29, 2019 at 7:40 AM Uwe Kleine-König <uwe@kleine-koenig.org> wrote:
>>
>> I created a patch that teaches printk et al to emit a symbolic error
>> name for an error valued integer[1]. With that applied
>>
>> dev_err(&pdev->dev, "failed to get ipg clk: %dE\n", ret);
>>
>> emits
>>
>> ... failed to get ipg clk: EPROBE_DEFER
>>
>> if ret is -EPROBE_DEFER. Petr Mladek (i.e. one of the printk
>> maintainers) had concerns if this would be well received and worth the
>> effort. He asked to present it to a few subsystems. So for now, this
>> patch converting the imx UART driver shouldn't be applied yet but it
>> would be great to get some feedback about if you think that being able
>> to easily printk (for example) "EIO" instead of "-5" is a good idea.
>
>> Would it help you? Do you think it helps your users?
>
> No, it makes sense only for debug where the user is supposed to be
> developer and thus needs anyway to know code base better than average.
Would you go so far as to claim that
... failed to get ipg clk: -517
is better sometimes than the same message with a named error? I'd say it
is never better and in some cases worse because readers who don't
understand what -EPROBE_DEFER means won't understand -517 either. So
there is net win.
Best regards
Uwe
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 01/10] KVM: arm64: Document PV-time interface
From: Andrew Jones @ 2019-08-29 17:15 UTC (permalink / raw)
To: Steven Price
Cc: kvm, linux-doc, Marc Zyngier, linux-kernel, Russell King,
Catalin Marinas, Paolo Bonzini, Will Deacon, kvmarm,
linux-arm-kernel
In-Reply-To: <20190821153656.33429-2-steven.price@arm.com>
On Wed, Aug 21, 2019 at 04:36:47PM +0100, Steven Price wrote:
> Introduce a paravirtualization interface for KVM/arm64 based on the
> "Arm Paravirtualized Time for Arm-Base Systems" specification DEN 0057A.
>
> This only adds the details about "Stolen Time" as the details of "Live
> Physical Time" have not been fully agreed.
>
> User space can specify a reserved area of memory for the guest and
> inform KVM to populate the memory with information on time that the host
> kernel has stolen from the guest.
>
> A hypercall interface is provided for the guest to interrogate the
> hypervisor's support for this interface and the location of the shared
> memory structures.
>
> Signed-off-by: Steven Price <steven.price@arm.com>
> ---
> Documentation/virt/kvm/arm/pvtime.txt | 100 ++++++++++++++++++++++++++
> 1 file changed, 100 insertions(+)
> create mode 100644 Documentation/virt/kvm/arm/pvtime.txt
>
> diff --git a/Documentation/virt/kvm/arm/pvtime.txt b/Documentation/virt/kvm/arm/pvtime.txt
> new file mode 100644
> index 000000000000..1ceb118694e7
> --- /dev/null
> +++ b/Documentation/virt/kvm/arm/pvtime.txt
> @@ -0,0 +1,100 @@
> +Paravirtualized time support for arm64
> +======================================
> +
> +Arm specification DEN0057/A defined a standard for paravirtualised time
> +support for AArch64 guests:
> +
> +https://developer.arm.com/docs/den0057/a
> +
> +KVM/arm64 implements the stolen time part of this specification by providing
> +some hypervisor service calls to support a paravirtualized guest obtaining a
> +view of the amount of time stolen from its execution.
> +
> +Two new SMCCC compatible hypercalls are defined:
> +
> +PV_FEATURES 0xC5000020
> +PV_TIME_ST 0xC5000022
> +
> +These are only available in the SMC64/HVC64 calling convention as
> +paravirtualized time is not available to 32 bit Arm guests. The existence of
> +the PV_FEATURES hypercall should be probed using the SMCCC 1.1 ARCH_FEATURES
> +mechanism before calling it.
> +
> +PV_FEATURES
> + Function ID: (uint32) : 0xC5000020
> + PV_func_id: (uint32) : Either PV_TIME_LPT or PV_TIME_ST
> + Return value: (int32) : NOT_SUPPORTED (-1) or SUCCESS (0) if the relevant
> + PV-time feature is supported by the hypervisor.
> +
> +PV_TIME_ST
> + Function ID: (uint32) : 0xC5000022
> + Return value: (int64) : IPA of the stolen time data structure for this
> + (V)CPU. On failure:
Why the () around the V in VCPU?
> + NOT_SUPPORTED (-1)
> +
> +The IPA returned by PV_TIME_ST should be mapped by the guest as normal memory
> +with inner and outer write back caching attributes, in the inner shareable
> +domain. A total of 16 bytes from the IPA returned are guaranteed to be
> +meaningfully filled by the hypervisor (see structure below).
> +
> +PV_TIME_ST returns the structure for the calling VCPU.
The above sentence seems redundant here.
> +
> +Stolen Time
> +-----------
> +
> +The structure pointed to by the PV_TIME_ST hypercall is as follows:
> +
> + Field | Byte Length | Byte Offset | Description
> + ----------- | ----------- | ----------- | --------------------------
> + Revision | 4 | 0 | Must be 0 for version 0.1
> + Attributes | 4 | 4 | Must be 0
> + Stolen time | 8 | 8 | Stolen time in unsigned
> + | | | nanoseconds indicating how
> + | | | much time this VCPU thread
> + | | | was involuntarily not
> + | | | running on a physical CPU.
> +
> +The structure will be updated by the hypervisor prior to scheduling a VCPU. It
> +will be present within a reserved region of the normal memory given to the
> +guest. The guest should not attempt to write into this memory. There is a
> +structure per VCPU of the guest.
> +
> +User space interface
> +====================
> +
> +User space can request that KVM provide the paravirtualized time interface to
> +a guest by creating a KVM_DEV_TYPE_ARM_PV_TIME device, for example:
> +
> + struct kvm_create_device pvtime_device = {
> + .type = KVM_DEV_TYPE_ARM_PV_TIME,
> + .attr = 0,
> + .flags = 0,
> + };
> +
> + pvtime_fd = ioctl(vm_fd, KVM_CREATE_DEVICE, &pvtime_device);
The ioctl doesn't return the fd. If the ioctl returns zero the fd will be
in pvtime_device.fd.
> +
> +Creation of the device should be done after creating the vCPUs of the virtual
> +machine.
Or else what? Will an error be reported in that case?
> +
> +The IPA of the structures must be given to KVM. This is the base address
> +of an array of stolen time structures (one for each VCPU). The base address
> +must be page aligned. The size must be at least 64 * number of VCPUs and be a
> +multiple of PAGE_SIZE.
> +
> +The memory for these structures should be added to the guest in the usual
> +manner (e.g. using KVM_SET_USER_MEMORY_REGION).
Above it says the guest shouldn't attempt to write the memory. Should
KVM_MEM_READONLY be used with KVM_SET_USER_MEMORY_REGION for it?
> +
> +For example:
> +
> + struct kvm_dev_arm_st_region region = {
> + .gpa = <IPA of guest base address>,
> + .size = <size in bytes>
> + };
> +
> + struct kvm_device_attr st_base = {
> + .group = KVM_DEV_ARM_PV_TIME_PADDR,
This is KVM_DEV_ARM_PV_TIME_REGION in the code.
> + .attr = KVM_DEV_ARM_PV_TIME_ST,
> + .addr = (u64)®ion
> + };
> +
> + ioctl(pvtime_fd, KVM_SET_DEVICE_ATTR, &st_base);
> --
> 2.20.1
>
Thanks,
drew
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 0/3] arm64: meson-sm1: add support for the SM1 based VIM3L
From: Kevin Hilman @ 2019-08-29 17:09 UTC (permalink / raw)
To: Neil Armstrong; +Cc: linux-amlogic, linux-kernel, linux-arm-kernel
In-Reply-To: <70d75312-68f0-351c-26b8-0f357721dd9e@baylibre.com>
Neil Armstrong <narmstrong@baylibre.com> writes:
> On 28/08/2019 19:55, Kevin Hilman wrote:
>> Neil Armstrong <narmstrong@baylibre.com> writes:
>>
>>> This patchset adds support for the Amlogic SM1 based Khadas VIM3L variant.
>>>
>>> The S903D3 package variant of SM1 is pin-to-pin compatible with the
>>> S922X and A311d, so only internal DT changes are needed :
>>> - DVFS support is different
>>> - Audio support not yet available for SM1
>>>
>>> This patchset moved all the non-g12b nodes to meson-khadas-vim3.dtsi
>>> and add the sm1 specific nodes into meson-sm1-khadas-vim3l.dts.
>>
>> Reviewed-by: Kevin Hilman <khilman@baylibre.com>
>> Tested-by: Kevin Hilman <khilman@baylibre.com>
>>
>> Basic boot test + suspend/resume test OK on my vim3L (thanks to Khadas
>> for the board!)
>>
>>> Display has a color conversion bug on SM1 by using a more recent vendor
>>> bootloader on the SM1 based VIM3, this is out of scope of this patchset
>>> and will be fixed in the drm/meson driver.
>>>
>>> Dependencies:
>>> - patch 1,2: None
>>> - patch 3: Depends on the "arm64: meson-sm1: add support for DVFS" patchset at [1]
>>
>> I tested in my integ branch where this series is applied, but I'm not
>> seeing any OPPs created (/sys/devices/system/cpu/cpufreq/)
>
> These patches were sent from your integ branch, on top of :
> commit 395df5af4c782ad19fb34b9a2009ca240eeb9749 (khilman-amlogic/v5.4/integ)
> Merge: 2fcc5666dd45 9557737987bb
> Author: Kevin Hilman <khilman@baylibre.com>
> Date: Tue Aug 27 15:39:46 2019 -0700
>
> Merge branch 'v5.4/testing' into tmp/aml-rebuild
>
> Rebuilt and retested, and I get the OPPs just fine :
> # cat /sys/bus/cpu/devices/cpu0/cpufreq/scaling_available_frequencies
> 100000 250000 500000 666666 1000000 1200000 1404000 1500000 1608000 1704000 1800000 1908000
Thanks for confirming.
Indeed, there was an issue with my most recent `integ` branch (it was
missing the driver side of some SM1 clocks.) Fixing that issue, and
retesting this series it all works well.
Queuing for v5.4,
Thanks,
Kevin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 2/3] media: i2c: Add IMX290 CMOS image sensor driver
From: Manivannan Sadhasivam @ 2019-08-29 17:04 UTC (permalink / raw)
To: Sakari Ailus
Cc: devicetree, c.barrett, linux-kernel, a.brela, robh+dt, mchehab,
linux-arm-kernel, linux-media
In-Reply-To: <20190813105920.GH835@valkosipuli.retiisi.org.uk>
Hi Sakari,
Thanks for the review!
On Tue, Aug 13, 2019 at 01:59:20PM +0300, Sakari Ailus wrote:
> Hi Manivannan,
>
> On Tue, Aug 06, 2019 at 06:39:37PM +0530, Manivannan Sadhasivam wrote:
> > Add driver for Sony IMX290 CMOS image sensor driver. The driver only
> > supports I2C interface for programming and MIPI CSI-2 for sensor output.
> >
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > ---
> > drivers/media/i2c/Kconfig | 11 +
> > drivers/media/i2c/Makefile | 1 +
> > drivers/media/i2c/imx290.c | 845 +++++++++++++++++++++++++++++++++++++
> > 3 files changed, 857 insertions(+)
> > create mode 100644 drivers/media/i2c/imx290.c
> >
> > diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> > index cb8db944aa41..256edd289abe 100644
> > --- a/drivers/media/i2c/Kconfig
> > +++ b/drivers/media/i2c/Kconfig
> > @@ -594,6 +594,17 @@ config VIDEO_IMX274
> > This is a V4L2 sensor driver for the Sony IMX274
> > CMOS image sensor.
> >
> > +config VIDEO_IMX290
> > + tristate "Sony IMX290 sensor support"
> > + depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
> > + depends on MEDIA_CAMERA_SUPPORT
> > + help
> > + This is a Video4Linux2 sensor driver for the Sony
> > + IMX290 camera sensor.
> > +
> > + To compile this driver as a module, choose M here: the
> > + module will be called imx290.
> > +
> > config VIDEO_IMX319
> > tristate "Sony IMX319 sensor support"
> > depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
> > diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> > index d8ad9dad495d..490e59070407 100644
> > --- a/drivers/media/i2c/Makefile
> > +++ b/drivers/media/i2c/Makefile
> > @@ -111,6 +111,7 @@ obj-$(CONFIG_VIDEO_TC358743) += tc358743.o
> > obj-$(CONFIG_VIDEO_IMX214) += imx214.o
> > obj-$(CONFIG_VIDEO_IMX258) += imx258.o
> > obj-$(CONFIG_VIDEO_IMX274) += imx274.o
> > +obj-$(CONFIG_VIDEO_IMX290) += imx290.o
> > obj-$(CONFIG_VIDEO_IMX319) += imx319.o
> > obj-$(CONFIG_VIDEO_IMX355) += imx355.o
> > obj-$(CONFIG_VIDEO_ST_MIPID02) += st-mipid02.o
> > diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
> > new file mode 100644
> > index 000000000000..8c50fbd51ca5
> > --- /dev/null
> > +++ b/drivers/media/i2c/imx290.c
> > @@ -0,0 +1,845 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Sony IMX290 CMOS Image Sensor Driver
> > + *
> > + * Copyright (C) 2019 FRAMOS GmbH.
> > + *
> > + * Copyright (C) 2019 Linaro Ltd.
> > + * Author: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/delay.h>
> > +#include <linux/gpio/consumer.h>
> > +#include <linux/i2c.h>
> > +#include <linux/module.h>
> > +#include <linux/regmap.h>
> > +#include <linux/regulator/consumer.h>
> > +#include <media/media-entity.h>
> > +#include <media/v4l2-ctrls.h>
> > +#include <media/v4l2-device.h>
> > +#include <media/v4l2-fwnode.h>
> > +#include <media/v4l2-subdev.h>
> > +
> > +#define IMX290_STANDBY 0x3000
> > +#define IMX290_REGHOLD 0x3001
> > +#define IMX290_XMSTA 0x3002
> > +#define IMX290_GAIN 0x3014
> > +
> > +static const char * const imx290_supply_name[] = {
> > + "vdda",
> > + "vddd",
> > + "vdddo",
> > +};
> > +
> > +#define IMX290_NUM_SUPPLIES ARRAY_SIZE(imx290_supply_name)
> > +
> > +struct imx290_regval {
> > + u16 reg;
> > + u8 val;
> > +};
> > +
> > +struct imx290_mode {
> > + u32 width;
> > + u32 height;
> > + u32 pixel_rate;
> > + u32 link_freq_index;
> > +
> > + const struct imx290_regval *data;
> > + u32 data_size;
> > +};
> > +
> > +struct imx290 {
> > + struct device *dev;
> > + struct clk *xclk;
> > + struct regmap *regmap;
> > +
> > + struct v4l2_subdev sd;
> > + struct v4l2_fwnode_endpoint ep;
> > + struct media_pad pad;
> > + struct v4l2_mbus_framefmt current_format;
> > + const struct imx290_mode *current_mode;
> > +
> > + struct regulator_bulk_data supplies[IMX290_NUM_SUPPLIES];
> > + struct gpio_desc *rst_gpio;
> > +
> > + struct v4l2_ctrl_handler ctrls;
> > + struct v4l2_ctrl *link_freq;
> > + struct v4l2_ctrl *pixel_rate;
> > +
> > + struct mutex lock;
> > +};
> > +
> > +struct imx290_pixfmt {
> > + u32 code;
> > + u32 colorspace;
> > +};
> > +
> > +static const struct imx290_pixfmt imx290_formats[] = {
> > + { MEDIA_BUS_FMT_SRGGB10_1X10, V4L2_COLORSPACE_SRGB, },
>
> I bet you won't use other colorspaces in this driver. Therefore there's
> there's no need to put it in an array.
>
Ack.
> > +};
> > +
> > +static struct regmap_config imx290_regmap_config = {
> > + .reg_bits = 16,
> > + .val_bits = 8,
> > + .cache_type = REGCACHE_RBTREE,
> > +};
> > +
> > +static const struct imx290_regval imx290_global_init_settings[] = {
> > + { 0x3007, 0x00 },
> > + { 0x3009, 0x00 },
> > + { 0x3018, 0x65 },
> > + { 0x3019, 0x04 },
> > + { 0x301a, 0x00 },
> > + { 0x3443, 0x03 },
> > + { 0x3444, 0x20 },
> > + { 0x3445, 0x25 },
> > + { 0x3407, 0x03 },
> > + { 0x303a, 0x0c },
> > + { 0x3040, 0x00 },
> > + { 0x3041, 0x00 },
> > + { 0x303c, 0x00 },
> > + { 0x303d, 0x00 },
> > + { 0x3042, 0x9c },
> > + { 0x3043, 0x07 },
> > + { 0x303e, 0x49 },
> > + { 0x303f, 0x04 },
> > + { 0x304b, 0x0a },
> > + { 0x300f, 0x00 },
> > + { 0x3010, 0x21 },
> > + { 0x3012, 0x64 },
> > + { 0x3016, 0x09 },
> > + { 0x3070, 0x02 },
> > + { 0x3071, 0x11 },
> > + { 0x309b, 0x10 },
> > + { 0x309c, 0x22 },
> > + { 0x30a2, 0x02 },
> > + { 0x30a6, 0x20 },
> > + { 0x30a8, 0x20 },
> > + { 0x30aa, 0x20 },
> > + { 0x30ac, 0x20 },
> > + { 0x30b0, 0x43 },
> > + { 0x3119, 0x9e },
> > + { 0x311c, 0x1e },
> > + { 0x311e, 0x08 },
> > + { 0x3128, 0x05 },
> > + { 0x313d, 0x83 },
> > + { 0x3150, 0x03 },
> > + { 0x317e, 0x00 },
> > + { 0x32b8, 0x50 },
> > + { 0x32b9, 0x10 },
> > + { 0x32ba, 0x00 },
> > + { 0x32bb, 0x04 },
> > + { 0x32c8, 0x50 },
> > + { 0x32c9, 0x10 },
> > + { 0x32ca, 0x00 },
> > + { 0x32cb, 0x04 },
> > + { 0x332c, 0xd3 },
> > + { 0x332d, 0x10 },
> > + { 0x332e, 0x0d },
> > + { 0x3358, 0x06 },
> > + { 0x3359, 0xe1 },
> > + { 0x335a, 0x11 },
> > + { 0x3360, 0x1e },
> > + { 0x3361, 0x61 },
> > + { 0x3362, 0x10 },
> > + { 0x33b0, 0x50 },
> > + { 0x33b2, 0x1a },
> > + { 0x33b3, 0x04 },
> > +};
> > +
> > +static const struct imx290_regval imx290_1080p_settings[] = {
> > + /* mode settings */
> > + { 0x3007, 0x00 },
> > + { 0x303a, 0x0c },
> > + { 0x3414, 0x0a },
> > + { 0x3472, 0x80 },
> > + { 0x3473, 0x07 },
> > + { 0x3418, 0x38 },
> > + { 0x3419, 0x04 },
> > + { 0x3012, 0x64 },
> > + { 0x3013, 0x00 },
> > + { 0x305c, 0x18 },
> > + { 0x305d, 0x03 },
> > + { 0x305e, 0x20 },
> > + { 0x305f, 0x01 },
> > + { 0x315e, 0x1a },
> > + { 0x3164, 0x1a },
> > + { 0x3480, 0x49 },
> > + /* data rate settings */
> > + { 0x3009, 0x01 },
> > + { 0x3405, 0x10 },
> > + { 0x3446, 0x57 },
> > + { 0x3447, 0x00 },
> > + { 0x3448, 0x37 },
> > + { 0x3449, 0x00 },
> > + { 0x344a, 0x1f },
> > + { 0x344b, 0x00 },
> > + { 0x344c, 0x1f },
> > + { 0x344d, 0x00 },
> > + { 0x344e, 0x1f },
> > + { 0x344f, 0x00 },
> > + { 0x3450, 0x77 },
> > + { 0x3451, 0x00 },
> > + { 0x3452, 0x1f },
> > + { 0x3453, 0x00 },
> > + { 0x3454, 0x17 },
> > + { 0x3455, 0x00 },
> > + { 0x301c, 0x98 },
> > + { 0x301d, 0x08 },
> > +};
> > +
> > +static const struct imx290_regval imx290_720p_settings[] = {
> > + /* mode settings */
> > + { 0x3007, 0x10 },
> > + { 0x303a, 0x06 },
> > + { 0x3414, 0x04 },
> > + { 0x3472, 0x00 },
> > + { 0x3473, 0x05 },
> > + { 0x3418, 0xd0 },
> > + { 0x3419, 0x02 },
> > + { 0x3012, 0x64 },
> > + { 0x3013, 0x00 },
> > + { 0x305c, 0x20 },
> > + { 0x305d, 0x00 },
> > + { 0x305e, 0x20 },
> > + { 0x305f, 0x01 },
> > + { 0x315e, 0x1a },
> > + { 0x3164, 0x1a },
> > + { 0x3480, 0x49 },
> > + /* data rate settings */
> > + { 0x3009, 0x01 },
> > + { 0x3405, 0x10 },
> > + { 0x3446, 0x4f },
> > + { 0x3447, 0x00 },
> > + { 0x3448, 0x2f },
> > + { 0x3449, 0x00 },
> > + { 0x344a, 0x17 },
> > + { 0x344b, 0x00 },
> > + { 0x344c, 0x17 },
> > + { 0x344d, 0x00 },
> > + { 0x344e, 0x17 },
> > + { 0x344f, 0x00 },
> > + { 0x3450, 0x57 },
> > + { 0x3451, 0x00 },
> > + { 0x3452, 0x17 },
> > + { 0x3453, 0x00 },
> > + { 0x3454, 0x17 },
> > + { 0x3455, 0x00 },
> > + { 0x301c, 0xe4 },
> > + { 0x301d, 0x0c },
> > +};
> > +
> > +static const struct imx290_regval imx290_10bit_settings[] = {
> > + { 0x3005, 0x00},
> > + { 0x3046, 0x00},
> > + { 0x3129, 0x1d},
> > + { 0x317c, 0x12},
> > + { 0x31ec, 0x37},
> > + { 0x3441, 0x0a},
> > + { 0x3442, 0x0a},
> > + { 0x300a, 0x3c},
> > + { 0x300b, 0x00},
> > +};
> > +
> > +/* supported link frequencies */
> > +static const s64 imx290_link_freq[] = {
> > + 445500000,
> > +};
> > +
> > +/* Mode configs */
> > +static const struct imx290_mode imx290_modes[] = {
> > + {
> > + .width = 1920,
> > + .height = 1080,
> > + .data = imx290_1080p_settings,
> > + .data_size = ARRAY_SIZE(imx290_1080p_settings),
> > + .pixel_rate = 178200000,
> > + .link_freq_index = 0,
> > + },
> > + {
> > + .width = 1280,
> > + .height = 720,
> > + .data = imx290_720p_settings,
> > + .data_size = ARRAY_SIZE(imx290_720p_settings),
> > + .pixel_rate = 178200000,
> > + .link_freq_index = 0,
> > + },
> > +};
> > +
> > +static inline struct imx290 *to_imx290(struct v4l2_subdev *_sd)
> > +{
> > + return container_of(_sd, struct imx290, sd);
> > +}
> > +
> > +static inline int imx290_read_reg(struct imx290 *imx290, u16 addr, u8 *value)
> > +{
> > + u32 regval;
> > + int ret;
> > +
> > + ret = regmap_read(imx290->regmap, addr, ®val);
> > + if (ret) {
> > + dev_err(imx290->dev, "I2C read failed for addr: %x\n", addr);
> > + return ret;
> > + }
> > +
> > + *value = regval & 0xFF;
>
> Lower case hexadecimals are preferred.
>
Ack.
> > +
> > + return 0;
> > +}
> > +
> > +static int imx290_write_reg(struct imx290 *imx290, u16 addr, u8 value)
> > +{
> > + int ret;
> > +
> > + ret = regmap_write(imx290->regmap, addr, value);
> > + if (ret) {
> > + dev_err(imx290->dev, "I2C write failed for addr: %x\n", addr);
> > + return ret;
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +static int imx290_set_register_array(struct imx290 *imx290,
> > + const struct imx290_regval *settings,
> > + unsigned int num_settings)
> > +{
> > + unsigned int i;
> > + int ret;
> > +
> > + for (i = 0; i < num_settings; ++i, ++settings) {
> > + ret = imx290_write_reg(imx290, settings->reg, settings->val);
> > + if (ret < 0)
> > + return ret;
> > +
> > + /* Settle time is 10ms for all registers */
> > + msleep(10);
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int imx290_write_buffered_reg(struct imx290 *imx290, u16 address_low,
> > + u8 nr_regs, u32 value)
> > +{
> > + int ret, i;
>
> unsigned int i
>
okay.
> > + u8 val = 0;
> > +
> > + ret = imx290_write_reg(imx290, IMX290_REGHOLD, 0x01);
> > + if (ret) {
> > + dev_err(imx290->dev, "Error setting hold register\n");
> > + return ret;
> > + }
> > +
> > + for (i = 0; i < nr_regs; i++) {
> > + imx290_read_reg(imx290, address_low + i, &val);
>
> What's the purpose of reading back val, and doing the same again below?
>
> You could do this in a much more simple way in a single write, see e.g.
> ov5670_write_reg().
>
Right, this came from the vendor driver but looks like we don't need to read
before and after. Will remove the read calls.
> > + ret = imx290_write_reg(imx290, address_low + i,
> > + (u8)(value >> (i * 8)));
> > + if (ret) {
> > + dev_err(imx290->dev, "Error writing buffered registers\n");
> > + return ret;
> > + }
> > + imx290_read_reg(imx290, address_low + i, &val);
> > + }
> > +
> > + ret = imx290_write_reg(imx290, IMX290_REGHOLD, 0x00);
> > + if (ret) {
> > + dev_err(imx290->dev, "Error setting hold register\n");
> > + return ret;
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +static int imx290_set_gain(struct imx290 *imx290, u32 value)
> > +{
> > + int ret;
> > +
> > + u32 adjusted_value = (value * 10) / 3;
> > +
> > + ret = imx290_write_buffered_reg(imx290, IMX290_GAIN, 1, adjusted_value);
> > + if (ret)
> > + dev_err(imx290->dev, "Unable to write gain\n");
> > +
> > + return ret;
> > +}
> > +
> > +static int imx290_set_power_on(struct imx290 *imx290)
> > +{
> > + int ret;
> > +
> > + ret = clk_prepare_enable(imx290->xclk);
> > + if (ret) {
> > + dev_err(imx290->dev, "Failed to enable clock\n");
> > + return ret;
> > + }
> > +
> > + ret = regulator_bulk_enable(IMX290_NUM_SUPPLIES,
> > + imx290->supplies);
> > + if (ret) {
> > + dev_err(imx290->dev, "Failed to enable regulators\n");
> > + goto xclk_off;
> > + }
> > +
> > + usleep_range(1, 2);
> > + gpiod_set_value_cansleep(imx290->rst_gpio, 1);
> > + usleep_range(30000, 31000);
> > +
> > + return 0;
> > +
> > +xclk_off:
> > + clk_disable_unprepare(imx290->xclk);
> > + return ret;
> > +}
> > +
> > +/* Stop streaming */
> > +static int imx290_stop_streaming(struct imx290 *imx290)
> > +{
> > + int ret;
> > +
> > + ret = imx290_write_reg(imx290, IMX290_STANDBY, 0x01);
> > + if (ret < 0)
> > + return ret;
> > +
> > + msleep(30);
> > +
> > + return imx290_write_reg(imx290, IMX290_XMSTA, 0x01);
> > +}
> > +
> > +static void imx290_set_power_off(struct imx290 *imx290)
> > +{
> > + clk_disable_unprepare(imx290->xclk);
> > + gpiod_set_value_cansleep(imx290->rst_gpio, 0);
> > + regulator_bulk_disable(IMX290_NUM_SUPPLIES, imx290->supplies);
> > +}
> > +
> > +static int imx290_s_power(struct v4l2_subdev *sd, int on)
> > +{
> > + struct imx290 *imx290 = to_imx290(sd);
> > + int ret = 0;
> > +
> > + mutex_lock(&imx290->lock);
> > +
> > + if (on) {
> > + ret = imx290_set_power_on(imx290);
> > + if (ret < 0)
> > + goto exit;
> > +
> > + ret = imx290_set_register_array(imx290,
> > + imx290_global_init_settings,
> > + ARRAY_SIZE(imx290_global_init_settings));
> > + if (ret < 0) {
> > + dev_err(imx290->dev,
> > + "Could not set init registers\n");
> > + imx290_set_power_off(imx290);
> > + goto exit;
> > + }
> > +
> > + imx290_stop_streaming(imx290);
> > + } else {
> > + imx290_set_power_off(imx290);
> > + }
> > +
> > +exit:
> > + mutex_unlock(&imx290->lock);
> > +
> > + return ret;
>
> Please use runtime PM instead. It simplifies the driver. See e.g.
> drivers/media/i2c/ov5670.c or drivers/media/i2c/ov5695.c .
>
Ack.
> > +}
> > +
> > +static int imx290_set_ctrl(struct v4l2_ctrl *ctrl)
> > +{
> > + struct imx290 *imx290 = container_of(ctrl->handler,
> > + struct imx290, ctrls);
> > + int ret = 0;
> > +
> > + mutex_lock(&imx290->lock);
> > +
> > + switch (ctrl->id) {
> > + case V4L2_CID_GAIN:
> > + ret = imx290_set_gain(imx290, ctrl->val);
> > + break;
> > + default:
> > + dev_info(imx290->dev, "ctrl(id:0x%x,val:0x%x) is not handled",
> > + ctrl->id, ctrl->val);
>
> Please return an error; I'd do it instead of printing that.
>
Okay.
> > + break;
> > + }
> > +
> > + mutex_unlock(&imx290->lock);
> > +
> > + return ret;
> > +}
> > +
> > +static const struct v4l2_ctrl_ops imx290_ctrl_ops = {
> > + .s_ctrl = imx290_set_ctrl,
> > +};
> > +
> > +static int imx290_enum_mbus_code(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_pad_config *cfg,
> > + struct v4l2_subdev_mbus_code_enum *code)
> > +{
> > + if (code->index >= ARRAY_SIZE(imx290_formats))
> > + return -EINVAL;
> > +
> > + code->code = imx290_formats[code->index].code;
> > +
> > + return 0;
> > +}
> > +
> > +static int imx290_get_fmt(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_pad_config *cfg,
> > + struct v4l2_subdev_format *fmt)
> > +{
> > + struct imx290 *imx290 = to_imx290(sd);
> > + struct v4l2_mbus_framefmt *framefmt;
> > +
> > + if (fmt->which == V4L2_SUBDEV_FORMAT_TRY)
> > + framefmt = v4l2_subdev_get_try_format(&imx290->sd, cfg,
> > + fmt->pad);
> > + else
> > + framefmt = &imx290->current_format;
> > +
> > + fmt->format = *framefmt;
> > +
> > + return 0;
> > +}
> > +
> > +static int imx290_set_fmt(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_pad_config *cfg,
> > + struct v4l2_subdev_format *fmt)
> > +{
> > + struct imx290 *imx290 = to_imx290(sd);
> > + const struct imx290_mode *mode;
> > + struct v4l2_mbus_framefmt *format;
> > + int i, ret = 0;
>
> Note that sub-device drivers need to serialise access through the uAPI to
> their own data.
>
You mean guarding with mutex?
> > +
> > + mode = v4l2_find_nearest_size(imx290_modes,
> > + ARRAY_SIZE(imx290_modes),
> > + width, height,
> > + fmt->format.width, fmt->format.height);
> > +
> > + fmt->format.width = mode->width;
> > + fmt->format.height = mode->height;
> > +
> > + for (i = 0; i < ARRAY_SIZE(imx290_formats); i++)
> > + if (imx290_formats[i].code == fmt->format.code)
> > + break;
> > +
> > + if (i >= ARRAY_SIZE(imx290_formats))
> > + i = 0;
> > +
> > + fmt->format.code = imx290_formats[i].code;
> > + fmt->format.colorspace = imx290_formats[i].colorspace;
> > + fmt->format.field = V4L2_FIELD_NONE;
> > +
> > + if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
> > + format = v4l2_subdev_get_try_format(sd, cfg, fmt->pad);
> > + } else {
> > + format = &imx290->current_format;
> > + __v4l2_ctrl_s_ctrl(imx290->link_freq, mode->link_freq_index);
> > + __v4l2_ctrl_s_ctrl_int64(imx290->pixel_rate, mode->pixel_rate);
> > +
> > + imx290->current_mode = mode;
> > + }
> > +
> > + *format = fmt->format;
> > +
> > + return ret;
> > +}
> > +
> > +static int imx290_entity_init_cfg(struct v4l2_subdev *subdev,
> > + struct v4l2_subdev_pad_config *cfg)
> > +{
> > + struct v4l2_subdev_format fmt = { 0 };
> > +
> > + fmt.which = cfg ? V4L2_SUBDEV_FORMAT_TRY : V4L2_SUBDEV_FORMAT_ACTIVE;
> > + fmt.format.width = 1920;
> > + fmt.format.height = 1080;
> > +
> > + imx290_set_fmt(subdev, cfg, &fmt);
> > +
> > + return 0;
> > +}
> > +
> > +static int imx290_write_current_format(struct imx290 *imx290,
> > + struct v4l2_mbus_framefmt *format)
> > +{
> > + int ret;
> > +
> > + switch (format->code) {
> > + case MEDIA_BUS_FMT_SRGGB10_1X10:
> > + ret = imx290_set_register_array(imx290, imx290_10bit_settings,
> > + ARRAY_SIZE(imx290_10bit_settings));
> > + if (ret < 0) {
> > + dev_err(imx290->dev, "Could not set format registers\n");
> > + return ret;
> > + }
> > + break;
> > + default:
> > + dev_err(imx290->dev, "Unknown pixel format\n");
> > + return -EINVAL;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +/* Start streaming */
> > +static int imx290_start_streaming(struct imx290 *imx290)
> > +{
> > + int ret;
> > +
> > + /* Set current frame format */
> > + ret = imx290_write_current_format(imx290, &imx290->current_format);
> > + if (ret < 0) {
> > + dev_err(imx290->dev, "Could not set frame format\n");
> > + return ret;
> > + }
> > +
> > + /* Apply default values of current mode */
> > + ret = imx290_set_register_array(imx290, imx290->current_mode->data,
> > + imx290->current_mode->data_size);
> > + if (ret < 0) {
> > + dev_err(imx290->dev, "Could not set current mode\n");
> > + return ret;
> > + }
> > +
> > + /* Apply customized values from user */
> > + ret = v4l2_ctrl_handler_setup(imx290->sd.ctrl_handler);
> > + if (ret) {
> > + dev_err(imx290->dev, "Could not sync v4l2 controls\n");
> > + return ret;
> > + }
> > +
> > + ret = imx290_write_reg(imx290, IMX290_STANDBY, 0x00);
> > + if (ret < 0)
> > + return ret;
> > +
> > + msleep(30);
> > +
> > + /* Start streaming */
> > + return imx290_write_reg(imx290, IMX290_XMSTA, 0x00);
> > +}
> > +
> > +static int imx290_set_stream(struct v4l2_subdev *sd, int enable)
> > +{
> > + struct imx290 *imx290 = to_imx290(sd);
> > + int ret = 0;
> > +
> > + if (enable)
> > + ret = imx290_start_streaming(imx290);
> > + else
> > + imx290_stop_streaming(imx290);
> > +
> > + return ret;
> > +}
> > +
> > +static int imx290_get_regulators(struct device *dev, struct imx290 *imx290)
> > +{
> > + unsigned int i;
> > +
> > + for (i = 0; i < IMX290_NUM_SUPPLIES; i++)
> > + imx290->supplies[i].supply = imx290_supply_name[i];
> > +
> > + return devm_regulator_bulk_get(dev, IMX290_NUM_SUPPLIES,
> > + imx290->supplies);
> > +}
> > +
> > +static const struct v4l2_subdev_core_ops imx290_subdev_core_ops = {
> > + .s_power = imx290_s_power,
> > +};
> > +
> > +static const struct v4l2_subdev_video_ops imx290_video_ops = {
> > + .s_stream = imx290_set_stream,
> > +};
> > +
> > +static const struct v4l2_subdev_pad_ops imx290_pad_ops = {
> > + .init_cfg = imx290_entity_init_cfg,
> > + .enum_mbus_code = imx290_enum_mbus_code,
> > + .get_fmt = imx290_get_fmt,
> > + .set_fmt = imx290_set_fmt,
> > +};
> > +
> > +static const struct v4l2_subdev_ops imx290_subdev_ops = {
> > + .core = &imx290_subdev_core_ops,
> > + .video = &imx290_video_ops,
> > + .pad = &imx290_pad_ops,
> > +};
> > +
> > +static const struct media_entity_operations imx290_subdev_entity_ops = {
> > + .link_validate = v4l2_subdev_link_validate,
> > +};
> > +
> > +static int imx290_probe(struct i2c_client *client,
> > + const struct i2c_device_id *id)
> > +{
> > + struct device *dev = &client->dev;
> > + struct fwnode_handle *endpoint;
> > + struct imx290 *imx290;
> > + u32 xclk_freq;
> > + int ret;
> > +
> > + imx290 = devm_kzalloc(dev, sizeof(*imx290), GFP_KERNEL);
> > + if (!imx290)
> > + return -ENOMEM;
> > +
> > + imx290->dev = dev;
> > + imx290->regmap = devm_regmap_init_i2c(client, &imx290_regmap_config);
> > + if (IS_ERR(imx290->regmap)) {
> > + dev_err(dev, "Unable to initialize I2C\n");
> > + return -ENODEV;
> > + }
> > +
> > + endpoint = fwnode_graph_get_next_endpoint(dev_fwnode(dev), NULL);
> > + if (!endpoint) {
> > + dev_err(dev, "Endpoint node not found\n");
> > + return -EINVAL;
> > + }
> > +
> > + ret = v4l2_fwnode_endpoint_parse(endpoint, &imx290->ep);
> > + fwnode_handle_put(endpoint);
> > + if (ret < 0) {
> > + dev_err(dev, "Parsing endpoint node failed\n");
> > + return ret;
> > + }
> > +
> > + /* Only CSI2 is supported for now */
> > + if (imx290->ep.bus_type != V4L2_MBUS_CSI2_DPHY) {
> > + dev_err(dev, "Unsupported bus type, should be CSI2\n");
> > + return -EINVAL;
> > + }
> > +
> > + /* Set default mode to max resolution */
> > + imx290->current_mode = &imx290_modes[0];
> > +
> > + /* get system clock (xclk) */
> > + imx290->xclk = devm_clk_get(dev, "xclk");
> > + if (IS_ERR(imx290->xclk)) {
> > + dev_err(dev, "Could not get xclk");
> > + return PTR_ERR(imx290->xclk);
> > + }
> > +
> > + ret = of_property_read_u32(dev->of_node, "clock-frequency", &xclk_freq);
> > + if (ret) {
> > + dev_err(dev, "Could not get xclk frequency\n");
> > + return ret;
> > + }
> > +
> > + /* external clock must be 37.125 MHz */
> > + if (xclk_freq != 37125000) {
> > + dev_err(dev, "External clock frequency %u is not supported\n",
> > + xclk_freq);
> > + return -EINVAL;
> > + }
> > +
> > + ret = clk_set_rate(imx290->xclk, xclk_freq);
> > + if (ret) {
> > + dev_err(dev, "Could not set xclk frequency\n");
> > + return ret;
> > + }
> > +
> > + ret = imx290_get_regulators(dev, imx290);
> > + if (ret < 0) {
> > + dev_err(dev, "Cannot get regulators\n");
> > + return ret;
> > + }
> > +
> > + imx290->rst_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_ASIS);
> > + if (IS_ERR(imx290->rst_gpio)) {
> > + dev_err(dev, "Cannot get reset gpio\n");
>
> Remember to put the regulators from now on. Or grab them later.
>
Shouldn't that happen by default with devm_regulator* APIs?
> > + return PTR_ERR(imx290->rst_gpio);
> > + }
> > +
> > + mutex_init(&imx290->lock);
> > +
> > + v4l2_ctrl_handler_init(&imx290->ctrls, 3);
> > +
> > + v4l2_ctrl_new_std(&imx290->ctrls, &imx290_ctrl_ops,
> > + V4L2_CID_GAIN, 0, 72, 1, 0);
> > + imx290->link_freq = v4l2_ctrl_new_int_menu(&imx290->ctrls,
> > + &imx290_ctrl_ops,
> > + V4L2_CID_LINK_FREQ,
> > + ARRAY_SIZE(imx290_link_freq) - 1,
> > + 0, imx290_link_freq);
> > + if (imx290->link_freq)
> > + imx290->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY;
> > +
> > + imx290->pixel_rate = v4l2_ctrl_new_std(&imx290->ctrls, &imx290_ctrl_ops,
> > + V4L2_CID_PIXEL_RATE, 1,
> > + INT_MAX, 1,
> > + imx290_modes[0].pixel_rate);
> > +
> > + imx290->sd.ctrl_handler = &imx290->ctrls;
> > +
> > + if (imx290->ctrls.error) {
> > + dev_err(dev, "Control initialization error %d\n",
> > + imx290->ctrls.error);
> > + ret = imx290->ctrls.error;
> > + goto free_ctrl;
> > + }
> > +
> > + v4l2_i2c_subdev_init(&imx290->sd, client, &imx290_subdev_ops);
> > + imx290->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> > + imx290->sd.dev = &client->dev;
> > + imx290->sd.entity.ops = &imx290_subdev_entity_ops;
> > + imx290->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
> > +
> > + imx290->pad.flags = MEDIA_PAD_FL_SOURCE;
> > + ret = media_entity_pads_init(&imx290->sd.entity, 1, &imx290->pad);
> > + if (ret < 0) {
> > + dev_err(dev, "Could not register media entity\n");
> > + goto free_ctrl;
> > + }
> > +
> > + ret = v4l2_async_register_subdev(&imx290->sd);
> > + if (ret < 0) {
> > + dev_err(dev, "Could not register v4l2 device\n");
> > + goto free_entity;
> > + }
> > +
> > + return 0;
> > +
> > +free_entity:
> > + media_entity_cleanup(&imx290->sd.entity);
> > +free_ctrl:
> > + v4l2_ctrl_handler_free(&imx290->ctrls);
> > + mutex_destroy(&imx290->lock);
> > +
> > + return ret;
> > +}
> > +
> > +static int imx290_remove(struct i2c_client *client)
> > +{
> > + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> > + struct imx290 *imx290 = to_imx290(sd);
> > +
> > + v4l2_async_unregister_subdev(sd);
> > + media_entity_cleanup(&sd->entity);
> > + v4l2_ctrl_handler_free(sd->ctrl_handler);
> > +
> > + imx290_set_power_off(imx290);
> > + mutex_destroy(&imx290->lock);
> > +
> > + return 0;
> > +}
> > +
> > +static const struct of_device_id imx290_of_match[] = {
> > + { .compatible = "sony,imx290" },
> > + { /* sentinel */ }
> > +};
> > +MODULE_DEVICE_TABLE(of, imx290_of_match);
> > +
> > +static struct i2c_driver imx290_i2c_driver = {
> > + .probe = imx290_probe,
>
> Please use probe_new instead.
>
Ack.
Thanks,
Mani
> > + .remove = imx290_remove,
> > + .driver = {
> > + .name = "imx290",
> > + .of_match_table = of_match_ptr(imx290_of_match),
> > + },
> > +};
> > +
> > +module_i2c_driver(imx290_i2c_driver);
> > +
> > +MODULE_DESCRIPTION("Sony IMX290 CMOS Image Sensor Driver");
> > +MODULE_AUTHOR("FRAMOS GmbH");
> > +MODULE_AUTHOR("Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>");
> > +MODULE_LICENSE("GPL v2");
>
> --
> Kind regards,
>
> Sakari Ailus
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 6/7] media: don't do an unsigned int with a 31 bit shift
From: Kees Cook @ 2019-08-29 17:03 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Kate Stewart, Kamil Debski, Hans Verkuil, Andrzej Hajda,
Lad, Prabhakar, Bluecherry Maintainers, Krzysztof Kozlowski,
Sylwester Nawrocki, Nathan Chancellor, Andrzej Pietrasiewicz,
Anton Sviridenko, Ezequiel Garcia, Andrey Utkin, Antti Palosaari,
Steve Longerbeam, Ismael Luceno, Linux Media Mailing List,
linux-arm-msm, Stanimir Varbanov, Jeongtae Park,
linux-samsung-soc, Jacek Anaszewski, Thomas Gleixner,
Mauro Carvalho Chehab, Mike Isely, linux-arm-kernel, Andy Walls,
Richard Fontana, Greg Kroah-Hartman, Randy Dunlap, Andy Gross,
Tomasz Figa, Paul Kocialkowski, Kyungmin Park, Boris Brezillon,
Kukjin Kim, Sakari Ailus, Colin Ian King
In-Reply-To: <1a78a757b37d2628312e1d56d7a741ba89d42a91.1566502743.git.mchehab+samsung@kernel.org>
On Thu, Aug 22, 2019 at 04:39:33PM -0300, Mauro Carvalho Chehab wrote:
> Doing something like:
>
> i32 foo = 1, bar;
>
> bar = foo << 31;
>
> has an undefined behavior in C, as warned by cppcheck, as we're
> shifting a signed integer.
>
> Instead, force the numbers to be unsigned, in order to solve this
> issue.
I also recommend using the BIT() macro, which does the ULing correctly,
etc.
i.e. instead of:
- keyup = (gpio & ir->mask_keyup) ? 1 << 31 : 0;
+ keyup = (gpio & ir->mask_keyup) ? 1UL << 31 : 0;
use:
- keyup = (gpio & ir->mask_keyup) ? 1 << 31 : 0;
+ keyup = (gpio & ir->mask_keyup) ? BIT(31) : 0;
--
Kees Cook
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v5 10/10] arm64: atomics: Use K constraint when toolchain appears to support it
From: Will Deacon @ 2019-08-29 16:54 UTC (permalink / raw)
To: linux-arm-kernel
Cc: mark.rutland, peterz, catalin.marinas, ndesaulniers,
Ard.Biesheuvel, andrew.murray, natechancellor, robin.murphy
In-Reply-To: <20190829154834.26547-11-will@kernel.org>
On Thu, Aug 29, 2019 at 04:48:34PM +0100, Will Deacon wrote:
> diff --git a/arch/arm64/include/asm/atomic_ll_sc.h b/arch/arm64/include/asm/atomic_ll_sc.h
> index 95091f72228b..7fa042f5444e 100644
> --- a/arch/arm64/include/asm/atomic_ll_sc.h
> +++ b/arch/arm64/include/asm/atomic_ll_sc.h
> @@ -23,6 +23,10 @@ asm_ops "\n" \
> #define __LL_SC_FALLBACK(asm_ops) asm_ops
> #endif
>
> +#ifndef CONFIG_CC_HAS_K_CONSTRAINT
> +#define K
> +#endif
Bah, I need to use something like __stringify when the constraint is used
in order for this to get expanded properly. Updated diff below.
Will
--->8
diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
index 61de992bbea3..0cef056b5fb1 100644
--- a/arch/arm64/Makefile
+++ b/arch/arm64/Makefile
@@ -39,6 +39,12 @@ $(warning LSE atomics not supported by binutils)
endif
endif
+cc_has_k_constraint := $(call try-run,echo \
+ 'int main(void) { \
+ asm volatile("and w0, w0, %w0" :: "K" (4294967295)); \
+ return 0; \
+ }' | $(CC) -S -x c -o "$$TMP" -,,-DCONFIG_CC_HAS_K_CONSTRAINT=1)
+
ifeq ($(CONFIG_ARM64), y)
brokengasinst := $(call as-instr,1:\n.inst 0\n.rept . - 1b\n\nnop\n.endr\n,,-DCONFIG_BROKEN_GAS_INST=1)
@@ -63,7 +69,8 @@ ifeq ($(CONFIG_GENERIC_COMPAT_VDSO), y)
endif
endif
-KBUILD_CFLAGS += -mgeneral-regs-only $(lseinstr) $(brokengasinst) $(compat_vdso)
+KBUILD_CFLAGS += -mgeneral-regs-only $(lseinstr) $(brokengasinst) \
+ $(compat_vdso) $(cc_has_k_constraint)
KBUILD_CFLAGS += -fno-asynchronous-unwind-tables
KBUILD_CFLAGS += $(call cc-disable-warning, psabi)
KBUILD_AFLAGS += $(lseinstr) $(brokengasinst) $(compat_vdso)
diff --git a/arch/arm64/include/asm/atomic_ll_sc.h b/arch/arm64/include/asm/atomic_ll_sc.h
index 95091f72228b..7b012148bfd6 100644
--- a/arch/arm64/include/asm/atomic_ll_sc.h
+++ b/arch/arm64/include/asm/atomic_ll_sc.h
@@ -10,6 +10,8 @@
#ifndef __ASM_ATOMIC_LL_SC_H
#define __ASM_ATOMIC_LL_SC_H
+#include <linux/stringify.h>
+
#if IS_ENABLED(CONFIG_ARM64_LSE_ATOMICS) && IS_ENABLED(CONFIG_AS_LSE)
#define __LL_SC_FALLBACK(asm_ops) \
" b 3f\n" \
@@ -23,6 +25,10 @@ asm_ops "\n" \
#define __LL_SC_FALLBACK(asm_ops) asm_ops
#endif
+#ifndef CONFIG_CC_HAS_K_CONSTRAINT
+#define K
+#endif
+
/*
* AArch64 UP and SMP safe atomic ops. We use load exclusive and
* store exclusive to ensure that these are atomic. We may loop
@@ -44,7 +50,7 @@ __ll_sc_atomic_##op(int i, atomic_t *v) \
" stxr %w1, %w0, %2\n" \
" cbnz %w1, 1b\n") \
: "=&r" (result), "=&r" (tmp), "+Q" (v->counter) \
- : #constraint "r" (i)); \
+ : __stringify(constraint) "r" (i)); \
}
#define ATOMIC_OP_RETURN(name, mb, acq, rel, cl, op, asm_op, constraint)\
@@ -63,7 +69,7 @@ __ll_sc_atomic_##op##_return##name(int i, atomic_t *v) \
" cbnz %w1, 1b\n" \
" " #mb ) \
: "=&r" (result), "=&r" (tmp), "+Q" (v->counter) \
- : #constraint "r" (i) \
+ : __stringify(constraint) "r" (i) \
: cl); \
\
return result; \
@@ -85,7 +91,7 @@ __ll_sc_atomic_fetch_##op##name(int i, atomic_t *v) \
" cbnz %w2, 1b\n" \
" " #mb ) \
: "=&r" (result), "=&r" (val), "=&r" (tmp), "+Q" (v->counter) \
- : #constraint "r" (i) \
+ : __stringify(constraint) "r" (i) \
: cl); \
\
return result; \
@@ -113,10 +119,15 @@ ATOMIC_OPS(sub, sub, J)
ATOMIC_FETCH_OP (_acquire, , a, , "memory", __VA_ARGS__)\
ATOMIC_FETCH_OP (_release, , , l, "memory", __VA_ARGS__)
-ATOMIC_OPS(and, and, )
+ATOMIC_OPS(and, and, K)
+ATOMIC_OPS(or, orr, K)
+ATOMIC_OPS(xor, eor, K)
+/*
+ * GAS converts the mysterious and undocumented BIC (immediate) alias to
+ * an AND (immediate) instruction with the immediate inverted. We don't
+ * have a constraint for this, so fall back to register.
+ */
ATOMIC_OPS(andnot, bic, )
-ATOMIC_OPS(or, orr, )
-ATOMIC_OPS(xor, eor, )
#undef ATOMIC_OPS
#undef ATOMIC_FETCH_OP
@@ -138,7 +149,7 @@ __ll_sc_atomic64_##op(s64 i, atomic64_t *v) \
" stxr %w1, %0, %2\n" \
" cbnz %w1, 1b") \
: "=&r" (result), "=&r" (tmp), "+Q" (v->counter) \
- : #constraint "r" (i)); \
+ : __stringify(constraint) "r" (i)); \
}
#define ATOMIC64_OP_RETURN(name, mb, acq, rel, cl, op, asm_op, constraint)\
@@ -157,7 +168,7 @@ __ll_sc_atomic64_##op##_return##name(s64 i, atomic64_t *v) \
" cbnz %w1, 1b\n" \
" " #mb ) \
: "=&r" (result), "=&r" (tmp), "+Q" (v->counter) \
- : #constraint "r" (i) \
+ : __stringify(constraint) "r" (i) \
: cl); \
\
return result; \
@@ -179,7 +190,7 @@ __ll_sc_atomic64_fetch_##op##name(s64 i, atomic64_t *v) \
" cbnz %w2, 1b\n" \
" " #mb ) \
: "=&r" (result), "=&r" (val), "=&r" (tmp), "+Q" (v->counter) \
- : #constraint "r" (i) \
+ : __stringify(constraint) "r" (i) \
: cl); \
\
return result; \
@@ -208,9 +219,14 @@ ATOMIC64_OPS(sub, sub, J)
ATOMIC64_FETCH_OP (_release,, , l, "memory", __VA_ARGS__)
ATOMIC64_OPS(and, and, L)
-ATOMIC64_OPS(andnot, bic, )
ATOMIC64_OPS(or, orr, L)
ATOMIC64_OPS(xor, eor, L)
+/*
+ * GAS converts the mysterious and undocumented BIC (immediate) alias to
+ * an AND (immediate) instruction with the immediate inverted. We don't
+ * have a constraint for this, so fall back to register.
+ */
+ATOMIC64_OPS(andnot, bic, )
#undef ATOMIC64_OPS
#undef ATOMIC64_FETCH_OP
@@ -269,7 +285,7 @@ __ll_sc__cmpxchg_case_##name##sz(volatile void *ptr, \
"2:") \
: [tmp] "=&r" (tmp), [oldval] "=&r" (oldval), \
[v] "+Q" (*(u##sz *)ptr) \
- : [old] #constraint "r" (old), [new] "r" (new) \
+ : [old] __stringify(constraint) "r" (old), [new] "r" (new) \
: cl); \
\
return oldval; \
@@ -280,21 +296,21 @@ __ll_sc__cmpxchg_case_##name##sz(volatile void *ptr, \
* handle the 'K' constraint for the value 4294967295 - thus we use no
* constraint for 32 bit operations.
*/
-__CMPXCHG_CASE(w, b, , 8, , , , , )
-__CMPXCHG_CASE(w, h, , 16, , , , , )
-__CMPXCHG_CASE(w, , , 32, , , , , )
+__CMPXCHG_CASE(w, b, , 8, , , , , K)
+__CMPXCHG_CASE(w, h, , 16, , , , , K)
+__CMPXCHG_CASE(w, , , 32, , , , , K)
__CMPXCHG_CASE( , , , 64, , , , , L)
-__CMPXCHG_CASE(w, b, acq_, 8, , a, , "memory", )
-__CMPXCHG_CASE(w, h, acq_, 16, , a, , "memory", )
-__CMPXCHG_CASE(w, , acq_, 32, , a, , "memory", )
+__CMPXCHG_CASE(w, b, acq_, 8, , a, , "memory", K)
+__CMPXCHG_CASE(w, h, acq_, 16, , a, , "memory", K)
+__CMPXCHG_CASE(w, , acq_, 32, , a, , "memory", K)
__CMPXCHG_CASE( , , acq_, 64, , a, , "memory", L)
-__CMPXCHG_CASE(w, b, rel_, 8, , , l, "memory", )
-__CMPXCHG_CASE(w, h, rel_, 16, , , l, "memory", )
-__CMPXCHG_CASE(w, , rel_, 32, , , l, "memory", )
+__CMPXCHG_CASE(w, b, rel_, 8, , , l, "memory", K)
+__CMPXCHG_CASE(w, h, rel_, 16, , , l, "memory", K)
+__CMPXCHG_CASE(w, , rel_, 32, , , l, "memory", K)
__CMPXCHG_CASE( , , rel_, 64, , , l, "memory", L)
-__CMPXCHG_CASE(w, b, mb_, 8, dmb ish, , l, "memory", )
-__CMPXCHG_CASE(w, h, mb_, 16, dmb ish, , l, "memory", )
-__CMPXCHG_CASE(w, , mb_, 32, dmb ish, , l, "memory", )
+__CMPXCHG_CASE(w, b, mb_, 8, dmb ish, , l, "memory", K)
+__CMPXCHG_CASE(w, h, mb_, 16, dmb ish, , l, "memory", K)
+__CMPXCHG_CASE(w, , mb_, 32, dmb ish, , l, "memory", K)
__CMPXCHG_CASE( , , mb_, 64, dmb ish, , l, "memory", L)
#undef __CMPXCHG_CASE
@@ -332,5 +348,6 @@ __CMPXCHG_DBL( , , , )
__CMPXCHG_DBL(_mb, dmb ish, l, "memory")
#undef __CMPXCHG_DBL
+#undef K
#endif /* __ASM_ATOMIC_LL_SC_H */
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH V3 2/6] dt-bindings: PCI: tegra: Add PCIe slot supplies regulator entries
From: Thierry Reding @ 2019-08-29 16:41 UTC (permalink / raw)
To: Vidya Sagar
Cc: devicetree, lorenzo.pieralisi, mperttunen, mmaddireddy, kthota,
gustavo.pimentel, linux-kernel, kishon, linux-tegra, robh+dt,
linux-pci, bhelgaas, andrew.murray, digetx, jonathanh,
linux-arm-kernel, sagar.tv
In-Reply-To: <cd106d64-e06c-e7a2-d807-f5f080625363@nvidia.com>
[-- Attachment #1.1: Type: text/plain, Size: 4704 bytes --]
On Thu, Aug 29, 2019 at 08:48:39PM +0530, Vidya Sagar wrote:
> On 8/29/2019 5:33 PM, Thierry Reding wrote:
> > On Wed, Aug 28, 2019 at 10:58:46PM +0530, Vidya Sagar wrote:
> > > Add optional bindings "vpcie3v3-supply" and "vpcie12v-supply" to describe
> > > regulators of a PCIe slot's supplies 3.3V and 12V provided the platform
> > > is designed to have regulator controlled slot supplies.
> > >
> > > Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
> > > ---
> > > V3:
> > > * None
> > >
> > > V2:
> > > * None
> > >
> > > .../devicetree/bindings/pci/nvidia,tegra194-pcie.txt | 8 ++++++++
> > > 1 file changed, 8 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.txt b/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.txt
> > > index 0ac1b867ac24..b739f92da58e 100644
> > > --- a/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.txt
> > > +++ b/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.txt
> > > @@ -104,6 +104,12 @@ Optional properties:
> > > specified in microseconds
> > > - nvidia,aspm-l0s-entrance-latency-us: ASPM L0s entrance latency to be
> > > specified in microseconds
> > > +- vpcie3v3-supply: A phandle to the regulator node that supplies 3.3V to the slot
> > > + if the platform has one such slot. (Ex:- x16 slot owned by C5 controller
> > > + in p2972-0000 platform).
> > > +- vpcie12v-supply: A phandle to the regulator node that supplies 12V to the slot
> > > + if the platform has one such slot. (Ex:- x16 slot owned by C5 controller
> > > + in p2972-0000 platform).
> >
> > There's an ongoing discussion regarding the use of optional power
> > supplies and I'm wondering if we're not abusing this here. Why exactly
> > are these regulators optional?
> I made them optional because, the number and type of supplies typically depend on the
> kind of slot the controller is owning. If it is a CEM slot, then, it needs 3.3V & 12V
> supplies and if it is an M.2 Key-E/M slot, it needs only 3.3V supply. Also, if there are
> on-board PCIe endpoint devices, supplies may vary again from vendor to vendor.
> Considering all these, I made them optional instead of mandatory.
> Also, I agree that regulator framework supplies a dummy regulator if we make them mandatory
> but doesn't supply one, but it does so with a warning print in the log which I feel is
> an unwanted alert and to avoid that one has to supply dummy/fixed regulators which again
> seems an overkill when all of this can be addressed by making slot regulators optional.
Okay. That sounds like a good reason to make these optional indeed.
There is no way for the PCI controller to know exactly which regulators
will be needed. The only case where it is known is that of the regular
PCIe slot where the 3.3 V and 12 V are mandatory. But since it isn't
always a standard PCIe slot that the controller drives, I think optional
is okay in this case.
Thierry
> > The distinction is somewhat subtle, but the other way to look at
> > modelling this in DT is that the supplies are in fact required, but may
> > be connected to an always-on regulator with a fixed voltage. Or in some
> > cases they may also be shorted to ground. In both cases the PCI
> > controller, or rather the slot that the controller connects to, actually
> > "requires" the supplies, it's just that we can get away without
> > describing them because they can't be controlled anyway.
> >
> > Looking at the PCI connector pinout for PCI Express, I do see a bunch of
> > +3.3 V and +12 V pins. To me that indicates that the 3.3 V and 12 V
> > supplies are indeed required for PCI slots. I'm not sure about devices
> > that are directly connected to the PCI controller, though. I'll need to
> > go look at some schematics to get a better understanding of these.
> >
> > Bottom line: I'm wondering if we shouldn't really make these supplies
> > mandatory and in case where we don't care either just leave them away
> > (the regulator framework will supply a dummy regulator in that case) or
> > hook them up to a fixed regulator if that matches the hardware design.
> >
> > Any thoughts?
> >
> > Thierry
> >
> > > Examples:
> > > =========
> > > @@ -156,6 +162,8 @@ Tegra194:
> > > 0xc2000000 0x18 0x00000000 0x18 0x00000000 0x4 0x00000000>; /* prefetchable memory (16GB) */
> > > vddio-pex-ctl-supply = <&vdd_1v8ao>;
> > > + vpcie3v3-supply = <&vdd_3v3_pcie>;
> > > + vpcie12v-supply = <&vdd_12v_pcie>;
> > > phys = <&p2u_hsio_2>, <&p2u_hsio_3>, <&p2u_hsio_4>,
> > > <&p2u_hsio_5>;
> > > --
> > > 2.17.1
> > >
>
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v9 2/3] fdt: add support for rng-seed
From: Kees Cook @ 2019-08-29 16:25 UTC (permalink / raw)
To: Theodore Y. Ts'o, Hsin-Yi Wang, Stephen Boyd,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Russell King, Catalin Marinas, Will Deacon, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, H . Peter Anvin, Paul E . McKenney,
Kate Stewart, David S . Miller, Viresh Kumar, Marek Szyprowski,
Arnd Bergmann, Marc Zyngier, Julien Thierry, Greg Kroah-Hartman,
Wei Li, Anders Roxell, Rob Herring, Aaro Koskinen,
Daniel Thompson, Tim Chen, Rik van Riel, Waiman Long,
Marcelo Tosatti, Peter Zijlstra, Armijn Hemel, Grzegorz Halat,
Len Brown, Shaokun Zhang, Mike Rapoport, Guenter Roeck,
Andrew Morton, Mathieu Desnoyers, Alexey Dobriyan, Yury Norov,
Josh Poimboeuf, Jiri Kosina, Mukesh Ojha, lkml
In-Reply-To: <20190829154505.GB10779@mit.edu>
On Thu, Aug 29, 2019 at 11:45:05AM -0400, Theodore Y. Ts'o wrote:
> I thought about asking for this, but we really want to do this as
> early as possible, so that it can be used by KASLR and other services
KASLR entropy is sent via a separate DT entry, since it's needed before
the kernel is even running.
> that are run super early. Also, whether or not we can trust the
Are there things using the kernel RNG before the stack canary is set up?
I tried to move all of that code as early as possible already.
> bootloader is going to be a system-level thing. This should probably
> be defaulted to off, and only enabled by the system integrator if they
> are 100%, positively sure, that the entire system is one where we can
> trust the source of randomness which the bootloader is using --- or
> for that matter, that the bootloader is trustworthy!
>
> Is it really going to be that useful for a random system administrator
> to be able to flip this on or off from the command line? Hopefully
> there will be an easy way to configure the firmware or the bootloader
> to simply not supply entropy, if for some reason it's not trustworthy.
You have a point about bootloader vs system design. I'm cautiously
convinced we don't need a boot param, but if we CAN put all the entropy
collection in a single place, that'd be nice.
--
Kees Cook
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v9 2/3] fdt: add support for rng-seed
From: Kees Cook @ 2019-08-29 16:22 UTC (permalink / raw)
To: Hsin-Yi Wang
Cc: Kate Stewart, Peter Zijlstra, Catalin Marinas, Mukesh Ojha,
Grzegorz Halat, H . Peter Anvin, Guenter Roeck, Will Deacon,
Marek Szyprowski, Rob Herring, Daniel Thompson, Anders Roxell,
Yury Norov, Marc Zyngier, Russell King, Aaro Koskinen,
Ingo Molnar, Viresh Kumar, Waiman Long, Paul E . McKenney, Wei Li,
Alexey Dobriyan, Julien Thierry, Len Brown, Arnd Bergmann,
Rik van Riel, Stephen Boyd, Shaokun Zhang, Mike Rapoport,
Borislav Petkov, Josh Poimboeuf, Thomas Gleixner,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Theodore Y . Ts'o, Greg Kroah-Hartman, Marcelo Tosatti, lkml,
Armijn Hemel, Jiri Kosina, Mathieu Desnoyers, Andrew Morton,
Tim Chen, David S . Miller
In-Reply-To: <CAJMQK-iDoPxbFUH3JUeJ7SehCptZOnjKZiUoFd1PqLjDdGHujA@mail.gmail.com>
On Thu, Aug 29, 2019 at 06:03:57PM +0800, Hsin-Yi Wang wrote:
> On Thu, Aug 29, 2019 at 1:36 AM Kees Cook <keescook@chromium.org> wrote:
> >
> > Can this please be a boot param (with the default controlled by the
> > CONFIG)? See how CONFIG_RANDOM_TRUST_CPU is wired up...
> >
> > -Kees
> >
>
> Currently rng-seed read and added in setup_arch() -->
> setup_machine_fdt().. -> early_init_dt_scan_chosen(), which is earlier
> than parse_early_param() that initializes early_param.
>
> If we want to set it as a boot param, add_bootloader_randomness() can
> only be called after parse_early_param(). The seed can't be directly
> added to pool after it's read in. We need to store into global
> variable and load it later.
> If this seems okay then I'll add a patch for this. Thanks
This seems like a good idea to me.
>
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -1096,13 +1096,15 @@ static const char *config_cmdline = CONFIG_CMDLINE;
>
> +const void* rng_seed;
> +int rng_seed_len;
These should be __initdata, yes?
> +
> int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
> int depth,
> void *data)
> {
> int l = 0;
> const char *p = NULL;
> char *cmdline = data;
> - const void *rng_seed;
>
> pr_debug("search \"chosen\", depth: %d, uname: %s\n", depth, uname);
>
> @@ -1137,10 +1139,8 @@ int __init early_init_dt_scan_chosen(unsigned
> long node, const char *uname,
>
> pr_debug("Command line is: %s\n", (char*)data);
>
> - rng_seed = of_get_flat_dt_prop(node, "rng-seed", &l);
> - if (rng_seed && l > 0) {
> - add_bootloader_randomness(rng_seed, l); //
> Originally it's added to entropy pool here
> -
> + rng_seed = of_get_flat_dt_prop(node, "rng-seed", &rng_seed_len);
> + if (rng_seed && rng_seed_len > 0) {
> /* try to clear seed so it won't be found. */
>
> diff --git a/include/linux/random.h b/include/linux/random.h
> index 831a002a1882..946840bba7c1 100644
> --- a/include/linux/random.h
> +++ b/include/linux/random.h
> @@ -31,6 +31,15 @@ static inline void add_latent_entropy(void)
> static inline void add_latent_entropy(void) {}
> #endif
>
> +extern const void* rng_seed;
> +extern int rng_seed_len;
> +
> +static inline void add_bootloader_entropy(void)
> +{
> + if (rng_seed && rng_seed_len > 0)
> + add_bootloader_randomness(rng_seed, rng_seed_len);
> +}
And this should be __init
> +
> extern void add_input_randomness(unsigned int type, unsigned int code,
> unsigned int value) __latent_entropy;
> extern void add_interrupt_randomness(int irq, int irq_flags) __latent_entropy;
> diff --git a/init/main.c b/init/main.c
> index 71847af32e4e..f74a8c7b34af 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -645,6 +645,7 @@ asmlinkage __visible void __init start_kernel(void)
> * - adding command line entropy
> */
> rand_initialize();
> + add_bootloader_entropy();
> add_latent_entropy();
> add_device_randomness(command_line, strlen(command_line));
> boot_init_stack_canary();
But yeah, looks reasonable to me.
--
Kees Cook
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] ARM: Emit __gnu_mcount_nc when using Clang 10.0.0 or newer
From: Matthias Kaehlcke @ 2019-08-29 16:20 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Arnd Bergmann, Nick Desaulniers, Russell King, Stefan Agner,
linux-kernel, clang-built-linux, linux-arm-kernel
In-Reply-To: <20190829062635.45609-1-natechancellor@gmail.com>
On Wed, Aug 28, 2019 at 11:26:35PM -0700, Nathan Chancellor wrote:
> Currently, multi_v7_defconfig + CONFIG_FUNCTION_TRACER fails to build
> with clang:
>
> arm-linux-gnueabi-ld: kernel/softirq.o: in function `_local_bh_enable':
> softirq.c:(.text+0x504): undefined reference to `mcount'
> arm-linux-gnueabi-ld: kernel/softirq.o: in function `__local_bh_enable_ip':
> softirq.c:(.text+0x58c): undefined reference to `mcount'
> arm-linux-gnueabi-ld: kernel/softirq.o: in function `do_softirq':
> softirq.c:(.text+0x6c8): undefined reference to `mcount'
> arm-linux-gnueabi-ld: kernel/softirq.o: in function `irq_enter':
> softirq.c:(.text+0x75c): undefined reference to `mcount'
> arm-linux-gnueabi-ld: kernel/softirq.o: in function `irq_exit':
> softirq.c:(.text+0x840): undefined reference to `mcount'
> arm-linux-gnueabi-ld: kernel/softirq.o:softirq.c:(.text+0xa50): more undefined references to `mcount' follow
>
> clang can emit a working mcount symbol, __gnu_mcount_nc, when
> '-meabi gnu' is passed to it. Until r369147 in LLVM, this was
> broken and caused the kernel not to boot because the calling
> convention was not correct. Now that it is fixed, add this to
> the command line when clang is 10.0.0 or newer so everything
> works properly.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/35
> Link: https://bugs.llvm.org/show_bug.cgi?id=33845
> Link: https://github.com/llvm/llvm-project/commit/16fa8b09702378bacfa3d07081afe6b353b99e60
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> ---
> arch/arm/Makefile | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/arch/arm/Makefile b/arch/arm/Makefile
> index c3624ca6c0bc..7b5a26a866fc 100644
> --- a/arch/arm/Makefile
> +++ b/arch/arm/Makefile
> @@ -112,6 +112,12 @@ ifeq ($(CONFIG_ARM_UNWIND),y)
> CFLAGS_ABI +=-funwind-tables
> endif
>
> +ifeq ($(CONFIG_CC_IS_CLANG),y)
> +ifeq ($(shell test $(CONFIG_CLANG_VERSION) -ge 100000; echo $$?),0)
> +CFLAGS_ABI +=-meabi gnu
> +endif
> +endif
Is this also correct/needed when CONFIG_AEABI is not set?
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH ARM64 v4.4 V3 00/44] V4.4 backport of arm64 Spectre patches
From: Mark Rutland @ 2019-08-29 16:18 UTC (permalink / raw)
To: Viresh Kumar
Cc: Julien Thierry, Marc Zyngier, Catalin Marinas, Will Deacon,
stable, mark.brown, Russell King, linux-arm-kernel
In-Reply-To: <cover.1567077734.git.viresh.kumar@linaro.org>
On Thu, Aug 29, 2019 at 05:03:45PM +0530, Viresh Kumar wrote:
> Hello,
Hi Viresh,
> This series backports arm64 spectre patches to v4.4 stable kernel. I
> have started this backport with Mark Rutland's backport of Spectre to
> 4.9 [1] and tried applying the upstream version of them over 4.4 and
> resolved conflicts by checking how they have been resolved in 4.9.
>
> The KVM changes are mostly dropped as the KVM code in v4.4 is quite
> different and it makes backport more complex. This was suggested by the
> ARM team.
I'm reviewing this backport now, but it's going to take me a few days to
get through the entire series, so please don't be surprised if I'm quiet
until the middle of next week.
If I spot any issues I'll reply immediately, but please poke if I
haven't said anything by Wednesday.
Thanks,
Mark.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] ARM: dts: imx7: fix USB controller 'size' parameter
From: Marco Felsch @ 2019-08-29 16:01 UTC (permalink / raw)
To: Fabio Estevam
Cc: Peter Chen, Sascha Hauer, Thomas Schaefer, NXP Linux Team,
Sascha Hauer, Shawn Guo,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <CAOMZO5BOYrpedOSH9fbBZkDj7n2dcVj8cG5h2U=mvw17F9hu5A@mail.gmail.com>
On 19-08-29 12:58, Fabio Estevam wrote:
> Hi Thomas,
>
> [Adding Peter Chen]
>
> On Thu, Aug 29, 2019 at 12:49 PM Thomas Schaefer
> <thomas.schaefer@kontron.com> wrote:
> >
> > Currently the size parameter in the reg property of usbotg and
> > usbh nodes in imx7s and imx7d dts includes is set to 0x200 which
> > is wrong for the i.MX7 CPU. According to reference manual, spacing
> > of USB controller registers is 0x10000 instead.
> >
> > This patch will fix this and set the 'size' to 0x10000.
> >
> > Signed-off-by: Thomas Schaefer <thomas.schaefer@kontron.com>
>
> Thanks for the patch.
>
> I would suggest adding one more sentence in the commit log explaining
> that this size mismatch caused real world problems in U-Boot.
>
> This way it makes clear that this change was not done merely upon
> inspection, but it fixes a real bug instead.
Do we need a fixes tag too?
Regards,
Marco
> Thanks,
>
> Fabio Estevam
>
>
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] ARM: dts: imx7: fix USB controller 'size' parameter
From: Fabio Estevam @ 2019-08-29 15:58 UTC (permalink / raw)
To: Thomas Schaefer
Cc: Peter Chen, Sascha Hauer, NXP Linux Team, Sascha Hauer, Shawn Guo,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <20190829154913.2049-1-thomas.schaefer@kontron.com>
Hi Thomas,
[Adding Peter Chen]
On Thu, Aug 29, 2019 at 12:49 PM Thomas Schaefer
<thomas.schaefer@kontron.com> wrote:
>
> Currently the size parameter in the reg property of usbotg and
> usbh nodes in imx7s and imx7d dts includes is set to 0x200 which
> is wrong for the i.MX7 CPU. According to reference manual, spacing
> of USB controller registers is 0x10000 instead.
>
> This patch will fix this and set the 'size' to 0x10000.
>
> Signed-off-by: Thomas Schaefer <thomas.schaefer@kontron.com>
Thanks for the patch.
I would suggest adding one more sentence in the commit log explaining
that this size mismatch caused real world problems in U-Boot.
This way it makes clear that this change was not done merely upon
inspection, but it fixes a real bug instead.
Thanks,
Fabio Estevam
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH] ARM: dts: imx7: fix USB controller 'size' parameter
From: Thomas Schaefer @ 2019-08-29 15:49 UTC (permalink / raw)
To: thomas.schaefer, shawnguo, s.hauer, kernel, festevam, linux-imx,
linux-arm-kernel
Currently the size parameter in the reg property of usbotg and
usbh nodes in imx7s and imx7d dts includes is set to 0x200 which
is wrong for the i.MX7 CPU. According to reference manual, spacing
of USB controller registers is 0x10000 instead.
This patch will fix this and set the 'size' to 0x10000.
Signed-off-by: Thomas Schaefer <thomas.schaefer@kontron.com>
---
arch/arm/boot/dts/imx7d.dtsi | 2 +-
arch/arm/boot/dts/imx7s.dtsi | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm/boot/dts/imx7d.dtsi b/arch/arm/boot/dts/imx7d.dtsi
index 42528d2812a2..f1b098d28b6e 100644
--- a/arch/arm/boot/dts/imx7d.dtsi
+++ b/arch/arm/boot/dts/imx7d.dtsi
@@ -117,7 +117,7 @@
&aips3 {
usbotg2: usb@30b20000 {
compatible = "fsl,imx7d-usb", "fsl,imx27-usb";
- reg = <0x30b20000 0x200>;
+ reg = <0x30b20000 0x10000>;
interrupts = <GIC_SPI 42 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&clks IMX7D_USB_CTRL_CLK>;
fsl,usbphy = <&usbphynop2>;
diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
index c1a4fff5ceda..9e25fccf33f0 100644
--- a/arch/arm/boot/dts/imx7s.dtsi
+++ b/arch/arm/boot/dts/imx7s.dtsi
@@ -1088,7 +1088,7 @@
usbotg1: usb@30b10000 {
compatible = "fsl,imx7d-usb", "fsl,imx27-usb";
- reg = <0x30b10000 0x200>;
+ reg = <0x30b10000 0x10000>;
interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&clks IMX7D_USB_CTRL_CLK>;
fsl,usbphy = <&usbphynop1>;
@@ -1099,7 +1099,7 @@
usbh: usb@30b30000 {
compatible = "fsl,imx7d-usb", "fsl,imx27-usb";
- reg = <0x30b30000 0x200>;
+ reg = <0x30b30000 0x10000>;
interrupts = <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&clks IMX7D_USB_CTRL_CLK>;
fsl,usbphy = <&usbphynop3>;
--
2.11.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH 5/7] arm64: compat: vdso: Remove unused VDSO_HAS_32BIT_FALLBACK
From: Vincenzo Frascino @ 2019-08-29 15:52 UTC (permalink / raw)
To: Thomas Gleixner
Cc: linux-arch, catalin.marinas, 0x7f454c46, linux-kernel, linux-mips,
paul.burton, luto, salyzyn, will, linux-arm-kernel
In-Reply-To: <alpine.DEB.2.21.1908291420060.1938@nanos.tec.linutronix.de>
On 29/08/2019 13:21, Thomas Gleixner wrote:
> On Thu, 29 Aug 2019, Vincenzo Frascino wrote:
>
>> As a consequence of Commit 623fa33f7bd6 ("lib:vdso: Remove
>
> -ENOSUCH commit ....
>
> Just say:
>
> VDSO_HAS_32BIT_FALLBACK has been removed from the core ....
>
Thanks Thomas, I will fix it in v2.
> Thanks,
>
> tglx
>
--
Regards,
Vincenzo
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 4/7] lib: vdso: Remove VDSO_HAS_32BIT_FALLBACK
From: Vincenzo Frascino @ 2019-08-29 15:51 UTC (permalink / raw)
To: Andy Lutomirski
Cc: linux-arch, Catalin Marinas, Dmitry Safonov, LKML, linux-mips,
Paul Burton, Thomas Gleixner, Mark Salyzyn, Will Deacon,
linux-arm-kernel
In-Reply-To: <CALCETrVprrrR3TSVSAnHfLW4HDQG=gcVrdjmsk6ss6Z3+vKOBg@mail.gmail.com>
On 29/08/2019 16:25, Andy Lutomirski wrote:
> On Thu, Aug 29, 2019 at 4:19 AM Vincenzo Frascino
> <vincenzo.frascino@arm.com> wrote:
>>
>> VDSO_HAS_32BIT_FALLBACK was introduced to address a regression which
>> caused seccomp to deny access to the applications to clock_gettime64()
>> and clock_getres64() because they are not enabled in the existing
>> filters.
>>
>> The purpose of VDSO_HAS_32BIT_FALLBACK was to simplify the conditional
>> implementation of __cvdso_clock_get*time32() variants.
>>
>> Now that all the architectures that support the generic vDSO library
>> have been converted to support the 32 bit fallbacks the conditional
>> can be removed.
>>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> CC: Andy Lutomirski <luto@kernel.org>
>> References: c60a32ea4f45 ("lib/vdso/32: Provide legacy syscall fallbacks")
>> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
>> ---
>> lib/vdso/gettimeofday.c | 10 ----------
>> 1 file changed, 10 deletions(-)
>>
>> diff --git a/lib/vdso/gettimeofday.c b/lib/vdso/gettimeofday.c
>> index a86e89e6dedc..2c4b311c226d 100644
>> --- a/lib/vdso/gettimeofday.c
>> +++ b/lib/vdso/gettimeofday.c
>> @@ -126,13 +126,8 @@ __cvdso_clock_gettime32(clockid_t clock, struct old_timespec32 *res)
>>
>> ret = __cvdso_clock_gettime_common(clock, &ts);
>>
>> -#ifdef VDSO_HAS_32BIT_FALLBACK
>> if (unlikely(ret))
>> return clock_gettime32_fallback(clock, res);
>> -#else
>> - if (unlikely(ret))
>> - ret = clock_gettime_fallback(clock, &ts);
>> -#endif
>>
>> if (likely(!ret)) {
>> res->tv_sec = ts.tv_sec;
>
> I think you could have a little follow-up patch to remove the if
> statement -- by the time you get here, it's guaranteed that ret == 0.
>
Thanks, I will add a new patch that does that to v2 (with a comment).
> --Andy
>
--
Regards,
Vincenzo
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 2/7] lib: vdso: Build 32 bit specific functions in the right context
From: Vincenzo Frascino @ 2019-08-29 15:48 UTC (permalink / raw)
To: Andy Lutomirski
Cc: linux-arch, Catalin Marinas, Dmitry Safonov, LKML, linux-mips,
Paul Burton, Thomas Gleixner, Mark Salyzyn, Will Deacon,
linux-arm-kernel
In-Reply-To: <CALCETrWNbMhYwpsKtutCTW4M7rMmOF0YUy-k1QgGEpY-Gd1xQw@mail.gmail.com>
On 29/08/2019 16:23, Andy Lutomirski wrote:
> On Thu, Aug 29, 2019 at 4:19 AM Vincenzo Frascino
> <vincenzo.frascino@arm.com> wrote:
>>
>> clock_gettime32 and clock_getres_time32 should be compiled only with a
>> 32 bit vdso library.
>>
>> Exclude these symbols when BUILD_VDSO32 is not defined.
>
> Reviewed-by: Andy Lutomirski <luto@kernel.org>
>
> BTW, this is a great patch: it's either correct or it won't build. I
> like patches like that.
>
Thanks :)
--
Regards,
Vincenzo
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v5 09/10] arm64: atomics: Undefine internal macros after use
From: Will Deacon @ 2019-08-29 15:48 UTC (permalink / raw)
To: linux-arm-kernel
Cc: mark.rutland, peterz, catalin.marinas, ndesaulniers, robin.murphy,
Ard.Biesheuvel, andrew.murray, natechancellor, Will Deacon
In-Reply-To: <20190829154834.26547-1-will@kernel.org>
We use a bunch of internal macros when constructing our atomic and
cmpxchg routines in order to save on boilerplate. Avoid exposing these
directly to users of the header files.
Signed-off-by: Will Deacon <will@kernel.org>
---
arch/arm64/include/asm/atomic.h | 7 +++++++
arch/arm64/include/asm/cmpxchg.h | 4 ++++
2 files changed, 11 insertions(+)
diff --git a/arch/arm64/include/asm/atomic.h b/arch/arm64/include/asm/atomic.h
index 7c334337674d..916e5a6d5454 100644
--- a/arch/arm64/include/asm/atomic.h
+++ b/arch/arm64/include/asm/atomic.h
@@ -32,6 +32,7 @@ ATOMIC_OP(atomic_add)
ATOMIC_OP(atomic_and)
ATOMIC_OP(atomic_sub)
+#undef ATOMIC_OP
#define ATOMIC_FETCH_OP(name, op) \
static inline int arch_##op##name(int i, atomic_t *v) \
@@ -54,6 +55,8 @@ ATOMIC_FETCH_OPS(atomic_fetch_sub)
ATOMIC_FETCH_OPS(atomic_add_return)
ATOMIC_FETCH_OPS(atomic_sub_return)
+#undef ATOMIC_FETCH_OP
+#undef ATOMIC_FETCH_OPS
#define ATOMIC64_OP(op) \
static inline void arch_##op(long i, atomic64_t *v) \
@@ -68,6 +71,7 @@ ATOMIC64_OP(atomic64_add)
ATOMIC64_OP(atomic64_and)
ATOMIC64_OP(atomic64_sub)
+#undef ATOMIC64_OP
#define ATOMIC64_FETCH_OP(name, op) \
static inline long arch_##op##name(long i, atomic64_t *v) \
@@ -90,6 +94,9 @@ ATOMIC64_FETCH_OPS(atomic64_fetch_sub)
ATOMIC64_FETCH_OPS(atomic64_add_return)
ATOMIC64_FETCH_OPS(atomic64_sub_return)
+#undef ATOMIC64_FETCH_OP
+#undef ATOMIC64_FETCH_OPS
+
static inline long arch_atomic64_dec_if_positive(atomic64_t *v)
{
return __lse_ll_sc_body(atomic64_dec_if_positive, v);
diff --git a/arch/arm64/include/asm/cmpxchg.h b/arch/arm64/include/asm/cmpxchg.h
index afaba73e0b2c..a1398f2f9994 100644
--- a/arch/arm64/include/asm/cmpxchg.h
+++ b/arch/arm64/include/asm/cmpxchg.h
@@ -129,6 +129,8 @@ __CMPXCHG_CASE(mb_, 16)
__CMPXCHG_CASE(mb_, 32)
__CMPXCHG_CASE(mb_, 64)
+#undef __CMPXCHG_CASE
+
#define __CMPXCHG_DBL(name) \
static inline long __cmpxchg_double##name(unsigned long old1, \
unsigned long old2, \
@@ -143,6 +145,8 @@ static inline long __cmpxchg_double##name(unsigned long old1, \
__CMPXCHG_DBL( )
__CMPXCHG_DBL(_mb)
+#undef __CMPXCHG_DBL
+
#define __CMPXCHG_GEN(sfx) \
static inline unsigned long __cmpxchg##sfx(volatile void *ptr, \
unsigned long old, \
--
2.11.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v5 10/10] arm64: atomics: Use K constraint when toolchain appears to support it
From: Will Deacon @ 2019-08-29 15:48 UTC (permalink / raw)
To: linux-arm-kernel
Cc: mark.rutland, peterz, catalin.marinas, ndesaulniers, robin.murphy,
Ard.Biesheuvel, andrew.murray, natechancellor, Will Deacon
In-Reply-To: <20190829154834.26547-1-will@kernel.org>
The 'K' constraint is a documented AArch64 machine constraint supported
by GCC for matching integer constants that can be used with a 32-bit
logical instruction. Unfortunately, some released compilers erroneously
accept the immediate '4294967295' for this constraint, which is later
refused by GAS at assembly time. This had led us to avoid the use of
the 'K' constraint altogether.
Instead, detect whether the compiler is up to the job when building the
kernel and pass the 'K' constraint to our 32-bit atomic macros when it
appears to be supported.
Signed-off-by: Will Deacon <will@kernel.org>
---
arch/arm64/Makefile | 9 ++++++-
arch/arm64/include/asm/atomic_ll_sc.h | 47 +++++++++++++++++++++++------------
2 files changed, 39 insertions(+), 17 deletions(-)
diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
index 61de992bbea3..0cef056b5fb1 100644
--- a/arch/arm64/Makefile
+++ b/arch/arm64/Makefile
@@ -39,6 +39,12 @@ $(warning LSE atomics not supported by binutils)
endif
endif
+cc_has_k_constraint := $(call try-run,echo \
+ 'int main(void) { \
+ asm volatile("and w0, w0, %w0" :: "K" (4294967295)); \
+ return 0; \
+ }' | $(CC) -S -x c -o "$$TMP" -,,-DCONFIG_CC_HAS_K_CONSTRAINT=1)
+
ifeq ($(CONFIG_ARM64), y)
brokengasinst := $(call as-instr,1:\n.inst 0\n.rept . - 1b\n\nnop\n.endr\n,,-DCONFIG_BROKEN_GAS_INST=1)
@@ -63,7 +69,8 @@ ifeq ($(CONFIG_GENERIC_COMPAT_VDSO), y)
endif
endif
-KBUILD_CFLAGS += -mgeneral-regs-only $(lseinstr) $(brokengasinst) $(compat_vdso)
+KBUILD_CFLAGS += -mgeneral-regs-only $(lseinstr) $(brokengasinst) \
+ $(compat_vdso) $(cc_has_k_constraint)
KBUILD_CFLAGS += -fno-asynchronous-unwind-tables
KBUILD_CFLAGS += $(call cc-disable-warning, psabi)
KBUILD_AFLAGS += $(lseinstr) $(brokengasinst) $(compat_vdso)
diff --git a/arch/arm64/include/asm/atomic_ll_sc.h b/arch/arm64/include/asm/atomic_ll_sc.h
index 95091f72228b..7fa042f5444e 100644
--- a/arch/arm64/include/asm/atomic_ll_sc.h
+++ b/arch/arm64/include/asm/atomic_ll_sc.h
@@ -23,6 +23,10 @@ asm_ops "\n" \
#define __LL_SC_FALLBACK(asm_ops) asm_ops
#endif
+#ifndef CONFIG_CC_HAS_K_CONSTRAINT
+#define K
+#endif
+
/*
* AArch64 UP and SMP safe atomic ops. We use load exclusive and
* store exclusive to ensure that these are atomic. We may loop
@@ -113,10 +117,15 @@ ATOMIC_OPS(sub, sub, J)
ATOMIC_FETCH_OP (_acquire, , a, , "memory", __VA_ARGS__)\
ATOMIC_FETCH_OP (_release, , , l, "memory", __VA_ARGS__)
-ATOMIC_OPS(and, and, )
+ATOMIC_OPS(and, and, K)
+ATOMIC_OPS(or, orr, K)
+ATOMIC_OPS(xor, eor, K)
+/*
+ * GAS converts the mysterious and undocumented BIC (immediate) alias to
+ * an AND (immediate) instruction with the immediate inverted. We don't
+ * have a constraint for this, so fall back to register.
+ */
ATOMIC_OPS(andnot, bic, )
-ATOMIC_OPS(or, orr, )
-ATOMIC_OPS(xor, eor, )
#undef ATOMIC_OPS
#undef ATOMIC_FETCH_OP
@@ -208,9 +217,14 @@ ATOMIC64_OPS(sub, sub, J)
ATOMIC64_FETCH_OP (_release,, , l, "memory", __VA_ARGS__)
ATOMIC64_OPS(and, and, L)
-ATOMIC64_OPS(andnot, bic, )
ATOMIC64_OPS(or, orr, L)
ATOMIC64_OPS(xor, eor, L)
+/*
+ * GAS converts the mysterious and undocumented BIC (immediate) alias to
+ * an AND (immediate) instruction with the immediate inverted. We don't
+ * have a constraint for this, so fall back to register.
+ */
+ATOMIC64_OPS(andnot, bic, )
#undef ATOMIC64_OPS
#undef ATOMIC64_FETCH_OP
@@ -280,21 +294,21 @@ __ll_sc__cmpxchg_case_##name##sz(volatile void *ptr, \
* handle the 'K' constraint for the value 4294967295 - thus we use no
* constraint for 32 bit operations.
*/
-__CMPXCHG_CASE(w, b, , 8, , , , , )
-__CMPXCHG_CASE(w, h, , 16, , , , , )
-__CMPXCHG_CASE(w, , , 32, , , , , )
+__CMPXCHG_CASE(w, b, , 8, , , , , K)
+__CMPXCHG_CASE(w, h, , 16, , , , , K)
+__CMPXCHG_CASE(w, , , 32, , , , , K)
__CMPXCHG_CASE( , , , 64, , , , , L)
-__CMPXCHG_CASE(w, b, acq_, 8, , a, , "memory", )
-__CMPXCHG_CASE(w, h, acq_, 16, , a, , "memory", )
-__CMPXCHG_CASE(w, , acq_, 32, , a, , "memory", )
+__CMPXCHG_CASE(w, b, acq_, 8, , a, , "memory", K)
+__CMPXCHG_CASE(w, h, acq_, 16, , a, , "memory", K)
+__CMPXCHG_CASE(w, , acq_, 32, , a, , "memory", K)
__CMPXCHG_CASE( , , acq_, 64, , a, , "memory", L)
-__CMPXCHG_CASE(w, b, rel_, 8, , , l, "memory", )
-__CMPXCHG_CASE(w, h, rel_, 16, , , l, "memory", )
-__CMPXCHG_CASE(w, , rel_, 32, , , l, "memory", )
+__CMPXCHG_CASE(w, b, rel_, 8, , , l, "memory", K)
+__CMPXCHG_CASE(w, h, rel_, 16, , , l, "memory", K)
+__CMPXCHG_CASE(w, , rel_, 32, , , l, "memory", K)
__CMPXCHG_CASE( , , rel_, 64, , , l, "memory", L)
-__CMPXCHG_CASE(w, b, mb_, 8, dmb ish, , l, "memory", )
-__CMPXCHG_CASE(w, h, mb_, 16, dmb ish, , l, "memory", )
-__CMPXCHG_CASE(w, , mb_, 32, dmb ish, , l, "memory", )
+__CMPXCHG_CASE(w, b, mb_, 8, dmb ish, , l, "memory", K)
+__CMPXCHG_CASE(w, h, mb_, 16, dmb ish, , l, "memory", K)
+__CMPXCHG_CASE(w, , mb_, 32, dmb ish, , l, "memory", K)
__CMPXCHG_CASE( , , mb_, 64, dmb ish, , l, "memory", L)
#undef __CMPXCHG_CASE
@@ -332,5 +346,6 @@ __CMPXCHG_DBL( , , , )
__CMPXCHG_DBL(_mb, dmb ish, l, "memory")
#undef __CMPXCHG_DBL
+#undef K
#endif /* __ASM_ATOMIC_LL_SC_H */
--
2.11.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v5 08/10] arm64: lse: Make ARM64_LSE_ATOMICS depend on JUMP_LABEL
From: Will Deacon @ 2019-08-29 15:48 UTC (permalink / raw)
To: linux-arm-kernel
Cc: mark.rutland, peterz, catalin.marinas, ndesaulniers, robin.murphy,
Ard.Biesheuvel, andrew.murray, natechancellor, Will Deacon
In-Reply-To: <20190829154834.26547-1-will@kernel.org>
Support for LSE atomic instructions (CONFIG_ARM64_LSE_ATOMICS) relies on
a static key to select between the legacy LL/SC implementation which is
available on all arm64 CPUs and the super-duper LSE implementation which
is available on CPUs implementing v8.1 and later.
Unfortunately, when building a kernel with CONFIG_JUMP_LABEL disabled
(e.g. because the toolchain doesn't support 'asm goto'), the static key
inside the atomics code tries to use atomics itself. This results in a
mess of circular includes and a build failure:
In file included from ./arch/arm64/include/asm/lse.h:11,
from ./arch/arm64/include/asm/atomic.h:16,
from ./include/linux/atomic.h:7,
from ./include/asm-generic/bitops/atomic.h:5,
from ./arch/arm64/include/asm/bitops.h:26,
from ./include/linux/bitops.h:19,
from ./include/linux/kernel.h:12,
from ./include/asm-generic/bug.h:18,
from ./arch/arm64/include/asm/bug.h:26,
from ./include/linux/bug.h:5,
from ./include/linux/page-flags.h:10,
from kernel/bounds.c:10:
./include/linux/jump_label.h: In function ‘static_key_count’:
./include/linux/jump_label.h:254:9: error: implicit declaration of function ‘atomic_read’ [-Werror=implicit-function-declaration]
return atomic_read(&key->enabled);
^~~~~~~~~~~
[ ... more of the same ... ]
Since LSE atomic instructions are not critical to the operation of the
kernel, make them depend on JUMP_LABEL at compile time.
Signed-off-by: Will Deacon <will@kernel.org>
---
arch/arm64/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 3adcec05b1f6..27405ac94228 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1263,6 +1263,7 @@ config ARM64_PAN
config ARM64_LSE_ATOMICS
bool "Atomic instructions"
+ depends on JUMP_LABEL
default y
help
As part of the Large System Extensions, ARMv8.1 introduces new
--
2.11.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v5 07/10] arm64: asm: Kill 'asm/atomic_arch.h'
From: Will Deacon @ 2019-08-29 15:48 UTC (permalink / raw)
To: linux-arm-kernel
Cc: mark.rutland, peterz, catalin.marinas, ndesaulniers, robin.murphy,
Ard.Biesheuvel, andrew.murray, natechancellor, Will Deacon
In-Reply-To: <20190829154834.26547-1-will@kernel.org>
The contents of 'asm/atomic_arch.h' can be split across some of our
other 'asm/' headers. Remove it.
Signed-off-by: Will Deacon <will@kernel.org>
---
arch/arm64/include/asm/atomic.h | 77 ++++++++++++++++-
arch/arm64/include/asm/atomic_arch.h | 155 -----------------------------------
arch/arm64/include/asm/cmpxchg.h | 41 ++++++++-
arch/arm64/include/asm/lse.h | 24 ++++++
4 files changed, 140 insertions(+), 157 deletions(-)
delete mode 100644 arch/arm64/include/asm/atomic_arch.h
diff --git a/arch/arm64/include/asm/atomic.h b/arch/arm64/include/asm/atomic.h
index c70d3f389d29..7c334337674d 100644
--- a/arch/arm64/include/asm/atomic.h
+++ b/arch/arm64/include/asm/atomic.h
@@ -17,9 +17,84 @@
#ifdef __KERNEL__
-#include <asm/atomic_arch.h>
#include <asm/cmpxchg.h>
+#define ATOMIC_OP(op) \
+static inline void arch_##op(int i, atomic_t *v) \
+{ \
+ __lse_ll_sc_body(op, i, v); \
+}
+
+ATOMIC_OP(atomic_andnot)
+ATOMIC_OP(atomic_or)
+ATOMIC_OP(atomic_xor)
+ATOMIC_OP(atomic_add)
+ATOMIC_OP(atomic_and)
+ATOMIC_OP(atomic_sub)
+
+
+#define ATOMIC_FETCH_OP(name, op) \
+static inline int arch_##op##name(int i, atomic_t *v) \
+{ \
+ return __lse_ll_sc_body(op##name, i, v); \
+}
+
+#define ATOMIC_FETCH_OPS(op) \
+ ATOMIC_FETCH_OP(_relaxed, op) \
+ ATOMIC_FETCH_OP(_acquire, op) \
+ ATOMIC_FETCH_OP(_release, op) \
+ ATOMIC_FETCH_OP( , op)
+
+ATOMIC_FETCH_OPS(atomic_fetch_andnot)
+ATOMIC_FETCH_OPS(atomic_fetch_or)
+ATOMIC_FETCH_OPS(atomic_fetch_xor)
+ATOMIC_FETCH_OPS(atomic_fetch_add)
+ATOMIC_FETCH_OPS(atomic_fetch_and)
+ATOMIC_FETCH_OPS(atomic_fetch_sub)
+ATOMIC_FETCH_OPS(atomic_add_return)
+ATOMIC_FETCH_OPS(atomic_sub_return)
+
+
+#define ATOMIC64_OP(op) \
+static inline void arch_##op(long i, atomic64_t *v) \
+{ \
+ __lse_ll_sc_body(op, i, v); \
+}
+
+ATOMIC64_OP(atomic64_andnot)
+ATOMIC64_OP(atomic64_or)
+ATOMIC64_OP(atomic64_xor)
+ATOMIC64_OP(atomic64_add)
+ATOMIC64_OP(atomic64_and)
+ATOMIC64_OP(atomic64_sub)
+
+
+#define ATOMIC64_FETCH_OP(name, op) \
+static inline long arch_##op##name(long i, atomic64_t *v) \
+{ \
+ return __lse_ll_sc_body(op##name, i, v); \
+}
+
+#define ATOMIC64_FETCH_OPS(op) \
+ ATOMIC64_FETCH_OP(_relaxed, op) \
+ ATOMIC64_FETCH_OP(_acquire, op) \
+ ATOMIC64_FETCH_OP(_release, op) \
+ ATOMIC64_FETCH_OP( , op)
+
+ATOMIC64_FETCH_OPS(atomic64_fetch_andnot)
+ATOMIC64_FETCH_OPS(atomic64_fetch_or)
+ATOMIC64_FETCH_OPS(atomic64_fetch_xor)
+ATOMIC64_FETCH_OPS(atomic64_fetch_add)
+ATOMIC64_FETCH_OPS(atomic64_fetch_and)
+ATOMIC64_FETCH_OPS(atomic64_fetch_sub)
+ATOMIC64_FETCH_OPS(atomic64_add_return)
+ATOMIC64_FETCH_OPS(atomic64_sub_return)
+
+static inline long arch_atomic64_dec_if_positive(atomic64_t *v)
+{
+ return __lse_ll_sc_body(atomic64_dec_if_positive, v);
+}
+
#define ATOMIC_INIT(i) { (i) }
#define arch_atomic_read(v) READ_ONCE((v)->counter)
diff --git a/arch/arm64/include/asm/atomic_arch.h b/arch/arm64/include/asm/atomic_arch.h
deleted file mode 100644
index 1aac7fc65084..000000000000
--- a/arch/arm64/include/asm/atomic_arch.h
+++ /dev/null
@@ -1,155 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-/*
- * Selection between LSE and LL/SC atomics.
- *
- * Copyright (C) 2018 ARM Ltd.
- * Author: Andrew Murray <andrew.murray@arm.com>
- */
-
-#ifndef __ASM_ATOMIC_ARCH_H
-#define __ASM_ATOMIC_ARCH_H
-
-
-#include <linux/jump_label.h>
-
-#include <asm/cpucaps.h>
-#include <asm/atomic_ll_sc.h>
-#include <asm/atomic_lse.h>
-
-extern struct static_key_false cpu_hwcap_keys[ARM64_NCAPS];
-extern struct static_key_false arm64_const_caps_ready;
-
-static inline bool system_uses_lse_atomics(void)
-{
- return (IS_ENABLED(CONFIG_ARM64_LSE_ATOMICS) &&
- IS_ENABLED(CONFIG_AS_LSE) &&
- static_branch_likely(&arm64_const_caps_ready)) &&
- static_branch_likely(&cpu_hwcap_keys[ARM64_HAS_LSE_ATOMICS]);
-}
-
-#define __lse_ll_sc_body(op, ...) \
-({ \
- system_uses_lse_atomics() ? \
- __lse_##op(__VA_ARGS__) : \
- __ll_sc_##op(__VA_ARGS__); \
-})
-
-#define ATOMIC_OP(op) \
-static inline void arch_##op(int i, atomic_t *v) \
-{ \
- __lse_ll_sc_body(op, i, v); \
-}
-
-ATOMIC_OP(atomic_andnot)
-ATOMIC_OP(atomic_or)
-ATOMIC_OP(atomic_xor)
-ATOMIC_OP(atomic_add)
-ATOMIC_OP(atomic_and)
-ATOMIC_OP(atomic_sub)
-
-
-#define ATOMIC_FETCH_OP(name, op) \
-static inline int arch_##op##name(int i, atomic_t *v) \
-{ \
- return __lse_ll_sc_body(op##name, i, v); \
-}
-
-#define ATOMIC_FETCH_OPS(op) \
- ATOMIC_FETCH_OP(_relaxed, op) \
- ATOMIC_FETCH_OP(_acquire, op) \
- ATOMIC_FETCH_OP(_release, op) \
- ATOMIC_FETCH_OP( , op)
-
-ATOMIC_FETCH_OPS(atomic_fetch_andnot)
-ATOMIC_FETCH_OPS(atomic_fetch_or)
-ATOMIC_FETCH_OPS(atomic_fetch_xor)
-ATOMIC_FETCH_OPS(atomic_fetch_add)
-ATOMIC_FETCH_OPS(atomic_fetch_and)
-ATOMIC_FETCH_OPS(atomic_fetch_sub)
-ATOMIC_FETCH_OPS(atomic_add_return)
-ATOMIC_FETCH_OPS(atomic_sub_return)
-
-
-#define ATOMIC64_OP(op) \
-static inline void arch_##op(long i, atomic64_t *v) \
-{ \
- __lse_ll_sc_body(op, i, v); \
-}
-
-ATOMIC64_OP(atomic64_andnot)
-ATOMIC64_OP(atomic64_or)
-ATOMIC64_OP(atomic64_xor)
-ATOMIC64_OP(atomic64_add)
-ATOMIC64_OP(atomic64_and)
-ATOMIC64_OP(atomic64_sub)
-
-
-#define ATOMIC64_FETCH_OP(name, op) \
-static inline long arch_##op##name(long i, atomic64_t *v) \
-{ \
- return __lse_ll_sc_body(op##name, i, v); \
-}
-
-#define ATOMIC64_FETCH_OPS(op) \
- ATOMIC64_FETCH_OP(_relaxed, op) \
- ATOMIC64_FETCH_OP(_acquire, op) \
- ATOMIC64_FETCH_OP(_release, op) \
- ATOMIC64_FETCH_OP( , op)
-
-ATOMIC64_FETCH_OPS(atomic64_fetch_andnot)
-ATOMIC64_FETCH_OPS(atomic64_fetch_or)
-ATOMIC64_FETCH_OPS(atomic64_fetch_xor)
-ATOMIC64_FETCH_OPS(atomic64_fetch_add)
-ATOMIC64_FETCH_OPS(atomic64_fetch_and)
-ATOMIC64_FETCH_OPS(atomic64_fetch_sub)
-ATOMIC64_FETCH_OPS(atomic64_add_return)
-ATOMIC64_FETCH_OPS(atomic64_sub_return)
-
-
-static inline long arch_atomic64_dec_if_positive(atomic64_t *v)
-{
- return __lse_ll_sc_body(atomic64_dec_if_positive, v);
-}
-
-#define __CMPXCHG_CASE(name, sz) \
-static inline u##sz __cmpxchg_case_##name##sz(volatile void *ptr, \
- u##sz old, \
- u##sz new) \
-{ \
- return __lse_ll_sc_body(_cmpxchg_case_##name##sz, \
- ptr, old, new); \
-}
-
-__CMPXCHG_CASE( , 8)
-__CMPXCHG_CASE( , 16)
-__CMPXCHG_CASE( , 32)
-__CMPXCHG_CASE( , 64)
-__CMPXCHG_CASE(acq_, 8)
-__CMPXCHG_CASE(acq_, 16)
-__CMPXCHG_CASE(acq_, 32)
-__CMPXCHG_CASE(acq_, 64)
-__CMPXCHG_CASE(rel_, 8)
-__CMPXCHG_CASE(rel_, 16)
-__CMPXCHG_CASE(rel_, 32)
-__CMPXCHG_CASE(rel_, 64)
-__CMPXCHG_CASE(mb_, 8)
-__CMPXCHG_CASE(mb_, 16)
-__CMPXCHG_CASE(mb_, 32)
-__CMPXCHG_CASE(mb_, 64)
-
-
-#define __CMPXCHG_DBL(name) \
-static inline long __cmpxchg_double##name(unsigned long old1, \
- unsigned long old2, \
- unsigned long new1, \
- unsigned long new2, \
- volatile void *ptr) \
-{ \
- return __lse_ll_sc_body(_cmpxchg_double##name, \
- old1, old2, new1, new2, ptr); \
-}
-
-__CMPXCHG_DBL( )
-__CMPXCHG_DBL(_mb)
-
-#endif /* __ASM_ATOMIC_LSE_H */
diff --git a/arch/arm64/include/asm/cmpxchg.h b/arch/arm64/include/asm/cmpxchg.h
index e5fff8cd4904..afaba73e0b2c 100644
--- a/arch/arm64/include/asm/cmpxchg.h
+++ b/arch/arm64/include/asm/cmpxchg.h
@@ -10,7 +10,6 @@
#include <linux/build_bug.h>
#include <linux/compiler.h>
-#include <asm/atomic_arch.h>
#include <asm/barrier.h>
#include <asm/lse.h>
@@ -104,6 +103,46 @@ __XCHG_GEN(_mb)
#define arch_xchg_release(...) __xchg_wrapper(_rel, __VA_ARGS__)
#define arch_xchg(...) __xchg_wrapper( _mb, __VA_ARGS__)
+#define __CMPXCHG_CASE(name, sz) \
+static inline u##sz __cmpxchg_case_##name##sz(volatile void *ptr, \
+ u##sz old, \
+ u##sz new) \
+{ \
+ return __lse_ll_sc_body(_cmpxchg_case_##name##sz, \
+ ptr, old, new); \
+}
+
+__CMPXCHG_CASE( , 8)
+__CMPXCHG_CASE( , 16)
+__CMPXCHG_CASE( , 32)
+__CMPXCHG_CASE( , 64)
+__CMPXCHG_CASE(acq_, 8)
+__CMPXCHG_CASE(acq_, 16)
+__CMPXCHG_CASE(acq_, 32)
+__CMPXCHG_CASE(acq_, 64)
+__CMPXCHG_CASE(rel_, 8)
+__CMPXCHG_CASE(rel_, 16)
+__CMPXCHG_CASE(rel_, 32)
+__CMPXCHG_CASE(rel_, 64)
+__CMPXCHG_CASE(mb_, 8)
+__CMPXCHG_CASE(mb_, 16)
+__CMPXCHG_CASE(mb_, 32)
+__CMPXCHG_CASE(mb_, 64)
+
+#define __CMPXCHG_DBL(name) \
+static inline long __cmpxchg_double##name(unsigned long old1, \
+ unsigned long old2, \
+ unsigned long new1, \
+ unsigned long new2, \
+ volatile void *ptr) \
+{ \
+ return __lse_ll_sc_body(_cmpxchg_double##name, \
+ old1, old2, new1, new2, ptr); \
+}
+
+__CMPXCHG_DBL( )
+__CMPXCHG_DBL(_mb)
+
#define __CMPXCHG_GEN(sfx) \
static inline unsigned long __cmpxchg##sfx(volatile void *ptr, \
unsigned long old, \
diff --git a/arch/arm64/include/asm/lse.h b/arch/arm64/include/asm/lse.h
index 08e818e53ed7..80b388278149 100644
--- a/arch/arm64/include/asm/lse.h
+++ b/arch/arm64/include/asm/lse.h
@@ -2,22 +2,46 @@
#ifndef __ASM_LSE_H
#define __ASM_LSE_H
+#include <asm/atomic_ll_sc.h>
+
#if defined(CONFIG_AS_LSE) && defined(CONFIG_ARM64_LSE_ATOMICS)
#include <linux/compiler_types.h>
#include <linux/export.h>
+#include <linux/jump_label.h>
#include <linux/stringify.h>
#include <asm/alternative.h>
+#include <asm/atomic_lse.h>
#include <asm/cpucaps.h>
__asm__(".arch_extension lse");
+extern struct static_key_false cpu_hwcap_keys[ARM64_NCAPS];
+extern struct static_key_false arm64_const_caps_ready;
+
+static inline bool system_uses_lse_atomics(void)
+{
+ return (static_branch_likely(&arm64_const_caps_ready)) &&
+ static_branch_likely(&cpu_hwcap_keys[ARM64_HAS_LSE_ATOMICS]);
+}
+
+#define __lse_ll_sc_body(op, ...) \
+({ \
+ system_uses_lse_atomics() ? \
+ __lse_##op(__VA_ARGS__) : \
+ __ll_sc_##op(__VA_ARGS__); \
+})
+
/* In-line patching at runtime */
#define ARM64_LSE_ATOMIC_INSN(llsc, lse) \
ALTERNATIVE(llsc, lse, ARM64_HAS_LSE_ATOMICS)
#else /* CONFIG_AS_LSE && CONFIG_ARM64_LSE_ATOMICS */
+static inline bool system_uses_lse_atomics(void) { return false; }
+
+#define __lse_ll_sc_body(op, ...) __ll_sc_##op(__VA_ARGS__)
+
#define ARM64_LSE_ATOMIC_INSN(llsc, lse) llsc
#endif /* CONFIG_AS_LSE && CONFIG_ARM64_LSE_ATOMICS */
--
2.11.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v5 05/10] arm64: atomics: Remove atomic_ll_sc compilation unit
From: Will Deacon @ 2019-08-29 15:48 UTC (permalink / raw)
To: linux-arm-kernel
Cc: mark.rutland, peterz, catalin.marinas, ndesaulniers, robin.murphy,
Ard.Biesheuvel, andrew.murray, natechancellor, Will Deacon
In-Reply-To: <20190829154834.26547-1-will@kernel.org>
From: Andrew Murray <andrew.murray@arm.com>
We no longer fall back to out-of-line atomics on systems with
CONFIG_ARM64_LSE_ATOMICS where ARM64_HAS_LSE_ATOMICS is not set.
Remove the unused compilation unit which provided these symbols.
Signed-off-by: Andrew Murray <andrew.murray@arm.com>
Signed-off-by: Will Deacon <will@kernel.org>
---
arch/arm64/lib/Makefile | 19 -------------------
arch/arm64/lib/atomic_ll_sc.c | 3 ---
2 files changed, 22 deletions(-)
delete mode 100644 arch/arm64/lib/atomic_ll_sc.c
diff --git a/arch/arm64/lib/Makefile b/arch/arm64/lib/Makefile
index 33c2a4abda04..f10809ef1690 100644
--- a/arch/arm64/lib/Makefile
+++ b/arch/arm64/lib/Makefile
@@ -11,25 +11,6 @@ CFLAGS_REMOVE_xor-neon.o += -mgeneral-regs-only
CFLAGS_xor-neon.o += -ffreestanding
endif
-# Tell the compiler to treat all general purpose registers (with the
-# exception of the IP registers, which are already handled by the caller
-# in case of a PLT) as callee-saved, which allows for efficient runtime
-# patching of the bl instruction in the caller with an atomic instruction
-# when supported by the CPU. Result and argument registers are handled
-# correctly, based on the function prototype.
-lib-$(CONFIG_ARM64_LSE_ATOMICS) += atomic_ll_sc.o
-CFLAGS_atomic_ll_sc.o := -ffixed-x1 -ffixed-x2 \
- -ffixed-x3 -ffixed-x4 -ffixed-x5 -ffixed-x6 \
- -ffixed-x7 -fcall-saved-x8 -fcall-saved-x9 \
- -fcall-saved-x10 -fcall-saved-x11 -fcall-saved-x12 \
- -fcall-saved-x13 -fcall-saved-x14 -fcall-saved-x15 \
- -fcall-saved-x18 -fomit-frame-pointer
-CFLAGS_REMOVE_atomic_ll_sc.o := $(CC_FLAGS_FTRACE)
-GCOV_PROFILE_atomic_ll_sc.o := n
-KASAN_SANITIZE_atomic_ll_sc.o := n
-KCOV_INSTRUMENT_atomic_ll_sc.o := n
-UBSAN_SANITIZE_atomic_ll_sc.o := n
-
lib-$(CONFIG_ARCH_HAS_UACCESS_FLUSHCACHE) += uaccess_flushcache.o
obj-$(CONFIG_CRC32) += crc32.o
diff --git a/arch/arm64/lib/atomic_ll_sc.c b/arch/arm64/lib/atomic_ll_sc.c
deleted file mode 100644
index b0c538b0da28..000000000000
--- a/arch/arm64/lib/atomic_ll_sc.c
+++ /dev/null
@@ -1,3 +0,0 @@
-#include <asm/atomic.h>
-#define __ARM64_IN_ATOMIC_IMPL
-#include <asm/atomic_ll_sc.h>
--
2.11.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v5 06/10] arm64: lse: Remove unused 'alt_lse' assembly macro
From: Will Deacon @ 2019-08-29 15:48 UTC (permalink / raw)
To: linux-arm-kernel
Cc: mark.rutland, peterz, catalin.marinas, ndesaulniers, robin.murphy,
Ard.Biesheuvel, andrew.murray, natechancellor, Will Deacon
In-Reply-To: <20190829154834.26547-1-will@kernel.org>
The 'alt_lse' assembly macro has been unused since 7c8fc35dfc32
("locking/atomics/arm64: Replace our atomic/lock bitop implementations
with asm-generic").
Remove it.
Signed-off-by: Will Deacon <will@kernel.org>
---
arch/arm64/include/asm/lse.h | 22 ----------------------
1 file changed, 22 deletions(-)
diff --git a/arch/arm64/include/asm/lse.h b/arch/arm64/include/asm/lse.h
index 52b80846d1b7..08e818e53ed7 100644
--- a/arch/arm64/include/asm/lse.h
+++ b/arch/arm64/include/asm/lse.h
@@ -10,37 +10,15 @@
#include <asm/alternative.h>
#include <asm/cpucaps.h>
-#ifdef __ASSEMBLER__
-
-.arch_extension lse
-
-.macro alt_lse, llsc, lse
- alternative_insn "\llsc", "\lse", ARM64_HAS_LSE_ATOMICS
-.endm
-
-#else /* __ASSEMBLER__ */
-
__asm__(".arch_extension lse");
-
/* In-line patching at runtime */
#define ARM64_LSE_ATOMIC_INSN(llsc, lse) \
ALTERNATIVE(llsc, lse, ARM64_HAS_LSE_ATOMICS)
-#endif /* __ASSEMBLER__ */
#else /* CONFIG_AS_LSE && CONFIG_ARM64_LSE_ATOMICS */
-#ifdef __ASSEMBLER__
-
-.macro alt_lse, llsc, lse
- \llsc
-.endm
-
-#else /* __ASSEMBLER__ */
-
-
#define ARM64_LSE_ATOMIC_INSN(llsc, lse) llsc
-#endif /* __ASSEMBLER__ */
#endif /* CONFIG_AS_LSE && CONFIG_ARM64_LSE_ATOMICS */
#endif /* __ASM_LSE_H */
--
2.11.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v5 04/10] arm64: avoid using hard-coded registers for LSE atomics
From: Will Deacon @ 2019-08-29 15:48 UTC (permalink / raw)
To: linux-arm-kernel
Cc: mark.rutland, peterz, catalin.marinas, ndesaulniers, robin.murphy,
Ard.Biesheuvel, andrew.murray, natechancellor, Will Deacon
In-Reply-To: <20190829154834.26547-1-will@kernel.org>
From: Andrew Murray <andrew.murray@arm.com>
Now that we have removed the out-of-line ll/sc atomics we can give
the compiler the freedom to choose its own register allocation.
Remove the hard-coded use of x30.
Signed-off-by: Andrew Murray <andrew.murray@arm.com>
Signed-off-by: Will Deacon <will@kernel.org>
---
arch/arm64/include/asm/atomic_lse.h | 70 ++++++++++++++++++++++---------------
1 file changed, 41 insertions(+), 29 deletions(-)
diff --git a/arch/arm64/include/asm/atomic_lse.h b/arch/arm64/include/asm/atomic_lse.h
index 7dce5e1f074e..c6bd87d2915b 100644
--- a/arch/arm64/include/asm/atomic_lse.h
+++ b/arch/arm64/include/asm/atomic_lse.h
@@ -55,12 +55,14 @@ ATOMIC_FETCH_OPS(add, ldadd)
#define ATOMIC_OP_ADD_RETURN(name, mb, cl...) \
static inline int __lse_atomic_add_return##name(int i, atomic_t *v) \
{ \
+ u32 tmp; \
+ \
asm volatile( \
- " ldadd" #mb " %w[i], w30, %[v]\n" \
- " add %w[i], %w[i], w30" \
- : [i] "+r" (i), [v] "+Q" (v->counter) \
+ " ldadd" #mb " %w[i], %w[tmp], %[v]\n" \
+ " add %w[i], %w[i], %w[tmp]" \
+ : [i] "+r" (i), [v] "+Q" (v->counter), [tmp] "=&r" (tmp) \
: "r" (v) \
- : "x30", ##cl); \
+ : cl); \
\
return i; \
}
@@ -113,13 +115,15 @@ static inline void __lse_atomic_sub(int i, atomic_t *v)
#define ATOMIC_OP_SUB_RETURN(name, mb, cl...) \
static inline int __lse_atomic_sub_return##name(int i, atomic_t *v) \
{ \
+ u32 tmp; \
+ \
asm volatile( \
" neg %w[i], %w[i]\n" \
- " ldadd" #mb " %w[i], w30, %[v]\n" \
- " add %w[i], %w[i], w30" \
- : [i] "+&r" (i), [v] "+Q" (v->counter) \
+ " ldadd" #mb " %w[i], %w[tmp], %[v]\n" \
+ " add %w[i], %w[i], %w[tmp]" \
+ : [i] "+&r" (i), [v] "+Q" (v->counter), [tmp] "=&r" (tmp) \
: "r" (v) \
- : "x30", ##cl); \
+ : cl); \
\
return i; \
}
@@ -196,12 +200,14 @@ ATOMIC64_FETCH_OPS(add, ldadd)
#define ATOMIC64_OP_ADD_RETURN(name, mb, cl...) \
static inline long __lse_atomic64_add_return##name(s64 i, atomic64_t *v)\
{ \
+ unsigned long tmp; \
+ \
asm volatile( \
- " ldadd" #mb " %[i], x30, %[v]\n" \
- " add %[i], %[i], x30" \
- : [i] "+r" (i), [v] "+Q" (v->counter) \
+ " ldadd" #mb " %[i], %x[tmp], %[v]\n" \
+ " add %[i], %[i], %x[tmp]" \
+ : [i] "+r" (i), [v] "+Q" (v->counter), [tmp] "=&r" (tmp) \
: "r" (v) \
- : "x30", ##cl); \
+ : cl); \
\
return i; \
}
@@ -254,13 +260,15 @@ static inline void __lse_atomic64_sub(s64 i, atomic64_t *v)
#define ATOMIC64_OP_SUB_RETURN(name, mb, cl...) \
static inline long __lse_atomic64_sub_return##name(s64 i, atomic64_t *v) \
{ \
+ unsigned long tmp; \
+ \
asm volatile( \
" neg %[i], %[i]\n" \
- " ldadd" #mb " %[i], x30, %[v]\n" \
- " add %[i], %[i], x30" \
- : [i] "+&r" (i), [v] "+Q" (v->counter) \
+ " ldadd" #mb " %[i], %x[tmp], %[v]\n" \
+ " add %[i], %[i], %x[tmp]" \
+ : [i] "+&r" (i), [v] "+Q" (v->counter), [tmp] "=&r" (tmp) \
: "r" (v) \
- : "x30", ##cl); \
+ : cl); \
\
return i; \
}
@@ -294,18 +302,20 @@ ATOMIC64_FETCH_OP_SUB( , al, "memory")
static inline s64 __lse_atomic64_dec_if_positive(atomic64_t *v)
{
+ unsigned long tmp;
+
asm volatile(
- "1: ldr x30, %[v]\n"
- " subs %[ret], x30, #1\n"
+ "1: ldr %x[tmp], %[v]\n"
+ " subs %[ret], %x[tmp], #1\n"
" b.lt 2f\n"
- " casal x30, %[ret], %[v]\n"
- " sub x30, x30, #1\n"
- " sub x30, x30, %[ret]\n"
- " cbnz x30, 1b\n"
+ " casal %x[tmp], %[ret], %[v]\n"
+ " sub %x[tmp], %x[tmp], #1\n"
+ " sub %x[tmp], %x[tmp], %[ret]\n"
+ " cbnz %x[tmp], 1b\n"
"2:"
- : [ret] "+&r" (v), [v] "+Q" (v->counter)
+ : [ret] "+&r" (v), [v] "+Q" (v->counter), [tmp] "=&r" (tmp)
:
- : "x30", "cc", "memory");
+ : "cc", "memory");
return (long)v;
}
@@ -318,14 +328,16 @@ static inline u##sz __lse__cmpxchg_case_##name##sz(volatile void *ptr, \
register unsigned long x0 asm ("x0") = (unsigned long)ptr; \
register u##sz x1 asm ("x1") = old; \
register u##sz x2 asm ("x2") = new; \
+ unsigned long tmp; \
\
asm volatile( \
- " mov " #w "30, %" #w "[old]\n" \
- " cas" #mb #sfx "\t" #w "30, %" #w "[new], %[v]\n" \
- " mov %" #w "[ret], " #w "30" \
- : [ret] "+r" (x0), [v] "+Q" (*(unsigned long *)ptr) \
+ " mov %" #w "[tmp], %" #w "[old]\n" \
+ " cas" #mb #sfx "\t%" #w "[tmp], %" #w "[new], %[v]\n" \
+ " mov %" #w "[ret], %" #w "[tmp]" \
+ : [ret] "+r" (x0), [v] "+Q" (*(unsigned long *)ptr), \
+ [tmp] "=&r" (tmp) \
: [old] "r" (x1), [new] "r" (x2) \
- : "x30", ##cl); \
+ : cl); \
\
return x0; \
}
--
2.11.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox