On Fri, Oct 20, 2023 at 02:08:59PM +0100, Kieran Bingham wrote:
> Quoting Chen, Jason Z (2023-10-20 05:09:08)
> > From: Jason Chen <jason.z.chen@xxxxxxxxx>
> >
> > Due to certain MIPI hardware designs using overly long cables, there
> > is a loss of signal strength, resulting in failed signal integration.
> > The patch includes changes to adjust the driving strength in the register
> > settings for a specific project.
>
> For a specific project? Will this now affect 'all' projects using this
> sensor? Will that be adverse in anyway?
>
> Do 'short' cables still function with this patch?
>
> > Signed-off-by: Jason Chen <jason.z.chen@xxxxxxxxx>
> > ---
> > drivers/media/i2c/ov08x40.c | 17 +++++++++++++++++
> > 1 file changed, 17 insertions(+)
> >
> > diff --git a/drivers/media/i2c/ov08x40.c b/drivers/media/i2c/ov08x40.c
> > index 0b3b906ebef..4f13e23b325 100644
> > --- a/drivers/media/i2c/ov08x40.c
> > +++ b/drivers/media/i2c/ov08x40.c
> > @@ -160,6 +160,18 @@ static const struct ov08x40_reg mipi_data_rate_800mbps[] = {
> > {0x6002, 0x2e},
> > };
> >
> > +static const struct ov08x40_reg mipi_si_changed_regs[] = {
> > + {0x481b, 0x2c}, /* HS Exit: Data Tx TEOT - reducing 8ns */
> > + {0x4826, 0x42}, /* HS Entry: Data Tx TREOT - raising 8ns */
> > + {0x4829, 0x54}, /* HS Exit: Data Tx TREOT - reducing 8ns */
> > + {0x4885, 0x1f}, /* driving strength change */
>
> Are these changes 'relative' to already written values?
Looks like it's time to address MIPI PHY timings properly, by
standardizing DT/ACPI device properties to convey timing information,
and implementing helpers to parse those properties and calculate timing
parameters for drivers.
> > +};
> > +
> > +struct ov08x40_reg_list si_regs = {
> > + .regs = mipi_si_changed_regs,
> > + .num_of_regs = ARRAY_SIZE(mipi_si_changed_regs),
> > +};
> > +
> > static const struct ov08x40_reg mode_3856x2416_regs[] = {
> > {0x5000, 0x5d},
> > {0x5001, 0x20},
> > @@ -2917,6 +2929,11 @@ static int ov08x40_start_streaming(struct ov08x40 *ov08x)
> > return ret;
> > }
> >
> > + /* Apply SI change to current project */
> > + reg_list = &si_regs;
> > +
> > + ov08x40_write_reg_list(ov08x, reg_list);
> > +
>
> This looks odd. Why isn't this just:
>
> ov08x40_write_reg_list(0v08x, &si_regs);
>
> > /* Apply default values of current mode */
> > reg_list = &ov08x->cur_mode->reg_list;
> > ret = ov08x40_write_reg_list(ov08x, reg_list);
--
Regards,
Laurent Pinchart
