RE: [PATCH v6 03/11] pwm: rzg2l-gpt: Add support for gpt linking with poeg

[Biju Das]

Hi Uwe,

> -----Original Message-----
> From: Biju Das
> Sent: 27 July 2026 08:35
> Subject: RE: [PATCH v6 03/11] pwm: rzg2l-gpt: Add support for gpt linking with poeg
> 
> Hello Uwe,
> 
> Thanks for the feedback.
> 
> > -----Original Message-----
> > From: Uwe Kleine-König <ukleinek@xxxxxxxxxx>
> > Sent: 16 July 2026 09:26
> > Subject: Re: [PATCH v6 03/11] pwm: rzg2l-gpt: Add support for gpt
> > linking with poeg
> >
> > Hello Biju,
> >
> > On Thu, Jun 04, 2026 at 10:56:33AM +0100, Biju wrote:
> > > From: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > >
> > > The General PWM Timer (GPT) is capable of detecting "dead time error
> > > and short-circuits between output pins" and send Output disable
> > > request to poeg(Port Output Enable for GPT).
> >
> > What is a dead time error?
> 
> Hardware manual does not mention any thing related to dead time error.
> 
> I believe it is standard one
> 
> "
> Dead-time error is the unwanted change in average output voltage and current caused by adding a safety
> delay between complementary switching transistors.
> 
> To prevent a short circuit ("shoot-through"), the high-side and low-side transistors in a switching leg
> must never turn on simultaneously.
> The intentional delay where both switches are off is called dead-time.
> However, this safety window distorts the ideal PWM waveform, creating an error."
> 
> >
> > > Add support for linking poeg group with gpt, so that gpt can control
> > > the output disable function by adding rzg2l_gpt_poeg_init() to parse
> > > the renesas,poegs device tree property and establish links between
> > > POEG groups (A–D) and GPT hardware channels (0–7). For each valid,
> > > enabled POEG phandle entry, the driver:
> > >  - Reads the renesas,poeg-id from the POEG node and validates it against
> > >    the supported range
> > >  - Records the GPT–POEG association in a per-chip bitmap
> > > (poeg_gpt_link)
> > >  - Configures GTINTAD to route the output disable request to the correct
> > >    POEG group
> > >  - Configures GTIOR (OADF/OBDF fields) to set both output pins to
> > >    high-impedance on an output disable event
> >
> > For my understanding: If GPT is linked to a POEG, an error detected by GPT makes the pin High-Z?
> 
> It is configurable. Currently the driver configured for High-Z as default.
> 
> b10, b9 OADF[1:0] All 0 R/W GTIOCA Pin Disable Value Setting
> 
> 0 0: Prohibit output disable
> 0 1: Set GTIOCA pin to Hi-Z on output disable
> 1 0: Set GTIOCA pin to 0 on output disable
> 1 1: Set GTIOCA pin to 1 on output disable.
> 
> >
> > > +/*
> > > + * This function links a POEG group{A,B,C,D} with a GPT
> > > +channel{0..7} and
> > > + * configures the pin for output disable.
> > > + */
> > > +static int rzg2l_gpt_poeg_init(struct platform_device *pdev,
> > > +			       struct rzg2l_gpt_chip *rzg2l_gpt) {
> > > +	const char *poeg_name = "renesas,poegs";
> > > +	struct of_phandle_args of_args;
> > > +	struct property *poegs;
> > > +	unsigned int i;
> > > +	u32 poeg_grp;
> > > +	u32 bitpos;
> > > +	int cells;
> > > +	int ret;
> > > +
> > > +	poegs = of_find_property(pdev->dev.of_node, poeg_name, NULL);
> > > +	if (!poegs)
> > > +		return 0;
> > > +
> > > +	cells = of_property_count_u32_elems(pdev->dev.of_node, poeg_name);
> >
> > It's a bit sad that of_find_property() is called twice here. But I
> > didn't spot a function that implements what
> > of_property_count_u32_elems() does for a given struct property*.
> 
> OK.
> 
> >
> > > +	if (cells < 0)
> > > +		return cells;
> > > +
> > > +	if (cells & 1)
> >
> > Maybe add a comment here like:
> >
> > 	/* poegs is a list of pairs, so cells must be even */

I believe this comment is not required if we use "num_poeg_pairs"
as pairs must be exactly 2.

Cheers,
Biju

> 
> OK.


> 
> >
> > > +		return -EINVAL;
> > > +
> > > +	cells >>= 1;
> >
> > I think a better name for `cells` from here on would be beneficial,
> > something like `num_poeg_pairs`. For before here the name isn't optimal, but I don't have a spontanious
> suggestion here. `len` comes to mind.
> 
> OK will use `num_poeg_pairs` instead of cells.