[PATCH] Xilinx: framebuffer: add compatibility for ml507 dvi core.

Signed-off-by: Stephen Neuendorffer <redacted>
---
 drivers/video/xilinxfb.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/video/xilinxfb.c b/drivers/video/xilinxfb.c
index 848752e..a82c530 100644
--- a/drivers/video/xilinxfb.c
+++ b/drivers/video/xilinxfb.c

@@ -396,6 +396,7 @@ static int __devexit xilinxfb_of_remove(struct of_device *op)
 /* Match table for of_platform binding */
 static struct of_device_id xilinxfb_of_match[] __devinitdata = {
 	{ .compatible = "xlnx,plb-tft-cntlr-ref-1.00.a", },
+	{ .compatible = "xlnx,plb-dvi-cntlr-ref-1.00.c", },
 	{},
 };
 MODULE_DEVICE_TABLE(of, xilinxfb_of_match);

-- 
1.5.3.4-dirty

On Mon, May 5, 2008 at 11:59 AM, Stephen Neuendorffer
[off-list ref] wrote:

Signed-off-by: Stephen Neuendorffer <redacted>
 ---
  drivers/video/xilinxfb.c |    1 +
  1 files changed, 1 insertions(+), 0 deletions(-)

 diff --git a/drivers/video/xilinxfb.c b/drivers/video/xilinxfb.c
 index 848752e..a82c530 100644
 --- a/drivers/video/xilinxfb.c
 +++ b/drivers/video/xilinxfb.c
 @@ -396,6 +396,7 @@ static int __devexit xilinxfb_of_remove(struct of_device *op)
  /* Match table for of_platform binding */
  static struct of_device_id xilinxfb_of_match[] __devinitdata = {
        { .compatible = "xlnx,plb-tft-cntlr-ref-1.00.a", },
 +       { .compatible = "xlnx,plb-dvi-cntlr-ref-1.00.c", },

Can you get the dvi core to claim compatibility with the tft core
(assuming it is register level backwards compatible)?

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Not easily.  As we discussed before, there is nowhere that really
specifies this information.  In some cases I've modified the device tree
generator to understand what is backward compatible, as with ns16550,
but for other cores it makes more sense to me to put this compatibility
information with the driver if it has to be anywhere.

Steve

-----Original Message-----
From: <redacted> [mailto:<redacted>] On Behalf Of

Grant Likely

Sent: Monday, May 12, 2008 7:25 AM
To: Stephen Neuendorffer
Cc: <redacted>
Subject: Re: [PATCH] Xilinx: framebuffer: add compatibility for ml507

dvi core.

=20
On Mon, May 5, 2008 at 11:59 AM, Stephen Neuendorffer
[off-list ref] wrote:

quoted

Signed-off-by: Stephen Neuendorffer

[off-list ref]

quoted

 ---
  drivers/video/xilinxfb.c |    1 +
  1 files changed, 1 insertions(+), 0 deletions(-)

 diff --git a/drivers/video/xilinxfb.c b/drivers/video/xilinxfb.c
 index 848752e..a82c530 100644
 --- a/drivers/video/xilinxfb.c
 +++ b/drivers/video/xilinxfb.c
 @@ -396,6 +396,7 @@ static int __devexit xilinxfb_of_remove(struct

of_device *op)

quoted

  /* Match table for of_platform binding */
  static struct of_device_id xilinxfb_of_match[] __devinitdata =3D {
        { .compatible =3D "xlnx,plb-tft-cntlr-ref-1.00.a", },
 +       { .compatible =3D "xlnx,plb-dvi-cntlr-ref-1.00.c", },

=20
Can you get the dvi core to claim compatibility with the tft core
(assuming it is register level backwards compatible)?
=20
Cheers,
g.
=20
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

On Mon, May 12, 2008 at 11:31 AM, Stephen Neuendorffer
[off-list ref] wrote:

 Not easily.  As we discussed before, there is nowhere that really
 specifies this information.  In some cases I've modified the device tree
 generator to understand what is backward compatible, as with ns16550,
 but for other cores it makes more sense to me to put this compatibility
 information with the driver if it has to be anywhere.

The main concern is that the driver will balloon with data which
impacts the load size for all builds that compile it in.  It also
reduces the impact of minor modifications of the IP cores (ie. the
kernel still works when I've up-revved an ip core from 1.00a to
1.00b.)

I understand that it is hard within the EDK TCL scripts context, but
this is exactly what compatible is designed for and I don't think
continually adding more versions to the list is sustainable in the
long term.  The more I think about it, the more I'm uneasy about this
approach.

For the time being, I'm going to resist merging this change into my
tree, but I'll keep it in my "don't forget about this" list.  It's not
going into .26 anyway, so there is no time pressure.  Later, when
we're closer to the .27 merge window, I'll look at it again, but I
really would prefer to find a way to add to the compatible list.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

The best possibility that I see is glopping the compatibility
information about what cores are compatible with which drivers and
generating something.  This is moderately better than blindly treating
all cores with the same major version as interface compatible, but still
has the potential to blindly declare that core versions are compatible
when they are not really compatible.

I *really* don't want to put this into the device tree generator on a
case-by-case basis, so unless there is something that can be generated
from whatever meta-information EDK has, I think we're going to have to
just have the explicit versions in the drivers for the time being.

Steve=20

-----Original Message-----
From: <redacted> [mailto:<redacted>] On Behalf Of

Grant Likely

Sent: Monday, May 12, 2008 11:30 AM
To: Stephen Neuendorffer
Cc: <redacted>
Subject: Re: [PATCH] Xilinx: framebuffer: add compatibility for ml507

dvi core.

=20
On Mon, May 12, 2008 at 11:31 AM, Stephen Neuendorffer
[off-list ref] wrote:

quoted

 Not easily.  As we discussed before, there is nowhere that really
 specifies this information.  In some cases I've modified the device

tree

quoted

 generator to understand what is backward compatible, as with

ns16550,

quoted

 but for other cores it makes more sense to me to put this

compatibility

quoted

 information with the driver if it has to be anywhere.

=20
The main concern is that the driver will balloon with data which
impacts the load size for all builds that compile it in.  It also
reduces the impact of minor modifications of the IP cores (ie. the
kernel still works when I've up-revved an ip core from 1.00a to
1.00b.)
=20
I understand that it is hard within the EDK TCL scripts context, but
this is exactly what compatible is designed for and I don't think
continually adding more versions to the list is sustainable in the
long term.  The more I think about it, the more I'm uneasy about this
approach.
=20
For the time being, I'm going to resist merging this change into my
tree, but I'll keep it in my "don't forget about this" list.  It's not
going into .26 anyway, so there is no time pressure.  Later, when
we're closer to the .27 merge window, I'll look at it again, but I
really would prefer to find a way to add to the compatible list.
=20
Cheers,
g.
=20
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

On Mon, May 12, 2008 at 1:10 PM, Stephen Neuendorffer
[off-list ref] wrote:

 The best possibility that I see is glopping the compatibility
 information about what cores are compatible with which drivers and
 generating something.  This is moderately better than blindly treating
 all cores with the same major version as interface compatible, but still
 has the potential to blindly declare that core versions are compatible
 when they are not really compatible.

That's okay, the device tree conventions provide for that uncertainty.
 If compatible includes both the *exact* version and the oldest known
*compatible* version (the one the drivers know about) then we're in
the situation where 99% of the time it just works.  For the 1% of the
time when mistakes are made we still have the necessary information to
write exceptions in the code to work around bad data.  This means code
only needs to changes when mistakes are discovered, not for every IP
core uprev.

 I *really* don't want to put this into the device tree generator on a
 case-by-case basis, so unless there is something that can be generated
 from whatever meta-information EDK has, I think we're going to have to
 just have the explicit versions in the drivers for the time being.

Can we post process the generated device tree with a table of known
compatibility strings (or regexps) for adding the older compatible
values?  I don't expect the list will be particularly long or hard to
maintain and the code to do so should be trivial.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

-----Original Message-----
From: <redacted> [mailto:<redacted>] On Behalf Of

Grant Likely

Sent: Monday, May 12, 2008 12:46 PM
To: Stephen Neuendorffer
Cc: <redacted>; git-dev
Subject: Re: [PATCH] Xilinx: framebuffer: add compatibility for ml507

dvi core.

=20
On Mon, May 12, 2008 at 1:10 PM, Stephen Neuendorffer
[off-list ref] wrote:

quoted

 The best possibility that I see is glopping the compatibility
 information about what cores are compatible with which drivers and
 generating something.  This is moderately better than blindly

treating

quoted

 all cores with the same major version as interface compatible, but

still

quoted

 has the potential to blindly declare that core versions are

compatible

quoted

 when they are not really compatible.

=20
That's okay, the device tree conventions provide for that uncertainty.
 If compatible includes both the *exact* version and the oldest known
*compatible* version (the one the drivers know about) then we're in
the situation where 99% of the time it just works.  For the 1% of the
time when mistakes are made we still have the necessary information to
write exceptions in the code to work around bad data.  This means code
only needs to changes when mistakes are discovered, not for every IP
core uprev.

My argument was that we should do this by truncating the major version,
which is also an established standard that cores *should* follow, but
you didn't like that. :)  It makes at least as much sense as expressing
the compatibility of the driver in the tree in terms of compatibility
with some other random driver.  In the case of the tft core in
particular, there *is* no other driver AFAIK.

quoted

 I *really* don't want to put this into the device tree generator on

a

quoted

 case-by-case basis, so unless there is something that can be

generated

quoted

 from whatever meta-information EDK has, I think we're going to have

to

quoted

 just have the explicit versions in the drivers for the time being.

=20
Can we post process the generated device tree with a table of known
compatibility strings (or regexps) for adding the older compatible
values?  I don't expect the list will be particularly long or hard to
maintain and the code to do so should be trivial.

Ug, that's just pushing the problem around.
This seems as much like an argument for putting wildcards in the
compatible bindings in the kernel as anything...

Steve

On Mon, May 12, 2008 at 1:59 PM, Stephen Neuendorffer
[off-list ref] wrote:

quoted

-----Original Message-----
From: <redacted> [mailto:<redacted>] On Behalf Of

Grant Likely

quoted

Sent: Monday, May 12, 2008 12:46 PM
To: Stephen Neuendorffer
Cc: <redacted>; git-dev
Subject: Re: [PATCH] Xilinx: framebuffer: add compatibility for ml507

dvi core.

quoted

On Mon, May 12, 2008 at 1:10 PM, Stephen Neuendorffer
[off-list ref] wrote:

quoted

 The best possibility that I see is glopping the compatibility
 information about what cores are compatible with which drivers and
 generating something.  This is moderately better than blindly

treating

quoted

quoted

 all cores with the same major version as interface compatible, but

still

quoted

quoted

 has the potential to blindly declare that core versions are

compatible

quoted

quoted

 when they are not really compatible.

That's okay, the device tree conventions provide for that uncertainty.
 If compatible includes both the *exact* version and the oldest known
*compatible* version (the one the drivers know about) then we're in
the situation where 99% of the time it just works.  For the 1% of the
time when mistakes are made we still have the necessary information to
write exceptions in the code to work around bad data.  This means code
only needs to changes when mistakes are discovered, not for every IP
core uprev.

My argument was that we should do this by truncating the major version,
which is also an established standard that cores *should* follow, but
you didn't like that. :)  It makes at least as much sense as expressing
the compatibility of the driver in the tree in terms of compatibility
with some other random driver.  In the case of the tft core in
particular, there *is* no other driver AFAIK.

The whole point of compatible is to describe software interfaces...
*if* a device is register level backwards compatible with an older
already supported device, *then* it is appropriate to claim
compatibility with it.  True, the TFT core is not the same as the VGA
core.  But (as your patch suggests) the register interface is
backwards compatible.

As for truncating the major version; you're right, I don't like it,
but that doesn't mean I cannot be swayed.  Best argument is to
analogize it with a similar SoC situation.  A particular SoC (let's
say the MPC8349) will have several revisions to it, but none of that
is reflected in the device tree.  It's just referred to as
"fsl,mpc8349".  Actual silicon revision is obtainable by software from
the PVR/SVR if it *really* needs it.  The counter example is the
MPC5200/MPC5200b where the 'b' version is explicitly specified in the
compatible list (see arch/powerpc/boot/dts/lite5200b.dts).  The 5200b
is 99% compatible with the original 5200, with only a couple of on
chip peripherals not being register level compatible.  I'm still not
completely certain that "fsl,mpc5200b-<blah>" was the right decision,
but by being conservative early on means that I can still drop most of
the 'b' specifiers at some point in the future without breaking board
support.

What makes me nervous about FPGA IP cores is that the potential for
change is much higher than for an SoC.  SoC vendors get very angry
customers if a silicon uprev breaks their drivers; especially
considering that they could very easily have boards with both the old
and new silicon.  It does not seem to me like there is quite the same
level of pressure too keep the register level interface 100%
compatible.  So this is the pressure point to apply if you want to
change my mind.  How confident are you that the major (or minor)
revision will remain register level compatible?

n>> >  I *really* don't want to put this into the device tree generator on

a

quoted

quoted

 case-by-case basis, so unless there is something that can be

generated

quoted

quoted

 from whatever meta-information EDK has, I think we're going to have

to

quoted

quoted

 just have the explicit versions in the drivers for the time being.

Can we post process the generated device tree with a table of known
compatibility strings (or regexps) for adding the older compatible
values?  I don't expect the list will be particularly long or hard to
maintain and the code to do so should be trivial.

Ug, that's just pushing the problem around.
This seems as much like an argument for putting wildcards in the
compatible bindings in the kernel as anything...

Not quite.  There is a difference between method used to generate the
data, and how the data is interpreted by the kernel (the boundary
being what data is actually passed to the kernel).  If the tool
generates bad/inaccurate data, then it is a bug and it should be
fixed.  Even better, it is a bug that will be found quickly because
the device simply won't work if it binds to the wrong driver.  I do
not want to load knowledge of all those permutation into the kernel
image.

(But I do agree that my suggestion was rather smelly)

As for wildcards, the big problem is that the definition of a wildcard
changes over time as new devices are released.  Consider this;
aesthetics aside, what is the difference between using
"xlnx,plb-tft-cntlr-ref-1.xx" and "xlnx,plb-tft-cntlr-ref-1.00.a" as a
'generic' compatible value?  The value still needs to be coded into
the tool somewhere; but the second one is anchored to a single real
implementation which means that it's definition is fixed.

I know I'm being very conservative here; but it is safer to be
conservative now and relax stuff later than the other way around.
Being liberal is what got arch/ppc into the whole bdinfo_t mess in the
first place.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

-----Original Message-----
From: <redacted> [mailto:<redacted>] On Behalf Of

Grant Likely

Sent: Wednesday, May 14, 2008 8:35 AM
To: Stephen Neuendorffer
Cc: <redacted>; git-dev
Subject: Re: [PATCH] Xilinx: framebuffer: add compatibility for ml507

dvi core.

=20
On Mon, May 12, 2008 at 1:59 PM, Stephen Neuendorffer
[off-list ref] wrote:

quoted

quoted

-----Original Message-----
From: <redacted> [mailto:<redacted>] On Behalf

Of

quoted

Grant Likely

quoted

Sent: Monday, May 12, 2008 12:46 PM
To: Stephen Neuendorffer
Cc: <redacted>; git-dev
Subject: Re: [PATCH] Xilinx: framebuffer: add compatibility for

ml507

quoted

dvi core.

quoted

On Mon, May 12, 2008 at 1:10 PM, Stephen Neuendorffer
[off-list ref] wrote:

quoted

 The best possibility that I see is glopping the compatibility
 information about what cores are compatible with which drivers

and

quoted

quoted

quoted

 generating something.  This is moderately better than blindly

treating

quoted

quoted

 all cores with the same major version as interface compatible,

but

quoted

still

quoted

quoted

 has the potential to blindly declare that core versions are

compatible

quoted

quoted

 when they are not really compatible.

That's okay, the device tree conventions provide for that

uncertainty.

quoted

quoted

 If compatible includes both the *exact* version and the oldest

known

quoted

quoted

*compatible* version (the one the drivers know about) then we're in
the situation where 99% of the time it just works.  For the 1% of

the

quoted

quoted

time when mistakes are made we still have the necessary information

to

quoted

quoted

write exceptions in the code to work around bad data.  This means

code

quoted

quoted

only needs to changes when mistakes are discovered, not for every

IP

quoted

quoted

core uprev.

My argument was that we should do this by truncating the major

version,

quoted

which is also an established standard that cores *should* follow,

but

quoted

you didn't like that. :)  It makes at least as much sense as

expressing

quoted

the compatibility of the driver in the tree in terms of

compatibility

quoted

with some other random driver.  In the case of the tft core in
particular, there *is* no other driver AFAIK.

=20
The whole point of compatible is to describe software interfaces...
*if* a device is register level backwards compatible with an older
already supported device, *then* it is appropriate to claim
compatibility with it.  True, the TFT core is not the same as the VGA
core.  But (as your patch suggests) the register interface is
backwards compatible.

Certainly... I don't think the question is how to do it, I think the
question is how to systematically get this information from the people
who build the cores, rather than having it put in on a case-by-case
basis somewhere down the line.  This is as much a Xilinx organizational
issue as anything.  If we *can't* get it in a systematic way, then I see
nothing better than putting it explicitly in the compatible list.  The
current answer that I've gotten from the IP group is that the systematic
way to represent this information is through the major/minor version.
Specifically:

"The brief summary is that in a <major>.<minor>.<letter> version string:
- a change of the <letter> guarantees complete backward compatibility
- a change of the <minor> is new or modified behavior.  The register
interface should remain compatible, although new features may be added.
I wouldn't say this guarantees backward compatibility, though, because
default values can change or behavior in general can change.
- a change of the <major> is quite likely to lose backward
compatibility."

As for truncating the major version; you're right, I don't like it,
but that doesn't mean I cannot be swayed.  Best argument is to
analogize it with a similar SoC situation.  A particular SoC (let's
say the MPC8349) will have several revisions to it, but none of that
is reflected in the device tree.  It's just referred to as
"fsl,mpc8349".  Actual silicon revision is obtainable by software from
the PVR/SVR if it *really* needs it.  The counter example is the
MPC5200/MPC5200b where the 'b' version is explicitly specified in the
compatible list (see arch/powerpc/boot/dts/lite5200b.dts).  The 5200b
is 99% compatible with the original 5200, with only a couple of on
chip peripherals not being register level compatible.  I'm still not
completely certain that "fsl,mpc5200b-<blah>" was the right decision,
but by being conservative early on means that I can still drop most of
the 'b' specifiers at some point in the future without breaking board
support.
=20
What makes me nervous about FPGA IP cores is that the potential for
change is much higher than for an SoC.  SoC vendors get very angry
customers if a silicon uprev breaks their drivers; especially
considering that they could very easily have boards with both the old
and new silicon.  It does not seem to me like there is quite the same
level of pressure too keep the register level interface 100%
compatible.  So this is the pressure point to apply if you want to
change my mind.  How confident are you that the major (or minor)
revision will remain register level compatible?

I'd actually turn it around: In most cases users of an FPGA device have
*much* more control over these versions than in a microprocessor
situation.  Generally speaking, if there is a letter-version increment,
EDK generally forces you to accept the version increment when moving
from one version of EDK (with one set of IP versions) to another.  If
there is a minor-version increment, then EDK asks you if you want to
change.  If there is a major-version increment, then EDK notifies you of
the change, but doesn't do anything automatically.  You certainly can't
do that in an SoC.

n>> >  I *really* don't want to put this into the device tree

generator on

quoted

a

quoted

quoted

 case-by-case basis, so unless there is something that can be

generated

quoted

quoted

 from whatever meta-information EDK has, I think we're going to

have

quoted

to

quoted

quoted

 just have the explicit versions in the drivers for the time

being.

quoted

quoted

Can we post process the generated device tree with a table of known
compatibility strings (or regexps) for adding the older compatible
values?  I don't expect the list will be particularly long or hard

to

quoted

quoted

maintain and the code to do so should be trivial.

Ug, that's just pushing the problem around.
This seems as much like an argument for putting wildcards in the
compatible bindings in the kernel as anything...

=20
Not quite.  There is a difference between method used to generate the
data, and how the data is interpreted by the kernel (the boundary
being what data is actually passed to the kernel).  If the tool
generates bad/inaccurate data, then it is a bug and it should be
fixed.  Even better, it is a bug that will be found quickly because
the device simply won't work if it binds to the wrong driver.  I do
not want to load knowledge of all those permutation into the kernel
image.

(But I do agree that my suggestion was rather smelly)

HA!  Well, I guess I was trying to be polite.. :)  The much bigger
problem than binding to the wrong driver is not binding to the right
driver, particularly for anything having to do with the console.

So, what I hear is that you might be accepting of using compatible
strings of the form:

xlnx,opb-hwicap-1   (saving ~5 compatible entries in the current xilinx
git tree)
or=20
xlnx,opb-hwicap-1.00  (saving ~4 more)

Steve

arch/powerpc/boot/serial.c:     else if (dt_is_compatible(devp,
"xlnx,opb-uartlite-1.00.b") ||
arch/powerpc/boot/serial.c:              dt_is_compatible(devp,
"xlnx,xps-uartlite-1.00.a"))
arch/powerpc/boot/serial.c:     else if (dt_is_compatible(devp,
"xlnx,opb-uart16550-1.00.c") ||
arch/powerpc/boot/serial.c:              dt_is_compatible(devp,
"xlnx,opb-uart16550-1.00.d") ||
arch/powerpc/boot/serial.c:              dt_is_compatible(devp,
"xlnx,opb-uart16550-1.00.e") ||
arch/powerpc/boot/serial.c:              dt_is_compatible(devp,
"xlnx,plb-uart16550-1.00.c") ||
arch/powerpc/boot/serial.c:              dt_is_compatible(devp,
"xlnx,xps-uart16550-1.00.a") ||
arch/powerpc/boot/serial.c:              dt_is_compatible(devp,
"xlnx,xps-uart16550-2.00.a") ||
arch/powerpc/boot/serial.c:              dt_is_compatible(devp,
"xlnx,xps-uart16550-2.00.b"))
arch/powerpc/platforms/44x/virtex.c:    { .compatible =3D
"xlnx,plb-v46-1.00.a", },
arch/powerpc/platforms/44x/virtex.c:    { .compatible =3D
"xlnx,plb-v46-1.02.a", },
arch/powerpc/platforms/44x/virtex.c:    { .compatible =3D
"xlnx,plb-v34-1.01.a", },
arch/powerpc/platforms/44x/virtex.c:    { .compatible =3D

92 more lines