Hi Laurent
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.
Reviewed-by: Jacopo Mondi <jacopo+renesas@xxxxxxxxxx>
Thanks
j
> + 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
>
