| Message ID | 631df9a44b366af4129d00f1d4e1d3baad7d4903.1381315928.git.michal.simek@xilinx.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Oct 09, 2013 at 11:52:12AM +0100, Michal Simek wrote: > s/op/pdev/ in xilinxfb_of_probe(). > No functional chagnes. > > Signed-off-by: Michal Simek <michal.simek@xilinx.com> > --- > Changes in v2: None > > drivers/video/xilinxfb.c | 18 +++++++++--------- > 1 file changed, 9 insertions(+), 9 deletions(-) > > diff --git a/drivers/video/xilinxfb.c b/drivers/video/xilinxfb.c > index 0e1dd33..d12345f 100644 > --- a/drivers/video/xilinxfb.c > +++ b/drivers/video/xilinxfb.c > @@ -411,7 +411,7 @@ static int xilinxfb_release(struct device *dev) > * OF bus binding > */ > > -static int xilinxfb_of_probe(struct platform_device *op) > +static int xilinxfb_of_probe(struct platform_device *pdev) > { > const u32 *prop; > u32 tft_access = 0; > @@ -425,7 +425,7 @@ static int xilinxfb_of_probe(struct platform_device *op) > /* Allocate the driver data region */ > drvdata = kzalloc(sizeof(*drvdata), GFP_KERNEL); > if (!drvdata) { > - dev_err(&op->dev, "Couldn't allocate device private record\n"); > + dev_err(&pdev->dev, "Couldn't allocate device private record\n"); > return -ENOMEM; > } > > @@ -433,7 +433,7 @@ static int xilinxfb_of_probe(struct platform_device *op) > * To check whether the core is connected directly to DCR or BUS > * interface and initialize the tft_access accordingly. > */ > - of_property_read_u32(op->dev.of_node, "xlnx,dcr-splb-slave-if", > + of_property_read_u32(pdev->dev.of_node, "xlnx,dcr-splb-slave-if", > &tft_access); > > /* > @@ -457,29 +457,29 @@ static int xilinxfb_of_probe(struct platform_device *op) > } > #endif > > - prop = of_get_property(op->dev.of_node, "phys-size", &size); > + prop = of_get_property(pdev->dev.of_node, "phys-size", &size); > if ((prop) && (size >= sizeof(u32)*2)) { > pdata.screen_width_mm = prop[0]; > pdata.screen_height_mm = prop[1]; > } While you're changing these lines, it would be nice to change this pattern (here and elsewhere) to use of_property_read_u32_array, so that it's endian-safe and consistent with other devicetree parsing code: of_property_read_u32_array(pdev->dev.of_node, "phys-size", prop, 2); It won't read the values if the property data's too short, so that should be consistent with the existing code. It would also make the diffstat negative :) > > - prop = of_get_property(op->dev.of_node, "resolution", &size); > + prop = of_get_property(pdev->dev.of_node, "resolution", &size); > if ((prop) && (size >= sizeof(u32)*2)) { > pdata.xres = prop[0]; > pdata.yres = prop[1]; > } > > - prop = of_get_property(op->dev.of_node, "virtual-resolution", &size); > + prop = of_get_property(pdev->dev.of_node, "virtual-resolution", &size); > if ((prop) && (size >= sizeof(u32)*2)) { > pdata.xvirt = prop[0]; > pdata.yvirt = prop[1]; > } > > - if (of_find_property(op->dev.of_node, "rotate-display", NULL)) > + if (of_find_property(pdev->dev.of_node, "rotate-display", NULL)) > pdata.rotate_screen = 1; Similarly, this could use of_property_read_bool: pdata.rotate_screen = of_property_read_bool(pdev->dev.of_node, "rotate-display"); It won't help the diffstat, but it makes the intent clearer. Cheers, Mark. -- 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 10/09/2013 01:06 PM, Mark Rutland wrote: > On Wed, Oct 09, 2013 at 11:52:12AM +0100, Michal Simek wrote: >> s/op/pdev/ in xilinxfb_of_probe(). >> No functional chagnes. >> >> Signed-off-by: Michal Simek <michal.simek@xilinx.com> >> --- >> Changes in v2: None >> >> drivers/video/xilinxfb.c | 18 +++++++++--------- >> 1 file changed, 9 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/video/xilinxfb.c b/drivers/video/xilinxfb.c >> index 0e1dd33..d12345f 100644 >> --- a/drivers/video/xilinxfb.c >> +++ b/drivers/video/xilinxfb.c >> @@ -411,7 +411,7 @@ static int xilinxfb_release(struct device *dev) >> * OF bus binding >> */ >> >> -static int xilinxfb_of_probe(struct platform_device *op) >> +static int xilinxfb_of_probe(struct platform_device *pdev) >> { >> const u32 *prop; >> u32 tft_access = 0; >> @@ -425,7 +425,7 @@ static int xilinxfb_of_probe(struct platform_device *op) >> /* Allocate the driver data region */ >> drvdata = kzalloc(sizeof(*drvdata), GFP_KERNEL); >> if (!drvdata) { >> - dev_err(&op->dev, "Couldn't allocate device private record\n"); >> + dev_err(&pdev->dev, "Couldn't allocate device private record\n"); >> return -ENOMEM; >> } >> >> @@ -433,7 +433,7 @@ static int xilinxfb_of_probe(struct platform_device *op) >> * To check whether the core is connected directly to DCR or BUS >> * interface and initialize the tft_access accordingly. >> */ >> - of_property_read_u32(op->dev.of_node, "xlnx,dcr-splb-slave-if", >> + of_property_read_u32(pdev->dev.of_node, "xlnx,dcr-splb-slave-if", >> &tft_access); >> >> /* >> @@ -457,29 +457,29 @@ static int xilinxfb_of_probe(struct platform_device *op) >> } >> #endif >> >> - prop = of_get_property(op->dev.of_node, "phys-size", &size); >> + prop = of_get_property(pdev->dev.of_node, "phys-size", &size); >> if ((prop) && (size >= sizeof(u32)*2)) { >> pdata.screen_width_mm = prop[0]; >> pdata.screen_height_mm = prop[1]; >> } > > While you're changing these lines, it would be nice to change this > pattern (here and elsewhere) to use of_property_read_u32_array, so that > it's endian-safe and consistent with other devicetree parsing code: > > of_property_read_u32_array(pdev->dev.of_node, "phys-size", prop, 2); > > It won't read the values if the property data's too short, so that > should be consistent with the existing code. > > It would also make the diffstat negative :) The intention of this patch is simple rename which is exactly how patch should look like. It means one change per patch. It means these changes you have describe should be in separate patch and I definitely agree with them. Thanks, Michal
diff --git a/drivers/video/xilinxfb.c b/drivers/video/xilinxfb.c index 0e1dd33..d12345f 100644 --- a/drivers/video/xilinxfb.c +++ b/drivers/video/xilinxfb.c @@ -411,7 +411,7 @@ static int xilinxfb_release(struct device *dev) * OF bus binding */ -static int xilinxfb_of_probe(struct platform_device *op) +static int xilinxfb_of_probe(struct platform_device *pdev) { const u32 *prop; u32 tft_access = 0; @@ -425,7 +425,7 @@ static int xilinxfb_of_probe(struct platform_device *op) /* Allocate the driver data region */ drvdata = kzalloc(sizeof(*drvdata), GFP_KERNEL); if (!drvdata) { - dev_err(&op->dev, "Couldn't allocate device private record\n"); + dev_err(&pdev->dev, "Couldn't allocate device private record\n"); return -ENOMEM; } @@ -433,7 +433,7 @@ static int xilinxfb_of_probe(struct platform_device *op) * To check whether the core is connected directly to DCR or BUS * interface and initialize the tft_access accordingly. */ - of_property_read_u32(op->dev.of_node, "xlnx,dcr-splb-slave-if", + of_property_read_u32(pdev->dev.of_node, "xlnx,dcr-splb-slave-if", &tft_access); /* @@ -457,29 +457,29 @@ static int xilinxfb_of_probe(struct platform_device *op) } #endif - prop = of_get_property(op->dev.of_node, "phys-size", &size); + prop = of_get_property(pdev->dev.of_node, "phys-size", &size); if ((prop) && (size >= sizeof(u32)*2)) { pdata.screen_width_mm = prop[0]; pdata.screen_height_mm = prop[1]; } - prop = of_get_property(op->dev.of_node, "resolution", &size); + prop = of_get_property(pdev->dev.of_node, "resolution", &size); if ((prop) && (size >= sizeof(u32)*2)) { pdata.xres = prop[0]; pdata.yres = prop[1]; } - prop = of_get_property(op->dev.of_node, "virtual-resolution", &size); + prop = of_get_property(pdev->dev.of_node, "virtual-resolution", &size); if ((prop) && (size >= sizeof(u32)*2)) { pdata.xvirt = prop[0]; pdata.yvirt = prop[1]; } - if (of_find_property(op->dev.of_node, "rotate-display", NULL)) + if (of_find_property(pdev->dev.of_node, "rotate-display", NULL)) pdata.rotate_screen = 1; - dev_set_drvdata(&op->dev, drvdata); - return xilinxfb_assign(op, drvdata, &pdata); + dev_set_drvdata(&pdev->dev, drvdata); + return xilinxfb_assign(pdev, drvdata, &pdata); } static int xilinxfb_of_remove(struct platform_device *op)
s/op/pdev/ in xilinxfb_of_probe(). No functional chagnes. Signed-off-by: Michal Simek <michal.simek@xilinx.com> --- Changes in v2: None drivers/video/xilinxfb.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) -- 1.8.2.3