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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxx wrote:
> > From: Vijayakannan Ayyathurai <vijayakannan.ayyathurai@xxxxxxxxx>
> > 
> > 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





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux