RE: [PATCH v4 2/2] media: i2c: Add imx334 camera sensor driver

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

 



Hi Sakari,

> -----Original Message-----
> From: 'Sakari Ailus' <sakari.ailus@xxxxxxxxxxxxxxx>
> Sent: Monday, January 25, 2021 12:55 PM
> To: Martina Krasteva <martinax.krasteva@xxxxxxxxxxxxxxx>
> Cc: linux-media@xxxxxxxxxxxxxxx; mchehab@xxxxxxxxxx; robh+dt@xxxxxxxxxx;
> devicetree@xxxxxxxxxxxxxxx; daniele.alessandrelli@xxxxxxxxxxxxxxx;
> paul.j.murphy@xxxxxxxxxxxxxxx; gjorgjix.rosikopulos@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH v4 2/2] media: i2c: Add imx334 camera sensor driver
> 
> Hi Martina,
> 
> On Fri, Dec 11, 2020 at 11:51:05AM -0000, Martina Krasteva wrote:
> > Hi Sakari,
> >
> > > -----Original Message-----
> > > From: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> > > Sent: Friday, December 11, 2020 11:32 AM
> > > To: Martina Krasteva <martinax.krasteva@xxxxxxxxxxxxxxx>
> > > Cc: linux-media@xxxxxxxxxxxxxxx; mchehab@xxxxxxxxxx;
> > > robh+dt@xxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx;
> > > daniele.alessandrelli@xxxxxxxxxxxxxxx;
> > > paul.j.murphy@xxxxxxxxxxxxxxx; gjorgjix.rosikopulos@xxxxxxxxxxxxxxx
> > > Subject: Re: [PATCH v4 2/2] media: i2c: Add imx334 camera sensor
> > > driver
> > >
> > > Hi Martina,
> > >
> > > On Fri, Dec 11, 2020 at 10:56:33AM +0000, Martina Krasteva wrote:
> > > ...
> > > > +static int imx334_read_reg(struct imx334 *imx334, u16 reg, u32
> > > > +len,
> > > > +u32 *val) {
> > > > +	struct i2c_client *client =
v4l2_get_subdevdata(&imx334->sd);
> > > > +	struct i2c_msg msgs[2] = { 0 };
> > > > +	u8 addr_buf[2] = { 0 };
> > > > +	u8 data_buf[4] = { 0 };
> > > > +	int ret;
> > > > +
> > > > +	if (WARN_ON(len > 4))
> > > > +		return -EINVAL;
> > > > +
> > > > +	put_unaligned_be16(reg, addr_buf);
> > > > +
> > > > +	/* Write register address */
> > > > +	msgs[0].addr = client->addr;
> > > > +	msgs[0].flags = 0;
> > > > +	msgs[0].len = ARRAY_SIZE(addr_buf);
> > > > +	msgs[0].buf = addr_buf;
> > > > +
> > > > +	/* Read data from register */
> > > > +	msgs[1].addr = client->addr;
> > > > +	msgs[1].flags = I2C_M_RD;
> > > > +	msgs[1].len = len;
> > > > +	msgs[1].buf = &data_buf[4 - len];
> > > > +
> > > > +	ret = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
> > > > +	if (ret != ARRAY_SIZE(msgs))
> > > > +		return -EIO;
> > > > +
> > > > +	*val = get_unaligned_le32(data_buf + (4 - len));
> > >
> > > Hmm. The device native endianness is big (on control interface)
> > > unless something very unexpected happened in hardware development.
> > >
> > > You also can't do this as this will overrun data_buf.
> >
> > Imx334 uses little endian so I have to convert the values.
> 
> Ack. That's very unusual, and probably the first raw camera sensor
implemented
> that way.
> 
> Could you fix the data_buf overrun?
> 
Yes, will be fixed in next version :)
> --
> Sakari Ailus

Kind Regards,
Martina




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux