* Re: [PATCH v8 0/5] support reserving crashkernel above 4G on arm64 kdump
From: Baoquan He @ 2020-05-26 1:42 UTC (permalink / raw)
To: Chen Zhou
Cc: tglx, mingo, catalin.marinas, will, dyoung, robh+dt,
John.p.donnelly, arnd, devicetree, linux-doc, kexec, linux-kernel,
horms, guohanjun, pkushwaha, linux-arm-kernel
In-Reply-To: <20200521093805.64398-1-chenzhou10@huawei.com>
On 05/21/20 at 05:38pm, Chen Zhou wrote:
> This patch series enable reserving crashkernel above 4G in arm64.
>
> There are following issues in arm64 kdump:
> 1. We use crashkernel=X to reserve crashkernel below 4G, which will fail
> when there is no enough low memory.
> 2. Currently, crashkernel=Y@X can be used to reserve crashkernel above 4G,
> in this case, if swiotlb or DMA buffers are required, crash dump kernel
> will boot failure because there is no low memory available for allocation.
>
> To solve these issues, introduce crashkernel=X,low to reserve specified
> size low memory.
> Crashkernel=X tries to reserve memory for the crash dump kernel under
> 4G. If crashkernel=Y,low is specified simultaneously, reserve spcified
> size low memory for crash kdump kernel devices firstly and then reserve
> memory above 4G.
>
> When crashkernel is reserved above 4G in memory, that is, crashkernel=X,low
> is specified simultaneously, kernel should reserve specified size low memory
> for crash dump kernel devices. So there may be two crash kernel regions, one
> is below 4G, the other is above 4G.
> In order to distinct from the high region and make no effect to the use of
> kexec-tools, rename the low region as "Crash kernel (low)", and add DT property
> "linux,low-memory-range" to crash dump kernel's dtb to pass the low region.
>
> Besides, we need to modify kexec-tools:
> arm64: kdump: add another DT property to crash dump kernel's dtb(see [1])
>
> The previous changes and discussions can be retrieved from:
>
> Changes since [v7]
> - Move x86 CRASH_ALIGN to 2M
> Suggested by Dave and do some test, move x86 CRASH_ALIGN to 2M.
OK, moving x86 CRASH_ALIGN to 2M is suggested by Dave. Because
CONFIG_PHYSICAL_ALIGN can be selected from 2M to 16M. So 2M seems good.
But, anyway, we should tell the reason why it need be changed in commit
log.
arch/x86/Kconfig:
config PHYSICAL_ALIGN
hex "Alignment value to which kernel should be aligned"
default "0x200000"
range 0x2000 0x1000000 if X86_32
range 0x200000 0x1000000 if X86_64
> - Update Documentation/devicetree/bindings/chosen.txt
> Add corresponding documentation to Documentation/devicetree/bindings/chosen.txt suggested by Arnd.
> - Add Tested-by from Jhon and pk
>
> Changes since [v6]
> - Fix build errors reported by kbuild test robot.
>
> Changes since [v5]
> - Move reserve_crashkernel_low() into kernel/crash_core.c.
> - Delete crashkernel=X,high.
And the crashkernel=X,high being deleted need be told too. Otherwise
people reading the commit have to check why themselves. I didn't follow
the old version, can't see why ,high can't be specified explicitly.
> - Modify crashkernel=X,low.
> If crashkernel=X,low is specified simultaneously, reserve spcified size low
> memory for crash kdump kernel devices firstly and then reserve memory above 4G.
> In addition, rename crashk_low_res as "Crash kernel (low)" for arm64, and then
> pass to crash dump kernel by DT property "linux,low-memory-range".
> - Update Documentation/admin-guide/kdump/kdump.rst.
>
> Changes since [v4]
> - Reimplement memblock_cap_memory_ranges for multiple ranges by Mike.
>
> Changes since [v3]
> - Add memblock_cap_memory_ranges back for multiple ranges.
> - Fix some compiling warnings.
>
> Changes since [v2]
> - Split patch "arm64: kdump: support reserving crashkernel above 4G" as
> two. Put "move reserve_crashkernel_low() into kexec_core.c" in a separate
> patch.
>
> Changes since [v1]:
> - Move common reserve_crashkernel_low() code into kernel/kexec_core.c.
> - Remove memblock_cap_memory_ranges() i added in v1 and implement that
> in fdt_enforce_memory_region().
> There are at most two crash kernel regions, for two crash kernel regions
> case, we cap the memory range [min(regs[*].start), max(regs[*].end)]
> and then remove the memory range in the middle.
>
> [1]: http://lists.infradead.org/pipermail/kexec/2020-May/025128.html
> [v1]: https://lkml.org/lkml/2019/4/2/1174
> [v2]: https://lkml.org/lkml/2019/4/9/86
> [v3]: https://lkml.org/lkml/2019/4/9/306
> [v4]: https://lkml.org/lkml/2019/4/15/273
> [v5]: https://lkml.org/lkml/2019/5/6/1360
> [v6]: https://lkml.org/lkml/2019/8/30/142
> [v7]: https://lkml.org/lkml/2019/12/23/411
>
> Chen Zhou (5):
> x86: kdump: move reserve_crashkernel_low() into crash_core.c
> arm64: kdump: reserve crashkenel above 4G for crash dump kernel
> arm64: kdump: add memory for devices by DT property, low-memory-range
> kdump: update Documentation about crashkernel on arm64
> dt-bindings: chosen: Document linux,low-memory-range for arm64 kdump
>
> Documentation/admin-guide/kdump/kdump.rst | 13 ++-
> .../admin-guide/kernel-parameters.txt | 12 ++-
> Documentation/devicetree/bindings/chosen.txt | 25 ++++++
> arch/arm64/kernel/setup.c | 8 +-
> arch/arm64/mm/init.c | 61 ++++++++++++-
> arch/x86/kernel/setup.c | 66 ++------------
> include/linux/crash_core.h | 3 +
> include/linux/kexec.h | 2 -
> kernel/crash_core.c | 85 +++++++++++++++++++
> kernel/kexec_core.c | 17 ----
> 10 files changed, 208 insertions(+), 84 deletions(-)
>
> --
> 2.20.1
>
>
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
>
^ permalink raw reply
* Re: [PATCH v8 2/5] arm64: kdump: reserve crashkenel above 4G for crash dump kernel
From: Baoquan He @ 2020-05-26 0:59 UTC (permalink / raw)
To: Chen Zhou
Cc: tglx, mingo, catalin.marinas, will, dyoung, robh+dt,
John.p.donnelly, arnd, devicetree, linux-doc, kexec, linux-kernel,
horms, guohanjun, pkushwaha, linux-arm-kernel
In-Reply-To: <20200521093805.64398-3-chenzhou10@huawei.com>
On 05/21/20 at 05:38pm, Chen Zhou wrote:
> Crashkernel=X tries to reserve memory for the crash dump kernel under
> 4G. If crashkernel=X,low is specified simultaneously, reserve spcified
> size low memory for crash kdump kernel devices firstly and then reserve
> memory above 4G.
Wondering why crashkernel=,high is not introduced to arm64 to be
consistent with x86_64, to make the behaviour be the same on all
architecutres.
>
> Signed-off-by: Chen Zhou <chenzhou10@huawei.com>
> Tested-by: John Donnelly <John.p.donnelly@oracle.com>
> Tested-by: Prabhakar Kushwaha <pkushwaha@marvell.com>
> ---
> arch/arm64/kernel/setup.c | 8 +++++++-
> arch/arm64/mm/init.c | 31 +++++++++++++++++++++++++++++--
> 2 files changed, 36 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
> index 3fd2c11c09fc..a8487e4d3e5a 100644
> --- a/arch/arm64/kernel/setup.c
> +++ b/arch/arm64/kernel/setup.c
> @@ -238,7 +238,13 @@ static void __init request_standard_resources(void)
> kernel_data.end <= res->end)
> request_resource(res, &kernel_data);
> #ifdef CONFIG_KEXEC_CORE
> - /* Userspace will find "Crash kernel" region in /proc/iomem. */
> + /*
> + * Userspace will find "Crash kernel" region in /proc/iomem.
> + * Note: the low region is renamed as Crash kernel (low).
> + */
> + if (crashk_low_res.end && crashk_low_res.start >= res->start &&
> + crashk_low_res.end <= res->end)
> + request_resource(res, &crashk_low_res);
> if (crashk_res.end && crashk_res.start >= res->start &&
> crashk_res.end <= res->end)
> request_resource(res, &crashk_res);
> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> index e42727e3568e..71498acf0cd8 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -81,6 +81,7 @@ static void __init reserve_crashkernel(void)
> {
> unsigned long long crash_base, crash_size;
> int ret;
> + phys_addr_t crash_max = arm64_dma32_phys_limit;
>
> ret = parse_crashkernel(boot_command_line, memblock_phys_mem_size(),
> &crash_size, &crash_base);
> @@ -88,12 +89,38 @@ static void __init reserve_crashkernel(void)
> if (ret || !crash_size)
> return;
>
> + ret = reserve_crashkernel_low();
> + if (!ret && crashk_low_res.end) {
> + /*
> + * If crashkernel=X,low specified, there may be two regions,
> + * we need to make some changes as follows:
> + *
> + * 1. rename the low region as "Crash kernel (low)"
> + * In order to distinct from the high region and make no effect
> + * to the use of existing kexec-tools, rename the low region as
> + * "Crash kernel (low)".
> + *
> + * 2. change the upper bound for crash memory
> + * Set MEMBLOCK_ALLOC_ACCESSIBLE upper bound for crash memory.
> + *
> + * 3. mark the low region as "nomap"
> + * The low region is intended to be used for crash dump kernel
> + * devices, just mark the low region as "nomap" simply.
> + */
> + const char *rename = "Crash kernel (low)";
> +
> + crashk_low_res.name = rename;
> + crash_max = MEMBLOCK_ALLOC_ACCESSIBLE;
> + memblock_mark_nomap(crashk_low_res.start,
> + resource_size(&crashk_low_res));
> + }
> +
> crash_size = PAGE_ALIGN(crash_size);
>
> if (crash_base == 0) {
> /* Current arm64 boot protocol requires 2MB alignment */
> - crash_base = memblock_find_in_range(0, arm64_dma32_phys_limit,
> - crash_size, SZ_2M);
> + crash_base = memblock_find_in_range(0, crash_max, crash_size,
> + SZ_2M);
> if (crash_base == 0) {
> pr_warn("cannot allocate crashkernel (size:0x%llx)\n",
> crash_size);
> --
> 2.20.1
>
>
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
>
^ permalink raw reply
* Re: [PATCH v8 1/5] x86: kdump: move reserve_crashkernel_low() into crash_core.c
From: Baoquan He @ 2020-05-26 0:56 UTC (permalink / raw)
To: Chen Zhou
Cc: tglx, mingo, catalin.marinas, will, dyoung, robh+dt,
John.p.donnelly, arnd, devicetree, linux-doc, kexec, linux-kernel,
horms, guohanjun, pkushwaha, linux-arm-kernel
In-Reply-To: <20200521093805.64398-2-chenzhou10@huawei.com>
On 05/21/20 at 05:38pm, Chen Zhou wrote:
> In preparation for supporting reserve_crashkernel_low in arm64 as
> x86_64 does, move reserve_crashkernel_low() into kernel/crash_core.c.
> BTW, move x86 CRASH_ALIGN to 2M.
The reason is?
>
> Note, in arm64, we reserve low memory if and only if crashkernel=X,low
> is specified. Different with x86_64, don't set low memory automatically.
>
> Reported-by: kbuild test robot <lkp@intel.com>
> Signed-off-by: Chen Zhou <chenzhou10@huawei.com>
> Tested-by: John Donnelly <John.p.donnelly@oracle.com>
> Tested-by: Prabhakar Kushwaha <pkushwaha@marvell.com>
> ---
> arch/x86/kernel/setup.c | 66 ++++-------------------------
> include/linux/crash_core.h | 3 ++
> include/linux/kexec.h | 2 -
> kernel/crash_core.c | 85 ++++++++++++++++++++++++++++++++++++++
> kernel/kexec_core.c | 17 --------
> 5 files changed, 96 insertions(+), 77 deletions(-)
>
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index 4b3fa6cd3106..de75fec73d47 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -395,8 +395,8 @@ static void __init memblock_x86_reserve_range_setup_data(void)
>
> #ifdef CONFIG_KEXEC_CORE
>
> -/* 16M alignment for crash kernel regions */
> -#define CRASH_ALIGN SZ_16M
> +/* 2M alignment for crash kernel regions */
> +#define CRASH_ALIGN SZ_2M
>
> /*
> * Keep the crash kernel below this limit.
> @@ -419,59 +419,6 @@ static void __init memblock_x86_reserve_range_setup_data(void)
> # define CRASH_ADDR_HIGH_MAX SZ_64T
> #endif
>
> -static int __init reserve_crashkernel_low(void)
> -{
> -#ifdef CONFIG_X86_64
> - unsigned long long base, low_base = 0, low_size = 0;
> - unsigned long total_low_mem;
> - int ret;
> -
> - total_low_mem = memblock_mem_size(1UL << (32 - PAGE_SHIFT));
> -
> - /* crashkernel=Y,low */
> - ret = parse_crashkernel_low(boot_command_line, total_low_mem, &low_size, &base);
> - if (ret) {
> - /*
> - * two parts from kernel/dma/swiotlb.c:
> - * -swiotlb size: user-specified with swiotlb= or default.
> - *
> - * -swiotlb overflow buffer: now hardcoded to 32k. We round it
> - * to 8M for other buffers that may need to stay low too. Also
> - * make sure we allocate enough extra low memory so that we
> - * don't run out of DMA buffers for 32-bit devices.
> - */
> - low_size = max(swiotlb_size_or_default() + (8UL << 20), 256UL << 20);
> - } else {
> - /* passed with crashkernel=0,low ? */
> - if (!low_size)
> - return 0;
> - }
> -
> - low_base = memblock_find_in_range(0, 1ULL << 32, low_size, CRASH_ALIGN);
> - if (!low_base) {
> - pr_err("Cannot reserve %ldMB crashkernel low memory, please try smaller size.\n",
> - (unsigned long)(low_size >> 20));
> - return -ENOMEM;
> - }
> -
> - ret = memblock_reserve(low_base, low_size);
> - if (ret) {
> - pr_err("%s: Error reserving crashkernel low memblock.\n", __func__);
> - return ret;
> - }
> -
> - pr_info("Reserving %ldMB of low memory at %ldMB for crashkernel (System low RAM: %ldMB)\n",
> - (unsigned long)(low_size >> 20),
> - (unsigned long)(low_base >> 20),
> - (unsigned long)(total_low_mem >> 20));
> -
> - crashk_low_res.start = low_base;
> - crashk_low_res.end = low_base + low_size - 1;
> - insert_resource(&iomem_resource, &crashk_low_res);
> -#endif
> - return 0;
> -}
> -
> static void __init reserve_crashkernel(void)
> {
> unsigned long long crash_size, crash_base, total_mem;
> @@ -535,9 +482,12 @@ static void __init reserve_crashkernel(void)
> return;
> }
>
> - if (crash_base >= (1ULL << 32) && reserve_crashkernel_low()) {
> - memblock_free(crash_base, crash_size);
> - return;
> + if (crash_base >= (1ULL << 32)) {
> + if (reserve_crashkernel_low()) {
> + memblock_free(crash_base, crash_size);
> + return;
> + }
> + insert_resource(&iomem_resource, &crashk_low_res);
> }
>
> pr_info("Reserving %ldMB of memory at %ldMB for crashkernel (System RAM: %ldMB)\n",
> diff --git a/include/linux/crash_core.h b/include/linux/crash_core.h
> index 525510a9f965..4df8c0bff03e 100644
> --- a/include/linux/crash_core.h
> +++ b/include/linux/crash_core.h
> @@ -63,6 +63,8 @@ phys_addr_t paddr_vmcoreinfo_note(void);
> extern unsigned char *vmcoreinfo_data;
> extern size_t vmcoreinfo_size;
> extern u32 *vmcoreinfo_note;
> +extern struct resource crashk_res;
> +extern struct resource crashk_low_res;
>
> Elf_Word *append_elf_note(Elf_Word *buf, char *name, unsigned int type,
> void *data, size_t data_len);
> @@ -74,5 +76,6 @@ int parse_crashkernel_high(char *cmdline, unsigned long long system_ram,
> unsigned long long *crash_size, unsigned long long *crash_base);
> int parse_crashkernel_low(char *cmdline, unsigned long long system_ram,
> unsigned long long *crash_size, unsigned long long *crash_base);
> +int __init reserve_crashkernel_low(void);
>
> #endif /* LINUX_CRASH_CORE_H */
> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> index 1776eb2e43a4..5d5d9635b18d 100644
> --- a/include/linux/kexec.h
> +++ b/include/linux/kexec.h
> @@ -330,8 +330,6 @@ extern int kexec_load_disabled;
>
> /* Location of a reserved region to hold the crash kernel.
> */
> -extern struct resource crashk_res;
> -extern struct resource crashk_low_res;
> extern note_buf_t __percpu *crash_notes;
>
> /* flag to track if kexec reboot is in progress */
> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
> index 9f1557b98468..a7580d291c37 100644
> --- a/kernel/crash_core.c
> +++ b/kernel/crash_core.c
> @@ -7,6 +7,8 @@
> #include <linux/crash_core.h>
> #include <linux/utsname.h>
> #include <linux/vmalloc.h>
> +#include <linux/memblock.h>
> +#include <linux/swiotlb.h>
>
> #include <asm/page.h>
> #include <asm/sections.h>
> @@ -19,6 +21,22 @@ u32 *vmcoreinfo_note;
> /* trusted vmcoreinfo, e.g. we can make a copy in the crash memory */
> static unsigned char *vmcoreinfo_data_safecopy;
>
> +/* Location of the reserved area for the crash kernel */
> +struct resource crashk_res = {
> + .name = "Crash kernel",
> + .start = 0,
> + .end = 0,
> + .flags = IORESOURCE_BUSY | IORESOURCE_SYSTEM_RAM,
> + .desc = IORES_DESC_CRASH_KERNEL
> +};
> +struct resource crashk_low_res = {
> + .name = "Crash kernel",
> + .start = 0,
> + .end = 0,
> + .flags = IORESOURCE_BUSY | IORESOURCE_SYSTEM_RAM,
> + .desc = IORES_DESC_CRASH_KERNEL
> +};
> +
> /*
> * parsing the "crashkernel" commandline
> *
> @@ -292,6 +310,73 @@ int __init parse_crashkernel_low(char *cmdline,
> "crashkernel=", suffix_tbl[SUFFIX_LOW]);
> }
>
> +#if defined(CONFIG_X86_64) || defined(CONFIG_ARM64)
> +#define CRASH_ALIGN SZ_2M
> +#endif
> +
> +int __init reserve_crashkernel_low(void)
> +{
> +#if defined(CONFIG_X86_64) || defined(CONFIG_ARM64)
> + unsigned long long base, low_base = 0, low_size = 0;
> + unsigned long total_low_mem;
> + int ret;
> +
> + total_low_mem = memblock_mem_size(1UL << (32 - PAGE_SHIFT));
> +
> + /* crashkernel=Y,low */
> + ret = parse_crashkernel_low(boot_command_line, total_low_mem, &low_size,
> + &base);
> + if (ret) {
> +#ifdef CONFIG_X86_64
> + /*
> + * two parts from lib/swiotlb.c:
> + * -swiotlb size: user-specified with swiotlb= or default.
> + *
> + * -swiotlb overflow buffer: now hardcoded to 32k. We round it
> + * to 8M for other buffers that may need to stay low too. Also
> + * make sure we allocate enough extra low memory so that we
> + * don't run out of DMA buffers for 32-bit devices.
> + */
> + low_size = max(swiotlb_size_or_default() + (8UL << 20),
> + 256UL << 20);
> +#else
> + /*
> + * in arm64, reserve low memory if and only if crashkernel=X,low
> + * specified.
> + */
> + return -EINVAL;
> +#endif
> + } else {
> + /* passed with crashkernel=0,low ? */
> + if (!low_size)
> + return 0;
> + }
> +
> + low_base = memblock_find_in_range(0, 1ULL << 32, low_size, CRASH_ALIGN);
> + if (!low_base) {
> + pr_err("Cannot reserve %ldMB crashkernel low memory, please try smaller size.\n",
> + (unsigned long)(low_size >> 20));
> + return -ENOMEM;
> + }
> +
> + ret = memblock_reserve(low_base, low_size);
> + if (ret) {
> + pr_err("%s: Error reserving crashkernel low memblock.\n",
> + __func__);
> + return ret;
> + }
> +
> + pr_info("Reserving %ldMB of low memory at %ldMB for crashkernel (System low RAM: %ldMB)\n",
> + (unsigned long)(low_size >> 20),
> + (unsigned long)(low_base >> 20),
> + (unsigned long)(total_low_mem >> 20));
> +
> + crashk_low_res.start = low_base;
> + crashk_low_res.end = low_base + low_size - 1;
> +#endif
> + return 0;
> +}
> +
> Elf_Word *append_elf_note(Elf_Word *buf, char *name, unsigned int type,
> void *data, size_t data_len)
> {
> diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
> index c19c0dad1ebe..db66bbabfff3 100644
> --- a/kernel/kexec_core.c
> +++ b/kernel/kexec_core.c
> @@ -53,23 +53,6 @@ note_buf_t __percpu *crash_notes;
> /* Flag to indicate we are going to kexec a new kernel */
> bool kexec_in_progress = false;
>
> -
> -/* Location of the reserved area for the crash kernel */
> -struct resource crashk_res = {
> - .name = "Crash kernel",
> - .start = 0,
> - .end = 0,
> - .flags = IORESOURCE_BUSY | IORESOURCE_SYSTEM_RAM,
> - .desc = IORES_DESC_CRASH_KERNEL
> -};
> -struct resource crashk_low_res = {
> - .name = "Crash kernel",
> - .start = 0,
> - .end = 0,
> - .flags = IORESOURCE_BUSY | IORESOURCE_SYSTEM_RAM,
> - .desc = IORES_DESC_CRASH_KERNEL
> -};
> -
> int kexec_should_crash(struct task_struct *p)
> {
> /*
> --
> 2.20.1
>
>
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
>
^ permalink raw reply
* Re: [PATCH v4 01/16] spi: dw: Add Tx/Rx finish wait methods to the MID DMA
From: Serge Semin @ 2020-05-25 21:36 UTC (permalink / raw)
To: Mark Brown
Cc: Serge Semin, Andy Shevchenko, Vinod Koul, Feng Tang, Grant Likely,
Georgy Vlasov, Ramil Zaripov, Alexey Malahov, Thomas Bogendoerfer,
Paul Burton, Ralf Baechle, Arnd Bergmann, Rob Herring, linux-mips,
devicetree, Wan Ahmad Zainie, Thomas Gleixner, Jarkko Nikula,
wuxu.wu, Clement Leger, Linus Walleij, linux-spi, linux-kernel
In-Reply-To: <20200525114132.GC4544@sirena.org.uk>
On Mon, May 25, 2020 at 12:41:32PM +0100, Mark Brown wrote:
> On Sat, May 23, 2020 at 11:34:10AM +0300, Serge Semin wrote:
> > On Fri, May 22, 2020 at 04:22:41PM +0100, Mark Brown wrote:
>
> > > Right, that definitely needs to be fixed then - 8MHz is indeed a totally
> > > normal clock rate for SPI so people will hit it. I guess if there's a
> > > noticable performance hit to defer to thread then we could implement
> > > both and look at how long the delay is going to be to decide which to
> > > use, that's annoyingly complicated though so if the overhead is small
> > > enough we could just not bother.
>
> > As I suggested before we can implement a solution without performance drop.
> > Just wait for the DMA completion locally in the dw_spi_dma_transfer() method and
> > return 0 instead of 1 from the transfer_one() callback. In that function we'll
> > wait while DMA finishes its business, after that we can check the Tx/Rx FIFO
> > emptiness and wait for the data to be completely transferred with delays or
> > sleeps or whatever.
>
> No extra context switches there at least, that's the main issue.
Right. There won't be extra context switch.
>
> > NOTE Currently the DW APB SSI driver doesn't set xfer->effective_speed_hz, though as
> > far as I can see that field exists there to be initialized by the SPI controller
> > driver, right? If so, strange it isn't done in any SPI drivers...
>
> Yes. Not that many people are concerned about the exact timing it turns
> out, the work that was being used for never fully made it upstream.
>
> > What do think about this?
>
> Sure.
Great. I'll send a new patchset soon. It'll fix the Tx/Rx non-empty issue in
accordance with the proposed design.
-Sergey
>
> > patchset "spi: dw: Add generic DW DMA controller support" (it's being under
> > review in this email thread) ? Anyway, if the fixup is getting to be that
> > complicated, will it have to be backported to another stable kernels?
>
> No, if it's too invasive it shouldn't be (though the stable people might
> decide they want it anyway these days :/ ).
^ permalink raw reply
* Re: [PATCH v2 2/2] hwmon: Add Baikal-T1 PVT sensor driver
From: Serge Semin @ 2020-05-25 21:26 UTC (permalink / raw)
To: Guenter Roeck
Cc: Serge Semin, Jean Delvare, Jonathan Corbet, Maxim Kaurkin,
Alexey Malahov, Thomas Bogendoerfer, Paul Burton, Ralf Baechle,
Arnd Bergmann, Rob Herring, linux-mips, devicetree,
Jonathan Cameron, Linus Walleij, Nuno Sá, Jisheng Zhang,
Paul Cercueil, Dan Robertson, Eric Tremblay, Beniamin Bia,
Jean Delvare, Dr. David Alan Gilbert, linux-hwmon, linux-doc,
linux-kernel
In-Reply-To: <20200522141955.GA172468@roeck-us.net>
On Fri, May 22, 2020 at 07:19:55AM -0700, Guenter Roeck wrote:
> On Sun, May 10, 2020 at 01:32:11PM +0300, Serge Semin wrote:
> > Baikal-T1 SoC provides an embedded process, voltage and temperature
> > sensor to monitor an internal SoC environment (chip temperature, supply
> > voltage and process monitor) and on time detect critical situations,
> > which may cause the system instability and even damages. The IP-block
> > is based on the Analog Bits PVT sensor, but is equipped with a
> > dedicated control wrapper, which provides a MMIO registers-based access
> > to the sensor core functionality (APB3-bus based) and exposes an
> > additional functions like thresholds/data ready interrupts, its status
> > and masks, measurements timeout. All of these is used to create a hwmon
> > driver being added to the kernel by this commit.
> >
> > The driver implements support for the hardware monitoring capabilities
> > of Baikal-T1 process, voltage and temperature sensors. PVT IP-core
> > consists of one temperature and four voltage sensors, each of which is
> > implemented as a dedicated hwmon channel config.
> >
> > The driver can optionally provide the hwmon alarms for each sensor the
> > PVT controller supports. The alarms functionality is made compile-time
> > configurable due to the hardware interface implementation peculiarity,
> > which is connected with an ability to convert data from only one sensor
> > at a time. Additional limitation is that the controller performs the
> > thresholds checking synchronously with the data conversion procedure.
> > Due to these limitations in order to have the hwmon alarms
> > automatically detected the driver code must switch from one sensor to
> > another, read converted data and manually check the threshold status
> > bits. Depending on the measurements timeout settings this design may
> > cause additional burden on the system performance. By default if the
> > alarms kernel config is disabled the data conversion is performed by
> > the driver on demand when read operation is requested via corresponding
> > _input-file.
> >
> > Co-developed-by: Maxim Kaurkin <maxim.kaurkin@baikalelectronics.ru>
> > Signed-off-by: Maxim Kaurkin <maxim.kaurkin@baikalelectronics.ru>
> > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> > Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> > Cc: Paul Burton <paulburton@kernel.org>
> > Cc: Ralf Baechle <ralf@linux-mips.org>
> > Cc: Arnd Bergmann <arnd@arndb.de>
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Cc: linux-mips@vger.kernel.org
> > Cc: devicetree@vger.kernel.org
> > ---
> >
> > Changelog v2:
> > - Discard handwritten IO-access wrappers. Use normal readl/writel instead.
> > - Use generic FIELD_{GET,PREP} macros instead of handwritten ones.
> > - Since the driver depends on the OF config we can remove of_match_ptr()
> > macro utilization.
> > - Don't print error-message if no platform IRQ found. Just return an error.
> > - Remove probe-status info string printout.
> > ---
> > Documentation/hwmon/bt1-pvt.rst | 113 +++
>
> Needs to be added to index.rst.
Ok.
>
> > drivers/hwmon/Kconfig | 29 +
> > drivers/hwmon/Makefile | 1 +
> > drivers/hwmon/bt1-pvt.c | 1154 +++++++++++++++++++++++++++++++
> > drivers/hwmon/bt1-pvt.h | 237 +++++++
> > 5 files changed, 1534 insertions(+)
> > create mode 100644 Documentation/hwmon/bt1-pvt.rst
> > create mode 100644 drivers/hwmon/bt1-pvt.c
> > create mode 100644 drivers/hwmon/bt1-pvt.h
> >
> > diff --git a/Documentation/hwmon/bt1-pvt.rst b/Documentation/hwmon/bt1-pvt.rst
> > new file mode 100644
> > index 000000000000..7ad8574bce98
> > --- /dev/null
> > +++ b/Documentation/hwmon/bt1-pvt.rst
> > @@ -0,0 +1,113 @@
> > +.. SPDX-License-Identifier: GPL-2.0-only
> > +
> > +
[nip]
> > +Sysfs Attributes
> > +----------------
> > +
> > +Following is a list of all sysfs attributes that the driver provides, their
> > +permissions and a short description:
> > +
> > +=============================== ======= =======================================
> > +Name Perm Description
> > +=============================== ======= =======================================
> > +update_interval RW Measurements update interval per
> > + sensor.
> > +temp1_type RO Sensor type (always 1 as CPU embedded
> > + diode).
> > +temp1_label RO MIPS P5600 Temperature sensor.
> > +temp1_input RO Measured temperature in millidegree
> > + Celsius.
> > +temp1_min RW Low limit for temp input.
> > +temp1_max RW High limit for temp input.
> > +temp1_min_alarm RO Temperature input alarm. Returns 1 if
> > + temperature input went below min limit,
> > + 0 otherwise.
> > +temp1_max_alarm RO Temperature input alarm. Returns 1 if
> > + temperature input went above max limit,
> > + 0 otherwise.
> > +temp1_trim RW Temperature sensor trimming factor
> > + (specific to the SoC application).
>
> This needs a much better documentation as well as explanation why it needs to be
> a sysfs attribute instead of a devicetree node (ie why it needs to be
> runtime-controlled).
Trimming factor can be used to manually tune the temperature sensor characteristics
(simple speaking shifts one with respect to the base range limits) in case if in
accordance to some more reliable source of the temperature measurements, they
don't correspond to the real core temperature.
In my opinion, it would be better to have both sysfs node and DT property. The former
will be used to select a proper value at run-time when the system engineer finds
out the temperature sensor lies regarding the real core temperature. So by
picking up a proper trim value, one will make sure the sensor shows a real
temperature in accordance with a reliable reference device measurements. Then
the later (DT property) can be used to fix the determined trim value to the
particular platform configuration in the DTS file of the corresponding board.
Additionally if there is no way to alter the DT file, but a user needs to tune
the temperature sensor measurements up, then sysfs interface will be the only
way to do so.
I can also improve the description like this:
-temp1_trim RW Temperature sensor trimming factor
- (specific to the SoC application).
+temp1_trim RW Temperature sensor trimming factor
+ can be used to manually tune the
+ absolute temperature measurements up.
What do you think about new description and having both sysf node and DT
property to set the temperature trim value?
>
> > +in[0-3]_label RO MIPS P5600 Voltage sensor (either core
> > + or low/high/standard thresholds).
> > +in[0-3]_input RO Measured voltage in millivolts.
> > +in[0-3]_min RW Low limit for voltage input.
> > +in[0-3]_max RW High limit for voltage input.
> > +in[0-3]_min_alarm RO Voltage input alarm. Returns 1 if
> > + voltage input went below min limit,
> > + 0 otherwise.
> > +in[0-3]_max_alarm RO Voltage input alarm. Returns 1 if
> > + voltage input went above max limit,
> > + 0 otherwise.
> > +=============================== ======= =======================================
> > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> > index 4c62f900bf7e..b80fc7d398c3 100644
> > --- a/drivers/hwmon/Kconfig
> > +++ b/drivers/hwmon/Kconfig
> > @@ -404,6 +404,35 @@ config SENSORS_ATXP1
> > This driver can also be built as a module. If so, the module
> > will be called atxp1.
> >
> > +config SENSORS_BT1_PVT
> > + tristate "Baikal-T1 Process, Voltage, Temperature sensor driver"
> > + depends on (MIPS_BAIKAL_T1 && OF) || COMPILE_TEST
> > + help
> > + If you say yes here you get support for Baikal-T1 PVT sensor
> > + embedded into the SoC.
> > +
> > + This driver can also be built as a module. If so, the module will be
> > + called bt1-pvt.
> > +
> > +if SENSORS_BT1_PVT
> > +
> > +config SENSORS_BT1_PVT_ALARMS
> > + bool "Enable Baikal-T1 PVT sensor alarms"
> > + default n
>
> "default n" is unnecessary. Also, instead of "if SENSORS_BT1_PVT", please
> use "depends on SENSORS_BT1_PVT".
Ok.
>
> > + help
> > + Baikal-T1 PVT IP-block provides threshold registers for each
> > + supported sensor. But the corresponding interrupts might be
> > + generated by the thresholds comparator only in synchronization with
> > + a data conversion. Additionally there is only one sensor data can
> > + be converted at a time. All of these makes the interface impossible
> > + to be used for the hwmon alarms implementation without periodic
> > + switch between the PVT sensors. By default the data conversion is
> > + performed on demand from the user-space. If this config is enabled
> > + the data conversion will be periodically performed and the data will be
> > + saved in the internal driver cache.
> > +
> > +endif
> > +
> > config SENSORS_DRIVETEMP
> > tristate "Hard disk drives with temperature sensors"
> > depends on SCSI && ATA
> > diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> > index b0b9c8e57176..408b86a5e71d 100644
> > --- a/drivers/hwmon/Makefile
> > +++ b/drivers/hwmon/Makefile
> > @@ -53,6 +53,7 @@ obj-$(CONFIG_SENSORS_ASC7621) += asc7621.o
> > obj-$(CONFIG_SENSORS_ASPEED) += aspeed-pwm-tacho.o
> > obj-$(CONFIG_SENSORS_ATXP1) += atxp1.o
> > obj-$(CONFIG_SENSORS_AXI_FAN_CONTROL) += axi-fan-control.o
> > +obj-$(CONFIG_SENSORS_BT1_PVT) += bt1-pvt.o
> > obj-$(CONFIG_SENSORS_CORETEMP) += coretemp.o
> > obj-$(CONFIG_SENSORS_DA9052_ADC)+= da9052-hwmon.o
> > obj-$(CONFIG_SENSORS_DA9055)+= da9055-hwmon.o
> > diff --git a/drivers/hwmon/bt1-pvt.c b/drivers/hwmon/bt1-pvt.c
> > new file mode 100644
> > index 000000000000..1a29fd01aa2b
> > --- /dev/null
> > +++ b/drivers/hwmon/bt1-pvt.c
> > @@ -0,0 +1,1154 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright (C) 2020 BAIKAL ELECTRONICS, JSC
> > + *
> > + * Authors:
> > + * Maxim Kaurkin <maxim.kaurkin@baikalelectronics.ru>
> > + * Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > + *
> > + * Baikal-T1 Process, Voltage, Temperature sensor driver
> > + */
> > +
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/types.h>
> > +#include <linux/bitfield.h>
> > +#include <linux/device.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/bitops.h>
> > +#include <linux/mutex.h>
> > +#include <linux/seqlock.h>
> > +#include <linux/completion.h>
> > +#include <linux/limits.h>
> > +#include <linux/ktime.h>
> > +#include <linux/clk.h>
> > +#include <linux/sysfs.h>
> > +#include <linux/hwmon.h>
> > +#include <linux/hwmon-sysfs.h>
>
> Alphabetic order, please.
Ok.
>
> > +
> > +#include "bt1-pvt.h"
> > +
[nip]
> > +
> > +static irqreturn_t pvt_soft_isr(int irq, void *data)
> > +{
> > + const struct pvt_sensor_info *info;
> > + struct pvt_hwmon *pvt = data;
> > + struct pvt_cache *cache;
> > + u32 val, thres_sts, old;
> > +
> > + /*
> > + * DVALID bit will be cleared by reading the data. We need to save the
> > + * status before the next conversion happens. Threshold events will be
> > + * handled a bit later.
> > + */
> > + thres_sts = readl(pvt->regs + PVT_RAW_INTR_STAT);
> > +
> > + /*
> > + * Then lets recharge the PVT interface with the next sampling mode.
> > + * Lock the interface mutex to serialize trim, timeouts and alarm
> > + * thresholds settings.
> > + */
> > + cache = &pvt->cache[pvt->sensor];
> > + info = &pvt_info[pvt->sensor];
> > + pvt->sensor = (pvt->sensor == PVT_SENSOR_LAST) ?
> > + PVT_SENSOR_FIRST : (pvt->sensor + 1);
> > +
> > + /*
> > + * For some reason we have to mask the interrupt before changing the
> > + * mode, otherwise sometimes the temperature mode doesn't get
> > + * activated even though the actual mode in ctrl register correspond
> > + * to one. Then we read the data. By doing so we also recharge the data
> > + * conversion. After this the mode corresponding to the next
> > + * sensor in the row is set. Finally we enable the interrupts back.
> > + */
> > + mutex_lock(&pvt->iface_mtx);
> > +
> > + old = pvt_update(pvt->regs + PVT_INTR_MASK, PVT_INTR_DVALID,
> > + PVT_INTR_DVALID);
> > +
> > + val = readl(pvt->regs + PVT_DATA);
> > +
> > + pvt_set_mode(pvt, pvt_info[pvt->sensor].mode);
> > +
> > + pvt_update(pvt->regs + PVT_INTR_MASK, PVT_INTR_DVALID, old);
> > +
> > + mutex_unlock(&pvt->iface_mtx);
> > +
> > + /*
> > + * We can now update the data cache with data just retrieved from the
> > + * sensor. Lock write-seqlock to make sure the reader has a coherent
> > + * data.
> > + */
> > + write_seqlock(&cache->data_seqlock);
> > +
> > + cache->data = FIELD_GET(PVT_DATA_DATA_MASK, val);
> > +
> > + write_sequnlock(&cache->data_seqlock);
> > +
> > + /*
> > + * While PVT core is doing the next mode data conversion, we'll check
> > + * whether the alarms were triggered for the current sensor. Note that
> > + * according to the documentation only one threshold IRQ status can be
> > + * set at a time, that's why if-else statement is utilized.
> > + */
> > + if ((thres_sts & info->thres_sts_lo) ^ cache->thres_sts_lo) {
> > + WRITE_ONCE(cache->thres_sts_lo, thres_sts & info->thres_sts_lo);
> > + sysfs_notify(&pvt->hwmon->kobj, NULL, info->sysfs_min_alarm);
> > + kobject_uevent(&pvt->hwmon->kobj, KOBJ_CHANGE);
> > + if (cache->thres_sts_lo) {
> > + dev_warn(pvt->dev, "%s is below the lower limit\n",
> > + info->label);
> > + } else {
> > + dev_warn(pvt->dev, "%s is back to normal\n",
> > + info->label);
> > + }
> > + } else if ((thres_sts & info->thres_sts_hi) ^ cache->thres_sts_hi) {
> > + WRITE_ONCE(cache->thres_sts_hi, thres_sts & info->thres_sts_hi);
> > + sysfs_notify(&pvt->hwmon->kobj, NULL, info->sysfs_max_alarm);
> > + kobject_uevent(&pvt->hwmon->kobj, KOBJ_CHANGE);
> > + if (cache->thres_sts_hi) {
> > + dev_warn(pvt->dev, "%s is above the upper limit\n",
> > + info->label);
> > + } else {
> > + dev_warn(pvt->dev, "%s is back to normal\n",
> > + info->label);
> > + }
> > + }
>
> The point of userspace notification is to let userspace deal with it.
> I do not think it is a good idea to clog the kernel log with this
> information. It may sound like a good idea, but in a system prone to
> exceeding temperature limits all that is left in the log will be
> repeated temperature warnings. Let userspace handle it, and let
> userspace decide if it wants to log the events somewhere.
Agreed. I'll remove the warnings.
>
> Also, please have a look at https://patchwork.kernel.org/patch/11277347/
> and check if you can use it instead of creating notifications locally.
Seems reasonable. I'll apply that patch in my tree before the patches from this
series, then use the hwmon_notify_event() to notify all the required objects at
once instead of manually pushing the notification to each of them. After a smoke
tests I'll resend the series together with your patch being initial.
Note. I will have to fix the patch a bit, since:
1) It's been created a half year ago, so there are tiny merge conflicts with
kernel 5.7-rcX.
2) hwmon_notify_event() is a bit incorrect in the incrementing channel for the
hwmon_in. It's applicable for the sattr, but not for hwmon_thermal_notify().
If I find anything else, I'll add an info about it into the patch changelog.
>
> > +
> > + return IRQ_HANDLED;
> > +}
> > +
[nip]
> > +
> > +static int pvt_read_timeout(struct pvt_hwmon *pvt, long *val)
> > +{
> > + unsigned long rate;
> > + ktime_t kt;
> > + u32 data;
> > +
> > + rate = clk_get_rate(pvt->clks[PVT_CLOCK_REF].clk);
>
> Is this a dynamic clock ? If not, it would be better to read its frequency once
> in the probe function
No, It's not dynamic. I can do as you say, though in fact there is no much point
in it. The clock doesn't have the CLK_GET_RATE_NOCACHE flag set. Then calling
clk_get_rate() will have nearly the same performance as reading from the locally
defined reference clock frequency variable except a single lock/unlock preformed
and a few conditional statements. If I had CLK_GET_RATE_NOCACHE flag set, then
the clocks would have been dynamic and I would have had to use clk_get_rate()
here anyway. So caching the reference clock value in the PVT private data won't
change much, but will add a redundant abstraction.
So I have to ask back, are you really sure that moving the clk_get_rate() into the
probe will make the situation better? If you still think so and my reasoning didn't
change your mind, please say so explicitly.
>
> > + if (!rate)
> > + return -ENODEV;
>
> ... and return -ENODEV there if it is 0
Please, see my answer above.
>
> > +
> > + /*
> > + * Don't bother with mutex here, since we just read data from MMIO.
> > + * We also have to scale the ticks timeout up to compensate the
> > + * ms-ns-data translations.
> > + */
> > + data = readl(pvt->regs + PVT_TTIMEOUT) + 1;
> > +
> > + /*
> > + * Calculate ref-clock based delay (Ttotal) between two consecutive
> > + * data samples of the same sensor. So we first must calculate the
> > + * delay introduced by the internal ref-clock timer (Tref * Fclk).
> > + * Then add the constant timeout cuased by each conversion latency
> > + * (Tmin). The basic formulae for each conversion is following:
> > + * Ttotal = Tref * Fclk + Tmin
> > + * Note if alarms are enabled the sensors are polled one after
> > + * another, so in order to have the delay being applicable for each
> > + * sensor the requested value must be equally redistirbuted.
> > + */
> > +#if defined(CONFIG_SENSORS_BT1_PVT_ALARMS)
> > + kt = ktime_set(PVT_SENSORS_NUM * (u64)data, 0);
> > + kt = ktime_divns(kt, rate);
> > + kt = ktime_add_ns(kt, PVT_SENSORS_NUM * PVT_TOUT_MIN);
> > +#else
> > + kt = ktime_set(data, 0);
> > + kt = ktime_divns(kt, rate);
> > + kt = ktime_add_ns(kt, PVT_TOUT_MIN);
> > +#endif
> > +
> > + /* Return the result in msec as hwmon sysfs interface requires. */
> > + *val = ktime_to_ms(kt);
>
> Personally I am not sure about the value of using ktime for those
> calculations, but that is your call.
ktime provides a very convenient and self-explanatory interface. So instead of
dividing, adding, multiplying, sec<->msec factoring, avoiding overflows, etc,
I can do this by using a well-defined ktime interface.
> AFAICS ktime_to_ms() rounds down,
> so I assume you understand that this can return 0.
Yeah. There is no problem with it here. It's just a value returned to the
user-space. We are also ok with getting zero from user-space, since in fact
there is always 1/3000 sec conversion lag. So even if user-space asks to set
zero update interval, it will be 333 usec and we don't get the IRQs flood.
>
> > +
> > + return 0;
> > +}
> > +
> > +static int pvt_write_timeout(struct pvt_hwmon *pvt, long val)
> > +{
> > + unsigned long rate;
> > + ktime_t kt;
> > + u32 data;
> > + int ret;
> > +
> > + rate = clk_get_rate(pvt->clks[PVT_CLOCK_REF].clk);
> > + if (!rate)
> > + return -ENODEV;
> > +
> > + /*
> > + * If alarms are enabled, the requested timeout must be divided
> > + * between all available sensors to have the requested delay
> > + * applicable to each individual sensor.
> > + */
> > + kt = ms_to_ktime(val);
> > +#if defined(CONFIG_SENSORS_BT1_PVT_ALARMS)
> > + kt = ktime_divns(kt, PVT_SENSORS_NUM);
> > +#endif
> > +
> > + /*
> > + * Subtract a constant lag, which always persists due to the limited
> > + * PVT sampling rate. Make sure the timeout is non-zero.
> > + */
> > + kt = ktime_sub_ns(kt, PVT_TOUT_MIN);
> > + if (ktime_to_ns(kt) < 0)
> > + kt = ktime_set(0, 0);
>
> Doesn't that set the timeout to 0 ? That seems to conflict with the "non-zero"
> comment above. Should that be "not negative" ?
Right. It should have been "not negative". Thanks for noticing this.
>
> > +
> > + /*
> > + * Finally recalculate the timeout in terms of the reference clock
> > + * period.
> > + */
> > + data = ktime_divns(kt * rate, NSEC_PER_SEC);
> > +
> > + /*
> > + * Update the measurements delay, but lock the interface first, since
> > + * we have to disable PVT in order to have the new delay actually
> > + * updated.
> > + */
> > + ret = mutex_lock_interruptible(&pvt->iface_mtx);
> > + if (ret)
> > + return ret;
> > +
> > + pvt_set_tout(pvt, data);
> > +
> > + mutex_unlock(&pvt->iface_mtx);
> > +
> > + return 0;
> > +}
> > +
[nip]
> > +
> > +static void pvt_clear_data(void *data)
> > +{
> > + struct pvt_hwmon *pvt = data;
> > +#if !defined(CONFIG_SENSORS_BT1_PVT_ALARMS)
> > + int idx;
> > +
> > + for (idx = 0; idx < PVT_SENSORS_NUM; ++idx)
> > + complete_all(&pvt->cache[idx].conversion);
> > +#endif
> > +
> > + mutex_destroy(&pvt->iface_mtx);
> > + platform_set_drvdata(to_platform_device(pvt->dev), NULL);
>
> Unnecessary.
Ok.
>
> > +}
> > +
> > +static struct pvt_hwmon *pvt_create_data(struct platform_device *pdev)
> > +{
> > + struct device *dev = &pdev->dev;
> > + struct pvt_hwmon *pvt;
> > + int ret, idx;
> > +
> > + pvt = devm_kzalloc(dev, sizeof(*pvt), GFP_KERNEL);
> > + if (!pvt)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + ret = devm_add_action(dev, pvt_clear_data, pvt);
> > + if (ret) {
> > + dev_err(dev, "Can't add PVT data clear action\n");
> > + return ERR_PTR(ret);
> > + }
> > +
> > + pvt->dev = dev;
> > + pvt->sensor = PVT_SENSOR_FIRST;
> > + platform_set_drvdata(pdev, pvt);
>
> I don't immediately see where this is used. Can you point to it ?
Because there is no use of it. I'll remove this setting. Thanks.
>
> > + mutex_init(&pvt->iface_mtx);
> > +
> > +#if defined(CONFIG_SENSORS_BT1_PVT_ALARMS)
> > + for (idx = 0; idx < PVT_SENSORS_NUM; ++idx)
> > + seqlock_init(&pvt->cache[idx].data_seqlock);
> > +#else
> > + for (idx = 0; idx < PVT_SENSORS_NUM; ++idx)
> > + init_completion(&pvt->cache[idx].conversion);
> > +#endif
> > +
> > + return pvt;
> > +}
> > +
[nip]
> > +
> > +static int pvt_create_hwmon(struct pvt_hwmon *pvt)
> > +{
> > + pvt->hwmon = devm_hwmon_device_register_with_info(pvt->dev,
> > + dev_name(pvt->dev), pvt, &pvt_hwmon_info, pvt_hwmon_groups);
>
> Is the device name guaranteed to be a valid hwmon device name ?
Hm, According to the DT convention it may have dashes, which AFAICS isn't valid
hwmon device name. I'll replace dev_name() here with a string literal like
"pvt". Is it ok to have several hwmon devices with the same name?
>
> > + if (IS_ERR(pvt->hwmon)) {
> > + dev_err(pvt->dev, "Couldn't create hwmon device\n");
> > + return PTR_ERR(pvt->hwmon);
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +#if defined(CONFIG_SENSORS_BT1_PVT_ALARMS)
> > +
> > +static void pvt_disable_iface(void *data)
> > +{
> > + struct pvt_hwmon *pvt = data;
> > +
> > + mutex_lock(&pvt->iface_mtx);
> > + pvt_update(pvt->regs + PVT_CTRL, PVT_CTRL_EN, 0);
> > + pvt_update(pvt->regs + PVT_INTR_MASK, PVT_INTR_DVALID,
> > + PVT_INTR_DVALID);
> > + mutex_unlock(&pvt->iface_mtx);
> > +}
> > +
> > +static int pvt_enable_iface(struct pvt_hwmon *pvt)
> > +{
> > + int ret;
> > +
> > + ret = devm_add_action(pvt->dev, pvt_disable_iface, pvt);
> > + if (ret) {
> > + dev_err(pvt->dev, "Can't add PVT disable interface action\n");
> > + return ret;
> > + }
> > +
> > + /*
> > + * Enable sensors data conversion and IRQ. We need to lock the
> > + * interface mutex since hwmon has just been created and the
> > + * corresponding sysfs files are accessible from user-space,
> > + * which theoretically may cause races.
> > + */
>
> What if those userspace actions make a request which requires an interrupt ?
In case if CONFIG_SENSORS_BT1_PVT_ALARMS is enabled, then there is no such
action. Though user-space may set some alarm thresholds, which could cause
immediate hwmon event raise. But even if they did the event status will
be left pending and handled after we enable the IRQs in the next lines.
If CONFIG_SENSORS_BT1_PVT_ALARMS is disabled, then the IRQs are enabled/disabled
in the pvt_read_data() directly to perform the sensor conversion.
>
> > + mutex_lock(&pvt->iface_mtx);
> > + pvt_update(pvt->regs + PVT_INTR_MASK, PVT_INTR_DVALID, 0);
> > + pvt_update(pvt->regs + PVT_CTRL, PVT_CTRL_EN, PVT_CTRL_EN);
> > + mutex_unlock(&pvt->iface_mtx);
> > +
> > + return 0;
> > +}
> > +
> > +#else /* !CONFIG_SENSORS_BT1_PVT_ALARMS */
> > +
[nip]
> > diff --git a/drivers/hwmon/bt1-pvt.h b/drivers/hwmon/bt1-pvt.h
> > new file mode 100644
> > index 000000000000..7851ed947877
> > --- /dev/null
> > +++ b/drivers/hwmon/bt1-pvt.h
> > @@ -0,0 +1,237 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * Copyright (C) 2020 BAIKAL ELECTRONICS, JSC
> > + *
> > + * Baikal-T1 Process, Voltage, Temperature sensor driver
> > + */
> > +#ifndef __HWMON_BT1_PVT_H__
> > +#define __HWMON_BT1_PVT_H__
> > +
> > +#include <linux/kernel.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/io.h>
> > +#include <linux/mutex.h>
> > +#include <linux/seqlock.h>
> > +#include <linux/completion.h>
> > +#include <linux/sysfs.h>
> > +#include <linux/hwmon.h>
>
> Alphabetic order, please, and drop unnecessary includes.
Ok.
>
> > +
[nip]
> > +
> > +/*
> > + * struct pvt_sensor_info - Baikal-T1 PVT sensor informational structure
> > + * @label: hwmon sensor label.
> > + * @mode: PVT mode corresponding to the channel.
> > + * @thres_base: upper and lower threshold values of the sensor.
> > + * @thres_sts_lo: low threshold status bitfield.
> > + * @thres_sts_hi: high threshold status bitfield.
> > + * @sysfs_min_alarm: pointer to a min-alarm sysfs filename.
> > + * @sysfs_min_alarm: pointer to a max-alarm sysfs filename.
> > + */
> > +struct pvt_sensor_info {
> > + const char *label;
> > + u32 mode;
> > + unsigned long thres_base;
> > + u32 thres_sts_lo;
> > + u32 thres_sts_hi;
> > + const char *sysfs_min_alarm;
> > + const char *sysfs_max_alarm;
> > +};
> > +
> > +#define PVT_SENSOR_INFO(_label, _file, _mode, _thres) \
>
> _file seems to be a global define or variable, or at least checkpatch
> complains about its use.
Hm, mine didn't complain about it. Otherwise I would have fixed it in the first
place. Though I'll remove that name from here anyway, since the
hwmon_notify_event() notifier is utilized, which requires sensor type, attribute
and channel/index. Thus explicit sysfs_min_alarm and sysfs_max_alarm names
aren't needed anymore.
-Sergey
>
> > + { \
> > + .label = _label, \
> > + .mode = PVT_CTRL_MODE_ ##_mode, \
> > + .thres_base = PVT_ ##_thres, \
> > + .thres_sts_lo = PVT_INTR_ ##_thres## _LO, \
> > + .thres_sts_hi = PVT_INTR_ ##_thres## _HI, \
> > + .sysfs_min_alarm = _file "_min_alarm", \
> > + .sysfs_max_alarm = _file "_max_alarm" \
> > + }
> > +
[nip]
> > +
> > +#endif /* __HWMON_BT1_PVT_H__ */
^ permalink raw reply
* Re: [PATCH 3/3] hwrng: ba431-rng: add support for BA431 hwrng
From: Arnd Bergmann @ 2020-05-25 20:28 UTC (permalink / raw)
To: Olivier Sobrie
Cc: Matt Mackall, Herbert Xu, Rob Herring, Greg Kroah-Hartman,
open list:HARDWARE RANDOM NUMBER GENERATOR CORE, DTML,
linux-kernel@vger.kernel.org, Waleed Ziad, sebastien.rabou
In-Reply-To: <20200525195606.2941649-4-olivier.sobrie@silexinsight.com>
On Mon, May 25, 2020 at 10:07 PM Olivier Sobrie
<olivier.sobrie@silexinsight.com> wrote:
>
> Silex insight BA431 is an IP designed to generate random numbers that
> can be integrated in various FPGA.
> This driver adds support for it through the hwrng interface.
>
> This driver is used in Silex Insight Viper OEM boards.
>
> Signed-off-by: Olivier Sobrie <olivier.sobrie@silexinsight.com>
> Signed-off-by: Waleed Ziad <waleed94ziad@gmail.com>
The driver looks good to me.
Acked-by: Arnd Bergmann <arnd@arndb.de>
> drivers/char/hw_random/Kconfig | 10 ++
> drivers/char/hw_random/Makefile | 1 +
> drivers/char/hw_random/ba431-rng.c | 240 +++++++++++++++++++++++++++++
I wonder if we should move drivers/char/hw_random to its own top-level drivers
subsystem outside of drivers/char. It seems to be growing steadily and is larger
than a lot of other subsystems with currently 34 drivers in there.
Not your problem though.
> + /* Wait until the state changed */
> + for (i = 0; i < BA431_RESET_READ_STATUS_RETRIES; ++i) {
> + state = ba431_trng_get_state(ba431);
> + if (state >= BA431_STATE_STARTUP)
> + break;
> +
> + udelay(BA431_RESET_READ_STATUS_INTERVAL);
> + }
Looking for something to improve, I noticed that this loop can take over
a millisecond to time out, and it always runs in non-atomic context.
It may be better to use usleep_range() than udelay().
Arnd
^ permalink raw reply
* Re: [PATCH v3 1/8] dt-bindings: net: meson-dwmac: Add the amlogic,rx-delay-ns property
From: Pavel Machek @ 2020-05-25 20:17 UTC (permalink / raw)
To: Andrew Lunn
Cc: Florian Fainelli, Martin Blumenstingl, robh+dt, linux-amlogic,
devicetree, jianxin.pan, davem, netdev, linux-kernel,
linux-arm-kernel
In-Reply-To: <20200525135728.GE752669@lunn.ch>
[-- Attachment #1: Type: text/plain, Size: 858 bytes --]
On Mon 2020-05-25 15:57:28, Andrew Lunn wrote:
> > > standardizing on rx-delay-ps and tx-delay-ps would make sense since that
> > > is the lowest resolution and the property would be correctly named with
> > > an unit in the name.
> >
> > Seems like similar patch is already being reviewed from Dan Murphy (?)
> > from TI.
>
> Dan is working on the PHY side. But there is probably code which can
> be shared.
>
> One question to consider, do we want the same properties names for MAC
> and PHY, or do we want to make them different, to avoid confusion?
We have same properties accross different hardware (compatible, reg),
so same property between MAC and PHY seems to make sense.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply
* [PATCH 3/3] hwrng: ba431-rng: add support for BA431 hwrng
From: Olivier Sobrie @ 2020-05-25 19:56 UTC (permalink / raw)
To: Matt Mackall, Herbert Xu, Rob Herring, Arnd Bergmann,
Greg Kroah-Hartman, linux-crypto, devicetree, linux-kernel
Cc: Olivier Sobrie, Waleed Ziad, sebastien.rabou
In-Reply-To: <20200525195606.2941649-1-olivier.sobrie@silexinsight.com>
Silex insight BA431 is an IP designed to generate random numbers that
can be integrated in various FPGA.
This driver adds support for it through the hwrng interface.
This driver is used in Silex Insight Viper OEM boards.
Signed-off-by: Olivier Sobrie <olivier.sobrie@silexinsight.com>
Signed-off-by: Waleed Ziad <waleed94ziad@gmail.com>
---
drivers/char/hw_random/Kconfig | 10 ++
drivers/char/hw_random/Makefile | 1 +
drivers/char/hw_random/ba431-rng.c | 240 +++++++++++++++++++++++++++++
3 files changed, 251 insertions(+)
create mode 100644 drivers/char/hw_random/ba431-rng.c
diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
index 9bc46da8d77a8..ef8928fc9999d 100644
--- a/drivers/char/hw_random/Kconfig
+++ b/drivers/char/hw_random/Kconfig
@@ -74,6 +74,16 @@ config HW_RANDOM_ATMEL
If unsure, say Y.
+config HW_RANDOM_BA431
+ tristate "Silex Insight BA431 Random Number Generator support"
+ default HW_RANDOM
+ help
+ This driver provides kernel-side support for the Random Number
+ Generator hardware based on Silex Insight BA431 IP.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ba431-rng
+
config HW_RANDOM_BCM2835
tristate "Broadcom BCM2835/BCM63xx Random Number Generator support"
depends on ARCH_BCM2835 || ARCH_BCM_NSP || ARCH_BCM_5301X || \
diff --git a/drivers/char/hw_random/Makefile b/drivers/char/hw_random/Makefile
index a7801b49ce6c0..02ccadafcca99 100644
--- a/drivers/char/hw_random/Makefile
+++ b/drivers/char/hw_random/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_HW_RANDOM_TIMERIOMEM) += timeriomem-rng.o
obj-$(CONFIG_HW_RANDOM_INTEL) += intel-rng.o
obj-$(CONFIG_HW_RANDOM_AMD) += amd-rng.o
obj-$(CONFIG_HW_RANDOM_ATMEL) += atmel-rng.o
+obj-$(CONFIG_HW_RANDOM_BA431) += ba431-rng.o
obj-$(CONFIG_HW_RANDOM_GEODE) += geode-rng.o
obj-$(CONFIG_HW_RANDOM_N2RNG) += n2-rng.o
n2-rng-y := n2-drv.o n2-asm.o
diff --git a/drivers/char/hw_random/ba431-rng.c b/drivers/char/hw_random/ba431-rng.c
new file mode 100644
index 0000000000000..d3bec75d405e4
--- /dev/null
+++ b/drivers/char/hw_random/ba431-rng.c
@@ -0,0 +1,240 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2018 Silex Insight sa.
+
+#include <linux/delay.h>
+#include <linux/hw_random.h>
+#include <linux/io.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/workqueue.h>
+
+#define BA431_RESET_DELAY 1 /* usec */
+#define BA431_RESET_READ_STATUS_RETRIES 100
+#define BA431_RESET_READ_STATUS_INTERVAL 10 /* usec */
+#define BA431_READ_RETRY_INTERVAL 1 /* usec */
+
+#define BA431_REG_CTRL 0x00
+#define BA431_REG_FIFO_LEVEL 0x04
+#define BA431_REG_STATUS 0x30
+#define BA431_REG_FIFODATA 0x80
+
+#define BA431_CTRL_ENABLE BIT(0)
+#define BA431_CTRL_SOFTRESET BIT(8)
+
+#define BA431_STATUS_STATE_MASK (BIT(1) | BIT(2) | BIT(3))
+#define BA431_STATUS_STATE_OFFSET 1
+
+enum ba431_state {
+ BA431_STATE_RESET,
+ BA431_STATE_STARTUP,
+ BA431_STATE_FIFOFULLON,
+ BA431_STATE_FIFOFULLOFF,
+ BA431_STATE_RUNNING,
+ BA431_STATE_ERROR
+};
+
+struct ba431_trng {
+ struct device *dev;
+ void __iomem *base;
+ struct hwrng rng;
+ atomic_t reset_pending;
+ struct work_struct reset_work;
+};
+
+static inline u32 ba431_trng_read_reg(struct ba431_trng *ba431, u32 reg)
+{
+ return ioread32(ba431->base + reg);
+}
+
+static inline void ba431_trng_write_reg(struct ba431_trng *ba431, u32 reg,
+ u32 val)
+{
+ iowrite32(val, ba431->base + reg);
+}
+
+static inline enum ba431_state ba431_trng_get_state(struct ba431_trng *ba431)
+{
+ u32 status = ba431_trng_read_reg(ba431, BA431_REG_STATUS);
+
+ return (status & BA431_STATUS_STATE_MASK) >> BA431_STATUS_STATE_OFFSET;
+}
+
+static int ba431_trng_is_in_error(struct ba431_trng *ba431)
+{
+ enum ba431_state state = ba431_trng_get_state(ba431);
+
+ if ((state < BA431_STATE_STARTUP) ||
+ (state >= BA431_STATE_ERROR))
+ return 1;
+
+ return 0;
+}
+
+static int ba431_trng_reset(struct ba431_trng *ba431)
+{
+ unsigned int i;
+ enum ba431_state state;
+
+ /* Disable interrupts, random generation and enable the softreset */
+ ba431_trng_write_reg(ba431, BA431_REG_CTRL, BA431_CTRL_SOFTRESET);
+ udelay(BA431_RESET_DELAY);
+ ba431_trng_write_reg(ba431, BA431_REG_CTRL, BA431_CTRL_ENABLE);
+
+ /* Wait until the state changed */
+ for (i = 0; i < BA431_RESET_READ_STATUS_RETRIES; ++i) {
+ state = ba431_trng_get_state(ba431);
+ if (state >= BA431_STATE_STARTUP)
+ break;
+
+ udelay(BA431_RESET_READ_STATUS_INTERVAL);
+ }
+
+ if ((state < BA431_STATE_STARTUP) || (state >= BA431_STATE_ERROR)) {
+ dev_err(ba431->dev, "reset failed (state: %d)\n",
+ state);
+ return -1;
+ }
+
+ dev_info(ba431->dev, "reset done\n");
+
+ return 0;
+}
+
+static void ba431_trng_reset_work(struct work_struct *work)
+{
+ struct ba431_trng *ba431 = container_of(work, struct ba431_trng,
+ reset_work);
+ ba431_trng_reset(ba431);
+ atomic_set(&ba431->reset_pending, 0);
+}
+
+static void ba431_trng_schedule_reset(struct ba431_trng *ba431)
+{
+ if (atomic_cmpxchg(&ba431->reset_pending, 0, 1))
+ return;
+
+ schedule_work(&ba431->reset_work);
+}
+
+static int ba431_trng_read(struct hwrng *rng, void *buf, size_t max, bool wait)
+{
+ struct ba431_trng *ba431 = container_of(rng, struct ba431_trng, rng);
+ u32 *data = buf;
+ unsigned int level, i;
+ int n = 0;
+
+ while (max > 0) {
+ level = ba431_trng_read_reg(ba431, BA431_REG_FIFO_LEVEL);
+ if (!level) {
+ if (ba431_trng_is_in_error(ba431)) {
+ ba431_trng_schedule_reset(ba431);
+ break;
+ }
+
+ if (!wait)
+ break;
+
+ udelay(BA431_READ_RETRY_INTERVAL);
+ continue;
+ }
+
+ i = level;
+ do {
+ data[n++] = ba431_trng_read_reg(ba431,
+ BA431_REG_FIFODATA);
+ max -= sizeof(*data);
+ } while (--i && (max > 0));
+
+ if (ba431_trng_is_in_error(ba431)) {
+ n -= (level - i);
+ ba431_trng_schedule_reset(ba431);
+ break;
+ }
+ }
+
+ n *= sizeof(data);
+ return (n || !wait) ? n : -EIO;
+}
+
+static void ba431_trng_cleanup(struct hwrng *rng)
+{
+ struct ba431_trng *ba431 = container_of(rng, struct ba431_trng, rng);
+
+ ba431_trng_write_reg(ba431, BA431_REG_CTRL, 0);
+ cancel_work_sync(&ba431->reset_work);
+}
+
+static int ba431_trng_init(struct hwrng *rng)
+{
+ struct ba431_trng *ba431 = container_of(rng, struct ba431_trng, rng);
+
+ return ba431_trng_reset(ba431);
+}
+
+static int ba431_trng_probe(struct platform_device *pdev)
+{
+ struct ba431_trng *ba431;
+ struct resource *res;
+ int ret;
+
+ ba431 = devm_kzalloc(&pdev->dev, sizeof(*ba431), GFP_KERNEL);
+ if (!ba431)
+ return -ENOMEM;
+
+ ba431->dev = &pdev->dev;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ ba431->base = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(ba431->base))
+ return PTR_ERR(ba431->base);
+
+ atomic_set(&ba431->reset_pending, 0);
+ INIT_WORK(&ba431->reset_work, ba431_trng_reset_work);
+ ba431->rng.name = pdev->name;
+ ba431->rng.init = ba431_trng_init;
+ ba431->rng.cleanup = ba431_trng_cleanup;
+ ba431->rng.read = ba431_trng_read;
+
+ platform_set_drvdata(pdev, ba431);
+
+ ret = hwrng_register(&ba431->rng);
+ if (ret) {
+ dev_err(&pdev->dev, "BA431 registration failed (%d)\n", ret);
+ return ret;
+ }
+
+ dev_info(&pdev->dev, "BA431 TRNG registered\n");
+
+ return 0;
+}
+
+static int ba431_trng_remove(struct platform_device *pdev)
+{
+ struct ba431_trng *ba431 = platform_get_drvdata(pdev);
+
+ hwrng_unregister(&ba431->rng);
+
+ return 0;
+}
+
+static const struct of_device_id ba431_trng_dt_ids[] = {
+ { .compatible = "silex-insight,ba431-rng", .data = NULL },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, ba431_trng_dt_ids);
+
+static struct platform_driver ba431_trng_driver = {
+ .driver = {
+ .name = "ba431-rng",
+ .of_match_table = ba431_trng_dt_ids,
+ },
+ .probe = ba431_trng_probe,
+ .remove = ba431_trng_remove,
+};
+
+module_platform_driver(ba431_trng_driver);
+
+MODULE_AUTHOR("Olivier Sobrie <olivier@sobrie.be>");
+MODULE_DESCRIPTION("TRNG driver for Silex Insight BA431");
+MODULE_LICENSE("GPL");
--
2.26.2
^ permalink raw reply related
* [PATCH 2/3] dt-bindings: rng: document Silex Insight BA431 hwrng
From: Olivier Sobrie @ 2020-05-25 19:56 UTC (permalink / raw)
To: Matt Mackall, Herbert Xu, Rob Herring, Arnd Bergmann,
Greg Kroah-Hartman, linux-crypto, devicetree, linux-kernel
Cc: Olivier Sobrie, Waleed Ziad, sebastien.rabou
In-Reply-To: <20200525195606.2941649-1-olivier.sobrie@silexinsight.com>
This patch documents the device tree bindings of the BA431 hardware
random number generator.
This IP is for instance present in the Viper OEM boards sold by Silex
Insight.
Signed-off-by: Olivier Sobrie <olivier.sobrie@silexinsight.com>
---
.../bindings/rng/silex-insight,ba431-rng.yaml | 36 +++++++++++++++++++
1 file changed, 36 insertions(+)
create mode 100644 Documentation/devicetree/bindings/rng/silex-insight,ba431-rng.yaml
diff --git a/Documentation/devicetree/bindings/rng/silex-insight,ba431-rng.yaml b/Documentation/devicetree/bindings/rng/silex-insight,ba431-rng.yaml
new file mode 100644
index 0000000000000..48ab82abf50ec
--- /dev/null
+++ b/Documentation/devicetree/bindings/rng/silex-insight,ba431-rng.yaml
@@ -0,0 +1,36 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/rng/silex-insight,ba431-rng.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Silex Insight BA431 RNG bindings
+
+description: |
+ The BA431 hardware random number generator is an IP that is FIPS-140-2/3
+ certified.
+
+maintainers:
+ - Olivier Sobrie <olivier.sobrie@silexinsight.com>
+
+properties:
+ compatible:
+ const: silex-insight,ba431-rng
+
+ reg:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+
+additionalProperties: false
+
+examples:
+ - |
+ rng@42800000 {
+ compatible = "silex-insight,ba431-rng";
+ reg = <0x42800000 0x1000>;
+ };
+
+...
--
2.26.2
^ permalink raw reply related
* [PATCH 1/3] dt-bindings: vendor-prefixes: Add Silex Insight vendor prefix
From: Olivier Sobrie @ 2020-05-25 19:56 UTC (permalink / raw)
To: Matt Mackall, Herbert Xu, Rob Herring, Arnd Bergmann,
Greg Kroah-Hartman, linux-crypto, devicetree, linux-kernel
Cc: Olivier Sobrie, Waleed Ziad, sebastien.rabou
In-Reply-To: <20200525195606.2941649-1-olivier.sobrie@silexinsight.com>
Silex Insight is a microelectronic company whose headquarter is located
in Belgium.
Web site of the company: https://www.silexinsight.com/
Signed-off-by: Olivier Sobrie <olivier.sobrie@silexinsight.com>
---
Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index d3891386d6710..45e555774be42 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -918,6 +918,8 @@ patternProperties:
description: Silead Inc.
"^silergy,.*":
description: Silergy Corp.
+ "^silex-insight,.*":
+ description: Silex Insight
"^siliconmitus,.*":
description: Silicon Mitus, Inc.
"^simtek,.*":
--
2.26.2
^ permalink raw reply related
* [PATCH 0/3] hwrng: add support for Silex Insight BA431
From: Olivier Sobrie @ 2020-05-25 19:56 UTC (permalink / raw)
To: Matt Mackall, Herbert Xu, Rob Herring, Arnd Bergmann,
Greg Kroah-Hartman, linux-crypto, devicetree, linux-kernel
Cc: Olivier Sobrie, Waleed Ziad, sebastien.rabou
Hello all,
This set of patches aims at introducing a linux hwrng driver for the
Silex Insight BA431 IP which is available for various FPGA.
This hardware is for instance present in Silex Insight Viper OEM boards.
The two first patches are documenting the device tree bindings.
The last one contains the BA431 hwrng driver.
Olivier Sobrie (3):
dt-bindings: vendor-prefixes: Add Silex Insight vendor prefix
dt-bindings: rng: document Silex Insight BA431 hwrng
hwrng: ba431-rng: add support for BA431 hwrng
.../bindings/rng/silex-insight,ba431-rng.yaml | 33 +++
.../devicetree/bindings/vendor-prefixes.yaml | 2 +
drivers/char/hw_random/Kconfig | 10 +
drivers/char/hw_random/Makefile | 1 +
drivers/char/hw_random/ba431-rng.c | 241 ++++++++++++++++++
5 files changed, 287 insertions(+)
create mode 100644 Documentation/devicetree/bindings/rng/silex-insight,ba431-rng.yaml
create mode 100644 drivers/char/hw_random/ba431-rng.c
--
2.26.2
^ permalink raw reply
* [PATCH 3/3] MIPS: ingenic: Default to a generic board
From: Paul Cercueil @ 2020-05-25 17:41 UTC (permalink / raw)
To: Thomas Bogendoerfer
Cc: od, devicetree, linux-mips, linux-kernel, Paul Cercueil
In-Reply-To: <20200525174155.457491-1-paul@crapouillou.net>
Having a generic board option makes it possible to create a kernel that
will run on various Ingenic SoCs, as long as the right devicetree is
provided.
Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
arch/mips/jz4740/Kconfig | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/arch/mips/jz4740/Kconfig b/arch/mips/jz4740/Kconfig
index 412d2faa3cdf..9c2e8c15bb97 100644
--- a/arch/mips/jz4740/Kconfig
+++ b/arch/mips/jz4740/Kconfig
@@ -2,7 +2,14 @@
choice
prompt "Machine type"
depends on MACH_INGENIC
- default JZ4740_QI_LB60
+ default INGENIC_GENERIC_BOARD
+
+config INGENIC_GENERIC_BOARD
+ bool "Generic board"
+ select MACH_JZ4740
+ select MACH_JZ4770
+ select MACH_JZ4780
+ select MACH_X1000
config JZ4740_QI_LB60
bool "Qi Hardware Ben NanoNote"
--
2.26.2
^ permalink raw reply related
* [PATCH 2/3] MIPS: ingenic: Add support for GCW Zero prototype
From: Paul Cercueil @ 2020-05-25 17:41 UTC (permalink / raw)
To: Thomas Bogendoerfer
Cc: od, devicetree, linux-mips, linux-kernel, Paul Cercueil
In-Reply-To: <20200525174155.457491-1-paul@crapouillou.net>
Add support for the GCW Zero prototype. The only (?) difference is that
it only has 256 MiB of RAM, compared to the 512 MiB of RAM of the retail
device.
Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
arch/mips/boot/dts/ingenic/gcw0_proto.dts | 13 +++++++++++++
1 file changed, 13 insertions(+)
create mode 100644 arch/mips/boot/dts/ingenic/gcw0_proto.dts
diff --git a/arch/mips/boot/dts/ingenic/gcw0_proto.dts b/arch/mips/boot/dts/ingenic/gcw0_proto.dts
new file mode 100644
index 000000000000..02df22f8ae0f
--- /dev/null
+++ b/arch/mips/boot/dts/ingenic/gcw0_proto.dts
@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+
+#include "gcw0.dts"
+
+/ {
+ model = "GCW Zero Prototype";
+};
+
+&memory {
+ /* Prototype has only 256 MiB of RAM */
+ reg = <0x0 0x10000000>;
+};
--
2.26.2
^ permalink raw reply related
* [PATCH 1/3] MIPS: ingenic: DTS: Add memory info of GCW Zero
From: Paul Cercueil @ 2020-05-25 17:41 UTC (permalink / raw)
To: Thomas Bogendoerfer
Cc: od, devicetree, linux-mips, linux-kernel, Paul Cercueil
Add memory info of the GCW Zero in its devicetree. The bootloader
generally provides this information, but since it is fixed to 512 MiB,
it doesn't hurt to have it in devicetree. It allows the kernel to boot
without any parameter passed as argument.
Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
arch/mips/boot/dts/ingenic/gcw0.dts | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/arch/mips/boot/dts/ingenic/gcw0.dts b/arch/mips/boot/dts/ingenic/gcw0.dts
index c2396b5ca175..8d22828787d8 100644
--- a/arch/mips/boot/dts/ingenic/gcw0.dts
+++ b/arch/mips/boot/dts/ingenic/gcw0.dts
@@ -19,6 +19,12 @@ aliases {
serial3 = &uart3;
};
+ memory: memory {
+ device_type = "memory";
+ reg = <0x0 0x10000000>,
+ <0x30000000 0x10000000>;
+ };
+
chosen {
stdout-path = "serial2:57600n8";
};
--
2.26.2
^ permalink raw reply related
* Re: [PATCH v3 03/16] mfd: mfd-core: match device tree node against reg property
From: Michael Walle @ 2020-05-25 17:36 UTC (permalink / raw)
To: Lee Jones
Cc: Andy Shevchenko, Linus Walleij, Bartosz Golaszewski, Rob Herring,
Jean Delvare, Guenter Roeck, Thierry Reding,
Uwe Kleine-König, Wim Van Sebroeck, Shawn Guo, Li Yang,
Thomas Gleixner, Jason Cooper, Marc Zyngier, Mark Brown,
Greg Kroah-Hartman, linux-gpio, devicetree, linux-kernel,
linux-hwmon, linux-pwm, linux-watchdog, linux-arm-kernel
In-Reply-To: <20200515102848.GH271301@dell>
Am 2020-05-15 12:28, schrieb Lee Jones:
> On Thu, 30 Apr 2020, Michael Walle wrote:
>
>> Hi Lee,
>>
>> Am 2020-04-23 19:45, schrieb Michael Walle:
>> > There might be multiple children with the device tree compatible, for
>> > example if a MFD has multiple instances of the same function. In this
>> > case only the first is matched and the other children get a wrong
>> > of_node reference.
>> > Add a new option to match also against the unit address of the child
>> > node. Additonally, a new helper OF_MFD_CELL_REG is added.
>>
>>
>> Do you think this is feasible? I guess this is the biggest uncertainty
>> for me at the moment in this patch series.
>
> I think it sounds fine in principle. So long as it doesn't change the
> existing behaviour when of_reg isn't set.
>
>> > Signed-off-by: Michael Walle <michael@walle.cc>
>> > ---
>> > drivers/mfd/mfd-core.c | 29 ++++++++++++++++++++---------
>> > include/linux/mfd/core.h | 26 ++++++++++++++++++++------
>> > 2 files changed, 40 insertions(+), 15 deletions(-)
>> >
>> > diff --git a/drivers/mfd/mfd-core.c b/drivers/mfd/mfd-core.c
>> > index e735565969b3..4ecb376338f7 100644
>> > --- a/drivers/mfd/mfd-core.c
>> > +++ b/drivers/mfd/mfd-core.c
>> > @@ -117,6 +117,7 @@ static int mfd_add_device(struct device *parent, int
>> > id,
>> > struct device_node *np = NULL;
>> > int ret = -ENOMEM;
>> > int platform_id;
>> > + u32 of_reg;
>> > int r;
>> >
>> > if (id == PLATFORM_DEVID_AUTO)
>> > @@ -151,16 +152,26 @@ static int mfd_add_device(struct device *parent,
>> > int id,
>> >
>> > if (parent->of_node && cell->of_compatible) {
>> > for_each_child_of_node(parent->of_node, np) {
>> > - if (of_device_is_compatible(np, cell->of_compatible)) {
>> > - if (!of_device_is_available(np)) {
>> > - /* Ignore disabled devices error free */
>> > - ret = 0;
>> > - goto fail_alias;
>> > - }
>> > - pdev->dev.of_node = np;
>> > - pdev->dev.fwnode = &np->fwnode;
>> > - break;
>> > + if (!of_device_is_compatible(np, cell->of_compatible))
>> > + continue;
>> > +
>> > + /* also match the unit address if set */
>
> Please use correct grammar in comments (leaving off the full-stop).
>
>> > + if (cell->of_reg & MFD_OF_REG_VALID) {
>> > + if (of_property_read_u32(np, "reg", &of_reg))
>> > + continue;
>> > + if ((cell->of_reg & MFD_OF_REG_MASK) != of_reg)
>> > + continue;
>> > }
>> > +
>> > + if (!of_device_is_available(np)) {
>> > + /* Ignore disabled devices error free */
>> > + ret = 0;
>> > + goto fail_alias;
>> > + }
>> > +
>> > + pdev->dev.of_node = np;
>> > + pdev->dev.fwnode = &np->fwnode;
>> > + break;
>> > }
>> > }
>> >
>> > diff --git a/include/linux/mfd/core.h b/include/linux/mfd/core.h
>> > index d01d1299e49d..c2c0ad6b14f3 100644
>> > --- a/include/linux/mfd/core.h
>> > +++ b/include/linux/mfd/core.h
>> > @@ -13,8 +13,11 @@
>> > #include <linux/platform_device.h>
>> >
>> > #define MFD_RES_SIZE(arr) (sizeof(arr) / sizeof(struct resource))
>> > +#define MFD_OF_REG_VALID BIT(31)
>
> What about 64bit platforms?
The idea was to have this as a logical number. I.e. for now you may only
have one subdevice per unique compatible string. In fact, if you have a
look at the ab8500.c, there are multiple "stericsson,ab8500-pwm"
subdevices. But there is only one DT node for all three of it. I guess
this works as long as you don't use phandles to reference the pwm node
in the device tree. Or you don't want to use device tree properties
per subdevice (for example the "timeout-sec" of a watchdog device).
So to circumvent this, I thought of having the unit-address (and thus
the "reg" property) to differentiate between multiple subdevices. Now
there is one special case for me: this board management controller
might be upgradable and it might change internally. Thus I came up
with that logical numbering of subdevices. Rob doesn't seem to be a
fan of that, though. Therefore, having bit 31 as a valid indicator
leaves you with 2^31 logical devices, which should be enough ;)
Rob proposed to have the internal offset as the unit-address. But
in that case I can also use devm_of_platform_populate() and don't
need the OF_MFD_CELL_REG; I'd just parse the reg offset in each
individual subdevice driver. But like I said, I wanted to keep the
internal offsets out of the device tree.
-michael
>
>> > +#define MFD_OF_REG_MASK GENMASK(30, 0)
>> >
>> > -#define MFD_CELL_ALL(_name, _res, _pdata, _pdsize, _id, _compat,
>> > _match)\
>> > +#define MFD_CELL_ALL(_name, _res, _pdata, _pdsize, _id, _compat, \
>> > + _of_reg, _match) \
>> > { \
>> > .name = (_name), \
>> > .resources = (_res), \
>> > @@ -22,24 +25,32 @@
>> > .platform_data = (_pdata), \
>> > .pdata_size = (_pdsize), \
>> > .of_compatible = (_compat), \
>> > + .of_reg = (_of_reg), \
>> > .acpi_match = (_match), \
>> > .id = (_id), \
>> > }
>> >
>> > +#define OF_MFD_CELL_REG(_name, _res, _pdata, _pdsize, _id, _compat, \
>> > + _of_reg) \
>> > + MFD_CELL_ALL(_name, _res, _pdata, _pdsize, _id, _compat, \
>> > + ((_of_reg) | MFD_OF_REG_VALID), NULL) \
>> > +
>> > #define OF_MFD_CELL(_name, _res, _pdata, _pdsize,_id, _compat) \
>> > - MFD_CELL_ALL(_name, _res, _pdata, _pdsize, _id, _compat, NULL) \
>> > + MFD_CELL_ALL(_name, _res, _pdata, _pdsize, _id, _compat, \
>> > + 0, NULL) \
>> >
>> > #define ACPI_MFD_CELL(_name, _res, _pdata, _pdsize, _id, _match) \
>> > - MFD_CELL_ALL(_name, _res, _pdata, _pdsize, _id, NULL, _match) \
>> > + MFD_CELL_ALL(_name, _res, _pdata, _pdsize, _id, NULL, 0, \
>> > + _match) \
>> >
>> > #define MFD_CELL_BASIC(_name, _res, _pdata, _pdsize, _id) \
>> > - MFD_CELL_ALL(_name, _res, _pdata, _pdsize, _id, NULL, NULL) \
>> > + MFD_CELL_ALL(_name, _res, _pdata, _pdsize, _id, NULL, 0, NULL) \
>> >
>> > #define MFD_CELL_RES(_name, _res) \
>> > - MFD_CELL_ALL(_name, _res, NULL, 0, 0, NULL, NULL) \
>> > + MFD_CELL_ALL(_name, _res, NULL, 0, 0, NULL, 0, NULL) \
>> >
>> > #define MFD_CELL_NAME(_name) \
>> > - MFD_CELL_ALL(_name, NULL, NULL, 0, 0, NULL, NULL) \
>> > + MFD_CELL_ALL(_name, NULL, NULL, 0, 0, NULL, 0, NULL) \
>> >
>> > struct irq_domain;
>> > struct property_entry;
>> > @@ -78,6 +89,9 @@ struct mfd_cell {
>> > */
>> > const char *of_compatible;
>> >
>> > + /* matching the reg property if set */
>
> Proper grammar please.
>
> "OF unit address for device matching"
>
>> > + unsigned int of_reg;
>> > +
>> > /* Matches ACPI */
>> > const struct mfd_cell_acpi_match *acpi_match;
^ permalink raw reply
* [PATCH v4 5/5] iio: imu: bmi160: added mount-matrix support
From: Jonathan Albrieux @ 2020-05-25 16:46 UTC (permalink / raw)
To: linux-kernel
Cc: ~postmarketos/upstreaming, daniel.baluta, Jonathan Albrieux,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Hartmut Knaack, Lars-Peter Clausen,
open list:IIO SUBSYSTEM AND DRIVERS, Peter Meerwald-Stadler,
Jonathan Cameron
In-Reply-To: <20200525164615.14962-1-jonathan.albrieux@gmail.com>
Add mount-matrix binding support. As chip could have different orientations
a mount matrix support is needed to correctly translate these differences.
Signed-off-by: Jonathan Albrieux <jonathan.albrieux@gmail.com>
---
drivers/iio/imu/bmi160/bmi160.h | 1 +
drivers/iio/imu/bmi160/bmi160_core.c | 20 ++++++++++++++++++++
2 files changed, 21 insertions(+)
diff --git a/drivers/iio/imu/bmi160/bmi160.h b/drivers/iio/imu/bmi160/bmi160.h
index 923c3b274fde..a82e040bd109 100644
--- a/drivers/iio/imu/bmi160/bmi160.h
+++ b/drivers/iio/imu/bmi160/bmi160.h
@@ -9,6 +9,7 @@ struct bmi160_data {
struct regmap *regmap;
struct iio_trigger *trig;
struct regulator_bulk_data supplies[2];
+ struct iio_mount_matrix orientation;
};
extern const struct regmap_config bmi160_regmap_config;
diff --git a/drivers/iio/imu/bmi160/bmi160_core.c b/drivers/iio/imu/bmi160/bmi160_core.c
index d3316ca02fbd..26d586daee26 100644
--- a/drivers/iio/imu/bmi160/bmi160_core.c
+++ b/drivers/iio/imu/bmi160/bmi160_core.c
@@ -110,6 +110,7 @@
.storagebits = 16, \
.endianness = IIO_LE, \
}, \
+ .ext_info = bmi160_ext_info, \
}
/* scan indexes follow DATA register order */
@@ -265,6 +266,20 @@ static const struct bmi160_odr_item bmi160_odr_table[] = {
},
};
+static const struct iio_mount_matrix *
+bmi160_get_mount_matrix(const struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan)
+{
+ struct bmi160_data *data = iio_priv(indio_dev);
+
+ return &data->orientation;
+}
+
+static const struct iio_chan_spec_ext_info bmi160_ext_info[] = {
+ IIO_MOUNT_MATRIX(IIO_SHARED_BY_DIR, bmi160_get_mount_matrix),
+ { }
+};
+
static const struct iio_chan_spec bmi160_channels[] = {
BMI160_CHANNEL(IIO_ACCEL, X, BMI160_SCAN_ACCEL_X),
BMI160_CHANNEL(IIO_ACCEL, Y, BMI160_SCAN_ACCEL_Y),
@@ -839,6 +854,11 @@ int bmi160_core_probe(struct device *dev, struct regmap *regmap,
return ret;
}
+ ret = iio_read_mount_matrix(dev, "mount-matrix",
+ &data->orientation);
+ if (ret)
+ return ret;
+
ret = bmi160_chip_init(data, use_spi);
if (ret)
return ret;
--
2.17.1
^ permalink raw reply related
* [PATCH v4 4/5] iio: imu: bmi160: added regulator support
From: Jonathan Albrieux @ 2020-05-25 16:46 UTC (permalink / raw)
To: linux-kernel
Cc: ~postmarketos/upstreaming, daniel.baluta, Jonathan Albrieux,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Hartmut Knaack, Lars-Peter Clausen,
open list:IIO SUBSYSTEM AND DRIVERS, Peter Meerwald-Stadler,
Jonathan Cameron
In-Reply-To: <20200525164615.14962-1-jonathan.albrieux@gmail.com>
Add vdd-supply and vddio-supply support.
While working on an msm8916 device and having explicit declarations for
regulators, without setting these regulators to regulators-always-on it
happened those lines weren't ready because they could have been controlled
by other components, causing failure in module's probe.
This patch aim is to solve this situation by adding regulators control
during bmi160_chip_init() and bmi160_chip_uninit(), assuring power to
this component.
Signed-off-by: Jonathan Albrieux <jonathan.albrieux@gmail.com>
---
drivers/iio/imu/bmi160/bmi160.h | 2 ++
drivers/iio/imu/bmi160/bmi160_core.c | 24 ++++++++++++++++++++++++
2 files changed, 26 insertions(+)
diff --git a/drivers/iio/imu/bmi160/bmi160.h b/drivers/iio/imu/bmi160/bmi160.h
index 621f5309d735..923c3b274fde 100644
--- a/drivers/iio/imu/bmi160/bmi160.h
+++ b/drivers/iio/imu/bmi160/bmi160.h
@@ -3,10 +3,12 @@
#define BMI160_H_
#include <linux/iio/iio.h>
+#include <linux/regulator/consumer.h>
struct bmi160_data {
struct regmap *regmap;
struct iio_trigger *trig;
+ struct regulator_bulk_data supplies[2];
};
extern const struct regmap_config bmi160_regmap_config;
diff --git a/drivers/iio/imu/bmi160/bmi160_core.c b/drivers/iio/imu/bmi160/bmi160_core.c
index 77b05bd4a2b2..d3316ca02fbd 100644
--- a/drivers/iio/imu/bmi160/bmi160_core.c
+++ b/drivers/iio/imu/bmi160/bmi160_core.c
@@ -15,6 +15,7 @@
#include <linux/delay.h>
#include <linux/irq.h>
#include <linux/of_irq.h>
+#include <linux/regulator/consumer.h>
#include <linux/iio/iio.h>
#include <linux/iio/triggered_buffer.h>
@@ -709,6 +710,12 @@ static int bmi160_chip_init(struct bmi160_data *data, bool use_spi)
unsigned int val;
struct device *dev = regmap_get_device(data->regmap);
+ ret = regulator_bulk_enable(ARRAY_SIZE(data->supplies), data->supplies);
+ if (ret) {
+ dev_err(dev, "Failed to enable regulators: %d\n", ret);
+ return ret;
+ }
+
ret = regmap_write(data->regmap, BMI160_REG_CMD, BMI160_CMD_SOFTRESET);
if (ret)
return ret;
@@ -793,9 +800,16 @@ int bmi160_probe_trigger(struct iio_dev *indio_dev, int irq, u32 irq_type)
static void bmi160_chip_uninit(void *data)
{
struct bmi160_data *bmi_data = data;
+ struct device *dev = regmap_get_device(bmi_data->regmap);
+ int ret;
bmi160_set_mode(bmi_data, BMI160_GYRO, false);
bmi160_set_mode(bmi_data, BMI160_ACCEL, false);
+
+ ret = regulator_bulk_disable(ARRAY_SIZE(bmi_data->supplies),
+ bmi_data->supplies);
+ if (ret)
+ dev_err(dev, "Failed to disable regulators: %d\n", ret);
}
int bmi160_core_probe(struct device *dev, struct regmap *regmap,
@@ -815,6 +829,16 @@ int bmi160_core_probe(struct device *dev, struct regmap *regmap,
dev_set_drvdata(dev, indio_dev);
data->regmap = regmap;
+ data->supplies[0].supply = "vdd";
+ data->supplies[1].supply = "vddio";
+ ret = devm_regulator_bulk_get(dev,
+ ARRAY_SIZE(data->supplies),
+ data->supplies);
+ if (ret) {
+ dev_err(dev, "Failed to get regulators: %d\n", ret);
+ return ret;
+ }
+
ret = bmi160_chip_init(data, use_spi);
if (ret)
return ret;
--
2.17.1
^ permalink raw reply related
* [PATCH v4 2/5] dt-bindings: iio: imu: bmi160: add regulators and mount-matrix
From: Jonathan Albrieux @ 2020-05-25 16:46 UTC (permalink / raw)
To: linux-kernel
Cc: ~postmarketos/upstreaming, daniel.baluta, Jonathan Albrieux,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Hartmut Knaack, Lars-Peter Clausen,
open list:IIO SUBSYSTEM AND DRIVERS, Peter Meerwald-Stadler,
Jonathan Cameron, Rob Herring
In-Reply-To: <20200525164615.14962-1-jonathan.albrieux@gmail.com>
Add vdd-supply and vddio-supply support.
Add mount-matrix support.
Signed-off-by: Jonathan Albrieux <jonathan.albrieux@gmail.com>
---
.../bindings/iio/imu/bosch,bmi160.yaml | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/Documentation/devicetree/bindings/iio/imu/bosch,bmi160.yaml b/Documentation/devicetree/bindings/iio/imu/bosch,bmi160.yaml
index 0d0ef84e22b9..cfe40dbcd723 100644
--- a/Documentation/devicetree/bindings/iio/imu/bosch,bmi160.yaml
+++ b/Documentation/devicetree/bindings/iio/imu/bosch,bmi160.yaml
@@ -37,6 +37,17 @@ properties:
set if the specified interrupt pin should be configured as
open drain. If not set, defaults to push-pull.
+ vdd-supply:
+ maxItems: 1
+ description: provide VDD power to the sensor.
+
+ vddio-supply:
+ maxItems: 1
+ description: provide VDD IO power to the sensor.
+
+ mount-matrix:
+ description: an optional 3x3 mounting rotation matrix
+
required:
- compatible
- reg
@@ -52,9 +63,14 @@ examples:
bmi160@68 {
compatible = "bosch,bmi160";
reg = <0x68>;
+ vdd-supply = <&pm8916_l17>;
+ vddio-supply = <&pm8916_l6>;
interrupt-parent = <&gpio4>;
interrupts = <12 IRQ_TYPE_EDGE_RISING>;
interrupt-names = "INT1";
+ mount-matrix = "0", "1", "0",
+ "-1", "0", "0",
+ "0", "0", "1";
};
};
- |
--
2.17.1
^ permalink raw reply related
* [PATCH v4 3/5] iio: imu: bmi160: fix typo
From: Jonathan Albrieux @ 2020-05-25 16:46 UTC (permalink / raw)
To: linux-kernel
Cc: ~postmarketos/upstreaming, daniel.baluta, Jonathan Albrieux,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Hartmut Knaack, Lars-Peter Clausen,
open list:IIO SUBSYSTEM AND DRIVERS, Peter Meerwald-Stadler,
Jonathan Cameron
In-Reply-To: <20200525164615.14962-1-jonathan.albrieux@gmail.com>
Fix a typo in MODULE_AUTHOR() argument.
Signed-off-by: Jonathan Albrieux <jonathan.albrieux@gmail.com>
---
drivers/iio/imu/bmi160/bmi160_core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iio/imu/bmi160/bmi160_core.c b/drivers/iio/imu/bmi160/bmi160_core.c
index 6af65d6f1d28..77b05bd4a2b2 100644
--- a/drivers/iio/imu/bmi160/bmi160_core.c
+++ b/drivers/iio/imu/bmi160/bmi160_core.c
@@ -853,6 +853,6 @@ int bmi160_core_probe(struct device *dev, struct regmap *regmap,
}
EXPORT_SYMBOL_GPL(bmi160_core_probe);
-MODULE_AUTHOR("Daniel Baluta <daniel.baluta@intel.com");
+MODULE_AUTHOR("Daniel Baluta <daniel.baluta@intel.com>");
MODULE_DESCRIPTION("Bosch BMI160 driver");
MODULE_LICENSE("GPL v2");
--
2.17.1
^ permalink raw reply related
* [PATCH v4 1/5] dt-bindings: iio: imu: bmi160: convert format to yaml, add maintainer
From: Jonathan Albrieux @ 2020-05-25 16:46 UTC (permalink / raw)
To: linux-kernel
Cc: ~postmarketos/upstreaming, daniel.baluta, Jonathan Albrieux,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Hartmut Knaack, Lars-Peter Clausen,
open list:IIO SUBSYSTEM AND DRIVERS, Peter Meerwald-Stadler,
Jonathan Cameron, Rob Herring
In-Reply-To: <20200525164615.14962-1-jonathan.albrieux@gmail.com>
Converts documentation from txt format to yaml.
Signed-off-by: Jonathan Albrieux <jonathan.albrieux@gmail.com>
---
.../devicetree/bindings/iio/imu/bmi160.txt | 37 ---------
.../bindings/iio/imu/bosch,bmi160.yaml | 75 +++++++++++++++++++
2 files changed, 75 insertions(+), 37 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/iio/imu/bmi160.txt
create mode 100644 Documentation/devicetree/bindings/iio/imu/bosch,bmi160.yaml
diff --git a/Documentation/devicetree/bindings/iio/imu/bmi160.txt b/Documentation/devicetree/bindings/iio/imu/bmi160.txt
deleted file mode 100644
index 900c169de00f..000000000000
--- a/Documentation/devicetree/bindings/iio/imu/bmi160.txt
+++ /dev/null
@@ -1,37 +0,0 @@
-Bosch BMI160 - Inertial Measurement Unit with Accelerometer, Gyroscope
-and externally connectable Magnetometer
-
-https://www.bosch-sensortec.com/bst/products/all_products/bmi160
-
-Required properties:
- - compatible : should be "bosch,bmi160"
- - reg : the I2C address or SPI chip select number of the sensor
- - spi-max-frequency : set maximum clock frequency (only for SPI)
-
-Optional properties:
- - interrupts : interrupt mapping for IRQ
- - interrupt-names : set to "INT1" if INT1 pin should be used as interrupt
- input, set to "INT2" if INT2 pin should be used instead
- - drive-open-drain : set if the specified interrupt pin should be configured as
- open drain. If not set, defaults to push-pull.
-
-Examples:
-
-bmi160@68 {
- compatible = "bosch,bmi160";
- reg = <0x68>;
-
- interrupt-parent = <&gpio4>;
- interrupts = <12 IRQ_TYPE_EDGE_RISING>;
- interrupt-names = "INT1";
-};
-
-bmi160@0 {
- compatible = "bosch,bmi160";
- reg = <0>;
- spi-max-frequency = <10000000>;
-
- interrupt-parent = <&gpio2>;
- interrupts = <12 IRQ_TYPE_LEVEL_LOW>;
- interrupt-names = "INT2";
-};
diff --git a/Documentation/devicetree/bindings/iio/imu/bosch,bmi160.yaml b/Documentation/devicetree/bindings/iio/imu/bosch,bmi160.yaml
new file mode 100644
index 000000000000..0d0ef84e22b9
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/imu/bosch,bmi160.yaml
@@ -0,0 +1,75 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/imu/bosch,bmi160.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Bosch BMI160
+
+maintainers:
+ - Jonathan Cameron <jic23@kernel.org>
+
+description: |
+ Inertial Measurement Unit with Accelerometer, Gyroscope and externally
+ connectable Magnetometer
+ https://www.bosch-sensortec.com/bst/products/all_products/bmi160
+
+properties:
+ compatible:
+ const: bosch,bmi160
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ interrupt-names:
+ enum:
+ - INT1
+ - INT2
+ description: |
+ set to "INT1" if INT1 pin should be used as interrupt input, set
+ to "INT2" if INT2 pin should be used instead
+
+ drive-open-drain:
+ description: |
+ set if the specified interrupt pin should be configured as
+ open drain. If not set, defaults to push-pull.
+
+required:
+ - compatible
+ - reg
+
+examples:
+ - |
+ // Example for I2C
+ #include <dt-bindings/interrupt-controller/irq.h>
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ bmi160@68 {
+ compatible = "bosch,bmi160";
+ reg = <0x68>;
+ interrupt-parent = <&gpio4>;
+ interrupts = <12 IRQ_TYPE_EDGE_RISING>;
+ interrupt-names = "INT1";
+ };
+ };
+ - |
+ // Example for SPI
+ #include <dt-bindings/interrupt-controller/irq.h>
+ spi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ bmi160@0 {
+ compatible = "bosch,bmi160";
+ reg = <0>;
+ spi-max-frequency = <10000000>;
+ interrupt-parent = <&gpio2>;
+ interrupts = <12 IRQ_TYPE_EDGE_RISING>;
+ interrupt-names = "INT2";
+ };
+ };
--
2.17.1
^ permalink raw reply related
* [PATCH v4 0/5] iio: imu: bmi160: added regulator and mount-matrix support
From: Jonathan Albrieux @ 2020-05-25 16:45 UTC (permalink / raw)
To: linux-kernel
Cc: ~postmarketos/upstreaming, daniel.baluta, Jonathan Albrieux,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Hartmut Knaack, Lars-Peter Clausen,
open list:IIO SUBSYSTEM AND DRIVERS, Peter Meerwald-Stadler
v4:
- add maintainer
- clean up of documentation
- added case concerning the need to add regulators, see [PATCH v4 4/5]
v3:
- separate typo fix into another patch
- clean up of documentation
- clean up of patch messages
https://lore.kernel.org/linux-iio/20200520194656.16218-1-jonathan.albrieux@gmail.com/
v2:
- fixed missing description for iio: imu: bmi160: added regulator
support
https://lore.kernel.org/linux-iio/20200519075111.6356-1-jonathan.albrieux@gmail.com/
v1:
- initial patch submission
https://lore.kernel.org/linux-iio/20200518133358.18978-1-jonathan.albrieux@gmail.com/
Convert txt format documentation to yaml.
Add documentation about vdd-supply, vddio-supply and mount-matrix.
Add vdd-supply and vddio-supply support.
Add mount-matrix binding support. As chip could have different
orientations a mount matrix support is needed to correctly translate
these differences.
Jonathan Albrieux (5):
dt-bindings: iio: imu: bmi160: convert format to yaml, add maintainer
dt-bindings: iio: imu: bmi160: add regulators and mount-matrix
iio: imu: bmi160: fix typo
iio: imu: bmi160: added regulator support
iio: imu: bmi160: added mount-matrix support
.../devicetree/bindings/iio/imu/bmi160.txt | 37 --------
.../bindings/iio/imu/bosch,bmi160.yaml | 91 +++++++++++++++++++
drivers/iio/imu/bmi160/bmi160.h | 3 +
drivers/iio/imu/bmi160/bmi160_core.c | 46 +++++++++-
4 files changed, 139 insertions(+), 38 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/iio/imu/bmi160.txt
create mode 100644 Documentation/devicetree/bindings/iio/imu/bosch,bmi160.yaml
--
2.17.1
^ permalink raw reply
* [PATCH net v3 4/4] ARM: dts: imx6qdl-sabresd: enable fec wake-on-lan
From: fugang.duan @ 2020-05-25 16:27 UTC (permalink / raw)
To: andrew, martin.fuzzey, davem, s.hauer
Cc: netdev, robh+dt, shawnguo, devicetree, kuba, fugang.duan
In-Reply-To: <1590424033-16906-1-git-send-email-fugang.duan@nxp.com>
From: Fugang Duan <fugang.duan@nxp.com>
Enable ethernet wake-on-lan feature for imx6q/dl/qp sabresd
boards since the PHY clock is supplied by external osc.
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Fugang Duan <fugang.duan@nxp.com>
---
arch/arm/boot/dts/imx6qdl-sabresd.dtsi | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/boot/dts/imx6qdl-sabresd.dtsi b/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
index fe59dde..28b35cc 100644
--- a/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
@@ -204,6 +204,7 @@
pinctrl-0 = <&pinctrl_enet>;
phy-mode = "rgmii-id";
phy-reset-gpios = <&gpio1 25 GPIO_ACTIVE_LOW>;
+ fsl,magic-packet;
status = "okay";
};
--
2.7.4
^ permalink raw reply related
* [PATCH net v3 3/4] ARM: dts: imx: add ethernet stop mode property
From: fugang.duan @ 2020-05-25 16:27 UTC (permalink / raw)
To: andrew, martin.fuzzey, davem, s.hauer
Cc: netdev, robh+dt, shawnguo, devicetree, kuba, fugang.duan
In-Reply-To: <1590424033-16906-1-git-send-email-fugang.duan@nxp.com>
From: Fugang Duan <fugang.duan@nxp.com>
- Update the imx6qdl gpr property to define gpr register
offset and bit in DT.
- Add imx6sx/imx6ul/imx7d ethernet stop mode property.
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Fugang Duan <fugang.duan@nxp.com>
---
arch/arm/boot/dts/imx6qdl.dtsi | 2 +-
arch/arm/boot/dts/imx6sx.dtsi | 2 ++
arch/arm/boot/dts/imx6ul.dtsi | 2 ++
arch/arm/boot/dts/imx7d.dtsi | 1 +
arch/arm/boot/dts/imx7s.dtsi | 1 +
5 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index 98da446..48f5016 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -1045,7 +1045,7 @@
<&clks IMX6QDL_CLK_ENET>,
<&clks IMX6QDL_CLK_ENET_REF>;
clock-names = "ipg", "ahb", "ptp";
- gpr = <&gpr>;
+ fsl,stop-mode = <&gpr 0x34 27>;
status = "disabled";
};
diff --git a/arch/arm/boot/dts/imx6sx.dtsi b/arch/arm/boot/dts/imx6sx.dtsi
index d6f8317..09f21aa 100644
--- a/arch/arm/boot/dts/imx6sx.dtsi
+++ b/arch/arm/boot/dts/imx6sx.dtsi
@@ -930,6 +930,7 @@
"enet_clk_ref", "enet_out";
fsl,num-tx-queues = <3>;
fsl,num-rx-queues = <3>;
+ fsl,stop-mode = <&gpr 0x10 3>;
status = "disabled";
};
@@ -1039,6 +1040,7 @@
<&clks IMX6SX_CLK_ENET_PTP>;
clock-names = "ipg", "ahb", "ptp",
"enet_clk_ref", "enet_out";
+ fsl,stop-mode = <&gpr 0x10 4>;
status = "disabled";
};
diff --git a/arch/arm/boot/dts/imx6ul.dtsi b/arch/arm/boot/dts/imx6ul.dtsi
index 2ccf67c..345ae9b 100644
--- a/arch/arm/boot/dts/imx6ul.dtsi
+++ b/arch/arm/boot/dts/imx6ul.dtsi
@@ -520,6 +520,7 @@
"enet_clk_ref", "enet_out";
fsl,num-tx-queues = <1>;
fsl,num-rx-queues = <1>;
+ fsl,stop-mode = <&gpr 0x10 4>;
status = "disabled";
};
@@ -856,6 +857,7 @@
"enet_clk_ref", "enet_out";
fsl,num-tx-queues = <1>;
fsl,num-rx-queues = <1>;
+ fsl,stop-mode = <&gpr 0x10 3>;
status = "disabled";
};
diff --git a/arch/arm/boot/dts/imx7d.dtsi b/arch/arm/boot/dts/imx7d.dtsi
index 4c22828..cff875b 100644
--- a/arch/arm/boot/dts/imx7d.dtsi
+++ b/arch/arm/boot/dts/imx7d.dtsi
@@ -153,6 +153,7 @@
"enet_clk_ref", "enet_out";
fsl,num-tx-queues = <3>;
fsl,num-rx-queues = <3>;
+ fsl,stop-mode = <&gpr 0x10 4>;
status = "disabled";
};
diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
index 76e3ffb..5bf0b39 100644
--- a/arch/arm/boot/dts/imx7s.dtsi
+++ b/arch/arm/boot/dts/imx7s.dtsi
@@ -1190,6 +1190,7 @@
"enet_clk_ref", "enet_out";
fsl,num-tx-queues = <3>;
fsl,num-rx-queues = <3>;
+ fsl,stop-mode = <&gpr 0x10 3>;
status = "disabled";
};
};
--
2.7.4
^ permalink raw reply related
* [PATCH net v3 2/4] dt-bindings: fec: update the gpr property
From: fugang.duan @ 2020-05-25 16:27 UTC (permalink / raw)
To: andrew, martin.fuzzey, davem, s.hauer
Cc: netdev, robh+dt, shawnguo, devicetree, kuba, fugang.duan
In-Reply-To: <1590424033-16906-1-git-send-email-fugang.duan@nxp.com>
From: Fugang Duan <fugang.duan@nxp.com>
- rename the 'gpr' property string to 'fsl,stop-mode'.
- Update the property to define gpr register offset and
bit in DT, since different instance have different gpr bit.
v2:
* rename 'gpr' property string to 'fsl,stop-mode'.
Signed-off-by: Fugang Duan <fugang.duan@nxp.com>
---
Documentation/devicetree/bindings/net/fsl-fec.txt | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/fsl-fec.txt b/Documentation/devicetree/bindings/net/fsl-fec.txt
index 26c492a..9b54378 100644
--- a/Documentation/devicetree/bindings/net/fsl-fec.txt
+++ b/Documentation/devicetree/bindings/net/fsl-fec.txt
@@ -22,8 +22,11 @@ Optional properties:
- fsl,err006687-workaround-present: If present indicates that the system has
the hardware workaround for ERR006687 applied and does not need a software
workaround.
-- gpr: phandle of SoC general purpose register mode. Required for wake on LAN
- on some SoCs
+- fsl,stop-mode: register bits of stop mode control, the format is
+ <&gpr req_gpr req_bit>.
+ gpr is the phandle to general purpose register node.
+ req_gpr is the gpr register offset for ENET stop request.
+ req_bit is the gpr bit offset for ENET stop request.
-interrupt-names: names of the interrupts listed in interrupts property in
the same order. The defaults if not specified are
__Number of interrupts__ __Default__
--
2.7.4
^ permalink raw reply related
* [PATCH net v3 1/4] net: ethernet: fec: move GPR register offset and bit into DT
From: fugang.duan @ 2020-05-25 16:27 UTC (permalink / raw)
To: andrew, martin.fuzzey, davem, s.hauer
Cc: netdev, robh+dt, shawnguo, devicetree, kuba, fugang.duan
In-Reply-To: <1590424033-16906-1-git-send-email-fugang.duan@nxp.com>
From: Fugang Duan <fugang.duan@nxp.com>
The commit da722186f654 (net: fec: set GPR bit on suspend by DT
configuration) set the GPR reigster offset and bit in driver for
wake on lan feature.
But it introduces two issues here:
- one SOC has two instances, they have different bit
- different SOCs may have different offset and bit
So to support wake-on-lan feature on other i.MX platforms, it should
configure the GPR reigster offset and bit from DT.
So the patch is to improve the commit da722186f654 (net: fec: set GPR
bit on suspend by DT configuration) to support multiple ethernet
instances on i.MX series.
v2:
* switch back to store the quirks bitmask in driver_data
v3:
* suggested by Sascha Hauer, use a struct fec_devinfo for
abstracting differences between different hardware variants,
it can give more freedom to describe the differences.
Signed-off-by: Fugang Duan <fugang.duan@nxp.com>
---
drivers/net/ethernet/freescale/fec_main.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 2e20914..4acb91d 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -88,8 +88,6 @@ static void fec_enet_itr_coal_init(struct net_device *ndev);
struct fec_devinfo {
u32 quirks;
- u8 stop_gpr_reg;
- u8 stop_gpr_bit;
};
static const struct fec_devinfo fec_imx25_info = {
@@ -112,8 +110,6 @@ static const struct fec_devinfo fec_imx6q_info = {
FEC_QUIRK_HAS_BUFDESC_EX | FEC_QUIRK_HAS_CSUM |
FEC_QUIRK_HAS_VLAN | FEC_QUIRK_ERR006358 |
FEC_QUIRK_HAS_RACC,
- .stop_gpr_reg = 0x34,
- .stop_gpr_bit = 27,
};
static const struct fec_devinfo fec_mvf600_info = {
@@ -3476,19 +3472,23 @@ static int fec_enet_get_irq_cnt(struct platform_device *pdev)
}
static int fec_enet_init_stop_mode(struct fec_enet_private *fep,
- struct fec_devinfo *dev_info,
struct device_node *np)
{
struct device_node *gpr_np;
+ u32 out_val[3];
int ret = 0;
- if (!dev_info)
- return 0;
-
- gpr_np = of_parse_phandle(np, "gpr", 0);
+ gpr_np = of_parse_phandle(np, "fsl,stop-mode", 0);
if (!gpr_np)
return 0;
+ ret = of_property_read_u32_array(np, "fsl,stop-mode", out_val,
+ ARRAY_SIZE(out_val));
+ if (ret) {
+ dev_dbg(&fep->pdev->dev, "no stop mode property\n");
+ return ret;
+ }
+
fep->stop_gpr.gpr = syscon_node_to_regmap(gpr_np);
if (IS_ERR(fep->stop_gpr.gpr)) {
dev_err(&fep->pdev->dev, "could not find gpr regmap\n");
@@ -3497,8 +3497,8 @@ static int fec_enet_init_stop_mode(struct fec_enet_private *fep,
goto out;
}
- fep->stop_gpr.reg = dev_info->stop_gpr_reg;
- fep->stop_gpr.bit = dev_info->stop_gpr_bit;
+ fep->stop_gpr.reg = out_val[1];
+ fep->stop_gpr.bit = out_val[2];
out:
of_node_put(gpr_np);
@@ -3575,7 +3575,7 @@ fec_probe(struct platform_device *pdev)
if (of_get_property(np, "fsl,magic-packet", NULL))
fep->wol_flag |= FEC_WOL_HAS_MAGIC_PACKET;
- ret = fec_enet_init_stop_mode(fep, dev_info, np);
+ ret = fec_enet_init_stop_mode(fep, np);
if (ret)
goto failed_stop_mode;
--
2.7.4
^ 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