[PATCH v3 09/10] arch: arm: boot: dts: Introduce HPE GXP Device tree

[Hawkins, Nick]

-----Original Message-----
From: Arnd Bergmann [mailto:arnd at arndb.de] 
Sent: Friday, April 1, 2022 11:30 AM
To: Hawkins, Nick <nick.hawkins at hpe.com>
Cc: Arnd Bergmann <arnd at arndb.de>; Verdun, Jean-Marie <verdun at hpe.com>; Olof Johansson <olof at lixom.net>; soc at kernel.org; Rob Herring <robh+dt at kernel.org>; linux-arm-kernel at lists.infradead.org; devicetree at vger.kernel.org; linux-kernel at vger.kernel.org
Subject: Re: [PATCH v3 09/10] arch: arm: boot: dts: Introduce HPE GXP Device tree

On Fri, Apr 1, 2022 at 6:05 PM Hawkins, Nick <nick.hawkins at hpe.com> wrote:
> > > I don't think you can do this, if you are using the syscon regmap, you go through the regmap indirection rather than accessing the mmio register by virtual address, and this may result in some extra code in your driver, and a little runtime overhead.
> >
> > > If you prefer to avoid that, you can go back to having the timer node as the parent, but without being a syscon. In this case, the watchdog would be handled in one of these ways:
> >
> > > a) a child device gets created from the clocksource driver and bound 
> > > to the
> >     watchdog driver, which then uses a private interface between the clocksource
> >     and the watchdog to access the registers
> >
> > > b) the clocksource driver itself registers as a watchdog driver, 
> > > without
> >     having a separate driver module
> >
> > > One thing to consider is whether the register range here contains any registers that may be used in another driver, e.g. a second timer, a PWM, or a clk controller. If not, you are fairly free to pick any of these approaches.
> >
> > I will try to use the b) approach everything in that range is timer or watchdog related. There is a second timer however there are no plans on using that. Should the combined code still live inside the driver/timer directory or should it be moved to mfd?

> > I would put it into drivers/clocksource/, I don't think drivers/mtd would be any better, but there is a chance that the clocksource maintainers don't want to have the watchdog code in their tree.

While trying to discover how to creating two devices in one driver I ran across an interesting .dtsi and I was wondering if this would be a valid approach for my situation as well. The pertinent files are:
1) drivers/clocksource/timer-digicolor.c
2) arch/arm/boot/dts/cx92755.dtsi
3) drivers/watchdog/digicolor_wdt.c

Here they are just sharing the same register area:

timer at f0000fc0 {
	compatible = "cnxt,cx92755-timer";
	reg = <0xf0000fc0 0x40>;
	interrupts = <19>, <31>, <34>, <35>, <52>, <53>, <54>, <55>;
	clocks = <&main_clk>;
};

rtc at f0000c30 {
	compatible = "cnxt,cx92755-rtc";
	reg = <0xf0000c30 0x18>;
	interrupts = <25>;
};

watchdog at f0000fc0 {
	compatible = "cnxt,cx92755-wdt";
	reg = <0xf0000fc0 0x8>;
	clocks = <&main_clk>;
	timeout-sec = <15>;
};

Thanks,

-Nick Hawkins