diff mbox series

video: simplefb: modernise DT parsing

Message ID 20240216183840.879482-1-caleb.connolly@linaro.org
State Accepted
Delegated to: Anatolij Gustschin
Headers show
Series video: simplefb: modernise DT parsing | expand

Commit Message

Caleb Connolly Feb. 16, 2024, 6:38 p.m. UTC 
simplefb was using old style FDT parsing which doesn't behave well in
combination with livetree. Update it to use ofnode instead and add a
missing null check for the "format" property.

Standardise the error logging while we're here.

Fixes: 971d7e64245d ("video: simplefb")
Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
---
 drivers/video/simplefb.c | 32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)

Comments

Dan Carpenter Feb. 20, 2024, 1:21 p.m. UTC | #1 
On Fri, Feb 16, 2024 at 06:38:06PM +0000, Caleb Connolly wrote:
> @@ -41,17 +41,25 @@ static int simple_video_probe(struct udevice *dev)
>  
>  	debug("%s: Query resolution...\n", __func__);
>  
> -	uc_priv->xsize = fdtdec_get_uint(blob, node, "width", 0);
> -	uc_priv->ysize = fdtdec_get_uint(blob, node, "height", 0);
> -	uc_priv->rot = fdtdec_get_uint(blob, node, "rot", 0);
> -	if (uc_priv->rot > 3) {
> -		log_debug("%s: invalid rot\n", __func__);
> -		return log_msg_ret("rot", -EINVAL);
> +	ret = ofnode_read_u32(node, "width", &width);
> +	ret = ret ?: ofnode_read_u32(node, "height", &height);
> +	if (ret || !width || !height) {
> +		log_err("%s: invalid width or height: %d\n", __func__, ret);
> +		return ret;

This should be something like:

	return ret ?: -EINVAL;

Perhaps print the width and height in the error message as well.

regards,
dan carpenter
Anatolij Gustschin April 20, 2024, 11:12 p.m. UTC | #2 
On Fri, 16 Feb 2024 18:38:06 +0000
Caleb Connolly caleb.connolly@linaro.org wrote:

> simplefb was using old style FDT parsing which doesn't behave well in
> combination with livetree. Update it to use ofnode instead and add a
> missing null check for the "format" property.
> 
> Standardise the error logging while we're here.
> 
> Fixes: 971d7e64245d ("video: simplefb")
> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
> ---
>  drivers/video/simplefb.c | 32 ++++++++++++++++++++------------
>  1 file changed, 20 insertions(+), 12 deletions(-)

applied to u-boot-video.

--
Anatolij
diff mbox series

Patch

diff --git a/drivers/video/simplefb.c b/drivers/video/simplefb.c
index 235ec761f70b..3d83bf8f2e88 100644
--- a/drivers/video/simplefb.c
+++ b/drivers/video/simplefb.c
@@ -15,14 +15,14 @@  static int simple_video_probe(struct udevice *dev)
 {
 	struct video_uc_plat *plat = dev_get_uclass_plat(dev);
 	struct video_priv *uc_priv = dev_get_uclass_priv(dev);
-	const void *blob = gd->fdt_blob;
-	const int node = dev_of_offset(dev);
+	ofnode node = dev_ofnode(dev);
 	const char *format;
+	int ret;
 	fdt_addr_t base;
 	fdt_size_t size;
+	u32 width, height, rot;
 
-	base = fdtdec_get_addr_size_auto_parent(blob, dev_of_offset(dev->parent),
-			node, "reg", 0, &size, false);
+	base = dev_read_addr_size(dev, &size);
 	if (base == FDT_ADDR_T_NONE) {
 		debug("%s: Failed to decode memory region\n", __func__);
 		return -EINVAL;
@@ -41,17 +41,25 @@  static int simple_video_probe(struct udevice *dev)
 
 	debug("%s: Query resolution...\n", __func__);
 
-	uc_priv->xsize = fdtdec_get_uint(blob, node, "width", 0);
-	uc_priv->ysize = fdtdec_get_uint(blob, node, "height", 0);
-	uc_priv->rot = fdtdec_get_uint(blob, node, "rot", 0);
-	if (uc_priv->rot > 3) {
-		log_debug("%s: invalid rot\n", __func__);
-		return log_msg_ret("rot", -EINVAL);
+	ret = ofnode_read_u32(node, "width", &width);
+	ret = ret ?: ofnode_read_u32(node, "height", &height);
+	if (ret || !width || !height) {
+		log_err("%s: invalid width or height: %d\n", __func__, ret);
+		return ret;
 	}
+	ofnode_read_u32(node, "rot", &rot);
+	uc_priv->rot = rot;
+	uc_priv->xsize = width;
+	uc_priv->ysize = height;
 
-	format = fdt_getprop(blob, node, "format", NULL);
+	format = ofnode_read_string(node, "format");
 	debug("%s: %dx%d@%s\n", __func__, uc_priv->xsize, uc_priv->ysize, format);
 
+	if (!format) {
+		log_err("%s: please add required property \"format\"\n", __func__);
+		return -EINVAL;
+	}
+
 	if (strcmp(format, "r5g6b5") == 0) {
 		uc_priv->bpix = VIDEO_BPP16;
 	} else if (strcmp(format, "a8b8g8r8") == 0 ||
@@ -67,7 +75,7 @@  static int simple_video_probe(struct udevice *dev)
 		uc_priv->bpix = VIDEO_BPP32;
 		uc_priv->format = VIDEO_X2R10G10B10;
 	} else {
-		printf("%s: invalid format: %s\n", __func__, format);
+		log_err("%s: invalid format: %s\n", __func__, format);
 		return -EINVAL;
 	}