Re: [PATCH v2 3/3] media: atmel-isi: add sanity check for supported formats in set_fmt()
From: Josh Wu
Date: Thu Aug 20 2015 - 23:11:54 EST
Hi, Laurent
Thanks for the review.
On 8/21/2015 2:30 AM, Laurent Pinchart wrote:
Hi Josh,
Thank you for the patch.
On Wednesday 05 August 2015 11:26:29 Josh Wu wrote:
After adding the format check in set_fmt(), we don't need any format check
in configure_geometry(). So make configure_geometry() as void type.
Signed-off-by: Josh Wu <josh.wu@xxxxxxxxx>
---
Changes in v2:
- new added patch
drivers/media/platform/soc_camera/atmel-isi.c | 39 ++++++++++++++++++------
1 file changed, 31 insertions(+), 8 deletions(-)
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
b/drivers/media/platform/soc_camera/atmel-isi.c index cb46aec..d0df518
100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -103,17 +103,19 @@ static u32 isi_readl(struct atmel_isi *isi, u32 reg)
return readl(isi->regs + reg);
}
-static int configure_geometry(struct atmel_isi *isi, u32 width,
+static void configure_geometry(struct atmel_isi *isi, u32 width,
u32 height, u32 code)
{
u32 cfg2;
/* According to sensor's output format to set cfg2 */
switch (code) {
- /* YUV, including grey */
+ default:
+ /* Grey */
case MEDIA_BUS_FMT_Y8_1X8:
cfg2 = ISI_CFG2_GRAYSCALE;
break;
+ /* YUV */
case MEDIA_BUS_FMT_VYUY8_2X8:
cfg2 = ISI_CFG2_YCC_SWAP_MODE_3;
break;
@@ -127,8 +129,6 @@ static int configure_geometry(struct atmel_isi *isi, u32
width, cfg2 = ISI_CFG2_YCC_SWAP_DEFAULT;
break;
/* RGB, TODO */
- default:
- return -EINVAL;
}
isi_writel(isi, ISI_CTRL, ISI_CTRL_DIS);
@@ -139,8 +139,29 @@ static int configure_geometry(struct atmel_isi *isi,
u32 width, cfg2 |= ((height - 1) << ISI_CFG2_IM_VSIZE_OFFSET)
& ISI_CFG2_IM_VSIZE_MASK;
isi_writel(isi, ISI_CFG2, cfg2);
+}
- return 0;
+static bool is_supported(struct soc_camera_device *icd,
+ const struct soc_camera_format_xlate *xlate)
+{
+ bool ret = true;
+
+ switch (xlate->code) {
+ /* YUV, including grey */
+ case MEDIA_BUS_FMT_Y8_1X8:
+ case MEDIA_BUS_FMT_VYUY8_2X8:
+ case MEDIA_BUS_FMT_UYVY8_2X8:
+ case MEDIA_BUS_FMT_YVYU8_2X8:
+ case MEDIA_BUS_FMT_YUYV8_2X8:
I would just return true here and false below, and remove the ret variable.
Ok.
+ break;
+ /* RGB, TODO */
+ default:
+ dev_err(icd->parent, "not supported format: %d\n",
+ xlate->code);
If this can happen when userspace asks for an unsupported format I don't think
you should print an error message to the kernel log.
ok, I will remove this.
+ ret = false;
+ }
+
+ return ret;
}
static irqreturn_t atmel_isi_handle_streaming(struct atmel_isi *isi)
@@ -391,10 +412,8 @@ static int start_streaming(struct vb2_queue *vq,
unsigned int count) /* Disable all interrupts */
isi_writel(isi, ISI_INTDIS, (u32)~0UL);
- ret = configure_geometry(isi, icd->user_width, icd->user_height,
+ configure_geometry(isi, icd->user_width, icd->user_height,
icd->current_fmt->code);
- if (ret < 0)
- return ret;
spin_lock_irq(&isi->lock);
/* Clear any pending interrupt */
@@ -515,6 +534,10 @@ static int isi_camera_set_fmt(struct soc_camera_device
*icd, if (mf->code != xlate->code)
return -EINVAL;
+ /* check with atmel-isi support format */
+ if (!is_supported(icd, xlate))
+ return -EINVAL;
+
S_FMT is supposed to pick a suitable default format when the requested format
isn't supported. It shouldn't return an error.
So I will move this check to beginning of S_FMT and if format not
support, I will select a default format for it.
Best Regards,
Josh Wu
pix->width = mf->width;
pix->height = mf->height;
pix->field = mf->field;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/