Hi Jacopo, On Sun, Jan 09, 2022 at 11:56:34AM +0100, Jacopo Mondi wrote: > On Sat, Jan 01, 2022 at 08:28:05PM +0200, Laurent Pinchart wrote: > > The GMSL serial data bus width is normally selected through the BWS pin. > > On some systems, the pin may not be wired to the correct value. Support > > overriding the bus width by software, using the value specified in the > > device tree. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx> > > --- > > drivers/media/i2c/max9286.c | 36 ++++++++++++++++++++++++++++++++++++ > > 1 file changed, 36 insertions(+) > > > > diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c > > index d88a4d8e63ab..07ebb01640a1 100644 > > --- a/drivers/media/i2c/max9286.c > > +++ b/drivers/media/i2c/max9286.c > > @@ -90,6 +90,11 @@ > > /* Register 0x1b */ > > #define MAX9286_SWITCHIN(n) (1 << ((n) + 4)) > > #define MAX9286_ENEQ(n) (1 << (n)) > > +/* Register 0x1c */ > > +#define MAX9286_HIGHIMM(n) BIT((n) + 4) > > +#define MAX9286_I2CSEL BIT(2) > > +#define MAX9286_HIBW BIT(1) > > +#define MAX9286_BWS BIT(0) > > /* Register 0x27 */ > > #define MAX9286_LOCKED BIT(7) > > /* Register 0x31 */ > > @@ -182,6 +187,7 @@ struct max9286_priv { > > u32 init_rev_chan_mv; > > u32 rev_chan_mv; > > u8 i2c_mstbt; > > + u32 bus_width; > > > > struct v4l2_ctrl_handler ctrls; > > struct v4l2_ctrl *pixelrate_ctrl; > > @@ -1159,6 +1165,23 @@ static int max9286_setup(struct max9286_priv *priv) > > max9286_set_video_format(priv, &max9286_default_format); > > max9286_set_fsync_period(priv); > > > > + if (priv->bus_width) { > > + int val; > > + > > + val = max9286_read(priv, 0x1c); > > + if (val < 0) > > + return val; > > + > > + val &= ~(MAX9286_HIBW | MAX9286_BWS); > > + > > + if (priv->bus_width == 27) > > + val |= MAX9286_HIBW; > > + else if (priv->bus_width == 32) > > + val |= MAX9286_BWS; > > + > > + max9286_write(priv, 0x1c, val); > > + } > > + > > /* > > * The overlap window seems to provide additional validation by tracking > > * the delay between vsync and frame sync, generating an error if the > > @@ -1429,6 +1452,19 @@ static int max9286_parse_dt(struct max9286_priv *priv) > > } > > of_node_put(node); > > > > + of_property_read_u32(dev->of_node, "maxim,bus-width", &priv->bus_width); > > + switch (priv->bus_width) { > > + case 0: > > Can you add a comment that in this case we rely on HW configuration ? > Looking at this with the above function in the same patch makes it > clear, but it might get lost when merged. I'll add /* * The property isn't specified in the device tree, the driver * will keep the default value selected by the BWS pin. */ > Reviewed-by: Jacopo Mondi <jacopo+renesas@xxxxxxxxxx> > > > + case 24: > > + case 27: > > + case 32: > > + break; > > + default: > > + dev_err(dev, "Invalid %s value %u\n", "maxim,bus-width", > > + priv->bus_width); > > + return -EINVAL; > > + } > > + > > of_property_read_u32(dev->of_node, "maxim,i2c-clock-frequency", > > &i2c_clk_freq); > > for (i = 0; i < ARRAY_SIZE(max9286_i2c_speeds); ++i) { -- Regards, Laurent Pinchart