Dear Guennadi

Thank you for checking patch

> Can you explain a bit why this patch is needed? Apart from a slight 
> stylistic improvement and a saving of 4 bytes of platform data per camera 
> instance? Is it going to be needed for some future changes?

This patch is not so important/necessary at once.
 -> for saving of 4 bytes.

>       if (!is_power_of_2(priv->info->flags & (OV772X_FLAG_8BIT | 
> OV772X_FLAG_10BIT)))
>               return 0;
> 
> make sense here? Or even just modify your tests above to
(snip)
> Adding a "default:" case just above the "case OV772X_FLAG_10BIT:" line 
> would seem like a good idea to me too.

I understand.

> > +#define OV772X_FLAG_8BIT   (1 << 2) /*  8bit buswidth */
> > +#define OV772X_FLAG_10BIT  (1 << 3) /* 10bit buswidth */

May I suggest here ?
What do you think if it have only 10BIT flag,
and check/operation like this ?

        if (priv->info->flags & OV772X_FLAG_10BIT) {
                flags |= SOCAM_DATAWIDTH_10;
        else
                flags |= SOCAM_DATAWIDTH_8;

This case, below check became not needed,
Does this operation make sense for you ?

> >     /*
> > -    * ov772x only use 8 or 10 bit bus width
> > -    */
> > -   if (SOCAM_DATAWIDTH_10 != priv->info->buswidth &&
> > -       SOCAM_DATAWIDTH_8  != priv->info->buswidth) {
> > -           dev_err(&client->dev, "bus width error\n");
> > -           return -ENODEV;
> > -   }


Best regards
--
Kuninori Morimoto
 
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to