[PATCH v2] Device tree bindings for Xilinx devices

[PATCH v2] Device tree bindings for Xilinx devices

[Grant Likely]

From: Grant Likely <grant.likely@secretlab.ca>

Here's my second version of xilinx device tree bindings.  Please review
and comment.  I'd like to push these out to Paulus in the next couple
of days.

Cheers,
g.

Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
---

 Documentation/powerpc/booting-without-of.txt |   55 ++++++++++++++++++++++++++
 1 files changed, 55 insertions(+), 0 deletions(-)

diff --git a/Documentation/powerpc/booting-without-of.txt b/Documentation/powerpc/booting-without-of.txt
index 7a6c5f2..21afaea 100644
--- a/Documentation/powerpc/booting-without-of.txt
+++ b/Documentation/powerpc/booting-without-of.txt
@@ -2089,6 +2089,61 @@ platforms are moved over to use the flattened-device-tree model.
 
    More devices will be defined as this spec matures.
 
+   l) Xilinx ML300 Framebuffer
+
+   Simple framebuffer device from the ML300 reference design (also on the
+   ML403 reference design as well as others).
+
+   Required properties:
+    - compatible : Must include "xilinx,ml300-fb"
+    - reg : offset and length of the framebuffer register set
+
+   Optional properties:
+    - resolution : <xres yres> pixel resolution of framebuffer.  Some
+                   implementations use a different resolution.  Default
+                   is <d#640 d#480>
+    - virt-resolution : <xvirt yvirt> Size of framebuffer in memory.
+                        Default is <d#1024 d#480>.
+    - rotate-display (empty) : rotate display 180 degrees.
+
+   m) Xilinx SystemACE
+
+   The Xilinx SystemACE device is used to program FPGAs from an FPGA
+   bitstream stored on a CF card.  It can also be used as a generic CF
+   interface device.
+
+   Required properties:
+    - compatible : Must include "xilinx,sysace"
+    - reg : offset and length of SystemACE register set
+
+   Recommended properties:
+    - interrupt-parent, interrupts : Connection of device irq signal.
+
+   Optional properties:
+    - 8-bit (empty) : Set this property if the SystemACE must be in 8 bit mode
+
+   n) Xilinx EMAC and Xilinx TEMAC
+
+   Xilinx Ethernet devices.  Uses common properties from other Ethernet
+   devices with the following constraints:
+   
+   Required properties:
+    - compatible : Must include one of: "xilinx,plb-temac",
+                   "xilinx,plb-emac", "xilinx-opb-emac"
+    - dma-mode : Must be one of "none", "simple", "sg" (sg == scatter gather)
+
+   o) Xilinx Uartlite
+
+   Xilinx uartlite devices are simple fixed speed serial ports.  Uartlite
+   ports should be described in a node with the following properties.
+
+   Requred properties:
+    - compatible : Must include "xilinx,uartlite"
+    - reg : offset and length of uartlite register set
+
+    Recommended properties:
+    - interrupt-parent, interrupts : Connection of device irq signal.
+
 VII - Specifying interrupt information for devices
 ===================================================
 

Re: [PATCH v2] Device tree bindings for Xilinx devices

[Arnd Bergmann]

On Monday 15 October 2007, Grant Likely wrote:
> From: Grant Likely <grant.likely@secretlab.ca>
> 
> Here's my second version of xilinx device tree bindings.  Please review
> and comment.  I'd like to push these out to Paulus in the next couple
> of days.

There are a few more properties that I can imagine you might need.
Not sure if it makes sense specifying them now, but here are my thoughts:
 
>     More devices will be defined as this spec matures.
>  
> +   l) Xilinx ML300 Framebuffer
> +
> +   Simple framebuffer device from the ML300 reference design (also on the
> +   ML403 reference design as well as others).
> +
> +   Required properties:
> +    - compatible : Must include "xilinx,ml300-fb"
> +    - reg : offset and length of the framebuffer register set
> +
> +   Optional properties:
> +    - resolution : <xres yres> pixel resolution of framebuffer.  Some
> +                   implementations use a different resolution.  Default
> +                   is <d#640 d#480>
> +    - virt-resolution : <xvirt yvirt> Size of framebuffer in memory.
> +                        Default is <d#1024 d#480>.
> +    - rotate-display (empty) : rotate display 180 degrees.

rotate-display could be defined as something that allows 0/90/180/270
degrees, as well as mirroring, not just 180 degree rotation.

> +   o) Xilinx Uartlite
> +
> +   Xilinx uartlite devices are simple fixed speed serial ports.  Uartlite
> +   ports should be described in a node with the following properties.
> +
> +   Requred properties:
> +    - compatible : Must include "xilinx,uartlite"
> +    - reg : offset and length of uartlite register set
> +
> +    Recommended properties:
> +    - interrupt-parent, interrupts : Connection of device irq signal.
> +

typically, serial ports include properties for current-speed and
clock-frequency. I guess it would be good to include at least one
of the two here.

	Arnd <><

Re: [PATCH v2] Device tree bindings for Xilinx devices

[Grant Likely]

On 10/15/07, Arnd Bergmann <arnd@arndb.de> wrote:
> On Monday 15 October 2007, Grant Likely wrote:
> > From: Grant Likely <grant.likely@secretlab.ca>
> >
> > Here's my second version of xilinx device tree bindings.  Please review
> > and comment.  I'd like to push these out to Paulus in the next couple
> > of days.
>
> There are a few more properties that I can imagine you might need.
> Not sure if it makes sense specifying them now, but here are my thoughts:
>
> >     More devices will be defined as this spec matures.
> >
> > +   l) Xilinx ML300 Framebuffer
> > +
> > +   Simple framebuffer device from the ML300 reference design (also on the
> > +   ML403 reference design as well as others).
> > +
> > +   Required properties:
> > +    - compatible : Must include "xilinx,ml300-fb"
> > +    - reg : offset and length of the framebuffer register set
> > +
> > +   Optional properties:
> > +    - resolution : <xres yres> pixel resolution of framebuffer.  Some
> > +                   implementations use a different resolution.  Default
> > +                   is <d#640 d#480>
> > +    - virt-resolution : <xvirt yvirt> Size of framebuffer in memory.
> > +                        Default is <d#1024 d#480>.
> > +    - rotate-display (empty) : rotate display 180 degrees.
>
> rotate-display could be defined as something that allows 0/90/180/270
> degrees, as well as mirroring, not just 180 degree rotation.

Yeah, I'm not sure what to do here.  The current xilinxfb device is
quite simple; but being that it's implemented in an FPGA, it's
conceivable that HW designers could build in any number of added
features in variant designs.  rotate-display reflects the currently
implemented behavior.  I'd like to future-proof it, but I'm not sure
how far down that path to go.  (suggestions welcome!)

>
> > +   o) Xilinx Uartlite
> > +
> > +   Xilinx uartlite devices are simple fixed speed serial ports.  Uartlite
> > +   ports should be described in a node with the following properties.
> > +
> > +   Requred properties:
> > +    - compatible : Must include "xilinx,uartlite"
> > +    - reg : offset and length of uartlite register set
> > +
> > +    Recommended properties:
> > +    - interrupt-parent, interrupts : Connection of device irq signal.
> > +
>
> typically, serial ports include properties for current-speed and
> clock-frequency. I guess it would be good to include at least one
> of the two here.

Is 'current-speed' the current baud rate?  clock-frequency is unneeded
here because the uartlite is a fixed speed serial device.  :-)

Thanks for the feedback,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

RE: [PATCH v2] Device tree bindings for Xilinx devices

[Stephen Neuendorffer]

> +   n) Xilinx EMAC and Xilinx TEMAC
> +
> +   Xilinx Ethernet devices.  Uses common properties from=20
> other Ethernet
> +   devices with the following constraints:
> +  =20
> +   Required properties:
> +    - compatible : Must include one of: "xilinx,plb-temac",
> +                   "xilinx,plb-emac", "xilinx-opb-emac"
> +    - dma-mode : Must be one of "none", "simple", "sg" (sg=20
> =3D=3D scatter gather)

I think it's going to be a significant headache to remap things like the
dma-mode from the xilinx configurations to something else, and then
interpret them correctly in the drivers.

Although it lacks a bit in style, perhaps, I'd greatly prefer having
something like:

	Ethernet_MAC {
		xilinx,C_DMA_PRESENT =3D <1>;
            ...
      }

(which happens to correspond to "none" above)
DMA mode is perhaps a bad example, since the Xilinx EDK encoding for
this is so unnecessarily obfuscated, but I'd like to avoid setting
precedent for defining a new set of parameterizations here.  In the long
term, I'm afraid this will just be an added source of confusion and more
code to maintain.

Steve

Re: [PATCH v2] Device tree bindings for Xilinx devices

[Grant Likely]

On 10/16/07, Stephen Neuendorffer <stephen.neuendorffer@xilinx.com> wrote:
>
> > +   n) Xilinx EMAC and Xilinx TEMAC
> > +
> > +   Xilinx Ethernet devices.  Uses common properties from
> > other Ethernet
> > +   devices with the following constraints:
> > +
> > +   Required properties:
> > +    - compatible : Must include one of: "xilinx,plb-temac",
> > +                   "xilinx,plb-emac", "xilinx-opb-emac"
> > +    - dma-mode : Must be one of "none", "simple", "sg" (sg
> > == scatter gather)
>
> I think it's going to be a significant headache to remap things like the
> dma-mode from the xilinx configurations to something else, and then
> interpret them correctly in the drivers.
>
> Although it lacks a bit in style, perhaps, I'd greatly prefer having
> something like:
>
>         Ethernet_MAC {
>                 xilinx,C_DMA_PRESENT = <1>;
>             ...
>       }
>
> (which happens to correspond to "none" above)

Ugh.  Can't say I'm thrilled about this....

But in this case is might be a necessity.  The IP core is already
parameterized and the best way to describe the device is to use the
existing parameter names.

I want to be careful though.  Parameters can change from one version
of an IP core to another.  The driver needs to be able to determine
what version of the IP core is used so that the parameters make sense.
 I'm also nervous about adding every possible C_ value to the device
node for each xilinx IP-core; that could make the device tree quite
large.  (on the other hand; maybe that doesn't matter... It is
probably a bigger deal for microblaze than powerpc too.).

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

RE: [PATCH v2] Device tree bindings for Xilinx devices

[Stephen Neuendorffer]

It occurs to me that the 'compatible' bindings should probably be the
name of the preferred driver for the device.=20

> +   l) Xilinx ML300 Framebuffer
> +    - compatible : Must include "xilinx,ml300-fb"
Should probably be 'xilinxfb', and probably shouldn't reference ML300 at
all.

> +   n) Xilinx EMAC and Xilinx TEMAC
> +
> +   Xilinx Ethernet devices.  Uses common properties from=20
> other Ethernet
> +   devices with the following constraints:
> +  =20
> +   Required properties:
> +    - compatible : Must include one of: "xilinx,plb-temac",
> +                   "xilinx,plb-emac", "xilinx-opb-emac"
Should probably be just 'emac' and 'temac'.

This should be possible to extract using Michel Simek's EDK-integrated
device tree generator.  The compatibility information is stored in the
EDK driver's .mdd file (although this is somewhat backwards from what
one might expect).

Steve

RE: [PATCH v2] Device tree bindings for Xilinx devices

[Stephen Neuendorffer]

=20

> -----Original Message-----
> From: glikely@secretlab.ca [mailto:glikely@secretlab.ca] On=20
> Behalf Of Grant Likely
> Sent: Tuesday, October 16, 2007 12:36 PM
> To: Stephen Neuendorffer
> Cc: linuxppc-dev@ozlabs.org; Wolfgang Reissnegger; Leonid;=20
> microblaze-uclinux@itee.uq.edu.au
> Subject: Re: [PATCH v2] Device tree bindings for Xilinx devices
>=20
> On 10/16/07, Stephen Neuendorffer=20
> <stephen.neuendorffer@xilinx.com> wrote:
> >
> > > +   n) Xilinx EMAC and Xilinx TEMAC
> > > +
> > > +   Xilinx Ethernet devices.  Uses common properties from
> > > other Ethernet
> > > +   devices with the following constraints:
> > > +
> > > +   Required properties:
> > > +    - compatible : Must include one of: "xilinx,plb-temac",
> > > +                   "xilinx,plb-emac", "xilinx-opb-emac"
> > > +    - dma-mode : Must be one of "none", "simple", "sg" (sg
> > > =3D=3D scatter gather)
> >
> > I think it's going to be a significant headache to remap=20
> things like the
> > dma-mode from the xilinx configurations to something else, and then
> > interpret them correctly in the drivers.
> >
> > Although it lacks a bit in style, perhaps, I'd greatly prefer having
> > something like:
> >
> >         Ethernet_MAC {
> >                 xilinx,C_DMA_PRESENT =3D <1>;
> >             ...
> >       }
> >
> > (which happens to correspond to "none" above)
>=20
> Ugh.  Can't say I'm thrilled about this....
>=20
> But in this case is might be a necessity.  The IP core is already
> parameterized and the best way to describe the device is to use the
> existing parameter names.

I don't really like it either, expecially after thinking about it more.
The problem is that a single IP core might correspond to multiple
logical devices.  The parameters actually need to be from the point of
view of the drivers. That is, they need to be names like in the
xparameters:

	xilinx,DMA_PRESENT =3D <1>;

In this case, the translation looks simple, but it is currently
implemented by a bunch of gnarly .tcl code.  (Don't ask... :)  In the
future, this *could* be simple code, but in the short term, it won't be
any simpler than having pretty names.  Furthermore, the best driver code
that I can come up with for using this essentially looks like:

static int __devinit xenet_of_probe(struct of_device *ofdev, const
struct of_device_id *match)
{
	struct xemac_platform_data pdata_struct;
	struct resource r_irq_struct;
	struct resource r_mem_struct;

	struct resource *r_irq =3D &r_irq_struct;	/* Interrupt resources
*/
	struct resource *r_mem =3D &r_mem_struct;	/* IO mem resources */
	struct xemac_platform_data *pdata =3D &pdata_struct;
	int rc =3D 0;

	printk(KERN_ERR "Device Tree Probing \'%s\'\n",
                        ofdev->node->name);

	/* Get iospace for the device */
	rc =3D of_address_to_resource(ofdev->node, 0, r_mem);
	if(rc) {
		dev_warn(&ofdev->dev, "invalid address\n");
		return rc;
	}

	/* Get IRQ for the device */
	rc =3D of_irq_to_resource(ofdev->node, 0, r_irq);
	if(rc =3D=3D NO_IRQ) {
		dev_warn(&ofdev->dev, "no IRQ found.\n");
		return rc;
	}

	pdata_struct.dma_mode           =3D get_u32(ofdev,
"C_DMA_PRESENT");
	pdata_struct.has_mii		=3D get_u32(ofdev, "C_MII_EXIST");
	pdata_struct.has_cam		=3D get_u32(ofdev, "C_CAM_EXIST");
	pdata_struct.has_err_cnt	=3D get_u32(ofdev,
"C_ERR_COUNT_EXIST");
	pdata_struct.has_jumbo		=3D get_u32(ofdev,
"C_JUMBO_EXIST");
	pdata_struct.tx_dre		=3D get_u32(ofdev,
"C_TX_DRE_TYPE");
	pdata_struct.rx_dre		=3D get_u32(ofdev,
"C_RX_DRE_TYPE");
	pdata_struct.tx_hw_csum		=3D get_u32(ofdev,
"C_TX_INCLUDE_CSUM");
	pdata_struct.rx_hw_csum		=3D get_u32(ofdev,
"C_RX_INCLUDE_CSUM");
	memcpy(pdata_struct.mac_addr, of_get_mac_address(ofdev->node),
6);

        return probe_initialization(&ofdev->dev, r_mem, r_irq, pdata);
}

Where probe_initialization contains shares code with the platform_device
version of probe.  This means that the string is still translated into
another name anyway.  Anybody have any better ideas for doing this?

>=20
> I want to be careful though.  Parameters can change from one version
> of an IP core to another.  The driver needs to be able to determine
> what version of the IP core is used so that the parameters make sense.
>  I'm also nervous about adding every possible C_ value to the device
> node for each xilinx IP-core; that could make the device tree quite
> large.  (on the other hand; maybe that doesn't matter... It is
> probably a bigger deal for microblaze than powerpc too.).

Using the xparameters set of values could solve this problem, since only
things that make sense to the driver are actually set.

Steve

>=20
> Cheers,
> g.
>=20
> --=20
> Grant Likely, B.Sc., P.Eng.
> Secret Lab Technologies Ltd.
> grant.likely@secretlab.ca
> (403) 399-0195
>=20
>=20

RE: [PATCH v2] Device tree bindings for Xilinx devices

[Josh Boyer]

On Tue, 2007-10-16 at 16:23 -0700, Stephen Neuendorffer wrote:
> It occurs to me that the 'compatible' bindings should probably be the
> name of the preferred driver for the device. 
> 
> > +   l) Xilinx ML300 Framebuffer
> > +    - compatible : Must include "xilinx,ml300-fb"
> Should probably be 'xilinxfb', and probably shouldn't reference ML300 at
> all.
> 
> > +   n) Xilinx EMAC and Xilinx TEMAC
> > +
> > +   Xilinx Ethernet devices.  Uses common properties from 
> > other Ethernet
> > +   devices with the following constraints:
> > +   
> > +   Required properties:
> > +    - compatible : Must include one of: "xilinx,plb-temac",
> > +                   "xilinx,plb-emac", "xilinx-opb-emac"
> Should probably be just 'emac' and 'temac'.

