Re: [PATCH] soc-camera: ov772x: Add buswidth selection flags for platform

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux