Re: [PATCH v4 4/8] dt-bindings: Add doc for the Ingenic TCU drivers

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

 



Hi Rob,

Le mer. 28 mars 2018 à 18:28, Rob Herring <robh@xxxxxxxxxx> a écrit :
On Wed, Mar 28, 2018 at 10:33 AM, Paul Cercueil <paul@xxxxxxxxxxxxxxx> wrote:
 Le 2018-03-27 16:46, Rob Herring a écrit :

 On Sun, Mar 18, 2018 at 12:28:57AM +0100, Paul Cercueil wrote:

 Add documentation about how to properly use the Ingenic TCU
 (Timer/Counter Unit) drivers from devicetree.

 Signed-off-by: Paul Cercueil <paul@xxxxxxxxxxxxxxx>
 ---
.../bindings/clock/ingenic,tcu-clocks.txt | 42 ++++++++++++++++ .../bindings/interrupt-controller/ingenic,tcu.txt | 39 +++++++++++++++
  .../devicetree/bindings/mfd/ingenic,tcu.txt        | 56
 ++++++++++++++++++++++
.../devicetree/bindings/timer/ingenic,tcu.txt | 41 ++++++++++++++++
  4 files changed, 178 insertions(+)
  create mode 100644
 Documentation/devicetree/bindings/clock/ingenic,tcu-clocks.txt
  create mode 100644
Documentation/devicetree/bindings/interrupt-controller/ingenic,tcu.txt create mode 100644 Documentation/devicetree/bindings/mfd/ingenic,tcu.txt
  create mode 100644
 Documentation/devicetree/bindings/timer/ingenic,tcu.txt

v4: New patch in this series. Corresponds to V2 patches 3-4-5 with
  added content.
 +/ {
 +       tcu: mfd@10002000 {
+ compatible = "ingenic,tcu", "simple-mfd", "syscon";
 +               reg = <0x10002000 0x1000>;
 +               #address-cells = <1>;
 +               #size-cells = <1>;
 +               ranges = <0x0 0x10002000 0x1000>;
 +
 +               tcu_timer: timer@10 {
 +                       compatible = "ingenic,jz4740-tcu";
 +                       reg = <0x10 0xff0>;
 +
+ clocks = <&tcu_clk 0>, <&tcu_clk 1>, <&tcu_clk
 2>, <&tcu_clk 3>,
+ <&tcu_clk 4>, <&tcu_clk 5>,
 <&tcu_clk 6>, <&tcu_clk 7>;
+ clock-names = "timer0", "timer1", "timer2",
 "timer3",
+ "timer4", "timer5",
 "timer6", "timer7";
 +
 +                       interrupt-parent = <&tcu_irq>;
 +                       interrupts = <0 1 2 3 4 5 6 7>;


Thinking about this some more... You simply have 8 timers (and no other functions?) with some internal clock and irq controls for each timer. I
 don't think it really makes sense to create separate clock and irq
 drivers in that case. That would be like creating clock drivers for
every clock divider in timers, pwms, uarts, etc. Unless the clocks get
 exposed to other parts of the system, then there is no point.


I have 8 timers with some internal clock and IRQ controls, that can be used
 as PWM.

Please include how you plan to do the PWM support too. I need a
complete picture of the h/w, not piecemeal, evolving bindings.

Alright.

 But the TCU also controls the IRQ of the OS Timer (which is
 separate),
 as well as masking of the clocks for the OS timer and the watchdog.

The OS timer and watchdog are different blocks outside the TCU? This
doesn't seem to be the case based on the register definitions.

Their register areas are mostly separate, although contiguous. On the other hand, the watchdog and OST can be started/stopped from a bit within a TCU
register, so they're probably part of the same h/w block.

I thought having clean drivers for different frameworks working on the same regmap was cleaner than having one big-ass driver handling everything.

DT is not the only way to instantiate drivers and how one OS splits
drivers should not define your DT binding. An MFD driver can create
child devices and a single DT node can be a provider of multiple
things. It is appropriate for an MFD to have child nodes primarily
when the sub devices need their own resources defined as properties in
DT or when the sub device is an IP block reused in multiple devices.
Just to have a node per driver/provider is not what should drive the
decision.

The idea is not to have necesarily one node per driver. I just wanted to keep
it simple.

Regards,
-Paul



[Index of Archives]     [Linux MIPS Home]     [LKML Archive]     [Linux ARM Kernel]     [Linux ARM]     [Linux]     [Git]     [Yosemite News]     [Linux SCSI]     [Linux Hams]

  Powered by Linux