* [PATCH v2 1/3] media: atmel-isi: setup the ISI_CFG2 register directly @ 2015-08-05 3:26 Josh Wu 2015-08-05 3:26 ` [PATCH v2 2/3] media: atmel-isi: move configure_geometry() to start_streaming() Josh Wu 2015-08-05 3:26 ` [PATCH v2 3/3] media: atmel-isi: add sanity check for supported formats in set_fmt() Josh Wu 0 siblings, 2 replies; 6+ messages in thread From: Josh Wu @ 2015-08-05 3:26 UTC (permalink / raw) To: Linux Media Mailing List, Guennadi Liakhovetski Cc: Laurent Pinchart, Josh Wu, Mauro Carvalho Chehab, linux-kernel In the function configure_geometry(), we will setup the ISI CFG2 according to the sensor output format. It make no sense to just read back the CFG2 register and just set part of it. So just set up this register directly makes things simpler. Currently only support YUV format from camera sensor. Signed-off-by: Josh Wu <josh.wu@atmel.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- Changes in v2: - add Laurent's reviewed-by tag. drivers/media/platform/soc_camera/atmel-isi.c | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/drivers/media/platform/soc_camera/atmel-isi.c b/drivers/media/platform/soc_camera/atmel-isi.c index 274a6f7..0fd6bc9 100644 --- a/drivers/media/platform/soc_camera/atmel-isi.c +++ b/drivers/media/platform/soc_camera/atmel-isi.c @@ -106,24 +106,25 @@ static u32 isi_readl(struct atmel_isi *isi, u32 reg) static int configure_geometry(struct atmel_isi *isi, u32 width, u32 height, u32 code) { - u32 cfg2, cr; + u32 cfg2; + /* According to sensor's output format to set cfg2 */ switch (code) { /* YUV, including grey */ case MEDIA_BUS_FMT_Y8_1X8: - cr = ISI_CFG2_GRAYSCALE; + cfg2 = ISI_CFG2_GRAYSCALE; break; case MEDIA_BUS_FMT_VYUY8_2X8: - cr = ISI_CFG2_YCC_SWAP_MODE_3; + cfg2 = ISI_CFG2_YCC_SWAP_MODE_3; break; case MEDIA_BUS_FMT_UYVY8_2X8: - cr = ISI_CFG2_YCC_SWAP_MODE_2; + cfg2 = ISI_CFG2_YCC_SWAP_MODE_2; break; case MEDIA_BUS_FMT_YVYU8_2X8: - cr = ISI_CFG2_YCC_SWAP_MODE_1; + cfg2 = ISI_CFG2_YCC_SWAP_MODE_1; break; case MEDIA_BUS_FMT_YUYV8_2X8: - cr = ISI_CFG2_YCC_SWAP_DEFAULT; + cfg2 = ISI_CFG2_YCC_SWAP_DEFAULT; break; /* RGB, TODO */ default: @@ -131,17 +132,10 @@ static int configure_geometry(struct atmel_isi *isi, u32 width, } isi_writel(isi, ISI_CTRL, ISI_CTRL_DIS); - - cfg2 = isi_readl(isi, ISI_CFG2); - /* Set YCC swap mode */ - cfg2 &= ~ISI_CFG2_YCC_SWAP_MODE_MASK; - cfg2 |= cr; /* Set width */ - cfg2 &= ~(ISI_CFG2_IM_HSIZE_MASK); cfg2 |= ((width - 1) << ISI_CFG2_IM_HSIZE_OFFSET) & ISI_CFG2_IM_HSIZE_MASK; /* Set height */ - cfg2 &= ~(ISI_CFG2_IM_VSIZE_MASK); cfg2 |= ((height - 1) << ISI_CFG2_IM_VSIZE_OFFSET) & ISI_CFG2_IM_VSIZE_MASK; isi_writel(isi, ISI_CFG2, cfg2); -- 1.9.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 2/3] media: atmel-isi: move configure_geometry() to start_streaming() 2015-08-05 3:26 [PATCH v2 1/3] media: atmel-isi: setup the ISI_CFG2 register directly Josh Wu @ 2015-08-05 3:26 ` Josh Wu 2015-08-05 3:26 ` [PATCH v2 3/3] media: atmel-isi: add sanity check for supported formats in set_fmt() Josh Wu 1 sibling, 0 replies; 6+ messages in thread From: Josh Wu @ 2015-08-05 3:26 UTC (permalink / raw) To: Linux Media Mailing List, Guennadi Liakhovetski Cc: Laurent Pinchart, Josh Wu, Mauro Carvalho Chehab, linux-kernel As in set_fmt() function we only need to know which format is been set, we don't need to access the ISI hardware in this moment. So move the configure_geometry(), which access the ISI hardware, to start_streaming() will make code more consistent and simpler. Signed-off-by: Josh Wu <josh.wu@atmel.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- Changes in v2: - Add Laurent's reviewed-by tag. drivers/media/platform/soc_camera/atmel-isi.c | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/drivers/media/platform/soc_camera/atmel-isi.c b/drivers/media/platform/soc_camera/atmel-isi.c index 0fd6bc9..cb46aec 100644 --- a/drivers/media/platform/soc_camera/atmel-isi.c +++ b/drivers/media/platform/soc_camera/atmel-isi.c @@ -391,6 +391,11 @@ 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, + icd->current_fmt->code); + if (ret < 0) + return ret; + spin_lock_irq(&isi->lock); /* Clear any pending interrupt */ isi_readl(isi, ISI_STATUS); @@ -478,8 +483,6 @@ static int isi_camera_init_videobuf(struct vb2_queue *q, static int isi_camera_set_fmt(struct soc_camera_device *icd, struct v4l2_format *f) { - struct soc_camera_host *ici = to_soc_camera_host(icd->parent); - struct atmel_isi *isi = ici->priv; struct v4l2_subdev *sd = soc_camera_to_subdev(icd); const struct soc_camera_format_xlate *xlate; struct v4l2_pix_format *pix = &f->fmt.pix; @@ -512,16 +515,6 @@ static int isi_camera_set_fmt(struct soc_camera_device *icd, if (mf->code != xlate->code) return -EINVAL; - /* Enable PM and peripheral clock before operate isi registers */ - pm_runtime_get_sync(ici->v4l2_dev.dev); - - ret = configure_geometry(isi, pix->width, pix->height, xlate->code); - - pm_runtime_put(ici->v4l2_dev.dev); - - if (ret < 0) - return ret; - pix->width = mf->width; pix->height = mf->height; pix->field = mf->field; -- 1.9.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 3/3] media: atmel-isi: add sanity check for supported formats in set_fmt() 2015-08-05 3:26 [PATCH v2 1/3] media: atmel-isi: setup the ISI_CFG2 register directly Josh Wu 2015-08-05 3:26 ` [PATCH v2 2/3] media: atmel-isi: move configure_geometry() to start_streaming() Josh Wu @ 2015-08-05 3:26 ` Josh Wu 2015-08-20 9:13 ` Josh Wu 2015-08-20 18:30 ` Laurent Pinchart 1 sibling, 2 replies; 6+ messages in thread From: Josh Wu @ 2015-08-05 3:26 UTC (permalink / raw) To: Linux Media Mailing List, Guennadi Liakhovetski Cc: Laurent Pinchart, Josh Wu, Mauro Carvalho Chehab, linux-kernel 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@atmel.com> --- 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: + break; + /* RGB, TODO */ + default: + dev_err(icd->parent, "not supported format: %d\n", + xlate->code); + 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; + pix->width = mf->width; pix->height = mf->height; pix->field = mf->field; -- 1.9.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 3/3] media: atmel-isi: add sanity check for supported formats in set_fmt() 2015-08-05 3:26 ` [PATCH v2 3/3] media: atmel-isi: add sanity check for supported formats in set_fmt() Josh Wu @ 2015-08-20 9:13 ` Josh Wu 2015-08-20 18:30 ` Laurent Pinchart 1 sibling, 0 replies; 6+ messages in thread From: Josh Wu @ 2015-08-20 9:13 UTC (permalink / raw) To: Linux Media Mailing List, Guennadi Liakhovetski Cc: Laurent Pinchart, Mauro Carvalho Chehab, linux-kernel Hi, Laurent Could you have time to review this patch? so that I can send a pull request with your reviewed-by tags. Best Regards, Josh Wu On 8/5/2015 11:26 AM, 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@atmel.com> > --- > > 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: > + break; > + /* RGB, TODO */ > + default: > + dev_err(icd->parent, "not supported format: %d\n", > + xlate->code); > + 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; > + > pix->width = mf->width; > pix->height = mf->height; > pix->field = mf->field; ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 3/3] media: atmel-isi: add sanity check for supported formats in set_fmt() 2015-08-05 3:26 ` [PATCH v2 3/3] media: atmel-isi: add sanity check for supported formats in set_fmt() Josh Wu 2015-08-20 9:13 ` Josh Wu @ 2015-08-20 18:30 ` Laurent Pinchart 2015-08-21 3:10 ` Josh Wu 1 sibling, 1 reply; 6+ messages in thread From: Laurent Pinchart @ 2015-08-20 18:30 UTC (permalink / raw) To: Josh Wu Cc: Linux Media Mailing List, Guennadi Liakhovetski, Mauro Carvalho Chehab, linux-kernel 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@atmel.com> > --- > > 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. > + 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. > + 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. > pix->width = mf->width; > pix->height = mf->height; > pix->field = mf->field; -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 3/3] media: atmel-isi: add sanity check for supported formats in set_fmt() 2015-08-20 18:30 ` Laurent Pinchart @ 2015-08-21 3:10 ` Josh Wu 0 siblings, 0 replies; 6+ messages in thread From: Josh Wu @ 2015-08-21 3:10 UTC (permalink / raw) To: Laurent Pinchart Cc: Linux Media Mailing List, Guennadi Liakhovetski, Mauro Carvalho Chehab, linux-kernel 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@atmel.com> >> --- >> >> 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; ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-08-21 3:11 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-08-05 3:26 [PATCH v2 1/3] media: atmel-isi: setup the ISI_CFG2 register directly Josh Wu 2015-08-05 3:26 ` [PATCH v2 2/3] media: atmel-isi: move configure_geometry() to start_streaming() Josh Wu 2015-08-05 3:26 ` [PATCH v2 3/3] media: atmel-isi: add sanity check for supported formats in set_fmt() Josh Wu 2015-08-20 9:13 ` Josh Wu 2015-08-20 18:30 ` Laurent Pinchart 2015-08-21 3:10 ` Josh Wu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox