On Thu, Mar 31, 2022 at 11:09 PM Hawkins, Nick <nick.hawkins@xxxxxxx> wrote: > On Thu, Mar 31, 2022 at 12:27 AM Hawkins, Nick <nick.hawkins@xxxxxxx> wrote: > > On Tue, Mar 29, 2022 at 9:38 PM Hawkins, Nick <nick.hawkins@xxxxxxx>> wrote: > > I'd have to study the other examples myself to see what is most common. > > > My feeling would be that it's better to either have a "hpe,gxp-timer" parent device with a watchdog child but no syscon, or to have a syscon/simple-mfd parent with both the timer and the watchdog as children. > > Arnd, thanks for the feedback. I am trying to use the approach you recommend where you have a syscon/simple-mfd parent with watchdog and timer as children. > > st: chip-controller@80 { > compatible = "hpe,gxp-ctrl-st","syscon","simple-mfd"; > reg = <0x80 0x16>; > > timer0: timer { > compatible = "hpe,gxp-timer"; > interrupts = <0>; > interrupt-parent = <&vic0>; > clocks = <&ppuclk>; > clock-names = "ppuclk"; > }; > > watchdog { > compatible = "hpe,gxp-wdt"; > }; > }; > > This compiles without any errors but I do have some questions about accessing the regmap in both drivers, specifically the timer code. How do you use a regmap with clocksource_mmio_init? I tried searching through the codebase and could not find the answer. I assume I am missing some vital step. 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. Arnd