Re: [PATCH v1 1/2] dt-bindings: timer: Add bindings for Intel Keem Bay SoC timer

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Tue, Dec 08, 2020 at 10:12:47AM -0600, Rob Herring wrote:
> On Thu, Nov 26, 2020 at 06:34:08PM +0800, vijayakannan.ayyathurai@intel.com wrote:
> > From: Vijayakannan Ayyathurai <vijayakannan.ayyathurai@intel.com>
> > 
> > Add Device Tree bindings for the Timer IP, which used as clocksource and
> > clockevent in the Intel Keem Bay SoC.

...

> > +examples:
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +    #include <dt-bindings/interrupt-controller/irq.h>
> > +    #define KEEM_BAY_A53_TIM
> > +
> > +    timer@20330010 {
> > +        compatible = "intel,keembay-timer";
> > +        reg = <0x20330010 0xc>,
> > +              <0x20331000 0xc>;
> > +        interrupts = <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>;
> > +        clocks = <&scmi_clk KEEM_BAY_A53_TIM>;
> > +    };
> > +
> > +    counter@203300e8 {
> > +        compatible = "intel,keembay-counter";
> > +        reg = <0x203300e8 0xc>,
> > +              <0x20331000 0xc>;
> 
> You have overlapping reg regions here. Don't do that. Define the DT 
> in terms of the h/w, not how you want to split things for Linux.
> 
> It looks like a single h/w block providing multiple functions.

Actually a good catch.

Perhaps it needs to have a parent device that provides three resources (one
common and one per each of two functions) and in the driver it should consume
them accordingly. Though I'm not an expert in DT, does it sound correct from
your perspective?

> > +        clocks = <&scmi_clk KEEM_BAY_A53_TIM>;
> > +    };

-- 
With Best Regards,
Andy Shevchenko