* Re: [PATCH v2 2/4] arm64: vdso32: Detect binutils support for dmb ishld
From: Vincenzo Frascino @ 2019-09-26 21:17 UTC (permalink / raw)
To: Catalin Marinas
Cc: ard.biesheuvel, ndesaulniers, linux-kernel, tglx, will,
linux-arm-kernel
In-Reply-To: <20190926155938.GM9689@arrakis.emea.arm.com>
On 9/26/19 4:59 PM, Catalin Marinas wrote:
> On Thu, Sep 26, 2019 at 02:38:03PM +0100, Vincenzo Frascino wrote:
>> arch/arm64/kernel/vdso32/Makefile | 9 +++++++++
>
> Could you please also remove the unnecessary gcc-goto.sh check in this
> file? We don't use jump labels in the vdso (can't run-time patch them).
> I found it while forcing COMPATCC=clang with my additional diff and I
> get the warning on 'make clean'.
>
I will do it in a separate cleanup patch, I want to keep this only for the error
reported by Will.
> diff --git a/arch/arm64/kernel/vdso32/Makefile b/arch/arm64/kernel/vdso32/Makefile
> index 22f0d31ea528..038357a1e835 100644
> --- a/arch/arm64/kernel/vdso32/Makefile
> +++ b/arch/arm64/kernel/vdso32/Makefile
> @@ -40,9 +38,6 @@ VDSO_CAFLAGS += $(call cc32-option,-fno-PIE)
> ifdef CONFIG_DEBUG_INFO
> VDSO_CAFLAGS += -g
> endif
> -ifeq ($(shell $(CONFIG_SHELL) $(srctree)/scripts/gcc-goto.sh $(COMPATCC)), y)
> -VDSO_CAFLAGS += -DCC_HAVE_ASM_GOTO
> -endif
>
> # From arm Makefile
> VDSO_CAFLAGS += $(call cc32-option,-fno-dwarf2-cfi-asm)
>
--
Regards,
Vincenzo
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC PATCH 00/18] crypto: wireguard using the existing crypto API
From: Andy Lutomirski @ 2019-09-26 21:36 UTC (permalink / raw)
To: Jason A. Donenfeld
Cc: Catalin Marinas, Herbert Xu, Arnd Bergmann, Ard Biesheuvel,
Greg KH, Eric Biggers, Samuel Neves, Will Deacon,
Linux Crypto Mailing List, Andy Lutomirski, Marc Zyngier,
Dan Carpenter, Linus Torvalds, David Miller, linux-arm-kernel
In-Reply-To: <CAHmME9rKFUvsQ6hhsKjxxVSnyNQsTaqBKGABoHibCiCBmfxCOA@mail.gmail.com>
On Thu, Sep 26, 2019 at 1:52 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> Hi Ard,
>
>
> Our goals are that chacha20_arch() from each of these arch glues gets
> included in the lib/crypto/chacha20.c compilation unit. The reason why
> we want it in its own unit is so that the inliner can get rid of
> unreached code and more tightly integrate the branches. For the MIPS
> case, the advantage is clear.
IMO this needs numbers. My suggestion from way back, which is at
least a good deal of the way toward being doable, is to do static
calls. This means that the common code will call out to the arch code
via a regular CALL instruction and will *not* inline the arch code.
This means that the arch code could live in its own module, it can be
selected at boot time, etc. For x86, inlining seems a but nuts to
avoid a whole mess of:
if (use avx2)
do_avx2_thing();
else if (use avx1)
do_avx1_thing();
else
etc;
On x86, direct calls are pretty cheap. Certainly for operations like
curve25519, I doubt you will ever see a real-world effect from
inlining. I'd be surprised for chacha20. If you really want inlining
to dictate the overall design, I think you need some real numbers for
why it's necessary. There also needs to be a clear story for how
exactly making everything inline plays with the actual decision of
which implementation to use. I think it's also worth noting that LTO
is coming.
--Andy
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v3 0/5]arm64: vdso32: Address various issues
From: Vincenzo Frascino @ 2019-09-26 21:43 UTC (permalink / raw)
To: linux-arm-kernel, linux-kernel
Cc: ard.biesheuvel, catalin.marinas, ndesaulniers, tglx,
vincenzo.frascino, will
In-Reply-To: <20190920142738.qlsjwguc6bpnez63@willie-the-truck>
Hi Will,
this patch series is meant to address the various compilation issues you
reported about arm64 vdso32.
Please let me know if there is still something missing.
Thanks,
Vincenzo
Cc: Will Deacon <will@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
v3:
- Exposed COMPATCC
- Addressed Review Comments
v2:
- Fixed binutils detection
- Addressed review comments
Vincenzo Frascino (5):
arm64: vdso32: Introduce COMPAT_CC_IS_GCC
arm64: vdso32: Detect binutils support for dmb ishld
arm64: vdso32: Fix compilation warning
arm64: Remove gettimeofday.S
arm64: vdso32: Remove jump label config option in Makefile
arch/arm64/Kconfig | 5 ++++-
arch/arm64/Makefile | 18 +++++-------------
arch/arm64/include/asm/memory.h | 3 +++
arch/arm64/include/asm/vdso/compat_barrier.h | 2 +-
arch/arm64/kernel/vdso/gettimeofday.S | 0
arch/arm64/kernel/vdso32/Makefile | 14 +++++++++-----
6 files changed, 22 insertions(+), 20 deletions(-)
delete mode 100644 arch/arm64/kernel/vdso/gettimeofday.S
--
2.23.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v3 1/5] arm64: vdso32: Introduce COMPAT_CC_IS_GCC
From: Vincenzo Frascino @ 2019-09-26 21:43 UTC (permalink / raw)
To: linux-arm-kernel, linux-kernel
Cc: ard.biesheuvel, catalin.marinas, ndesaulniers, tglx,
vincenzo.frascino, will
In-Reply-To: <20190926214342.34608-1-vincenzo.frascino@arm.com>
As reported by Will Deacon the .config file and the generated
include/config/auto.conf can end up out of sync after a set of
commands since CONFIG_CROSS_COMPILE_COMPAT_VDSO is not updated
correctly.
The sequence can be reproduced as follows:
$ make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- defconfig
[...]
$ make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- menuconfig
[set CONFIG_CROSS_COMPILE_COMPAT_VDSO="arm-linux-gnueabihf-"]
$ make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu-
Which results in:
arch/arm64/Makefile:62: CROSS_COMPILE_COMPAT not defined or empty,
the compat vDSO will not be built
even though the compat vDSO has been built:
$ file arch/arm64/kernel/vdso32/vdso.so
arch/arm64/kernel/vdso32/vdso.so: ELF 32-bit LSB pie executable, ARM,
EABI5 version 1 (SYSV), dynamically linked,
BuildID[sha1]=c67f6c786f2d2d6f86c71f708595594aa25247f6, stripped
A similar case that involves changing the configuration parameter multiple
times can be reconducted to the same family of problems.
The reason behind it comes from the fact that the master Makefile includes
that architecture Makefile twice, once before the syncconfig and one after.
Since the synchronization of the files happens only upon syncconfig, the
architecture Makefile included before this event does not see the change in
configuration.
As a consequence of this it is not possible to handle the cross compiler
definitions inside the architecture Makefile.
Address the problem removing CONFIG_CROSS_COMPILE_COMPAT_VDSO and moving
the detection of the correct compiler into Kconfig via COMPAT_CC_IS_GCC.
As a consequence of this it is not possible anymore to set the compat
cross compiler from menuconfig but it requires to be exported via
command line.
E.g.:
$ make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu-
CROSS_COMPILE_COMPAT=arm-linux-gnueabihf
Cc: Will Deacon <will@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
---
arch/arm64/Kconfig | 5 ++++-
arch/arm64/Makefile | 18 +++++-------------
arch/arm64/kernel/vdso32/Makefile | 2 --
3 files changed, 9 insertions(+), 16 deletions(-)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 37c610963eee..0e5beb928af5 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -110,7 +110,7 @@ config ARM64
select GENERIC_STRNLEN_USER
select GENERIC_TIME_VSYSCALL
select GENERIC_GETTIMEOFDAY
- select GENERIC_COMPAT_VDSO if (!CPU_BIG_ENDIAN && COMPAT)
+ select GENERIC_COMPAT_VDSO if (!CPU_BIG_ENDIAN && COMPAT && COMPATCC_IS_ARM_GCC)
select HANDLE_DOMAIN_IRQ
select HARDIRQS_SW_RESEND
select HAVE_PCI
@@ -313,6 +313,9 @@ config KASAN_SHADOW_OFFSET
default 0xeffffff900000000 if ARM64_VA_BITS_36 && KASAN_SW_TAGS
default 0xffffffffffffffff
+config COMPATCC_IS_ARM_GCC
+ def_bool $(success,$(COMPATCC) --version | head -n 1 | grep -q "arm-.*-gcc")
+
source "arch/arm64/Kconfig.platforms"
menu "Kernel Features"
diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
index 84a3d502c5a5..dfa6a5cb99e4 100644
--- a/arch/arm64/Makefile
+++ b/arch/arm64/Makefile
@@ -53,20 +53,12 @@ $(warning Detected assembler with broken .inst; disassembly will be unreliable)
endif
endif
+COMPATCC ?= $(CROSS_COMPILE_COMPAT)gcc
+export COMPATCC
+
ifeq ($(CONFIG_GENERIC_COMPAT_VDSO), y)
- CROSS_COMPILE_COMPAT ?= $(CONFIG_CROSS_COMPILE_COMPAT_VDSO:"%"=%)
-
- ifeq ($(CONFIG_CC_IS_CLANG), y)
- $(warning CROSS_COMPILE_COMPAT is clang, the compat vDSO will not be built)
- else ifeq ($(strip $(CROSS_COMPILE_COMPAT)),)
- $(warning CROSS_COMPILE_COMPAT not defined or empty, the compat vDSO will not be built)
- else ifeq ($(shell which $(CROSS_COMPILE_COMPAT)gcc 2> /dev/null),)
- $(error $(CROSS_COMPILE_COMPAT)gcc not found, check CROSS_COMPILE_COMPAT)
- else
- export CROSS_COMPILE_COMPAT
- export CONFIG_COMPAT_VDSO := y
- compat_vdso := -DCONFIG_COMPAT_VDSO=1
- endif
+ export CONFIG_COMPAT_VDSO := y
+ compat_vdso := -DCONFIG_COMPAT_VDSO=1
endif
KBUILD_CFLAGS += -mgeneral-regs-only $(lseinstr) $(brokengasinst) \
diff --git a/arch/arm64/kernel/vdso32/Makefile b/arch/arm64/kernel/vdso32/Makefile
index 1fba0776ed40..19e0d3115ffe 100644
--- a/arch/arm64/kernel/vdso32/Makefile
+++ b/arch/arm64/kernel/vdso32/Makefile
@@ -8,8 +8,6 @@
ARCH_REL_TYPE_ABS := R_ARM_JUMP_SLOT|R_ARM_GLOB_DAT|R_ARM_ABS32
include $(srctree)/lib/vdso/Makefile
-COMPATCC := $(CROSS_COMPILE_COMPAT)gcc
-
# Same as cc-*option, but using COMPATCC instead of CC
cc32-option = $(call try-run,\
$(COMPATCC) $(1) -c -x c /dev/null -o "$$TMP",$(1),$(2))
--
2.23.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v3 2/5] arm64: vdso32: Detect binutils support for dmb ishld
From: Vincenzo Frascino @ 2019-09-26 21:43 UTC (permalink / raw)
To: linux-arm-kernel, linux-kernel
Cc: ard.biesheuvel, catalin.marinas, ndesaulniers, tglx,
vincenzo.frascino, will
In-Reply-To: <20190926214342.34608-1-vincenzo.frascino@arm.com>
As reported by Will Deacon, older versions of binutils that do not
support certain types of memory barriers can cause build failure of the
vdso32 library.
Add a compilation time mechanism that detects if binutils supports those
instructions and configure the kernel accordingly.
Cc: Will Deacon <will@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
---
arch/arm64/include/asm/vdso/compat_barrier.h | 2 +-
arch/arm64/kernel/vdso32/Makefile | 9 +++++++++
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/include/asm/vdso/compat_barrier.h b/arch/arm64/include/asm/vdso/compat_barrier.h
index fb60a88b5ed4..3fd8fd6d8fc2 100644
--- a/arch/arm64/include/asm/vdso/compat_barrier.h
+++ b/arch/arm64/include/asm/vdso/compat_barrier.h
@@ -20,7 +20,7 @@
#define dmb(option) __asm__ __volatile__ ("dmb " #option : : : "memory")
-#if __LINUX_ARM_ARCH__ >= 8
+#if __LINUX_ARM_ARCH__ >= 8 && defined(CONFIG_AS_DMB_ISHLD)
#define aarch32_smp_mb() dmb(ish)
#define aarch32_smp_rmb() dmb(ishld)
#define aarch32_smp_wmb() dmb(ishst)
diff --git a/arch/arm64/kernel/vdso32/Makefile b/arch/arm64/kernel/vdso32/Makefile
index 19e0d3115ffe..77aa61340374 100644
--- a/arch/arm64/kernel/vdso32/Makefile
+++ b/arch/arm64/kernel/vdso32/Makefile
@@ -15,6 +15,8 @@ cc32-disable-warning = $(call try-run,\
$(COMPATCC) -W$(strip $(1)) -c -x c /dev/null -o "$$TMP",-Wno-$(strip $(1)))
cc32-ldoption = $(call try-run,\
$(COMPATCC) $(1) -nostdlib -x c /dev/null -o "$$TMP",$(1),$(2))
+cc32-as-instr = $(call try-run,\
+ printf "%b\n" "$(1)" | $(COMPATCC) $(VDSO_AFLAGS) -c -x assembler -o "$$TMP" -,$(2),$(3))
# We cannot use the global flags to compile the vDSO files, the main reason
# being that the 32-bit compiler may be older than the main (64-bit) compiler
@@ -53,6 +55,7 @@ endif
VDSO_CAFLAGS += -fPIC -fno-builtin -fno-stack-protector
VDSO_CAFLAGS += -DDISABLE_BRANCH_PROFILING
+
# Try to compile for ARMv8. If the compiler is too old and doesn't support it,
# fall back to v7. There is no easy way to check for what architecture the code
# is being compiled, so define a macro specifying that (see arch/arm/Makefile).
@@ -89,6 +92,12 @@ VDSO_CFLAGS += -Wno-int-to-pointer-cast
VDSO_AFLAGS := $(VDSO_CAFLAGS)
VDSO_AFLAGS += -D__ASSEMBLY__
+# Check for binutils support for dmb ishld
+dmbinstr := $(call cc32-as-instr,dmb ishld,-DCONFIG_AS_DMB_ISHLD=1)
+
+VDSO_CFLAGS += $(dmbinstr)
+VDSO_AFLAGS += $(dmbinstr)
+
VDSO_LDFLAGS := $(VDSO_CPPFLAGS)
# From arm vDSO Makefile
VDSO_LDFLAGS += -Wl,-Bsymbolic -Wl,--no-undefined -Wl,-soname=linux-vdso.so.1
--
2.23.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v3 3/5] arm64: vdso32: Fix compilation warning
From: Vincenzo Frascino @ 2019-09-26 21:43 UTC (permalink / raw)
To: linux-arm-kernel, linux-kernel
Cc: ard.biesheuvel, catalin.marinas, ndesaulniers, tglx,
vincenzo.frascino, will
In-Reply-To: <20190926214342.34608-1-vincenzo.frascino@arm.com>
As reported by Will Deacon the following compilation warning appears
during the compilation of the vdso32:
In file included from ./arch/arm64/include/asm/thread_info.h:17:0,
from ./include/linux/thread_info.h:38,
from ./arch/arm64/include/asm/preempt.h:5,
from ./include/linux/preempt.h:78,
from ./include/linux/spinlock.h:51,
from ./include/linux/seqlock.h:36,
from ./include/linux/time.h:6,
from .../work/linux/lib/vdso/gettimeofday.c:7,
from <command-line>:0:
./arch/arm64/include/asm/memory.h: In function ‘__tag_set’:
./arch/arm64/include/asm/memory.h:233:15: warning: cast from pointer to
integer of different size [-Wpointer-to-int-cast]
u64 __addr = (u64)addr & ~__tag_shifted(0xff);
^
In file included from ./arch/arm64/include/asm/pgtable-hwdef.h:8:0,
from ./arch/arm64/include/asm/processor.h:34,
from ./arch/arm64/include/asm/elf.h:118,
from ./include/linux/elf.h:5,
from ./include/linux/elfnote.h:62,
from arch/arm64/kernel/vdso32/note.c:11:
./arch/arm64/include/asm/memory.h: In function ‘__tag_set’:
./arch/arm64/include/asm/memory.h:233:15: warning: cast from pointer to
integer of different size [-Wpointer-to-int-cast]
u64 __addr = (u64)addr & ~__tag_shifted(0xff);
^
This happens because few 64 bit compilation headers are included during
the generation of vdso32.
Fix the issue redefining the __tag_set function.
Note: This fix is meant to be temporary, a more comprehensive solution
based on the refactoring of the generic headers will be provided with a
future patch set. At that point it will be possible to revert this patch.
Cc: Will Deacon <will@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
---
arch/arm64/include/asm/memory.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
index b61b50bf68b1..c61b2eb50a3b 100644
--- a/arch/arm64/include/asm/memory.h
+++ b/arch/arm64/include/asm/memory.h
@@ -228,11 +228,14 @@ static inline unsigned long kaslr_offset(void)
#define __tag_get(addr) 0
#endif /* CONFIG_KASAN_SW_TAGS */
+#ifdef __aarch64__
+/* Do not attempt to compile this code with compat vdso */
static inline const void *__tag_set(const void *addr, u8 tag)
{
u64 __addr = (u64)addr & ~__tag_shifted(0xff);
return (const void *)(__addr | __tag_shifted(tag));
}
+#endif
/*
* Physical vs virtual RAM address space conversion. These are
--
2.23.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v3 4/5] arm64: Remove gettimeofday.S
From: Vincenzo Frascino @ 2019-09-26 21:43 UTC (permalink / raw)
To: linux-arm-kernel, linux-kernel
Cc: ard.biesheuvel, catalin.marinas, ndesaulniers, tglx,
vincenzo.frascino, will
In-Reply-To: <20190926214342.34608-1-vincenzo.frascino@arm.com>
gettimeofday.S was originally removed with the introduction of the
support for Unified vDSOs in arm64 and replaced with the C
implementation.
The file seems again present in the repository due to a side effect of
rebase.
Remove the file again.
Cc: Will Deacon <will@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
---
arch/arm64/kernel/vdso/gettimeofday.S | 0
1 file changed, 0 insertions(+), 0 deletions(-)
delete mode 100644 arch/arm64/kernel/vdso/gettimeofday.S
diff --git a/arch/arm64/kernel/vdso/gettimeofday.S b/arch/arm64/kernel/vdso/gettimeofday.S
deleted file mode 100644
index e69de29bb2d1..000000000000
--
2.23.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v3 5/5] arm64: vdso32: Remove jump label config option in Makefile
From: Vincenzo Frascino @ 2019-09-26 21:43 UTC (permalink / raw)
To: linux-arm-kernel, linux-kernel
Cc: ard.biesheuvel, catalin.marinas, ndesaulniers, tglx,
vincenzo.frascino, will
In-Reply-To: <20190926214342.34608-1-vincenzo.frascino@arm.com>
The jump labels are not used in vdso32 since it is not possible to run
runtime patching on them.
Remove the configuration option from the Makefile.
Cc: Will Deacon <will@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
---
arch/arm64/kernel/vdso32/Makefile | 3 ---
1 file changed, 3 deletions(-)
diff --git a/arch/arm64/kernel/vdso32/Makefile b/arch/arm64/kernel/vdso32/Makefile
index 77aa61340374..038357a1e835 100644
--- a/arch/arm64/kernel/vdso32/Makefile
+++ b/arch/arm64/kernel/vdso32/Makefile
@@ -38,9 +38,6 @@ VDSO_CAFLAGS += $(call cc32-option,-fno-PIE)
ifdef CONFIG_DEBUG_INFO
VDSO_CAFLAGS += -g
endif
-ifeq ($(shell $(CONFIG_SHELL) $(srctree)/scripts/gcc-goto.sh $(COMPATCC)), y)
-VDSO_CAFLAGS += -DCC_HAVE_ASM_GOTO
-endif
# From arm Makefile
VDSO_CAFLAGS += $(call cc32-option,-fno-dwarf2-cfi-asm)
--
2.23.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH 05/11] PCI: versatile: Use pci_parse_request_of_pci_ranges()
From: Rob Herring @ 2019-09-26 21:44 UTC (permalink / raw)
To: Andrew Murray
Cc: Bjorn Helgaas, linux-pci, Lorenzo Pieralisi,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <20190925103752.GS9720@e119886-lin.cambridge.arm.com>
On Wed, Sep 25, 2019 at 5:37 AM Andrew Murray <andrew.murray@arm.com> wrote:
>
> On Tue, Sep 24, 2019 at 04:46:24PM -0500, Rob Herring wrote:
> > Convert ARM Versatile host bridge to use the common
> > pci_parse_request_of_pci_ranges().
> >
> > Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > Signed-off-by: Rob Herring <robh@kernel.org>
> > ---
> > drivers/pci/controller/pci-versatile.c | 62 +++++---------------------
> > 1 file changed, 11 insertions(+), 51 deletions(-)
> >
> > diff --git a/drivers/pci/controller/pci-versatile.c b/drivers/pci/controller/pci-versatile.c
> > index f59ad2728c0b..237b1abb26f2 100644
> > --- a/drivers/pci/controller/pci-versatile.c
> > +++ b/drivers/pci/controller/pci-versatile.c
> > @@ -62,60 +62,12 @@ static struct pci_ops pci_versatile_ops = {
> > .write = pci_generic_config_write,
> > };
> >
> > -static int versatile_pci_parse_request_of_pci_ranges(struct device *dev,
> > - struct list_head *res)
> > -{
> > - int err, mem = 1, res_valid = 0;
> > - resource_size_t iobase;
> > - struct resource_entry *win, *tmp;
> > -
> > - err = devm_of_pci_get_host_bridge_resources(dev, 0, 0xff, res, &iobase);
> > - if (err)
> > - return err;
> > -
> > - err = devm_request_pci_bus_resources(dev, res);
> > - if (err)
> > - goto out_release_res;
> > -
> > - resource_list_for_each_entry_safe(win, tmp, res) {
> > - struct resource *res = win->res;
> > -
> > - switch (resource_type(res)) {
> > - case IORESOURCE_IO:
> > - err = devm_pci_remap_iospace(dev, res, iobase);
> > - if (err) {
> > - dev_warn(dev, "error %d: failed to map resource %pR\n",
> > - err, res);
> > - resource_list_destroy_entry(win);
> > - }
> > - break;
> > - case IORESOURCE_MEM:
> > - res_valid |= !(res->flags & IORESOURCE_PREFETCH);
> > -
> > - writel(res->start >> 28, PCI_IMAP(mem));
> > - writel(PHYS_OFFSET >> 28, PCI_SMAP(mem));
> > - mem++;
> > -
> > - break;
> > - }
> > - }
> > -
> > - if (res_valid)
> > - return 0;
> > -
> > - dev_err(dev, "non-prefetchable memory resource required\n");
> > - err = -EINVAL;
> > -
> > -out_release_res:
> > - pci_free_resource_list(res);
> > - return err;
> > -}
> > -
> > static int versatile_pci_probe(struct platform_device *pdev)
> > {
> > struct device *dev = &pdev->dev;
> > struct resource *res;
> > - int ret, i, myslot = -1;
> > + struct resource_entry *entry;
> > + int ret, i, myslot = -1, mem = 0;
>
> I think 'mem' should be initialised to 1, at least that's what the original
> code did. However I'm not sure why it should start from 1.
The original code I moved from arch/arm had 32MB @ 0x0c000000 called
"PCI unused" which was requested with request_resource(), but never
provided to the PCI core. Otherwise, I kept the setup the same. No one
has complained in 4 years, though I'm not sure anyone would have
noticed if I just deleted PCI support...
Rob
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v5 3/3] PM / AVS: SVS: Introduce SVS engine
From: Kevin Hilman @ 2019-09-26 22:39 UTC (permalink / raw)
To: Roger Lu, Kevin Hilman, Rob Herring, Nicolas Boichat,
Stephen Boyd
Cc: Mark Rutland, Nishanth Menon, Angus Lin, devicetree, linux-pm,
Roger Lu, linux-kernel, HenryC Chen, yt.lee, Fan Chen,
linux-mediatek, Matthias Brugger, linux-arm-kernel
In-Reply-To: <20190906100514.30803-4-roger.lu@mediatek.com>
Hi Roger,
Roger Lu <roger.lu@mediatek.com> writes:
> The SVS (Smart Voltage Scaling) engine is a piece of hardware which is
> used to calculate optimized voltage values of several power domains, e.g.
> CPU/GPU/CCI, according to chip process corner, temperatures, and other
> factors. Then DVFS driver could apply those optimized voltage values to
> reduce power consumption.
>
> Signed-off-by: Roger Lu <roger.lu@mediatek.com>
I don't have any documentation on this SVS hardware, so I won't be able
to comment on the specific functionality, so I attempted a first-pass
high-level review...
In the absence of documentation, it would be helpful to give a high
level description of how this hardware works. In particular, a
high-level overview of the control loop, which appears to be primarily
interrupt driven, would be helpful. e.g. how often are the interrupts
firing, and why.
It's hard to understand the "init01" vs the "init02" patch since
those names aren't very descriptive. :)
Also, there are lots of hard-coded constants (masks, shifts, etc.) used
throughout the calculations. #defines could help readability for many
of these, especially the masks.
> ---
> drivers/power/avs/Kconfig | 10 +
> drivers/power/avs/Makefile | 1 +
> drivers/power/avs/mtk_svs.c | 2075 +++++++++++++++++++++++++++++++++
> include/linux/power/mtk_svs.h | 23 +
> 4 files changed, 2109 insertions(+)
> create mode 100644 drivers/power/avs/mtk_svs.c
> create mode 100644 include/linux/power/mtk_svs.h
>
> diff --git a/drivers/power/avs/Kconfig b/drivers/power/avs/Kconfig
> index b5a217b828dc..7c72504d5593 100644
> --- a/drivers/power/avs/Kconfig
> +++ b/drivers/power/avs/Kconfig
> @@ -19,3 +19,13 @@ config ROCKCHIP_IODOMAIN
> Say y here to enable support io domains on Rockchip SoCs. It is
> necessary for the io domain setting of the SoC to match the
> voltage supplied by the regulators.
> +
> +config MTK_SVS
> + bool "MediaTek Smart Voltage Scaling(SVS)"
> + depends on POWER_AVS && MTK_EFUSE
> + help
> + The SVS engine is a piece of hardware which is used to calculate
> + optimized voltage values of several power domains, e.g.
> + CPU clusters/GPU/CCI, according to chip process corner, temperatures,
> + and other factors. Then DVFS driver could apply those optimized voltage
> + values to reduce power consumption.
> diff --git a/drivers/power/avs/Makefile b/drivers/power/avs/Makefile
> index a1b8cd453f19..57246b977a93 100644
> --- a/drivers/power/avs/Makefile
> +++ b/drivers/power/avs/Makefile
> @@ -1,3 +1,4 @@
> # SPDX-License-Identifier: GPL-2.0-only
> obj-$(CONFIG_POWER_AVS_OMAP) += smartreflex.o
> obj-$(CONFIG_ROCKCHIP_IODOMAIN) += rockchip-io-domain.o
> +obj-$(CONFIG_MTK_SVS) += mtk_svs.o
> diff --git a/drivers/power/avs/mtk_svs.c b/drivers/power/avs/mtk_svs.c
> new file mode 100644
> index 000000000000..78ec93c3a4a5
> --- /dev/null
> +++ b/drivers/power/avs/mtk_svs.c
> @@ -0,0 +1,2075 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2018 MediaTek Inc.
> + */
> +
> +#define pr_fmt(fmt) "[mtk_svs] " fmt
> +
> +#include <linux/clk.h>
> +#include <linux/completion.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/kthread.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/nvmem-consumer.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
> +#include <linux/pm_opp.h>
> +#include <linux/pm_qos.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/power/mtk_svs.h>
> +#include <linux/proc_fs.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/seq_file.h>
> +#include <linux/spinlock.h>
> +#include <linux/thermal.h>
> +#include <linux/uaccess.h>
> +
> +#define SVS_INIT01_VOLT_IGNORE 1
> +#define SVS_INIT01_VOLT_INC_ONLY 2
> +
> +#define SVS_PHASE_INIT01 0
> +#define SVS_PHASE_INIT02 1
> +#define SVS_PHASE_MON 2
> +#define SVS_PHASE_ERROR 3
> +
> +#define SVS_CPU_LITTLE 1
> +#define SVS_CPU_BIG 2
> +#define SVS_CCI 3
> +#define SVS_GPU 4
> +
> +#define proc_fops_rw(name) \
> + static int name ## _proc_open(struct inode *inode, \
> + struct file *file) \
> + { \
> + return single_open(file, name ## _proc_show, \
> + PDE_DATA(inode)); \
> + } \
> + static const struct file_operations name ## _proc_fops = { \
> + .owner = THIS_MODULE, \
> + .open = name ## _proc_open, \
> + .read = seq_read, \
> + .llseek = seq_lseek, \
> + .release = single_release, \
> + .write = name ## _proc_write, \
> + }
> +
> +#define proc_fops_ro(name) \
> + static int name ## _proc_open(struct inode *inode, \
> + struct file *file) \
> + { \
> + return single_open(file, name ## _proc_show, \
> + PDE_DATA(inode)); \
> + } \
> + static const struct file_operations name ## _proc_fops = { \
> + .owner = THIS_MODULE, \
> + .open = name ## _proc_open, \
> + .read = seq_read, \
> + .llseek = seq_lseek, \
> + .release = single_release, \
> + }
> +
> +#define proc_entry(name) {__stringify(name), &name ## _proc_fops}
> +
> +static DEFINE_SPINLOCK(mtk_svs_lock);
> +struct mtk_svs;
> +
> +enum reg_index {
> + TEMPMONCTL0 = 0,
> + TEMPMONCTL1,
> + TEMPMONCTL2,
> + TEMPMONINT,
> + TEMPMONINTSTS,
> + TEMPMONIDET0,
> + TEMPMONIDET1,
> + TEMPMONIDET2,
> + TEMPH2NTHRE,
> + TEMPHTHRE,
> + TEMPCTHRE,
> + TEMPOFFSETH,
> + TEMPOFFSETL,
> + TEMPMSRCTL0,
> + TEMPMSRCTL1,
> + TEMPAHBPOLL,
> + TEMPAHBTO,
> + TEMPADCPNP0,
> + TEMPADCPNP1,
> + TEMPADCPNP2,
> + TEMPADCMUX,
> + TEMPADCEXT,
> + TEMPADCEXT1,
> + TEMPADCEN,
> + TEMPPNPMUXADDR,
> + TEMPADCMUXADDR,
> + TEMPADCEXTADDR,
> + TEMPADCEXT1ADDR,
> + TEMPADCENADDR,
> + TEMPADCVALIDADDR,
> + TEMPADCVOLTADDR,
> + TEMPRDCTRL,
> + TEMPADCVALIDMASK,
> + TEMPADCVOLTAGESHIFT,
> + TEMPADCWRITECTRL,
> + TEMPMSR0,
> + TEMPMSR1,
> + TEMPMSR2,
> + TEMPADCHADDR,
> + TEMPIMMD0,
> + TEMPIMMD1,
> + TEMPIMMD2,
> + TEMPMONIDET3,
> + TEMPADCPNP3,
> + TEMPMSR3,
> + TEMPIMMD3,
> + TEMPPROTCTL,
> + TEMPPROTTA,
> + TEMPPROTTB,
> + TEMPPROTTC,
> + TEMPSPARE0,
> + TEMPSPARE1,
> + TEMPSPARE2,
> + TEMPSPARE3,
> + TEMPMSR0_1,
> + TEMPMSR1_1,
> + TEMPMSR2_1,
> + TEMPMSR3_1,
> + DESCHAR,
> + TEMPCHAR,
> + DETCHAR,
> + AGECHAR,
> + DCCONFIG,
> + AGECONFIG,
> + FREQPCT30,
> + FREQPCT74,
> + LIMITVALS,
> + VBOOT,
> + DETWINDOW,
> + CONFIG,
> + TSCALCS,
> + RUNCONFIG,
> + SVSEN,
> + INIT2VALS,
> + DCVALUES,
> + AGEVALUES,
> + VOP30,
> + VOP74,
> + TEMP,
> + INTSTS,
> + INTSTSRAW,
> + INTEN,
> + CHKINT,
> + CHKSHIFT,
> + STATUS,
> + VDESIGN30,
> + VDESIGN74,
> + DVT30,
> + DVT74,
> + AGECOUNT,
> + SMSTATE0,
> + SMSTATE1,
> + CTL0,
> + DESDETSEC,
> + TEMPAGESEC,
> + CTRLSPARE0,
> + CTRLSPARE1,
> + CTRLSPARE2,
> + CTRLSPARE3,
> + CORESEL,
> + THERMINTST,
> + INTST,
> + THSTAGE0ST,
> + THSTAGE1ST,
> + THSTAGE2ST,
> + THAHBST0,
> + THAHBST1,
> + SPARE0,
> + SPARE1,
> + SPARE2,
> + SPARE3,
> + THSLPEVEB,
> + reg_num,
> +};
> +
> +static const u32 svs_regs_v2[] = {
> + [TEMPMONCTL0] = 0x000,
> + [TEMPMONCTL1] = 0x004,
> + [TEMPMONCTL2] = 0x008,
> + [TEMPMONINT] = 0x00c,
> + [TEMPMONINTSTS] = 0x010,
> + [TEMPMONIDET0] = 0x014,
> + [TEMPMONIDET1] = 0x018,
> + [TEMPMONIDET2] = 0x01c,
> + [TEMPH2NTHRE] = 0x024,
> + [TEMPHTHRE] = 0x028,
> + [TEMPCTHRE] = 0x02c,
> + [TEMPOFFSETH] = 0x030,
> + [TEMPOFFSETL] = 0x034,
> + [TEMPMSRCTL0] = 0x038,
> + [TEMPMSRCTL1] = 0x03c,
> + [TEMPAHBPOLL] = 0x040,
> + [TEMPAHBTO] = 0x044,
> + [TEMPADCPNP0] = 0x048,
> + [TEMPADCPNP1] = 0x04c,
> + [TEMPADCPNP2] = 0x050,
> + [TEMPADCMUX] = 0x054,
> + [TEMPADCEXT] = 0x058,
> + [TEMPADCEXT1] = 0x05c,
> + [TEMPADCEN] = 0x060,
> + [TEMPPNPMUXADDR] = 0x064,
> + [TEMPADCMUXADDR] = 0x068,
> + [TEMPADCEXTADDR] = 0x06c,
> + [TEMPADCEXT1ADDR] = 0x070,
> + [TEMPADCENADDR] = 0x074,
> + [TEMPADCVALIDADDR] = 0x078,
> + [TEMPADCVOLTADDR] = 0x07c,
> + [TEMPRDCTRL] = 0x080,
> + [TEMPADCVALIDMASK] = 0x084,
> + [TEMPADCVOLTAGESHIFT] = 0x088,
> + [TEMPADCWRITECTRL] = 0x08c,
> + [TEMPMSR0] = 0x090,
> + [TEMPMSR1] = 0x094,
> + [TEMPMSR2] = 0x098,
> + [TEMPADCHADDR] = 0x09c,
> + [TEMPIMMD0] = 0x0a0,
> + [TEMPIMMD1] = 0x0a4,
> + [TEMPIMMD2] = 0x0a8,
> + [TEMPMONIDET3] = 0x0b0,
> + [TEMPADCPNP3] = 0x0b4,
> + [TEMPMSR3] = 0x0b8,
> + [TEMPIMMD3] = 0x0bc,
> + [TEMPPROTCTL] = 0x0c0,
> + [TEMPPROTTA] = 0x0c4,
> + [TEMPPROTTB] = 0x0c8,
> + [TEMPPROTTC] = 0x0cc,
> + [TEMPSPARE0] = 0x0f0,
> + [TEMPSPARE1] = 0x0f4,
> + [TEMPSPARE2] = 0x0f8,
> + [TEMPSPARE3] = 0x0fc,
> + [TEMPMSR0_1] = 0x190,
> + [TEMPMSR1_1] = 0x194,
> + [TEMPMSR2_1] = 0x198,
> + [TEMPMSR3_1] = 0x1b8,
> + [DESCHAR] = 0xc00,
> + [TEMPCHAR] = 0xc04,
> + [DETCHAR] = 0xc08,
> + [AGECHAR] = 0xc0c,
> + [DCCONFIG] = 0xc10,
> + [AGECONFIG] = 0xc14,
> + [FREQPCT30] = 0xc18,
> + [FREQPCT74] = 0xc1c,
> + [LIMITVALS] = 0xc20,
> + [VBOOT] = 0xc24,
> + [DETWINDOW] = 0xc28,
> + [CONFIG] = 0xc2c,
> + [TSCALCS] = 0xc30,
> + [RUNCONFIG] = 0xc34,
> + [SVSEN] = 0xc38,
> + [INIT2VALS] = 0xc3c,
> + [DCVALUES] = 0xc40,
> + [AGEVALUES] = 0xc44,
> + [VOP30] = 0xc48,
> + [VOP74] = 0xc4c,
> + [TEMP] = 0xc50,
> + [INTSTS] = 0xc54,
> + [INTSTSRAW] = 0xc58,
> + [INTEN] = 0xc5c,
> + [CHKINT] = 0xc60,
> + [CHKSHIFT] = 0xc64,
> + [STATUS] = 0xc68,
> + [VDESIGN30] = 0xc6c,
> + [VDESIGN74] = 0xc70,
> + [DVT30] = 0xc74,
> + [DVT74] = 0xc78,
> + [AGECOUNT] = 0xc7c,
> + [SMSTATE0] = 0xc80,
> + [SMSTATE1] = 0xc84,
> + [CTL0] = 0xc88,
> + [DESDETSEC] = 0xce0,
> + [TEMPAGESEC] = 0xce4,
> + [CTRLSPARE0] = 0xcf0,
> + [CTRLSPARE1] = 0xcf4,
> + [CTRLSPARE2] = 0xcf8,
> + [CTRLSPARE3] = 0xcfc,
> + [CORESEL] = 0xf00,
> + [THERMINTST] = 0xf04,
> + [INTST] = 0xf08,
> + [THSTAGE0ST] = 0xf0c,
> + [THSTAGE1ST] = 0xf10,
> + [THSTAGE2ST] = 0xf14,
> + [THAHBST0] = 0xf18,
> + [THAHBST1] = 0xf1c,
> + [SPARE0] = 0xf20,
> + [SPARE1] = 0xf24,
> + [SPARE2] = 0xf28,
> + [SPARE3] = 0xf2c,
> + [THSLPEVEB] = 0xf30,
> +};
> +
> +struct thermal_parameter {
> + int adc_ge_t;
> + int adc_oe_t;
> + int ge;
> + int oe;
> + int gain;
> + int o_vtsabb;
> + int o_vtsmcu1;
> + int o_vtsmcu2;
> + int o_vtsmcu3;
> + int o_vtsmcu4;
> + int o_vtsmcu5;
> + int degc_cali;
> + int adc_cali_en_t;
> + int o_slope;
> + int o_slope_sign;
> + int ts_id;
> +};
> +
> +struct svs_bank_ops {
> + void (*set_freqs_pct)(struct mtk_svs *svs);
> + void (*get_vops)(struct mtk_svs *svs);
> +};
> +
> +struct svs_bank {
> + struct svs_bank_ops *ops;
> + struct completion init_completion;
> + struct device *dev;
> + struct regulator *buck;
> + struct mutex lock; /* lock to protect update voltage process */
> + bool suspended;
> + bool mtcmos_request;
> + bool init01_support;
> + bool init02_support;
> + bool mon_mode_support;
> + s32 volt_offset;
> + u32 *opp_freqs;
> + u32 *freqs_pct;
> + u32 *opp_volts;
> + u32 *init02_volts;
> + u32 *volts;
> + u32 reg_data[3][reg_num];
> + u32 freq_base;
> + u32 vboot;
> + u32 volt_step;
> + u32 volt_base;
> + u32 init01_volt_flag;
> + u32 phase;
> + u32 vmax;
> + u32 vmin;
> + u32 bts;
> + u32 mts;
> + u32 bdes;
> + u32 mdes;
> + u32 mtdes;
> + u32 dcbdet;
> + u32 dcmdet;
> + u32 dthi;
> + u32 dtlo;
> + u32 det_window;
> + u32 det_max;
> + u32 age_config;
> + u32 age_voffset_in;
> + u32 agem;
> + u32 dc_config;
> + u32 dc_voffset_in;
> + u32 dvt_fixed;
> + u32 vco;
> + u32 chkshift;
> + u32 svs_temp;
> + u32 upper_temp_bound;
> + u32 lower_temp_bound;
> + u32 low_temp_threashold;
> + u32 low_temp_offset;
> + u32 coresel;
> + u32 opp_count;
> + u32 intst;
> + u32 systemclk_en;
> + u32 sw_id;
> + u32 hw_id;
> + u32 ctl0;
> + u8 *of_compatible;
> + u8 *name;
> + u8 *zone_name;
> + u8 *buck_name;
> +};
> +
> +struct svs_platform {
> + struct svs_bank *banks;
> + int (*efuse_parsing)(struct mtk_svs *svs);
> + bool fake_efuse;
> + const u32 *regs;
> + u32 bank_num;
> + u32 efuse_num;
> + u32 efuse_check;
> + u32 thermal_efuse_num;
> + u8 *name;
> +};
> +
> +struct mtk_svs {
> + const struct svs_platform *platform;
> + struct svs_bank *bank;
> + struct device *dev;
> + void __iomem *base;
> + struct clk *main_clk;
> + u32 *efuse;
> + u32 *thermal_efuse;
> +};
> +
> +unsigned long claim_mtk_svs_lock(void)
> +{
> + unsigned long flags;
> +
> + spin_lock_irqsave(&mtk_svs_lock, flags);
> +
> + return flags;
> +}
> +EXPORT_SYMBOL_GPL(claim_mtk_svs_lock);
> +
> +void release_mtk_svs_lock(unsigned long flags)
> +{
> + spin_unlock_irqrestore(&mtk_svs_lock, flags);
> +}
> +EXPORT_SYMBOL_GPL(release_mtk_svs_lock);
I don't see the need for these wrapper functions. You seem to be
declaring this lock in the header which implies there are external
users, but there is none in this series, so this lock should stay local
IMO.
> +static u32 percent(u32 numerator, u32 denominator)
> +{
> + u32 percent;
> +
> + /* If not divide 1000, "numerator * 100" would be data overflow. */
> + numerator /= 1000;
> + denominator /= 1000;
> + percent = ((numerator * 100) + denominator - 1) / denominator;
> +
> + return percent;
> +}
Maybe some math helpers from math64.h would help you not have to
open-code this?
> +static u32 svs_readl(struct mtk_svs *svs, enum reg_index i)
> +{
> + return readl(svs->base + svs->platform->regs[i]);
> +}
> +
> +static void svs_writel(struct mtk_svs *svs, u32 val, enum reg_index i)
> +{
> + writel(val, svs->base + svs->platform->regs[i]);
> +}
> +
> +static void svs_switch_bank(struct mtk_svs *svs)
> +{
> + struct svs_bank *svsb = svs->bank;
> +
> + svs_writel(svs, svsb->coresel, CORESEL);
> +}
> +
> +static u32 svs_volt_to_opp_volt(u32 svsb_volt,
> + u32 svsb_volt_step, u32 svsb_volt_base)
> +{
> + u32 u_volt;
> +
> + u_volt = (svsb_volt * svsb_volt_step) + svsb_volt_base;
> +
> + return u_volt;
> +}
> +
> +static int svs_get_zone_temperature(struct svs_bank *svsb, int *zone_temp)
> +{
> + struct thermal_zone_device *tzd;
> + int ret;
> +
> + tzd = thermal_zone_get_zone_by_name(svsb->zone_name);
> + ret = thermal_zone_get_temp(tzd, zone_temp);
> +
> + return ret;
> +}
> +
> +static int svs_set_volts(struct svs_bank *svsb, bool force_update)
> +{
> + u32 i, svsb_volt, opp_volt, low_temp_offset = 0;
> + int zone_temp, ret;
> +
> + mutex_lock(&svsb->lock);
> +
> + /* If bank is suspended, it means init02 voltage is applied.
> + * Don't need to update opp voltage anymore.
> + */
minor nit: fix multi-line comment style
> + if (svsb->suspended && !force_update) {
> + pr_notice("%s: bank is suspended\n", svsb->name);
> + mutex_unlock(&svsb->lock);
> + return -EPERM;
> + }
> +
> + /* get thermal effect */
> + if (svsb->phase == SVS_PHASE_MON) {
> + if (svsb->svs_temp > svsb->upper_temp_bound &&
> + svsb->svs_temp < svsb->lower_temp_bound) {
> + pr_err("%s: svs_temp is abnormal (0x%x)?\n",
> + svsb->name, svsb->svs_temp);
> + mutex_unlock(&svsb->lock);
> + return -EINVAL;
> + }
> +
> + ret = svs_get_zone_temperature(svsb, &zone_temp);
> + if (ret) {
> + pr_err("%s: cannot get zone \"%s\" temperature\n",
> + svsb->name, svsb->zone_name);
> + pr_err("%s: add low_temp_offset = %u\n",
> + svsb->name, svsb->low_temp_offset);
> + zone_temp = svsb->low_temp_threashold;
> + }
> +
> + if (zone_temp <= svsb->low_temp_threashold)
> + low_temp_offset = svsb->low_temp_offset;
> + }
> +
> + /* vmin <= svsb_volt (opp_volt) <= signed-off voltage */
> + for (i = 0; i < svsb->opp_count; i++) {
> + if (svsb->phase == SVS_PHASE_MON) {
> + svsb_volt = max((svsb->volts[i] + svsb->volt_offset +
> + low_temp_offset), svsb->vmin);
> + opp_volt = svs_volt_to_opp_volt(svsb_volt,
> + svsb->volt_step,
> + svsb->volt_base);
> + } else if (svsb->phase == SVS_PHASE_INIT02) {
> + svsb_volt = max((svsb->init02_volts[i] +
> + svsb->volt_offset), svsb->vmin);
> + opp_volt = svs_volt_to_opp_volt(svsb_volt,
> + svsb->volt_step,
> + svsb->volt_base);
> + } else if (svsb->phase == SVS_PHASE_ERROR) {
> + opp_volt = svsb->opp_volts[i];
> + } else {
> + pr_err("%s: unknown phase: %u?\n",
> + svsb->name, svsb->phase);
> + mutex_unlock(&svsb->lock);
> + return -EINVAL;
> + }
> +
> + opp_volt = min(opp_volt, svsb->opp_volts[i]);
> + ret = dev_pm_opp_adjust_voltage(svsb->dev, svsb->opp_freqs[i],
> + opp_volt);
> + if (ret) {
> + pr_err("%s: set voltage failed: %d\n", svsb->name, ret);
> + mutex_unlock(&svsb->lock);
> + return ret;
> + }
> + }
> +
> + mutex_unlock(&svsb->lock);
> +
> + return 0;
> +}
> +
> +static u32 interpolate(u32 f0, u32 f1, u32 v0, u32 v1, u32 fx)
> +{
> + u32 vy;
> +
> + if (v0 == v1 || f0 == f1)
> + return v0;
> +
> + /* *100 to have decimal fraction factor, +99 for rounding up. */
> + vy = (v0 * 100) - ((((v0 - v1) * 100) / (f0 - f1)) * (f0 - fx));
> + vy = (vy + 99) / 100;
> +
> + return vy;
> +}
> +
> +static void svs_get_vops_v2(struct mtk_svs *svs)
> +{
> + struct svs_bank *svsb = svs->bank;
> + u32 temp, i;
> +
> + temp = svs_readl(svs, VOP30);
> + svsb->volts[6] = (temp >> 24) & 0xff;
> + svsb->volts[4] = (temp >> 16) & 0xff;
> + svsb->volts[2] = (temp >> 8) & 0xff;
> + svsb->volts[0] = (temp & 0xff);
> +
> + temp = svs_readl(svs, VOP74);
> + svsb->volts[14] = (temp >> 24) & 0xff;
> + svsb->volts[12] = (temp >> 16) & 0xff;
> + svsb->volts[10] = (temp >> 8) & 0xff;
> + svsb->volts[8] = (temp & 0xff);
> +
> + for (i = 0; i <= 7; i++) {
> + if (i < 7) {
> + svsb->volts[(i * 2) + 1] =
> + interpolate(svsb->freqs_pct[i * 2],
> + svsb->freqs_pct[(i + 1) * 2],
> + svsb->volts[i * 2],
> + svsb->volts[(i + 1) * 2],
> + svsb->freqs_pct[(i * 2) + 1]);
> + } else if (i == 7) {
> + svsb->volts[(i * 2) + 1] =
> + interpolate(svsb->freqs_pct[(i - 1) * 2],
> + svsb->freqs_pct[i * 2],
> + svsb->volts[(i - 1) * 2],
> + svsb->volts[i * 2],
> + svsb->freqs_pct[(i * 2) + 1]);
> + }
> + }
> +}
> +
> +static void svs_set_freqs_pct_v2(struct mtk_svs *svs)
> +{
> + struct svs_bank *svsb = svs->bank;
> +
> + svs_writel(svs,
> + ((svsb->freqs_pct[6] << 24) & 0xff000000) |
> + ((svsb->freqs_pct[4] << 16) & 0xff0000) |
> + ((svsb->freqs_pct[2] << 8) & 0xff00) |
> + (svsb->freqs_pct[0] & 0xff),
> + FREQPCT30);
> + svs_writel(svs,
> + ((svsb->freqs_pct[14] << 24) & 0xff000000) |
> + ((svsb->freqs_pct[12] << 16) & 0xff0000) |
> + ((svsb->freqs_pct[10] << 8) & 0xff00) |
> + ((svsb->freqs_pct[8]) & 0xff),
> + FREQPCT74);
> +}
> +
> +static void svs_set_phase(struct mtk_svs *svs, u32 target_phase)
> +{
> + struct svs_bank *svsb = svs->bank;
> + u32 des_char, temp_char, det_char, limit_vals;
> + u32 init2vals, ts_calcs, val, filter, i;
> +
> + svs_switch_bank(svs);
> +
> + des_char = ((svsb->bdes << 8) & 0xff00) | (svsb->mdes & 0xff);
> + svs_writel(svs, des_char, DESCHAR);
> +
> + temp_char = ((svsb->vco << 16) & 0xff0000) |
> + ((svsb->mtdes << 8) & 0xff00) |
> + (svsb->dvt_fixed & 0xff);
> + svs_writel(svs, temp_char, TEMPCHAR);
> +
> + det_char = ((svsb->dcbdet << 8) & 0xff00) | (svsb->dcmdet & 0xff);
> + svs_writel(svs, det_char, DETCHAR);
> +
> + svs_writel(svs, svsb->dc_config, DCCONFIG);
> + svs_writel(svs, svsb->age_config, AGECONFIG);
> +
> + if (svsb->agem == 0x0) {
> + svs_writel(svs, 0x80000000, RUNCONFIG);
> + } else {
> + val = 0x0;
> +
> + for (i = 0; i < 24; i += 2) {
> + filter = 0x3 << i;
> +
> + if ((svsb->age_config & filter) == 0x0)
> + val |= (0x1 << i);
> + else
> + val |= (svsb->age_config & filter);
> + }
> + svs_writel(svs, val, RUNCONFIG);
> + }
> +
> + svsb->ops->set_freqs_pct(svs);
> +
> + limit_vals = ((svsb->vmax << 24) & 0xff000000) |
> + ((svsb->vmin << 16) & 0xff0000) |
> + ((svsb->dthi << 8) & 0xff00) |
> + (svsb->dtlo & 0xff);
> + svs_writel(svs, limit_vals, LIMITVALS);
> + svs_writel(svs, (svsb->vboot & 0xff), VBOOT);
> + svs_writel(svs, (svsb->det_window & 0xffff), DETWINDOW);
> + svs_writel(svs, (svsb->det_max & 0xffff), CONFIG);
> +
> + if (svsb->chkshift != 0)
> + svs_writel(svs, (svsb->chkshift & 0xff), CHKSHIFT);
> +
> + if (svsb->ctl0 != 0)
> + svs_writel(svs, svsb->ctl0, CTL0);
> +
> + svs_writel(svs, 0x00ffffff, INTSTS);
> +
> + switch (target_phase) {
> + case SVS_PHASE_INIT01:
> + svs_writel(svs, 0x00005f01, INTEN);
> + svs_writel(svs, 0x00000001, SVSEN);
> + break;
> + case SVS_PHASE_INIT02:
> + svs_writel(svs, 0x00005f01, INTEN);
> + init2vals = ((svsb->age_voffset_in << 16) & 0xffff0000) |
> + (svsb->dc_voffset_in & 0xffff);
> + svs_writel(svs, init2vals, INIT2VALS);
> + svs_writel(svs, 0x00000005, SVSEN);
> + break;
> + case SVS_PHASE_MON:
> + ts_calcs = ((svsb->bts << 12) & 0xfff000) | (svsb->mts & 0xfff);
> + svs_writel(svs, ts_calcs, TSCALCS);
> + svs_writel(svs, 0x00FF0000, INTEN);
> + svs_writel(svs, 0x00000002, SVSEN);
> + break;
> + default:
> + WARN_ON(1);
> + break;
> + }
> +}
lots of magic constants for these register writes. #defines w/bit
descriptions would be very helpful for the reader.
> +static inline void svs_init01_isr_handler(struct mtk_svs *svs)
> +{
> + struct svs_bank *svsb = svs->bank;
> + enum reg_index rg_i;
> +
> + pr_notice("%s: %s: VDN74:0x%08x, VDN30:0x%08x, DCVALUES:0x%08x\n",
> + svsb->name, __func__, svs_readl(svs, VDESIGN74),
> + svs_readl(svs, VDESIGN30), svs_readl(svs, DCVALUES));
> +
> + for (rg_i = TEMPMONCTL0; rg_i < reg_num; rg_i++)
> + svsb->reg_data[SVS_PHASE_INIT01][rg_i] = svs_readl(svs, rg_i);
> +
> + svsb->dc_voffset_in = ~(svs_readl(svs, DCVALUES) & 0xffff) + 1;
> + if (svsb->init01_volt_flag == SVS_INIT01_VOLT_IGNORE)
> + svsb->dc_voffset_in = 0;
> + else if ((svsb->dc_voffset_in & 0x8000) &&
> + (svsb->init01_volt_flag == SVS_INIT01_VOLT_INC_ONLY))
> + svsb->dc_voffset_in = 0;
> +
> + svsb->age_voffset_in = svs_readl(svs, AGEVALUES) & 0xffff;
> +
> + svs_writel(svs, 0x0, SVSEN);
> + svs_writel(svs, 0x1, INTSTS);
> +
> + /* svs init01 clock gating */
> + svsb->coresel &= ~svsb->systemclk_en;
> +
> + svsb->phase = SVS_PHASE_INIT01;
> + complete(&svsb->init_completion);
> +}
> +
> +static inline void svs_init02_isr_handler(struct mtk_svs *svs)
> +{
> + struct svs_bank *svsb = svs->bank;
> + enum reg_index rg_i;
> +
> + pr_notice("%s: %s: VOP74:0x%08x, VOP30:0x%08x, DCVALUES:0x%08x\n",
> + svsb->name, __func__, svs_readl(svs, VOP74),
> + svs_readl(svs, VOP30), svs_readl(svs, DCVALUES));
> +
> + for (rg_i = TEMPMONCTL0; rg_i < reg_num; rg_i++)
> + svsb->reg_data[SVS_PHASE_INIT02][rg_i] = svs_readl(svs, rg_i);
> +
> + svsb->ops->get_vops(svs);
> + memcpy(svsb->init02_volts, svsb->volts, 4 * svsb->opp_count);
> + svsb->phase = SVS_PHASE_INIT02;
> +
> + svs_writel(svs, 0x0, SVSEN);
> + svs_writel(svs, 0x1, INTSTS);
> +
> + complete(&svsb->init_completion);
> +}
> +
> +static inline void svs_mon_mode_isr_handler(struct mtk_svs *svs)
> +{
> + struct svs_bank *svsb = svs->bank;
> + enum reg_index rg_i;
> +
> + for (rg_i = TEMPMONCTL0; rg_i < reg_num; rg_i++)
> + svsb->reg_data[SVS_PHASE_MON][rg_i] = svs_readl(svs, rg_i);
> +
> + svsb->svs_temp = svs_readl(svs, TEMP) & 0xff;
> +
> + svsb->ops->get_vops(svs);
> + svsb->phase = SVS_PHASE_MON;
> +
> + svs_writel(svs, 0x00ff0000, INTSTS);
> +}
> +
> +static inline void svs_error_isr_handler(struct mtk_svs *svs)
> +{
> + const struct svs_platform *svsp = svs->platform;
> + struct svs_bank *svsb = svs->bank;
> + enum reg_index rg_i;
> +
> + pr_err("%s(): %s(%s)", __func__, svsp->name, svsb->name);
> + pr_err("CORESEL(0x%x) = 0x%08x\n",
> + svsp->regs[CORESEL], svs_readl(svs, CORESEL)),
> + pr_err("SVSEN(0x%x) = 0x%08x, INTSTS(0x%x) = 0x%08x\n",
> + svsp->regs[SVSEN], svs_readl(svs, SVSEN),
> + svsp->regs[INTSTS], svs_readl(svs, INTSTS));
> + pr_err("SMSTATE0(0x%x) = 0x%08x, SMSTATE1(0x%x) = 0x%08x\n",
> + svsp->regs[SMSTATE0], svs_readl(svs, SMSTATE0),
> + svsp->regs[SMSTATE1], svs_readl(svs, SMSTATE1));
> +
> + for (rg_i = TEMPMONCTL0; rg_i < reg_num; rg_i++)
> + svsb->reg_data[SVS_PHASE_MON][rg_i] = svs_readl(svs, rg_i);
> +
> + svsb->init01_support = false;
> + svsb->init02_support = false;
> + svsb->mon_mode_support = false;
> +
> + if (svsb->phase == SVS_PHASE_MON)
> + svsb->phase = SVS_PHASE_INIT02;
> +
> + svs_writel(svs, 0x0, SVSEN);
> + svs_writel(svs, 0x00ffffff, INTSTS);
> +}
> +
> +static inline void svs_isr_handler(struct mtk_svs *svs)
> +{
> + u32 intsts, svsen;
> +
> + svs_switch_bank(svs);
> +
> + intsts = svs_readl(svs, INTSTS);
> + svsen = svs_readl(svs, SVSEN);
> +
> + if (intsts == 0x1 && ((svsen & 0x7) == 0x1))
> + svs_init01_isr_handler(svs);
> + else if ((intsts == 0x1) && ((svsen & 0x7) == 0x5))
> + svs_init02_isr_handler(svs);
> + else if ((intsts & 0x00ff0000) != 0x0)
> + svs_mon_mode_isr_handler(svs);
> + else
> + svs_error_isr_handler(svs);
> +}
> +
> +static irqreturn_t svs_isr(int irq, void *data)
> +{
> + struct mtk_svs *svs = (struct mtk_svs *)data;
> + const struct svs_platform *svsp = svs->platform;
> + struct svs_bank *svsb = NULL;
> + unsigned long flags;
> + u32 idx;
> +
> + flags = claim_mtk_svs_lock();
> + for (idx = 0; idx < svsp->bank_num; idx++) {
> + svsb = &svsp->banks[idx];
> + svs->bank = svsb;
> +
> + if (svsb->suspended)
> + continue;
> + else if (svsb->intst & svs_readl(svs, INTST))
> + continue;
> +
> + svs_isr_handler(svs);
> + break;
> + }
> + release_mtk_svs_lock(flags);
> +
> + if (svsb->phase != SVS_PHASE_INIT01)
> + svs_set_volts(svsb, false);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static void svs_mon_mode(struct mtk_svs *svs)
> +{
> + const struct svs_platform *svsp = svs->platform;
> + struct svs_bank *svsb;
> + unsigned long flags;
> + u32 idx;
> +
> + flags = claim_mtk_svs_lock();
> + for (idx = 0; idx < svsp->bank_num; idx++) {
> + svsb = &svsp->banks[idx];
> + svs->bank = svsb;
> +
> + if (!svsb->mon_mode_support)
> + continue;
> +
> + svs_set_phase(svs, SVS_PHASE_MON);
> + }
> + release_mtk_svs_lock(flags);
> +}
> +
> +static int svs_init02(struct mtk_svs *svs)
> +{
> + const struct svs_platform *svsp = svs->platform;
> + struct svs_bank *svsb;
> + unsigned long flags, time_left;
> + u32 idx;
> +
> + for (idx = 0; idx < svsp->bank_num; idx++) {
> + svsb = &svsp->banks[idx];
> + svs->bank = svsb;
> +
> + if (!svsb->init02_support)
> + continue;
> +
> + reinit_completion(&svsb->init_completion);
> + flags = claim_mtk_svs_lock();
> + svs_set_phase(svs, SVS_PHASE_INIT02);
> + release_mtk_svs_lock(flags);
> + time_left =
> + wait_for_completion_timeout(&svsb->init_completion,
> + msecs_to_jiffies(2000));
> + if (time_left == 0) {
> + pr_err("%s: init02 completion timeout\n", svsb->name);
> + return -EBUSY;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int svs_init01(struct mtk_svs *svs)
> +{
> + const struct svs_platform *svsp = svs->platform;
> + struct svs_bank *svsb;
> + struct pm_qos_request qos_request = { {0} };
> + unsigned long flags, time_left;
> + bool search_done;
> + int ret = -EINVAL;
> + u32 opp_freqs, opp_vboot, buck_volt, idx, i;
> +
> + /* Let CPUs leave idle-off state for initializing svs_init01. */
> + pm_qos_add_request(&qos_request, PM_QOS_CPU_DMA_LATENCY, 0);
> +
> + /* Sometimes two svs_bank use the same buck.
> + * Therefore, we set each svs_bank to vboot voltage first.
> + */
> + for (idx = 0; idx < svsp->bank_num; idx++) {
> + svsb = &svsp->banks[idx];
> + search_done = false;
> +
> + if (!svsb->init01_support)
> + continue;
> +
> + ret = regulator_set_mode(svsb->buck, REGULATOR_MODE_FAST);
> + if (ret)
> + pr_notice("%s: fail to set fast mode: %d\n",
> + svsb->name, ret);
> +
> + if (svsb->mtcmos_request) {
> + ret = regulator_enable(svsb->buck);
> + if (ret) {
> + pr_err("%s: fail to enable %s power: %d\n",
> + svsb->name, svsb->buck_name, ret);
> + goto init01_finish;
> + }
> +
> + ret = dev_pm_domain_attach(svsb->dev, false);
> + if (ret) {
> + pr_err("%s: attach pm domain fail: %d\n",
> + svsb->name, ret);
> + goto init01_finish;
> + }
> +
> + pm_runtime_enable(svsb->dev);
> + ret = pm_runtime_get_sync(svsb->dev);
> + if (ret < 0) {
> + pr_err("%s: turn mtcmos on fail: %d\n",
> + svsb->name, ret);
> + goto init01_finish;
> + }
> + }
> +
> + /* Find the fastest freq that can be run at vboot and
> + * fix to that freq until svs_init01 is done.
> + */
> + opp_vboot = svs_volt_to_opp_volt(svsb->vboot,
> + svsb->volt_step,
> + svsb->volt_base);
> +
> + for (i = 0; i < svsb->opp_count; i++) {
> + opp_freqs = svsb->opp_freqs[i];
> + if (!search_done && svsb->opp_volts[i] <= opp_vboot) {
> + ret = dev_pm_opp_adjust_voltage(svsb->dev,
> + opp_freqs,
> + opp_vboot);
> + if (ret) {
> + pr_err("%s: set voltage failed: %d\n",
> + svsb->name, ret);
> + goto init01_finish;
> + }
> +
> + search_done = true;
> + } else {
> + dev_pm_opp_disable(svsb->dev,
> + svsb->opp_freqs[i]);
> + }
> + }
> + }
> +
> + for (idx = 0; idx < svsp->bank_num; idx++) {
> + svsb = &svsp->banks[idx];
> + svs->bank = svsb;
> +
> + if (!svsb->init01_support)
> + continue;
> +
> + opp_vboot = svs_volt_to_opp_volt(svsb->vboot,
> + svsb->volt_step,
> + svsb->volt_base);
> +
> + buck_volt = regulator_get_voltage(svsb->buck);
> + if (buck_volt != opp_vboot) {
> + pr_err("%s: buck voltage: %u, expected vboot: %u\n",
> + svsb->name, buck_volt, opp_vboot);
> + ret = -EPERM;
> + goto init01_finish;
> + }
> +
> + init_completion(&svsb->init_completion);
> + flags = claim_mtk_svs_lock();
> + svs_set_phase(svs, SVS_PHASE_INIT01);
> + release_mtk_svs_lock(flags);
> + time_left =
> + wait_for_completion_timeout(&svsb->init_completion,
> + msecs_to_jiffies(2000));
> + if (time_left == 0) {
> + pr_err("%s: init01 completion timeout\n", svsb->name);
> + ret = -EBUSY;
> + goto init01_finish;
> + }
> + }
> +
> +init01_finish:
> + for (idx = 0; idx < svsp->bank_num; idx++) {
> + svsb = &svsp->banks[idx];
> +
> + if (!svsb->init01_support)
> + continue;
> +
> + for (i = 0; i < svsb->opp_count; i++)
> + dev_pm_opp_enable(svsb->dev, svsb->opp_freqs[i]);
> +
> + if (regulator_set_mode(svsb->buck, REGULATOR_MODE_NORMAL))
> + pr_notice("%s: fail to set normal mode: %d\n",
> + svsb->name, ret);
> +
> + if (svsb->mtcmos_request) {
> + if (pm_runtime_put_sync(svsb->dev))
> + pr_err("%s: turn mtcmos off fail: %d\n",
> + svsb->name, ret);
> + pm_runtime_disable(svsb->dev);
> + dev_pm_domain_detach(svsb->dev, 0);
> + if (regulator_disable(svsb->buck))
> + pr_err("%s: fail to disable %s power: %d\n",
> + svsb->name, svsb->buck_name, ret);
> + }
> + }
> +
> + pm_qos_remove_request(&qos_request);
> +
> + return ret;
> +}
> +
> +static int svs_start(struct mtk_svs *svs)
> +{
> + int ret;
> +
> + ret = svs_init01(svs);
> + if (ret)
> + return ret;
> +
> + ret = svs_init02(svs);
> + if (ret)
> + return ret;
> +
> + svs_mon_mode(svs);
> +
> + return ret;
> +}
> +
> +static int svs_mt8183_efuse_parsing(struct mtk_svs *svs)
> +{
> + const struct svs_platform *svsp = svs->platform;
> + struct thermal_parameter tp;
> + struct svs_bank *svsb;
> + bool mon_mode_support = true;
> + int format[6], x_roomt[6], tb_roomt;
> + u32 idx, i, ft_pgm, mts, temp0, temp1, temp2;
> +
> + if (svsp->fake_efuse) {
> + pr_notice("fake efuse\n");
> + svs->efuse[0] = 0x00310080;
> + svs->efuse[1] = 0xabfbf757;
> + svs->efuse[2] = 0x47c747c7;
> + svs->efuse[3] = 0xabfbf757;
> + svs->efuse[4] = 0xe7fca0ec;
> + svs->efuse[5] = 0x47bf4b88;
> + svs->efuse[6] = 0xabfb8fa5;
> + svs->efuse[7] = 0xabfb217b;
> + svs->efuse[8] = 0x4bf34be1;
> + svs->efuse[9] = 0xabfb670d;
> + svs->efuse[16] = 0xabfbc653;
> + svs->efuse[17] = 0x47f347e1;
> + svs->efuse[18] = 0xabfbd848;
> +
> + svs->thermal_efuse[0] = 0x02873f69;
> + svs->thermal_efuse[1] = 0xa11d9142;
> + svs->thermal_efuse[2] = 0xa2526900;
> + }
> +
> + /* svs efuse parsing */
> + ft_pgm = (svs->efuse[0] >> 4) & 0xf;
> +
> + for (idx = 0; idx < svsp->bank_num; idx++) {
> + svsb = &svsp->banks[idx];
> + if (ft_pgm <= 1)
> + svsb->init01_volt_flag = SVS_INIT01_VOLT_IGNORE;
> +
> + switch (svsb->sw_id) {
> + case SVS_CPU_LITTLE:
> + svsb->bdes = svs->efuse[16] & 0xff;
> + svsb->mdes = (svs->efuse[16] >> 8) & 0xff;
> + svsb->dcbdet = (svs->efuse[16] >> 16) & 0xff;
> + svsb->dcmdet = (svs->efuse[16] >> 24) & 0xff;
> + svsb->mtdes = (svs->efuse[17] >> 16) & 0xff;
> +
> + if (ft_pgm <= 3)
> + svsb->volt_offset += 10;
> + else
> + svsb->volt_offset += 2;
> + break;
> + case SVS_CPU_BIG:
> + svsb->bdes = svs->efuse[18] & 0xff;
> + svsb->mdes = (svs->efuse[18] >> 8) & 0xff;
> + svsb->dcbdet = (svs->efuse[18] >> 16) & 0xff;
> + svsb->dcmdet = (svs->efuse[18] >> 24) & 0xff;
> + svsb->mtdes = svs->efuse[17] & 0xff;
> +
> + if (ft_pgm <= 3)
> + svsb->volt_offset += 15;
> + else
> + svsb->volt_offset += 12;
> + break;
> + case SVS_CCI:
> + svsb->bdes = svs->efuse[4] & 0xff;
> + svsb->mdes = (svs->efuse[4] >> 8) & 0xff;
> + svsb->dcbdet = (svs->efuse[4] >> 16) & 0xff;
> + svsb->dcmdet = (svs->efuse[4] >> 24) & 0xff;
> + svsb->mtdes = (svs->efuse[5] >> 16) & 0xff;
> +
> + if (ft_pgm <= 3)
> + svsb->volt_offset += 10;
> + else
> + svsb->volt_offset += 2;
> + break;
> + case SVS_GPU:
> + svsb->bdes = svs->efuse[6] & 0xff;
> + svsb->mdes = (svs->efuse[6] >> 8) & 0xff;
> + svsb->dcbdet = (svs->efuse[6] >> 16) & 0xff;
> + svsb->dcmdet = (svs->efuse[6] >> 24) & 0xff;
> + svsb->mtdes = svs->efuse[5] & 0xff;
> +
> + if (ft_pgm >= 2) {
> + svsb->freq_base = 800000000; /* 800MHz */
> + svsb->dvt_fixed = 2;
> + }
> + break;
> + default:
> + break;
> + }
> + }
> +
> + for (i = 0; i < svsp->efuse_num; i++) {
> + if (svs->efuse[i])
> + pr_notice("M_HW_RES%d: 0x%08x\n", i, svs->efuse[i]);
> + }
> +
> + /* thermal efuse parsing */
> + if (!svs->thermal_efuse)
> + return 0;
> +
> + tp.adc_ge_t = (svs->thermal_efuse[1] >> 22) & 0x3ff;
> + tp.adc_oe_t = (svs->thermal_efuse[1] >> 12) & 0x3ff;
> +
> + tp.o_vtsmcu1 = (svs->thermal_efuse[0] >> 17) & 0x1ff;
> + tp.o_vtsmcu2 = (svs->thermal_efuse[0] >> 8) & 0x1ff;
> + tp.o_vtsmcu3 = svs->thermal_efuse[1] & 0x1ff;
> + tp.o_vtsmcu4 = (svs->thermal_efuse[2] >> 23) & 0x1ff;
> + tp.o_vtsmcu5 = (svs->thermal_efuse[2] >> 5) & 0x1ff;
> + tp.o_vtsabb = (svs->thermal_efuse[2] >> 14) & 0x1ff;
> +
> + tp.degc_cali = (svs->thermal_efuse[0] >> 1) & 0x3f;
> + tp.adc_cali_en_t = svs->thermal_efuse[0] & BIT(0);
> + tp.o_slope_sign = (svs->thermal_efuse[0] >> 7) & BIT(0);
> +
> + tp.ts_id = (svs->thermal_efuse[1] >> 9) & BIT(0);
> + tp.o_slope = (svs->thermal_efuse[0] >> 26) & 0x3f;
> +
> + if (tp.adc_cali_en_t == 1) {
> + if (tp.ts_id == 0)
> + tp.o_slope = 0;
> +
> + if ((tp.adc_ge_t < 265 || tp.adc_ge_t > 758) ||
> + (tp.adc_oe_t < 265 || tp.adc_oe_t > 758) ||
> + (tp.o_vtsmcu1 < -8 || tp.o_vtsmcu1 > 484) ||
> + (tp.o_vtsmcu2 < -8 || tp.o_vtsmcu2 > 484) ||
> + (tp.o_vtsmcu3 < -8 || tp.o_vtsmcu3 > 484) ||
> + (tp.o_vtsmcu4 < -8 || tp.o_vtsmcu4 > 484) ||
> + (tp.o_vtsmcu5 < -8 || tp.o_vtsmcu5 > 484) ||
> + (tp.o_vtsabb < -8 || tp.o_vtsabb > 484) ||
> + (tp.degc_cali < 1 || tp.degc_cali > 63)) {
> + pr_err("bad thermal efuse data. disable mon mode\n");
> + mon_mode_support = false;
> + }
> + } else {
> + pr_err("no thermal efuse data. disable mon mode\n");
> + mon_mode_support = false;
> + }
> +
> + if (!mon_mode_support) {
> + for (i = 0; i < svsp->thermal_efuse_num; i++)
> + pr_err("thermal_efuse[%u] = 0x%08x\n",
> + i, svs->thermal_efuse[i]);
> +
> + for (idx = 0; idx < svsp->bank_num; idx++) {
> + svsb = &svsp->banks[idx];
> + svsb->mon_mode_support = false;
> + }
> +
> + return 0;
> + }
> +
> + tp.ge = ((tp.adc_ge_t - 512) * 10000) / 4096;
> + tp.oe = (tp.adc_oe_t - 512);
> + tp.gain = (10000 + tp.ge);
> +
> + format[0] = (tp.o_vtsmcu1 + 3350 - tp.oe);
> + format[1] = (tp.o_vtsmcu2 + 3350 - tp.oe);
> + format[2] = (tp.o_vtsmcu3 + 3350 - tp.oe);
> + format[3] = (tp.o_vtsmcu4 + 3350 - tp.oe);
> + format[4] = (tp.o_vtsmcu5 + 3350 - tp.oe);
> + format[5] = (tp.o_vtsabb + 3350 - tp.oe);
> +
> + for (i = 0; i < 6; i++)
> + x_roomt[i] = (((format[i] * 10000) / 4096) * 10000) / tp.gain;
> +
> + temp0 = (10000 * 100000 / tp.gain) * 15 / 18;
> +
> + if (tp.o_slope_sign == 0)
> + mts = (temp0 * 10) / (1534 + tp.o_slope * 10);
> + else
> + mts = (temp0 * 10) / (1534 - tp.o_slope * 10);
> +
> + for (idx = 0; idx < svsp->bank_num; idx++) {
> + svsb = &svsp->banks[idx];
> + svsb->mts = mts;
> +
> + switch (svsb->sw_id) {
> + case SVS_CPU_LITTLE:
> + tb_roomt = x_roomt[3];
> + break;
> + case SVS_CPU_BIG:
> + tb_roomt = x_roomt[4];
> + break;
> + case SVS_CCI:
> + tb_roomt = x_roomt[3];
> + break;
> + case SVS_GPU:
> + tb_roomt = x_roomt[1];
> + break;
> + default:
> + pr_err("unknown svsb_id = %u? disable svs\n",
> + svsb->sw_id);
> + return -EINVAL;
> + }
> +
> + temp0 = (tp.degc_cali * 10 / 2);
> + temp1 = ((10000 * 100000 / 4096 / tp.gain) *
> + tp.oe + tb_roomt * 10) * 15 / 18;
> +
> + if (tp.o_slope_sign == 0)
> + temp2 = temp1 * 100 / (1534 + tp.o_slope * 10);
> + else
> + temp2 = temp1 * 100 / (1534 - tp.o_slope * 10);
> +
> + svsb->bts = (temp0 + temp2 - 250) * 4 / 10;
> + }
> +
> + return 0;
> +}
> +
> +static int svs_is_support(struct mtk_svs *svs)
nit: this appears to be a check if the platform supports SVS. If so,
then it should probably be 'bool' and be called svs_is_supported().
> +{
> + const struct svs_platform *svsp = svs->platform;
> + struct svs_bank *svsb;
> + struct nvmem_cell *cell;
> + size_t len;
> + int ret;
> + u32 idx, i;
> +
> + if (svsp->fake_efuse) {
> + len = svsp->efuse_num * 4;
> + svs->efuse = kzalloc(len, GFP_KERNEL);
> + if (!svs->efuse)
> + return -ENOMEM;
> +
> + len = svsp->thermal_efuse_num * 4;
> + svs->thermal_efuse = kzalloc(len, GFP_KERNEL);
> + if (!svs->thermal_efuse)
> + return -ENOMEM;
> +
> + goto svsp_efuse_parsing;
> + }
> +
> + /* get svs efuse by nvmem */
> + cell = nvmem_cell_get(svs->dev, "svs-calibration-data");
> + if (IS_ERR(cell)) {
> + pr_err("no \"svs-calibration-data\" from dts? disable svs\n");
> + return PTR_ERR(cell);
> + }
> +
> + svs->efuse = (u32 *)nvmem_cell_read(cell, &len);
> + nvmem_cell_put(cell);
> +
> + ret = (svs->efuse[svsp->efuse_check] == 0) ? -EPERM : 0;
> + if (ret) {
> + pr_err("no svs efuse. disable svs.\n");
> + for (i = 0; i < svsp->efuse_num; i++)
> + pr_err("M_HW_RES%d: 0x%08x\n", i, svs->efuse[i]);
> + return ret;
> + }
> +
> + /* get thermal efuse by nvmem */
> + cell = nvmem_cell_get(svs->dev, "calibration-data");
> + if (IS_ERR(cell)) {
> + pr_err("no \"calibration-data\" from dts? disable mon mode\n");
> + svs->thermal_efuse = NULL;
> + for (idx = 0; idx < svsp->bank_num; idx++) {
> + svsb = &svsp->banks[idx];
> + svsb->mon_mode_support = false;
> + }
> + goto svsp_efuse_parsing;
> + }
> +
> + svs->thermal_efuse = (u32 *)nvmem_cell_read(cell, &len);
> + nvmem_cell_put(cell);
> +
> +svsp_efuse_parsing:
> + ret = svsp->efuse_parsing(svs);
> +
> + return ret;
> +}
> +
> +static int svs_resource_setup(struct mtk_svs *svs)
> +{
> + const struct svs_platform *svsp = svs->platform;
> + struct svs_bank *svsb;
> + struct platform_device *pdev;
> + struct device_node *np = NULL;
> + struct dev_pm_opp *opp;
> + unsigned long freq;
> + size_t opp_size;
> + int count, ret;
> + u32 idx, i;
> +
> + for (idx = 0; idx < svsp->bank_num; idx++) {
> + svsb = &svsp->banks[idx];
> +
> + if (!svsb->init01_support)
> + continue;
> +
> + switch (svsb->sw_id) {
> + case SVS_CPU_LITTLE:
> + svsb->name = "SVS_CPU_LITTLE";
> + break;
> + case SVS_CPU_BIG:
> + svsb->name = "SVS_CPU_BIG";
> + break;
> + case SVS_CCI:
> + svsb->name = "SVS_CCI";
> + break;
> + case SVS_GPU:
> + svsb->name = "SVS_GPU";
> + break;
> + default:
> + WARN_ON(1);
> + return -EINVAL;
> + }
> +
> + /* Add svs_bank device for opp-table/mtcmos/buck control */
> + pdev = platform_device_alloc(svsb->name, 0);
> + if (!pdev) {
> + pr_err("%s: fail to alloc pdev for svs_bank\n",
> + svsb->name);
> + return -ENOMEM;
> + }
> +
> + for_each_child_of_node(svs->dev->of_node, np) {
> + if (of_device_is_compatible(np, svsb->of_compatible)) {
> + pdev->dev.of_node = np;
> + break;
> + }
> + }
> +
> + ret = platform_device_add(pdev);
> + if (ret) {
> + pr_err("%s: fail to add svs_bank device: %d\n",
> + svsb->name, ret);
> + return ret;
> + }
> +
> + svsb->dev = &pdev->dev;
> + dev_set_drvdata(svsb->dev, svs);
> + ret = dev_pm_opp_of_add_table(svsb->dev);
> + if (ret) {
> + pr_err("%s: fail to add opp table: %d\n",
> + svsb->name, ret);
> + return ret;
> + }
> +
> + mutex_init(&svsb->lock);
> +
> + svsb->buck = devm_regulator_get_optional(svsb->dev,
> + svsb->buck_name);
> + if (IS_ERR(svsb->buck)) {
> + pr_err("%s: cannot get regulator \"%s-supply\"\n",
> + svsb->name, svsb->buck_name);
> + return PTR_ERR(svsb->buck);
> + }
> +
> + count = dev_pm_opp_get_opp_count(svsb->dev);
> + if (svsb->opp_count != count) {
> + pr_err("%s: opp_count not \"%u\" but get \"%d\"?\n",
> + svsb->name, svsb->opp_count, count);
> + return count;
> + }
> +
> + opp_size = 4 * svsb->opp_count;
> + svsb->opp_volts = kmalloc(opp_size, GFP_KERNEL);
> + if (!svsb->opp_volts)
> + return -ENOMEM;
> +
> + svsb->init02_volts = kmalloc(opp_size, GFP_KERNEL);
> + if (!svsb->init02_volts)
> + return -ENOMEM;
> +
> + svsb->volts = kmalloc(opp_size, GFP_KERNEL);
> + if (!svsb->volts)
> + return -ENOMEM;
> +
> + svsb->opp_freqs = kmalloc(opp_size, GFP_KERNEL);
> + if (!svsb->opp_freqs)
> + return -ENOMEM;
> +
> + svsb->freqs_pct = kmalloc(opp_size, GFP_KERNEL);
> + if (!svsb->freqs_pct)
> + return -ENOMEM;
> +
> + for (i = 0, freq = (u32)-1; i < svsb->opp_count; i++, freq--) {
> + opp = dev_pm_opp_find_freq_floor(svsb->dev, &freq);
> + if (IS_ERR(opp)) {
> + pr_err("%s: error opp entry!!, err = %ld\n",
> + svsb->name, PTR_ERR(opp));
> + return PTR_ERR(opp);
> + }
> +
> + svsb->opp_freqs[i] = freq;
> + svsb->opp_volts[i] = dev_pm_opp_get_voltage(opp);
> + svsb->freqs_pct[i] = percent(svsb->opp_freqs[i],
> + svsb->freq_base) & 0xff;
> + }
> + }
> +
> + return 0;
> +}
There are lots of resources (platform_device, all the kmalloc regions)
that are never free'd here. You might consider using devm for this.
Related, all the small malloc's are calculated based on the opp_count
for each bank, but they are all hard-coded to 16. Seems it would be
better to have a default/max size for this in the bank structure itself
instead of making these small allocations.
> +static int svs_suspend(struct device *dev)
> +{
> + struct mtk_svs *svs = dev_get_drvdata(dev);
> + const struct svs_platform *svsp = svs->platform;
> + struct svs_bank *svsb;
> + unsigned long flags;
> + u32 idx;
> +
> + /* Wait if there is processing svs_isr(). Suspend all banks. */
> + flags = claim_mtk_svs_lock();
> + for (idx = 0; idx < svsp->bank_num; idx++) {
> + svsb = &svsp->banks[idx];
> + svs->bank = svsb;
> + svs_switch_bank(svs);
> + svs_writel(svs, 0x0, SVSEN);
> + svs_writel(svs, 0x00ffffff, INTSTS);
I'm assuming this part will disable future interrupts?
I'm not seeing where they get re-enabled on the resume path.
> + svsb->suspended = true;
> + }
> + release_mtk_svs_lock(flags);
> +
> + for (idx = 0; idx < svsp->bank_num; idx++) {
> + svsb = &svsp->banks[idx];
> + if (svsb->phase == SVS_PHASE_MON) {
> + svsb->phase = SVS_PHASE_INIT02;
> + svs_set_volts(svsb, true);
> + }
> + }
> +
> + clk_disable_unprepare(svs->main_clk);
> +
> + return 0;
> +}
> +
> +static int svs_resume(struct device *dev)
> +{
> + struct mtk_svs *svs = dev_get_drvdata(dev);
> + const struct svs_platform *svsp = svs->platform;
> + struct svs_bank *svsb;
> + int ret;
> + u32 idx;
> +
> + ret = clk_prepare_enable(svs->main_clk);
> + if (ret)
> + pr_err("%s(): cannot enable main_clk\n", __func__);
> +
> + for (idx = 0; idx < svsp->bank_num; idx++) {
> + svsb = &svsp->banks[idx];
> + svsb->suspended = false;
> + }
> +
> + ret = svs_init02(svs);
> + if (ret)
> + return ret;
> +
> + svs_mon_mode(svs);
> +
> + return 0;
> +}
Kevin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: chapoly acceleration hardware [Was: Re: [RFC PATCH 00/18] crypto: wireguard using the existing crypto API]
From: Jakub Kicinski @ 2019-09-26 22:47 UTC (permalink / raw)
To: Jason A. Donenfeld
Cc: Toke Høiland-Jørgensen, Catalin Marinas, Herbert Xu,
Arnd Bergmann, Ard Biesheuvel, Greg KH, Eric Biggers, Will Deacon,
Dave Taht, Willy Tarreau, Samuel Neves, Pascal Van Leeuwen,
Netdev, Linux Crypto Mailing List, Andy Lutomirski, Marc Zyngier,
Dan Carpenter, Linus Torvalds, David Miller, linux-arm-kernel
In-Reply-To: <CAHmME9r5m7D-oMU6Lv_ZhEyWmrNscMr5HokzdK0wg2Ayzzbeow@mail.gmail.com>
On Thu, 26 Sep 2019 13:06:51 +0200, Jason A. Donenfeld wrote:
> On Thu, Sep 26, 2019 at 12:19 PM Pascal Van Leeuwen wrote:
> > Actually, that assumption is factually wrong. I don't know if anything
> > is *publicly* available, but I can assure you the silicon is running in
> > labs already. And something will be publicly available early next year
> > at the latest. Which could nicely coincide with having Wireguard support
> > in the kernel (which I would also like to see happen BTW) ...
> >
> > Not "at some point". It will. Very soon. Maybe not in consumer or server
> > CPUs, but definitely in the embedded (networking) space.
> > And it *will* be much faster than the embedded CPU next to it, so it will
> > be worth using it for something like bulk packet encryption.
>
> Super! I was wondering if you could speak a bit more about the
> interface. My biggest questions surround latency. Will it be
> synchronous or asynchronous? If the latter, why? What will its
> latencies be? How deep will its buffers be? The reason I ask is that a
> lot of crypto acceleration hardware of the past has been fast and
> having very deep buffers, but at great expense of latency. In the
> networking context, keeping latency low is pretty important.
FWIW are you familiar with existing kTLS, and IPsec offloads in the
networking stack? They offload the crypto into the NIC, inline, which
helps with the latency, and processing overhead.
There are also NIC silicon which can do some ChaCha/Poly, although
I'm not familiar enough with WireGuard to know if offload to existing
silicon will be possible.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH] of: reserved_mem: add missing of_node_put() for proper ref-counting
From: Chris Goldsworthy @ 2019-09-26 22:48 UTC (permalink / raw)
To: Rob Herring
Cc: linux-arm-kernel, devicetree, stable, Chris Goldsworthy,
linux-kernel
Commit d698a388146c ("of: reserved-memory: ignore disabled memory-region
nodes") added an early return in of_reserved_mem_device_init_by_idx(), but
didn't call of_node_put() on a device_node whose ref-count was incremented
in the call to of_parse_phandle() preceding the early exit.
Fixes: d698a388146c ("of: reserved-memory: ignore disabled memory-region nodes")
Signed-off-by: Chris Goldsworthy <cgoldswo@codeaurora.org>
To: Rob Herring <robh+dt@kernel.org>
Cc: devicetree@vger.kernel.org
Cc: stable@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
---
drivers/of/of_reserved_mem.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
index 7989703..6bd610e 100644
--- a/drivers/of/of_reserved_mem.c
+++ b/drivers/of/of_reserved_mem.c
@@ -324,8 +324,10 @@ int of_reserved_mem_device_init_by_idx(struct device *dev,
if (!target)
return -ENODEV;
- if (!of_device_is_available(target))
+ if (!of_device_is_available(target)) {
+ of_node_put(target);
return 0;
+ }
rmem = __find_rmem(target);
of_node_put(target);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: CPUfreq fail on rk3399-firefly
From: Kevin Hilman @ 2019-09-26 22:51 UTC (permalink / raw)
To: Kever Yang, Heiko Stuebner
Cc: kernel-build-reports, linux-rockchip, linux-next,
闫孝军, 张晴, linux-arm-kernel
In-Reply-To: <7h8srjzuen.fsf@baylibre.com>
Kevin Hilman <khilman@baylibre.com> writes:
> Kevin Hilman <khilman@baylibre.com> writes:
>
>> Kever Yang <kever.yang@rock-chips.com> writes:
>>
>>> Hi Kevin, Heiko,
>>>
>>> On 2019/8/22 上午2:59, Kevin Hilman wrote:
>>>> Hi Heiko,
>>>>
>>>> Heiko Stuebner <heiko@sntech.de> writes:
>>>>
>>>>> Am Dienstag, 13. August 2019, 19:35:31 CEST schrieb Kevin Hilman:
>>>>>> [ resent with correct addr for linux-rockchip list ]
>>>>>>
>>>>>> Mark Brown <broonie@kernel.org> writes:
>>>>>>
>>>>>>> On Thu, Jul 18, 2019 at 04:28:08AM -0700, kernelci.org bot wrote:
>>>>>>>
>>>>>>> Today's -next started failing to boot defconfig on rk3399-firefly:
>>>>>>>
>>>>>>>> arm64:
>>>>>>>> defconfig:
>>>>>>>> gcc-8:
>>>>>>>> rk3399-firefly: 1 failed lab
>>>>>>> It hits a BUG() trying to set up cpufreq:
>>>>>>>
>>>>>>> [ 87.381606] cpufreq: cpufreq_online: CPU0: Running at unlisted freq: 200000 KHz
>>>>>>> [ 87.393244] cpufreq: cpufreq_online: CPU0: Unlisted initial frequency changed to: 408000 KHz
>>>>>>> [ 87.469777] cpufreq: cpufreq_online: CPU4: Running at unlisted freq: 12000 KHz
>>>>>>> [ 87.488595] cpu cpu4: _generic_set_opp_clk_only: failed to set clock rate: -22
>>>>>>> [ 87.491881] cpufreq: __target_index: Failed to change cpu frequency: -22
>>>>>>> [ 87.495335] ------------[ cut here ]------------
>>>>>>> [ 87.496821] kernel BUG at drivers/cpufreq/cpufreq.c:1438!
>>>>>>> [ 87.498462] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
>>>>>>>
>>>>>>> I'm struggling to see anything relevant in the diff from yesterday, the
>>>>>>> unlisted frequency warnings were there in the logs yesterday but no oops
>>>>>>> and I'm not seeing any changes in cpufreq, clk or anything relevant
>>>>>>> looking.
>>>>>>>
>>>>>>> Full bootlog and other info can be found here:
>>>>>>>
>>>>>>> https://kernelci.org/boot/id/5d302d8359b51498d049e983/
>>>>>> I confirm that disabling CPUfreq in the defconfig (CONFIG_CPU_FREQ=n)
>>>>>> makes the firefly board start working again.
>>>>>>
>>>>>> Note that the default defconfig enables the "performance" CPUfreq
>>>>>> governor as the default governor, so during kernel boot, it will always
>>>>>> switch to the max frequency.
>>>>>>
>>>>>> For fun, I set the default governor to "userspace" so the kernel
>>>>>> wouldn't make any OPP changes, and that leads to a slightly more
>>>>>> informative splat[1]
>>>>>>
>>>>>> There is still an OPP change happening because the detected OPP is not
>>>>>> one that's listed in the table, so it tries to change to a listed OPP
>>>>>> and fails in the bowels of clk_set_rate()
>>>>> Though I think that might only be a symptom as well.
>>>>> Both the PLL setting code as well as the actual cpu-clock implementation
>>>>> is unchanged since 2017 (and runs just fine on all boards in my farm).
>>>>>
>>>>> One source for these issues is often the regulator supplying the cpu
>>>>> going haywire - aka the voltage not matching the opp.
>>>>>
>>>>> As in this error-case it's CPU4 being set, this would mean it might
>>>>> be the big cluster supplied by the external syr825 (fan5355 clone)
>>>>> that might act up. In the Firefly-rk3399 case this is even stranger.
>>>>>
>>>>> There is a discrepancy between the "fcs,suspend-voltage-selector"
>>>>> between different bootloader versions (how the selection-pin is set up),
>>>>> so the kernel might actually write his requested voltage to the wrong
>>>>> register (not the one for actual voltage, but the second set used for
>>>>> the suspend voltage).
>>>>>
>>>>> Did you by chance swap bootloaders at some point in recent past?
>>>> No, haven't touched bootloader since I initially setup the board.
>>>
>>> The CPU voltage does not affect by bootloader for kernel should have its
>>> own opp-table,
>>>
>>> the bootloader may only affect the center/logic power supply.
>>>
>>>>
>>>>> I'd assume [2] might actually be the same issue last year, though
>>>>> the CI-logs are not available anymore it seems.
>>>>>
>>>>> Could you try to set the vdd_cpu_b regulator to disabled, so that
>>>>> cpufreq for this cluster defers and see what happens?
>>>> Yes, this change[1] definitely makes things boot reliably again, so
>>>> there's defintiely something a bit unstable with this regulator, at
>>>> least on this firefly.
>>>
>>> Is it possible to target which patch introduce this bug? This board
>>> should have work correctly for a long time with upstream source code.
>>
>> Unfortunately, it seems to be a regular, but intermittent failure, so
>> bisection is not producing anything reliable.
>>
>> You can see that both in mainline[1] and in linux-next[2] there are
>> periodic failures, but it's hard to see any patterns.
>
> Even worse, I (re)tested mainline for versions that were previously
> passing (v5.2, v5.3-rc5) and they are also failing now.
>
> They work again if I disable that regulator as suggested by Heiko.
>
> So this is increasingly pointing to failing hardware.
This is now failing in the v5.2 stable tree.
Any suggestions on what to do? otherwise, I'll just need to disable
this board.
Or, if someone wants to donate a new rk3399-firefly for my lab, I'd be
glad to try replacing it.
Kevin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH] arm64: defconfig: Enable QCA Bluetooth over UART
From: Jeffrey Hugo @ 2019-09-26 22:56 UTC (permalink / raw)
To: bjorn.andersson; +Cc: linux-kernel, linux-arm-kernel, Jeffrey Hugo
This enables Bluetooth on the Lenovo Yoga C630.
Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
---
arch/arm64/configs/defconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 8e05c39eab08..0134a84481f8 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -169,6 +169,7 @@ CONFIG_BT_HCIBTUSB=m
CONFIG_BT_HCIUART=m
CONFIG_BT_HCIUART_LL=y
CONFIG_BT_HCIUART_BCM=y
+CONFIG_BT_HCIUART_QCA=y
CONFIG_CFG80211=m
CONFIG_MAC80211=m
CONFIG_MAC80211_LEDS=y
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH] arm64: defconfig: Enable SN65DSI86 display bridge
From: Jeffrey Hugo @ 2019-09-26 23:02 UTC (permalink / raw)
To: bjorn.andersson; +Cc: linux-kernel, linux-arm-kernel, Jeffrey Hugo
This enables display on the Lenovo Yoga C630 by connecting the DSI output
from the SoC to the eDP input of the panel.
Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
---
arch/arm64/configs/defconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 0134a84481f8..f318836bb9b8 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -541,6 +541,7 @@ CONFIG_DRM_MSM=m
CONFIG_DRM_TEGRA=m
CONFIG_DRM_PANEL_SIMPLE=m
CONFIG_DRM_SII902X=m
+CONFIG_DRM_TI_SN65DSI86=m
CONFIG_DRM_I2C_ADV7511=m
CONFIG_DRM_VC4=m
CONFIG_DRM_ETNAVIV=m
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: chapoly acceleration hardware [Was: Re: [RFC PATCH 00/18] crypto: wireguard using the existing crypto API]
From: Dave Taht @ 2019-09-26 23:13 UTC (permalink / raw)
To: Pascal Van Leeuwen
Cc: Jason A. Donenfeld, Toke Høiland-Jørgensen,
Catalin Marinas, Herbert Xu, Arnd Bergmann, Ard Biesheuvel,
Greg KH, Eric Biggers, Willy Tarreau, Samuel Neves, Will Deacon,
Netdev, Linux Crypto Mailing List, Andy Lutomirski, Marc Zyngier,
Dan Carpenter, Linus Torvalds, David Miller, linux-arm-kernel
In-Reply-To: <MN2PR20MB29733A5E504126B6F4098136CA860@MN2PR20MB2973.namprd20.prod.outlook.com>
On Thu, Sep 26, 2019 at 6:52 AM Pascal Van Leeuwen
<pvanleeuwen@verimatrix.com> wrote:
>
> > -----Original Message-----
> > From: Jason A. Donenfeld <Jason@zx2c4.com>
> > Sent: Thursday, September 26, 2019 1:07 PM
> > To: Pascal Van Leeuwen <pvanleeuwen@verimatrix.com>
> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Linux Crypto Mailing List <linux-
> > crypto@vger.kernel.org>; linux-arm-kernel <linux-arm-kernel@lists.infradead.org>;
> > Herbert Xu <herbert@gondor.apana.org.au>; David Miller <davem@davemloft.net>; Greg KH
> > <gregkh@linuxfoundation.org>; Linus Torvalds <torvalds@linux-foundation.org>; Samuel
> > Neves <sneves@dei.uc.pt>; Dan Carpenter <dan.carpenter@oracle.com>; Arnd Bergmann
> > <arnd@arndb.de>; Eric Biggers <ebiggers@google.com>; Andy Lutomirski <luto@kernel.org>;
> > Will Deacon <will@kernel.org>; Marc Zyngier <maz@kernel.org>; Catalin Marinas
> > <catalin.marinas@arm.com>; Willy Tarreau <w@1wt.eu>; Netdev <netdev@vger.kernel.org>;
> > Toke Høiland-Jørgensen <toke@toke.dk>; Dave Taht <dave.taht@gmail.com>
> > Subject: chapoly acceleration hardware [Was: Re: [RFC PATCH 00/18] crypto: wireguard
> > using the existing crypto API]
> >
> > [CC +willy, toke, dave, netdev]
> >
> > Hi Pascal
> >
> > On Thu, Sep 26, 2019 at 12:19 PM Pascal Van Leeuwen
> > <pvanleeuwen@verimatrix.com> wrote:
> > > Actually, that assumption is factually wrong. I don't know if anything
> > > is *publicly* available, but I can assure you the silicon is running in
> > > labs already. And something will be publicly available early next year
> > > at the latest. Which could nicely coincide with having Wireguard support
> > > in the kernel (which I would also like to see happen BTW) ...
> > >
> > > Not "at some point". It will. Very soon. Maybe not in consumer or server
> > > CPUs, but definitely in the embedded (networking) space.
> > > And it *will* be much faster than the embedded CPU next to it, so it will
> > > be worth using it for something like bulk packet encryption.
> >
> > Super! I was wondering if you could speak a bit more about the
> > interface. My biggest questions surround latency. Will it be
> > synchronous or asynchronous?
> >
> The hardware being external to the CPU and running in parallel with it,
> obviously asynchronous.
>
> > If the latter, why?
> >
> Because, as you probably already guessed, the round-trip latency is way
> longer than the actual processing time, at least for small packets.
>
> Partly because the only way to communicate between the CPU and the HW
> accelerator (whether that is crypto, a GPU, a NIC, etc.) that doesn't
> keep the CPU busy moving data is through memory, with the HW doing DMA.
> And, as any programmer should now, round trip times to memory are huge
> relative to the processing speed.
>
> And partly because these accelerators are very similar to CPU's in
> terms of architecture, doing pipelined processing and having multiple
> of such pipelines in parallel. Except that these pipelines are not
> working on low-level instructions but on full packets/blocks. So they
> need to have many packets in flight to keep those pipelines fully
> occupied. And packets need to move through the various pipeline stages,
> so they incur the time needed to process them multiple times. (just
> like e.g. a multiply instruction with a throughput of 1 per cycle
> actually may need 4 or more cycles to actually provide its result)
>
> Could you do that from a synchronous interface? In theory, probably,
> if you would spawn a new thread for every new packet arriving and
> rely on the scheduler to preempt the waiting threads. But you'd need
> as many threads as the HW accelerator can have packets in flight,
> while an async would need only 2 threads: one to handle the input to
> the accelerator and one to handle the output (or at most one thread
> per CPU, if you want to divide the workload)
>
> Such a many-thread approach seems very inefficient to me.
>
> > What will its latencies be?
> >
> Depends very much on the specific integration scenario (i.e. bus
> speed, bus hierarchy, cache hierarchy, memory speed, etc.) but on
> the order of a few thousand CPU clocks is not unheard of.
> Which is an eternity for the CPU, but still only a few uSec in
> human time. Not a problem unless you're a high-frequency trader and
> every ns counts ...
> It's not like the CPU would process those packets in zero time.
>
> > How deep will its buffers be?
> >
> That of course depends on the specific accelerator implementation,
> but possibly dozens of small packets in our case, as you'd need
> at least width x depth packets in there to keep the pipes busy.
> Just like a modern CPU needs hundreds of instructions in flight
> to keep all its resources busy.
>
> > The reason I ask is that a
> > lot of crypto acceleration hardware of the past has been fast and
> > having very deep buffers, but at great expense of latency.
> >
> Define "great expense". Everything is relative. The latency is very
> high compared to per-packet processing time but at the same time it's
> only on the order of a few uSec. Which may not even be significant on
> the total time it takes for the packet to travel from input MAC to
> output MAC, considering the CPU will still need to parse and classify
> it and do pre- and postprocessing on it.
>
> > In the networking context, keeping latency low is pretty important.
> >
> I've been doing this for IPsec for nearly 20 years now and I've never
> heard anyone complain about our latency, so it must be OK.
Well, it depends on where your bottlenecks are. On low-end hardware
you can and do tend to bottleneck on the crypto step, and with
uncontrolled, non-fq'd non-aqm'd buffering you get results like this:
http://blog.cerowrt.org/post/wireguard/
so in terms of "threads" I would prefer to think of flows entering
the tunnel and attempting to multiplex them as best as possible
across the crypto hard/software so that minimal in-hw latencies are experienced
for most packets and that the coupled queue length does not grow out of control,
Adding fq_codel's hashing algo and queuing to ipsec as was done in
commit: 264b87fa617e758966108db48db220571ff3d60e to leverage
the inner hash...
Had some nice results:
before: http://www.taht.net/~d/ipsec_fq_codel/oldqos.png (100ms spikes)
After: http://www.taht.net/~d/ipsec_fq_codel/newqos.png (2ms spikes)
I'd love to see more vpn vendors using the rrul test or something even
nastier to evaluate their results, rather than dragstrip bulk throughput tests,
steering multiple flows over multiple cores.
> We're also doing (fully inline, no CPU involved) MACsec cores, which
> operate at layer 2 and I know it's a concern there for very specific
> use cases (high frequency trading, precision time protocol, ...).
> For "normal" VPN's though, a few uSec more or less should be a non-issue.
Measured buffering is typically 1000 packets in userspace vpns. If you
can put data in, faster than you can get it out, well....
> > Already
> > WireGuard is multi-threaded which isn't super great all the time for
> > latency (improvements are a work in progress). If you're involved with
> > the design of the hardware, perhaps this is something you can help
> > ensure winds up working well? For example, AES-NI is straightforward
> > and good, but Intel can do that because they are the CPU. It sounds
> > like your silicon will be adjacent. How do you envision this working
> > in a low latency environment?
> >
> Depends on how low low-latency is. If you really need minimal latency,
> you need an inline implementation. Which we can also provide, BTW :-)
>
> Regards,
> Pascal van Leeuwen
> Silicon IP Architect, Multi-Protocol Engines @ Verimatrix
> www.insidesecure.com
--
Dave Täht
CTO, TekLibre, LLC
http://www.teklibre.com
Tel: 1-831-205-9740
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH 2/3] ARM: dts: mvebu: add sdram controller node to Armada-38x
From: Chris Packham @ 2019-09-26 23:28 UTC (permalink / raw)
To: jason, andrew, gregory.clement, sebastian.hesselbarth, robh+dt,
mark.rutland
Cc: devicetree, Chris Packham, linux-kernel, linux-arm-kernel
In-Reply-To: <20190926232820.27676-1-chris.packham@alliedtelesis.co.nz>
The Armada-38x uses an SDRAM controller that is compatible with the
Armada-XP. The key difference is the width of the bus (XP is 64/32, 38x
is 32/16). The SDRAM controller registers are the same between the two
SoCs.
Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
arch/arm/boot/dts/armada-38x.dtsi | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/arch/arm/boot/dts/armada-38x.dtsi b/arch/arm/boot/dts/armada-38x.dtsi
index 3f4bb44d85f0..e038abc0c6b4 100644
--- a/arch/arm/boot/dts/armada-38x.dtsi
+++ b/arch/arm/boot/dts/armada-38x.dtsi
@@ -103,6 +103,11 @@
#size-cells = <1>;
ranges = <0 MBUS_ID(0xf0, 0x01) 0 0x100000>;
+ sdramc: sdramc@1400 {
+ compatible = "marvell,armada-xp-sdram-controller";
+ reg = <0x1400 0x500>;
+ };
+
L2: cache-controller@8000 {
compatible = "arm,pl310-cache";
reg = <0x8000 0x1000>;
--
2.23.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 0/3] ARM: dts: SDRAM and L2 cache EDAC for Armada SoCs
From: Chris Packham @ 2019-09-26 23:28 UTC (permalink / raw)
To: jason, andrew, gregory.clement, sebastian.hesselbarth, robh+dt,
mark.rutland
Cc: devicetree, Chris Packham, linux-kernel, linux-arm-kernel
This series was waiting for the armada_xp edac driver to be accepted.
Now that it has the relevant nodes can be added to the Armada SoCs. So
that boards can use the EDAC driver if they have the hardware support.
The db-xc3-24g4xg.dts board doesn't have an ECC chip for it's DDR but it
can use the L2 cache parity and ecc support.
Chris Packham (3):
ARM: dts: armada-xp: enable L2 cache parity and ecc on db-xc3-24g4xg
ARM: dts: mvebu: add sdram controller node to Armada-38x
ARM: dts: armada-xp: add label to sdram-controller node
arch/arm/boot/dts/armada-38x.dtsi | 5 +++++
arch/arm/boot/dts/armada-xp-98dx3236.dtsi | 2 +-
arch/arm/boot/dts/armada-xp-db-xc3-24g4xg.dts | 5 +++++
arch/arm/boot/dts/armada-xp.dtsi | 2 +-
4 files changed, 12 insertions(+), 2 deletions(-)
--
2.23.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH 3/3] ARM: dts: armada-xp: add label to sdram-controller node
From: Chris Packham @ 2019-09-26 23:28 UTC (permalink / raw)
To: jason, andrew, gregory.clement, sebastian.hesselbarth, robh+dt,
mark.rutland
Cc: devicetree, Chris Packham, linux-kernel, linux-arm-kernel
In-Reply-To: <20190926232820.27676-1-chris.packham@alliedtelesis.co.nz>
Add the label "sdramc" to the sdram-controller nodes for the Armada-XP
and 98dx3236 SoCs.
Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
arch/arm/boot/dts/armada-xp-98dx3236.dtsi | 2 +-
arch/arm/boot/dts/armada-xp.dtsi | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/armada-xp-98dx3236.dtsi b/arch/arm/boot/dts/armada-xp-98dx3236.dtsi
index 267d0c178e55..654648b05c7c 100644
--- a/arch/arm/boot/dts/armada-xp-98dx3236.dtsi
+++ b/arch/arm/boot/dts/armada-xp-98dx3236.dtsi
@@ -90,7 +90,7 @@
};
internal-regs {
- sdramc@1400 {
+ sdramc: sdramc@1400 {
compatible = "marvell,armada-xp-sdram-controller";
reg = <0x1400 0x500>;
};
diff --git a/arch/arm/boot/dts/armada-xp.dtsi b/arch/arm/boot/dts/armada-xp.dtsi
index ee15c77d3689..6c19984d668e 100644
--- a/arch/arm/boot/dts/armada-xp.dtsi
+++ b/arch/arm/boot/dts/armada-xp.dtsi
@@ -36,7 +36,7 @@
};
internal-regs {
- sdramc@1400 {
+ sdramc: sdramc@1400 {
compatible = "marvell,armada-xp-sdram-controller";
reg = <0x1400 0x500>;
};
--
2.23.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 1/3] ARM: dts: armada-xp: enable L2 cache parity and ecc on db-xc3-24g4xg
From: Chris Packham @ 2019-09-26 23:28 UTC (permalink / raw)
To: jason, andrew, gregory.clement, sebastian.hesselbarth, robh+dt,
mark.rutland
Cc: devicetree, Chris Packham, linux-kernel, linux-arm-kernel
In-Reply-To: <20190926232820.27676-1-chris.packham@alliedtelesis.co.nz>
Enable L2 cache parity and ECC on the db-xc3-24g4xg board so that cache
operations are protected and errors can be flagged to the EDAC
subsystem.
Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
arch/arm/boot/dts/armada-xp-db-xc3-24g4xg.dts | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/arch/arm/boot/dts/armada-xp-db-xc3-24g4xg.dts b/arch/arm/boot/dts/armada-xp-db-xc3-24g4xg.dts
index df048050615f..4ec0ae01b61d 100644
--- a/arch/arm/boot/dts/armada-xp-db-xc3-24g4xg.dts
+++ b/arch/arm/boot/dts/armada-xp-db-xc3-24g4xg.dts
@@ -33,6 +33,11 @@
};
};
+&L2 {
+ arm,parity-enable;
+ marvell,ecc-enable;
+};
+
&devbus_bootcs {
status = "okay";
--
2.23.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH RFC 1/2] clk: introduce clk_invalidate_rate()
From: Stephen Boyd @ 2019-09-27 0:14 UTC (permalink / raw)
To: Neil Armstrong, jbrunet, mturquette
Cc: linux-amlogic, linux-kernel, linux-clk, linux-arm-kernel,
Neil Armstrong
In-Reply-To: <20190919102518.25126-2-narmstrong@baylibre.com>
Quoting Neil Armstrong (2019-09-19 03:25:17)
> This introduces the clk_invalidate_rate() call used to recalculate the
> rate and parent tree of a particular clock if it's known that the
> underlying registers set has been altered by the firmware, like from
> a suspend/resume handler running in trusted cpu mode.
>
> The call refreshes the actual parent and when changed, instructs CCF
> the parent has changed. Finally the call will recalculate the rate of
> each part of the tree to make sure the CCF cached tree is in sync with
> the hardware.
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
The knee-jerk reaction to these patches is that it shouldn't be a
consumer API (i.e. taking a struct clk) but a provider API (i.e. taking
a struct clk_hw). I haven't looked in any more detail but just know that
it's a non-starter to be a consumer based API because we don't want
random consumers out there to be telling the CCF or provider drivers
that some clk has lost state and needs to be "refreshed".
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* RE: [RFC PATCH 18/18] net: wireguard - switch to crypto API for packet encryption
From: Pascal Van Leeuwen @ 2019-09-27 0:15 UTC (permalink / raw)
To: Linus Torvalds
Cc: Jason A . Donenfeld, Catalin Marinas, Herbert Xu, Arnd Bergmann,
Ard Biesheuvel, Greg KH, Eric Biggers, Samuel Neves,
Linux Crypto Mailing List, Andy Lutomirski, Marc Zyngier,
Dan Carpenter, Will Deacon, David Miller, Linux ARM
In-Reply-To: <CAHk-=wgR_KsYw2GmZwkG3GmtX6nbyj0LEi7rSqC+uFi3ScTYcw@mail.gmail.com>
> -----Original Message-----
> From: Linus Torvalds <torvalds@linux-foundation.org>
> Sent: Thursday, September 26, 2019 6:35 PM
> To: Pascal Van Leeuwen <pvanleeuwen@verimatrix.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Linux Crypto Mailing List <linux-
> crypto@vger.kernel.org>; Linux ARM <linux-arm-kernel@lists.infradead.org>; Herbert Xu
> <herbert@gondor.apana.org.au>; David Miller <davem@davemloft.net>; Greg KH
> <gregkh@linuxfoundation.org>; Jason A . Donenfeld <Jason@zx2c4.com>; Samuel Neves
> <sneves@dei.uc.pt>; Dan Carpenter <dan.carpenter@oracle.com>; Arnd Bergmann <arnd@arndb.de>;
> Eric Biggers <ebiggers@google.com>; Andy Lutomirski <luto@kernel.org>; Will Deacon
> <will@kernel.org>; Marc Zyngier <maz@kernel.org>; Catalin Marinas <catalin.marinas@arm.com>
> Subject: Re: [RFC PATCH 18/18] net: wireguard - switch to crypto API for packet encryption
>
> On Thu, Sep 26, 2019 at 2:40 AM Pascal Van Leeuwen
> <pvanleeuwen@verimatrix.com> wrote:
> >
> > While I agree with the principle of first merging Wireguard without
> > hooking it up to the Crypto API and doing the latter in a later,
> > separate patch, I DONT'T agree with your bashing of the Crypto API
> > or HW crypto acceleration in general.
>
> I'm not bashing hardware crypto acceleration.
>
> But I *am* bashing bad interfaces to it.
>
And I'm arguing the interface is not that bad, it is the way it is
for good reasons. I think we all agree it's not suitable for the
occasional crypto operation using a fixed algorithm. For that, by
all means use direct library calls. No one is arguing against that.
However, I dare you to come up with something better that would
provide the same flexibility for doing configurable crypto and
offloading these (combined) operations to crypto acceleration
hardware, depending on its actual capabilities.
i.e. something that would allow offloading rfc7539esp to
accelerators capable of doing that whole transform, while also being
able to offload separate chacha and/or poly operations to less
capable hardware. While actually being able to keep those deep HW
pipelines filled.
> Honestly, you need to face a few facts, Pascal.
>
> Really.
>
> First off: availability.
>
> (a) hardware doesn't exist right now outside your lab
>
> This is a fact.
>
Actually, that's _not_ a fact at all. For three reaons:
a) I don't even have real hardware (for this). We're an IP provider,
we don't make actual hardware. True, I have an FPGA dev board
for prototyping but an ASIC guy like me considers that "just SW".
b) The actual hardware is in our customers labs, so definitely
outside of "my" lab. I don't even _have_ a lab. Just a full desk :-)
c) NXP Layerscape chips supporting Poly-Chacha acceleration can be bought
right now (i.e NXP LX2160A, look it up). CAAM support for Poly-Chacha
has been in mainline since kernel 5.0. So there's your hardware.
And does it matter that it doesn't exist today if it is a known fact
it *will* be there in just a few months? The general idea is that you
make sure the SW support is ready *before* you start selling the HW.
> (b) hardware simply WILL NOT exist in any huge number for years,
> possibly decades. If ever,
>
That remark is just very stupid. The hardware ALREADY exists, and
more hardware is in the pipeline. Once this stuff is designed in, it
usually stays in for many years to come. And these are chips sold in
_serious_ quantities, to be used in things like wireless routers and
DSL, cable and FTTH modems, 5G base stations, etc. etc.
> This is just reality, Pascal.
>
I guess you must live in a different reality from mine? Because I see
some clear mismatch with _known facts_ in *my* reality. But then again,
I'm in this business so I know exactly what's out there. Unlike you.
> Even if you release your hardware tomorrow,
>
We actually released this hardware a looooong time ago, it just takes
very long for silicon to reach the market. And that time is soon.
> where do you think it will exists? Laptops? PC's? Phones?
>
I already answered this above. Many embedded networking use cases.
> No. No. And no.
>
Shouting "no" many times won't make it go away ;-)
> Phones are likely the strongest argument for power use, but phones
> won't really start using it until it's on the SoC, because while they
> care deeply about power, they care even more deeply about a lot of
> other things a whole lot more. Form factor, price, and customers that
> care.
> So phones aren't happening. Not for years, and not until it's a big
> deal and standard IP that everybody wants.
>
It will likely not be OUR HW, as the Qualcomms, Samsungs and Mediateks
of this world are very tough cookies to crack for an IP provider, but
I would expect them to do their own at some point. I would also NOT
expect them to upstream any driver code for that. It may already exist!
> Laptops and PC's? No. Look at all the crypto acceleration they have today.
>
No argument there. If an Intel or AMD adds crypto acceleration, they will
add it to the CPU core itself, for obvious reasons. If you don't actually
design the CPU, you don't have that choice though. (and from a power
consumption perspective, it's actually not that great of a solution)
> That was sarcasm, btw, just to make it clear. It's simply not a market.
>
Eh ... we've been selling into that market for more than 20 years and
we still exist today? How is that possible if it doesn't exist?
> End result: even with hardware, the hardware will be very rare. Maybe
> routers that want to sell particular features in the near future.
>
No, these are just the routers going into *everyone's* home. And 5G
basestations arriving at every other street corner. I wouldn't call
that rare, exactly.
> Again, this isn't theory. This is that pesky little thing called
> "reality". It sucks, I know.
>
You very obviously have absolutely NO idea what you're talking about.
Either that or you're living in some alternate reality.
> But even if you *IGNORE* the fact that hardware doesn't exist right
> now
>
Which I've proven to be FALSE
>, and won't be widely available for years (or longer),
>
Which again doesn't match the FACTS.
> there's another little fact that you are ignoring:
>
> The async crypto interfaces are badly designed. Full stop.
>
They may not be perfect. I think you are free to come up with solutions
to improve on that? But if such a solution would make it impossible to
offload to crypto hardware then *that* would be truly bad interface
design. Do you have similar arguments about the interfacing to e.g.
graphics processing on the GPU? I'm sure those could be simplified to
be easier to use and make a full SW implementation run that much more
efficiently ... (/sarcasm)
> Seriously. This isn't rocket science. This is very very basic Computer
> Science 101.
>
I know. Rocket science is _easy_ ;-)
> Tell me, what's _the_ most important part about optimizing something?
>
> Yeah, it's "you optimize for the common case". But let's ignore that
> part, since I already said that hardware isn't the common case but I
> promised that I'd ignore that part.
>
> The _other_ most important part of optimization is that you optimize
> for the case that _matters_.
>
Matters to _whom_. What matters to someone running a fat desktop or
server CPU is _not_ what matters to someone running on a relatively
weak embedded CPU that _does_ have powerful crypto acceleration on the
side.
And when it comes to the _common_ case: there's actually a heck of a
lot more embedded SoCs out there than x86 server/desktop CPU's. Fact!
You probably just don't know much about most of them.
But if you're arguing that the API should be lightweight and not add
significant overhead to a pure SW implementation, then I can agree.
However, I have not seen any PROOF (i.e. actual measurements, not
theory) that it actually DOES add a lot of overhead. Nor any suggestions
(beyond the hopelessly naive) for improving it.
> And the async crypto case got that completely wrong, and the wireguard
> patch shows that issue very clearly.
>
> The fact is, even you admit that a few CPU cycles don't matter for the
> async case where you have hardware acceleration, because the real cost
> is going to be in the IO - whether it's DMA, cache coherent
> interconnects, or some day some SoC special bus.
>
I was talking _latency_ not _throughput_. I am _very_ interested in
reducing (API/driver) CPU overhead, if only because it doesn't allow
our HW to reach it's full potential. I'm working hard on optimizing our
driver for that right now.
And if something can be improved in the Crypto API itself there, without
compromising it's functionality and flexibility, then I'm all for that.
> The CPU cycles just don't matter, they are entirely in the noise.
>
> What does that mean? Think about it.
>
> [ Time passes ]
>
> Ok, I'll tell you: it means that the interface shouldn't be designed
> for async hw-assisted crypto.
>
If you don't design them with that in mind, you simply won't be able
to effectively use the HW-assisted crypto at all. Just like you don't
design an API to a GPU for running a software implementation on the
CPU, but for minimizing state changes and batch-queuing large strips
of triangles as that's what the _HW_ needs to be efficiently used.
Just sayin'.
> The interface should be designed for the
> case that _does_ care about CPU cycles, and then the async hw-assisted
> crypto should be hidden by a conditional, and its (extra) data
> structures should be the ones that are behind some indirect pointers
> etc. Because, as we agreed, the async external hw case really doesn't
> care. It it has to traverse a pointer or two, and if it has to have a
> *SEPARATE* keystore that has longer lifetimes, then the async code can
> set that up on its own, but that should not affect the case that
> cares.
>
What the hardware cares about is that you can batch queue your requests
and not busy-wait for any results. What the hardware cares about is
that you don't send separate requests for encryption, authentication,
IV generation, etc, but combine this in a single request, hence the
templates in the Crypto API. What the hardware may care about, is that
you keep your key changes limited to just those actually required.
That is _fundamental_ to getting performance.
Note that many SW implementations that require multiple independent
operations in flight to achieve maximum efficiency due to deep
(dual) pipelines, and/or spend significant cycles on running the key
scheduling will ALSO benefit from these properties.
An async interface also makes it possible to run the actual crypto ops
in multiple independent threads, on multiple CPUs, although I'm not
sure if the current Crypto API actually leverages that right now.
> Really, this is fundamental, and really, the crypto interfaces got this wrong.
>
> This is in fact _so_ fundamental that the only possible reason you can
> disagree is because you don't care about reality or fundamentals, and
> you only care about the small particular hardware niche you work in
> and nothing else.
>
Well, to be completely honest, I for sure don't care about making the SW
implementations run faster at the expense of HW offload capabilities.
Which is obvious, as I make a _living_ creating HW offload solutions.
Why would I actively work towards obsoleting my work?
FACT is that dedicated HW, in many cases, is still MUCH faster than the
CPU. At much lower consumption to boot. So why would you NOT want to
leverage that, if it's available? That would just be dumb.
Again, I don't see you making the same argument about moving graphics
functionality from the GPU to the CPU. So why does crypto *have* to be
on the CPU? I just don't understand _why_ you care about that so much.
> You really should think about this a bit.
>
I've been thinking about this daily for about 19 years now. It's my job.
> > However, if you're doing bulk crypto like network packet processing
> > (as Wireguard does!) or disk/filesystem encryption, then that cipher
> > allocation only happens once every blue moon and the overhead for
> > that is totally *irrelevant* as it is amortized over many hours or
> > days of runtime.
>
> This is not true. It's not true because the overhead happens ALL THE TIME.
>
The overhead for the _cipher allocation_ (because that's what _I_ was
talking about specifically) happens all the time? You understand you
really only need to do that twice per connection? (once per direction)
But there will be some overhead on the cipher requests themselves,
sure. A small compromise to make for the _possibility_ to use HW
offload when it IS available. I would agree that if that overhead
turns out to be very significant, then something needs to be done
about that. But then PROVE that that's the case and provide solutions
that do not immediately make HW offload downright impossible.
As our HW is _for sure_ much faster than the CPU cluster (yes, all
CPU's combined at full utilization) it is _usually_ paired with.
> And in 99.9% of all cases there are no upsides from the hw crypto,
> because the hardware DOES NOT EXIST.
>
Which is a _false_ assumption, we covered that several times before.
> You think the "common case" is that hardware encryption case, but see
> above. It's really not.
>
See my argument above about there being many more embedded SoC's out
there than x86 CPU's. Which usually have some form of crypto accel
on the side. Which will, eventually, have Chacha-Poly support
because that's what the industry is currently gravitating towards.
So define _common_ case.
> And when you _do_ have HW encryption, you could do the indirection.
>
Again, not if the API was not architected to do so from the get-go.
> But that's not an argument for always having the indirection.
>
> What indirection am I talking about?
>
> There's multiple levels of indirection in the existing bad crypto interfaces:
>
> (a) the data structures themselves. This is things like keys and
> state storage, both of which are (often) small enough that they would
> be better off on a stack, or embedded in the data structures of the
> callers.
>
> (b) the calling conventions. This is things like indirection -
> usually several levels - just to find the function pointer to call to,
> and then an indirect call to that function pointer.
>
> and both of those are actively bad things when you don't have those
> hardware accelerators.
>
I would say those things are not required just for hardware acceleration,
so perhaps something can be improved there in the existing code.
Ever tried suggesting these to the Crypto API maintainers before?
> When you *do* have them, you don't care. Everybody agrees about that.
> But you're ignoring the big white elephant in the room, which is that
> the hw really is very rare in the end, even if it were to exist at
> all.
>
Crypto acceleration in general is _not_ rare, almost any embedded SoC
has it. The big white elephant in the room is _actually_ that there
never were decent, standard, ubiquitous API's available to use them
so most of them could only be used from dedicated in-house applications.
Which seriously hampered general acceptance and actual _use_.
> > While I generally dislike this whole hype of storing stuff in
> > textual formats like XML and JSON and then wasting lots of CPU
> > cycles on parsing that, I've learned to appreciate the power of
> > these textual Crypto API templates, as they allow a hardware
> > accelerator to advertise complex combined operations as single
> > atomic calls, amortizing the communication overhead between SW
> > and HW. It's actually very flexible and powerful!
>
> BUT IT IS FUNDAMENTALLY BADLY DESIGNED!
>
> Really.
>
> You can get the advantages of hw-accelerated crypto with good designs.
> The current crypto layer is not that.
>
> The current crypto layer simply has indirection at the wrong level.
>
> Here's how it should have been done:
>
> - make the interfaces be direct calls to the crypto you know you want.
>
Which wouldn't work for stuff like IPsec and dmcrypt, where you want
to be able to configure the crypto to be used, i.e. it's _not_ fixed.
And you don't want to have to modify those applications _everytime_ a
new algorithm is added. As the application shouldn't care about that,
it should just be able to leverage it for what it is.
Also, for combined operations, there needs to be some central place
where they are decomposed into optimal sub-operations, if necessary,
without bothering the actual applications with that.
Having a simple direct crypto call is just a very naive solution
that would require changing _every_ application (for which this is
relevant) anytime you add a ciphersuite. It does not scale.
Yes, that will - by necessity - involve some indirection but as long as
you don't process anything crazy short, a single indirect call (or 2)
should really not be that significant on the total operation.
(and modern CPU's can predict indirect branches pretty well, too)
Note that all these arguments have actually _nothing_ to do with
supporting HW acceleration, they apply just as well to SW only.
> - make the basic key and state buffer be explicit and let people
> allocate them on their own stacks or whatever
>
Hey, no argument there. I don't see any good reason why the key can't
be on the stack. I doubt any hardware would be able to DMA that as-is
directly, and in any case, key changes should be infrequent, so copying
it to some DMA buffer should not be a performance problem.
So maybe that's an area for improvement: allow that to be on the stack.
> "But what about hw acceleration?"
>
> - add a single indirect private pointer that the direct calls can use
> for their own state IF THEY HAVE REASON TO
>
> - make the direct crypto calls just have a couple of conditionals
> inside of them
>
> Why? Direct calls with a couple of conditionals are really cheap for
> the non-accelerated case. MUCH cheaper than the indirection overhead
> (both on a state level and on a "crypto description" level) that the
> current code has.
>
I already explained the reasons for _not_ doing direct calls above.
> Seriously. The hw accelerated crypto won't care about the "call a
> static routine" part.
>
Correct! It's totally unrelated.
> The hw accelerated crypto won't care about the
> "I need to allocate a copy of the key because I can't have it on
> stack, and need to have it in a DMA'able region".
>
Our HW surely won't, some HW might care but copying it should be OK.
> The hw accelerated
> crypto won't care about the two extra instructions that do "do I have
> any extra state at all, or should I just do the synchronous CPU
> version" before it gets called through some indirect pointer.
>
Actually, here I _do_ care. I want minimal CPU overhead just a much
as you do, probably even more desperately. But OK, I would be able to
live with that, if that were the _only_ downside.
> So there is absolutely NO DOWNSIDE for hw accelerated crypto to just
> do it right, and use an interface like this:
>
> if (!chacha20poly1305_decrypt_sg(sg, sg, skb->len, NULL, 0,
> PACKET_CB(skb)->nonce, key->key,
> simd_context))
> return false;
>
Well, for one thing, a HW API should not expect the result to be
available when the function call returns. (if that's what you
mean here). That would just be WRONG.
HW offload doesn't work like that. Results come much later, and
you need to keep dispatching more requests until the HW starts
asserting backpressure. You need to keep that HW pipeline filled.
> because for the hw accelerated case the costs are all elsewhere.
>
> But for synchronous encryption code on the CPU? Avoiding the
> indirection can be a huge win. Avoiding allocations, extra cachelines,
> all that overhead. Avoiding indirect calls entirely, because doing a
> few conditional branches (that will predict perfectly) on the state
> will be a lot more efficient, both in direct CPU cycles and in things
> like I$ etc.
>
Again, HW acceleration does not depend on the indirection _at all_,
that's there for entirely different purposes I explained above.
HW acceleration _does_ depend greatly on a truly async ifc though.
So queue requests on one side, handle results from the other side
in some callback func off of an interrupt handler. (with proper
interrupt coalescing, of course, and perhaps some NAPI-like
functionality to further reduce interrupt rates when busy)
> In contrast, forcing people to use this model:
>
> if (unlikely(crypto_aead_reqsize(key->tfm) > 0)) {
> req = aead_request_alloc(key->tfm, GFP_ATOMIC);
> if (!req)
> return false;
> } else {
> req = &stackreq;
> aead_request_set_tfm(req, key->tfm);
> }
>
Agree that is fishy, but it is something that could be fixed.
> aead_request_set_ad(req, 0);
>
I'd rather see this being part of the set_crypt call as well.
I never said I liked _all_ decisions made in the API.
Likely this is because AEAD was added as an afterthought.
> aead_request_set_callback(req, 0, NULL, NULL);
>
This is just inevitable for HW acceration ...
> aead_request_set_crypt(req, sg, sg, skb->len,
> (u8 *)&PACKET_CB(skb)->ivpad);
> err = crypto_aead_decrypt(req);
>
It would probably be more efficient if set_crypt and _decrypt
could be combined in a single call (together with _set_ad).
No argument there and these decisions have _nothing_ to do
with being able to do HW acceleration or not.
Trust me, I have whole list of things I don't like about the
API myself, it's not exacty ideal for HW acceleration either.
(Note that SW overhead actually matters _more_ when you do HW
acceleration, as the HW is often so fast that the SW is the
actual bottleneck!).
But I have faith that, over time, I may be able to get some
improvements in (which should improve both HW _and_ SW use
cases by the way). By working _with_ the Crypto API people
and being _patient_. Not by telling them they suck.
> if (unlikely(req != &stackreq))
> aead_request_free(req);
> if (err)
> return false;
>
> isn't going to help anybody. It sure as hell doesn't help the
> CPU-synchronous case, and see above: it doesn't even help the hw
> accelerated case. It could have had _all_ that "tfm" work behind a
> private pointer that the CPU case never touches except to see "ok,
> it's NULL, I don't have any".
>
> See?
>
Yes, I agree with the point you have here. So let's fix that.
> The interface *should* be that chacha20poly1305_decrypt_sg() library
> interface, just give it a private pointer it can use and update. Then,
> *internally* if can do something like
>
> bool chacha20poly1305_decrypt_sg(...)
> {
> struct cc20p1305_ptr *state = *state_p;
> if (state) {
> .. do basically the above complex thing ..
> return ret; .. or fall back to sw if the hw
> queues are full..
> }
> .. do the CPU only thing..
> }
>
But even the CPU only thing may have several implementations, of which
you want to select the fastest one supported by the _detected_ CPU
features (i.e. SSE, AES-NI, AVX, AVX512, NEON, etc. etc.)
Do you think this would still be efficient if that would be some
large if-else tree? Also, such a fixed implementation wouldn't scale.
> and now you'd have no extra obverhead for the no-hw-accel case, you'd
> have a more pleasant interface to use, and you could still use hw
> accel if you wanted to.
>
I still get the impression you're mostly interested in the "pleasant
interface" while I don't see why that should be more important than
being able to use HW crypto efficiently. That reminds me of way back
when I was a junior designer and some - more senior - software
engineer forced me to implement a full hardware divider(!) for some
parameter that needed to be set only once at initialization time, just
because he was too lazy to do a few simple precomputes in the driver.
He considered that to be "unpleasant" as well.
> THIS is why I say that the crypto interface is bad. It was designed
> for the wrong objectives. It was designed purely for a SSL-like model
> where you do a complex key and algorithm exchange dance, and you
> simply don't know ahead of time what crypto you are even using.
>
I guess it was designed for that, sure. And that's how the IPsec stack
and dmcrypt (to name a few examples) need it. It's also how Wireguard
_will_ need it when we start adding more ciphersuites to Wireguard.
Which is a MUST anyway, if Wireguard wants to be taken seriously:
there MUST be a fallback ciphersuite. At least one. Just in case
Chacha-Poly gets broken overnight somehow, in which case you need to
switch over instantly and can't wait for some new implementation.
If you really _don't_ need that, but just need bit of fixed algorithm
crypto, then by all means, don't go through the Crypto API. I've a
already argued that on many occasions. I think people like Ard are
_already_ working on doing such crypto calls directly.
> And yes, that "I'm doing the SSL thing" used to be a major use of
> encryption. I understand why it happened. It was what people did in
> the 90's. People thought it was a good idea back then, and it was also
> most of the hw acceleration world.
>
> And yes, in that model of "I don't have a clue of what crypto I'm even
> using" the model works fine. But honestly, if you can't admit to
> yourself that it's wrong for the case where you _do_ know the
> algorithm, you have some serious blinders on.
>
But the point is - there are those case where you _don't_ know and
_that_ is what the Crypto API is for. And just generally, crypto
really _should_ be switchable. So you don't need to wait for a
fix to ripple through a kernel release cycle when an algorithm gets
broken. I don't know many use cases for just one fixed algorithm.
> Just from a user standpoint, do you seriously think users _like_
> having to do the above 15+ lines of code, vs the single function call?
>
I know I wouldn't. I also know I would do it anyway as I would
understand _why_ I would be doing it.
> The crypto interface really isn't pleasant, and you're wrong to
> believe that it really helps. The hw acceleration capability could
> have been abstracted away, instead of making that indirection be front
> and center.
>
Again, the Crypto API aims to do more than just allow for HW
acceleration and your main gripes actually seem to be with the
"other" stuff.
> And again - I do realize the historical reasons for it. But
> understanding that doesn't magically make it wonderful.
>
No one said it was wonderful. Or pleasant. Or perfect.
You definitely raised some points that I think _could_ be
improved without compromising any functionality.
But some stuff you don't like just has good reasons to exist.
Reasons you may not agree with, but that doesn't make them invalid.
> Linus
Regards,
Pascal van Leeuwen
Silicon IP Architect, Multi-Protocol Engines @ Verimatrix
www.insidesecure.com
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH 00/11] of: dma-ranges fixes and improvements
From: Rob Herring @ 2019-09-27 0:24 UTC (permalink / raw)
To: devicetree, linux-arm-kernel
Cc: Florian Fainelli, Arnd Bergmann, Frank Rowand, linux-pci,
linux-kernel, Marek Vasut, Lorenzo Pieralisi, Oza Pawandeep,
Stefan Wahren, Simon Horman, Geert Uytterhoeven, Robin Murphy,
Nicolas Saenz Julienne
This series fixes several issues related to 'dma-ranges'. Primarily,
'dma-ranges' in a PCI bridge node does correctly set dma masks for PCI
devices not described in the DT. A common case needing dma-ranges is a
32-bit PCIe bridge on a 64-bit system. This affects several platforms
including Broadcom, NXP, Renesas, and Arm Juno. There's been several
attempts to fix these issues, most recently earlier this week[1].
In the process, I found several bugs in the address translation. It
appears that things have happened to work as various DTs happen to use
1:1 addresses.
First 3 patches are just some clean-up. The 4th patch adds a unittest
exhibiting the issues. Patches 5-9 rework how of_dma_configure() works
making it work on either a struct device child node or a struct
device_node parent node so that it works on bus leaf nodes like PCI
bridges. Patches 10 and 11 fix 2 issues with address translation for
dma-ranges.
My testing on this has been with QEMU virt machine hacked up to set PCI
dma-ranges and the unittest. Nicolas reports this series resolves the
issues on Rpi4 and NXP Layerscape platforms.
Rob
[1] https://lore.kernel.org/linux-arm-kernel/20190924181244.7159-1-nsaenzjulienne@suse.de/
Rob Herring (5):
of: Remove unused of_find_matching_node_by_address()
of: Make of_dma_get_range() private
of/unittest: Add dma-ranges address translation tests
of/address: Translate 'dma-ranges' for parent nodes missing
'dma-ranges'
of/address: Fix of_pci_range_parser_one translation of DMA addresses
Robin Murphy (6):
of: address: Report of_dma_get_range() errors meaningfully
of: Ratify of_dma_configure() interface
of/address: Introduce of_get_next_dma_parent() helper
of: address: Follow DMA parent for "dma-coherent"
of: Factor out #{addr,size}-cells parsing
of: Make of_dma_get_range() work on bus nodes
drivers/of/address.c | 83 +++++++++----------
drivers/of/base.c | 32 ++++---
drivers/of/device.c | 12 ++-
drivers/of/of_private.h | 14 ++++
drivers/of/unittest-data/testcases.dts | 1 +
drivers/of/unittest-data/tests-address.dtsi | 48 +++++++++++
drivers/of/unittest.c | 92 +++++++++++++++++++++
include/linux/of_address.h | 21 +----
include/linux/of_device.h | 4 +-
9 files changed, 227 insertions(+), 80 deletions(-)
create mode 100644 drivers/of/unittest-data/tests-address.dtsi
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH 01/11] of: Remove unused of_find_matching_node_by_address()
From: Rob Herring @ 2019-09-27 0:24 UTC (permalink / raw)
To: devicetree, linux-arm-kernel
Cc: Florian Fainelli, Arnd Bergmann, Frank Rowand, linux-pci,
linux-kernel, Marek Vasut, Lorenzo Pieralisi, Oza Pawandeep,
Stefan Wahren, Simon Horman, Geert Uytterhoeven, Robin Murphy,
Nicolas Saenz Julienne
In-Reply-To: <20190927002455.13169-1-robh@kernel.org>
of_find_matching_node_by_address() is unused, so remove it.
Cc: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Rob Herring <robh@kernel.org>
---
drivers/of/address.c | 19 -------------------
include/linux/of_address.h | 12 ------------
2 files changed, 31 deletions(-)
diff --git a/drivers/of/address.c b/drivers/of/address.c
index 978427a9d5e6..0c3cf515c510 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -826,25 +826,6 @@ int of_address_to_resource(struct device_node *dev, int index,
}
EXPORT_SYMBOL_GPL(of_address_to_resource);
-struct device_node *of_find_matching_node_by_address(struct device_node *from,
- const struct of_device_id *matches,
- u64 base_address)
-{
- struct device_node *dn = of_find_matching_node(from, matches);
- struct resource res;
-
- while (dn) {
- if (!of_address_to_resource(dn, 0, &res) &&
- res.start == base_address)
- return dn;
-
- dn = of_find_matching_node(dn, matches);
- }
-
- return NULL;
-}
-
-
/**
* of_iomap - Maps the memory mapped IO for a given device_node
* @device: the device whose io range will be mapped
diff --git a/include/linux/of_address.h b/include/linux/of_address.h
index 30e40fb6936b..e317f375374a 100644
--- a/include/linux/of_address.h
+++ b/include/linux/of_address.h
@@ -33,10 +33,6 @@ extern u64 of_translate_dma_address(struct device_node *dev,
extern u64 of_translate_address(struct device_node *np, const __be32 *addr);
extern int of_address_to_resource(struct device_node *dev, int index,
struct resource *r);
-extern struct device_node *of_find_matching_node_by_address(
- struct device_node *from,
- const struct of_device_id *matches,
- u64 base_address);
extern void __iomem *of_iomap(struct device_node *device, int index);
void __iomem *of_io_request_and_map(struct device_node *device,
int index, const char *name);
@@ -71,14 +67,6 @@ static inline u64 of_translate_address(struct device_node *np,
return OF_BAD_ADDR;
}
-static inline struct device_node *of_find_matching_node_by_address(
- struct device_node *from,
- const struct of_device_id *matches,
- u64 base_address)
-{
- return NULL;
-}
-
static inline const __be32 *of_get_address(struct device_node *dev, int index,
u64 *size, unsigned int *flags)
{
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 02/11] of: Make of_dma_get_range() private
From: Rob Herring @ 2019-09-27 0:24 UTC (permalink / raw)
To: devicetree, linux-arm-kernel
Cc: Florian Fainelli, Arnd Bergmann, Frank Rowand, linux-pci,
linux-kernel, Marek Vasut, Lorenzo Pieralisi, Oza Pawandeep,
Stefan Wahren, Simon Horman, Geert Uytterhoeven, Robin Murphy,
Nicolas Saenz Julienne
In-Reply-To: <20190927002455.13169-1-robh@kernel.org>
of_dma_get_range() is only used within the DT core code, so remove the
export and move the header declaration to the private header.
Cc: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Rob Herring <robh@kernel.org>
---
drivers/of/address.c | 1 -
drivers/of/of_private.h | 11 +++++++++++
include/linux/of_address.h | 8 --------
3 files changed, 11 insertions(+), 9 deletions(-)
diff --git a/drivers/of/address.c b/drivers/of/address.c
index 0c3cf515c510..8e354d12fb04 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -972,7 +972,6 @@ int of_dma_get_range(struct device_node *np, u64 *dma_addr, u64 *paddr, u64 *siz
return ret;
}
-EXPORT_SYMBOL_GPL(of_dma_get_range);
/**
* of_dma_is_coherent - Check if device is coherent
diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
index 24786818e32e..f8c58615c393 100644
--- a/drivers/of/of_private.h
+++ b/drivers/of/of_private.h
@@ -158,4 +158,15 @@ extern void __of_sysfs_remove_bin_file(struct device_node *np,
#define for_each_transaction_entry_reverse(_oft, _te) \
list_for_each_entry_reverse(_te, &(_oft)->te_list, node)
+#ifdef CONFIG_OF_ADDRESS
+extern int of_dma_get_range(struct device_node *np, u64 *dma_addr,
+ u64 *paddr, u64 *size);
+#else
+static inline int of_dma_get_range(struct device_node *np, u64 *dma_addr,
+ u64 *paddr, u64 *size)
+{
+ return -ENODEV;
+}
+#endif
+
#endif /* _LINUX_OF_PRIVATE_H */
diff --git a/include/linux/of_address.h b/include/linux/of_address.h
index e317f375374a..ddda3936039c 100644
--- a/include/linux/of_address.h
+++ b/include/linux/of_address.h
@@ -51,8 +51,6 @@ extern int of_pci_dma_range_parser_init(struct of_pci_range_parser *parser,
extern struct of_pci_range *of_pci_range_parser_one(
struct of_pci_range_parser *parser,
struct of_pci_range *range);
-extern int of_dma_get_range(struct device_node *np, u64 *dma_addr,
- u64 *paddr, u64 *size);
extern bool of_dma_is_coherent(struct device_node *np);
#else /* CONFIG_OF_ADDRESS */
static inline void __iomem *of_io_request_and_map(struct device_node *device,
@@ -92,12 +90,6 @@ static inline struct of_pci_range *of_pci_range_parser_one(
return NULL;
}
-static inline int of_dma_get_range(struct device_node *np, u64 *dma_addr,
- u64 *paddr, u64 *size)
-{
- return -ENODEV;
-}
-
static inline bool of_dma_is_coherent(struct device_node *np)
{
return false;
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox