RE: [PATCH V4 1/8] sxgbe: Add device-tree binding support document

Mark Rutland [off-list ref] wrote :

On Wed, Mar 19, 2014 at 10:32:48PM +0000, Byungho An wrote:

quoted

Mark Rutland [off-list ref] :

quoted

On Tue, Mar 18, 2014 at 04:27:46PM +0000, Byungho An wrote:

quoted

Mark Rutland [off-list ref] :

quoted

Hi,

As a general note it's helpful for devicetree to be Cc'd on the
entire

series

quoted

(though the binding document should be a separate patch) as it
provides

useful

quoted

context for reviewing the binding.

OK.

quoted

On Tue, Mar 18, 2014 at 06:47:13AM +0000, Byungho An wrote:

quoted

From: Siva Reddy <redacted>

This patch adds binding document for SXGBE ethernet driver via

device-tree.

quoted

quoted

Signed-off-by: Siva Reddy Kallam <redacted>
Signed-off-by: Byungho An <redacted>
---
 .../devicetree/bindings/net/samsung-sxgbe.txt      |   53
++++++++++++++++++++
 1 file changed, 53 insertions(+)  create mode 100644
Documentation/devicetree/bindings/net/samsung-sxgbe.txt

diff --git
a/Documentation/devicetree/bindings/net/samsung-sxgbe.txt
b/Documentation/devicetree/bindings/net/samsung-sxgbe.txt
new file mode 100644
index 0000000..ca27947

--- /dev/null
+++ b/Documentation/devicetree/bindings/net/samsung-sxgbe.txt

@@ -0,0 +1,53 @@
+* Samsung 10G Ethernet driver (SXGBE)
+
+Required properties:
+- compatible: Should be "samsung,sxgbe-v2.0a"
+- reg: Address and length of the register set for the device
+- interrupt-parent: Should be the phandle for the interrupt
+controller
+  that services interrupts for this device
+- interrupts: Should contain the SXGBE interrupts
+  These interrupts are ordered by fixed and follows variable
+  trasmit DMA interrupts, receive DMA interrupts and lpi

interrupt.

quoted

quoted

quoted

quoted

quoted

+  index 0 - this is fixed common interrupt of SXGBE and it is
+always
+  available.
+  index 1 to 25 - 8 variable trasmit interrupts, variable 16
+receive
interrupts
+  and 1 optional lpi interrupt.
+- phy-mode: String, operation mode of the PHY interface.
+  Supported values are: "xaui", "gmii".
+- samsung,pbl: Integer, Programmable Burst Length.
+  Supported values are 1, 2, 4, 8, 16, or 32.

There's no need to abbreviate to "pbl".

Is this a property of the hardware, or configuration that the
kernel will

program

quoted

in? If the latter, why can the kernel not choose?

Yes, this is hardware property

Ok.

quoted

quoted

quoted

+- samsung,fixed-burst: Boolean, Program the DMA to use the
+fixed burst mode
+- samsung,burst-map: Integer, Program the possible bursts
+supported by sxgbe
+  This is an interger and represents allowable DMA bursts
+when fixed

burst.

quoted

quoted

+  Allowable range is 0x00-0x3F. This field is valid only when
+fixed burst is
+  enabled, otherwise ignored.

If that's the case, why not have just this property and have it
imply the

use of

quoted

fixed burst mode?

OK. It will be implemented in next patch set.

quoted

When is it necessary to use fixed burst mode?

This is the configurable mode of DMA an used internally by
hardware to fetch data from platform bus

Sure, but that doesn't describe when it is necessary. Is this the
way the

DMA

quoted

was configured at integration time, or the way the kernel should
configure

it?
It is needed when fixed length of burst is needed.
if it is not configured, the length of burst will be variable(not fixed).
Anyway, I'll add description more for it.

And when is it necessary to have a fixed length of burst?

It's up to situation. If most of data size have similar in size,  fixed burst
is better.

Is this a property of the system, or the conenction of the system to

another?
This is a choice given by SXGBE to configure it's burst mode. It can work in
Fixed and Undefined burst mode. 
SXGBE can't select the burst mode on it's own. so, to configure the burst , we
need configurable parameter from DT. 
if DT doesn't provide fixed burst, it means SXGBE driver will configure as
Undefined burst

Do you think it should be moved to optional properties?

quoted

quoted

If the latter, is it absolutely necessary for correctness to use
fixed-burst

mode?

quoted

Or is it just always sensible to use it if available?

It is not absolutely necessary, as I mentioned above.

The description above does not make this clear.

quoted

quoted

What does the driver do if fixed burst mode is not available? Would
this

work in

quoted

the presence of fixed-burt mode?

Fixed burst mode is always available so driver doesn't need to care about

it.

quoted

quoted

I'm not arguing to remove these properties. I'd just like to
understand if

all

quoted

you're describing is the presence of a feature or that the use of
the

feature is

quoted

absolutely necessary for correctness.

OK

quoted

I'm perfectly happy for Linux to always decide to use these features
if

available.

quoted

quoted

quoted

quoted

+- samsung,adv-addr-mode: Boolean, Program the DMA to use
+Enhanced address
mode.

When would this be selected, and why can the kernel not choose?

Kernel doesn't have the provision to find out a way to select this.
So, need to pass from DT

Likewise, it is always absolutely necessary, or just always sensible
to use enhanced address mode if present?

Not always necessary. When extended address mode is needed, it can be
set in the DT.

When is this needed?

It is always confusing between when and why.
It is for extended dma addressing  for future. 
when extended dma addressing is needed it can be selected.
Anyway it will be removed this series because it is not mandatory.
Should I explain about extended dma addressing?

Cheers,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in the

body of

a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at
http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html