Hi Chris,

On Wed, Sep 5, 2018 at 5:02 PM Chris Brandt [off-list ref] wrote:

On Wednesday, September 05, 2018 1, Geert Uytterhoeven wrote:

quoted

quoted

So, I think I need to rework this driver to add the ability to add
multiple address regions.

Your main driver for this block (clock?) can register the watchdog.
Either directly with the watchdog subsystem, or by registering an
"rza_wdt" platform device.

It would seem strange that the clock driver would register the watchdog
timer for no other reason other than where the chip designer happen to
put the register addresses.

That not that uncommon for "syscon" drivers...

Now that I look at it:
 CPG is at FCFE_0010 - FCFE_0104
MSTP is at FCFE_0400 - FCFE_0464

Nothing sits in between CPG and MSTP.

OK.

In the hardware manual, there is other 'random other stuff' that they
put in that chapter that is located in multiple address places, but that
doesn't mean I have to map them all in this driver.

OK.

I simply changed the DTS to just map CPG and MSTP and now everything is
fine. My watchdog driver is happy and this CPG/MSTP driver works the
same.

Good, I will resume review of the patch in $subject ;-)

So....I guess I didn't really have an issue after all.

You do want to:
  1. Document the two register ranges in the DT bindings,
  2. Update the driver to map both ranges on RZ/A2.
Right now it works by luck, as ioremap()'s granularity is PAGE_SIZE.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds