Hi Wolfgang, thanks for the quick response. Comments below... On Fri, May 22, 2009 at 8:46 AM, Wolfgang Grandegger <w...@grandegger.com> wrote: > +++ net-next-2.6/Documentation/powerpc/dts-bindings/can/sja1000.txt > @@ -0,0 +1,37 @@ > +Memory mapped SJA1000 CAN controller from NXP (formerly Philips) > + > +Required properties: > + > +- compatible : should be "nxp,sja1000". > +- reg : should specify the chip select, address offset and size used > + for the chip depending on the bus it is connected to. > +- interrupts: property with a value describing the interrupt source > + (number and sensitivity) for that device. The encoding depends > + on the type of interrupt controller used.
Hmmm, "reg", "interrupts", and "interrupt-parent" are well understood properties. I don't think we need to keep boilerplate defining the meaning every time a new binding is added. (general musing; not an ack or nack of this patch) However, what should be defined is *what* the register range is (ie. one tuple; location of device registers), and what the interrupts are (ie. single tuple for device's irq line). Granted this is a trivial case, but in the case of devices with more than one address range or irq line, the meaning of each tuple is critical information. I think it would be a good pattern to establish. > +Optional properties: > + > +- interrupt-parent : the phandle for the interrupt controller that > + services interrupts for that device. Thinking further; I wouldn't even mention "interrupt-parent" here. Anyone working with this stuff must already understand irq routing. > +- clock-frequency : CAN system clock frequency in Hz, which is normally > + half of the oscillator clock frequency. If not specified, a > + default value of 8000000 (8 MHz) is used. A clock-frequency property typically refers to the bus clock frequency. Something like can-frequency would be better. > +- cdr-reg : value of the SJA1000 clock divider register according to > + the SJA1000 data sheet. If not specified, a default value of > + 0x48 is used. Ewh. The driver should be clueful enough to derive the clock divider value given both the bus and can frequencies. I don't like this property. > +- ocr-reg : value of the SJA1000 output control register according to > + the SJA1000 data sheet. If not specified, a default value of > + 0x0a is used. Ditto here; the binding should describe the usage mode; not the register settings to get the usage mode. What sort of settings will the .dts author be writing here? Cheers, g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
