Re: [PATCH 3/4 v4] video, sm501: add OF binding to support SM501
Hi Heiko, On Mon, Jan 24, 2011 at 10:57:38AM +0100, Heiko Schocher wrote: - add binding to OF, compatible name smi,sm501 The MFD part looks fine to me: Acked-by: Samuel Ortiz sa...@linux.intel.com Cheers, Samuel. -- Intel Open Source Technology Centre http://oss.intel.com/ ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 3/4 v4] video, sm501: add OF binding to support SM501
Hello Paul,
Paul Mundt wrote:
On Tue, Jan 25, 2011 at 08:20:31AM +0100, Heiko Schocher wrote:
@@ -1934,7 +1943,29 @@ static int __devinit sm501fb_probe(struct
platform_device *pdev)
}
if (info-pdata == NULL) {
-dev_info(dev, using default configuration data\n);
+int found = 0;
+#if defined(CONFIG_OF)
+struct device_node *np = pdev-dev.parent-of_node;
+const u8 *prop;
+const char *cp;
+int len;
+
+info-pdata = sm501fb_def_pdata;
+if (np) {
+/* Get EDID */
+cp = of_get_property(np, mode, len);
+if (cp)
+strcpy(fb_mode, cp);
+prop = of_get_property(np, edid, len);
+if (prop len == EDID_LENGTH) {
+info-edid_data = kmemdup(prop, EDID_LENGTH,
+ GFP_KERNEL);
+found = 1;
+}
+}
+#endif
+if (!found)
+dev_info(dev, using default configuration data\n);
}
/* probe for the presence of each panel */
Starting to get a bit pedantic.. but kmemdup() tries to do a kmalloc(),
No problem!
and so can fail. Your other patches handle the info-edid_data == NULL
case, in addition to the kfree(), but you're probably going to want to
chomp that found assignment incase of the allocation failing and falling
back on the default mode.
You also don't really have any need to keep the EDID block around after
probe as far as I can tell, so you should be able to rework this in to
something more like:
info-edid_data = kmemdup(..);
...
if (info-edid_data) {
fb_edid_to_monspecs(..);
kfree(info-edid_data);
fb_videomode_to_modelist(..);
}
...
Ok, rework this part, thanks!
bye,
Heiko
--
DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 3/4 v4] video, sm501: add OF binding to support SM501
- add binding to OF, compatible name smi,sm501
Signed-off-by: Heiko Schocher h...@denx.de
cc: linux-fb...@vger.kernel.org
cc: devicetree-disc...@ozlabs.org
cc: Ben Dooks b...@simtec.co.uk
cc: Vincent Sanders vi...@simtec.co.uk
cc: Samuel Ortiz sa...@linux.intel.com
cc: linux-ker...@vger.kernel.org
cc: Randy Dunlap rdun...@xenotime.net
cc: Paul Mundt let...@linux-sh.org
---
- changes since v1:
add Ben Dooks, Vincent Sanders and Samuel Ortiz to cc, as suggested from
Paul Mundt.
- changes since v2:
add comments from Randy Dunlap:
- move parameter documentation to Documentation/fb/sm501.txt
- changes since v3:
- rebased against v2.6.38-rc2
- split in 3 patches
- of support patch
- get rid of #if defined(CONFIG_PPC_MPC52xx) usage
hide this in DTS, as Paul suggested.
- i/o routine patch
- edid support patch
./scripts/checkpatch.pl 0003-video-sm501-add-OF-binding-to-support-SM501.patch
total: 0 errors, 0 warnings, 117 lines checked
0003-video-sm501-add-OF-binding-to-support-SM501.patch has no obvious style
problems and is ready for submission.
Documentation/powerpc/dts-bindings/sm501.txt | 34 ++
drivers/mfd/sm501.c | 16 +++-
drivers/video/sm501fb.c | 33 -
3 files changed, 81 insertions(+), 2 deletions(-)
create mode 100644 Documentation/powerpc/dts-bindings/sm501.txt
diff --git a/Documentation/powerpc/dts-bindings/sm501.txt
b/Documentation/powerpc/dts-bindings/sm501.txt
new file mode 100644
index 000..7d319fb
--- /dev/null
+++ b/Documentation/powerpc/dts-bindings/sm501.txt
@@ -0,0 +1,34 @@
+* SM SM501
+
+The SM SM501 is a LCD controller, with proper hardware, it can also
+drive DVI monitors.
+
+Required properties:
+- compatible : should be smi,sm501.
+- reg : contain two entries:
+- First entry: System Configuration register
+- Second entry: IO space (Display Controller register)
+- interrupts : SMI interrupt to the cpu should be described here.
+- interrupt-parent : the phandle for the interrupt controller that
+ services interrupts for this device.
+
+Optional properties:
+- mode : select a video mode:
+xresxyres[-bpp][@refresh]
+- edid : verbatim EDID data block describing attached display.
+ Data from the detailed timing descriptor will be used to
+ program the display controller.
+- little-endian: availiable on big endian systems, to
+ set different foreign endian.
+- big-endian: availiable on little endian systems, to
+ set different foreign endian.
+
+Example for MPC5200:
+ display@1,0 {
+ compatible = smi,sm501;
+ reg = 1 0x 0x0080
+ 1 0x03e0 0x0020;
+ interrupts = 1 1 3;
+ mode = 640x480-32@60;
+ edid = [edid-data];
+ };
diff --git a/drivers/mfd/sm501.c b/drivers/mfd/sm501.c
index 558d5f3..5b7a8f4 100644
--- a/drivers/mfd/sm501.c
+++ b/drivers/mfd/sm501.c
@@ -1377,7 +1377,7 @@ static int __devinit sm501_init_dev(struct sm501_devdata
*sm)
sm501_register_gpio(sm);
}
- if (pdata-gpio_i2c != NULL pdata-gpio_i2c_nr 0) {
+ if (pdata pdata-gpio_i2c != NULL pdata-gpio_i2c_nr 0) {
if (!sm501_gpio_isregistered(sm))
dev_err(sm-dev, no gpio available for i2c gpio.\n);
else
@@ -1422,6 +1422,14 @@ static int __devinit sm501_plat_probe(struct
platform_device *dev)
sm-io_res = platform_get_resource(dev, IORESOURCE_MEM, 1);
sm-mem_res = platform_get_resource(dev, IORESOURCE_MEM, 0);
+
+ if (sm-mem_res)
+ pr_debug(sm501 mem 0x%lx, 0x%lx\n,
+sm-mem_res-start, sm-mem_res-end);
+ if (sm-io_res)
+ pr_debug(sm501 io 0x%lx, 0x%lx\n,
+sm-io_res-start, sm-io_res-end);
+
if (sm-io_res == NULL || sm-mem_res == NULL) {
dev_err(dev-dev, failed to get IO resource\n);
ret = -ENOENT;
@@ -1735,10 +1743,16 @@ static struct pci_driver sm501_pci_driver = {
MODULE_ALIAS(platform:sm501);
+static struct of_device_id __devinitdata of_sm501_match_tbl[] = {
+ { .compatible = smi,sm501, },
+ { /* end */ }
+};
+
static struct platform_driver sm501_plat_driver = {
.driver = {
.name = sm501,
.owner = THIS_MODULE,
+ .of_match_table = of_sm501_match_tbl,
},
.probe = sm501_plat_probe,
.remove = sm501_plat_remove,
diff --git a/drivers/video/sm501fb.c b/drivers/video/sm501fb.c
index 30b53ae..2ae57aa 100644
--- a/drivers/video/sm501fb.c
+++ b/drivers/video/sm501fb.c
@@ -1729,6 +1729,15 @@ static int sm501fb_init_fb(struct fb_info *fb,
FBINFO_HWACCEL_COPYAREA | FBINFO_HWACCEL_FILLRECT |
FBINFO_HWACCEL_XPAN | FBINFO_HWACCEL_YPAN;
+#if
[PATCH 3/4 v4] video, sm501: add OF binding to support SM501
- add binding to OF, compatible name smi,sm501
Signed-off-by: Heiko Schocher h...@denx.de
cc: linux-fb...@vger.kernel.org
cc: devicetree-disc...@ozlabs.org
cc: Ben Dooks b...@simtec.co.uk
cc: Vincent Sanders vi...@simtec.co.uk
cc: Samuel Ortiz sa...@linux.intel.com
cc: linux-ker...@vger.kernel.org
cc: Randy Dunlap rdun...@xenotime.net
cc: Paul Mundt let...@linux-sh.org
---
- changes since v1:
add Ben Dooks, Vincent Sanders and Samuel Ortiz to cc, as suggested from
Paul Mundt.
- changes since v2:
add comments from Randy Dunlap:
- move parameter documentation to Documentation/fb/sm501.txt
- changes since v3:
- rebased against v2.6.38-rc2
- split in 3 patches
- of support patch
- get rid of #if defined(CONFIG_PPC_MPC52xx) usage
hide this in DTS, as Paul suggested.
- i/o routine patch
- edid support patch
- changes since v4
replace remaining CONFIG_PPC_MPC52xx with CONFIG_OF, as
it is no longer MPC52xx only.
./scripts/checkpatch.pl 0003-video-sm501-add-OF-binding-to-support-SM501.patch
total: 0 errors, 0 warnings, 117 lines checked
0003-video-sm501-add-OF-binding-to-support-SM501.patch has no obvious style
problems and is ready for submission.
Documentation/powerpc/dts-bindings/sm501.txt | 34 ++
drivers/mfd/sm501.c | 16 +++-
drivers/video/sm501fb.c | 33 -
3 files changed, 81 insertions(+), 2 deletions(-)
create mode 100644 Documentation/powerpc/dts-bindings/sm501.txt
diff --git a/Documentation/powerpc/dts-bindings/sm501.txt
b/Documentation/powerpc/dts-bindings/sm501.txt
new file mode 100644
index 000..7d319fb
--- /dev/null
+++ b/Documentation/powerpc/dts-bindings/sm501.txt
@@ -0,0 +1,34 @@
+* SM SM501
+
+The SM SM501 is a LCD controller, with proper hardware, it can also
+drive DVI monitors.
+
+Required properties:
+- compatible : should be smi,sm501.
+- reg : contain two entries:
+- First entry: System Configuration register
+- Second entry: IO space (Display Controller register)
+- interrupts : SMI interrupt to the cpu should be described here.
+- interrupt-parent : the phandle for the interrupt controller that
+ services interrupts for this device.
+
+Optional properties:
+- mode : select a video mode:
+xresxyres[-bpp][@refresh]
+- edid : verbatim EDID data block describing attached display.
+ Data from the detailed timing descriptor will be used to
+ program the display controller.
+- little-endian: availiable on big endian systems, to
+ set different foreign endian.
+- big-endian: availiable on little endian systems, to
+ set different foreign endian.
+
+Example for MPC5200:
+ display@1,0 {
+ compatible = smi,sm501;
+ reg = 1 0x 0x0080
+ 1 0x03e0 0x0020;
+ interrupts = 1 1 3;
+ mode = 640x480-32@60;
+ edid = [edid-data];
+ };
diff --git a/drivers/mfd/sm501.c b/drivers/mfd/sm501.c
index 558d5f3..5b7a8f4 100644
--- a/drivers/mfd/sm501.c
+++ b/drivers/mfd/sm501.c
@@ -1377,7 +1377,7 @@ static int __devinit sm501_init_dev(struct sm501_devdata
*sm)
sm501_register_gpio(sm);
}
- if (pdata-gpio_i2c != NULL pdata-gpio_i2c_nr 0) {
+ if (pdata pdata-gpio_i2c != NULL pdata-gpio_i2c_nr 0) {
if (!sm501_gpio_isregistered(sm))
dev_err(sm-dev, no gpio available for i2c gpio.\n);
else
@@ -1422,6 +1422,14 @@ static int __devinit sm501_plat_probe(struct
platform_device *dev)
sm-io_res = platform_get_resource(dev, IORESOURCE_MEM, 1);
sm-mem_res = platform_get_resource(dev, IORESOURCE_MEM, 0);
+
+ if (sm-mem_res)
+ pr_debug(sm501 mem 0x%lx, 0x%lx\n,
+sm-mem_res-start, sm-mem_res-end);
+ if (sm-io_res)
+ pr_debug(sm501 io 0x%lx, 0x%lx\n,
+sm-io_res-start, sm-io_res-end);
+
if (sm-io_res == NULL || sm-mem_res == NULL) {
dev_err(dev-dev, failed to get IO resource\n);
ret = -ENOENT;
@@ -1735,10 +1743,16 @@ static struct pci_driver sm501_pci_driver = {
MODULE_ALIAS(platform:sm501);
+static struct of_device_id __devinitdata of_sm501_match_tbl[] = {
+ { .compatible = smi,sm501, },
+ { /* end */ }
+};
+
static struct platform_driver sm501_plat_driver = {
.driver = {
.name = sm501,
.owner = THIS_MODULE,
+ .of_match_table = of_sm501_match_tbl,
},
.probe = sm501_plat_probe,
.remove = sm501_plat_remove,
diff --git a/drivers/video/sm501fb.c b/drivers/video/sm501fb.c
index 30b53ae..bbdb359 100644
--- a/drivers/video/sm501fb.c
+++ b/drivers/video/sm501fb.c
@@ -1729,6 +1729,15 @@ static int sm501fb_init_fb(struct fb_info *fb,
FBINFO_HWACCEL_COPYAREA |
Re: [PATCH 3/4 v4] video, sm501: add OF binding to support SM501
On Tue, Jan 25, 2011 at 08:20:31AM +0100, Heiko Schocher wrote:
@@ -1934,7 +1943,29 @@ static int __devinit sm501fb_probe(struct
platform_device *pdev)
}
if (info-pdata == NULL) {
- dev_info(dev, using default configuration data\n);
+ int found = 0;
+#if defined(CONFIG_OF)
+ struct device_node *np = pdev-dev.parent-of_node;
+ const u8 *prop;
+ const char *cp;
+ int len;
+
+ info-pdata = sm501fb_def_pdata;
+ if (np) {
+ /* Get EDID */
+ cp = of_get_property(np, mode, len);
+ if (cp)
+ strcpy(fb_mode, cp);
+ prop = of_get_property(np, edid, len);
+ if (prop len == EDID_LENGTH) {
+ info-edid_data = kmemdup(prop, EDID_LENGTH,
+ GFP_KERNEL);
+ found = 1;
+ }
+ }
+#endif
+ if (!found)
+ dev_info(dev, using default configuration data\n);
}
/* probe for the presence of each panel */
Starting to get a bit pedantic.. but kmemdup() tries to do a kmalloc(),
and so can fail. Your other patches handle the info-edid_data == NULL
case, in addition to the kfree(), but you're probably going to want to
chomp that found assignment incase of the allocation failing and falling
back on the default mode.
You also don't really have any need to keep the EDID block around after
probe as far as I can tell, so you should be able to rework this in to
something more like:
info-edid_data = kmemdup(..);
...
if (info-edid_data) {
fb_edid_to_monspecs(..);
kfree(info-edid_data);
fb_videomode_to_modelist(..);
}
...
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
