Message ID | 20220723175720.76933-1-u.kleine-koenig@pengutronix.de (mailing list archive) |
---|---|
State | Accepted, archived |
Commit | e948d32c54fa |
Headers | show |
Series | [1/4] video: fb: imxfb: Drop platform data support | expand |
Hi Uwe, On Sat, Jul 23, 2022 at 07:57:17PM +0200, Uwe Kleine-König wrote: > No source file but the driver itself includes the header containing the > platform data definition. The last user is gone since commit > 8485adf17a15 ("ARM: imx: Remove imx device directory"). > > So we can safely drop platform data support. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Do imxfb offer something that is not supported by the DRM drivers? If yes then the clean-up is good, if not then we could drop the driver? Sam
Hi Sam, On Sat, Jul 23, 2022 at 09:23:43PM +0200, Sam Ravnborg wrote: > On Sat, Jul 23, 2022 at 07:57:17PM +0200, Uwe Kleine-König wrote: > > No source file but the driver itself includes the header containing the > > platform data definition. The last user is gone since commit > > 8485adf17a15 ("ARM: imx: Remove imx device directory"). > > > > So we can safely drop platform data support. > > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > > Do imxfb offer something that is not supported by the DRM drivers? > If yes then the clean-up is good, if not then we could drop the driver? It's for different i.MX variants. imxfb is for i.MX2x while the DRM drivers in mainline are for i.MX6. (Not sure about the i.MX3 and i.MX5x variants.) Somewhere in the middle of my todo list is to mainline an i.MX2x DRM driver that could replace the imxfb driver. If I only had a bit more time ... Best regards Uwe
Hi Uwe, On Sun, Jul 24, 2022 at 12:02:18AM +0200, Uwe Kleine-König wrote: > Hi Sam, > > On Sat, Jul 23, 2022 at 09:23:43PM +0200, Sam Ravnborg wrote: > > On Sat, Jul 23, 2022 at 07:57:17PM +0200, Uwe Kleine-König wrote: > > > No source file but the driver itself includes the header containing the > > > platform data definition. The last user is gone since commit > > > 8485adf17a15 ("ARM: imx: Remove imx device directory"). > > > > > > So we can safely drop platform data support. > > > > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > > > > Do imxfb offer something that is not supported by the DRM drivers? > > If yes then the clean-up is good, if not then we could drop the driver? > > It's for different i.MX variants. imxfb is for i.MX2x while the DRM > drivers in mainline are for i.MX6. (Not sure about the i.MX3 and i.MX5x > variants.) > > Somewhere in the middle of my todo list is to mainline an i.MX2x DRM > driver that could replace the imxfb driver. If I only had a bit more > time ... There is drm/mxsfb, where Kconfig says: "including i.MX23, i.MX28, i.MX6SX, i.MX7 and i.MX8M". So there is already something but if this is a 1:1 replacement I dunno. Sam
Hi Uwe, On Sun, Jul 24, 2022 at 11:40:22AM +0200, Sam Ravnborg wrote: > Hi Uwe, > > On Sun, Jul 24, 2022 at 12:02:18AM +0200, Uwe Kleine-König wrote: > > Hi Sam, > > > > On Sat, Jul 23, 2022 at 09:23:43PM +0200, Sam Ravnborg wrote: > > > On Sat, Jul 23, 2022 at 07:57:17PM +0200, Uwe Kleine-König wrote: > > > > No source file but the driver itself includes the header containing the > > > > platform data definition. The last user is gone since commit > > > > 8485adf17a15 ("ARM: imx: Remove imx device directory"). > > > > > > > > So we can safely drop platform data support. > > > > > > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > > > > > > Do imxfb offer something that is not supported by the DRM drivers? > > > If yes then the clean-up is good, if not then we could drop the driver? > > > > It's for different i.MX variants. imxfb is for i.MX2x while the DRM > > drivers in mainline are for i.MX6. (Not sure about the i.MX3 and i.MX5x > > variants.) > > > > Somewhere in the middle of my todo list is to mainline an i.MX2x DRM > > driver that could replace the imxfb driver. If I only had a bit more > > time ... > > There is drm/mxsfb, where Kconfig says: > "including i.MX23, i.MX28, i.MX6SX, i.MX7 and i.MX8M". > > So there is already something but if this is a 1:1 replacement I dunno. I suddenly remembered we had the following commit: f225f1393f034e17281274180626086276da766c ("video: fbdev: mxsfb: Remove driver") So the fbdev counterpart of drm/mxsfb is already dropped. Sam
On 7/23/22 19:57, Uwe Kleine-König wrote: > No source file but the driver itself includes the header containing the > platform data definition. The last user is gone since commit > 8485adf17a15 ("ARM: imx: Remove imx device directory"). > > So we can safely drop platform data support. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> I've applied all 4 patches to the fbdev git tree. Thanks! Helge > --- > drivers/video/fbdev/imxfb.c | 99 ++++++++--------------- > include/linux/platform_data/video-imxfb.h | 12 --- > 2 files changed, 34 insertions(+), 77 deletions(-) > > diff --git a/drivers/video/fbdev/imxfb.c b/drivers/video/fbdev/imxfb.c > index a2f644c97f28..85a5bf5639d9 100644 > --- a/drivers/video/fbdev/imxfb.c > +++ b/drivers/video/fbdev/imxfb.c > @@ -656,7 +656,6 @@ static int imxfb_activate_var(struct fb_var_screeninfo *var, struct fb_info *inf > > static int imxfb_init_fbinfo(struct platform_device *pdev) > { > - struct imx_fb_platform_data *pdata = dev_get_platdata(&pdev->dev); > struct fb_info *info = platform_get_drvdata(pdev); > struct imxfb_info *fbi = info->par; > struct device_node *np; > @@ -690,25 +689,20 @@ static int imxfb_init_fbinfo(struct platform_device *pdev) > info->fbops = &imxfb_ops; > info->flags = FBINFO_FLAG_DEFAULT | > FBINFO_READS_FAST; > - if (pdata) { > - fbi->lscr1 = pdata->lscr1; > - fbi->dmacr = pdata->dmacr; > - fbi->pwmr = pdata->pwmr; > - } else { > - np = pdev->dev.of_node; > - info->var.grayscale = of_property_read_bool(np, > - "cmap-greyscale"); > - fbi->cmap_inverse = of_property_read_bool(np, "cmap-inverse"); > - fbi->cmap_static = of_property_read_bool(np, "cmap-static"); > > - fbi->lscr1 = IMXFB_LSCR1_DEFAULT; > + np = pdev->dev.of_node; > + info->var.grayscale = of_property_read_bool(np, > + "cmap-greyscale"); > + fbi->cmap_inverse = of_property_read_bool(np, "cmap-inverse"); > + fbi->cmap_static = of_property_read_bool(np, "cmap-static"); > > - of_property_read_u32(np, "fsl,lpccr", &fbi->pwmr); > + fbi->lscr1 = IMXFB_LSCR1_DEFAULT; > > - of_property_read_u32(np, "fsl,lscr1", &fbi->lscr1); > + of_property_read_u32(np, "fsl,lpccr", &fbi->pwmr); > > - of_property_read_u32(np, "fsl,dmacr", &fbi->dmacr); > - } > + of_property_read_u32(np, "fsl,lscr1", &fbi->lscr1); > + > + of_property_read_u32(np, "fsl,dmacr", &fbi->dmacr); > > return 0; > } > @@ -863,10 +857,10 @@ static int imxfb_probe(struct platform_device *pdev) > struct imxfb_info *fbi; > struct lcd_device *lcd; > struct fb_info *info; > - struct imx_fb_platform_data *pdata; > struct resource *res; > struct imx_fb_videomode *m; > const struct of_device_id *of_id; > + struct device_node *display_np; > int ret, i; > int bytes_per_pixel; > > @@ -884,8 +878,6 @@ static int imxfb_probe(struct platform_device *pdev) > if (!res) > return -ENODEV; > > - pdata = dev_get_platdata(&pdev->dev); > - > info = framebuffer_alloc(sizeof(struct imxfb_info), &pdev->dev); > if (!info) > return -ENOMEM; > @@ -898,43 +890,34 @@ static int imxfb_probe(struct platform_device *pdev) > if (ret < 0) > goto failed_init; > > - if (pdata) { > - if (!fb_mode) > - fb_mode = pdata->mode[0].mode.name; > - > - fbi->mode = pdata->mode; > - fbi->num_modes = pdata->num_modes; > - } else { > - struct device_node *display_np; > - fb_mode = NULL; > - > - display_np = of_parse_phandle(pdev->dev.of_node, "display", 0); > - if (!display_np) { > - dev_err(&pdev->dev, "No display defined in devicetree\n"); > - ret = -EINVAL; > - goto failed_of_parse; > - } > + fb_mode = NULL; > > - /* > - * imxfb does not support more modes, we choose only the native > - * mode. > - */ > - fbi->num_modes = 1; > - > - fbi->mode = devm_kzalloc(&pdev->dev, > - sizeof(struct imx_fb_videomode), GFP_KERNEL); > - if (!fbi->mode) { > - ret = -ENOMEM; > - of_node_put(display_np); > - goto failed_of_parse; > - } > + display_np = of_parse_phandle(pdev->dev.of_node, "display", 0); > + if (!display_np) { > + dev_err(&pdev->dev, "No display defined in devicetree\n"); > + ret = -EINVAL; > + goto failed_of_parse; > + } > > - ret = imxfb_of_read_mode(&pdev->dev, display_np, fbi->mode); > + /* > + * imxfb does not support more modes, we choose only the native > + * mode. > + */ > + fbi->num_modes = 1; > + > + fbi->mode = devm_kzalloc(&pdev->dev, > + sizeof(struct imx_fb_videomode), GFP_KERNEL); > + if (!fbi->mode) { > + ret = -ENOMEM; > of_node_put(display_np); > - if (ret) > - goto failed_of_parse; > + goto failed_of_parse; > } > > + ret = imxfb_of_read_mode(&pdev->dev, display_np, fbi->mode); > + of_node_put(display_np); > + if (ret) > + goto failed_of_parse; > + > /* Calculate maximum bytes used per pixel. In most cases this should > * be the same as m->bpp/8 */ > m = &fbi->mode[0]; > @@ -1001,13 +984,6 @@ static int imxfb_probe(struct platform_device *pdev) > > info->fix.smem_start = fbi->map_dma; > > - if (pdata && pdata->init) { > - ret = pdata->init(fbi->pdev); > - if (ret) > - goto failed_platform_init; > - } > - > - > INIT_LIST_HEAD(&info->modelist); > for (i = 0; i < fbi->num_modes; i++) > fb_add_videomode(&fbi->mode[i].mode, &info->modelist); > @@ -1059,9 +1035,6 @@ static int imxfb_probe(struct platform_device *pdev) > failed_register: > fb_dealloc_cmap(&info->cmap); > failed_cmap: > - if (pdata && pdata->exit) > - pdata->exit(fbi->pdev); > -failed_platform_init: > dma_free_wc(&pdev->dev, fbi->map_size, info->screen_buffer, > fbi->map_dma); > failed_map: > @@ -1079,7 +1052,6 @@ static int imxfb_probe(struct platform_device *pdev) > > static int imxfb_remove(struct platform_device *pdev) > { > - struct imx_fb_platform_data *pdata; > struct fb_info *info = platform_get_drvdata(pdev); > struct imxfb_info *fbi = info->par; > struct resource *res; > @@ -1092,9 +1064,6 @@ static int imxfb_remove(struct platform_device *pdev) > > unregister_framebuffer(info); > fb_dealloc_cmap(&info->cmap); > - pdata = dev_get_platdata(&pdev->dev); > - if (pdata && pdata->exit) > - pdata->exit(fbi->pdev); > dma_free_wc(&pdev->dev, fbi->map_size, info->screen_buffer, > fbi->map_dma); > iounmap(fbi->regs); > diff --git a/include/linux/platform_data/video-imxfb.h b/include/linux/platform_data/video-imxfb.h > index 02812651af7d..b80a156a6617 100644 > --- a/include/linux/platform_data/video-imxfb.h > +++ b/include/linux/platform_data/video-imxfb.h > @@ -55,16 +55,4 @@ struct imx_fb_videomode { > unsigned char bpp; > }; > > -struct imx_fb_platform_data { > - struct imx_fb_videomode *mode; > - int num_modes; > - > - u_int pwmr; > - u_int lscr1; > - u_int dmacr; > - > - int (*init)(struct platform_device *); > - void (*exit)(struct platform_device *); > -}; > - > #endif /* ifndef __MACH_IMXFB_H__ */ > > base-commit: f2906aa863381afb0015a9eb7fefad885d4e5a56
diff --git a/drivers/video/fbdev/imxfb.c b/drivers/video/fbdev/imxfb.c index a2f644c97f28..85a5bf5639d9 100644 --- a/drivers/video/fbdev/imxfb.c +++ b/drivers/video/fbdev/imxfb.c @@ -656,7 +656,6 @@ static int imxfb_activate_var(struct fb_var_screeninfo *var, struct fb_info *inf static int imxfb_init_fbinfo(struct platform_device *pdev) { - struct imx_fb_platform_data *pdata = dev_get_platdata(&pdev->dev); struct fb_info *info = platform_get_drvdata(pdev); struct imxfb_info *fbi = info->par; struct device_node *np; @@ -690,25 +689,20 @@ static int imxfb_init_fbinfo(struct platform_device *pdev) info->fbops = &imxfb_ops; info->flags = FBINFO_FLAG_DEFAULT | FBINFO_READS_FAST; - if (pdata) { - fbi->lscr1 = pdata->lscr1; - fbi->dmacr = pdata->dmacr; - fbi->pwmr = pdata->pwmr; - } else { - np = pdev->dev.of_node; - info->var.grayscale = of_property_read_bool(np, - "cmap-greyscale"); - fbi->cmap_inverse = of_property_read_bool(np, "cmap-inverse"); - fbi->cmap_static = of_property_read_bool(np, "cmap-static"); - fbi->lscr1 = IMXFB_LSCR1_DEFAULT; + np = pdev->dev.of_node; + info->var.grayscale = of_property_read_bool(np, + "cmap-greyscale"); + fbi->cmap_inverse = of_property_read_bool(np, "cmap-inverse"); + fbi->cmap_static = of_property_read_bool(np, "cmap-static"); - of_property_read_u32(np, "fsl,lpccr", &fbi->pwmr); + fbi->lscr1 = IMXFB_LSCR1_DEFAULT; - of_property_read_u32(np, "fsl,lscr1", &fbi->lscr1); + of_property_read_u32(np, "fsl,lpccr", &fbi->pwmr); - of_property_read_u32(np, "fsl,dmacr", &fbi->dmacr); - } + of_property_read_u32(np, "fsl,lscr1", &fbi->lscr1); + + of_property_read_u32(np, "fsl,dmacr", &fbi->dmacr); return 0; } @@ -863,10 +857,10 @@ static int imxfb_probe(struct platform_device *pdev) struct imxfb_info *fbi; struct lcd_device *lcd; struct fb_info *info; - struct imx_fb_platform_data *pdata; struct resource *res; struct imx_fb_videomode *m; const struct of_device_id *of_id; + struct device_node *display_np; int ret, i; int bytes_per_pixel; @@ -884,8 +878,6 @@ static int imxfb_probe(struct platform_device *pdev) if (!res) return -ENODEV; - pdata = dev_get_platdata(&pdev->dev); - info = framebuffer_alloc(sizeof(struct imxfb_info), &pdev->dev); if (!info) return -ENOMEM; @@ -898,43 +890,34 @@ static int imxfb_probe(struct platform_device *pdev) if (ret < 0) goto failed_init; - if (pdata) { - if (!fb_mode) - fb_mode = pdata->mode[0].mode.name; - - fbi->mode = pdata->mode; - fbi->num_modes = pdata->num_modes; - } else { - struct device_node *display_np; - fb_mode = NULL; - - display_np = of_parse_phandle(pdev->dev.of_node, "display", 0); - if (!display_np) { - dev_err(&pdev->dev, "No display defined in devicetree\n"); - ret = -EINVAL; - goto failed_of_parse; - } + fb_mode = NULL; - /* - * imxfb does not support more modes, we choose only the native - * mode. - */ - fbi->num_modes = 1; - - fbi->mode = devm_kzalloc(&pdev->dev, - sizeof(struct imx_fb_videomode), GFP_KERNEL); - if (!fbi->mode) { - ret = -ENOMEM; - of_node_put(display_np); - goto failed_of_parse; - } + display_np = of_parse_phandle(pdev->dev.of_node, "display", 0); + if (!display_np) { + dev_err(&pdev->dev, "No display defined in devicetree\n"); + ret = -EINVAL; + goto failed_of_parse; + } - ret = imxfb_of_read_mode(&pdev->dev, display_np, fbi->mode); + /* + * imxfb does not support more modes, we choose only the native + * mode. + */ + fbi->num_modes = 1; + + fbi->mode = devm_kzalloc(&pdev->dev, + sizeof(struct imx_fb_videomode), GFP_KERNEL); + if (!fbi->mode) { + ret = -ENOMEM; of_node_put(display_np); - if (ret) - goto failed_of_parse; + goto failed_of_parse; } + ret = imxfb_of_read_mode(&pdev->dev, display_np, fbi->mode); + of_node_put(display_np); + if (ret) + goto failed_of_parse; + /* Calculate maximum bytes used per pixel. In most cases this should * be the same as m->bpp/8 */ m = &fbi->mode[0]; @@ -1001,13 +984,6 @@ static int imxfb_probe(struct platform_device *pdev) info->fix.smem_start = fbi->map_dma; - if (pdata && pdata->init) { - ret = pdata->init(fbi->pdev); - if (ret) - goto failed_platform_init; - } - - INIT_LIST_HEAD(&info->modelist); for (i = 0; i < fbi->num_modes; i++) fb_add_videomode(&fbi->mode[i].mode, &info->modelist); @@ -1059,9 +1035,6 @@ static int imxfb_probe(struct platform_device *pdev) failed_register: fb_dealloc_cmap(&info->cmap); failed_cmap: - if (pdata && pdata->exit) - pdata->exit(fbi->pdev); -failed_platform_init: dma_free_wc(&pdev->dev, fbi->map_size, info->screen_buffer, fbi->map_dma); failed_map: @@ -1079,7 +1052,6 @@ static int imxfb_probe(struct platform_device *pdev) static int imxfb_remove(struct platform_device *pdev) { - struct imx_fb_platform_data *pdata; struct fb_info *info = platform_get_drvdata(pdev); struct imxfb_info *fbi = info->par; struct resource *res; @@ -1092,9 +1064,6 @@ static int imxfb_remove(struct platform_device *pdev) unregister_framebuffer(info); fb_dealloc_cmap(&info->cmap); - pdata = dev_get_platdata(&pdev->dev); - if (pdata && pdata->exit) - pdata->exit(fbi->pdev); dma_free_wc(&pdev->dev, fbi->map_size, info->screen_buffer, fbi->map_dma); iounmap(fbi->regs); diff --git a/include/linux/platform_data/video-imxfb.h b/include/linux/platform_data/video-imxfb.h index 02812651af7d..b80a156a6617 100644 --- a/include/linux/platform_data/video-imxfb.h +++ b/include/linux/platform_data/video-imxfb.h @@ -55,16 +55,4 @@ struct imx_fb_videomode { unsigned char bpp; }; -struct imx_fb_platform_data { - struct imx_fb_videomode *mode; - int num_modes; - - u_int pwmr; - u_int lscr1; - u_int dmacr; - - int (*init)(struct platform_device *); - void (*exit)(struct platform_device *); -}; - #endif /* ifndef __MACH_IMXFB_H__ */
No source file but the driver itself includes the header containing the platform data definition. The last user is gone since commit 8485adf17a15 ("ARM: imx: Remove imx device directory"). So we can safely drop platform data support. Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> --- drivers/video/fbdev/imxfb.c | 99 ++++++++--------------- include/linux/platform_data/video-imxfb.h | 12 --- 2 files changed, 34 insertions(+), 77 deletions(-) base-commit: f2906aa863381afb0015a9eb7fefad885d4e5a56