You mean xilinx,emac and xilinx,temac, right?  emac and temac are too
generic by themselves.

josh

Re: [PATCH v2] Device tree bindings for Xilinx devices

[Grant Likely]

On 10/16/07, Stephen Neuendorffer <stephen.neuendorffer@xilinx.com> wrote:
>
> It occurs to me that the 'compatible' bindings should probably be the
> name of the preferred driver for the device.
>
> > +   l) Xilinx ML300 Framebuffer
> > +    - compatible : Must include "xilinx,ml300-fb"
> Should probably be 'xilinxfb', and probably shouldn't reference ML300 at
> all.

Actually, I don't think xilinx,ml300-fb is not specific enough (as
opposed to not general enough).  Given that the device tree is
supposed to describe the hardware as uniquely as possible, compatible
should probably contain a value like "xilinx,plb-tft-cntlr-ref-1.00.c"
(<vendor>,<ip core>,<version>).

A design with a modified fb core might specify:

        compatible = "acme,super-tft-1.3","xilinx,plb-tft-cntlr-ref-1.00.c";

Which indicates that it is a different part, but it provides the same
interface as the preexisting plb-tft-cntrl-ref ip core.  Similarly, a
newer reference design which uses a new version of the tft core should
specifiy:

        compatible =
"xilinx,plb-tft-cntrl-ref-1.00.d","xilinx,plb-tft-cntlr-ref-1.00.c";

That way the exact type of device is specified; but it is *compatible*
with the older device.  If the newer device has a greater feature set,
then a driver that can match against the leftmost compatible value can
make use of the extra features.

>
> > +   n) Xilinx EMAC and Xilinx TEMAC
> > +
> > +   Xilinx Ethernet devices.  Uses common properties from
> > other Ethernet
> > +   devices with the following constraints:
> > +
> > +   Required properties:
> > +    - compatible : Must include one of: "xilinx,plb-temac",
> > +                   "xilinx,plb-emac", "xilinx-opb-emac"
> Should probably be just 'emac' and 'temac'.

Actually; looking at the available ip cores; it should probably be:

"xilinx,opb-ethernet-1.02.a" for an emac driver
"xilinx,plb-temac-3.00.a" for a temac.

We should probably use the exact names of the ip core.  Less ambiguity that way.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

Re: [PATCH v2] Device tree bindings for Xilinx devices

[Grant Likely]

