Ok, let's take this one, but, please, address the below couple of minor issues in an incremental patch.
On Mon, 27 Sep 2010, Janusz Krzysztofik wrote:
+/* write a register */ +static int ov6650_reg_write(struct i2c_client *client, u8 reg, u8 val) +{
- int ret;
- unsigned char data[2] = { reg, val };
- struct i2c_msg msg = {
.addr = client->addr,.flags = 0,.len = 2,.buf = data,- };
- ret = i2c_transfer(client->adapter, &msg, 1);
- usleep_range(100, 1000);
So, 100us are enough? Then I'd just go with udelay(100).
static bool is_unscaled_ok(int width, int height, struct v4l2_rect *rect) +{
- return (width > rect->width >> 1 || height > rect->height >> 1);
+}
Ok, just one more pair of brackets to remove;)
+static u8 to_clkrc(struct v4l2_fract *timeperframe,
unsigned long pclk_limit, unsigned long pclk_max)+{
- unsigned long pclk;
- if (timeperframe->numerator && timeperframe->denominator)
pclk = pclk_max * timeperframe->denominator /(FRAME_RATE_MAX * timeperframe->numerator);- else
pclk = pclk_max;- if (pclk_limit && pclk_limit < pclk)
pclk = pclk_limit;- return (pclk_max - 1) / pclk;
+}
+/* set the format we will capture in */ +static int ov6650_s_fmt(struct v4l2_subdev *sd, struct v4l2_mbus_framefmt *mf) +{
- struct i2c_client *client = sd->priv;
- struct soc_camera_device *icd = client->dev.platform_data;
- struct soc_camera_sense *sense = icd->sense;
- struct ov6650 *priv = to_ov6650(client);
- bool half_scale = !is_unscaled_ok(mf->width, mf->height, &priv->rect);
- struct v4l2_crop a = {
.type = V4L2_BUF_TYPE_VIDEO_CAPTURE,.c = {.left = priv->rect.left + (priv->rect.width >> 1) -(mf->width >> (1 - half_scale)),.top = priv->rect.top + (priv->rect.height >> 1) -(mf->height >> (1 - half_scale)),.width = mf->width << half_scale,.height = mf->height << half_scale,},- };
Hm, this seems wrong to me... You calculate left and top to preserve the center, right? This is good, but: if output is unscaled, you want
.left = priv->rect.left + (priv->rect.width - mf->width) / 2;
in this case half_scale = 0 and the above is correct. Now, is the output is scaled, you want
.left = priv->rect.left + (priv->rect.width - mf->width * 2) / 2; which is not, what you have above. Am I missing anything?
- case V4L2_MBUS_FMT_UYVY8_2X8:
dev_dbg(&client->dev, "pixel format YUYV8_2X8_BE\n");if (half_scale) {coma_mask |= COMA_RGB | COMA_BW | COMA_WORD_SWAP;coma_set |= COMA_BYTE_SWAP;} else {coma_mask |= COMA_RGB | COMA_BW;coma_set |= COMA_BYTE_SWAP | COMA_WORD_SWAP;}break;- case V4L2_MBUS_FMT_VYUY8_2X8:
dev_dbg(&client->dev, "pixel format YVYU8_2X8_BE (untested)\n");if (half_scale) {coma_mask |= COMA_RGB | COMA_BW;coma_set |= COMA_BYTE_SWAP | COMA_WORD_SWAP;} else {coma_mask |= COMA_RGB | COMA_BW | COMA_WORD_SWAP;coma_set |= COMA_BYTE_SWAP;}break;
...since there anyway will be an incremental patch . what does word-swapping have to do with scaling?...
- case V4L2_MBUS_FMT_SBGGR8_1X8:
dev_dbg(&client->dev, "pixel format SBGGR8_1X8 (untested)\n");coma_mask |= COMA_BW | COMA_BYTE_SWAP | COMA_WORD_SWAP;coma_set |= COMA_RAW_RGB | COMA_RGB;break;- case 0:
break;
0 is defined as reserved... What for do you need it here?
Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/