RE: [PATCH v7 2/2] pwm: Add support for RZ/G2L GPT

[Biju Das <biju.das.jz@bp.renesas.com>]

Hi Thierry Reding,

+ Rob H.

Thanks for feedback.

> Subject: Re: [PATCH v7 2/2] pwm: Add support for RZ/G2L GPT
> 
> On Wed, Sep 28, 2022 at 05:34:49PM +0000, Biju Das wrote:
> > Hi Thierry Reding,
> >
> > Thanks for the feedback.
> >
> > > Subject: Re: [PATCH v7 2/2] pwm: Add support for RZ/G2L GPT
> > >
> > > On Wed, Sep 21, 2022 at 03:57:41PM +0100, Biju Das 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>
> > > > ---
> > > > v6->v7:
> > > >  * Added the comment for cacheing rzg2l_gpt->state_period.
> > > >  * Fixed boundary values for pv and dc.
> > > >  * Added comment for modifying mode, prescaler, timer counter
> and
> > > buffer enable
> > > >    registers.
> > > >  * Fixed buffer overflow in get_state()
> > > >  * Removed unnecessary assignment of state->period value in
> > > get_state().
> > > >  * Fixed state->duty_cycle value in get_state().
> > > >  * Added a limitation for disabling the channels, when both
> > > > channels
> > > used.
> > > > v5->v6:
> > > >  * Updated macros RZG2L_GTIOR_GTIOB_OUT_HI_END_TOGGLE_CMP_MATCH
> and
> > > >    RZG2L_GTIOR_GTIOB_OUT_LO_END_TOGGLE_CMP_MATCH with
> computation
> > > >    involving FIELD_PREP macro.
> > > >  * Removed struct rzg2l_gpt_phase and started using RZG2L_GTCCR
> > > macro
> > > >    for duty_offset.
> > > >  * replaced misnomer real_period->state_period.
> > > >  * Added handling for values >= (1024 << 32) for both period
> > > >    and duty cycle.
> > > >  * Added comments for pwm {en,dis}abled by bootloader during
> probe.
> > > > v4->v5:
> > > >  * Added Hardware manual details
> > > >  * Replaced the comment GTCNT->Counter
> > > >  * Removed the macros RZG2L_GPT_IO_PER_CHANNEL and chip.npwm
> > > directly
> > > >    used in probe.
> > > >  * Removed the unsed macro RZG2L_GTPR_MAX_VALUE
> > > >  * Added driver prefix for the type name and the variable.
> > > >  * Initialization of per_channel data moved from request->probe.
> > > >  * Updated clr parameter for rzg2l_gpt_modify for Start count.
> > > >  * Started using mutex and usage_count for handling shared
> > > >    period and prescalar for the 2 channels.
> > > >  * Updated the comment cycle->period.
> > > >  * Removed clk_disable from rzg2l_gpt_reset_assert_pm_disable()
> > > >  * Replaced pc->rzg2l_gpt.
> > > >  * Updated prescale calculation.
> > > >  * Moved pm_runtime_{get_sync,put} from
> > > > {request,free}->{enable,disable}
> > > >  * Removed platform_set_drvdata as it is unused
> > > >  * Removed the variable pwm_enabled_by_bootloader
> > > >  * Added dev_err_probe in various error paths in probe.
> > > >  * Added an error message, if devm_pwmchip_add() fails.
> > > > v3->v4:
> > > >  * Changed the local variable type i from u16->u8 and
> > > prescaled_period_
> > > >    cycles from u64->u32 in calculate_prescale().
> > > >  * Replaced mul_u64_u64_div_u64()->mul_u64_u32_div()
> > > >  * Dropped the comma after the sentinel.
> > > >  * Add a variable to track pwm enabled by bootloader and added
> > > comments
> > > >    in probe().
> > > >  * Removed unnecessary rzg2l_gpt_reset_assert_pm_disable() from
> > > probe.
> > > >  * Replaced devm_clk_get()->devm_clk_get_prepared()
> > > >  * Removed devm_clk_get_optional_enabled()
> > > > 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 | 423
> > > > ++++++++++++++++++++++++++++++++++++
> > > >  3 files changed, 435 insertions(+)  create mode 100644
> > > > drivers/pwm/pwm-rzg2l-gpt.c
> > > >
> > > > diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig index
> > > > 60d13a949bc5..2723a3e9ff65 100644
> > > > --- a/drivers/pwm/Kconfig
> > > > +++ b/drivers/pwm/Kconfig
> > > > @@ -481,6 +481,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 7bf1a29f02b8..cac39b18d1ee 100644
> > > > --- a/drivers/pwm/Makefile
> > > > +++ b/drivers/pwm/Makefile
> > > > @@ -44,6 +44,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..c4e011f2a843
> > > > --- /dev/null
> > > > +++ b/drivers/pwm/pwm-rzg2l-gpt.c
> > > [...]
> > > > +static void rzg2l_gpt_write(struct rzg2l_gpt_chip *rzg2l_gpt,
> u32
> > > > +reg, u32 data) {
> > > > +	iowrite32(data, rzg2l_gpt->mmio + reg); }
> > > > +
> > > > +static u32 rzg2l_gpt_read(struct rzg2l_gpt_chip *rzg2l_gpt, u32
> > > reg)
> > > > +{
> > > > +	return ioread32(rzg2l_gpt->mmio + reg); }
> > >
> > > Do you really need to support PCI I/O space in this driver? If
> not,
> > > make this writel() and readl() instead.
> >
> > Agreed.
> >
> > >
> > > [...]
> > > > +static int rzg2l_gpt_probe(struct platform_device *pdev) {
> > > > +	struct rzg2l_gpt_chip *rzg2l_gpt;
> > > > +	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");
> > > > +
> > > > +	ret = reset_control_deassert(rzg2l_gpt->rstc);
> > > > +	if (ret)
> > > > +		return dev_err_probe(&pdev->dev, ret,
> > > > +				     "cannot deassert reset control\n");
> > > > +
> > > > +	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;
> > > > +
> > > > +	rzg2l_gpt->clk = devm_clk_get_enabled(&pdev->dev, NULL);
> > > > +	if (IS_ERR(rzg2l_gpt->clk))
> > > > +		return dev_err_probe(&pdev->dev, PTR_ERR(rzg2l_gpt-
> >clk),
> > > > +				     "cannot get clock\n");
> > > > +
> > > > +	rzg2l_gpt->rate = clk_get_rate(rzg2l_gpt->clk);
> > > > +	/*
> > > > +	 *  We need to keep the clock on, in case the bootloader
> has
> > > enabled the
> > > > +	 *  PWM and is running during probe(). A variable
> pwm_enabled_by_
> > > > +	 *  bootloader is set to true in that case and during first
> > > > +	 *  pwm_disable(), it is set to false and release the clock
> > > resource.
> > > > +	 *
> > > > +	 *  In case the pwm is not enabled by the bootloader,
> > > devm_clk_put
> > > > +	 *  disables the clk and holding a reference on the clk
> isn't
> > > needed
> > > > +	 *  because runtime-pm handles the needed enabling.
> > > > +	 */
> > >
> > > Where does this happen? I'm not aware of any code that would
> > > automatically enable clocks for runtime PM. Typically you would
> need
> > > to implement driver-specific callbacks for that to happen.
> >
> > As we are using clock domain in the PM framework and supports
> multiple
> > clocks in clock domain, a single call to pm calls such as
> > pm_runtime_get_{sync, resume) enables multiple clocks on each
> device.
> >
> > >
> > > > +	if (rzg2l_gpt_read(rzg2l_gpt, RZG2L_GTCR) & RZG2L_GTCR_CST)
> > > > +		rzg2l_gpt->pwm_enabled_by_bootloader = true;
> > > > +	else
> > > > +		devm_clk_put(&pdev->dev, rzg2l_gpt->clk);
> > >
> > > So in either case I would expect you to want to hold on to the
> clock
> > > pointer here and use that in the runtime PM callbacks.
> >
> > But the api used here is "devm_clk_get_enabled".
> > This will enable the clocks and holds the reference (for pwm enabled
> > by bootloader case) as it avoids turning "off"
> > the clock during later part of the boot process (it prevents clock
> off
> > by clk_disable_unused())
> >
> > On the other case, this api will simply waste power, so we need to
> > disable and remove the clk reference.
> >
> > >
> > > This whole business of keeping a separate variable to track this
> > > also seems a bit fishy to me because it only partially reflects in
> > > the software state what's really going on. So I think what you
> > > really want here is to call pm_runtime_set_active() before
> > > pm_runtime_enable() to make sure that your device is marked as
> such.
> > >
> >
> > I can add pm_runtime_set_active() followed by pm_runtime_enable() in
> > the next version( This will make RPM_ACTIVE active state, But clock
> is not "on"
> > as we did not call pm_runtime_get_{sync, resume) to change it to
> > RPM_RESUME state)
> >
> > > I wonder if that alone wouldn't already solve this problem. IIRC,
> > > the runtime PM infrastructure will consider a device to be runtime
> > > suspended after ->probe() by default. And if the clock is indeed
> > > managed by runtime PM somehow, then that might just cause it to
> get
> > > disabled.
> >
> > Yes, that won't solve.
> >
> > Here we have 2 cases in probe,
> >
> > in 1 case clk to be "on" after probe(PM RESUME STATE) --> pwm
> enabled
> > by bootloader and other case clk to be "off" after probe(PM
> SUSPENDING
> > state) --> pwm disabled by bootloader
> >
> > > Again, it'd be great to know how exactly runtime PM knows how to
> > > manage the clock if you don't tell it here.
> >
> > pm_runtime_get_{sync, resume) will change the state to RPM_RESUME
> and
> > pm_runtime_put will change the state to RPM_SUSPENDING.
> >
> > Please see get_state() and apply().
> >
> > > Is the clock perhaps
> > > shared between multiple IPs? Perhaps a parent device that managed
> it
> > > in runtime PM?
> >
> > No, it doesn't shared.
> >
> > Currently we need to take care of the 2 cases in probe as mentioned
> > above
> >
> > 1) pwm enabled by bootloader:-
> >
> > we are calling "devm_clk_get_enabled" which will hold reference of
> > enabled Clk( this will prevent clk_disable_unused() turning off the
> > clocks during late init)
> >
> > 2) pwm disabled by bootloader:-
> >
> > Clock is off during probe, but in apply state we will turn it on by
> > calling pm_runtime_get_sync.
> 
> I understand what and why you're doing this, but I think you're doing
> this at the wrong level of abstraction. And that's what's leading to
> this strange construct where you need to manually fiddle with the
> clock.
> If you want to abstract all of this behind PM domains, then this
> should be reflected properly within those PM domains.
> 
> That is, if the PWM has been enabled by the bootloader, then
> effectively it's had to enable the PM domain as well (at least
> conceptually, even if the bootloader may not have the same objects as
> Linux). So you need to find a way to mark the PM domain as enabled as
> a whole rather than just one of the clocks that happens to be part of
> it.
> 
> Remember, the purpose of abstraction is to hide the implementation
> details (i.e. the clock). You break that abstraction if you go and
> fiddle with the implementation details.

