| Message ID | 1375208791-15781-19-git-send-email-detheridge@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 30/07/13 21:26, Darren Etheridge wrote: > From: Afzal Mohammed <afzal@ti.com> > > Driver is provided a means to have the probe triggered by DT. > > Signed-off-by: Afzal Mohammed <afzal@ti.com> > Signed-off-by: Darren Etheridge <detheridge@ti.com> > --- > .../devicetree/bindings/video/fb-da8xx.txt | 16 ++++++++++++++++ > drivers/video/da8xx-fb.c | 7 +++++++ > 2 files changed, 23 insertions(+), 0 deletions(-) > create mode 100644 Documentation/devicetree/bindings/video/fb-da8xx.txt > > diff --git a/Documentation/devicetree/bindings/video/fb-da8xx.txt b/Documentation/devicetree/bindings/video/fb-da8xx.txt > new file mode 100644 > index 0000000..581e014 > --- /dev/null > +++ b/Documentation/devicetree/bindings/video/fb-da8xx.txt > @@ -0,0 +1,16 @@ > +TI LCD Controller on DA830/DA850/AM335x SoC's > + > +Required properties: > +- compatible: > + DA830 - "ti,da830-lcdc" > + AM335x SoC's - "ti,am3352-lcdc", "ti,da830-lcdc" I'm not totally sure about this, but how I understand the compatible property, the above reads as: "this device is ti,am3352-lcdc and it's fully compatible with ti,da830-lcdc". I.e. if the kernel has a driver for da830-lcdc, it should work with AM335x also (without any of the new features in AM335x, obviously). Which I believe is not the case, as the point of this series is to add the AM335x support. Or should the current da830-lcdc work with AM335x also, but it just didn't because there were bugs in da830-lcdc? Tomi
Tomi Valkeinen <tomi.valkeinen@ti.com> wrote on Wed [2013-Jul-31 13:19:01 +0300]: > > +Required properties: > > +- compatible: > > + DA830 - "ti,da830-lcdc" > > + AM335x SoC's - "ti,am3352-lcdc", "ti,da830-lcdc" > > I'm not totally sure about this, but how I understand the compatible > property, the above reads as: "this device is ti,am3352-lcdc and it's > fully compatible with ti,da830-lcdc". I.e. if the kernel has a driver > for da830-lcdc, it should work with AM335x also (without any of the new > features in AM335x, obviously). Which I believe is not the case, as the > point of this series is to add the AM335x support. > > Or should the current da830-lcdc work with AM335x also, but it just > didn't because there were bugs in da830-lcdc? > > Tomi > > OK I agree there is something wrong here, for one I don't think setting ti,am3352-lcdc would do anything anyway given the driver only reports .compatible with ti,da830-lcdc so at the very least the document is wrong. I will look into this and decide what is the best way of resolving this. I will go ahead and submit the series without the DT support anyway and then I will look into this. Darren -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Darren Etheridge <detheridge@ti.com> wrote on Thu [2013-Aug-01 08:36:02 -0500]: > Tomi Valkeinen <tomi.valkeinen@ti.com> wrote on Wed [2013-Jul-31 13:19:01 +0300]: > > > +Required properties: > > > +- compatible: > > > + DA830 - "ti,da830-lcdc" > > > + AM335x SoC's - "ti,am3352-lcdc", "ti,da830-lcdc" > > > > I'm not totally sure about this, but how I understand the compatible > > property, the above reads as: "this device is ti,am3352-lcdc and it's > > fully compatible with ti,da830-lcdc". I.e. if the kernel has a driver > > for da830-lcdc, it should work with AM335x also (without any of the new > > features in AM335x, obviously). Which I believe is not the case, as the > > point of this series is to add the AM335x support. > > > > Or should the current da830-lcdc work with AM335x also, but it just > > didn't because there were bugs in da830-lcdc? > > > > Tomi > > > > > OK I agree there is something wrong here, for one I don't think setting > ti,am3352-lcdc would do anything anyway given the driver only reports > .compatible with ti,da830-lcdc so at the very least the document is > wrong. I will look into this and decide what is the best way of > resolving this. I will go ahead and submit the series without the DT > support anyway and then I will look into this. > Reading the device tree documentation I think what is here in the patch is actually correct so ignore my first response. In answer to your original question in theory the da830 driver would be fully compatible with the am335x - am335x implements a version 2 of the lcd controller that adds some extra reg bits that support larger horizontal and vertical resolutions and a sync loss interrupt. However the driver takes care of those differences internally. I think the main thing that was needed from this patch set to support am335x were the dt changes, the Kconfig changes and the clock changes - the rest of it are enhancements and bug fixes that are not strictly needed for am335x support. Perhaps the correct thing to do and maybe what you are getting at is really am3352 is the superset driver which we have now added complete support for with these patches. However because we handle the differences between the two IP's directly in the driver we should have the driver provide both ti,am3352-lcdc and ti,da830-lcdc as .compatible entries because really the driver supports both version 1 and version 2 IP's. -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 01/08/13 23:21, Darren Etheridge wrote: > Darren Etheridge <detheridge@ti.com> wrote on Thu [2013-Aug-01 08:36:02 -0500]: >> Tomi Valkeinen <tomi.valkeinen@ti.com> wrote on Wed [2013-Jul-31 13:19:01 +0300]: >>>> +Required properties: >>>> +- compatible: >>>> + DA830 - "ti,da830-lcdc" >>>> + AM335x SoC's - "ti,am3352-lcdc", "ti,da830-lcdc" >>> >>> I'm not totally sure about this, but how I understand the compatible >>> property, the above reads as: "this device is ti,am3352-lcdc and it's >>> fully compatible with ti,da830-lcdc". I.e. if the kernel has a driver >>> for da830-lcdc, it should work with AM335x also (without any of the new >>> features in AM335x, obviously). Which I believe is not the case, as the >>> point of this series is to add the AM335x support. >>> >>> Or should the current da830-lcdc work with AM335x also, but it just >>> didn't because there were bugs in da830-lcdc? >>> >>> Tomi >>> >>> >> OK I agree there is something wrong here, for one I don't think setting >> ti,am3352-lcdc would do anything anyway given the driver only reports >> .compatible with ti,da830-lcdc so at the very least the document is >> wrong. I will look into this and decide what is the best way of >> resolving this. I will go ahead and submit the series without the DT >> support anyway and then I will look into this. >> > Reading the device tree documentation I think what is here in the > patch is actually correct so ignore my first response. > > In answer to your original question in theory the da830 driver would > be fully compatible with the am335x - am335x implements a version 2 of > the lcd controller that adds some extra reg bits that support larger > horizontal and vertical resolutions and a sync loss interrupt. > However the driver takes care of those differences internally. I > think the main thing that was needed from this patch set to support > am335x were the dt changes, the Kconfig changes and the clock changes > - the rest of it are enhancements and bug fixes that are not strictly > needed for am335x support. Ok. And the clock changes are also valid in da8xx context also? The need for them just didn't realize with our current use of da8xx? > Perhaps the correct thing to do and maybe what you are getting at is > really am3352 is the superset driver which we have now added complete > support for with these patches. However because we handle the > differences between the two IP's directly in the driver we should have > the driver provide both ti,am3352-lcdc and ti,da830-lcdc as > .compatible entries because really the driver supports both version 1 and > version 2 IP's. Hmm, right, I believe the driver should provide "ti,am3352-lcdc" also, because it does support the extra features offered in am3352. A bit side tracking, but: I've been wondering about the device names we use. Naming the lcdc driver by the SoC is not quite correct. I don't know all the IPs TI has, but what I see is that TI has two types of display controllers: the more basic LCDC, and the bigger DSS. Could we just call the device lcdc or dss? So, for example, DA830 has "ti,lcdc1", AM3352 has "ti,lcdc2", OMAP2 has "ti,dss2", OMAP4 has "ti,dss4", etc. Then again, I'm not sure if the HW development of LCDC or DSS goes quite that linearly. There are multiple SoCs that have OMAP3's DSS, but are they the same DSS, or does each of them have a slightly different version, without there clearly being a DSS version 3.1, 3.2, 3.3, etc... Tomi
diff --git a/Documentation/devicetree/bindings/video/fb-da8xx.txt b/Documentation/devicetree/bindings/video/fb-da8xx.txt new file mode 100644 index 0000000..581e014 --- /dev/null +++ b/Documentation/devicetree/bindings/video/fb-da8xx.txt @@ -0,0 +1,16 @@ +TI LCD Controller on DA830/DA850/AM335x SoC's + +Required properties: +- compatible: + DA830 - "ti,da830-lcdc" + AM335x SoC's - "ti,am3352-lcdc", "ti,da830-lcdc" +- reg: Address range of lcdc register set +- interrupts: lcdc interrupt + +Example: + +lcdc@4830e000 { + compatible = "ti,am3352-lcdc", "ti,da830-lcdc"; + reg = <0x4830e000 0x1000>; + interrupts = <36>; +}; diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c index 983176a..97d6b2d 100644 --- a/drivers/video/da8xx-fb.c +++ b/drivers/video/da8xx-fb.c @@ -1596,6 +1596,12 @@ static int fb_resume(struct platform_device *dev) #define fb_resume NULL #endif +static const struct of_device_id da8xx_fb_of_match[] = { + {.compatible = "ti,da830-lcdc", }, + {}, +}; +MODULE_DEVICE_TABLE(of, da8xx_fb_of_match); + static struct platform_driver da8xx_fb_driver = { .probe = fb_probe, .remove = fb_remove, @@ -1604,6 +1610,7 @@ static struct platform_driver da8xx_fb_driver = { .driver = { .name = DRIVER_NAME, .owner = THIS_MODULE, + .of_match_table = of_match_ptr(da8xx_fb_of_match), }, };