diff mbox series

[1/4] video: fb: imxfb: Drop platform data support

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

Commit Message

Uwe Kleine-König July 23, 2022, 5:57 p.m. UTC
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

Comments

Sam Ravnborg July 23, 2022, 7:23 p.m. UTC | #1
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
Uwe Kleine-König July 23, 2022, 10:02 p.m. UTC | #2
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
Sam Ravnborg July 24, 2022, 9:40 a.m. UTC | #3
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
Sam Ravnborg July 24, 2022, 9:46 a.m. UTC | #4
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
Helge Deller July 26, 2022, 7:08 a.m. UTC | #5
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 mbox series

Patch

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__ */