Thanks Mark, On 16/09/13 15:10, Mark Rutland wrote: >> + >> > +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. > >> > + - 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. >> > + >> > +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? > > If you don't have a tx-mode or rx-mode, I assume you can't do > anything... Yes, driver errs out. > >> > + - 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. > >> > + - clocks : phandle of clock. > This is not just a phandle. This is a phandle + clock-specifier pair. Yep, will change it. Thanks, srini > > Cheers, -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
