Re: [PATCH 4/6] powerpc/corenet: Create the dts components for the DPAA FMan

Hello Scott,


On 05/05/2014 06:25 PM, Scott Wood wrote:

On Sat, 2014-05-03 at 05:02 -0500, Emil Medve wrote:

quoted

Hello Scott,


On 04/21/2014 05:11 PM, Scott Wood wrote:

quoted

On Fri, 2014-04-18 at 07:21 -0500, Shruti Kanetkar wrote:

quoted

+fman@400000 {
+	mdio@f1000 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		compatible = "fsl,fman-xmdio";
+		reg = <0xf1000 0x1000>;
+	};
+};

I'd like to see a complete fman binding before we start adding pieces.

The driver for the FMan 10 Gb/s MDIO has upstreamed a couple of years
ago: '9f35a73 net/fsl: introduce Freescale 10G MDIO driver', granted
without a binding writeup.

Pushing driver code through the netdev tree does not establish device
tree ABI.  Binding documents and dts files do.

Sure, ideally and formally. But upstreaming a driver represents, if
nothing else, a statement of intent to observe a device tree ABI. Via
the SDK, FSL customers are using the device tree ABI the driver de facto
establishes. I guess a driver that makes it upstream can establish an
device tree ABI

We'll re-spin adding the binding document

quoted

This patch series should probably include a
binding blurb. However, let's not gate this patchset on a complete
binding for the FMan

I at least want to see enough of the FMan binding to have confidence
that what we're adding now is correct.

I'm not sure what you're looking for. The nodes we're adding are
describing a very common CCSR space interface for quite common device blocks

quoted

As you know we don't own the FMan work and the FMan work is... not ready
for upstreaming.

I'm not asking for a driver, just a binding that describes hardware.  Is
there any reason why the fman node needs to be anywhere near as
complicated as it is in the SDK, if we're limiting it to actual hardware
description?

Is this a trick question? :-) Of course it doesn't need to be more
complicated than actual hardware. But, to repeat myself, said
description is not... ready and I don't know when it will be. Somebody
else owns pushing the bulk of FMan upstream and I'd rather not step on
their turf quite like this

Do we really need to have nodes for all the sub-blocks?

Definitely no, and internally I'm pushing to clean that up. However, you
surely remember we've been pushing from the early days of P4080 and it's
been, to put it optimistically, slow

quoted

In an attempt to make some sort of progress we've
decided to upstream the pieces that are less controversial and MDIO is
an obvious candidate

quoted

quoted

+fman@400000 {
+	mdio0: mdio@e1120 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		compatible = "fsl,fman-mdio";
+		reg = <0xe1120 0xee0>;
+	};
+};

What is the difference between "fsl,fman-mdio" and "fsl,fman-xmdio"?  I
don't see the latter on the list of compatibles in patch 3/6.

'fsl,fman-mdio' is the 1 Gb/s MDIO (Clause 22 only). 'fsl,fman-xmdio' is
the 10 Gb/s MDIO (Clause 45 only). We can respin this patch wi

"respin this patch wi..."?

Not sure where the end of that sentence went. I meant we'll re-spin with
a binding for the 10 Gb/s MDIO block

quoted

I believe 'fsl,fman-mdio' (and others on that list) was added
gratuitously as the FMan MDIO is completely compatible with the
eTSEC/gianfar MDIO driver, but we can deal with that later

It's still good to identify the specific device, even if it's believed
to be 100% compatible.

You suggesting we create new compatibles for every instance/integration
of a hardware block even though is identical with an earlier hardware
integration? Well, I guess that's been done that and now we have about 8
different compatibles that convey no real difference at all

Plus, IIRC there's been enough badness in the
eTSEC MDIO binding that it'd be good to steer clear of it.

Hmm... I guess we can leave things as they are. I wasn't going to touch
this just now anyway

quoted

quoted

Within each category, is the exact fman version discoverable from the
mdio registers?

No, but that's irrelevant as that's not the difference between the two
compatibles

It's relevant because it means the compatible string should have a block
version number in it, or at least some other way in the MDIO node to
indicate the block version.

The 1 Gb/s MDIO block doesn't track a version of its own and from a
programming interface perspective it has no visible difference since
eTSEC. The 10 Gb/s MDIO doesn't track a version of its own either and
across the existing FMan versions is identical from a programming
interface perspective

I guess we can append a 'v1.0' to the MDIO compatible(s). However, given
the SDK we'll have to support the compatibles the (already upstream)
drivers support. Dealing with all that legacy is going to be so tedious

quoted

quoted

quoted

+fman@500000 {
+	#address-cells = <1>;
+	#size-cells = <1>;
+	compatible = "simple-bus";

Why is this simple-bus?

Because that's the translation type for the FMan sub-nodes.

What do you mean by "translation type"?

I mean address translation across buses

quoted

We need it now to get the MDIO nodes probed

No.  "simple-bus" is stating an attribute of the hardware, that the
child nodes represent simple memory-mapped devices that can be used
without special bus knowledge.  I don't think that applies here.

Yes it does. The FMan sub-nodes are "simple memory-mapped devices that
can be used without special bus knowledge". Perhaps you're thinking
about the PHY devices on the MDIO bus

You can get the MDIO node probed without misusing simple-bus by adding
the fman node's compatible to the probe list in the kernel code.

I think that's gratuitous and it's been done gratuitously in the past
for CCSR space (sub-)nodes

This sort of thing is why I want to see what the rest of the fman
binding will look like.

quoted

 and we'll needed later to probe other nodes/devices that will have
standalone drivers: MAC, MURAM. etc. 

How are they truly standalone?

I meant that they have individual drivers and they are not handled by
the high-level FMan driver

The exist in service to the greater
entity that is fman.  They presumably work together in some fashion.

Some blocks can work independently. The MURAM is an example and it seems
the existing CPM/QE MURAM code allows it to be used as regular memory.
The MDIO block could handle PHY(s) for other MACs in the system.

quoted

quoted

quoted

+	/* mdio nodes for fman v3 @ 0x500000 */
+	mdio@fc000 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		reg = <0xfc000 0x1000>;
+	};
+
+	mdio@fd000 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		reg = <0xfd000 0x1000>;
+	};
+};

Where's the compatible?  Why is this file different from all the others?

The FMan v3 MDIO block (supports both Clause 22/45) is compatible with
the FMan v2 10 Gb/s MDIO (the xgmac-mdio driver). However, the driver
needs a small clean-up patch (still in internal review) that will get it
working for FMan v3 MDIO.

This suggests that it is not 100% backwards compatible.

It is. The code is just not everything it should be


Cheers,

quoted

 With that patch will add the compatible to these nodes. However, we
need these nodes now for the board level MDIO bus muxing support
(included in this patchset)

If you need these nodes now then add the compatible property now.

-Scott

_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev