Hi Stephen,

Thanks for your detailed explanation!

On Sat, 23 Jul 2022 at 06:02, Stephen Boyd [off-list ref] wrote:

Quoting Tomer Maimon (2022-07-19 03:04:43)

quoted

On Mon, 18 Jul 2022 at 22:14, Stephen Boyd [off-list ref] wrote:

quoted

So the clk and reset driver should be the same driver, or one driver
should register the other and use the auxiliary bus to express the
relationship. That way we know that the drivers are tightly coupled and
aren't going to stomp over each other.

I think it is very problematic to use the same driver for the reset
and the clocks also because The NPCM reset driver is an old driver
that was used also to the older NPCM BMC SoC so it will be problematic
to use the clock and reset driver in the same space.
indeed the reset and clocks are using the same memory region but they
are not using the same registers, is it not enough?
Please be aware that the NPCM reset driver is checking that it is
using the reset registers before calling I/O functions.

To put it simply, platform device drivers should use platform device
APIs. The platform device APIs hide the fact that the firmware is ACPI
or DT or nothing at all. The usage of of_address_to_resource() is
problematic.

After converting that to platform APIs you'll get janitor style cleanups
trying to convert to devm_platform_ioremap_resource(). We'll have to
discuss this again when that happens, even if there's a comment in the
code indicating we can't reserve the IO space because there's another
driver. These problems have happened in the past, fun times!

Furthermore, in DT, reg properties aren't supposed to overlap. When that
happens it usually indicates the DT is being written to describe driver
structure instead of the IP blocks that are delivered by the hardware
engineer. In this case it sounds like a combined clk and reset IP block
because they piled all the SoC glue stuff into a register range. Are
there more features in this IO range?

No, this range only combined the reset and clock together, but it
combined in a way that we cannot split it to two or even three
different registers...

I do see a way to combine the clock and the reset driver, the NPCM
reset driver is serving other NPCM BMC's.
Should we use regmap to handle the clock registers instead of ioremap?

Best regards,

Tomer