* RE: [PATCH] fbdev/xilinxfb: Fix improper casting and tighen up probe path
2009-06-12 17:57 [PATCH] fbdev/xilinxfb: Fix improper casting and tighen up probe path Grant Likely
@ 2009-06-15 15:16 ` John Linn
2009-06-15 17:06 ` John Linn
1 sibling, 0 replies; 3+ messages in thread
From: John Linn @ 2009-06-15 15:16 UTC (permalink / raw)
To: Grant Likely, linuxppc-dev, linux-kernel, benh
> -----Original Message-----
> From: Grant Likely [mailto:grant.likely@secretlab.ca]
> Sent: Friday, June 12, 2009 11:57 AM
> To: linuxppc-dev@ozlabs.org; linux-kernel@vger.kernel.org;
benh@kernel.crashing.org; John Linn
> Subject: [PATCH] fbdev/xilinxfb: Fix improper casting and tighen up
probe path
> =
> From: Grant Likely <grant.likely@secretlab.ca>
> =
> The xilinxfb driver is improperly casting a physical address to a
> u32, and the probe routine isn't as straight forward as it could be.
> (discovered by gcc spitting out warnings on most recent change to
> xilinxfb driver).
> =
> This patch fixes the cast and simplifies the probe path.
> =
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> ---
> =
> John, can you please test and verify this fix is correct?
I will try to test it today.
-- John
> =
> Thanks,
> g.
> =
> drivers/video/xilinxfb.c | 59
++++++++++++++++++----------------------------
> 1 files changed, 23 insertions(+), 36 deletions(-)
> =
> =
> diff --git a/drivers/video/xilinxfb.c b/drivers/video/xilinxfb.c
> index 7a868bd..b68b331 100644
> --- a/drivers/video/xilinxfb.c
> +++ b/drivers/video/xilinxfb.c
> @@ -124,7 +124,6 @@ struct xilinxfb_drvdata {
> registers */
> =
> dcr_host_t dcr_host;
> - unsigned int dcr_start;
> unsigned int dcr_len;
> =
> void *fb_virt; /* virt. address of the frame
buffer */
> @@ -325,8 +324,8 @@ static int xilinxfb_assign(struct device *dev,
> drvdata->regs);
> }
> /* Put a banner in the log (for DEBUG) */
> - dev_dbg(dev, "fb: phys=3D%p, virt=3D%p, size=3D%x\n",
> - (void *)drvdata->fb_phys, drvdata->fb_virt, fbsize);
> + dev_dbg(dev, "fb: phys=3D%llx, virt=3D%p, size=3D%x\n",
> + (unsigned long long)drvdata->fb_phys, drvdata->fb_virt,
fbsize);
> =
> return 0; /* success */
> =
> @@ -404,9 +403,7 @@ xilinxfb_of_probe(struct of_device *op, const
struct of_device_id *match)
> u32 tft_access;
> struct xilinxfb_platform_data pdata;
> struct resource res;
> - int size, rc;
> - int start =3D 0, len =3D 0;
> - dcr_host_t dcr_host;
> + int size, rc, start;
> struct xilinxfb_drvdata *drvdata;
> =
> /* Copy with the default pdata (not a ptr reference!) */
> @@ -414,35 +411,39 @@ xilinxfb_of_probe(struct of_device *op, const
struct of_device_id *match)
> =
> dev_dbg(&op->dev, "xilinxfb_of_probe(%p, %p)\n", op, match);
> =
> + /* Allocate the driver data region */
> + drvdata =3D kzalloc(sizeof(*drvdata), GFP_KERNEL);
> + if (!drvdata) {
> + dev_err(&op->dev, "Couldn't allocate device private
record\n");
> + return -ENOMEM;
> + }
> +
> /*
> * To check whether the core is connected directly to DCR or PLB
> * interface and initialize the tft_access accordingly.
> */
> p =3D (u32 *)of_get_property(op->node, "xlnx,dcr-splb-slave-if",
NULL);
> -
> - if (p)
> - tft_access =3D *p;
> - else
> - tft_access =3D 0; /* For backward compatibility */
> + tft_access =3D p ? *p : 0;
> =
> /*
> * Fill the resource structure if its direct PLB interface
> * otherwise fill the dcr_host structure.
> */
> if (tft_access) {
> + drvdata->flags |=3D PLB_ACCESS_FLAG;
> rc =3D of_address_to_resource(op->node, 0, &res);
> if (rc) {
> dev_err(&op->dev, "invalid address\n");
> - return -ENODEV;
> + goto err;
> }
> -
> } else {
> + res.start =3D 0;
> start =3D dcr_resource_start(op->node, 0);
> - len =3D dcr_resource_len(op->node, 0);
> - dcr_host =3D dcr_map(op->node, start, len);
> - if (!DCR_MAP_OK(dcr_host)) {
> - dev_err(&op->dev, "invalid address\n");
> - return -ENODEV;
> + drvdata->dcr_len =3D dcr_resource_len(op->node, 0);
> + drvdata->dcr_host =3D dcr_map(op->node, start,
drvdata->len);
> + if (!DCR_MAP_OK(drvdata->dcr_host)) {
> + dev_err(&op->dev, "invalid DCR address\n");
> + goto err;
> }
> }
> =
> @@ -467,26 +468,12 @@ xilinxfb_of_probe(struct of_device *op, const
struct of_device_id *match)
> if (of_find_property(op->node, "rotate-display", NULL))
> pdata.rotate_screen =3D 1;
> =
> - /* Allocate the driver data region */
> - drvdata =3D kzalloc(sizeof(*drvdata), GFP_KERNEL);
> - if (!drvdata) {
> - dev_err(&op->dev, "Couldn't allocate device private
record\n");
> - return -ENOMEM;
> - }
> dev_set_drvdata(&op->dev, drvdata);
> + return xilinxfb_assign(&op->dev, drvdata, res.start, &pdata);
> =
> - if (tft_access)
> - drvdata->flags |=3D PLB_ACCESS_FLAG;
> -
> - /* Arguments are passed based on the interface */
> - if (drvdata->flags & PLB_ACCESS_FLAG) {
> - return xilinxfb_assign(&op->dev, drvdata, res.start,
&pdata);
> - } else {
> - drvdata->dcr_start =3D start;
> - drvdata->dcr_len =3D len;
> - drvdata->dcr_host =3D dcr_host;
> - return xilinxfb_assign(&op->dev, drvdata, 0, &pdata);
> - }
> + err:
> + kfree(drvdata);
> + return -ENODEV;
> }
> =
> static int __devexit xilinxfb_of_remove(struct of_device *op)
> =
This email and any attachments are intended for the sole use of the named r=
ecipient(s) and contain(s) confidential information that may be proprietary=
, privileged or copyrighted under applicable law. If you are not the intend=
ed recipient, do not read, copy, or forward this email message or any attac=
hments. Delete this email message and any attachments immediately.
^ permalink raw reply [flat|nested] 3+ messages in thread* RE: [PATCH] fbdev/xilinxfb: Fix improper casting and tighen up probe path
2009-06-12 17:57 [PATCH] fbdev/xilinxfb: Fix improper casting and tighen up probe path Grant Likely
2009-06-15 15:16 ` John Linn
@ 2009-06-15 17:06 ` John Linn
1 sibling, 0 replies; 3+ messages in thread
From: John Linn @ 2009-06-15 17:06 UTC (permalink / raw)
To: Grant Likely, linuxppc-dev, linux-kernel, benh
> -----Original Message-----
> From: Grant Likely [mailto:grant.likely@secretlab.ca]
> Sent: Friday, June 12, 2009 11:57 AM
> To: linuxppc-dev@ozlabs.org; linux-kernel@vger.kernel.org;
benh@kernel.crashing.org; John Linn
> Subject: [PATCH] fbdev/xilinxfb: Fix improper casting and tighen up
probe path
> =
> From: Grant Likely <grant.likely@secretlab.ca>
> =
> The xilinxfb driver is improperly casting a physical address to a
> u32, and the probe routine isn't as straight forward as it could be.
> (discovered by gcc spitting out warnings on most recent change to
> xilinxfb driver).
> =
> This patch fixes the cast and simplifies the probe path.
> =
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> ---
> =
> John, can you please test and verify this fix is correct?
After a small fix to get it to compile, I did some quick testing with
DCR and without and
it's working fine on the ML507.
-- John
Here's the small change to get it to compile.
diff --git a/drivers/video/xilinxfb.c b/drivers/video/xilinxfb.c
index e3d2992..596157c 100644
--- a/drivers/video/xilinxfb.c
+++ b/drivers/video/xilinxfb.c
@@ -441,7 +441,7 @@ xilinxfb_of_probe(struct of_device *op, const struct
of_device_id *match)
res.start =3D 0;
start =3D dcr_resource_start(op->node, 0);
drvdata->dcr_len =3D dcr_resource_len(op->node, 0);
- drvdata->dcr_host =3D dcr_map(op->node, start,
drvdata->len);
+ drvdata->dcr_host =3D dcr_map(op->node, start,
drvdata->dcr_len);
if (!DCR_MAP_OK(drvdata->dcr_host)) {
dev_err(&op->dev, "invalid DCR address\n");
goto err;
--
> =
> Thanks,
> g.
> =
> drivers/video/xilinxfb.c | 59
++++++++++++++++++----------------------------
> 1 files changed, 23 insertions(+), 36 deletions(-)
> =
> =
> diff --git a/drivers/video/xilinxfb.c b/drivers/video/xilinxfb.c
> index 7a868bd..b68b331 100644
> --- a/drivers/video/xilinxfb.c
> +++ b/drivers/video/xilinxfb.c
> @@ -124,7 +124,6 @@ struct xilinxfb_drvdata {
> registers */
> =
> dcr_host_t dcr_host;
> - unsigned int dcr_start;
> unsigned int dcr_len;
> =
> void *fb_virt; /* virt. address of the frame
buffer */
> @@ -325,8 +324,8 @@ static int xilinxfb_assign(struct device *dev,
> drvdata->regs);
> }
> /* Put a banner in the log (for DEBUG) */
> - dev_dbg(dev, "fb: phys=3D%p, virt=3D%p, size=3D%x\n",
> - (void *)drvdata->fb_phys, drvdata->fb_virt, fbsize);
> + dev_dbg(dev, "fb: phys=3D%llx, virt=3D%p, size=3D%x\n",
> + (unsigned long long)drvdata->fb_phys, drvdata->fb_virt,
fbsize);
> =
> return 0; /* success */
> =
> @@ -404,9 +403,7 @@ xilinxfb_of_probe(struct of_device *op, const
struct of_device_id *match)
> u32 tft_access;
> struct xilinxfb_platform_data pdata;
> struct resource res;
> - int size, rc;
> - int start =3D 0, len =3D 0;
> - dcr_host_t dcr_host;
> + int size, rc, start;
> struct xilinxfb_drvdata *drvdata;
> =
> /* Copy with the default pdata (not a ptr reference!) */
> @@ -414,35 +411,39 @@ xilinxfb_of_probe(struct of_device *op, const
struct of_device_id *match)
> =
> dev_dbg(&op->dev, "xilinxfb_of_probe(%p, %p)\n", op, match);
> =
> + /* Allocate the driver data region */
> + drvdata =3D kzalloc(sizeof(*drvdata), GFP_KERNEL);
> + if (!drvdata) {
> + dev_err(&op->dev, "Couldn't allocate device private
record\n");
> + return -ENOMEM;
> + }
> +
> /*
> * To check whether the core is connected directly to DCR or PLB
> * interface and initialize the tft_access accordingly.
> */
> p =3D (u32 *)of_get_property(op->node, "xlnx,dcr-splb-slave-if",
NULL);
> -
> - if (p)
> - tft_access =3D *p;
> - else
> - tft_access =3D 0; /* For backward compatibility */
> + tft_access =3D p ? *p : 0;
> =
> /*
> * Fill the resource structure if its direct PLB interface
> * otherwise fill the dcr_host structure.
> */
> if (tft_access) {
> + drvdata->flags |=3D PLB_ACCESS_FLAG;
> rc =3D of_address_to_resource(op->node, 0, &res);
> if (rc) {
> dev_err(&op->dev, "invalid address\n");
> - return -ENODEV;
> + goto err;
> }
> -
> } else {
> + res.start =3D 0;
> start =3D dcr_resource_start(op->node, 0);
> - len =3D dcr_resource_len(op->node, 0);
> - dcr_host =3D dcr_map(op->node, start, len);
> - if (!DCR_MAP_OK(dcr_host)) {
> - dev_err(&op->dev, "invalid address\n");
> - return -ENODEV;
> + drvdata->dcr_len =3D dcr_resource_len(op->node, 0);
> + drvdata->dcr_host =3D dcr_map(op->node, start,
drvdata->len);
> + if (!DCR_MAP_OK(drvdata->dcr_host)) {
> + dev_err(&op->dev, "invalid DCR address\n");
> + goto err;
> }
> }
> =
> @@ -467,26 +468,12 @@ xilinxfb_of_probe(struct of_device *op, const
struct of_device_id *match)
> if (of_find_property(op->node, "rotate-display", NULL))
> pdata.rotate_screen =3D 1;
> =
> - /* Allocate the driver data region */
> - drvdata =3D kzalloc(sizeof(*drvdata), GFP_KERNEL);
> - if (!drvdata) {
> - dev_err(&op->dev, "Couldn't allocate device private
record\n");
> - return -ENOMEM;
> - }
> dev_set_drvdata(&op->dev, drvdata);
> + return xilinxfb_assign(&op->dev, drvdata, res.start, &pdata);
> =
> - if (tft_access)
> - drvdata->flags |=3D PLB_ACCESS_FLAG;
> -
> - /* Arguments are passed based on the interface */
> - if (drvdata->flags & PLB_ACCESS_FLAG) {
> - return xilinxfb_assign(&op->dev, drvdata, res.start,
&pdata);
> - } else {
> - drvdata->dcr_start =3D start;
> - drvdata->dcr_len =3D len;
> - drvdata->dcr_host =3D dcr_host;
> - return xilinxfb_assign(&op->dev, drvdata, 0, &pdata);
> - }
> + err:
> + kfree(drvdata);
> + return -ENODEV;
> }
> =
> static int __devexit xilinxfb_of_remove(struct of_device *op)
> =
This email and any attachments are intended for the sole use of the named r=
ecipient(s) and contain(s) confidential information that may be proprietary=
, privileged or copyrighted under applicable law. If you are not the intend=
ed recipient, do not read, copy, or forward this email message or any attac=
hments. Delete this email message and any attachments immediately.
^ permalink raw reply related [flat|nested] 3+ messages in thread