[U-Boot] [PATCH V2 2/2] net: add driver for Synopsys Ethernet QoS device

[Stephen Warren]

On 10/11/2016 05:41 PM, Joe Hershberger wrote:
> On Tue, Oct 4, 2016 at 12:45 AM, Stephen Warren <swarren at wwwdotorg.org> wrote:
>> From: Stephen Warren <swarren at nvidia.com>
>>
>> This driver supports the Synopsys Designware Ethernet QoS (Quality of
>> Service) a/k/a eqos IP block, which is a different design than the HW
>> supported by the existing designware.c driver. The IP supports many
>> options for bus type, clocking/reset structure, and feature list. This
>> driver currently supports the specific configuration used in NVIDIA's
>> Tegra186 chip, but should be extensible to other combinations quite
>> easily, as explained in the source.

>> diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c

>> +/*
>> + * This represents the "Transmit Normal Descriptor (Read Format). The format

It looks like I missed a trailing " over here -->                   ^

>> + * written by HW is different, except for the OWN bit in the flags field. Field
>
> "The format written by HW is different, except for the OWN bit in the
> flags field."
>
> This sentence is confusing. Different how?  If it's written by HW,
> isn't it read by driver? Hence it's this format? What is this "Read
> Format" naming convention?

There's a descriptor in memory. SW writes data to it in one format (the 
"write format") and HW consumes that. Once HW has completed the work 
associated with the descriptor, if re-writes the entire contents in a 
completely different format (the "read format"). The only commonality 
between the two formats is the location of the OWN bit which indicates 
whether SW has written the data and HW now owns the descriptor, or 
whether HW has written the data and SW owns the descriptor. The current 
driver doesn't interpret any of the bits that HW writes except for the 
OWN bit, which tells SW that HW has finished processing the packet 
associated with the descriptor. Other fields (e.g. timestamps IIRC) are 
ignored.

"read format" and "write format" are terms from the Synopsis documentation.

>> + * naming assumes SW places uses single buffer per descriptor, rather than

There's "a" missing -->             ^

>> + * separate header/payload buffers, such that a single 64-bit pointer is used.
>
> This sentence is also odd. Is this copied from a data sheet that was
> processed through Google translate?

There are multiple possible formats for the descriptor SW writes. One 
allows two 32-bit buffer pointers (e.g. packet header and body) too be 
included in the descriptor and the other allows a single 64-bit buffer 
to be included in the descriptor.

The descriptions do perhaps rely on terminology from the Synopsis 
documentation, but without that reliance, the comment would need to 
duplicate rather a lot of the Synopsis documentation, which would be 
problematic.