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

[Uwe Kleine-König]

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

Attachment: signature.asc
Description: PGP signature