[PATCH 0/3] Smiapp quirk call order and best scaling ratio fixes

[PATCH 0/3] Smiapp quirk call order and best scaling ratio fixes

[Sakari Ailus]

Hi,

The most important patch is the third one: wrong scaling ratio was selected
in many (or most?) cases due to the wrong signedness of the variable. The
other two have less effect on the functionality.

-- 
Kind regards,
Sakari

[PATCH 1/3] smiapp: Print the index of the format descriptor

[Sakari Ailus]

This makes constructing quirks easier.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/smiapp/smiapp-core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c
index 3af8df8..da4422e 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -741,8 +741,8 @@ static int smiapp_get_mbus_formats(struct smiapp_sensor *sensor)
 		if (rval)
 			return rval;
 
-		dev_dbg(&client->dev, "bpp %d, compressed %d\n",
-			fmt >> 8, (u8)fmt);
+		dev_dbg(&client->dev, "%u: bpp %u, compressed %u\n",
+			i, fmt >> 8, (u8)fmt);
 
 		for (j = 0; j < ARRAY_SIZE(smiapp_csi_data_formats); j++) {
 			const struct smiapp_csi_data_format *f =
-- 
1.8.3.2

[PATCH 2/3] smiapp: Call limits quirk immediately after retrieving the limits

[Sakari Ailus]

Some of the limits are used before the limits quirk is called. Move the call
immediately after obtaining the limits.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/smiapp/smiapp-core.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c
index da4422e..0a74e14 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -2423,6 +2423,12 @@ static int smiapp_registered(struct v4l2_subdev *subdev)
 		sensor->hvflip_inv_mask = SMIAPP_IMAGE_ORIENTATION_HFLIP |
 					  SMIAPP_IMAGE_ORIENTATION_VFLIP;
 
+	rval = smiapp_call_quirk(sensor, limits);
+	if (rval) {
+		dev_err(&client->dev, "limits quirks failed\n");
+		goto out_power_off;
+	}
+
 	rval = smiapp_get_mbus_formats(sensor);
 	if (rval) {
 		rval = -ENODEV;
@@ -2483,12 +2489,6 @@ static int smiapp_registered(struct v4l2_subdev *subdev)
 		}
 	}
 
-	rval = smiapp_call_quirk(sensor, limits);
-	if (rval) {
-		dev_err(&client->dev, "limits quirks failed\n");
-		goto out_nvm_release;
-	}
-
 	/* We consider this as profile 0 sensor if any of these are zero. */
 	if (!sensor->limits[SMIAPP_LIMIT_MIN_OP_SYS_CLK_DIV] ||
 	    !sensor->limits[SMIAPP_LIMIT_MAX_OP_SYS_CLK_DIV] ||
-- 
1.8.3.2

[PATCH 3/3] smiapp: Scaling goodness is signed

[Sakari Ailus]

The "best" value was unsigned however, leading to signed-to-unsigned
comparison and wrong results. Possibly only on a newer GCC. Fix this by
making the best value signed as well.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/smiapp/smiapp-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c
index 0a74e14..db3d5a6 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -1766,7 +1766,7 @@ static void smiapp_set_compose_binner(struct v4l2_subdev *subdev,
 	struct smiapp_sensor *sensor = to_smiapp_sensor(subdev);
 	unsigned int i;
 	unsigned int binh = 1, binv = 1;
-	unsigned int best = scaling_goodness(
+	int best = scaling_goodness(
 		subdev,
 		crops[SMIAPP_PAD_SINK]->width, sel->r.width,
 		crops[SMIAPP_PAD_SINK]->height, sel->r.height, sel->flags);
-- 
1.8.3.2

Re: [PATCH 0/3] Smiapp quirk call order and best scaling ratio fixes

[Laurent Pinchart]

Hi Sakari,

Thank you for the patches.

On Wednesday 23 April 2014 22:33:56 Sakari Ailus wrote:
> Hi,
> 
> The most important patch is the third one: wrong scaling ratio was selected
> in many (or most?) cases due to the wrong signedness of the variable. The
> other two have less effect on the functionality.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

-- 
Regards,

Laurent Pinchart