On Wed, Sep 18, 2013 at 11:11:29AM +0100, Srinivas KANDAGATLA wrote:

Thanks Mark,

On 16/09/13 15:10, Mark Rutland wrote:

quoted

quoted

+

quoted

+Required properties:
+       - compatible: should be "st,comms-irb".

This should probably say "should contain" rather than "should be". There
may be future vairants of this device, which will also have a more
specific compatible string.

Ok, will change it to the suggest.

quoted

quoted

quoted

+       - reg: base physical address of the controller and length of memory
+       mapped  region.
+       - interrupts: interrupt number to the cpu. The interrupt specifier
+       format depends on the interrupt controller parent.

I don't like the phrase "interrupt number to the cpu". We already have
the term interrupt-specifier to precisely define this. How about:

- interrupts: interrupt-specifier for the sole interrupt generated by
              the device.

TBH, I did copy them from one of the existing bindings docs.
I will change it.

There's a lot of inconsistency with existing docs, it would be nice to
use consistent, correct terminology going forward :)

quoted

quoted

quoted

+
+Optional properties:
+       - rx-mode: can be "infrared" or "uhf".
+       - tx-mode: should be "infrared".

Are these required to use rx/tx?

Yes, these are required for driver to be in rx/tx mode.

One of them can be optional depending on the board setup.
So, Is it ok to move such properties to required properties section?

I'd probably just make a note on each stating what they mean (i.e. that
rx-mode should be present iff the rx pins are wired up).

quoted

If you don't have a tx-mode or rx-mode, I assume you can't do
anything...

Yes, driver errs out.

quoted

quoted

quoted

+       - pinctrl-names, pinctrl-0: the pincontrol settings to configure
+       muxing properly for IRB pins.

If we're expecting names, the names we expect should be defined.

quoted

quoted

+       - clocks : phandle of clock.

This is not just a phandle. This is a phandle + clock-specifier pair.

Yep, will change it.

Cheers.

Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html