[PATCH v11 0/2] PWM support for HiFive Unleashed
This patch series adds a PWM driver and DT documentation for HiFive Unleashed board. The patches are mostly based on Wesley's patch. This patchset is based on Linux 5.0-rc1 and tested on HiFive Unleashed board with additional board related patches needed for testing can be found at dev/yashs/pwm_5.0-rc1 branch of: https://github.com/yashshah7/riscv-linux.git 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 v10 - Use DIV_ROUND_CLOSEST_ULL instead of div_u64_round - Change 'num' defination to u64 bit (in pwm_sifive_apply). - Remove the usage of pwm_get_state() v9 - Use appropriate bitfield macros - Add approx_period in pwm_sifive_ddata struct and related changes - Correct the eqn for calculation of frac (in pwm_sifive_apply) - Other minor fixes v8 - Typo corrections - Remove active_user and related code - Do not clear PWM_SIFIVE_PWMCFG_EN_ALWAYS - Other minor fixes v7 - Modify description of compatible property in DT documentation - Use mutex locks at appropriate places - Fix all bad line breaks - Allow enabling/disabling PWM only when the user is the only active user - Remove Deglitch logic - Other minor fixes v6 - Remove the global property 'sifive,period-ns' - Implement free and request callbacks to maintain user counts. - Add user_count member to struct pwm_sifive_ddata - Allow period change only if user_count is one - Add pwm_sifive_enable function to enable/disable PWM - Change calculation logic of frac (in pwm_sifive_apply) - Remove state correction - Remove pwm_sifive_xlate function - Clock to be enabled only when PWM is enabled - Other minor fixes v5 - Correct the order of compatible string properties - PWM state correction to be done always - Other minor fixes based upon feedback on v4 v4 - Rename macros with appropriate names - Remove unused macros - Rename struct sifive_pwm_device to struct pwm_sifive_ddata - Rename function prefix as per driver name - Other minor fixes based upon feedback on v3 v3 - Add a link to the reference manaul - Use appropriate apis for division operation - Add check for polarity - Enable clk before calling clk_get_rate - Other minor fixes based upon feedback on v2 V2 changed from V1: - Remove inclusion of dt-bindings/pwm/pwm.h - Remove artificial alignments - Replace ioread32/iowrite32 with readl/writel - Remove camelcase - Change dev_info to dev_dbg for unnecessary log - Correct typo in driver name - Remove use of of_match_ptr macro - Update the DT compatible strings and Add reference to a common versioning document Yash Shah (2): pwm: sifive: Add DT documentation for SiFive PWM Controller pwm: sifive: Add a driver for SiFive SoC PWM .../devicetree/bindings/pwm/pwm-sifive.txt | 33 ++ drivers/pwm/Kconfig| 11 + drivers/pwm/Makefile | 1 + drivers/pwm/pwm-sifive.c | 338 + 4 files changed, 383 insertions(+) create mode 100644 Documentation/devicetree/bindings/pwm/pwm-sifive.txt create mode 100644 drivers/pwm/pwm-sifive.c -- 1.9.1
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."
