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
