diff mbox series

[13/16] media: i2c: rdacm2x: Implement .init() subdev op

Message ID 20210216174146.106639-14-jacopo+renesas@jmondi.org
State New
Headers show
Series [01/16] media: i2c: rdacm20: Enable noise immunity | expand

Commit Message

Jacopo Mondi Feb. 16, 2021, 5:41 p.m. UTC
The current probe() procedure of the RDACM20 and RDACM20 performs
initialization of the serializer image sensors and increases the noise
immunity threshold as last operation, which is then compensated by the
remote deserializer by increasing the reverse channel signal amplitude
once all remotes have bound.

The probe routine is then run without noise immunity activated which
in noisy environment conditions makes the probe sequence less reliable as
the chips configuration requires a relevant amount of i2c transactions.

Break chip initialization in two:
- At probe time only configure the serializer's reverse channel with
  noise immunity activated, to reduce the number of transactions
  performed without noise immunity protection
- Move the chips initialization to the .init() core subdev operation to
  be invoked by the deserializer after the camera has probed and it has
  increased the reverse channel amplitude

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 drivers/media/i2c/rdacm20.c | 65 ++++++++++++++++++++++---------------
 drivers/media/i2c/rdacm21.c | 65 ++++++++++++++++++++++---------------
 2 files changed, 78 insertions(+), 52 deletions(-)

Comments

Geert Uytterhoeven Feb. 17, 2021, 8:17 a.m. UTC | #1
On Tue, Feb 16, 2021 at 6:41 PM Jacopo Mondi <jacopo+renesas@jmondi.org> wrote:
> The current probe() procedure of the RDACM20 and RDACM20 performs

> initialization of the serializer image sensors and increases the noise

> immunity threshold as last operation, which is then compensated by the

> remote deserializer by increasing the reverse channel signal amplitude

> once all remotes have bound.

>

> The probe routine is then run without noise immunity activated which

> in noisy environment conditions makes the probe sequence less reliable as

> the chips configuration requires a relevant amount of i2c transactions.

>

> Break chip initialization in two:

> - At probe time only configure the serializer's reverse channel with

>   noise immunity activated, to reduce the number of transactions


deactivated?

>   performed without noise immunity protection

> - Move the chips initialization to the .init() core subdev operation to

>   be invoked by the deserializer after the camera has probed and it has

>   increased the reverse channel amplitude

>

> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>


Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Kieran Bingham Feb. 18, 2021, 4:13 p.m. UTC | #2
Hi Jacopo,

On 16/02/2021 17:41, Jacopo Mondi wrote:
> The current probe() procedure of the RDACM20 and RDACM20 performs


and RDACM21?

> initialization of the serializer image sensors and increases the noise


Of the serializer 'and' image sensors ?
or perhaps just

s/serializer/serializer,/ ?

> immunity threshold as last operation, which is then compensated by the


as a last operation
or
as a final operation


> remote deserializer by increasing the reverse channel signal amplitude

> once all remotes have bound.

> 

> The probe routine is then run without noise immunity activated which

> in noisy environment conditions makes the probe sequence less reliable as

> the chips configuration requires a relevant amount of i2c transactions.


s/relevant/relatively high/ ?

> 

> Break chip initialization in two:

> - At probe time only configure the serializer's reverse channel with

>   noise immunity activated, to reduce the number of transactions

>   performed without noise immunity protection

> - Move the chips initialization to the .init() core subdev operation to

>   be invoked by the deserializer after the camera has probed and it has

>   increased the reverse channel amplitude


Is this the op you said was deprecated?


Functionally in this code, it seems fine, but as mentioned on the next
patch, I suspect it might need squashing to make sure it stays functional...

I'm not fully sure of the implications of this patch, but your tests
have reportede that this series is helping a lot with reliability so I
don't want to block it.

The code changes themselves look ok, with the following thougts:

 - If this op/methodology is deprecated it might be harder to get
   acceptance?

 - now we have _init and _initialise - should that be made more
   distinct?

 - Seeing the duplication of the MAX9271_DEFAULT_ADDR / ping again
   really makes me want to see that wrapped in the max9271.c ;-)

 - Likely needs squashed with relevant changes in max9286?

But even with those thoughts, I don't think this is necessarily wrong so:

Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>


> 

> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

> ---

>  drivers/media/i2c/rdacm20.c | 65 ++++++++++++++++++++++---------------

>  drivers/media/i2c/rdacm21.c | 65 ++++++++++++++++++++++---------------

>  2 files changed, 78 insertions(+), 52 deletions(-)

> 

> diff --git a/drivers/media/i2c/rdacm20.c b/drivers/media/i2c/rdacm20.c

> index 39e4b4241870..0632ef98eea7 100644

> --- a/drivers/media/i2c/rdacm20.c

> +++ b/drivers/media/i2c/rdacm20.c

> @@ -437,36 +437,12 @@ static int rdacm20_get_fmt(struct v4l2_subdev *sd,

>  	return 0;

>  }

>  

> -static const struct v4l2_subdev_video_ops rdacm20_video_ops = {

> -	.s_stream	= rdacm20_s_stream,

> -};

> -

> -static const struct v4l2_subdev_pad_ops rdacm20_subdev_pad_ops = {

> -	.enum_mbus_code = rdacm20_enum_mbus_code,

> -	.get_fmt	= rdacm20_get_fmt,

> -	.set_fmt	= rdacm20_get_fmt,

> -};

> -

> -static const struct v4l2_subdev_ops rdacm20_subdev_ops = {

> -	.video		= &rdacm20_video_ops,

> -	.pad		= &rdacm20_subdev_pad_ops,

> -};

> -

> -static int rdacm20_initialize(struct rdacm20_device *dev)

> +static int rdacm20_init(struct v4l2_subdev *sd, unsigned int val)

>  {

> +	struct rdacm20_device *dev = sd_to_rdacm20(sd);

>  	unsigned int i;

>  	int ret;

>  

> -	/* Verify communication with the MAX9271: ping to wakeup. */

> -	dev->serializer.client->addr = MAX9271_DEFAULT_ADDR;

> -	i2c_smbus_read_byte(dev->serializer.client);

> -	usleep_range(5000, 8000);

> -

> -	/* Serial link disabled during config as it needs a valid pixel clock. */

> -	ret = max9271_set_serial_link(&dev->serializer, false);

> -	if (ret)

> -		return ret;

> -

>  	/*

>  	 *  Ensure that we have a good link configuration before attempting to

>  	 *  identify the device.

> @@ -537,6 +513,43 @@ static int rdacm20_initialize(struct rdacm20_device *dev)

>  

>  	dev_info(dev->dev, "Identified RDACM20 camera module\n");

>  

> +	return 0;

> +}

> +

> +static const struct v4l2_subdev_core_ops rdacm20_core_ops = {

> +	.init           = rdacm20_init,

> +};

> +

> +static const struct v4l2_subdev_video_ops rdacm20_video_ops = {

> +	.s_stream	= rdacm20_s_stream,

> +};

> +

> +static const struct v4l2_subdev_pad_ops rdacm20_subdev_pad_ops = {

> +	.enum_mbus_code = rdacm20_enum_mbus_code,

> +	.get_fmt	= rdacm20_get_fmt,

> +	.set_fmt	= rdacm20_get_fmt,

> +};

> +

> +static const struct v4l2_subdev_ops rdacm20_subdev_ops = {

> +	.core		= &rdacm20_core_ops,

> +	.video		= &rdacm20_video_ops,

> +	.pad		= &rdacm20_subdev_pad_ops,

> +};

> +

> +static int rdacm20_initialize(struct rdacm20_device *dev)

> +{

> +	int ret;

> +

> +	/* Verify communication with the MAX9271: ping to wakeup. */

> +	dev->serializer.client->addr = MAX9271_DEFAULT_ADDR;

> +	i2c_smbus_read_byte(dev->serializer.client);

> +	usleep_range(5000, 8000);

> +

> +	/* Serial link disabled during config as it needs a valid pixel clock. */

> +	ret = max9271_set_serial_link(&dev->serializer, false);

> +	if (ret)

> +		return ret;

> +

>  	/*

>  	 * Set reverse channel high threshold to increase noise immunity.

>  	 *

> diff --git a/drivers/media/i2c/rdacm21.c b/drivers/media/i2c/rdacm21.c

> index c420a6b96879..80b6f16f87a8 100644

> --- a/drivers/media/i2c/rdacm21.c

> +++ b/drivers/media/i2c/rdacm21.c

> @@ -314,21 +314,6 @@ static int rdacm21_get_fmt(struct v4l2_subdev *sd,

>  	return 0;

>  }

>  

> -static const struct v4l2_subdev_video_ops rdacm21_video_ops = {

> -	.s_stream	= rdacm21_s_stream,

> -};

> -

> -static const struct v4l2_subdev_pad_ops rdacm21_subdev_pad_ops = {

> -	.enum_mbus_code = rdacm21_enum_mbus_code,

> -	.get_fmt	= rdacm21_get_fmt,

> -	.set_fmt	= rdacm21_get_fmt,

> -};

> -

> -static const struct v4l2_subdev_ops rdacm21_subdev_ops = {

> -	.video		= &rdacm21_video_ops,

> -	.pad		= &rdacm21_subdev_pad_ops,

> -};

> -

>  static int ov10640_initialize(struct rdacm21_device *dev)

>  {

>  	u8 val;

> @@ -448,20 +433,11 @@ static int ov490_initialize(struct rdacm21_device *dev)

>  	return 0;

>  }

>  

> -static int rdacm21_initialize(struct rdacm21_device *dev)

> +static int rdacm21_init(struct v4l2_subdev *sd, unsigned int val)

>  {

> +	struct rdacm21_device *dev = sd_to_rdacm21(sd);

>  	int ret;

>  

> -	/* Verify communication with the MAX9271: ping to wakeup. */

> -	dev->serializer.client->addr = MAX9271_DEFAULT_ADDR;

> -	i2c_smbus_read_byte(dev->serializer.client);

> -	usleep_range(5000, 8000);

> -

> -	/* Enable reverse channel and disable the serial link. */

> -	ret = max9271_set_serial_link(&dev->serializer, false);

> -	if (ret)

> -		return ret;

> -

>  	/* Configure I2C bus at 105Kbps speed and configure GMSL. */

>  	ret = max9271_configure_i2c(&dev->serializer,

>  				    MAX9271_I2CSLVSH_469NS_234NS |

> @@ -508,6 +484,43 @@ static int rdacm21_initialize(struct rdacm21_device *dev)

>  	if (ret)

>  		return ret;

>  

> +	return 0;

> +}

> +

> +static const struct v4l2_subdev_core_ops rdacm21_core_ops = {

> +	.init		= rdacm21_init,

> +};

> +

> +static const struct v4l2_subdev_video_ops rdacm21_video_ops = {

> +	.s_stream	= rdacm21_s_stream,

> +};

> +

> +static const struct v4l2_subdev_pad_ops rdacm21_subdev_pad_ops = {

> +	.enum_mbus_code = rdacm21_enum_mbus_code,

> +	.get_fmt	= rdacm21_get_fmt,

> +	.set_fmt	= rdacm21_get_fmt,

> +};

> +

> +static const struct v4l2_subdev_ops rdacm21_subdev_ops = {

> +	.core		= &rdacm21_core_ops,

> +	.video		= &rdacm21_video_ops,

> +	.pad		= &rdacm21_subdev_pad_ops,

> +};

> +

> +static int rdacm21_initialize(struct rdacm21_device *dev)

> +{

> +	int ret;

> +

> +	/* Verify communication with the MAX9271: ping to wakeup. */

> +	dev->serializer.client->addr = MAX9271_DEFAULT_ADDR;

> +	i2c_smbus_read_byte(dev->serializer.client);

> +	usleep_range(5000, 8000);

> +

> +	/* Enable reverse channel and disable the serial link. */

> +	ret = max9271_set_serial_link(&dev->serializer, false);

> +	if (ret)

> +		return ret;

> +

>  	/*

>  	 * Set reverse channel high threshold to increase noise immunity.

>  	 *

>
Laurent Pinchart Feb. 22, 2021, 1:52 a.m. UTC | #3
Hi Jacopo,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>


On Thu, Feb 18, 2021 at 04:13:14PM +0000, Kieran Bingham wrote:
> On 16/02/2021 17:41, Jacopo Mondi wrote:

> > The current probe() procedure of the RDACM20 and RDACM20 performs

> 

> and RDACM21?

> 

> > initialization of the serializer image sensors and increases the noise

> 

> Of the serializer 'and' image sensors ?

> or perhaps just

> 

> s/serializer/serializer,/ ?

> 

> > immunity threshold as last operation, which is then compensated by the

> 

> as a last operation

> or

> as a final operation

> 

> 

> > remote deserializer by increasing the reverse channel signal amplitude

> > once all remotes have bound.

> > 

> > The probe routine is then run without noise immunity activated which


Maybe s/then/thus/ ? I initially interpreted this as meaning the probe
routine is run after the operations listed above.

> > in noisy environment conditions makes the probe sequence less reliable as

> > the chips configuration requires a relevant amount of i2c transactions.

> 

> s/relevant/relatively high/ ?

> 

> > Break chip initialization in two:

> > - At probe time only configure the serializer's reverse channel with

> >   noise immunity activated, to reduce the number of transactions

> >   performed without noise immunity protection

> > - Move the chips initialization to the .init() core subdev operation to

> >   be invoked by the deserializer after the camera has probed and it has

> >   increased the reverse channel amplitude

> 

> Is this the op you said was deprecated?

> 

> 

> Functionally in this code, it seems fine, but as mentioned on the next

> patch, I suspect it might need squashing to make sure it stays functional...

> 

> I'm not fully sure of the implications of this patch, but your tests

> have reportede that this series is helping a lot with reliability so I

> don't want to block it.

> 

> The code changes themselves look ok, with the following thougts:

> 

>  - If this op/methodology is deprecated it might be harder to get

>    acceptance?


I think we need to propose de-deprecating the operation, which should
include a clear explanation of the valid use cases.

>  - now we have _init and _initialise - should that be made more

>    distinct?

> 

>  - Seeing the duplication of the MAX9271_DEFAULT_ADDR / ping again

>    really makes me want to see that wrapped in the max9271.c ;-)

> 

>  - Likely needs squashed with relevant changes in max9286?


I share the bisectability concerns.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>


> But even with those thoughts, I don't think this is necessarily wrong so:

> 

> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

> 

> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

> > ---

> >  drivers/media/i2c/rdacm20.c | 65 ++++++++++++++++++++++---------------

> >  drivers/media/i2c/rdacm21.c | 65 ++++++++++++++++++++++---------------

> >  2 files changed, 78 insertions(+), 52 deletions(-)

> > 

> > diff --git a/drivers/media/i2c/rdacm20.c b/drivers/media/i2c/rdacm20.c

> > index 39e4b4241870..0632ef98eea7 100644

> > --- a/drivers/media/i2c/rdacm20.c

> > +++ b/drivers/media/i2c/rdacm20.c

> > @@ -437,36 +437,12 @@ static int rdacm20_get_fmt(struct v4l2_subdev *sd,

> >  	return 0;

> >  }

> >  

> > -static const struct v4l2_subdev_video_ops rdacm20_video_ops = {

> > -	.s_stream	= rdacm20_s_stream,

> > -};

> > -

> > -static const struct v4l2_subdev_pad_ops rdacm20_subdev_pad_ops = {

> > -	.enum_mbus_code = rdacm20_enum_mbus_code,

> > -	.get_fmt	= rdacm20_get_fmt,

> > -	.set_fmt	= rdacm20_get_fmt,

> > -};

> > -

> > -static const struct v4l2_subdev_ops rdacm20_subdev_ops = {

> > -	.video		= &rdacm20_video_ops,

> > -	.pad		= &rdacm20_subdev_pad_ops,

> > -};

> > -

> > -static int rdacm20_initialize(struct rdacm20_device *dev)

> > +static int rdacm20_init(struct v4l2_subdev *sd, unsigned int val)

> >  {

> > +	struct rdacm20_device *dev = sd_to_rdacm20(sd);

> >  	unsigned int i;

> >  	int ret;

> >  

> > -	/* Verify communication with the MAX9271: ping to wakeup. */

> > -	dev->serializer.client->addr = MAX9271_DEFAULT_ADDR;

> > -	i2c_smbus_read_byte(dev->serializer.client);

> > -	usleep_range(5000, 8000);

> > -

> > -	/* Serial link disabled during config as it needs a valid pixel clock. */

> > -	ret = max9271_set_serial_link(&dev->serializer, false);

> > -	if (ret)

> > -		return ret;

> > -

> >  	/*

> >  	 *  Ensure that we have a good link configuration before attempting to

> >  	 *  identify the device.

> > @@ -537,6 +513,43 @@ static int rdacm20_initialize(struct rdacm20_device *dev)

> >  

> >  	dev_info(dev->dev, "Identified RDACM20 camera module\n");

> >  

> > +	return 0;

> > +}

> > +

> > +static const struct v4l2_subdev_core_ops rdacm20_core_ops = {

> > +	.init           = rdacm20_init,

> > +};

> > +

> > +static const struct v4l2_subdev_video_ops rdacm20_video_ops = {

> > +	.s_stream	= rdacm20_s_stream,

> > +};

> > +

> > +static const struct v4l2_subdev_pad_ops rdacm20_subdev_pad_ops = {

> > +	.enum_mbus_code = rdacm20_enum_mbus_code,

> > +	.get_fmt	= rdacm20_get_fmt,

> > +	.set_fmt	= rdacm20_get_fmt,

> > +};

> > +

> > +static const struct v4l2_subdev_ops rdacm20_subdev_ops = {

> > +	.core		= &rdacm20_core_ops,

> > +	.video		= &rdacm20_video_ops,

> > +	.pad		= &rdacm20_subdev_pad_ops,

> > +};

> > +

> > +static int rdacm20_initialize(struct rdacm20_device *dev)

> > +{

> > +	int ret;

> > +

> > +	/* Verify communication with the MAX9271: ping to wakeup. */

> > +	dev->serializer.client->addr = MAX9271_DEFAULT_ADDR;

> > +	i2c_smbus_read_byte(dev->serializer.client);

> > +	usleep_range(5000, 8000);

> > +

> > +	/* Serial link disabled during config as it needs a valid pixel clock. */

> > +	ret = max9271_set_serial_link(&dev->serializer, false);

> > +	if (ret)

> > +		return ret;

> > +

> >  	/*

> >  	 * Set reverse channel high threshold to increase noise immunity.

> >  	 *

> > diff --git a/drivers/media/i2c/rdacm21.c b/drivers/media/i2c/rdacm21.c

> > index c420a6b96879..80b6f16f87a8 100644

> > --- a/drivers/media/i2c/rdacm21.c

> > +++ b/drivers/media/i2c/rdacm21.c

> > @@ -314,21 +314,6 @@ static int rdacm21_get_fmt(struct v4l2_subdev *sd,

> >  	return 0;

> >  }

> >  

> > -static const struct v4l2_subdev_video_ops rdacm21_video_ops = {

> > -	.s_stream	= rdacm21_s_stream,

> > -};

> > -

> > -static const struct v4l2_subdev_pad_ops rdacm21_subdev_pad_ops = {

> > -	.enum_mbus_code = rdacm21_enum_mbus_code,

> > -	.get_fmt	= rdacm21_get_fmt,

> > -	.set_fmt	= rdacm21_get_fmt,

> > -};

> > -

> > -static const struct v4l2_subdev_ops rdacm21_subdev_ops = {

> > -	.video		= &rdacm21_video_ops,

> > -	.pad		= &rdacm21_subdev_pad_ops,

> > -};

> > -

> >  static int ov10640_initialize(struct rdacm21_device *dev)

> >  {

> >  	u8 val;

> > @@ -448,20 +433,11 @@ static int ov490_initialize(struct rdacm21_device *dev)

> >  	return 0;

> >  }

> >  

> > -static int rdacm21_initialize(struct rdacm21_device *dev)

> > +static int rdacm21_init(struct v4l2_subdev *sd, unsigned int val)

> >  {

> > +	struct rdacm21_device *dev = sd_to_rdacm21(sd);

> >  	int ret;

> >  

> > -	/* Verify communication with the MAX9271: ping to wakeup. */

> > -	dev->serializer.client->addr = MAX9271_DEFAULT_ADDR;

> > -	i2c_smbus_read_byte(dev->serializer.client);

> > -	usleep_range(5000, 8000);

> > -

> > -	/* Enable reverse channel and disable the serial link. */

> > -	ret = max9271_set_serial_link(&dev->serializer, false);

> > -	if (ret)

> > -		return ret;

> > -

> >  	/* Configure I2C bus at 105Kbps speed and configure GMSL. */

> >  	ret = max9271_configure_i2c(&dev->serializer,

> >  				    MAX9271_I2CSLVSH_469NS_234NS |

> > @@ -508,6 +484,43 @@ static int rdacm21_initialize(struct rdacm21_device *dev)

> >  	if (ret)

> >  		return ret;

> >  

> > +	return 0;

> > +}

> > +

> > +static const struct v4l2_subdev_core_ops rdacm21_core_ops = {

> > +	.init		= rdacm21_init,

> > +};

> > +

> > +static const struct v4l2_subdev_video_ops rdacm21_video_ops = {

> > +	.s_stream	= rdacm21_s_stream,

> > +};

> > +

> > +static const struct v4l2_subdev_pad_ops rdacm21_subdev_pad_ops = {

> > +	.enum_mbus_code = rdacm21_enum_mbus_code,

> > +	.get_fmt	= rdacm21_get_fmt,

> > +	.set_fmt	= rdacm21_get_fmt,

> > +};

> > +

> > +static const struct v4l2_subdev_ops rdacm21_subdev_ops = {

> > +	.core		= &rdacm21_core_ops,

> > +	.video		= &rdacm21_video_ops,

> > +	.pad		= &rdacm21_subdev_pad_ops,

> > +};

> > +

> > +static int rdacm21_initialize(struct rdacm21_device *dev)

> > +{

> > +	int ret;

> > +

> > +	/* Verify communication with the MAX9271: ping to wakeup. */

> > +	dev->serializer.client->addr = MAX9271_DEFAULT_ADDR;

> > +	i2c_smbus_read_byte(dev->serializer.client);

> > +	usleep_range(5000, 8000);

> > +

> > +	/* Enable reverse channel and disable the serial link. */

> > +	ret = max9271_set_serial_link(&dev->serializer, false);

> > +	if (ret)

> > +		return ret;

> > +

> >  	/*

> >  	 * Set reverse channel high threshold to increase noise immunity.

> >  	 *


-- 
Regards,

Laurent Pinchart
diff mbox series

Patch

diff --git a/drivers/media/i2c/rdacm20.c b/drivers/media/i2c/rdacm20.c
index 39e4b4241870..0632ef98eea7 100644
--- a/drivers/media/i2c/rdacm20.c
+++ b/drivers/media/i2c/rdacm20.c
@@ -437,36 +437,12 @@  static int rdacm20_get_fmt(struct v4l2_subdev *sd,
 	return 0;
 }
 
-static const struct v4l2_subdev_video_ops rdacm20_video_ops = {
-	.s_stream	= rdacm20_s_stream,
-};
-
-static const struct v4l2_subdev_pad_ops rdacm20_subdev_pad_ops = {
-	.enum_mbus_code = rdacm20_enum_mbus_code,
-	.get_fmt	= rdacm20_get_fmt,
-	.set_fmt	= rdacm20_get_fmt,
-};
-
-static const struct v4l2_subdev_ops rdacm20_subdev_ops = {
-	.video		= &rdacm20_video_ops,
-	.pad		= &rdacm20_subdev_pad_ops,
-};
-
-static int rdacm20_initialize(struct rdacm20_device *dev)
+static int rdacm20_init(struct v4l2_subdev *sd, unsigned int val)
 {
+	struct rdacm20_device *dev = sd_to_rdacm20(sd);
 	unsigned int i;
 	int ret;
 
-	/* Verify communication with the MAX9271: ping to wakeup. */
-	dev->serializer.client->addr = MAX9271_DEFAULT_ADDR;
-	i2c_smbus_read_byte(dev->serializer.client);
-	usleep_range(5000, 8000);
-
-	/* Serial link disabled during config as it needs a valid pixel clock. */
-	ret = max9271_set_serial_link(&dev->serializer, false);
-	if (ret)
-		return ret;
-
 	/*
 	 *  Ensure that we have a good link configuration before attempting to
 	 *  identify the device.
@@ -537,6 +513,43 @@  static int rdacm20_initialize(struct rdacm20_device *dev)
 
 	dev_info(dev->dev, "Identified RDACM20 camera module\n");
 
+	return 0;
+}
+
+static const struct v4l2_subdev_core_ops rdacm20_core_ops = {
+	.init           = rdacm20_init,
+};
+
+static const struct v4l2_subdev_video_ops rdacm20_video_ops = {
+	.s_stream	= rdacm20_s_stream,
+};
+
+static const struct v4l2_subdev_pad_ops rdacm20_subdev_pad_ops = {
+	.enum_mbus_code = rdacm20_enum_mbus_code,
+	.get_fmt	= rdacm20_get_fmt,
+	.set_fmt	= rdacm20_get_fmt,
+};
+
+static const struct v4l2_subdev_ops rdacm20_subdev_ops = {
+	.core		= &rdacm20_core_ops,
+	.video		= &rdacm20_video_ops,
+	.pad		= &rdacm20_subdev_pad_ops,
+};
+
+static int rdacm20_initialize(struct rdacm20_device *dev)
+{
+	int ret;
+
+	/* Verify communication with the MAX9271: ping to wakeup. */
+	dev->serializer.client->addr = MAX9271_DEFAULT_ADDR;
+	i2c_smbus_read_byte(dev->serializer.client);
+	usleep_range(5000, 8000);
+
+	/* Serial link disabled during config as it needs a valid pixel clock. */
+	ret = max9271_set_serial_link(&dev->serializer, false);
+	if (ret)
+		return ret;
+
 	/*
 	 * Set reverse channel high threshold to increase noise immunity.
 	 *
diff --git a/drivers/media/i2c/rdacm21.c b/drivers/media/i2c/rdacm21.c
index c420a6b96879..80b6f16f87a8 100644
--- a/drivers/media/i2c/rdacm21.c
+++ b/drivers/media/i2c/rdacm21.c
@@ -314,21 +314,6 @@  static int rdacm21_get_fmt(struct v4l2_subdev *sd,
 	return 0;
 }
 
-static const struct v4l2_subdev_video_ops rdacm21_video_ops = {
-	.s_stream	= rdacm21_s_stream,
-};
-
-static const struct v4l2_subdev_pad_ops rdacm21_subdev_pad_ops = {
-	.enum_mbus_code = rdacm21_enum_mbus_code,
-	.get_fmt	= rdacm21_get_fmt,
-	.set_fmt	= rdacm21_get_fmt,
-};
-
-static const struct v4l2_subdev_ops rdacm21_subdev_ops = {
-	.video		= &rdacm21_video_ops,
-	.pad		= &rdacm21_subdev_pad_ops,
-};
-
 static int ov10640_initialize(struct rdacm21_device *dev)
 {
 	u8 val;
@@ -448,20 +433,11 @@  static int ov490_initialize(struct rdacm21_device *dev)
 	return 0;
 }
 
-static int rdacm21_initialize(struct rdacm21_device *dev)
+static int rdacm21_init(struct v4l2_subdev *sd, unsigned int val)
 {
+	struct rdacm21_device *dev = sd_to_rdacm21(sd);
 	int ret;
 
-	/* Verify communication with the MAX9271: ping to wakeup. */
-	dev->serializer.client->addr = MAX9271_DEFAULT_ADDR;
-	i2c_smbus_read_byte(dev->serializer.client);
-	usleep_range(5000, 8000);
-
-	/* Enable reverse channel and disable the serial link. */
-	ret = max9271_set_serial_link(&dev->serializer, false);
-	if (ret)
-		return ret;
-
 	/* Configure I2C bus at 105Kbps speed and configure GMSL. */
 	ret = max9271_configure_i2c(&dev->serializer,
 				    MAX9271_I2CSLVSH_469NS_234NS |
@@ -508,6 +484,43 @@  static int rdacm21_initialize(struct rdacm21_device *dev)
 	if (ret)
 		return ret;
 
+	return 0;
+}
+
+static const struct v4l2_subdev_core_ops rdacm21_core_ops = {
+	.init		= rdacm21_init,
+};
+
+static const struct v4l2_subdev_video_ops rdacm21_video_ops = {
+	.s_stream	= rdacm21_s_stream,
+};
+
+static const struct v4l2_subdev_pad_ops rdacm21_subdev_pad_ops = {
+	.enum_mbus_code = rdacm21_enum_mbus_code,
+	.get_fmt	= rdacm21_get_fmt,
+	.set_fmt	= rdacm21_get_fmt,
+};
+
+static const struct v4l2_subdev_ops rdacm21_subdev_ops = {
+	.core		= &rdacm21_core_ops,
+	.video		= &rdacm21_video_ops,
+	.pad		= &rdacm21_subdev_pad_ops,
+};
+
+static int rdacm21_initialize(struct rdacm21_device *dev)
+{
+	int ret;
+
+	/* Verify communication with the MAX9271: ping to wakeup. */
+	dev->serializer.client->addr = MAX9271_DEFAULT_ADDR;
+	i2c_smbus_read_byte(dev->serializer.client);
+	usleep_range(5000, 8000);
+
+	/* Enable reverse channel and disable the serial link. */
+	ret = max9271_set_serial_link(&dev->serializer, false);
+	if (ret)
+		return ret;
+
 	/*
 	 * Set reverse channel high threshold to increase noise immunity.
 	 *