OK, I have added PM runtime callbacks and holding the clock reference in probe()
and added below code in probe() which will take care of both the cases.

+     pm_runtime_set_active(&pdev->dev);                                       
+	pm_runtime_enable(&pdev->dev);
+	clk_prepare_enable(rzg2l_gpt->clk);

+	ch0_en = rzg2l_gpt_is_ch_enabled(rzg2l_gpt, 0);
+	if (ch0_en)
+		pm_runtime_get_sync(&pdev->dev);
 
+	ch1_en = rzg2l_gpt_is_ch_enabled(rzg2l_gpt, 1);
+	if (ch1_en)
+		pm_runtime_get_sync(&pdev->dev);

> 
> > > > +	mutex_init(&rzg2l_gpt->lock);
> > > > +
> > > > +	rzg2l_gpt->chip.dev = &pdev->dev;
> > > > +	rzg2l_gpt->chip.ops = &rzg2l_gpt_ops;
> > > > +	rzg2l_gpt->chip.npwm = 2;
> > >
> > > The changelog above mentions that you use a shared reset because
> the
> > > reset is shared between 8 channels, but here you only expose 2.
> > > What's going on there?
> >
> > Each hwchannel has 2 IOs. Each IO is modelled as separate channel.
> > Basically, we have 8 hwchannels * 2 IO's  = 16 pwm channels in
> total.
> >
> > Please correct me if anything wrong here.
> 
> I'm asking because I recently came across a similar driver but where
> the mistake was made to describe the hardware as 4 separate devices
> with 2 channels (or, depending on SoC generation, 1 channel) per
> device.
> Looking at the device tree it's pretty obvious that in that case this
> was really a single device with 8 (or 4, depending) channels. Most of
> the time this doesn't matter and everything works, but then on some HW
> generations all of a sudden you have a shared interrupt for all 8
> channels, and now this becomes really difficult to describe because
> the interrupt needs to be shared between 4 devices, or an extra layer
> is needed to slot in the interrupt somehow.

