* [PATCH] ARM: soft-reboot into same mode that we entered the kernel
From: Russell King - ARM Linux @ 2016-12-13 11:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161213105410.GB19985@leverpostej>
On Tue, Dec 13, 2016 at 10:54:11AM +0000, Mark Rutland wrote:
> Hi,
>
> On Fri, Dec 09, 2016 at 07:49:37PM +0000, Russell King wrote:
> > When we soft-reboot (eg, kexec) from one kernel into the next, we need
> > to ensure that we enter the new kernel in the same processor mode as
> > when we were entered, so that (eg) the new kernel can install its own
> > hypervisor - the old kernel's hypervisor will have been overwritten.
> >
> > In order to do this, we need to pass a flag to cpu_reset() so it knows
> > what to do, and we need to modify the kernel's own hypervisor stub to
> > allow it to handle a soft-reboot.
> >
> > As we are always guaranteed to install our own hypervisor if we're
> > entered in HYP32 mode, and KVM will have moved itself out of the way
> > on kexec/normal reboot, we can assume that our hypervisor is in place
> > when we want to kexec, so changing our hypervisor API should not be a
> > problem.
> >
> > Tested-by: Keerthy <j-keerthy@ti.com>
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > ---
> > Mark,
> >
> > Any opinions on this?
> >
> > Thanks.
>
> The above sounds like the right thing to do, but I'm not familiar enough
> with the 32-bit hyp + kvm code to say much about the implementation.
>
> Hopefully Dave, Marc, or Christoffer have thoughts.
>
> Otherwise, I only have a couple of minor comments below.
>
> >
> > arch/arm/include/asm/proc-fns.h | 4 ++--
> > arch/arm/kernel/hyp-stub.S | 36 ++++++++++++++++++++++++++++++------
> > arch/arm/kernel/reboot.c | 12 ++++++++++--
> > arch/arm/mm/proc-v7.S | 12 ++++++++----
> > 4 files changed, 50 insertions(+), 14 deletions(-)
> >
> > diff --git a/arch/arm/include/asm/proc-fns.h b/arch/arm/include/asm/proc-fns.h
> > index 8877ad5ffe10..f2e1af45bd6f 100644
> > --- a/arch/arm/include/asm/proc-fns.h
> > +++ b/arch/arm/include/asm/proc-fns.h
> > @@ -43,7 +43,7 @@ extern struct processor {
> > /*
> > * Special stuff for a reset
> > */
> > - void (*reset)(unsigned long addr) __attribute__((noreturn));
> > + void (*reset)(unsigned long addr, bool hvc) __attribute__((noreturn));
> > /*
> > * Idle the processor
> > */
> > @@ -88,7 +88,7 @@ extern void cpu_set_pte_ext(pte_t *ptep, pte_t pte);
> > #else
> > extern void cpu_set_pte_ext(pte_t *ptep, pte_t pte, unsigned int ext);
> > #endif
> > -extern void cpu_reset(unsigned long addr) __attribute__((noreturn));
> > +extern void cpu_reset(unsigned long addr, bool hvc) __attribute__((noreturn));
> >
> > /* These three are private to arch/arm/kernel/suspend.c */
> > extern void cpu_do_suspend(void *);
> > diff --git a/arch/arm/kernel/hyp-stub.S b/arch/arm/kernel/hyp-stub.S
> > index 15d073ae5da2..cab1ac36939d 100644
> > --- a/arch/arm/kernel/hyp-stub.S
> > +++ b/arch/arm/kernel/hyp-stub.S
> > @@ -22,6 +22,9 @@
> > #include <asm/assembler.h>
> > #include <asm/virt.h>
> >
> > +#define HVC_GET_VECTORS -1
> > +#define HVC_SOFT_RESTART 1
> > +
> > #ifndef ZIMAGE
> > /*
> > * For the kernel proper, we need to find out the CPU boot mode long after
> > @@ -202,9 +205,18 @@ ARM_BE8(orr r7, r7, #(1 << 25)) @ HSCTLR.EE
> > ENDPROC(__hyp_stub_install_secondary)
> >
> > __hyp_stub_do_trap:
> > - cmp r0, #-1
> > - mrceq p15, 4, r0, c12, c0, 0 @ get HVBAR
> > - mcrne p15, 4, r0, c12, c0, 0 @ set HVBAR
> > + cmp r0, #HVC_GET_VECTORS
> > + bne 1f
> > + mrc p15, 4, r0, c12, c0, 0 @ get HVBAR
> > + b __hyp_stub_exit
> > +
> > +1: teq r0, #HVC_SOFT_RESTART
> > + bne 1f
> > + mov r0, r3
> > + bx r0
> > +
> > +1: mcrne p15, 4, r0, c12, c0, 0 @ set HVBAR
> > +__hyp_stub_exit:
> > __ERET
> > ENDPROC(__hyp_stub_do_trap)
> >
> > @@ -231,14 +243,26 @@ ENDPROC(__hyp_stub_do_trap)
> > * initialisation entry point.
> > */
> > ENTRY(__hyp_get_vectors)
> > - mov r0, #-1
> > + mov r0, #HVC_GET_VECTORS
> > + __HVC(0)
> > + ret lr
> > ENDPROC(__hyp_get_vectors)
> > - @ fall through
> > +
> > ENTRY(__hyp_set_vectors)
> > + tst r0, #31
> > + bne 1f
> > __HVC(0)
> > - ret lr
> > +1: ret lr
> > ENDPROC(__hyp_set_vectors)
>
> Why the new check? This looks unrelated to the rest of the patch.
It's not unrelated. The ARM32 hyp-stub has a total crap ABI:
- r0 = -1 => read VBAR
- r0 != -1 => write r0 to VBAR
So, this check is there to ensure that you can't do something stupid
like:
__hyp_set_vectors(1)
and inadvertently end up invoking the restart method - the check is
there to "make room" for the new hyp call in the ABI.
It hasn't been clear what the scope of the API, or the stub ABI actually
is - nothing about that is really documented, so I didn't want to
radically redesign the stub ABI to be more sensible and risk breakage
elsewhere - especially as I'm reliant on others to test this. (All my
32-bit platforms enter the kernel in SVC mode from the boot loader, even
those which are virtualisation-capable.)
> > +ENTRY(__hyp_soft_restart)
> > + mov r3, r0
> > + mov r0, #HVC_SOFT_RESTART
> > + __HVC(0)
> > + mov r0, r3
> > + ret lr
> > +ENDPROC(__hyp_soft_restart)
> > +
> > #ifndef ZIMAGE
> > .align 2
> > .L__boot_cpu_mode_offset:
> > diff --git a/arch/arm/kernel/reboot.c b/arch/arm/kernel/reboot.c
> > index 3fa867a2aae6..f0e3c7f1ea21 100644
> > --- a/arch/arm/kernel/reboot.c
> > +++ b/arch/arm/kernel/reboot.c
> > @@ -12,10 +12,11 @@
> >
> > #include <asm/cacheflush.h>
> > #include <asm/idmap.h>
> > +#include <asm/virt.h>
> >
> > #include "reboot.h"
> >
> > -typedef void (*phys_reset_t)(unsigned long);
> > +typedef void (*phys_reset_t)(unsigned long, bool);
> >
> > /*
> > * Function pointers to optional machine specific functions
> > @@ -36,6 +37,7 @@ static u64 soft_restart_stack[16];
> > static void __soft_restart(void *addr)
> > {
> > phys_reset_t phys_reset;
> > + bool hvc = false;
> >
> > /* Take out a flat memory mapping. */
> > setup_mm_for_reboot();
> > @@ -51,7 +53,13 @@ static void __soft_restart(void *addr)
> >
> > /* Switch to the identity mapping. */
> > phys_reset = (phys_reset_t)virt_to_idmap(cpu_reset);
> > - phys_reset((unsigned long)addr);
> > +
> > +#ifdef CONFIG_ARM_VIRT_EXT
> > + /* original stub should be restored by kvm */
> > + hvc = is_hyp_mode_available();
> > +#endif
>
> When !CONFIG_ARM_VIRT_EXT, is_hyp_mode_available() is defined so as to
> always be false, so we can drop the ifdef here.
Ok.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply
* [PATCH V6 00/10] Add UEFI 2.6 and ACPI 6.1 updates for RAS on ARM64
From: Shiju Jose @ 2016-12-13 11:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481147303-7979-1-git-send-email-tbaicar@codeaurora.org>
Hi Tyler,
We have tested V6 patch set on our platform. It worked fine.
Thanks,
Shiju
> -----Original Message-----
> From: Tyler Baicar [mailto:tbaicar at codeaurora.org]
> Sent: 07 December 2016 21:48
> To: christoffer.dall at linaro.org; marc.zyngier at arm.com;
> pbonzini at redhat.com; rkrcmar at redhat.com; linux at armlinux.org.uk;
> catalin.marinas at arm.com; will.deacon at arm.com; rjw at rjwysocki.net;
> lenb at kernel.org; matt at codeblueprint.co.uk; robert.moore at intel.com;
> lv.zheng at intel.com; nkaje at codeaurora.org; zjzhang at codeaurora.org;
> mark.rutland at arm.com; james.morse at arm.com; akpm at linux-foundation.org;
> eun.taik.lee at samsung.com; sandeepa.s.prabhu at gmail.com;
> labbott at redhat.com; shijie.huang at arm.com; rruigrok at codeaurora.org;
> paul.gortmaker at windriver.com; tn at semihalf.com; fu.wei at linaro.org;
> rostedt at goodmis.org; bristot at redhat.com; linux-arm-
> kernel at lists.infradead.org; kvmarm at lists.cs.columbia.edu;
> kvm at vger.kernel.org; linux-kernel at vger.kernel.org; linux-
> acpi at vger.kernel.org; linux-efi at vger.kernel.org; devel at acpica.org;
> Suzuki.Poulose at arm.com; punit.agrawal at arm.com; astone at redhat.com;
> harba at codeaurora.org; hanjun.guo at linaro.org; John Garry; Shiju Jose
> Cc: Tyler Baicar
> Subject: [PATCH V6 00/10] Add UEFI 2.6 and ACPI 6.1 updates for RAS on
> ARM64
>
> When a memory error, CPU error, PCIe error, or other type of hardware
> error that's covered by RAS occurs, firmware should populate the shared
> GHES memory location with the proper GHES structures to notify the OS
> of the error.
> For example, platforms that implement firmware first handling may
> implement separate GHES sources for corrected errors and uncorrected
> errors. If the error is an uncorrectable error, then the firmware will
> notify the OS immediately since the error needs to be handled ASAP. The
> OS will then be able to take the appropriate action needed such as
> offlining a page. If the error is a corrected error, then the firmware
> will not interrupt the OS immediately.
> Instead, the OS will see and report the error the next time it's GHES
> timer expires. The kernel will first parse the GHES structures and
> report the errors through the kernel logs and then notify the user
> space through RAS trace events. This allows user space applications
> such as RAS Daemon to see the errors and report them however the user
> desires. This patchset extends the kernel functionality for RAS errors
> based on updates in the UEFI 2.6 and ACPI 6.1 specifications.
>
> An example flow from firmware to user space could be:
>
> +---------------+
> +-------->| |
> | | GHES polling |--+
> +-------------+ | source | | +---------------+ +----------
> --+
> | | +---------------+ | | Kernel GHES | |
> |
> | Firmware | +-->| CPER AER and |-->| RAS
> trace |
> | | +---------------+ | | EDAC drivers | | event
> |
> +-------------+ | | | +---------------+ +----------
> --+
> | | GHES sci |--+
> +-------->| source |
> +---------------+
>
> Add support for Generic Hardware Error Source (GHES) v2, which
> introduces the capability for the OS to acknowledge the consumption of
> the error record generated by the Reliability, Availability and
> Serviceability (RAS) controller.
> This eliminates potential race conditions between the OS and the RAS
> controller.
>
> Add support for the timestamp field added to the Generic Error Data
> Entry v3, allowing the OS to log the time that the error is generated
> by the firmware, rather than the time the error is consumed. This
> improves the correctness of event sequences when analyzing error logs.
> The timestamp is added in ACPI 6.1, reference Table 18-343 Generic
> Error Data Entry.
>
> Add support for ARMv8 Common Platform Error Record (CPER) per UEFI 2.6
> specification. ARMv8 specific processor error information is reported
> as part of the CPER records. This provides more detail on for
> processor error logs. This can help describe ARMv8 cache, tlb, and bus
> errors.
>
> Synchronous External Abort (SEA) represents a specific processor error
> condition in ARM systems. A handler is added to recognize SEA errors,
> and a notifier is added to parse and report the errors before the
> process is killed. Refer to section N.2.1.1 in the Common Platform
> Error Record appendix of the UEFI 2.6 specification.
>
> Currently the kernel ignores CPER records that are unrecognized.
> On the other hand, UEFI spec allows for non-standard (eg. vendor
> proprietary) error section type in CPER (Common Platform Error Record),
> as defined in section N2.3 of UEFI version 2.5. Therefore, user is not
> able to see hardware error data of non-standard section.
>
> If section Type field of Generic Error Data Entry is unrecognized,
> prints out the raw data in dmesg buffer, and also adds a tracepoint for
> reporting such hardware errors.
>
> Currently even if an error status block's severity is fatal, the kernel
> does not honor the severity level and panic. With the firmware first
> model, the platform could inform the OS about a fatal hardware error
> through the non-NMI GHES notification type. The OS should panic when a
> hardware error record is received with this severity.
>
> Add support to handle SEAs that occur while a KVM guest kernel is
> running. Currently these are unsupported by the guest abort handling.
>
> Depends on: [PATCH v15] acpi, apei, arm64: APEI initial support for
> aarch64.
> https://lkml.org/lkml/2016/12/1/312
>
> V6: Change HEST_TYPE_GENERIC_V2 to IS_HEST_TYPE_GENERIC_V2 for
> readability
> Move APEI helper defines from cper.h to ghes.h
> Add data_len decrement back into print loop
> Change references to ARMv8 to just ARM
> Rewrite ARM processor context info parsing
> Check valid bit of ARM error info field before printing it
> Add include of linux/uuid.h in ghes.c
>
> V5: Fix GHES goto logic for error conditions
> Change ghes_do_read_ack to ghes_ack_error
> Make sure data version check is >= 3
> Use CPER helper functions in print functions
> Make handle_guest_sea() dummy function static for arm
> Add arm to subject line for KVM patch
>
> V4: Add bit offset left shift to read_ack_write value
> Make HEST generic and generic_v2 structures a union in the ghes
> structure
> Move gdata v3 helper functions into ghes.h to avoid duplication
> Reorder the timestamp print and avoid memcpy
> Add helper functions for gdata size checking
> Rename the SEA functions
> Add helper function for GHES panics
> Set fru_id to NULL UUID at variable declaration
> Limit ARM trace event parameters to the needed structures
> Reorder the ARM trace event variables to save space
> Add comment for why we don't pass SEAs to the guest when it aborts
> Move ARM trace event call into GHES driver instead of CPER
>
> V3: Fix unmapped address to the read_ack_register in ghes.c
> Add helper function to get the proper payload based on generic data
> entry
> version
> Move timestamp print to avoid changing function calls in cper.c
> Remove patch "arm64: exception: handle instruction abort at current
> EL"
> since the el1_ia handler is already added in 4.8
> Add EFI and ARM64 dependencies for HAVE_ACPI_APEI_SEA
> Add a new trace event for ARM type errors
> Add support to handle KVM guest SEAs
>
> V2: Add PSCI state print for the ARMv8 error type.
> Separate timestamp year into year and century using BCD format.
> Rebase on top of ACPICA 20160318 release and remove header file
> changes
> in include/acpi/actbl1.h.
> Add panic OS with fatal error status block patch.
> Add processing of unrecognized CPER error section patches with
> updates
> from previous comments. Original patches:
> https://lkml.org/lkml/2015/9/8/646
>
> V1: https://lkml.org/lkml/2016/2/5/544
>
> Jonathan (Zhixiong) Zhang (1):
> acpi: apei: panic OS with fatal error status block
>
> Tyler Baicar (9):
> acpi: apei: read ack upon ghes record consumption
> ras: acpi/apei: cper: generic error data entry v3 per ACPI 6.1
> efi: parse ARM processor error
> arm64: exception: handle Synchronous External Abort
> acpi: apei: handle SEA notification type for ARMv8
> efi: print unrecognized CPER section
> ras: acpi / apei: generate trace event for unrecognized CPER section
> trace, ras: add ARM processor error trace event
> arm/arm64: KVM: add guest SEA support
>
> arch/arm/include/asm/kvm_arm.h | 1 +
> arch/arm/include/asm/system_misc.h | 5 +
> arch/arm/kvm/mmu.c | 18 +++-
> arch/arm64/Kconfig | 1 +
> arch/arm64/include/asm/kvm_arm.h | 1 +
> arch/arm64/include/asm/system_misc.h | 15 +++
> arch/arm64/mm/fault.c | 71 ++++++++++--
> drivers/acpi/apei/Kconfig | 14 +++
> drivers/acpi/apei/ghes.c | 189
> +++++++++++++++++++++++++++++---
> drivers/acpi/apei/hest.c | 7 +-
> drivers/firmware/efi/cper.c | 204
> ++++++++++++++++++++++++++++++++---
> drivers/ras/ras.c | 2 +
> include/acpi/ghes.h | 27 ++++-
> include/linux/cper.h | 53 +++++++++
> include/ras/ras_event.h | 100 +++++++++++++++++
> 15 files changed, 664 insertions(+), 44 deletions(-)
>
> --
> Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
> Technologies, Inc.
> Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a
> Linux Foundation Collaborative Project.
^ permalink raw reply
* [PATCH v3 6/6] mfd: dt: Move syscon bindings to syscon subdirectory
From: Lee Jones @ 2016-12-13 11:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481604793.3112.32.camel@aj.id.au>
On Tue, 13 Dec 2016, Andrew Jeffery wrote:
> On Mon, 2016-12-12 at 09:39 -0600, Rob Herring wrote:
> > On Tue, Dec 06, 2016 at 01:53:21PM +1100, Andrew Jeffery wrote:
> > > The use of syscons is growing, lets collate them in their own part of
> > > the bindings tree.
> > >
> > > > > Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> > > ---
> > > ?Documentation/devicetree/bindings/mfd/{ => syscon}/aspeed-scu.txt?????????| 0
> > > ?Documentation/devicetree/bindings/mfd/{ => syscon}/atmel-gpbr.txt?????????| 0
> > > ?Documentation/devicetree/bindings/mfd/{ => syscon}/atmel-matrix.txt???????| 0
> > > ?Documentation/devicetree/bindings/mfd/{ => syscon}/atmel-smc.txt??????????| 0
> > > ?Documentation/devicetree/bindings/mfd/{ => syscon}/qcom,tcsr.txt??????????| 0
> > > ?Documentation/devicetree/bindings/mfd/{ => syscon}/syscon.txt?????????????| 0
> > > ?.../devicetree/bindings/mfd/{ => syscon}/ti-keystone-devctrl.txt??????????| 0
> > > ?7 files changed, 0 insertions(+), 0 deletions(-)
> > > ?rename Documentation/devicetree/bindings/mfd/{ => syscon}/aspeed-scu.txt (100%)
> > > ?rename Documentation/devicetree/bindings/mfd/{ => syscon}/atmel-gpbr.txt (100%)
> > > ?rename Documentation/devicetree/bindings/mfd/{ => syscon}/atmel-matrix.txt (100%)
> > > ?rename Documentation/devicetree/bindings/mfd/{ => syscon}/atmel-smc.txt (100%)
> > > ?rename Documentation/devicetree/bindings/mfd/{ => syscon}/qcom,tcsr.txt (100%)
> > > ?rename Documentation/devicetree/bindings/mfd/{ => syscon}/syscon.txt (100%)
> > > ?rename Documentation/devicetree/bindings/mfd/{ => syscon}/ti-keystone-devctrl.txt (100%)
> >
> > I'm not so sure this is the right direction. syscon usage is pretty much?
> > spread throughout the tree.
>
> This patch was created based on my interpretation of Lee's feedback
> here:
>
> https://lkml.org/lkml/2016/11/18/650
>
> Lee's next email in the chain poked Arnd for an opinion, but Arnd
> didn't reply.
>
> I don't mind. I moved these bindings separately so we could just drop
> the patch if there was push-back. If we drop the whole idea I'll need
> to apply a small fix to patch 5/6 to avoid creating the syscon
> subdirectory.
The sub-directory is a good idea for drivers who are *solely* syscon
based.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* [PATCH] ARM: soft-reboot into same mode that we entered the kernel
From: Mark Rutland @ 2016-12-13 10:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <E1cFRAn-0003ob-HH@rmk-PC.armlinux.org.uk>
Hi,
On Fri, Dec 09, 2016 at 07:49:37PM +0000, Russell King wrote:
> When we soft-reboot (eg, kexec) from one kernel into the next, we need
> to ensure that we enter the new kernel in the same processor mode as
> when we were entered, so that (eg) the new kernel can install its own
> hypervisor - the old kernel's hypervisor will have been overwritten.
>
> In order to do this, we need to pass a flag to cpu_reset() so it knows
> what to do, and we need to modify the kernel's own hypervisor stub to
> allow it to handle a soft-reboot.
>
> As we are always guaranteed to install our own hypervisor if we're
> entered in HYP32 mode, and KVM will have moved itself out of the way
> on kexec/normal reboot, we can assume that our hypervisor is in place
> when we want to kexec, so changing our hypervisor API should not be a
> problem.
>
> Tested-by: Keerthy <j-keerthy@ti.com>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
> Mark,
>
> Any opinions on this?
>
> Thanks.
The above sounds like the right thing to do, but I'm not familiar enough
with the 32-bit hyp + kvm code to say much about the implementation.
Hopefully Dave, Marc, or Christoffer have thoughts.
Otherwise, I only have a couple of minor comments below.
>
> arch/arm/include/asm/proc-fns.h | 4 ++--
> arch/arm/kernel/hyp-stub.S | 36 ++++++++++++++++++++++++++++++------
> arch/arm/kernel/reboot.c | 12 ++++++++++--
> arch/arm/mm/proc-v7.S | 12 ++++++++----
> 4 files changed, 50 insertions(+), 14 deletions(-)
>
> diff --git a/arch/arm/include/asm/proc-fns.h b/arch/arm/include/asm/proc-fns.h
> index 8877ad5ffe10..f2e1af45bd6f 100644
> --- a/arch/arm/include/asm/proc-fns.h
> +++ b/arch/arm/include/asm/proc-fns.h
> @@ -43,7 +43,7 @@ extern struct processor {
> /*
> * Special stuff for a reset
> */
> - void (*reset)(unsigned long addr) __attribute__((noreturn));
> + void (*reset)(unsigned long addr, bool hvc) __attribute__((noreturn));
> /*
> * Idle the processor
> */
> @@ -88,7 +88,7 @@ extern void cpu_set_pte_ext(pte_t *ptep, pte_t pte);
> #else
> extern void cpu_set_pte_ext(pte_t *ptep, pte_t pte, unsigned int ext);
> #endif
> -extern void cpu_reset(unsigned long addr) __attribute__((noreturn));
> +extern void cpu_reset(unsigned long addr, bool hvc) __attribute__((noreturn));
>
> /* These three are private to arch/arm/kernel/suspend.c */
> extern void cpu_do_suspend(void *);
> diff --git a/arch/arm/kernel/hyp-stub.S b/arch/arm/kernel/hyp-stub.S
> index 15d073ae5da2..cab1ac36939d 100644
> --- a/arch/arm/kernel/hyp-stub.S
> +++ b/arch/arm/kernel/hyp-stub.S
> @@ -22,6 +22,9 @@
> #include <asm/assembler.h>
> #include <asm/virt.h>
>
> +#define HVC_GET_VECTORS -1
> +#define HVC_SOFT_RESTART 1
> +
> #ifndef ZIMAGE
> /*
> * For the kernel proper, we need to find out the CPU boot mode long after
> @@ -202,9 +205,18 @@ ARM_BE8(orr r7, r7, #(1 << 25)) @ HSCTLR.EE
> ENDPROC(__hyp_stub_install_secondary)
>
> __hyp_stub_do_trap:
> - cmp r0, #-1
> - mrceq p15, 4, r0, c12, c0, 0 @ get HVBAR
> - mcrne p15, 4, r0, c12, c0, 0 @ set HVBAR
> + cmp r0, #HVC_GET_VECTORS
> + bne 1f
> + mrc p15, 4, r0, c12, c0, 0 @ get HVBAR
> + b __hyp_stub_exit
> +
> +1: teq r0, #HVC_SOFT_RESTART
> + bne 1f
> + mov r0, r3
> + bx r0
> +
> +1: mcrne p15, 4, r0, c12, c0, 0 @ set HVBAR
> +__hyp_stub_exit:
> __ERET
> ENDPROC(__hyp_stub_do_trap)
>
> @@ -231,14 +243,26 @@ ENDPROC(__hyp_stub_do_trap)
> * initialisation entry point.
> */
> ENTRY(__hyp_get_vectors)
> - mov r0, #-1
> + mov r0, #HVC_GET_VECTORS
> + __HVC(0)
> + ret lr
> ENDPROC(__hyp_get_vectors)
> - @ fall through
> +
> ENTRY(__hyp_set_vectors)
> + tst r0, #31
> + bne 1f
> __HVC(0)
> - ret lr
> +1: ret lr
> ENDPROC(__hyp_set_vectors)
Why the new check? This looks unrelated to the rest of the patch.
> +ENTRY(__hyp_soft_restart)
> + mov r3, r0
> + mov r0, #HVC_SOFT_RESTART
> + __HVC(0)
> + mov r0, r3
> + ret lr
> +ENDPROC(__hyp_soft_restart)
> +
> #ifndef ZIMAGE
> .align 2
> .L__boot_cpu_mode_offset:
> diff --git a/arch/arm/kernel/reboot.c b/arch/arm/kernel/reboot.c
> index 3fa867a2aae6..f0e3c7f1ea21 100644
> --- a/arch/arm/kernel/reboot.c
> +++ b/arch/arm/kernel/reboot.c
> @@ -12,10 +12,11 @@
>
> #include <asm/cacheflush.h>
> #include <asm/idmap.h>
> +#include <asm/virt.h>
>
> #include "reboot.h"
>
> -typedef void (*phys_reset_t)(unsigned long);
> +typedef void (*phys_reset_t)(unsigned long, bool);
>
> /*
> * Function pointers to optional machine specific functions
> @@ -36,6 +37,7 @@ static u64 soft_restart_stack[16];
> static void __soft_restart(void *addr)
> {
> phys_reset_t phys_reset;
> + bool hvc = false;
>
> /* Take out a flat memory mapping. */
> setup_mm_for_reboot();
> @@ -51,7 +53,13 @@ static void __soft_restart(void *addr)
>
> /* Switch to the identity mapping. */
> phys_reset = (phys_reset_t)virt_to_idmap(cpu_reset);
> - phys_reset((unsigned long)addr);
> +
> +#ifdef CONFIG_ARM_VIRT_EXT
> + /* original stub should be restored by kvm */
> + hvc = is_hyp_mode_available();
> +#endif
When !CONFIG_ARM_VIRT_EXT, is_hyp_mode_available() is defined so as to
always be false, so we can drop the ifdef here.
> +
> + phys_reset((unsigned long)addr, hvc);
>
> /* Should never get here. */
> BUG();
> diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S
> index d00d52c9de3e..1846ca4255d0 100644
> --- a/arch/arm/mm/proc-v7.S
> +++ b/arch/arm/mm/proc-v7.S
> @@ -53,11 +53,15 @@ ENDPROC(cpu_v7_proc_fin)
> .align 5
> .pushsection .idmap.text, "ax"
> ENTRY(cpu_v7_reset)
> - mrc p15, 0, r1, c1, c0, 0 @ ctrl register
> - bic r1, r1, #0x1 @ ...............m
> - THUMB( bic r1, r1, #1 << 30 ) @ SCTLR.TE (Thumb exceptions)
> - mcr p15, 0, r1, c1, c0, 0 @ disable MMU
> + mrc p15, 0, r2, c1, c0, 0 @ ctrl register
> + bic r2, r2, #0x1 @ ...............m
> + THUMB( bic r2, r2, #1 << 30 ) @ SCTLR.TE (Thumb exceptions)
> + mcr p15, 0, r2, c1, c0, 0 @ disable MMU
> isb
> +#ifdef CONFIG_ARM_VIRT_EXT
> + teq r1, #0
> + bne __hyp_soft_restart
> +#endif
> bx r0
> ENDPROC(cpu_v7_reset)
> .popsection
> --
> 2.7.4
Thanks,
Mark.
^ permalink raw reply
* [PATCH v6 4/5] ARM: dts: da850-lcdk: add the vga-bridge node
From: Bartosz Golaszewski @ 2016-12-13 10:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <9d5fa409-bb4f-f62b-2548-0a3b82228e08@ti.com>
2016-12-13 9:46 GMT+01:00 Tomi Valkeinen <tomi.valkeinen@ti.com>:
> Hi,
>
> On 12/12/16 15:05, Bartosz Golaszewski wrote:
>
>> +&lcdc {
>> + status = "okay";
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&lcd_pins>;
>> +
>> + ports {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + lcdc_out: port at 1 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + reg = <1>;
>> +
>> + lcdc_out_vga: endpoint {
>> + reg = <0>;
>> + remote-endpoint = <&vga_bridge_in>;
>> + };
>> + };
>> + };
>> +};
>>
>
> This is not correct. LCDC has just one output, so port at 1 doesn't make
> sense. It's port at 0. But with just one port, you can leave "ports" away.
> And you don't need the port's label for anything, if I'm not mistaken. So:
>
> &lcdc {
> status = "okay";
> pinctrl-names = "default";
> pinctrl-0 = <&lcd_pins>;
>
> port {
> lcdc_out_vga: endpoint {
> remote-endpoint = <&vga_bridge_in>;
> };
> };
> };
>
> Tomi
>
Right, fixed in v7.
Thanks,
Bartosz
^ permalink raw reply
* [PATCH v7 5/5] ARM: dts: da850: specify the maximum pixel clock rate for tilcdc
From: Bartosz Golaszewski @ 2016-12-13 10:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481623759-12786-1-git-send-email-bgolaszewski@baylibre.com>
At maximum CPU frequency of 300 MHz the maximum pixel clock frequency
is 37.5 MHz[1]. We must filter out any mode for which the calculated
pixel clock rate would exceed this value.
Specify the max-pixelclock property for the display node for
da850-lcdk.
[1] http://processors.wiki.ti.com/index.php/OMAP-L1x/C674x/AM1x_LCD_Controller_(LCDC)_Throughput_and_Optimization_Techniques
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
arch/arm/boot/dts/da850.dtsi | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
index 6b0ef3d..58b1566 100644
--- a/arch/arm/boot/dts/da850.dtsi
+++ b/arch/arm/boot/dts/da850.dtsi
@@ -462,6 +462,7 @@
compatible = "ti,da850-tilcdc";
reg = <0x213000 0x1000>;
interrupts = <52>;
+ max-pixelclock = <37500>;
status = "disabled";
};
};
--
2.9.3
^ permalink raw reply related
* [PATCH v7 4/5] ARM: dts: da850-lcdk: add the vga-bridge node
From: Bartosz Golaszewski @ 2016-12-13 10:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481623759-12786-1-git-send-email-bgolaszewski@baylibre.com>
Add the vga-bridge node to the board DT together with corresponding
ports and vga connector. This allows to retrieve the edid info from
the display automatically.
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
arch/arm/boot/dts/da850-lcdk.dts | 51 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 51 insertions(+)
diff --git a/arch/arm/boot/dts/da850-lcdk.dts b/arch/arm/boot/dts/da850-lcdk.dts
index afcb482..1ae2260 100644
--- a/arch/arm/boot/dts/da850-lcdk.dts
+++ b/arch/arm/boot/dts/da850-lcdk.dts
@@ -51,6 +51,45 @@
system-clock-frequency = <24576000>;
};
};
+
+ vga-bridge {
+ compatible = "ti,ths8135";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port at 0 {
+ reg = <0>;
+
+ vga_bridge_in: endpoint {
+ remote-endpoint = <&lcdc_out_vga>;
+ };
+ };
+
+ port at 1 {
+ reg = <1>;
+
+ vga_bridge_out: endpoint {
+ remote-endpoint = <&vga_con_in>;
+ };
+ };
+ };
+ };
+
+ vga {
+ compatible = "vga-connector";
+
+ ddc-i2c-bus = <&i2c0>;
+
+ port {
+ vga_con_in: endpoint {
+ remote-endpoint = <&vga_bridge_out>;
+ };
+ };
+ };
};
&pmx_core {
@@ -236,3 +275,15 @@
&memctrl {
status = "okay";
};
+
+&lcdc {
+ status = "okay";
+ pinctrl-names = "default";
+ pinctrl-0 = <&lcd_pins>;
+
+ port {
+ lcdc_out_vga: endpoint {
+ remote-endpoint = <&vga_bridge_in>;
+ };
+ };
+};
--
2.9.3
^ permalink raw reply related
* [PATCH v7 3/5] drm: bridge: add support for TI ths8135
From: Bartosz Golaszewski @ 2016-12-13 10:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481623759-12786-1-git-send-email-bgolaszewski@baylibre.com>
THS8135 is a configurable video DAC, but no configuration is actually
necessary to make it work.
For now use the dumb-vga-dac driver to support it.
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/gpu/drm/bridge/dumb-vga-dac.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c
index e570698..86e9f9c 100644
--- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
+++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
@@ -237,6 +237,7 @@ static int dumb_vga_remove(struct platform_device *pdev)
static const struct of_device_id dumb_vga_match[] = {
{ .compatible = "dumb-vga-dac" },
+ { .compatible = "ti,ths8135" },
{},
};
MODULE_DEVICE_TABLE(of, dumb_vga_match);
--
2.9.3
^ permalink raw reply related
* [PATCH v7 2/5] drm: bridge: add DT bindings for TI ths8135
From: Bartosz Golaszewski @ 2016-12-13 10:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481623759-12786-1-git-send-email-bgolaszewski@baylibre.com>
THS8135 is a configurable video DAC. Add DT bindings for this chip.
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Rob Herring <robh@kernel.org>
---
.../bindings/display/bridge/ti,ths8135.txt | 46 ++++++++++++++++++++++
1 file changed, 46 insertions(+)
create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,ths8135.txt
diff --git a/Documentation/devicetree/bindings/display/bridge/ti,ths8135.txt b/Documentation/devicetree/bindings/display/bridge/ti,ths8135.txt
new file mode 100644
index 0000000..6ec1a88
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/ti,ths8135.txt
@@ -0,0 +1,46 @@
+THS8135 Video DAC
+-----------------
+
+This is the binding for Texas Instruments THS8135 Video DAC bridge.
+
+Required properties:
+
+- compatible: Must be "ti,ths8135"
+
+Required nodes:
+
+This device has two video ports. Their connections are modelled using the OF
+graph bindings specified in Documentation/devicetree/bindings/graph.txt.
+
+- Video port 0 for RGB input
+- Video port 1 for VGA output
+
+Example
+-------
+
+vga-bridge {
+ compatible = "ti,ths8135";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port at 0 {
+ reg = <0>;
+
+ vga_bridge_in: endpoint {
+ remote-endpoint = <&lcdc_out_vga>;
+ };
+ };
+
+ port at 1 {
+ reg = <1>;
+
+ vga_bridge_out: endpoint {
+ remote-endpoint = <&vga_con_in>;
+ };
+ };
+ };
+};
--
2.9.3
^ permalink raw reply related
* [PATCH v7 1/5] ARM: dts: da850: rename the display node label
From: Bartosz Golaszewski @ 2016-12-13 10:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481623759-12786-1-git-send-email-bgolaszewski@baylibre.com>
The tilcdc node name is 'display' as per the ePAPR 1.1 recommendation.
The label is also 'display', but change it to 'lcdc' to make it clear
what the underlying hardware is.
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
arch/arm/boot/dts/da850.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
index 104155d..6b0ef3d 100644
--- a/arch/arm/boot/dts/da850.dtsi
+++ b/arch/arm/boot/dts/da850.dtsi
@@ -458,7 +458,7 @@
dma-names = "tx", "rx";
};
- display: display at 213000 {
+ lcdc: display at 213000 {
compatible = "ti,da850-tilcdc";
reg = <0x213000 0x1000>;
interrupts = <52>;
--
2.9.3
^ permalink raw reply related
* [PATCH v7 0/5] ARM: dts: da850: tilcdc related DT changes
From: Bartosz Golaszewski @ 2016-12-13 10:09 UTC (permalink / raw)
To: linux-arm-kernel
This series contains the last DT changes required for LCDC support
on da850-lcdk. The first one adds the dumb-vga-dac nodes, the second
limits the maximum pixel clock rate.
v1 -> v2:
- drop patch 3/3 (already merged)
- use max-pixelclock instead of max-bandwidth for display mode limiting
v2 -> v3:
- make the commit message in patch [2/2] more detailed
- move the max-pixelclock property to da850.dtsi as the limit
affects all da850-based boards
v3 -> v4:
- remove the input port from the display node
- move the display ports node to da850-lcdk.dts
- rename the vga_bridge node to vga-bridge
- move the LCDC pins to the LCDC node (from the vga bridge node)
v4 -> v5:
- rename the display label to lcdc
- instead of using the 'dumb-vga-dac' compatible, add bindings for
ti,ths8135 and use it as the vga-bridge node compatible
v5 -> v6:
- drop the endpoint numbers for nodes with single endpoints
- split patch 2/4 from v5 into two separate patches: one adding DT
bindings and one adding actual support for ths8135
v6 -> v7:
- simplified patch 4/5 by removing unnecessary properties from ports
and endpoints nodes
Bartosz Golaszewski (5):
ARM: dts: da850: rename the display node label
drm: bridge: add DT bindings for TI ths8135
drm: bridge: add support for TI ths8135
ARM: dts: da850-lcdk: add the vga-bridge node
ARM: dts: da850: specify the maximum pixel clock rate for tilcdc
.../bindings/display/bridge/ti,ths8135.txt | 46 +++++++++++++++++++
arch/arm/boot/dts/da850-lcdk.dts | 51 ++++++++++++++++++++++
arch/arm/boot/dts/da850.dtsi | 3 +-
drivers/gpu/drm/bridge/dumb-vga-dac.c | 1 +
4 files changed, 100 insertions(+), 1 deletion(-)
create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,ths8135.txt
--
2.9.3
^ permalink raw reply
* [PATCH v6 5/5] ARM: configs: stm32: Add I2C support for STM32 defconfig
From: Alexandre Torgue @ 2016-12-13 10:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481559342-6106-6-git-send-email-cedric.madianga@gmail.com>
Hi Cedric,
On 12/12/2016 05:15 PM, M'boumba Cedric Madianga wrote:
> Signed-off-by: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
Can you please add a commit message.
Thx in advance
Alex
> ---
> arch/arm/configs/stm32_defconfig | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/arch/arm/configs/stm32_defconfig b/arch/arm/configs/stm32_defconfig
> index e7b56d4..9494eaf 100644
> --- a/arch/arm/configs/stm32_defconfig
> +++ b/arch/arm/configs/stm32_defconfig
> @@ -52,6 +52,9 @@ CONFIG_SERIAL_NONSTANDARD=y
> CONFIG_SERIAL_STM32=y
> CONFIG_SERIAL_STM32_CONSOLE=y
> # CONFIG_HW_RANDOM is not set
> +CONFIG_I2C=y
> +CONFIG_I2C_CHARDEV=y
> +CONFIG_I2C_STM32F4=y
> # CONFIG_HWMON is not set
> # CONFIG_USB_SUPPORT is not set
> CONFIG_NEW_LEDS=y
>
^ permalink raw reply
* [PATCH v6 4/5] ARM: dts: stm32: Add I2C1 support for STM32429 eval board
From: Alexandre Torgue @ 2016-12-13 10:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481559342-6106-5-git-send-email-cedric.madianga@gmail.com>
Hi Cedric,
On 12/12/2016 05:15 PM, M'boumba Cedric Madianga wrote:
> Signed-off-by: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
Can you please add a commit message.
> ---
> arch/arm/boot/dts/stm32429i-eval.dts | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/arch/arm/boot/dts/stm32429i-eval.dts b/arch/arm/boot/dts/stm32429i-eval.dts
> index afb90bc..74e0045 100644
> --- a/arch/arm/boot/dts/stm32429i-eval.dts
> +++ b/arch/arm/boot/dts/stm32429i-eval.dts
> @@ -141,3 +141,9 @@
> pinctrl-names = "default";
> status = "okay";
> };
> +
> +&i2c1 {
> + pinctrl-0 = <&i2c1_pins_b>;
> + pinctrl-names = "default";
> + status = "okay";
> +};
>
^ permalink raw reply
* [PATCH RFC 2/2] ARM: nommu: remap exception base address to RAM
From: Russell King - ARM Linux @ 2016-12-13 10:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161211131255.3221-1-afzal.mohd.ma@gmail.com>
On Sun, Dec 11, 2016 at 06:42:55PM +0530, Afzal Mohammed wrote:
> Remap exception base address to start of RAM in Kernel in !MMU mode.
>
> Based on existing Kconfig help, Kernel was expecting it to be
> configured by external support. Also earlier it was not possible to
> copy the exception table to start of RAM due to Kconfig dependency,
> which has been fixed by a change prior to this.
>
> Kernel text start at an offset of at least 32K to account for page
> tables in MMU case. On a !MMU build too this space is kept aside, and
> since 2 pages (8K) is the maximum for exception plus stubs, it can be
> placed at the start of RAM.
>
> Signed-off-by: Afzal Mohammed <afzal.mohd.ma@gmail.com>
> ---
>
> i am a bit shaky about this change, though it works here on Cortex-A9,
> this probably would have to be made robust so as to not cause issue on
> other v7-A's upon trying to do !MMU (this won't affect normal MMU boot),
> or specifically where security extensions are not enabled. Also effect
> of hypervisor extension also need to be considered. Please let know if
> any better ways to handle this.
>
>
> arch/arm/Kconfig-nommu | 6 +++---
> arch/arm/kernel/head-nommu.S | 6 ++++++
> 2 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/Kconfig-nommu b/arch/arm/Kconfig-nommu
> index b7576349528c..f57fbe3d5eb0 100644
> --- a/arch/arm/Kconfig-nommu
> +++ b/arch/arm/Kconfig-nommu
> @@ -46,9 +46,9 @@ config REMAP_VECTORS_TO_RAM
> If your CPU provides a remap facility which allows the exception
> vectors to be mapped to writable memory, say 'n' here.
>
> - Otherwise, say 'y' here. In this case, the kernel will require
> - external support to redirect the hardware exception vectors to
> - the writable versions located at DRAM_BASE.
> + Otherwise, say 'y' here. In this case, the kernel will
> + redirect the hardware exception vectors to the writable
> + versions located at DRAM_BASE.
>
> config ARM_MPU
> bool 'Use the ARM v7 PMSA Compliant MPU'
> diff --git a/arch/arm/kernel/head-nommu.S b/arch/arm/kernel/head-nommu.S
> index 6b4eb27b8758..ac31c9647830 100644
> --- a/arch/arm/kernel/head-nommu.S
> +++ b/arch/arm/kernel/head-nommu.S
> @@ -158,6 +158,12 @@ __after_proc_init:
> bic r0, r0, #CR_V
> #endif
> mcr p15, 0, r0, c1, c0, 0 @ write control reg
> +
> +#ifdef CONFIG_REMAP_VECTORS_TO_RAM
> + mov r3, #CONFIG_VECTORS_BASE @ read VECTORS_BASE
> + mcr p15, 0, r3, c12, c0, 0 @ write to VBAR
> +#endif
> +
Is there really any need to do this in head.S ? I believe it's
entirely possible to do it later - arch/arm/mm/nommu.c:paging_init().
Also, if the region setup for the vectors was moved as well, it would
then be possible to check the ID registers to determine whether this
is supported, and make the decision where to locate the vectors base
more dynamically.
That leaves one pr_notice() call using the CONFIG_VECTORS_BASE
constant...
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply
* [PATCH v6 3/5] ARM: dts: stm32: Add I2C1 support for STM32F429 SoC
From: Alexandre Torgue @ 2016-12-13 10:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481559342-6106-4-git-send-email-cedric.madianga@gmail.com>
Hi Cedric,
On 12/12/2016 05:15 PM, M'boumba Cedric Madianga wrote:
> Signed-off-by: Patrice Chotard <patrice.chotard@st.com>
> Signed-off-by: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
Please Add a commit message.
> ---
> arch/arm/boot/dts/stm32f429.dtsi | 23 +++++++++++++++++++++++
> 1 file changed, 23 insertions(+)
>
> diff --git a/arch/arm/boot/dts/stm32f429.dtsi b/arch/arm/boot/dts/stm32f429.dtsi
> index 7de52ee..cbdece7 100644
> --- a/arch/arm/boot/dts/stm32f429.dtsi
> +++ b/arch/arm/boot/dts/stm32f429.dtsi
> @@ -48,6 +48,7 @@
> #include "skeleton.dtsi"
> #include "armv7-m.dtsi"
> #include <dt-bindings/pinctrl/stm32f429-pinfunc.h>
> +#include <dt-bindings/mfd/stm32f4-rcc.h>
>
> / {
> clocks {
> @@ -337,6 +338,16 @@
> slew-rate = <2>;
> };
> };
> +
> + i2c1_pins_b: i2c1 at 0 {
> + pins1 {
> + pinmux = <STM32F429_PB9_FUNC_I2C1_SDA>;
> + drive-open-drain;
> + };
> + pins2 {
> + pinmux = <STM32F429_PB6_FUNC_I2C1_SCL>;
> + };
> + };
> };
>
> rcc: rcc at 40023810 {
> @@ -409,6 +420,18 @@
> interrupts = <80>;
> clocks = <&rcc 0 38>;
> };
> +
> + i2c1: i2c at 40005400 {
Can you check the order on device node please ? (should follow base@)
> + compatible = "st,stm32f4-i2c";
> + reg = <0x40005400 0x400>;
> + interrupts = <31>,
> + <32>;
> + resets = <&rcc STM32F4_APB1_RESET(I2C1)>;
> + clocks = <&rcc 0 STM32F4_APB1_CLOCK(I2C1)>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + status = "disabled";
> + };
> };
> };
>
>
^ permalink raw reply
* Re: EXT: imx-sdma UART series failed for i.MX51
From: Alexander Shiyan @ 2016-12-13 10:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4f8cf2ee-3cd6-31ce-dc12-8a763f19f8e5@ge.com>
>???????????, 12 ??????? 2016, 16:48 +03:00 ?? "Han, Nandor (GE Healthcare)" <nandor.han@ge.com>:
>
>Hi Alexander,
>????Thanks for info. I have already posted a patch that probably will
>fix this issue.
>
>https://lkml.org/lkml/2016/10/11/319
>
>It was already tested on imx6 and imx53. Let me know how it works.
>
>Reference:
>http://lists-archives.com/linux-kernel/28679793-ext-pre-4-9-rc-dma-regression-imx6-sound-etc.html
Yes, it helps me, thanks!
---
^ permalink raw reply
* [PATCH 1/2] ARM: dts: dra7-evm: Remove pinmux configurations for erratum i869
From: Sekhar Nori @ 2016-12-13 9:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <b80e3cbc8a377abdb40ab11de78ce0b3fb67596f.1480411308.git.nsekhar@ti.com>
On Tuesday 29 November 2016 02:54 PM, Sekhar Nori wrote:
> A side-effect of this patch is that NAND support is removed. NAND
> pins clash with VOUT3 on DRA7-EVM. U-Boot selects VOUT3 over NAND
> as per TI EVM application needs.
I claimed this ..
> &gpmc {
> status = "okay";
> - pinctrl-names = "default";
> - pinctrl-0 = <&nand_flash_x16>;
> ranges = <0 0 0x08000000 0x01000000>; /* minimum GPMC partition = 16MB */
> nand at 0,0 {
> compatible = "ti,omap2-nand";
.. but forgot to keep the gpmc node disabled. I will fix this and send a
v2. dra72-evm-common.dtsi needs a similar patch. Will include that in v2
as well.
Thanks,
Sekhar
^ permalink raw reply
* [PATCH v1 2/3] clk: rockchip: add clock controller for rk3328
From: Shawn Lin @ 2016-12-13 9:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481618869-1239-3-git-send-email-zhangqing@rock-chips.com>
Hi, Elaine,
I always only keep an eye for mmc stuff here. :)
On 2016/12/13 16:47, Elaine Zhang wrote:
> Add the clock tree definition for the new rk3328 SoC.
>
> Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
> ---
----8<--------------
> +
> + /* PD_MMC */
> + MMC(SCLK_SDMMC_DRV, "sdmmc_drv", "sclk_sdmmc",
> + RK3328_SDMMC_CON0, 1),
> + MMC(SCLK_SDMMC_SAMPLE, "sdmmc_sample", "sclk_sdmmc",
> + RK3328_SDMMC_CON1, 0),
> +
All of offset for these *_sample are wrong, and they should be 1
, the same as *_drv. You could refer to P565 of TRM instead the
section of CRU for these details.
> + MMC(SCLK_SDIO_DRV, "sdio_drv", "sclk_sdio",
> + RK3328_SDIO_CON0, 1),
> + MMC(SCLK_SDIO_SAMPLE, "sdio_sample", "sclk_sdio",
> + RK3328_SDIO_CON1, 0),
> +
> + MMC(SCLK_EMMC_DRV, "emmc_drv", "sclk_emmc",
> + RK3328_EMMC_CON0, 1),
> + MMC(SCLK_EMMC_SAMPLE, "emmc_sample", "sclk_emmc",
> + RK3328_EMMC_CON1, 0),
> +
> + MMC(SCLK_SDMMC_EXT_DRV, "sdmmc_ext_drv", "sclk_sdmmc_ext",
> + RK3328_SDMMC_EXT_CON0, 1),
> + MMC(SCLK_SDMMC_EXT_SAMPLE, "sdmmc_ext_sample", "sclk_sdmmc_ext",
> + RK3328_SDMMC_EXT_CON1, 0),
> +};
> +
----8<--------
> +#define RK3328_SDMMC_CON0 0x380
> +#define RK3328_SDMMC_CON1 0x384
> +#define RK3328_SDIO_CON0 0x388
> +#define RK3328_SDIO_CON1 0x38c
> +#define RK3328_EMMC_CON0 0x390
> +#define RK3328_EMMC_CON1 0x394
> +#define RK3328_SDMMC_EXT_CON0 0x398
> +#define RK3328_SDMMC_EXT_CON1 0x39C
Just wondering is it worth, but this uppercase 'C' isn't
consistent with the former lowercase
> +
> #define RK3368_PLL_CON(x) RK2928_PLL_CON(x)
> #define RK3368_CLKSEL_CON(x) ((x) * 0x4 + 0x100)
> #define RK3368_CLKGATE_CON(x) ((x) * 0x4 + 0x200)
> @@ -130,6 +152,7 @@
> enum rockchip_pll_type {
> pll_rk3036,
> pll_rk3066,
> + pll_rk3328,
> pll_rk3399,
> };
>
>
--
Best Regards
Shawn Lin
^ permalink raw reply
* [PATCH RFC 2/2] ARM: nommu: remap exception base address to RAM
From: Vladimir Murzin @ 2016-12-13 9:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161211131255.3221-1-afzal.mohd.ma@gmail.com>
On 11/12/16 13:12, Afzal Mohammed wrote:
> Remap exception base address to start of RAM in Kernel in !MMU mode.
>
> Based on existing Kconfig help, Kernel was expecting it to be
> configured by external support. Also earlier it was not possible to
> copy the exception table to start of RAM due to Kconfig dependency,
> which has been fixed by a change prior to this.
>
> Kernel text start at an offset of at least 32K to account for page
> tables in MMU case. On a !MMU build too this space is kept aside, and
> since 2 pages (8K) is the maximum for exception plus stubs, it can be
> placed at the start of RAM.
>
> Signed-off-by: Afzal Mohammed <afzal.mohd.ma@gmail.com>
> ---
>
> i am a bit shaky about this change, though it works here on Cortex-A9,
> this probably would have to be made robust so as to not cause issue on
> other v7-A's upon trying to do !MMU (this won't affect normal MMU boot),
> or specifically where security extensions are not enabled. Also effect
> of hypervisor extension also need to be considered. Please let know if
> any better ways to handle this.
You might need to check ID_PFR1 for that.
>
>
> arch/arm/Kconfig-nommu | 6 +++---
> arch/arm/kernel/head-nommu.S | 6 ++++++
> 2 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/Kconfig-nommu b/arch/arm/Kconfig-nommu
> index b7576349528c..f57fbe3d5eb0 100644
> --- a/arch/arm/Kconfig-nommu
> +++ b/arch/arm/Kconfig-nommu
> @@ -46,9 +46,9 @@ config REMAP_VECTORS_TO_RAM
> If your CPU provides a remap facility which allows the exception
> vectors to be mapped to writable memory, say 'n' here.
>
> - Otherwise, say 'y' here. In this case, the kernel will require
> - external support to redirect the hardware exception vectors to
> - the writable versions located at DRAM_BASE.
> + Otherwise, say 'y' here. In this case, the kernel will
> + redirect the hardware exception vectors to the writable
> + versions located at DRAM_BASE.
>
> config ARM_MPU
> bool 'Use the ARM v7 PMSA Compliant MPU'
> diff --git a/arch/arm/kernel/head-nommu.S b/arch/arm/kernel/head-nommu.S
> index 6b4eb27b8758..ac31c9647830 100644
> --- a/arch/arm/kernel/head-nommu.S
> +++ b/arch/arm/kernel/head-nommu.S
> @@ -158,6 +158,12 @@ __after_proc_init:
> bic r0, r0, #CR_V
> #endif
> mcr p15, 0, r0, c1, c0, 0 @ write control reg
> +
> +#ifdef CONFIG_REMAP_VECTORS_TO_RAM
> + mov r3, #CONFIG_VECTORS_BASE @ read VECTORS_BASE
ldr r3,=CONFIG_VECTORS_BASE
would be more robust. I hit this in [1]
[1] https://www.spinics.net/lists/arm-kernel/msg546825.html
Cheers
Vladimir
> + mcr p15, 0, r3, c12, c0, 0 @ write to VBAR
> +#endif
> +
> #elif defined (CONFIG_CPU_V7M)
> /* For V7M systems we want to modify the CCR similarly to the SCTLR */
> #ifdef CONFIG_CPU_DCACHE_DISABLE
>
^ permalink raw reply
* [PATCH v6 7/8] ARM: dts: stm32: add Timers driver for stm32f429 MCU
From: Benjamin Gaignard @ 2016-12-13 9:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161212185943.ph7njaqb2lxtgdn4@rob-hp-laptop>
2016-12-12 19:59 GMT+01:00 Rob Herring <robh@kernel.org>:
> On Fri, Dec 09, 2016 at 03:15:18PM +0100, Benjamin Gaignard wrote:
>> Add Timers and it sub-nodes into DT for stm32f429 family.
>>
>> version 6:
>> - split patch in two: one for SoC family and one for stm32f469
>> discovery board.
>>
>> version 5:
>> - rename gptimer node to timers
>> - re-order timers node par addresses
>>
>> version 4:
>> - remove unwanted indexing in pwm@ and timer@ node name
>> - use "reg" instead of additional parameters to set timer
>> configuration
>>
>> version 3:
>> - use "st,stm32-timer-trigger" in DT
>>
>> version 2:
>> - use parameters to describe hardware capabilities
>> - do not use references for pwm and iio timer subnodes
>>
>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
>> ---
>> arch/arm/boot/dts/stm32f429.dtsi | 275 +++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 275 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/stm32f429.dtsi b/arch/arm/boot/dts/stm32f429.dtsi
>> index bca491d..d0fb9cc 100644
>> --- a/arch/arm/boot/dts/stm32f429.dtsi
>> +++ b/arch/arm/boot/dts/stm32f429.dtsi
>> @@ -355,6 +355,21 @@
>> slew-rate = <2>;
>> };
>> };
>> +
>> + pwm1_pins: pwm at 1 {
>
> No reg prop, so should not have a unit-address. Given the names in the
> define below, seems like "timer1" would be appropriate.
>
Here pins muxing is only targeting PWM part of the the MFD , that why I have
labeled it with "pwm".
>> + pins {
>> + pinmux = <STM32F429_PA8_FUNC_TIM1_CH1>,
>> + <STM32F429_PB13_FUNC_TIM1_CH1N>,
>> + <STM32F429_PB12_FUNC_TIM1_BKIN>;
>> + };
>> + };
>> +
>> + pwm3_pins: pwm at 3 {
>> + pins {
>> + pinmux = <STM32F429_PB4_FUNC_TIM3_CH1>,
>> + <STM32F429_PB5_FUNC_TIM3_CH2>;
>> + };
>> + };
>> };
>>
>> rcc: rcc at 40023810 {
>> @@ -426,6 +441,266 @@
>> interrupts = <80>;
>> clocks = <&rcc 0 38>;
>> };
>> +
>> + timers2: timers at 40000000 {
>
> timer at ...
>
> It may be more than just a timer, there's not a better generic name.
"timer" is already used in DT for clocksource driver.
"timers" cover "advanced-control", "generic" and "basic" hardware timers IPs,
which share the same registers mapping (only the level of feature are different)
>
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + compatible = "st,stm32-timers";
>> + reg = <0x40000000 0x400>;
>> + clocks = <&rcc 0 128>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm {
>> + compatible = "st,stm32-pwm";
>> + status = "disabled";
>> + };
>> +
>> + timer {
>> + compatible = "st,stm32-timer-trigger";
>> + reg = <1>;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + timers3: timers at 40000400 {
>
> ditto
>
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + compatible = "st,stm32-timers";
>> + reg = <0x40000400 0x400>;
>> + clocks = <&rcc 0 129>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm {
>> + compatible = "st,stm32-pwm";
>> + status = "disabled";
>> + };
>> +
>> + timer {
>> + compatible = "st,stm32-timer-trigger";
>> + reg = <2>;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + timers4: timers at 40000800 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + compatible = "st,stm32-timers";
>> + reg = <0x40000800 0x400>;
>> + clocks = <&rcc 0 130>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm {
>> + compatible = "st,stm32-pwm";
>> + status = "disabled";
>> + };
>> +
>> + timer {
>> + compatible = "st,stm32-timer-trigger";
>> + reg = <3>;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + timers5: timers at 40000C00 {
>
> timer at ...
>
> And use lowercase hex.
ok
>
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + compatible = "st,stm32-timers";
>> + reg = <0x40000C00 0x400>;
>
> ditto
>
>> + clocks = <&rcc 0 131>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm {
>> + compatible = "st,stm32-pwm";
>> + status = "disabled";
>> + };
>> +
>> + timer {
>> + compatible = "st,stm32-timer-trigger";
>> + reg = <4>;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + timers6: timers at 40001000 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + compatible = "st,stm32-timers";
>> + reg = <0x40001000 0x400>;
>> + clocks = <&rcc 0 132>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + timer {
>> + compatible = "st,stm32-timer-trigger";
>> + reg = <5>;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + timers7: timers at 40001400 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + compatible = "st,stm32-timers";
>> + reg = <0x40001400 0x400>;
>> + clocks = <&rcc 0 133>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + timer {
>> + compatible = "st,stm32-timer-trigger";
>> + reg = <6>;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + timers12: timers at 40001800 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + compatible = "st,stm32-timers";
>> + reg = <0x40001800 0x400>;
>> + clocks = <&rcc 0 134>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm {
>> + compatible = "st,stm32-pwm";
>> + status = "disabled";
>> + };
>> +
>> + timer {
>> + compatible = "st,stm32-timer-trigger";
>> + reg = <9>;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + timers13: timers at 40001C00 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + compatible = "st,stm32-timers";
>> + reg = <0x40001C00 0x400>;
>> + clocks = <&rcc 0 135>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm {
>> + compatible = "st,stm32-pwm";
>> + status = "disabled";
>> + };
>> + };
>> +
>> + timers14: timers at 40002000 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + compatible = "st,stm32-timers";
>> + reg = <0x40002000 0x400>;
>> + clocks = <&rcc 0 136>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm {
>> + compatible = "st,stm32-pwm";
>> + status = "disabled";
>> + };
>> + };
>> +
>> + timers1: timers at 40010000 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + compatible = "st,stm32-timers";
>> + reg = <0x40010000 0x400>;
>> + clocks = <&rcc 0 160>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm {
>> + compatible = "st,stm32-pwm";
>> + status = "disabled";
>> + };
>> +
>> + timer {
>> + compatible = "st,stm32-timer-trigger";
>> + reg = <0>;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + timers8: timers at 40010400 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + compatible = "st,stm32-timers";
>> + reg = <0x40010400 0x400>;
>> + clocks = <&rcc 0 161>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm {
>> + compatible = "st,stm32-pwm";
>> + status = "disabled";
>> + };
>> +
>> + timer {
>> + compatible = "st,stm32-timer-trigger";
>> + reg = <7>;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + timers9: timers at 40014000 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + compatible = "st,stm32-timers";
>> + reg = <0x40014000 0x400>;
>> + clocks = <&rcc 0 176>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm {
>> + compatible = "st,stm32-pwm";
>> + status = "disabled";
>> + };
>> +
>> + timer {
>> + compatible = "st,stm32-timer-trigger";
>> + reg = <8>;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + timers10: timers at 40014400 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + compatible = "st,stm32-timers";
>> + reg = <0x40014400 0x400>;
>> + clocks = <&rcc 0 177>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm {
>> + compatible = "st,stm32-pwm";
>> + status = "disabled";
>> + };
>> + };
>> +
>> + timers11: timers at 40014800 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + compatible = "st,stm32-timers";
>> + reg = <0x40014800 0x400>;
>> + clocks = <&rcc 0 178>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm {
>> + compatible = "st,stm32-pwm";
>> + status = "disabled";
>> + };
>> + };
>> };
>> };
>>
>> --
>> 1.9.1
>>
^ permalink raw reply
* [PATCH v6 1/8] MFD: add bindings for STM32 Timers driver
From: Benjamin Gaignard @ 2016-12-13 9:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161212185149.rt3xqpn3mbaavb4l@rob-hp-laptop>
2016-12-12 19:51 GMT+01:00 Rob Herring <robh@kernel.org>:
> On Fri, Dec 09, 2016 at 03:15:12PM +0100, Benjamin Gaignard wrote:
>> Add bindings information for STM32 Timers
>>
>> version 6:
>> - rename stm32-gtimer to stm32-timers
>> - change compatible
>> - add description about the IPs
>>
>> version 2:
>> - rename stm32-mfd-timer to stm32-gptimer
>> - only keep one compatible string
>>
>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
>> ---
>> .../devicetree/bindings/mfd/stm32-timers.txt | 46 ++++++++++++++++++++++
>> 1 file changed, 46 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/mfd/stm32-timers.txt
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/stm32-timers.txt b/Documentation/devicetree/bindings/mfd/stm32-timers.txt
>> new file mode 100644
>> index 0000000..b30868e
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mfd/stm32-timers.txt
>> @@ -0,0 +1,46 @@
>> +STM32 Timers driver bindings
>> +
>> +This IP provides 3 types of timer along with PWM functionality:
>> +- advanced-control timers consist of a 16-bit auto-reload counter driven by a programmable
>> + prescaler, break input feature, PWM outputs and complementary PWM ouputs channels.
>> +- general-purpose timers consist of a 16-bit or 32-bit auto-reload counter driven by a
>> + programmable prescaler and PWM outputs.
>> +- basic timers consist of a 16-bit auto-reload counter driven by a programmable prescaler.
>> +
>> +Required parameters:
>> +- compatible: must be "st,stm32-timers"
>> +
>> +- reg: Physical base address and length of the controller's
>> + registers.
>> +- clock-names: Set to "clk_int".
>
> 'clk' is redundant. Also, you don't really need -names when there is
> only one of them.
I use devm_regmap_init_mmio_clk() which get the clock by it name so
I have to define it in DT.
>> +- clocks: Phandle to the clock used by the timer module.
>> + For Clk properties, please refer to ../clock/clock-bindings.txt
>> +
>> +Optional parameters:
>> +- resets: Phandle to the parent reset controller.
>> + See ../reset/st,stm32-rcc.txt
>> +
>> +Optional subnodes:
>> +- pwm: See ../pwm/pwm-stm32.txt
>> +- timer: See ../iio/timer/stm32-timer-trigger.txt
>> +
>> +Example:
>> + timers at 40010000 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + compatible = "st,stm32-timers";
>> + reg = <0x40010000 0x400>;
>> + clocks = <&rcc 0 160>;
>> + clock-names = "clk_int";
>> +
>> + pwm {
>> + compatible = "st,stm32-pwm";
>> + pinctrl-0 = <&pwm1_pins>;
>> + pinctrl-names = "default";
>> + };
>> +
>> + timer {
>> + compatible = "st,stm32-timer-trigger";
>> + reg = <0>;
>
> You don't need reg here as there is only one. In turn, you don't need
> #address-cells or #size-cells.
I use "reg" to set each timer configuration.
>From hardware point of view they are all the same except for which hardware
signals they could consume and/or send.
"reg" is used as index of the two tables in driver code.
>
>> + };
>> + };
>> --
>> 1.9.1
>>
^ permalink raw reply
* [PATCH v6 2/5] i2c: Add STM32F4 I2C driver
From: Uwe Kleine-König @ 2016-12-13 9:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481559342-6106-3-git-send-email-cedric.madianga@gmail.com>
Hello,
On Mon, Dec 12, 2016 at 05:15:39PM +0100, M'boumba Cedric Madianga wrote:
> This patch adds support for the STM32F4 I2C controller.
>
> Signed-off-by: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
> ---
> drivers/i2c/busses/Kconfig | 10 +
> drivers/i2c/busses/Makefile | 1 +
> drivers/i2c/busses/i2c-stm32f4.c | 849 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 860 insertions(+)
> create mode 100644 drivers/i2c/busses/i2c-stm32f4.c
>
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index 0cdc844..2719208 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -886,6 +886,16 @@ config I2C_ST
> This driver can also be built as module. If so, the module
> will be called i2c-st.
>
> +config I2C_STM32F4
> + tristate "STMicroelectronics STM32F4 I2C support"
> + depends on ARCH_STM32 || COMPILE_TEST
> + help
> + Enable this option to add support for STM32 I2C controller embedded
> + in STM32F4 SoCs.
> +
> + This driver can also be built as module. If so, the module
> + will be called i2c-stm32f4.
> +
> config I2C_STU300
> tristate "ST Microelectronics DDC I2C interface"
> depends on MACH_U300
> diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
> index 1c1bac8..a2c6ff5 100644
> --- a/drivers/i2c/busses/Makefile
> +++ b/drivers/i2c/busses/Makefile
> @@ -85,6 +85,7 @@ obj-$(CONFIG_I2C_SH_MOBILE) += i2c-sh_mobile.o
> obj-$(CONFIG_I2C_SIMTEC) += i2c-simtec.o
> obj-$(CONFIG_I2C_SIRF) += i2c-sirf.o
> obj-$(CONFIG_I2C_ST) += i2c-st.o
> +obj-$(CONFIG_I2C_STM32F4) += i2c-stm32f4.o
> obj-$(CONFIG_I2C_STU300) += i2c-stu300.o
> obj-$(CONFIG_I2C_SUN6I_P2WI) += i2c-sun6i-p2wi.o
> obj-$(CONFIG_I2C_TEGRA) += i2c-tegra.o
> diff --git a/drivers/i2c/busses/i2c-stm32f4.c b/drivers/i2c/busses/i2c-stm32f4.c
> new file mode 100644
> index 0000000..89ad579
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-stm32f4.c
> @@ -0,0 +1,849 @@
> +/*
> + * Driver for STMicroelectronics STM32 I2C controller
> + *
> + * Copyright (C) M'boumba Cedric Madianga 2015
> + * Author: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
> + *
> + * This driver is based on i2c-st.c
> + *
> + * License terms: GNU General Public License (GPL), version 2
> + */
If there is a public description available for the device, a link here
would be great.
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset.h>
> +
> +/* STM32F4 I2C offset registers */
> +#define STM32F4_I2C_CR1 0x00
> +#define STM32F4_I2C_CR2 0x04
> +#define STM32F4_I2C_DR 0x10
> +#define STM32F4_I2C_SR1 0x14
> +#define STM32F4_I2C_SR2 0x18
> +#define STM32F4_I2C_CCR 0x1C
> +#define STM32F4_I2C_TRISE 0x20
> +#define STM32F4_I2C_FLTR 0x24
> +
> +/* STM32F4 I2C control 1*/
> +#define STM32F4_I2C_CR1_SWRST BIT(15)
> +#define STM32F4_I2C_CR1_POS BIT(11)
> +#define STM32F4_I2C_CR1_ACK BIT(10)
> +#define STM32F4_I2C_CR1_STOP BIT(9)
> +#define STM32F4_I2C_CR1_START BIT(8)
> +#define STM32F4_I2C_CR1_PE BIT(0)
> +
> +/* STM32F4 I2C control 2 */
> +#define STM32F4_I2C_CR2_FREQ_MASK GENMASK(5, 0)
> +#define STM32F4_I2C_CR2_FREQ(n) ((n & STM32F4_I2C_CR2_FREQ_MASK))
This should better be ((n) & STM32F4_I2C_CR2_FREQ_MASK). There a few
more constants that need the same fix.
> +#define STM32F4_I2C_CR2_ITBUFEN BIT(10)
> +#define STM32F4_I2C_CR2_ITEVTEN BIT(9)
> +#define STM32F4_I2C_CR2_ITERREN BIT(8)
> +#define STM32F4_I2C_CR2_IRQ_MASK (STM32F4_I2C_CR2_ITBUFEN \
> + | STM32F4_I2C_CR2_ITEVTEN \
> + | STM32F4_I2C_CR2_ITERREN)
I'd layout this like:
#define STM32F4_I2C_CR2_IRQ_MASK (STM32F4_I2C_CR2_ITBUFEN | \
STM32F4_I2C_CR2_ITEVTEN | \
STM32F4_I2C_CR2_ITERREN)
which is more usual I think.
> +/* STM32F4 I2C Status 1 */
> +#define STM32F4_I2C_SR1_AF BIT(10)
> +#define STM32F4_I2C_SR1_ARLO BIT(9)
> +#define STM32F4_I2C_SR1_BERR BIT(8)
> +#define STM32F4_I2C_SR1_TXE BIT(7)
> +#define STM32F4_I2C_SR1_RXNE BIT(6)
> +#define STM32F4_I2C_SR1_BTF BIT(2)
> +#define STM32F4_I2C_SR1_ADDR BIT(1)
> +#define STM32F4_I2C_SR1_SB BIT(0)
> +#define STM32F4_I2C_SR1_ITEVTEN_MASK (STM32F4_I2C_SR1_BTF \
> + | STM32F4_I2C_SR1_ADDR \
> + | STM32F4_I2C_SR1_SB)
> +#define STM32F4_I2C_SR1_ITBUFEN_MASK (STM32F4_I2C_SR1_TXE \
> + | STM32F4_I2C_SR1_RXNE)
> +#define STM32F4_I2C_SR1_ITERREN_MASK (STM32F4_I2C_SR1_AF \
> + | STM32F4_I2C_SR1_ARLO \
> + | STM32F4_I2C_SR1_BERR)
> +
> +/* STM32F4 I2C Status 2 */
> +#define STM32F4_I2C_SR2_BUSY BIT(1)
> +
> +/* STM32F4 I2C Control Clock */
> +#define STM32F4_I2C_CCR_CCR_MASK GENMASK(11, 0)
> +#define STM32F4_I2C_CCR_CCR(n) ((n & STM32F4_I2C_CCR_CCR_MASK))
> +#define STM32F4_I2C_CCR_FS BIT(15)
> +#define STM32F4_I2C_CCR_DUTY BIT(14)
> +
> +/* STM32F4 I2C Trise */
> +#define STM32F4_I2C_TRISE_VALUE_MASK GENMASK(5, 0)
> +#define STM32F4_I2C_TRISE_VALUE(n) ((n & STM32F4_I2C_TRISE_VALUE_MASK))
> +
> +/* STM32F4 I2C Filter */
> +#define STM32F4_I2C_FLTR_DNF_MASK GENMASK(3, 0)
> +#define STM32F4_I2C_FLTR_DNF(n) ((n & STM32F4_I2C_FLTR_DNF_MASK))
> +#define STM32F4_I2C_FLTR_ANOFF BIT(4)
> +
> +#define STM32F4_I2C_MIN_FREQ 2U
> +#define STM32F4_I2C_MAX_FREQ 42U
> +#define FAST_MODE_MAX_RISE_TIME 1000
> +#define STD_MODE_MAX_RISE_TIME 300
Are these supposed to be the values "rise time of both SDA and SCL
signals" from the i2c specification? If so, you got it wrong, fast mode
has the smaller value.
Maybe these constants could get a home in a more central place?
Also I'd add /* ns */ to the definition.
> +#define MHZ_TO_HZ 1000000
> +
> +enum stm32f4_i2c_speed {
> + STM32F4_I2C_SPEED_STANDARD, /* 100 kHz */
> + STM32F4_I2C_SPEED_FAST, /* 400 kHz */
> + STM32F4_I2C_SPEED_END,
> +};
> +
> +/**
> + * struct stm32f4_i2c_timings - per-Mode tuning parameters
> + * @duty: Fast mode duty cycle
> + * @mul_ccr: Value to be multiplied to CCR to reach 100Khz/400Khz SCL frequency
> + * @min_ccr: Minimum clock ctrl reg value to reach 100Khz/400Khz SCL frequency
> + */
> +struct stm32f4_i2c_timings {
> + u32 rate;
rate is undocumented and unused.
> + u32 duty;
> + u32 mul_ccr;
> + u32 min_ccr;
> +};
> +
> +/**
> + * struct stm32f4_i2c_msg - client specific data
> + * @addr: 8-bit slave addr, including r/w bit
> + * @count: number of bytes to be transferred
> + * @buf: data buffer
> + * @result: result of the transfer
> + * @stop: last I2C msg to be sent, i.e. STOP to be generated
> + */
> +struct stm32f4_i2c_msg {
> + u8 addr;
> + u32 count;
> + u8 *buf;
> + int result;
> + bool stop;
> +};
> +
> +/**
> + * struct stm32f4_i2c_dev - private data of the controller
> + * @adap: I2C adapter for this controller
> + * @dev: device for this controller
> + * @base: virtual memory area
> + * @complete: completion of I2C message
> + * @irq_event: interrupt event line for the controller
> + * @irq_error: interrupt error line for the controller
> + * @clk: hw i2c clock
> + * speed: I2C clock frequency of the controller. Standard or Fast only supported
> + * @msg: I2C transfer information
> + */
> +struct stm32f4_i2c_dev {
> + struct i2c_adapter adap;
> + struct device *dev;
> + void __iomem *base;
> + struct completion complete;
> + int irq_event;
> + int irq_error;
You only use irq_event in the probe function. So there is no need to
remember this one and you could use a local variable instead.
> + struct clk *clk;
> + int speed;
> + struct stm32f4_i2c_msg msg;
> +};
> +
> +static struct stm32f4_i2c_timings i2c_timings[] = {
> + [STM32F4_I2C_SPEED_STANDARD] = {
> + .mul_ccr = 1,
> + .min_ccr = 4,
> + .duty = 0,
> + },
> + [STM32F4_I2C_SPEED_FAST] = {
> + .mul_ccr = 16,
> + .min_ccr = 1,
> + .duty = 1,
> + },
Are these values from the datasheet?
> +};
> +
> +static inline void stm32f4_i2c_set_bits(void __iomem *reg, u32 mask)
> +{
> + writel_relaxed(readl_relaxed(reg) | mask, reg);
> +}
> +
> +static inline void stm32f4_i2c_clr_bits(void __iomem *reg, u32 mask)
> +{
> + writel_relaxed(readl_relaxed(reg) & ~mask, reg);
> +}
> +
> +static void stm32f4_i2c_soft_reset(struct stm32f4_i2c_dev *i2c_dev)
> +{
> + void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR1;
> +
> + stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_SWRST);
> + stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_SWRST);
Not very critical, but you're doing an unneeded register access here
because the register is read twice.
Also I think readability would improve if you dropped
stm32f4_i2c_{set,clr}_bits and do their logic explicitly in the callers.
stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_SWRST);
stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_SWRST);
vs
val = readl_relaxed(reg);
writel_relaxed(val | STM32F4_I2C_CR1_SWRST, reg);
writel_relaxed(val, reg);
> +}
> +
> +static void stm32f4_i2c_disable_it(struct stm32f4_i2c_dev *i2c_dev)
What is "it"? If it stands for "interrupt" the more usual abbrev is
"irq".
> +{
> + void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR2;
> +
> + stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR2_IRQ_MASK);
> +}
> +
> +static void stm32f4_i2c_set_periph_clk_freq(struct stm32f4_i2c_dev *i2c_dev)
> +{
> + u32 clk_rate, cr2, freq;
> +
> + cr2 = readl_relaxed(i2c_dev->base + STM32F4_I2C_CR2);
> + cr2 &= ~STM32F4_I2C_CR2_FREQ_MASK;
> + clk_rate = clk_get_rate(i2c_dev->clk);
> + freq = clk_rate / MHZ_TO_HZ;
> + freq = clamp(freq, STM32F4_I2C_MIN_FREQ, STM32F4_I2C_MAX_FREQ);
> + cr2 |= STM32F4_I2C_CR2_FREQ(freq);
> + writel_relaxed(cr2, i2c_dev->base + STM32F4_I2C_CR2);
Can you quote the data sheet enough in a comment here to make it obvious
that your calculation is right?
Would it be more sensible to error out if clk_rate / MHZ_TO_HZ isn't in
the interval [STM32F4_I2C_MIN_FREQ, STM32F4_I2C_MAX_FREQ]?
Usually I would expect that you need to use
DIV_ROUND_UP(clk_rate, MHZ_TO_HZ) instead of a plain division.
> +}
> +
> +static void stm32f4_i2c_set_rise_time(struct stm32f4_i2c_dev *i2c_dev)
> +{
> + u32 trise, freq, cr2, val;
> +
> + cr2 = readl_relaxed(i2c_dev->base + STM32F4_I2C_CR2);
> + freq = cr2 & STM32F4_I2C_CR2_FREQ_MASK;
> +
> + trise = readl_relaxed(i2c_dev->base + STM32F4_I2C_TRISE);
> + trise &= ~STM32F4_I2C_TRISE_VALUE_MASK;
Are you required to use rmw for STM32F4_I2C_TRISE? I'd prefer
writel_relaxed(STM32F4_I2C_TRISE_VALUE(..), i2c_dev->base + STM32F4_I2C_TRISE);
unless the datasheet requires rmw.
> + /* Maximum rise time computation */
> + if (i2c_dev->speed == STM32F4_I2C_SPEED_STANDARD) {
> + trise |= STM32F4_I2C_TRISE_VALUE((freq + 1));
A single pair of parenthesis is enough when you fix
STM32F4_I2C_TRISE_VALUE as suggested above.
> + } else {
> + val = freq * FAST_MODE_MAX_RISE_TIME / STD_MODE_MAX_RISE_TIME;
> + trise |= STM32F4_I2C_TRISE_VALUE((val + 1));
val could be local to this branch.
Or make it shorter using:
freq = cr2 & STM32F4_I2C_CR2_FREQ_MASK;
if (i2c_dev->speed == STM32F4_I2C_SPEED_FAST)
freq = freq * FAST_MODE_MAX_RISE_TIME / STD_MODE_MAX_RISE_TIME;
writel_relaxed(STM32F4_I2C_TRISE_VALUE(freq + 1), ...);
A quote from the data sheet about the algorithm would be good here, too.
> + }
> +
> + writel_relaxed(trise, i2c_dev->base + STM32F4_I2C_TRISE);
> +}
> +
> +static void stm32f4_i2c_set_speed_mode(struct stm32f4_i2c_dev *i2c_dev)
> +{
> + struct stm32f4_i2c_timings *t = &i2c_timings[i2c_dev->speed];
> + u32 ccr, clk_rate;
> + int val;
> +
> + ccr = readl_relaxed(i2c_dev->base + STM32F4_I2C_CCR);
> + ccr &= ~(STM32F4_I2C_CCR_FS | STM32F4_I2C_CCR_DUTY |
> + STM32F4_I2C_CCR_CCR_MASK);
> +
> + clk_rate = clk_get_rate(i2c_dev->clk);
> + val = clk_rate / MHZ_TO_HZ * t->mul_ccr;
Is the rounding done right? Again please describe the hardware in a
comment.
> + if (val < t->min_ccr)
> + val = t->min_ccr;
> + ccr |= STM32F4_I2C_CCR_CCR(val);
> +
> + if (t->duty)
> + ccr |= STM32F4_I2C_CCR_FS | STM32F4_I2C_CCR_DUTY;
> +
> + writel_relaxed(ccr, i2c_dev->base + STM32F4_I2C_CCR);
> +}
> +[...]
> +
> +static int stm32f4_i2c_wait_free_bus(struct stm32f4_i2c_dev *i2c_dev)
> +{
> + u32 status;
> + int ret;
> +
> + ret = readl_relaxed_poll_timeout(i2c_dev->base + STM32F4_I2C_SR2,
> + status,
> + !(status & STM32F4_I2C_SR2_BUSY),
> + 10, 1000);
> + if (ret) {
> + dev_err(i2c_dev->dev, "bus not free\n");
> + ret = -EBUSY;
I'm not sure if "bus not free" deserves an error message. Wolfram?
> + }
> +
> + return ret;
> +}
> +
> +[...]
> +static void stm32f4_i2c_read_msg(struct stm32f4_i2c_dev *i2c_dev)
> +{
> + struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
> + u32 rbuf;
> +
> + rbuf = readl_relaxed(i2c_dev->base + STM32F4_I2C_DR);
> + *msg->buf++ = (u8)rbuf & 0xff;
unneeded cast (or unneeded & 0xff).
> + msg->count--;
> +}
> +
> +[...]
> +/**
> + * stm32f4_i2c_handle_read() - Handle FIFO empty interrupt in case of read
> + * @i2c_dev: Controller's private data
> + */
> +static void stm32f4_i2c_handle_read(struct stm32f4_i2c_dev *i2c_dev)
> +{
> + struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
> + void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR2;
> +
> + switch (msg->count) {
> + case 1:
> + stm32f4_i2c_disable_it(i2c_dev);
> + stm32f4_i2c_read_msg(i2c_dev);
> + complete(&i2c_dev->complete);
> + break;
> + case 2:
> + case 3:
> + stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR2_ITBUFEN);
> + break;
> + default:
> + stm32f4_i2c_read_msg(i2c_dev);
> + }
It looks wrong that you don't call stm32f4_i2c_read_msg if msg->count is
2 or 3. I guess that's because these cases are handled in
stm32f4_i2c_handle_rx_btf? Maybe you can simplify the logic a bit?
> +}
> +
> +/**
> + * stm32f4_i2c_handle_rx_btf() - Handle byte transfer finished interrupt
> + * in case of read
> + * @i2c_dev: Controller's private data
> + */
> +static void stm32f4_i2c_handle_rx_btf(struct stm32f4_i2c_dev *i2c_dev)
> +{
> + struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
> + void __iomem *reg;
> + u32 mask;
> + int i;
> +
> + switch (msg->count) {
I don't understand why the handling depends on the number of messages.
> + case 2:
> + reg = i2c_dev->base + STM32F4_I2C_CR1;
> + /* Generate STOP or REPSTART */
I stumbled about "REPSTART" and would spell it out as "repeated Start".
> + if (msg->stop)
> + stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_STOP);
> + else
> + stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_START);
> +
> + /* Read two last data bytes */
> + for (i = 2; i > 0; i--)
> + stm32f4_i2c_read_msg(i2c_dev);
> +
> + /* Disable EVT and ERR interrupt */
> + reg = i2c_dev->base + STM32F4_I2C_CR2;
> + mask = STM32F4_I2C_CR2_ITEVTEN | STM32F4_I2C_CR2_ITERREN;
> + stm32f4_i2c_clr_bits(reg, mask);
> +
> + complete(&i2c_dev->complete);
> + break;
> + case 3:
> + /* Enable ACK and read data */
> + reg = i2c_dev->base + STM32F4_I2C_CR1;
> + stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_ACK);
> + stm32f4_i2c_read_msg(i2c_dev);
> + break;
> + default:
> + stm32f4_i2c_read_msg(i2c_dev);
> + }
> +}
> +
> +/**
> + * stm32f4_i2c_handle_rx_addr() - Handle address matched interrupt in case of
> + * master receiver
> + * @i2c_dev: Controller's private data
> + */
> +static void stm32f4_i2c_handle_rx_addr(struct stm32f4_i2c_dev *i2c_dev)
> +{
> + struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
> + void __iomem *reg;
> +
> + switch (msg->count) {
> + case 0:
> + stm32f4_i2c_terminate_xfer(i2c_dev);
> + /* Clear ADDR flag */
> + readl_relaxed(i2c_dev->base + STM32F4_I2C_SR2);
> + break;
> + case 1:
> + /*
> + * Single byte reception:
> + * Enable NACK, clear ADDR flag and generate STOP or RepSTART
> + */
> + reg = i2c_dev->base + STM32F4_I2C_CR1;
> + stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_ACK);
> + if (msg->stop)
> + stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_STOP);
> + else
> + stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_START);
> + break;
> + case 2:
> + /*
> + * 2-byte reception:
> + * Enable NACK and PEC Position Ack and clear ADDR flag
What is PEC?
> + */
> + reg = i2c_dev->base + STM32F4_I2C_CR1;
> + stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_ACK);
> + stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_POS);
> + readl_relaxed(i2c_dev->base + STM32F4_I2C_SR2);
> + break;
> +
> + default:
> + /* N-byte reception: Enable ACK and clear ADDR flag */
> + reg = i2c_dev->base + STM32F4_I2C_CR1;
> + stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_ACK);
> + readl_relaxed(i2c_dev->base + STM32F4_I2C_SR2);
> + break;
> + }
> +}
> +
> +/**
> + * stm32f4_i2c_isr_event() - Interrupt routine for I2C bus event
> + * @irq: interrupt number
> + * @data: Controller's private data
> + */
> +static irqreturn_t stm32f4_i2c_isr_event(int irq, void *data)
> +{
> +[...]
> + real_status = readl_relaxed(i2c_dev->base + STM32F4_I2C_SR1);
s/real_status/status/ ?
> +
> + if (!(real_status & possible_status)) {
> + dev_dbg(i2c_dev->dev,
> + "spurious evt it (status=0x%08x, ien=0x%08x)\n",
> + real_status, ien);
s/it/irq/
> + return IRQ_NONE;
> + }
> +
> + /* Use __fls() to check error bits first */
> + flag = __fls(real_status & possible_status);
If you get several events reported you only handle a single one. Is this
effective?
> + switch (1 << flag) {
> + case STM32F4_I2C_SR1_SB:
> + stm32f4_i2c_write_byte(i2c_dev, msg->addr);
> + break;
> +
> + case STM32F4_I2C_SR1_ADDR:
> + if (msg->addr & I2C_M_RD)
> + stm32f4_i2c_handle_rx_addr(i2c_dev);
> + else
> + readl_relaxed(i2c_dev->base + STM32F4_I2C_SR2);
> +
> + /* Enable ITBUF interrupts */
What is ITBUF?
> + reg = i2c_dev->base + STM32F4_I2C_CR2;
> + stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR2_ITBUFEN);
> + break;
> +
> + case STM32F4_I2C_SR1_BTF:
> + if (msg->addr & I2C_M_RD)
> + stm32f4_i2c_handle_rx_btf(i2c_dev);
> + else
> + stm32f4_i2c_handle_write(i2c_dev);
> + break;
> +
> + case STM32F4_I2C_SR1_TXE:
> + stm32f4_i2c_handle_write(i2c_dev);
> + break;
> +
> + case STM32F4_I2C_SR1_RXNE:
> + stm32f4_i2c_handle_read(i2c_dev);
> + break;
> +
> + default:
> + dev_err(i2c_dev->dev,
> + "evt it unhandled: status=0x%08x)\n", real_status);
s/it/irq/
> + return IRQ_NONE;
> + }
> +
> + return IRQ_HANDLED;
> +}
> +
> +[...]
> +static int stm32f4_i2c_xfer_msg(struct stm32f4_i2c_dev *i2c_dev,
> + struct i2c_msg *msg, bool is_first,
> + bool is_last)
> +{
> +[...]
> + /* Enable ITEVT and ITERR interrupts */
This comment isn't helpful. Mentioning their meaning would be great
instead.
> +[...]
> +static int stm32f4_i2c_xfer(struct i2c_adapter *i2c_adap, struct i2c_msg msgs[],
> + int num)
> +{
> + struct stm32f4_i2c_dev *i2c_dev = i2c_get_adapdata(i2c_adap);
> + int ret, i;
> +
> + ret = clk_enable(i2c_dev->clk);
> + if (ret) {
> + dev_err(i2c_dev->dev, "Failed to enable clock\n");
> + return ret;
> + }
> +
> + stm32f4_i2c_hw_config(i2c_dev);
> +
> + for (i = 0; i < num && !ret; i++)
> + ret = stm32f4_i2c_xfer_msg(i2c_dev, &msgs[i], i == 0,
> + i == num - 1);
> +
> + clk_disable(i2c_dev->clk);
> +
> + return (ret < 0) ? ret : i;
using num instead of i would be a bit more obvious.
> +static int stm32f4_i2c_probe(struct platform_device *pdev)
> +{
> +[...]
> + i2c_dev->speed = STM32F4_I2C_SPEED_STANDARD;
> + ret = of_property_read_u32(np, "clock-frequency", &clk_rate);
> + if ((!ret) && (clk_rate == 400000))
> + i2c_dev->speed = STM32F4_I2C_SPEED_FAST;
I'd use
if (!ret && clk_rate >= 400000)
i2c_dev->speed = STM32F4_I2C_SPEED_FAST;
. That's less parenthesis and a more robust selection of the bus
frequency.
> +
> + i2c_dev->dev = &pdev->dev;
> +
> + ret = devm_request_threaded_irq(&pdev->dev, i2c_dev->irq_event,
> + NULL, stm32f4_i2c_isr_event,
> + IRQF_ONESHOT, pdev->name, i2c_dev);
> + if (ret) {
> + dev_err(&pdev->dev, "Failed to request irq %i\n",
> + i2c_dev->irq_error);
That's wrong. Requesting irq_event failed.
> + goto clk_free;
> + }
> +
> + ret = devm_request_threaded_irq(&pdev->dev, i2c_dev->irq_error,
> + NULL, stm32f4_i2c_isr_error,
> + IRQF_ONESHOT, pdev->name, i2c_dev);
> + if (ret) {
> + dev_err(&pdev->dev, "Failed to request irq %i\n",
> + i2c_dev->irq_error);
> + goto clk_free;
It would also be nice to know for which type of irq this failed. I.e.
please point out if this is the error irq or the event irq in the
message. Ditto for checking the return type of irq_of_parse_and_map.
> + }
> +
> + adap = &i2c_dev->adap;
> + i2c_set_adapdata(adap, i2c_dev);
> + snprintf(adap->name, sizeof(adap->name), "STM32 I2C(%pa)", &res->start);
> + adap->owner = THIS_MODULE;
> + adap->timeout = 2 * HZ;
> + adap->retries = 0;
> + adap->algo = &stm32f4_i2c_algo;
> + adap->dev.parent = &pdev->dev;
> + adap->dev.of_node = pdev->dev.of_node;
> +
> + init_completion(&i2c_dev->complete);
> +
> + ret = i2c_add_adapter(adap);
> + if (ret)
> + goto clk_free;
> +
> + platform_set_drvdata(pdev, i2c_dev);
> +
> + dev_info(i2c_dev->dev, "STM32F4 I2C driver initialized\n");
This is wrong. The driver is bound now to a device, not initialized.
> +static const struct of_device_id stm32f4_i2c_match[] = {
> + { .compatible = "st,stm32f4-i2c", },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, stm32f4_i2c_match);
> +
> +static struct platform_driver stm32f4_i2c_driver = {
> + .driver = {
> + .name = "stm32f4-i2c",
> + .of_match_table = stm32f4_i2c_match,
Is this needed?
> + },
> + .probe = stm32f4_i2c_probe,
> + .remove = stm32f4_i2c_remove,
> +};
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* [PATCH 1/2] ARM: nommu: allow enabling REMAP_VECTORS_TO_RAM
From: Vladimir Murzin @ 2016-12-13 9:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161211131158.3120-1-afzal.mohd.ma@gmail.com>
On 11/12/16 13:11, Afzal Mohammed wrote:
> REMAP_VECTORS_TO_RAM depends on DRAM_BASE, but since DRAM_BASE is a
> hex, REMAP_VECTORS_TO_RAM could never get enabled. Also depending on
> DRAM_BASE is redundant as whenever REMAP_VECTORS_TO_RAM makes itself
> available to Kconfig, DRAM_BASE also is available as the Kconfig gets
> sourced on !MMU.
>
> Signed-off-by: Afzal Mohammed <afzal.mohd.ma@gmail.com>
> ---
> arch/arm/Kconfig-nommu | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/arch/arm/Kconfig-nommu b/arch/arm/Kconfig-nommu
> index aed66d5df7f1..b7576349528c 100644
> --- a/arch/arm/Kconfig-nommu
> +++ b/arch/arm/Kconfig-nommu
> @@ -34,8 +34,7 @@ config PROCESSOR_ID
> used instead of the auto-probing which utilizes the register.
>
> config REMAP_VECTORS_TO_RAM
> - bool 'Install vectors to the beginning of RAM' if DRAM_BASE
> - depends on DRAM_BASE
> + bool 'Install vectors to the beginning of RAM'
> help
> The kernel needs to change the hardware exception vectors.
> In nommu mode, the hardware exception vectors are normally
>
I have similar change in my local tree, so FWIW:
Reviewed-by: Vladimir Murzin <vladimir.murzin@arm.com>
^ permalink raw reply
* [PATCH v6 7/8] ARM: dts: stm32: add Timers driver for stm32f429 MCU
From: Benjamin Gaignard @ 2016-12-13 9:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161212185943.ph7njaqb2lxtgdn4@rob-hp-laptop>
2016-12-12 19:59 GMT+01:00 Rob Herring <robh@kernel.org>:
> On Fri, Dec 09, 2016 at 03:15:18PM +0100, Benjamin Gaignard wrote:
>> Add Timers and it sub-nodes into DT for stm32f429 family.
>>
>> version 6:
>> - split patch in two: one for SoC family and one for stm32f469
>> discovery board.
>>
>> version 5:
>> - rename gptimer node to timers
>> - re-order timers node par addresses
>>
>> version 4:
>> - remove unwanted indexing in pwm@ and timer@ node name
>> - use "reg" instead of additional parameters to set timer
>> configuration
>>
>> version 3:
>> - use "st,stm32-timer-trigger" in DT
>>
>> version 2:
>> - use parameters to describe hardware capabilities
>> - do not use references for pwm and iio timer subnodes
>>
>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
>> ---
>> arch/arm/boot/dts/stm32f429.dtsi | 275 +++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 275 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/stm32f429.dtsi b/arch/arm/boot/dts/stm32f429.dtsi
>> index bca491d..d0fb9cc 100644
>> --- a/arch/arm/boot/dts/stm32f429.dtsi
>> +++ b/arch/arm/boot/dts/stm32f429.dtsi
>> @@ -355,6 +355,21 @@
>> slew-rate = <2>;
>> };
>> };
>> +
>> + pwm1_pins: pwm at 1 {
>
> No reg prop, so should not have a unit-address. Given the names in the
> define below, seems like "timer1" would be appropriate.
>
>> + pins {
>> + pinmux = <STM32F429_PA8_FUNC_TIM1_CH1>,
>> + <STM32F429_PB13_FUNC_TIM1_CH1N>,
>> + <STM32F429_PB12_FUNC_TIM1_BKIN>;
>> + };
>> + };
>> +
>> + pwm3_pins: pwm at 3 {
>> + pins {
>> + pinmux = <STM32F429_PB4_FUNC_TIM3_CH1>,
>> + <STM32F429_PB5_FUNC_TIM3_CH2>;
>> + };
>> + };
>> };
>>
>> rcc: rcc at 40023810 {
>> @@ -426,6 +441,266 @@
>> interrupts = <80>;
>> clocks = <&rcc 0 38>;
>> };
>> +
>> + timers2: timers at 40000000 {
>
> timer at ...
>
> It may be more than just a timer, there's not a better generic name.
>
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + compatible = "st,stm32-timers";
>> + reg = <0x40000000 0x400>;
>> + clocks = <&rcc 0 128>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm {
>> + compatible = "st,stm32-pwm";
>> + status = "disabled";
>> + };
>> +
>> + timer {
>> + compatible = "st,stm32-timer-trigger";
>> + reg = <1>;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + timers3: timers at 40000400 {
>
> ditto
>
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + compatible = "st,stm32-timers";
>> + reg = <0x40000400 0x400>;
>> + clocks = <&rcc 0 129>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm {
>> + compatible = "st,stm32-pwm";
>> + status = "disabled";
>> + };
>> +
>> + timer {
>> + compatible = "st,stm32-timer-trigger";
>> + reg = <2>;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + timers4: timers at 40000800 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + compatible = "st,stm32-timers";
>> + reg = <0x40000800 0x400>;
>> + clocks = <&rcc 0 130>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm {
>> + compatible = "st,stm32-pwm";
>> + status = "disabled";
>> + };
>> +
>> + timer {
>> + compatible = "st,stm32-timer-trigger";
>> + reg = <3>;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + timers5: timers at 40000C00 {
>
> timer at ...
>
> And use lowercase hex.
>
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + compatible = "st,stm32-timers";
>> + reg = <0x40000C00 0x400>;
>
> ditto
>
>> + clocks = <&rcc 0 131>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm {
>> + compatible = "st,stm32-pwm";
>> + status = "disabled";
>> + };
>> +
>> + timer {
>> + compatible = "st,stm32-timer-trigger";
>> + reg = <4>;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + timers6: timers at 40001000 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + compatible = "st,stm32-timers";
>> + reg = <0x40001000 0x400>;
>> + clocks = <&rcc 0 132>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + timer {
>> + compatible = "st,stm32-timer-trigger";
>> + reg = <5>;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + timers7: timers at 40001400 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + compatible = "st,stm32-timers";
>> + reg = <0x40001400 0x400>;
>> + clocks = <&rcc 0 133>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + timer {
>> + compatible = "st,stm32-timer-trigger";
>> + reg = <6>;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + timers12: timers at 40001800 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + compatible = "st,stm32-timers";
>> + reg = <0x40001800 0x400>;
>> + clocks = <&rcc 0 134>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm {
>> + compatible = "st,stm32-pwm";
>> + status = "disabled";
>> + };
>> +
>> + timer {
>> + compatible = "st,stm32-timer-trigger";
>> + reg = <9>;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + timers13: timers at 40001C00 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + compatible = "st,stm32-timers";
>> + reg = <0x40001C00 0x400>;
>> + clocks = <&rcc 0 135>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm {
>> + compatible = "st,stm32-pwm";
>> + status = "disabled";
>> + };
>> + };
>> +
>> + timers14: timers at 40002000 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + compatible = "st,stm32-timers";
>> + reg = <0x40002000 0x400>;
>> + clocks = <&rcc 0 136>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm {
>> + compatible = "st,stm32-pwm";
>> + status = "disabled";
>> + };
>> + };
>> +
>> + timers1: timers at 40010000 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + compatible = "st,stm32-timers";
>> + reg = <0x40010000 0x400>;
>> + clocks = <&rcc 0 160>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm {
>> + compatible = "st,stm32-pwm";
>> + status = "disabled";
>> + };
>> +
>> + timer {
>> + compatible = "st,stm32-timer-trigger";
>> + reg = <0>;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + timers8: timers at 40010400 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + compatible = "st,stm32-timers";
>> + reg = <0x40010400 0x400>;
>> + clocks = <&rcc 0 161>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm {
>> + compatible = "st,stm32-pwm";
>> + status = "disabled";
>> + };
>> +
>> + timer {
>> + compatible = "st,stm32-timer-trigger";
>> + reg = <7>;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + timers9: timers at 40014000 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + compatible = "st,stm32-timers";
>> + reg = <0x40014000 0x400>;
>> + clocks = <&rcc 0 176>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm {
>> + compatible = "st,stm32-pwm";
>> + status = "disabled";
>> + };
>> +
>> + timer {
>> + compatible = "st,stm32-timer-trigger";
>> + reg = <8>;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + timers10: timers at 40014400 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + compatible = "st,stm32-timers";
>> + reg = <0x40014400 0x400>;
>> + clocks = <&rcc 0 177>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm {
>> + compatible = "st,stm32-pwm";
>> + status = "disabled";
>> + };
>> + };
>> +
>> + timers11: timers at 40014800 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + compatible = "st,stm32-timers";
>> + reg = <0x40014800 0x400>;
>> + clocks = <&rcc 0 178>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm {
>> + compatible = "st,stm32-pwm";
>> + status = "disabled";
>> + };
>> + };
>> };
>> };
>>
>> --
>> 1.9.1
>>
--
Benjamin Gaignard
Graphic Study Group
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* [PATCH v2] PCI: Add information about describing PCI in ACPI
From: Jon Masters @ 2016-12-13 9:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161129213955.15663.21173.stgit@bhelgaas-glaptop.roam.corp.google.com>
On 11/29/2016 04:39 PM, Bjorn Helgaas wrote:
> +New architectures should be able to use "Consumer" Extended Address Space
> +descriptors in the PNP0A03 device for bridge registers, including ECAM,
> +although a strict interpretation of [6] might prohibit this. Old x86 and
> +ia64 kernels assume all address space descriptors, including "Consumer"
> +Extended Address Space ones, are windows, so it would not be safe to
> +describe bridge registers this way on those architectures.
<snip>
> +[6] PCI Firmware 3.0, sec 4.1.2:
<snip>
Thanks for the revised writeup, Bjorn. It's great. I'm trying to get the
above clarified explicitly in terms of the spec, and in terms of what
other Operating Systems would like to see as general preference.
To your point about second generation ARM (server) systems: we're actually
on generation 3+ now and finally getting to the point where people are
listening. A great many times over the past few years, people have had
to be sat on until they did what was needed. Fortunately, we are going
to finally have upstream kernels (and distros based upon them) that
boot out of the box on compliant hardware and will be able to point
people at the usual "upstream first" messaging we've been pushing.
I had originally fallen for the SoC koolaid that PCIe was not essential,
and was convinced fairly early that this was nonsense. But it has taken
a few years for everyone else to get onto that bandwagon. First you give
them exactly what they know and love (a 1-2 socket Xeon class machine
with lots of PCIe lanes), then you go and fix the design to give them
what they actually need (which logically enumerates as PCIe but isn't) ;)
Jon.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox