* [PATCH v4 0/4] Add RZ/G2L MTU3a MFD, Counter and pwm driver
@ 2022-10-10 14:52 Biju Das
2022-10-10 14:52 ` [PATCH v4 1/4] dt-bindings: mfd: Document RZ/G2L MTU3a bindings Biju Das
` (3 more replies)
0 siblings, 4 replies; 32+ messages in thread
From: Biju Das @ 2022-10-10 14:52 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, William Breathitt Gray, Thierry Reding
Cc: Biju Das, Lee Jones, Uwe Kleine-König, devicetree,
linux-pwm, Geert Uytterhoeven, Chris Paterson, Biju Das,
Prabhakar Mahadev Lad, linux-renesas-soc
The RZ/G2L multi-function timer pulse unit 3 (MTU3a) is embedded in
the Renesas RZ/G2L family SoC's. It consists of eight 16-bit timer
channels and one 32-bit timer channel. It supports the following
functions
- Counter
- Timer
- PWM
This patch series aim to add MFD and pwm driver for MTU3a.
The 8/16/32 bit registers are mixed in each channel. The HW
specifications of the IP is described in patch#1.
Current patch set is tested for PWM mode1 on MTU3 channel
and 16 and 32 bit phase counting modes.
v3->v4:
* Dropped counter and pwm compatibeles as they don't have any resources.
* Made rz-mtu3 as pwm provider.
* Updated the example and description.
* A single driver that registers both the counter and the pwm functionalities
that binds against "renesas,rz-mtu3".
* Moved PM handling from child devices to here.
* replaced include/linux/mfd/rz-mtu3.h->drivers/mfd/rz-mtu3.h
* Removed "remove" callback from mfd driver
* There is no resource associated with "rz-mtu3-counter" and "rz-mtu3-pwm"
compatible and moved the code to mfd subsystem as it binds against "rz-mtu".
* Removed struct platform_driver rz_mtu3_cnt_driver.
* Removed struct platform_driver rz_mtu3_pwm_driver.
* Updated commit description
* Updated Kconfig description
* Added macros RZ_MTU3_16_BIT_MTU{1,2}_CH for MTU1 and MTU2 channels
* Added RZ_MTU3_GET_HW_CH macro for getting channel ID.
* replaced priv->ch[id]->priv->ch[0] in rz_mtu3_count_read()
* Cached counter max values
* replaced cnt->tsr in rz_mtu3_count_direction_read()
* Added comments for RZ_MTU3_TCR_CCLR_NONE
* Replaced if with switch in rz_mtu3_initialize_counter() and
rz_mtu3_count_ceiling_write()
* Added locks in initialize, terminate and enable_read to prevent races.
* Updated rz_mtu3_action_read to take care of MTU2 signals.
* Added separate distinct array for each group of Synapse.
* Moved pm handling to parent.
v2->v3:
* Dropped counter bindings and integrated with mfd as it has only one property.
* Removed "#address-cells" and "#size-cells" as it do not have children with
unit addresses.
* Removed quotes from counter and pwm.
* Provided full path for pwm bindings.
* Updated the binding example.
* removed unwanted header files
* Added LUT for 32 bit registers as it needed for 32-bit cascade counting.
* Exported 32 bit read/write functions.
* Modelled as a counter device supporting 3 counters(2 16-bit and
32-bit)
* Add kernel-doc comments to document struct rz_mtu3_cnt
* Removed mmio variable from struct rz_mtu3_cnt
* Removed cnt local variable from rz_mtu3_count_read()
* Replaced -EINVAL->-ERANGE for out of range error conditions.
* Removed explicit cast from write functions.
* Removed local variable val from rz_mtu3_count_ceiling_read()
* Added lock for RMW for counter/ceiling updates.
* Added different synapses for counter0 and counter{1,2}
* Used ARRAY for assigning num_counts.
* Added PM runtime for managing clocks.
* Add MODULE_IMPORT_NS(COUNTER) to import the COUNTER namespace.
RFC->v2:
* replaced devm_reset_control_get->devm_reset_control_get_exclusive
* Dropped 'bindings' from the binding title
* Updated the binding example
* Added additionalProperties: false for counter bindings
* Squashed all the binding patches
* Modelled as a single counter device providing both 16-bit
and 32-bit phase counting modes
* Modelled as a single pwm device for supporting different pwm modes.
* Moved counter and pwm bindings to respective subsystems.
Biju Das (4):
dt-bindings: mfd: Document RZ/G2L MTU3a bindings
mfd: Add RZ/G2L MTU3 driver
mfd: Add RZ/G2L MTU3 counter driver
mfd: Add RZ/G2L MTU3 PWM driver
.../bindings/mfd/renesas,rz-mtu3.yaml | 305 ++++++++++
drivers/mfd/Kconfig | 26 +
drivers/mfd/Makefile | 5 +
drivers/mfd/rz-mtu3-cnt.c | 554 ++++++++++++++++++
drivers/mfd/rz-mtu3-core.c | 476 +++++++++++++++
drivers/mfd/rz-mtu3-pwm.c | 405 +++++++++++++
drivers/mfd/rz-mtu3.h | 221 +++++++
7 files changed, 1992 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/renesas,rz-mtu3.yaml
create mode 100644 drivers/mfd/rz-mtu3-cnt.c
create mode 100644 drivers/mfd/rz-mtu3-core.c
create mode 100644 drivers/mfd/rz-mtu3-pwm.c
create mode 100644 drivers/mfd/rz-mtu3.h
--
2.25.1
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v4 1/4] dt-bindings: mfd: Document RZ/G2L MTU3a bindings
2022-10-10 14:52 [PATCH v4 0/4] Add RZ/G2L MTU3a MFD, Counter and pwm driver Biju Das
@ 2022-10-10 14:52 ` Biju Das
2022-10-11 14:45 ` Krzysztof Kozlowski
2022-10-10 14:52 ` [PATCH v4 2/4] mfd: Add RZ/G2L MTU3 driver Biju Das
` (2 subsequent siblings)
3 siblings, 1 reply; 32+ messages in thread
From: Biju Das @ 2022-10-10 14:52 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, William Breathitt Gray, Thierry Reding
Cc: Biju Das, Lee Jones, Uwe Kleine-König, devicetree,
linux-pwm, Geert Uytterhoeven, Chris Paterson, Biju Das,
Prabhakar Mahadev Lad, linux-renesas-soc
The RZ/G2L multi-function timer pulse unit 3 (MTU3a) is embedded in
the Renesas RZ/G2L family SoC's. It consists of eight 16-bit timer
channels and one 32-bit timer channel. It supports the following
functions
- Counter
- Timer
- PWM
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v3->v4:
* Dropped counter and pwm compatibeles as they don't have any resources.
* Made rz-mtu3 as pwm provider.
* Updated the example and description.
v2->v3:
* Dropped counter bindings and integrated with mfd as it has only one property.
* Removed "#address-cells" and "#size-cells" as it do not have children with
unit addresses.
* Removed quotes from counter and pwm.
* Provided full path for pwm bindings.
* Updated the example.
v1->v2:
* Modelled counter and pwm as a single device that handles
multiple channels.
* Moved counter and pwm bindings to respective subsystems
* Dropped 'bindings' from MFD binding title.
* Updated the example
* Changed the compatible names.
---
.../bindings/mfd/renesas,rz-mtu3.yaml | 305 ++++++++++++++++++
1 file changed, 305 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/renesas,rz-mtu3.yaml
diff --git a/Documentation/devicetree/bindings/mfd/renesas,rz-mtu3.yaml b/Documentation/devicetree/bindings/mfd/renesas,rz-mtu3.yaml
new file mode 100644
index 000000000000..1b0be9f5cd18
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/renesas,rz-mtu3.yaml
@@ -0,0 +1,305 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/renesas,rz-mtu3.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas RZ/G2L Multi-Function Timer Pulse Unit 3 (MTU3a)
+
+maintainers:
+ - Biju Das <biju.das.jz@bp.renesas.com>
+
+description: |
+ This hardware block pconsisting of eight 16-bit timer channels and one
+ 32- bit timer channel. It supports the following specifications:
+ - Pulse input/output: 28 lines max.
+ - Pulse input 3 lines
+ - Count clock 11 clocks for each channel (14 clocks for MTU0, 12 clocks
+ for MTU2, and 10 clocks for MTU5, four clocks for MTU1-MTU2 combination
+ (when LWA = 1))
+ - Operating frequency Up to 100 MHz
+ - Available operations [MTU0 to MTU4, MTU6, MTU7, and MTU8]
+ - Waveform output on compare match
+ - Input capture function (noise filter setting available)
+ - Counter-clearing operation
+ - Simultaneous writing to multiple timer counters (TCNT)
+ (excluding MTU8).
+ - Simultaneous clearing on compare match or input capture
+ (excluding MTU8).
+ - Simultaneous input and output to registers in synchronization with
+ counter operations (excluding MTU8).
+ - Up to 12-phase PWM output in combination with synchronous operation
+ (excluding MTU8)
+ - [MTU0 MTU3, MTU4, MTU6, MTU7, and MTU8]
+ - Buffer operation specifiable
+ - [MTU1, MTU2]
+ - Phase counting mode can be specified independently
+ - 32-bit phase counting mode can be specified for interlocked operation
+ of MTU1 and MTU2 (when TMDR3.LWA = 1)
+ - Cascade connection operation available
+ - [MTU3, MTU4, MTU6, and MTU7]
+ - Through interlocked operation of MTU3/4 and MTU6/7, the positive and
+ negative signals in six phases (12 phases in total) can be output in
+ complementary PWM and reset-synchronized PWM operation.
+ - In complementary PWM mode, values can be transferred from buffer
+ registers to temporary registers at crests and troughs of the timer-
+ counter values or when the buffer registers (TGRD registers in MTU4
+ and MTU7) are written to.
+ - Double-buffering selectable in complementary PWM mode.
+ - [MTU3 and MTU4]
+ - Through interlocking with MTU0, a mode for driving AC synchronous
+ motors (brushless DC motors) by using complementary PWM output and
+ reset-synchronized PWM output is settable and allows the selection
+ of two types of waveform output (chopping or level).
+ - [MTU5]
+ - Capable of operation as a dead-time compensation counter.
+ - [MTU0/MTU5, MTU1, MTU2, and MTU8]
+ - 32-bit phase counting mode specifiable by combining MTU1 and MTU2 and
+ through interlocked operation with MTU0/MTU5 and MTU8.
+ - Interrupt-skipping function
+ - In complementary PWM mode, interrupts on crests and troughs of counter
+ values and triggers to start conversion by the A/D converter can be
+ skipped.
+ - Interrupt sources: 43 sources.
+ - Buffer operation:
+ - Automatic transfer of register data (transfer from the buffer
+ register to the timer register).
+ - Trigger generation
+ - A/D converter start triggers can be generated
+ - A/D converter start request delaying function enables A/D converter
+ to be started with any desired timing and to be synchronized with
+ PWM output.
+ - Low power consumption function
+ - The MTU3a can be placed in the module-stop state.
+
+ There are two phase counting modes. 16-bit phase counting mode in which
+ MTU1 and MTU2 operate independently, and cascade connection 32-bit phase
+ counting mode in which MTU1 and MTU2 are cascaded.
+
+ In phase counting mode, the phase difference between two external input
+ clocks is detected and the corresponding TCNT is incremented or
+ decremented.
+ The below counters are supported
+ count0 - MTU1 16-bit phase counting
+ count1 - MTU2 16-bit phase counting
+ count2 - MTU1+ MTU2 32-bit phase counting
+
+ The module supports PWM mode{1,2}, Reset-synchronized PWM mode and
+ complementary PWM mode{1,2,3}.
+
+ In complementary PWM mode, six positive-phase and six negative-phase PWM
+ waveforms (12 phases in total) with dead time can be output by
+ combining MTU{3,4} and MTU{6,7}.
+
+ The below pwm channels are supported in pwm mode 1.
+ pwm0 - MTU0.MTIOC0A PWM mode 1
+ pwm1 - MTU0.MTIOC0C PWM mode 1
+ pwm2 - MTU1.MTIOC1A PWM mode 1
+ pwm3 - MTU2.MTIOC2A PWM mode 1
+ pwm4 - MTU3.MTIOC3A PWM mode 1
+ pwm5 - MTU3.MTIOC3C PWM mode 1
+ pwm6 - MTU4.MTIOC4A PWM mode 1
+ pwm7 - MTU4.MTIOC4C PWM mode 1
+ pwm8 - MTU6.MTIOC6A PWM mode 1
+ pwm9 - MTU6.MTIOC6C PWM mode 1
+ pwm10 - MTU7.MTIOC7A PWM mode 1
+ pwm11 - MTU7.MTIOC7C PWM mode 1
+
+properties:
+ compatible:
+ items:
+ - enum:
+ - renesas,r9a07g044-mtu3 # RZ/G2{L,LC}
+ - renesas,r9a07g054-mtu3 # RZ/V2L
+ - const: renesas,rz-mtu3
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ items:
+ - description: MTU0.TGRA input capture/compare match
+ - description: MTU0.TGRB input capture/compare match
+ - description: MTU0.TGRC input capture/compare match
+ - description: MTU0.TGRD input capture/compare match
+ - description: MTU0.TCNT overflow
+ - description: MTU0.TGRE compare match
+ - description: MTU0.TGRF compare match
+ - description: MTU1.TGRA input capture/compare match
+ - description: MTU1.TGRB input capture/compare match
+ - description: MTU1.TCNT overflow
+ - description: MTU1.TCNT underflow
+ - description: MTU2.TGRA input capture/compare match
+ - description: MTU2.TGRB input capture/compare match
+ - description: MTU2.TCNT overflow
+ - description: MTU2.TCNT underflow
+ - description: MTU3.TGRA input capture/compare match
+ - description: MTU3.TGRB input capture/compare match
+ - description: MTU3.TGRC input capture/compare match
+ - description: MTU3.TGRD input capture/compare match
+ - description: MTU3.TCNT overflow
+ - description: MTU4.TGRA input capture/compare match
+ - description: MTU4.TGRB input capture/compare match
+ - description: MTU4.TGRC input capture/compare match
+ - description: MTU4.TGRD input capture/compare match
+ - description: MTU4.TCNT overflow/underflow
+ - description: MTU5.TGRU input capture/compare match
+ - description: MTU5.TGRV input capture/compare match
+ - description: MTU5.TGRW input capture/compare match
+ - description: MTU6.TGRA input capture/compare match
+ - description: MTU6.TGRB input capture/compare match
+ - description: MTU6.TGRC input capture/compare match
+ - description: MTU6.TGRD input capture/compare match
+ - description: MTU6.TCNT overflow
+ - description: MTU7.TGRA input capture/compare match
+ - description: MTU7.TGRB input capture/compare match
+ - description: MTU7.TGRC input capture/compare match
+ - description: MTU7.TGRD input capture/compare match
+ - description: MTU7.TCNT overflow/underflow
+ - description: MTU8.TGRA input capture/compare match
+ - description: MTU8.TGRB input capture/compare match
+ - description: MTU8.TGRC input capture/compare match
+ - description: MTU8.TGRD input capture/compare match
+ - description: MTU8.TCNT overflow
+ - description: MTU8.TCNT underflow
+
+ interrupt-names:
+ items:
+ - const: tgia0
+ - const: tgib0
+ - const: tgic0
+ - const: tgid0
+ - const: tgiv0
+ - const: tgie0
+ - const: tgif0
+ - const: tgia1
+ - const: tgib1
+ - const: tgiv1
+ - const: tgiu1
+ - const: tgia2
+ - const: tgib2
+ - const: tgiv2
+ - const: tgiu2
+ - const: tgia3
+ - const: tgib3
+ - const: tgic3
+ - const: tgid3
+ - const: tgiv3
+ - const: tgia4
+ - const: tgib4
+ - const: tgic4
+ - const: tgid4
+ - const: tgiv4
+ - const: tgiu5
+ - const: tgiv5
+ - const: tgiw5
+ - const: tgia6
+ - const: tgib6
+ - const: tgic6
+ - const: tgid6
+ - const: tgiv6
+ - const: tgia7
+ - const: tgib7
+ - const: tgic7
+ - const: tgid7
+ - const: tgiv7
+ - const: tgia8
+ - const: tgib8
+ - const: tgic8
+ - const: tgid8
+ - const: tgiv8
+ - const: tgiu8
+
+ clocks:
+ maxItems: 1
+
+ power-domains:
+ maxItems: 1
+
+ resets:
+ maxItems: 1
+
+ "#pwm-cells":
+ const: 2
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - interrupt-names
+ - clocks
+ - power-domains
+ - resets
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/r9a07g044-cpg.h>
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+ mtu3: timer@10001200 {
+ compatible = "renesas,r9a07g044-mtu3",
+ "renesas,rz-mtu3";
+ reg = <0x10001200 0xb00>;
+ interrupts = <GIC_SPI 170 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 171 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 172 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 173 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 174 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 175 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 176 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 177 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 178 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 179 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 180 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 181 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 182 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 183 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 184 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 185 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 186 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 187 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 188 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 189 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 190 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 191 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 192 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 193 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 194 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 195 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 196 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 197 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 198 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 199 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 200 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 201 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 202 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 203 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 204 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 205 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 206 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 207 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 208 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 209 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 210 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 211 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 212 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 213 IRQ_TYPE_EDGE_RISING>;
+ interrupt-names = "tgia0", "tgib0", "tgic0", "tgid0", "tgiv0", "tgie0",
+ "tgif0",
+ "tgia1", "tgib1", "tgiv1", "tgiu1",
+ "tgia2", "tgib2", "tgiv2", "tgiu2",
+ "tgia3", "tgib3", "tgic3", "tgid3", "tgiv3",
+ "tgia4", "tgib4", "tgic4", "tgid4", "tgiv4",
+ "tgiu5", "tgiv5", "tgiw5",
+ "tgia6", "tgib6", "tgic6", "tgid6", "tgiv6",
+ "tgia7", "tgib7", "tgic7", "tgid7", "tgiv7",
+ "tgia8", "tgib8", "tgic8", "tgid8", "tgiv8", "tgiu8";
+ clocks = <&cpg CPG_MOD R9A07G044_MTU_X_MCK_MTU3>;
+ power-domains = <&cpg>;
+ resets = <&cpg R9A07G044_MTU_X_PRESET_MTU3>;
+ #pwm-cells = <2>;
+ };
+
+...
--
2.25.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v4 2/4] mfd: Add RZ/G2L MTU3 driver
2022-10-10 14:52 [PATCH v4 0/4] Add RZ/G2L MTU3a MFD, Counter and pwm driver Biju Das
2022-10-10 14:52 ` [PATCH v4 1/4] dt-bindings: mfd: Document RZ/G2L MTU3a bindings Biju Das
@ 2022-10-10 14:52 ` Biju Das
2022-10-10 14:52 ` [PATCH v4 3/4] mfd: Add RZ/G2L MTU3 counter driver Biju Das
2022-10-10 14:52 ` [PATCH v4 4/4] mfd: Add RZ/G2L MTU3 PWM driver Biju Das
3 siblings, 0 replies; 32+ messages in thread
From: Biju Das @ 2022-10-10 14:52 UTC (permalink / raw)
To: Philipp Zabel, William Breathitt Gray, Thierry Reding
Cc: Biju Das, Lee Jones, Geert Uytterhoeven, Chris Paterson,
Biju Das, Prabhakar Mahadev Lad, linux-renesas-soc
Add RZ/G2L MTU3 MFD driver. This driver exposes the following functions
1) 8/16/32 register access
2) handles runtime PM.
3) channel enable/disable for client modules.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v3->v4:
* A single driver that registers both the counter and the pwm functionalities
that binds against "renesas,rz-mtu3".
* Moved PM handling from child devices to here.
* replaced include/linux/mfd/rz-mtu3.h->drivers/mfd/rz-mtu3.h
* Removed "remove" callback
v2->v3:
* removed unwanted header files
* Added LUT for 32 bit registers as it needed for 32-bit cascade counting.
* Exported 32 bit read/write functions.
v1->v2:
* Changed the compatible name
* Replaced devm_reset_control_get->devm_reset_control_get_exclusive
* Renamed function names rzg2l_mtu3->rz_mtu3 as this is generic IP
in RZ family SoC's.
---
drivers/mfd/Kconfig | 12 +
drivers/mfd/Makefile | 2 +
drivers/mfd/rz-mtu3-core.c | 476 +++++++++++++++++++++++++++++++++++++
drivers/mfd/rz-mtu3.h | 221 +++++++++++++++++
4 files changed, 711 insertions(+)
create mode 100644 drivers/mfd/rz-mtu3-core.c
create mode 100644 drivers/mfd/rz-mtu3.h
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index abb58ab1a1a4..7329971a3bdf 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1974,6 +1974,18 @@ config MFD_ROHM_BD957XMUF
BD9573MUF Power Management ICs. BD9576 and BD9573 are primarily
designed to be used to power R-Car series processors.
+config MFD_RZ_MTU3
+ tristate "Support for RZ/G2L Multi-Function Timer Pulse Unit 3 MTU3a"
+ depends on (ARCH_RZG2L && OF) || COMPILE_TEST
+ select MFD_CORE
+ help
+ Select this option to enable RZ/G2L MTU3 timers driver used
+ for PWM, Clock Source, Clock event and Counter. This driver allow to
+ share the registers between the others drivers.
+
+ To compile this driver as a module, choose M here: the
+ module will be called rz-mtu3.
+
config MFD_STM32_LPTIMER
tristate "Support for STM32 Low-Power Timer"
depends on (ARCH_STM32 && OF) || COMPILE_TEST
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 858cacf659d6..cd492903411c 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -251,6 +251,8 @@ obj-$(CONFIG_MFD_ALTERA_SYSMGR) += altera-sysmgr.o
obj-$(CONFIG_MFD_STPMIC1) += stpmic1.o
obj-$(CONFIG_MFD_SUN4I_GPADC) += sun4i-gpadc.o
+rz-mtu3-objs := rz-mtu3-core.o
+obj-$(CONFIG_MFD_RZ_MTU3) += rz-mtu3.o
obj-$(CONFIG_MFD_STM32_LPTIMER) += stm32-lptimer.o
obj-$(CONFIG_MFD_STM32_TIMERS) += stm32-timers.o
obj-$(CONFIG_MFD_MXS_LRADC) += mxs-lradc.o
diff --git a/drivers/mfd/rz-mtu3-core.c b/drivers/mfd/rz-mtu3-core.c
new file mode 100644
index 000000000000..a9386765f51c
--- /dev/null
+++ b/drivers/mfd/rz-mtu3-core.c
@@ -0,0 +1,476 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Renesas RZ/G2L Multi-Function Timer Pulse Unit 3 - MTU3a
+ *
+ * Copyright (C) 2022 Renesas Electronics Corporation
+ */
+
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/module.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/of_platform.h>
+#include <linux/pm_runtime.h>
+#include <linux/reset.h>
+
+#include "rz-mtu3.h"
+
+static const unsigned long rz_mtu3_8bit_ch_reg_offs[][13] = {
+ {
+ [RZ_MTU3_TIER] = 0x4, [RZ_MTU3_NFCR] = 0x70,
+ [RZ_MTU3_TCR] = 0x0, [RZ_MTU3_TCR2] = 0x28,
+ [RZ_MTU3_TMDR1] = 0x1, [RZ_MTU3_TIORH] = 0x2,
+ [RZ_MTU3_TIORL] = 0x3
+ },
+ {
+ [RZ_MTU3_TIER] = 0x4, [RZ_MTU3_NFCR] = 0xef,
+ [RZ_MTU3_TSR] = 0x5, [RZ_MTU3_TCR] = 0x0,
+ [RZ_MTU3_TCR2] = 0x14, [RZ_MTU3_TMDR1] = 0x1,
+ [RZ_MTU3_TIOR] = 0x2
+ },
+ {
+ [RZ_MTU3_TIER] = 0x4, [RZ_MTU3_NFCR] = 0x16e,
+ [RZ_MTU3_TSR] = 0x5, [RZ_MTU3_TCR] = 0x0,
+ [RZ_MTU3_TCR2] = 0xc, [RZ_MTU3_TMDR1] = 0x1,
+ [RZ_MTU3_TIOR] = 0x2
+ },
+ {
+ [RZ_MTU3_TIER] = 0x8, [RZ_MTU3_NFCR] = 0x93,
+ [RZ_MTU3_TSR] = 0x2c, [RZ_MTU3_TCR] = 0x0,
+ [RZ_MTU3_TCR2] = 0x4c, [RZ_MTU3_TMDR1] = 0x2,
+ [RZ_MTU3_TIORH] = 0x4, [RZ_MTU3_TIORL] = 0x5,
+ [RZ_MTU3_TBTM] = 0x38
+ },
+ {
+ [RZ_MTU3_TIER] = 0x8, [RZ_MTU3_NFCR] = 0x93,
+ [RZ_MTU3_TSR] = 0x2c, [RZ_MTU3_TCR] = 0x0,
+ [RZ_MTU3_TCR2] = 0x4c, [RZ_MTU3_TMDR1] = 0x2,
+ [RZ_MTU3_TIORH] = 0x5, [RZ_MTU3_TIORL] = 0x6,
+ [RZ_MTU3_TBTM] = 0x38
+ },
+ {
+ [RZ_MTU3_TIER] = 0x32, [RZ_MTU3_NFCR] = 0x1eb,
+ [RZ_MTU3_TSTR] = 0x34, [RZ_MTU3_TCNTCMPCLR] = 0x36,
+ [RZ_MTU3_TCRU] = 0x4, [RZ_MTU3_TCR2U] = 0x5,
+ [RZ_MTU3_TIORU] = 0x6, [RZ_MTU3_TCRV] = 0x14,
+ [RZ_MTU3_TCR2V] = 0x15, [RZ_MTU3_TIORV] = 0x16,
+ [RZ_MTU3_TCRW] = 0x24, [RZ_MTU3_TCR2W] = 0x25,
+ [RZ_MTU3_TIORW] = 0x26
+ },
+ {
+ [RZ_MTU3_TIER] = 0x8, [RZ_MTU3_NFCR] = 0x93,
+ [RZ_MTU3_TSR] = 0x2c, [RZ_MTU3_TCR] = 0x0,
+ [RZ_MTU3_TCR2] = 0x4c, [RZ_MTU3_TMDR1] = 0x2,
+ [RZ_MTU3_TIORH] = 0x4, [RZ_MTU3_TIORL] = 0x5,
+ [RZ_MTU3_TBTM] = 0x38
+ },
+ {
+ [RZ_MTU3_TIER] = 0x8, [RZ_MTU3_NFCR] = 0x93,
+ [RZ_MTU3_TSR] = 0x2c, [RZ_MTU3_TCR] = 0x0,
+ [RZ_MTU3_TCR2] = 0x4c, [RZ_MTU3_TMDR1] = 0x2,
+ [RZ_MTU3_TIORH] = 0x5, [RZ_MTU3_TIORL] = 0x6,
+ [RZ_MTU3_TBTM] = 0x38
+ },
+ {
+ [RZ_MTU3_TIER] = 0x4, [RZ_MTU3_NFCR] = 0x368,
+ [RZ_MTU3_TCR] = 0x0, [RZ_MTU3_TCR2] = 0x6,
+ [RZ_MTU3_TMDR1] = 0x1, [RZ_MTU3_TIORH] = 0x2,
+ [RZ_MTU3_TIORL] = 0x3
+ }
+};
+
+static const unsigned long rz_mtu3_16bit_ch_reg_offs[][12] = {
+ {
+ [RZ_MTU3_TCNT] = 0x6, [RZ_MTU3_TGRA] = 0x8,
+ [RZ_MTU3_TGRB] = 0xa, [RZ_MTU3_TGRC] = 0xc,
+ [RZ_MTU3_TGRD] = 0xe, [RZ_MTU3_TGRE] = 0x20,
+ [RZ_MTU3_TGRF] = 0x22
+ },
+ {
+ [RZ_MTU3_TCNT] = 0x6, [RZ_MTU3_TGRA] = 0x8,
+ [RZ_MTU3_TGRB] = 0xa
+ },
+ {
+ [RZ_MTU3_TCNT] = 0x6, [RZ_MTU3_TGRA] = 0x8,
+ [RZ_MTU3_TGRB] = 0xa
+ },
+ {
+ [RZ_MTU3_TCNT] = 0x10, [RZ_MTU3_TGRA] = 0x18,
+ [RZ_MTU3_TGRB] = 0x1a, [RZ_MTU3_TGRC] = 0x24,
+ [RZ_MTU3_TGRD] = 0x26, [RZ_MTU3_TGRE] = 0x72
+ },
+ {
+ [RZ_MTU3_TCNT] = 0x11, [RZ_MTU3_TGRA] = 0x1b,
+ [RZ_MTU3_TGRB] = 0x1d, [RZ_MTU3_TGRC] = 0x27,
+ [RZ_MTU3_TGRD] = 0x29, [RZ_MTU3_TGRE] = 0x73,
+ [RZ_MTU3_TGRF] = 0x75, [RZ_MTU3_TADCR] = 0x3f,
+ [RZ_MTU3_TADCORA] = 0x43, [RZ_MTU3_TADCORB] = 0x45,
+ [RZ_MTU3_TADCOBRA] = 0x47,
+ [RZ_MTU3_TADCOBRB] = 0x49
+ },
+ {
+ [RZ_MTU3_TCNTU] = 0x0, [RZ_MTU3_TGRU] = 0x2,
+ [RZ_MTU3_TCNTV] = 0x10, [RZ_MTU3_TGRV] = 0x12,
+ [RZ_MTU3_TCNTW] = 0x20, [RZ_MTU3_TGRW] = 0x22
+ },
+ {
+ [RZ_MTU3_TCNT] = 0x10, [RZ_MTU3_TGRA] = 0x18,
+ [RZ_MTU3_TGRB] = 0x1a, [RZ_MTU3_TGRC] = 0x24,
+ [RZ_MTU3_TGRD] = 0x26, [RZ_MTU3_TGRE] = 0x72
+ },
+ {
+ [RZ_MTU3_TCNT] = 0x11, [RZ_MTU3_TGRA] = 0x1b,
+ [RZ_MTU3_TGRB] = 0x1d, [RZ_MTU3_TGRC] = 0x27,
+ [RZ_MTU3_TGRD] = 0x29, [RZ_MTU3_TGRE] = 0x73,
+ [RZ_MTU3_TGRF] = 0x75, [RZ_MTU3_TADCR] = 0x3f,
+ [RZ_MTU3_TADCORA] = 0x43, [RZ_MTU3_TADCORB] = 0x45,
+ [RZ_MTU3_TADCOBRA] = 0x47,
+ [RZ_MTU3_TADCOBRB] = 0x49
+ },
+};
+
+static const unsigned long rz_mtu3_32bit_ch_reg_offs[][5] = {
+ {
+ [RZ_MTU3_TCNTLW] = 0x20, [RZ_MTU3_TGRALW] = 0x24,
+ [RZ_MTU3_TGRBLW] = 0x28
+ },
+ { [RZ_MTU3_TCNT] = 0x8, [RZ_MTU3_TGRA] = 0xc,
+ [RZ_MTU3_TGRB] = 0x10, [RZ_MTU3_TGRC] = 0x14,
+ [RZ_MTU3_TGRD] = 0x18
+ }
+};
+
+static bool rz_mtu3_is_16bit_shared_reg(u16 off)
+{
+ return (off == RZ_MTU3_TDDRA || off == RZ_MTU3_TDDRB ||
+ off == RZ_MTU3_TCDRA || off == RZ_MTU3_TCDRB ||
+ off == RZ_MTU3_TCBRA || off == RZ_MTU3_TCBRB ||
+ off == RZ_MTU3_TCNTSA || off == RZ_MTU3_TCNTSB);
+}
+
+static u16 rz_mtu3_shared_reg_read(struct rz_mtu3_channel *ch, u16 off)
+{
+ struct rz_mtu3 *mtu = dev_get_drvdata(ch->dev);
+
+ if (rz_mtu3_is_16bit_shared_reg(off))
+ return readw(mtu->mmio + off);
+ else
+ return readb(mtu->mmio + off);
+}
+
+u8 rz_mtu3_8bit_ch_read(struct rz_mtu3_channel *ch, u16 off)
+{
+ u16 ch_offs;
+
+ ch_offs = rz_mtu3_8bit_ch_reg_offs[ch->index][off];
+ if (off != RZ_MTU3_TCR && ch_offs == 0)
+ return -EINVAL;
+
+ /*
+ * NFCR register addresses on MTU{0,1,2,5,8} channels are smaller than
+ * channel's base address.
+ */
+ if (off == RZ_MTU3_NFCR && (ch->index <= RZ_MTU2 ||
+ ch->index == RZ_MTU5 ||
+ ch->index == RZ_MTU8))
+ return readb(ch->base - ch_offs);
+ else
+ return readb(ch->base + ch_offs);
+}
+EXPORT_SYMBOL_GPL(rz_mtu3_8bit_ch_read);
+
+u16 rz_mtu3_16bit_ch_read(struct rz_mtu3_channel *ch, u16 off)
+{
+ u16 ch_offs;
+
+ /* MTU8 doesn't have 16-bit registers */
+ if (ch->index == RZ_MTU8)
+ return 0;
+
+ ch_offs = rz_mtu3_16bit_ch_reg_offs[ch->index][off];
+ if (ch->index != RZ_MTU5 && off != RZ_MTU3_TCNTU && ch_offs == 0)
+ return 0;
+
+ return readw(ch->base + ch_offs);
+}
+EXPORT_SYMBOL_GPL(rz_mtu3_16bit_ch_read);
+
+u32 rz_mtu3_32bit_ch_read(struct rz_mtu3_channel *ch, u16 off)
+{
+ u16 ch_offs;
+
+ if (ch->index == RZ_MTU1)
+ ch_offs = rz_mtu3_32bit_ch_reg_offs[0][off];
+ else if (ch->index == RZ_MTU8)
+ ch_offs = rz_mtu3_32bit_ch_reg_offs[1][off];
+
+ if (!ch_offs)
+ return -EINVAL;
+
+ return readl(ch->base + ch_offs);
+}
+EXPORT_SYMBOL_GPL(rz_mtu3_32bit_ch_read);
+
+void rz_mtu3_8bit_ch_write(struct rz_mtu3_channel *ch, u16 off, u8 val)
+{
+ u16 ch_offs;
+
+ ch_offs = rz_mtu3_8bit_ch_reg_offs[ch->index][off];
+ if (ch->index != RZ_MTU5 && off != RZ_MTU3_TCR && ch_offs == 0)
+ return;
+
+ /*
+ * NFCR register addresses on MTU{0,1,2,5,8} channels are smaller than
+ * channel's base address.
+ */
+ if (off == RZ_MTU3_NFCR && (ch->index <= RZ_MTU2 ||
+ ch->index == RZ_MTU5 ||
+ ch->index == RZ_MTU8))
+ writeb(val, ch->base - ch_offs);
+ else
+ writeb(val, ch->base + ch_offs);
+}
+EXPORT_SYMBOL_GPL(rz_mtu3_8bit_ch_write);
+
+void rz_mtu3_16bit_ch_write(struct rz_mtu3_channel *ch, u16 off, u16 val)
+{
+ u16 ch_offs;
+
+ /* MTU8 doesn't have 16-bit registers */
+ if (ch->index == RZ_MTU8)
+ return;
+
+ ch_offs = rz_mtu3_16bit_ch_reg_offs[ch->index][off];
+ if (ch->index != RZ_MTU5 && off != RZ_MTU3_TCNTU && ch_offs == 0)
+ return;
+
+ writew(val, ch->base + ch_offs);
+}
+EXPORT_SYMBOL_GPL(rz_mtu3_16bit_ch_write);
+
+void rz_mtu3_32bit_ch_write(struct rz_mtu3_channel *ch, u16 off, u32 val)
+{
+ u16 ch_offs;
+
+ if (ch->index == RZ_MTU1)
+ ch_offs = rz_mtu3_32bit_ch_reg_offs[0][off];
+ else if (ch->index == RZ_MTU8)
+ ch_offs = rz_mtu3_32bit_ch_reg_offs[1][off];
+
+ if (!ch_offs)
+ return;
+
+ writel(val, ch->base + ch_offs);
+}
+EXPORT_SYMBOL_GPL(rz_mtu3_32bit_ch_write);
+
+void rz_mtu3_shared_reg_write(struct rz_mtu3_channel *ch, u16 off, u16 value)
+{
+ struct rz_mtu3 *mtu = dev_get_drvdata(ch->dev);
+
+ if (rz_mtu3_is_16bit_shared_reg(off))
+ writew(value, mtu->mmio + off);
+ else
+ writeb((u8)value, mtu->mmio + off);
+}
+EXPORT_SYMBOL_GPL(rz_mtu3_shared_reg_write);
+
+static void rz_mtu3_start_stop_ch(struct rz_mtu3_channel *ch, bool start)
+{
+ struct rz_mtu3 *mtu = dev_get_drvdata(ch->dev);
+ unsigned long flags, value;
+ u8 offs;
+
+ /* start stop register shared by multiple timer channels */
+ raw_spin_lock_irqsave(&mtu->lock, flags);
+
+ if (ch->index == RZ_MTU6 || ch->index == RZ_MTU7) {
+ value = rz_mtu3_shared_reg_read(ch, RZ_MTU3_TSTRB);
+ if (start)
+ value |= 1 << ch->index;
+ else
+ value &= ~(1 << ch->index);
+ rz_mtu3_shared_reg_write(ch, RZ_MTU3_TSTRB, value);
+ } else if (ch->index != RZ_MTU5) {
+ value = rz_mtu3_shared_reg_read(ch, RZ_MTU3_TSTRA);
+ if (ch->index == RZ_MTU8)
+ offs = 0x08;
+ else if (ch->index < RZ_MTU3)
+ offs = 1 << ch->index;
+ else
+ offs = 1 << (ch->index + 3);
+ if (start)
+ value |= offs;
+ else
+ value &= ~offs;
+ rz_mtu3_shared_reg_write(ch, RZ_MTU3_TSTRA, value);
+ }
+
+ raw_spin_unlock_irqrestore(&mtu->lock, flags);
+}
+
+bool rz_mtu3_is_enabled(struct rz_mtu3_channel *ch)
+{
+ struct rz_mtu3 *mtu = dev_get_drvdata(ch->dev);
+ unsigned long flags, value;
+ bool ret = false;
+ u8 offs;
+
+ /* start stop register shared by multiple timer channels */
+ raw_spin_lock_irqsave(&mtu->lock, flags);
+
+ if (ch->index == RZ_MTU6 || ch->index == RZ_MTU7) {
+ value = rz_mtu3_shared_reg_read(ch, RZ_MTU3_TSTRB);
+ ret = value & (1 << ch->index);
+ } else if (ch->index != RZ_MTU5) {
+ value = rz_mtu3_shared_reg_read(ch, RZ_MTU3_TSTRA);
+ if (ch->index == RZ_MTU8)
+ offs = 0x08;
+ else if (ch->index < RZ_MTU3)
+ offs = 1 << ch->index;
+ else
+ offs = 1 << (ch->index + 3);
+
+ ret = value & offs;
+ }
+
+ raw_spin_unlock_irqrestore(&mtu->lock, flags);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(rz_mtu3_is_enabled);
+
+int rz_mtu3_enable(struct rz_mtu3_channel *ch)
+{
+ /* enable channel */
+ rz_mtu3_start_stop_ch(ch, true);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(rz_mtu3_enable);
+
+void rz_mtu3_disable(struct rz_mtu3_channel *ch)
+{
+ /* disable channel */
+ rz_mtu3_start_stop_ch(ch, false);
+}
+EXPORT_SYMBOL_GPL(rz_mtu3_disable);
+
+static const unsigned int ch_reg_offsets[] = {
+ 0x100, 0x180, 0x200, 0x000, 0x001, 0xa80, 0x800, 0x801, 0x400
+};
+
+static void rz_mtu3_reset_assert(void *data)
+{
+ struct reset_control *rstc = data;
+
+ reset_control_assert(rstc);
+}
+
+static int __maybe_unused rz_mtu3_pm_runtime_suspend(struct device *dev)
+{
+ struct rz_mtu3 *ddata = dev_get_drvdata(dev);
+
+ clk_disable_unprepare(ddata->clk);
+
+ return 0;
+}
+
+static int __maybe_unused rz_mtu3_pm_runtime_resume(struct device *dev)
+{
+ struct rz_mtu3 *ddata = dev_get_drvdata(dev);
+
+ clk_prepare_enable(ddata->clk);
+
+ return 0;
+}
+
+static void rz_mtu3_pm_disable(void *data)
+{
+ pm_runtime_disable(data);
+ pm_runtime_set_suspended(data);
+}
+
+static const struct dev_pm_ops rz_mtu3_pm_ops = {
+ SET_RUNTIME_PM_OPS(rz_mtu3_pm_runtime_suspend, rz_mtu3_pm_runtime_resume, NULL)
+};
+
+static int rz_mtu3_probe(struct platform_device *pdev)
+{
+ struct reset_control *rstc;
+ struct rz_mtu3 *ddata;
+ unsigned int i;
+ int ret;
+
+ ddata = devm_kzalloc(&pdev->dev, sizeof(*ddata), GFP_KERNEL);
+ if (!ddata)
+ return -ENOMEM;
+
+ ddata->mmio = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(ddata->mmio))
+ return PTR_ERR(ddata->mmio);
+
+ rstc = devm_reset_control_get_exclusive(&pdev->dev, NULL);
+ if (IS_ERR(rstc))
+ return PTR_ERR(rstc);
+
+ ret = devm_add_action_or_reset(&pdev->dev, rz_mtu3_reset_assert,
+ rstc);
+ if (ret < 0)
+ return ret;
+
+ ddata->clk = devm_clk_get(&pdev->dev, NULL);
+ if (IS_ERR(ddata->clk))
+ return PTR_ERR(ddata->clk);
+
+ raw_spin_lock_init(&ddata->lock);
+ reset_control_deassert(rstc);
+
+ clk_prepare_enable(ddata->clk);
+ pm_runtime_set_active(&pdev->dev);
+ pm_runtime_enable(&pdev->dev);
+ ret = devm_add_action_or_reset(&pdev->dev,
+ rz_mtu3_pm_disable,
+ &pdev->dev);
+ if (ret < 0)
+ goto disable_clock;
+
+ for (i = 0; i < RZ_MTU_NUM_CHANNELS; i++) {
+ ddata->channels[i].index = i;
+ ddata->channels[i].function = RZ_MTU3_NORMAL;
+ ddata->channels[i].base = ddata->mmio + ch_reg_offsets[i];
+ }
+
+ platform_set_drvdata(pdev, ddata);
+
+ rz_mtu3_cnt_probe(pdev);
+ rz_mtu3_pwm_probe(pdev);
+
+ return 0;
+
+disable_clock:
+ clk_disable_unprepare(ddata->clk);
+
+ return ret;
+}
+
+static const struct of_device_id rz_mtu3_of_match[] = {
+ { .compatible = "renesas,rz-mtu3", },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, rz_mtu3_of_match);
+
+static struct platform_driver rz_mtu3_driver = {
+ .probe = rz_mtu3_probe,
+ .driver = {
+ .name = "rz-mtu3",
+ .pm = &rz_mtu3_pm_ops,
+ .of_match_table = rz_mtu3_of_match,
+ },
+};
+module_platform_driver(rz_mtu3_driver);
+
+MODULE_AUTHOR("Biju Das <biju.das.jz@bp.renesas.com>");
+MODULE_DESCRIPTION("Renesas RZ/G2L MTU3 Driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/mfd/rz-mtu3.h b/drivers/mfd/rz-mtu3.h
new file mode 100644
index 000000000000..48949c05ec0e
--- /dev/null
+++ b/drivers/mfd/rz-mtu3.h
@@ -0,0 +1,221 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2022 Renesas Electronics Corporation
+ */
+
+#ifndef __LINUX_RZ_MTU3_H__
+#define __LINUX_RZ_MTU3_H__
+
+/* 8-bit shared register offsets macros */
+#define RZ_MTU3_TSTRA 0x080 /* Timer start register A */
+#define RZ_MTU3_TSTRB 0x880 /* Timer start register B */
+
+/* 16-bit shared register offset macros */
+#define RZ_MTU3_TDDRA 0x016 /* Timer dead time data register A */
+#define RZ_MTU3_TDDRB 0x816 /* Timer dead time data register B */
+#define RZ_MTU3_TCDRA 0x014 /* Timer cycle data register A */
+#define RZ_MTU3_TCDRB 0x814 /* Timer cycle data register B */
+#define RZ_MTU3_TCBRA 0x022 /* Timer cycle buffer register A */
+#define RZ_MTU3_TCBRB 0x822 /* Timer cycle buffer register B */
+#define RZ_MTU3_TCNTSA 0x020 /* Timer subcounter A */
+#define RZ_MTU3_TCNTSB 0x820 /* Timer subcounter B */
+
+/*
+ * MTU5 contains 3 timer counter registers and is totaly different
+ * from other channels, so we must separate its offset
+ */
+
+/* 8-bit register offset macros of MTU3 channels except MTU5 */
+#define RZ_MTU3_TIER 0 /* Timer interrupt register */
+#define RZ_MTU3_NFCR 1 /* Noise filter control register */
+#define RZ_MTU3_TSR 2 /* Timer status register */
+#define RZ_MTU3_TCR 3 /* Timer control register */
+#define RZ_MTU3_TCR2 4 /* Timer control register 2 */
+#define RZ_MTU3_TMDR1 5 /* Timer mode register 1 */
+#define RZ_MTU3_TIOR 6 /* Timer I/O control register */
+#define RZ_MTU3_TIORH 6 /* Timer I/O control register H */
+#define RZ_MTU3_TIORL 7 /* Timer I/O control register L */
+/* Only MTU3/4/6/7 have TBTM registers */
+#define RZ_MTU3_TBTM 8 /* Timer buffer operation transfer mode register */
+
+/* 8-bit MTU5 register offset macros */
+#define RZ_MTU3_TSTR 2 /* MTU5 Timer start register */
+#define RZ_MTU3_TCNTCMPCLR 3 /* MTU5 Timer compare match clear register */
+#define RZ_MTU3_TCRU 4 /* Timer control register U */
+#define RZ_MTU3_TCR2U 5 /* Timer control register 2U */
+#define RZ_MTU3_TIORU 6 /* Timer I/O control register U */
+#define RZ_MTU3_TCRV 7 /* Timer control register V */
+#define RZ_MTU3_TCR2V 8 /* Timer control register 2V */
+#define RZ_MTU3_TIORV 9 /* Timer I/O control register V */
+#define RZ_MTU3_TCRW 10 /* Timer control register W */
+#define RZ_MTU3_TCR2W 11 /* Timer control register 2W */
+#define RZ_MTU3_TIORW 12 /* Timer I/O control register W */
+
+/* 16-bit register offset macros of MTU3 channels except MTU5 */
+#define RZ_MTU3_TCNT 0 /* Timer counter */
+#define RZ_MTU3_TGRA 1 /* Timer general register A */
+#define RZ_MTU3_TGRB 2 /* Timer general register B */
+#define RZ_MTU3_TGRC 3 /* Timer general register C */
+#define RZ_MTU3_TGRD 4 /* Timer general register D */
+#define RZ_MTU3_TGRE 5 /* Timer general register E */
+#define RZ_MTU3_TGRF 6 /* Timer general register F */
+/* Timer A/D converter start request registers */
+#define RZ_MTU3_TADCR 7 /* control register */
+#define RZ_MTU3_TADCORA 8 /* cycle set register A */
+#define RZ_MTU3_TADCORB 9 /* cycle set register B */
+#define RZ_MTU3_TADCOBRA 10 /* cycle set buffer register A */
+#define RZ_MTU3_TADCOBRB 11 /* cycle set buffer register B */
+
+/* 16-bit MTU5 register offset macros */
+#define RZ_MTU3_TCNTU 0 /* MTU5 Timer counter U */
+#define RZ_MTU3_TGRU 1 /* MTU5 Timer general register U */
+#define RZ_MTU3_TCNTV 2 /* MTU5 Timer counter V */
+#define RZ_MTU3_TGRV 3 /* MTU5 Timer general register V */
+#define RZ_MTU3_TCNTW 4 /* MTU5 Timer counter W */
+#define RZ_MTU3_TGRW 5 /* MTU5 Timer general register W */
+
+/* 32-bit register offset */
+#define RZ_MTU3_TCNTLW 0 /* Timer longword counter */
+#define RZ_MTU3_TGRALW 1 /* Timer longword general register A */
+#define RZ_MTU3_TGRBLW 2 /* Timer longowrd general register B */
+
+#define RZ_MTU3_TMDR3 0x191 /* MTU1 Timer Mode Register 3 */
+
+/* Macros for setting registers */
+#define RZ_MTU3_TCR_CCLR_TGRA BIT(5)
+
+struct platform_device;
+
+enum rz_mtu3_channels {
+ RZ_MTU0,
+ RZ_MTU1,
+ RZ_MTU2,
+ RZ_MTU3,
+ RZ_MTU4,
+ RZ_MTU5,
+ RZ_MTU6,
+ RZ_MTU7,
+ RZ_MTU8,
+ RZ_MTU_NUM_CHANNELS
+};
+
+enum rz_mtu3_functions {
+ RZ_MTU3_NORMAL,
+ RZ_MTU3_16BIT_PHASE_COUNTING,
+ RZ_MTU3_32BIT_PHASE_COUNTING,
+ RZ_MTU3_PWM_MODE_1,
+};
+
+/**
+ * struct rz_mtu3_channel - MTU3 channel private data
+ *
+ * @dev: device handle
+ * @index: channel index
+ * @base: channel base address
+ * @function: channel function
+ */
+struct rz_mtu3_channel {
+ struct device *dev;
+ unsigned int index;
+ void __iomem *base;
+ enum rz_mtu3_functions function;
+};
+
+/**
+ * struct rz_mtu3 - MTU3 MFD private data
+ *
+ * @clk: MTU3 module clock
+ * @mmio: MTU3 module clock
+ * @lock: Lock to protect shared register access
+ * @cnt_priv: counter private data
+ * @pwm_priv: pwm private data
+ * @rz_mtu3_channel: HW channels
+ */
+struct rz_mtu3 {
+ struct clk *clk;
+ void __iomem *mmio;
+ raw_spinlock_t lock; /* Protect the shared registers */
+ void *cnt_priv;
+ void *pwm_priv;
+ struct rz_mtu3_channel channels[RZ_MTU_NUM_CHANNELS];
+};
+
+#if IS_ENABLED(CONFIG_MFD_RZ_MTU3)
+bool rz_mtu3_is_enabled(struct rz_mtu3_channel *ch);
+void rz_mtu3_disable(struct rz_mtu3_channel *ch);
+int rz_mtu3_enable(struct rz_mtu3_channel *ch);
+
+u8 rz_mtu3_8bit_ch_read(struct rz_mtu3_channel *ch, u16 off);
+u16 rz_mtu3_16bit_ch_read(struct rz_mtu3_channel *ch, u16 off);
+u32 rz_mtu3_32bit_ch_read(struct rz_mtu3_channel *ch, u16 off);
+
+void rz_mtu3_8bit_ch_write(struct rz_mtu3_channel *ch, u16 off, u8 val);
+void rz_mtu3_16bit_ch_write(struct rz_mtu3_channel *ch, u16 off, u16 val);
+void rz_mtu3_32bit_ch_write(struct rz_mtu3_channel *ch, u16 off, u32 val);
+void rz_mtu3_shared_reg_write(struct rz_mtu3_channel *ch, u16 off, u16 val);
+#else
+static inline bool rz_mtu3_is_enabled(struct rz_mtu3_channel *ch)
+{
+ return false;
+}
+
+static inline void rz_mtu3_disable(struct rz_mtu3_channel *ch)
+{
+}
+
+static inline int rz_mtu3_enable(struct rz_mtu3_channel *ch)
+{
+ return 0;
+}
+
+static inline u8 rz_mtu3_8bit_ch_read(struct rz_mtu3_channel *ch, u16 off)
+{
+ return 0;
+}
+
+static inline u16 rz_mtu3_16bit_ch_read(struct rz_mtu3_channel *ch, u16 off)
+{
+ return 0;
+}
+
+static inline u32 rz_mtu3_32bit_ch_read(struct rz_mtu3_channel *ch, u16 off)
+{
+ return 0;
+}
+
+static inline void rz_mtu3_8bit_ch_write(struct rz_mtu3_channel *ch, u16 off, u8 val)
+{
+}
+
+static inline void rz_mtu3_16bit_ch_write(struct rz_mtu3_channel *ch, u16 off, u16 val)
+{
+}
+
+static inline void rz_mtu3_32bit_ch_write(struct rz_mtu3_channel *ch, u16 off, u32 val)
+{
+}
+
+static inline void rz_mtu3_shared_reg_write(struct rz_mtu3_channel *ch, u16 off, u16 val)
+{
+}
+#endif
+
+#if IS_ENABLED(CONFIG_MFD_RZ_MTU3_PWM)
+int rz_mtu3_pwm_probe(struct platform_device *pdev);
+#else
+static inline int rz_mtu3_pwm_probe(struct platform_device *pdev)
+{
+ return -ENODEV;
+}
+#endif
+
+#if IS_ENABLED(CONFIG_MFD_RZ_MTU3_CNT)
+int rz_mtu3_cnt_probe(struct platform_device *pdev);
+#else
+static inline int rz_mtu3_cnt_probe(struct platform_device *pdev)
+{
+ return -ENODEV;
+}
+#endif
+
+#endif /* __LINUX_RZ_MTU3_H__ */
--
2.25.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v4 3/4] mfd: Add RZ/G2L MTU3 counter driver
2022-10-10 14:52 [PATCH v4 0/4] Add RZ/G2L MTU3a MFD, Counter and pwm driver Biju Das
2022-10-10 14:52 ` [PATCH v4 1/4] dt-bindings: mfd: Document RZ/G2L MTU3a bindings Biju Das
2022-10-10 14:52 ` [PATCH v4 2/4] mfd: Add RZ/G2L MTU3 driver Biju Das
@ 2022-10-10 14:52 ` Biju Das
2022-10-17 16:40 ` William Breathitt Gray
2022-10-25 13:50 ` Geert Uytterhoeven
2022-10-10 14:52 ` [PATCH v4 4/4] mfd: Add RZ/G2L MTU3 PWM driver Biju Das
3 siblings, 2 replies; 32+ messages in thread
From: Biju Das @ 2022-10-10 14:52 UTC (permalink / raw)
To: William Breathitt Gray, Philipp Zabel
Cc: Biju Das, Lee Jones, linux-iio, Geert Uytterhoeven,
Chris Paterson, Biju Das, Prabhakar Mahadev Lad,
linux-renesas-soc
Add RZ/G2L MTU3 counter driver. This IP supports the following
phase counting modes on MTU1 and MTU2 channels
1) 16-bit phase counting modes on MTU1 and MTU2 channels.
2) 32-bit phase counting mode by cascading MTU1 and MTU2.
This patch adds 3 counters by creating 3 logical channels
counter0: 16-bit phase counter on MTU1 channel
counter1: 16-bit phase counter on MTU2 channel
counter2: 32-bit phase counter by cascading MTU1 and MTU2
channels.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v3->v4:
* There is no resource associated with "rz-mtu3-counter" compatible
and moved the code to mfd subsystem as it binds against "rz-mtu".
* Removed struct platform_driver rz_mtu3_cnt_driver.
* Updated commit description
* Updated Kconfig description
* Added macros RZ_MTU3_16_BIT_MTU{1,2}_CH for MTU1 and MTU2 channels
* Added RZ_MTU3_GET_HW_CH macro for getting channel ID.
* replaced priv->ch[id]->priv->ch[0] in rz_mtu3_count_read()
* Cached counter max values
* replaced cnt->tsr in rz_mtu3_count_direction_read()
* Added comments for RZ_MTU3_TCR_CCLR_NONE
* Replaced if with switch in rz_mtu3_initialize_counter() and
rz_mtu3_count_ceiling_write()
* Added locks in initialize, terminate and enable_read to prevent races.
* Updated rz_mtu3_action_read to take care of MTU2 signals.
* Added separate distinct array for each group of Synapse.
* Moved pm handling to parent.
v1->v3:
* Modelled as a counter device supporting 3 counters(2 16-bit and
32-bit)
* Add kernel-doc comments to document struct rz_mtu3_cnt
* Removed mmio variable from struct rz_mtu3_cnt
* Removed cnt local variable from rz_mtu3_count_read()
* Replaced -EINVAL->-ERANGE for out of range error conditions.
* Removed explicit cast from write functions.
* Removed local variable val from rz_mtu3_count_ceiling_read()
* Added lock for RMW for counter/ceiling updates.
* Added different synapses for counter0 and counter{1,2}
* Used ARRAY for assigning num_counts.
* Added PM runtime for managing clocks.
* Add MODULE_IMPORT_NS(COUNTER) to import the COUNTER namespace.
---
drivers/mfd/Kconfig | 8 +
drivers/mfd/Makefile | 1 +
drivers/mfd/rz-mtu3-cnt.c | 554 ++++++++++++++++++++++++++++++++++++++
3 files changed, 563 insertions(+)
create mode 100644 drivers/mfd/rz-mtu3-cnt.c
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 7329971a3bdf..fa88056224c9 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1986,6 +1986,14 @@ config MFD_RZ_MTU3
To compile this driver as a module, choose M here: the
module will be called rz-mtu3.
+config MFD_RZ_MTU3_CNT
+ tristate "RZ/G2L MTU3 counter driver"
+ depends on MFD_RZ_MTU3 || COMPILE_TEST
+ help
+ Enable support for MTU3 counter driver found on Renesas RZ/G2L alike
+ SoCs. This IP supports both 16-bit and 32-bit phase counting mode
+ support.
+
config MFD_STM32_LPTIMER
tristate "Support for STM32 Low-Power Timer"
depends on (ARCH_STM32 && OF) || COMPILE_TEST
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index cd492903411c..9d0d1fd22f99 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -252,6 +252,7 @@ obj-$(CONFIG_MFD_STPMIC1) += stpmic1.o
obj-$(CONFIG_MFD_SUN4I_GPADC) += sun4i-gpadc.o
rz-mtu3-objs := rz-mtu3-core.o
+rz-mtu3-$(CONFIG_MFD_RZ_MTU3_CNT) += rz-mtu3-cnt.o
obj-$(CONFIG_MFD_RZ_MTU3) += rz-mtu3.o
obj-$(CONFIG_MFD_STM32_LPTIMER) += stm32-lptimer.o
obj-$(CONFIG_MFD_STM32_TIMERS) += stm32-timers.o
diff --git a/drivers/mfd/rz-mtu3-cnt.c b/drivers/mfd/rz-mtu3-cnt.c
new file mode 100644
index 000000000000..e4aef25be462
--- /dev/null
+++ b/drivers/mfd/rz-mtu3-cnt.c
@@ -0,0 +1,554 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Renesas RZ/G2L MTU3a Counter driver
+ *
+ * Copyright (C) 2022 Renesas Electronics Corporation
+ */
+#include <linux/counter.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/types.h>
+
+#include "rz-mtu3.h"
+
+#define RZ_MTU3_TSR_TCFD BIT(7)
+#define RZ_MTU3_MAX_HW_CNTR_CHANNELS (2)
+
+#define RZ_MTU3_TMDR1_PH_CNT_MODE_1 (4)
+#define RZ_MTU3_TMDR1_PH_CNT_MODE_2 (5)
+#define RZ_MTU3_TMDR1_PH_CNT_MODE_3 (6)
+#define RZ_MTU3_TMDR1_PH_CNT_MODE_4 (7)
+#define RZ_MTU3_TMDR1_PH_CNT_MODE_5 (9)
+#define RZ_MTU3_TMDR1_PH_CNT_MODE_MASK (0xf)
+
+#define RZ_MTU3_TCR_CCLR GENMASK(7, 5)
+#define RZ_MTU3_TCR_CCLR_NONE FIELD_PREP(RZ_MTU3_TCR_CCLR, 0)
+
+#define RZ_MTU3_TMDR3_LWA BIT(0)
+#define RZ_MTU3_16_BIT_MTU1_CH (0)
+#define RZ_MTU3_16_BIT_MTU2_CH (1)
+#define RZ_MTU3_32_BIT_CH (2)
+
+#define RZ_MTU3_TIOR_IC_BOTH (10)
+
+#define RZ_MTU3_GET_HW_CH(id) (((id) == RZ_MTU3_32_BIT_CH) ? 0 : (id))
+
+/**
+ * struct rz_mtu3_cnt - MTU3 counter private data
+ *
+ * @lock: Lock to prevent concurrent access for ceiling and count
+ * @mtu_16bit_max: Cache for 16-bit counters
+ * @mtu_32bit_max: Cache for 32-bit counters
+ * @rz_mtu3_channel: HW channels for the counters
+ */
+struct rz_mtu3_cnt {
+ struct mutex lock;
+ u16 mtu_16bit_max[RZ_MTU3_MAX_HW_CNTR_CHANNELS];
+ u32 mtu_32bit_max;
+ struct rz_mtu3_channel *ch[RZ_MTU3_MAX_HW_CNTR_CHANNELS];
+};
+
+static const enum counter_function rz_mtu3_count_functions[] = {
+ COUNTER_FUNCTION_QUADRATURE_X4,
+ COUNTER_FUNCTION_PULSE_DIRECTION,
+ COUNTER_FUNCTION_QUADRATURE_X2_B,
+};
+
+static int rz_mtu3_count_read(struct counter_device *counter,
+ struct counter_count *count, u64 *val)
+{
+ struct rz_mtu3_cnt *const priv = counter_priv(counter);
+
+ if (count->id == RZ_MTU3_32_BIT_CH)
+ *val = rz_mtu3_32bit_ch_read(priv->ch[0], RZ_MTU3_TCNTLW);
+ else
+ *val = rz_mtu3_16bit_ch_read(priv->ch[count->id], RZ_MTU3_TCNT);
+
+ return 0;
+}
+
+static int rz_mtu3_count_write(struct counter_device *counter,
+ struct counter_count *count, const u64 val)
+{
+ struct rz_mtu3_cnt *const priv = counter_priv(counter);
+ const size_t ch_id = RZ_MTU3_GET_HW_CH(count->id);
+ u32 ceiling;
+
+ mutex_lock(&priv->lock);
+ if (count->id == RZ_MTU3_32_BIT_CH)
+ ceiling = priv->mtu_32bit_max;
+ else
+ ceiling = priv->mtu_16bit_max[ch_id];
+
+ if (val > ceiling) {
+ mutex_unlock(&priv->lock);
+ return -ERANGE;
+ }
+
+ if (count->id == RZ_MTU3_32_BIT_CH)
+ rz_mtu3_32bit_ch_write(priv->ch[0], RZ_MTU3_TCNTLW, val);
+ else
+ rz_mtu3_16bit_ch_write(priv->ch[ch_id], RZ_MTU3_TCNT, val);
+
+ mutex_unlock(&priv->lock);
+
+ return 0;
+}
+
+static int rz_mtu3_count_function_read(struct counter_device *counter,
+ struct counter_count *count,
+ enum counter_function *function)
+{
+ struct rz_mtu3_cnt *const priv = counter_priv(counter);
+ const size_t ch_id = RZ_MTU3_GET_HW_CH(count->id);
+ u8 val;
+
+ val = rz_mtu3_8bit_ch_read(priv->ch[ch_id], RZ_MTU3_TMDR1);
+
+ switch (val & RZ_MTU3_TMDR1_PH_CNT_MODE_MASK) {
+ case RZ_MTU3_TMDR1_PH_CNT_MODE_1:
+ *function = COUNTER_FUNCTION_QUADRATURE_X4;
+ break;
+ case RZ_MTU3_TMDR1_PH_CNT_MODE_2:
+ *function = COUNTER_FUNCTION_PULSE_DIRECTION;
+ break;
+ case RZ_MTU3_TMDR1_PH_CNT_MODE_4:
+ *function = COUNTER_FUNCTION_QUADRATURE_X2_B;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int rz_mtu3_count_function_write(struct counter_device *counter,
+ struct counter_count *count,
+ enum counter_function function)
+{
+ struct rz_mtu3_cnt *const priv = counter_priv(counter);
+ const size_t ch_id = RZ_MTU3_GET_HW_CH(count->id);
+ u8 mode;
+
+ switch (function) {
+ case COUNTER_FUNCTION_QUADRATURE_X4:
+ mode = RZ_MTU3_TMDR1_PH_CNT_MODE_1;
+ break;
+ case COUNTER_FUNCTION_PULSE_DIRECTION:
+ mode = RZ_MTU3_TMDR1_PH_CNT_MODE_2;
+ break;
+ case COUNTER_FUNCTION_QUADRATURE_X2_B:
+ mode = RZ_MTU3_TMDR1_PH_CNT_MODE_4;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ rz_mtu3_8bit_ch_write(priv->ch[ch_id], RZ_MTU3_TMDR1, mode);
+
+ return 0;
+}
+
+static int rz_mtu3_count_direction_read(struct counter_device *counter,
+ struct counter_count *count,
+ enum counter_count_direction *direction)
+{
+ struct rz_mtu3_cnt *const priv = counter_priv(counter);
+ const size_t ch_id = RZ_MTU3_GET_HW_CH(count->id);
+ u8 tsr;
+
+ tsr = rz_mtu3_8bit_ch_read(priv->ch[ch_id], RZ_MTU3_TSR);
+
+ if (tsr & RZ_MTU3_TSR_TCFD)
+ *direction = COUNTER_COUNT_DIRECTION_FORWARD;
+ else
+ *direction = COUNTER_COUNT_DIRECTION_BACKWARD;
+
+ return 0;
+}
+
+static int rz_mtu3_count_ceiling_read(struct counter_device *counter,
+ struct counter_count *count,
+ u64 *ceiling)
+{
+ struct rz_mtu3_cnt *const priv = counter_priv(counter);
+ const size_t ch_id = RZ_MTU3_GET_HW_CH(count->id);
+
+ if (count->id == RZ_MTU3_32_BIT_CH)
+ *ceiling = priv->mtu_32bit_max;
+ else
+ *ceiling = priv->mtu_16bit_max[ch_id];
+
+ return 0;
+}
+
+static int rz_mtu3_count_ceiling_write(struct counter_device *counter,
+ struct counter_count *count,
+ u64 ceiling)
+{
+ struct rz_mtu3_cnt *const priv = counter_priv(counter);
+ const size_t ch_id = RZ_MTU3_GET_HW_CH(count->id);
+
+ switch (count->id) {
+ case RZ_MTU3_16_BIT_MTU1_CH:
+ case RZ_MTU3_16_BIT_MTU2_CH:
+ if (ceiling > U16_MAX)
+ return -ERANGE;
+ priv->mtu_16bit_max[ch_id] = ceiling;
+ break;
+ case RZ_MTU3_32_BIT_CH:
+ if (ceiling > U32_MAX)
+ return -ERANGE;
+ priv->mtu_32bit_max = ceiling;
+ break;
+ }
+
+ mutex_lock(&priv->lock);
+ if (ceiling == 0) {
+ /* Disable counter clear source */
+ rz_mtu3_8bit_ch_write(priv->ch[ch_id], RZ_MTU3_TCR,
+ RZ_MTU3_TCR_CCLR_NONE);
+ } else {
+ if (count->id == RZ_MTU3_32_BIT_CH)
+ rz_mtu3_32bit_ch_write(priv->ch[ch_id], RZ_MTU3_TGRALW,
+ ceiling);
+ else
+ rz_mtu3_16bit_ch_write(priv->ch[ch_id], RZ_MTU3_TGRA,
+ ceiling);
+
+ rz_mtu3_8bit_ch_write(priv->ch[ch_id], RZ_MTU3_TCR,
+ RZ_MTU3_TCR_CCLR_TGRA);
+ }
+ mutex_unlock(&priv->lock);
+
+ return 0;
+}
+
+static void rz_mtu3_32bit_cnt_setting(struct counter_device *counter, int id)
+{
+ struct rz_mtu3_cnt *const priv = counter_priv(counter);
+
+ /*
+ * 32-bit phase counting need MTU1 and MTU2 to create 32-bit cascade
+ * counter.
+ */
+ priv->ch[0]->function = RZ_MTU3_32BIT_PHASE_COUNTING;
+ priv->ch[1]->function = RZ_MTU3_32BIT_PHASE_COUNTING;
+
+ rz_mtu3_shared_reg_write(priv->ch[0], RZ_MTU3_TMDR3, RZ_MTU3_TMDR3_LWA);
+
+ /* Phase counting mode 1 is used as default in initialization. */
+ rz_mtu3_8bit_ch_write(priv->ch[0], RZ_MTU3_TMDR1,
+ RZ_MTU3_TMDR1_PH_CNT_MODE_1);
+
+ rz_mtu3_8bit_ch_write(priv->ch[0], RZ_MTU3_TCR, RZ_MTU3_TCR_CCLR_TGRA);
+ rz_mtu3_8bit_ch_write(priv->ch[0], RZ_MTU3_TIOR, RZ_MTU3_TIOR_IC_BOTH);
+
+ rz_mtu3_enable(priv->ch[0]);
+ rz_mtu3_enable(priv->ch[1]);
+}
+
+static void rz_mtu3_16bit_cnt_setting(struct counter_device *counter, int id)
+{
+ struct rz_mtu3_cnt *const priv = counter_priv(counter);
+
+ priv->ch[id]->function = RZ_MTU3_16BIT_PHASE_COUNTING;
+
+ rz_mtu3_shared_reg_write(priv->ch[0], RZ_MTU3_TMDR3, 0);
+ /* Phase counting mode 1 is used as default in initialization. */
+ rz_mtu3_8bit_ch_write(priv->ch[id], RZ_MTU3_TMDR1,
+ RZ_MTU3_TMDR1_PH_CNT_MODE_1);
+
+ rz_mtu3_8bit_ch_write(priv->ch[id], RZ_MTU3_TCR, RZ_MTU3_TCR_CCLR_TGRA);
+ rz_mtu3_enable(priv->ch[id]);
+}
+
+static int rz_mtu3_initialize_counter(struct counter_device *counter, int id)
+{
+ struct rz_mtu3_cnt *const priv = counter_priv(counter);
+
+ mutex_lock(&priv->lock);
+ switch (id) {
+ case RZ_MTU3_16_BIT_MTU1_CH:
+ case RZ_MTU3_16_BIT_MTU2_CH:
+ if (priv->ch[id]->function != RZ_MTU3_NORMAL) {
+ mutex_unlock(&priv->lock);
+ return -EBUSY;
+ }
+
+ rz_mtu3_16bit_cnt_setting(counter, id);
+ break;
+ case RZ_MTU3_32_BIT_CH:
+ if (priv->ch[0]->function != RZ_MTU3_NORMAL ||
+ priv->ch[1]->function != RZ_MTU3_NORMAL) {
+ mutex_unlock(&priv->lock);
+ return -EBUSY;
+ }
+ rz_mtu3_32bit_cnt_setting(counter, id);
+ break;
+ }
+ mutex_unlock(&priv->lock);
+
+ return 0;
+}
+
+static void rz_mtu3_terminate_counter(struct counter_device *counter, int id)
+{
+ struct rz_mtu3_cnt *const priv = counter_priv(counter);
+
+ mutex_lock(&priv->lock);
+ if (id == RZ_MTU3_32_BIT_CH) {
+ priv->ch[0]->function = RZ_MTU3_NORMAL;
+ priv->ch[1]->function = RZ_MTU3_NORMAL;
+ rz_mtu3_shared_reg_write(priv->ch[0], RZ_MTU3_TMDR3, 0);
+ rz_mtu3_disable(priv->ch[1]);
+ rz_mtu3_disable(priv->ch[0]);
+ } else {
+ priv->ch[id]->function = RZ_MTU3_NORMAL;
+ rz_mtu3_disable(priv->ch[id]);
+ }
+ mutex_unlock(&priv->lock);
+}
+
+static int rz_mtu3_count_enable_read(struct counter_device *counter,
+ struct counter_count *count, u8 *enable)
+{
+ struct rz_mtu3_cnt *const priv = counter_priv(counter);
+
+ if (count->id == RZ_MTU3_32_BIT_CH) {
+ mutex_lock(&priv->lock);
+ *enable = rz_mtu3_is_enabled(priv->ch[0]) &&
+ rz_mtu3_is_enabled(priv->ch[1]);
+ mutex_unlock(&priv->lock);
+ } else {
+ *enable = rz_mtu3_is_enabled(priv->ch[count->id]);
+ }
+
+ return 0;
+}
+
+static int rz_mtu3_count_enable_write(struct counter_device *counter,
+ struct counter_count *count, u8 enable)
+{
+ struct rz_mtu3_cnt *const priv = counter_priv(counter);
+ const size_t ch_id = RZ_MTU3_GET_HW_CH(count->id);
+ struct rz_mtu3_channel *ch = priv->ch[ch_id];
+ int ret = 0;
+
+ if (enable) {
+ pm_runtime_get_sync(ch->dev);
+ ret = rz_mtu3_initialize_counter(counter, count->id);
+ } else {
+ rz_mtu3_terminate_counter(counter, count->id);
+ pm_runtime_put(ch->dev);
+ }
+
+ return ret;
+}
+
+static struct counter_comp rz_mtu3_count_ext[] = {
+ COUNTER_COMP_DIRECTION(rz_mtu3_count_direction_read),
+ COUNTER_COMP_ENABLE(rz_mtu3_count_enable_read,
+ rz_mtu3_count_enable_write),
+ COUNTER_COMP_CEILING(rz_mtu3_count_ceiling_read,
+ rz_mtu3_count_ceiling_write),
+};
+
+static const enum counter_synapse_action rz_mtu3_synapse_actions[] = {
+ COUNTER_SYNAPSE_ACTION_BOTH_EDGES,
+ COUNTER_SYNAPSE_ACTION_RISING_EDGE,
+ COUNTER_SYNAPSE_ACTION_NONE,
+};
+
+static int rz_mtu3_action_read(struct counter_device *counter,
+ struct counter_count *count,
+ struct counter_synapse *synapse,
+ enum counter_synapse_action *action)
+{
+ const size_t signal_a_id = count->synapses[0].signal->id;
+ const size_t signal_b_id = count->synapses[1].signal->id;
+ size_t signal_c_id;
+ size_t signal_d_id;
+ enum counter_function function;
+ int err;
+
+ if (count->id != RZ_MTU3_16_BIT_MTU1_CH) {
+ signal_c_id = count->synapses[2].signal->id;
+ signal_d_id = count->synapses[3].signal->id;
+ }
+
+ err = rz_mtu3_count_function_read(counter, count, &function);
+ if (err)
+ return err;
+
+ /* Default action mode */
+ *action = COUNTER_SYNAPSE_ACTION_NONE;
+
+ switch (function) {
+ case COUNTER_FUNCTION_PULSE_DIRECTION:
+ /*
+ * Rising edges on signal A updates the respective count.
+ * The input level of signal B determines direction.
+ */
+ if (synapse->signal->id == signal_a_id ||
+ synapse->signal->id == signal_c_id)
+ *action = COUNTER_SYNAPSE_ACTION_RISING_EDGE;
+ break;
+ case COUNTER_FUNCTION_QUADRATURE_X2_B:
+ /*
+ * Any state transition on quadrature pair signal B updates
+ * the respective count.
+ */
+ if (synapse->signal->id == signal_b_id ||
+ synapse->signal->id == signal_d_id)
+ *action = COUNTER_SYNAPSE_ACTION_BOTH_EDGES;
+ break;
+ case COUNTER_FUNCTION_QUADRATURE_X4:
+ /* counts up/down on both edges of A and B signal*/
+ *action = COUNTER_SYNAPSE_ACTION_BOTH_EDGES;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static const struct counter_ops rz_mtu3_cnt_ops = {
+ .count_read = rz_mtu3_count_read,
+ .count_write = rz_mtu3_count_write,
+ .function_read = rz_mtu3_count_function_read,
+ .function_write = rz_mtu3_count_function_write,
+ .action_read = rz_mtu3_action_read,
+};
+
+#define RZ_MTU3_PHASE_SIGNAL(_id, _name) { \
+ .id = (_id), \
+ .name = (_name), \
+}
+
+static struct counter_signal rz_mtu3_signals[] = {
+ RZ_MTU3_PHASE_SIGNAL(0, "MTU1 MTCLKA"),
+ RZ_MTU3_PHASE_SIGNAL(1, "MTU1 MTCLKB"),
+ RZ_MTU3_PHASE_SIGNAL(2, "MTU2 MTCLKC"),
+ RZ_MTU3_PHASE_SIGNAL(3, "MTU2 MTCLKD"),
+};
+
+static struct counter_synapse rz_mtu3_mtu1_count_synapses[] = {
+ {
+ .actions_list = rz_mtu3_synapse_actions,
+ .num_actions = ARRAY_SIZE(rz_mtu3_synapse_actions),
+ .signal = rz_mtu3_signals,
+ },
+ {
+ .actions_list = rz_mtu3_synapse_actions,
+ .num_actions = ARRAY_SIZE(rz_mtu3_synapse_actions),
+ .signal = rz_mtu3_signals + 1,
+ }
+};
+
+static struct counter_synapse rz_mtu3_mtu2_count_synapses[] = {
+ {
+ .actions_list = rz_mtu3_synapse_actions,
+ .num_actions = ARRAY_SIZE(rz_mtu3_synapse_actions),
+ .signal = rz_mtu3_signals,
+ },
+ {
+ .actions_list = rz_mtu3_synapse_actions,
+ .num_actions = ARRAY_SIZE(rz_mtu3_synapse_actions),
+ .signal = rz_mtu3_signals + 1,
+ },
+ {
+ .actions_list = rz_mtu3_synapse_actions,
+ .num_actions = ARRAY_SIZE(rz_mtu3_synapse_actions),
+ .signal = rz_mtu3_signals + 2,
+ },
+ {
+ .actions_list = rz_mtu3_synapse_actions,
+ .num_actions = ARRAY_SIZE(rz_mtu3_synapse_actions),
+ .signal = rz_mtu3_signals + 3,
+ }
+};
+
+static struct counter_count rz_mtu3_counts[] = {
+ {
+ .id = RZ_MTU3_16_BIT_MTU1_CH,
+ .name = "Channel 1 phase counter",
+ .functions_list = rz_mtu3_count_functions,
+ .num_functions = ARRAY_SIZE(rz_mtu3_count_functions),
+ .synapses = rz_mtu3_mtu1_count_synapses,
+ .num_synapses = ARRAY_SIZE(rz_mtu3_mtu1_count_synapses),
+ .ext = rz_mtu3_count_ext,
+ .num_ext = ARRAY_SIZE(rz_mtu3_count_ext),
+ },
+ {
+ .id = RZ_MTU3_16_BIT_MTU2_CH,
+ .name = "Channel 2 phase counter",
+ .functions_list = rz_mtu3_count_functions,
+ .num_functions = ARRAY_SIZE(rz_mtu3_count_functions),
+ .synapses = rz_mtu3_mtu2_count_synapses,
+ .num_synapses = ARRAY_SIZE(rz_mtu3_mtu2_count_synapses),
+ .ext = rz_mtu3_count_ext,
+ .num_ext = ARRAY_SIZE(rz_mtu3_count_ext),
+ },
+ {
+ .id = RZ_MTU3_32_BIT_CH,
+ .name = "Channel 3 Cascaded phase counter",
+ .functions_list = rz_mtu3_count_functions,
+ .num_functions = ARRAY_SIZE(rz_mtu3_count_functions),
+ .synapses = rz_mtu3_mtu2_count_synapses,
+ .num_synapses = ARRAY_SIZE(rz_mtu3_mtu2_count_synapses),
+ .ext = rz_mtu3_count_ext,
+ .num_ext = ARRAY_SIZE(rz_mtu3_count_ext),
+ }
+};
+
+int rz_mtu3_cnt_probe(struct platform_device *pdev)
+{
+ struct rz_mtu3 *ddata = dev_get_drvdata(&pdev->dev);
+ struct device *dev = &pdev->dev;
+ struct counter_device *counter;
+ struct rz_mtu3_cnt *priv;
+ unsigned int i;
+ int ret;
+
+ counter = devm_counter_alloc(dev, sizeof(*priv));
+ if (!counter)
+ return -ENOMEM;
+
+ priv = counter_priv(counter);
+ ddata->cnt_priv = priv;
+
+ for (i = 0; i < RZ_MTU3_MAX_HW_CNTR_CHANNELS; i++) {
+ priv->ch[i] = &ddata->channels[RZ_MTU1 + i];
+ priv->ch[i]->dev = dev;
+ priv->mtu_16bit_max[i] = U16_MAX;
+ }
+
+ priv->mtu_32bit_max = U32_MAX;
+
+ mutex_init(&priv->lock);
+
+ counter->name = dev_name(dev);
+ counter->parent = dev;
+ counter->ops = &rz_mtu3_cnt_ops;
+ counter->counts = rz_mtu3_counts;
+ counter->num_counts = ARRAY_SIZE(rz_mtu3_counts);
+ counter->signals = rz_mtu3_signals;
+ counter->num_signals = ARRAY_SIZE(rz_mtu3_signals);
+
+ /* Register Counter device */
+ ret = devm_counter_add(dev, counter);
+ if (ret < 0)
+ dev_err_probe(dev, ret, "Failed to add counter\n");
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(rz_mtu3_cnt_probe);
+
+MODULE_AUTHOR("Biju Das <biju.das.jz@bp.renesas.com>");
+MODULE_DESCRIPTION("Renesas RZ/G2L MTU3a counter driver");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(COUNTER);
--
2.25.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v4 4/4] mfd: Add RZ/G2L MTU3 PWM driver
2022-10-10 14:52 [PATCH v4 0/4] Add RZ/G2L MTU3a MFD, Counter and pwm driver Biju Das
` (2 preceding siblings ...)
2022-10-10 14:52 ` [PATCH v4 3/4] mfd: Add RZ/G2L MTU3 counter driver Biju Das
@ 2022-10-10 14:52 ` Biju Das
2022-10-11 18:53 ` Krzysztof Kozlowski
2022-10-25 14:05 ` Geert Uytterhoeven
3 siblings, 2 replies; 32+ messages in thread
From: Biju Das @ 2022-10-10 14:52 UTC (permalink / raw)
To: Thierry Reding, Philipp Zabel
Cc: Biju Das, Lee Jones, Uwe Kleine-König, linux-pwm,
Geert Uytterhoeven, Chris Paterson, Biju Das,
Prabhakar Mahadev Lad, linux-renesas-soc
Add support for RZ/G2L MTU3 PWM driver. The IP supports
following PWM modes
1) PWM mode{1,2}
2) Reset-synchronized PWM mode
3) Complementary PWM mode{1,2,3}
This patch adds basic pwm mode 1 support for RZ/G2L MTU3 driver
by creating separate logical channels for each IOs.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v3->v4:
* There is no resource associated with "rz-mtu3-pwm" compatible
and moved the code to mfd subsystem as it binds against "rz-mtu".
* Removed struct platform_driver rz_mtu3_pwm_driver.
v2->v3:
* No change.
v1->v2:
* Modelled as a single PWM device handling multiple channles.
* Used PM framework to manage the clocks.
---
drivers/mfd/Kconfig | 6 +
drivers/mfd/Makefile | 2 +
drivers/mfd/rz-mtu3-pwm.c | 405 ++++++++++++++++++++++++++++++++++++++
3 files changed, 413 insertions(+)
create mode 100644 drivers/mfd/rz-mtu3-pwm.c
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index fa88056224c9..1a0208236adc 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1994,6 +1994,12 @@ config MFD_RZ_MTU3_CNT
SoCs. This IP supports both 16-bit and 32-bit phase counting mode
support.
+config MFD_RZ_MTU3_PWM
+ tristate "Renesas RZ/G2L MTU3 PWM Timer support"
+ depends on MFD_RZ_MTU3
+ help
+ Enable support for RZ/G2L MTU3 PWM Timer controller.
+
config MFD_STM32_LPTIMER
tristate "Support for STM32 Low-Power Timer"
depends on (ARCH_STM32 && OF) || COMPILE_TEST
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 9d0d1fd22f99..50c7467f7c79 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -253,7 +253,9 @@ obj-$(CONFIG_MFD_SUN4I_GPADC) += sun4i-gpadc.o
rz-mtu3-objs := rz-mtu3-core.o
rz-mtu3-$(CONFIG_MFD_RZ_MTU3_CNT) += rz-mtu3-cnt.o
+rz-mtu3-$(CONFIG_MFD_RZ_MTU3_PWM) += rz-mtu3-pwm.o
obj-$(CONFIG_MFD_RZ_MTU3) += rz-mtu3.o
+
obj-$(CONFIG_MFD_STM32_LPTIMER) += stm32-lptimer.o
obj-$(CONFIG_MFD_STM32_TIMERS) += stm32-timers.o
obj-$(CONFIG_MFD_MXS_LRADC) += mxs-lradc.o
diff --git a/drivers/mfd/rz-mtu3-pwm.c b/drivers/mfd/rz-mtu3-pwm.c
new file mode 100644
index 000000000000..8aa67577e022
--- /dev/null
+++ b/drivers/mfd/rz-mtu3-pwm.c
@@ -0,0 +1,405 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Renesas RZ/G2L MTU3 PWM Timer driver
+ *
+ * Copyright (C) 2022 Renesas Electronics Corporation
+ *
+ * Hardware manual for this IP can be found here
+ * https://www.renesas.com/eu/en/document/mah/rzg2l-group-rzg2lc-group-users-manual-hardware-0?language=en
+ *
+ * Limitations:
+ * - When PWM is disabled, the output is driven to Hi-Z.
+ * - While the hardware supports both polarities, the driver (for now)
+ * only handles normal polarity.
+ * - While the hardware supports pwm mode{1,2}, reset-synchronized pwm and
+ * complementary pwm modes, the driver (for now) only handles pwm mode1.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/limits.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/pwm.h>
+#include <linux/time.h>
+
+#include "rz-mtu3.h"
+
+#define RZ_MTU3_TMDR1_MD_NORMAL (0)
+#define RZ_MTU3_TMDR1_MD_PWM_MODE_1 (2)
+
+#define RZ_MTU3_TIOR_OC_RETAIN (0)
+#define RZ_MTU3_TIOR_OC_0_H_COMP_MATCH (2)
+#define RZ_MTU3_TIOR_OC_1_TOGGLE (7)
+#define RZ_MTU3_TIOR_OC_IOA GENMASK(3, 0)
+
+#define RZ_MTU3_TCR_CCLR_TGRC (5 << 5)
+#define RZ_MTU3_TCR_CKEG_RISING (0 << 3)
+
+#define RZ_MTU3_TCR_TPCS GENMASK(2, 0)
+
+#define RZ_MTU3_MAX_PWM_MODE1_CHANNELS (12)
+
+#define RZ_MTU3_MAX_HW_PWM_CHANNELS (7)
+
+static const u8 rz_mtu3_pwm_mode1_num_ios[] = { 2, 1, 1, 2, 2, 2, 2 };
+
+/**
+ * struct rz_mtu3_pwm_chip - MTU3 pwm private data
+ *
+ * @chip: MTU3 pwm chip data
+ * @lock: Lock to prevent concurrent access for usage count
+ * @rate: MTU3 clock rate
+ * @user_count: MTU3 usage count
+ * @rz_mtu3_channel: HW channels for the PWM
+ */
+
+struct rz_mtu3_pwm_chip {
+ struct pwm_chip chip;
+ struct mutex lock;
+ unsigned long rate;
+ u32 user_count[RZ_MTU3_MAX_HW_PWM_CHANNELS];
+ struct rz_mtu3_channel *ch[RZ_MTU3_MAX_HW_PWM_CHANNELS];
+};
+
+static inline struct rz_mtu3_pwm_chip *to_rz_mtu3_pwm_chip(struct pwm_chip *chip)
+{
+ return container_of(chip, struct rz_mtu3_pwm_chip, chip);
+}
+
+static u8 rz_mtu3_pwm_calculate_prescale(struct rz_mtu3_pwm_chip *rz_mtu3,
+ u64 period_cycles)
+{
+ u32 prescaled_period_cycles;
+ u8 prescale;
+
+ prescaled_period_cycles = period_cycles >> 16;
+ if (prescaled_period_cycles >= 16)
+ prescale = 3;
+ else
+ prescale = (fls(prescaled_period_cycles) + 1) / 2;
+
+ return prescale;
+}
+
+static struct rz_mtu3_channel *
+rz_mtu3_get_hw_channel(struct rz_mtu3_pwm_chip *rz_mtu3_pwm, u32 channel)
+{
+ unsigned int i, ch_index = 0;
+
+ for (i = 0; i < ARRAY_SIZE(rz_mtu3_pwm_mode1_num_ios); i++) {
+ ch_index += rz_mtu3_pwm_mode1_num_ios[i];
+
+ if (ch_index > channel)
+ break;
+ }
+
+ return rz_mtu3_pwm->ch[i];
+}
+
+static u32 rz_mtu3_get_hw_channel_index(struct rz_mtu3_pwm_chip *rz_mtu3_pwm,
+ struct rz_mtu3_channel *ch)
+{
+ u32 i;
+
+ for (i = 0; i < ARRAY_SIZE(rz_mtu3_pwm_mode1_num_ios); i++) {
+ if (ch == rz_mtu3_pwm->ch[i])
+ break;
+ }
+
+ return i;
+}
+
+static bool rz_mtu3_pwm_is_second_channel(u32 ch_index, u32 hwpwm)
+{
+ u32 i, pwm_ch_index = 0;
+
+ for (i = 0; i < ch_index; i++)
+ pwm_ch_index += rz_mtu3_pwm_mode1_num_ios[i];
+
+ return pwm_ch_index != hwpwm;
+}
+
+static bool rz_mtu3_pwm_is_ch_enabled(struct rz_mtu3_pwm_chip *rz_mtu3_pwm,
+ u32 hwpwm)
+{
+ struct rz_mtu3_channel *ch;
+ bool is_channel_en;
+ u32 ch_index;
+ u8 val;
+
+ ch = rz_mtu3_get_hw_channel(rz_mtu3_pwm, hwpwm);
+ ch_index = rz_mtu3_get_hw_channel_index(rz_mtu3_pwm, ch);
+ is_channel_en = rz_mtu3_is_enabled(ch);
+
+ if (rz_mtu3_pwm_is_second_channel(ch_index, hwpwm))
+ val = rz_mtu3_8bit_ch_read(ch, RZ_MTU3_TIORL);
+ else
+ val = rz_mtu3_8bit_ch_read(ch, RZ_MTU3_TIORH);
+
+ return (is_channel_en && (val & RZ_MTU3_TIOR_OC_IOA));
+}
+
+static int rz_mtu3_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+ struct rz_mtu3_pwm_chip *rz_mtu3_pwm = to_rz_mtu3_pwm_chip(chip);
+ struct rz_mtu3_channel *ch;
+ u32 ch_index;
+
+ ch = rz_mtu3_get_hw_channel(rz_mtu3_pwm, pwm->hwpwm);
+ ch_index = rz_mtu3_get_hw_channel_index(rz_mtu3_pwm, ch);
+
+ mutex_lock(&rz_mtu3_pwm->lock);
+ rz_mtu3_pwm->user_count[ch_index]++;
+ mutex_unlock(&rz_mtu3_pwm->lock);
+
+ ch->function = RZ_MTU3_PWM_MODE_1;
+
+ return 0;
+}
+
+static void rz_mtu3_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+ struct rz_mtu3_pwm_chip *rz_mtu3_pwm = to_rz_mtu3_pwm_chip(chip);
+ struct rz_mtu3_channel *ch;
+ u32 ch_index;
+
+ ch = rz_mtu3_get_hw_channel(rz_mtu3_pwm, pwm->hwpwm);
+ ch_index = rz_mtu3_get_hw_channel_index(rz_mtu3_pwm, ch);
+
+ mutex_lock(&rz_mtu3_pwm->lock);
+ rz_mtu3_pwm->user_count[ch_index]--;
+ mutex_unlock(&rz_mtu3_pwm->lock);
+
+ if (!rz_mtu3_pwm->user_count[ch_index])
+ ch->function = RZ_MTU3_NORMAL;
+}
+
+static int rz_mtu3_pwm_enable(struct rz_mtu3_pwm_chip *rz_mtu3_pwm,
+ struct pwm_device *pwm)
+{
+ struct rz_mtu3_channel *ch;
+ u32 ch_index;
+ u8 val;
+
+ ch = rz_mtu3_get_hw_channel(rz_mtu3_pwm, pwm->hwpwm);
+ ch_index = rz_mtu3_get_hw_channel_index(rz_mtu3_pwm, ch);
+ val = (RZ_MTU3_TIOR_OC_1_TOGGLE << 4) | RZ_MTU3_TIOR_OC_0_H_COMP_MATCH;
+
+ rz_mtu3_8bit_ch_write(ch, RZ_MTU3_TMDR1, RZ_MTU3_TMDR1_MD_PWM_MODE_1);
+ if (rz_mtu3_pwm_is_second_channel(ch_index, pwm->hwpwm))
+ rz_mtu3_8bit_ch_write(ch, RZ_MTU3_TIORL, val);
+ else
+ rz_mtu3_8bit_ch_write(ch, RZ_MTU3_TIORH, val);
+
+ if (rz_mtu3_pwm->user_count[ch_index] <= 1)
+ rz_mtu3_enable(ch);
+
+ return 0;
+}
+
+static void rz_mtu3_pwm_disable(struct rz_mtu3_pwm_chip *rz_mtu3_pwm,
+ struct pwm_device *pwm)
+{
+ struct rz_mtu3_channel *ch;
+ u32 ch_index;
+
+ ch = rz_mtu3_get_hw_channel(rz_mtu3_pwm, pwm->hwpwm);
+ ch_index = rz_mtu3_get_hw_channel_index(rz_mtu3_pwm, ch);
+
+ /* Return to normal mode and disable output pins of MTU3 channel */
+ if (rz_mtu3_pwm->user_count[ch_index] <= 1)
+ rz_mtu3_8bit_ch_write(ch, RZ_MTU3_TMDR1, RZ_MTU3_TMDR1_MD_NORMAL);
+
+ if (rz_mtu3_pwm_is_second_channel(ch_index, pwm->hwpwm))
+ rz_mtu3_8bit_ch_write(ch, RZ_MTU3_TIORL, RZ_MTU3_TIOR_OC_RETAIN);
+ else
+ rz_mtu3_8bit_ch_write(ch, RZ_MTU3_TIORH, RZ_MTU3_TIOR_OC_RETAIN);
+
+ if (rz_mtu3_pwm->user_count[ch_index] <= 1)
+ rz_mtu3_disable(ch);
+}
+
+static int rz_mtu3_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
+ const struct pwm_state *state)
+{
+ struct rz_mtu3_pwm_chip *rz_mtu3_pwm = to_rz_mtu3_pwm_chip(chip);
+ struct rz_mtu3_channel *ch;
+ unsigned long pv, dc;
+ u64 period_cycles;
+ u64 duty_cycles;
+ u32 ch_index;
+ u8 prescale;
+ u8 val;
+
+ /*
+ * Refuse clk rates > 1 GHz to prevent overflowing the following
+ * calculation.
+ */
+ if (rz_mtu3_pwm->rate > NSEC_PER_SEC)
+ return -EINVAL;
+
+ ch = rz_mtu3_get_hw_channel(rz_mtu3_pwm, pwm->hwpwm);
+ ch_index = rz_mtu3_get_hw_channel_index(rz_mtu3_pwm, ch);
+ duty_cycles = state->duty_cycle;
+ if (!state->enabled)
+ duty_cycles = 0;
+
+ period_cycles = mul_u64_u32_div(state->period, rz_mtu3_pwm->rate,
+ NSEC_PER_SEC);
+ prescale = rz_mtu3_pwm_calculate_prescale(rz_mtu3_pwm, period_cycles);
+
+ if (period_cycles >> (2 * prescale) <= U16_MAX)
+ pv = period_cycles >> (2 * prescale);
+ else
+ pv = U16_MAX;
+
+ duty_cycles = mul_u64_u32_div(duty_cycles, rz_mtu3_pwm->rate,
+ NSEC_PER_SEC);
+ if (duty_cycles >> (2 * prescale) <= U16_MAX)
+ dc = duty_cycles >> (2 * prescale);
+ else
+ dc = U16_MAX;
+
+ val = RZ_MTU3_TCR_CKEG_RISING | prescale;
+ if (rz_mtu3_pwm_is_second_channel(ch_index, pwm->hwpwm)) {
+ rz_mtu3_8bit_ch_write(ch, RZ_MTU3_TCR,
+ RZ_MTU3_TCR_CCLR_TGRC | val);
+ rz_mtu3_16bit_ch_write(ch, RZ_MTU3_TGRD, dc);
+ rz_mtu3_16bit_ch_write(ch, RZ_MTU3_TGRC, pv);
+ } else {
+ rz_mtu3_8bit_ch_write(ch, RZ_MTU3_TCR,
+ RZ_MTU3_TCR_CCLR_TGRA | val);
+ rz_mtu3_16bit_ch_write(ch, RZ_MTU3_TGRB, dc);
+ rz_mtu3_16bit_ch_write(ch, RZ_MTU3_TGRA, pv);
+ }
+
+ return 0;
+}
+
+static void rz_mtu3_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
+ struct pwm_state *state)
+{
+ struct rz_mtu3_pwm_chip *rz_mtu3_pwm = to_rz_mtu3_pwm_chip(chip);
+ struct rz_mtu3_channel *ch;
+ u8 prescale, val;
+ u32 ch_index;
+ u16 dc, pv;
+ u64 tmp;
+
+ ch = rz_mtu3_get_hw_channel(rz_mtu3_pwm, pwm->hwpwm);
+ ch_index = rz_mtu3_get_hw_channel_index(rz_mtu3_pwm, ch);
+ pm_runtime_get_sync(chip->dev);
+ state->enabled = rz_mtu3_pwm_is_ch_enabled(rz_mtu3_pwm, pwm->hwpwm);
+ if (state->enabled) {
+ val = rz_mtu3_8bit_ch_read(ch, RZ_MTU3_TCR);
+ prescale = FIELD_GET(RZ_MTU3_TCR_TPCS, val);
+
+ if (rz_mtu3_pwm_is_second_channel(ch_index, pwm->hwpwm)) {
+ dc = rz_mtu3_16bit_ch_read(ch, RZ_MTU3_TGRD);
+ pv = rz_mtu3_16bit_ch_read(ch, RZ_MTU3_TGRC);
+ } else {
+ dc = rz_mtu3_16bit_ch_read(ch, RZ_MTU3_TGRB);
+ pv = rz_mtu3_16bit_ch_read(ch, RZ_MTU3_TGRA);
+ }
+
+ tmp = NSEC_PER_SEC * (u64)pv << (2 * prescale);
+ state->period = DIV_ROUND_UP_ULL(tmp, rz_mtu3_pwm->rate);
+
+ tmp = NSEC_PER_SEC * (u64)dc << (2 * prescale);
+ state->duty_cycle = DIV_ROUND_UP_ULL(tmp, rz_mtu3_pwm->rate);
+ }
+
+ state->polarity = PWM_POLARITY_NORMAL;
+ pm_runtime_put(chip->dev);
+}
+
+static int rz_mtu3_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
+ const struct pwm_state *state)
+{
+ struct rz_mtu3_pwm_chip *rz_mtu3_pwm = to_rz_mtu3_pwm_chip(chip);
+ struct pwm_state cur_state;
+ bool enabled;
+ int ret;
+
+ cur_state = pwm->state;
+ enabled = cur_state.enabled;
+ if (state->polarity != PWM_POLARITY_NORMAL)
+ return -EINVAL;
+
+ if (!enabled && state->enabled)
+ pm_runtime_get_sync(chip->dev);
+
+ ret = rz_mtu3_pwm_config(chip, pwm, state);
+ if (ret && state->enabled)
+ goto done;
+
+ if (!state->enabled) {
+ if (enabled)
+ rz_mtu3_pwm_disable(rz_mtu3_pwm, pwm);
+ ret = 0;
+ goto done;
+ }
+
+ return rz_mtu3_pwm_enable(rz_mtu3_pwm, pwm);
+done:
+ if (enabled && !state->enabled)
+ pm_runtime_put(chip->dev);
+
+ return ret;
+}
+
+static const struct pwm_ops rz_mtu3_pwm_ops = {
+ .request = rz_mtu3_pwm_request,
+ .free = rz_mtu3_pwm_free,
+ .get_state = rz_mtu3_pwm_get_state,
+ .apply = rz_mtu3_pwm_apply,
+ .owner = THIS_MODULE,
+};
+
+int rz_mtu3_pwm_probe(struct platform_device *pdev)
+{
+ struct rz_mtu3 *ddata = dev_get_drvdata(&pdev->dev);
+ struct rz_mtu3_pwm_chip *rz_mtu3_pwm;
+ struct device *dev = &pdev->dev;
+ int num_pwm_hw_ch;
+ unsigned int i;
+ int ret;
+
+ rz_mtu3_pwm = devm_kzalloc(&pdev->dev, sizeof(*rz_mtu3_pwm), GFP_KERNEL);
+ if (!rz_mtu3_pwm)
+ return -ENOMEM;
+
+ ddata->pwm_priv = rz_mtu3_pwm;
+ num_pwm_hw_ch = 0;
+ for (i = 0; i < RZ_MTU_NUM_CHANNELS; i++) {
+ if (i == RZ_MTU5 || i == RZ_MTU8)
+ continue;
+
+ rz_mtu3_pwm->ch[num_pwm_hw_ch] = &ddata->channels[i];
+ rz_mtu3_pwm->ch[num_pwm_hw_ch]->dev = dev;
+ num_pwm_hw_ch++;
+ }
+
+ rz_mtu3_pwm->rate = clk_get_rate(ddata->clk);
+
+ mutex_init(&rz_mtu3_pwm->lock);
+
+ rz_mtu3_pwm->chip.dev = &pdev->dev;
+ rz_mtu3_pwm->chip.ops = &rz_mtu3_pwm_ops;
+ rz_mtu3_pwm->chip.npwm = RZ_MTU3_MAX_PWM_MODE1_CHANNELS;
+
+ ret = devm_pwmchip_add(&pdev->dev, &rz_mtu3_pwm->chip);
+ if (ret)
+ dev_err_probe(&pdev->dev, ret, "failed to add PWM chip\n");
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(rz_mtu3_pwm_probe);
+
+MODULE_AUTHOR("Biju Das <biju.das.jz@bp.renesas.com>");
+MODULE_DESCRIPTION("Renesas RZ/G2L MTU3 PWM Timer Driver");
+MODULE_LICENSE("GPL");
--
2.25.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH v4 1/4] dt-bindings: mfd: Document RZ/G2L MTU3a bindings
2022-10-10 14:52 ` [PATCH v4 1/4] dt-bindings: mfd: Document RZ/G2L MTU3a bindings Biju Das
@ 2022-10-11 14:45 ` Krzysztof Kozlowski
2022-10-11 14:55 ` Biju Das
0 siblings, 1 reply; 32+ messages in thread
From: Krzysztof Kozlowski @ 2022-10-11 14:45 UTC (permalink / raw)
To: Biju Das, Rob Herring, Krzysztof Kozlowski,
William Breathitt Gray, Thierry Reding
Cc: Lee Jones, Uwe Kleine-König, devicetree, linux-pwm,
Geert Uytterhoeven, Chris Paterson, Biju Das,
Prabhakar Mahadev Lad, linux-renesas-soc
On 10/10/2022 10:52, Biju Das wrote:
> The RZ/G2L multi-function timer pulse unit 3 (MTU3a) is embedded in
> the Renesas RZ/G2L family SoC's. It consists of eight 16-bit timer
> channels and one 32-bit timer channel. It supports the following
> functions
> - Counter
> - Timer
> - PWM
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
> v3->v4:
> * Dropped counter and pwm compatibeles as they don't have any resources.
> * Made rz-mtu3 as pwm provider.
> * Updated the example and description.
> v2->v3:
> * Dropped counter bindings and integrated with mfd as it has only one property.
> * Removed "#address-cells" and "#size-cells" as it do not have children with
> unit addresses.
> * Removed quotes from counter and pwm.
> * Provided full path for pwm bindings.
> * Updated the example.
> v1->v2:
> * Modelled counter and pwm as a single device that handles
> multiple channels.
> * Moved counter and pwm bindings to respective subsystems
> * Dropped 'bindings' from MFD binding title.
> * Updated the example
> * Changed the compatible names.
> ---
> .../bindings/mfd/renesas,rz-mtu3.yaml | 305 ++++++++++++++++++
> 1 file changed, 305 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mfd/renesas,rz-mtu3.yaml
This should not be in MFD. Just because some device has few features,
does not mean it should go to MFD... Choose either timer or pwm.
>
> diff --git a/Documentation/devicetree/bindings/mfd/renesas,rz-mtu3.yaml b/Documentation/devicetree/bindings/mfd/renesas,rz-mtu3.yaml
> new file mode 100644
> index 000000000000..1b0be9f5cd18
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/renesas,rz-mtu3.yaml
> @@ -0,0 +1,305 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/renesas,rz-mtu3.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Renesas RZ/G2L Multi-Function Timer Pulse Unit 3 (MTU3a)
> +
> +maintainers:
> + - Biju Das <biju.das.jz@bp.renesas.com>
> +
> +description: |
> + This hardware block pconsisting of eight 16-bit timer channels and one
"This hardware block consists of..."
> + 32- bit timer channel. It supports the following specifications:
> + - Pulse input/output: 28 lines max.
> + - Pulse input 3 lines
> + - Count clock 11 clocks for each channel (14 clocks for MTU0, 12 clocks
> + for MTU2, and 10 clocks for MTU5, four clocks for MTU1-MTU2 combination
> + (when LWA = 1))
> + - Operating frequency Up to 100 MHz
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 32+ messages in thread
* RE: [PATCH v4 1/4] dt-bindings: mfd: Document RZ/G2L MTU3a bindings
2022-10-11 14:45 ` Krzysztof Kozlowski
@ 2022-10-11 14:55 ` Biju Das
2022-10-11 18:54 ` Krzysztof Kozlowski
2022-10-18 7:10 ` Lee Jones
0 siblings, 2 replies; 32+ messages in thread
From: Biju Das @ 2022-10-11 14:55 UTC (permalink / raw)
To: Krzysztof Kozlowski, Rob Herring, Krzysztof Kozlowski,
William Breathitt Gray, Thierry Reding
Cc: Lee Jones, Uwe Kleine-König, devicetree, linux-pwm,
Geert Uytterhoeven, Chris Paterson, Biju Das,
Prabhakar Mahadev Lad, linux-renesas-soc
Hi Krzysztof Kozlowski,
> Subject: Re: [PATCH v4 1/4] dt-bindings: mfd: Document RZ/G2L MTU3a
> bindings
>
> On 10/10/2022 10:52, Biju Das wrote:
> > The RZ/G2L multi-function timer pulse unit 3 (MTU3a) is embedded in
> > the Renesas RZ/G2L family SoC's. It consists of eight 16-bit timer
> > channels and one 32-bit timer channel. It supports the following
> > functions
> > - Counter
> > - Timer
> > - PWM
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > ---
> > v3->v4:
> > * Dropped counter and pwm compatibeles as they don't have any
> resources.
> > * Made rz-mtu3 as pwm provider.
> > * Updated the example and description.
> > v2->v3:
> > * Dropped counter bindings and integrated with mfd as it has only
> one property.
> > * Removed "#address-cells" and "#size-cells" as it do not have
> children with
> > unit addresses.
> > * Removed quotes from counter and pwm.
> > * Provided full path for pwm bindings.
> > * Updated the example.
> > v1->v2:
> > * Modelled counter and pwm as a single device that handles
> > multiple channels.
> > * Moved counter and pwm bindings to respective subsystems
> > * Dropped 'bindings' from MFD binding title.
> > * Updated the example
> > * Changed the compatible names.
> > ---
> > .../bindings/mfd/renesas,rz-mtu3.yaml | 305
> ++++++++++++++++++
> > 1 file changed, 305 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/mfd/renesas,rz-mtu3.yaml
>
> This should not be in MFD. Just because some device has few features,
> does not mean it should go to MFD... Choose either timer or pwm.
MFD is for multifunction device. This IP supports multiple functions
like timer, pwm, clock source/events. That is the reason I have added
here. MFD is core which provides register access for client devices.
For me moving it to pwm or counter is not a big problem.
Why do you think it cannot be MFD?
Cheers,
Biju
>
>
> >
> > diff --git
> > a/Documentation/devicetree/bindings/mfd/renesas,rz-mtu3.yaml
> > b/Documentation/devicetree/bindings/mfd/renesas,rz-mtu3.yaml
> > new file mode 100644
> > index 000000000000..1b0be9f5cd18
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mfd/renesas,rz-mtu3.yaml
> > @@ -0,0 +1,305 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> > +---
> > +$id:
> >
> > +title: Renesas RZ/G2L Multi-Function Timer Pulse Unit 3 (MTU3a)
> > +
> > +maintainers:
> > + - Biju Das <biju.das.jz@bp.renesas.com>
> > +
> > +description: |
> > + This hardware block pconsisting of eight 16-bit timer channels
> and
> > +one
>
> "This hardware block consists of..."
OK, will fix this in next version.
Cheers,
Biju
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v4 4/4] mfd: Add RZ/G2L MTU3 PWM driver
2022-10-10 14:52 ` [PATCH v4 4/4] mfd: Add RZ/G2L MTU3 PWM driver Biju Das
@ 2022-10-11 18:53 ` Krzysztof Kozlowski
2022-10-11 19:13 ` Biju Das
2022-10-25 14:05 ` Geert Uytterhoeven
1 sibling, 1 reply; 32+ messages in thread
From: Krzysztof Kozlowski @ 2022-10-11 18:53 UTC (permalink / raw)
To: Biju Das, Thierry Reding, Philipp Zabel
Cc: Lee Jones, Uwe Kleine-König, linux-pwm, Geert Uytterhoeven,
Chris Paterson, Biju Das, Prabhakar Mahadev Lad,
linux-renesas-soc
On 10/10/2022 10:52, Biju Das wrote:
> Add support for RZ/G2L MTU3 PWM driver. The IP supports
> following PWM modes
>
> 1) PWM mode{1,2}
> 2) Reset-synchronized PWM mode
> 3) Complementary PWM mode{1,2,3}
>
> This patch adds basic pwm mode 1 support for RZ/G2L MTU3 driver
> by creating separate logical channels for each IOs.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
> v3->v4:
> * There is no resource associated with "rz-mtu3-pwm" compatible
> and moved the code to mfd subsystem as it binds against "rz-mtu".
> * Removed struct platform_driver rz_mtu3_pwm_driver.
> v2->v3:
> * No change.
> v1->v2:
> * Modelled as a single PWM device handling multiple channles.
> * Used PM framework to manage the clocks.
> ---
> drivers/mfd/Kconfig | 6 +
> drivers/mfd/Makefile | 2 +
> drivers/mfd/rz-mtu3-pwm.c | 405 ++++++++++++++++++++++++++++++++++++++
That's not a MFD driver. That's a PWM. Use proper subsystem and email
prefix.
The same applies to your other patches.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v4 1/4] dt-bindings: mfd: Document RZ/G2L MTU3a bindings
2022-10-11 14:55 ` Biju Das
@ 2022-10-11 18:54 ` Krzysztof Kozlowski
2022-10-11 19:23 ` Biju Das
2022-10-18 7:10 ` Lee Jones
1 sibling, 1 reply; 32+ messages in thread
From: Krzysztof Kozlowski @ 2022-10-11 18:54 UTC (permalink / raw)
To: Biju Das, Rob Herring, Krzysztof Kozlowski,
William Breathitt Gray, Thierry Reding
Cc: Lee Jones, Uwe Kleine-König, devicetree, linux-pwm,
Geert Uytterhoeven, Chris Paterson, Biju Das,
Prabhakar Mahadev Lad, linux-renesas-soc
On 11/10/2022 10:55, Biju Das wrote:
>
>>> .../bindings/mfd/renesas,rz-mtu3.yaml | 305
>> ++++++++++++++++++
>>> 1 file changed, 305 insertions(+)
>>> create mode 100644
>>> Documentation/devicetree/bindings/mfd/renesas,rz-mtu3.yaml
>>
>> This should not be in MFD. Just because some device has few features,
>> does not mean it should go to MFD... Choose either timer or pwm.
>
> MFD is for multifunction device. This IP supports multiple functions
> like timer, pwm, clock source/events. That is the reason I have added
> here. MFD is core which provides register access for client devices.
>
> For me moving it to pwm or counter is not a big problem.
> Why do you think it cannot be MFD?
Because it makes MFD a dump for everything where author did not want to
think about real device aspects, but instead represented driver design
(MFD driver).
MFDs are pretty often combining unrelated features, e.g. PMICs which
have wakeup and system power control, regulator, 32 kHz clocks, RTC and
some USB connector.
Just because you will have clocksource driver, PWM driver and timer
driver does not make it a MFD.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 32+ messages in thread
* RE: [PATCH v4 4/4] mfd: Add RZ/G2L MTU3 PWM driver
2022-10-11 18:53 ` Krzysztof Kozlowski
@ 2022-10-11 19:13 ` Biju Das
2022-10-11 20:10 ` Krzysztof Kozlowski
0 siblings, 1 reply; 32+ messages in thread
From: Biju Das @ 2022-10-11 19:13 UTC (permalink / raw)
To: Krzysztof Kozlowski, Thierry Reding, Philipp Zabel
Cc: Lee Jones, Uwe Kleine-König, linux-pwm, Geert Uytterhoeven,
Chris Paterson, Biju Das, Prabhakar Mahadev Lad,
linux-renesas-soc
> Subject: Re: [PATCH v4 4/4] mfd: Add RZ/G2L MTU3 PWM driver
>
> On 10/10/2022 10:52, Biju Das wrote:
> > Add support for RZ/G2L MTU3 PWM driver. The IP supports following
> PWM
> > modes
> >
> > 1) PWM mode{1,2}
> > 2) Reset-synchronized PWM mode
> > 3) Complementary PWM mode{1,2,3}
> >
> > This patch adds basic pwm mode 1 support for RZ/G2L MTU3 driver by
> > creating separate logical channels for each IOs.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > ---
> > v3->v4:
> > * There is no resource associated with "rz-mtu3-pwm" compatible
> > and moved the code to mfd subsystem as it binds against "rz-mtu".
> > * Removed struct platform_driver rz_mtu3_pwm_driver.
> > v2->v3:
> > * No change.
> > v1->v2:
> > * Modelled as a single PWM device handling multiple channles.
> > * Used PM framework to manage the clocks.
> > ---
> > drivers/mfd/Kconfig | 6 +
> > drivers/mfd/Makefile | 2 +
> > drivers/mfd/rz-mtu3-pwm.c | 405
> > ++++++++++++++++++++++++++++++++++++++
>
> That's not a MFD driver. That's a PWM. Use proper subsystem and email
> prefix.
See [1]
[1] https://patchwork.kernel.org/project/linux-renesas-soc/patch/20221006135717.1748560-2-biju.das.jz@bp.renesas.com/
It is a single driver that binds against "renesas,rz-mtu3", and registers both the counter and the pwm
functionalities. Just like the clock driver, which registers clock, reset, and PM Domain functionalities.
It is same here, a single MFD driver which binds against ""renesas,rz-mtu3" and registers counter
And pwm functionalities.
rz-mtu-core is core driver which provides resources to child devices like counter and pwm.
I already copied PWM subsystem in the loop. Am I missing anything related to [1]
Cheers,
Biju
^ permalink raw reply [flat|nested] 32+ messages in thread
* RE: [PATCH v4 1/4] dt-bindings: mfd: Document RZ/G2L MTU3a bindings
2022-10-11 18:54 ` Krzysztof Kozlowski
@ 2022-10-11 19:23 ` Biju Das
2022-10-11 20:17 ` Krzysztof Kozlowski
0 siblings, 1 reply; 32+ messages in thread
From: Biju Das @ 2022-10-11 19:23 UTC (permalink / raw)
To: Krzysztof Kozlowski, Rob Herring, Krzysztof Kozlowski,
William Breathitt Gray, Thierry Reding
Cc: Lee Jones, Uwe Kleine-König, devicetree, linux-pwm,
Geert Uytterhoeven, Chris Paterson, Biju Das,
Prabhakar Mahadev Lad, linux-renesas-soc
> Subject: Re: [PATCH v4 1/4] dt-bindings: mfd: Document RZ/G2L MTU3a
> bindings
>
> On 11/10/2022 10:55, Biju Das wrote:
> >
> >>> .../bindings/mfd/renesas,rz-mtu3.yaml | 305
> >> ++++++++++++++++++
> >>> 1 file changed, 305 insertions(+)
> >>> create mode 100644
> >>> Documentation/devicetree/bindings/mfd/renesas,rz-mtu3.yaml
> >>
> >> This should not be in MFD. Just because some device has few
> features,
> >> does not mean it should go to MFD... Choose either timer or pwm.
> >
> > MFD is for multifunction device. This IP supports multiple functions
> > like timer, pwm, clock source/events. That is the reason I have
> added
> > here. MFD is core which provides register access for client devices.
> >
> > For me moving it to pwm or counter is not a big problem.
> > Why do you think it cannot be MFD?
>
>
> Because it makes MFD a dump for everything where author did not want
> to think about real device aspects, but instead represented driver
> design (MFD driver).
Core driver is MFD, just provides resources to child devices
and is not aware of any real device aspects.
>
> MFDs are pretty often combining unrelated features, e.g. PMICs which
> have wakeup and system power control, regulator, 32 kHz clocks, RTC
> and some USB connector.
Here also same right? pwm, counter and clock are 3 unrelated features.
That is the reason we have separate subsystems for these features.
>
> Just because you will have clocksource driver, PWM driver and timer
> driver does not make it a MFD.
MFD is multi function device. So are are you agreeing Clock source, PWM and
timer are different functionalities or not? If not, why do we have 3 subsystems,
if it is same?
Where do keep these bindings as there is only single "rz_mtu" bindings for these 3 different functionalities?
pwm or counter or mfd?
Please let me know.
Cheers,
Biju
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v4 4/4] mfd: Add RZ/G2L MTU3 PWM driver
2022-10-11 19:13 ` Biju Das
@ 2022-10-11 20:10 ` Krzysztof Kozlowski
2022-10-11 20:18 ` Biju Das
0 siblings, 1 reply; 32+ messages in thread
From: Krzysztof Kozlowski @ 2022-10-11 20:10 UTC (permalink / raw)
To: Biju Das, Thierry Reding, Philipp Zabel
Cc: Lee Jones, Uwe Kleine-König, linux-pwm, Geert Uytterhoeven,
Chris Paterson, Biju Das, Prabhakar Mahadev Lad,
linux-renesas-soc
On 11/10/2022 15:13, Biju Das wrote:
>> Subject: Re: [PATCH v4 4/4] mfd: Add RZ/G2L MTU3 PWM driver
>>
>> On 10/10/2022 10:52, Biju Das wrote:
>>> Add support for RZ/G2L MTU3 PWM driver. The IP supports following
>> PWM
>>> modes
>>>
>>> 1) PWM mode{1,2}
>>> 2) Reset-synchronized PWM mode
>>> 3) Complementary PWM mode{1,2,3}
>>>
>>> This patch adds basic pwm mode 1 support for RZ/G2L MTU3 driver by
>>> creating separate logical channels for each IOs.
>>>
>>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
>>> ---
>>> v3->v4:
>>> * There is no resource associated with "rz-mtu3-pwm" compatible
>>> and moved the code to mfd subsystem as it binds against "rz-mtu".
>>> * Removed struct platform_driver rz_mtu3_pwm_driver.
>>> v2->v3:
>>> * No change.
>>> v1->v2:
>>> * Modelled as a single PWM device handling multiple channles.
>>> * Used PM framework to manage the clocks.
>>> ---
>>> drivers/mfd/Kconfig | 6 +
>>> drivers/mfd/Makefile | 2 +
>>> drivers/mfd/rz-mtu3-pwm.c | 405
>>> ++++++++++++++++++++++++++++++++++++++
>>
>> That's not a MFD driver. That's a PWM. Use proper subsystem and email
>> prefix.
>
> See [1]
> [1] https://patchwork.kernel.org/project/linux-renesas-soc/patch/20221006135717.1748560-2-biju.das.jz@bp.renesas.com/
>
> It is a single driver that binds against "renesas,rz-mtu3", and registers both the counter and the pwm
> functionalities. Just like the clock driver, which registers clock, reset, and PM Domain functionalities.
No, it is not a single driver. You just added a new file - PWM.
>
> It is same here, a single MFD driver which binds against ""renesas,rz-mtu3" and registers counter
> And pwm functionalities.
>
> rz-mtu-core is core driver which provides resources to child devices like counter and pwm.
>
> I already copied PWM subsystem in the loop. Am I missing anything related to [1]
MFD subsystem is only a wrapper/parent over actual drivers. It's not
meant to hold the subsystem-specific code, because relevant maintainers
will not look here.
So no, here and in other files - don't put subsystem specific code like
PWM or timer into MFD.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v4 1/4] dt-bindings: mfd: Document RZ/G2L MTU3a bindings
2022-10-11 19:23 ` Biju Das
@ 2022-10-11 20:17 ` Krzysztof Kozlowski
2022-10-11 20:31 ` Biju Das
0 siblings, 1 reply; 32+ messages in thread
From: Krzysztof Kozlowski @ 2022-10-11 20:17 UTC (permalink / raw)
To: Biju Das, Rob Herring, Krzysztof Kozlowski,
William Breathitt Gray, Thierry Reding
Cc: Lee Jones, Uwe Kleine-König, devicetree, linux-pwm,
Geert Uytterhoeven, Chris Paterson, Biju Das,
Prabhakar Mahadev Lad, linux-renesas-soc
On 11/10/2022 15:23, Biju Das wrote:
>> Subject: Re: [PATCH v4 1/4] dt-bindings: mfd: Document RZ/G2L MTU3a
>> bindings
>>
>> On 11/10/2022 10:55, Biju Das wrote:
>>>
>>>>> .../bindings/mfd/renesas,rz-mtu3.yaml | 305
>>>> ++++++++++++++++++
>>>>> 1 file changed, 305 insertions(+)
>>>>> create mode 100644
>>>>> Documentation/devicetree/bindings/mfd/renesas,rz-mtu3.yaml
>>>>
>>>> This should not be in MFD. Just because some device has few
>> features,
>>>> does not mean it should go to MFD... Choose either timer or pwm.
>>>
>>> MFD is for multifunction device. This IP supports multiple functions
>>> like timer, pwm, clock source/events. That is the reason I have
>> added
>>> here. MFD is core which provides register access for client devices.
>>>
>>> For me moving it to pwm or counter is not a big problem.
>>> Why do you think it cannot be MFD?
>>
>>
>> Because it makes MFD a dump for everything where author did not want
>> to think about real device aspects, but instead represented driver
>> design (MFD driver).
>
> Core driver is MFD, just provides resources to child devices
> and is not aware of any real device aspects.
>
>>
>> MFDs are pretty often combining unrelated features, e.g. PMICs which
>> have wakeup and system power control, regulator, 32 kHz clocks, RTC
>> and some USB connector.
>
> Here also same right? pwm, counter and clock are 3 unrelated features.
> That is the reason we have separate subsystems for these features.
These are quite similar features of a similar piece of hardware.
Sometimes called timer.
>
>>
>> Just because you will have clocksource driver, PWM driver and timer
>> driver does not make it a MFD.
>
> MFD is multi function device.
No. MFD is a Linux subsystem name. Not a device type. The bindings are
located in respective type.
> So are are you agreeing Clock source, PWM and
> timer are different functionalities or not? If not, why do we have 3 subsystems,
> if it is same?
Linux subsystems? We can have millions of them and it is not related to
bindings.
> Where do keep these bindings as there is only single "rz_mtu" bindings for these 3 different functionalities?
Again, focus on hardware not on Linux drivers. Hardware is called MTU -
Multi-Function TIMER Unit. Timer.
> pwm or counter or mfd?
Not MFD. I already proposed where to put it. Other Timer/PWM/Counter
units are also in timer.
Renesas is not special to get some exceptions.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 32+ messages in thread
* RE: [PATCH v4 4/4] mfd: Add RZ/G2L MTU3 PWM driver
2022-10-11 20:10 ` Krzysztof Kozlowski
@ 2022-10-11 20:18 ` Biju Das
2022-10-11 20:27 ` Krzysztof Kozlowski
0 siblings, 1 reply; 32+ messages in thread
From: Biju Das @ 2022-10-11 20:18 UTC (permalink / raw)
To: Krzysztof Kozlowski, Thierry Reding, Philipp Zabel
Cc: Lee Jones, Uwe Kleine-König, linux-pwm, Geert Uytterhoeven,
Chris Paterson, Biju Das, Prabhakar Mahadev Lad,
linux-renesas-soc
> Subject: Re: [PATCH v4 4/4] mfd: Add RZ/G2L MTU3 PWM driver
>
> On 11/10/2022 15:13, Biju Das wrote:
> >> Subject: Re: [PATCH v4 4/4] mfd: Add RZ/G2L MTU3 PWM driver
> >>
> >> On 10/10/2022 10:52, Biju Das wrote:
> >>> Add support for RZ/G2L MTU3 PWM driver. The IP supports following
> >> PWM
> >>> modes
> >>>
> >>> 1) PWM mode{1,2}
> >>> 2) Reset-synchronized PWM mode
> >>> 3) Complementary PWM mode{1,2,3}
> >>>
> >>> This patch adds basic pwm mode 1 support for RZ/G2L MTU3 driver by
> >>> creating separate logical channels for each IOs.
> >>>
> >>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> >>> ---
> >>> v3->v4:
> >>> * There is no resource associated with "rz-mtu3-pwm" compatible
> >>> and moved the code to mfd subsystem as it binds against "rz-
> mtu".
> >>> * Removed struct platform_driver rz_mtu3_pwm_driver.
> >>> v2->v3:
> >>> * No change.
> >>> v1->v2:
> >>> * Modelled as a single PWM device handling multiple channles.
> >>> * Used PM framework to manage the clocks.
> >>> ---
> >>> drivers/mfd/Kconfig | 6 +
> >>> drivers/mfd/Makefile | 2 +
> >>> drivers/mfd/rz-mtu3-pwm.c | 405
> >>> ++++++++++++++++++++++++++++++++++++++
> >>
> >> That's not a MFD driver. That's a PWM. Use proper subsystem and
> email
> >> prefix.
> >
> > See [1]
> > [1]
=03qSqax5tr5tAuDHBytn7xH%2BS6oU2xguui9mrshI
> > tCI%3D&reserved=0
> >
> > It is a single driver that binds against "renesas,rz-mtu3", and
> > registers both the counter and the pwm functionalities. Just like
> the clock driver, which registers clock, reset, and PM Domain
> functionalities.
>
> No, it is not a single driver. You just added a new file - PWM.
It is a single driver rz-mtu.ko binds with "renesas,rz-mtu3"
>
> >
> > It is same here, a single MFD driver which binds against
> > ""renesas,rz-mtu3" and registers counter And pwm functionalities.
> >
> > rz-mtu-core is core driver which provides resources to child devices
> like counter and pwm.
> >
> > I already copied PWM subsystem in the loop. Am I missing anything
> > related to [1]
>
> MFD subsystem is only a wrapper/parent over actual drivers. It's not
> meant to hold the subsystem-specific code, because relevant
> maintainers will not look here.
>
> So no, here and in other files - don't put subsystem specific code
> like PWM or timer into MFD.
Where should do we put, if there is single driver to be bind against
"renesas,rz-mtu3" and register functionalities for pwm and counter??
Please suggest.
Cheers,
Biju
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v4 4/4] mfd: Add RZ/G2L MTU3 PWM driver
2022-10-11 20:18 ` Biju Das
@ 2022-10-11 20:27 ` Krzysztof Kozlowski
2022-10-11 20:35 ` Biju Das
0 siblings, 1 reply; 32+ messages in thread
From: Krzysztof Kozlowski @ 2022-10-11 20:27 UTC (permalink / raw)
To: Biju Das, Thierry Reding, Philipp Zabel
Cc: Lee Jones, Uwe Kleine-König, linux-pwm, Geert Uytterhoeven,
Chris Paterson, Biju Das, Prabhakar Mahadev Lad,
linux-renesas-soc
On 11/10/2022 16:18, Biju Das wrote:
>> Subject: Re: [PATCH v4 4/4] mfd: Add RZ/G2L MTU3 PWM driver
>>
>> On 11/10/2022 15:13, Biju Das wrote:
>>>> Subject: Re: [PATCH v4 4/4] mfd: Add RZ/G2L MTU3 PWM driver
>>>>
>>>> On 10/10/2022 10:52, Biju Das wrote:
>>>>> Add support for RZ/G2L MTU3 PWM driver. The IP supports following
>>>> PWM
>>>>> modes
>>>>>
>>>>> 1) PWM mode{1,2}
>>>>> 2) Reset-synchronized PWM mode
>>>>> 3) Complementary PWM mode{1,2,3}
>>>>>
>>>>> This patch adds basic pwm mode 1 support for RZ/G2L MTU3 driver by
>>>>> creating separate logical channels for each IOs.
>>>>>
>>>>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
>>>>> ---
>>>>> v3->v4:
>>>>> * There is no resource associated with "rz-mtu3-pwm" compatible
>>>>> and moved the code to mfd subsystem as it binds against "rz-
>> mtu".
>>>>> * Removed struct platform_driver rz_mtu3_pwm_driver.
>>>>> v2->v3:
>>>>> * No change.
>>>>> v1->v2:
>>>>> * Modelled as a single PWM device handling multiple channles.
>>>>> * Used PM framework to manage the clocks.
>>>>> ---
>>>>> drivers/mfd/Kconfig | 6 +
>>>>> drivers/mfd/Makefile | 2 +
>>>>> drivers/mfd/rz-mtu3-pwm.c | 405
>>>>> ++++++++++++++++++++++++++++++++++++++
>>>>
>>>> That's not a MFD driver. That's a PWM. Use proper subsystem and
>> email
>>>> prefix.
>>>
>>> See [1]
>>> [1]
> =03qSqax5tr5tAuDHBytn7xH%2BS6oU2xguui9mrshI
>>> tCI%3D&reserved=0
>>>
>>> It is a single driver that binds against "renesas,rz-mtu3", and
>>> registers both the counter and the pwm functionalities. Just like
>> the clock driver, which registers clock, reset, and PM Domain
>> functionalities.
>>
>> No, it is not a single driver. You just added a new file - PWM.
>
> It is a single driver rz-mtu.ko binds with "renesas,rz-mtu3"
Binding to compatible is not really related.
>
>>
>>>
>>> It is same here, a single MFD driver which binds against
>>> ""renesas,rz-mtu3" and registers counter And pwm functionalities.
>>>
>>> rz-mtu-core is core driver which provides resources to child devices
>> like counter and pwm.
>>>
>>> I already copied PWM subsystem in the loop. Am I missing anything
>>> related to [1]
>>
>> MFD subsystem is only a wrapper/parent over actual drivers. It's not
>> meant to hold the subsystem-specific code, because relevant
>> maintainers will not look here.
>>
>> So no, here and in other files - don't put subsystem specific code
>> like PWM or timer into MFD.
>
> Where should do we put, if there is single driver to be bind against
> "renesas,rz-mtu3" and register functionalities for pwm and counter??
Again - how binding is related to this problem? If you have separate
drivers, e.g. counter, timer and PWM, all go to their respective
subsystems. Counter goes to counter, timer to timer, PWM to pwm.
MFD is only the glue/parent/wrapper to instantiate them.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 32+ messages in thread
* RE: [PATCH v4 1/4] dt-bindings: mfd: Document RZ/G2L MTU3a bindings
2022-10-11 20:17 ` Krzysztof Kozlowski
@ 2022-10-11 20:31 ` Biju Das
2022-10-15 14:28 ` William Breathitt Gray
0 siblings, 1 reply; 32+ messages in thread
From: Biju Das @ 2022-10-11 20:31 UTC (permalink / raw)
To: Krzysztof Kozlowski, Rob Herring, Krzysztof Kozlowski,
William Breathitt Gray, Thierry Reding
Cc: Lee Jones, Uwe Kleine-König, devicetree, linux-pwm,
Geert Uytterhoeven, Chris Paterson, Biju Das,
Prabhakar Mahadev Lad, linux-renesas-soc
> Subject: Re: [PATCH v4 1/4] dt-bindings: mfd: Document RZ/G2L MTU3a
> bindings
>
> On 11/10/2022 15:23, Biju Das wrote:
> >> Subject: Re: [PATCH v4 1/4] dt-bindings: mfd: Document RZ/G2L MTU3a
> >> bindings
> >>
> >> On 11/10/2022 10:55, Biju Das wrote:
> >>>
> >>>>> .../bindings/mfd/renesas,rz-mtu3.yaml | 305
> >>>> ++++++++++++++++++
> >>>>> 1 file changed, 305 insertions(+) create mode 100644
> >>>>> Documentation/devicetree/bindings/mfd/renesas,rz-mtu3.yaml
> >>>>
> >>>> This should not be in MFD. Just because some device has few
> >> features,
> >>>> does not mean it should go to MFD... Choose either timer or pwm.
> >>>
> >>> MFD is for multifunction device. This IP supports multiple
> functions
> >>> like timer, pwm, clock source/events. That is the reason I have
> >> added
> >>> here. MFD is core which provides register access for client
> devices.
> >>>
> >>> For me moving it to pwm or counter is not a big problem.
> >>> Why do you think it cannot be MFD?
> >>
> >>
> >> Because it makes MFD a dump for everything where author did not
> want
> >> to think about real device aspects, but instead represented driver
> >> design (MFD driver).
> >
> > Core driver is MFD, just provides resources to child devices and is
> > not aware of any real device aspects.
> >
> >>
> >> MFDs are pretty often combining unrelated features, e.g. PMICs
> which
> >> have wakeup and system power control, regulator, 32 kHz clocks, RTC
> >> and some USB connector.
> >
> > Here also same right? pwm, counter and clock are 3 unrelated
> features.
> > That is the reason we have separate subsystems for these features.
>
> These are quite similar features of a similar piece of hardware.
> Sometimes called timer.
>
> >
> >>
> >> Just because you will have clocksource driver, PWM driver and timer
> >> driver does not make it a MFD.
> >
> > MFD is multi function device.
>
> No. MFD is a Linux subsystem name. Not a device type. The bindings are
> located in respective type.
>
> > So are are you agreeing Clock source, PWM and timer are different
> > functionalities or not? If not, why do we have 3 subsystems, if it
> is
> > same?
>
> Linux subsystems? We can have millions of them and it is not related
> to bindings.
OK.
>
>
> > Where do keep these bindings as there is only single "rz_mtu"
> bindings for these 3 different functionalities?
>
> Again, focus on hardware not on Linux drivers. Hardware is called MTU
> - Multi-Function TIMER Unit. Timer.
OK
>
> > pwm or counter or mfd?
>
> Not MFD. I already proposed where to put it. Other Timer/PWM/Counter
> units are also in timer.
>
I guess for counter/pwm maintainers, it is ok to model MTU3 as a single
binding "rz-mtu3" in timer that binds against counter and pwm
functionalities as well??
Cheers,
Biju
^ permalink raw reply [flat|nested] 32+ messages in thread
* RE: [PATCH v4 4/4] mfd: Add RZ/G2L MTU3 PWM driver
2022-10-11 20:27 ` Krzysztof Kozlowski
@ 2022-10-11 20:35 ` Biju Das
2022-10-11 20:37 ` Krzysztof Kozlowski
0 siblings, 1 reply; 32+ messages in thread
From: Biju Das @ 2022-10-11 20:35 UTC (permalink / raw)
To: Krzysztof Kozlowski, Thierry Reding, Philipp Zabel
Cc: Lee Jones, Uwe Kleine-König, linux-pwm, Geert Uytterhoeven,
Chris Paterson, Biju Das, Prabhakar Mahadev Lad,
linux-renesas-soc
> Subject: Re: [PATCH v4 4/4] mfd: Add RZ/G2L MTU3 PWM driver
>
> On 11/10/2022 16:18, Biju Das wrote:
> >> Subject: Re: [PATCH v4 4/4] mfd: Add RZ/G2L MTU3 PWM driver
> >>
> >> On 11/10/2022 15:13, Biju Das wrote:
> >>>> Subject: Re: [PATCH v4 4/4] mfd: Add RZ/G2L MTU3 PWM driver
> >>>>
> >>>> On 10/10/2022 10:52, Biju Das wrote:
> >>>>> Add support for RZ/G2L MTU3 PWM driver. The IP supports
> following
> >>>> PWM
> >>>>> modes
> >>>>>
> >>>>> 1) PWM mode{1,2}
> >>>>> 2) Reset-synchronized PWM mode
> >>>>> 3) Complementary PWM mode{1,2,3}
> >>>>>
> >>>>> This patch adds basic pwm mode 1 support for RZ/G2L MTU3 driver
> by
> >>>>> creating separate logical channels for each IOs.
> >>>>>
> >>>>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> >>>>> ---
> >>>>> v3->v4:
> >>>>> * There is no resource associated with "rz-mtu3-pwm" compatible
> >>>>> and moved the code to mfd subsystem as it binds against "rz-
> >> mtu".
> >>>>> * Removed struct platform_driver rz_mtu3_pwm_driver.
> >>>>> v2->v3:
> >>>>> * No change.
> >>>>> v1->v2:
> >>>>> * Modelled as a single PWM device handling multiple channles.
> >>>>> * Used PM framework to manage the clocks.
> >>>>> ---
> >>>>> drivers/mfd/Kconfig | 6 +
> >>>>> drivers/mfd/Makefile | 2 +
> >>>>> drivers/mfd/rz-mtu3-pwm.c | 405
> >>>>> ++++++++++++++++++++++++++++++++++++++
> >>>>
> >>>> That's not a MFD driver. That's a PWM. Use proper subsystem and
> >> email
> >>>> prefix.
> >>>
> >>> See [1]
> >>> [1]
> > =03qSqax5tr5tAuDHBytn7xH%2BS6oU2xguui9mrshI
> >>> tCI%3D&reserved=0
> >>>
> >>> It is a single driver that binds against "renesas,rz-mtu3", and
> >>> registers both the counter and the pwm functionalities. Just like
> >> the clock driver, which registers clock, reset, and PM Domain
> >> functionalities.
> >>
> >> No, it is not a single driver. You just added a new file - PWM.
> >
> > It is a single driver rz-mtu.ko binds with "renesas,rz-mtu3"
>
> Binding to compatible is not really related.
>
> >
> >>
> >>>
> >>> It is same here, a single MFD driver which binds against
> >>> ""renesas,rz-mtu3" and registers counter And pwm functionalities.
> >>>
> >>> rz-mtu-core is core driver which provides resources to child
> devices
> >> like counter and pwm.
> >>>
> >>> I already copied PWM subsystem in the loop. Am I missing anything
> >>> related to [1]
> >>
> >> MFD subsystem is only a wrapper/parent over actual drivers. It's
> not
> >> meant to hold the subsystem-specific code, because relevant
> >> maintainers will not look here.
> >>
> >> So no, here and in other files - don't put subsystem specific code
> >> like PWM or timer into MFD.
> >
> > Where should do we put, if there is single driver to be bind against
> > "renesas,rz-mtu3" and register functionalities for pwm and counter??
>
> Again - how binding is related to this problem? If you have separate
> drivers, e.g. counter, timer and PWM, all go to their respective
> subsystems. Counter goes to counter, timer to timer, PWM to pwm.
How do you instantiate these drivers with a single compatible "renesas,rz-mtu3"?
If it is separate drivers.
Please let me know.
Cheers,
Biju
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v4 4/4] mfd: Add RZ/G2L MTU3 PWM driver
2022-10-11 20:35 ` Biju Das
@ 2022-10-11 20:37 ` Krzysztof Kozlowski
2022-10-11 20:43 ` Biju Das
0 siblings, 1 reply; 32+ messages in thread
From: Krzysztof Kozlowski @ 2022-10-11 20:37 UTC (permalink / raw)
To: Biju Das, Thierry Reding, Philipp Zabel
Cc: Lee Jones, Uwe Kleine-König, linux-pwm, Geert Uytterhoeven,
Chris Paterson, Biju Das, Prabhakar Mahadev Lad,
linux-renesas-soc
On 11/10/2022 16:35, Biju Das wrote:
>>>>
>>>> So no, here and in other files - don't put subsystem specific code
>>>> like PWM or timer into MFD.
>>>
>>> Where should do we put, if there is single driver to be bind against
>>> "renesas,rz-mtu3" and register functionalities for pwm and counter??
>>
>> Again - how binding is related to this problem? If you have separate
>> drivers, e.g. counter, timer and PWM, all go to their respective
>> subsystems. Counter goes to counter, timer to timer, PWM to pwm.
>
> How do you instantiate these drivers with a single compatible "renesas,rz-mtu3"?
> If it is separate drivers.
>
With MFD framework and mfd_cell, just like many, many other drivers.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 32+ messages in thread
* RE: [PATCH v4 4/4] mfd: Add RZ/G2L MTU3 PWM driver
2022-10-11 20:37 ` Krzysztof Kozlowski
@ 2022-10-11 20:43 ` Biju Das
2022-10-11 20:53 ` Krzysztof Kozlowski
0 siblings, 1 reply; 32+ messages in thread
From: Biju Das @ 2022-10-11 20:43 UTC (permalink / raw)
To: Krzysztof Kozlowski, Thierry Reding, Philipp Zabel
Cc: Lee Jones, Uwe Kleine-König, linux-pwm, Geert Uytterhoeven,
Chris Paterson, Biju Das, Prabhakar Mahadev Lad,
linux-renesas-soc
> Subject: Re: [PATCH v4 4/4] mfd: Add RZ/G2L MTU3 PWM driver
>
> On 11/10/2022 16:35, Biju Das wrote:
> >>>>
> >>>> So no, here and in other files - don't put subsystem specific
> code
> >>>> like PWM or timer into MFD.
> >>>
> >>> Where should do we put, if there is single driver to be bind
> against
> >>> "renesas,rz-mtu3" and register functionalities for pwm and
> counter??
> >>
> >> Again - how binding is related to this problem? If you have
> separate
> >> drivers, e.g. counter, timer and PWM, all go to their respective
> >> subsystems. Counter goes to counter, timer to timer, PWM to pwm.
> >
> > How do you instantiate these drivers with a single compatible
> "renesas,rz-mtu3"?
> > If it is separate drivers.
> >
>
> With MFD framework and mfd_cell, just like many, many other drivers.
They all have compatibles for child devices, right??
Can you please Provide an MFD example which has a single compatible and
Multiple child devices in different subsystems??
Cheers,
Biju
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v4 4/4] mfd: Add RZ/G2L MTU3 PWM driver
2022-10-11 20:43 ` Biju Das
@ 2022-10-11 20:53 ` Krzysztof Kozlowski
2022-10-15 13:56 ` Biju Das
0 siblings, 1 reply; 32+ messages in thread
From: Krzysztof Kozlowski @ 2022-10-11 20:53 UTC (permalink / raw)
To: Biju Das, Thierry Reding, Philipp Zabel
Cc: Lee Jones, Uwe Kleine-König, linux-pwm, Geert Uytterhoeven,
Chris Paterson, Biju Das, Prabhakar Mahadev Lad,
linux-renesas-soc
On 11/10/2022 16:43, Biju Das wrote:
>> Subject: Re: [PATCH v4 4/4] mfd: Add RZ/G2L MTU3 PWM driver
>>
>> On 11/10/2022 16:35, Biju Das wrote:
>>>>>>
>>>>>> So no, here and in other files - don't put subsystem specific
>> code
>>>>>> like PWM or timer into MFD.
>>>>>
>>>>> Where should do we put, if there is single driver to be bind
>> against
>>>>> "renesas,rz-mtu3" and register functionalities for pwm and
>> counter??
>>>>
>>>> Again - how binding is related to this problem? If you have
>> separate
>>>> drivers, e.g. counter, timer and PWM, all go to their respective
>>>> subsystems. Counter goes to counter, timer to timer, PWM to pwm.
>>>
>>> How do you instantiate these drivers with a single compatible
>> "renesas,rz-mtu3"?
>>> If it is separate drivers.
>>>
>>
>> With MFD framework and mfd_cell, just like many, many other drivers.
>
> They all have compatibles for child devices, right??
No
> Can you please Provide an MFD example which has a single compatible and
> Multiple child devices in different subsystems??
There is plenty of examples:
git grep -C 4 'struct mfd_cell'
Even the first search result in MFD directory fits your needs, doesn't it?
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 32+ messages in thread
* RE: [PATCH v4 4/4] mfd: Add RZ/G2L MTU3 PWM driver
2022-10-11 20:53 ` Krzysztof Kozlowski
@ 2022-10-15 13:56 ` Biju Das
0 siblings, 0 replies; 32+ messages in thread
From: Biju Das @ 2022-10-15 13:56 UTC (permalink / raw)
To: Krzysztof Kozlowski, Thierry Reding, Philipp Zabel
Cc: Lee Jones, Uwe Kleine-König, linux-pwm, Geert Uytterhoeven,
Chris Paterson, Biju Das, Prabhakar Mahadev Lad,
linux-renesas-soc
Hi Krzysztof Kozlowski,
> Subject: Re: [PATCH v4 4/4] mfd: Add RZ/G2L MTU3 PWM driver
>
> On 11/10/2022 16:43, Biju Das wrote:
> >> Subject: Re: [PATCH v4 4/4] mfd: Add RZ/G2L MTU3 PWM driver
> >>
> >> On 11/10/2022 16:35, Biju Das wrote:
> >>>>>>
> >>>>>> So no, here and in other files - don't put subsystem specific
> >> code
> >>>>>> like PWM or timer into MFD.
> >>>>>
> >>>>> Where should do we put, if there is single driver to be bind
> >> against
> >>>>> "renesas,rz-mtu3" and register functionalities for pwm and
> >> counter??
> >>>>
> >>>> Again - how binding is related to this problem? If you have
> >> separate
> >>>> drivers, e.g. counter, timer and PWM, all go to their respective
> >>>> subsystems. Counter goes to counter, timer to timer, PWM to pwm.
> >>>
> >>> How do you instantiate these drivers with a single compatible
> >> "renesas,rz-mtu3"?
> >>> If it is separate drivers.
> >>>
> >>
> >> With MFD framework and mfd_cell, just like many, many other
> drivers.
> >
> > They all have compatibles for child devices, right??
>
> No
>
> > Can you please Provide an MFD example which has a single compatible
> > and Multiple child devices in different subsystems??
>
> There is plenty of examples:
> git grep -C 4 'struct mfd_cell'
>
> Even the first search result in MFD directory fits your needs, doesn't
> it?
Thanks for the pointer. Will send next version based on this.
Cheers,
Biju
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v4 1/4] dt-bindings: mfd: Document RZ/G2L MTU3a bindings
2022-10-11 20:31 ` Biju Das
@ 2022-10-15 14:28 ` William Breathitt Gray
2022-10-15 15:17 ` Biju Das
2022-10-17 14:00 ` Thierry Reding
0 siblings, 2 replies; 32+ messages in thread
From: William Breathitt Gray @ 2022-10-15 14:28 UTC (permalink / raw)
To: Biju Das
Cc: Krzysztof Kozlowski, Rob Herring, Krzysztof Kozlowski,
Thierry Reding, Lee Jones, Uwe Kleine-König, devicetree,
linux-pwm, Geert Uytterhoeven, Chris Paterson, Biju Das,
Prabhakar Mahadev Lad, linux-renesas-soc
[-- Attachment #1: Type: text/plain, Size: 4568 bytes --]
On Tue, Oct 11, 2022 at 08:31:48PM +0000, Biju Das wrote:
> > Subject: Re: [PATCH v4 1/4] dt-bindings: mfd: Document RZ/G2L MTU3a
> > bindings
> >
> > On 11/10/2022 15:23, Biju Das wrote:
> > >> Subject: Re: [PATCH v4 1/4] dt-bindings: mfd: Document RZ/G2L MTU3a
> > >> bindings
> > >>
> > >> On 11/10/2022 10:55, Biju Das wrote:
> > >>>
> > >>>>> .../bindings/mfd/renesas,rz-mtu3.yaml | 305
> > >>>> ++++++++++++++++++
> > >>>>> 1 file changed, 305 insertions(+) create mode 100644
> > >>>>> Documentation/devicetree/bindings/mfd/renesas,rz-mtu3.yaml
> > >>>>
> > >>>> This should not be in MFD. Just because some device has few
> > >> features,
> > >>>> does not mean it should go to MFD... Choose either timer or pwm.
> > >>>
> > >>> MFD is for multifunction device. This IP supports multiple
> > functions
> > >>> like timer, pwm, clock source/events. That is the reason I have
> > >> added
> > >>> here. MFD is core which provides register access for client
> > devices.
> > >>>
> > >>> For me moving it to pwm or counter is not a big problem.
> > >>> Why do you think it cannot be MFD?
> > >>
> > >>
> > >> Because it makes MFD a dump for everything where author did not
> > want
> > >> to think about real device aspects, but instead represented driver
> > >> design (MFD driver).
> > >
> > > Core driver is MFD, just provides resources to child devices and is
> > > not aware of any real device aspects.
> > >
> > >>
> > >> MFDs are pretty often combining unrelated features, e.g. PMICs
> > which
> > >> have wakeup and system power control, regulator, 32 kHz clocks, RTC
> > >> and some USB connector.
> > >
> > > Here also same right? pwm, counter and clock are 3 unrelated
> > features.
> > > That is the reason we have separate subsystems for these features.
> >
> > These are quite similar features of a similar piece of hardware.
> > Sometimes called timer.
> >
> > >
> > >>
> > >> Just because you will have clocksource driver, PWM driver and timer
> > >> driver does not make it a MFD.
> > >
> > > MFD is multi function device.
> >
> > No. MFD is a Linux subsystem name. Not a device type. The bindings are
> > located in respective type.
> >
> > > So are are you agreeing Clock source, PWM and timer are different
> > > functionalities or not? If not, why do we have 3 subsystems, if it
> > is
> > > same?
> >
> > Linux subsystems? We can have millions of them and it is not related
> > to bindings.
>
> OK.
>
> >
> >
> > > Where do keep these bindings as there is only single "rz_mtu"
> > bindings for these 3 different functionalities?
> >
> > Again, focus on hardware not on Linux drivers. Hardware is called MTU
> > - Multi-Function TIMER Unit. Timer.
>
> OK
> >
> > > pwm or counter or mfd?
> >
> > Not MFD. I already proposed where to put it. Other Timer/PWM/Counter
> > units are also in timer.
> >
>
> I guess for counter/pwm maintainers, it is ok to model MTU3 as a single
> binding "rz-mtu3" in timer that binds against counter and pwm
> functionalities as well??
>
> Cheers,
> Biju
I'm okay with putting the MTU3 binding under timer; we already have
Documentation/devicetree/bindings/timer/renesas,mtu2.yaml there so
adding a new renesas,mtu3.yaml next to it seems reasonable.
Just to reiterate Krzysztof's point, the subsystems in Linux serve as a
way to group drivers together that utilize the same ABIs, whereas the
devicetree is a structure for organizing physical hardware. The
structure of physical hardware types don't necessarily match the
organization of the ABIs we use to support them. This is why you may end
up with differing heirarchies between the devicetree and driver
subsystems.
To illustrate the point, take for example a hypothetical
digital-to-analog (DAC) device with a few GPIO lines. Those GPIO
input signals could be tied to buttons used to indicate to the system
that a user wants to reset or adjust the DAC output, while the GPIO
output signals could be status lights or triggers indicating that the
DAC is operating. The respective driver for this device may utilize the
IIO subsystem to support the DAC and the GPIO subsystem to support those
GPIO lines, but it would be incorrect to put this under MFD because the
purpose of the GPIO lines is to assist in the operation of the DAC; in
other words, this is primarily a DAC device with some auxiliary
convenience functionalities, not a MFD with distinct unrelated separate
components.
William Breathitt Gray
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread
* RE: [PATCH v4 1/4] dt-bindings: mfd: Document RZ/G2L MTU3a bindings
2022-10-15 14:28 ` William Breathitt Gray
@ 2022-10-15 15:17 ` Biju Das
2022-10-17 14:00 ` Thierry Reding
1 sibling, 0 replies; 32+ messages in thread
From: Biju Das @ 2022-10-15 15:17 UTC (permalink / raw)
To: William Breathitt Gray
Cc: Krzysztof Kozlowski, Rob Herring, Krzysztof Kozlowski,
Thierry Reding, Lee Jones, Uwe Kleine-König, devicetree,
linux-pwm, Geert Uytterhoeven, Chris Paterson, Biju Das,
Prabhakar Mahadev Lad, linux-renesas-soc
Hi William Breathitt Gray,
> Subject: Re: [PATCH v4 1/4] dt-bindings: mfd: Document RZ/G2L MTU3a
> bindings
>
> On Tue, Oct 11, 2022 at 08:31:48PM +0000, Biju Das wrote:
> > > Subject: Re: [PATCH v4 1/4] dt-bindings: mfd: Document RZ/G2L
> MTU3a
> > > bindings
> > >
> > > On 11/10/2022 15:23, Biju Das wrote:
> > > >> Subject: Re: [PATCH v4 1/4] dt-bindings: mfd: Document RZ/G2L
> > > >> MTU3a bindings
> > > >>
> > > >> On 11/10/2022 10:55, Biju Das wrote:
> > > >>>
> > > >>>>> .../bindings/mfd/renesas,rz-mtu3.yaml | 305
> > > >>>> ++++++++++++++++++
> > > >>>>> 1 file changed, 305 insertions(+) create mode 100644
> > > >>>>> Documentation/devicetree/bindings/mfd/renesas,rz-mtu3.yaml
> > > >>>>
> > > >>>> This should not be in MFD. Just because some device has few
> > > >> features,
> > > >>>> does not mean it should go to MFD... Choose either timer or
> pwm.
> > > >>>
> > > >>> MFD is for multifunction device. This IP supports multiple
> > > functions
> > > >>> like timer, pwm, clock source/events. That is the reason I
> have
> > > >> added
> > > >>> here. MFD is core which provides register access for client
> > > devices.
> > > >>>
> > > >>> For me moving it to pwm or counter is not a big problem.
> > > >>> Why do you think it cannot be MFD?
> > > >>
> > > >>
> > > >> Because it makes MFD a dump for everything where author did not
> > > want
> > > >> to think about real device aspects, but instead represented
> > > >> driver design (MFD driver).
> > > >
> > > > Core driver is MFD, just provides resources to child devices and
> > > > is not aware of any real device aspects.
> > > >
> > > >>
> > > >> MFDs are pretty often combining unrelated features, e.g. PMICs
> > > which
> > > >> have wakeup and system power control, regulator, 32 kHz clocks,
> > > >> RTC and some USB connector.
> > > >
> > > > Here also same right? pwm, counter and clock are 3 unrelated
> > > features.
> > > > That is the reason we have separate subsystems for these
> features.
> > >
> > > These are quite similar features of a similar piece of hardware.
> > > Sometimes called timer.
> > >
> > > >
> > > >>
> > > >> Just because you will have clocksource driver, PWM driver and
> > > >> timer driver does not make it a MFD.
> > > >
> > > > MFD is multi function device.
> > >
> > > No. MFD is a Linux subsystem name. Not a device type. The bindings
> > > are located in respective type.
> > >
> > > > So are are you agreeing Clock source, PWM and timer are
> different
> > > > functionalities or not? If not, why do we have 3 subsystems, if
> it
> > > is
> > > > same?
> > >
> > > Linux subsystems? We can have millions of them and it is not
> related
> > > to bindings.
> >
> > OK.
> >
> > >
> > >
> > > > Where do keep these bindings as there is only single "rz_mtu"
> > > bindings for these 3 different functionalities?
> > >
> > > Again, focus on hardware not on Linux drivers. Hardware is called
> > > MTU
> > > - Multi-Function TIMER Unit. Timer.
> >
> > OK
> > >
> > > > pwm or counter or mfd?
> > >
> > > Not MFD. I already proposed where to put it. Other
> Timer/PWM/Counter
> > > units are also in timer.
> > >
> >
> > I guess for counter/pwm maintainers, it is ok to model MTU3 as a
> > single binding "rz-mtu3" in timer that binds against counter and pwm
> > functionalities as well??
> >
> > Cheers,
> > Biju
>
> I'm okay with putting the MTU3 binding under timer; we already have
> Documentation/devicetree/bindings/timer/renesas,mtu2.yaml there so
> adding a new renesas,mtu3.yaml next to it seems reasonable.
OK.
>
> Just to reiterate Krzysztof's point, the subsystems in Linux serve as
> a way to group drivers together that utilize the same ABIs, whereas
> the devicetree is a structure for organizing physical hardware. The
> structure of physical hardware types don't necessarily match the
> organization of the ABIs we use to support them. This is why you may
> end up with differing heirarchies between the devicetree and driver
> subsystems.
>
> To illustrate the point, take for example a hypothetical digital-to-
> analog (DAC) device with a few GPIO lines. Those GPIO input signals
> could be tied to buttons used to indicate to the system that a user
> wants to reset or adjust the DAC output, while the GPIO output signals
> could be status lights or triggers indicating that the DAC is
> operating. The respective driver for this device may utilize the IIO
> subsystem to support the DAC and the GPIO subsystem to support those
> GPIO lines, but it would be incorrect to put this under MFD because
> the purpose of the GPIO lines is to assist in the operation of the
> DAC; in other words, this is primarily a DAC device with some
> auxiliary convenience functionalities, not a MFD with distinct
> unrelated separate components.
OK agreed. Thanks for the explanation.
Cheers,
Biju
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v4 1/4] dt-bindings: mfd: Document RZ/G2L MTU3a bindings
2022-10-15 14:28 ` William Breathitt Gray
2022-10-15 15:17 ` Biju Das
@ 2022-10-17 14:00 ` Thierry Reding
1 sibling, 0 replies; 32+ messages in thread
From: Thierry Reding @ 2022-10-17 14:00 UTC (permalink / raw)
To: William Breathitt Gray
Cc: Biju Das, Krzysztof Kozlowski, Rob Herring, Krzysztof Kozlowski,
Lee Jones, Uwe Kleine-König, devicetree, linux-pwm,
Geert Uytterhoeven, Chris Paterson, Biju Das,
Prabhakar Mahadev Lad, linux-renesas-soc
[-- Attachment #1: Type: text/plain, Size: 5776 bytes --]
On Sat, Oct 15, 2022 at 10:28:53AM -0400, William Breathitt Gray wrote:
> On Tue, Oct 11, 2022 at 08:31:48PM +0000, Biju Das wrote:
> > > Subject: Re: [PATCH v4 1/4] dt-bindings: mfd: Document RZ/G2L MTU3a
> > > bindings
> > >
> > > On 11/10/2022 15:23, Biju Das wrote:
> > > >> Subject: Re: [PATCH v4 1/4] dt-bindings: mfd: Document RZ/G2L MTU3a
> > > >> bindings
> > > >>
> > > >> On 11/10/2022 10:55, Biju Das wrote:
> > > >>>
> > > >>>>> .../bindings/mfd/renesas,rz-mtu3.yaml | 305
> > > >>>> ++++++++++++++++++
> > > >>>>> 1 file changed, 305 insertions(+) create mode 100644
> > > >>>>> Documentation/devicetree/bindings/mfd/renesas,rz-mtu3.yaml
> > > >>>>
> > > >>>> This should not be in MFD. Just because some device has few
> > > >> features,
> > > >>>> does not mean it should go to MFD... Choose either timer or pwm.
> > > >>>
> > > >>> MFD is for multifunction device. This IP supports multiple
> > > functions
> > > >>> like timer, pwm, clock source/events. That is the reason I have
> > > >> added
> > > >>> here. MFD is core which provides register access for client
> > > devices.
> > > >>>
> > > >>> For me moving it to pwm or counter is not a big problem.
> > > >>> Why do you think it cannot be MFD?
> > > >>
> > > >>
> > > >> Because it makes MFD a dump for everything where author did not
> > > want
> > > >> to think about real device aspects, but instead represented driver
> > > >> design (MFD driver).
> > > >
> > > > Core driver is MFD, just provides resources to child devices and is
> > > > not aware of any real device aspects.
> > > >
> > > >>
> > > >> MFDs are pretty often combining unrelated features, e.g. PMICs
> > > which
> > > >> have wakeup and system power control, regulator, 32 kHz clocks, RTC
> > > >> and some USB connector.
> > > >
> > > > Here also same right? pwm, counter and clock are 3 unrelated
> > > features.
> > > > That is the reason we have separate subsystems for these features.
> > >
> > > These are quite similar features of a similar piece of hardware.
> > > Sometimes called timer.
> > >
> > > >
> > > >>
> > > >> Just because you will have clocksource driver, PWM driver and timer
> > > >> driver does not make it a MFD.
> > > >
> > > > MFD is multi function device.
> > >
> > > No. MFD is a Linux subsystem name. Not a device type. The bindings are
> > > located in respective type.
> > >
> > > > So are are you agreeing Clock source, PWM and timer are different
> > > > functionalities or not? If not, why do we have 3 subsystems, if it
> > > is
> > > > same?
> > >
> > > Linux subsystems? We can have millions of them and it is not related
> > > to bindings.
> >
> > OK.
> >
> > >
> > >
> > > > Where do keep these bindings as there is only single "rz_mtu"
> > > bindings for these 3 different functionalities?
> > >
> > > Again, focus on hardware not on Linux drivers. Hardware is called MTU
> > > - Multi-Function TIMER Unit. Timer.
> >
> > OK
> > >
> > > > pwm or counter or mfd?
> > >
> > > Not MFD. I already proposed where to put it. Other Timer/PWM/Counter
> > > units are also in timer.
> > >
> >
> > I guess for counter/pwm maintainers, it is ok to model MTU3 as a single
> > binding "rz-mtu3" in timer that binds against counter and pwm
> > functionalities as well??
> >
> > Cheers,
> > Biju
>
> I'm okay with putting the MTU3 binding under timer; we already have
> Documentation/devicetree/bindings/timer/renesas,mtu2.yaml there so
> adding a new renesas,mtu3.yaml next to it seems reasonable.
>
> Just to reiterate Krzysztof's point, the subsystems in Linux serve as a
> way to group drivers together that utilize the same ABIs, whereas the
> devicetree is a structure for organizing physical hardware. The
> structure of physical hardware types don't necessarily match the
> organization of the ABIs we use to support them. This is why you may end
> up with differing heirarchies between the devicetree and driver
> subsystems.
>
> To illustrate the point, take for example a hypothetical
> digital-to-analog (DAC) device with a few GPIO lines. Those GPIO
> input signals could be tied to buttons used to indicate to the system
> that a user wants to reset or adjust the DAC output, while the GPIO
> output signals could be status lights or triggers indicating that the
> DAC is operating. The respective driver for this device may utilize the
> IIO subsystem to support the DAC and the GPIO subsystem to support those
> GPIO lines, but it would be incorrect to put this under MFD because the
> purpose of the GPIO lines is to assist in the operation of the DAC; in
> other words, this is primarily a DAC device with some auxiliary
> convenience functionalities, not a MFD with distinct unrelated separate
> components.
Exactly. In addition to the DT aspect, another misconception is that a
driver for a multi-function device needs to be somehow split up to match
the Linux subsystem structure. In most cases that's not true. Pick the
subsystem that most closely fits the main function of a device and
implement the driver. If you can expose further functions using other
subsystems, that's absolutely fine. Drivers can register with multiple
subsystems at the same time.
Yes, that's not quite as neat as having individual drivers for each
subsystem, but it's a much better approximation of the hardware and
therefore will lead to more compact and stable drivers. Trying to split
things up arbitrarily often makes for wonky constructs.
We've gained powerful tools over the years to easily deal with cross-
subsystem drivers, so there's seldom a reason to strictly split things
across subsystem boundaries anymore.
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v4 3/4] mfd: Add RZ/G2L MTU3 counter driver
2022-10-10 14:52 ` [PATCH v4 3/4] mfd: Add RZ/G2L MTU3 counter driver Biju Das
@ 2022-10-17 16:40 ` William Breathitt Gray
2022-10-17 19:58 ` Biju Das
2022-10-25 13:50 ` Geert Uytterhoeven
1 sibling, 1 reply; 32+ messages in thread
From: William Breathitt Gray @ 2022-10-17 16:40 UTC (permalink / raw)
To: Biju Das
Cc: Philipp Zabel, Lee Jones, linux-iio, Geert Uytterhoeven,
Chris Paterson, Biju Das, Prabhakar Mahadev Lad,
linux-renesas-soc
[-- Attachment #1: Type: text/plain, Size: 7624 bytes --]
On Mon, Oct 10, 2022 at 03:52:21PM +0100, Biju Das wrote:
> Add RZ/G2L MTU3 counter driver. This IP supports the following
> phase counting modes on MTU1 and MTU2 channels
>
> 1) 16-bit phase counting modes on MTU1 and MTU2 channels.
> 2) 32-bit phase counting mode by cascading MTU1 and MTU2.
>
> This patch adds 3 counters by creating 3 logical channels
> counter0: 16-bit phase counter on MTU1 channel
> counter1: 16-bit phase counter on MTU2 channel
> counter2: 32-bit phase counter by cascading MTU1 and MTU2
> channels.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Hello Biju,
We discussed some changes already for v5, but I have some additional
comments and questions below.
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 7329971a3bdf..fa88056224c9 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -1986,6 +1986,14 @@ config MFD_RZ_MTU3
> To compile this driver as a module, choose M here: the
> module will be called rz-mtu3.
>
> +config MFD_RZ_MTU3_CNT
> + tristate "RZ/G2L MTU3 counter driver"
This is a nitpick, but include the manufacturer name in the tristate
string for the sake of clarity: "Renesas RZ/G2L MTU3 counter driver".
> + depends on MFD_RZ_MTU3 || COMPILE_TEST
I noticed you include <linux/of.h> in the rz-mtu3-cnt.c file; do you
need to depend on OF here in the Kconfig as well?
> +static int rz_mtu3_count_read(struct counter_device *counter,
> + struct counter_count *count, u64 *val)
> +{
> + struct rz_mtu3_cnt *const priv = counter_priv(counter);
> +
> + if (count->id == RZ_MTU3_32_BIT_CH)
> + *val = rz_mtu3_32bit_ch_read(priv->ch[0], RZ_MTU3_TCNTLW);
> + else
> + *val = rz_mtu3_16bit_ch_read(priv->ch[count->id], RZ_MTU3_TCNT);
After considering this again, I think it'll be better to match the
structure of the rest of the functions in this driver for consistency.
Rather than hardcoding priv->ch[0], determine the ch_id first and pass
that instead::
const size_t ch_id = RZ_MTU3_GET_HW_CH(count->id);
if (count->id == RZ_MTU3_32_BIT_CH)
*val = rz_mtu3_32bit_ch_read(priv->ch[ch_id], RZ_MTU3_TCNTLW);
else
*val = rz_mtu3_16bit_ch_read(priv->ch[ch_id], RZ_MTU3_TCNT);
> +static int rz_mtu3_count_write(struct counter_device *counter,
> + struct counter_count *count, const u64 val)
> +{
> + struct rz_mtu3_cnt *const priv = counter_priv(counter);
> + const size_t ch_id = RZ_MTU3_GET_HW_CH(count->id);
> + u32 ceiling;
> +
> + mutex_lock(&priv->lock);
> + if (count->id == RZ_MTU3_32_BIT_CH)
> + ceiling = priv->mtu_32bit_max;
> + else
> + ceiling = priv->mtu_16bit_max[ch_id];
> +
> + if (val > ceiling) {
> + mutex_unlock(&priv->lock);
> + return -ERANGE;
> + }
> +
> + if (count->id == RZ_MTU3_32_BIT_CH)
> + rz_mtu3_32bit_ch_write(priv->ch[0], RZ_MTU3_TCNTLW, val);
Like in count_read(), use ch_id here instead of 0 for the sake of
consistency.
> +static int rz_mtu3_count_ceiling_write(struct counter_device *counter,
> + struct counter_count *count,
> + u64 ceiling)
> +{
> + struct rz_mtu3_cnt *const priv = counter_priv(counter);
> + const size_t ch_id = RZ_MTU3_GET_HW_CH(count->id);
> +
> + switch (count->id) {
> + case RZ_MTU3_16_BIT_MTU1_CH:
> + case RZ_MTU3_16_BIT_MTU2_CH:
> + if (ceiling > U16_MAX)
> + return -ERANGE;
> + priv->mtu_16bit_max[ch_id] = ceiling;
> + break;
> + case RZ_MTU3_32_BIT_CH:
> + if (ceiling > U32_MAX)
> + return -ERANGE;
> + priv->mtu_32bit_max = ceiling;
> + break;
> + }
> +
> + mutex_lock(&priv->lock);
> + if (ceiling == 0) {
> + /* Disable counter clear source */
> + rz_mtu3_8bit_ch_write(priv->ch[ch_id], RZ_MTU3_TCR,
> + RZ_MTU3_TCR_CCLR_NONE);
Refer to our discussions in the v3 review thread regarding ceiling set
to 0; in particular, don't disable the count value ceiling but rather
limit it to a maximum value of 0.
> +static int rz_mtu3_count_enable_write(struct counter_device *counter,
> + struct counter_count *count, u8 enable)
> +{
> + struct rz_mtu3_cnt *const priv = counter_priv(counter);
> + const size_t ch_id = RZ_MTU3_GET_HW_CH(count->id);
> + struct rz_mtu3_channel *ch = priv->ch[ch_id];
> + int ret = 0;
> +
> + if (enable) {
> + pm_runtime_get_sync(ch->dev);
> + ret = rz_mtu3_initialize_counter(counter, count->id);
> + } else {
> + rz_mtu3_terminate_counter(counter, count->id);
> + pm_runtime_put(ch->dev);
> + }
Refer to our discussions in the v3 review thread regarding the "enable"
Count extension; in particular, "enable" pauses/unpauses counting.
> +static int rz_mtu3_action_read(struct counter_device *counter,
> + struct counter_count *count,
> + struct counter_synapse *synapse,
> + enum counter_synapse_action *action)
> +{
> + const size_t signal_a_id = count->synapses[0].signal->id;
> + const size_t signal_b_id = count->synapses[1].signal->id;
> + size_t signal_c_id;
> + size_t signal_d_id;
> + enum counter_function function;
> + int err;
> +
> + if (count->id != RZ_MTU3_16_BIT_MTU1_CH) {
> + signal_c_id = count->synapses[2].signal->id;
> + signal_d_id = count->synapses[3].signal->id;
> + }
The Signal ids are constants so you remove them from this function and
use preprocessor defines instead to represent SIGNAL_A_ID, SIGNAL_B_ID,
SIGNAL_C_ID, and SIGNAL_D_ID. Remember to set the Signal ids in the
rz_mtu3_signals[] array accordingly.
> +static struct counter_signal rz_mtu3_signals[] = {
> + RZ_MTU3_PHASE_SIGNAL(0, "MTU1 MTCLKA"),
> + RZ_MTU3_PHASE_SIGNAL(1, "MTU1 MTCLKB"),
> + RZ_MTU3_PHASE_SIGNAL(2, "MTU2 MTCLKC"),
> + RZ_MTU3_PHASE_SIGNAL(3, "MTU2 MTCLKD"),
> +};
The relationship of these Signals still has me somewhat confused so I'm
hoping you can help me properly ironed out my understanding. This is how
I currently understand it, so please point out any mistakes I have:
MTU1 (Channel 1):
* Pulse-Direction mode:
- MTCLKA updates count
- MTCLKB determines direction
* Quadrature x2 B:
- MTCLKA is quadrature phase A
- MTCLKB is quadrature phase B
- Any state transition on MTCLKB updates count
* Quadrature x4:
- MTCLKA is quadrature phase A
- MTCLKB is quadrature phase B
- Any state transition on either MTCLKA or MTCLKB updates count
MTU2 (Channel 2):
- Same as MTU1, but optionally can select MTCLKC and MTCLKD instead of
MTCLKA and MTCLKB respectively
* Pulse-Direction mode:
- MTCLKA(C) updates count
- MTCLKB(D) determines direction
* Quadrature x2 B:
- MTCLKA(C) is quadrature phase A
- MTCLKB(D) is quadrature phase B
- Any state transition on MTCLKB updates count
* Quadrature x4:
- MTCLKA(C) is quadrature phase A
- MTCLKB(D) is quadrature phase B
- Any state transition on either MTCLKA(C) or MTCLKB(D) updates count
MTU3 (Channel 1 and 2 cascading):
- Same as MTU2 (but count is now 32-bit)
* Pulse-Direction mode:
- MTCLKA(C) updates count
- MTCLKB(D) determines direction
* Quadrature x2 B:
- MTCLKA(C) is quadrature phase A
- MTCLKB(D) is quadrature phase B
- Any state transition on MTCLKB updates count
* Quadrature x4:
- MTCLKA(C) is quadrature phase A
- MTCLKB(D) is quadrature phase B
- Any state transition on either MTCLKA(C) or MTCLKB(D) updates count
Is my understanding correct here? Is the selection between MTCLKA/MTCLKB
and MTCLKC/MTCLKD done in software, and should we expose it in sysfs?
William Breathitt Gray
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread
* RE: [PATCH v4 3/4] mfd: Add RZ/G2L MTU3 counter driver
2022-10-17 16:40 ` William Breathitt Gray
@ 2022-10-17 19:58 ` Biju Das
2022-10-17 23:02 ` William Breathitt Gray
0 siblings, 1 reply; 32+ messages in thread
From: Biju Das @ 2022-10-17 19:58 UTC (permalink / raw)
To: William Breathitt Gray
Cc: Philipp Zabel, Lee Jones, linux-iio, Geert Uytterhoeven,
Chris Paterson, Biju Das, Prabhakar Mahadev Lad,
linux-renesas-soc
Hi William Breathitt Gray,
> Subject: Re: [PATCH v4 3/4] mfd: Add RZ/G2L MTU3 counter driver
>
> On Mon, Oct 10, 2022 at 03:52:21PM +0100, Biju Das wrote:
> > Add RZ/G2L MTU3 counter driver. This IP supports the following phase
> > counting modes on MTU1 and MTU2 channels
> >
> > 1) 16-bit phase counting modes on MTU1 and MTU2 channels.
> > 2) 32-bit phase counting mode by cascading MTU1 and MTU2.
> >
> > This patch adds 3 counters by creating 3 logical channels
> > counter0: 16-bit phase counter on MTU1 channel
> > counter1: 16-bit phase counter on MTU2 channel
> > counter2: 32-bit phase counter by cascading MTU1 and MTU2
> > channels.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
>
> Hello Biju,
>
> We discussed some changes already for v5, but I have some additional
> comments and questions below.
OK.
>
> > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig index
> > 7329971a3bdf..fa88056224c9 100644
> > --- a/drivers/mfd/Kconfig
> > +++ b/drivers/mfd/Kconfig
> > @@ -1986,6 +1986,14 @@ config MFD_RZ_MTU3
> > To compile this driver as a module, choose M here: the
> > module will be called rz-mtu3.
> >
> > +config MFD_RZ_MTU3_CNT
> > + tristate "RZ/G2L MTU3 counter driver"
>
> This is a nitpick, but include the manufacturer name in the tristate
> string for the sake of clarity: "Renesas RZ/G2L MTU3 counter driver".
>
> > + depends on MFD_RZ_MTU3 || COMPILE_TEST
>
> I noticed you include <linux/of.h> in the rz-mtu3-cnt.c file; do you
> need to depend on OF here in the Kconfig as well?
I will take out "of.h", as there is no compatible.
>
> > +static int rz_mtu3_count_read(struct counter_device *counter,
> > + struct counter_count *count, u64 *val) {
> > + struct rz_mtu3_cnt *const priv = counter_priv(counter);
> > +
> > + if (count->id == RZ_MTU3_32_BIT_CH)
> > + *val = rz_mtu3_32bit_ch_read(priv->ch[0], RZ_MTU3_TCNTLW);
> > + else
> > + *val = rz_mtu3_16bit_ch_read(priv->ch[count->id],
> RZ_MTU3_TCNT);
>
> After considering this again, I think it'll be better to match the
> structure of the rest of the functions in this driver for consistency.
> Rather than hardcoding priv->ch[0], determine the ch_id first and pass
> that instead::
OK.
>
>
> const size_t ch_id = RZ_MTU3_GET_HW_CH(count->id);
>
> if (count->id == RZ_MTU3_32_BIT_CH)
> *val = rz_mtu3_32bit_ch_read(priv->ch[ch_id],
> RZ_MTU3_TCNTLW);
> else
> *val = rz_mtu3_16bit_ch_read(priv->ch[ch_id],
> RZ_MTU3_TCNT);
>
> > +static int rz_mtu3_count_write(struct counter_device *counter,
> > + struct counter_count *count, const u64 val) {
> > + struct rz_mtu3_cnt *const priv = counter_priv(counter);
> > + const size_t ch_id = RZ_MTU3_GET_HW_CH(count->id);
> > + u32 ceiling;
> > +
> > + mutex_lock(&priv->lock);
> > + if (count->id == RZ_MTU3_32_BIT_CH)
> > + ceiling = priv->mtu_32bit_max;
> > + else
> > + ceiling = priv->mtu_16bit_max[ch_id];
> > +
> > + if (val > ceiling) {
> > + mutex_unlock(&priv->lock);
> > + return -ERANGE;
> > + }
> > +
> > + if (count->id == RZ_MTU3_32_BIT_CH)
> > + rz_mtu3_32bit_ch_write(priv->ch[0], RZ_MTU3_TCNTLW, val);
>
> Like in count_read(), use ch_id here instead of 0 for the sake of
> consistency.
>
> > +static int rz_mtu3_count_ceiling_write(struct counter_device
> *counter,
> > + struct counter_count *count,
> > + u64 ceiling)
> > +{
> > + struct rz_mtu3_cnt *const priv = counter_priv(counter);
> > + const size_t ch_id = RZ_MTU3_GET_HW_CH(count->id);
> > +
> > + switch (count->id) {
> > + case RZ_MTU3_16_BIT_MTU1_CH:
> > + case RZ_MTU3_16_BIT_MTU2_CH:
> > + if (ceiling > U16_MAX)
> > + return -ERANGE;
> > + priv->mtu_16bit_max[ch_id] = ceiling;
> > + break;
> > + case RZ_MTU3_32_BIT_CH:
> > + if (ceiling > U32_MAX)
> > + return -ERANGE;
> > + priv->mtu_32bit_max = ceiling;
> > + break;
> > + }
> > +
> > + mutex_lock(&priv->lock);
> > + if (ceiling == 0) {
> > + /* Disable counter clear source */
> > + rz_mtu3_8bit_ch_write(priv->ch[ch_id], RZ_MTU3_TCR,
> > + RZ_MTU3_TCR_CCLR_NONE);
>
> Refer to our discussions in the v3 review thread regarding ceiling set
> to 0; in particular, don't disable the count value ceiling but rather
> limit it to a maximum value of 0.
>
> > +static int rz_mtu3_count_enable_write(struct counter_device
> *counter,
> > + struct counter_count *count, u8 enable) {
> > + struct rz_mtu3_cnt *const priv = counter_priv(counter);
> > + const size_t ch_id = RZ_MTU3_GET_HW_CH(count->id);
> > + struct rz_mtu3_channel *ch = priv->ch[ch_id];
> > + int ret = 0;
> > +
> > + if (enable) {
> > + pm_runtime_get_sync(ch->dev);
> > + ret = rz_mtu3_initialize_counter(counter, count->id);
> > + } else {
> > + rz_mtu3_terminate_counter(counter, count->id);
> > + pm_runtime_put(ch->dev);
> > + }
> Refer to our discussions in the v3 review thread regarding the
> "enable"
> Count extension; in particular, "enable" pauses/unpauses counting.
>
> > +static int rz_mtu3_action_read(struct counter_device *counter,
> > + struct counter_count *count,
> > + struct counter_synapse *synapse,
> > + enum counter_synapse_action *action) {
> > + const size_t signal_a_id = count->synapses[0].signal->id;
> > + const size_t signal_b_id = count->synapses[1].signal->id;
> > + size_t signal_c_id;
> > + size_t signal_d_id;
> > + enum counter_function function;
> > + int err;
> > +
> > + if (count->id != RZ_MTU3_16_BIT_MTU1_CH) {
> > + signal_c_id = count->synapses[2].signal->id;
> > + signal_d_id = count->synapses[3].signal->id;
> > + }
>
> The Signal ids are constants so you remove them from this function and
> use preprocessor defines instead to represent SIGNAL_A_ID,
> SIGNAL_B_ID, SIGNAL_C_ID, and SIGNAL_D_ID. Remember to set the Signal
> ids in the rz_mtu3_signals[] array accordingly.
>
> > +static struct counter_signal rz_mtu3_signals[] = {
> > + RZ_MTU3_PHASE_SIGNAL(0, "MTU1 MTCLKA"),
> > + RZ_MTU3_PHASE_SIGNAL(1, "MTU1 MTCLKB"),
> > + RZ_MTU3_PHASE_SIGNAL(2, "MTU2 MTCLKC"),
> > + RZ_MTU3_PHASE_SIGNAL(3, "MTU2 MTCLKD"), };
>
> The relationship of these Signals still has me somewhat confused so
> I'm hoping you can help me properly ironed out my understanding. This
> is how I currently understand it, so please point out any mistakes I
> have:
>
> MTU1 (Channel 1):
> * Pulse-Direction mode:
> - MTCLKA updates count
> - MTCLKB determines direction
> * Quadrature x2 B:
> - MTCLKA is quadrature phase A
> - MTCLKB is quadrature phase B
> - Any state transition on MTCLKB updates count
> * Quadrature x4:
> - MTCLKA is quadrature phase A
> - MTCLKB is quadrature phase B
> - Any state transition on either MTCLKA or MTCLKB updates count
>
> MTU2 (Channel 2):
> - Same as MTU1, but optionally can select MTCLKC and MTCLKD instead
> of
> MTCLKA and MTCLKB respectively
> * Pulse-Direction mode:
> - MTCLKA(C) updates count
> - MTCLKB(D) determines direction
> * Quadrature x2 B:
> - MTCLKA(C) is quadrature phase A
> - MTCLKB(D) is quadrature phase B
> - Any state transition on MTCLKB updates count
> * Quadrature x4:
> - MTCLKA(C) is quadrature phase A
> - MTCLKB(D) is quadrature phase B
> - Any state transition on either MTCLKA(C) or MTCLKB(D) updates
> count
>
> MTU3 (Channel 1 and 2 cascading):
> - Same as MTU2 (but count is now 32-bit)
> * Pulse-Direction mode:
> - MTCLKA(C) updates count
> - MTCLKB(D) determines direction
> * Quadrature x2 B:
> - MTCLKA(C) is quadrature phase A
> - MTCLKB(D) is quadrature phase B
> - Any state transition on MTCLKB updates count
> * Quadrature x4:
> - MTCLKA(C) is quadrature phase A
> - MTCLKB(D) is quadrature phase B
> - Any state transition on either MTCLKA(C) or MTCLKB(D) updates
> count
>
> Is my understanding correct here? Is the selection between
> MTCLKA/MTCLKB and MTCLKC/MTCLKD done in software, and should we expose
> it in sysfs?
Yes, we need to expose this to sysfs. Is it same as phase mode?
Can you please provide an example, if possible how to handle this?
Cheers,
Biju
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v4 3/4] mfd: Add RZ/G2L MTU3 counter driver
2022-10-17 19:58 ` Biju Das
@ 2022-10-17 23:02 ` William Breathitt Gray
0 siblings, 0 replies; 32+ messages in thread
From: William Breathitt Gray @ 2022-10-17 23:02 UTC (permalink / raw)
To: Biju Das
Cc: Philipp Zabel, Lee Jones, linux-iio, Geert Uytterhoeven,
Chris Paterson, Biju Das, Prabhakar Mahadev Lad,
linux-renesas-soc
[-- Attachment #1: Type: text/plain, Size: 616 bytes --]
On Mon, Oct 17, 2022 at 07:58:54PM +0000, Biju Das wrote:
> > Is my understanding correct here? Is the selection between
> > MTCLKA/MTCLKB and MTCLKC/MTCLKD done in software, and should we expose
> > it in sysfs?
>
> Yes, we need to expose this to sysfs. Is it same as phase mode?
> Can you please provide an example, if possible how to handle this?
>
> Cheers,
> Biju
Tentatively, I think you can handle it the same way as the phase
counting mode by creating a new Counter device extension; the code
should be pretty similar except the name you give it will be different.
William Breathitt Gray
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v4 1/4] dt-bindings: mfd: Document RZ/G2L MTU3a bindings
2022-10-11 14:55 ` Biju Das
2022-10-11 18:54 ` Krzysztof Kozlowski
@ 2022-10-18 7:10 ` Lee Jones
1 sibling, 0 replies; 32+ messages in thread
From: Lee Jones @ 2022-10-18 7:10 UTC (permalink / raw)
To: Biju Das
Cc: Krzysztof Kozlowski, Rob Herring, Krzysztof Kozlowski,
William Breathitt Gray, Thierry Reding, Uwe Kleine-König,
devicetree, linux-pwm, Geert Uytterhoeven, Chris Paterson,
Biju Das, Prabhakar Mahadev Lad, linux-renesas-soc
On Tue, 11 Oct 2022, Biju Das wrote:
>
> Hi Krzysztof Kozlowski,
>
> > Subject: Re: [PATCH v4 1/4] dt-bindings: mfd: Document RZ/G2L MTU3a
> > bindings
> >
> > On 10/10/2022 10:52, Biju Das wrote:
> > > The RZ/G2L multi-function timer pulse unit 3 (MTU3a) is embedded in
> > > the Renesas RZ/G2L family SoC's. It consists of eight 16-bit timer
> > > channels and one 32-bit timer channel. It supports the following
> > > functions
> > > - Counter
> > > - Timer
> > > - PWM
> > >
> > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > ---
> > > v3->v4:
> > > * Dropped counter and pwm compatibeles as they don't have any
> > resources.
> > > * Made rz-mtu3 as pwm provider.
> > > * Updated the example and description.
> > > v2->v3:
> > > * Dropped counter bindings and integrated with mfd as it has only
> > one property.
> > > * Removed "#address-cells" and "#size-cells" as it do not have
> > children with
> > > unit addresses.
> > > * Removed quotes from counter and pwm.
> > > * Provided full path for pwm bindings.
> > > * Updated the example.
> > > v1->v2:
> > > * Modelled counter and pwm as a single device that handles
> > > multiple channels.
> > > * Moved counter and pwm bindings to respective subsystems
> > > * Dropped 'bindings' from MFD binding title.
> > > * Updated the example
> > > * Changed the compatible names.
> > > ---
> > > .../bindings/mfd/renesas,rz-mtu3.yaml | 305
> > ++++++++++++++++++
> > > 1 file changed, 305 insertions(+)
> > > create mode 100644
> > > Documentation/devicetree/bindings/mfd/renesas,rz-mtu3.yaml
> >
> > This should not be in MFD. Just because some device has few features,
> > does not mean it should go to MFD... Choose either timer or pwm.
>
> MFD is for multifunction device. This IP supports multiple functions
> like timer, pwm, clock source/events. That is the reason I have added
> here. MFD is core which provides register access for client devices.
>
> For me moving it to pwm or counter is not a big problem.
> Why do you think it cannot be MFD?
Sorry for jumping in late here. I see this has been resolved.
The TL;DR is: if you're not using the MFD Core (and including
mfd/core.h), it's not an MFD. You *could* split this up into its
component parts, place them into their own subsystems and use an MFD
core driver to register them all, but as Thierry says, this is not a
hard requirement either.
--
Lee Jones [李琼斯]
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v4 3/4] mfd: Add RZ/G2L MTU3 counter driver
2022-10-10 14:52 ` [PATCH v4 3/4] mfd: Add RZ/G2L MTU3 counter driver Biju Das
2022-10-17 16:40 ` William Breathitt Gray
@ 2022-10-25 13:50 ` Geert Uytterhoeven
2022-10-25 13:58 ` Biju Das
1 sibling, 1 reply; 32+ messages in thread
From: Geert Uytterhoeven @ 2022-10-25 13:50 UTC (permalink / raw)
To: Biju Das
Cc: William Breathitt Gray, Philipp Zabel, Lee Jones, linux-iio,
Geert Uytterhoeven, Chris Paterson, Biju Das,
Prabhakar Mahadev Lad, linux-renesas-soc
Hi Biju,
On Mon, Oct 10, 2022 at 4:53 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> Add RZ/G2L MTU3 counter driver. This IP supports the following
> phase counting modes on MTU1 and MTU2 channels
>
> 1) 16-bit phase counting modes on MTU1 and MTU2 channels.
> 2) 32-bit phase counting mode by cascading MTU1 and MTU2.
>
> This patch adds 3 counters by creating 3 logical channels
> counter0: 16-bit phase counter on MTU1 channel
> counter1: 16-bit phase counter on MTU2 channel
> counter2: 32-bit phase counter by cascading MTU1 and MTU2
> channels.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Thanks for your patch!
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -1986,6 +1986,14 @@ config MFD_RZ_MTU3
> To compile this driver as a module, choose M here: the
> module will be called rz-mtu3.
>
> +config MFD_RZ_MTU3_CNT
> + tristate "RZ/G2L MTU3 counter driver"
"depends on COUNTER", else it fails to link.
> + depends on MFD_RZ_MTU3 || COMPILE_TEST
> + help
> + Enable support for MTU3 counter driver found on Renesas RZ/G2L alike
> + SoCs. This IP supports both 16-bit and 32-bit phase counting mode
> + support.
> +
> config MFD_STM32_LPTIMER
> tristate "Support for STM32 Low-Power Timer"
> depends on (ARCH_STM32 && OF) || COMPILE_TEST
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 32+ messages in thread
* RE: [PATCH v4 3/4] mfd: Add RZ/G2L MTU3 counter driver
2022-10-25 13:50 ` Geert Uytterhoeven
@ 2022-10-25 13:58 ` Biju Das
0 siblings, 0 replies; 32+ messages in thread
From: Biju Das @ 2022-10-25 13:58 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: William Breathitt Gray, Philipp Zabel, Lee Jones, linux-iio,
Geert Uytterhoeven, Chris Paterson, Biju Das,
Prabhakar Mahadev Lad, linux-renesas-soc
Hi Geert,
Thanks for the feedback.
> Subject: Re: [PATCH v4 3/4] mfd: Add RZ/G2L MTU3 counter driver
>
> Hi Biju,
>
> On Mon, Oct 10, 2022 at 4:53 PM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > Add RZ/G2L MTU3 counter driver. This IP supports the following phase
> > counting modes on MTU1 and MTU2 channels
> >
> > 1) 16-bit phase counting modes on MTU1 and MTU2 channels.
> > 2) 32-bit phase counting mode by cascading MTU1 and MTU2.
> >
> > This patch adds 3 counters by creating 3 logical channels
> > counter0: 16-bit phase counter on MTU1 channel
> > counter1: 16-bit phase counter on MTU2 channel
> > counter2: 32-bit phase counter by cascading MTU1 and MTU2
> > channels.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
>
> Thanks for your patch!
>
> > --- a/drivers/mfd/Kconfig
> > +++ b/drivers/mfd/Kconfig
> > @@ -1986,6 +1986,14 @@ config MFD_RZ_MTU3
> > To compile this driver as a module, choose M here: the
> > module will be called rz-mtu3.
> >
> > +config MFD_RZ_MTU3_CNT
> > + tristate "RZ/G2L MTU3 counter driver"
>
> "depends on COUNTER", else it fails to link.
OK, will add this dependency in counter/Kconfig.
As pwm/counter maintainers agreed to move the core to timer bindings.
and instantiate child devices using mfd_add_device().
Cheers,
Biju
>
> > + depends on MFD_RZ_MTU3 || COMPILE_TEST
> > + help
> > + Enable support for MTU3 counter driver found on Renesas
> RZ/G2L alike
> > + SoCs. This IP supports both 16-bit and 32-bit phase
> counting mode
> > + support.
> > +
> > config MFD_STM32_LPTIMER
> > tristate "Support for STM32 Low-Power Timer"
> > depends on (ARCH_STM32 && OF) || COMPILE_TEST
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 --
> geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a
> hacker. But when I'm talking to journalists I just say "programmer" or
> something like that.
> -- Linus Torvalds
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v4 4/4] mfd: Add RZ/G2L MTU3 PWM driver
2022-10-10 14:52 ` [PATCH v4 4/4] mfd: Add RZ/G2L MTU3 PWM driver Biju Das
2022-10-11 18:53 ` Krzysztof Kozlowski
@ 2022-10-25 14:05 ` Geert Uytterhoeven
2022-10-25 14:13 ` Biju Das
1 sibling, 1 reply; 32+ messages in thread
From: Geert Uytterhoeven @ 2022-10-25 14:05 UTC (permalink / raw)
To: Biju Das
Cc: Thierry Reding, Philipp Zabel, Lee Jones, Uwe Kleine-König,
linux-pwm, Geert Uytterhoeven, Chris Paterson, Biju Das,
Prabhakar Mahadev Lad, linux-renesas-soc
Hi Biju,
On Mon, Oct 10, 2022 at 4:52 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> Add support for RZ/G2L MTU3 PWM driver. The IP supports
> following PWM modes
>
> 1) PWM mode{1,2}
> 2) Reset-synchronized PWM mode
> 3) Complementary PWM mode{1,2,3}
>
> This patch adds basic pwm mode 1 support for RZ/G2L MTU3 driver
> by creating separate logical channels for each IOs.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Thanks for your patch!
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -1994,6 +1994,12 @@ config MFD_RZ_MTU3_CNT
> SoCs. This IP supports both 16-bit and 32-bit phase counting mode
> support.
>
> +config MFD_RZ_MTU3_PWM
> + tristate "Renesas RZ/G2L MTU3 PWM Timer support"
> + depends on MFD_RZ_MTU3
depends on PWM || COMPILE_TEST
Note that currently the build fails if CONFIG_PWM=n.
I have sent a patch to fix that
"[PATCH] pwm: Add missing dummy for devm_pwmchip_add()".
https://lore.kernel.org/r/12f2142991690d2b1d6890821f6e7779a4d4bdc0.1666706435.git.geert+renesas@glider.be
> + help
> + Enable support for RZ/G2L MTU3 PWM Timer controller.
> +
> config MFD_STM32_LPTIMER
> tristate "Support for STM32 Low-Power Timer"
> depends on (ARCH_STM32 && OF) || COMPILE_TEST
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 32+ messages in thread
* RE: [PATCH v4 4/4] mfd: Add RZ/G2L MTU3 PWM driver
2022-10-25 14:05 ` Geert Uytterhoeven
@ 2022-10-25 14:13 ` Biju Das
0 siblings, 0 replies; 32+ messages in thread
From: Biju Das @ 2022-10-25 14:13 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Thierry Reding, Philipp Zabel, Lee Jones, Uwe Kleine-König,
linux-pwm, Geert Uytterhoeven, Chris Paterson, Biju Das,
Prabhakar Mahadev Lad, linux-renesas-soc
Hi Geert,
Thanks for the feedback.
> Subject: Re: [PATCH v4 4/4] mfd: Add RZ/G2L MTU3 PWM driver
>
> Hi Biju,
>
> On Mon, Oct 10, 2022 at 4:52 PM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > Add support for RZ/G2L MTU3 PWM driver. The IP supports following
> PWM
> > modes
> >
> > 1) PWM mode{1,2}
> > 2) Reset-synchronized PWM mode
> > 3) Complementary PWM mode{1,2,3}
> >
> > This patch adds basic pwm mode 1 support for RZ/G2L MTU3 driver by
> > creating separate logical channels for each IOs.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
>
> Thanks for your patch!
>
> > --- a/drivers/mfd/Kconfig
> > +++ b/drivers/mfd/Kconfig
> > @@ -1994,6 +1994,12 @@ config MFD_RZ_MTU3_CNT
> > SoCs. This IP supports both 16-bit and 32-bit phase
> counting mode
> > support.
> >
> > +config MFD_RZ_MTU3_PWM
> > + tristate "Renesas RZ/G2L MTU3 PWM Timer support"
> > + depends on MFD_RZ_MTU3
>
> depends on PWM || COMPILE_TEST
OK, this will move to PWM as the core is going to be part of timer bindings
And pwm device is instantiated using mfd_add_device()
>
> Note that currently the build fails if CONFIG_PWM=n.
> I have sent a patch to fix that
> "[PATCH] pwm: Add missing dummy for devm_pwmchip_add()".
OK, looks currently no one outside pwm subsystem is accessing this function.
Cheers,
Biju
>
> > + help
> > + Enable support for RZ/G2L MTU3 PWM Timer controller.
> > +
> > config MFD_STM32_LPTIMER
> > tristate "Support for STM32 Low-Power Timer"
> > depends on (ARCH_STM32 && OF) || COMPILE_TEST
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 --
> geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a
> hacker. But when I'm talking to journalists I just say "programmer" or
> something like that.
> -- Linus Torvalds
^ permalink raw reply [flat|nested] 32+ messages in thread
end of thread, other threads:[~2022-10-25 14:14 UTC | newest]
Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-10 14:52 [PATCH v4 0/4] Add RZ/G2L MTU3a MFD, Counter and pwm driver Biju Das
2022-10-10 14:52 ` [PATCH v4 1/4] dt-bindings: mfd: Document RZ/G2L MTU3a bindings Biju Das
2022-10-11 14:45 ` Krzysztof Kozlowski
2022-10-11 14:55 ` Biju Das
2022-10-11 18:54 ` Krzysztof Kozlowski
2022-10-11 19:23 ` Biju Das
2022-10-11 20:17 ` Krzysztof Kozlowski
2022-10-11 20:31 ` Biju Das
2022-10-15 14:28 ` William Breathitt Gray
2022-10-15 15:17 ` Biju Das
2022-10-17 14:00 ` Thierry Reding
2022-10-18 7:10 ` Lee Jones
2022-10-10 14:52 ` [PATCH v4 2/4] mfd: Add RZ/G2L MTU3 driver Biju Das
2022-10-10 14:52 ` [PATCH v4 3/4] mfd: Add RZ/G2L MTU3 counter driver Biju Das
2022-10-17 16:40 ` William Breathitt Gray
2022-10-17 19:58 ` Biju Das
2022-10-17 23:02 ` William Breathitt Gray
2022-10-25 13:50 ` Geert Uytterhoeven
2022-10-25 13:58 ` Biju Das
2022-10-10 14:52 ` [PATCH v4 4/4] mfd: Add RZ/G2L MTU3 PWM driver Biju Das
2022-10-11 18:53 ` Krzysztof Kozlowski
2022-10-11 19:13 ` Biju Das
2022-10-11 20:10 ` Krzysztof Kozlowski
2022-10-11 20:18 ` Biju Das
2022-10-11 20:27 ` Krzysztof Kozlowski
2022-10-11 20:35 ` Biju Das
2022-10-11 20:37 ` Krzysztof Kozlowski
2022-10-11 20:43 ` Biju Das
2022-10-11 20:53 ` Krzysztof Kozlowski
2022-10-15 13:56 ` Biju Das
2022-10-25 14:05 ` Geert Uytterhoeven
2022-10-25 14:13 ` Biju Das
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.