Yes it a large IP block with 8 hardware channels. Each channel has its
own interrupts, register space and dedicated IO pins.

> 
> So I'm just trying to avoid another such situation. Looking at the DTS
> example from the binding in patch 1, you have gpt4 at 0x10048400 with
> 0x100 bytes. Does gpt3 sit at 0x10048300 with 0x100 bytes? If so, it's

Yes, Base Address of the larger IP block: 0x10048000
Each Channel Offset Address = Base Address + H’0100×m -> where m = 0 to 7.

> likely that this is really a single large IP block that you're
> artificially subdividing and that could turn into a similar issue as
> above.

Yes, you are correct. But each channel has 10 interrupts and 2 dedicated IO pins. 
It has 4 shared External trigger input pins for all channels. The output of
dedicated IO pins can be controlled by another dedicated IP called POEG which has 4 channels.
Each GPT needs to link with any 1 POEG channel for output disable control.

Looks like in our case we need separate channels rather than
combining into 1 block like option 3. I have listed 3 options.
Please let me know your feedback on these options.

Option1 (Current case):-
Channel specific enablement, but we won't be able to handle the shared cases as you mentioned.

		gpt3: pwm@10048300 {
			compatible = "renesas,r9a07g044-gpt",
				     "renesas,rzg2l-gpt";
			reg = <0 0x10048300 0 0x100>;
			#pwm-cells = <2>;
			interrupts = <GIC_SPI 257 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 258 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 259 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 260 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 261 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 262 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 263 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 264 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 265 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 266 IRQ_TYPE_EDGE_RISING>;
			interrupt-names = "ccmpa", "ccmpb", "cmpc", "cmpd",
					  "cmpe", "cmpf", "adtrga", "adtrgb",
					  "ovf", "unf";
			clocks = <&cpg CPG_MOD R9A07G044_GPT_PCLK>;
			resets =  <&cpg R9A07G044_GPT_RST_C>;
			power-domains = <&cpg>;
			status = "disabled";

##################################################
This part may be added in future, when we link poeg with gpt for Output control
     Using GPT request signal or using POEG register.

			port {
				gpt3_channel: endpoint {
				remote-endpoint = <&poeg_group_d>;
				};
			};
###################
		};