On 10/16/07, Stephen Neuendorffer <stephen.neuendorffer@xilinx.com> wrote:
>
>
> > -----Original Message-----
> > From: glikely@secretlab.ca [mailto:glikely@secretlab.ca] On
> > Behalf Of Grant Likely
> > Sent: Tuesday, October 16, 2007 12:36 PM
> > To: Stephen Neuendorffer
> > Cc: linuxppc-dev@ozlabs.org; Wolfgang Reissnegger; Leonid;
> > microblaze-uclinux@itee.uq.edu.au
> > Subject: Re: [PATCH v2] Device tree bindings for Xilinx devices
> >
> > On 10/16/07, Stephen Neuendorffer
> > <stephen.neuendorffer@xilinx.com> wrote:
> > >
> > > > +   n) Xilinx EMAC and Xilinx TEMAC
> > > > +
> > > > +   Xilinx Ethernet devices.  Uses common properties from
> > > > other Ethernet
> > > > +   devices with the following constraints:
> > > > +
> > > > +   Required properties:
> > > > +    - compatible : Must include one of: "xilinx,plb-temac",
> > > > +                   "xilinx,plb-emac", "xilinx-opb-emac"
> > > > +    - dma-mode : Must be one of "none", "simple", "sg" (sg
> > > > == scatter gather)
> > >
> > > I think it's going to be a significant headache to remap
> > things like the
> > > dma-mode from the xilinx configurations to something else, and then
> > > interpret them correctly in the drivers.
> > >
> > > Although it lacks a bit in style, perhaps, I'd greatly prefer having
> > > something like:
> > >
> > >         Ethernet_MAC {
> > >                 xilinx,C_DMA_PRESENT = <1>;
> > >             ...
> > >       }
> > >
> > > (which happens to correspond to "none" above)
> >
> > Ugh.  Can't say I'm thrilled about this....
> >
> > But in this case is might be a necessity.  The IP core is already
> > parameterized and the best way to describe the device is to use the
> > existing parameter names.
>
> I don't really like it either, expecially after thinking about it more.
> The problem is that a single IP core might correspond to multiple
> logical devices.  The parameters actually need to be from the point of
> view of the drivers. That is, they need to be names like in the
> xparameters:
>
>         xilinx,DMA_PRESENT = <1>;

Hmm, not bad.  It does still get a little harry when one IP core
implements multiple logical devices, but maybe the IP core should
create a node and the logical devices can be sub nodes with the
appropriate properties needed to describe which logical device it is.

