Messages in this thread Patch in this message |  | | Date | Tue, 17 Feb 2015 14:51:24 +0000 | From | Mark Rutland <> | Subject | Re: [PATCH v2 1/3] Ethernet packet sniffer: Device tree binding and vendor prefix |
| |
On Tue, Feb 17, 2015 at 02:03:31PM +0000, Stathis Voukelatos wrote: > Signed-off-by: Stathis Voukelatos <stathis.voukelatos@linn.co.uk> > --- > .../bindings/net/linn-ether-packet-sniffer.txt | 42 ++++++++++++++++++++++ > .../devicetree/bindings/vendor-prefixes.txt | 1 + > 2 files changed, 43 insertions(+) > create mode 100644 Documentation/devicetree/bindings/net/linn-ether-packet-sniffer.txt > > diff --git a/Documentation/devicetree/bindings/net/linn-ether-packet-sniffer.txt b/Documentation/devicetree/bindings/net/linn-ether-packet-sniffer.txt > new file mode 100644 > index 0000000..74bac5e > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/linn-ether-packet-sniffer.txt > @@ -0,0 +1,42 @@ > +* Linn Products Ethernet Packet Sniffer > +The module allows Ethernet packets to be parsed, matched against > +a user-defined pattern and timestamped. It sits between a 100M > +Ethernet MAC and PHY and is completely passive with respect to > +Ethernet frames. > +Matched packet bytes and timestamp values are returned through a > +FIFO. Timestamps are provided to the module through an externally > +generated Gray-encoded counter. Does this counter unit need to be enabled (or have any input clocks enabled)?
> + > +Required properties: > +- compatible : must be "linn,eth-sniffer"
Is there not a more precise name for this IP block?
> +- reg : physical addresses and sizes of registers. Must contain 3 entries: > + - registers memory space > + - TX command string memory > + - RX command string memory > +- reg-names : must contain the following 3 entries: > + "regs", "tx-ram", "rx-ram"
It would be nicer to format this as:
- reg: A list of physical address and size pairs corresponding to each entry in reg-names
- reg-names: must contain: * "regs" for the control registers * "tx-ram" for the TX command string memory * "rx-ram" for the RX command string memory
Which avoids redundancy.
The phrase "command string" sounds a bit odd. What are these used for exactly?
> +- interrupts : sniffer interrupt specifier > +- clocks : specify the system clock for the peripheral and > + the enable clock for the timestamp counter > +- clock-names : must contain the "sys" and "tstamp" entries
Similarly here clocks should just be defined in terms of clock-names.
> +- fifo-block-words : number of words in one data FIFO entry
I didn't see a data FIFO described. Is that dynamically allocated and handed to the sniffer, or does that correspond to one of the memory regions above?
> +- tstamp-hz : frequency of the timestamp counter > +- tstamp-shift : shift value for the timestamp cyclecounter struct
What exactly is this used for?
Are there any docs?
> +- tstamp-bits : width in bits of the timestamp counter > + > +Example: > + > +sniffer@1814a000 { > + compatible = "linn,eth-sniffer"; > + reg = <0x1814a000 0x100>, <0x1814a400 0x400>, > + <0x1814a800 0x400>; > + reg-names = "regs", "tx-ram", "rx-ram"; > + interrupts = <GIC_SHARED 58 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&clk_core CLK_AUDIO>, > + <&cr_periph SYS_CLK_EVENT_TIMER>; > + clock-names = "sys", "tstamp"; > + fifo-block-words = <4>; > + tstamp-hz = <52000000>; > + tstamp-shift = <27>; > + tstamp-bits = <30>;
This property wasn't documented.
As mentioned previously, I think the relation between this unit and the MAC and/or PHY needs to be explicitly described in the DT.
> +}; > diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt > index d443279..891c224 100644 > --- a/Documentation/devicetree/bindings/vendor-prefixes.txt > +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt > @@ -90,6 +90,7 @@ lacie LaCie > lantiq Lantiq Semiconductor > lenovo Lenovo Group Ltd. > lg LG Corporation > +linn Linn Products Ltd.
This addition looks fine to me. For some reason it seems to be padded with spaces instead of tabs though; is my mail server corrupting things or is that the case in the original patch?
Thanks, Mark.
|  |