| Message ID | 20240126133116.121981-6-biju.das.jz@bp.renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | Add RZ/G2UL CRU and CSI support | expand |
Hi Biju, On Fri, Jan 26, 2024 at 2:31 PM Biju Das <biju.das.jz@bp.renesas.com> wrote: > Reduce i2c freq from 400->100 kHz on RZ/G2UL SMARC EVK as the captured > image is not proper with the sensor configuration over I2C. > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> Thanks for your patch! > --- a/arch/arm64/boot/dts/renesas/r9a07g043u11-smarc-cru-csi-ov5645.dtso > +++ b/arch/arm64/boot/dts/renesas/r9a07g043u11-smarc-cru-csi-ov5645.dtso > @@ -19,3 +19,7 @@ &ov5645 { > enable-gpios = <&pinctrl RZG2L_GPIO(4, 4) GPIO_ACTIVE_HIGH>; > reset-gpios = <&pinctrl RZG2L_GPIO(0, 1) GPIO_ACTIVE_LOW>; > }; > + > +&i2c0 { > + clock-frequency = <100000>; > +}; Is this a limitation of one of the I2C devices on the bus, or a PCB design issue? Doesn't this need a Fixes tag? Gr{oetje,eeting}s, Geert
Hi Geert, Thanks for the feedback. > -----Original Message----- > From: Geert Uytterhoeven <geert@linux-m68k.org> > Sent: Friday, January 26, 2024 1:53 PM > Subject: Re: [PATCH v2 5/5] arm64: dts: renesas: r9a07g043u11-smarc-cru- > csi-ov5645: Reduce I2C frequency > > Hi Biju, > > On Fri, Jan 26, 2024 at 2:31 PM Biju Das <biju.das.jz@bp.renesas.com> > wrote: > > Reduce i2c freq from 400->100 kHz on RZ/G2UL SMARC EVK as the captured > > image is not proper with the sensor configuration over I2C. > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > Thanks for your patch! > > > --- > > a/arch/arm64/boot/dts/renesas/r9a07g043u11-smarc-cru-csi-ov5645.dtso > > +++ b/arch/arm64/boot/dts/renesas/r9a07g043u11-smarc-cru-csi-ov5645.dt > > +++ so > > @@ -19,3 +19,7 @@ &ov5645 { > > enable-gpios = <&pinctrl RZG2L_GPIO(4, 4) GPIO_ACTIVE_HIGH>; > > reset-gpios = <&pinctrl RZG2L_GPIO(0, 1) GPIO_ACTIVE_LOW>; }; > > + > > +&i2c0 { > > + clock-frequency = <100000>; > > +}; > > Is this a limitation of one of the I2C devices on the bus, or a PCB design > issue? Currently versa3 clock generator connected to the same bus and it works ok with 400kHz clock. Maybe it is stressed not that much compared to OV5645 sensor configuration. At the moment with 400kHz I2C bus clock, Camera capture is not working properly on RZ/G2UL, but with same bus frequency the same works fine on RZ/{G2L,G2LC,V2L}. There may be some hardware differences which is causing this issue. > > Doesn't this need a Fixes tag? I can create a new patch updating bus frequency as 100kHz and add fixes tag. After this I will drop this patch as it no longer needed. Please let me know. Cheers, Biju
Hi Geert, > -----Original Message----- > From: Biju Das > Sent: Friday, January 26, 2024 3:57 PM > Subject: RE: [PATCH v2 5/5] arm64: dts: renesas: r9a07g043u11-smarc-cru- > csi-ov5645: Reduce I2C frequency > > Hi Geert, > > Thanks for the feedback. > > > -----Original Message----- > > From: Geert Uytterhoeven <geert@linux-m68k.org> > > Sent: Friday, January 26, 2024 1:53 PM > > Subject: Re: [PATCH v2 5/5] arm64: dts: renesas: > > r9a07g043u11-smarc-cru- > > csi-ov5645: Reduce I2C frequency > > > > Hi Biju, > > > > On Fri, Jan 26, 2024 at 2:31 PM Biju Das <biju.das.jz@bp.renesas.com> > > wrote: > > > Reduce i2c freq from 400->100 kHz on RZ/G2UL SMARC EVK as the > > > captured image is not proper with the sensor configuration over I2C. > > > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > > > Thanks for your patch! > > > > > --- > > > a/arch/arm64/boot/dts/renesas/r9a07g043u11-smarc-cru-csi-ov5645.dtso > > > +++ b/arch/arm64/boot/dts/renesas/r9a07g043u11-smarc-cru-csi-ov5645. > > > +++ dt > > > +++ so > > > @@ -19,3 +19,7 @@ &ov5645 { > > > enable-gpios = <&pinctrl RZG2L_GPIO(4, 4) GPIO_ACTIVE_HIGH>; > > > reset-gpios = <&pinctrl RZG2L_GPIO(0, 1) GPIO_ACTIVE_LOW>; > > > }; > > > + > > > +&i2c0 { > > > + clock-frequency = <100000>; > > > +}; > > > > Is this a limitation of one of the I2C devices on the bus, or a PCB > > design issue? > > Currently versa3 clock generator connected to the same bus and it works ok > with 400kHz clock. Maybe it is stressed not that much compared to OV5645 > sensor configuration. > > At the moment with 400kHz I2C bus clock, Camera capture is not working > properly on RZ/G2UL, but with same bus frequency the same works fine on > RZ/{G2L,G2LC,V2L}. > There may be some hardware differences which is causing this issue. > > > > > Doesn't this need a Fixes tag? > > I can create a new patch updating bus frequency as 100kHz and add fixes > tag. > After this I will drop this patch as it no longer needed. > > Please let me know. + media Adding a delay after Software reset makes it to work at 400kHz with RZ/G2UL SMARC EVK. So not sure we need to add delay after software reset? Now after OV5645 gpio reset, then there is 20msec delay and then again we are issuing software reset and there is no delay after this for this software reset. diff --git a/drivers/media/i2c/ov5645.c b/drivers/media/i2c/ov5645.c index a26ac11c989d..d67a5e23fe5a 100644 --- a/drivers/media/i2c/ov5645.c +++ b/drivers/media/i2c/ov5645.c @@ -622,11 +622,19 @@ static int ov5645_set_register_array(struct ov5645 *ov5645, { unsigned int i; int ret; for (i = 0; i < num_settings; ++i, ++settings) { ret = ov5645_write_reg(ov5645, settings->reg, settings->val); if (ret < 0) return ret; + + if (settings->reg == OV5645_SYSTEM_CTRL0) + fsleep(1000); Cheers, Biju
Hi Geert, > -----Original Message----- > From: Biju Das > Sent: Tuesday, January 30, 2024 2:15 PM > Subject: RE: [PATCH v2 5/5] arm64: dts: renesas: r9a07g043u11-smarc-cru- > csi-ov5645: Reduce I2C frequency > > Hi Geert, > > > -----Original Message----- > > From: Biju Das > > Sent: Friday, January 26, 2024 3:57 PM > > Subject: RE: [PATCH v2 5/5] arm64: dts: renesas: > > r9a07g043u11-smarc-cru- > > csi-ov5645: Reduce I2C frequency > > > > Hi Geert, > > > > Thanks for the feedback. > > > > > -----Original Message----- > > > From: Geert Uytterhoeven <geert@linux-m68k.org> > > > Sent: Friday, January 26, 2024 1:53 PM > > > Subject: Re: [PATCH v2 5/5] arm64: dts: renesas: > > > r9a07g043u11-smarc-cru- > > > csi-ov5645: Reduce I2C frequency > > > > > > Hi Biju, > > > > > > On Fri, Jan 26, 2024 at 2:31 PM Biju Das > > > <biju.das.jz@bp.renesas.com> > > > wrote: > > > > Reduce i2c freq from 400->100 kHz on RZ/G2UL SMARC EVK as the > > > > captured image is not proper with the sensor configuration over I2C. > > > > > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > > > > > Thanks for your patch! > > > > > > > --- > > > > a/arch/arm64/boot/dts/renesas/r9a07g043u11-smarc-cru-csi-ov5645.dt > > > > so > > > > +++ b/arch/arm64/boot/dts/renesas/r9a07g043u11-smarc-cru-csi-ov5645. > > > > +++ dt > > > > +++ so > > > > @@ -19,3 +19,7 @@ &ov5645 { > > > > enable-gpios = <&pinctrl RZG2L_GPIO(4, 4) GPIO_ACTIVE_HIGH>; > > > > reset-gpios = <&pinctrl RZG2L_GPIO(0, 1) GPIO_ACTIVE_LOW>; > > > > }; > > > > + > > > > +&i2c0 { > > > > + clock-frequency = <100000>; }; > > > > > > Is this a limitation of one of the I2C devices on the bus, or a PCB > > > design issue? > > > > Currently versa3 clock generator connected to the same bus and it > > works ok with 400kHz clock. Maybe it is stressed not that much > > compared to OV5645 sensor configuration. > > > > At the moment with 400kHz I2C bus clock, Camera capture is not working > > properly on RZ/G2UL, but with same bus frequency the same works fine > > on RZ/{G2L,G2LC,V2L}. > > There may be some hardware differences which is causing this issue. > > > > > > > > Doesn't this need a Fixes tag? > > > > I can create a new patch updating bus frequency as 100kHz and add > > fixes tag. > > After this I will drop this patch as it no longer needed. > > > > Please let me know. > > + media > > Adding a delay after Software reset makes it to work at 400kHz with > RZ/G2UL SMARC EVK. > > So not sure we need to add delay after software reset? > > Now after OV5645 gpio reset, then there is 20msec delay and then again we > are issuing software reset and there is no delay after this for this > software reset. > > diff --git a/drivers/media/i2c/ov5645.c b/drivers/media/i2c/ov5645.c index > a26ac11c989d..d67a5e23fe5a 100644 > --- a/drivers/media/i2c/ov5645.c > +++ b/drivers/media/i2c/ov5645.c > @@ -622,11 +622,19 @@ static int ov5645_set_register_array(struct ov5645 > *ov5645, { > unsigned int i; > int ret; > > for (i = 0; i < num_settings; ++i, ++settings) { > ret = ov5645_write_reg(ov5645, settings->reg, settings- > >val); > if (ret < 0) > return ret; > + > + if (settings->reg == OV5645_SYSTEM_CTRL0) > + fsleep(1000); > This issue seen on RZ/G2L SMARC EVK as well. My testing on G2L family shows we need to add delay to make OV5645 to work @400kHZ. I am not sure any one tested OV5645 with I2C bus frequency 400kHz? Cheers, Biju
Hi folks, On Fri, Feb 09, 2024 at 02:36:22PM +0000, Biju Das wrote: > Hi Geert, > > > -----Original Message----- > > From: Biju Das > > Sent: Tuesday, January 30, 2024 2:15 PM > > Subject: RE: [PATCH v2 5/5] arm64: dts: renesas: r9a07g043u11-smarc-cru- > > csi-ov5645: Reduce I2C frequency > > > > Hi Geert, > > > > > -----Original Message----- > > > From: Biju Das > > > Sent: Friday, January 26, 2024 3:57 PM > > > Subject: RE: [PATCH v2 5/5] arm64: dts: renesas: > > > r9a07g043u11-smarc-cru- > > > csi-ov5645: Reduce I2C frequency > > > > > > Hi Geert, > > > > > > Thanks for the feedback. > > > > > > > -----Original Message----- > > > > From: Geert Uytterhoeven <geert@linux-m68k.org> > > > > Sent: Friday, January 26, 2024 1:53 PM > > > > Subject: Re: [PATCH v2 5/5] arm64: dts: renesas: > > > > r9a07g043u11-smarc-cru- > > > > csi-ov5645: Reduce I2C frequency > > > > > > > > Hi Biju, > > > > > > > > On Fri, Jan 26, 2024 at 2:31 PM Biju Das > > > > <biju.das.jz@bp.renesas.com> > > > > wrote: > > > > > Reduce i2c freq from 400->100 kHz on RZ/G2UL SMARC EVK as the > > > > > captured image is not proper with the sensor configuration over I2C. > > > > > > > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > > > > > > > Thanks for your patch! > > > > > > > > > --- > > > > > a/arch/arm64/boot/dts/renesas/r9a07g043u11-smarc-cru-csi-ov5645.dt > > > > > so > > > > > +++ b/arch/arm64/boot/dts/renesas/r9a07g043u11-smarc-cru-csi-ov5645. > > > > > +++ dt > > > > > +++ so > > > > > @@ -19,3 +19,7 @@ &ov5645 { > > > > > enable-gpios = <&pinctrl RZG2L_GPIO(4, 4) GPIO_ACTIVE_HIGH>; > > > > > reset-gpios = <&pinctrl RZG2L_GPIO(0, 1) GPIO_ACTIVE_LOW>; > > > > > }; > > > > > + > > > > > +&i2c0 { > > > > > + clock-frequency = <100000>; }; > > > > > > > > Is this a limitation of one of the I2C devices on the bus, or a PCB > > > > design issue? > > > > > > Currently versa3 clock generator connected to the same bus and it > > > works ok with 400kHz clock. Maybe it is stressed not that much > > > compared to OV5645 sensor configuration. > > > > > > At the moment with 400kHz I2C bus clock, Camera capture is not working > > > properly on RZ/G2UL, but with same bus frequency the same works fine > > > on RZ/{G2L,G2LC,V2L}. > > > There may be some hardware differences which is causing this issue. > > > > > > > > > > > Doesn't this need a Fixes tag? > > > > > > I can create a new patch updating bus frequency as 100kHz and add > > > fixes tag. > > > After this I will drop this patch as it no longer needed. > > > > > > Please let me know. > > > > + media > > > > Adding a delay after Software reset makes it to work at 400kHz with > > RZ/G2UL SMARC EVK. > > > > So not sure we need to add delay after software reset? > > > > Now after OV5645 gpio reset, then there is 20msec delay and then again we > > are issuing software reset and there is no delay after this for this > > software reset. > > > > diff --git a/drivers/media/i2c/ov5645.c b/drivers/media/i2c/ov5645.c index > > a26ac11c989d..d67a5e23fe5a 100644 > > --- a/drivers/media/i2c/ov5645.c > > +++ b/drivers/media/i2c/ov5645.c > > @@ -622,11 +622,19 @@ static int ov5645_set_register_array(struct ov5645 > > *ov5645, { > > unsigned int i; > > int ret; > > > > for (i = 0; i < num_settings; ++i, ++settings) { > > ret = ov5645_write_reg(ov5645, settings->reg, settings- > > >val); > > if (ret < 0) > > return ret; > > + > > + if (settings->reg == OV5645_SYSTEM_CTRL0) > > + fsleep(1000); > > > > This issue seen on RZ/G2L SMARC EVK as well. My testing on G2L family shows > we need to add delay to make OV5645 to work @400kHZ. Typically there's a delay before the chip is accessible over I²C after resetting it. It's a bit open whether this one needs it, very probably it does. It'd be nicer nonetheless to do this outside the register list and instead use a separate ov5645_write_reg() call. Probably the first write is redundant. The second write resets the device. 0x80 should be sufficient value for that.
Hi Sakari Ailus, > -----Original Message----- > From: Sakari Ailus <sakari.ailus@linux.intel.com> > Sent: Friday, February 9, 2024 3:18 PM > Subject: Re: [PATCH v2 5/5] arm64: dts: renesas: r9a07g043u11-smarc-cru- > csi-ov5645: Reduce I2C frequency > > Hi folks, > > On Fri, Feb 09, 2024 at 02:36:22PM +0000, Biju Das wrote: > > Hi Geert, > > > > > -----Original Message----- > > > From: Biju Das > > > Sent: Tuesday, January 30, 2024 2:15 PM > > > Subject: RE: [PATCH v2 5/5] arm64: dts: renesas: > > > r9a07g043u11-smarc-cru- > > > csi-ov5645: Reduce I2C frequency > > > > > > Hi Geert, > > > > > > > -----Original Message----- > > > > From: Biju Das > > > > Sent: Friday, January 26, 2024 3:57 PM > > > > Subject: RE: [PATCH v2 5/5] arm64: dts: renesas: > > > > r9a07g043u11-smarc-cru- > > > > csi-ov5645: Reduce I2C frequency > > > > > > > > Hi Geert, > > > > > > > > Thanks for the feedback. > > > > > > > > > -----Original Message----- > > > > > From: Geert Uytterhoeven <geert@linux-m68k.org> > > > > > Sent: Friday, January 26, 2024 1:53 PM > > > > > Subject: Re: [PATCH v2 5/5] arm64: dts: renesas: > > > > > r9a07g043u11-smarc-cru- > > > > > csi-ov5645: Reduce I2C frequency > > > > > > > > > > Hi Biju, > > > > > > > > > > On Fri, Jan 26, 2024 at 2:31 PM Biju Das > > > > > <biju.das.jz@bp.renesas.com> > > > > > wrote: > > > > > > Reduce i2c freq from 400->100 kHz on RZ/G2UL SMARC EVK as the > > > > > > captured image is not proper with the sensor configuration over > I2C. > > > > > > > > > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > > > > > > > > > Thanks for your patch! > > > > > > > > > > > --- > > > > > > a/arch/arm64/boot/dts/renesas/r9a07g043u11-smarc-cru-csi-ov564 > > > > > > 5.dt > > > > > > so > > > > > > +++ b/arch/arm64/boot/dts/renesas/r9a07g043u11-smarc-cru-csi- > ov5645. > > > > > > +++ dt > > > > > > +++ so > > > > > > @@ -19,3 +19,7 @@ &ov5645 { > > > > > > enable-gpios = <&pinctrl RZG2L_GPIO(4, 4) > GPIO_ACTIVE_HIGH>; > > > > > > reset-gpios = <&pinctrl RZG2L_GPIO(0, 1) > > > > > > GPIO_ACTIVE_LOW>; }; > > > > > > + > > > > > > +&i2c0 { > > > > > > + clock-frequency = <100000>; }; > > > > > > > > > > Is this a limitation of one of the I2C devices on the bus, or a > > > > > PCB design issue? > > > > > > > > Currently versa3 clock generator connected to the same bus and it > > > > works ok with 400kHz clock. Maybe it is stressed not that much > > > > compared to OV5645 sensor configuration. > > > > > > > > At the moment with 400kHz I2C bus clock, Camera capture is not > > > > working properly on RZ/G2UL, but with same bus frequency the same > > > > works fine on RZ/{G2L,G2LC,V2L}. > > > > There may be some hardware differences which is causing this issue. > > > > > > > > > > > > > > Doesn't this need a Fixes tag? > > > > > > > > I can create a new patch updating bus frequency as 100kHz and add > > > > fixes tag. > > > > After this I will drop this patch as it no longer needed. > > > > > > > > Please let me know. > > > > > > + media > > > > > > Adding a delay after Software reset makes it to work at 400kHz with > > > RZ/G2UL SMARC EVK. > > > > > > So not sure we need to add delay after software reset? > > > > > > Now after OV5645 gpio reset, then there is 20msec delay and then > > > again we are issuing software reset and there is no delay after this > > > for this software reset. > > > > > > diff --git a/drivers/media/i2c/ov5645.c b/drivers/media/i2c/ov5645.c > > > index a26ac11c989d..d67a5e23fe5a 100644 > > > --- a/drivers/media/i2c/ov5645.c > > > +++ b/drivers/media/i2c/ov5645.c > > > @@ -622,11 +622,19 @@ static int ov5645_set_register_array(struct > > > ov5645 *ov5645, { > > > unsigned int i; > > > int ret; > > > > > > for (i = 0; i < num_settings; ++i, ++settings) { > > > ret = ov5645_write_reg(ov5645, settings->reg, > > > settings- > > > >val); > > > if (ret < 0) > > > return ret; > > > + > > > + if (settings->reg == OV5645_SYSTEM_CTRL0) > > > + fsleep(1000); > > > > > > > This issue seen on RZ/G2L SMARC EVK as well. My testing on G2L family > > shows we need to add delay to make OV5645 to work @400kHZ. > > Typically there's a delay before the chip is accessible over I²C after > resetting it. It's a bit open whether this one needs it, very probably it > does. It'd be nicer nonetheless to do this outside the register list and > instead use a separate ov5645_write_reg() call. OK. > > Probably the first write is redundant. The second write resets the device. > 0x80 should be sufficient value for that. Will test and send patch for it. Cheers, Biju
diff --git a/arch/arm64/boot/dts/renesas/r9a07g043u11-smarc-cru-csi-ov5645.dtso b/arch/arm64/boot/dts/renesas/r9a07g043u11-smarc-cru-csi-ov5645.dtso index b41bb4b31a26..bb769c0f3ca5 100644 --- a/arch/arm64/boot/dts/renesas/r9a07g043u11-smarc-cru-csi-ov5645.dtso +++ b/arch/arm64/boot/dts/renesas/r9a07g043u11-smarc-cru-csi-ov5645.dtso @@ -19,3 +19,7 @@ &ov5645 { enable-gpios = <&pinctrl RZG2L_GPIO(4, 4) GPIO_ACTIVE_HIGH>; reset-gpios = <&pinctrl RZG2L_GPIO(0, 1) GPIO_ACTIVE_LOW>; }; + +&i2c0 { + clock-frequency = <100000>; +};
Reduce i2c freq from 400->100 kHz on RZ/G2UL SMARC EVK as the captured image is not proper with the sensor configuration over I2C. Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> --- v1->v2: * No change. --- .../boot/dts/renesas/r9a07g043u11-smarc-cru-csi-ov5645.dtso | 4 ++++ 1 file changed, 4 insertions(+)