Re: [PATCH v2 10/11] media: i2c: max9286: Configure bus width from device tree

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux