[PATCH v3 2/8] pwm: rzg2l-gpt: Add info variable to struct rzg2l_gpt_chip

Biju posted 8 patches 4 months, 2 weeks ago
There is a newer version of this series
[PATCH v3 2/8] pwm: rzg2l-gpt: Add info variable to struct rzg2l_gpt_chip
Posted by Biju 4 months, 2 weeks ago
From: Biju Das <biju.das.jz@bp.renesas.com>

RZ/G3E GPT IP is similar to the one found on RZ/G2L GPT, but there are
some differences. The field width of prescalar on RZ/G3E is 4 whereas on
RZ/G2L it is 3. Add rzg2l_gpt_info variable to handle this differences.
The FIELD_PREP and FIELD_GET macro is giving compilation issue as the
parameters are not build time constants. So added Non-constant mask
variant of FIELD_GET() and FIELD_PREP().

Reviewed-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v2->v3:
 * No change.
v1->v2:
 * Collected tag.
---
 drivers/pwm/pwm-rzg2l-gpt.c | 33 ++++++++++++++++++++++++++++-----
 1 file changed, 28 insertions(+), 5 deletions(-)

diff --git a/drivers/pwm/pwm-rzg2l-gpt.c b/drivers/pwm/pwm-rzg2l-gpt.c
index 392bd129574b..1d09fb01c72f 100644
--- a/drivers/pwm/pwm-rzg2l-gpt.c
+++ b/drivers/pwm/pwm-rzg2l-gpt.c
@@ -33,6 +33,19 @@
 #include <linux/time.h>
 #include <linux/units.h>
 
+/* Non-constant mask variant of FIELD_GET() and FIELD_PREP() */
+#define field_get(_mask, _reg)	\
+({\
+	typeof(_mask) (mask) = (_mask); \
+	(((_reg) & (mask)) >> (ffs(mask) - 1)); \
+})
+
+#define field_prep(_mask, _val)	\
+({\
+	typeof(_mask) (mask) = (_mask); \
+	(((_val) << (ffs(mask) - 1)) & (mask)); \
+})
+
 #define RZG2L_GET_CH(hwpwm)	((hwpwm) / 2)
 #define RZG2L_GET_CH_OFFS(ch)	(0x100 * (ch))
 
@@ -46,7 +59,6 @@
 
 #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)
 
@@ -77,9 +89,14 @@
 #define RZG2L_MAX_SCALE_FACTOR	1024
 #define RZG2L_MAX_TICKS		((u64)U32_MAX * RZG2L_MAX_SCALE_FACTOR)
 
+struct rzg2l_gpt_info {
+	u32 gtcr_tpcs_mask;
+};
+
 struct rzg2l_gpt_chip {
 	void __iomem *mmio;
 	struct mutex lock; /* lock to protect shared channel resources */
+	const struct rzg2l_gpt_info *info;
 	unsigned long rate_khz;
 	u32 period_ticks[RZG2L_MAX_HW_CHANNELS];
 	u32 channel_request_count[RZG2L_MAX_HW_CHANNELS];
@@ -324,7 +341,7 @@ static int rzg2l_gpt_read_waveform(struct pwm_chip *chip,
 
 	guard(mutex)(&rzg2l_gpt->lock);
 	if (rzg2l_gpt_is_ch_enabled(rzg2l_gpt, pwm->hwpwm, &gtcr)) {
-		wfhw->prescale = FIELD_GET(RZG2L_GTCR_TPCS, gtcr);
+		wfhw->prescale = field_get(rzg2l_gpt->info->gtcr_tpcs_mask, gtcr);
 		wfhw->gtpr = rzg2l_gpt_read(rzg2l_gpt, RZG2L_GTPR(ch));
 		wfhw->gtccr = rzg2l_gpt_read(rzg2l_gpt, RZG2L_GTCCR(ch, sub_ch));
 		if (wfhw->gtccr > wfhw->gtpr)
@@ -364,8 +381,8 @@ static int rzg2l_gpt_write_waveform(struct pwm_chip *chip,
 		rzg2l_gpt_write(rzg2l_gpt, RZG2L_GTUDDTYC(ch), RZG2L_GTUDDTYC_UP_COUNTING);
 
 		/* Select count clock */
-		rzg2l_gpt_modify(rzg2l_gpt, RZG2L_GTCR(ch), RZG2L_GTCR_TPCS,
-				 FIELD_PREP(RZG2L_GTCR_TPCS, wfhw->prescale));
+		rzg2l_gpt_modify(rzg2l_gpt, RZG2L_GTCR(ch), rzg2l_gpt->info->gtcr_tpcs_mask,
+				 field_prep(rzg2l_gpt->info->gtcr_tpcs_mask, wfhw->prescale));
 
 		/* Set period */
 		rzg2l_gpt_write(rzg2l_gpt, RZG2L_GTPR(ch), wfhw->gtpr);
@@ -430,6 +447,8 @@ static int rzg2l_gpt_probe(struct platform_device *pdev)
 	if (IS_ERR(rzg2l_gpt->mmio))
 		return PTR_ERR(rzg2l_gpt->mmio);
 
+	rzg2l_gpt->info = of_device_get_match_data(dev);
+
 	rstc = devm_reset_control_get_exclusive_deasserted(dev, NULL);
 	if (IS_ERR(rstc))
 		return dev_err_probe(dev, PTR_ERR(rstc), "Cannot deassert reset control\n");
@@ -472,8 +491,12 @@ static int rzg2l_gpt_probe(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct rzg2l_gpt_info rzg2l_data = {
+	.gtcr_tpcs_mask = GENMASK(26, 24),
+};
+
 static const struct of_device_id rzg2l_gpt_of_table[] = {
-	{ .compatible = "renesas,rzg2l-gpt", },
+	{ .compatible = "renesas,rzg2l-gpt", .data = &rzg2l_data },
 	{ /* Sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, rzg2l_gpt_of_table);
-- 
2.43.0
Re: [PATCH v3 2/8] pwm: rzg2l-gpt: Add info variable to struct rzg2l_gpt_chip
Posted by Uwe Kleine-König 2 months, 1 week ago
Hello Biju,

thanks for your patience, now I finally come around to tackle your
series.

On Tue, Sep 23, 2025 at 03:45:06PM +0100, Biju wrote:
>  
> @@ -46,7 +59,6 @@
>  
>  #define RZG2L_GTCR_CST		BIT(0)
>  #define RZG2L_GTCR_MD		GENMASK(18, 16)
> -#define RZG2L_GTCR_TPCS		GENMASK(26, 24)

Even though this is only used once now, I wonder if it's beneficial to
keep the name to have the definitions relevant to registers all
together.
 
>  #define RZG2L_GTCR_MD_SAW_WAVE_PWM_MODE	FIELD_PREP(RZG2L_GTCR_MD, 0)
>  
> @@ -77,9 +89,14 @@
>  #define RZG2L_MAX_SCALE_FACTOR	1024
>  #define RZG2L_MAX_TICKS		((u64)U32_MAX * RZG2L_MAX_SCALE_FACTOR)
>  
> +struct rzg2l_gpt_info {
> +	u32 gtcr_tpcs_mask;

For consistency I would have called this only gtcr_tpcs without _mask.
But here I'm not entirely sure if this will be confused by the
occasional reader with the actual value. What's your thought here?

> +};
> +
>  struct rzg2l_gpt_chip {
>  	void __iomem *mmio;
>  	struct mutex lock; /* lock to protect shared channel resources */
> +	const struct rzg2l_gpt_info *info;
>  	unsigned long rate_khz;
>  	u32 period_ticks[RZG2L_MAX_HW_CHANNELS];
>  	u32 channel_request_count[RZG2L_MAX_HW_CHANNELS];

Just these two very weak suggestions. Please consider these and tell me
what you prefer. If you like to keep them as they are, that's fine for
me.

Best regards
Uwe