Re: [PATCH v1] [media] atmel-isi: code cleanup

[Wu, Songjun]

Hi Hugues,

Thank you for your patch.
Is it necessary to ensure ISI is clocked before starting sensor sub device?

On 5/19/2017 20:08, Hugues FRUCHET wrote:

Adding Songjun and Ludovic as Atmel maintainers, sorry for inconvenience.

On 05/19/2017 12:04 PM, Hugues Fruchet wrote:

Ensure that ISI is clocked before starting sensor sub device.
Remove un-needed type check in try_fmt().
Use clamp() macro for hardware capabilities.
Fix wrong tabulation to space.

Signed-off-by: Hugues Fruchet <hugues.fruc...@st.com>
---
   drivers/media/platform/atmel/atmel-isi.c | 24 ++++++++++--------------
   1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/drivers/media/platform/atmel/atmel-isi.c 
b/drivers/media/platform/atmel/atmel-isi.c
index e4867f8..7bf9f7d 100644
--- a/drivers/media/platform/atmel/atmel-isi.c
+++ b/drivers/media/platform/atmel/atmel-isi.c
@@ -36,8 +36,8 @@

#include "atmel-isi.h" -#define MAX_SUPPORT_WIDTH 2048

-#define MAX_SUPPORT_HEIGHT             2048
+#define MAX_SUPPORT_WIDTH              2048U
+#define MAX_SUPPORT_HEIGHT             2048U
   #define MIN_FRAME_RATE                       15
   #define FRAME_INTERVAL_MILLI_SEC     (1000 / MIN_FRAME_RATE)

@@ -424,6 +424,8 @@ static int start_streaming(struct vb2_queue *vq, unsigned int count)

        struct frame_buffer *buf, *node;
        int ret;

+ pm_runtime_get_sync(isi->dev);

+
        /* Enable stream on the sub device */
        ret = v4l2_subdev_call(isi->entity.subdev, video, s_stream, 1);
        if (ret && ret != -ENOIOCTLCMD) {
@@ -431,8 +433,6 @@ static int start_streaming(struct vb2_queue *vq, unsigned 
int count)
                goto err_start_stream;
        }

- pm_runtime_get_sync(isi->dev);

-
        /* Reset ISI */
        ret = atmel_isi_wait_status(isi, WAIT_ISI_RESET);
        if (ret < 0) {
@@ -455,10 +455,11 @@ static int start_streaming(struct vb2_queue *vq, unsigned 
int count)
        return 0;

err_reset:

-       pm_runtime_put(isi->dev);
        v4l2_subdev_call(isi->entity.subdev, video, s_stream, 0);

err_start_stream:

+       pm_runtime_put(isi->dev);
+
        spin_lock_irq(&isi->irqlock);
        isi->active = NULL;
        /* Release all active buffers */
@@ -566,20 +567,15 @@ static int isi_try_fmt(struct atmel_isi *isi, struct 
v4l2_format *f,
        };
        int ret;

- if (f->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)

-               return -EINVAL;
-
        isi_fmt = find_format_by_fourcc(isi, pixfmt->pixelformat);
        if (!isi_fmt) {
                isi_fmt = isi->user_formats[isi->num_user_formats - 1];
                pixfmt->pixelformat = isi_fmt->fourcc;
        }

- /* Limit to Atmel ISC hardware capabilities */

-       if (pixfmt->width > MAX_SUPPORT_WIDTH)
-               pixfmt->width = MAX_SUPPORT_WIDTH;
-       if (pixfmt->height > MAX_SUPPORT_HEIGHT)
-               pixfmt->height = MAX_SUPPORT_HEIGHT;
+       /* Limit to Atmel ISI hardware capabilities */
+       pixfmt->width = clamp(pixfmt->width, 0U, MAX_SUPPORT_WIDTH);
+       pixfmt->height = clamp(pixfmt->height, 0U, MAX_SUPPORT_HEIGHT);

v4l2_fill_mbus_format(&format.format, pixfmt, isi_fmt->mbus_code);

        ret = v4l2_subdev_call(isi->entity.subdev, pad, set_fmt,
@@ -1058,7 +1054,7 @@ static int isi_graph_notify_complete(struct 
v4l2_async_notifier *notifier)
        struct atmel_isi *isi = notifier_to_isi(notifier);
        int ret;

- isi->vdev->ctrl_handler = isi->entity.subdev->ctrl_handler;

+       isi->vdev->ctrl_handler = isi->entity.subdev->ctrl_handler;
        ret = isi_formats_init(isi);
        if (ret) {
                dev_err(isi->dev, "No supported mediabus format found\n");