All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/2] Add support for RZ/G2L GPT
@ 2022-07-19 15:59 Biju Das
  2022-07-19 15:59 ` [PATCH v3 1/2] dt-bindings: pwm: Add RZ/G2L GPT binding Biju Das
  2022-07-19 15:59 ` [PATCH v3 2/2] pwm: Add support for RZ/G2L GPT Biju Das
  0 siblings, 2 replies; 5+ messages in thread
From: Biju Das @ 2022-07-19 15:59 UTC (permalink / raw)
  To: Thierry Reding, Lee Jones, Rob Herring, Krzysztof Kozlowski,
	Philipp Zabel
  Cc: Biju Das, Uwe Kleine-König, linux-pwm, devicetree,
	Geert Uytterhoeven, Chris Paterson, Biju Das,
	Prabhakar Mahadev Lad, linux-renesas-soc

RZ/G2L General PWM Timer (GPT) composed of 8 channels with 32-bit timer
(GPT32E). It supports the following functions
 * 32 bits × 8 channels
 * Up-counting or down-counting (saw waves) or up/down-counting
   (triangle waves) for each counter.
 * Clock sources independently selectable for each channel
 * Two I/O pins per channel
 * Two output compare/input capture registers per channel
 * For the two output compare/input capture registers of each channel,
   four registers are provided as buffer registers and are capable of
   operating as comparison registers when buffering is not in use.
 * In output compare operation, buffer switching can be at crests or
   troughs, enabling the generation of laterally asymmetric PWM waveforms.
 * Registers for setting up frame cycles in each channel (with capability
   for generating interrupts at overflow or underflow)
 * Generation of dead times in PWM operation
 * Synchronous starting, stopping and clearing counters for arbitrary
   channels
 * Starting, stopping, clearing and up/down counters in response to input
   level comparison
 * Starting, clearing, stopping and up/down counters in response to a
   maximum of four external triggers
 * Output pin disable function by dead time error and detected
   short-circuits between output pins
 * A/D converter start triggers can be generated (GPT32E0 to GPT32E3)
 * Enables the noise filter for input capture and external trigger
   operation

This patch series aims to add basic pwm support for RZ/G2L GPT driver
by creating separate logical channels for each IOs.

v2->v3:
 * Added Rb tag from Rob for the bindings.
 * Updated limitation section
 * Added prefix "RZG2L_" for all macros
 * Modified prescale calculation
 * Removed pwm_set_chip_data
 * Updated comment related to modifying Mode and Prescaler
 * Updated setting of prescale value in rzg2l_gpt_config()
 * Removed else branch from rzg2l_gpt_get_state()
 * removed the err label from rzg2l_gpt_apply()
 * Added devm_clk_get_optional_enabled() to retain clk on status,
   in case bootloader turns on the clk of pwm.
 * Replaced devm_reset_control_get_exclusive->devm_reset_control_get_shared
   as single reset shared between 8 channels.
V1->v2:
 * Added '|' after 'description:' to preserve formatting.
 * Removed description for pwm_cells as it is common property.
 * Changed the reg size in example from 0xa4->0x100
 * Added Rb tag from Geert for bindings.
 * Added Limitations section
 * dropped "_MASK" from the define names.
 * used named initializer for struct phase
 * Added gpt_pwm_device into a flexible array member in rzg2l_gpt_chip
 * Revised the logic for prescale
 * Added .get_state callback
 * Improved error handling in rzg2l_gpt_apply
 * Removed .remove callback
 * Tested the driver with PWM_DEBUG enabled.

RFC->v1:
 * Added Description in binding patch
 * Removed comments from reg and clock
 * replaced rzg2l_gpt_write_mask()->rzg2l_gpt_modify()
 * Added rzg2l_gpt_read() and updated macros
 * Removed dtsi patches, will send it separately

RFC:
 * https://lore.kernel.org/linux-renesas-soc/20220430075915.5036-1-biju.das.jz@bp.renesas.com/T/#t

Biju Das (2):
  dt-bindings: pwm: Add RZ/G2L GPT binding
  pwm: Add support for RZ/G2L GPT

 .../bindings/pwm/renesas,rzg2l-gpt.yaml       | 129 ++++++
 drivers/pwm/Kconfig                           |  11 +
 drivers/pwm/Makefile                          |   1 +
 drivers/pwm/pwm-rzg2l-gpt.c                   | 367 ++++++++++++++++++
 4 files changed, 508 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pwm/renesas,rzg2l-gpt.yaml
 create mode 100644 drivers/pwm/pwm-rzg2l-gpt.c


base-commit: 036ad6daa8f0fd357af7f50f9da58539eaa6f68c
-- 
2.25.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v3 1/2] dt-bindings: pwm: Add RZ/G2L GPT binding
  2022-07-19 15:59 [PATCH v3 0/2] Add support for RZ/G2L GPT Biju Das
@ 2022-07-19 15:59 ` Biju Das
  2022-07-19 15:59 ` [PATCH v3 2/2] pwm: Add support for RZ/G2L GPT Biju Das
  1 sibling, 0 replies; 5+ messages in thread
From: Biju Das @ 2022-07-19 15:59 UTC (permalink / raw)
  To: Thierry Reding, Lee Jones, Rob Herring, Krzysztof Kozlowski
  Cc: Biju Das, Uwe Kleine-König, linux-pwm, devicetree,
	Geert Uytterhoeven, Chris Paterson, Biju Das,
	Prabhakar Mahadev Lad, linux-renesas-soc, Rob Herring

Add device tree bindings for the General PWM Timer (GPT).

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Rob Herring <robh@kernel.org>
---
v2->v3:
 * Added Rb tag from Rob.
v1->v2:
 * Added '|' after 'description:' to preserve formatting.
 * Removed description for pwm_cells as it is common property.
 * Changed the reg size in example from 0xa4->0x100
 * Added Rb tag from Geert.
RFC->v1:
 * Added Description
 * Removed comments from reg and clock
---
 .../bindings/pwm/renesas,rzg2l-gpt.yaml       | 129 ++++++++++++++++++
 1 file changed, 129 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pwm/renesas,rzg2l-gpt.yaml

diff --git a/Documentation/devicetree/bindings/pwm/renesas,rzg2l-gpt.yaml b/Documentation/devicetree/bindings/pwm/renesas,rzg2l-gpt.yaml
new file mode 100644
index 000000000000..e8f7b9947eaa
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/renesas,rzg2l-gpt.yaml
@@ -0,0 +1,129 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pwm/renesas,rzg2l-gpt.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas RZ/G2L General PWM Timer (GPT)
+
+maintainers:
+  - Biju Das <biju.das.jz@bp.renesas.com>
+
+description: |
+  RZ/G2L General PWM Timer (GPT) composed of 8 channels with 32-bit timer
+  (GPT32E). It supports the following functions
+  * 32 bits × 8 channels.
+  * Up-counting or down-counting (saw waves) or up/down-counting
+    (triangle waves) for each counter.
+  * Clock sources independently selectable for each channel.
+  * Two I/O pins per channel.
+  * Two output compare/input capture registers per channel.
+  * For the two output compare/input capture registers of each channel,
+    four registers are provided as buffer registers and are capable of
+    operating as comparison registers when buffering is not in use.
+  * In output compare operation, buffer switching can be at crests or
+    troughs, enabling the generation of laterally asymmetric PWM waveforms.
+  * Registers for setting up frame cycles in each channel (with capability
+    for generating interrupts at overflow or underflow)
+  * Generation of dead times in PWM operation.
+  * Synchronous starting, stopping and clearing counters for arbitrary
+    channels.
+  * Starting, stopping, clearing and up/down counters in response to input
+    level comparison.
+  * Starting, clearing, stopping and up/down counters in response to a
+    maximum of four external triggers.
+  * Output pin disable function by dead time error and detected
+    short-circuits between output pins.
+  * A/D converter start triggers can be generated (GPT32E0 to GPT32E3)
+  * Enables the noise filter for input capture and external trigger
+    operation.
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - renesas,r9a07g044-gpt  # RZ/G2{L,LC}
+          - renesas,r9a07g054-gpt  # RZ/V2L
+      - const: renesas,rzg2l-gpt
+
+  reg:
+    maxItems: 1
+
+  '#pwm-cells':
+    const: 2
+
+  interrupts:
+    items:
+      - description: GTCCRA input capture/compare match
+      - description: GTCCRB input capture/compare
+      - description: GTCCRC compare match
+      - description: GTCCRD compare match
+      - description: GTCCRE compare match
+      - description: GTCCRF compare match
+      - description: GTADTRA compare match
+      - description: GTADTRB compare match
+      - description: GTCNT overflow/GTPR compare match
+      - description: GTCNT underflow
+
+  interrupt-names:
+    items:
+      - const: ccmpa
+      - const: ccmpb
+      - const: cmpc
+      - const: cmpd
+      - const: cmpe
+      - const: cmpf
+      - const: adtrga
+      - const: adtrgb
+      - const: ovf
+      - const: unf
+
+  clocks:
+    maxItems: 1
+
+  power-domains:
+    maxItems: 1
+
+  resets:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - interrupt-names
+  - clocks
+  - power-domains
+  - resets
+
+allOf:
+  - $ref: pwm.yaml#
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/r9a07g044-cpg.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    gpt4: pwm@10048400 {
+        compatible = "renesas,r9a07g044-gpt", "renesas,rzg2l-gpt";
+        reg = <0x10048400 0x100>;
+        interrupts = <GIC_SPI 270 IRQ_TYPE_EDGE_RISING>,
+                     <GIC_SPI 271 IRQ_TYPE_EDGE_RISING>,
+                     <GIC_SPI 272 IRQ_TYPE_EDGE_RISING>,
+                     <GIC_SPI 273 IRQ_TYPE_EDGE_RISING>,
+                     <GIC_SPI 274 IRQ_TYPE_EDGE_RISING>,
+                     <GIC_SPI 275 IRQ_TYPE_EDGE_RISING>,
+                     <GIC_SPI 276 IRQ_TYPE_EDGE_RISING>,
+                     <GIC_SPI 277 IRQ_TYPE_EDGE_RISING>,
+                     <GIC_SPI 278 IRQ_TYPE_EDGE_RISING>,
+                     <GIC_SPI 279 IRQ_TYPE_EDGE_RISING>;
+        interrupt-names = "ccmpa", "ccmpb", "cmpc", "cmpd",
+                          "cmpe", "cmpf", "adtrga", "adtrgb",
+                          "ovf", "unf";
+        clocks = <&cpg CPG_MOD R9A07G044_GPT_PCLK>;
+        power-domains = <&cpg>;
+        resets = <&cpg R9A07G044_GPT_RST_C>;
+        #pwm-cells = <2>;
+    };
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v3 2/2] pwm: Add support for RZ/G2L GPT
  2022-07-19 15:59 [PATCH v3 0/2] Add support for RZ/G2L GPT Biju Das
  2022-07-19 15:59 ` [PATCH v3 1/2] dt-bindings: pwm: Add RZ/G2L GPT binding Biju Das
@ 2022-07-19 15:59 ` Biju Das
  2022-07-21 12:41   ` Geert Uytterhoeven
  1 sibling, 1 reply; 5+ messages in thread
From: Biju Das @ 2022-07-19 15:59 UTC (permalink / raw)
  To: Thierry Reding, Lee Jones, Philipp Zabel
  Cc: Biju Das, Uwe Kleine-König, linux-pwm, Geert Uytterhoeven,
	Chris Paterson, Biju Das, Prabhakar Mahadev Lad,
	linux-renesas-soc

RZ/G2L General PWM Timer (GPT) composed of 8 channels with 32-bit timer
(GPT32E). It supports the following functions
 * 32 bits × 8 channels
 * Up-counting or down-counting (saw waves) or up/down-counting
   (triangle waves) for each counter.
 * Clock sources independently selectable for each channel
 * Two I/O pins per channel
 * Two output compare/input capture registers per channel
 * For the two output compare/input capture registers of each channel,
   four registers are provided as buffer registers and are capable of
   operating as comparison registers when buffering is not in use.
 * In output compare operation, buffer switching can be at crests or
   troughs, enabling the generation of laterally asymmetric PWM waveforms.
 * Registers for setting up frame cycles in each channel (with capability
   for generating interrupts at overflow or underflow)
 * Generation of dead times in PWM operation
 * Synchronous starting, stopping and clearing counters for arbitrary
   channels
 * Starting, stopping, clearing and up/down counters in response to input
   level comparison
 * Starting, clearing, stopping and up/down counters in response to a
   maximum of four external triggers
 * Output pin disable function by dead time error and detected
   short-circuits between output pins
 * A/D converter start triggers can be generated (GPT32E0 to GPT32E3)
 * Enables the noise filter for input capture and external trigger
   operation

This patch adds basic pwm support for RZ/G2L GPT driver by creating
separate logical channels for each IOs.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v2->v3:
 * Updated limitation section
 * Added prefix "RZG2L_" for all macros
 * Modified prescale calculation
 * Removed pwm_set_chip_data
 * Updated comment related to modifying Mode and Prescaler
 * Updated setting of prescale value in rzg2l_gpt_config()
 * Removed else branch from rzg2l_gpt_get_state()
 * removed the err label from rzg2l_gpt_apply()
 * Added devm_clk_get_optional_enabled() to retain clk on status,
   in case bootloader turns on the clk of pwm.
 * Replaced devm_reset_control_get_exclusive->devm_reset_control_get_shared
   as single reset shared between 8 channels.
v1->v2:
 * Added Limitations section
 * dropped "_MASK" from the define names.
 * used named initializer for struct phase
 * Added gpt_pwm_device into a flexible array member in rzg2l_gpt_chip
 * Revised the logic for prescale
 * Added .get_state callback
 * Improved error handling in rzg2l_gpt_apply
 * Removed .remove callback
 * Tested driver with PWM_DEBUG enabled
RFC->V1:
 * Updated macros
 * replaced rzg2l_gpt_write_mask()->rzg2l_gpt_modify()
 * Added rzg2l_gpt_read()
---
 drivers/pwm/Kconfig         |  11 ++
 drivers/pwm/Makefile        |   1 +
 drivers/pwm/pwm-rzg2l-gpt.c | 367 ++++++++++++++++++++++++++++++++++++
 3 files changed, 379 insertions(+)
 create mode 100644 drivers/pwm/pwm-rzg2l-gpt.c

diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index 904de8d61828..a6cf24cb31e0 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -471,6 +471,17 @@ config PWM_ROCKCHIP
 	  Generic PWM framework driver for the PWM controller found on
 	  Rockchip SoCs.
 
+config PWM_RZG2L_GPT
+	tristate "Renesas RZ/G2L General PWM Timer support"
+	depends on ARCH_RENESAS || COMPILE_TEST
+	depends on HAS_IOMEM
+	help
+	  This driver exposes the General PWM Timer controller found in Renesas
+	  RZ/G2L like chips through the PWM API.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called pwm-rzg2l-gpt.
+
 config PWM_SAMSUNG
 	tristate "Samsung PWM support"
 	depends on PLAT_SAMSUNG || ARCH_S5PV210 || ARCH_EXYNOS || COMPILE_TEST
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index 5c08bdb817b4..12bc2a005e24 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -43,6 +43,7 @@ obj-$(CONFIG_PWM_RASPBERRYPI_POE)	+= pwm-raspberrypi-poe.o
 obj-$(CONFIG_PWM_RCAR)		+= pwm-rcar.o
 obj-$(CONFIG_PWM_RENESAS_TPU)	+= pwm-renesas-tpu.o
 obj-$(CONFIG_PWM_ROCKCHIP)	+= pwm-rockchip.o
+obj-$(CONFIG_PWM_RZG2L_GPT)	+= pwm-rzg2l-gpt.o
 obj-$(CONFIG_PWM_SAMSUNG)	+= pwm-samsung.o
 obj-$(CONFIG_PWM_SIFIVE)	+= pwm-sifive.o
 obj-$(CONFIG_PWM_SL28CPLD)	+= pwm-sl28cpld.o
diff --git a/drivers/pwm/pwm-rzg2l-gpt.c b/drivers/pwm/pwm-rzg2l-gpt.c
new file mode 100644
index 000000000000..95e0d2fd37f6
--- /dev/null
+++ b/drivers/pwm/pwm-rzg2l-gpt.c
@@ -0,0 +1,367 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Renesas RZ/G2L General PWM Timer (GPT) driver
+ *
+ * Copyright (C) 2022 Renesas Electronics Corporation
+ *
+ * Limitations:
+ * - GTCNT must be stopped before modifying Mode and Prescaler.
+ * - When PWM is disabled, the output is driven to inactive.
+ * - While the hardware supports both polarities, the driver (for now)
+ *   only handles normal polarity.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/pwm.h>
+#include <linux/reset.h>
+#include <linux/time.h>
+
+#define RZG2L_GPT_IO_PER_CHANNEL	2
+
+#define RZG2L_GTPR_MAX_VALUE	0xFFFFFFFF
+#define RZG2L_GTCR		0x2c
+#define RZG2L_GTUDDTYC		0x30
+#define RZG2L_GTIOR		0x34
+#define RZG2L_GTBER		0x40
+#define RZG2L_GTCNT		0x48
+#define RZG2L_GTCCRA		0x4c
+#define RZG2L_GTCCRB		0x50
+#define RZG2L_GTPR		0x64
+
+#define RZG2L_GTCR_CST		BIT(0)
+#define RZG2L_GTCR_MD		GENMASK(18, 16)
+#define RZG2L_GTCR_TPCS		GENMASK(26, 24)
+
+#define RZG2L_GTCR_MD_SAW_WAVE_PWM_MODE	FIELD_PREP(RZG2L_GTCR_MD, 0)
+
+#define RZG2L_GTUDDTYC_UP	BIT(0)
+#define RZG2L_GTUDDTYC_UDF	BIT(1)
+#define RZG2L_UP_COUNTING	(RZG2L_GTUDDTYC_UP | RZG2L_GTUDDTYC_UDF)
+
+#define RZG2L_GTIOR_GTIOA	GENMASK(4, 0)
+#define RZG2L_GTIOR_GTIOB	GENMASK(20, 16)
+#define RZG2L_GTIOR_OAE		BIT(8)
+#define RZG2L_GTIOR_OBE		BIT(24)
+
+#define RZG2L_INIT_OUT_LO_OUT_LO_END_TOGGLE	0x07
+#define RZG2L_INIT_OUT_HI_OUT_HI_END_TOGGLE	0x1b
+
+#define RZG2L_GTIOR_GTIOA_OUT_HI_END_TOGGLE_CMP_MATCH \
+	(RZG2L_INIT_OUT_HI_OUT_HI_END_TOGGLE | RZG2L_GTIOR_OAE)
+#define RZG2L_GTIOR_GTIOA_OUT_LO_END_TOGGLE_CMP_MATCH \
+	(RZG2L_INIT_OUT_LO_OUT_LO_END_TOGGLE | RZG2L_GTIOR_OAE)
+#define RZG2L_GTIOR_GTIOB_OUT_HI_END_TOGGLE_CMP_MATCH \
+	((RZG2L_INIT_OUT_HI_OUT_HI_END_TOGGLE << 16) | RZG2L_GTIOR_OBE)
+#define RZG2L_GTIOR_GTIOB_OUT_LO_END_TOGGLE_CMP_MATCH \
+	((RZG2L_INIT_OUT_LO_OUT_LO_END_TOGGLE << 16) | RZG2L_GTIOR_OBE)
+
+struct phase {
+	u32 value;
+	u32 mask;
+	u32 duty_reg_offset;
+};
+
+static const struct phase phase_params[] = {
+	/* Setting for phase A */
+	{
+		.value = RZG2L_GTIOR_GTIOA_OUT_HI_END_TOGGLE_CMP_MATCH,
+		.mask = RZG2L_GTIOR_GTIOA | RZG2L_GTIOR_OAE,
+		.duty_reg_offset = RZG2L_GTCCRA,
+	},
+	/* Setting for phase B */
+	{
+		.value = RZG2L_GTIOR_GTIOB_OUT_HI_END_TOGGLE_CMP_MATCH,
+		.mask = RZG2L_GTIOR_GTIOB | RZG2L_GTIOR_OBE,
+		.duty_reg_offset = RZG2L_GTCCRB,
+	},
+};
+
+struct gpt_pwm_device {
+	const struct phase *ph;
+};
+
+struct rzg2l_gpt_chip {
+	struct pwm_chip chip;
+	void __iomem *mmio;
+	struct reset_control *rstc;
+	struct clk *clk;
+	struct gpt_pwm_device gpt[2];
+};
+
+static inline struct rzg2l_gpt_chip *to_rzg2l_gpt_chip(struct pwm_chip *chip)
+{
+	return container_of(chip, struct rzg2l_gpt_chip, chip);
+}
+
+static void rzg2l_gpt_write(struct rzg2l_gpt_chip *pc, u32 reg, u32 data)
+{
+	iowrite32(data, pc->mmio + reg);
+}
+
+static u32 rzg2l_gpt_read(struct rzg2l_gpt_chip *pc, u32 reg)
+{
+	return ioread32(pc->mmio + reg);
+}
+
+static void rzg2l_gpt_modify(struct rzg2l_gpt_chip *pc, u32 reg, u32 clr, u32 set)
+{
+	rzg2l_gpt_write(pc, reg, (rzg2l_gpt_read(pc, reg) & ~clr) | set);
+}
+
+static u8 rzg2l_calculate_prescale(struct rzg2l_gpt_chip *pc, u64 period_cycles)
+{
+	u64 prescaled_period_cycles;
+	u8 prescale;
+	u16 i;
+
+	prescaled_period_cycles = period_cycles >> 32;
+	prescale = 5;
+	/* prescale 1, 4, 16, 64, 256 and 1024 */
+	for (i = 0; i < 6; i++) {
+		if ((1 << (2 * i)) > prescaled_period_cycles) {
+			prescale = i;
+			break;
+		}
+	}
+
+	return prescale;
+}
+
+static int rzg2l_gpt_request(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+	struct rzg2l_gpt_chip *pc = to_rzg2l_gpt_chip(chip);
+	struct gpt_pwm_device *gpt;
+
+	if (pwm->hwpwm >= RZG2L_GPT_IO_PER_CHANNEL)
+		return -EINVAL;
+
+	gpt = &pc->gpt[pwm->hwpwm];
+	gpt->ph = &phase_params[pwm->hwpwm];
+
+	pm_runtime_get_sync(chip->dev);
+
+	return 0;
+}
+
+static void rzg2l_gpt_free(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+	pm_runtime_put(chip->dev);
+}
+
+static int rzg2l_gpt_enable(struct rzg2l_gpt_chip *pc)
+{
+	/* Start count */
+	rzg2l_gpt_modify(pc, RZG2L_GTCR, RZG2L_GTCR_CST, RZG2L_GTCR_CST);
+
+	return 0;
+}
+
+static void rzg2l_gpt_disable(struct rzg2l_gpt_chip *pc)
+{
+	/* Stop count, Output low on GTIOCx pin when counting stops */
+	rzg2l_gpt_modify(pc, RZG2L_GTCR, RZG2L_GTCR_CST, 0);
+}
+
+static int rzg2l_gpt_config(struct pwm_chip *chip, struct pwm_device *pwm,
+			    u64 duty_ns, u64 period_ns)
+{
+	struct rzg2l_gpt_chip *pc = to_rzg2l_gpt_chip(chip);
+	struct gpt_pwm_device *gpt = &pc->gpt[pwm->hwpwm];
+	unsigned long rate, pv, dc;
+	u64 period_cycles;
+	u8 prescale;
+
+	rate = clk_get_rate(pc->clk);
+	/*
+	 * Refuse clk rates > 1 GHz to prevent overflowing the following
+	 * calculation.
+	 */
+	if (rate > NSEC_PER_SEC)
+		return -EINVAL;
+
+	period_cycles = mul_u64_u64_div_u64(rate, period_ns, NSEC_PER_SEC);
+	prescale = rzg2l_calculate_prescale(pc, period_cycles);
+
+	pv = round_down(period_cycles >> (2 * prescale), 1 << (2 * prescale));
+	period_cycles = mul_u64_u64_div_u64(rate, duty_ns, NSEC_PER_SEC);
+	dc = round_down(period_cycles >> (2 * prescale), 1 << (2 * prescale));
+
+	/* GTCNT must be stopped before modifying Mode and Prescaler */
+	if (rzg2l_gpt_read(pc, RZG2L_GTCR) & RZG2L_GTCR_CST)
+		rzg2l_gpt_disable(pc);
+
+	/* GPT set operating mode (saw-wave up-counting) */
+	rzg2l_gpt_modify(pc, RZG2L_GTCR, RZG2L_GTCR_MD, RZG2L_GTCR_MD_SAW_WAVE_PWM_MODE);
+
+	/* Set count direction */
+	rzg2l_gpt_write(pc, RZG2L_GTUDDTYC, RZG2L_UP_COUNTING);
+
+	/* Select count clock */
+	rzg2l_gpt_modify(pc, RZG2L_GTCR, RZG2L_GTCR_TPCS, FIELD_PREP(RZG2L_GTCR_TPCS, prescale));
+
+	/* Set cycle */
+	rzg2l_gpt_write(pc, RZG2L_GTPR, pv);
+
+	/* Set duty cycle */
+	rzg2l_gpt_write(pc, gpt->ph->duty_reg_offset, dc);
+
+	/* Set initial value for counter */
+	rzg2l_gpt_write(pc, RZG2L_GTCNT, 0);
+
+	/* Set no buffer operation */
+	rzg2l_gpt_write(pc, RZG2L_GTBER, 0);
+
+	/* Enable pin output */
+	rzg2l_gpt_modify(pc, RZG2L_GTIOR, gpt->ph->mask, gpt->ph->value);
+
+	return 0;
+}
+
+static void rzg2l_gpt_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
+				struct pwm_state *state)
+{
+	struct rzg2l_gpt_chip *pc = to_rzg2l_gpt_chip(chip);
+	struct gpt_pwm_device *gpt = &pc->gpt[pwm->hwpwm];
+	unsigned long rate;
+	u8 prescale;
+	u64 tmp;
+	u32 val;
+
+	val = rzg2l_gpt_read(pc, RZG2L_GTCR);
+	state->enabled = val & RZG2L_GTCR_CST;
+	if (state->enabled) {
+		rate = clk_get_rate(pc->clk);
+		prescale = FIELD_GET(RZG2L_GTCR_TPCS, val);
+
+		val = rzg2l_gpt_read(pc, RZG2L_GTPR);
+		tmp = NSEC_PER_SEC * val << (2 * prescale);
+		state->period = DIV_ROUND_UP_ULL(tmp, rate);
+
+		val = rzg2l_gpt_read(pc, gpt->ph->duty_reg_offset);
+		tmp = NSEC_PER_SEC * val << (2 * prescale);
+		state->duty_cycle = DIV_ROUND_UP_ULL(tmp, rate);
+	}
+
+	state->polarity = PWM_POLARITY_NORMAL;
+}
+
+static int rzg2l_gpt_apply(struct pwm_chip *chip, struct pwm_device *pwm,
+			   const struct pwm_state *state)
+{
+	struct rzg2l_gpt_chip *pc = to_rzg2l_gpt_chip(chip);
+	int ret;
+
+	if (state->polarity != PWM_POLARITY_NORMAL)
+		return -EINVAL;
+
+	if (!state->enabled) {
+		rzg2l_gpt_disable(pc);
+		return 0;
+	}
+
+	ret = rzg2l_gpt_config(chip, pwm, state->duty_cycle, state->period);
+	if (ret)
+		return ret;
+
+	return rzg2l_gpt_enable(pc);
+}
+
+static const struct pwm_ops rzg2l_gpt_ops = {
+	.request = rzg2l_gpt_request,
+	.free = rzg2l_gpt_free,
+	.get_state = rzg2l_gpt_get_state,
+	.apply = rzg2l_gpt_apply,
+	.owner = THIS_MODULE,
+};
+
+static const struct of_device_id rzg2l_gpt_of_table[] = {
+	{ .compatible = "renesas,rzg2l-gpt", },
+	{ /* Sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, rzg2l_gpt_of_table);
+
+static void rzg2l_gpt_reset_assert_pm_disable(void *data)
+{
+	struct rzg2l_gpt_chip *pc = data;
+
+	pm_runtime_disable(pc->chip.dev);
+	reset_control_assert(pc->rstc);
+}
+
+static int rzg2l_gpt_probe(struct platform_device *pdev)
+{
+	struct rzg2l_gpt_chip *rzg2l_gpt;
+	struct clk *clk;
+	int ret;
+
+	rzg2l_gpt = devm_kzalloc(&pdev->dev, sizeof(*rzg2l_gpt), GFP_KERNEL);
+	if (!rzg2l_gpt)
+		return -ENOMEM;
+
+	rzg2l_gpt->mmio = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(rzg2l_gpt->mmio))
+		return PTR_ERR(rzg2l_gpt->mmio);
+
+	rzg2l_gpt->rstc = devm_reset_control_get_shared(&pdev->dev, NULL);
+	if (IS_ERR(rzg2l_gpt->rstc))
+		return dev_err_probe(&pdev->dev, PTR_ERR(rzg2l_gpt->rstc),
+				     "get reset failed\n");
+
+	rzg2l_gpt->clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(rzg2l_gpt->clk))
+		return dev_err_probe(&pdev->dev, PTR_ERR(rzg2l_gpt->clk),
+				     "cannot get clock\n");
+
+	platform_set_drvdata(pdev, rzg2l_gpt);
+
+	ret = reset_control_deassert(rzg2l_gpt->rstc);
+	if (ret) {
+		dev_err(&pdev->dev, "cannot deassert reset control: %pe\n",
+			ERR_PTR(ret));
+		return ret;
+	}
+
+	pm_runtime_enable(&pdev->dev);
+
+	ret = devm_add_action_or_reset(&pdev->dev,
+				       rzg2l_gpt_reset_assert_pm_disable,
+				       rzg2l_gpt);
+	if (ret < 0)
+		return ret;
+
+	clk = devm_clk_get_optional_enabled(&pdev->dev, NULL);
+	if (IS_ERR(clk)) {
+		rzg2l_gpt_reset_assert_pm_disable(rzg2l_gpt);
+		return dev_err_probe(&pdev->dev, PTR_ERR(clk),
+				     "clk operation failed");
+	}
+
+	if (!(rzg2l_gpt_read(rzg2l_gpt, RZG2L_GTCR) & RZG2L_GTCR_CST))
+		devm_clk_put(&pdev->dev, clk);
+
+	rzg2l_gpt->chip.dev = &pdev->dev;
+	rzg2l_gpt->chip.ops = &rzg2l_gpt_ops;
+	rzg2l_gpt->chip.npwm = RZG2L_GPT_IO_PER_CHANNEL;
+
+	return devm_pwmchip_add(&pdev->dev, &rzg2l_gpt->chip);
+}
+
+static struct platform_driver rzg2l_gpt_driver = {
+	.driver = {
+		.name = "pwm-rzg2l-gpt",
+		.of_match_table = of_match_ptr(rzg2l_gpt_of_table),
+	},
+	.probe = rzg2l_gpt_probe,
+};
+module_platform_driver(rzg2l_gpt_driver);
+
+MODULE_AUTHOR("Biju Das <biju.das.jz@bp.renesas.com>");
+MODULE_DESCRIPTION("Renesas RZ/G2L General PWM Timer (GPT) Driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:pwm-rzg2l-gpt");
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v3 2/2] pwm: Add support for RZ/G2L GPT
  2022-07-19 15:59 ` [PATCH v3 2/2] pwm: Add support for RZ/G2L GPT Biju Das
@ 2022-07-21 12:41   ` Geert Uytterhoeven
  2022-07-21 15:24     ` Biju Das
  0 siblings, 1 reply; 5+ messages in thread
From: Geert Uytterhoeven @ 2022-07-21 12:41 UTC (permalink / raw)
  To: Biju Das
  Cc: Thierry Reding, Lee Jones, Philipp Zabel, Uwe Kleine-König,
	Linux PWM List, Geert Uytterhoeven, Chris Paterson, Biju Das,
	Prabhakar Mahadev Lad, Linux-Renesas

Hi Biju,

On Tue, Jul 19, 2022 at 5:59 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> RZ/G2L General PWM Timer (GPT) composed of 8 channels with 32-bit timer
> (GPT32E). It supports the following functions
>  * 32 bits × 8 channels
>  * Up-counting or down-counting (saw waves) or up/down-counting
>    (triangle waves) for each counter.
>  * Clock sources independently selectable for each channel
>  * Two I/O pins per channel
>  * Two output compare/input capture registers per channel
>  * For the two output compare/input capture registers of each channel,
>    four registers are provided as buffer registers and are capable of
>    operating as comparison registers when buffering is not in use.
>  * In output compare operation, buffer switching can be at crests or
>    troughs, enabling the generation of laterally asymmetric PWM waveforms.
>  * Registers for setting up frame cycles in each channel (with capability
>    for generating interrupts at overflow or underflow)
>  * Generation of dead times in PWM operation
>  * Synchronous starting, stopping and clearing counters for arbitrary
>    channels
>  * Starting, stopping, clearing and up/down counters in response to input
>    level comparison
>  * Starting, clearing, stopping and up/down counters in response to a
>    maximum of four external triggers
>  * Output pin disable function by dead time error and detected
>    short-circuits between output pins
>  * A/D converter start triggers can be generated (GPT32E0 to GPT32E3)
>  * Enables the noise filter for input capture and external trigger
>    operation
>
> This patch adds basic pwm support for RZ/G2L GPT driver by creating
> separate logical channels for each IOs.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
> v2->v3:

Thanks for the update!

> --- /dev/null
> +++ b/drivers/pwm/pwm-rzg2l-gpt.c

> +static u8 rzg2l_calculate_prescale(struct rzg2l_gpt_chip *pc, u64 period_cycles)
> +{
> +       u64 prescaled_period_cycles;
> +       u8 prescale;
> +       u16 i;

Why do prescale and i differ in type?
Why not use unsigned int?

> +
> +       prescaled_period_cycles = period_cycles >> 32;

So prescaled_period_cycles can be u32?

> +       prescale = 5;
> +       /* prescale 1, 4, 16, 64, 256 and 1024 */
> +       for (i = 0; i < 6; i++) {
> +               if ((1 << (2 * i)) > prescaled_period_cycles) {
> +                       prescale = i;
> +                       break;
> +               }
> +       }
> +
> +       return prescale;
> +}

> +static int rzg2l_gpt_config(struct pwm_chip *chip, struct pwm_device *pwm,
> +                           u64 duty_ns, u64 period_ns)
> +{
> +       struct rzg2l_gpt_chip *pc = to_rzg2l_gpt_chip(chip);
> +       struct gpt_pwm_device *gpt = &pc->gpt[pwm->hwpwm];
> +       unsigned long rate, pv, dc;
> +       u64 period_cycles;
> +       u8 prescale;
> +
> +       rate = clk_get_rate(pc->clk);
> +       /*
> +        * Refuse clk rates > 1 GHz to prevent overflowing the following
> +        * calculation.
> +        */
> +       if (rate > NSEC_PER_SEC)
> +               return -EINVAL;
> +
> +       period_cycles = mul_u64_u64_div_u64(rate, period_ns, NSEC_PER_SEC);

As NSEC_PER_SEC fits in 32-bit, and we know rate does, too, you can
use the cheaper mul_u64_u32_div(period_ns, rate, NSEC_PER_SEC) instead.

> +       prescale = rzg2l_calculate_prescale(pc, period_cycles);
> +
> +       pv = round_down(period_cycles >> (2 * prescale), 1 << (2 * prescale));
> +       period_cycles = mul_u64_u64_div_u64(rate, duty_ns, NSEC_PER_SEC);

Likewise.

> +       dc = round_down(period_cycles >> (2 * prescale), 1 << (2 * prescale));

Why are pv and dc unsigned long, which is either 32-bit or 64-bit?

> +static const struct of_device_id rzg2l_gpt_of_table[] = {
> +       { .compatible = "renesas,rzg2l-gpt", },
> +       { /* Sentinel */ },

Please drop the comma after the sentinel.

> +};
> +MODULE_DEVICE_TABLE(of, rzg2l_gpt_of_table);

> +static int rzg2l_gpt_probe(struct platform_device *pdev)
> +{
> +       struct rzg2l_gpt_chip *rzg2l_gpt;
> +       struct clk *clk;
> +       int ret;
> +
> +       rzg2l_gpt = devm_kzalloc(&pdev->dev, sizeof(*rzg2l_gpt), GFP_KERNEL);
> +       if (!rzg2l_gpt)
> +               return -ENOMEM;
> +
> +       rzg2l_gpt->mmio = devm_platform_ioremap_resource(pdev, 0);
> +       if (IS_ERR(rzg2l_gpt->mmio))
> +               return PTR_ERR(rzg2l_gpt->mmio);
> +
> +       rzg2l_gpt->rstc = devm_reset_control_get_shared(&pdev->dev, NULL);
> +       if (IS_ERR(rzg2l_gpt->rstc))
> +               return dev_err_probe(&pdev->dev, PTR_ERR(rzg2l_gpt->rstc),
> +                                    "get reset failed\n");
> +
> +       rzg2l_gpt->clk = devm_clk_get(&pdev->dev, NULL);
> +       if (IS_ERR(rzg2l_gpt->clk))
> +               return dev_err_probe(&pdev->dev, PTR_ERR(rzg2l_gpt->clk),
> +                                    "cannot get clock\n");
> +
> +       platform_set_drvdata(pdev, rzg2l_gpt);
> +
> +       ret = reset_control_deassert(rzg2l_gpt->rstc);
> +       if (ret) {
> +               dev_err(&pdev->dev, "cannot deassert reset control: %pe\n",
> +                       ERR_PTR(ret));
> +               return ret;
> +       }
> +
> +       pm_runtime_enable(&pdev->dev);
> +
> +       ret = devm_add_action_or_reset(&pdev->dev,
> +                                      rzg2l_gpt_reset_assert_pm_disable,
> +                                      rzg2l_gpt);
> +       if (ret < 0)
> +               return ret;
> +
> +       clk = devm_clk_get_optional_enabled(&pdev->dev, NULL);

Why optional?
You already have a pointer to this clock?

> +       if (IS_ERR(clk)) {
> +               rzg2l_gpt_reset_assert_pm_disable(rzg2l_gpt);

Isn't this called automatically, due to devm_add_action_or_reset()?

> +               return dev_err_probe(&pdev->dev, PTR_ERR(clk),
> +                                    "clk operation failed");
> +       }
> +
> +       if (!(rzg2l_gpt_read(rzg2l_gpt, RZG2L_GTCR) & RZG2L_GTCR_CST))
> +               devm_clk_put(&pdev->dev, clk);

This is done to keep the clock enabled in case it was enabled before?
I think this deserves a comment.

> +
> +       rzg2l_gpt->chip.dev = &pdev->dev;
> +       rzg2l_gpt->chip.ops = &rzg2l_gpt_ops;
> +       rzg2l_gpt->chip.npwm = RZG2L_GPT_IO_PER_CHANNEL;
> +
> +       return devm_pwmchip_add(&pdev->dev, &rzg2l_gpt->chip);
> +}

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] 5+ messages in thread

* RE: [PATCH v3 2/2] pwm: Add support for RZ/G2L GPT
  2022-07-21 12:41   ` Geert Uytterhoeven
@ 2022-07-21 15:24     ` Biju Das
  0 siblings, 0 replies; 5+ messages in thread
From: Biju Das @ 2022-07-21 15:24 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Thierry Reding, Lee Jones, Philipp Zabel, Uwe Kleine-König,
	Linux PWM List, Geert Uytterhoeven, Chris Paterson, Biju Das,
	Prabhakar Mahadev Lad, Linux-Renesas

Hi Geert,

Thanks for the feedback.

> Subject: Re: [PATCH v3 2/2] pwm: Add support for RZ/G2L GPT
> 
> Hi Biju,
> 
> On Tue, Jul 19, 2022 at 5:59 PM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > RZ/G2L General PWM Timer (GPT) composed of 8 channels with 32-bit
> > timer (GPT32E). It supports the following functions
> >  * 32 bits × 8 channels
> >  * Up-counting or down-counting (saw waves) or up/down-counting
> >    (triangle waves) for each counter.
> >  * Clock sources independently selectable for each channel
> >  * Two I/O pins per channel
> >  * Two output compare/input capture registers per channel
> >  * For the two output compare/input capture registers of each channel,
> >    four registers are provided as buffer registers and are capable of
> >    operating as comparison registers when buffering is not in use.
> >  * In output compare operation, buffer switching can be at crests or
> >    troughs, enabling the generation of laterally asymmetric PWM
> waveforms.
> >  * Registers for setting up frame cycles in each channel (with
> capability
> >    for generating interrupts at overflow or underflow)
> >  * Generation of dead times in PWM operation
> >  * Synchronous starting, stopping and clearing counters for arbitrary
> >    channels
> >  * Starting, stopping, clearing and up/down counters in response to
> input
> >    level comparison
> >  * Starting, clearing, stopping and up/down counters in response to a
> >    maximum of four external triggers
> >  * Output pin disable function by dead time error and detected
> >    short-circuits between output pins
> >  * A/D converter start triggers can be generated (GPT32E0 to GPT32E3)
> >  * Enables the noise filter for input capture and external trigger
> >    operation
> >
> > This patch adds basic pwm support for RZ/G2L GPT driver by creating
> > separate logical channels for each IOs.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > ---
> > v2->v3:
> 
> Thanks for the update!
> 
> > --- /dev/null
> > +++ b/drivers/pwm/pwm-rzg2l-gpt.c
> 
> > +static u8 rzg2l_calculate_prescale(struct rzg2l_gpt_chip *pc, u64
> > +period_cycles) {
> > +       u64 prescaled_period_cycles;
> > +       u8 prescale;
> > +       u16 i;
> 
> Why do prescale and i differ in type?
> Why not use unsigned int?

It should be u8 for both.

prescale max value is 5, that corresponds to div by 1024.
So u8 is sufficient. Yes, I should be changed to u8

> 
> > +
> > +       prescaled_period_cycles = period_cycles >> 32;
> 
> So prescaled_period_cycles can be u32?
Yes, will change it to u32.

> 
> > +       prescale = 5;
> > +       /* prescale 1, 4, 16, 64, 256 and 1024 */
> > +       for (i = 0; i < 6; i++) {
> > +               if ((1 << (2 * i)) > prescaled_period_cycles) {
> > +                       prescale = i;
> > +                       break;
> > +               }
> > +       }
> > +
> > +       return prescale;
> > +}
> 
> > +static int rzg2l_gpt_config(struct pwm_chip *chip, struct pwm_device
> *pwm,
> > +                           u64 duty_ns, u64 period_ns) {
> > +       struct rzg2l_gpt_chip *pc = to_rzg2l_gpt_chip(chip);
> > +       struct gpt_pwm_device *gpt = &pc->gpt[pwm->hwpwm];
> > +       unsigned long rate, pv, dc;
> > +       u64 period_cycles;
> > +       u8 prescale;
> > +
> > +       rate = clk_get_rate(pc->clk);
> > +       /*
> > +        * Refuse clk rates > 1 GHz to prevent overflowing the
> following
> > +        * calculation.
> > +        */
> > +       if (rate > NSEC_PER_SEC)
> > +               return -EINVAL;
> > +
> > +       period_cycles = mul_u64_u64_div_u64(rate, period_ns,
> > + NSEC_PER_SEC);
> 
> As NSEC_PER_SEC fits in 32-bit, and we know rate does, too, you can use
> the cheaper mul_u64_u32_div(period_ns, rate, NSEC_PER_SEC) instead.

OK, agreed.

> 
> > +       prescale = rzg2l_calculate_prescale(pc, period_cycles);
> > +
> > +       pv = round_down(period_cycles >> (2 * prescale), 1 << (2 *
> prescale));
> > +       period_cycles = mul_u64_u64_div_u64(rate, duty_ns,
> > + NSEC_PER_SEC);
> 
> Likewise.
OK.
> 
> > +       dc = round_down(period_cycles >> (2 * prescale), 1 << (2 *
> > + prescale));
> 
> Why are pv and dc unsigned long, which is either 32-bit or 64-bit?

It should be 64 bit(aarch64). 

Our timer is 32 bit and we have 100 MHz clock
With this max period achievable is 42 sec and min is 0.1 nsec.

Max -> (2^32 /100M) = 42 sec = 42 sec which is 32 bit.
Min -> 1 / 100M = 0.1 nsec.

Since we have dividers up to 1024,

We can have max period of 42 * 1024 sec = 43000 * 10^9 nano secs which is more than 32-bit.


> 
> > +static const struct of_device_id rzg2l_gpt_of_table[] = {
> > +       { .compatible = "renesas,rzg2l-gpt", },
> > +       { /* Sentinel */ },
> 
> Please drop the comma after the sentinel.

OK

> 
> > +};
> > +MODULE_DEVICE_TABLE(of, rzg2l_gpt_of_table);
> 
> > +static int rzg2l_gpt_probe(struct platform_device *pdev) {
> > +       struct rzg2l_gpt_chip *rzg2l_gpt;
> > +       struct clk *clk;
> > +       int ret;
> > +
> > +       rzg2l_gpt = devm_kzalloc(&pdev->dev, sizeof(*rzg2l_gpt),
> GFP_KERNEL);
> > +       if (!rzg2l_gpt)
> > +               return -ENOMEM;
> > +
> > +       rzg2l_gpt->mmio = devm_platform_ioremap_resource(pdev, 0);
> > +       if (IS_ERR(rzg2l_gpt->mmio))
> > +               return PTR_ERR(rzg2l_gpt->mmio);
> > +
> > +       rzg2l_gpt->rstc = devm_reset_control_get_shared(&pdev->dev,
> NULL);
> > +       if (IS_ERR(rzg2l_gpt->rstc))
> > +               return dev_err_probe(&pdev->dev, PTR_ERR(rzg2l_gpt-
> >rstc),
> > +                                    "get reset failed\n");
> > +
> > +       rzg2l_gpt->clk = devm_clk_get(&pdev->dev, NULL);
> > +       if (IS_ERR(rzg2l_gpt->clk))
> > +               return dev_err_probe(&pdev->dev, PTR_ERR(rzg2l_gpt-
> >clk),
> > +                                    "cannot get clock\n");
> > +
> > +       platform_set_drvdata(pdev, rzg2l_gpt);
> > +
> > +       ret = reset_control_deassert(rzg2l_gpt->rstc);
> > +       if (ret) {
> > +               dev_err(&pdev->dev, "cannot deassert reset control:
> %pe\n",
> > +                       ERR_PTR(ret));
> > +               return ret;
> > +       }
> > +
> > +       pm_runtime_enable(&pdev->dev);
> > +
> > +       ret = devm_add_action_or_reset(&pdev->dev,
> > +
> rzg2l_gpt_reset_assert_pm_disable,
> > +                                      rzg2l_gpt);
> > +       if (ret < 0)
> > +               return ret;
> > +
> > +       clk = devm_clk_get_optional_enabled(&pdev->dev, NULL);
> 
> Why optional?
> You already have a pointer to this clock?

My bad, will use non_optional one.

> 
> > +       if (IS_ERR(clk)) {
> > +               rzg2l_gpt_reset_assert_pm_disable(rzg2l_gpt);
> 
> Isn't this called automatically, due to devm_add_action_or_reset()?

Yes, will remove it.
> 
> > +               return dev_err_probe(&pdev->dev, PTR_ERR(clk),
> > +                                    "clk operation failed");
> > +       }
> > +
> > +       if (!(rzg2l_gpt_read(rzg2l_gpt, RZG2L_GTCR) & RZG2L_GTCR_CST))
> > +               devm_clk_put(&pdev->dev, clk);
> 
> This is done to keep the clock enabled in case it was enabled before?
> I think this deserves a comment.

OK will add comment.

Cheers,
Biju

> 
> > +
> > +       rzg2l_gpt->chip.dev = &pdev->dev;
> > +       rzg2l_gpt->chip.ops = &rzg2l_gpt_ops;
> > +       rzg2l_gpt->chip.npwm = RZG2L_GPT_IO_PER_CHANNEL;
> > +
> > +       return devm_pwmchip_add(&pdev->dev, &rzg2l_gpt->chip); }
> 
> 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] 5+ messages in thread

end of thread, other threads:[~2022-07-21 15:24 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-19 15:59 [PATCH v3 0/2] Add support for RZ/G2L GPT Biju Das
2022-07-19 15:59 ` [PATCH v3 1/2] dt-bindings: pwm: Add RZ/G2L GPT binding Biju Das
2022-07-19 15:59 ` [PATCH v3 2/2] pwm: Add support for RZ/G2L GPT Biju Das
2022-07-21 12:41   ` Geert Uytterhoeven
2022-07-21 15:24     ` 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.