* [PATCH v2 0/7] FlexTimer Module Quadrature decoder counter @ 2019-03-06 11:12 Patrick Havelange 2019-03-06 11:12 ` [PATCH v2 1/7] include/fsl: add common FlexTimer #defines in a separate header Patrick Havelange ` (7 more replies) 0 siblings, 8 replies; 19+ messages in thread From: Patrick Havelange @ 2019-03-06 11:12 UTC (permalink / raw) To: William Breathitt Gray, Rob Herring, Mark Rutland, Shawn Guo, Li Yang, Daniel Lezcano, Thomas Gleixner, Thierry Reding, Esben Haabendal, linux-iio, linux-kernel, devicetree, linux-arm-kernel, linux-pwm, linuxppc-dev, Jonathan Cameron Cc: Patrick Havelange This patch serie is to be applied on top of https://patchwork.kernel.org/project/linux-iio/list/?series=147 (a more recent version of the serie is available here : https://gitlab.com/vilhelmgray/iio/tree/generic_counter_v10 ) Main changes in v2: The code is a bit simpler, thanks to more use of devm_* functions. The polling/32bit signed version has been dropped, as not needed and no other driver is doing that. Patrick Havelange (7): include/fsl: add common FlexTimer #defines in a separate header. drivers/pwm: pwm-fsl-ftm: use common header for FlexTimer #defines drivers/clocksource: timer-fsl-ftm: use common header for FlexTimer #defines dt-bindings: counter: ftm-quaddec counter: add FlexTimer Module Quadrature decoder counter driver counter: ftm-quaddec: Documentation: Add specific counter sysfs documentation LS1021A: dtsi: add ftm quad decoder entries .../ABI/testing/sysfs-bus-counter-ftm-quaddec | 16 + .../bindings/counter/ftm-quaddec.txt | 18 + arch/arm/boot/dts/ls1021a.dtsi | 28 ++ drivers/clocksource/timer-fsl-ftm.c | 15 +- drivers/counter/Kconfig | 9 + drivers/counter/Makefile | 1 + drivers/counter/ftm-quaddec.c | 356 ++++++++++++++++++ drivers/pwm/pwm-fsl-ftm.c | 44 +-- include/linux/fsl/ftm.h | 88 +++++ 9 files changed, 519 insertions(+), 56 deletions(-) create mode 100644 Documentation/ABI/testing/sysfs-bus-counter-ftm-quaddec create mode 100644 Documentation/devicetree/bindings/counter/ftm-quaddec.txt create mode 100644 drivers/counter/ftm-quaddec.c create mode 100644 include/linux/fsl/ftm.h -- 2.19.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 1/7] include/fsl: add common FlexTimer #defines in a separate header. 2019-03-06 11:12 [PATCH v2 0/7] FlexTimer Module Quadrature decoder counter Patrick Havelange @ 2019-03-06 11:12 ` Patrick Havelange 2019-03-06 11:12 ` [PATCH v2 2/7] drivers/pwm: pwm-fsl-ftm: use common header for FlexTimer #defines Patrick Havelange ` (6 subsequent siblings) 7 siblings, 0 replies; 19+ messages in thread From: Patrick Havelange @ 2019-03-06 11:12 UTC (permalink / raw) To: William Breathitt Gray, Rob Herring, Mark Rutland, Shawn Guo, Li Yang, Daniel Lezcano, Thomas Gleixner, Thierry Reding, Esben Haabendal, linux-iio, linux-kernel, devicetree, linux-arm-kernel, linux-pwm, linuxppc-dev, Jonathan Cameron Cc: Patrick Havelange Several files are/will be using the same #defines to use the Flextimer module. Regroup them in a common file. Signed-off-by: Patrick Havelange <patrick.havelange@essensium.com> Reviewed-by: Esben Haabendal <esben@haabendal.dk> --- Changes v2 - Commit message --- include/linux/fsl/ftm.h | 88 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 88 insertions(+) create mode 100644 include/linux/fsl/ftm.h diff --git a/include/linux/fsl/ftm.h b/include/linux/fsl/ftm.h new file mode 100644 index 000000000000..d59011acf66c --- /dev/null +++ b/include/linux/fsl/ftm.h @@ -0,0 +1,88 @@ +// SPDX-License-Identifier: GPL-2.0 +#ifndef __FSL_FTM_H__ +#define __FSL_FTM_H__ + +#define FTM_SC 0x0 /* Status And Control */ +#define FTM_CNT 0x4 /* Counter */ +#define FTM_MOD 0x8 /* Modulo */ + +#define FTM_CNTIN 0x4C /* Counter Initial Value */ +#define FTM_STATUS 0x50 /* Capture And Compare Status */ +#define FTM_MODE 0x54 /* Features Mode Selection */ +#define FTM_SYNC 0x58 /* Synchronization */ +#define FTM_OUTINIT 0x5C /* Initial State For Channels Output */ +#define FTM_OUTMASK 0x60 /* Output Mask */ +#define FTM_COMBINE 0x64 /* Function For Linked Channels */ +#define FTM_DEADTIME 0x68 /* Deadtime Insertion Control */ +#define FTM_EXTTRIG 0x6C /* FTM External Trigger */ +#define FTM_POL 0x70 /* Channels Polarity */ +#define FTM_FMS 0x74 /* Fault Mode Status */ +#define FTM_FILTER 0x78 /* Input Capture Filter Control */ +#define FTM_FLTCTRL 0x7C /* Fault Control */ +#define FTM_QDCTRL 0x80 /* Quadrature Decoder Control And Status */ +#define FTM_CONF 0x84 /* Configuration */ +#define FTM_FLTPOL 0x88 /* FTM Fault Input Polarity */ +#define FTM_SYNCONF 0x8C /* Synchronization Configuration */ +#define FTM_INVCTRL 0x90 /* FTM Inverting Control */ +#define FTM_SWOCTRL 0x94 /* FTM Software Output Control */ +#define FTM_PWMLOAD 0x98 /* FTM PWM Load */ + +#define FTM_SC_CLK_MASK_SHIFT 3 +#define FTM_SC_CLK_MASK (3 << FTM_SC_CLK_MASK_SHIFT) +#define FTM_SC_TOF 0x80 +#define FTM_SC_TOIE 0x40 +#define FTM_SC_CPWMS 0x20 +#define FTM_SC_CLKS 0x18 +#define FTM_SC_PS_1 0x0 +#define FTM_SC_PS_2 0x1 +#define FTM_SC_PS_4 0x2 +#define FTM_SC_PS_8 0x3 +#define FTM_SC_PS_16 0x4 +#define FTM_SC_PS_32 0x5 +#define FTM_SC_PS_64 0x6 +#define FTM_SC_PS_128 0x7 +#define FTM_SC_PS_MASK 0x7 + +#define FTM_MODE_FAULTIE 0x80 +#define FTM_MODE_FAULTM 0x60 +#define FTM_MODE_CAPTEST 0x10 +#define FTM_MODE_PWMSYNC 0x8 +#define FTM_MODE_WPDIS 0x4 +#define FTM_MODE_INIT 0x2 +#define FTM_MODE_FTMEN 0x1 + +/* NXP Errata: The PHAFLTREN and PHBFLTREN bits are tide to zero internally + * and these bits cannot be set. Flextimer cannot use Filter in + * Quadrature Decoder Mode. + * https://community.nxp.com/thread/467648#comment-1010319 + */ +#define FTM_QDCTRL_PHAFLTREN 0x80 +#define FTM_QDCTRL_PHBFLTREN 0x40 +#define FTM_QDCTRL_PHAPOL 0x20 +#define FTM_QDCTRL_PHBPOL 0x10 +#define FTM_QDCTRL_QUADMODE 0x8 +#define FTM_QDCTRL_QUADDIR 0x4 +#define FTM_QDCTRL_TOFDIR 0x2 +#define FTM_QDCTRL_QUADEN 0x1 + +#define FTM_FMS_FAULTF 0x80 +#define FTM_FMS_WPEN 0x40 +#define FTM_FMS_FAULTIN 0x10 +#define FTM_FMS_FAULTF3 0x8 +#define FTM_FMS_FAULTF2 0x4 +#define FTM_FMS_FAULTF1 0x2 +#define FTM_FMS_FAULTF0 0x1 + +#define FTM_CSC_BASE 0xC +#define FTM_CSC_MSB 0x20 +#define FTM_CSC_MSA 0x10 +#define FTM_CSC_ELSB 0x8 +#define FTM_CSC_ELSA 0x4 +#define FTM_CSC(_channel) (FTM_CSC_BASE + ((_channel) * 8)) + +#define FTM_CV_BASE 0x10 +#define FTM_CV(_channel) (FTM_CV_BASE + ((_channel) * 8)) + +#define FTM_PS_MAX 7 + +#endif -- 2.19.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v2 2/7] drivers/pwm: pwm-fsl-ftm: use common header for FlexTimer #defines 2019-03-06 11:12 [PATCH v2 0/7] FlexTimer Module Quadrature decoder counter Patrick Havelange 2019-03-06 11:12 ` [PATCH v2 1/7] include/fsl: add common FlexTimer #defines in a separate header Patrick Havelange @ 2019-03-06 11:12 ` Patrick Havelange 2019-03-06 11:12 ` [PATCH v2 3/7] drivers/clocksource: timer-fsl-ftm: " Patrick Havelange ` (5 subsequent siblings) 7 siblings, 0 replies; 19+ messages in thread From: Patrick Havelange @ 2019-03-06 11:12 UTC (permalink / raw) To: William Breathitt Gray, Rob Herring, Mark Rutland, Shawn Guo, Li Yang, Daniel Lezcano, Thomas Gleixner, Thierry Reding, Esben Haabendal, linux-iio, linux-kernel, devicetree, linux-arm-kernel, linux-pwm, linuxppc-dev, Jonathan Cameron Cc: Patrick Havelange This also fixes the wrong value for the previously defined FTM_MODE_INIT macro (it was not used). Signed-off-by: Patrick Havelange <patrick.havelange@essensium.com> Reviewed-by: Esben Haabendal <esben@haabendal.dk> --- Changes v2 - None --- drivers/pwm/pwm-fsl-ftm.c | 44 +-------------------------------------- 1 file changed, 1 insertion(+), 43 deletions(-) diff --git a/drivers/pwm/pwm-fsl-ftm.c b/drivers/pwm/pwm-fsl-ftm.c index 883378d055c6..f21ea1b97116 100644 --- a/drivers/pwm/pwm-fsl-ftm.c +++ b/drivers/pwm/pwm-fsl-ftm.c @@ -22,51 +22,9 @@ #include <linux/pwm.h> #include <linux/regmap.h> #include <linux/slab.h> +#include <linux/fsl/ftm.h> -#define FTM_SC 0x00 -#define FTM_SC_CLK_MASK_SHIFT 3 -#define FTM_SC_CLK_MASK (3 << FTM_SC_CLK_MASK_SHIFT) #define FTM_SC_CLK(c) (((c) + 1) << FTM_SC_CLK_MASK_SHIFT) -#define FTM_SC_PS_MASK 0x7 - -#define FTM_CNT 0x04 -#define FTM_MOD 0x08 - -#define FTM_CSC_BASE 0x0C -#define FTM_CSC_MSB BIT(5) -#define FTM_CSC_MSA BIT(4) -#define FTM_CSC_ELSB BIT(3) -#define FTM_CSC_ELSA BIT(2) -#define FTM_CSC(_channel) (FTM_CSC_BASE + ((_channel) * 8)) - -#define FTM_CV_BASE 0x10 -#define FTM_CV(_channel) (FTM_CV_BASE + ((_channel) * 8)) - -#define FTM_CNTIN 0x4C -#define FTM_STATUS 0x50 - -#define FTM_MODE 0x54 -#define FTM_MODE_FTMEN BIT(0) -#define FTM_MODE_INIT BIT(2) -#define FTM_MODE_PWMSYNC BIT(3) - -#define FTM_SYNC 0x58 -#define FTM_OUTINIT 0x5C -#define FTM_OUTMASK 0x60 -#define FTM_COMBINE 0x64 -#define FTM_DEADTIME 0x68 -#define FTM_EXTTRIG 0x6C -#define FTM_POL 0x70 -#define FTM_FMS 0x74 -#define FTM_FILTER 0x78 -#define FTM_FLTCTRL 0x7C -#define FTM_QDCTRL 0x80 -#define FTM_CONF 0x84 -#define FTM_FLTPOL 0x88 -#define FTM_SYNCONF 0x8C -#define FTM_INVCTRL 0x90 -#define FTM_SWOCTRL 0x94 -#define FTM_PWMLOAD 0x98 enum fsl_pwm_clk { FSL_PWM_CLK_SYS, -- 2.19.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v2 3/7] drivers/clocksource: timer-fsl-ftm: use common header for FlexTimer #defines 2019-03-06 11:12 [PATCH v2 0/7] FlexTimer Module Quadrature decoder counter Patrick Havelange 2019-03-06 11:12 ` [PATCH v2 1/7] include/fsl: add common FlexTimer #defines in a separate header Patrick Havelange 2019-03-06 11:12 ` [PATCH v2 2/7] drivers/pwm: pwm-fsl-ftm: use common header for FlexTimer #defines Patrick Havelange @ 2019-03-06 11:12 ` Patrick Havelange 2019-04-11 16:40 ` Daniel Lezcano 2019-03-06 11:12 ` [PATCH v2 4/7] dt-bindings: counter: ftm-quaddec Patrick Havelange ` (4 subsequent siblings) 7 siblings, 1 reply; 19+ messages in thread From: Patrick Havelange @ 2019-03-06 11:12 UTC (permalink / raw) To: William Breathitt Gray, Rob Herring, Mark Rutland, Shawn Guo, Li Yang, Daniel Lezcano, Thomas Gleixner, Thierry Reding, Esben Haabendal, linux-iio, linux-kernel, devicetree, linux-arm-kernel, linux-pwm, linuxppc-dev, Jonathan Cameron Cc: Patrick Havelange Common #defines have been moved to "linux/fsl/ftm.h". Thus making use of this file. Also FTM_SC_CLK_SHIFT has been renamed to FTM_SC_CLK_MASK_SHIFT. Signed-off-by: Patrick Havelange <patrick.havelange@essensium.com> Reviewed-by: Esben Haabendal <esben@haabendal.dk> --- Changes v2 - None --- drivers/clocksource/timer-fsl-ftm.c | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/drivers/clocksource/timer-fsl-ftm.c b/drivers/clocksource/timer-fsl-ftm.c index 846d18daf893..e1c34b2f53a5 100644 --- a/drivers/clocksource/timer-fsl-ftm.c +++ b/drivers/clocksource/timer-fsl-ftm.c @@ -19,20 +19,9 @@ #include <linux/of_irq.h> #include <linux/sched_clock.h> #include <linux/slab.h> +#include <linux/fsl/ftm.h> -#define FTM_SC 0x00 -#define FTM_SC_CLK_SHIFT 3 -#define FTM_SC_CLK_MASK (0x3 << FTM_SC_CLK_SHIFT) -#define FTM_SC_CLK(c) ((c) << FTM_SC_CLK_SHIFT) -#define FTM_SC_PS_MASK 0x7 -#define FTM_SC_TOIE BIT(6) -#define FTM_SC_TOF BIT(7) - -#define FTM_CNT 0x04 -#define FTM_MOD 0x08 -#define FTM_CNTIN 0x4C - -#define FTM_PS_MAX 7 +#define FTM_SC_CLK(c) ((c) << FTM_SC_CLK_MASK_SHIFT) struct ftm_clock_device { void __iomem *clksrc_base; -- 2.19.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v2 3/7] drivers/clocksource: timer-fsl-ftm: use common header for FlexTimer #defines 2019-03-06 11:12 ` [PATCH v2 3/7] drivers/clocksource: timer-fsl-ftm: " Patrick Havelange @ 2019-04-11 16:40 ` Daniel Lezcano 0 siblings, 0 replies; 19+ messages in thread From: Daniel Lezcano @ 2019-04-11 16:40 UTC (permalink / raw) To: Patrick Havelange, William Breathitt Gray, Rob Herring, Mark Rutland, Shawn Guo, Li Yang, Thomas Gleixner, Thierry Reding, Esben Haabendal, linux-iio, linux-kernel, devicetree, linux-arm-kernel, linux-pwm, linuxppc-dev, Jonathan Cameron On 06/03/2019 12:12, Patrick Havelange wrote: > Common #defines have been moved to "linux/fsl/ftm.h". Thus making use of > this file. > Also FTM_SC_CLK_SHIFT has been renamed to FTM_SC_CLK_MASK_SHIFT. > > Signed-off-by: Patrick Havelange <patrick.havelange@essensium.com> > Reviewed-by: Esben Haabendal <esben@haabendal.dk> Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org> > --- > Changes v2 > - None > --- > drivers/clocksource/timer-fsl-ftm.c | 15 ++------------- > 1 file changed, 2 insertions(+), 13 deletions(-) > > diff --git a/drivers/clocksource/timer-fsl-ftm.c b/drivers/clocksource/timer-fsl-ftm.c > index 846d18daf893..e1c34b2f53a5 100644 > --- a/drivers/clocksource/timer-fsl-ftm.c > +++ b/drivers/clocksource/timer-fsl-ftm.c > @@ -19,20 +19,9 @@ > #include <linux/of_irq.h> > #include <linux/sched_clock.h> > #include <linux/slab.h> > +#include <linux/fsl/ftm.h> > > -#define FTM_SC 0x00 > -#define FTM_SC_CLK_SHIFT 3 > -#define FTM_SC_CLK_MASK (0x3 << FTM_SC_CLK_SHIFT) > -#define FTM_SC_CLK(c) ((c) << FTM_SC_CLK_SHIFT) > -#define FTM_SC_PS_MASK 0x7 > -#define FTM_SC_TOIE BIT(6) > -#define FTM_SC_TOF BIT(7) > - > -#define FTM_CNT 0x04 > -#define FTM_MOD 0x08 > -#define FTM_CNTIN 0x4C > - > -#define FTM_PS_MAX 7 > +#define FTM_SC_CLK(c) ((c) << FTM_SC_CLK_MASK_SHIFT) > > struct ftm_clock_device { > void __iomem *clksrc_base; > -- <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 4/7] dt-bindings: counter: ftm-quaddec 2019-03-06 11:12 [PATCH v2 0/7] FlexTimer Module Quadrature decoder counter Patrick Havelange ` (2 preceding siblings ...) 2019-03-06 11:12 ` [PATCH v2 3/7] drivers/clocksource: timer-fsl-ftm: " Patrick Havelange @ 2019-03-06 11:12 ` Patrick Havelange 2019-03-12 19:09 ` Rob Herring 2019-03-06 11:12 ` [PATCH v2 5/7] counter: add FlexTimer Module Quadrature decoder counter driver Patrick Havelange ` (3 subsequent siblings) 7 siblings, 1 reply; 19+ messages in thread From: Patrick Havelange @ 2019-03-06 11:12 UTC (permalink / raw) To: William Breathitt Gray, Rob Herring, Mark Rutland, Shawn Guo, Li Yang, Daniel Lezcano, Thomas Gleixner, Thierry Reding, Esben Haabendal, linux-iio, linux-kernel, devicetree, linux-arm-kernel, linux-pwm, linuxppc-dev, Jonathan Cameron Cc: Patrick Havelange FlexTimer quadrature decoder driver. Signed-off-by: Patrick Havelange <patrick.havelange@essensium.com> Reviewed-by: Esben Haabendal <esben@haabendal.dk> --- Changes v2 - None --- .../bindings/counter/ftm-quaddec.txt | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 Documentation/devicetree/bindings/counter/ftm-quaddec.txt diff --git a/Documentation/devicetree/bindings/counter/ftm-quaddec.txt b/Documentation/devicetree/bindings/counter/ftm-quaddec.txt new file mode 100644 index 000000000000..4d18cd722074 --- /dev/null +++ b/Documentation/devicetree/bindings/counter/ftm-quaddec.txt @@ -0,0 +1,18 @@ +FlexTimer Quadrature decoder counter + +This driver exposes a simple counter for the quadrature decoder mode. + +Required properties: +- compatible: Must be "fsl,ftm-quaddec". +- reg: Must be set to the memory region of the flextimer. + +Optional property: +- big-endian: Access the device registers in big-endian mode. + +Example: + counter0: counter@29d0000 { + compatible = "fsl,ftm-quaddec"; + reg = <0x0 0x29d0000 0x0 0x10000>; + big-endian; + status = "disabled"; + }; -- 2.19.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v2 4/7] dt-bindings: counter: ftm-quaddec 2019-03-06 11:12 ` [PATCH v2 4/7] dt-bindings: counter: ftm-quaddec Patrick Havelange @ 2019-03-12 19:09 ` Rob Herring 2019-03-16 14:21 ` Jonathan Cameron 0 siblings, 1 reply; 19+ messages in thread From: Rob Herring @ 2019-03-12 19:09 UTC (permalink / raw) To: Patrick Havelange Cc: Mark Rutland, devicetree, linuxppc-dev, Jonathan Cameron, linux-pwm, linux-iio, linux-kernel, Daniel Lezcano, William Breathitt Gray, Li Yang, Thierry Reding, linux-arm-kernel, Thomas Gleixner, Shawn Guo, Esben Haabendal On Wed, Mar 06, 2019 at 12:12:05PM +0100, Patrick Havelange wrote: > FlexTimer quadrature decoder driver. > > Signed-off-by: Patrick Havelange <patrick.havelange@essensium.com> > Reviewed-by: Esben Haabendal <esben@haabendal.dk> > --- > Changes v2 > - None > --- > .../bindings/counter/ftm-quaddec.txt | 18 ++++++++++++++++++ > 1 file changed, 18 insertions(+) > create mode 100644 Documentation/devicetree/bindings/counter/ftm-quaddec.txt > > diff --git a/Documentation/devicetree/bindings/counter/ftm-quaddec.txt b/Documentation/devicetree/bindings/counter/ftm-quaddec.txt > new file mode 100644 > index 000000000000..4d18cd722074 > --- /dev/null > +++ b/Documentation/devicetree/bindings/counter/ftm-quaddec.txt > @@ -0,0 +1,18 @@ > +FlexTimer Quadrature decoder counter > + > +This driver exposes a simple counter for the quadrature decoder mode. Seems like this is more a mode of a h/w block than describing a h/w block. Bindings should do the latter. > + > +Required properties: > +- compatible: Must be "fsl,ftm-quaddec". > +- reg: Must be set to the memory region of the flextimer. > + > +Optional property: > +- big-endian: Access the device registers in big-endian mode. > + > +Example: > + counter0: counter@29d0000 { > + compatible = "fsl,ftm-quaddec"; > + reg = <0x0 0x29d0000 0x0 0x10000>; > + big-endian; > + status = "disabled"; > + }; > -- > 2.19.1 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 4/7] dt-bindings: counter: ftm-quaddec 2019-03-12 19:09 ` Rob Herring @ 2019-03-16 14:21 ` Jonathan Cameron 2019-03-26 15:43 ` Arnout Vandecappelle 0 siblings, 1 reply; 19+ messages in thread From: Jonathan Cameron @ 2019-03-16 14:21 UTC (permalink / raw) To: Rob Herring Cc: Mark Rutland, devicetree, linuxppc-dev, linux-kernel, linux-pwm, linux-iio, Patrick Havelange, Daniel Lezcano, William Breathitt Gray, Li Yang, Thierry Reding, linux-arm-kernel, Thomas Gleixner, Shawn Guo, Esben Haabendal On Tue, 12 Mar 2019 14:09:52 -0500 Rob Herring <robh@kernel.org> wrote: > On Wed, Mar 06, 2019 at 12:12:05PM +0100, Patrick Havelange wrote: > > FlexTimer quadrature decoder driver. > > > > Signed-off-by: Patrick Havelange <patrick.havelange@essensium.com> > > Reviewed-by: Esben Haabendal <esben@haabendal.dk> > > --- > > Changes v2 > > - None > > --- > > .../bindings/counter/ftm-quaddec.txt | 18 ++++++++++++++++++ > > 1 file changed, 18 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/counter/ftm-quaddec.txt > > > > diff --git a/Documentation/devicetree/bindings/counter/ftm-quaddec.txt b/Documentation/devicetree/bindings/counter/ftm-quaddec.txt > > new file mode 100644 > > index 000000000000..4d18cd722074 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/counter/ftm-quaddec.txt > > @@ -0,0 +1,18 @@ > > +FlexTimer Quadrature decoder counter > > + > > +This driver exposes a simple counter for the quadrature decoder mode. > > Seems like this is more a mode of a h/w block than describing a h/w > block. Bindings should do the latter. The snag is that we need to dig ourselves out of the hole set by: fsl,vf610-ftm-pwm etc. Documentation/devicetree/bindings/pwm/pwm-fsl-ftm.txt Documentation/devicetree/bindings/timer/fsl,ftm-timer.txt (I'm assuming these are the same IP block). Can probably be sorted out though. One core driver binds against the ftm and deals with instantiating the others depending on the configuration (note that this mode for instance does make sense in DT as it's really reflecting the fact there is a quadrature encoder connected to the ftm). Fiddly though :) J > > > + > > +Required properties: > > +- compatible: Must be "fsl,ftm-quaddec". > > +- reg: Must be set to the memory region of the flextimer. > > + > > +Optional property: > > +- big-endian: Access the device registers in big-endian mode. > > + > > +Example: > > + counter0: counter@29d0000 { > > + compatible = "fsl,ftm-quaddec"; > > + reg = <0x0 0x29d0000 0x0 0x10000>; > > + big-endian; > > + status = "disabled"; > > + }; > > -- > > 2.19.1 > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 4/7] dt-bindings: counter: ftm-quaddec 2019-03-16 14:21 ` Jonathan Cameron @ 2019-03-26 15:43 ` Arnout Vandecappelle 2019-03-26 16:06 ` Rob Herring 0 siblings, 1 reply; 19+ messages in thread From: Arnout Vandecappelle @ 2019-03-26 15:43 UTC (permalink / raw) To: Patrick Havelange, Jonathan Cameron, Rob Herring Cc: Mark Rutland, devicetree, linuxppc-dev, linux-pwm, linux-iio, linux-kernel, Daniel Lezcano, William Breathitt Gray, Li Yang, Thierry Reding, linux-arm-kernel, Thomas Gleixner, Shawn Guo, Esben Haabendal [Full disclosure: I'm a colleague of Patrick.] On 2019-03-16 14:21:38, Jonathan Cameron wrote: > On Tue, 12 Mar 2019 14:09:52 -0500 > Rob Herring <robh@kernel.org> wrote: > > > On Wed, Mar 06, 2019 at 12:12:05PM +0100, Patrick Havelange wrote: > > > FlexTimer quadrature decoder driver. > > > > > > Signed-off-by: Patrick Havelange <patrick.havelange@essensium.com> > > > Reviewed-by: Esben Haabendal <esben@haabendal.dk> > > > --- > > > Changes v2 > > > - None > > > --- > > > .../bindings/counter/ftm-quaddec.txt | 18 ++++++++++++++++++ > > > 1 file changed, 18 insertions(+) > > > create mode 100644 Documentation/devicetree/bindings/counter/ftm-quaddec.txt > > > > > > diff --git a/Documentation/devicetree/bindings/counter/ftm-quaddec.txt \ > > > b/Documentation/devicetree/bindings/counter/ftm-quaddec.txt new file mode 100644 > > > index 000000000000..4d18cd722074 > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/counter/ftm-quaddec.txt > > > @@ -0,0 +1,18 @@ > > > +FlexTimer Quadrature decoder counter > > > + > > > +This driver exposes a simple counter for the quadrature decoder mode. > > > > Seems like this is more a mode of a h/w block than describing a h/w > > block. Bindings should do the latter. As Jonathan writes below, it really is a "hardware mode", since it is tied very closely to how the device is wired up. Basically, the same block can be used for pretty diverse functions: a PWM where the pins are output, a counter where the pins are input, or a timer where the interrupt or timer value is used purely internally. This smells a bit like an MFD, but IMO it really isn't, because only one of the functions can be enabled. So indeed, it's more like a mode. > The snag is that we need to dig ourselves out of the hole set by: > fsl,vf610-ftm-pwm etc. > > Documentation/devicetree/bindings/pwm/pwm-fsl-ftm.txt > Documentation/devicetree/bindings/timer/fsl,ftm-timer.txt > (I'm assuming these are the same IP block). > > Can probably be sorted out though. One core driver binds against the > ftm and deals with instantiating the others depending on the configuration > (note that this mode for instance does make sense in DT as it's > really reflecting the fact there is a quadrature encoder > connected to the ftm). > > Fiddly though :) The way I see it, there are 3 ways this could be modelled (other than the current way: several nodes with the same address). 1. The SoC's .dtsi defines a single node, and the compatible string (which would be defined by the board .dts) both enables the node and sets the compatible string. The .dtsi would also define the static properties of all the different functions: some "static" properties, e.g. reg, but also some function specific-properties, e.g.#pwm-cells (only for PWM), interrupts (only for timer). The driver (selected through compatible) anyway only uses the properties that it needs, so it doesn't hurt to have those other-function properties there. 2. Like 1, but instead of defining the compatible string in the .dts, use a single compatible string for all the different drivers which is set in the .dtsi, and add a mode property (to be set in the .dts) to select the driver. The selection can be done either by having a top-level driver that calls out to the subsystem-specific one based on the mode, or by having each driver bail out of its probe function if the mode is not as expected. 3. Have a common node that essentially does nothing except occupy the memory resource, and sub-nodes for each function. This can again be combined with a common driver that does the common resource allocation, or each function driver can just look at its parent node to find the resources. A disadvantage of this one is that it is possible to enable several functions in the DT, while only one can actually work. Option 3 is what is used for e.g. stm32-lptimer. It also uses an mfd driver to model the common part. But possibly it actually allows the different functions to operate simultaneously. Option 3 has the additional disadvantage that it requires changes in existing DTs for ftm-pwm and ftm-timer, because some properties are moved one level down. Since we need to retain backward compatibility, we'd need to look for those properties both in the node itself and in the parent node. In particular, the common driver part would be fairly complicated to implement in a backward compatible way because it's not enough to do a simple devm_of_platform_populate(). Personally I don't like the common driver part too much. This common driver does almost nothing (iomap and clock) and it creates dependencies between different drivers. Combined with the backward compatibility problem, I don't see much point to it. I personally like option 2 the most. It's easy to be backward compatible (if mode is not set, revert to the current behaviour, i.e. assume that the compatible string has encoded the mode and that you're the only driver). It doesn't introduce subnodes that have no hardware equivalent. The only messy thing about it is that properties belonging to the different modes are mixed together in a single node. And also, I don't think this kind of model is currently used anywhere else in the kernel. Regards, Arnout _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 4/7] dt-bindings: counter: ftm-quaddec 2019-03-26 15:43 ` Arnout Vandecappelle @ 2019-03-26 16:06 ` Rob Herring 0 siblings, 0 replies; 19+ messages in thread From: Rob Herring @ 2019-03-26 16:06 UTC (permalink / raw) To: Arnout Vandecappelle Cc: Mark Rutland, devicetree, linuxppc-dev, linux-kernel, Linux PWM List, open list:IIO SUBSYSTEM AND DRIVERS, Patrick Havelange, Daniel Lezcano, William Breathitt Gray, Li Yang, Thierry Reding, Esben Haabendal, Thomas Gleixner, Shawn Guo, Jonathan Cameron, moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE On Tue, Mar 26, 2019 at 10:43 AM Arnout Vandecappelle <arnout@mind.be> wrote: > > [Full disclosure: I'm a colleague of Patrick.] > > On 2019-03-16 14:21:38, Jonathan Cameron wrote: > > > On Tue, 12 Mar 2019 14:09:52 -0500 > > Rob Herring <robh@kernel.org> wrote: > > > > > On Wed, Mar 06, 2019 at 12:12:05PM +0100, Patrick Havelange wrote: > > > > FlexTimer quadrature decoder driver. > > > > > > > > Signed-off-by: Patrick Havelange <patrick.havelange@essensium.com> > > > > Reviewed-by: Esben Haabendal <esben@haabendal.dk> > > > > --- > > > > Changes v2 > > > > - None > > > > --- > > > > .../bindings/counter/ftm-quaddec.txt | 18 ++++++++++++++++++ > > > > 1 file changed, 18 insertions(+) > > > > create mode 100644 Documentation/devicetree/bindings/counter/ftm-quaddec.txt > > > > > > > > diff --git a/Documentation/devicetree/bindings/counter/ftm-quaddec.txt \ > > > > b/Documentation/devicetree/bindings/counter/ftm-quaddec.txt new file mode 100644 > > > > index 000000000000..4d18cd722074 > > > > --- /dev/null > > > > +++ b/Documentation/devicetree/bindings/counter/ftm-quaddec.txt > > > > @@ -0,0 +1,18 @@ > > > > +FlexTimer Quadrature decoder counter > > > > + > > > > +This driver exposes a simple counter for the quadrature decoder mode. > > > > > > Seems like this is more a mode of a h/w block than describing a h/w > > > block. Bindings should do the latter. > > > As Jonathan writes below, it really is a "hardware mode", since it is tied > very closely to how the device is wired up. Okay, as it is describing what is attached, I agree. The thing to consider is whether you will need to describe more than just the mode. We often start adding properties of an attached device in the controller node only to realize later that we should have a node for the device itself. Even for something as simple as an LED we've ended up there. > Basically, the same block can be used for pretty diverse functions: a PWM where > the pins are output, a counter where the pins are input, or a timer where the > interrupt or timer value is used purely internally. > > This smells a bit like an MFD, but IMO it really isn't, because only one of the > functions can be enabled. So indeed, it's more like a mode. Agreed. > > The snag is that we need to dig ourselves out of the hole set by: > > fsl,vf610-ftm-pwm etc. > > > > Documentation/devicetree/bindings/pwm/pwm-fsl-ftm.txt > > Documentation/devicetree/bindings/timer/fsl,ftm-timer.txt > > (I'm assuming these are the same IP block). > > > > Can probably be sorted out though. One core driver binds against the > > ftm and deals with instantiating the others depending on the configuration > > (note that this mode for instance does make sense in DT as it's > > really reflecting the fact there is a quadrature encoder > > connected to the ftm). > > > > Fiddly though :) > > > The way I see it, there are 3 ways this could be modelled (other than the > current way: several nodes with the same address). > > > 1. The SoC's .dtsi defines a single node, and the compatible string (which would > be defined by the board .dts) both enables the node and sets the compatible > string. The .dtsi would also define the static properties of all the different > functions: some "static" properties, e.g. reg, but also some function > specific-properties, e.g.#pwm-cells (only for PWM), interrupts (only for timer). > The driver (selected through compatible) anyway only uses the properties that it > needs, so it doesn't hurt to have those other-function properties there. > > > 2. Like 1, but instead of defining the compatible string in the .dts, use a > single compatible string for all the different drivers which is set in the > .dtsi, and add a mode property (to be set in the .dts) to select the driver. The > selection can be done either by having a top-level driver that calls out to the > subsystem-specific one based on the mode, or by having each driver bail out of > its probe function if the mode is not as expected. > > > 3. Have a common node that essentially does nothing except occupy the memory > resource, and sub-nodes for each function. This can again be combined with a > common driver that does the common resource allocation, or each function driver > can just look at its parent node to find the resources. A disadvantage of this > one is that it is possible to enable several functions in the DT, while only one > can actually work. > > > Option 3 is what is used for e.g. stm32-lptimer. It also uses an mfd driver to > model the common part. But possibly it actually allows the different functions > to operate simultaneously. I'm not all that thrilled with how that one ended up. > Option 3 has the additional disadvantage that it requires changes in existing > DTs for ftm-pwm and ftm-timer, because some properties are moved one level down. > Since we need to retain backward compatibility, we'd need to look for those > properties both in the node itself and in the parent node. In particular, the > common driver part would be fairly complicated to implement in a backward > compatible way because it's not enough to do a simple devm_of_platform_populate(). > > > Personally I don't like the common driver part too much. This common driver > does almost nothing (iomap and clock) and it creates dependencies between > different drivers. Combined with the backward compatibility problem, I don't see > much point to it. > > > I personally like option 2 the most. It's easy to be backward compatible (if > mode is not set, revert to the current behaviour, i.e. assume that the > compatible string has encoded the mode and that you're the only driver). It > doesn't introduce subnodes that have no hardware equivalent. The only messy > thing about it is that properties belonging to the different modes are mixed > together in a single node. And also, I don't think this kind of model is > currently used anywhere else in the kernel. Option 2 seems best to me. Rob _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 5/7] counter: add FlexTimer Module Quadrature decoder counter driver 2019-03-06 11:12 [PATCH v2 0/7] FlexTimer Module Quadrature decoder counter Patrick Havelange ` (3 preceding siblings ...) 2019-03-06 11:12 ` [PATCH v2 4/7] dt-bindings: counter: ftm-quaddec Patrick Havelange @ 2019-03-06 11:12 ` Patrick Havelange 2019-03-07 11:32 ` William Breathitt Gray 2019-03-11 12:24 ` Jonathan Cameron 2019-03-06 11:12 ` [PATCH v2 6/7] counter: ftm-quaddec: Documentation: Add specific counter sysfs documentation Patrick Havelange ` (2 subsequent siblings) 7 siblings, 2 replies; 19+ messages in thread From: Patrick Havelange @ 2019-03-06 11:12 UTC (permalink / raw) To: William Breathitt Gray, Rob Herring, Mark Rutland, Shawn Guo, Li Yang, Daniel Lezcano, Thomas Gleixner, Thierry Reding, Esben Haabendal, linux-iio, linux-kernel, devicetree, linux-arm-kernel, linux-pwm, linuxppc-dev, Jonathan Cameron Cc: Patrick Havelange This driver exposes the counter for the quadrature decoder of the FlexTimer Module, present in the LS1021A soc. Signed-off-by: Patrick Havelange <patrick.havelange@essensium.com> --- Changes v2 - Rebased on new counter subsystem - Cleaned up included headers - Use devm_ioremap() - Correct order of devm_ and unmanaged resources --- drivers/counter/Kconfig | 9 + drivers/counter/Makefile | 1 + drivers/counter/ftm-quaddec.c | 356 ++++++++++++++++++++++++++++++++++ 3 files changed, 366 insertions(+) create mode 100644 drivers/counter/ftm-quaddec.c diff --git a/drivers/counter/Kconfig b/drivers/counter/Kconfig index 87c491a19c63..233ac305d878 100644 --- a/drivers/counter/Kconfig +++ b/drivers/counter/Kconfig @@ -48,4 +48,13 @@ config STM32_LPTIMER_CNT To compile this driver as a module, choose M here: the module will be called stm32-lptimer-cnt. +config FTM_QUADDEC + tristate "Flex Timer Module Quadrature decoder driver" + help + Select this option to enable the Flex Timer Quadrature decoder + driver. + + To compile this driver as a module, choose M here: the + module will be called ftm-quaddec. + endif # COUNTER diff --git a/drivers/counter/Makefile b/drivers/counter/Makefile index 5589976d37f8..0c9e622a6bea 100644 --- a/drivers/counter/Makefile +++ b/drivers/counter/Makefile @@ -7,3 +7,4 @@ obj-$(CONFIG_COUNTER) += counter.o obj-$(CONFIG_104_QUAD_8) += 104-quad-8.o obj-$(CONFIG_STM32_TIMER_CNT) += stm32-timer-cnt.o obj-$(CONFIG_STM32_LPTIMER_CNT) += stm32-lptimer-cnt.o +obj-$(CONFIG_FTM_QUADDEC) += ftm-quaddec.o diff --git a/drivers/counter/ftm-quaddec.c b/drivers/counter/ftm-quaddec.c new file mode 100644 index 000000000000..1bc9e075a386 --- /dev/null +++ b/drivers/counter/ftm-quaddec.c @@ -0,0 +1,356 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Flex Timer Module Quadrature decoder + * + * This module implements a driver for decoding the FTM quadrature + * of ex. a LS1021A + */ + +#include <linux/fsl/ftm.h> +#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/of.h> +#include <linux/io.h> +#include <linux/mutex.h> +#include <linux/counter.h> + +struct ftm_quaddec { + struct counter_device counter; + struct platform_device *pdev; + void __iomem *ftm_base; + bool big_endian; + struct mutex ftm_quaddec_mutex; +}; + +static void ftm_read(struct ftm_quaddec *ftm, uint32_t offset, uint32_t *data) +{ + if (ftm->big_endian) + *data = ioread32be(ftm->ftm_base + offset); + else + *data = ioread32(ftm->ftm_base + offset); +} + +static void ftm_write(struct ftm_quaddec *ftm, uint32_t offset, uint32_t data) +{ + if (ftm->big_endian) + iowrite32be(data, ftm->ftm_base + offset); + else + iowrite32(data, ftm->ftm_base + offset); +} + +/* + * take mutex + * call ftm_clear_write_protection + * update settings + * call ftm_set_write_protection + * release mutex + */ +static void ftm_clear_write_protection(struct ftm_quaddec *ftm) +{ + uint32_t flag; + + /* First see if it is enabled */ + ftm_read(ftm, FTM_FMS, &flag); + + if (flag & FTM_FMS_WPEN) { + ftm_read(ftm, FTM_MODE, &flag); + ftm_write(ftm, FTM_MODE, flag | FTM_MODE_WPDIS); + } +} + +static void ftm_set_write_protection(struct ftm_quaddec *ftm) +{ + ftm_write(ftm, FTM_FMS, FTM_FMS_WPEN); +} + +static void ftm_reset_counter(struct ftm_quaddec *ftm) +{ + /* Reset hardware counter to CNTIN */ + ftm_write(ftm, FTM_CNT, 0x0); +} + +static void ftm_quaddec_init(struct ftm_quaddec *ftm) +{ + ftm_clear_write_protection(ftm); + + /* + * Do not write in the region from the CNTIN register through the + * PWMLOAD register when FTMEN = 0. + */ + ftm_write(ftm, FTM_MODE, FTM_MODE_FTMEN); + ftm_write(ftm, FTM_CNTIN, 0x0000); + ftm_write(ftm, FTM_MOD, 0xffff); + ftm_write(ftm, FTM_CNT, 0x0); + ftm_write(ftm, FTM_SC, FTM_SC_PS_1); + + /* Select quad mode */ + ftm_write(ftm, FTM_QDCTRL, FTM_QDCTRL_QUADEN); + + /* Unused features and reset to default section */ + ftm_write(ftm, FTM_POL, 0x0); + ftm_write(ftm, FTM_FLTCTRL, 0x0); + ftm_write(ftm, FTM_SYNCONF, 0x0); + ftm_write(ftm, FTM_SYNC, 0xffff); + + /* Lock the FTM */ + ftm_set_write_protection(ftm); +} + +static void ftm_quaddec_disable(struct ftm_quaddec *ftm) +{ + ftm_write(ftm, FTM_MODE, 0); +} + +static int ftm_quaddec_get_prescaler(struct counter_device *counter, + struct counter_count *count, + size_t *cnt_mode) +{ + struct ftm_quaddec *ftm = counter->priv; + uint32_t scflags; + + ftm_read(ftm, FTM_SC, &scflags); + + *cnt_mode = scflags & FTM_SC_PS_MASK; + + return 0; +} + +static int ftm_quaddec_set_prescaler(struct counter_device *counter, + struct counter_count *count, + size_t cnt_mode) +{ + struct ftm_quaddec *ftm = counter->priv; + + uint32_t scflags; + + mutex_lock(&ftm->ftm_quaddec_mutex); + + ftm_read(ftm, FTM_SC, &scflags); + + scflags &= ~FTM_SC_PS_MASK; + cnt_mode &= FTM_SC_PS_MASK; /*just to be 100% sure*/ + + scflags |= cnt_mode; + + /* Write */ + ftm_clear_write_protection(ftm); + ftm_write(ftm, FTM_SC, scflags); + ftm_set_write_protection(ftm); + + /* Also resets the counter as it is undefined anyway now */ + ftm_reset_counter(ftm); + + mutex_unlock(&ftm->ftm_quaddec_mutex); + return 0; +} + +static const char * const ftm_quaddec_prescaler[] = { + "1", "2", "4", "8", "16", "32", "64", "128" +}; + +static struct counter_count_enum_ext ftm_quaddec_prescaler_enum = { + .items = ftm_quaddec_prescaler, + .num_items = ARRAY_SIZE(ftm_quaddec_prescaler), + .get = ftm_quaddec_get_prescaler, + .set = ftm_quaddec_set_prescaler +}; + +enum ftm_quaddec_synapse_action { + FTM_QUADDEC_SYNAPSE_ACTION_BOTH_EDGES, +}; + +static enum counter_synapse_action ftm_quaddec_synapse_actions[] = { + [FTM_QUADDEC_SYNAPSE_ACTION_BOTH_EDGES] = + COUNTER_SYNAPSE_ACTION_BOTH_EDGES +}; + +enum ftm_quaddec_count_function { + FTM_QUADDEC_COUNT_ENCODER_MODE_1, +}; + +static const enum counter_count_function ftm_quaddec_count_functions[] = { + [FTM_QUADDEC_COUNT_ENCODER_MODE_1] = + COUNTER_COUNT_FUNCTION_QUADRATURE_X4 +}; + +static int ftm_quaddec_count_read(struct counter_device *counter, + struct counter_count *count, + struct counter_count_read_value *val) +{ + struct ftm_quaddec *const ftm = counter->priv; + uint32_t cntval; + + ftm_read(ftm, FTM_CNT, &cntval); + + counter_count_read_value_set(val, COUNTER_COUNT_POSITION, &cntval); + + return 0; +} + +static int ftm_quaddec_count_write(struct counter_device *counter, + struct counter_count *count, + struct counter_count_write_value *val) +{ + struct ftm_quaddec *const ftm = counter->priv; + u32 cnt; + int err; + + err = counter_count_write_value_get(&cnt, COUNTER_COUNT_POSITION, val); + if (err) + return err; + + if (cnt != 0) { + dev_warn(&ftm->pdev->dev, "Can only accept '0' as new counter value\n"); + return -EINVAL; + } + + ftm_reset_counter(ftm); + + return 0; +} + +static int ftm_quaddec_count_function_get(struct counter_device *counter, + struct counter_count *count, + size_t *function) +{ + *function = FTM_QUADDEC_COUNT_ENCODER_MODE_1; + + return 0; +} + +static int ftm_quaddec_action_get(struct counter_device *counter, + struct counter_count *count, + struct counter_synapse *synapse, + size_t *action) +{ + *action = FTM_QUADDEC_SYNAPSE_ACTION_BOTH_EDGES; + + return 0; +} + +static const struct counter_ops ftm_quaddec_cnt_ops = { + .count_read = ftm_quaddec_count_read, + .count_write = ftm_quaddec_count_write, + .function_get = ftm_quaddec_count_function_get, + .action_get = ftm_quaddec_action_get, +}; + +static struct counter_signal ftm_quaddec_signals[] = { + { + .id = 0, + .name = "Channel 1 Quadrature A" + }, + { + .id = 1, + .name = "Channel 1 Quadrature B" + } +}; + +static struct counter_synapse ftm_quaddec_count_synapses[] = { + { + .actions_list = ftm_quaddec_synapse_actions, + .num_actions = ARRAY_SIZE(ftm_quaddec_synapse_actions), + .signal = &ftm_quaddec_signals[0] + }, + { + .actions_list = ftm_quaddec_synapse_actions, + .num_actions = ARRAY_SIZE(ftm_quaddec_synapse_actions), + .signal = &ftm_quaddec_signals[1] + } +}; + +static const struct counter_count_ext ftm_quaddec_count_ext[] = { + COUNTER_COUNT_ENUM("prescaler", &ftm_quaddec_prescaler_enum), + COUNTER_COUNT_ENUM_AVAILABLE("prescaler", &ftm_quaddec_prescaler_enum), +}; + +static struct counter_count ftm_quaddec_counts = { + .id = 0, + .name = "Channel 1 Count", + .functions_list = ftm_quaddec_count_functions, + .num_functions = ARRAY_SIZE(ftm_quaddec_count_functions), + .synapses = ftm_quaddec_count_synapses, + .num_synapses = ARRAY_SIZE(ftm_quaddec_count_synapses), + .ext = ftm_quaddec_count_ext, + .num_ext = ARRAY_SIZE(ftm_quaddec_count_ext) +}; + +static int ftm_quaddec_probe(struct platform_device *pdev) +{ + struct ftm_quaddec *ftm; + + struct device_node *node = pdev->dev.of_node; + struct resource *io; + int ret; + + ftm = devm_kzalloc(&pdev->dev, sizeof(*ftm), GFP_KERNEL); + if (!ftm) + return -ENOMEM; + + platform_set_drvdata(pdev, ftm); + + io = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (!io) { + dev_err(&pdev->dev, "Failed to get memory region\n"); + return -ENODEV; + } + + ftm->pdev = pdev; + ftm->big_endian = of_property_read_bool(node, "big-endian"); + ftm->ftm_base = devm_ioremap(&pdev->dev, io->start, resource_size(io)); + + if (!ftm->ftm_base) { + dev_err(&pdev->dev, "Failed to map memory region\n"); + return -EINVAL; + } + ftm->counter.name = dev_name(&pdev->dev); + ftm->counter.parent = &pdev->dev; + ftm->counter.ops = &ftm_quaddec_cnt_ops; + ftm->counter.counts = &ftm_quaddec_counts; + ftm->counter.num_counts = 1; + ftm->counter.signals = ftm_quaddec_signals; + ftm->counter.num_signals = ARRAY_SIZE(ftm_quaddec_signals); + ftm->counter.priv = ftm; + + mutex_init(&ftm->ftm_quaddec_mutex); + + ftm_quaddec_init(ftm); + + ret = counter_register(&ftm->counter); + if (ret) + ftm_quaddec_disable(ftm); + + return ret; +} + +static int ftm_quaddec_remove(struct platform_device *pdev) +{ + struct ftm_quaddec *ftm = platform_get_drvdata(pdev); + + counter_unregister(&ftm->counter); + + ftm_quaddec_disable(ftm); + + return 0; +} + +static const struct of_device_id ftm_quaddec_match[] = { + { .compatible = "fsl,ftm-quaddec" }, + {}, +}; + +static struct platform_driver ftm_quaddec_driver = { + .driver = { + .name = "ftm-quaddec", + .owner = THIS_MODULE, + .of_match_table = ftm_quaddec_match, + }, + .probe = ftm_quaddec_probe, + .remove = ftm_quaddec_remove, +}; + +module_platform_driver(ftm_quaddec_driver); + +MODULE_LICENSE("GPL"); +MODULE_AUTHOR("Kjeld Flarup <kfa@deif.com"); +MODULE_AUTHOR("Patrick Havelange <patrick.havelange@essensium.com"); -- 2.19.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v2 5/7] counter: add FlexTimer Module Quadrature decoder counter driver 2019-03-06 11:12 ` [PATCH v2 5/7] counter: add FlexTimer Module Quadrature decoder counter driver Patrick Havelange @ 2019-03-07 11:32 ` William Breathitt Gray 2019-03-11 11:22 ` Patrick Havelange 2019-03-11 12:24 ` Jonathan Cameron 1 sibling, 1 reply; 19+ messages in thread From: William Breathitt Gray @ 2019-03-07 11:32 UTC (permalink / raw) To: Patrick Havelange Cc: Mark Rutland, devicetree, Jonathan Cameron, linux-pwm, linux-iio, linuxppc-dev, Daniel Lezcano, linux-kernel, Li Yang, Rob Herring, Thierry Reding, linux-arm-kernel, Thomas Gleixner, Shawn Guo, Esben Haabendal On Wed, Mar 06, 2019 at 12:12:06PM +0100, Patrick Havelange wrote: > This driver exposes the counter for the quadrature decoder of the > FlexTimer Module, present in the LS1021A soc. > > Signed-off-by: Patrick Havelange <patrick.havelange@essensium.com> > --- > Changes v2 > - Rebased on new counter subsystem > - Cleaned up included headers > - Use devm_ioremap() > - Correct order of devm_ and unmanaged resources > --- > drivers/counter/Kconfig | 9 + > drivers/counter/Makefile | 1 + > drivers/counter/ftm-quaddec.c | 356 ++++++++++++++++++++++++++++++++++ > 3 files changed, 366 insertions(+) > create mode 100644 drivers/counter/ftm-quaddec.c Overall, I think you utilized the Generic Counter interface correctly. So good job on the conversion from your initial patch. :-) Some minor inline suggestions/comments follow. > > diff --git a/drivers/counter/Kconfig b/drivers/counter/Kconfig > index 87c491a19c63..233ac305d878 100644 > --- a/drivers/counter/Kconfig > +++ b/drivers/counter/Kconfig > @@ -48,4 +48,13 @@ config STM32_LPTIMER_CNT > To compile this driver as a module, choose M here: the > module will be called stm32-lptimer-cnt. > > +config FTM_QUADDEC > + tristate "Flex Timer Module Quadrature decoder driver" > + help > + Select this option to enable the Flex Timer Quadrature decoder > + driver. > + > + To compile this driver as a module, choose M here: the > + module will be called ftm-quaddec. > + > endif # COUNTER > diff --git a/drivers/counter/Makefile b/drivers/counter/Makefile > index 5589976d37f8..0c9e622a6bea 100644 > --- a/drivers/counter/Makefile > +++ b/drivers/counter/Makefile > @@ -7,3 +7,4 @@ obj-$(CONFIG_COUNTER) += counter.o > obj-$(CONFIG_104_QUAD_8) += 104-quad-8.o > obj-$(CONFIG_STM32_TIMER_CNT) += stm32-timer-cnt.o > obj-$(CONFIG_STM32_LPTIMER_CNT) += stm32-lptimer-cnt.o > +obj-$(CONFIG_FTM_QUADDEC) += ftm-quaddec.o > diff --git a/drivers/counter/ftm-quaddec.c b/drivers/counter/ftm-quaddec.c > new file mode 100644 > index 000000000000..1bc9e075a386 > --- /dev/null > +++ b/drivers/counter/ftm-quaddec.c > @@ -0,0 +1,356 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Flex Timer Module Quadrature decoder > + * > + * This module implements a driver for decoding the FTM quadrature > + * of ex. a LS1021A > + */ > + > +#include <linux/fsl/ftm.h> > +#include <linux/module.h> > +#include <linux/platform_device.h> > +#include <linux/of.h> > +#include <linux/io.h> > +#include <linux/mutex.h> > +#include <linux/counter.h> > + > +struct ftm_quaddec { > + struct counter_device counter; > + struct platform_device *pdev; > + void __iomem *ftm_base; > + bool big_endian; > + struct mutex ftm_quaddec_mutex; > +}; > + > +static void ftm_read(struct ftm_quaddec *ftm, uint32_t offset, uint32_t *data) > +{ > + if (ftm->big_endian) > + *data = ioread32be(ftm->ftm_base + offset); > + else > + *data = ioread32(ftm->ftm_base + offset); > +} > + > +static void ftm_write(struct ftm_quaddec *ftm, uint32_t offset, uint32_t data) > +{ > + if (ftm->big_endian) > + iowrite32be(data, ftm->ftm_base + offset); > + else > + iowrite32(data, ftm->ftm_base + offset); > +} > + > +/* > + * take mutex > + * call ftm_clear_write_protection > + * update settings > + * call ftm_set_write_protection > + * release mutex > + */ Jonathan mentioned it before in the previous review, and I think I agree too, that this comment block is superfluous: the context of this code is simple enough that the function call order is naturally obvious (i.e. write protection must be cleared before settings are modified). The only important thing to mention here is that the mutex must be held before the write protection state is modified so a comment along the following lines should suffice: /* hold mutex before modifying write protection state */ > +static void ftm_clear_write_protection(struct ftm_quaddec *ftm) > +{ > + uint32_t flag; > + > + /* First see if it is enabled */ > + ftm_read(ftm, FTM_FMS, &flag); > + > + if (flag & FTM_FMS_WPEN) { > + ftm_read(ftm, FTM_MODE, &flag); > + ftm_write(ftm, FTM_MODE, flag | FTM_MODE_WPDIS); > + } > +} > + > +static void ftm_set_write_protection(struct ftm_quaddec *ftm) > +{ > + ftm_write(ftm, FTM_FMS, FTM_FMS_WPEN); > +} > + > +static void ftm_reset_counter(struct ftm_quaddec *ftm) > +{ > + /* Reset hardware counter to CNTIN */ > + ftm_write(ftm, FTM_CNT, 0x0); > +} > + > +static void ftm_quaddec_init(struct ftm_quaddec *ftm) > +{ > + ftm_clear_write_protection(ftm); > + > + /* > + * Do not write in the region from the CNTIN register through the > + * PWMLOAD register when FTMEN = 0. > + */ > + ftm_write(ftm, FTM_MODE, FTM_MODE_FTMEN); > + ftm_write(ftm, FTM_CNTIN, 0x0000); > + ftm_write(ftm, FTM_MOD, 0xffff); > + ftm_write(ftm, FTM_CNT, 0x0); > + ftm_write(ftm, FTM_SC, FTM_SC_PS_1); > + > + /* Select quad mode */ > + ftm_write(ftm, FTM_QDCTRL, FTM_QDCTRL_QUADEN); > + > + /* Unused features and reset to default section */ > + ftm_write(ftm, FTM_POL, 0x0); > + ftm_write(ftm, FTM_FLTCTRL, 0x0); > + ftm_write(ftm, FTM_SYNCONF, 0x0); > + ftm_write(ftm, FTM_SYNC, 0xffff); > + > + /* Lock the FTM */ > + ftm_set_write_protection(ftm); > +} > + > +static void ftm_quaddec_disable(struct ftm_quaddec *ftm) > +{ > + ftm_write(ftm, FTM_MODE, 0); > +} The ftm_quaddec_disable function is only used for cleanup when the driver is being removed. Is disabling the FTM counter on removal actually something we need to do? While it's true that the register will keep updating, since the driver is no longer loaded, we don't care about that register value. Once we take control of the hardware again (by reloading our driver or via a new one), we reinitialize the counter and set the count value back to 0 anyway -- so whatever value the register had no longer matters. > + > +static int ftm_quaddec_get_prescaler(struct counter_device *counter, > + struct counter_count *count, > + size_t *cnt_mode) > +{ > + struct ftm_quaddec *ftm = counter->priv; > + uint32_t scflags; > + > + ftm_read(ftm, FTM_SC, &scflags); > + > + *cnt_mode = scflags & FTM_SC_PS_MASK; > + > + return 0; > +} > + > +static int ftm_quaddec_set_prescaler(struct counter_device *counter, > + struct counter_count *count, > + size_t cnt_mode) > +{ > + struct ftm_quaddec *ftm = counter->priv; > + > + uint32_t scflags; > + > + mutex_lock(&ftm->ftm_quaddec_mutex); > + > + ftm_read(ftm, FTM_SC, &scflags); > + > + scflags &= ~FTM_SC_PS_MASK; > + cnt_mode &= FTM_SC_PS_MASK; /*just to be 100% sure*/ > + > + scflags |= cnt_mode; > + > + /* Write */ > + ftm_clear_write_protection(ftm); > + ftm_write(ftm, FTM_SC, scflags); > + ftm_set_write_protection(ftm); > + > + /* Also resets the counter as it is undefined anyway now */ > + ftm_reset_counter(ftm); > + > + mutex_unlock(&ftm->ftm_quaddec_mutex); > + return 0; > +} > + > +static const char * const ftm_quaddec_prescaler[] = { > + "1", "2", "4", "8", "16", "32", "64", "128" > +}; > + > +static struct counter_count_enum_ext ftm_quaddec_prescaler_enum = { > + .items = ftm_quaddec_prescaler, > + .num_items = ARRAY_SIZE(ftm_quaddec_prescaler), > + .get = ftm_quaddec_get_prescaler, > + .set = ftm_quaddec_set_prescaler > +}; > + > +enum ftm_quaddec_synapse_action { > + FTM_QUADDEC_SYNAPSE_ACTION_BOTH_EDGES, > +}; > + > +static enum counter_synapse_action ftm_quaddec_synapse_actions[] = { > + [FTM_QUADDEC_SYNAPSE_ACTION_BOTH_EDGES] = > + COUNTER_SYNAPSE_ACTION_BOTH_EDGES > +}; > + > +enum ftm_quaddec_count_function { > + FTM_QUADDEC_COUNT_ENCODER_MODE_1, > +}; The FlexTimer Module supports more than just a quadrature counter mode doesn't it? We should keep this initial patch simple since we are still introducing the Counter subsystem, but it'll be nice to add support in the future for the other counter modes such as single-edge capture. > + > +static const enum counter_count_function ftm_quaddec_count_functions[] = { > + [FTM_QUADDEC_COUNT_ENCODER_MODE_1] = > + COUNTER_COUNT_FUNCTION_QUADRATURE_X4 > +}; > + > +static int ftm_quaddec_count_read(struct counter_device *counter, > + struct counter_count *count, > + struct counter_count_read_value *val) > +{ > + struct ftm_quaddec *const ftm = counter->priv; > + uint32_t cntval; > + > + ftm_read(ftm, FTM_CNT, &cntval); > + > + counter_count_read_value_set(val, COUNTER_COUNT_POSITION, &cntval); > + > + return 0; > +} > + > +static int ftm_quaddec_count_write(struct counter_device *counter, > + struct counter_count *count, > + struct counter_count_write_value *val) > +{ > + struct ftm_quaddec *const ftm = counter->priv; > + u32 cnt; > + int err; > + > + err = counter_count_write_value_get(&cnt, COUNTER_COUNT_POSITION, val); > + if (err) > + return err; > + > + if (cnt != 0) { > + dev_warn(&ftm->pdev->dev, "Can only accept '0' as new counter value\n"); > + return -EINVAL; > + } > + > + ftm_reset_counter(ftm); > + > + return 0; > +} > + > +static int ftm_quaddec_count_function_get(struct counter_device *counter, > + struct counter_count *count, > + size_t *function) > +{ > + *function = FTM_QUADDEC_COUNT_ENCODER_MODE_1; > + > + return 0; > +} > + > +static int ftm_quaddec_action_get(struct counter_device *counter, > + struct counter_count *count, > + struct counter_synapse *synapse, > + size_t *action) > +{ > + *action = FTM_QUADDEC_SYNAPSE_ACTION_BOTH_EDGES; > + > + return 0; > +} > + > +static const struct counter_ops ftm_quaddec_cnt_ops = { > + .count_read = ftm_quaddec_count_read, > + .count_write = ftm_quaddec_count_write, > + .function_get = ftm_quaddec_count_function_get, > + .action_get = ftm_quaddec_action_get, > +}; > + > +static struct counter_signal ftm_quaddec_signals[] = { > + { > + .id = 0, > + .name = "Channel 1 Quadrature A" > + }, > + { > + .id = 1, > + .name = "Channel 1 Quadrature B" > + } > +}; If possible, these names should match the FTM datasheet naming convention. The reason is to make it easier for users to match the input signals described in the datasheet with the Signal data provided by the Generic Counter interface. I think the FTM datasheet describes these signals as "Phase A" and "Phase B", so perhaps "Channel 1 Phase A" and "Channel 1 Phase B" may be more appropriate names in this case. > + > +static struct counter_synapse ftm_quaddec_count_synapses[] = { > + { > + .actions_list = ftm_quaddec_synapse_actions, > + .num_actions = ARRAY_SIZE(ftm_quaddec_synapse_actions), > + .signal = &ftm_quaddec_signals[0] > + }, > + { > + .actions_list = ftm_quaddec_synapse_actions, > + .num_actions = ARRAY_SIZE(ftm_quaddec_synapse_actions), > + .signal = &ftm_quaddec_signals[1] > + } > +}; > + > +static const struct counter_count_ext ftm_quaddec_count_ext[] = { > + COUNTER_COUNT_ENUM("prescaler", &ftm_quaddec_prescaler_enum), > + COUNTER_COUNT_ENUM_AVAILABLE("prescaler", &ftm_quaddec_prescaler_enum), > +}; > + > +static struct counter_count ftm_quaddec_counts = { > + .id = 0, > + .name = "Channel 1 Count", > + .functions_list = ftm_quaddec_count_functions, > + .num_functions = ARRAY_SIZE(ftm_quaddec_count_functions), > + .synapses = ftm_quaddec_count_synapses, > + .num_synapses = ARRAY_SIZE(ftm_quaddec_count_synapses), > + .ext = ftm_quaddec_count_ext, > + .num_ext = ARRAY_SIZE(ftm_quaddec_count_ext) > +}; > + > +static int ftm_quaddec_probe(struct platform_device *pdev) > +{ > + struct ftm_quaddec *ftm; > + > + struct device_node *node = pdev->dev.of_node; > + struct resource *io; > + int ret; > + > + ftm = devm_kzalloc(&pdev->dev, sizeof(*ftm), GFP_KERNEL); > + if (!ftm) > + return -ENOMEM; > + > + platform_set_drvdata(pdev, ftm); > + > + io = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (!io) { > + dev_err(&pdev->dev, "Failed to get memory region\n"); > + return -ENODEV; > + } > + > + ftm->pdev = pdev; > + ftm->big_endian = of_property_read_bool(node, "big-endian"); > + ftm->ftm_base = devm_ioremap(&pdev->dev, io->start, resource_size(io)); > + > + if (!ftm->ftm_base) { > + dev_err(&pdev->dev, "Failed to map memory region\n"); > + return -EINVAL; > + } > + ftm->counter.name = dev_name(&pdev->dev); > + ftm->counter.parent = &pdev->dev; > + ftm->counter.ops = &ftm_quaddec_cnt_ops; > + ftm->counter.counts = &ftm_quaddec_counts; > + ftm->counter.num_counts = 1; > + ftm->counter.signals = ftm_quaddec_signals; > + ftm->counter.num_signals = ARRAY_SIZE(ftm_quaddec_signals); > + ftm->counter.priv = ftm; > + > + mutex_init(&ftm->ftm_quaddec_mutex); > + > + ftm_quaddec_init(ftm); > + > + ret = counter_register(&ftm->counter); > + if (ret) > + ftm_quaddec_disable(ftm); > + > + return ret; > +} > + > +static int ftm_quaddec_remove(struct platform_device *pdev) > +{ > + struct ftm_quaddec *ftm = platform_get_drvdata(pdev); > + > + counter_unregister(&ftm->counter); > + > + ftm_quaddec_disable(ftm); > + > + return 0; > +} If the ftm_quaddec_disable is not necessary, then we can eliminate the ftm_quaddec_remove function as well by replacing the counter_register call with a devm_counter_register call. William Breathitt Gray > + > +static const struct of_device_id ftm_quaddec_match[] = { > + { .compatible = "fsl,ftm-quaddec" }, > + {}, > +}; > + > +static struct platform_driver ftm_quaddec_driver = { > + .driver = { > + .name = "ftm-quaddec", > + .owner = THIS_MODULE, > + .of_match_table = ftm_quaddec_match, > + }, > + .probe = ftm_quaddec_probe, > + .remove = ftm_quaddec_remove, > +}; > + > +module_platform_driver(ftm_quaddec_driver); > + > +MODULE_LICENSE("GPL"); > +MODULE_AUTHOR("Kjeld Flarup <kfa@deif.com"); > +MODULE_AUTHOR("Patrick Havelange <patrick.havelange@essensium.com"); > -- > 2.19.1 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 5/7] counter: add FlexTimer Module Quadrature decoder counter driver 2019-03-07 11:32 ` William Breathitt Gray @ 2019-03-11 11:22 ` Patrick Havelange 0 siblings, 0 replies; 19+ messages in thread From: Patrick Havelange @ 2019-03-11 11:22 UTC (permalink / raw) To: William Breathitt Gray Cc: Mark Rutland, devicetree, Jonathan Cameron, linux-pwm, linux-iio, linuxppc-dev, Daniel Lezcano, linux-kernel, Li Yang, Rob Herring, Thierry Reding, linux-arm-kernel, Thomas Gleixner, Shawn Guo, Esben Haabendal On Thu, Mar 7, 2019 at 12:32 PM William Breathitt Gray <vilhelm.gray@gmail.com> wrote: > > +/* > > + * take mutex > > + * call ftm_clear_write_protection > > + * update settings > > + * call ftm_set_write_protection > > + * release mutex > > + */ > > Jonathan mentioned it before in the previous review, and I think I agree > too, that this comment block is superfluous: the context of this code is > simple enough that the function call order is naturally obvious (i.e. > write protection must be cleared before settings are modified). > > The only important thing to mention here is that the mutex must be held > before the write protection state is modified so a comment along the > following lines should suffice: > > /* hold mutex before modifying write protection state */ I think that keeping the more verbose comments is better. You directly see what operations are needed, and is a good reminder, especially if you are not familiar with the driver. I'll use your comment on the next version if you insist (see below for why new versoion). > > +static void ftm_quaddec_disable(struct ftm_quaddec *ftm) > > +{ > > + ftm_write(ftm, FTM_MODE, 0); > > +} > > The ftm_quaddec_disable function is only used for cleanup when the > driver is being removed. Is disabling the FTM counter on removal > actually something we need to do? It might provide some power-saving, so I would keep that function. > > While it's true that the register will keep updating, since the driver > is no longer loaded, we don't care about that register value. Once we > take control of the hardware again (by reloading our driver or via a new > one), we reinitialize the counter and set the count value back to 0 > anyway -- so whatever value the register had no longer matters. > Indeed the previous values at start do not matter. It's there just to shut down the device properly. This discussion made me verify again the specs and in its current form the disable doesn't even work at all : - That register should be written with write protection disabled (duh!) - It doesn't even stop the FTM from running, the clock must be disabled for that. So I'll probably provide a fix for that (in some days/weeks). > > + > > +enum ftm_quaddec_count_function { > > + FTM_QUADDEC_COUNT_ENCODER_MODE_1, > > +}; > > The FlexTimer Module supports more than just a quadrature counter mode > doesn't it? > > We should keep this initial patch simple since we are still introducing > the Counter subsystem, but it'll be nice to add support in the future > for the other counter modes such as single-edge capture. yes it provides more features, those are in a backlog ;). I would prefer if this simple version(I mean, with the disable/shutdown fixed) of the driver could be merged already before extending support. > > > + > > +static struct counter_signal ftm_quaddec_signals[] = { > > + { > > + .id = 0, > > + .name = "Channel 1 Quadrature A" > > + }, > > + { > > + .id = 1, > > + .name = "Channel 1 Quadrature B" > > + } > > +}; > > If possible, these names should match the FTM datasheet naming > convention. The reason is to make it easier for users to match the > input signals described in the datasheet with the Signal data provided > by the Generic Counter interface. > > I think the FTM datasheet describes these signals as "Phase A" and > "Phase B", so perhaps "Channel 1 Phase A" and "Channel 1 Phase B" may be > more appropriate names in this case. I'll verify those, > > +static int ftm_quaddec_remove(struct platform_device *pdev) > > +{ > > + struct ftm_quaddec *ftm = platform_get_drvdata(pdev); > > + > > + counter_unregister(&ftm->counter); > > + > > + ftm_quaddec_disable(ftm); > > + > > + return 0; > > +} > > If the ftm_quaddec_disable is not necessary, then we can eliminate the > ftm_quaddec_remove function as well by replacing the counter_register > call with a devm_counter_register call. yes, but as stated before, I would keep it for potential energy saving. Thanks for your feedback :) _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 5/7] counter: add FlexTimer Module Quadrature decoder counter driver 2019-03-06 11:12 ` [PATCH v2 5/7] counter: add FlexTimer Module Quadrature decoder counter driver Patrick Havelange 2019-03-07 11:32 ` William Breathitt Gray @ 2019-03-11 12:24 ` Jonathan Cameron 1 sibling, 0 replies; 19+ messages in thread From: Jonathan Cameron @ 2019-03-11 12:24 UTC (permalink / raw) To: Patrick Havelange Cc: Mark Rutland, devicetree, linuxppc-dev, Jonathan Cameron, linux-pwm, linux-iio, linux-kernel, Daniel Lezcano, William Breathitt Gray, Li Yang, Rob Herring, Thierry Reding, linux-arm-kernel, Thomas Gleixner, Shawn Guo, Esben Haabendal On Wed, 6 Mar 2019 12:12:06 +0100 Patrick Havelange <patrick.havelange@essensium.com> wrote: > This driver exposes the counter for the quadrature decoder of the > FlexTimer Module, present in the LS1021A soc. > > Signed-off-by: Patrick Havelange <patrick.havelange@essensium.com> A few really trivial bits inline to add to William's feedback. Otherwise I'm happy enough, Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > --- > Changes v2 > - Rebased on new counter subsystem > - Cleaned up included headers > - Use devm_ioremap() > - Correct order of devm_ and unmanaged resources > --- > drivers/counter/Kconfig | 9 + > drivers/counter/Makefile | 1 + > drivers/counter/ftm-quaddec.c | 356 ++++++++++++++++++++++++++++++++++ > 3 files changed, 366 insertions(+) > create mode 100644 drivers/counter/ftm-quaddec.c > > diff --git a/drivers/counter/Kconfig b/drivers/counter/Kconfig > index 87c491a19c63..233ac305d878 100644 > --- a/drivers/counter/Kconfig > +++ b/drivers/counter/Kconfig > @@ -48,4 +48,13 @@ config STM32_LPTIMER_CNT > To compile this driver as a module, choose M here: the > module will be called stm32-lptimer-cnt. > > +config FTM_QUADDEC > + tristate "Flex Timer Module Quadrature decoder driver" > + help > + Select this option to enable the Flex Timer Quadrature decoder > + driver. > + > + To compile this driver as a module, choose M here: the > + module will be called ftm-quaddec. > + > endif # COUNTER > diff --git a/drivers/counter/Makefile b/drivers/counter/Makefile > index 5589976d37f8..0c9e622a6bea 100644 > --- a/drivers/counter/Makefile > +++ b/drivers/counter/Makefile > @@ -7,3 +7,4 @@ obj-$(CONFIG_COUNTER) += counter.o > obj-$(CONFIG_104_QUAD_8) += 104-quad-8.o > obj-$(CONFIG_STM32_TIMER_CNT) += stm32-timer-cnt.o > obj-$(CONFIG_STM32_LPTIMER_CNT) += stm32-lptimer-cnt.o > +obj-$(CONFIG_FTM_QUADDEC) += ftm-quaddec.o > diff --git a/drivers/counter/ftm-quaddec.c b/drivers/counter/ftm-quaddec.c > new file mode 100644 > index 000000000000..1bc9e075a386 > --- /dev/null > +++ b/drivers/counter/ftm-quaddec.c > @@ -0,0 +1,356 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Flex Timer Module Quadrature decoder > + * > + * This module implements a driver for decoding the FTM quadrature > + * of ex. a LS1021A > + */ > + > +#include <linux/fsl/ftm.h> > +#include <linux/module.h> > +#include <linux/platform_device.h> > +#include <linux/of.h> > +#include <linux/io.h> > +#include <linux/mutex.h> > +#include <linux/counter.h> > + > +struct ftm_quaddec { > + struct counter_device counter; > + struct platform_device *pdev; > + void __iomem *ftm_base; > + bool big_endian; > + struct mutex ftm_quaddec_mutex; > +}; > + > +static void ftm_read(struct ftm_quaddec *ftm, uint32_t offset, uint32_t *data) > +{ > + if (ftm->big_endian) > + *data = ioread32be(ftm->ftm_base + offset); > + else > + *data = ioread32(ftm->ftm_base + offset); > +} > + > +static void ftm_write(struct ftm_quaddec *ftm, uint32_t offset, uint32_t data) > +{ > + if (ftm->big_endian) > + iowrite32be(data, ftm->ftm_base + offset); > + else > + iowrite32(data, ftm->ftm_base + offset); > +} > + > +/* > + * take mutex > + * call ftm_clear_write_protection > + * update settings > + * call ftm_set_write_protection > + * release mutex > + */ > +static void ftm_clear_write_protection(struct ftm_quaddec *ftm) > +{ > + uint32_t flag; > + > + /* First see if it is enabled */ > + ftm_read(ftm, FTM_FMS, &flag); > + > + if (flag & FTM_FMS_WPEN) { > + ftm_read(ftm, FTM_MODE, &flag); > + ftm_write(ftm, FTM_MODE, flag | FTM_MODE_WPDIS); > + } > +} > + > +static void ftm_set_write_protection(struct ftm_quaddec *ftm) > +{ > + ftm_write(ftm, FTM_FMS, FTM_FMS_WPEN); > +} > + > +static void ftm_reset_counter(struct ftm_quaddec *ftm) > +{ > + /* Reset hardware counter to CNTIN */ > + ftm_write(ftm, FTM_CNT, 0x0); > +} > + > +static void ftm_quaddec_init(struct ftm_quaddec *ftm) > +{ > + ftm_clear_write_protection(ftm); > + > + /* > + * Do not write in the region from the CNTIN register through the > + * PWMLOAD register when FTMEN = 0. > + */ > + ftm_write(ftm, FTM_MODE, FTM_MODE_FTMEN); > + ftm_write(ftm, FTM_CNTIN, 0x0000); > + ftm_write(ftm, FTM_MOD, 0xffff); > + ftm_write(ftm, FTM_CNT, 0x0); > + ftm_write(ftm, FTM_SC, FTM_SC_PS_1); > + > + /* Select quad mode */ > + ftm_write(ftm, FTM_QDCTRL, FTM_QDCTRL_QUADEN); > + > + /* Unused features and reset to default section */ > + ftm_write(ftm, FTM_POL, 0x0); > + ftm_write(ftm, FTM_FLTCTRL, 0x0); > + ftm_write(ftm, FTM_SYNCONF, 0x0); > + ftm_write(ftm, FTM_SYNC, 0xffff); > + > + /* Lock the FTM */ > + ftm_set_write_protection(ftm); > +} > + > +static void ftm_quaddec_disable(struct ftm_quaddec *ftm) > +{ > + ftm_write(ftm, FTM_MODE, 0); > +} > + > +static int ftm_quaddec_get_prescaler(struct counter_device *counter, > + struct counter_count *count, > + size_t *cnt_mode) > +{ > + struct ftm_quaddec *ftm = counter->priv; > + uint32_t scflags; > + > + ftm_read(ftm, FTM_SC, &scflags); > + > + *cnt_mode = scflags & FTM_SC_PS_MASK; FIELD_GET and FIELD_PREP are a tidy way of doing these sorts of accesses. Helpfully they also save a reviewer having to sanity check that the offset is 0 as it is here. > + > + return 0; > +} > + > +static int ftm_quaddec_set_prescaler(struct counter_device *counter, > + struct counter_count *count, > + size_t cnt_mode) > +{ > + struct ftm_quaddec *ftm = counter->priv; > + > + uint32_t scflags; > + > + mutex_lock(&ftm->ftm_quaddec_mutex); > + > + ftm_read(ftm, FTM_SC, &scflags); > + > + scflags &= ~FTM_SC_PS_MASK; > + cnt_mode &= FTM_SC_PS_MASK; /*just to be 100% sure*/ nitpick: Comment syntax is /* just to be 100% sure */ > + > + scflags |= cnt_mode; > + > + /* Write */ > + ftm_clear_write_protection(ftm); > + ftm_write(ftm, FTM_SC, scflags); > + ftm_set_write_protection(ftm); > + > + /* Also resets the counter as it is undefined anyway now */ > + ftm_reset_counter(ftm); > + > + mutex_unlock(&ftm->ftm_quaddec_mutex); > + return 0; > +} > + > +static const char * const ftm_quaddec_prescaler[] = { > + "1", "2", "4", "8", "16", "32", "64", "128" > +}; > + > +static struct counter_count_enum_ext ftm_quaddec_prescaler_enum = { > + .items = ftm_quaddec_prescaler, > + .num_items = ARRAY_SIZE(ftm_quaddec_prescaler), > + .get = ftm_quaddec_get_prescaler, > + .set = ftm_quaddec_set_prescaler > +}; > + > +enum ftm_quaddec_synapse_action { > + FTM_QUADDEC_SYNAPSE_ACTION_BOTH_EDGES, > +}; > + > +static enum counter_synapse_action ftm_quaddec_synapse_actions[] = { > + [FTM_QUADDEC_SYNAPSE_ACTION_BOTH_EDGES] = > + COUNTER_SYNAPSE_ACTION_BOTH_EDGES > +}; > + > +enum ftm_quaddec_count_function { > + FTM_QUADDEC_COUNT_ENCODER_MODE_1, > +}; > + > +static const enum counter_count_function ftm_quaddec_count_functions[] = { > + [FTM_QUADDEC_COUNT_ENCODER_MODE_1] = > + COUNTER_COUNT_FUNCTION_QUADRATURE_X4 > +}; > + > +static int ftm_quaddec_count_read(struct counter_device *counter, > + struct counter_count *count, > + struct counter_count_read_value *val) > +{ > + struct ftm_quaddec *const ftm = counter->priv; > + uint32_t cntval; > + > + ftm_read(ftm, FTM_CNT, &cntval); > + > + counter_count_read_value_set(val, COUNTER_COUNT_POSITION, &cntval); > + > + return 0; > +} > + > +static int ftm_quaddec_count_write(struct counter_device *counter, > + struct counter_count *count, > + struct counter_count_write_value *val) > +{ > + struct ftm_quaddec *const ftm = counter->priv; > + u32 cnt; > + int err; > + > + err = counter_count_write_value_get(&cnt, COUNTER_COUNT_POSITION, val); > + if (err) > + return err; > + > + if (cnt != 0) { > + dev_warn(&ftm->pdev->dev, "Can only accept '0' as new counter value\n"); > + return -EINVAL; > + } > + > + ftm_reset_counter(ftm); > + > + return 0; > +} > + > +static int ftm_quaddec_count_function_get(struct counter_device *counter, > + struct counter_count *count, > + size_t *function) > +{ > + *function = FTM_QUADDEC_COUNT_ENCODER_MODE_1; > + > + return 0; > +} > + > +static int ftm_quaddec_action_get(struct counter_device *counter, > + struct counter_count *count, > + struct counter_synapse *synapse, > + size_t *action) > +{ > + *action = FTM_QUADDEC_SYNAPSE_ACTION_BOTH_EDGES; > + > + return 0; > +} > + > +static const struct counter_ops ftm_quaddec_cnt_ops = { > + .count_read = ftm_quaddec_count_read, > + .count_write = ftm_quaddec_count_write, > + .function_get = ftm_quaddec_count_function_get, > + .action_get = ftm_quaddec_action_get, > +}; > + > +static struct counter_signal ftm_quaddec_signals[] = { > + { > + .id = 0, > + .name = "Channel 1 Quadrature A" > + }, > + { > + .id = 1, > + .name = "Channel 1 Quadrature B" > + } > +}; > + > +static struct counter_synapse ftm_quaddec_count_synapses[] = { > + { > + .actions_list = ftm_quaddec_synapse_actions, > + .num_actions = ARRAY_SIZE(ftm_quaddec_synapse_actions), > + .signal = &ftm_quaddec_signals[0] > + }, > + { > + .actions_list = ftm_quaddec_synapse_actions, > + .num_actions = ARRAY_SIZE(ftm_quaddec_synapse_actions), > + .signal = &ftm_quaddec_signals[1] > + } > +}; > + > +static const struct counter_count_ext ftm_quaddec_count_ext[] = { > + COUNTER_COUNT_ENUM("prescaler", &ftm_quaddec_prescaler_enum), > + COUNTER_COUNT_ENUM_AVAILABLE("prescaler", &ftm_quaddec_prescaler_enum), > +}; > + > +static struct counter_count ftm_quaddec_counts = { > + .id = 0, > + .name = "Channel 1 Count", > + .functions_list = ftm_quaddec_count_functions, > + .num_functions = ARRAY_SIZE(ftm_quaddec_count_functions), > + .synapses = ftm_quaddec_count_synapses, > + .num_synapses = ARRAY_SIZE(ftm_quaddec_count_synapses), > + .ext = ftm_quaddec_count_ext, > + .num_ext = ARRAY_SIZE(ftm_quaddec_count_ext) > +}; > + > +static int ftm_quaddec_probe(struct platform_device *pdev) > +{ > + struct ftm_quaddec *ftm; > + > + struct device_node *node = pdev->dev.of_node; > + struct resource *io; > + int ret; > + > + ftm = devm_kzalloc(&pdev->dev, sizeof(*ftm), GFP_KERNEL); > + if (!ftm) > + return -ENOMEM; > + > + platform_set_drvdata(pdev, ftm); > + > + io = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (!io) { > + dev_err(&pdev->dev, "Failed to get memory region\n"); > + return -ENODEV; > + } > + > + ftm->pdev = pdev; > + ftm->big_endian = of_property_read_bool(node, "big-endian"); > + ftm->ftm_base = devm_ioremap(&pdev->dev, io->start, resource_size(io)); > + > + if (!ftm->ftm_base) { > + dev_err(&pdev->dev, "Failed to map memory region\n"); > + return -EINVAL; > + } > + ftm->counter.name = dev_name(&pdev->dev); > + ftm->counter.parent = &pdev->dev; > + ftm->counter.ops = &ftm_quaddec_cnt_ops; > + ftm->counter.counts = &ftm_quaddec_counts; > + ftm->counter.num_counts = 1; > + ftm->counter.signals = ftm_quaddec_signals; > + ftm->counter.num_signals = ARRAY_SIZE(ftm_quaddec_signals); > + ftm->counter.priv = ftm; > + > + mutex_init(&ftm->ftm_quaddec_mutex); > + > + ftm_quaddec_init(ftm); > + > + ret = counter_register(&ftm->counter); > + if (ret) > + ftm_quaddec_disable(ftm); > + > + return ret; > +} > + > +static int ftm_quaddec_remove(struct platform_device *pdev) > +{ > + struct ftm_quaddec *ftm = platform_get_drvdata(pdev); > + > + counter_unregister(&ftm->counter); > + > + ftm_quaddec_disable(ftm); > + > + return 0; > +} > + > +static const struct of_device_id ftm_quaddec_match[] = { > + { .compatible = "fsl,ftm-quaddec" }, > + {}, > +}; > + > +static struct platform_driver ftm_quaddec_driver = { > + .driver = { > + .name = "ftm-quaddec", > + .owner = THIS_MODULE, No need to set this, the call to __platform_driver_register that comes from the module_platform_driver macro below will deal with this for you. > + .of_match_table = ftm_quaddec_match, > + }, > + .probe = ftm_quaddec_probe, > + .remove = ftm_quaddec_remove, > +}; > + > +module_platform_driver(ftm_quaddec_driver); > + > +MODULE_LICENSE("GPL"); > +MODULE_AUTHOR("Kjeld Flarup <kfa@deif.com"); > +MODULE_AUTHOR("Patrick Havelange <patrick.havelange@essensium.com"); _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 6/7] counter: ftm-quaddec: Documentation: Add specific counter sysfs documentation 2019-03-06 11:12 [PATCH v2 0/7] FlexTimer Module Quadrature decoder counter Patrick Havelange ` (4 preceding siblings ...) 2019-03-06 11:12 ` [PATCH v2 5/7] counter: add FlexTimer Module Quadrature decoder counter driver Patrick Havelange @ 2019-03-06 11:12 ` Patrick Havelange 2019-03-07 11:42 ` William Breathitt Gray 2019-03-06 11:12 ` [PATCH v2 7/7] LS1021A: dtsi: add ftm quad decoder entries Patrick Havelange 2019-03-07 12:03 ` [PATCH v2 0/7] FlexTimer Module Quadrature decoder counter William Breathitt Gray 7 siblings, 1 reply; 19+ messages in thread From: Patrick Havelange @ 2019-03-06 11:12 UTC (permalink / raw) To: William Breathitt Gray, Rob Herring, Mark Rutland, Shawn Guo, Li Yang, Daniel Lezcano, Thomas Gleixner, Thierry Reding, Esben Haabendal, linux-iio, linux-kernel, devicetree, linux-arm-kernel, linux-pwm, linuxppc-dev, Jonathan Cameron Cc: Patrick Havelange This adds documentation for the specific prescaler entry. Signed-off-by: Patrick Havelange <patrick.havelange@essensium.com> --- Changes v2 - Add doc for prescaler entry --- .../ABI/testing/sysfs-bus-counter-ftm-quaddec | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-bus-counter-ftm-quaddec diff --git a/Documentation/ABI/testing/sysfs-bus-counter-ftm-quaddec b/Documentation/ABI/testing/sysfs-bus-counter-ftm-quaddec new file mode 100644 index 000000000000..2da629d6d485 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-bus-counter-ftm-quaddec @@ -0,0 +1,16 @@ +What: /sys/bus/counter/devices/counterX/countY/prescaler_available +KernelVersion: 5.1 +Contact: linux-iio@vger.kernel.org +Description: + Discrete set of available values for the respective Count Y + configuration are listed in this file. Values are delimited by + newline characters. + +What: /sys/bus/counter/devices/counterX/countY/prescaler +KernelVersion: 5.1 +Contact: linux-iio@vger.kernel.org +Description: + Configure the prescaler value associated with Count Y. + On the FlexTimer, the counter clock source passes through a + prescaler that is a 7-bit counter. This acts like a clock + divider. -- 2.19.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v2 6/7] counter: ftm-quaddec: Documentation: Add specific counter sysfs documentation 2019-03-06 11:12 ` [PATCH v2 6/7] counter: ftm-quaddec: Documentation: Add specific counter sysfs documentation Patrick Havelange @ 2019-03-07 11:42 ` William Breathitt Gray 2019-03-11 12:36 ` Jonathan Cameron 0 siblings, 1 reply; 19+ messages in thread From: William Breathitt Gray @ 2019-03-07 11:42 UTC (permalink / raw) To: Patrick Havelange Cc: Mark Rutland, devicetree, Jonathan Cameron, linux-pwm, linux-iio, linuxppc-dev, Daniel Lezcano, linux-kernel, Li Yang, Rob Herring, Thierry Reding, linux-arm-kernel, Thomas Gleixner, Shawn Guo, Esben Haabendal On Wed, Mar 06, 2019 at 12:12:07PM +0100, Patrick Havelange wrote: > This adds documentation for the specific prescaler entry. > > Signed-off-by: Patrick Havelange <patrick.havelange@essensium.com> > --- > Changes v2 > - Add doc for prescaler entry > --- > .../ABI/testing/sysfs-bus-counter-ftm-quaddec | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > create mode 100644 Documentation/ABI/testing/sysfs-bus-counter-ftm-quaddec > > diff --git a/Documentation/ABI/testing/sysfs-bus-counter-ftm-quaddec b/Documentation/ABI/testing/sysfs-bus-counter-ftm-quaddec > new file mode 100644 > index 000000000000..2da629d6d485 > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-bus-counter-ftm-quaddec > @@ -0,0 +1,16 @@ > +What: /sys/bus/counter/devices/counterX/countY/prescaler_available > +KernelVersion: 5.1 > +Contact: linux-iio@vger.kernel.org > +Description: > + Discrete set of available values for the respective Count Y > + configuration are listed in this file. Values are delimited by > + newline characters. > + > +What: /sys/bus/counter/devices/counterX/countY/prescaler > +KernelVersion: 5.1 > +Contact: linux-iio@vger.kernel.org > +Description: > + Configure the prescaler value associated with Count Y. > + On the FlexTimer, the counter clock source passes through a > + prescaler that is a 7-bit counter. This acts like a clock > + divider. > -- > 2.19.1 Hmm, prescalers seem common enough among counter devices to permit these attributes to be listed in the sysfs-bus-counter documentation file. However, I'd like to wait until we get another counter driver for a device with a prescaler before we make that move. From there, we'll have a better vantage point to determine a fitting standard prescaler attribute behavior. So for now, we'll keep these attributes documented here in the sysfs-bus-counter-ftm-quaddec file, until the time comes to broach the discussion again. William Breathitt Gray _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 6/7] counter: ftm-quaddec: Documentation: Add specific counter sysfs documentation 2019-03-07 11:42 ` William Breathitt Gray @ 2019-03-11 12:36 ` Jonathan Cameron 0 siblings, 0 replies; 19+ messages in thread From: Jonathan Cameron @ 2019-03-11 12:36 UTC (permalink / raw) To: William Breathitt Gray Cc: Mark Rutland, devicetree, linuxppc-dev, Jonathan Cameron, linux-pwm, linux-iio, Patrick Havelange, Daniel Lezcano, linux-kernel, Li Yang, Rob Herring, Thierry Reding, linux-arm-kernel, Thomas Gleixner, Shawn Guo, Esben Haabendal On Thu, 7 Mar 2019 20:42:16 +0900 William Breathitt Gray <vilhelm.gray@gmail.com> wrote: > On Wed, Mar 06, 2019 at 12:12:07PM +0100, Patrick Havelange wrote: > > This adds documentation for the specific prescaler entry. > > > > Signed-off-by: Patrick Havelange <patrick.havelange@essensium.com> > > --- > > Changes v2 > > - Add doc for prescaler entry > > --- > > .../ABI/testing/sysfs-bus-counter-ftm-quaddec | 16 ++++++++++++++++ > > 1 file changed, 16 insertions(+) > > create mode 100644 Documentation/ABI/testing/sysfs-bus-counter-ftm-quaddec > > > > diff --git a/Documentation/ABI/testing/sysfs-bus-counter-ftm-quaddec b/Documentation/ABI/testing/sysfs-bus-counter-ftm-quaddec > > new file mode 100644 > > index 000000000000..2da629d6d485 > > --- /dev/null > > +++ b/Documentation/ABI/testing/sysfs-bus-counter-ftm-quaddec > > @@ -0,0 +1,16 @@ > > +What: /sys/bus/counter/devices/counterX/countY/prescaler_available > > +KernelVersion: 5.1 > > +Contact: linux-iio@vger.kernel.org > > +Description: > > + Discrete set of available values for the respective Count Y > > + configuration are listed in this file. Values are delimited by > > + newline characters. > > + > > +What: /sys/bus/counter/devices/counterX/countY/prescaler > > +KernelVersion: 5.1 > > +Contact: linux-iio@vger.kernel.org > > +Description: > > + Configure the prescaler value associated with Count Y. > > + On the FlexTimer, the counter clock source passes through a > > + prescaler that is a 7-bit counter. This acts like a clock > > + divider. > > -- > > 2.19.1 > > Hmm, prescalers seem common enough among counter devices to permit these > attributes to be listed in the sysfs-bus-counter documentation file. > However, I'd like to wait until we get another counter driver for a > device with a prescaler before we make that move. From there, we'll have > a better vantage point to determine a fitting standard prescaler > attribute behavior. > > So for now, we'll keep these attributes documented here in the > sysfs-bus-counter-ftm-quaddec file, until the time comes to broach the > discussion again. Agreed. As long as the definition is sufficiently non-specific so it can be moved later. I'm not sure for example that the docs need to say that it is a 7 bit counter. That should be apparent from prescaler_available - or at least possible values should be which is all we need to know. Jonathan > > William Breathitt Gray _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 7/7] LS1021A: dtsi: add ftm quad decoder entries 2019-03-06 11:12 [PATCH v2 0/7] FlexTimer Module Quadrature decoder counter Patrick Havelange ` (5 preceding siblings ...) 2019-03-06 11:12 ` [PATCH v2 6/7] counter: ftm-quaddec: Documentation: Add specific counter sysfs documentation Patrick Havelange @ 2019-03-06 11:12 ` Patrick Havelange 2019-03-07 12:03 ` [PATCH v2 0/7] FlexTimer Module Quadrature decoder counter William Breathitt Gray 7 siblings, 0 replies; 19+ messages in thread From: Patrick Havelange @ 2019-03-06 11:12 UTC (permalink / raw) To: William Breathitt Gray, Rob Herring, Mark Rutland, Shawn Guo, Li Yang, Daniel Lezcano, Thomas Gleixner, Thierry Reding, Esben Haabendal, linux-iio, linux-kernel, devicetree, linux-arm-kernel, linux-pwm, linuxppc-dev, Jonathan Cameron Cc: Patrick Havelange Add the 4 Quadrature counters for this board. Signed-off-by: Patrick Havelange <patrick.havelange@essensium.com> Reviewed-by: Esben Haabendal <esben@haabendal.dk> --- Changes v2 - None --- arch/arm/boot/dts/ls1021a.dtsi | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/arch/arm/boot/dts/ls1021a.dtsi b/arch/arm/boot/dts/ls1021a.dtsi index ed0941292172..0168fb62590a 100644 --- a/arch/arm/boot/dts/ls1021a.dtsi +++ b/arch/arm/boot/dts/ls1021a.dtsi @@ -433,6 +433,34 @@ status = "disabled"; }; + counter0: counter@29d0000 { + compatible = "fsl,ftm-quaddec"; + reg = <0x0 0x29d0000 0x0 0x10000>; + big-endian; + status = "disabled"; + }; + + counter1: counter@29e0000 { + compatible = "fsl,ftm-quaddec"; + reg = <0x0 0x29e0000 0x0 0x10000>; + big-endian; + status = "disabled"; + }; + + counter2: counter@29f0000 { + compatible = "fsl,ftm-quaddec"; + reg = <0x0 0x29f0000 0x0 0x10000>; + big-endian; + status = "disabled"; + }; + + counter3: counter@2a00000 { + compatible = "fsl,ftm-quaddec"; + reg = <0x0 0x2a00000 0x0 0x10000>; + big-endian; + status = "disabled"; + }; + gpio0: gpio@2300000 { compatible = "fsl,ls1021a-gpio", "fsl,qoriq-gpio"; reg = <0x0 0x2300000 0x0 0x10000>; -- 2.19.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v2 0/7] FlexTimer Module Quadrature decoder counter 2019-03-06 11:12 [PATCH v2 0/7] FlexTimer Module Quadrature decoder counter Patrick Havelange ` (6 preceding siblings ...) 2019-03-06 11:12 ` [PATCH v2 7/7] LS1021A: dtsi: add ftm quad decoder entries Patrick Havelange @ 2019-03-07 12:03 ` William Breathitt Gray 7 siblings, 0 replies; 19+ messages in thread From: William Breathitt Gray @ 2019-03-07 12:03 UTC (permalink / raw) To: Patrick Havelange, jic23 Cc: Mark Rutland, devicetree, linux-pwm, linux-iio, linuxppc-dev, Daniel Lezcano, linux-kernel, Li Yang, Rob Herring, Thierry Reding, linux-arm-kernel, Thomas Gleixner, Shawn Guo, Esben Haabendal On Wed, Mar 06, 2019 at 12:12:01PM +0100, Patrick Havelange wrote: > This patch serie is to be applied on top of > https://patchwork.kernel.org/project/linux-iio/list/?series=147 > (a more recent version of the serie is available here : > https://gitlab.com/vilhelmgray/iio/tree/generic_counter_v10 ) > > Main changes in v2: > The code is a bit simpler, thanks to more use of devm_* functions. > The polling/32bit signed version has been dropped, as not needed and > no other driver is doing that. > > > Patrick Havelange (7): > include/fsl: add common FlexTimer #defines in a separate header. > drivers/pwm: pwm-fsl-ftm: use common header for FlexTimer #defines > drivers/clocksource: timer-fsl-ftm: use common header for FlexTimer > #defines > dt-bindings: counter: ftm-quaddec > counter: add FlexTimer Module Quadrature decoder counter driver > counter: ftm-quaddec: Documentation: Add specific counter sysfs > documentation > LS1021A: dtsi: add ftm quad decoder entries > > .../ABI/testing/sysfs-bus-counter-ftm-quaddec | 16 + > .../bindings/counter/ftm-quaddec.txt | 18 + > arch/arm/boot/dts/ls1021a.dtsi | 28 ++ > drivers/clocksource/timer-fsl-ftm.c | 15 +- > drivers/counter/Kconfig | 9 + > drivers/counter/Makefile | 1 + > drivers/counter/ftm-quaddec.c | 356 ++++++++++++++++++ > drivers/pwm/pwm-fsl-ftm.c | 44 +-- > include/linux/fsl/ftm.h | 88 +++++ > 9 files changed, 519 insertions(+), 56 deletions(-) > create mode 100644 Documentation/ABI/testing/sysfs-bus-counter-ftm-quaddec > create mode 100644 Documentation/devicetree/bindings/counter/ftm-quaddec.txt > create mode 100644 drivers/counter/ftm-quaddec.c > create mode 100644 include/linux/fsl/ftm.h > > -- > 2.19.1 Patrick, I see you dropped the polling support in this version. If the need arises in the future, we can discuss a possible ways of resolving your latency issues; I imagine interrupts during overflow/underflow events to be a common behavior among counter devices so those too may result in latency issues as you discovered in your case. Jonathan, If you are satisfied with the changes in this patchset, let me know which patches you like and I'll add respective Reviewed-by tags for you for the next Counter subsystem introduction patchset submission. William Breathitt Gray _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2019-04-11 16:40 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-03-06 11:12 [PATCH v2 0/7] FlexTimer Module Quadrature decoder counter Patrick Havelange 2019-03-06 11:12 ` [PATCH v2 1/7] include/fsl: add common FlexTimer #defines in a separate header Patrick Havelange 2019-03-06 11:12 ` [PATCH v2 2/7] drivers/pwm: pwm-fsl-ftm: use common header for FlexTimer #defines Patrick Havelange 2019-03-06 11:12 ` [PATCH v2 3/7] drivers/clocksource: timer-fsl-ftm: " Patrick Havelange 2019-04-11 16:40 ` Daniel Lezcano 2019-03-06 11:12 ` [PATCH v2 4/7] dt-bindings: counter: ftm-quaddec Patrick Havelange 2019-03-12 19:09 ` Rob Herring 2019-03-16 14:21 ` Jonathan Cameron 2019-03-26 15:43 ` Arnout Vandecappelle 2019-03-26 16:06 ` Rob Herring 2019-03-06 11:12 ` [PATCH v2 5/7] counter: add FlexTimer Module Quadrature decoder counter driver Patrick Havelange 2019-03-07 11:32 ` William Breathitt Gray 2019-03-11 11:22 ` Patrick Havelange 2019-03-11 12:24 ` Jonathan Cameron 2019-03-06 11:12 ` [PATCH v2 6/7] counter: ftm-quaddec: Documentation: Add specific counter sysfs documentation Patrick Havelange 2019-03-07 11:42 ` William Breathitt Gray 2019-03-11 12:36 ` Jonathan Cameron 2019-03-06 11:12 ` [PATCH v2 7/7] LS1021A: dtsi: add ftm quad decoder entries Patrick Havelange 2019-03-07 12:03 ` [PATCH v2 0/7] FlexTimer Module Quadrature decoder counter William Breathitt Gray
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).