Option 2:
Channel specific enablement, We will be able to handle the shared cases as you mentioned.

	gpt: pwm@10048000 {
			compatible = "renesas,r9a07g044-gpt",
				     "renesas,rzg2l-gpt";
			reg = <0 0x10048000 0 0x800>;
			clocks = <&cpg CPG_MOD R9A07G044_GPT_PCLK>;
			resets =  <&cpg R9A07G044_GPT_RST_C>;
			power-domains = <&cpg>;
			status = "disabled";

		gpt0: pwm@0 {
			reg = <0>;
			#pwm-cells = <2>;
			interrupts = <GIC_SPI 218 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 219 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 220 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 221 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 222 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 223 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 224 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 225 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 226 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 227 IRQ_TYPE_EDGE_RISING>;
			interrupt-names = "ccmpa", "ccmpb", "cmpc", "cmpd",
					  "cmpe", "cmpf", "adtrga", "adtrgb",
					  "ovf", "unf";
			status = "disabled";

##################################################
This part may be added in future, when we link poeg with gpt for Output control
     Using GPT request signal or using POEG register.

			port {
				gpt0_channel: endpoint {
				remote-endpoint = <&poeg_group_d>;
				};
			};
###################
		};

		gpt1: pwm@100 {
			reg = <0x100>;
			#pwm-cells = <2>;
			interrupts = <GIC_SPI 231 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 232 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 233 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 234 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 235 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 236 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 237 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 238 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 239 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 240 IRQ_TYPE_EDGE_RISING>;
			interrupt-names = "ccmpa", "ccmpb", "cmpc", "cmpd",
					  "cmpe", "cmpf", "adtrga", "adtrgb",
					  "ovf", "unf";
			status = "disabled";
		};

		gpt2: pwm@200 {
			reg = <0x200>;
			#pwm-cells = <2>;
			interrupts = <GIC_SPI 244 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 245 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 246 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 247 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 248 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 249 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 250 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 251 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 252 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 253 IRQ_TYPE_EDGE_RISING>;
			interrupt-names = "ccmpa", "ccmpb", "cmpc", "cmpd",
					  "cmpe", "cmpf", "adtrga", "adtrgb",
					  "ovf", "unf";
			status = "disabled";
		};

		gpt3: pwm@300 {
			reg = <0x300>;
			#pwm-cells = <2>;
			interrupts = <GIC_SPI 257 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 258 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 259 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 260 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 261 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 262 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 263 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 264 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 265 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 266 IRQ_TYPE_EDGE_RISING>;
			interrupt-names = "ccmpa", "ccmpb", "cmpc", "cmpd",
					  "cmpe", "cmpf", "adtrga", "adtrgb",
					  "ovf", "unf";
			status = "disabled";
		};

		gpt4: pwm@400 {
			reg = <0x400>;
			#pwm-cells = <2>;
			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";
			status = "disabled";
		};

		gpt5: pwm@500 {
			reg = <0x500>;
			#pwm-cells = <2>;
			interrupts = <GIC_SPI 283 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 284 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 285 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 286 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 287 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 288 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 289 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 290 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 291 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 292 IRQ_TYPE_EDGE_RISING>;
			interrupt-names = "ccmpa", "ccmpb", "cmpc", "cmpd",
					  "cmpe", "cmpf", "adtrga", "adtrgb",
					  "ovf", "unf";
			clocks = <&cpg CPG_MOD R9A07G044_GPT_PCLK>;
			status = "disabled";
		};

		gpt6: pwm@600 {
			reg = <0x600>;
			#pwm-cells = <2>;
			interrupts = <GIC_SPI 296 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 297 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 298 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 299 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 300 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 301 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 302 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 303 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 304 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 305 IRQ_TYPE_EDGE_RISING>;
			interrupt-names = "ccmpa", "ccmpb", "cmpc", "cmpd",
					  "cmpe", "cmpf", "adtrga", "adtrgb",
					  "ovf", "unf";
			status = "disabled";
		};

		gpt7: pwm@700 {
			reg = <0x700>;
			#pwm-cells = <2>;
			interrupts = <GIC_SPI 309 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 310 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 311 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 312 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 313 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 314 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 315 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 316 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 317 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 318 IRQ_TYPE_EDGE_RISING>;
			interrupt-names = "ccmpa", "ccmpb", "cmpc", "cmpd",
					  "cmpe", "cmpf", "adtrga", "adtrgb",
					  "ovf", "unf";
			status = "disabled";
		};
	};