>
> In this case, the translation looks simple, but it is currently
> implemented by a bunch of gnarly .tcl code.  (Don't ask... :)  In the
> future, this *could* be simple code, but in the short term, it won't be
> any simpler than having pretty names.  Furthermore, the best driver code
> that I can come up with for using this essentially looks like:
>
> static int __devinit xenet_of_probe(struct of_device *ofdev, const
> struct of_device_id *match)
> {
>         struct xemac_platform_data pdata_struct;
>         struct resource r_irq_struct;
>         struct resource r_mem_struct;
>
>         struct resource *r_irq = &r_irq_struct; /* Interrupt resources
> */
>         struct resource *r_mem = &r_mem_struct; /* IO mem resources */
>         struct xemac_platform_data *pdata = &pdata_struct;
>         int rc = 0;
>
>         printk(KERN_ERR "Device Tree Probing \'%s\'\n",
>                         ofdev->node->name);
>
>         /* Get iospace for the device */
>         rc = of_address_to_resource(ofdev->node, 0, r_mem);
>         if(rc) {
>                 dev_warn(&ofdev->dev, "invalid address\n");
>                 return rc;
>         }
>
>         /* Get IRQ for the device */
>         rc = of_irq_to_resource(ofdev->node, 0, r_irq);
>         if(rc == NO_IRQ) {
>                 dev_warn(&ofdev->dev, "no IRQ found.\n");
>                 return rc;
>         }
>
>         pdata_struct.dma_mode           = get_u32(ofdev,
> "C_DMA_PRESENT");
>         pdata_struct.has_mii            = get_u32(ofdev, "C_MII_EXIST");
>         pdata_struct.has_cam            = get_u32(ofdev, "C_CAM_EXIST");
>         pdata_struct.has_err_cnt        = get_u32(ofdev,
> "C_ERR_COUNT_EXIST");
>         pdata_struct.has_jumbo          = get_u32(ofdev,
> "C_JUMBO_EXIST");
>         pdata_struct.tx_dre             = get_u32(ofdev,
> "C_TX_DRE_TYPE");
>         pdata_struct.rx_dre             = get_u32(ofdev,
> "C_RX_DRE_TYPE");
>         pdata_struct.tx_hw_csum         = get_u32(ofdev,
> "C_TX_INCLUDE_CSUM");
>         pdata_struct.rx_hw_csum         = get_u32(ofdev,
> "C_RX_INCLUDE_CSUM");
>         memcpy(pdata_struct.mac_addr, of_get_mac_address(ofdev->node),
> 6);
>
>         return probe_initialization(&ofdev->dev, r_mem, r_irq, pdata);
> }
>
> Where probe_initialization contains shares code with the platform_device
> version of probe.  This means that the string is still translated into
> another name anyway.  Anybody have any better ideas for doing this?

That's pretty close to what I've done on the sysace, uartlite and
xilinxfb drivers.

> > I want to be careful though.  Parameters can change from one version
> > of an IP core to another.  The driver needs to be able to determine
> > what version of the IP core is used so that the parameters make sense.
> >  I'm also nervous about adding every possible C_ value to the device
> > node for each xilinx IP-core; that could make the device tree quite
> > large.  (on the other hand; maybe that doesn't matter... It is
> > probably a bigger deal for microblaze than powerpc too.).
>
> Using the xparameters set of values could solve this problem, since only
> things that make sense to the driver are actually set.

I've thought about this more since I wrote the above paragraph.  The
compatible property was designed to solve that exact problem.  We'll
just embed the version in the compatible list.

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

Re: [PATCH v2] Device tree bindings for Xilinx devices

[Grant Likely]

On 10/16/07, Grant Likely <grant.likely@secretlab.ca> wrote:
> On 10/16/07, Stephen Neuendorffer <stephen.neuendorffer@xilinx.com> wrote:
> >
> >
> > > -----Original Message-----
> > > From: glikely@secretlab.ca [mailto:glikely@secretlab.ca] On
> > > Behalf Of Grant Likely
> > > Sent: Tuesday, October 16, 2007 12:36 PM
> > > To: Stephen Neuendorffer
> > > Cc: linuxppc-dev@ozlabs.org; Wolfgang Reissnegger; Leonid;
> > > microblaze-uclinux@itee.uq.edu.au
> > > Subject: Re: [PATCH v2] Device tree bindings for Xilinx devices
> > >
> > > On 10/16/07, Stephen Neuendorffer
> > > <stephen.neuendorffer@xilinx.com> wrote:
> > > >
> > > > > +   n) Xilinx EMAC and Xilinx TEMAC
> > > > > +
> > > > > +   Xilinx Ethernet devices.  Uses common properties from
> > > > > other Ethernet
> > > > > +   devices with the following constraints:
> > > > > +
> > > > > +   Required properties:
> > > > > +    - compatible : Must include one of: "xilinx,plb-temac",
> > > > > +                   "xilinx,plb-emac", "xilinx-opb-emac"
> > > > > +    - dma-mode : Must be one of "none", "simple", "sg" (sg
> > > > > == scatter gather)
> > > >
> > > > I think it's going to be a significant headache to remap
> > > things like the
> > > > dma-mode from the xilinx configurations to something else, and then
> > > > interpret them correctly in the drivers.
> > > >
> > > > Although it lacks a bit in style, perhaps, I'd greatly prefer having
> > > > something like:
> > > >
> > > >         Ethernet_MAC {
> > > >                 xilinx,C_DMA_PRESENT = <1>;
> > > >             ...
> > > >       }
> > > >
> > > > (which happens to correspond to "none" above)
> > >
> > > Ugh.  Can't say I'm thrilled about this....
> > >
> > > But in this case is might be a necessity.  The IP core is already
> > > parameterized and the best way to describe the device is to use the
> > > existing parameter names.
> >
> > I don't really like it either, expecially after thinking about it more.
> > The problem is that a single IP core might correspond to multiple
> > logical devices.  The parameters actually need to be from the point of
> > view of the drivers. That is, they need to be names like in the
> > xparameters:
> >
> >         xilinx,DMA_PRESENT = <1>;
>
> Hmm, not bad.  It does still get a little harry when one IP core
> implements multiple logical devices, but maybe the IP core should
> create a node and the logical devices can be sub nodes with the
> appropriate properties needed to describe which logical device it is.
>
> >
> > In this case, the translation looks simple, but it is currently
> > implemented by a bunch of gnarly .tcl code.  (Don't ask... :)  In the
> > future, this *could* be simple code, but in the short term, it won't be
> > any simpler than having pretty names.  Furthermore, the best driver code
> > that I can come up with for using this essentially looks like:
> >
> > static int __devinit xenet_of_probe(struct of_device *ofdev, const
> > struct of_device_id *match)
> > {
> >         struct xemac_platform_data pdata_struct;
> >         struct resource r_irq_struct;
> >         struct resource r_mem_struct;
> >
> >         struct resource *r_irq = &r_irq_struct; /* Interrupt resources
> > */
> >         struct resource *r_mem = &r_mem_struct; /* IO mem resources */
> >         struct xemac_platform_data *pdata = &pdata_struct;
> >         int rc = 0;
> >
> >         printk(KERN_ERR "Device Tree Probing \'%s\'\n",
> >                         ofdev->node->name);
> >
> >         /* Get iospace for the device */
> >         rc = of_address_to_resource(ofdev->node, 0, r_mem);
> >         if(rc) {
> >                 dev_warn(&ofdev->dev, "invalid address\n");
> >                 return rc;
> >         }
> >
> >         /* Get IRQ for the device */
> >         rc = of_irq_to_resource(ofdev->node, 0, r_irq);
> >         if(rc == NO_IRQ) {
> >                 dev_warn(&ofdev->dev, "no IRQ found.\n");
> >                 return rc;
> >         }
> >
> >         pdata_struct.dma_mode           = get_u32(ofdev,
> > "C_DMA_PRESENT");
> >         pdata_struct.has_mii            = get_u32(ofdev, "C_MII_EXIST");
> >         pdata_struct.has_cam            = get_u32(ofdev, "C_CAM_EXIST");
> >         pdata_struct.has_err_cnt        = get_u32(ofdev,
> > "C_ERR_COUNT_EXIST");
> >         pdata_struct.has_jumbo          = get_u32(ofdev,
> > "C_JUMBO_EXIST");
> >         pdata_struct.tx_dre             = get_u32(ofdev,
> > "C_TX_DRE_TYPE");
> >         pdata_struct.rx_dre             = get_u32(ofdev,
> > "C_RX_DRE_TYPE");
> >         pdata_struct.tx_hw_csum         = get_u32(ofdev,
> > "C_TX_INCLUDE_CSUM");
> >         pdata_struct.rx_hw_csum         = get_u32(ofdev,
> > "C_RX_INCLUDE_CSUM");
> >         memcpy(pdata_struct.mac_addr, of_get_mac_address(ofdev->node),
> > 6);
> >
> >         return probe_initialization(&ofdev->dev, r_mem, r_irq, pdata);
> > }
> >
> > Where probe_initialization contains shares code with the platform_device
> > version of probe.  This means that the string is still translated into
> > another name anyway.  Anybody have any better ideas for doing this?
>
> That's pretty close to what I've done on the sysace, uartlite and
> xilinxfb drivers.
>
> > > I want to be careful though.  Parameters can change from one version
> > > of an IP core to another.  The driver needs to be able to determine
> > > what version of the IP core is used so that the parameters make sense.
> > >  I'm also nervous about adding every possible C_ value to the device
> > > node for each xilinx IP-core; that could make the device tree quite
> > > large.  (on the other hand; maybe that doesn't matter... It is
> > > probably a bigger deal for microblaze than powerpc too.).
> >
> > Using the xparameters set of values could solve this problem, since only
> > things that make sense to the driver are actually set.
>
> I've thought about this more since I wrote the above paragraph.  The
> compatible property was designed to solve that exact problem.  We'll
> just embed the version in the compatible list.

And, yes, you're right.  Using the xparams value set is probably the
place to start for choosing which properties to put in the device
tree.  I'll document this stuff up and respin my patch.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195