Re: [PATCH] staging: remove unneeded static set .owner field in platform_driver
On Wed, Sep 12, 2018 at 9:22 AM zhong jiang wrote:
>
> platform_driver_register will set the .owner field. So it is safe
> to remove the redundant assignment.
>
> The issue is detected with the help of Coccinelle.
>
> Signed-off-by: zhong jiang
> ---
> drivers/staging/greybus/audio_codec.c| 1 -
> drivers/staging/mt7621-eth/gsw_mt7621.c | 1 -
> drivers/staging/mt7621-eth/mtk_eth_soc.c | 1 -
> 3 files changed, 3 deletions(-)
>
> diff --git a/drivers/staging/greybus/audio_codec.c
> b/drivers/staging/greybus/audio_codec.c
> index 35acd55..08746c8 100644
> --- a/drivers/staging/greybus/audio_codec.c
> +++ b/drivers/staging/greybus/audio_codec.c
> @@ -1087,7 +1087,6 @@ static int gbaudio_codec_remove(struct platform_device
> *pdev)
> static struct platform_driver gbaudio_codec_driver = {
> .driver = {
> .name = "apb-dummy-codec",
> - .owner = THIS_MODULE,
> #ifdef CONFIG_PM
> .pm = _codec_pm_ops,
> #endif
> diff --git a/drivers/staging/mt7621-eth/gsw_mt7621.c
> b/drivers/staging/mt7621-eth/gsw_mt7621.c
> index 2c07b55..53767b1 100644
> --- a/drivers/staging/mt7621-eth/gsw_mt7621.c
> +++ b/drivers/staging/mt7621-eth/gsw_mt7621.c
> @@ -286,7 +286,6 @@ static int mt7621_gsw_remove(struct platform_device *pdev)
> .remove = mt7621_gsw_remove,
> .driver = {
> .name = "mt7621-gsw",
> - .owner = THIS_MODULE,
> .of_match_table = mediatek_gsw_match,
> },
> };
> diff --git a/drivers/staging/mt7621-eth/mtk_eth_soc.c
> b/drivers/staging/mt7621-eth/mtk_eth_soc.c
> index 7135075..363d3c9 100644
> --- a/drivers/staging/mt7621-eth/mtk_eth_soc.c
> +++ b/drivers/staging/mt7621-eth/mtk_eth_soc.c
> @@ -2167,7 +2167,6 @@ static int mtk_remove(struct platform_device *pdev)
> .remove = mtk_remove,
> .driver = {
> .name = "mtk_soc_eth",
> - .owner = THIS_MODULE,
> .of_match_table = of_mtk_match,
> },
> };
> --
> 1.7.12.4
>
Acked-by: Vaibhav Agarwal
Re: [PATCH] staging: remove unneeded static set .owner field in platform_driver
On Wed, Sep 12, 2018 at 9:22 AM zhong jiang wrote:
>
> platform_driver_register will set the .owner field. So it is safe
> to remove the redundant assignment.
>
> The issue is detected with the help of Coccinelle.
>
> Signed-off-by: zhong jiang
> ---
> drivers/staging/greybus/audio_codec.c| 1 -
> drivers/staging/mt7621-eth/gsw_mt7621.c | 1 -
> drivers/staging/mt7621-eth/mtk_eth_soc.c | 1 -
> 3 files changed, 3 deletions(-)
>
> diff --git a/drivers/staging/greybus/audio_codec.c
> b/drivers/staging/greybus/audio_codec.c
> index 35acd55..08746c8 100644
> --- a/drivers/staging/greybus/audio_codec.c
> +++ b/drivers/staging/greybus/audio_codec.c
> @@ -1087,7 +1087,6 @@ static int gbaudio_codec_remove(struct platform_device
> *pdev)
> static struct platform_driver gbaudio_codec_driver = {
> .driver = {
> .name = "apb-dummy-codec",
> - .owner = THIS_MODULE,
> #ifdef CONFIG_PM
> .pm = _codec_pm_ops,
> #endif
> diff --git a/drivers/staging/mt7621-eth/gsw_mt7621.c
> b/drivers/staging/mt7621-eth/gsw_mt7621.c
> index 2c07b55..53767b1 100644
> --- a/drivers/staging/mt7621-eth/gsw_mt7621.c
> +++ b/drivers/staging/mt7621-eth/gsw_mt7621.c
> @@ -286,7 +286,6 @@ static int mt7621_gsw_remove(struct platform_device *pdev)
> .remove = mt7621_gsw_remove,
> .driver = {
> .name = "mt7621-gsw",
> - .owner = THIS_MODULE,
> .of_match_table = mediatek_gsw_match,
> },
> };
> diff --git a/drivers/staging/mt7621-eth/mtk_eth_soc.c
> b/drivers/staging/mt7621-eth/mtk_eth_soc.c
> index 7135075..363d3c9 100644
> --- a/drivers/staging/mt7621-eth/mtk_eth_soc.c
> +++ b/drivers/staging/mt7621-eth/mtk_eth_soc.c
> @@ -2167,7 +2167,6 @@ static int mtk_remove(struct platform_device *pdev)
> .remove = mtk_remove,
> .driver = {
> .name = "mtk_soc_eth",
> - .owner = THIS_MODULE,
> .of_match_table = of_mtk_match,
> },
> };
> --
> 1.7.12.4
>
Acked-by: Vaibhav Agarwal
Re: [PATCH V3 4/6] x86/intel_rdt: Create required perf event attributes
Hi Reinette,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on tip/x86/core]
[also build test ERROR on v4.19-rc3 next-20180911]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Reinette-Chatre/perf-core-and-x86-intel_rdt-Fix-lack-of-coordination-with-perf/20180912-101526
config: i386-randconfig-x001-201836 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
Note: the
linux-review/Reinette-Chatre/perf-core-and-x86-intel_rdt-Fix-lack-of-coordination-with-perf/20180912-101526
HEAD b684b8727deb9e3cf635badb292b3314904d17b2 builds fine.
It only hurts bisectibility.
All error/warnings (new ones prefixed by >>):
>> arch/x86//kernel/cpu/intel_rdt_pseudo_lock.c:927:15: error: variable
>> 'perf_miss_attr' has initializer but incomplete type
static struct perf_event_attr __attribute__((unused)) perf_miss_attr = {
^~~
>> arch/x86//kernel/cpu/intel_rdt_pseudo_lock.c:928:3: error: 'struct
>> perf_event_attr' has no member named 'type'
.type = PERF_TYPE_RAW,
^~~~
>> arch/x86//kernel/cpu/intel_rdt_pseudo_lock.c:928:11: error: 'PERF_TYPE_RAW'
>> undeclared here (not in a function); did you mean 'PIDTYPE_MAX'?
.type = PERF_TYPE_RAW,
^
PIDTYPE_MAX
>> arch/x86//kernel/cpu/intel_rdt_pseudo_lock.c:928:11: warning: excess
>> elements in struct initializer
arch/x86//kernel/cpu/intel_rdt_pseudo_lock.c:928:11: note: (near
initialization for 'perf_miss_attr')
>> arch/x86//kernel/cpu/intel_rdt_pseudo_lock.c:929:3: error: 'struct
>> perf_event_attr' has no member named 'size'
.size = sizeof(struct perf_event_attr),
^~~~
>> arch/x86//kernel/cpu/intel_rdt_pseudo_lock.c:929:18: error: invalid
>> application of 'sizeof' to incomplete type 'struct perf_event_attr'
.size = sizeof(struct perf_event_attr),
^~
arch/x86//kernel/cpu/intel_rdt_pseudo_lock.c:929:11: warning: excess
elements in struct initializer
.size = sizeof(struct perf_event_attr),
^~
arch/x86//kernel/cpu/intel_rdt_pseudo_lock.c:929:11: note: (near
initialization for 'perf_miss_attr')
>> arch/x86//kernel/cpu/intel_rdt_pseudo_lock.c:930:3: error: 'struct
>> perf_event_attr' has no member named 'pinned'
.pinned = 1,
^~
arch/x86//kernel/cpu/intel_rdt_pseudo_lock.c:930:13: warning: excess
elements in struct initializer
.pinned = 1,
^
arch/x86//kernel/cpu/intel_rdt_pseudo_lock.c:930:13: note: (near
initialization for 'perf_miss_attr')
>> arch/x86//kernel/cpu/intel_rdt_pseudo_lock.c:931:3: error: 'struct
>> perf_event_attr' has no member named 'disabled'
.disabled = 0,
^~~~
arch/x86//kernel/cpu/intel_rdt_pseudo_lock.c:931:14: warning: excess
elements in struct initializer
.disabled = 0,
^
arch/x86//kernel/cpu/intel_rdt_pseudo_lock.c:931:14: note: (near
initialization for 'perf_miss_attr')
>> arch/x86//kernel/cpu/intel_rdt_pseudo_lock.c:932:3: error: 'struct
>> perf_event_attr' has no member named 'exclude_user'
.exclude_user = 1,
^~~~
arch/x86//kernel/cpu/intel_rdt_pseudo_lock.c:932:18: warning: excess
elements in struct initializer
.exclude_user = 1,
^
arch/x86//kernel/cpu/intel_rdt_pseudo_lock.c:932:18: note: (near
initialization for 'perf_miss_attr')
>> arch/x86//kernel/cpu/intel_rdt_pseudo_lock.c:935:15: error: variable
>> 'perf_hit_attr' has initializer but incomplete type
static struct perf_event_attr __attribute__((unused)) perf_hit_attr = {
^~~
arch/x86//kernel/cpu/intel_rdt_pseudo_lock.c:936:3: error: 'struct
perf_event_attr' has no member named 'type'
.type = PERF_TYPE_RAW,
^~~~
arch/x86//kernel/cpu/intel_rdt_pseudo_lock.c:936:11: warning: excess
elements in struct initializer
.type = PERF_TYPE_RAW,
^
arch/x86//kernel/cpu/intel_rdt_pseudo_lock.c:936:11: note: (near
initialization for 'perf_hit_attr')
arch/x86//kernel/cpu/intel_rdt_pseudo_lock.c:937:3: error: 'struct
perf_event_attr' has no member named 'size'
.size = sizeof(struct perf_event_attr),
^~~~
arch/x86//kernel/cpu/intel_rdt_pseudo_lock.c:937:18: error: invalid
application of 'sizeof' to incomplete type 'struct perf_event_attr'
.size = sizeof(struct perf_event_attr),
^~
arch/x86//kernel/cpu/intel_rdt_pseudo_lock.c:937:11: warning: excess
elements in struct initializer
.size = sizeof(struct perf_event_attr),
^~
arch/x86//kernel/cpu/intel_rdt_pseudo_lock.c:
Re: [PATCH V3 4/6] x86/intel_rdt: Create required perf event attributes
Hi Reinette,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on tip/x86/core]
[also build test ERROR on v4.19-rc3 next-20180911]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Reinette-Chatre/perf-core-and-x86-intel_rdt-Fix-lack-of-coordination-with-perf/20180912-101526
config: i386-randconfig-x001-201836 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
Note: the
linux-review/Reinette-Chatre/perf-core-and-x86-intel_rdt-Fix-lack-of-coordination-with-perf/20180912-101526
HEAD b684b8727deb9e3cf635badb292b3314904d17b2 builds fine.
It only hurts bisectibility.
All error/warnings (new ones prefixed by >>):
>> arch/x86//kernel/cpu/intel_rdt_pseudo_lock.c:927:15: error: variable
>> 'perf_miss_attr' has initializer but incomplete type
static struct perf_event_attr __attribute__((unused)) perf_miss_attr = {
^~~
>> arch/x86//kernel/cpu/intel_rdt_pseudo_lock.c:928:3: error: 'struct
>> perf_event_attr' has no member named 'type'
.type = PERF_TYPE_RAW,
^~~~
>> arch/x86//kernel/cpu/intel_rdt_pseudo_lock.c:928:11: error: 'PERF_TYPE_RAW'
>> undeclared here (not in a function); did you mean 'PIDTYPE_MAX'?
.type = PERF_TYPE_RAW,
^
PIDTYPE_MAX
>> arch/x86//kernel/cpu/intel_rdt_pseudo_lock.c:928:11: warning: excess
>> elements in struct initializer
arch/x86//kernel/cpu/intel_rdt_pseudo_lock.c:928:11: note: (near
initialization for 'perf_miss_attr')
>> arch/x86//kernel/cpu/intel_rdt_pseudo_lock.c:929:3: error: 'struct
>> perf_event_attr' has no member named 'size'
.size = sizeof(struct perf_event_attr),
^~~~
>> arch/x86//kernel/cpu/intel_rdt_pseudo_lock.c:929:18: error: invalid
>> application of 'sizeof' to incomplete type 'struct perf_event_attr'
.size = sizeof(struct perf_event_attr),
^~
arch/x86//kernel/cpu/intel_rdt_pseudo_lock.c:929:11: warning: excess
elements in struct initializer
.size = sizeof(struct perf_event_attr),
^~
arch/x86//kernel/cpu/intel_rdt_pseudo_lock.c:929:11: note: (near
initialization for 'perf_miss_attr')
>> arch/x86//kernel/cpu/intel_rdt_pseudo_lock.c:930:3: error: 'struct
>> perf_event_attr' has no member named 'pinned'
.pinned = 1,
^~
arch/x86//kernel/cpu/intel_rdt_pseudo_lock.c:930:13: warning: excess
elements in struct initializer
.pinned = 1,
^
arch/x86//kernel/cpu/intel_rdt_pseudo_lock.c:930:13: note: (near
initialization for 'perf_miss_attr')
>> arch/x86//kernel/cpu/intel_rdt_pseudo_lock.c:931:3: error: 'struct
>> perf_event_attr' has no member named 'disabled'
.disabled = 0,
^~~~
arch/x86//kernel/cpu/intel_rdt_pseudo_lock.c:931:14: warning: excess
elements in struct initializer
.disabled = 0,
^
arch/x86//kernel/cpu/intel_rdt_pseudo_lock.c:931:14: note: (near
initialization for 'perf_miss_attr')
>> arch/x86//kernel/cpu/intel_rdt_pseudo_lock.c:932:3: error: 'struct
>> perf_event_attr' has no member named 'exclude_user'
.exclude_user = 1,
^~~~
arch/x86//kernel/cpu/intel_rdt_pseudo_lock.c:932:18: warning: excess
elements in struct initializer
.exclude_user = 1,
^
arch/x86//kernel/cpu/intel_rdt_pseudo_lock.c:932:18: note: (near
initialization for 'perf_miss_attr')
>> arch/x86//kernel/cpu/intel_rdt_pseudo_lock.c:935:15: error: variable
>> 'perf_hit_attr' has initializer but incomplete type
static struct perf_event_attr __attribute__((unused)) perf_hit_attr = {
^~~
arch/x86//kernel/cpu/intel_rdt_pseudo_lock.c:936:3: error: 'struct
perf_event_attr' has no member named 'type'
.type = PERF_TYPE_RAW,
^~~~
arch/x86//kernel/cpu/intel_rdt_pseudo_lock.c:936:11: warning: excess
elements in struct initializer
.type = PERF_TYPE_RAW,
^
arch/x86//kernel/cpu/intel_rdt_pseudo_lock.c:936:11: note: (near
initialization for 'perf_hit_attr')
arch/x86//kernel/cpu/intel_rdt_pseudo_lock.c:937:3: error: 'struct
perf_event_attr' has no member named 'size'
.size = sizeof(struct perf_event_attr),
^~~~
arch/x86//kernel/cpu/intel_rdt_pseudo_lock.c:937:18: error: invalid
application of 'sizeof' to incomplete type 'struct perf_event_attr'
.size = sizeof(struct perf_event_attr),
^~
arch/x86//kernel/cpu/intel_rdt_pseudo_lock.c:937:11: warning: excess
elements in struct initializer
.size = sizeof(struct perf_event_attr),
^~
arch/x86//kernel/cpu/intel_rdt_pseudo_lock.c:
Re: [LKP] [rcu] 02a5c550b2: BUG:kernel_reboot-without-warning_in_test_stage
On Wed, Sep 12, 2018 at 01:25:27PM +0800, kernel test robot wrote:
> FYI, we noticed the following commit (built with gcc-7):
>
> commit: 02a5c550b2738f2bfea8e1e00aa75944d71c9e18 ("rcu: Abstract extended
> quiescent state determination")
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git master
>
> in testcase: perf_event_tests
> with following parameters:
>
> paranoid: disallow_kernel_profiling
>
> test-description: The Perf Event Testsuite.
> test-url: https://github.com/deater/perf_event_tests
>
>
> on test machine: qemu-system-x86_64 -enable-kvm -cpu host -smp 2 -m 2G
This is a blast from the past! Almost two years ago.
I will take a closer look (and also check for any later fixes), but at
first glance I am not seeing anything in this commit that would actually
change behavior.
But is it possible that this is due to vCPU preemption on a heavily
loaded test system? I have done that to myself from time to time...
Thanx, Paul
> caused below changes (please refer to attached dmesg/kmsg for entire
> log/backtrace):
>
>
> +-+++
> | | 2625d469ba | 02a5c550b2 |
> +-+++
> | boot_successes | 18 | 0 |
> | boot_failures | 0 | 17 |
> | BUG:kernel_reboot-without-warning_in_test_stage | 0 | 17 |
> +-+++
>
>
>
> [ 21.715217]
> [ 22.950524] perf: interrupt took too long (5334 > 5235), lowering
> kernel.perf_event_max_sample_rate to 37250
> [ 22.956921] perf: interrupt took too long (6735 > 6667), lowering
> kernel.perf_event_max_sample_rate to 29500
> [ 22.970150] perf: interrupt took too long (8494 > 8418), lowering
> kernel.perf_event_max_sample_rate to 23500
> [ 22.976586] perf: interrupt took too long (10754 > 10617), lowering
> kernel.perf_event_max_sample_rate to 18500
> BUG: kernel reboot-without-warning in test stage
>
> Elapsed time: 30
>
> #!/bin/bash
>
>
>
> To reproduce:
>
> git clone https://github.com/intel/lkp-tests.git
> cd lkp-tests
> bin/lkp qemu -k job-script # job-script is attached in this
> email
>
>
>
> Thanks,
> lkp
> #
> # Automatically generated file; DO NOT EDIT.
> # Linux/x86_64 4.10.0-rc3 Kernel Configuration
> #
> CONFIG_64BIT=y
> CONFIG_X86_64=y
> CONFIG_X86=y
> CONFIG_INSTRUCTION_DECODER=y
> CONFIG_OUTPUT_FORMAT="elf64-x86-64"
> CONFIG_ARCH_DEFCONFIG="arch/x86/configs/x86_64_defconfig"
> CONFIG_LOCKDEP_SUPPORT=y
> CONFIG_STACKTRACE_SUPPORT=y
> CONFIG_MMU=y
> CONFIG_ARCH_MMAP_RND_BITS_MIN=28
> CONFIG_ARCH_MMAP_RND_BITS_MAX=32
> CONFIG_ARCH_MMAP_RND_COMPAT_BITS_MIN=8
> CONFIG_ARCH_MMAP_RND_COMPAT_BITS_MAX=16
> CONFIG_NEED_DMA_MAP_STATE=y
> CONFIG_NEED_SG_DMA_LENGTH=y
> CONFIG_GENERIC_ISA_DMA=y
> CONFIG_GENERIC_BUG=y
> CONFIG_GENERIC_BUG_RELATIVE_POINTERS=y
> CONFIG_GENERIC_HWEIGHT=y
> CONFIG_ARCH_MAY_HAVE_PC_FDC=y
> CONFIG_RWSEM_XCHGADD_ALGORITHM=y
> CONFIG_GENERIC_CALIBRATE_DELAY=y
> CONFIG_ARCH_HAS_CPU_RELAX=y
> CONFIG_ARCH_HAS_CACHE_LINE_SIZE=y
> CONFIG_HAVE_SETUP_PER_CPU_AREA=y
> CONFIG_NEED_PER_CPU_EMBED_FIRST_CHUNK=y
> CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK=y
> CONFIG_ARCH_HIBERNATION_POSSIBLE=y
> CONFIG_ARCH_SUSPEND_POSSIBLE=y
> CONFIG_ARCH_WANT_HUGE_PMD_SHARE=y
> CONFIG_ARCH_WANT_GENERAL_HUGETLB=y
> CONFIG_ZONE_DMA32=y
> CONFIG_AUDIT_ARCH=y
> CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING=y
> CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC=y
> CONFIG_X86_64_SMP=y
> CONFIG_ARCH_SUPPORTS_UPROBES=y
> CONFIG_FIX_EARLYCON_MEM=y
> CONFIG_DEBUG_RODATA=y
> CONFIG_PGTABLE_LEVELS=4
> CONFIG_DEFCONFIG_LIST="/lib/modules/$UNAME_RELEASE/.config"
> CONFIG_IRQ_WORK=y
> CONFIG_BUILDTIME_EXTABLE_SORT=y
> CONFIG_THREAD_INFO_IN_TASK=y
>
> #
> # General setup
> #
> CONFIG_INIT_ENV_ARG_LIMIT=32
> CONFIG_CROSS_COMPILE=""
> # CONFIG_COMPILE_TEST is not set
> CONFIG_LOCALVERSION=""
> CONFIG_LOCALVERSION_AUTO=y
> CONFIG_HAVE_KERNEL_GZIP=y
> CONFIG_HAVE_KERNEL_BZIP2=y
> CONFIG_HAVE_KERNEL_LZMA=y
> CONFIG_HAVE_KERNEL_XZ=y
> CONFIG_HAVE_KERNEL_LZO=y
> CONFIG_HAVE_KERNEL_LZ4=y
> CONFIG_KERNEL_GZIP=y
> # CONFIG_KERNEL_BZIP2 is not set
> # CONFIG_KERNEL_LZMA is not set
> # CONFIG_KERNEL_XZ is not set
> # CONFIG_KERNEL_LZO is not set
> # CONFIG_KERNEL_LZ4 is not set
> CONFIG_DEFAULT_HOSTNAME="(none)"
> CONFIG_SWAP=y
> CONFIG_SYSVIPC=y
> CONFIG_SYSVIPC_SYSCTL=y
> CONFIG_POSIX_MQUEUE=y
> CONFIG_POSIX_MQUEUE_SYSCTL=y
> CONFIG_CROSS_MEMORY_ATTACH=y
> CONFIG_FHANDLE=y
> CONFIG_USELIB=y
> # CONFIG_AUDIT is not set
> CONFIG_HAVE_ARCH_AUDITSYSCALL=y
>
> #
> # IRQ subsystem
> #
> CONFIG_GENERIC_IRQ_PROBE=y
> CONFIG_GENERIC_IRQ_SHOW=y
> CONFIG_GENERIC_PENDING_IRQ=y
> CONFIG_IRQ_DOMAIN=y
> CONFIG_IRQ_DOMAIN_HIERARCHY=y
>
Re: [LKP] [rcu] 02a5c550b2: BUG:kernel_reboot-without-warning_in_test_stage
On Wed, Sep 12, 2018 at 01:25:27PM +0800, kernel test robot wrote:
> FYI, we noticed the following commit (built with gcc-7):
>
> commit: 02a5c550b2738f2bfea8e1e00aa75944d71c9e18 ("rcu: Abstract extended
> quiescent state determination")
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git master
>
> in testcase: perf_event_tests
> with following parameters:
>
> paranoid: disallow_kernel_profiling
>
> test-description: The Perf Event Testsuite.
> test-url: https://github.com/deater/perf_event_tests
>
>
> on test machine: qemu-system-x86_64 -enable-kvm -cpu host -smp 2 -m 2G
This is a blast from the past! Almost two years ago.
I will take a closer look (and also check for any later fixes), but at
first glance I am not seeing anything in this commit that would actually
change behavior.
But is it possible that this is due to vCPU preemption on a heavily
loaded test system? I have done that to myself from time to time...
Thanx, Paul
> caused below changes (please refer to attached dmesg/kmsg for entire
> log/backtrace):
>
>
> +-+++
> | | 2625d469ba | 02a5c550b2 |
> +-+++
> | boot_successes | 18 | 0 |
> | boot_failures | 0 | 17 |
> | BUG:kernel_reboot-without-warning_in_test_stage | 0 | 17 |
> +-+++
>
>
>
> [ 21.715217]
> [ 22.950524] perf: interrupt took too long (5334 > 5235), lowering
> kernel.perf_event_max_sample_rate to 37250
> [ 22.956921] perf: interrupt took too long (6735 > 6667), lowering
> kernel.perf_event_max_sample_rate to 29500
> [ 22.970150] perf: interrupt took too long (8494 > 8418), lowering
> kernel.perf_event_max_sample_rate to 23500
> [ 22.976586] perf: interrupt took too long (10754 > 10617), lowering
> kernel.perf_event_max_sample_rate to 18500
> BUG: kernel reboot-without-warning in test stage
>
> Elapsed time: 30
>
> #!/bin/bash
>
>
>
> To reproduce:
>
> git clone https://github.com/intel/lkp-tests.git
> cd lkp-tests
> bin/lkp qemu -k job-script # job-script is attached in this
> email
>
>
>
> Thanks,
> lkp
> #
> # Automatically generated file; DO NOT EDIT.
> # Linux/x86_64 4.10.0-rc3 Kernel Configuration
> #
> CONFIG_64BIT=y
> CONFIG_X86_64=y
> CONFIG_X86=y
> CONFIG_INSTRUCTION_DECODER=y
> CONFIG_OUTPUT_FORMAT="elf64-x86-64"
> CONFIG_ARCH_DEFCONFIG="arch/x86/configs/x86_64_defconfig"
> CONFIG_LOCKDEP_SUPPORT=y
> CONFIG_STACKTRACE_SUPPORT=y
> CONFIG_MMU=y
> CONFIG_ARCH_MMAP_RND_BITS_MIN=28
> CONFIG_ARCH_MMAP_RND_BITS_MAX=32
> CONFIG_ARCH_MMAP_RND_COMPAT_BITS_MIN=8
> CONFIG_ARCH_MMAP_RND_COMPAT_BITS_MAX=16
> CONFIG_NEED_DMA_MAP_STATE=y
> CONFIG_NEED_SG_DMA_LENGTH=y
> CONFIG_GENERIC_ISA_DMA=y
> CONFIG_GENERIC_BUG=y
> CONFIG_GENERIC_BUG_RELATIVE_POINTERS=y
> CONFIG_GENERIC_HWEIGHT=y
> CONFIG_ARCH_MAY_HAVE_PC_FDC=y
> CONFIG_RWSEM_XCHGADD_ALGORITHM=y
> CONFIG_GENERIC_CALIBRATE_DELAY=y
> CONFIG_ARCH_HAS_CPU_RELAX=y
> CONFIG_ARCH_HAS_CACHE_LINE_SIZE=y
> CONFIG_HAVE_SETUP_PER_CPU_AREA=y
> CONFIG_NEED_PER_CPU_EMBED_FIRST_CHUNK=y
> CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK=y
> CONFIG_ARCH_HIBERNATION_POSSIBLE=y
> CONFIG_ARCH_SUSPEND_POSSIBLE=y
> CONFIG_ARCH_WANT_HUGE_PMD_SHARE=y
> CONFIG_ARCH_WANT_GENERAL_HUGETLB=y
> CONFIG_ZONE_DMA32=y
> CONFIG_AUDIT_ARCH=y
> CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING=y
> CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC=y
> CONFIG_X86_64_SMP=y
> CONFIG_ARCH_SUPPORTS_UPROBES=y
> CONFIG_FIX_EARLYCON_MEM=y
> CONFIG_DEBUG_RODATA=y
> CONFIG_PGTABLE_LEVELS=4
> CONFIG_DEFCONFIG_LIST="/lib/modules/$UNAME_RELEASE/.config"
> CONFIG_IRQ_WORK=y
> CONFIG_BUILDTIME_EXTABLE_SORT=y
> CONFIG_THREAD_INFO_IN_TASK=y
>
> #
> # General setup
> #
> CONFIG_INIT_ENV_ARG_LIMIT=32
> CONFIG_CROSS_COMPILE=""
> # CONFIG_COMPILE_TEST is not set
> CONFIG_LOCALVERSION=""
> CONFIG_LOCALVERSION_AUTO=y
> CONFIG_HAVE_KERNEL_GZIP=y
> CONFIG_HAVE_KERNEL_BZIP2=y
> CONFIG_HAVE_KERNEL_LZMA=y
> CONFIG_HAVE_KERNEL_XZ=y
> CONFIG_HAVE_KERNEL_LZO=y
> CONFIG_HAVE_KERNEL_LZ4=y
> CONFIG_KERNEL_GZIP=y
> # CONFIG_KERNEL_BZIP2 is not set
> # CONFIG_KERNEL_LZMA is not set
> # CONFIG_KERNEL_XZ is not set
> # CONFIG_KERNEL_LZO is not set
> # CONFIG_KERNEL_LZ4 is not set
> CONFIG_DEFAULT_HOSTNAME="(none)"
> CONFIG_SWAP=y
> CONFIG_SYSVIPC=y
> CONFIG_SYSVIPC_SYSCTL=y
> CONFIG_POSIX_MQUEUE=y
> CONFIG_POSIX_MQUEUE_SYSCTL=y
> CONFIG_CROSS_MEMORY_ATTACH=y
> CONFIG_FHANDLE=y
> CONFIG_USELIB=y
> # CONFIG_AUDIT is not set
> CONFIG_HAVE_ARCH_AUDITSYSCALL=y
>
> #
> # IRQ subsystem
> #
> CONFIG_GENERIC_IRQ_PROBE=y
> CONFIG_GENERIC_IRQ_SHOW=y
> CONFIG_GENERIC_PENDING_IRQ=y
> CONFIG_IRQ_DOMAIN=y
> CONFIG_IRQ_DOMAIN_HIERARCHY=y
>
Re: [PATCH -next] staging: mt7621-pci: Use PTR_ERR_OR_ZERO in mt7621_pcie_parse_dt()
On Wed, Sep 12, 2018 at 02:50:08AM +, YueHaibing wrote: > Use PTR_ERR_OR_ZERO rather than if(IS_ERR(...)) + PTR_ERR > > Signed-off-by: YueHaibing > --- > drivers/staging/mt7621-pci/pci-mt7621.c | 5 + > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/drivers/staging/mt7621-pci/pci-mt7621.c > b/drivers/staging/mt7621-pci/pci-mt7621.c > index ba1f117..d2cb910 100644 > --- a/drivers/staging/mt7621-pci/pci-mt7621.c > +++ b/drivers/staging/mt7621-pci/pci-mt7621.c > @@ -396,10 +396,7 @@ static int mt7621_pcie_parse_dt(struct mt7621_pcie *pcie) > } > > pcie->base = devm_ioremap_resource(dev, ); > - if (IS_ERR(pcie->base)) > - return PTR_ERR(pcie->base); > - > - return 0; > + return PTR_ERR_OR_ZERO(pcie->base); > } This patch looks good but the 'mt7621_pcie_parse_dt' function is not completed at all. There is a lot of missing for each pci node to be parsed yet and some patch series which are doing this have not been tested yet so those patches are not included. Please see: http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2018-September/125937.html Best regards, Sergio Paracuellos > > static int mt7621_pcie_request_resources(struct mt7621_pcie *pcie, > > >
Re: [PATCH -next] staging: mt7621-pci: Use PTR_ERR_OR_ZERO in mt7621_pcie_parse_dt()
On Wed, Sep 12, 2018 at 02:50:08AM +, YueHaibing wrote: > Use PTR_ERR_OR_ZERO rather than if(IS_ERR(...)) + PTR_ERR > > Signed-off-by: YueHaibing > --- > drivers/staging/mt7621-pci/pci-mt7621.c | 5 + > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/drivers/staging/mt7621-pci/pci-mt7621.c > b/drivers/staging/mt7621-pci/pci-mt7621.c > index ba1f117..d2cb910 100644 > --- a/drivers/staging/mt7621-pci/pci-mt7621.c > +++ b/drivers/staging/mt7621-pci/pci-mt7621.c > @@ -396,10 +396,7 @@ static int mt7621_pcie_parse_dt(struct mt7621_pcie *pcie) > } > > pcie->base = devm_ioremap_resource(dev, ); > - if (IS_ERR(pcie->base)) > - return PTR_ERR(pcie->base); > - > - return 0; > + return PTR_ERR_OR_ZERO(pcie->base); > } This patch looks good but the 'mt7621_pcie_parse_dt' function is not completed at all. There is a lot of missing for each pci node to be parsed yet and some patch series which are doing this have not been tested yet so those patches are not included. Please see: http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2018-September/125937.html Best regards, Sergio Paracuellos > > static int mt7621_pcie_request_resources(struct mt7621_pcie *pcie, > > >
Re: [PATCH 06/11] compat_ioctl: remove /dev/random commands
On Tue, 11 Sep 2018 22:26:54 +0200 Arnd Bergmann wrote: > On Sun, Sep 9, 2018 at 6:12 AM Al Viro wrote: > > > > On Sat, Sep 08, 2018 at 04:28:12PM +0200, Arnd Bergmann wrote: > > > These are all handled by the random driver, so instead of listing > > > each ioctl, we can just use the same function to deal with both > > > native and compat commands. > > > > Umm... I don't think it's right - > > > > > .unlocked_ioctl = random_ioctl, > > > + .compat_ioctl = random_ioctl, > > > > > > ->compat_ioctl() gets called in > > error = f.file->f_op->compat_ioctl(f.file, cmd, > > arg); > > so you do *NOT* get compat_ptr() for those - they have to do it on their > > own. It's not hard to provide a proper compat_ioctl() instance for that > > one, but this is not it. What you need in drivers/char/random.c part of > > that one is something like > > Looping in some s390 folks. > > As you suggested in another reply, I had a look at what other drivers > do the same thing and have only pointer arguments. I created a > patch to move them all over to using a new helper function that > adds the compat_ptr(), and arrived at > > drivers/android/binder.c| 2 +- > drivers/crypto/qat/qat_common/adf_ctl_drv.c | 2 +- > drivers/dma-buf/dma-buf.c | 4 +--- > drivers/dma-buf/sw_sync.c | 2 +- > drivers/dma-buf/sync_file.c | 2 +- > drivers/gpu/drm/amd/amdkfd/kfd_chardev.c| 2 +- > drivers/hid/hidraw.c| 4 +--- > drivers/iio/industrialio-core.c | 2 +- > drivers/infiniband/core/uverbs_main.c | 4 ++-- > drivers/media/rc/lirc_dev.c | 4 +--- > drivers/mfd/cros_ec_dev.c | 4 +--- > drivers/misc/vmw_vmci/vmci_host.c | 2 +- > drivers/nvdimm/bus.c| 4 ++-- > drivers/nvme/host/core.c| 6 +++--- > drivers/pci/switch/switchtec.c | 2 +- > drivers/platform/x86/wmi.c | 2 +- > drivers/rpmsg/rpmsg_char.c | 4 ++-- > drivers/s390/char/sclp_ctl.c| 8 ++-- > drivers/s390/char/vmcp.c| 2 ++ > drivers/s390/cio/chsc_sch.c | 8 ++-- > drivers/sbus/char/display7seg.c | 2 +- > drivers/sbus/char/envctrl.c | 4 +--- > drivers/scsi/3w-.c | 4 +--- > drivers/scsi/cxlflash/main.c| 2 +- > drivers/scsi/esas2r/esas2r_main.c | 2 +- > drivers/scsi/pmcraid.c | 4 +--- > drivers/staging/android/ion/ion.c | 4 +--- > drivers/staging/vme/devices/vme_user.c | 2 +- > drivers/tee/tee_core.c | 2 +- > drivers/usb/class/cdc-wdm.c | 2 +- > drivers/usb/class/usbtmc.c | 4 +--- > drivers/video/fbdev/ps3fb.c | 2 +- > drivers/video/fbdev/sis/sis_main.c | 4 +--- > drivers/virt/fsl_hypervisor.c | 2 +- > fs/btrfs/super.c| 2 +- > fs/ceph/dir.c | 2 +- > fs/ceph/file.c | 2 +- > fs/fuse/dev.c | 2 +- > fs/notify/fanotify/fanotify_user.c | 2 +- > fs/userfaultfd.c| 2 +- > net/rfkill/core.c | 2 +- > 41 files changed, 48 insertions(+), 76 deletions(-) > > Out of those, there are only a few that may get used on s390, > in particular at most infiniband/uverbs, nvme, nvdimm, > btrfs, ceph, fuse, fanotify and userfaultfd. > [Note: there are three s390 drivers in the list, which use > a different method: they check in_compat_syscall() from > a shared handler to decide whether to do compat_ptr(). Using in_compat_syscall() seems to be a good solution, no? > According to my memory from when I last worked on this, > the compat_ptr() is mainly a safeguard for legacy binaries > that got created with ancient C compilers (or compilers for > something other than C) and might leave the high bit set > in a pointer, but modern C compilers (gcc-3+) won't ever > do that. And compat_ptr clears the upper 32-bit of the register. If the register is loaded to e.g. "lr" or "l" there will be junk in the 4 upper bytes. > You are probably right about /dev/random, which could be > used in lots of weird code, but I wonder to what degree we > need to worry about it for the rest. -- blue skies, Martin. "Reality continues to ruin my life." - Calvin.
Re: [PATCH 06/11] compat_ioctl: remove /dev/random commands
On Tue, 11 Sep 2018 22:26:54 +0200 Arnd Bergmann wrote: > On Sun, Sep 9, 2018 at 6:12 AM Al Viro wrote: > > > > On Sat, Sep 08, 2018 at 04:28:12PM +0200, Arnd Bergmann wrote: > > > These are all handled by the random driver, so instead of listing > > > each ioctl, we can just use the same function to deal with both > > > native and compat commands. > > > > Umm... I don't think it's right - > > > > > .unlocked_ioctl = random_ioctl, > > > + .compat_ioctl = random_ioctl, > > > > > > ->compat_ioctl() gets called in > > error = f.file->f_op->compat_ioctl(f.file, cmd, > > arg); > > so you do *NOT* get compat_ptr() for those - they have to do it on their > > own. It's not hard to provide a proper compat_ioctl() instance for that > > one, but this is not it. What you need in drivers/char/random.c part of > > that one is something like > > Looping in some s390 folks. > > As you suggested in another reply, I had a look at what other drivers > do the same thing and have only pointer arguments. I created a > patch to move them all over to using a new helper function that > adds the compat_ptr(), and arrived at > > drivers/android/binder.c| 2 +- > drivers/crypto/qat/qat_common/adf_ctl_drv.c | 2 +- > drivers/dma-buf/dma-buf.c | 4 +--- > drivers/dma-buf/sw_sync.c | 2 +- > drivers/dma-buf/sync_file.c | 2 +- > drivers/gpu/drm/amd/amdkfd/kfd_chardev.c| 2 +- > drivers/hid/hidraw.c| 4 +--- > drivers/iio/industrialio-core.c | 2 +- > drivers/infiniband/core/uverbs_main.c | 4 ++-- > drivers/media/rc/lirc_dev.c | 4 +--- > drivers/mfd/cros_ec_dev.c | 4 +--- > drivers/misc/vmw_vmci/vmci_host.c | 2 +- > drivers/nvdimm/bus.c| 4 ++-- > drivers/nvme/host/core.c| 6 +++--- > drivers/pci/switch/switchtec.c | 2 +- > drivers/platform/x86/wmi.c | 2 +- > drivers/rpmsg/rpmsg_char.c | 4 ++-- > drivers/s390/char/sclp_ctl.c| 8 ++-- > drivers/s390/char/vmcp.c| 2 ++ > drivers/s390/cio/chsc_sch.c | 8 ++-- > drivers/sbus/char/display7seg.c | 2 +- > drivers/sbus/char/envctrl.c | 4 +--- > drivers/scsi/3w-.c | 4 +--- > drivers/scsi/cxlflash/main.c| 2 +- > drivers/scsi/esas2r/esas2r_main.c | 2 +- > drivers/scsi/pmcraid.c | 4 +--- > drivers/staging/android/ion/ion.c | 4 +--- > drivers/staging/vme/devices/vme_user.c | 2 +- > drivers/tee/tee_core.c | 2 +- > drivers/usb/class/cdc-wdm.c | 2 +- > drivers/usb/class/usbtmc.c | 4 +--- > drivers/video/fbdev/ps3fb.c | 2 +- > drivers/video/fbdev/sis/sis_main.c | 4 +--- > drivers/virt/fsl_hypervisor.c | 2 +- > fs/btrfs/super.c| 2 +- > fs/ceph/dir.c | 2 +- > fs/ceph/file.c | 2 +- > fs/fuse/dev.c | 2 +- > fs/notify/fanotify/fanotify_user.c | 2 +- > fs/userfaultfd.c| 2 +- > net/rfkill/core.c | 2 +- > 41 files changed, 48 insertions(+), 76 deletions(-) > > Out of those, there are only a few that may get used on s390, > in particular at most infiniband/uverbs, nvme, nvdimm, > btrfs, ceph, fuse, fanotify and userfaultfd. > [Note: there are three s390 drivers in the list, which use > a different method: they check in_compat_syscall() from > a shared handler to decide whether to do compat_ptr(). Using in_compat_syscall() seems to be a good solution, no? > According to my memory from when I last worked on this, > the compat_ptr() is mainly a safeguard for legacy binaries > that got created with ancient C compilers (or compilers for > something other than C) and might leave the high bit set > in a pointer, but modern C compilers (gcc-3+) won't ever > do that. And compat_ptr clears the upper 32-bit of the register. If the register is loaded to e.g. "lr" or "l" there will be junk in the 4 upper bytes. > You are probably right about /dev/random, which could be > used in lots of weird code, but I wonder to what degree we > need to worry about it for the rest. -- blue skies, Martin. "Reality continues to ruin my life." - Calvin.
Re: [PATCH v2 2/2] dmaengine: uniphier-mdmac: add UniPhier MIO DMAC driver
2018-09-12 13:35 GMT+09:00 Vinod :
> On 12-09-18, 12:01, Masahiro Yamada wrote:
>> Hi Vinod,
>>
>>
>> 2018-09-11 16:00 GMT+09:00 Vinod :
>> > On 24-08-18, 10:41, Masahiro Yamada wrote:
>> >
>> >> +/* mc->vc.lock must be held by caller */
>> >> +static u32 __uniphier_mdmac_get_residue(struct uniphier_mdmac_desc *md)
>> >> +{
>> >> + u32 residue = 0;
>> >> + int i;
>> >> +
>> >> + for (i = md->sg_cur; i < md->sg_len; i++)
>> >> + residue += sg_dma_len(>sgl[i]);
>> >
>> > so if the descriptor is submitted to hardware, we return the descriptor
>> > length, which is not correct.
>> >
>> > Two cases are required to be handled:
>> > 1. Descriptor is in queue (IMO above logic is fine for that, but it can
>> > be calculated at descriptor submit and looked up here)
>>
>> Where do you want it to be calculated?
>
> where is it calculated now?
Please see __uniphier_mdmac_handle().
It gets the address and size by sg_dma_address(), sg_dma_len()
just before setting them to the hardware registers.
sg = >sgl[md->sg_cur];
if (md->dir == DMA_MEM_TO_DEV) {
src_mode = UNIPHIER_MDMAC_CH_MODE__ADDR_INC;
src_addr = sg_dma_address(sg);
dest_mode = UNIPHIER_MDMAC_CH_MODE__ADDR_FIXED;
dest_addr = 0;
} else {
src_mode = UNIPHIER_MDMAC_CH_MODE__ADDR_FIXED;
src_addr = 0;
dest_mode = UNIPHIER_MDMAC_CH_MODE__ADDR_INC;
dest_addr = sg_dma_address(sg);
}
>> This hardware provides only simple registers (address and size)
>> for one-shot transfer instead of descriptors.
>>
>> So, I used sgl as-is because I did not see a good reason
>> to transform sgl to another data structure.
>
>
>> > this seems missing stuff. Where do you do register calculation for the
>> > descriptor and where is slave_config here, how do you know where to
>> > send/receive data form/to (peripheral)
>>
>>
>> This dmac is really simple, and un-flexible.
>>
>> The peripheral address to send/receive data from/to is hard-weird.
>> cfg->{src_addr,dst_addr} is not configurable.
>>
>> Look at __uniphier_mdmac_handle().
>> 'dest_addr' and 'src_addr' must be set to 0 for the peripheral.
>
> Fair enough, what about other values like addr_width and maxburst?
None of them is configurable.
--
Best Regards
Masahiro Yamada
Re: [PATCH v2 2/2] dmaengine: uniphier-mdmac: add UniPhier MIO DMAC driver
2018-09-12 13:35 GMT+09:00 Vinod :
> On 12-09-18, 12:01, Masahiro Yamada wrote:
>> Hi Vinod,
>>
>>
>> 2018-09-11 16:00 GMT+09:00 Vinod :
>> > On 24-08-18, 10:41, Masahiro Yamada wrote:
>> >
>> >> +/* mc->vc.lock must be held by caller */
>> >> +static u32 __uniphier_mdmac_get_residue(struct uniphier_mdmac_desc *md)
>> >> +{
>> >> + u32 residue = 0;
>> >> + int i;
>> >> +
>> >> + for (i = md->sg_cur; i < md->sg_len; i++)
>> >> + residue += sg_dma_len(>sgl[i]);
>> >
>> > so if the descriptor is submitted to hardware, we return the descriptor
>> > length, which is not correct.
>> >
>> > Two cases are required to be handled:
>> > 1. Descriptor is in queue (IMO above logic is fine for that, but it can
>> > be calculated at descriptor submit and looked up here)
>>
>> Where do you want it to be calculated?
>
> where is it calculated now?
Please see __uniphier_mdmac_handle().
It gets the address and size by sg_dma_address(), sg_dma_len()
just before setting them to the hardware registers.
sg = >sgl[md->sg_cur];
if (md->dir == DMA_MEM_TO_DEV) {
src_mode = UNIPHIER_MDMAC_CH_MODE__ADDR_INC;
src_addr = sg_dma_address(sg);
dest_mode = UNIPHIER_MDMAC_CH_MODE__ADDR_FIXED;
dest_addr = 0;
} else {
src_mode = UNIPHIER_MDMAC_CH_MODE__ADDR_FIXED;
src_addr = 0;
dest_mode = UNIPHIER_MDMAC_CH_MODE__ADDR_INC;
dest_addr = sg_dma_address(sg);
}
>> This hardware provides only simple registers (address and size)
>> for one-shot transfer instead of descriptors.
>>
>> So, I used sgl as-is because I did not see a good reason
>> to transform sgl to another data structure.
>
>
>> > this seems missing stuff. Where do you do register calculation for the
>> > descriptor and where is slave_config here, how do you know where to
>> > send/receive data form/to (peripheral)
>>
>>
>> This dmac is really simple, and un-flexible.
>>
>> The peripheral address to send/receive data from/to is hard-weird.
>> cfg->{src_addr,dst_addr} is not configurable.
>>
>> Look at __uniphier_mdmac_handle().
>> 'dest_addr' and 'src_addr' must be set to 0 for the peripheral.
>
> Fair enough, what about other values like addr_width and maxburst?
None of them is configurable.
--
Best Regards
Masahiro Yamada
[LKP] [rcu] 02a5c550b2: BUG:kernel_reboot-without-warning_in_test_stage
FYI, we noticed the following commit (built with gcc-7):
commit: 02a5c550b2738f2bfea8e1e00aa75944d71c9e18 ("rcu: Abstract extended
quiescent state determination")
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git master
in testcase: perf_event_tests
with following parameters:
paranoid: disallow_kernel_profiling
test-description: The Perf Event Testsuite.
test-url: https://github.com/deater/perf_event_tests
on test machine: qemu-system-x86_64 -enable-kvm -cpu host -smp 2 -m 2G
caused below changes (please refer to attached dmesg/kmsg for entire
log/backtrace):
+-+++
| | 2625d469ba | 02a5c550b2 |
+-+++
| boot_successes | 18 | 0 |
| boot_failures | 0 | 17 |
| BUG:kernel_reboot-without-warning_in_test_stage | 0 | 17 |
+-+++
[ 21.715217]
[ 22.950524] perf: interrupt took too long (5334 > 5235), lowering
kernel.perf_event_max_sample_rate to 37250
[ 22.956921] perf: interrupt took too long (6735 > 6667), lowering
kernel.perf_event_max_sample_rate to 29500
[ 22.970150] perf: interrupt took too long (8494 > 8418), lowering
kernel.perf_event_max_sample_rate to 23500
[ 22.976586] perf: interrupt took too long (10754 > 10617), lowering
kernel.perf_event_max_sample_rate to 18500
BUG: kernel reboot-without-warning in test stage
Elapsed time: 30
#!/bin/bash
To reproduce:
git clone https://github.com/intel/lkp-tests.git
cd lkp-tests
bin/lkp qemu -k job-script # job-script is attached in this
email
Thanks,
lkp
#
# Automatically generated file; DO NOT EDIT.
# Linux/x86_64 4.10.0-rc3 Kernel Configuration
#
CONFIG_64BIT=y
CONFIG_X86_64=y
CONFIG_X86=y
CONFIG_INSTRUCTION_DECODER=y
CONFIG_OUTPUT_FORMAT="elf64-x86-64"
CONFIG_ARCH_DEFCONFIG="arch/x86/configs/x86_64_defconfig"
CONFIG_LOCKDEP_SUPPORT=y
CONFIG_STACKTRACE_SUPPORT=y
CONFIG_MMU=y
CONFIG_ARCH_MMAP_RND_BITS_MIN=28
CONFIG_ARCH_MMAP_RND_BITS_MAX=32
CONFIG_ARCH_MMAP_RND_COMPAT_BITS_MIN=8
CONFIG_ARCH_MMAP_RND_COMPAT_BITS_MAX=16
CONFIG_NEED_DMA_MAP_STATE=y
CONFIG_NEED_SG_DMA_LENGTH=y
CONFIG_GENERIC_ISA_DMA=y
CONFIG_GENERIC_BUG=y
CONFIG_GENERIC_BUG_RELATIVE_POINTERS=y
CONFIG_GENERIC_HWEIGHT=y
CONFIG_ARCH_MAY_HAVE_PC_FDC=y
CONFIG_RWSEM_XCHGADD_ALGORITHM=y
CONFIG_GENERIC_CALIBRATE_DELAY=y
CONFIG_ARCH_HAS_CPU_RELAX=y
CONFIG_ARCH_HAS_CACHE_LINE_SIZE=y
CONFIG_HAVE_SETUP_PER_CPU_AREA=y
CONFIG_NEED_PER_CPU_EMBED_FIRST_CHUNK=y
CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK=y
CONFIG_ARCH_HIBERNATION_POSSIBLE=y
CONFIG_ARCH_SUSPEND_POSSIBLE=y
CONFIG_ARCH_WANT_HUGE_PMD_SHARE=y
CONFIG_ARCH_WANT_GENERAL_HUGETLB=y
CONFIG_ZONE_DMA32=y
CONFIG_AUDIT_ARCH=y
CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING=y
CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC=y
CONFIG_X86_64_SMP=y
CONFIG_ARCH_SUPPORTS_UPROBES=y
CONFIG_FIX_EARLYCON_MEM=y
CONFIG_DEBUG_RODATA=y
CONFIG_PGTABLE_LEVELS=4
CONFIG_DEFCONFIG_LIST="/lib/modules/$UNAME_RELEASE/.config"
CONFIG_IRQ_WORK=y
CONFIG_BUILDTIME_EXTABLE_SORT=y
CONFIG_THREAD_INFO_IN_TASK=y
#
# General setup
#
CONFIG_INIT_ENV_ARG_LIMIT=32
CONFIG_CROSS_COMPILE=""
# CONFIG_COMPILE_TEST is not set
CONFIG_LOCALVERSION=""
CONFIG_LOCALVERSION_AUTO=y
CONFIG_HAVE_KERNEL_GZIP=y
CONFIG_HAVE_KERNEL_BZIP2=y
CONFIG_HAVE_KERNEL_LZMA=y
CONFIG_HAVE_KERNEL_XZ=y
CONFIG_HAVE_KERNEL_LZO=y
CONFIG_HAVE_KERNEL_LZ4=y
CONFIG_KERNEL_GZIP=y
# CONFIG_KERNEL_BZIP2 is not set
# CONFIG_KERNEL_LZMA is not set
# CONFIG_KERNEL_XZ is not set
# CONFIG_KERNEL_LZO is not set
# CONFIG_KERNEL_LZ4 is not set
CONFIG_DEFAULT_HOSTNAME="(none)"
CONFIG_SWAP=y
CONFIG_SYSVIPC=y
CONFIG_SYSVIPC_SYSCTL=y
CONFIG_POSIX_MQUEUE=y
CONFIG_POSIX_MQUEUE_SYSCTL=y
CONFIG_CROSS_MEMORY_ATTACH=y
CONFIG_FHANDLE=y
CONFIG_USELIB=y
# CONFIG_AUDIT is not set
CONFIG_HAVE_ARCH_AUDITSYSCALL=y
#
# IRQ subsystem
#
CONFIG_GENERIC_IRQ_PROBE=y
CONFIG_GENERIC_IRQ_SHOW=y
CONFIG_GENERIC_PENDING_IRQ=y
CONFIG_IRQ_DOMAIN=y
CONFIG_IRQ_DOMAIN_HIERARCHY=y
CONFIG_GENERIC_MSI_IRQ=y
CONFIG_GENERIC_MSI_IRQ_DOMAIN=y
# CONFIG_IRQ_DOMAIN_DEBUG is not set
CONFIG_IRQ_FORCED_THREADING=y
CONFIG_SPARSE_IRQ=y
CONFIG_CLOCKSOURCE_WATCHDOG=y
CONFIG_ARCH_CLOCKSOURCE_DATA=y
CONFIG_CLOCKSOURCE_VALIDATE_LAST_CYCLE=y
CONFIG_GENERIC_TIME_VSYSCALL=y
CONFIG_GENERIC_CLOCKEVENTS=y
CONFIG_GENERIC_CLOCKEVENTS_BROADCAST=y
CONFIG_GENERIC_CLOCKEVENTS_MIN_ADJUST=y
CONFIG_GENERIC_CMOS_UPDATE=y
#
# Timers subsystem
#
CONFIG_TICK_ONESHOT=y
CONFIG_NO_HZ_COMMON=y
# CONFIG_HZ_PERIODIC is not set
CONFIG_NO_HZ_IDLE=y
# CONFIG_NO_HZ_FULL is not set
CONFIG_NO_HZ=y
CONFIG_HIGH_RES_TIMERS=y
#
# CPU/Task time and stats accounting
#
CONFIG_TICK_CPU_ACCOUNTING=y
# CONFIG_VIRT_CPU_ACCOUNTING_GEN is not set
# CONFIG_IRQ_TIME_ACCOUNTING is not set
CONFIG_BSD_PROCESS_ACCT=y
[LKP] [rcu] 02a5c550b2: BUG:kernel_reboot-without-warning_in_test_stage
FYI, we noticed the following commit (built with gcc-7):
commit: 02a5c550b2738f2bfea8e1e00aa75944d71c9e18 ("rcu: Abstract extended
quiescent state determination")
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git master
in testcase: perf_event_tests
with following parameters:
paranoid: disallow_kernel_profiling
test-description: The Perf Event Testsuite.
test-url: https://github.com/deater/perf_event_tests
on test machine: qemu-system-x86_64 -enable-kvm -cpu host -smp 2 -m 2G
caused below changes (please refer to attached dmesg/kmsg for entire
log/backtrace):
+-+++
| | 2625d469ba | 02a5c550b2 |
+-+++
| boot_successes | 18 | 0 |
| boot_failures | 0 | 17 |
| BUG:kernel_reboot-without-warning_in_test_stage | 0 | 17 |
+-+++
[ 21.715217]
[ 22.950524] perf: interrupt took too long (5334 > 5235), lowering
kernel.perf_event_max_sample_rate to 37250
[ 22.956921] perf: interrupt took too long (6735 > 6667), lowering
kernel.perf_event_max_sample_rate to 29500
[ 22.970150] perf: interrupt took too long (8494 > 8418), lowering
kernel.perf_event_max_sample_rate to 23500
[ 22.976586] perf: interrupt took too long (10754 > 10617), lowering
kernel.perf_event_max_sample_rate to 18500
BUG: kernel reboot-without-warning in test stage
Elapsed time: 30
#!/bin/bash
To reproduce:
git clone https://github.com/intel/lkp-tests.git
cd lkp-tests
bin/lkp qemu -k job-script # job-script is attached in this
email
Thanks,
lkp
#
# Automatically generated file; DO NOT EDIT.
# Linux/x86_64 4.10.0-rc3 Kernel Configuration
#
CONFIG_64BIT=y
CONFIG_X86_64=y
CONFIG_X86=y
CONFIG_INSTRUCTION_DECODER=y
CONFIG_OUTPUT_FORMAT="elf64-x86-64"
CONFIG_ARCH_DEFCONFIG="arch/x86/configs/x86_64_defconfig"
CONFIG_LOCKDEP_SUPPORT=y
CONFIG_STACKTRACE_SUPPORT=y
CONFIG_MMU=y
CONFIG_ARCH_MMAP_RND_BITS_MIN=28
CONFIG_ARCH_MMAP_RND_BITS_MAX=32
CONFIG_ARCH_MMAP_RND_COMPAT_BITS_MIN=8
CONFIG_ARCH_MMAP_RND_COMPAT_BITS_MAX=16
CONFIG_NEED_DMA_MAP_STATE=y
CONFIG_NEED_SG_DMA_LENGTH=y
CONFIG_GENERIC_ISA_DMA=y
CONFIG_GENERIC_BUG=y
CONFIG_GENERIC_BUG_RELATIVE_POINTERS=y
CONFIG_GENERIC_HWEIGHT=y
CONFIG_ARCH_MAY_HAVE_PC_FDC=y
CONFIG_RWSEM_XCHGADD_ALGORITHM=y
CONFIG_GENERIC_CALIBRATE_DELAY=y
CONFIG_ARCH_HAS_CPU_RELAX=y
CONFIG_ARCH_HAS_CACHE_LINE_SIZE=y
CONFIG_HAVE_SETUP_PER_CPU_AREA=y
CONFIG_NEED_PER_CPU_EMBED_FIRST_CHUNK=y
CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK=y
CONFIG_ARCH_HIBERNATION_POSSIBLE=y
CONFIG_ARCH_SUSPEND_POSSIBLE=y
CONFIG_ARCH_WANT_HUGE_PMD_SHARE=y
CONFIG_ARCH_WANT_GENERAL_HUGETLB=y
CONFIG_ZONE_DMA32=y
CONFIG_AUDIT_ARCH=y
CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING=y
CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC=y
CONFIG_X86_64_SMP=y
CONFIG_ARCH_SUPPORTS_UPROBES=y
CONFIG_FIX_EARLYCON_MEM=y
CONFIG_DEBUG_RODATA=y
CONFIG_PGTABLE_LEVELS=4
CONFIG_DEFCONFIG_LIST="/lib/modules/$UNAME_RELEASE/.config"
CONFIG_IRQ_WORK=y
CONFIG_BUILDTIME_EXTABLE_SORT=y
CONFIG_THREAD_INFO_IN_TASK=y
#
# General setup
#
CONFIG_INIT_ENV_ARG_LIMIT=32
CONFIG_CROSS_COMPILE=""
# CONFIG_COMPILE_TEST is not set
CONFIG_LOCALVERSION=""
CONFIG_LOCALVERSION_AUTO=y
CONFIG_HAVE_KERNEL_GZIP=y
CONFIG_HAVE_KERNEL_BZIP2=y
CONFIG_HAVE_KERNEL_LZMA=y
CONFIG_HAVE_KERNEL_XZ=y
CONFIG_HAVE_KERNEL_LZO=y
CONFIG_HAVE_KERNEL_LZ4=y
CONFIG_KERNEL_GZIP=y
# CONFIG_KERNEL_BZIP2 is not set
# CONFIG_KERNEL_LZMA is not set
# CONFIG_KERNEL_XZ is not set
# CONFIG_KERNEL_LZO is not set
# CONFIG_KERNEL_LZ4 is not set
CONFIG_DEFAULT_HOSTNAME="(none)"
CONFIG_SWAP=y
CONFIG_SYSVIPC=y
CONFIG_SYSVIPC_SYSCTL=y
CONFIG_POSIX_MQUEUE=y
CONFIG_POSIX_MQUEUE_SYSCTL=y
CONFIG_CROSS_MEMORY_ATTACH=y
CONFIG_FHANDLE=y
CONFIG_USELIB=y
# CONFIG_AUDIT is not set
CONFIG_HAVE_ARCH_AUDITSYSCALL=y
#
# IRQ subsystem
#
CONFIG_GENERIC_IRQ_PROBE=y
CONFIG_GENERIC_IRQ_SHOW=y
CONFIG_GENERIC_PENDING_IRQ=y
CONFIG_IRQ_DOMAIN=y
CONFIG_IRQ_DOMAIN_HIERARCHY=y
CONFIG_GENERIC_MSI_IRQ=y
CONFIG_GENERIC_MSI_IRQ_DOMAIN=y
# CONFIG_IRQ_DOMAIN_DEBUG is not set
CONFIG_IRQ_FORCED_THREADING=y
CONFIG_SPARSE_IRQ=y
CONFIG_CLOCKSOURCE_WATCHDOG=y
CONFIG_ARCH_CLOCKSOURCE_DATA=y
CONFIG_CLOCKSOURCE_VALIDATE_LAST_CYCLE=y
CONFIG_GENERIC_TIME_VSYSCALL=y
CONFIG_GENERIC_CLOCKEVENTS=y
CONFIG_GENERIC_CLOCKEVENTS_BROADCAST=y
CONFIG_GENERIC_CLOCKEVENTS_MIN_ADJUST=y
CONFIG_GENERIC_CMOS_UPDATE=y
#
# Timers subsystem
#
CONFIG_TICK_ONESHOT=y
CONFIG_NO_HZ_COMMON=y
# CONFIG_HZ_PERIODIC is not set
CONFIG_NO_HZ_IDLE=y
# CONFIG_NO_HZ_FULL is not set
CONFIG_NO_HZ=y
CONFIG_HIGH_RES_TIMERS=y
#
# CPU/Task time and stats accounting
#
CONFIG_TICK_CPU_ACCOUNTING=y
# CONFIG_VIRT_CPU_ACCOUNTING_GEN is not set
# CONFIG_IRQ_TIME_ACCOUNTING is not set
CONFIG_BSD_PROCESS_ACCT=y
Re: [PATCH] firmware: vpd: fix spelling mistake "partion" -> "partition"
On Tue, Sep 11, 2018 at 09:58:48PM -0700, Guenter Roeck wrote: > On 09/11/2018 09:58 AM, Colin King wrote: > > From: Colin Ian King > > > > Trivial fix to spelling mistake in comment > > > > Signed-off-by: Colin Ian King > > Reviewed-by: Guenter Roeck > > Interesting - drivers/firmware/google/ does not have a maintainer. > Greg - is it correct to assume that you are the de-facto maintainer ? Yeah, I am :( I'll queue this up, thanks. greg k-h
Re: [PATCH] firmware: vpd: fix spelling mistake "partion" -> "partition"
On Tue, Sep 11, 2018 at 09:58:48PM -0700, Guenter Roeck wrote: > On 09/11/2018 09:58 AM, Colin King wrote: > > From: Colin Ian King > > > > Trivial fix to spelling mistake in comment > > > > Signed-off-by: Colin Ian King > > Reviewed-by: Guenter Roeck > > Interesting - drivers/firmware/google/ does not have a maintainer. > Greg - is it correct to assume that you are the de-facto maintainer ? Yeah, I am :( I'll queue this up, thanks. greg k-h
[PATCH v2 0/1] gpio: mvebu: Add support for multiple PWM lines
Hi everyone, Helios4, an Armada 388 based NAS SBC, provides 2 (4-pins) fan connectors. The PWM pins on both connector are connected to GPIO on bank 1. Current gpio-mvebu does not allow more than one PWM on the same bank. Aditya --- Changes v1->v2: * Merge/squash "[Patch 2/2] gpio: mvebu: Allow to use non-default PWM counter" * Allow only two PWMs as suggested by Andrew Lunn and Richard Genoud --- Aditya Prayoga (1): gpio: mvebu: Add support for multiple PWM lines per GPIO chip drivers/gpio/gpio-mvebu.c | 73 ++- 1 file changed, 60 insertions(+), 13 deletions(-) -- 2.7.4
[PATCH v2 1/1] gpio: mvebu: Add support for multiple PWM lines per GPIO chip
Allow more than 1 PWM request (eg. PWM fan) on the same GPIO chip. If
the other PWM counter is unused, allocate it to next PWM request.
The priority would be:
1. Default counter assigned to the bank
2. Unused counter that is assigned to other bank
Since there are only two PWM counters, only two PWMs supported.
Signed-off-by: Aditya Prayoga
---
drivers/gpio/gpio-mvebu.c | 73 ++-
1 file changed, 60 insertions(+), 13 deletions(-)
diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
index 6e02148..2d46b87 100644
--- a/drivers/gpio/gpio-mvebu.c
+++ b/drivers/gpio/gpio-mvebu.c
@@ -92,9 +92,16 @@
#define MVEBU_MAX_GPIO_PER_BANK32
+enum mvebu_pwm_counter {
+ MVEBU_PWM_CTRL_SET_A = 0,
+ MVEBU_PWM_CTRL_SET_B,
+ MVEBU_PWM_CTRL_MAX
+};
+
struct mvebu_pwm {
void __iomem*membase;
unsigned longclk_rate;
+ enum mvebu_pwm_counter id;
struct gpio_desc*gpiod;
struct pwm_chip chip;
spinlock_t lock;
@@ -128,6 +135,8 @@ struct mvebu_gpio_chip {
u32level_mask_regs[4];
};
+static struct mvebu_pwm*mvebu_pwm_list[MVEBU_PWM_CTRL_MAX];
+
/*
* Functions returning addresses of individual registers for a given
* GPIO controller.
@@ -594,34 +603,59 @@ static struct mvebu_pwm *to_mvebu_pwm(struct pwm_chip
*chip)
return container_of(chip, struct mvebu_pwm, chip);
}
+static struct mvebu_pwm *mvebu_pwm_get_avail_counter(void)
+{
+ enum mvebu_pwm_counter i;
+
+ for (i = MVEBU_PWM_CTRL_SET_A; i < MVEBU_PWM_CTRL_MAX; i++) {
+ if (!mvebu_pwm_list[i]->gpiod)
+ return mvebu_pwm_list[i];
+ }
+ return NULL;
+}
+
static int mvebu_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
{
struct mvebu_pwm *mvpwm = to_mvebu_pwm(chip);
struct mvebu_gpio_chip *mvchip = mvpwm->mvchip;
struct gpio_desc *desc;
+ struct mvebu_pwm *counter;
unsigned long flags;
int ret = 0;
spin_lock_irqsave(>lock, flags);
- if (mvpwm->gpiod) {
- ret = -EBUSY;
- } else {
- desc = gpiochip_request_own_desc(>chip,
-pwm->hwpwm, "mvebu-pwm");
- if (IS_ERR(desc)) {
- ret = PTR_ERR(desc);
+ counter = mvpwm;
+ if (counter->gpiod) {
+ counter = mvebu_pwm_get_avail_counter();
+ if (!counter) {
+ ret = -EBUSY;
goto out;
}
- ret = gpiod_direction_output(desc, 0);
- if (ret) {
- gpiochip_free_own_desc(desc);
- goto out;
- }
+ pwm->chip_data = counter;
+ }
- mvpwm->gpiod = desc;
+ desc = gpiochip_request_own_desc(>chip,
+pwm->hwpwm, "mvebu-pwm");
+ if (IS_ERR(desc)) {
+ ret = PTR_ERR(desc);
+ goto out;
}
+
+ ret = gpiod_direction_output(desc, 0);
+ if (ret) {
+ gpiochip_free_own_desc(desc);
+ goto out;
+ }
+
+ regmap_update_bits(mvchip->regs, GPIO_BLINK_CNT_SELECT_OFF +
+ mvchip->offset, BIT(pwm->hwpwm),
+ counter->id ? BIT(pwm->hwpwm) : 0);
+ regmap_read(mvchip->regs, GPIO_BLINK_CNT_SELECT_OFF +
+ mvchip->offset, >blink_select);
+
+ counter->gpiod = desc;
out:
spin_unlock_irqrestore(>lock, flags);
return ret;
@@ -632,6 +666,11 @@ static void mvebu_pwm_free(struct pwm_chip *chip, struct
pwm_device *pwm)
struct mvebu_pwm *mvpwm = to_mvebu_pwm(chip);
unsigned long flags;
+ if (pwm->chip_data) {
+ mvpwm = (struct mvebu_pwm *) pwm->chip_data;
+ pwm->chip_data = NULL;
+ }
+
spin_lock_irqsave(>lock, flags);
gpiochip_free_own_desc(mvpwm->gpiod);
mvpwm->gpiod = NULL;
@@ -648,6 +687,9 @@ static void mvebu_pwm_get_state(struct pwm_chip *chip,
unsigned long flags;
u32 u;
+ if (pwm->chip_data)
+ mvpwm = (struct mvebu_pwm *) pwm->chip_data;
+
spin_lock_irqsave(>lock, flags);
val = (unsigned long long)
@@ -695,6 +737,9 @@ static int mvebu_pwm_apply(struct pwm_chip *chip, struct
pwm_device *pwm,
unsigned long flags;
unsigned int on, off;
+ if (pwm->chip_data)
+ mvpwm = (struct mvebu_pwm *) pwm->chip_data;
+
val = (unsigned long long) mvpwm->clk_rate * state->duty_cycle;
do_div(val, NSEC_PER_SEC);
if (val > UINT_MAX)
@@ -804,6 +849,7 @@ static int mvebu_pwm_probe(struct platform_device *pdev,
return -ENOMEM;
mvchip->mvpwm = mvpwm;
[PATCH v2 0/1] gpio: mvebu: Add support for multiple PWM lines
Hi everyone, Helios4, an Armada 388 based NAS SBC, provides 2 (4-pins) fan connectors. The PWM pins on both connector are connected to GPIO on bank 1. Current gpio-mvebu does not allow more than one PWM on the same bank. Aditya --- Changes v1->v2: * Merge/squash "[Patch 2/2] gpio: mvebu: Allow to use non-default PWM counter" * Allow only two PWMs as suggested by Andrew Lunn and Richard Genoud --- Aditya Prayoga (1): gpio: mvebu: Add support for multiple PWM lines per GPIO chip drivers/gpio/gpio-mvebu.c | 73 ++- 1 file changed, 60 insertions(+), 13 deletions(-) -- 2.7.4
[PATCH v2 1/1] gpio: mvebu: Add support for multiple PWM lines per GPIO chip
Allow more than 1 PWM request (eg. PWM fan) on the same GPIO chip. If
the other PWM counter is unused, allocate it to next PWM request.
The priority would be:
1. Default counter assigned to the bank
2. Unused counter that is assigned to other bank
Since there are only two PWM counters, only two PWMs supported.
Signed-off-by: Aditya Prayoga
---
drivers/gpio/gpio-mvebu.c | 73 ++-
1 file changed, 60 insertions(+), 13 deletions(-)
diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
index 6e02148..2d46b87 100644
--- a/drivers/gpio/gpio-mvebu.c
+++ b/drivers/gpio/gpio-mvebu.c
@@ -92,9 +92,16 @@
#define MVEBU_MAX_GPIO_PER_BANK32
+enum mvebu_pwm_counter {
+ MVEBU_PWM_CTRL_SET_A = 0,
+ MVEBU_PWM_CTRL_SET_B,
+ MVEBU_PWM_CTRL_MAX
+};
+
struct mvebu_pwm {
void __iomem*membase;
unsigned longclk_rate;
+ enum mvebu_pwm_counter id;
struct gpio_desc*gpiod;
struct pwm_chip chip;
spinlock_t lock;
@@ -128,6 +135,8 @@ struct mvebu_gpio_chip {
u32level_mask_regs[4];
};
+static struct mvebu_pwm*mvebu_pwm_list[MVEBU_PWM_CTRL_MAX];
+
/*
* Functions returning addresses of individual registers for a given
* GPIO controller.
@@ -594,34 +603,59 @@ static struct mvebu_pwm *to_mvebu_pwm(struct pwm_chip
*chip)
return container_of(chip, struct mvebu_pwm, chip);
}
+static struct mvebu_pwm *mvebu_pwm_get_avail_counter(void)
+{
+ enum mvebu_pwm_counter i;
+
+ for (i = MVEBU_PWM_CTRL_SET_A; i < MVEBU_PWM_CTRL_MAX; i++) {
+ if (!mvebu_pwm_list[i]->gpiod)
+ return mvebu_pwm_list[i];
+ }
+ return NULL;
+}
+
static int mvebu_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
{
struct mvebu_pwm *mvpwm = to_mvebu_pwm(chip);
struct mvebu_gpio_chip *mvchip = mvpwm->mvchip;
struct gpio_desc *desc;
+ struct mvebu_pwm *counter;
unsigned long flags;
int ret = 0;
spin_lock_irqsave(>lock, flags);
- if (mvpwm->gpiod) {
- ret = -EBUSY;
- } else {
- desc = gpiochip_request_own_desc(>chip,
-pwm->hwpwm, "mvebu-pwm");
- if (IS_ERR(desc)) {
- ret = PTR_ERR(desc);
+ counter = mvpwm;
+ if (counter->gpiod) {
+ counter = mvebu_pwm_get_avail_counter();
+ if (!counter) {
+ ret = -EBUSY;
goto out;
}
- ret = gpiod_direction_output(desc, 0);
- if (ret) {
- gpiochip_free_own_desc(desc);
- goto out;
- }
+ pwm->chip_data = counter;
+ }
- mvpwm->gpiod = desc;
+ desc = gpiochip_request_own_desc(>chip,
+pwm->hwpwm, "mvebu-pwm");
+ if (IS_ERR(desc)) {
+ ret = PTR_ERR(desc);
+ goto out;
}
+
+ ret = gpiod_direction_output(desc, 0);
+ if (ret) {
+ gpiochip_free_own_desc(desc);
+ goto out;
+ }
+
+ regmap_update_bits(mvchip->regs, GPIO_BLINK_CNT_SELECT_OFF +
+ mvchip->offset, BIT(pwm->hwpwm),
+ counter->id ? BIT(pwm->hwpwm) : 0);
+ regmap_read(mvchip->regs, GPIO_BLINK_CNT_SELECT_OFF +
+ mvchip->offset, >blink_select);
+
+ counter->gpiod = desc;
out:
spin_unlock_irqrestore(>lock, flags);
return ret;
@@ -632,6 +666,11 @@ static void mvebu_pwm_free(struct pwm_chip *chip, struct
pwm_device *pwm)
struct mvebu_pwm *mvpwm = to_mvebu_pwm(chip);
unsigned long flags;
+ if (pwm->chip_data) {
+ mvpwm = (struct mvebu_pwm *) pwm->chip_data;
+ pwm->chip_data = NULL;
+ }
+
spin_lock_irqsave(>lock, flags);
gpiochip_free_own_desc(mvpwm->gpiod);
mvpwm->gpiod = NULL;
@@ -648,6 +687,9 @@ static void mvebu_pwm_get_state(struct pwm_chip *chip,
unsigned long flags;
u32 u;
+ if (pwm->chip_data)
+ mvpwm = (struct mvebu_pwm *) pwm->chip_data;
+
spin_lock_irqsave(>lock, flags);
val = (unsigned long long)
@@ -695,6 +737,9 @@ static int mvebu_pwm_apply(struct pwm_chip *chip, struct
pwm_device *pwm,
unsigned long flags;
unsigned int on, off;
+ if (pwm->chip_data)
+ mvpwm = (struct mvebu_pwm *) pwm->chip_data;
+
val = (unsigned long long) mvpwm->clk_rate * state->duty_cycle;
do_div(val, NSEC_PER_SEC);
if (val > UINT_MAX)
@@ -804,6 +849,7 @@ static int mvebu_pwm_probe(struct platform_device *pdev,
return -ENOMEM;
mvchip->mvpwm = mvpwm;
Re: [PATCH] xtensa: remove unnecessary KBUILD_SRC ifeq conditional
On Tue, Sep 11, 2018 at 9:25 PM, Masahiro Yamada wrote: > You can always prefix variant/platform header search paths with > $(srctree)/ because $(srctree) is '.' for in-tree building. > > Signed-off-by: Masahiro Yamada > --- > > arch/xtensa/Makefile | 4 > 1 file changed, 4 deletions(-) Thanks, applied to my xtensa tree. -- Thanks. -- Max
Re: [PATCH] xtensa: remove unnecessary KBUILD_SRC ifeq conditional
On Tue, Sep 11, 2018 at 9:25 PM, Masahiro Yamada wrote: > You can always prefix variant/platform header search paths with > $(srctree)/ because $(srctree) is '.' for in-tree building. > > Signed-off-by: Masahiro Yamada > --- > > arch/xtensa/Makefile | 4 > 1 file changed, 4 deletions(-) Thanks, applied to my xtensa tree. -- Thanks. -- Max
Re: [LKP] 0a3856392c [ 10.513760] INFO: trying to register non-static key.
On 09/07/2018 10:19 AM, Matthew Wilcox wrote:
On Fri, Sep 07, 2018 at 09:05:39AM +0800, kernel test robot wrote:
Greetings,
0day kernel testing robot got the below dmesg and the first bad commit is
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
commit 0a3856392cff1542170b5bc37211c9a21fd0c3f6
Author: Matthew Wilcox
AuthorDate: Mon Jun 18 17:23:37 2018 -0400
Commit: Matthew Wilcox
CommitDate: Tue Aug 21 23:54:20 2018 -0400
test_ida: Move ida_check_leaf
Convert to new API and move to kernel space. Take the opportunity to
test the situation a little more thoroughly (ie at different offsets).
Signed-off-by: Matthew Wilcox
Thank you test-bot. Can you check if this patch fixes the problem?
Thanks, It works.
Best Regards,
Rong Chen
diff --git a/lib/test_ida.c b/lib/test_ida.c
index 2d1637d8136b..b06880625961 100644
--- a/lib/test_ida.c
+++ b/lib/test_ida.c
@@ -150,10 +150,10 @@ static void ida_check_conv(struct ida *ida)
IDA_BUG_ON(ida, !ida_is_empty(ida));
}
+static DEFINE_IDA(ida);
+
static int ida_checks(void)
{
- DEFINE_IDA(ida);
-
IDA_BUG_ON(, !ida_is_empty());
ida_check_alloc();
ida_check_destroy();
Re: [LKP] 0a3856392c [ 10.513760] INFO: trying to register non-static key.
On 09/07/2018 10:19 AM, Matthew Wilcox wrote:
On Fri, Sep 07, 2018 at 09:05:39AM +0800, kernel test robot wrote:
Greetings,
0day kernel testing robot got the below dmesg and the first bad commit is
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
commit 0a3856392cff1542170b5bc37211c9a21fd0c3f6
Author: Matthew Wilcox
AuthorDate: Mon Jun 18 17:23:37 2018 -0400
Commit: Matthew Wilcox
CommitDate: Tue Aug 21 23:54:20 2018 -0400
test_ida: Move ida_check_leaf
Convert to new API and move to kernel space. Take the opportunity to
test the situation a little more thoroughly (ie at different offsets).
Signed-off-by: Matthew Wilcox
Thank you test-bot. Can you check if this patch fixes the problem?
Thanks, It works.
Best Regards,
Rong Chen
diff --git a/lib/test_ida.c b/lib/test_ida.c
index 2d1637d8136b..b06880625961 100644
--- a/lib/test_ida.c
+++ b/lib/test_ida.c
@@ -150,10 +150,10 @@ static void ida_check_conv(struct ida *ida)
IDA_BUG_ON(ida, !ida_is_empty(ida));
}
+static DEFINE_IDA(ida);
+
static int ida_checks(void)
{
- DEFINE_IDA(ida);
-
IDA_BUG_ON(, !ida_is_empty());
ida_check_alloc();
ida_check_destroy();
Re: [PATCH] firmware: vpd: fix spelling mistake "partion" -> "partition"
On 09/11/2018 09:58 AM, Colin King wrote:
From: Colin Ian King
Trivial fix to spelling mistake in comment
Signed-off-by: Colin Ian King
Reviewed-by: Guenter Roeck
Interesting - drivers/firmware/google/ does not have a maintainer.
Greg - is it correct to assume that you are the de-facto maintainer ?
Guenter
---
drivers/firmware/google/vpd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/firmware/google/vpd.c b/drivers/firmware/google/vpd.c
index 1aa67bb5d8c0..c0c0b4e4e281 100644
--- a/drivers/firmware/google/vpd.c
+++ b/drivers/firmware/google/vpd.c
@@ -198,7 +198,7 @@ static int vpd_section_init(const char *name, struct
vpd_section *sec,
sec->name = name;
- /* We want to export the raw partion with name ${name}_raw */
+ /* We want to export the raw partition with name ${name}_raw */
sec->raw_name = kasprintf(GFP_KERNEL, "%s_raw", name);
if (!sec->raw_name) {
err = -ENOMEM;
Re: [PATCH] firmware: vpd: fix spelling mistake "partion" -> "partition"
On 09/11/2018 09:58 AM, Colin King wrote:
From: Colin Ian King
Trivial fix to spelling mistake in comment
Signed-off-by: Colin Ian King
Reviewed-by: Guenter Roeck
Interesting - drivers/firmware/google/ does not have a maintainer.
Greg - is it correct to assume that you are the de-facto maintainer ?
Guenter
---
drivers/firmware/google/vpd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/firmware/google/vpd.c b/drivers/firmware/google/vpd.c
index 1aa67bb5d8c0..c0c0b4e4e281 100644
--- a/drivers/firmware/google/vpd.c
+++ b/drivers/firmware/google/vpd.c
@@ -198,7 +198,7 @@ static int vpd_section_init(const char *name, struct
vpd_section *sec,
sec->name = name;
- /* We want to export the raw partion with name ${name}_raw */
+ /* We want to export the raw partition with name ${name}_raw */
sec->raw_name = kasprintf(GFP_KERNEL, "%s_raw", name);
if (!sec->raw_name) {
err = -ENOMEM;
[PATCH] kbuild: prefix Makefile.dtbinst path with $(srctree) unconditionally
$(srctree) always points to the top of the source tree whether KBUILD_SRC is set or not. Signed-off-by: Masahiro Yamada --- scripts/Kbuild.include | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include index ce53639..46cc43e 100644 --- a/scripts/Kbuild.include +++ b/scripts/Kbuild.include @@ -193,7 +193,7 @@ modbuiltin := -f $(srctree)/scripts/Makefile.modbuiltin obj # Shorthand for $(Q)$(MAKE) -f scripts/Makefile.dtbinst obj= # Usage: # $(Q)$(MAKE) $(dtbinst)=dir -dtbinst := -f $(if $(KBUILD_SRC),$(srctree)/)scripts/Makefile.dtbinst obj +dtbinst := -f $(srctree)/scripts/Makefile.dtbinst obj ### # Shorthand for $(Q)$(MAKE) -f scripts/Makefile.clean obj= -- 2.7.4
[PATCH] kbuild: prefix Makefile.dtbinst path with $(srctree) unconditionally
$(srctree) always points to the top of the source tree whether KBUILD_SRC is set or not. Signed-off-by: Masahiro Yamada --- scripts/Kbuild.include | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include index ce53639..46cc43e 100644 --- a/scripts/Kbuild.include +++ b/scripts/Kbuild.include @@ -193,7 +193,7 @@ modbuiltin := -f $(srctree)/scripts/Makefile.modbuiltin obj # Shorthand for $(Q)$(MAKE) -f scripts/Makefile.dtbinst obj= # Usage: # $(Q)$(MAKE) $(dtbinst)=dir -dtbinst := -f $(if $(KBUILD_SRC),$(srctree)/)scripts/Makefile.dtbinst obj +dtbinst := -f $(srctree)/scripts/Makefile.dtbinst obj ### # Shorthand for $(Q)$(MAKE) -f scripts/Makefile.clean obj= -- 2.7.4
linux-next: Tree for Sep 12
Hi all, News: there will be no linux-next releases on Friday or Monday. Changes since 20180911: Dropped trees: xarray, ida (temporarily) I applied a patch for a runtime problem in the vfs tree and I still disabled building some samples. The drm-misc tree gained a conflict against the drm tree. The tty tree lost its build failure. Non-merge commits (relative to Linus' tree): 3284 3703 files changed, 109374 insertions(+), 67935 deletions(-) I have created today's linux-next tree at git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git (patches at http://www.kernel.org/pub/linux/kernel/next/ ). If you are tracking the linux-next tree using git, you should not use "git pull" to do so as that will try to merge the new linux-next release with the old one. You should use "git fetch" and checkout or reset to the new master. You can see which trees have been included by looking in the Next/Trees file in the source. There are also quilt-import.log and merge.log files in the Next directory. Between each merge, the tree was built with a ppc64_defconfig for powerpc, an allmodconfig for x86_64, a multi_v7_defconfig for arm and a native build of tools/perf. After the final fixups (if any), I do an x86_64 modules_install followed by builds for x86_64 allnoconfig, powerpc allnoconfig (32 and 64 bit), ppc44x_defconfig, allyesconfig and pseries_le_defconfig and i386, sparc and sparc64 defconfig. And finally, a simple boot test of the powerpc pseries_le_defconfig kernel in qemu (with and without kvm enabled). Below is a summary of the state of the merge. I am currently merging 287 trees (counting Linus' and 66 trees of bug fix patches pending for the current merge release). Stats about the size of the tree over time can be seen at http://neuling.org/linux-next-size.html . Status of my local build tests will be at http://kisskb.ellerman.id.au/linux-next . If maintainers want to give advice about cross compilers/configs that work, we are always open to add more builds. Thanks to Randy Dunlap for doing many randconfig builds. And to Paul Gortmaker for triage and bug fixes. -- Cheers, Stephen Rothwell $ git checkout master $ git reset --hard stable Merging origin/master (11da3a7f84f1 Linux 4.19-rc3) Merging fixes/master (72358c0b59b7 linux-next: build warnings from the build of Linus' tree) Merging kbuild-current/fixes (11da3a7f84f1 Linux 4.19-rc3) Merging arc-current/for-curr (00a99339f0a3 ARCv2: build: use mcpu=hs38 iso generic mcpu=archs) Merging arm-current/fixes (afc9f65e01cd ARM: 8781/1: Fix Thumb-2 syscall return for binutils 2.29+) Merging arm64-fixes/for-next/fixes (84c57dbd3c48 arm64: kernel: arch_crash_save_vmcoreinfo() should depend on CONFIG_CRASH_CORE) Merging m68k-current/for-linus (0986b16ab49b m68k/mac: Use correct PMU response format) Merging powerpc-fixes/fixes (cca19f0b684f powerpc/64s/radix: Fix missing global invalidations when removing copro) Merging sparc/master (df2def49c57b Merge tag 'acpi-4.19-rc1-2' of git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm) Merging fscrypt-current/for-stable (ae64f9bd1d36 Linux 4.15-rc2) Merging net/master (7c5cca358854 qmi_wwan: Support dynamic config on Quectel EP06) Merging bpf/master (28619527b8a7 Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net) Merging ipsec/master (782710e333a5 xfrm: reset crypto_done when iterating over multiple input xfrms) Merging netfilter/master (1286df269f49 netfilter: xt_hashlimit: use s->file instead of s->private) Merging ipvs/master (feb9f55c33e5 netfilter: nft_dynset: allow dynamic updates of non-anonymous set) Merging wireless-drivers/master (5b394b2ddf03 Linux 4.19-rc1) Merging mac80211/master (c42055105785 mac80211: fix TX status reporting for ieee80211s) Merging rdma-fixes/for-rc (8f28b178f71c RDMA/mlx4: Ensure that maximal send/receive SGE less than supported by HW) Merging sound-current/for-linus (49434c6c575d ALSA: emu10k1: fix possible info leak to userspace on SNDRV_EMU10K1_IOCTL_INFO) Merging sound-asoc-fixes/for-linus (de7609683fef Merge branch 'asoc-4.19' into asoc-linus) Merging regmap-fixes/for-linus (57361846b52b Linux 4.19-rc2) Merging regulator-fixes/for-linus (b832dd4f2c04 Merge branch 'regulator-4.19' into regulator-linus) Merging spi-fixes/for-linus (9029858be9ef Merge branch 'spi-4.19' into spi-linus) Merging pci-current/for-linus (34fb6bf9b13a PCI: pciehp: Fix hot-add vs powerfault detection order) Merging driver-core.current/driver-core-linus (11da3a7f84f1 Linux 4.19-rc3) Merging tty.current/tty-linus (7f2bf7840b74 tty: hvc: hvc_write() fix break condition) Merging usb.current/usb-linus (df3aa13c7bbb Revert "cdc-acm: implement put_char() and flush_chars()") Merging usb-gadget-fixes/fixes (d9707490077b usb: dwc2: Fix call location of dwc2_check_core_endianness) Merging usb-serial-fixes/usb-linus (5dfdd24eb3d3 USB:
linux-next: Tree for Sep 12
Hi all, News: there will be no linux-next releases on Friday or Monday. Changes since 20180911: Dropped trees: xarray, ida (temporarily) I applied a patch for a runtime problem in the vfs tree and I still disabled building some samples. The drm-misc tree gained a conflict against the drm tree. The tty tree lost its build failure. Non-merge commits (relative to Linus' tree): 3284 3703 files changed, 109374 insertions(+), 67935 deletions(-) I have created today's linux-next tree at git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git (patches at http://www.kernel.org/pub/linux/kernel/next/ ). If you are tracking the linux-next tree using git, you should not use "git pull" to do so as that will try to merge the new linux-next release with the old one. You should use "git fetch" and checkout or reset to the new master. You can see which trees have been included by looking in the Next/Trees file in the source. There are also quilt-import.log and merge.log files in the Next directory. Between each merge, the tree was built with a ppc64_defconfig for powerpc, an allmodconfig for x86_64, a multi_v7_defconfig for arm and a native build of tools/perf. After the final fixups (if any), I do an x86_64 modules_install followed by builds for x86_64 allnoconfig, powerpc allnoconfig (32 and 64 bit), ppc44x_defconfig, allyesconfig and pseries_le_defconfig and i386, sparc and sparc64 defconfig. And finally, a simple boot test of the powerpc pseries_le_defconfig kernel in qemu (with and without kvm enabled). Below is a summary of the state of the merge. I am currently merging 287 trees (counting Linus' and 66 trees of bug fix patches pending for the current merge release). Stats about the size of the tree over time can be seen at http://neuling.org/linux-next-size.html . Status of my local build tests will be at http://kisskb.ellerman.id.au/linux-next . If maintainers want to give advice about cross compilers/configs that work, we are always open to add more builds. Thanks to Randy Dunlap for doing many randconfig builds. And to Paul Gortmaker for triage and bug fixes. -- Cheers, Stephen Rothwell $ git checkout master $ git reset --hard stable Merging origin/master (11da3a7f84f1 Linux 4.19-rc3) Merging fixes/master (72358c0b59b7 linux-next: build warnings from the build of Linus' tree) Merging kbuild-current/fixes (11da3a7f84f1 Linux 4.19-rc3) Merging arc-current/for-curr (00a99339f0a3 ARCv2: build: use mcpu=hs38 iso generic mcpu=archs) Merging arm-current/fixes (afc9f65e01cd ARM: 8781/1: Fix Thumb-2 syscall return for binutils 2.29+) Merging arm64-fixes/for-next/fixes (84c57dbd3c48 arm64: kernel: arch_crash_save_vmcoreinfo() should depend on CONFIG_CRASH_CORE) Merging m68k-current/for-linus (0986b16ab49b m68k/mac: Use correct PMU response format) Merging powerpc-fixes/fixes (cca19f0b684f powerpc/64s/radix: Fix missing global invalidations when removing copro) Merging sparc/master (df2def49c57b Merge tag 'acpi-4.19-rc1-2' of git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm) Merging fscrypt-current/for-stable (ae64f9bd1d36 Linux 4.15-rc2) Merging net/master (7c5cca358854 qmi_wwan: Support dynamic config on Quectel EP06) Merging bpf/master (28619527b8a7 Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net) Merging ipsec/master (782710e333a5 xfrm: reset crypto_done when iterating over multiple input xfrms) Merging netfilter/master (1286df269f49 netfilter: xt_hashlimit: use s->file instead of s->private) Merging ipvs/master (feb9f55c33e5 netfilter: nft_dynset: allow dynamic updates of non-anonymous set) Merging wireless-drivers/master (5b394b2ddf03 Linux 4.19-rc1) Merging mac80211/master (c42055105785 mac80211: fix TX status reporting for ieee80211s) Merging rdma-fixes/for-rc (8f28b178f71c RDMA/mlx4: Ensure that maximal send/receive SGE less than supported by HW) Merging sound-current/for-linus (49434c6c575d ALSA: emu10k1: fix possible info leak to userspace on SNDRV_EMU10K1_IOCTL_INFO) Merging sound-asoc-fixes/for-linus (de7609683fef Merge branch 'asoc-4.19' into asoc-linus) Merging regmap-fixes/for-linus (57361846b52b Linux 4.19-rc2) Merging regulator-fixes/for-linus (b832dd4f2c04 Merge branch 'regulator-4.19' into regulator-linus) Merging spi-fixes/for-linus (9029858be9ef Merge branch 'spi-4.19' into spi-linus) Merging pci-current/for-linus (34fb6bf9b13a PCI: pciehp: Fix hot-add vs powerfault detection order) Merging driver-core.current/driver-core-linus (11da3a7f84f1 Linux 4.19-rc3) Merging tty.current/tty-linus (7f2bf7840b74 tty: hvc: hvc_write() fix break condition) Merging usb.current/usb-linus (df3aa13c7bbb Revert "cdc-acm: implement put_char() and flush_chars()") Merging usb-gadget-fixes/fixes (d9707490077b usb: dwc2: Fix call location of dwc2_check_core_endianness) Merging usb-serial-fixes/usb-linus (5dfdd24eb3d3 USB:
Re: [PATCH v2 2/2] dmaengine: uniphier-mdmac: add UniPhier MIO DMAC driver
On 12-09-18, 12:01, Masahiro Yamada wrote:
> Hi Vinod,
>
>
> 2018-09-11 16:00 GMT+09:00 Vinod :
> > On 24-08-18, 10:41, Masahiro Yamada wrote:
> >
> >> +/* mc->vc.lock must be held by caller */
> >> +static u32 __uniphier_mdmac_get_residue(struct uniphier_mdmac_desc *md)
> >> +{
> >> + u32 residue = 0;
> >> + int i;
> >> +
> >> + for (i = md->sg_cur; i < md->sg_len; i++)
> >> + residue += sg_dma_len(>sgl[i]);
> >
> > so if the descriptor is submitted to hardware, we return the descriptor
> > length, which is not correct.
> >
> > Two cases are required to be handled:
> > 1. Descriptor is in queue (IMO above logic is fine for that, but it can
> > be calculated at descriptor submit and looked up here)
>
> Where do you want it to be calculated?
where is it calculated now?
> This hardware provides only simple registers (address and size)
> for one-shot transfer instead of descriptors.
>
> So, I used sgl as-is because I did not see a good reason
> to transform sgl to another data structure.
> > this seems missing stuff. Where do you do register calculation for the
> > descriptor and where is slave_config here, how do you know where to
> > send/receive data form/to (peripheral)
>
>
> This dmac is really simple, and un-flexible.
>
> The peripheral address to send/receive data from/to is hard-weird.
> cfg->{src_addr,dst_addr} is not configurable.
>
> Look at __uniphier_mdmac_handle().
> 'dest_addr' and 'src_addr' must be set to 0 for the peripheral.
Fair enough, what about other values like addr_width and maxburst?
--
~Vinod
Re: [PATCH v2 2/2] dmaengine: uniphier-mdmac: add UniPhier MIO DMAC driver
On 12-09-18, 12:01, Masahiro Yamada wrote:
> Hi Vinod,
>
>
> 2018-09-11 16:00 GMT+09:00 Vinod :
> > On 24-08-18, 10:41, Masahiro Yamada wrote:
> >
> >> +/* mc->vc.lock must be held by caller */
> >> +static u32 __uniphier_mdmac_get_residue(struct uniphier_mdmac_desc *md)
> >> +{
> >> + u32 residue = 0;
> >> + int i;
> >> +
> >> + for (i = md->sg_cur; i < md->sg_len; i++)
> >> + residue += sg_dma_len(>sgl[i]);
> >
> > so if the descriptor is submitted to hardware, we return the descriptor
> > length, which is not correct.
> >
> > Two cases are required to be handled:
> > 1. Descriptor is in queue (IMO above logic is fine for that, but it can
> > be calculated at descriptor submit and looked up here)
>
> Where do you want it to be calculated?
where is it calculated now?
> This hardware provides only simple registers (address and size)
> for one-shot transfer instead of descriptors.
>
> So, I used sgl as-is because I did not see a good reason
> to transform sgl to another data structure.
> > this seems missing stuff. Where do you do register calculation for the
> > descriptor and where is slave_config here, how do you know where to
> > send/receive data form/to (peripheral)
>
>
> This dmac is really simple, and un-flexible.
>
> The peripheral address to send/receive data from/to is hard-weird.
> cfg->{src_addr,dst_addr} is not configurable.
>
> Look at __uniphier_mdmac_handle().
> 'dest_addr' and 'src_addr' must be set to 0 for the peripheral.
Fair enough, what about other values like addr_width and maxburst?
--
~Vinod
Re: [PATCH] RISC-V: Show IPI stats
On Mon, Sep 10, 2018 at 7:16 PM, Christoph Hellwig wrote:
> On Fri, Sep 07, 2018 at 06:14:29PM +0530, Anup Patel wrote:
>> This patch provides arch_show_interrupts() implementation to
>> show IPI stats via /proc/interrupts.
>>
>> Now the contents of /proc/interrupts" will look like below:
>>CPU0 CPU1 CPU2 CPU3
>> 8: 17 7 6 14 SiFive PLIC 8 virtio0
>> 10: 10 10 9 11 SiFive PLIC 10 ttyS0
>> IPI0: 170673251 79 Rescheduling interrupts
>> IPI1: 1 12 27 1 Function call interrupts
>> IPI2: 0 0 0 0 CPU wake-up interrupts
>>
>> Signed-off-by: Anup Patel
>
> Thanks, this looks pretty sensible to me. Maybe we want to also show
> timer interrupts if we do this?
Let's not include timer stats here until RISCV INTC driver is
concluded. We can do it as separate patch if required.
>
>> --- a/arch/riscv/kernel/irq.c
>> +++ b/arch/riscv/kernel/irq.c
>> @@ -8,6 +8,7 @@
>> #include
>> #include
>> #include
>> +#include
>>
>> /*
>> * Possible interrupt causes:
>> @@ -24,6 +25,14 @@
>> */
>> #define INTERRUPT_CAUSE_FLAG (1UL << (__riscv_xlen - 1))
>>
>> +int arch_show_interrupts(struct seq_file *p, int prec)
>> +{
>> +#ifdef CONFIG_SMP
>> + show_ipi_stats(p, prec);
>> +#endif
>> + return 0;
>> +}
>
> If we don't also add timer stats I'd just move arch_show_interrupts
> to smp.c and make it conditional. If we don't this split might make
> more sense.
I understand you want to avoid #ifdef here. We can do same thing
by having empty inline function show_ipi_stats() in asm/smp.h for
!CONFIG_SMP case. This way we can keep arch_show_interrupts()
in kernel/irq.c which is intuitively correct location for
arch_show_interrupts().
>
>> +static const char *ipi_names[IPI_MAX] = {
>> + [IPI_RESCHEDULE] = "Rescheduling interrupts",
>> + [IPI_CALL_FUNC] = "Function call interrupts",
>> + [IPI_CALL_WAKEUP] = "CPU wake-up interrupts",
>> +};
>
> No need for the explicit array size. Also please use a few tabs to
> align this nicely:
>
> static const char *ipi_names[] = {
> [IPI_RESCHEDULE]= "Rescheduling interrupts",
> [IPI_CALL_FUNC] = "Function call interrupts",
> [IPI_CALL_WAKEUP] = "CPU wake-up interrupts",
> };
Sure, will do.
Regards,
Anup
Re: [PATCH] RISC-V: Show IPI stats
On Mon, Sep 10, 2018 at 7:16 PM, Christoph Hellwig wrote:
> On Fri, Sep 07, 2018 at 06:14:29PM +0530, Anup Patel wrote:
>> This patch provides arch_show_interrupts() implementation to
>> show IPI stats via /proc/interrupts.
>>
>> Now the contents of /proc/interrupts" will look like below:
>>CPU0 CPU1 CPU2 CPU3
>> 8: 17 7 6 14 SiFive PLIC 8 virtio0
>> 10: 10 10 9 11 SiFive PLIC 10 ttyS0
>> IPI0: 170673251 79 Rescheduling interrupts
>> IPI1: 1 12 27 1 Function call interrupts
>> IPI2: 0 0 0 0 CPU wake-up interrupts
>>
>> Signed-off-by: Anup Patel
>
> Thanks, this looks pretty sensible to me. Maybe we want to also show
> timer interrupts if we do this?
Let's not include timer stats here until RISCV INTC driver is
concluded. We can do it as separate patch if required.
>
>> --- a/arch/riscv/kernel/irq.c
>> +++ b/arch/riscv/kernel/irq.c
>> @@ -8,6 +8,7 @@
>> #include
>> #include
>> #include
>> +#include
>>
>> /*
>> * Possible interrupt causes:
>> @@ -24,6 +25,14 @@
>> */
>> #define INTERRUPT_CAUSE_FLAG (1UL << (__riscv_xlen - 1))
>>
>> +int arch_show_interrupts(struct seq_file *p, int prec)
>> +{
>> +#ifdef CONFIG_SMP
>> + show_ipi_stats(p, prec);
>> +#endif
>> + return 0;
>> +}
>
> If we don't also add timer stats I'd just move arch_show_interrupts
> to smp.c and make it conditional. If we don't this split might make
> more sense.
I understand you want to avoid #ifdef here. We can do same thing
by having empty inline function show_ipi_stats() in asm/smp.h for
!CONFIG_SMP case. This way we can keep arch_show_interrupts()
in kernel/irq.c which is intuitively correct location for
arch_show_interrupts().
>
>> +static const char *ipi_names[IPI_MAX] = {
>> + [IPI_RESCHEDULE] = "Rescheduling interrupts",
>> + [IPI_CALL_FUNC] = "Function call interrupts",
>> + [IPI_CALL_WAKEUP] = "CPU wake-up interrupts",
>> +};
>
> No need for the explicit array size. Also please use a few tabs to
> align this nicely:
>
> static const char *ipi_names[] = {
> [IPI_RESCHEDULE]= "Rescheduling interrupts",
> [IPI_CALL_FUNC] = "Function call interrupts",
> [IPI_CALL_WAKEUP] = "CPU wake-up interrupts",
> };
Sure, will do.
Regards,
Anup
[PATCH] xtensa: remove unnecessary KBUILD_SRC ifeq conditional
You can always prefix variant/platform header search paths with $(srctree)/ because $(srctree) is '.' for in-tree building. Signed-off-by: Masahiro Yamada --- arch/xtensa/Makefile | 4 1 file changed, 4 deletions(-) diff --git a/arch/xtensa/Makefile b/arch/xtensa/Makefile index 295c120..d67e30fa 100644 --- a/arch/xtensa/Makefile +++ b/arch/xtensa/Makefile @@ -64,11 +64,7 @@ endif vardirs := $(patsubst %,arch/xtensa/variants/%/,$(variant-y)) plfdirs := $(patsubst %,arch/xtensa/platforms/%/,$(platform-y)) -ifeq ($(KBUILD_SRC),) -KBUILD_CPPFLAGS += $(patsubst %,-I%include,$(vardirs) $(plfdirs)) -else KBUILD_CPPFLAGS += $(patsubst %,-I$(srctree)/%include,$(vardirs) $(plfdirs)) -endif KBUILD_DEFCONFIG := iss_defconfig -- 2.7.4
[PATCH] xtensa: remove unnecessary KBUILD_SRC ifeq conditional
You can always prefix variant/platform header search paths with $(srctree)/ because $(srctree) is '.' for in-tree building. Signed-off-by: Masahiro Yamada --- arch/xtensa/Makefile | 4 1 file changed, 4 deletions(-) diff --git a/arch/xtensa/Makefile b/arch/xtensa/Makefile index 295c120..d67e30fa 100644 --- a/arch/xtensa/Makefile +++ b/arch/xtensa/Makefile @@ -64,11 +64,7 @@ endif vardirs := $(patsubst %,arch/xtensa/variants/%/,$(variant-y)) plfdirs := $(patsubst %,arch/xtensa/platforms/%/,$(platform-y)) -ifeq ($(KBUILD_SRC),) -KBUILD_CPPFLAGS += $(patsubst %,-I%include,$(vardirs) $(plfdirs)) -else KBUILD_CPPFLAGS += $(patsubst %,-I$(srctree)/%include,$(vardirs) $(plfdirs)) -endif KBUILD_DEFCONFIG := iss_defconfig -- 2.7.4
[PATCH] ARM: remove unnecessary KBUILD_SRC ifeq conditional
You can always prefix machine/plat header search paths with $(srctree)/ because $(srctree) is '.' for in-tree building. Signed-off-by: Masahiro Yamada --- KernelVersion: v4.19-rc3 arch/arm/Makefile | 4 1 file changed, 4 deletions(-) diff --git a/arch/arm/Makefile b/arch/arm/Makefile index d1516f8..06ebff7 100644 --- a/arch/arm/Makefile +++ b/arch/arm/Makefile @@ -264,13 +264,9 @@ platdirs := $(patsubst %,arch/arm/plat-%/,$(sort $(plat-y))) ifneq ($(CONFIG_ARCH_MULTIPLATFORM),y) ifneq ($(CONFIG_ARM_SINGLE_ARMV7M),y) -ifeq ($(KBUILD_SRC),) -KBUILD_CPPFLAGS += $(patsubst %,-I%include,$(machdirs) $(platdirs)) -else KBUILD_CPPFLAGS += $(patsubst %,-I$(srctree)/%include,$(machdirs) $(platdirs)) endif endif -endif export TEXT_OFFSET GZFLAGS MMUEXT -- 2.7.4
[PATCH] ARM: remove unnecessary KBUILD_SRC ifeq conditional
You can always prefix machine/plat header search paths with $(srctree)/ because $(srctree) is '.' for in-tree building. Signed-off-by: Masahiro Yamada --- KernelVersion: v4.19-rc3 arch/arm/Makefile | 4 1 file changed, 4 deletions(-) diff --git a/arch/arm/Makefile b/arch/arm/Makefile index d1516f8..06ebff7 100644 --- a/arch/arm/Makefile +++ b/arch/arm/Makefile @@ -264,13 +264,9 @@ platdirs := $(patsubst %,arch/arm/plat-%/,$(sort $(plat-y))) ifneq ($(CONFIG_ARCH_MULTIPLATFORM),y) ifneq ($(CONFIG_ARM_SINGLE_ARMV7M),y) -ifeq ($(KBUILD_SRC),) -KBUILD_CPPFLAGS += $(patsubst %,-I%include,$(machdirs) $(platdirs)) -else KBUILD_CPPFLAGS += $(patsubst %,-I$(srctree)/%include,$(machdirs) $(platdirs)) endif endif -endif export TEXT_OFFSET GZFLAGS MMUEXT -- 2.7.4
Re: [PATCH] include/linux/compiler-clang.h: define __naked
Hi Arnd, Nick, Stefan,
On Mon, Sep 10, 2018 at 2:14 PM, Arnd Bergmann wrote:
> On Mon, Sep 10, 2018 at 8:05 AM Stefan Agner wrote:
>>
>> ARM32 arch code uses the __naked attribute. This has previously been
>> defined in include/linux/compiler-gcc.h, which is no longer included
>> for Clang. Define __naked for Clang. Conservatively add all attributes
>> previously used (and supported by Clang).
>>
>> This fixes compile errors when building ARM32 using Clang:
>> arch/arm/mach-exynos/mcpm-exynos.c:193:13: error: variable has incomplete
>> type 'void'
>> static void __naked exynos_pm_power_up_setup(unsigned int affinity_level)
>> ^
>>
>> Fixes: 815f0ddb346c ("include/linux/compiler*.h: make compiler-*.h mutually
>> exclusive")
>> Signed-off-by: Stefan Agner
>
>> +/*
>> + * ARM32 is currently the only user of __naked supported by Clang. Follow
>> + * gcc: Do not trace naked functions and make sure they don't get inlined.
>> + */
>> +#define __naked __attribute__((naked)) noinline notrace
>> +
>
> Please see patches 5 and 6 of the series that Miguel posted:
>
> https://lore.kernel.org/lkml/20180908212459.19736-6-miguel.ojeda.sando...@gmail.com/
>
> I suppose we want the patch to fix clang build as soon as possible though,
> and follow up with the cleanup for the next merge window, right?
Not sure what the plans of Linus et. al. are, if they have any; but
that would be a safe bet.
In case they want to speed this up and put the entire series into
v4.19 (instead of the two patches), I have done a binary & objdump
diff between -rc2 and v4 (based on -rc2) on all object files (with
UTS_RELEASE fixed to avoid some differences).
In a x86_64 tinyconfig with gcc 7.3, the differences I found are:
$ ./compare.py linux-rc2 linux-v4
[2018-09-12 06:16:39,483] [INFO] [arch/x86/boot/compressed/piggy.o]
Binary diff (use 'bash -c "cmp
linux-rc2/arch/x86/boot/compressed/piggy.o
linux-v4/arch/x86/boot/compressed/piggy.o"' to replicate)
[2018-09-12 06:16:39,606] [INFO] [arch/x86/boot/header.o] Binary diff
(use 'bash -c "cmp linux-rc2/arch/x86/boot/header.o
linux-v4/arch/x86/boot/header.o"' to replicate)
[2018-09-12 06:16:39,659] [INFO] [arch/x86/boot/version.o] Binary diff
(use 'bash -c "cmp linux-rc2/arch/x86/boot/version.o
linux-v4/arch/x86/boot/version.o"' to replicate)
[2018-09-12 06:16:40,483] [INFO] [init/version.o] Binary diff (use
'bash -c "cmp linux-rc2/init/version.o linux-v4/init/version.o"' to
replicate)
I will do a bigger one tomorrow or so and see if there are any
important differences. Regardless of what we do, I will send the
__naked patches separately as well (requested by Nick on GitHub).
Cheers,
Miguel
Re: [PATCH] include/linux/compiler-clang.h: define __naked
Hi Arnd, Nick, Stefan,
On Mon, Sep 10, 2018 at 2:14 PM, Arnd Bergmann wrote:
> On Mon, Sep 10, 2018 at 8:05 AM Stefan Agner wrote:
>>
>> ARM32 arch code uses the __naked attribute. This has previously been
>> defined in include/linux/compiler-gcc.h, which is no longer included
>> for Clang. Define __naked for Clang. Conservatively add all attributes
>> previously used (and supported by Clang).
>>
>> This fixes compile errors when building ARM32 using Clang:
>> arch/arm/mach-exynos/mcpm-exynos.c:193:13: error: variable has incomplete
>> type 'void'
>> static void __naked exynos_pm_power_up_setup(unsigned int affinity_level)
>> ^
>>
>> Fixes: 815f0ddb346c ("include/linux/compiler*.h: make compiler-*.h mutually
>> exclusive")
>> Signed-off-by: Stefan Agner
>
>> +/*
>> + * ARM32 is currently the only user of __naked supported by Clang. Follow
>> + * gcc: Do not trace naked functions and make sure they don't get inlined.
>> + */
>> +#define __naked __attribute__((naked)) noinline notrace
>> +
>
> Please see patches 5 and 6 of the series that Miguel posted:
>
> https://lore.kernel.org/lkml/20180908212459.19736-6-miguel.ojeda.sando...@gmail.com/
>
> I suppose we want the patch to fix clang build as soon as possible though,
> and follow up with the cleanup for the next merge window, right?
Not sure what the plans of Linus et. al. are, if they have any; but
that would be a safe bet.
In case they want to speed this up and put the entire series into
v4.19 (instead of the two patches), I have done a binary & objdump
diff between -rc2 and v4 (based on -rc2) on all object files (with
UTS_RELEASE fixed to avoid some differences).
In a x86_64 tinyconfig with gcc 7.3, the differences I found are:
$ ./compare.py linux-rc2 linux-v4
[2018-09-12 06:16:39,483] [INFO] [arch/x86/boot/compressed/piggy.o]
Binary diff (use 'bash -c "cmp
linux-rc2/arch/x86/boot/compressed/piggy.o
linux-v4/arch/x86/boot/compressed/piggy.o"' to replicate)
[2018-09-12 06:16:39,606] [INFO] [arch/x86/boot/header.o] Binary diff
(use 'bash -c "cmp linux-rc2/arch/x86/boot/header.o
linux-v4/arch/x86/boot/header.o"' to replicate)
[2018-09-12 06:16:39,659] [INFO] [arch/x86/boot/version.o] Binary diff
(use 'bash -c "cmp linux-rc2/arch/x86/boot/version.o
linux-v4/arch/x86/boot/version.o"' to replicate)
[2018-09-12 06:16:40,483] [INFO] [init/version.o] Binary diff (use
'bash -c "cmp linux-rc2/init/version.o linux-v4/init/version.o"' to
replicate)
I will do a bigger one tomorrow or so and see if there are any
important differences. Regardless of what we do, I will send the
__naked patches separately as well (requested by Nick on GitHub).
Cheers,
Miguel
[PATCH] dt-bindings: power: Introduce suspend states supported properties
Introuduce linux generic suspend states supported properties. It is convenient for the generic suspend path to have the knowledge of the suspend states supported based on the device tree properties based on which it can either be suspended or safely bailed out of suspend if none of the suspend states are supported. Signed-off-by: Keerthy --- .../devicetree/bindings/power/power-states.txt | 22 ++ 1 file changed, 22 insertions(+) create mode 100644 Documentation/devicetree/bindings/power/power-states.txt diff --git a/Documentation/devicetree/bindings/power/power-states.txt b/Documentation/devicetree/bindings/power/power-states.txt new file mode 100644 index 000..bb80b36 --- /dev/null +++ b/Documentation/devicetree/bindings/power/power-states.txt @@ -0,0 +1,22 @@ +* Generic system suspend states support + +Most platforms support multiple suspend states. Define system +suspend states so that one can target appropriate low power +states based on the SoC capabilities. + +linux,suspend-to-memory-supported + +Upon suspend to memory the system context is saved to primary memory. +All the clocks for all the peripherals including CPU are gated. + +linux,suspend-power-off-supported + +In this case in additon to the clocks all the voltage resources are +turned off except the ones needed to keep the primary memory +and a wake up source that can trigger a wakeup event. + +linux,suspend-to-disk-supported + +Upon suspend to disk that system context is saved to secondary memory. +All the clocks for all the peripherals including CPU are gated. Even +the primary memory is turned off. -- 1.9.1
[PATCH] dt-bindings: power: Introduce suspend states supported properties
Introuduce linux generic suspend states supported properties. It is convenient for the generic suspend path to have the knowledge of the suspend states supported based on the device tree properties based on which it can either be suspended or safely bailed out of suspend if none of the suspend states are supported. Signed-off-by: Keerthy --- .../devicetree/bindings/power/power-states.txt | 22 ++ 1 file changed, 22 insertions(+) create mode 100644 Documentation/devicetree/bindings/power/power-states.txt diff --git a/Documentation/devicetree/bindings/power/power-states.txt b/Documentation/devicetree/bindings/power/power-states.txt new file mode 100644 index 000..bb80b36 --- /dev/null +++ b/Documentation/devicetree/bindings/power/power-states.txt @@ -0,0 +1,22 @@ +* Generic system suspend states support + +Most platforms support multiple suspend states. Define system +suspend states so that one can target appropriate low power +states based on the SoC capabilities. + +linux,suspend-to-memory-supported + +Upon suspend to memory the system context is saved to primary memory. +All the clocks for all the peripherals including CPU are gated. + +linux,suspend-power-off-supported + +In this case in additon to the clocks all the voltage resources are +turned off except the ones needed to keep the primary memory +and a wake up source that can trigger a wakeup event. + +linux,suspend-to-disk-supported + +Upon suspend to disk that system context is saved to secondary memory. +All the clocks for all the peripherals including CPU are gated. Even +the primary memory is turned off. -- 1.9.1
Re: [PATCH 2/3] sound: enable interrupt after dma buffer initialization
On 11-09-18, 14:58, Yu Zhao wrote: > On Tue, Sep 11, 2018 at 08:06:49AM +0200, Takashi Iwai wrote: > > On Mon, 10 Sep 2018 23:21:50 +0200, > > Yu Zhao wrote: > > > > > > In snd_hdac_bus_init_chip(), we enable interrupt before > > > snd_hdac_bus_init_cmd_io() initializing dma buffers. If irq has > > > been acquired and irq handler uses the dma buffer, kernel may crash > > > when interrupt comes in. > > > > > > Fix the problem by postponing enabling irq after dma buffer > > > initialization. And warn once on null dma buffer pointer during the > > > initialization. > > > > > > Signed-off-by: Yu Zhao > > > > Looks good to me. > > > > Reviewed-by: Takashi Iwai > > > > > > BTW, the reason why this hasn't been hit on the legacy HD-audio driver > > is that we allocate usually with MSI, so the irq is isolated. > > > > Any reason that Intel SKL driver doesn't use MST? > > This I'm not sure. Vinod might have answer to it, according to > https://patchwork.kernel.org/patch/6375831/#13796611 IIRC (seemed quite some time back) we faced issues with using MSI on SKL and didnt try afterwards. If Intel folks can try it and check. Pierre is out, maybe Liam can help..? -- ~Vinod
Re: [PATCH 2/3] sound: enable interrupt after dma buffer initialization
On 11-09-18, 14:58, Yu Zhao wrote: > On Tue, Sep 11, 2018 at 08:06:49AM +0200, Takashi Iwai wrote: > > On Mon, 10 Sep 2018 23:21:50 +0200, > > Yu Zhao wrote: > > > > > > In snd_hdac_bus_init_chip(), we enable interrupt before > > > snd_hdac_bus_init_cmd_io() initializing dma buffers. If irq has > > > been acquired and irq handler uses the dma buffer, kernel may crash > > > when interrupt comes in. > > > > > > Fix the problem by postponing enabling irq after dma buffer > > > initialization. And warn once on null dma buffer pointer during the > > > initialization. > > > > > > Signed-off-by: Yu Zhao > > > > Looks good to me. > > > > Reviewed-by: Takashi Iwai > > > > > > BTW, the reason why this hasn't been hit on the legacy HD-audio driver > > is that we allocate usually with MSI, so the irq is isolated. > > > > Any reason that Intel SKL driver doesn't use MST? > > This I'm not sure. Vinod might have answer to it, according to > https://patchwork.kernel.org/patch/6375831/#13796611 IIRC (seemed quite some time back) we faced issues with using MSI on SKL and didnt try afterwards. If Intel folks can try it and check. Pierre is out, maybe Liam can help..? -- ~Vinod
Re: [Question] Are the trace APIs declared by "TRACE_EVENT(irq_handler_entry" allowed to be used in Ko?
On Wed, 12 Sep 2018 10:08:37 +0800
"Leizhen (ThunderTown)" wrote:
> After patch 7e066fb870fc ("tracepoints: add DECLARE_TRACE() and
> DEFINE_TRACE()"),
> the trace APIs declared by "TRACE_EVENT(irq_handler_entry" can not be
> directly used
> by ko, because it's not explicitly exported by EXPORT_TRACEPOINT_SYMBOL_GPL or
> EXPORT_TRACEPOINT_SYMBOL.
>
> Did we miss it? or it's not recommended to be used in ko?
>
Why do you need it. This patch is almost 10 years old, and you are just
now finding an issue with it?
-- Steve
>
> -
>
> commit 7e066fb870fcd1025ec3ba7bbde5d541094f4ce1
> Author: Mathieu Desnoyers
> Date: Fri Nov 14 17:47:47 2008 -0500
>
> tracepoints: add DECLARE_TRACE() and DEFINE_TRACE()
>
> Impact: API *CHANGE*. Must update all tracepoint users.
>
> Add DEFINE_TRACE() to tracepoints to let them declare the tracepoint
> structure in a single spot for all the kernel. It helps reducing memory
> consumption, especially when declaring a lot of tracepoints, e.g. for
> kmalloc tracing.
>
> *API CHANGE WARNING*: now, DECLARE_TRACE() must be used in headers for
> tracepoint declarations rather than DEFINE_TRACE(). This is the sane way
> to do it. The name previously used was misleading.
>
> Updates scheduler instrumentation to follow this API change.
>
>
Re: [Question] Are the trace APIs declared by "TRACE_EVENT(irq_handler_entry" allowed to be used in Ko?
On Wed, 12 Sep 2018 10:08:37 +0800
"Leizhen (ThunderTown)" wrote:
> After patch 7e066fb870fc ("tracepoints: add DECLARE_TRACE() and
> DEFINE_TRACE()"),
> the trace APIs declared by "TRACE_EVENT(irq_handler_entry" can not be
> directly used
> by ko, because it's not explicitly exported by EXPORT_TRACEPOINT_SYMBOL_GPL or
> EXPORT_TRACEPOINT_SYMBOL.
>
> Did we miss it? or it's not recommended to be used in ko?
>
Why do you need it. This patch is almost 10 years old, and you are just
now finding an issue with it?
-- Steve
>
> -
>
> commit 7e066fb870fcd1025ec3ba7bbde5d541094f4ce1
> Author: Mathieu Desnoyers
> Date: Fri Nov 14 17:47:47 2008 -0500
>
> tracepoints: add DECLARE_TRACE() and DEFINE_TRACE()
>
> Impact: API *CHANGE*. Must update all tracepoint users.
>
> Add DEFINE_TRACE() to tracepoints to let them declare the tracepoint
> structure in a single spot for all the kernel. It helps reducing memory
> consumption, especially when declaring a lot of tracepoints, e.g. for
> kmalloc tracing.
>
> *API CHANGE WARNING*: now, DECLARE_TRACE() must be used in headers for
> tracepoint declarations rather than DEFINE_TRACE(). This is the sane way
> to do it. The name previously used was misleading.
>
> Updates scheduler instrumentation to follow this API change.
>
>
[PATCH] ASoC: remove unneeded static set .owner field in platform_driver
platform_driver_register will set the .owner field. So it is safe
to remove the redundant assignment.
The issue is detected with the help of Coccinelle.
Signed-off-by: zhong jiang
---
sound/soc/mediatek/mt2701/mt2701-wm8960.c | 1 -
sound/soc/mediatek/mt6797/mt6797-mt6351.c | 1 -
sound/soc/rockchip/rk3288_hdmi_analog.c | 1 -
3 files changed, 3 deletions(-)
diff --git a/sound/soc/mediatek/mt2701/mt2701-wm8960.c
b/sound/soc/mediatek/mt2701/mt2701-wm8960.c
index 89f34ef..e5d49e6 100644
--- a/sound/soc/mediatek/mt2701/mt2701-wm8960.c
+++ b/sound/soc/mediatek/mt2701/mt2701-wm8960.c
@@ -150,7 +150,6 @@ static int mt2701_wm8960_machine_probe(struct
platform_device *pdev)
static struct platform_driver mt2701_wm8960_machine = {
.driver = {
.name = "mt2701-wm8960",
- .owner = THIS_MODULE,
#ifdef CONFIG_OF
.of_match_table = mt2701_wm8960_machine_dt_match,
#endif
diff --git a/sound/soc/mediatek/mt6797/mt6797-mt6351.c
b/sound/soc/mediatek/mt6797/mt6797-mt6351.c
index b1558c5..6e578e8 100644
--- a/sound/soc/mediatek/mt6797/mt6797-mt6351.c
+++ b/sound/soc/mediatek/mt6797/mt6797-mt6351.c
@@ -205,7 +205,6 @@ static int mt6797_mt6351_dev_probe(struct platform_device
*pdev)
static struct platform_driver mt6797_mt6351_driver = {
.driver = {
.name = "mt6797-mt6351",
- .owner = THIS_MODULE,
#ifdef CONFIG_OF
.of_match_table = mt6797_mt6351_dt_match,
#endif
diff --git a/sound/soc/rockchip/rk3288_hdmi_analog.c
b/sound/soc/rockchip/rk3288_hdmi_analog.c
index 929b3fe..a472d5e 100644
--- a/sound/soc/rockchip/rk3288_hdmi_analog.c
+++ b/sound/soc/rockchip/rk3288_hdmi_analog.c
@@ -286,7 +286,6 @@ static int snd_rk_mc_probe(struct platform_device *pdev)
.probe = snd_rk_mc_probe,
.driver = {
.name = DRV_NAME,
- .owner = THIS_MODULE,
.pm = _soc_pm_ops,
.of_match_table = rockchip_sound_of_match,
},
--
1.7.12.4
[PATCH TRIVIAL] Punctuation fixes
Signed-off-by: Diego Viola --- CREDITS | 2 +- MAINTAINERS | 2 +- Makefile| 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/CREDITS b/CREDITS index 5befd2d71..b82efb36d 100644 --- a/CREDITS +++ b/CREDITS @@ -1473,7 +1473,7 @@ W: http://www.linux-ide.org/ W: http://www.linuxdiskcert.org/ D: Random SMP kernel hacker... D: Uniform Multi-Platform E-IDE driver -D: Active-ATA-Chipset maddness.. +D: Active-ATA-Chipset maddness... D: Ultra DMA 133/100/66/33 w/48-bit Addressing D: ATA-Disconnect, ATA-TCQ D: ATA-Smart Kernel Daemon diff --git a/MAINTAINERS b/MAINTAINERS index d870cb57c..6567bf245 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -93,7 +93,7 @@ Descriptions of section entries: Supported: Someone is actually paid to look after this. Maintained: Someone actually looks after it. Odd Fixes: It has a maintainer but they don't have time to do - much other than throw the odd patch in. See below.. + much other than throw the odd patch in. See below. Orphan: No current maintainer [but maybe you could take the role as you write your new code]. Obsolete:Old code. Something tagged obsolete generally means diff --git a/Makefile b/Makefile index 4d5c883a9..7b5c5d634 100644 --- a/Makefile +++ b/Makefile @@ -1109,7 +1109,7 @@ archprepare: archheaders archscripts prepare1 scripts_basic prepare0: archprepare gcc-plugins $(Q)$(MAKE) $(build)=. -# All the preparing.. +# All the preparing... prepare: prepare0 prepare-objtool # Support for using generic headers in asm-generic -- 2.19.0
[PATCH] ASoC: remove unneeded static set .owner field in platform_driver
platform_driver_register will set the .owner field. So it is safe
to remove the redundant assignment.
The issue is detected with the help of Coccinelle.
Signed-off-by: zhong jiang
---
sound/soc/mediatek/mt2701/mt2701-wm8960.c | 1 -
sound/soc/mediatek/mt6797/mt6797-mt6351.c | 1 -
sound/soc/rockchip/rk3288_hdmi_analog.c | 1 -
3 files changed, 3 deletions(-)
diff --git a/sound/soc/mediatek/mt2701/mt2701-wm8960.c
b/sound/soc/mediatek/mt2701/mt2701-wm8960.c
index 89f34ef..e5d49e6 100644
--- a/sound/soc/mediatek/mt2701/mt2701-wm8960.c
+++ b/sound/soc/mediatek/mt2701/mt2701-wm8960.c
@@ -150,7 +150,6 @@ static int mt2701_wm8960_machine_probe(struct
platform_device *pdev)
static struct platform_driver mt2701_wm8960_machine = {
.driver = {
.name = "mt2701-wm8960",
- .owner = THIS_MODULE,
#ifdef CONFIG_OF
.of_match_table = mt2701_wm8960_machine_dt_match,
#endif
diff --git a/sound/soc/mediatek/mt6797/mt6797-mt6351.c
b/sound/soc/mediatek/mt6797/mt6797-mt6351.c
index b1558c5..6e578e8 100644
--- a/sound/soc/mediatek/mt6797/mt6797-mt6351.c
+++ b/sound/soc/mediatek/mt6797/mt6797-mt6351.c
@@ -205,7 +205,6 @@ static int mt6797_mt6351_dev_probe(struct platform_device
*pdev)
static struct platform_driver mt6797_mt6351_driver = {
.driver = {
.name = "mt6797-mt6351",
- .owner = THIS_MODULE,
#ifdef CONFIG_OF
.of_match_table = mt6797_mt6351_dt_match,
#endif
diff --git a/sound/soc/rockchip/rk3288_hdmi_analog.c
b/sound/soc/rockchip/rk3288_hdmi_analog.c
index 929b3fe..a472d5e 100644
--- a/sound/soc/rockchip/rk3288_hdmi_analog.c
+++ b/sound/soc/rockchip/rk3288_hdmi_analog.c
@@ -286,7 +286,6 @@ static int snd_rk_mc_probe(struct platform_device *pdev)
.probe = snd_rk_mc_probe,
.driver = {
.name = DRV_NAME,
- .owner = THIS_MODULE,
.pm = _soc_pm_ops,
.of_match_table = rockchip_sound_of_match,
},
--
1.7.12.4
[PATCH TRIVIAL] Punctuation fixes
Signed-off-by: Diego Viola --- CREDITS | 2 +- MAINTAINERS | 2 +- Makefile| 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/CREDITS b/CREDITS index 5befd2d71..b82efb36d 100644 --- a/CREDITS +++ b/CREDITS @@ -1473,7 +1473,7 @@ W: http://www.linux-ide.org/ W: http://www.linuxdiskcert.org/ D: Random SMP kernel hacker... D: Uniform Multi-Platform E-IDE driver -D: Active-ATA-Chipset maddness.. +D: Active-ATA-Chipset maddness... D: Ultra DMA 133/100/66/33 w/48-bit Addressing D: ATA-Disconnect, ATA-TCQ D: ATA-Smart Kernel Daemon diff --git a/MAINTAINERS b/MAINTAINERS index d870cb57c..6567bf245 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -93,7 +93,7 @@ Descriptions of section entries: Supported: Someone is actually paid to look after this. Maintained: Someone actually looks after it. Odd Fixes: It has a maintainer but they don't have time to do - much other than throw the odd patch in. See below.. + much other than throw the odd patch in. See below. Orphan: No current maintainer [but maybe you could take the role as you write your new code]. Obsolete:Old code. Something tagged obsolete generally means diff --git a/Makefile b/Makefile index 4d5c883a9..7b5c5d634 100644 --- a/Makefile +++ b/Makefile @@ -1109,7 +1109,7 @@ archprepare: archheaders archscripts prepare1 scripts_basic prepare0: archprepare gcc-plugins $(Q)$(MAKE) $(build)=. -# All the preparing.. +# All the preparing... prepare: prepare0 prepare-objtool # Support for using generic headers in asm-generic -- 2.19.0
[PATCH] staging: remove unneeded static set .owner field in platform_driver
platform_driver_register will set the .owner field. So it is safe
to remove the redundant assignment.
The issue is detected with the help of Coccinelle.
Signed-off-by: zhong jiang
---
drivers/staging/greybus/audio_codec.c| 1 -
drivers/staging/mt7621-eth/gsw_mt7621.c | 1 -
drivers/staging/mt7621-eth/mtk_eth_soc.c | 1 -
3 files changed, 3 deletions(-)
diff --git a/drivers/staging/greybus/audio_codec.c
b/drivers/staging/greybus/audio_codec.c
index 35acd55..08746c8 100644
--- a/drivers/staging/greybus/audio_codec.c
+++ b/drivers/staging/greybus/audio_codec.c
@@ -1087,7 +1087,6 @@ static int gbaudio_codec_remove(struct platform_device
*pdev)
static struct platform_driver gbaudio_codec_driver = {
.driver = {
.name = "apb-dummy-codec",
- .owner = THIS_MODULE,
#ifdef CONFIG_PM
.pm = _codec_pm_ops,
#endif
diff --git a/drivers/staging/mt7621-eth/gsw_mt7621.c
b/drivers/staging/mt7621-eth/gsw_mt7621.c
index 2c07b55..53767b1 100644
--- a/drivers/staging/mt7621-eth/gsw_mt7621.c
+++ b/drivers/staging/mt7621-eth/gsw_mt7621.c
@@ -286,7 +286,6 @@ static int mt7621_gsw_remove(struct platform_device *pdev)
.remove = mt7621_gsw_remove,
.driver = {
.name = "mt7621-gsw",
- .owner = THIS_MODULE,
.of_match_table = mediatek_gsw_match,
},
};
diff --git a/drivers/staging/mt7621-eth/mtk_eth_soc.c
b/drivers/staging/mt7621-eth/mtk_eth_soc.c
index 7135075..363d3c9 100644
--- a/drivers/staging/mt7621-eth/mtk_eth_soc.c
+++ b/drivers/staging/mt7621-eth/mtk_eth_soc.c
@@ -2167,7 +2167,6 @@ static int mtk_remove(struct platform_device *pdev)
.remove = mtk_remove,
.driver = {
.name = "mtk_soc_eth",
- .owner = THIS_MODULE,
.of_match_table = of_mtk_match,
},
};
--
1.7.12.4
[PATCH] staging: remove unneeded static set .owner field in platform_driver
platform_driver_register will set the .owner field. So it is safe
to remove the redundant assignment.
The issue is detected with the help of Coccinelle.
Signed-off-by: zhong jiang
---
drivers/staging/greybus/audio_codec.c| 1 -
drivers/staging/mt7621-eth/gsw_mt7621.c | 1 -
drivers/staging/mt7621-eth/mtk_eth_soc.c | 1 -
3 files changed, 3 deletions(-)
diff --git a/drivers/staging/greybus/audio_codec.c
b/drivers/staging/greybus/audio_codec.c
index 35acd55..08746c8 100644
--- a/drivers/staging/greybus/audio_codec.c
+++ b/drivers/staging/greybus/audio_codec.c
@@ -1087,7 +1087,6 @@ static int gbaudio_codec_remove(struct platform_device
*pdev)
static struct platform_driver gbaudio_codec_driver = {
.driver = {
.name = "apb-dummy-codec",
- .owner = THIS_MODULE,
#ifdef CONFIG_PM
.pm = _codec_pm_ops,
#endif
diff --git a/drivers/staging/mt7621-eth/gsw_mt7621.c
b/drivers/staging/mt7621-eth/gsw_mt7621.c
index 2c07b55..53767b1 100644
--- a/drivers/staging/mt7621-eth/gsw_mt7621.c
+++ b/drivers/staging/mt7621-eth/gsw_mt7621.c
@@ -286,7 +286,6 @@ static int mt7621_gsw_remove(struct platform_device *pdev)
.remove = mt7621_gsw_remove,
.driver = {
.name = "mt7621-gsw",
- .owner = THIS_MODULE,
.of_match_table = mediatek_gsw_match,
},
};
diff --git a/drivers/staging/mt7621-eth/mtk_eth_soc.c
b/drivers/staging/mt7621-eth/mtk_eth_soc.c
index 7135075..363d3c9 100644
--- a/drivers/staging/mt7621-eth/mtk_eth_soc.c
+++ b/drivers/staging/mt7621-eth/mtk_eth_soc.c
@@ -2167,7 +2167,6 @@ static int mtk_remove(struct platform_device *pdev)
.remove = mtk_remove,
.driver = {
.name = "mtk_soc_eth",
- .owner = THIS_MODULE,
.of_match_table = of_mtk_match,
},
};
--
1.7.12.4
[PATCH] sparc: vdso: clean-up vdso Makefile
arch/sparc/vdso/Makefile is a replica of arch/x86/entry/vdso/Makefile.
Clean-up the Makefile in the same way as I did for x86:
- Remove unnecessary export
- Put the generated linker script to $(obj)/ instead of $(src)/
- Simplify cmd_vdso2c
The corresponding x86 commits are:
- 61615faf0a89 ("x86/build/vdso: Remove unnecessary export in Makefile")
- 1742ed2088cc ("x86/build/vdso: Put generated linker scripts to $(obj)/")
- c5fcdbf15523 ("x86/build/vdso: Simplify 'cmd_vdso2c'")
Signed-off-by: Masahiro Yamada
---
arch/sparc/vdso/Makefile | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/arch/sparc/vdso/Makefile b/arch/sparc/vdso/Makefile
index dd0b5a9..dc85570 100644
--- a/arch/sparc/vdso/Makefile
+++ b/arch/sparc/vdso/Makefile
@@ -31,23 +31,21 @@ obj-y += $(vdso_img_objs)
targets += $(vdso_img_cfiles)
targets += $(vdso_img_sodbg) $(vdso_img-y:%=vdso%.so)
-export CPPFLAGS_vdso.lds += -P -C
+CPPFLAGS_vdso.lds += -P -C
VDSO_LDFLAGS_vdso.lds = -m64 -Wl,-soname=linux-vdso.so.1 \
-Wl,--no-undefined \
-Wl,-z,max-page-size=8192 -Wl,-z,common-page-size=8192 \
$(DISABLE_LTO)
-$(obj)/vdso64.so.dbg: $(src)/vdso.lds $(vobjs) FORCE
+$(obj)/vdso64.so.dbg: $(obj)/vdso.lds $(vobjs) FORCE
$(call if_changed,vdso)
HOST_EXTRACFLAGS += -I$(srctree)/tools/include
hostprogs-y+= vdso2c
quiet_cmd_vdso2c = VDSO2C $@
-define cmd_vdso2c
- $(obj)/vdso2c $< $(<:%.dbg=%) $@
-endef
+ cmd_vdso2c = $(obj)/vdso2c $< $(<:%.dbg=%) $@
$(obj)/vdso-image-%.c: $(obj)/vdso%.so.dbg $(obj)/vdso%.so $(obj)/vdso2c FORCE
$(call if_changed,vdso2c)
--
2.7.4
[PATCH] sparc: vdso: clean-up vdso Makefile
arch/sparc/vdso/Makefile is a replica of arch/x86/entry/vdso/Makefile.
Clean-up the Makefile in the same way as I did for x86:
- Remove unnecessary export
- Put the generated linker script to $(obj)/ instead of $(src)/
- Simplify cmd_vdso2c
The corresponding x86 commits are:
- 61615faf0a89 ("x86/build/vdso: Remove unnecessary export in Makefile")
- 1742ed2088cc ("x86/build/vdso: Put generated linker scripts to $(obj)/")
- c5fcdbf15523 ("x86/build/vdso: Simplify 'cmd_vdso2c'")
Signed-off-by: Masahiro Yamada
---
arch/sparc/vdso/Makefile | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/arch/sparc/vdso/Makefile b/arch/sparc/vdso/Makefile
index dd0b5a9..dc85570 100644
--- a/arch/sparc/vdso/Makefile
+++ b/arch/sparc/vdso/Makefile
@@ -31,23 +31,21 @@ obj-y += $(vdso_img_objs)
targets += $(vdso_img_cfiles)
targets += $(vdso_img_sodbg) $(vdso_img-y:%=vdso%.so)
-export CPPFLAGS_vdso.lds += -P -C
+CPPFLAGS_vdso.lds += -P -C
VDSO_LDFLAGS_vdso.lds = -m64 -Wl,-soname=linux-vdso.so.1 \
-Wl,--no-undefined \
-Wl,-z,max-page-size=8192 -Wl,-z,common-page-size=8192 \
$(DISABLE_LTO)
-$(obj)/vdso64.so.dbg: $(src)/vdso.lds $(vobjs) FORCE
+$(obj)/vdso64.so.dbg: $(obj)/vdso.lds $(vobjs) FORCE
$(call if_changed,vdso)
HOST_EXTRACFLAGS += -I$(srctree)/tools/include
hostprogs-y+= vdso2c
quiet_cmd_vdso2c = VDSO2C $@
-define cmd_vdso2c
- $(obj)/vdso2c $< $(<:%.dbg=%) $@
-endef
+ cmd_vdso2c = $(obj)/vdso2c $< $(<:%.dbg=%) $@
$(obj)/vdso-image-%.c: $(obj)/vdso%.so.dbg $(obj)/vdso%.so $(obj)/vdso2c FORCE
$(call if_changed,vdso2c)
--
2.7.4
[PATCH] pstore: fix incorrect persistent ram buffer mapping
persistent_ram_vmap() returns the page start vaddr.
persistent_ram_iomap() supports non-page-aligned mapping.
persistent_ram_buffer_map() always adds offset-in-page to the vaddr
returned from these two functions, which causes incorrect mapping of
non-page-aligned persistent ram buffer.
Signed-off-by: Bin Yang
---
fs/pstore/ram_core.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c
index 951a14e..7c05fdd 100644
--- a/fs/pstore/ram_core.c
+++ b/fs/pstore/ram_core.c
@@ -429,7 +429,7 @@ static void *persistent_ram_vmap(phys_addr_t start, size_t
size,
vaddr = vmap(pages, page_count, VM_MAP, prot);
kfree(pages);
- return vaddr;
+ return vaddr + offset_in_page(start);
}
static void *persistent_ram_iomap(phys_addr_t start, size_t size,
@@ -468,7 +468,7 @@ static int persistent_ram_buffer_map(phys_addr_t start,
phys_addr_t size,
return -ENOMEM;
}
- prz->buffer = prz->vaddr + offset_in_page(start);
+ prz->buffer = prz->vaddr;
prz->buffer_size = size - sizeof(struct persistent_ram_buffer);
return 0;
@@ -515,7 +515,7 @@ void persistent_ram_free(struct persistent_ram_zone *prz)
if (prz->vaddr) {
if (pfn_valid(prz->paddr >> PAGE_SHIFT)) {
- vunmap(prz->vaddr);
+ vunmap(prz->vaddr - offset_in_page(prz->paddr));
} else {
iounmap(prz->vaddr);
release_mem_region(prz->paddr, prz->size);
--
2.7.4
[PATCH] pstore: fix incorrect persistent ram buffer mapping
persistent_ram_vmap() returns the page start vaddr.
persistent_ram_iomap() supports non-page-aligned mapping.
persistent_ram_buffer_map() always adds offset-in-page to the vaddr
returned from these two functions, which causes incorrect mapping of
non-page-aligned persistent ram buffer.
Signed-off-by: Bin Yang
---
fs/pstore/ram_core.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c
index 951a14e..7c05fdd 100644
--- a/fs/pstore/ram_core.c
+++ b/fs/pstore/ram_core.c
@@ -429,7 +429,7 @@ static void *persistent_ram_vmap(phys_addr_t start, size_t
size,
vaddr = vmap(pages, page_count, VM_MAP, prot);
kfree(pages);
- return vaddr;
+ return vaddr + offset_in_page(start);
}
static void *persistent_ram_iomap(phys_addr_t start, size_t size,
@@ -468,7 +468,7 @@ static int persistent_ram_buffer_map(phys_addr_t start,
phys_addr_t size,
return -ENOMEM;
}
- prz->buffer = prz->vaddr + offset_in_page(start);
+ prz->buffer = prz->vaddr;
prz->buffer_size = size - sizeof(struct persistent_ram_buffer);
return 0;
@@ -515,7 +515,7 @@ void persistent_ram_free(struct persistent_ram_zone *prz)
if (prz->vaddr) {
if (pfn_valid(prz->paddr >> PAGE_SHIFT)) {
- vunmap(prz->vaddr);
+ vunmap(prz->vaddr - offset_in_page(prz->paddr));
} else {
iounmap(prz->vaddr);
release_mem_region(prz->paddr, prz->size);
--
2.7.4
Re: [PATCH v2 2/9] nios2: build .dtb files in dts directory
On Fri, 2018-09-07 at 13:09 -0500, Rob Herring wrote: > On Thu, Sep 6, 2018 at 9:21 PM Ley Foon Tan > wrote: > > > > > > On Wed, 2018-09-05 at 18:53 -0500, Rob Herring wrote: > > > > > > Align nios2 with other architectures which build the dtb files in > > > the > > > same directory as the dts files. This is also in line with most > > > other > > > build targets which are located in the same directory as the > > > source. > > > This move will help enable the 'dtbs' target which builds all the > > > dtbs > > > regardless of kernel config. > > > > > > This transition could break some scripts if they expect dtb files > > > in > > > the old location. > > > > > > Cc: Ley Foon Tan > > > Cc: nios2-...@lists.rocketboards.org > > > Signed-off-by: Rob Herring > > > --- > > > Please ack so I can take the whole series via the DT tree. > > > > > > arch/nios2/Makefile | 4 ++-- > > > arch/nios2/boot/Makefile | 4 > > > arch/nios2/boot/dts/Makefile | 1 + > > > 3 files changed, 3 insertions(+), 6 deletions(-) > > > create mode 100644 arch/nios2/boot/dts/Makefile > > > > > > diff --git a/arch/nios2/Makefile b/arch/nios2/Makefile > > > index 8673a79dca9c..50eece1c6adb 100644 > > > --- a/arch/nios2/Makefile > > > +++ b/arch/nios2/Makefile > > > @@ -59,10 +59,10 @@ archclean: > > > $(Q)$(MAKE) $(clean)=$(nios2-boot) > > > > > > %.dtb: | scripts > > > - $(Q)$(MAKE) $(build)=$(nios2-boot) $(nios2-boot)/$@ > > > + $(Q)$(MAKE) $(build)=$(nios2-boot)/dts $(nios2- > > > boot)/dts/$@ > > > > > > dtbs: > > > - $(Q)$(MAKE) $(build)=$(nios2-boot) $(nios2-boot)/$@ > > > + $(Q)$(MAKE) $(build)=$(nios2-boot)/dts > > > > > > $(BOOT_TARGETS): vmlinux > > > $(Q)$(MAKE) $(build)=$(nios2-boot) $(nios2-boot)/$@ > > > diff --git a/arch/nios2/boot/Makefile b/arch/nios2/boot/Makefile > > > index 2ba23a679732..007586094dde 100644 > > > --- a/arch/nios2/boot/Makefile > > > +++ b/arch/nios2/boot/Makefile > > > @@ -47,10 +47,6 @@ obj-$(CONFIG_NIOS2_DTB_SOURCE_BOOL) += > > > linked_dtb.o > > > > > > targets += $(dtb-y) > > > > > > -# Rule to build device tree blobs with make command > > > -$(obj)/%.dtb: $(src)/dts/%.dts FORCE > > > - $(call if_changed_dep,dtc) > > > - > > > $(obj)/dtbs: $(addprefix $(obj)/, $(dtb-y)) > > > > > > install: > > > diff --git a/arch/nios2/boot/dts/Makefile > > > b/arch/nios2/boot/dts/Makefile > > > new file mode 100644 > > > index ..f66554cd5c45 > > > --- /dev/null > > > +++ b/arch/nios2/boot/dts/Makefile > > > @@ -0,0 +1 @@ > > > +# SPDX-License-Identifier: GPL-2.0 > > > -- > > > 2.17.1 > > > > > Hi Rob > > > > I have synced your all-dtbs branch from here: https://git.kernel.or > > g/pu > > b/scm/linux/kernel/git/robh/linux.git/log/?h=all-dtbs > > > > It shows error when compile kernel image and also when "make > > dtbs_install". > Can you fetch the branch again and try it. I fixed a few dependency > issues. > > > > > make dtbs_install > > make[1]: *** No rule to make target > > 'arch/nios2/boot/dts/arch/nios2/boot/dts/10m50_devboard.dtb', > > needed by > > 'arch/nios2/boot/dts/arch/nios2/boot/dts/10m50_devboard.dtb.S'. St > > op. > What is the value of CONFIG_NIOS2_DTB_SOURCE? As patch 3 notes, it > now > should not have any path. > > If that's a problem, I could take the basename to strip the path, but > then sub directories wouldn't work either. > > BTW, next up, I want to consolidate the config variables for built-in > dtbs. > Hi Rob CONFIG_NIOS2_DTB_SOURCE has the relative path to dts file, arch/nios2/boot/dts/arch/nios2/boot/dts/10m50_devboard.dts Change CONFIG_NIOS2_DTB_SOURCE=10m50_devboard.dtb.S fix the dtb build issue. Regards Ley Foon
Re: [PATCH v2 2/9] nios2: build .dtb files in dts directory
On Fri, 2018-09-07 at 13:09 -0500, Rob Herring wrote: > On Thu, Sep 6, 2018 at 9:21 PM Ley Foon Tan > wrote: > > > > > > On Wed, 2018-09-05 at 18:53 -0500, Rob Herring wrote: > > > > > > Align nios2 with other architectures which build the dtb files in > > > the > > > same directory as the dts files. This is also in line with most > > > other > > > build targets which are located in the same directory as the > > > source. > > > This move will help enable the 'dtbs' target which builds all the > > > dtbs > > > regardless of kernel config. > > > > > > This transition could break some scripts if they expect dtb files > > > in > > > the old location. > > > > > > Cc: Ley Foon Tan > > > Cc: nios2-...@lists.rocketboards.org > > > Signed-off-by: Rob Herring > > > --- > > > Please ack so I can take the whole series via the DT tree. > > > > > > arch/nios2/Makefile | 4 ++-- > > > arch/nios2/boot/Makefile | 4 > > > arch/nios2/boot/dts/Makefile | 1 + > > > 3 files changed, 3 insertions(+), 6 deletions(-) > > > create mode 100644 arch/nios2/boot/dts/Makefile > > > > > > diff --git a/arch/nios2/Makefile b/arch/nios2/Makefile > > > index 8673a79dca9c..50eece1c6adb 100644 > > > --- a/arch/nios2/Makefile > > > +++ b/arch/nios2/Makefile > > > @@ -59,10 +59,10 @@ archclean: > > > $(Q)$(MAKE) $(clean)=$(nios2-boot) > > > > > > %.dtb: | scripts > > > - $(Q)$(MAKE) $(build)=$(nios2-boot) $(nios2-boot)/$@ > > > + $(Q)$(MAKE) $(build)=$(nios2-boot)/dts $(nios2- > > > boot)/dts/$@ > > > > > > dtbs: > > > - $(Q)$(MAKE) $(build)=$(nios2-boot) $(nios2-boot)/$@ > > > + $(Q)$(MAKE) $(build)=$(nios2-boot)/dts > > > > > > $(BOOT_TARGETS): vmlinux > > > $(Q)$(MAKE) $(build)=$(nios2-boot) $(nios2-boot)/$@ > > > diff --git a/arch/nios2/boot/Makefile b/arch/nios2/boot/Makefile > > > index 2ba23a679732..007586094dde 100644 > > > --- a/arch/nios2/boot/Makefile > > > +++ b/arch/nios2/boot/Makefile > > > @@ -47,10 +47,6 @@ obj-$(CONFIG_NIOS2_DTB_SOURCE_BOOL) += > > > linked_dtb.o > > > > > > targets += $(dtb-y) > > > > > > -# Rule to build device tree blobs with make command > > > -$(obj)/%.dtb: $(src)/dts/%.dts FORCE > > > - $(call if_changed_dep,dtc) > > > - > > > $(obj)/dtbs: $(addprefix $(obj)/, $(dtb-y)) > > > > > > install: > > > diff --git a/arch/nios2/boot/dts/Makefile > > > b/arch/nios2/boot/dts/Makefile > > > new file mode 100644 > > > index ..f66554cd5c45 > > > --- /dev/null > > > +++ b/arch/nios2/boot/dts/Makefile > > > @@ -0,0 +1 @@ > > > +# SPDX-License-Identifier: GPL-2.0 > > > -- > > > 2.17.1 > > > > > Hi Rob > > > > I have synced your all-dtbs branch from here: https://git.kernel.or > > g/pu > > b/scm/linux/kernel/git/robh/linux.git/log/?h=all-dtbs > > > > It shows error when compile kernel image and also when "make > > dtbs_install". > Can you fetch the branch again and try it. I fixed a few dependency > issues. > > > > > make dtbs_install > > make[1]: *** No rule to make target > > 'arch/nios2/boot/dts/arch/nios2/boot/dts/10m50_devboard.dtb', > > needed by > > 'arch/nios2/boot/dts/arch/nios2/boot/dts/10m50_devboard.dtb.S'. St > > op. > What is the value of CONFIG_NIOS2_DTB_SOURCE? As patch 3 notes, it > now > should not have any path. > > If that's a problem, I could take the basename to strip the path, but > then sub directories wouldn't work either. > > BTW, next up, I want to consolidate the config variables for built-in > dtbs. > Hi Rob CONFIG_NIOS2_DTB_SOURCE has the relative path to dts file, arch/nios2/boot/dts/arch/nios2/boot/dts/10m50_devboard.dts Change CONFIG_NIOS2_DTB_SOURCE=10m50_devboard.dtb.S fix the dtb build issue. Regards Ley Foon
Re: [PATCH v2 2/3] x86/mm/KASLR: Calculate the actual size of vmemmap region
On 09/11/18 at 08:08pm, Baoquan He wrote: > On 09/11/18 at 11:28am, Ingo Molnar wrote: > > Yeah, so proper context is still missing, this paragraph appears to assume > > from the reader a > > whole lot of prior knowledge, and this is one of the top comments in > > kaslr.c so there's nowhere > > else to go read about the background. > > > > For example what is the range of randomization of each region? Assuming the > > static, > > non-randomized description in Documentation/x86/x86_64/mm.txt is correct, > > in what way does > > KASLR modify that layout? Re-read this paragraph, found I missed saying the range for each memory region, and in what way KASLR modify the layout. > > > > All of this is very opaque and not explained very well anywhere that I > > could find. We need to > > generate a proper description ASAP. > > OK, let me try to give an context with my understanding. And copy the > static layout of memory regions at below for reference. > Here, Documentation/x86/x86_64/mm.txt is correct, and it's the guideline for us to manipulate the layout of kernel memory regions. Originally the starting address of each region is aligned to 512GB so that they are all mapped at the 0-th entry of PGD table in 4-level page mapping. Since we are so rich to have 120 TB virtual address space, they are aligned at 1 TB actually. So randomness comes from three parts mainly: 1) The direct mapping region for physical memory. 64 TB are reserved to cover the maximum physical memory support. However, most of systems only have much less RAM memory than 64 TB, even much less than 1 TB most of time. We can take the superfluous to join the randomization. This is often the biggest part. 2) The hole between memory regions, even though they are only 1 TB. 3) KASAN region takes up 16 TB, while it won't take effect when KASLR is enabled. This is another big part. As you can see, in these three memory regions, the physical memory mapping region has variable size according to the existing system RAM. However, the remaining two memory regions have fixed size, vmalloc is 32 TB, vmemmap is 1 TB. With this superfluous address space as well as changing the starting address of each memory region to be PUD level, namely 1 GB aligned, we can have thousands of candidate position to locate those three memory regions. Above is for 4-level paging mode . As for 5-level, since the virtual address space is too big, Kirill makes the starting address of regions P4D aligned, namely 512 GB. When randomize the layout, their order are kept, still the physical memory mapping region is handled fistly, next vmalloc and vmemmap. Let's take the physical memory mapping region as example, we limit the starting address to be taken from the 1st 1/3 part of the whole available virtual address space which is from 0x8800 to 0xfe00, namely the original starting address of the physical memory mapping region to the starting address of cpu_entry_area mapping region. Once a random address is chosen for the physical memory mapping, we jump over the region and add 1G to begin the next region handling with the remaining available space. ~~ 8800 - c7ff (=64 TB) direct mapping of all phys. memory 136T - 200T = 64TB c800 - c8ff (=40 bits) hole 200T - 201T = 1TB c900 - e8ff (=45 bits) vmalloc/ioremap space 201T - 233T = 32TB e900 - e9ff (=40 bits) hole 233T - 234T = 1TB ea00 - eaff (=40 bits) virtual memory map (1TB) 234T - 235T = 1TB ... unused hole ... ec00 - fbff (=44 bits) kasan shadow memory (16TB) 236T - 252T = 16TB ... unused hole ... vaddr_end for KASLR fe00 - fe7f (=39 bits) cpu_entry_area mapping 254T - 254T+512G Thanks Baoquan
Re: [PATCH v2 2/3] x86/mm/KASLR: Calculate the actual size of vmemmap region
On 09/11/18 at 08:08pm, Baoquan He wrote: > On 09/11/18 at 11:28am, Ingo Molnar wrote: > > Yeah, so proper context is still missing, this paragraph appears to assume > > from the reader a > > whole lot of prior knowledge, and this is one of the top comments in > > kaslr.c so there's nowhere > > else to go read about the background. > > > > For example what is the range of randomization of each region? Assuming the > > static, > > non-randomized description in Documentation/x86/x86_64/mm.txt is correct, > > in what way does > > KASLR modify that layout? Re-read this paragraph, found I missed saying the range for each memory region, and in what way KASLR modify the layout. > > > > All of this is very opaque and not explained very well anywhere that I > > could find. We need to > > generate a proper description ASAP. > > OK, let me try to give an context with my understanding. And copy the > static layout of memory regions at below for reference. > Here, Documentation/x86/x86_64/mm.txt is correct, and it's the guideline for us to manipulate the layout of kernel memory regions. Originally the starting address of each region is aligned to 512GB so that they are all mapped at the 0-th entry of PGD table in 4-level page mapping. Since we are so rich to have 120 TB virtual address space, they are aligned at 1 TB actually. So randomness comes from three parts mainly: 1) The direct mapping region for physical memory. 64 TB are reserved to cover the maximum physical memory support. However, most of systems only have much less RAM memory than 64 TB, even much less than 1 TB most of time. We can take the superfluous to join the randomization. This is often the biggest part. 2) The hole between memory regions, even though they are only 1 TB. 3) KASAN region takes up 16 TB, while it won't take effect when KASLR is enabled. This is another big part. As you can see, in these three memory regions, the physical memory mapping region has variable size according to the existing system RAM. However, the remaining two memory regions have fixed size, vmalloc is 32 TB, vmemmap is 1 TB. With this superfluous address space as well as changing the starting address of each memory region to be PUD level, namely 1 GB aligned, we can have thousands of candidate position to locate those three memory regions. Above is for 4-level paging mode . As for 5-level, since the virtual address space is too big, Kirill makes the starting address of regions P4D aligned, namely 512 GB. When randomize the layout, their order are kept, still the physical memory mapping region is handled fistly, next vmalloc and vmemmap. Let's take the physical memory mapping region as example, we limit the starting address to be taken from the 1st 1/3 part of the whole available virtual address space which is from 0x8800 to 0xfe00, namely the original starting address of the physical memory mapping region to the starting address of cpu_entry_area mapping region. Once a random address is chosen for the physical memory mapping, we jump over the region and add 1G to begin the next region handling with the remaining available space. ~~ 8800 - c7ff (=64 TB) direct mapping of all phys. memory 136T - 200T = 64TB c800 - c8ff (=40 bits) hole 200T - 201T = 1TB c900 - e8ff (=45 bits) vmalloc/ioremap space 201T - 233T = 32TB e900 - e9ff (=40 bits) hole 233T - 234T = 1TB ea00 - eaff (=40 bits) virtual memory map (1TB) 234T - 235T = 1TB ... unused hole ... ec00 - fbff (=44 bits) kasan shadow memory (16TB) 236T - 252T = 16TB ... unused hole ... vaddr_end for KASLR fe00 - fe7f (=39 bits) cpu_entry_area mapping 254T - 254T+512G Thanks Baoquan
Re: [PATCH] perf test: Add watchpoint test
> While testing, I got curious, as a 'perf test' user, why one of the
> tests had the "Skip" result:
>
> [root@seventh ~]# perf test watchpoint
> 22: Watchpoint:
> 22.1: Read Only Watchpoint: Skip
> 22.2: Write Only Watchpoint : Ok
> 22.3: Read / Write Watchpoint : Ok
> 22.4: Modify Watchpoint : Ok
> [root@seventh ~]#
>
> I tried with 'perf test -v watchpoint' but that didn't help, perhaps you
> could add some message after the "Skip" telling why it skipped that
> test? I.e. hardware doesn't have that capability, kernel driver not yet
> supporting that, something else?
Sure will add a message:
pr_debug("Hardware does not support read only watchpoints.");
Ravi
Re: [PATCH] perf test: Add watchpoint test
> While testing, I got curious, as a 'perf test' user, why one of the
> tests had the "Skip" result:
>
> [root@seventh ~]# perf test watchpoint
> 22: Watchpoint:
> 22.1: Read Only Watchpoint: Skip
> 22.2: Write Only Watchpoint : Ok
> 22.3: Read / Write Watchpoint : Ok
> 22.4: Modify Watchpoint : Ok
> [root@seventh ~]#
>
> I tried with 'perf test -v watchpoint' but that didn't help, perhaps you
> could add some message after the "Skip" telling why it skipped that
> test? I.e. hardware doesn't have that capability, kernel driver not yet
> supporting that, something else?
Sure will add a message:
pr_debug("Hardware does not support read only watchpoints.");
Ravi
Re: [PATCH v2 1/4] arm64: dts: rockchip: Split out common nodes for Rock960 based boards
Hi Ezequiel,
On Tue, Sep 11, 2018 at 04:40:29PM -0300, Ezequiel Garcia wrote:
> On Tue, 2018-09-11 at 08:00 +0530, Manivannan Sadhasivam wrote:
> > Since the same family members of Rock960 boards (Rock960 and Ficus)
> > share the same configuration, split out the common nodes into a common
> > dtsi file for reducing code duplication. The board specific nodes for
> > Ficus boards are then placed in corresponding board DTS file.
> >
>
> I think it should be possible to move the common USB nodes to the dtsi file,
> and keep the board-specific (phy-supply property) in the dts files:
>
> _host {
> phy-supply = <_host>;
> };
>
> _host {
> phy-supply = <_host>;
> };
>
We can do that but my intention was to entirely partition the nodes
which are not common. So that it would be less confusing when someone
looks at it (please correct me if I'm wrong).
> Also, I believe it would be good to have some more details
> in this commit log. The information on the cover letter is great,
> so I'd just repeat some of that here.
>
Sure, will add it in next iteration.
> Other than that, for the ficus bits:
>
> Reviewed-by: Ezequiel Garcia
>
Thanks a lot for the review!
Regards,
Mani
> Thanks very much for this work!
> Ezequiel
>
>
> > Signed-off-by: Manivannan Sadhasivam
> > ---
> > arch/arm64/boot/dts/rockchip/rk3399-ficus.dts | 429 +
> > .../boot/dts/rockchip/rk3399-rock960.dtsi | 439 ++
> > 2 files changed, 440 insertions(+), 428 deletions(-)
> > create mode 100644 arch/arm64/boot/dts/rockchip/rk3399-rock960.dtsi
> >
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3399-ficus.dts
> > b/arch/arm64/boot/dts/rockchip/rk3399-ficus.dts
> > index 8978d924eb83..7f6ec37d5a69 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3399-ficus.dts
> > +++ b/arch/arm64/boot/dts/rockchip/rk3399-ficus.dts
> > @@ -7,8 +7,7 @@
> > */
> >
> > /dts-v1/;
> > -#include "rk3399.dtsi"
> > -#include "rk3399-opp.dtsi"
> > +#include "rk3399-rock960.dtsi"
> >
> > / {
> > model = "96boards RK3399 Ficus";
> > @@ -25,31 +24,6 @@
> > #clock-cells = <0>;
> > };
> >
> > - vcc1v8_s0: vcc1v8-s0 {
> > - compatible = "regulator-fixed";
> > - regulator-name = "vcc1v8_s0";
> > - regulator-min-microvolt = <180>;
> > - regulator-max-microvolt = <180>;
> > - regulator-always-on;
> > - };
> > -
> > - vcc_sys: vcc-sys {
> > - compatible = "regulator-fixed";
> > - regulator-name = "vcc_sys";
> > - regulator-min-microvolt = <500>;
> > - regulator-max-microvolt = <500>;
> > - regulator-always-on;
> > - };
> > -
> > - vcc3v3_sys: vcc3v3-sys {
> > - compatible = "regulator-fixed";
> > - regulator-name = "vcc3v3_sys";
> > - regulator-min-microvolt = <330>;
> > - regulator-max-microvolt = <330>;
> > - regulator-always-on;
> > - vin-supply = <_sys>;
> > - };
> > -
> > vcc3v3_pcie: vcc3v3-pcie-regulator {
> > compatible = "regulator-fixed";
> > enable-active-high;
> > @@ -75,46 +49,6 @@
> > regulator-always-on;
> > vin-supply = <_sys>;
> > };
> > -
> > - vdd_log: vdd-log {
> > - compatible = "pwm-regulator";
> > - pwms = < 0 25000 0>;
> > - regulator-name = "vdd_log";
> > - regulator-min-microvolt = <80>;
> > - regulator-max-microvolt = <140>;
> > - regulator-always-on;
> > - regulator-boot-on;
> > - vin-supply = <_sys>;
> > - };
> > -
> > -};
> > -
> > -_l0 {
> > - cpu-supply = <_cpu_l>;
> > -};
> > -
> > -_l1 {
> > - cpu-supply = <_cpu_l>;
> > -};
> > -
> > -_l2 {
> > - cpu-supply = <_cpu_l>;
> > -};
> > -
> > -_l3 {
> > - cpu-supply = <_cpu_l>;
> > -};
> > -
> > -_b0 {
> > - cpu-supply = <_cpu_b>;
> > -};
> > -
> > -_b1 {
> > - cpu-supply = <_cpu_b>;
> > -};
> > -
> > -_phy {
> > - status = "okay";
> > };
> >
> > {
> > @@ -133,263 +67,6 @@
> > status = "okay";
> > };
> >
> > - {
> > - ddc-i2c-bus = <>;
> > - pinctrl-names = "default";
> > - pinctrl-0 = <_cec>;
> > - status = "okay";
> > -};
> > -
> > - {
> > - clock-frequency = <40>;
> > - i2c-scl-rising-time-ns = <168>;
> > - i2c-scl-falling-time-ns = <4>;
> > - status = "okay";
> > -
> > - vdd_cpu_b: regulator@40 {
> > - compatible = "silergy,syr827";
> > - reg = <0x40>;
> > - fcs,suspend-voltage-selector = <1>;
> > - regulator-name = "vdd_cpu_b";
> > - regulator-min-microvolt = <712500>;
> > - regulator-max-microvolt = <150>;
> > - regulator-ramp-delay = <1000>;
> > - regulator-always-on;
> > - regulator-boot-on;
> > - vin-supply = <_sys>;
> > - status = "okay";
> > -
> > - regulator-state-mem {
> > -
Re: [PATCH v2 1/4] arm64: dts: rockchip: Split out common nodes for Rock960 based boards
Hi Ezequiel,
On Tue, Sep 11, 2018 at 04:40:29PM -0300, Ezequiel Garcia wrote:
> On Tue, 2018-09-11 at 08:00 +0530, Manivannan Sadhasivam wrote:
> > Since the same family members of Rock960 boards (Rock960 and Ficus)
> > share the same configuration, split out the common nodes into a common
> > dtsi file for reducing code duplication. The board specific nodes for
> > Ficus boards are then placed in corresponding board DTS file.
> >
>
> I think it should be possible to move the common USB nodes to the dtsi file,
> and keep the board-specific (phy-supply property) in the dts files:
>
> _host {
> phy-supply = <_host>;
> };
>
> _host {
> phy-supply = <_host>;
> };
>
We can do that but my intention was to entirely partition the nodes
which are not common. So that it would be less confusing when someone
looks at it (please correct me if I'm wrong).
> Also, I believe it would be good to have some more details
> in this commit log. The information on the cover letter is great,
> so I'd just repeat some of that here.
>
Sure, will add it in next iteration.
> Other than that, for the ficus bits:
>
> Reviewed-by: Ezequiel Garcia
>
Thanks a lot for the review!
Regards,
Mani
> Thanks very much for this work!
> Ezequiel
>
>
> > Signed-off-by: Manivannan Sadhasivam
> > ---
> > arch/arm64/boot/dts/rockchip/rk3399-ficus.dts | 429 +
> > .../boot/dts/rockchip/rk3399-rock960.dtsi | 439 ++
> > 2 files changed, 440 insertions(+), 428 deletions(-)
> > create mode 100644 arch/arm64/boot/dts/rockchip/rk3399-rock960.dtsi
> >
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3399-ficus.dts
> > b/arch/arm64/boot/dts/rockchip/rk3399-ficus.dts
> > index 8978d924eb83..7f6ec37d5a69 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3399-ficus.dts
> > +++ b/arch/arm64/boot/dts/rockchip/rk3399-ficus.dts
> > @@ -7,8 +7,7 @@
> > */
> >
> > /dts-v1/;
> > -#include "rk3399.dtsi"
> > -#include "rk3399-opp.dtsi"
> > +#include "rk3399-rock960.dtsi"
> >
> > / {
> > model = "96boards RK3399 Ficus";
> > @@ -25,31 +24,6 @@
> > #clock-cells = <0>;
> > };
> >
> > - vcc1v8_s0: vcc1v8-s0 {
> > - compatible = "regulator-fixed";
> > - regulator-name = "vcc1v8_s0";
> > - regulator-min-microvolt = <180>;
> > - regulator-max-microvolt = <180>;
> > - regulator-always-on;
> > - };
> > -
> > - vcc_sys: vcc-sys {
> > - compatible = "regulator-fixed";
> > - regulator-name = "vcc_sys";
> > - regulator-min-microvolt = <500>;
> > - regulator-max-microvolt = <500>;
> > - regulator-always-on;
> > - };
> > -
> > - vcc3v3_sys: vcc3v3-sys {
> > - compatible = "regulator-fixed";
> > - regulator-name = "vcc3v3_sys";
> > - regulator-min-microvolt = <330>;
> > - regulator-max-microvolt = <330>;
> > - regulator-always-on;
> > - vin-supply = <_sys>;
> > - };
> > -
> > vcc3v3_pcie: vcc3v3-pcie-regulator {
> > compatible = "regulator-fixed";
> > enable-active-high;
> > @@ -75,46 +49,6 @@
> > regulator-always-on;
> > vin-supply = <_sys>;
> > };
> > -
> > - vdd_log: vdd-log {
> > - compatible = "pwm-regulator";
> > - pwms = < 0 25000 0>;
> > - regulator-name = "vdd_log";
> > - regulator-min-microvolt = <80>;
> > - regulator-max-microvolt = <140>;
> > - regulator-always-on;
> > - regulator-boot-on;
> > - vin-supply = <_sys>;
> > - };
> > -
> > -};
> > -
> > -_l0 {
> > - cpu-supply = <_cpu_l>;
> > -};
> > -
> > -_l1 {
> > - cpu-supply = <_cpu_l>;
> > -};
> > -
> > -_l2 {
> > - cpu-supply = <_cpu_l>;
> > -};
> > -
> > -_l3 {
> > - cpu-supply = <_cpu_l>;
> > -};
> > -
> > -_b0 {
> > - cpu-supply = <_cpu_b>;
> > -};
> > -
> > -_b1 {
> > - cpu-supply = <_cpu_b>;
> > -};
> > -
> > -_phy {
> > - status = "okay";
> > };
> >
> > {
> > @@ -133,263 +67,6 @@
> > status = "okay";
> > };
> >
> > - {
> > - ddc-i2c-bus = <>;
> > - pinctrl-names = "default";
> > - pinctrl-0 = <_cec>;
> > - status = "okay";
> > -};
> > -
> > - {
> > - clock-frequency = <40>;
> > - i2c-scl-rising-time-ns = <168>;
> > - i2c-scl-falling-time-ns = <4>;
> > - status = "okay";
> > -
> > - vdd_cpu_b: regulator@40 {
> > - compatible = "silergy,syr827";
> > - reg = <0x40>;
> > - fcs,suspend-voltage-selector = <1>;
> > - regulator-name = "vdd_cpu_b";
> > - regulator-min-microvolt = <712500>;
> > - regulator-max-microvolt = <150>;
> > - regulator-ramp-delay = <1000>;
> > - regulator-always-on;
> > - regulator-boot-on;
> > - vin-supply = <_sys>;
> > - status = "okay";
> > -
> > - regulator-state-mem {
> > -
Re: [PATCH] perf test: Add watchpoint test
On 09/10/2018 11:01 PM, Arnaldo Carvalho de Melo wrote: > Em Mon, Sep 10, 2018 at 11:18:30AM -0300, Arnaldo Carvalho de Melo escreveu: >> Em Mon, Sep 10, 2018 at 10:47:54AM -0300, Arnaldo Carvalho de Melo escreveu: >>> Em Mon, Sep 10, 2018 at 12:31:54PM +0200, Jiri Olsa escreveu: On Mon, Sep 10, 2018 at 03:28:11PM +0530, Ravi Bangoria wrote: > Ex on powerpc: > $ sudo ./perf test 22 > 22: Watchpoint: > 22.1: Read Only Watchpoint: Ok > 22.2: Write Only Watchpoint : Ok > 22.3: Read / Write Watchpoint : Ok > 22.4: Modify Watchpoint : Ok > cool, thanks! > Acked-by: Jiri Olsa > >>> Thanks, applied. > > Oops, fails when cross-building it to mips, I'll try to fix after lunch: Sorry for bit late reply. Will send v2 with the fix. Thanks Ravi > > 18 109.48 debian:experimental : Ok gcc (Debian 8.2.0-4) 8.2.0 > 1942.66 debian:experimental-x-arm64 : Ok aarch64-linux-gnu-gcc > (Debian 8.1.0-12) 8.1.0 > 2022.33 debian:experimental-x-mips: FAIL mips-linux-gnu-gcc (Debian > 8.1.0-12) 8.1.0 > 2120.05 debian:experimental-x-mips64 : FAIL mips64-linux-gnuabi64-gcc > (Debian 8.1.0-12) 8.1.0 > 2222.85 debian:experimental-x-mipsel : FAIL mipsel-linux-gnu-gcc > (Debian 8.1.0-12) 8.1.0 > > CC /tmp/build/perf/tests/bp_account.o > CC /tmp/build/perf/tests/wp.o > tests/wp.c:5:10: fatal error: arch-tests.h: No such file or directory > #include "arch-tests.h" > ^~ > compilation terminated. > mv: cannot stat '/tmp/build/perf/tests/.wp.o.tmp': No such file or directory > make[4]: *** [/git/linux/tools/build/Makefile.build:97: > /tmp/build/perf/tests/wp.o] Error 1 > make[4]: *** Waiting for unfinished jobs > CC /tmp/build/perf/util/record.o > CC /tmp/build/perf/util/srcline.o > make[3]: *** [/git/linux/tools/build/Makefile.build:139: tests] Error 2 > make[2]: *** [Makefile.perf:507: /tmp/build/perf/perf-in.o] Error 2 > make[2]: *** Waiting for unfinished jobs >
Re: [PATCH] perf test: Add watchpoint test
On 09/10/2018 11:01 PM, Arnaldo Carvalho de Melo wrote: > Em Mon, Sep 10, 2018 at 11:18:30AM -0300, Arnaldo Carvalho de Melo escreveu: >> Em Mon, Sep 10, 2018 at 10:47:54AM -0300, Arnaldo Carvalho de Melo escreveu: >>> Em Mon, Sep 10, 2018 at 12:31:54PM +0200, Jiri Olsa escreveu: On Mon, Sep 10, 2018 at 03:28:11PM +0530, Ravi Bangoria wrote: > Ex on powerpc: > $ sudo ./perf test 22 > 22: Watchpoint: > 22.1: Read Only Watchpoint: Ok > 22.2: Write Only Watchpoint : Ok > 22.3: Read / Write Watchpoint : Ok > 22.4: Modify Watchpoint : Ok > cool, thanks! > Acked-by: Jiri Olsa > >>> Thanks, applied. > > Oops, fails when cross-building it to mips, I'll try to fix after lunch: Sorry for bit late reply. Will send v2 with the fix. Thanks Ravi > > 18 109.48 debian:experimental : Ok gcc (Debian 8.2.0-4) 8.2.0 > 1942.66 debian:experimental-x-arm64 : Ok aarch64-linux-gnu-gcc > (Debian 8.1.0-12) 8.1.0 > 2022.33 debian:experimental-x-mips: FAIL mips-linux-gnu-gcc (Debian > 8.1.0-12) 8.1.0 > 2120.05 debian:experimental-x-mips64 : FAIL mips64-linux-gnuabi64-gcc > (Debian 8.1.0-12) 8.1.0 > 2222.85 debian:experimental-x-mipsel : FAIL mipsel-linux-gnu-gcc > (Debian 8.1.0-12) 8.1.0 > > CC /tmp/build/perf/tests/bp_account.o > CC /tmp/build/perf/tests/wp.o > tests/wp.c:5:10: fatal error: arch-tests.h: No such file or directory > #include "arch-tests.h" > ^~ > compilation terminated. > mv: cannot stat '/tmp/build/perf/tests/.wp.o.tmp': No such file or directory > make[4]: *** [/git/linux/tools/build/Makefile.build:97: > /tmp/build/perf/tests/wp.o] Error 1 > make[4]: *** Waiting for unfinished jobs > CC /tmp/build/perf/util/record.o > CC /tmp/build/perf/util/srcline.o > make[3]: *** [/git/linux/tools/build/Makefile.build:139: tests] Error 2 > make[2]: *** [Makefile.perf:507: /tmp/build/perf/perf-in.o] Error 2 > make[2]: *** Waiting for unfinished jobs >
Re: [PATCH v2 2/2] dmaengine: uniphier-mdmac: add UniPhier MIO DMAC driver
Hi Vinod,
2018-09-11 16:00 GMT+09:00 Vinod :
> On 24-08-18, 10:41, Masahiro Yamada wrote:
>
>> +/* mc->vc.lock must be held by caller */
>> +static u32 __uniphier_mdmac_get_residue(struct uniphier_mdmac_desc *md)
>> +{
>> + u32 residue = 0;
>> + int i;
>> +
>> + for (i = md->sg_cur; i < md->sg_len; i++)
>> + residue += sg_dma_len(>sgl[i]);
>
> so if the descriptor is submitted to hardware, we return the descriptor
> length, which is not correct.
>
> Two cases are required to be handled:
> 1. Descriptor is in queue (IMO above logic is fine for that, but it can
> be calculated at descriptor submit and looked up here)
Where do you want it to be calculated?
This hardware provides only simple registers (address and size)
for one-shot transfer instead of descriptors.
So, I used sgl as-is because I did not see a good reason
to transform sgl to another data structure.
> 2. Descriptor is running (interesting case), you need to read current
> register and offset that from descriptor length and return
OK, I will read out the register value
to retrieve the residue from the on-flight transfer.
>> +static struct dma_async_tx_descriptor *uniphier_mdmac_prep_slave_sg(
>> + struct dma_chan *chan,
>> + struct scatterlist *sgl,
>> + unsigned int sg_len,
>> + enum dma_transfer_direction direction,
>> + unsigned long flags, void *context)
>> +{
>> + struct virt_dma_chan *vc = to_virt_chan(chan);
>> + struct uniphier_mdmac_desc *md;
>> +
>> + if (!is_slave_direction(direction))
>> + return NULL;
>> +
>> + md = kzalloc(sizeof(*md), GFP_KERNEL);
>
> _prep calls can be invoked from atomic context, so this should be
> GFP_NOWAIT, see Documentation/driver-api/dmaengine/provider.rst
Will fix.
>> + if (!md)
>> + return NULL;
>> +
>> + md->sgl = sgl;
>> + md->sg_len = sg_len;
>> + md->dir = direction;
>> +
>> + return vchan_tx_prep(vc, >vd, flags);
>
> this seems missing stuff. Where do you do register calculation for the
> descriptor and where is slave_config here, how do you know where to
> send/receive data form/to (peripheral)
This dmac is really simple, and un-flexible.
The peripheral address to send/receive data from/to is hard-weird.
cfg->{src_addr,dst_addr} is not configurable.
Look at __uniphier_mdmac_handle().
'dest_addr' and 'src_addr' must be set to 0 for the peripheral.
>> +static enum dma_status uniphier_mdmac_tx_status(struct dma_chan *chan,
>> + dma_cookie_t cookie,
>> + struct dma_tx_state *txstate)
>> +{
>> + struct virt_dma_chan *vc;
>> + struct virt_dma_desc *vd;
>> + struct uniphier_mdmac_chan *mc;
>> + struct uniphier_mdmac_desc *md = NULL;
>> + enum dma_status stat;
>> + unsigned long flags;
>> +
>> + stat = dma_cookie_status(chan, cookie, txstate);
>> + if (stat == DMA_COMPLETE)
>> + return stat;
>> +
>> + vc = to_virt_chan(chan);
>> +
>> + spin_lock_irqsave(>lock, flags);
>> +
>> + mc = to_uniphier_mdmac_chan(vc);
>> +
>> + if (mc->md && mc->md->vd.tx.cookie == cookie)
>> + md = mc->md;
>> +
>> + if (!md) {
>> + vd = vchan_find_desc(vc, cookie);
>> + if (vd)
>> + md = to_uniphier_mdmac_desc(vd);
>> + }
>> +
>> + if (md)
>> + txstate->residue = __uniphier_mdmac_get_residue(md);
>
> txstate can be NULL and should be checked...
Will fix.
>> +static int uniphier_mdmac_probe(struct platform_device *pdev)
>> +{
>> + struct device *dev = >dev;
>> + struct uniphier_mdmac_device *mdev;
>> + struct dma_device *ddev;
>> + struct resource *res;
>> + int nr_chans, ret, i;
>> +
>> + nr_chans = platform_irq_count(pdev);
>> + if (nr_chans < 0)
>> + return nr_chans;
>> +
>> + ret = dma_set_mask(dev, DMA_BIT_MASK(32));
>> + if (ret)
>> + return ret;
>> +
>> + mdev = devm_kzalloc(dev, struct_size(mdev, channels, nr_chans),
>> + GFP_KERNEL);
>
> kcalloc variant?
No.
I allocate here
sizeof(*mdev) + nr_chans * sizeof(struct uniphier_mdmac_chan)
kcalloc does not cater to it.
You should check struct_size() helper macro.
>> + if (!mdev)
>> + return -ENOMEM;
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + mdev->reg_base = devm_ioremap_resource(dev, res);
>> + if (IS_ERR(mdev->reg_base))
>> + return PTR_ERR(mdev->reg_base);
>> +
>> + mdev->clk = devm_clk_get(dev, NULL);
>> + if (IS_ERR(mdev->clk)) {
>> + dev_err(dev, "failed to get clock\n");
>> + return PTR_ERR(mdev->clk);
>> + }
>> +
>> + ret = clk_prepare_enable(mdev->clk);
>> +
Re: [PATCH v2 2/2] dmaengine: uniphier-mdmac: add UniPhier MIO DMAC driver
Hi Vinod,
2018-09-11 16:00 GMT+09:00 Vinod :
> On 24-08-18, 10:41, Masahiro Yamada wrote:
>
>> +/* mc->vc.lock must be held by caller */
>> +static u32 __uniphier_mdmac_get_residue(struct uniphier_mdmac_desc *md)
>> +{
>> + u32 residue = 0;
>> + int i;
>> +
>> + for (i = md->sg_cur; i < md->sg_len; i++)
>> + residue += sg_dma_len(>sgl[i]);
>
> so if the descriptor is submitted to hardware, we return the descriptor
> length, which is not correct.
>
> Two cases are required to be handled:
> 1. Descriptor is in queue (IMO above logic is fine for that, but it can
> be calculated at descriptor submit and looked up here)
Where do you want it to be calculated?
This hardware provides only simple registers (address and size)
for one-shot transfer instead of descriptors.
So, I used sgl as-is because I did not see a good reason
to transform sgl to another data structure.
> 2. Descriptor is running (interesting case), you need to read current
> register and offset that from descriptor length and return
OK, I will read out the register value
to retrieve the residue from the on-flight transfer.
>> +static struct dma_async_tx_descriptor *uniphier_mdmac_prep_slave_sg(
>> + struct dma_chan *chan,
>> + struct scatterlist *sgl,
>> + unsigned int sg_len,
>> + enum dma_transfer_direction direction,
>> + unsigned long flags, void *context)
>> +{
>> + struct virt_dma_chan *vc = to_virt_chan(chan);
>> + struct uniphier_mdmac_desc *md;
>> +
>> + if (!is_slave_direction(direction))
>> + return NULL;
>> +
>> + md = kzalloc(sizeof(*md), GFP_KERNEL);
>
> _prep calls can be invoked from atomic context, so this should be
> GFP_NOWAIT, see Documentation/driver-api/dmaengine/provider.rst
Will fix.
>> + if (!md)
>> + return NULL;
>> +
>> + md->sgl = sgl;
>> + md->sg_len = sg_len;
>> + md->dir = direction;
>> +
>> + return vchan_tx_prep(vc, >vd, flags);
>
> this seems missing stuff. Where do you do register calculation for the
> descriptor and where is slave_config here, how do you know where to
> send/receive data form/to (peripheral)
This dmac is really simple, and un-flexible.
The peripheral address to send/receive data from/to is hard-weird.
cfg->{src_addr,dst_addr} is not configurable.
Look at __uniphier_mdmac_handle().
'dest_addr' and 'src_addr' must be set to 0 for the peripheral.
>> +static enum dma_status uniphier_mdmac_tx_status(struct dma_chan *chan,
>> + dma_cookie_t cookie,
>> + struct dma_tx_state *txstate)
>> +{
>> + struct virt_dma_chan *vc;
>> + struct virt_dma_desc *vd;
>> + struct uniphier_mdmac_chan *mc;
>> + struct uniphier_mdmac_desc *md = NULL;
>> + enum dma_status stat;
>> + unsigned long flags;
>> +
>> + stat = dma_cookie_status(chan, cookie, txstate);
>> + if (stat == DMA_COMPLETE)
>> + return stat;
>> +
>> + vc = to_virt_chan(chan);
>> +
>> + spin_lock_irqsave(>lock, flags);
>> +
>> + mc = to_uniphier_mdmac_chan(vc);
>> +
>> + if (mc->md && mc->md->vd.tx.cookie == cookie)
>> + md = mc->md;
>> +
>> + if (!md) {
>> + vd = vchan_find_desc(vc, cookie);
>> + if (vd)
>> + md = to_uniphier_mdmac_desc(vd);
>> + }
>> +
>> + if (md)
>> + txstate->residue = __uniphier_mdmac_get_residue(md);
>
> txstate can be NULL and should be checked...
Will fix.
>> +static int uniphier_mdmac_probe(struct platform_device *pdev)
>> +{
>> + struct device *dev = >dev;
>> + struct uniphier_mdmac_device *mdev;
>> + struct dma_device *ddev;
>> + struct resource *res;
>> + int nr_chans, ret, i;
>> +
>> + nr_chans = platform_irq_count(pdev);
>> + if (nr_chans < 0)
>> + return nr_chans;
>> +
>> + ret = dma_set_mask(dev, DMA_BIT_MASK(32));
>> + if (ret)
>> + return ret;
>> +
>> + mdev = devm_kzalloc(dev, struct_size(mdev, channels, nr_chans),
>> + GFP_KERNEL);
>
> kcalloc variant?
No.
I allocate here
sizeof(*mdev) + nr_chans * sizeof(struct uniphier_mdmac_chan)
kcalloc does not cater to it.
You should check struct_size() helper macro.
>> + if (!mdev)
>> + return -ENOMEM;
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + mdev->reg_base = devm_ioremap_resource(dev, res);
>> + if (IS_ERR(mdev->reg_base))
>> + return PTR_ERR(mdev->reg_base);
>> +
>> + mdev->clk = devm_clk_get(dev, NULL);
>> + if (IS_ERR(mdev->clk)) {
>> + dev_err(dev, "failed to get clock\n");
>> + return PTR_ERR(mdev->clk);
>> + }
>> +
>> + ret = clk_prepare_enable(mdev->clk);
>> +
[PATCH -next] staging: mt7621-pci: Use PTR_ERR_OR_ZERO in mt7621_pcie_parse_dt()
Use PTR_ERR_OR_ZERO rather than if(IS_ERR(...)) + PTR_ERR Signed-off-by: YueHaibing --- drivers/staging/mt7621-pci/pci-mt7621.c | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/staging/mt7621-pci/pci-mt7621.c b/drivers/staging/mt7621-pci/pci-mt7621.c index ba1f117..d2cb910 100644 --- a/drivers/staging/mt7621-pci/pci-mt7621.c +++ b/drivers/staging/mt7621-pci/pci-mt7621.c @@ -396,10 +396,7 @@ static int mt7621_pcie_parse_dt(struct mt7621_pcie *pcie) } pcie->base = devm_ioremap_resource(dev, ); - if (IS_ERR(pcie->base)) - return PTR_ERR(pcie->base); - - return 0; + return PTR_ERR_OR_ZERO(pcie->base); } static int mt7621_pcie_request_resources(struct mt7621_pcie *pcie,
[PATCH -next] staging: mt7621-pci: Use PTR_ERR_OR_ZERO in mt7621_pcie_parse_dt()
Use PTR_ERR_OR_ZERO rather than if(IS_ERR(...)) + PTR_ERR Signed-off-by: YueHaibing --- drivers/staging/mt7621-pci/pci-mt7621.c | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/staging/mt7621-pci/pci-mt7621.c b/drivers/staging/mt7621-pci/pci-mt7621.c index ba1f117..d2cb910 100644 --- a/drivers/staging/mt7621-pci/pci-mt7621.c +++ b/drivers/staging/mt7621-pci/pci-mt7621.c @@ -396,10 +396,7 @@ static int mt7621_pcie_parse_dt(struct mt7621_pcie *pcie) } pcie->base = devm_ioremap_resource(dev, ); - if (IS_ERR(pcie->base)) - return PTR_ERR(pcie->base); - - return 0; + return PTR_ERR_OR_ZERO(pcie->base); } static int mt7621_pcie_request_resources(struct mt7621_pcie *pcie,
[PATCH] drivers: pci: remove set but unused variable
This patch removes a set but unused variable in quirks.c. Fixes warning: variable ‘mmio_sys_info’ set but not used [-Wunused-but-set-variable] Signed-off-by: Joshua Abraham --- drivers/pci/quirks.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c index ef7143a274e0..690a3b71aa1f 100644 --- a/drivers/pci/quirks.c +++ b/drivers/pci/quirks.c @@ -4993,7 +4993,6 @@ static void quirk_switchtec_ntb_dma_alias(struct pci_dev *pdev) void __iomem *mmio; struct ntb_info_regs __iomem *mmio_ntb; struct ntb_ctrl_regs __iomem *mmio_ctrl; - struct sys_info_regs __iomem *mmio_sys_info; u64 partition_map; u8 partition; int pp; @@ -5014,7 +5013,6 @@ static void quirk_switchtec_ntb_dma_alias(struct pci_dev *pdev) mmio_ntb = mmio + SWITCHTEC_GAS_NTB_OFFSET; mmio_ctrl = (void __iomem *) mmio_ntb + SWITCHTEC_NTB_REG_CTRL_OFFSET; - mmio_sys_info = mmio + SWITCHTEC_GAS_SYS_INFO_OFFSET; partition = ioread8(_ntb->partition_id); -- 2.17.1
[PATCH] drivers: pci: remove set but unused variable
This patch removes a set but unused variable in quirks.c. Fixes warning: variable ‘mmio_sys_info’ set but not used [-Wunused-but-set-variable] Signed-off-by: Joshua Abraham --- drivers/pci/quirks.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c index ef7143a274e0..690a3b71aa1f 100644 --- a/drivers/pci/quirks.c +++ b/drivers/pci/quirks.c @@ -4993,7 +4993,6 @@ static void quirk_switchtec_ntb_dma_alias(struct pci_dev *pdev) void __iomem *mmio; struct ntb_info_regs __iomem *mmio_ntb; struct ntb_ctrl_regs __iomem *mmio_ctrl; - struct sys_info_regs __iomem *mmio_sys_info; u64 partition_map; u8 partition; int pp; @@ -5014,7 +5013,6 @@ static void quirk_switchtec_ntb_dma_alias(struct pci_dev *pdev) mmio_ntb = mmio + SWITCHTEC_GAS_NTB_OFFSET; mmio_ctrl = (void __iomem *) mmio_ntb + SWITCHTEC_NTB_REG_CTRL_OFFSET; - mmio_sys_info = mmio + SWITCHTEC_GAS_SYS_INFO_OFFSET; partition = ioread8(_ntb->partition_id); -- 2.17.1
Re: [WTF?] extremely old dead code
On Mon, Sep 10, 2018 at 1:55 PM Al Viro wrote: > > Hadn't that sucker been dead code since 0.98.2? What am I missing here? > Note that this thing had quite a few functionality changes over those > years; had they even been tested? Looks about right to me. The only point that actually acts on FIONBIO is the fs/ioctl.c code. Impressively, the dead tty code looks perfectly correct to me too, despite not ever being triggered. Linus
Re: [WTF?] extremely old dead code
On Mon, Sep 10, 2018 at 1:55 PM Al Viro wrote: > > Hadn't that sucker been dead code since 0.98.2? What am I missing here? > Note that this thing had quite a few functionality changes over those > years; had they even been tested? Looks about right to me. The only point that actually acts on FIONBIO is the fs/ioctl.c code. Impressively, the dead tty code looks perfectly correct to me too, despite not ever being triggered. Linus
Re: [RFC v9 PATCH 2/4] mm: mmap: zap pages with read mmap_sem in munmap
On Tue, Sep 11, 2018 at 04:35:03PM -0700, Yang Shi wrote:
> On 9/11/18 2:16 PM, Matthew Wilcox wrote:
> > On Wed, Sep 12, 2018 at 04:58:11AM +0800, Yang Shi wrote:
> > > mm/mmap.c | 97
> > > +--
> > I really think you're going about this the wrong way by duplicating
> > vm_munmap().
>
> If we don't duplicate vm_munmap() or do_munmap(), we need pass an extra
> parameter to them to tell when it is fine to downgrade write lock or if the
> lock has been acquired outside it (i.e. in mmap()/mremap()), right? But,
> vm_munmap() or do_munmap() is called not only by mmap-related, but also some
> other places, like arch-specific places, which don't need downgrade write
> lock or are not safe to do so.
>
> Actually, I did this way in the v1 patches, but it got pushed back by tglx
> who suggested duplicate the code so that the change could be done in mm only
> without touching other files, i.e. arch-specific stuff. I didn't have strong
> argument to convince him.
With my patch, there is nothing to change in arch-specific code.
Here it is again ...
diff --git a/mm/mmap.c b/mm/mmap.c
index de699523c0b7..06dc31d1da8c 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -2798,11 +2798,11 @@ int split_vma(struct mm_struct *mm, struct
vm_area_struct *vma,
* work. This now handles partial unmappings.
* Jeremy Fitzhardinge
*/
-int do_munmap(struct mm_struct *mm, unsigned long start, size_t len,
- struct list_head *uf)
+static int __do_munmap(struct mm_struct *mm, unsigned long start, size_t len,
+ struct list_head *uf, bool downgrade)
{
unsigned long end;
- struct vm_area_struct *vma, *prev, *last;
+ struct vm_area_struct *vma, *prev, *last, *tmp;
if ((offset_in_page(start)) || start > TASK_SIZE || len >
TASK_SIZE-start)
return -EINVAL;
@@ -2816,7 +2816,7 @@ int do_munmap(struct mm_struct *mm, unsigned long start,
size_t len,
if (!vma)
return 0;
prev = vma->vm_prev;
- /* we have start < vma->vm_end */
+ /* we have start < vma->vm_end */
/* if it doesn't overlap, we have nothing.. */
end = start + len;
@@ -2873,18 +2873,22 @@ int do_munmap(struct mm_struct *mm, unsigned long
start, size_t len,
/*
* unlock any mlock()ed ranges before detaching vmas
+* and check to see if there's any reason we might have to hold
+* the mmap_sem write-locked while unmapping regions.
*/
- if (mm->locked_vm) {
- struct vm_area_struct *tmp = vma;
- while (tmp && tmp->vm_start < end) {
- if (tmp->vm_flags & VM_LOCKED) {
- mm->locked_vm -= vma_pages(tmp);
- munlock_vma_pages_all(tmp);
- }
- tmp = tmp->vm_next;
+ for (tmp = vma; tmp && tmp->vm_start < end; tmp = tmp->vm_next) {
+ if (tmp->vm_flags & VM_LOCKED) {
+ mm->locked_vm -= vma_pages(tmp);
+ munlock_vma_pages_all(tmp);
}
+ if (tmp->vm_file &&
+ has_uprobes(tmp, tmp->vm_start, tmp->vm_end))
+ downgrade = false;
}
+ if (downgrade)
+ downgrade_write(>mmap_sem);
+
/*
* Remove the vma's, and unmap the actual pages
*/
@@ -2896,7 +2900,13 @@ int do_munmap(struct mm_struct *mm, unsigned long start,
size_t len,
/* Fix up all other VM information */
remove_vma_list(mm, vma);
- return 0;
+ return downgrade ? 1 : 0;
+}
+
+int do_unmap(struct mm_struct *mm, unsigned long start, size_t len,
+ struct list_head *uf)
+{
+ return __do_munmap(mm, start, len, uf, false);
}
int vm_munmap(unsigned long start, size_t len)
@@ -2905,11 +2915,12 @@ int vm_munmap(unsigned long start, size_t len)
struct mm_struct *mm = current->mm;
LIST_HEAD(uf);
- if (down_write_killable(>mmap_sem))
- return -EINTR;
-
- ret = do_munmap(mm, start, len, );
- up_write(>mmap_sem);
+ down_write(>mmap_sem);
+ ret = __do_munmap(mm, start, len, , true);
+ if (ret == 1)
+ up_read(>mmap_sem);
+ else
+ up_write(>mmap_sem);
userfaultfd_unmap_complete(mm, );
return ret;
}
Anybody calling do_munmap() will not get the lock dropped.
> And, Michal prefers have VM_HUGETLB and VM_PFNMAP handled separately for
> safe and bisectable sake, which needs call the regular do_munmap().
That can be introduced and then taken out ... indeed, you can split this into
many patches, starting with this:
+ if (tmp->vm_file)
+ downgrade = false;
to only allow this optimisation for anonymous mappings at first.
> In addition to this, I just found mpx code may call
Re: [RFC v9 PATCH 2/4] mm: mmap: zap pages with read mmap_sem in munmap
On Tue, Sep 11, 2018 at 04:35:03PM -0700, Yang Shi wrote:
> On 9/11/18 2:16 PM, Matthew Wilcox wrote:
> > On Wed, Sep 12, 2018 at 04:58:11AM +0800, Yang Shi wrote:
> > > mm/mmap.c | 97
> > > +--
> > I really think you're going about this the wrong way by duplicating
> > vm_munmap().
>
> If we don't duplicate vm_munmap() or do_munmap(), we need pass an extra
> parameter to them to tell when it is fine to downgrade write lock or if the
> lock has been acquired outside it (i.e. in mmap()/mremap()), right? But,
> vm_munmap() or do_munmap() is called not only by mmap-related, but also some
> other places, like arch-specific places, which don't need downgrade write
> lock or are not safe to do so.
>
> Actually, I did this way in the v1 patches, but it got pushed back by tglx
> who suggested duplicate the code so that the change could be done in mm only
> without touching other files, i.e. arch-specific stuff. I didn't have strong
> argument to convince him.
With my patch, there is nothing to change in arch-specific code.
Here it is again ...
diff --git a/mm/mmap.c b/mm/mmap.c
index de699523c0b7..06dc31d1da8c 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -2798,11 +2798,11 @@ int split_vma(struct mm_struct *mm, struct
vm_area_struct *vma,
* work. This now handles partial unmappings.
* Jeremy Fitzhardinge
*/
-int do_munmap(struct mm_struct *mm, unsigned long start, size_t len,
- struct list_head *uf)
+static int __do_munmap(struct mm_struct *mm, unsigned long start, size_t len,
+ struct list_head *uf, bool downgrade)
{
unsigned long end;
- struct vm_area_struct *vma, *prev, *last;
+ struct vm_area_struct *vma, *prev, *last, *tmp;
if ((offset_in_page(start)) || start > TASK_SIZE || len >
TASK_SIZE-start)
return -EINVAL;
@@ -2816,7 +2816,7 @@ int do_munmap(struct mm_struct *mm, unsigned long start,
size_t len,
if (!vma)
return 0;
prev = vma->vm_prev;
- /* we have start < vma->vm_end */
+ /* we have start < vma->vm_end */
/* if it doesn't overlap, we have nothing.. */
end = start + len;
@@ -2873,18 +2873,22 @@ int do_munmap(struct mm_struct *mm, unsigned long
start, size_t len,
/*
* unlock any mlock()ed ranges before detaching vmas
+* and check to see if there's any reason we might have to hold
+* the mmap_sem write-locked while unmapping regions.
*/
- if (mm->locked_vm) {
- struct vm_area_struct *tmp = vma;
- while (tmp && tmp->vm_start < end) {
- if (tmp->vm_flags & VM_LOCKED) {
- mm->locked_vm -= vma_pages(tmp);
- munlock_vma_pages_all(tmp);
- }
- tmp = tmp->vm_next;
+ for (tmp = vma; tmp && tmp->vm_start < end; tmp = tmp->vm_next) {
+ if (tmp->vm_flags & VM_LOCKED) {
+ mm->locked_vm -= vma_pages(tmp);
+ munlock_vma_pages_all(tmp);
}
+ if (tmp->vm_file &&
+ has_uprobes(tmp, tmp->vm_start, tmp->vm_end))
+ downgrade = false;
}
+ if (downgrade)
+ downgrade_write(>mmap_sem);
+
/*
* Remove the vma's, and unmap the actual pages
*/
@@ -2896,7 +2900,13 @@ int do_munmap(struct mm_struct *mm, unsigned long start,
size_t len,
/* Fix up all other VM information */
remove_vma_list(mm, vma);
- return 0;
+ return downgrade ? 1 : 0;
+}
+
+int do_unmap(struct mm_struct *mm, unsigned long start, size_t len,
+ struct list_head *uf)
+{
+ return __do_munmap(mm, start, len, uf, false);
}
int vm_munmap(unsigned long start, size_t len)
@@ -2905,11 +2915,12 @@ int vm_munmap(unsigned long start, size_t len)
struct mm_struct *mm = current->mm;
LIST_HEAD(uf);
- if (down_write_killable(>mmap_sem))
- return -EINTR;
-
- ret = do_munmap(mm, start, len, );
- up_write(>mmap_sem);
+ down_write(>mmap_sem);
+ ret = __do_munmap(mm, start, len, , true);
+ if (ret == 1)
+ up_read(>mmap_sem);
+ else
+ up_write(>mmap_sem);
userfaultfd_unmap_complete(mm, );
return ret;
}
Anybody calling do_munmap() will not get the lock dropped.
> And, Michal prefers have VM_HUGETLB and VM_PFNMAP handled separately for
> safe and bisectable sake, which needs call the regular do_munmap().
That can be introduced and then taken out ... indeed, you can split this into
many patches, starting with this:
+ if (tmp->vm_file)
+ downgrade = false;
to only allow this optimisation for anonymous mappings at first.
> In addition to this, I just found mpx code may call
Re: [PATCH 06/11] dts: arm: imx7{d,s}: Update coresight binding for hardware ports
On Tue, Sep 11, 2018 at 11:17:07AM +0100, Suzuki K Poulose wrote:
> Switch to the updated coresight bindings.
>
> Cc: Shawn Guo
> Cc: Sascha Hauer
> Cc: Pengutronix Kernel Team
> Cc: Fabio Estevam
> Cc: Mathieu Poirier
> Signed-off-by: Suzuki K Poulose
As per the convention we use for subject prefix, I suggest you use
'ARM: dts: imx7: ...'
Shawn
> ---
> arch/arm/boot/dts/imx7d.dtsi | 11 ---
> arch/arm/boot/dts/imx7s.dtsi | 78
> ++--
> 2 files changed, 53 insertions(+), 36 deletions(-)
>
> diff --git a/arch/arm/boot/dts/imx7d.dtsi b/arch/arm/boot/dts/imx7d.dtsi
> index 7cbc2ff..4ced17c 100644
> --- a/arch/arm/boot/dts/imx7d.dtsi
> +++ b/arch/arm/boot/dts/imx7d.dtsi
> @@ -63,9 +63,11 @@
> clocks = < IMX7D_MAIN_AXI_ROOT_CLK>;
> clock-names = "apb_pclk";
>
> - port {
> - etm1_out_port: endpoint {
> - remote-endpoint = <_funnel_in_port1>;
> + out-ports {
> + port {
> + etm1_out_port: endpoint {
> + remote-endpoint =
> <_funnel_in_port1>;
> + };
> };
> };
> };
> @@ -148,11 +150,10 @@
> };
> };
>
> -_funnel_ports {
> +_funnel_in_ports {
> port@1 {
> reg = <1>;
> ca_funnel_in_port1: endpoint {
> - slave-mode;
> remote-endpoint = <_out_port>;
> };
> };
> diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
> index a052198..9176885 100644
> --- a/arch/arm/boot/dts/imx7s.dtsi
> +++ b/arch/arm/boot/dts/imx7s.dtsi
> @@ -106,7 +106,7 @@
>*/
> compatible = "arm,coresight-replicator";
>
> - ports {
> + out-ports {
> #address-cells = <1>;
> #size-cells = <0>;
> /* replicator output ports */
> @@ -123,12 +123,15 @@
> remote-endpoint = <_in_port>;
> };
> };
> + };
>
> - /* replicator input port */
> - port@2 {
> + in-ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> reg = <0>;
> replicator_in_port0: endpoint {
> - slave-mode;
> remote-endpoint = <_out_port>;
> };
> };
> @@ -168,28 +171,31 @@
> clocks = < IMX7D_MAIN_AXI_ROOT_CLK>;
> clock-names = "apb_pclk";
>
> - ca_funnel_ports: ports {
> + ca_funnel_in_ports: in-ports {
> #address-cells = <1>;
> #size-cells = <0>;
>
> - /* funnel input ports */
> port@0 {
> reg = <0>;
> ca_funnel_in_port0: endpoint {
> - slave-mode;
> remote-endpoint =
> <_out_port>;
> };
> };
>
> - /* funnel output port */
> - port@2 {
> + /* the other input ports are not connect to
> anything */
> + };
> +
> + out-ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> reg = <0>;
> ca_funnel_out_port0: endpoint {
> remote-endpoint =
> <_funnel_in_port0>;
> };
> };
>
> - /* the other input ports are not connect to
> anything */
> };
> };
>
> @@ -200,9 +206,11 @@
> clocks = < IMX7D_MAIN_AXI_ROOT_CLK>;
> clock-names = "apb_pclk";
>
> - port {
> - etm0_out_port: endpoint {
> - remote-endpoint = <_funnel_in_port0>;
> + out-ports {
> + port {
> + etm0_out_port: endpoint {
> +
Re: [PATCH 06/11] dts: arm: imx7{d,s}: Update coresight binding for hardware ports
On Tue, Sep 11, 2018 at 11:17:07AM +0100, Suzuki K Poulose wrote:
> Switch to the updated coresight bindings.
>
> Cc: Shawn Guo
> Cc: Sascha Hauer
> Cc: Pengutronix Kernel Team
> Cc: Fabio Estevam
> Cc: Mathieu Poirier
> Signed-off-by: Suzuki K Poulose
As per the convention we use for subject prefix, I suggest you use
'ARM: dts: imx7: ...'
Shawn
> ---
> arch/arm/boot/dts/imx7d.dtsi | 11 ---
> arch/arm/boot/dts/imx7s.dtsi | 78
> ++--
> 2 files changed, 53 insertions(+), 36 deletions(-)
>
> diff --git a/arch/arm/boot/dts/imx7d.dtsi b/arch/arm/boot/dts/imx7d.dtsi
> index 7cbc2ff..4ced17c 100644
> --- a/arch/arm/boot/dts/imx7d.dtsi
> +++ b/arch/arm/boot/dts/imx7d.dtsi
> @@ -63,9 +63,11 @@
> clocks = < IMX7D_MAIN_AXI_ROOT_CLK>;
> clock-names = "apb_pclk";
>
> - port {
> - etm1_out_port: endpoint {
> - remote-endpoint = <_funnel_in_port1>;
> + out-ports {
> + port {
> + etm1_out_port: endpoint {
> + remote-endpoint =
> <_funnel_in_port1>;
> + };
> };
> };
> };
> @@ -148,11 +150,10 @@
> };
> };
>
> -_funnel_ports {
> +_funnel_in_ports {
> port@1 {
> reg = <1>;
> ca_funnel_in_port1: endpoint {
> - slave-mode;
> remote-endpoint = <_out_port>;
> };
> };
> diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
> index a052198..9176885 100644
> --- a/arch/arm/boot/dts/imx7s.dtsi
> +++ b/arch/arm/boot/dts/imx7s.dtsi
> @@ -106,7 +106,7 @@
>*/
> compatible = "arm,coresight-replicator";
>
> - ports {
> + out-ports {
> #address-cells = <1>;
> #size-cells = <0>;
> /* replicator output ports */
> @@ -123,12 +123,15 @@
> remote-endpoint = <_in_port>;
> };
> };
> + };
>
> - /* replicator input port */
> - port@2 {
> + in-ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> reg = <0>;
> replicator_in_port0: endpoint {
> - slave-mode;
> remote-endpoint = <_out_port>;
> };
> };
> @@ -168,28 +171,31 @@
> clocks = < IMX7D_MAIN_AXI_ROOT_CLK>;
> clock-names = "apb_pclk";
>
> - ca_funnel_ports: ports {
> + ca_funnel_in_ports: in-ports {
> #address-cells = <1>;
> #size-cells = <0>;
>
> - /* funnel input ports */
> port@0 {
> reg = <0>;
> ca_funnel_in_port0: endpoint {
> - slave-mode;
> remote-endpoint =
> <_out_port>;
> };
> };
>
> - /* funnel output port */
> - port@2 {
> + /* the other input ports are not connect to
> anything */
> + };
> +
> + out-ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> reg = <0>;
> ca_funnel_out_port0: endpoint {
> remote-endpoint =
> <_funnel_in_port0>;
> };
> };
>
> - /* the other input ports are not connect to
> anything */
> };
> };
>
> @@ -200,9 +206,11 @@
> clocks = < IMX7D_MAIN_AXI_ROOT_CLK>;
> clock-names = "apb_pclk";
>
> - port {
> - etm0_out_port: endpoint {
> - remote-endpoint = <_funnel_in_port0>;
> + out-ports {
> + port {
> + etm0_out_port: endpoint {
> +
Re: [PATCH v2 3/6] drivers: qcom: rpmh: disallow active requests in solver mode
On Tue, Sep 11 2018 at 17:02 -0600, Matthias Kaehlcke wrote:
Hi Raju/Lina,
On Fri, Jul 27, 2018 at 03:34:46PM +0530, Raju P L S S S N wrote:
From: Lina Iyer
Controllers may be in 'solver' state, where they could be in autonomous
mode executing low power modes for their hardware and as such are not
available for sending active votes. Device driver may notify RPMH API
that the controller is in solver mode and when in such mode, disallow
requests from platform drivers for state change using the RSC.
Signed-off-by: Lina Iyer
Signed-off-by: Raju P.L.S.S.S.N
---
drivers/soc/qcom/rpmh-internal.h | 2 ++
drivers/soc/qcom/rpmh.c | 59
include/soc/qcom/rpmh.h | 5
3 files changed, 66 insertions(+)
diff --git a/drivers/soc/qcom/rpmh-internal.h b/drivers/soc/qcom/rpmh-internal.h
index 4ff43bf..6cd2f78 100644
--- a/drivers/soc/qcom/rpmh-internal.h
+++ b/drivers/soc/qcom/rpmh-internal.h
@@ -72,12 +72,14 @@ struct rpmh_request {
* @cache_lock: synchronize access to the cache data
* @dirty: was the cache updated since flush
* @batch_cache: Cache sleep and wake requests sent as batch
+ * @in_solver_mode: Controller is busy in solver mode
*/
struct rpmh_ctrlr {
struct list_head cache;
spinlock_t cache_lock;
bool dirty;
struct list_head batch_cache;
+ bool in_solver_mode;
};
/**
diff --git a/drivers/soc/qcom/rpmh.c b/drivers/soc/qcom/rpmh.c
index 2382276..0d276fd 100644
--- a/drivers/soc/qcom/rpmh.c
+++ b/drivers/soc/qcom/rpmh.c
@@ -5,6 +5,7 @@
#include
#include
+#include
#include
#include
#include
@@ -75,6 +76,50 @@ static struct rpmh_ctrlr *get_rpmh_ctrlr(const struct device
*dev)
return >client;
}
+static int check_ctrlr_state(struct rpmh_ctrlr *ctrlr, enum rpmh_state state)
+{
+ unsigned long flags;
+ int ret = 0;
+
+ /* Do not allow setting active votes when in solver mode */
+ spin_lock_irqsave(>cache_lock, flags);
+ if (ctrlr->in_solver_mode && state == RPMH_ACTIVE_ONLY_STATE)
+ ret = -EBUSY;
+ spin_unlock_irqrestore(>cache_lock, flags);
+
+ return ret;
+}
+
+/**
+ * rpmh_mode_solver_set: Indicate that the RSC controller hardware has
+ * been configured to be in solver mode
+ *
+ * @dev: the device making the request
+ * @enable: Boolean value indicating if the controller is in solver mode.
+ *
+ * When solver mode is enabled, passthru API will not be able to send wake
+ * votes, just awake and active votes.
+ */
+int rpmh_mode_solver_set(const struct device *dev, bool enable)
+{
+ struct rpmh_ctrlr *ctrlr = get_rpmh_ctrlr(dev);
+ unsigned long flags;
+
+ for (;;) {
+ spin_lock_irqsave(>cache_lock, flags);
+ if (rpmh_rsc_ctrlr_is_idle(ctrlr_to_drv(ctrlr))) {
+ ctrlr->in_solver_mode = enable;
As commented on '[v2,1/6] drivers: qcom: rpmh-rsc: return if the
controller is idle', this seems potentially
racy. _is_idle() could report the controller as idle, even though some
TCSes are in use (after _is_idle() visited them).
Additional locking may be needed or a comment if this situation should
never happen on a sane system (I don't know enough about RPMh and its
clients to judge if this is the case).
Hmm.. Forgot that we call from here. May be a lock might be helpful.
-- Lina
Re: [PATCH v2 3/6] drivers: qcom: rpmh: disallow active requests in solver mode
On Tue, Sep 11 2018 at 17:02 -0600, Matthias Kaehlcke wrote:
Hi Raju/Lina,
On Fri, Jul 27, 2018 at 03:34:46PM +0530, Raju P L S S S N wrote:
From: Lina Iyer
Controllers may be in 'solver' state, where they could be in autonomous
mode executing low power modes for their hardware and as such are not
available for sending active votes. Device driver may notify RPMH API
that the controller is in solver mode and when in such mode, disallow
requests from platform drivers for state change using the RSC.
Signed-off-by: Lina Iyer
Signed-off-by: Raju P.L.S.S.S.N
---
drivers/soc/qcom/rpmh-internal.h | 2 ++
drivers/soc/qcom/rpmh.c | 59
include/soc/qcom/rpmh.h | 5
3 files changed, 66 insertions(+)
diff --git a/drivers/soc/qcom/rpmh-internal.h b/drivers/soc/qcom/rpmh-internal.h
index 4ff43bf..6cd2f78 100644
--- a/drivers/soc/qcom/rpmh-internal.h
+++ b/drivers/soc/qcom/rpmh-internal.h
@@ -72,12 +72,14 @@ struct rpmh_request {
* @cache_lock: synchronize access to the cache data
* @dirty: was the cache updated since flush
* @batch_cache: Cache sleep and wake requests sent as batch
+ * @in_solver_mode: Controller is busy in solver mode
*/
struct rpmh_ctrlr {
struct list_head cache;
spinlock_t cache_lock;
bool dirty;
struct list_head batch_cache;
+ bool in_solver_mode;
};
/**
diff --git a/drivers/soc/qcom/rpmh.c b/drivers/soc/qcom/rpmh.c
index 2382276..0d276fd 100644
--- a/drivers/soc/qcom/rpmh.c
+++ b/drivers/soc/qcom/rpmh.c
@@ -5,6 +5,7 @@
#include
#include
+#include
#include
#include
#include
@@ -75,6 +76,50 @@ static struct rpmh_ctrlr *get_rpmh_ctrlr(const struct device
*dev)
return >client;
}
+static int check_ctrlr_state(struct rpmh_ctrlr *ctrlr, enum rpmh_state state)
+{
+ unsigned long flags;
+ int ret = 0;
+
+ /* Do not allow setting active votes when in solver mode */
+ spin_lock_irqsave(>cache_lock, flags);
+ if (ctrlr->in_solver_mode && state == RPMH_ACTIVE_ONLY_STATE)
+ ret = -EBUSY;
+ spin_unlock_irqrestore(>cache_lock, flags);
+
+ return ret;
+}
+
+/**
+ * rpmh_mode_solver_set: Indicate that the RSC controller hardware has
+ * been configured to be in solver mode
+ *
+ * @dev: the device making the request
+ * @enable: Boolean value indicating if the controller is in solver mode.
+ *
+ * When solver mode is enabled, passthru API will not be able to send wake
+ * votes, just awake and active votes.
+ */
+int rpmh_mode_solver_set(const struct device *dev, bool enable)
+{
+ struct rpmh_ctrlr *ctrlr = get_rpmh_ctrlr(dev);
+ unsigned long flags;
+
+ for (;;) {
+ spin_lock_irqsave(>cache_lock, flags);
+ if (rpmh_rsc_ctrlr_is_idle(ctrlr_to_drv(ctrlr))) {
+ ctrlr->in_solver_mode = enable;
As commented on '[v2,1/6] drivers: qcom: rpmh-rsc: return if the
controller is idle', this seems potentially
racy. _is_idle() could report the controller as idle, even though some
TCSes are in use (after _is_idle() visited them).
Additional locking may be needed or a comment if this situation should
never happen on a sane system (I don't know enough about RPMh and its
clients to judge if this is the case).
Hmm.. Forgot that we call from here. May be a lock might be helpful.
-- Lina
[PATCH v3] iio: proximity: Add driver support for ST's VL53L0X ToF ranging sensor.
This driver was originally written by ST in 2016 as a misc input device
driver, and hasn't been maintained for a long time. I grabbed some code
from it's API and reformed it into a iio proximity device driver.
This version of driver uses i2c bus to talk to the sensor and
polling for measuring completes, so no irq line is needed.
This version of driver supports only one-shot mode, and it can be
tested with reading from
/sys/bus/iio/devices/iio:deviceX/in_distance_raw
Signed-off-by: Song Qiang
---
Changes in v2:
- Clean up the register table.
- Sort header files declarations.
- Replace some bit definations with GENMASK() and BIT().
- Clean up some code and comments that's useless for now.
- Change the order of some the definations of some variables to reversed
xmas tree order.
- Using do...while() rather while and check.
- Replace pr_err() with dev_err().
- Remove device id declaration since we recommend to use DT.
- Remove .owner = THIS_MODULE.
- Replace probe() with probe_new() hook.
- Remove IIO_BUFFER and IIO_TRIGGERED_BUFFER dependences.
- Change the driver module name to vl53l0x-i2c.
- Align all the parameters if they are in the same function with open
parentheses.
- Replace iio_device_register() with devm_iio_device_register
for better resource management.
- Remove the vl53l0x_remove() since it's not needed.
- Remove dev_set_drvdata() since it's already setted above.
Changes in v3:
- Recover ST's copyright.
- Clean up indio_dev member in vl53l0x_data struct since it's
useless now.
- Replace __le16_to_cpu() with le16_to_cpu().
- Remove the iio_device_{claim|release}_direct_mode() since it's
only needed when we use buffered mode.
- Clean up some coding style problems.
.../bindings/iio/proximity/vl53l0x.txt| 12 ++
drivers/iio/proximity/Kconfig | 11 ++
drivers/iio/proximity/Makefile| 2 +
drivers/iio/proximity/vl53l0x-i2c.c | 184 ++
4 files changed, 209 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/proximity/vl53l0x.txt
create mode 100644 drivers/iio/proximity/vl53l0x-i2c.c
diff --git a/Documentation/devicetree/bindings/iio/proximity/vl53l0x.txt
b/Documentation/devicetree/bindings/iio/proximity/vl53l0x.txt
new file mode 100644
index ..64b69442f08e
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/proximity/vl53l0x.txt
@@ -0,0 +1,12 @@
+ST's VL53L0X ToF ranging sensor
+
+Required properties:
+ - compatible: must be "st,vl53l0x-i2c"
+ - reg: i2c address where to find the device
+
+Example:
+
+vl53l0x@29 {
+ compatible = "st,vl53l0x-i2c";
+ reg = <0x29>;
+};
diff --git a/drivers/iio/proximity/Kconfig b/drivers/iio/proximity/Kconfig
index f726f9427602..5f421cbd37f3 100644
--- a/drivers/iio/proximity/Kconfig
+++ b/drivers/iio/proximity/Kconfig
@@ -79,4 +79,15 @@ config SRF08
To compile this driver as a module, choose M here: the
module will be called srf08.
+config VL53L0X_I2C
+ tristate "STMicroelectronics VL53L0X ToF ranger sensor (I2C)"
+ depends on I2C
+ help
+ Say Y here to build a driver for STMicroelectronics VL53L0X
+ ToF ranger sensors with i2c interface.
+ This driver can be used to measure the distance of objects.
+
+ To compile this driver as a module, choose M here: the
+ module will be called vl53l0x-i2c.
+
endmenu
diff --git a/drivers/iio/proximity/Makefile b/drivers/iio/proximity/Makefile
index 4f4ed45e87ef..dedfb5bf3475 100644
--- a/drivers/iio/proximity/Makefile
+++ b/drivers/iio/proximity/Makefile
@@ -10,3 +10,5 @@ obj-$(CONFIG_RFD77402)+= rfd77402.o
obj-$(CONFIG_SRF04)+= srf04.o
obj-$(CONFIG_SRF08)+= srf08.o
obj-$(CONFIG_SX9500) += sx9500.o
+obj-$(CONFIG_VL53L0X_I2C) += vl53l0x-i2c.o
+
diff --git a/drivers/iio/proximity/vl53l0x-i2c.c
b/drivers/iio/proximity/vl53l0x-i2c.c
new file mode 100644
index ..0f7f124a38ed
--- /dev/null
+++ b/drivers/iio/proximity/vl53l0x-i2c.c
@@ -0,0 +1,184 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Support for ST's VL53L0X FlightSense ToF Ranger Sensor on a i2c bus.
+ *
+ * Copyright (C) 2016 STMicroelectronics Imaging Division.
+ * Copyright (C) 2018 Song Qiang
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define VL53L0X_DRV_NAME "vl53l0x-i2c"
+
+#define VL_REG_SYSRANGE_MODE_MASK GENMASK(3, 0)
+#define VL_REG_SYSRANGE_START 0x00
+#define VL_REG_SYSRANGE_MODE_SINGLESHOT0x00
+#define VL_REG_SYSRANGE_MODE_START_STOPBIT(0)
+#define VL_REG_SYSRANGE_MODE_BACKTOBACKBIT(1)
[PATCH v3] iio: proximity: Add driver support for ST's VL53L0X ToF ranging sensor.
This driver was originally written by ST in 2016 as a misc input device
driver, and hasn't been maintained for a long time. I grabbed some code
from it's API and reformed it into a iio proximity device driver.
This version of driver uses i2c bus to talk to the sensor and
polling for measuring completes, so no irq line is needed.
This version of driver supports only one-shot mode, and it can be
tested with reading from
/sys/bus/iio/devices/iio:deviceX/in_distance_raw
Signed-off-by: Song Qiang
---
Changes in v2:
- Clean up the register table.
- Sort header files declarations.
- Replace some bit definations with GENMASK() and BIT().
- Clean up some code and comments that's useless for now.
- Change the order of some the definations of some variables to reversed
xmas tree order.
- Using do...while() rather while and check.
- Replace pr_err() with dev_err().
- Remove device id declaration since we recommend to use DT.
- Remove .owner = THIS_MODULE.
- Replace probe() with probe_new() hook.
- Remove IIO_BUFFER and IIO_TRIGGERED_BUFFER dependences.
- Change the driver module name to vl53l0x-i2c.
- Align all the parameters if they are in the same function with open
parentheses.
- Replace iio_device_register() with devm_iio_device_register
for better resource management.
- Remove the vl53l0x_remove() since it's not needed.
- Remove dev_set_drvdata() since it's already setted above.
Changes in v3:
- Recover ST's copyright.
- Clean up indio_dev member in vl53l0x_data struct since it's
useless now.
- Replace __le16_to_cpu() with le16_to_cpu().
- Remove the iio_device_{claim|release}_direct_mode() since it's
only needed when we use buffered mode.
- Clean up some coding style problems.
.../bindings/iio/proximity/vl53l0x.txt| 12 ++
drivers/iio/proximity/Kconfig | 11 ++
drivers/iio/proximity/Makefile| 2 +
drivers/iio/proximity/vl53l0x-i2c.c | 184 ++
4 files changed, 209 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/proximity/vl53l0x.txt
create mode 100644 drivers/iio/proximity/vl53l0x-i2c.c
diff --git a/Documentation/devicetree/bindings/iio/proximity/vl53l0x.txt
b/Documentation/devicetree/bindings/iio/proximity/vl53l0x.txt
new file mode 100644
index ..64b69442f08e
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/proximity/vl53l0x.txt
@@ -0,0 +1,12 @@
+ST's VL53L0X ToF ranging sensor
+
+Required properties:
+ - compatible: must be "st,vl53l0x-i2c"
+ - reg: i2c address where to find the device
+
+Example:
+
+vl53l0x@29 {
+ compatible = "st,vl53l0x-i2c";
+ reg = <0x29>;
+};
diff --git a/drivers/iio/proximity/Kconfig b/drivers/iio/proximity/Kconfig
index f726f9427602..5f421cbd37f3 100644
--- a/drivers/iio/proximity/Kconfig
+++ b/drivers/iio/proximity/Kconfig
@@ -79,4 +79,15 @@ config SRF08
To compile this driver as a module, choose M here: the
module will be called srf08.
+config VL53L0X_I2C
+ tristate "STMicroelectronics VL53L0X ToF ranger sensor (I2C)"
+ depends on I2C
+ help
+ Say Y here to build a driver for STMicroelectronics VL53L0X
+ ToF ranger sensors with i2c interface.
+ This driver can be used to measure the distance of objects.
+
+ To compile this driver as a module, choose M here: the
+ module will be called vl53l0x-i2c.
+
endmenu
diff --git a/drivers/iio/proximity/Makefile b/drivers/iio/proximity/Makefile
index 4f4ed45e87ef..dedfb5bf3475 100644
--- a/drivers/iio/proximity/Makefile
+++ b/drivers/iio/proximity/Makefile
@@ -10,3 +10,5 @@ obj-$(CONFIG_RFD77402)+= rfd77402.o
obj-$(CONFIG_SRF04)+= srf04.o
obj-$(CONFIG_SRF08)+= srf08.o
obj-$(CONFIG_SX9500) += sx9500.o
+obj-$(CONFIG_VL53L0X_I2C) += vl53l0x-i2c.o
+
diff --git a/drivers/iio/proximity/vl53l0x-i2c.c
b/drivers/iio/proximity/vl53l0x-i2c.c
new file mode 100644
index ..0f7f124a38ed
--- /dev/null
+++ b/drivers/iio/proximity/vl53l0x-i2c.c
@@ -0,0 +1,184 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Support for ST's VL53L0X FlightSense ToF Ranger Sensor on a i2c bus.
+ *
+ * Copyright (C) 2016 STMicroelectronics Imaging Division.
+ * Copyright (C) 2018 Song Qiang
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define VL53L0X_DRV_NAME "vl53l0x-i2c"
+
+#define VL_REG_SYSRANGE_MODE_MASK GENMASK(3, 0)
+#define VL_REG_SYSRANGE_START 0x00
+#define VL_REG_SYSRANGE_MODE_SINGLESHOT0x00
+#define VL_REG_SYSRANGE_MODE_START_STOPBIT(0)
+#define VL_REG_SYSRANGE_MODE_BACKTOBACKBIT(1)
Re: [PATCH v2 1/6] drivers: qcom: rpmh-rsc: return if the controller is idle
On Tue, Sep 11 2018 at 16:39 -0600, Matthias Kaehlcke wrote:
Hi Raju/Lina,
On Fri, Jul 27, 2018 at 03:34:44PM +0530, Raju P L S S S N wrote:
From: Lina Iyer
Allow the controller status be queried. The controller is busy if it is
actively processing request.
Signed-off-by: Lina Iyer
Signed-off-by: Raju P.L.S.S.S.N
---
Changes in v2:
- Remove unnecessary EXPORT_SYMBOL
---
drivers/soc/qcom/rpmh-internal.h | 1 +
drivers/soc/qcom/rpmh-rsc.c | 20
2 files changed, 21 insertions(+)
diff --git a/drivers/soc/qcom/rpmh-internal.h b/drivers/soc/qcom/rpmh-internal.h
index a76..4ff43bf 100644
--- a/drivers/soc/qcom/rpmh-internal.h
+++ b/drivers/soc/qcom/rpmh-internal.h
@@ -108,6 +108,7 @@ struct rsc_drv {
int rpmh_rsc_write_ctrl_data(struct rsc_drv *drv,
const struct tcs_request *msg);
int rpmh_rsc_invalidate(struct rsc_drv *drv);
+bool rpmh_rsc_ctrlr_is_idle(struct rsc_drv *drv);
void rpmh_tx_done(const struct tcs_request *msg, int r);
diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
index 33fe9f9..42d0041 100644
--- a/drivers/soc/qcom/rpmh-rsc.c
+++ b/drivers/soc/qcom/rpmh-rsc.c
@@ -496,6 +496,26 @@ static int tcs_ctrl_write(struct rsc_drv *drv, const
struct tcs_request *msg)
}
/**
+ * rpmh_rsc_ctrlr_is_idle: Check if any of the AMCs are busy.
+ *
+ * @drv: The controller
+ *
+ * Returns true if the TCSes are engaged in handling requests.
+ */
+bool rpmh_rsc_ctrlr_is_idle(struct rsc_drv *drv)
+{
+ int m;
+ struct tcs_group *tcs = get_tcs_of_type(drv, ACTIVE_TCS);
+
+ for (m = tcs->offset; m < tcs->offset + tcs->num_tcs; m++) {
+ if (!tcs_is_free(drv, m))
+ return false;
+ }
+
+ return true;
+}
This looks racy, tcs_write() could be running simultaneously and use
TCSes that were seen as free by _is_idle(). This could be fixed by
holding tcs->lock (assuming this doesn't cause lock ordering problems).
However even with this tcs_write() could run right after releasing the
lock, using TCSes and the caller of _is_idle() would consider the
controller to be idle.
We could run this without the lock, since we are only reading a status.
Generally, this function is called from the idle code of the last CPU
and no CPU or active TCS request should be in progress, but if it were,
then this function would let the caller know we are not ready to do
idle. If there were no requests that were running at that time we read
the registers, we would not be making one after, since we are already
in the idle code and no requests are made there.
I understand, how it might appear racy, the context of the callling
function helps resolve that.
-- Lina
Re: [PATCH v2 1/6] drivers: qcom: rpmh-rsc: return if the controller is idle
On Tue, Sep 11 2018 at 16:39 -0600, Matthias Kaehlcke wrote:
Hi Raju/Lina,
On Fri, Jul 27, 2018 at 03:34:44PM +0530, Raju P L S S S N wrote:
From: Lina Iyer
Allow the controller status be queried. The controller is busy if it is
actively processing request.
Signed-off-by: Lina Iyer
Signed-off-by: Raju P.L.S.S.S.N
---
Changes in v2:
- Remove unnecessary EXPORT_SYMBOL
---
drivers/soc/qcom/rpmh-internal.h | 1 +
drivers/soc/qcom/rpmh-rsc.c | 20
2 files changed, 21 insertions(+)
diff --git a/drivers/soc/qcom/rpmh-internal.h b/drivers/soc/qcom/rpmh-internal.h
index a76..4ff43bf 100644
--- a/drivers/soc/qcom/rpmh-internal.h
+++ b/drivers/soc/qcom/rpmh-internal.h
@@ -108,6 +108,7 @@ struct rsc_drv {
int rpmh_rsc_write_ctrl_data(struct rsc_drv *drv,
const struct tcs_request *msg);
int rpmh_rsc_invalidate(struct rsc_drv *drv);
+bool rpmh_rsc_ctrlr_is_idle(struct rsc_drv *drv);
void rpmh_tx_done(const struct tcs_request *msg, int r);
diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
index 33fe9f9..42d0041 100644
--- a/drivers/soc/qcom/rpmh-rsc.c
+++ b/drivers/soc/qcom/rpmh-rsc.c
@@ -496,6 +496,26 @@ static int tcs_ctrl_write(struct rsc_drv *drv, const
struct tcs_request *msg)
}
/**
+ * rpmh_rsc_ctrlr_is_idle: Check if any of the AMCs are busy.
+ *
+ * @drv: The controller
+ *
+ * Returns true if the TCSes are engaged in handling requests.
+ */
+bool rpmh_rsc_ctrlr_is_idle(struct rsc_drv *drv)
+{
+ int m;
+ struct tcs_group *tcs = get_tcs_of_type(drv, ACTIVE_TCS);
+
+ for (m = tcs->offset; m < tcs->offset + tcs->num_tcs; m++) {
+ if (!tcs_is_free(drv, m))
+ return false;
+ }
+
+ return true;
+}
This looks racy, tcs_write() could be running simultaneously and use
TCSes that were seen as free by _is_idle(). This could be fixed by
holding tcs->lock (assuming this doesn't cause lock ordering problems).
However even with this tcs_write() could run right after releasing the
lock, using TCSes and the caller of _is_idle() would consider the
controller to be idle.
We could run this without the lock, since we are only reading a status.
Generally, this function is called from the idle code of the last CPU
and no CPU or active TCS request should be in progress, but if it were,
then this function would let the caller know we are not ready to do
idle. If there were no requests that were running at that time we read
the registers, we would not be making one after, since we are already
in the idle code and no requests are made there.
I understand, how it might appear racy, the context of the callling
function helps resolve that.
-- Lina
[Question] Are the trace APIs declared by "TRACE_EVENT(irq_handler_entry" allowed to be used in Ko?
After patch 7e066fb870fc ("tracepoints: add DECLARE_TRACE() and
DEFINE_TRACE()"),
the trace APIs declared by "TRACE_EVENT(irq_handler_entry" can not be directly
used
by ko, because it's not explicitly exported by EXPORT_TRACEPOINT_SYMBOL_GPL or
EXPORT_TRACEPOINT_SYMBOL.
Did we miss it? or it's not recommended to be used in ko?
-
commit 7e066fb870fcd1025ec3ba7bbde5d541094f4ce1
Author: Mathieu Desnoyers
Date: Fri Nov 14 17:47:47 2008 -0500
tracepoints: add DECLARE_TRACE() and DEFINE_TRACE()
Impact: API *CHANGE*. Must update all tracepoint users.
Add DEFINE_TRACE() to tracepoints to let them declare the tracepoint
structure in a single spot for all the kernel. It helps reducing memory
consumption, especially when declaring a lot of tracepoints, e.g. for
kmalloc tracing.
*API CHANGE WARNING*: now, DECLARE_TRACE() must be used in headers for
tracepoint declarations rather than DEFINE_TRACE(). This is the sane way
to do it. The name previously used was misleading.
Updates scheduler instrumentation to follow this API change.
--
Thanks!
BestRegards
[Question] Are the trace APIs declared by "TRACE_EVENT(irq_handler_entry" allowed to be used in Ko?
After patch 7e066fb870fc ("tracepoints: add DECLARE_TRACE() and
DEFINE_TRACE()"),
the trace APIs declared by "TRACE_EVENT(irq_handler_entry" can not be directly
used
by ko, because it's not explicitly exported by EXPORT_TRACEPOINT_SYMBOL_GPL or
EXPORT_TRACEPOINT_SYMBOL.
Did we miss it? or it's not recommended to be used in ko?
-
commit 7e066fb870fcd1025ec3ba7bbde5d541094f4ce1
Author: Mathieu Desnoyers
Date: Fri Nov 14 17:47:47 2008 -0500
tracepoints: add DECLARE_TRACE() and DEFINE_TRACE()
Impact: API *CHANGE*. Must update all tracepoint users.
Add DEFINE_TRACE() to tracepoints to let them declare the tracepoint
structure in a single spot for all the kernel. It helps reducing memory
consumption, especially when declaring a lot of tracepoints, e.g. for
kmalloc tracing.
*API CHANGE WARNING*: now, DECLARE_TRACE() must be used in headers for
tracepoint declarations rather than DEFINE_TRACE(). This is the sane way
to do it. The name previously used was misleading.
Updates scheduler instrumentation to follow this API change.
--
Thanks!
BestRegards
Re: Question: How to switch a process namespace by nsfs "device" and inode number directly?
Thank you, Andi! Yes, that's a situation, also it's an important one I guess. Another case is that a process running inside a container has exited but the container still alive.I think this is also a common case. The potential fix solutions I am thinking are following: - Using nsfs "device" and inum. This is why I am asking for your help. As we already have nsfs "device" and inum of each thread at least. - If the current thread has exited, it's probably the parent thread and the leader thread of that container are still alive. If we could have those threads' pid, then we could use setns. If the first item is not doable, I would like to try the second one. Thanks, Chengdong 在 2018/9/11 上午12:02, Andi Kleen 写道: On Mon, Sep 10, 2018 at 04:50:42PM +0800, Chengdong Li wrote: Hi folks, I am getting stuck by the lack of approach to switch process namespace by nsfs "device" and inode number in user-space, for example (mnt: 0xf000) From my best understanding, the normal way to do that is by setns system call. But setns only accept fd that refer to a opened namespace, sometimes we couldn't get it. For example: After perf record, perf report couldn't work well once the process that runs inside a container has exited, as the /proc/pid/ns doesn't exist anymore after process exit. The kernel name space doesn't exist anymore at this point, so there is simply no way to reconstruct it. Perhaps would need some higher level side band data for perf, similar as what is done for JITed code. Somehow the container run time needs to tell perf where to find the code. -Andi
Re: Question: How to switch a process namespace by nsfs "device" and inode number directly?
Thank you, Andi! Yes, that's a situation, also it's an important one I guess. Another case is that a process running inside a container has exited but the container still alive.I think this is also a common case. The potential fix solutions I am thinking are following: - Using nsfs "device" and inum. This is why I am asking for your help. As we already have nsfs "device" and inum of each thread at least. - If the current thread has exited, it's probably the parent thread and the leader thread of that container are still alive. If we could have those threads' pid, then we could use setns. If the first item is not doable, I would like to try the second one. Thanks, Chengdong 在 2018/9/11 上午12:02, Andi Kleen 写道: On Mon, Sep 10, 2018 at 04:50:42PM +0800, Chengdong Li wrote: Hi folks, I am getting stuck by the lack of approach to switch process namespace by nsfs "device" and inode number in user-space, for example (mnt: 0xf000) From my best understanding, the normal way to do that is by setns system call. But setns only accept fd that refer to a opened namespace, sometimes we couldn't get it. For example: After perf record, perf report couldn't work well once the process that runs inside a container has exited, as the /proc/pid/ns doesn't exist anymore after process exit. The kernel name space doesn't exist anymore at this point, so there is simply no way to reconstruct it. Perhaps would need some higher level side band data for perf, similar as what is done for JITed code. Somehow the container run time needs to tell perf where to find the code. -Andi
Re: [PATCH i2c-next v6] i2c: aspeed: Handle master/slave combined irq events properly
On Tue, Sep 11, 2018 at 04:58:44PM -0700, Jae Hyun Yoo wrote: > On 9/11/2018 4:33 PM, Guenter Roeck wrote: > >Looking into the patch, clearing the interrupt status at the end of an > >interrupt handler is always suspicious and tends to result in race > >conditions (because additional interrupts may have arrived while handling > >the existing interrupts, or because interrupt handling itself may trigger > >another interrupt). With that in mind, the following patch fixes the > >problem for me. > > > >Guenter > > > >--- > > > >diff --git a/drivers/i2c/busses/i2c-aspeed.c > >b/drivers/i2c/busses/i2c-aspeed.c > >index c258c4d9a4c0..c488e6950b7c 100644 > >--- a/drivers/i2c/busses/i2c-aspeed.c > >+++ b/drivers/i2c/busses/i2c-aspeed.c > >@@ -552,6 +552,8 @@ static irqreturn_t aspeed_i2c_bus_irq(int irq, void > >*dev_id) > > spin_lock(>lock); > > irq_received = readl(bus->base + ASPEED_I2C_INTR_STS_REG); > >+/* Ack all interrupt bits. */ > >+writel(irq_received, bus->base + ASPEED_I2C_INTR_STS_REG); > > irq_remaining = irq_received; > > #if IS_ENABLED(CONFIG_I2C_SLAVE) > >@@ -584,8 +586,6 @@ static irqreturn_t aspeed_i2c_bus_irq(int irq, void > >*dev_id) > > "irq handled != irq. expected 0x%08x, but was 0x%08x\n", > > irq_received, irq_handled); > >-/* Ack all interrupt bits. */ > >-writel(irq_received, bus->base + ASPEED_I2C_INTR_STS_REG); > > spin_unlock(>lock); > > return irq_remaining ? IRQ_NONE : IRQ_HANDLED; > > } > > > > My intention of putting the code at the end of interrupt handler was, > to reduce possibility of combined irq calls which is explained in this > patch. But YES, I agree with you. It could make a potential race Hmm, yes, but that doesn't explain why it would make sense to acknowledge the interrupt late. The interrupt ack only means "I am going to handle these interrupts". If additional interrupts arrive while the interrupt handler is active, those will have to be acknowledged separately. Sure, there is a risk that an interrupt arrives while the handler is running, and that it is handled but not acknowledged. That can happen with pretty much all interrupt handlers, and there are mitigations to limit the impact (for example, read the interrupt status register in a loop until no more interrupts are pending). But acknowledging an interrupt that was possibly not handled is always bad idea. Thanks, Guenter
Re: [PATCH i2c-next v6] i2c: aspeed: Handle master/slave combined irq events properly
On Tue, Sep 11, 2018 at 04:58:44PM -0700, Jae Hyun Yoo wrote: > On 9/11/2018 4:33 PM, Guenter Roeck wrote: > >Looking into the patch, clearing the interrupt status at the end of an > >interrupt handler is always suspicious and tends to result in race > >conditions (because additional interrupts may have arrived while handling > >the existing interrupts, or because interrupt handling itself may trigger > >another interrupt). With that in mind, the following patch fixes the > >problem for me. > > > >Guenter > > > >--- > > > >diff --git a/drivers/i2c/busses/i2c-aspeed.c > >b/drivers/i2c/busses/i2c-aspeed.c > >index c258c4d9a4c0..c488e6950b7c 100644 > >--- a/drivers/i2c/busses/i2c-aspeed.c > >+++ b/drivers/i2c/busses/i2c-aspeed.c > >@@ -552,6 +552,8 @@ static irqreturn_t aspeed_i2c_bus_irq(int irq, void > >*dev_id) > > spin_lock(>lock); > > irq_received = readl(bus->base + ASPEED_I2C_INTR_STS_REG); > >+/* Ack all interrupt bits. */ > >+writel(irq_received, bus->base + ASPEED_I2C_INTR_STS_REG); > > irq_remaining = irq_received; > > #if IS_ENABLED(CONFIG_I2C_SLAVE) > >@@ -584,8 +586,6 @@ static irqreturn_t aspeed_i2c_bus_irq(int irq, void > >*dev_id) > > "irq handled != irq. expected 0x%08x, but was 0x%08x\n", > > irq_received, irq_handled); > >-/* Ack all interrupt bits. */ > >-writel(irq_received, bus->base + ASPEED_I2C_INTR_STS_REG); > > spin_unlock(>lock); > > return irq_remaining ? IRQ_NONE : IRQ_HANDLED; > > } > > > > My intention of putting the code at the end of interrupt handler was, > to reduce possibility of combined irq calls which is explained in this > patch. But YES, I agree with you. It could make a potential race Hmm, yes, but that doesn't explain why it would make sense to acknowledge the interrupt late. The interrupt ack only means "I am going to handle these interrupts". If additional interrupts arrive while the interrupt handler is active, those will have to be acknowledged separately. Sure, there is a risk that an interrupt arrives while the handler is running, and that it is handled but not acknowledged. That can happen with pretty much all interrupt handlers, and there are mitigations to limit the impact (for example, read the interrupt status register in a loop until no more interrupts are pending). But acknowledging an interrupt that was possibly not handled is always bad idea. Thanks, Guenter
Re: [PATCH v3] ARM: dts: imx6ul: Add DTS for ConnectCore 6UL SBC Pro
On Mon, Sep 10, 2018 at 11:37:52AM +0200, Alex Gonzalez wrote: > The ConnectCore 6UL Single Board Computer (SBC) Pro contains the > ConnectCore 6UL System-On-Module. > > Its hardware specifications are: > > * 256MB DDR3 memory > * On module 256MB NAND flash > * Dual 10/100 Ethernet > * USB Host and USB OTG > * Parallel RGB display header > * LVDS display header > * CSI camera > * GPIO header > * I2C, SPI, CAN headers > * PCIe mini card and micro SIM slot > * MicroSD external storage > * On board 4GB eMMC flash > * Audio headphone, line in/out, microphone lines > > Signed-off-by: Alex Gonzalez Applied, thanks.
Re: [PATCH v3] ARM: dts: imx6ul: Add DTS for ConnectCore 6UL SBC Pro
On Mon, Sep 10, 2018 at 11:37:52AM +0200, Alex Gonzalez wrote: > The ConnectCore 6UL Single Board Computer (SBC) Pro contains the > ConnectCore 6UL System-On-Module. > > Its hardware specifications are: > > * 256MB DDR3 memory > * On module 256MB NAND flash > * Dual 10/100 Ethernet > * USB Host and USB OTG > * Parallel RGB display header > * LVDS display header > * CSI camera > * GPIO header > * I2C, SPI, CAN headers > * PCIe mini card and micro SIM slot > * MicroSD external storage > * On board 4GB eMMC flash > * Audio headphone, line in/out, microphone lines > > Signed-off-by: Alex Gonzalez Applied, thanks.
Re: [PATCH v9 6/6] ARM: dts: imx6: RIoTboard provide standby on power off option
On Thu, Aug 02, 2018 at 12:34:25PM +0200, Oleksij Rempel wrote: > This board, as well as some other boards with i.MX6 and a PMIC, uses a > "PMIC_STBY_REQ" line to notify the PMIC about a state change. > The PMIC is programmed for a specific state change before triggering the > line. > In this case, PMIC_STBY_REQ can be used for stand by, sleep > and power off modes. > > Signed-off-by: Oleksij Rempel Applied, thanks.
Re: [PATCH v9 6/6] ARM: dts: imx6: RIoTboard provide standby on power off option
On Thu, Aug 02, 2018 at 12:34:25PM +0200, Oleksij Rempel wrote: > This board, as well as some other boards with i.MX6 and a PMIC, uses a > "PMIC_STBY_REQ" line to notify the PMIC about a state change. > The PMIC is programmed for a specific state change before triggering the > line. > In this case, PMIC_STBY_REQ can be used for stand by, sleep > and power off modes. > > Signed-off-by: Oleksij Rempel Applied, thanks.
Re: KASAN: use-after-free Read in cma_bind_port
syzbot has found a reproducer for the following crash on: HEAD commit:11da3a7f84f1 Linux 4.19-rc3 git tree: upstream console output: https://syzkaller.appspot.com/x/log.txt?x=11766c6940 kernel config: https://syzkaller.appspot.com/x/.config?x=9917ff4b798e1a1e dashboard link: https://syzkaller.appspot.com/bug?extid=da2591e115d57a9cbb8b compiler: gcc (GCC) 8.0.1 20180413 (experimental) syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1686969e40 IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+da2591e115d57a9cb...@syzkaller.appspotmail.com 8021q: adding VLAN 0 to HW filter on device team0 8021q: adding VLAN 0 to HW filter on device team0 8021q: adding VLAN 0 to HW filter on device team0 hrtimer: interrupt took 34369 ns == BUG: KASAN: use-after-free in cma_bind_port+0x35d/0x420 drivers/infiniband/core/cma.c:3059 Read of size 2 at addr 8801b7b056a0 by task syz-executor3/7271 CPU: 1 PID: 7271 Comm: syz-executor3 Not tainted 4.19.0-rc3+ #231 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Call Trace: __dump_stack lib/dump_stack.c:77 [inline] dump_stack+0x1c4/0x2b4 lib/dump_stack.c:113 print_address_description.cold.8+0x9/0x1ff mm/kasan/report.c:256 kasan_report_error mm/kasan/report.c:354 [inline] kasan_report.cold.9+0x242/0x309 mm/kasan/report.c:412 __asan_report_load2_noabort+0x14/0x20 mm/kasan/report.c:431 cma_bind_port+0x35d/0x420 drivers/infiniband/core/cma.c:3059 cma_alloc_port+0x115/0x180 drivers/infiniband/core/cma.c:3095 cma_alloc_any_port drivers/infiniband/core/cma.c:3160 [inline] cma_get_port drivers/infiniband/core/cma.c:3314 [inline] rdma_bind_addr+0x1765/0x23d0 drivers/infiniband/core/cma.c:3434 cma_bind_addr drivers/infiniband/core/cma.c:2963 [inline] rdma_resolve_addr+0x4e2/0x2770 drivers/infiniband/core/cma.c:2974 ucma_resolve_ip+0x242/0x2a0 drivers/infiniband/core/ucma.c:711 ucma_write+0x336/0x420 drivers/infiniband/core/ucma.c:1680 __vfs_write+0x119/0x9f0 fs/read_write.c:485 vfs_write+0x1fc/0x560 fs/read_write.c:549 ksys_write+0x101/0x260 fs/read_write.c:598 __do_sys_write fs/read_write.c:610 [inline] __se_sys_write fs/read_write.c:607 [inline] __x64_sys_write+0x73/0xb0 fs/read_write.c:607 do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290 entry_SYSCALL_64_after_hwframe+0x49/0xbe RIP: 0033:0x4572d9 Code: fd b4 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 cb b4 fb ff c3 66 2e 0f 1f 84 00 00 00 00 RSP: 002b:7f71074b0c78 EFLAGS: 0246 ORIG_RAX: 0001 RAX: ffda RBX: 7f71074b16d4 RCX: 004572d9 RDX: 0048 RSI: 2240 RDI: 0008 RBP: 009300a0 R08: R09: R10: R11: 0246 R12: R13: 004d83c0 R14: 004c1e90 R15: Allocated by task 7285: save_stack+0x43/0xd0 mm/kasan/kasan.c:448 set_track mm/kasan/kasan.c:460 [inline] kasan_kmalloc+0xc7/0xe0 mm/kasan/kasan.c:553 kmem_cache_alloc_trace+0x152/0x750 mm/slab.c:3620 kmalloc include/linux/slab.h:513 [inline] kzalloc include/linux/slab.h:707 [inline] __rdma_create_id+0xdf/0x790 drivers/infiniband/core/cma.c:782 ucma_create_id+0x39b/0x990 drivers/infiniband/core/ucma.c:502 ucma_write+0x336/0x420 drivers/infiniband/core/ucma.c:1680 __vfs_write+0x119/0x9f0 fs/read_write.c:485 vfs_write+0x1fc/0x560 fs/read_write.c:549 ksys_write+0x101/0x260 fs/read_write.c:598 __do_sys_write fs/read_write.c:610 [inline] __se_sys_write fs/read_write.c:607 [inline] __x64_sys_write+0x73/0xb0 fs/read_write.c:607 do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290 entry_SYSCALL_64_after_hwframe+0x49/0xbe Freed by task 7261: save_stack+0x43/0xd0 mm/kasan/kasan.c:448 set_track mm/kasan/kasan.c:460 [inline] __kasan_slab_free+0x102/0x150 mm/kasan/kasan.c:521 kasan_slab_free+0xe/0x10 mm/kasan/kasan.c:528 __cache_free mm/slab.c:3498 [inline] kfree+0xcf/0x230 mm/slab.c:3813 rdma_destroy_id+0x835/0xcc0 drivers/infiniband/core/cma.c:1737 ucma_close+0x100/0x300 drivers/infiniband/core/ucma.c:1759 __fput+0x385/0xa30 fs/file_table.c:278 fput+0x15/0x20 fs/file_table.c:309 task_work_run+0x1e8/0x2a0 kernel/task_work.c:113 tracehook_notify_resume include/linux/tracehook.h:193 [inline] exit_to_usermode_loop+0x318/0x380 arch/x86/entry/common.c:166 prepare_exit_to_usermode arch/x86/entry/common.c:197 [inline] syscall_return_slowpath arch/x86/entry/common.c:268 [inline] do_syscall_64+0x6be/0x820 arch/x86/entry/common.c:293 entry_SYSCALL_64_after_hwframe+0x49/0xbe The buggy address belongs to the object at 8801b7b05680 which belongs to the cache kmalloc-2048 of size 2048 The buggy address is located 32 bytes inside of 2048-byte region
Re: KASAN: use-after-free Read in cma_bind_port
syzbot has found a reproducer for the following crash on: HEAD commit:11da3a7f84f1 Linux 4.19-rc3 git tree: upstream console output: https://syzkaller.appspot.com/x/log.txt?x=11766c6940 kernel config: https://syzkaller.appspot.com/x/.config?x=9917ff4b798e1a1e dashboard link: https://syzkaller.appspot.com/bug?extid=da2591e115d57a9cbb8b compiler: gcc (GCC) 8.0.1 20180413 (experimental) syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1686969e40 IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+da2591e115d57a9cb...@syzkaller.appspotmail.com 8021q: adding VLAN 0 to HW filter on device team0 8021q: adding VLAN 0 to HW filter on device team0 8021q: adding VLAN 0 to HW filter on device team0 hrtimer: interrupt took 34369 ns == BUG: KASAN: use-after-free in cma_bind_port+0x35d/0x420 drivers/infiniband/core/cma.c:3059 Read of size 2 at addr 8801b7b056a0 by task syz-executor3/7271 CPU: 1 PID: 7271 Comm: syz-executor3 Not tainted 4.19.0-rc3+ #231 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Call Trace: __dump_stack lib/dump_stack.c:77 [inline] dump_stack+0x1c4/0x2b4 lib/dump_stack.c:113 print_address_description.cold.8+0x9/0x1ff mm/kasan/report.c:256 kasan_report_error mm/kasan/report.c:354 [inline] kasan_report.cold.9+0x242/0x309 mm/kasan/report.c:412 __asan_report_load2_noabort+0x14/0x20 mm/kasan/report.c:431 cma_bind_port+0x35d/0x420 drivers/infiniband/core/cma.c:3059 cma_alloc_port+0x115/0x180 drivers/infiniband/core/cma.c:3095 cma_alloc_any_port drivers/infiniband/core/cma.c:3160 [inline] cma_get_port drivers/infiniband/core/cma.c:3314 [inline] rdma_bind_addr+0x1765/0x23d0 drivers/infiniband/core/cma.c:3434 cma_bind_addr drivers/infiniband/core/cma.c:2963 [inline] rdma_resolve_addr+0x4e2/0x2770 drivers/infiniband/core/cma.c:2974 ucma_resolve_ip+0x242/0x2a0 drivers/infiniband/core/ucma.c:711 ucma_write+0x336/0x420 drivers/infiniband/core/ucma.c:1680 __vfs_write+0x119/0x9f0 fs/read_write.c:485 vfs_write+0x1fc/0x560 fs/read_write.c:549 ksys_write+0x101/0x260 fs/read_write.c:598 __do_sys_write fs/read_write.c:610 [inline] __se_sys_write fs/read_write.c:607 [inline] __x64_sys_write+0x73/0xb0 fs/read_write.c:607 do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290 entry_SYSCALL_64_after_hwframe+0x49/0xbe RIP: 0033:0x4572d9 Code: fd b4 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 cb b4 fb ff c3 66 2e 0f 1f 84 00 00 00 00 RSP: 002b:7f71074b0c78 EFLAGS: 0246 ORIG_RAX: 0001 RAX: ffda RBX: 7f71074b16d4 RCX: 004572d9 RDX: 0048 RSI: 2240 RDI: 0008 RBP: 009300a0 R08: R09: R10: R11: 0246 R12: R13: 004d83c0 R14: 004c1e90 R15: Allocated by task 7285: save_stack+0x43/0xd0 mm/kasan/kasan.c:448 set_track mm/kasan/kasan.c:460 [inline] kasan_kmalloc+0xc7/0xe0 mm/kasan/kasan.c:553 kmem_cache_alloc_trace+0x152/0x750 mm/slab.c:3620 kmalloc include/linux/slab.h:513 [inline] kzalloc include/linux/slab.h:707 [inline] __rdma_create_id+0xdf/0x790 drivers/infiniband/core/cma.c:782 ucma_create_id+0x39b/0x990 drivers/infiniband/core/ucma.c:502 ucma_write+0x336/0x420 drivers/infiniband/core/ucma.c:1680 __vfs_write+0x119/0x9f0 fs/read_write.c:485 vfs_write+0x1fc/0x560 fs/read_write.c:549 ksys_write+0x101/0x260 fs/read_write.c:598 __do_sys_write fs/read_write.c:610 [inline] __se_sys_write fs/read_write.c:607 [inline] __x64_sys_write+0x73/0xb0 fs/read_write.c:607 do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290 entry_SYSCALL_64_after_hwframe+0x49/0xbe Freed by task 7261: save_stack+0x43/0xd0 mm/kasan/kasan.c:448 set_track mm/kasan/kasan.c:460 [inline] __kasan_slab_free+0x102/0x150 mm/kasan/kasan.c:521 kasan_slab_free+0xe/0x10 mm/kasan/kasan.c:528 __cache_free mm/slab.c:3498 [inline] kfree+0xcf/0x230 mm/slab.c:3813 rdma_destroy_id+0x835/0xcc0 drivers/infiniband/core/cma.c:1737 ucma_close+0x100/0x300 drivers/infiniband/core/ucma.c:1759 __fput+0x385/0xa30 fs/file_table.c:278 fput+0x15/0x20 fs/file_table.c:309 task_work_run+0x1e8/0x2a0 kernel/task_work.c:113 tracehook_notify_resume include/linux/tracehook.h:193 [inline] exit_to_usermode_loop+0x318/0x380 arch/x86/entry/common.c:166 prepare_exit_to_usermode arch/x86/entry/common.c:197 [inline] syscall_return_slowpath arch/x86/entry/common.c:268 [inline] do_syscall_64+0x6be/0x820 arch/x86/entry/common.c:293 entry_SYSCALL_64_after_hwframe+0x49/0xbe The buggy address belongs to the object at 8801b7b05680 which belongs to the cache kmalloc-2048 of size 2048 The buggy address is located 32 bytes inside of 2048-byte region
Re: [PATCH v9 2/6] ARM: imx6: register pm_power_off handler if "fsl,pmic-stby-poweroff" is set
On Thu, Aug 02, 2018 at 12:34:21PM +0200, Oleksij Rempel wrote: > One of the Freescale recommended sequences for power off with external > PMIC is the following: > ... > 3. SoC is programming PMIC for power off when standby is asserted. > 4. In CCM STOP mode, Standby is asserted, PMIC gates SoC supplies. > > See: > http://www.nxp.com/assets/documents/data/en/reference-manuals/IMX6DQRM.pdf > page 5083 > > This patch implements step 4. of this sequence. > > Signed-off-by: Oleksij Rempel Applied, thanks.
Re: [PATCH v9 2/6] ARM: imx6: register pm_power_off handler if "fsl,pmic-stby-poweroff" is set
On Thu, Aug 02, 2018 at 12:34:21PM +0200, Oleksij Rempel wrote: > One of the Freescale recommended sequences for power off with external > PMIC is the following: > ... > 3. SoC is programming PMIC for power off when standby is asserted. > 4. In CCM STOP mode, Standby is asserted, PMIC gates SoC supplies. > > See: > http://www.nxp.com/assets/documents/data/en/reference-manuals/IMX6DQRM.pdf > page 5083 > > This patch implements step 4. of this sequence. > > Signed-off-by: Oleksij Rempel Applied, thanks.
Re: [PATCH v3 3/3] drivers: soc: xilinx: Add ZynqMP PM driver
Hi!
[ Thanks a lot for upstreaming this.. ]
On Tue, Sep 11, 2018 at 02:34:57PM -0700, Jolly Shah wrote:
> From: Rajan Vaja
>
> Add ZynqMP PM driver. PM driver provides power management
> support for ZynqMP.
>
> Signed-off-by: Rajan Vaja
> Signed-off-by: Jolly Shah
> ---
[...]
> +static irqreturn_t zynqmp_pm_isr(int irq, void *data)
> +{
> + u32 payload[CB_PAYLOAD_SIZE];
> +
> + zynqmp_pm_get_callback_data(payload);
> +
> + /* First element is callback API ID, others are callback arguments */
> + if (payload[0] == PM_INIT_SUSPEND_CB) {
> + if (work_pending(_pm_init_suspend_work->callback_work))
> + goto done;
> +
> + /* Copy callback arguments into work's structure */
> + memcpy(zynqmp_pm_init_suspend_work->args, [1],
> +sizeof(zynqmp_pm_init_suspend_work->args));
> +
> + queue_work(system_unbound_wq,
> +_pm_init_suspend_work->callback_work);
We already have devm_request_threaded_irq() which can does this
automatically for us.
Use that method to register the ISR instead, then if there's more
work to do, just do the memcpy and return IRQ_WAKE_THREAD.
> + }
> +
> +done:
> + return IRQ_HANDLED;
> +}
> +
> +/**
> + * zynqmp_pm_init_suspend_work_fn() - Initialize suspend
> + * @work:Pointer to work_struct
> + *
> + * Bottom-half of PM callback IRQ handler.
> + */
> +static void zynqmp_pm_init_suspend_work_fn(struct work_struct *work)
> +{
> + struct zynqmp_pm_work_struct *pm_work =
> + container_of(work, struct zynqmp_pm_work_struct, callback_work);
> +
> + if (pm_work->args[0] == ZYNQMP_PM_SUSPEND_REASON_SYSTEM_SHUTDOWN) {
we_really_seem_to_love_long_40_col_names_for_some_reason
> + orderly_poweroff(true);
> + } else if (pm_work->args[0] ==
> +ZYNQMP_PM_SUSPEND_REASON_POWER_UNIT_REQUEST) {
Ditto
[...]
> +/**
> + * zynqmp_pm_sysfs_init() - Initialize PM driver sysfs interface
> + * @dev: Pointer to device structure
> + *
> + * Return: 0 on success, negative error code otherwise
> + */
> +static int zynqmp_pm_sysfs_init(struct device *dev)
> +{
> + return sysfs_create_file(>kobj, _attr_suspend_mode.attr);
> +}
> +
Sysfs file is created in platform driver's probe(), but is not
removed anywhere in the code.
What happens if this is built as a module? Am I missing something
obvious?
Moreover, what's the wisdom of creating a one-liner function with
a huge six-line comment that:
a) _purely_ wraps sysfs_create_file(); no extra logic
b) is called only once
c) and not passed as a function pointer anywhere
IMO Such one-liner translators obfuscate the code and review
process with no apparent gain..
> +/**
> + * zynqmp_pm_probe() - Probe existence of the PMU Firmware
> + * and initialize debugfs interface
> + *
> + * @pdev:Pointer to the platform_device structure
> + *
> + * Return: Returns 0 on success, negative error code otherwise
> + */
Again, huge 8-line comment that provide no value.
If anyone wants to know what a platform driver probe() does, he
or she better check the primary references at:
- Documentation/driver-model/platform.txt
- include/linux/platform_device.h
and not the comment above..
> +static int zynqmp_pm_probe(struct platform_device *pdev)
> +{
> + int ret, irq;
> + u32 pm_api_version;
> +
> + const struct zynqmp_eemi_ops *eemi_ops = zynqmp_pm_get_eemi_ops();
> +
> + if (!eemi_ops || !eemi_ops->get_api_version || !eemi_ops->init_finalize)
> + return -ENXIO;
> +
> + eemi_ops->init_finalize();
> + eemi_ops->get_api_version(_api_version);
> +
> + /* Check PM API version number */
> + if (pm_api_version < ZYNQMP_PM_VERSION)
> + return -ENODEV;
> +
> + irq = platform_get_irq(pdev, 0);
> + if (irq <= 0)
> + return -ENXIO;
> +
> + ret = devm_request_irq(>dev, irq, zynqmp_pm_isr, IRQF_SHARED,
> +dev_name(>dev), pdev);
> + if (ret) {
> + dev_err(>dev, "request_irq '%d' failed with %d\n",
> + irq, ret);
> + return ret;
> + }
> +
> + zynqmp_pm_init_suspend_work =
> + devm_kzalloc(>dev, sizeof(struct zynqmp_pm_work_struct),
> + GFP_KERNEL);
> + if (!zynqmp_pm_init_suspend_work)
> + return -ENOMEM;
> +
> + INIT_WORK(_pm_init_suspend_work->callback_work,
> + zynqmp_pm_init_suspend_work_fn);
> +
Let's use devm_request_threaded_irq(). Then we can completely
remove the work_struct, INIT_WORK(), and queuue_work() bits.
> + ret = zynqmp_pm_sysfs_init(>dev);
> + if (ret) {
> + dev_err(>dev, "unable to initialize sysfs interface\n");
> + return ret;
> + }
> +
> + return ret;
Just return 0 please. BTW ret was declared without initialization.
> +}
> +
> +static const struct of_device_id
Re: [PATCH v3 3/3] drivers: soc: xilinx: Add ZynqMP PM driver
Hi!
[ Thanks a lot for upstreaming this.. ]
On Tue, Sep 11, 2018 at 02:34:57PM -0700, Jolly Shah wrote:
> From: Rajan Vaja
>
> Add ZynqMP PM driver. PM driver provides power management
> support for ZynqMP.
>
> Signed-off-by: Rajan Vaja
> Signed-off-by: Jolly Shah
> ---
[...]
> +static irqreturn_t zynqmp_pm_isr(int irq, void *data)
> +{
> + u32 payload[CB_PAYLOAD_SIZE];
> +
> + zynqmp_pm_get_callback_data(payload);
> +
> + /* First element is callback API ID, others are callback arguments */
> + if (payload[0] == PM_INIT_SUSPEND_CB) {
> + if (work_pending(_pm_init_suspend_work->callback_work))
> + goto done;
> +
> + /* Copy callback arguments into work's structure */
> + memcpy(zynqmp_pm_init_suspend_work->args, [1],
> +sizeof(zynqmp_pm_init_suspend_work->args));
> +
> + queue_work(system_unbound_wq,
> +_pm_init_suspend_work->callback_work);
We already have devm_request_threaded_irq() which can does this
automatically for us.
Use that method to register the ISR instead, then if there's more
work to do, just do the memcpy and return IRQ_WAKE_THREAD.
> + }
> +
> +done:
> + return IRQ_HANDLED;
> +}
> +
> +/**
> + * zynqmp_pm_init_suspend_work_fn() - Initialize suspend
> + * @work:Pointer to work_struct
> + *
> + * Bottom-half of PM callback IRQ handler.
> + */
> +static void zynqmp_pm_init_suspend_work_fn(struct work_struct *work)
> +{
> + struct zynqmp_pm_work_struct *pm_work =
> + container_of(work, struct zynqmp_pm_work_struct, callback_work);
> +
> + if (pm_work->args[0] == ZYNQMP_PM_SUSPEND_REASON_SYSTEM_SHUTDOWN) {
we_really_seem_to_love_long_40_col_names_for_some_reason
> + orderly_poweroff(true);
> + } else if (pm_work->args[0] ==
> +ZYNQMP_PM_SUSPEND_REASON_POWER_UNIT_REQUEST) {
Ditto
[...]
> +/**
> + * zynqmp_pm_sysfs_init() - Initialize PM driver sysfs interface
> + * @dev: Pointer to device structure
> + *
> + * Return: 0 on success, negative error code otherwise
> + */
> +static int zynqmp_pm_sysfs_init(struct device *dev)
> +{
> + return sysfs_create_file(>kobj, _attr_suspend_mode.attr);
> +}
> +
Sysfs file is created in platform driver's probe(), but is not
removed anywhere in the code.
What happens if this is built as a module? Am I missing something
obvious?
Moreover, what's the wisdom of creating a one-liner function with
a huge six-line comment that:
a) _purely_ wraps sysfs_create_file(); no extra logic
b) is called only once
c) and not passed as a function pointer anywhere
IMO Such one-liner translators obfuscate the code and review
process with no apparent gain..
> +/**
> + * zynqmp_pm_probe() - Probe existence of the PMU Firmware
> + * and initialize debugfs interface
> + *
> + * @pdev:Pointer to the platform_device structure
> + *
> + * Return: Returns 0 on success, negative error code otherwise
> + */
Again, huge 8-line comment that provide no value.
If anyone wants to know what a platform driver probe() does, he
or she better check the primary references at:
- Documentation/driver-model/platform.txt
- include/linux/platform_device.h
and not the comment above..
> +static int zynqmp_pm_probe(struct platform_device *pdev)
> +{
> + int ret, irq;
> + u32 pm_api_version;
> +
> + const struct zynqmp_eemi_ops *eemi_ops = zynqmp_pm_get_eemi_ops();
> +
> + if (!eemi_ops || !eemi_ops->get_api_version || !eemi_ops->init_finalize)
> + return -ENXIO;
> +
> + eemi_ops->init_finalize();
> + eemi_ops->get_api_version(_api_version);
> +
> + /* Check PM API version number */
> + if (pm_api_version < ZYNQMP_PM_VERSION)
> + return -ENODEV;
> +
> + irq = platform_get_irq(pdev, 0);
> + if (irq <= 0)
> + return -ENXIO;
> +
> + ret = devm_request_irq(>dev, irq, zynqmp_pm_isr, IRQF_SHARED,
> +dev_name(>dev), pdev);
> + if (ret) {
> + dev_err(>dev, "request_irq '%d' failed with %d\n",
> + irq, ret);
> + return ret;
> + }
> +
> + zynqmp_pm_init_suspend_work =
> + devm_kzalloc(>dev, sizeof(struct zynqmp_pm_work_struct),
> + GFP_KERNEL);
> + if (!zynqmp_pm_init_suspend_work)
> + return -ENOMEM;
> +
> + INIT_WORK(_pm_init_suspend_work->callback_work,
> + zynqmp_pm_init_suspend_work_fn);
> +
Let's use devm_request_threaded_irq(). Then we can completely
remove the work_struct, INIT_WORK(), and queuue_work() bits.
> + ret = zynqmp_pm_sysfs_init(>dev);
> + if (ret) {
> + dev_err(>dev, "unable to initialize sysfs interface\n");
> + return ret;
> + }
> +
> + return ret;
Just return 0 please. BTW ret was declared without initialization.
> +}
> +
> +static const struct of_device_id
