lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <20150217145124.GM8994@leverpostej> Date: Tue, 17 Feb 2015 14:51:24 +0000 From: Mark Rutland <mark.rutland@....com> To: Stathis Voukelatos <stathis.voukelatos@...n.co.uk> Cc: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>, "abrestic@...omium.org" <abrestic@...omium.org> 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@...n.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@...4a000 { > + 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. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists