| Message ID | 20240123115821.292787-3-biju.das.jz@bp.renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Kieran Bingham |
| Headers | show |
| Series | RZ/G2L CSI/CRU Improvements | expand |
Hi Biju, Thank you for the patch. On Tue, Jan 23, 2024 at 11:58:19AM +0000, Biju Das wrote: > As per section 35.3.1 Starting Reception for the MIPI CSI-2 Input on the > latest hardware manual(R01UH0914EJ0140 Rev.1.40) it is mentioned that > after DPHY reset, we need to wait for 1 msec or more before start > receiving data from the sensor. So add a delay after pre_streamon(). > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > --- > drivers/media/platform/renesas/rzg2l-cru/rzg2l-ip.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-ip.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-ip.c > index 9f351a05893e..5468dc179de7 100644 > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-ip.c > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-ip.c > @@ -5,6 +5,7 @@ > * Copyright (C) 2022 Renesas Electronics Corp. > */ > > +#include <linux/delay.h> > #include "rzg2l-cru.h" > > struct rzg2l_cru_ip_format { > @@ -71,6 +72,8 @@ static int rzg2l_cru_ip_s_stream(struct v4l2_subdev *sd, int enable) > if (ret) > return ret; > > + usleep_range(1000, 2000); > + What would you think of using fsleep(1000); instead ? With or without that, Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > ret = rzg2l_cru_start_image_processing(cru); > if (ret) { > v4l2_subdev_call(cru->ip.remote, video, post_streamoff);
Hi Laurent Pinchart, Thanks for the feedback. > -----Original Message----- > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Sent: Tuesday, January 23, 2024 3:30 PM > Subject: Re: [PATCH 2/4] media: platform: rzg2l-cru: rzg2l-ip: Add delay > after D-PHY reset > > Hi Biju, > > Thank you for the patch. > > On Tue, Jan 23, 2024 at 11:58:19AM +0000, Biju Das wrote: > > As per section 35.3.1 Starting Reception for the MIPI CSI-2 Input on > > the latest hardware manual(R01UH0914EJ0140 Rev.1.40) it is mentioned > > that after DPHY reset, we need to wait for 1 msec or more before start > > receiving data from the sensor. So add a delay after pre_streamon(). > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > --- > > drivers/media/platform/renesas/rzg2l-cru/rzg2l-ip.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-ip.c > > b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-ip.c > > index 9f351a05893e..5468dc179de7 100644 > > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-ip.c > > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-ip.c > > @@ -5,6 +5,7 @@ > > * Copyright (C) 2022 Renesas Electronics Corp. > > */ > > > > +#include <linux/delay.h> > > #include "rzg2l-cru.h" > > > > struct rzg2l_cru_ip_format { > > @@ -71,6 +72,8 @@ static int rzg2l_cru_ip_s_stream(struct v4l2_subdev > *sd, int enable) > > if (ret) > > return ret; > > > > + usleep_range(1000, 2000); > > + > > What would you think of using > > fsleep(1000); > > instead ? Essentially it is same by looking at the code[1]. OK will use fsleep() [1] static inline void fsleep(unsigned long usecs) { if (usecs <= 10) udelay(usecs); else if (usecs <= 20000) usleep_range(usecs, 2 * usecs); else msleep(DIV_ROUND_UP(usecs, 1000)); } Cheers, Biju > > With or without that, > > Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > > > ret = rzg2l_cru_start_image_processing(cru); > > if (ret) { > > v4l2_subdev_call(cru->ip.remote, video, post_streamoff); > > -- > Regards, > > Laurent Pinchart
On Tue, Jan 23, 2024 at 06:38:00PM +0000, Biju Das wrote: > Hi Laurent Pinchart, > > Thanks for the feedback. > > > -----Original Message----- > > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > Sent: Tuesday, January 23, 2024 3:30 PM > > Subject: Re: [PATCH 2/4] media: platform: rzg2l-cru: rzg2l-ip: Add delay > > after D-PHY reset > > > > Hi Biju, > > > > Thank you for the patch. > > > > On Tue, Jan 23, 2024 at 11:58:19AM +0000, Biju Das wrote: > > > As per section 35.3.1 Starting Reception for the MIPI CSI-2 Input on > > > the latest hardware manual(R01UH0914EJ0140 Rev.1.40) it is mentioned > > > that after DPHY reset, we need to wait for 1 msec or more before start > > > receiving data from the sensor. So add a delay after pre_streamon(). > > > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > > --- > > > drivers/media/platform/renesas/rzg2l-cru/rzg2l-ip.c | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-ip.c > > > b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-ip.c > > > index 9f351a05893e..5468dc179de7 100644 > > > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-ip.c > > > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-ip.c > > > @@ -5,6 +5,7 @@ > > > * Copyright (C) 2022 Renesas Electronics Corp. > > > */ > > > > > > +#include <linux/delay.h> > > > #include "rzg2l-cru.h" > > > > > > struct rzg2l_cru_ip_format { > > > @@ -71,6 +72,8 @@ static int rzg2l_cru_ip_s_stream(struct v4l2_subdev > > *sd, int enable) > > > if (ret) > > > return ret; > > > > > > + usleep_range(1000, 2000); > > > + > > > > What would you think of using > > > > fsleep(1000); > > > > instead ? > > Essentially it is same by looking at the code[1]. > OK will use fsleep() Yes, it will result in the same delay. fsleep() is recommended as the default sleep function unless there's a specific need to do something different. > [1] > static inline void fsleep(unsigned long usecs) > { > if (usecs <= 10) > udelay(usecs); > else if (usecs <= 20000) > usleep_range(usecs, 2 * usecs); > else > msleep(DIV_ROUND_UP(usecs, 1000)); > } > > > With or without that, > > > > Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > > > > > ret = rzg2l_cru_start_image_processing(cru); > > > if (ret) { > > > v4l2_subdev_call(cru->ip.remote, video, post_streamoff);
Hi Laurent Pinchart, > -----Original Message----- > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Sent: Tuesday, January 23, 2024 6:46 PM > Subject: Re: [PATCH 2/4] media: platform: rzg2l-cru: rzg2l-ip: Add delay > after D-PHY reset > > On Tue, Jan 23, 2024 at 06:38:00PM +0000, Biju Das wrote: > > Hi Laurent Pinchart, > > > > Thanks for the feedback. > > > > > -----Original Message----- > > > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > Sent: Tuesday, January 23, 2024 3:30 PM > > > Subject: Re: [PATCH 2/4] media: platform: rzg2l-cru: rzg2l-ip: Add > > > delay after D-PHY reset > > > > > > Hi Biju, > > > > > > Thank you for the patch. > > > > > > On Tue, Jan 23, 2024 at 11:58:19AM +0000, Biju Das wrote: > > > > As per section 35.3.1 Starting Reception for the MIPI CSI-2 Input > > > > on the latest hardware manual(R01UH0914EJ0140 Rev.1.40) it is > > > > mentioned that after DPHY reset, we need to wait for 1 msec or > > > > more before start receiving data from the sensor. So add a delay > after pre_streamon(). > > > > > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > > > --- > > > > drivers/media/platform/renesas/rzg2l-cru/rzg2l-ip.c | 3 +++ > > > > 1 file changed, 3 insertions(+) > > > > > > > > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-ip.c > > > > b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-ip.c > > > > index 9f351a05893e..5468dc179de7 100644 > > > > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-ip.c > > > > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-ip.c > > > > @@ -5,6 +5,7 @@ > > > > * Copyright (C) 2022 Renesas Electronics Corp. > > > > */ > > > > > > > > +#include <linux/delay.h> > > > > #include "rzg2l-cru.h" > > > > > > > > struct rzg2l_cru_ip_format { > > > > @@ -71,6 +72,8 @@ static int rzg2l_cru_ip_s_stream(struct > > > > v4l2_subdev > > > *sd, int enable) > > > > if (ret) > > > > return ret; > > > > > > > > + usleep_range(1000, 2000); > > > > + > > > > > > What would you think of using > > > > > > fsleep(1000); > > > > > > instead ? > > > > Essentially it is same by looking at the code[1]. > > OK will use fsleep() > > Yes, it will result in the same delay. fsleep() is recommended as the > default sleep function unless there's a specific need to do something > different. OK. Thanks for the explanation. Cheers, Biju > > > [1] > > static inline void fsleep(unsigned long usecs) { > > if (usecs <= 10) > > udelay(usecs); > > else if (usecs <= 20000) > > usleep_range(usecs, 2 * usecs); > > else > > msleep(DIV_ROUND_UP(usecs, 1000)); > > } > > > > > With or without that, > > > > > > Reviewed-by: Laurent Pinchart > > > <laurent.pinchart+renesas@ideasonboard.com> > > > > > > > ret = rzg2l_cru_start_image_processing(cru); > > > > if (ret) { > > > > v4l2_subdev_call(cru->ip.remote, video, > post_streamoff); > > -- > Regards, > > Laurent Pinchart
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-ip.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-ip.c index 9f351a05893e..5468dc179de7 100644 --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-ip.c +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-ip.c @@ -5,6 +5,7 @@ * Copyright (C) 2022 Renesas Electronics Corp. */ +#include <linux/delay.h> #include "rzg2l-cru.h" struct rzg2l_cru_ip_format { @@ -71,6 +72,8 @@ static int rzg2l_cru_ip_s_stream(struct v4l2_subdev *sd, int enable) if (ret) return ret; + usleep_range(1000, 2000); + ret = rzg2l_cru_start_image_processing(cru); if (ret) { v4l2_subdev_call(cru->ip.remote, video, post_streamoff);
As per section 35.3.1 Starting Reception for the MIPI CSI-2 Input on the latest hardware manual(R01UH0914EJ0140 Rev.1.40) it is mentioned that after DPHY reset, we need to wait for 1 msec or more before start receiving data from the sensor. So add a delay after pre_streamon(). Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> --- drivers/media/platform/renesas/rzg2l-cru/rzg2l-ip.c | 3 +++ 1 file changed, 3 insertions(+)