Re: [PATCH] Xilinx: framebuffer: add compatibility for ml507 dvi core.
On Mon, May 12, 2008 at 1:59 PM, Stephen Neuendorffer [EMAIL PROTECTED] wrote: -Original Message- From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On Behalf Of Grant Likely Sent: Monday, May 12, 2008 12:46 PM To: Stephen Neuendorffer Cc: linuxppc-dev@ozlabs.org; git-dev Subject: Re: [PATCH] Xilinx: framebuffer: add compatibility for ml507 dvi core. On Mon, May 12, 2008 at 1:10 PM, Stephen Neuendorffer [EMAIL PROTECTED] 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. 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 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. 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
RE: [PATCH] Xilinx: framebuffer: add compatibility for ml507 dvi core.
-Original Message- From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On Behalf Of Grant Likely Sent: Wednesday, May 14, 2008 8:35 AM To: Stephen Neuendorffer Cc: linuxppc-dev@ozlabs.org; git-dev Subject: Re: [PATCH] Xilinx: framebuffer: add compatibility for ml507 dvi core. On Mon, May 12, 2008 at 1:59 PM, Stephen Neuendorffer [EMAIL PROTECTED] wrote: -Original Message- From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On Behalf Of Grant Likely Sent: Monday, May 12, 2008 12:46 PM To: Stephen Neuendorffer Cc: linuxppc-dev@ozlabs.org; git-dev Subject: Re: [PATCH] Xilinx: framebuffer: add compatibility for ml507 dvi core. On Mon, May 12, 2008 at 1:10 PM, Stephen Neuendorffer [EMAIL PROTECTED] 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. 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. 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. 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
Re: [PATCH] Xilinx: framebuffer: add compatibility for ml507 dvi core.
On Mon, May 5, 2008 at 11:59 AM, Stephen Neuendorffer
[EMAIL PROTECTED] wrote:
Signed-off-by: Stephen Neuendorffer [EMAIL PROTECTED]
---
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.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
RE: [PATCH] Xilinx: framebuffer: add compatibility for ml507 dvi core.
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: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On Behalf Of
Grant Likely
Sent: Monday, May 12, 2008 7:25 AM
To: Stephen Neuendorffer
Cc: linuxppc-dev@ozlabs.org
Subject: Re: [PATCH] Xilinx: framebuffer: add compatibility for ml507
dvi core.
On Mon, May 5, 2008 at 11:59 AM, Stephen Neuendorffer
[EMAIL PROTECTED] wrote:
Signed-off-by: Stephen Neuendorffer
[EMAIL PROTECTED]
---
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.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] Xilinx: framebuffer: add compatibility for ml507 dvi core.
On Mon, May 12, 2008 at 11:31 AM, Stephen Neuendorffer [EMAIL PROTECTED] 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. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] Xilinx: framebuffer: add compatibility for ml507 dvi core.
On Mon, May 12, 2008 at 1:10 PM, Stephen Neuendorffer [EMAIL PROTECTED] 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. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
RE: [PATCH] Xilinx: framebuffer: add compatibility for ml507 dvi core.
-Original Message- From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On Behalf Of Grant Likely Sent: Monday, May 12, 2008 12:46 PM To: Stephen Neuendorffer Cc: linuxppc-dev@ozlabs.org; git-dev Subject: Re: [PATCH] Xilinx: framebuffer: add compatibility for ml507 dvi core. On Mon, May 12, 2008 at 1:10 PM, Stephen Neuendorffer [EMAIL PROTECTED] 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. 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. 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. 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 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH] Xilinx: framebuffer: add compatibility for ml507 dvi core.
Signed-off-by: Stephen Neuendorffer [EMAIL PROTECTED]
---
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
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