Option 3:
All 16 channels(8 hw channels * 2 IO) enabled by default. Associating channels with pins and linking
With POEG modules will be a problem.

	gpt: pwm@10048000 {
			compatible = "renesas,r9a07g044-gpt",
				     "renesas,rzg2l-gpt";
			reg = <0 0x10048000 0 0x800>;
			clocks = <&cpg CPG_MOD R9A07G044_GPT_PCLK>;
			resets =  <&cpg R9A07G044_GPT_RST_C>;
			power-domains = <&cpg>;
			status = "disabled";
			#pwm-cells = <2>;
			interrupts = <GIC_SPI 218 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 219 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 220 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 221 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 222 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 223 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 224 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 225 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 226 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 227 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 231 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 232 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 233 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 234 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 235 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 236 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 237 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 238 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 239 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 240 IRQ_TYPE_EDGE_RISING>,
			           <GIC_SPI 244 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 245 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 246 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 247 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 248 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 249 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 250 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 251 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 252 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 253 IRQ_TYPE_EDGE_RISING>,
			           <GIC_SPI 257 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 258 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 259 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 260 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 261 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 262 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 263 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 264 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 265 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 266 IRQ_TYPE_EDGE_RISING>,
			           <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>,
			           <GIC_SPI 283 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 284 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 285 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 286 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 287 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 288 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 289 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 290 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 291 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 292 IRQ_TYPE_EDGE_RISING>,
			           <GIC_SPI 296 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 297 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 298 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 299 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 300 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 301 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 302 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 303 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 304 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 305 IRQ_TYPE_EDGE_RISING>,
			           <GIC_SPI 309 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 310 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 311 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 312 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 313 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 314 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 315 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 316 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 317 IRQ_TYPE_EDGE_RISING>,
				     <GIC_SPI 318 IRQ_TYPE_EDGE_RISING>;
			interrupt-names = "ccmpa0", "ccmpb0", "cmpc0", "cmpd0",
					  "cmpe0", "cmpf0", "adtrga0", "adtrgb0",
					  "ovf0", "unf0";
			              "ccmpa1", "ccmpb1", "cmpc1", "cmpd1",
					  "cmpe1", "cmpf1", "adtrga1", "adtrgb1",
					  "ovf1", "unf1";
					  "ccmpa2", "ccmpb2", "cmpc2", "cmpd2",
					  "cmpe2", "cmpf2", "adtrga2", "adtrgb2",
					  "ovf2", "unf2";
					  "ccmpa3", "ccmpb3", "cmpc3", "cmpd3",
					  "cmpe3", "cmpf3", "adtrga3", "adtrgb3",
					  "ovf3", "unf3";
                                "ccmpa4", "ccmpb4", "cmpc4", "cmpd4",
					  "cmpe4", "cmpf4", "adtrga4", "adtrgb4",
					  "ovf4", "unf4";
					  "ccmpa5", "ccmpb5", "cmpc5", "cmpd5",
					  "cmpe5", "cmpf5", "adtrga5", "adtrgb5",
					  "ovf5", "unf5";
					  "ccmpa6", "ccmpb6", "cmpc6", "cmpd6",
					  "cmpe6", "cmpf6", "adtrga6", "adtrgb6",
					  "ovf6", "unf6";
					  "ccmpa7", "ccmpb7", "cmpc7", "cmpd7",
					  "cmpe7", "cmpf7", "adtrga7", "adtrgb7",
					  "ovf7", "unf7";
		};
	};

Cheers,
Biju