Re: [PATCH v11 0/2] PWM support for HiFive Unleashed
'On Mon, Mar 25, 2019 at 9:24 PM Andreas Schwab wrote: > > > I still don't see any improvement. FYI, this is the patch I use for the > device tree: > I am sharing you my test environment which has been working for me so that you can duplicate at your end I have tested the patch on Linux v5.0-rc1 commit id: bfeffd155283772bbe78c6a05dec7c0128ee500c with additional board related patches which can be found at dev/yashs/pwm_5.0-rc1 branch of: https://github.com/yashshah7/riscv-linux.git I have compiled the dt file (found at above repo) and replaced the compiled dtb file with 'fsbl/ux00_fsbl.dtb' and re-build fsbl.bin. Make sure your dt file changes are getting reflected on target under '/proc/device-tree/' Below is the .config file content: # # Automatically generated file; DO NOT EDIT. # Linux/riscv 5.0.0-rc1 Kernel Configuration # # # Compiler: riscv64-unknown-elf-gcc (GCC) 7.2.0 # CONFIG_CC_IS_GCC=y CONFIG_GCC_VERSION=70200 CONFIG_CLANG_VERSION=0 CONFIG_CC_HAS_ASM_GOTO=y CONFIG_IRQ_WORK=y CONFIG_THREAD_INFO_IN_TASK=y # # General setup # CONFIG_INIT_ENV_ARG_LIMIT=32 CONFIG_LOCALVERSION="" CONFIG_LOCALVERSION_AUTO=y CONFIG_BUILD_SALT="" 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 # # IRQ subsystem # CONFIG_GENERIC_IRQ_SHOW=y CONFIG_IRQ_DOMAIN=y CONFIG_GENERIC_MSI_IRQ=y CONFIG_SPARSE_IRQ=y CONFIG_GENERIC_IRQ_MULTI_HANDLER=y CONFIG_GENERIC_CLOCKEVENTS=y # # Timers subsystem # CONFIG_HZ_PERIODIC=y CONFIG_PREEMPT_NONE=y # # CPU/Task time and stats accounting # CONFIG_TICK_CPU_ACCOUNTING=y CONFIG_CPU_ISOLATION=y # # RCU Subsystem # CONFIG_TREE_RCU=y CONFIG_SRCU=y CONFIG_TREE_SRCU=y CONFIG_RCU_STALL_COMMON=y CONFIG_RCU_NEED_SEGCBLIST=y CONFIG_BUILD_BIN2C=y CONFIG_IKCONFIG=y CONFIG_IKCONFIG_PROC=y CONFIG_LOG_BUF_SHIFT=17 CONFIG_LOG_CPU_MAX_BUF_SHIFT=12 CONFIG_PRINTK_SAFE_LOG_BUF_SHIFT=13 CONFIG_GENERIC_SCHED_CLOCK=y CONFIG_ARCH_SUPPORTS_INT128=y CONFIG_CGROUPS=y CONFIG_CGROUP_SCHED=y CONFIG_FAIR_GROUP_SCHED=y CONFIG_CFS_BANDWIDTH=y CONFIG_CGROUP_BPF=y CONFIG_SOCK_CGROUP_DATA=y CONFIG_NAMESPACES=y CONFIG_UTS_NS=y CONFIG_IPC_NS=y CONFIG_USER_NS=y CONFIG_PID_NS=y CONFIG_NET_NS=y CONFIG_CHECKPOINT_RESTORE=y CONFIG_BLK_DEV_INITRD=y CONFIG_INITRAMFS_SOURCE="" CONFIG_RD_GZIP=y CONFIG_RD_BZIP2=y CONFIG_RD_LZMA=y CONFIG_RD_XZ=y CONFIG_RD_LZO=y CONFIG_RD_LZ4=y CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE=y CONFIG_SYSCTL=y CONFIG_ANON_INODES=y CONFIG_SYSCTL_EXCEPTION_TRACE=y CONFIG_BPF=y CONFIG_EXPERT=y CONFIG_MULTIUSER=y CONFIG_SYSFS_SYSCALL=y CONFIG_FHANDLE=y CONFIG_POSIX_TIMERS=y CONFIG_PRINTK=y CONFIG_BUG=y CONFIG_ELF_CORE=y CONFIG_BASE_FULL=y CONFIG_FUTEX=y CONFIG_FUTEX_PI=y CONFIG_HAVE_FUTEX_CMPXCHG=y CONFIG_EPOLL=y CONFIG_SIGNALFD=y CONFIG_TIMERFD=y CONFIG_EVENTFD=y CONFIG_SHMEM=y CONFIG_AIO=y CONFIG_ADVISE_SYSCALLS=y CONFIG_MEMBARRIER=y CONFIG_KALLSYMS=y CONFIG_KALLSYMS_BASE_RELATIVE=y CONFIG_BPF_SYSCALL=y CONFIG_HAVE_PERF_EVENTS=y # # Kernel Performance Events And Counters # CONFIG_VM_EVENT_COUNTERS=y CONFIG_SLUB_DEBUG=y CONFIG_COMPAT_BRK=y CONFIG_SLUB=y CONFIG_SLAB_MERGE_DEFAULT=y CONFIG_SLUB_CPU_PARTIAL=y CONFIG_64BIT=y CONFIG_RISCV=y CONFIG_MMU=y CONFIG_ZONE_DMA32=y CONFIG_PAGE_OFFSET=0xffe0 CONFIG_STACKTRACE_SUPPORT=y CONFIG_TRACE_IRQFLAGS_SUPPORT=y CONFIG_RWSEM_GENERIC_SPINLOCK=y CONFIG_GENERIC_BUG=y CONFIG_GENERIC_BUG_RELATIVE_POINTERS=y CONFIG_GENERIC_CALIBRATE_DELAY=y CONFIG_GENERIC_CSUM=y CONFIG_GENERIC_HWEIGHT=y CONFIG_PGTABLE_LEVELS=3 # # Platform type # CONFIG_ARCH_RV64I=y CONFIG_CMODEL_MEDANY=y CONFIG_MODULE_SECTIONS=y CONFIG_MAXPHYSMEM_128GB=y CONFIG_SMP=y CONFIG_NR_CPUS=8 CONFIG_TUNE_GENERIC=y CONFIG_RISCV_ISA_C=y CONFIG_RISCV_ISA_A=y CONFIG_FPU=y # # Kernel features # CONFIG_HZ_250=y CONFIG_HZ=250 # # Boot options # CONFIG_CMDLINE="console=ttySIF0,115200 ignore_loglevel debug" CONFIG_CMDLINE_FALLBACK=y # # General architecture-dependent options # CONFIG_HAVE_64BIT_ALIGNED_ACCESS=y CONFIG_HAVE_ARCH_TRACEHOOK=y CONFIG_HAVE_DMA_CONTIGUOUS=y CONFIG_GENERIC_SMP_IDLE_THREAD=y CONFIG_HAVE_CLK=y CONFIG_CC_HAS_STACKPROTECTOR_NONE=y CONFIG_HAVE_VIRT_CPU_ACCOUNTING_GEN=y CONFIG_HAVE_MOD_ARCH_SPECIFIC=y CONFIG_MODULES_USE_ELF_RELA=y CONFIG_CLONE_BACKWARDS=y # # GCOV-based kernel profiling # CONFIG_PLUGIN_HOSTCC="g++" CONFIG_RT_MUTEXES=y CONFIG_BASE_SMALL=0 CONFIG_MODULES=y CONFIG_MODULE_UNLOAD=y CONFIG_BLOCK=y CONFIG_BLK_SCSI_REQUEST=y CONFIG_BLK_DEV_BSG=y # # Partition Types # CONFIG_MSDOS_PARTITION=y CONFIG_EFI_PARTITION=y CONFIG_BLK_MQ_PCI=y CONFIG_BLK_MQ_VIRTIO=y # # IO Schedulers # CONFIG_MQ_IOSCHED_DEADLINE=y CONFIG_MQ_IOSCHED_KYBER=y CONFIG_INLINE_SPIN_UNLOCK_IRQ=y CONFIG_INLINE_READ_UNLOCK=y CONFIG_INLINE_READ_UNLOCK_IRQ=y CONFIG_INLINE_WRITE_UNLOCK=y CONFIG_INLINE_WRITE_UNLOCK_IRQ=y # # Executable file formats # CONFIG_BINFMT_ELF=y CONFIG_ELFCORE=y CONFIG_CORE_DUMP_DEFAULT_ELF_HEADERS=y CONFIG_BINFMT_SCRIPT=y CONFIG_COREDUMP=y # # Memory Management options # C
Re: [PATCH v11 0/2] PWM support for HiFive Unleashed
On Mär 26 2019, Yash Shah wrote: > 'On Mon, Mar 25, 2019 at 9:24 PM Andreas Schwab wrote: >> >> >> I still don't see any improvement. FYI, this is the patch I use for the >> device tree: >> > > I am sharing you my test environment which has been working for me so > that you can duplicate at your end Please test your driver with the DT as above. Andreas. -- Andreas Schwab, SUSE Labs, sch...@suse.de GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 "And now for something completely different."
Re: [PATCH v11 0/2] PWM support for HiFive Unleashed
I have now found out that the ledtrig modules don't load automatically. I would have expected that the linux,default-trigger entries would cause the load of the corresponding ledtrig modules. But there is another problem, that the leds are on by default. Shouldn't they be off by default? Andreas. -- Andreas Schwab, SUSE Labs, sch...@suse.de GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 "And now for something completely different."
Re: [PATCH v11 0/2] PWM support for HiFive Unleashed
On Mon, Apr 15, 2019 at 11:36:51AM +0530, Yash Shah wrote: > Hi, > > Any comments on this patch series? > Any more changes are needed or it looks good to be merged upstream? In my eyes it would be great if Andreas Schwab was able to resolve the problems he pointed out in this thread. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | http://www.pengutronix.de/ |
Re: [PATCH v11 0/2] PWM support for HiFive Unleashed
Hi Andreas, On Wed, Mar 27, 2019 at 2:34 PM Andreas Schwab wrote: > > I have now found out that the ledtrig modules don't load automatically. > I would have expected that the linux,default-trigger entries would cause > the load of the corresponding ledtrig modules. > > But there is another problem, that the leds are on by default. > Shouldn't they be off by default? The PWM default output state is high (When duty cycle is 0), So I guess leds will remain on by default. Are you able to test the PWM driver at your end? or you still facing some issues? > > Andreas. > > -- > Andreas Schwab, SUSE Labs, sch...@suse.de > GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 > "And now for something completely different."
Re: [PATCH v11 0/2] PWM support for HiFive Unleashed
On Mai 02 2019, Yash Shah wrote: > The PWM default output state is high (When duty cycle is 0), So I > guess leds will remain on by default. So that's the bug that needs to be fixed. Andreas. -- Andreas Schwab, SUSE Labs, sch...@suse.de GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 "And now for something completely different."
Re: [PATCH v11 0/2] PWM support for HiFive Unleashed
Hi Andreas, On Tue, May 7, 2019 at 3:09 PM Andreas Schwab wrote: > > On Mai 02 2019, Yash Shah wrote: > > > The PWM default output state is high (When duty cycle is 0), So I > > guess leds will remain on by default. > > So that's the bug that needs to be fixed. Sorry I didn't probably get you before. I now understood the scenario. Leds on HiFive Unleashed are wired to supply instead of ground. And as per ./Documentation/devicetree/bindings/leds/leds-pwm.txt, you need to provide additional property "active-low" in such case. - active-low : (optional) For PWMs where the LED is wired to supply rather than ground. The leds will remain off by default when you add the "active-low" property under the pwm-leds subnode in your DT file. So, this isn't a bug in the driver code. For DT file change, you may refer https://github.com/yashshah7/riscv-linux/commit/dd55057a26150e50525643a423b20e07b72617b5 Can you test this at your end and confirm? - Yash > > Andreas. > > -- > Andreas Schwab, SUSE Labs, sch...@suse.de > GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 > "And now for something completely different." -- The information transmitted is intended only for the person or entity to which it is addressed and may contain confidential and/or privileged material. If you are not the intended recipient of this message please do not read, copy, use or disclose this communication and notify the sender immediately. It should be noted that any review, retransmission, dissemination or other use of, or taking action or reliance upon, this information by persons or entities other than the intended recipient is prohibited.
Re: [PATCH v11 0/2] PWM support for HiFive Unleashed
On Mai 07 2019, Yash Shah wrote: > Leds on HiFive Unleashed are wired to supply instead of ground. > And as per ./Documentation/devicetree/bindings/leds/leds-pwm.txt, you > need to provide additional property "active-low" in such case. > > - active-low : (optional) For PWMs where the LED is wired to supply > rather than ground. This fixes the issue. Thanks, Andreas. -- Andreas Schwab, SUSE Labs, sch...@suse.de GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 "And now for something completely different."
Re: [PATCH v11 0/2] PWM support for HiFive Unleashed
Hi, Any comments on this patch series? Any more changes are needed or it looks good to be merged upstream? - Yash
Re: [PATCH v11 0/2] PWM support for HiFive Unleashed
On Mär 25 2019, Yash Shah wrote: > v11 > - Change naming convention for pwm_device and pwm_sifive_ddata pointers > - Assign of_pwm_xlate_with_flag() to of_xlate func ptr since this driver > use three pwm-cells (Issue reported by Andreas Schwab > - Other minor fixes I still don't see any improvement. FYI, this is the patch I use for the device tree: diff --git a/fsbl/ux00_fsbl.dts b/fsbl/ux00_fsbl.dts index f27cc22..56e94c0 100644 --- a/fsbl/ux00_fsbl.dts +++ b/fsbl/ux00_fsbl.dts @@ -432,7 +432,7 @@ reg-names = "control"; clocks = <&tlclk>; sifive,approx-period = <100>; - #pwm-cells = <2>; + #pwm-cells = <3>; }; L46: pwm@10021000 { compatible = "sifive,pwm0"; @@ -442,46 +442,46 @@ reg-names = "control"; clocks = <&tlclk>; sifive,approx-period = <100>; - #pwm-cells = <2>; + #pwm-cells = <3>; }; pwmleds { compatible = "pwm-leds"; heartbeat { - pwms = <&L45 0 0>; + pwms = <&L45 0 100 0>; max-brightness = <255>; linux,default-trigger = "heartbeat"; }; mtd { - pwms = <&L45 1 0>; + pwms = <&L45 1 100 0>; max-brightness = <255>; linux,default-trigger = "mtd"; }; netdev { - pwms = <&L45 2 0>; + pwms = <&L45 2 100 0>; max-brightness = <255>; linux,default-trigger = "netdev"; }; panic { - pwms = <&L45 3 0>; + pwms = <&L45 3 100 0>; max-brightness = <255>; linux,default-trigger = "panic"; }; /* These LEDs are on the tester board */ /* testled { - pwms = <&L46 0 0>; + pwms = <&L46 0 100 0>; max-brightness = <255>; }; green { - pwms = <&L46 1 0>; + pwms = <&L46 1 100 0>; max-brightness = <255>; }; red { - pwms = <&L46 2 0>; + pwms = <&L46 2 100 0>; max-brightness = <255>; }; blue { - pwms = <&L46 3 0>; + pwms = <&L46 3 100 0>; max-brightness = <255>; }; */ -- 2.21.0 Andreas. -- Andreas Schwab, SUSE Labs, sch...@suse.de GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 "And now for something completely different."