media: staging: max96712: Add support for MAX96724

Message ID 20240527133410.1690169-1-niklas.soderlund+renesas@ragnatech.se (mailing list archive)
State New
Headers
Series media: staging: max96712: Add support for MAX96724 |

Commit Message

Niklas Söderlund May 27, 2024, 1:34 p.m. UTC
  The MAX96724 is almost identical to the MAX96712 and can be supported by
the same driver, add support for it.

For the staging driver which only supports patter generation the big
difference is that the datasheet (rev 4) for MAX96724 do not describe
the DEBUG_EXTRA register, which is at offset 0x0009 on MAX96712. It's
not clear if this register is removed or moved to a different offset.
What is known is writing to register 0x0009 have no effect on MAX96724.

This makes it impossible to increase the test pattern clock frequency
from 25 MHz to 75Mhz on MAX96724. To be able to get a stable test
pattern the DPLL frequency have to be increase instead to compensate for
this. The frequency selected is found by experimentation as the MAX96724
datasheet is much sparser then what's available for MAX96712.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/staging/media/max96712/max96712.c | 26 ++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)
  

Comments

Julien Massot May 28, 2024, 8:04 a.m. UTC | #1
Hi Niklas,

On 5/27/24 3:34 PM, Niklas Söderlund wrote:
> The MAX96724 is almost identical to the MAX96712 and can be supported by
> the same driver, add support for it.
> 
> For the staging driver which only supports patter generation the big
> difference is that the datasheet (rev 4) for MAX96724 do not describe
> the DEBUG_EXTRA register, which is at offset 0x0009 on MAX96712. It's
> not clear if this register is removed or moved to a different offset.
> What is known is writing to register 0x0009 have no effect on MAX96724.
> 
> This makes it impossible to increase the test pattern clock frequency
> from 25 MHz to 75Mhz on MAX96724. To be able to get a stable test
> pattern the DPLL frequency have to be increase instead to compensate for
> this. The frequency selected is found by experimentation as the MAX96724
> datasheet is much sparser then what's available for MAX96712.

There is a specific User Guide for this chip[1] (under NDA) which 
describes the test pattern
clock frequency.

| Debug Extra 0x9 [1:0] | PATGEN_CLK_SRC (0x1dc [7]) | PCLK Frequency |
|                       |       Pipe 0               |                |
|-----------------------|----------------------------|----------------|
| 00                    | x                          | 25  MHz        |
| 01                    | x                          | 75  MHz        |
| 1x                    | 0                          | 150 MHz        |
| 1x                    | 1 (default)                | 375 MHz        |


PATGEN_CLK_SRC
Pipe 0 0x1dc
Pipe 1 0x1fc
Pipe 2 0x21c
Pipe 3 0x23c


The document also mention that "This internal Pclk is NOT related to the 
MIPI CSI
port clock rate" so increasing the dpll should not increase the pattern 
generation
clock.

Perhaps increasing the DPLL allows to transmit more data on the CSI port 
because the pattern
generator is running at a higher clock rate than what we expect.

Best regards,
Julien

[1]: GMSL2_Customers_MAX96724 User Guide (rev2)
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
>   drivers/staging/media/max96712/max96712.c | 26 ++++++++++++++++++-----
>   1 file changed, 21 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/staging/media/max96712/max96712.c b/drivers/staging/media/max96712/max96712.c
> index ea67bcf69c9d..69a0a6a16cf9 100644
> --- a/drivers/staging/media/max96712/max96712.c
> +++ b/drivers/staging/media/max96712/max96712.c
> @@ -17,8 +17,10 @@
>   #include <media/v4l2-subdev.h>
>   
>   #define MAX96712_ID 0x20
> +#define MAX96724_ID 0xA7
>   
>   #define MAX96712_DPLL_FREQ 1000
> +#define MAX96724_DPLL_FREQ 1200
>   
>   enum max96712_pattern {
>   	MAX96712_PATTERN_CHECKERBOARD = 0,
> @@ -31,6 +33,7 @@ struct max96712_priv {
>   	struct gpio_desc *gpiod_pwdn;
>   
>   	bool cphy;
> +	bool max96724;
>   	struct v4l2_mbus_config_mipi_csi2 mipi;
>   
>   	struct v4l2_subdev sd;
> @@ -120,6 +123,7 @@ static void max96712_mipi_enable(struct max96712_priv *priv, bool enable)
>   
>   static void max96712_mipi_configure(struct max96712_priv *priv)
>   {
> +	unsigned int dpll_freq;
>   	unsigned int i;
>   	u8 phy5 = 0;
>   
> @@ -152,10 +156,11 @@ static void max96712_mipi_configure(struct max96712_priv *priv)
>   	max96712_write(priv, 0x8a5, phy5);
>   
>   	/* Set link frequency for PHY0 and PHY1. */
> +	dpll_freq = priv->max96724 ? MAX96724_DPLL_FREQ : MAX96712_DPLL_FREQ;
>   	max96712_update_bits(priv, 0x415, 0x3f,
> -			     ((MAX96712_DPLL_FREQ / 100) & 0x1f) | BIT(5));
> +			     ((dpll_freq / 100) & 0x1f) | BIT(5));
>   	max96712_update_bits(priv, 0x418, 0x3f,
> -			     ((MAX96712_DPLL_FREQ / 100) & 0x1f) | BIT(5));
> +			     ((dpll_freq / 100) & 0x1f) | BIT(5));
>   
>   	/* Enable PHY0 and PHY1 */
>   	max96712_update_bits(priv, 0x8a2, 0xf0, 0x30);
> @@ -181,7 +186,8 @@ static void max96712_pattern_enable(struct max96712_priv *priv, bool enable)
>   	}
>   
>   	/* PCLK 75MHz. */
> -	max96712_write(priv, 0x0009, 0x01);
> +	if (!priv->max96724)
> +		max96712_write(priv, 0x0009, 0x01);
>   
>   	/* Configure Video Timing Generator for 1920x1080 @ 30 fps. */
>   	max96712_write_bulk_value(priv, 0x1052, 0, 3);
> @@ -290,6 +296,7 @@ static const struct v4l2_ctrl_ops max96712_ctrl_ops = {
>   
>   static int max96712_v4l2_register(struct max96712_priv *priv)
>   {
> +	unsigned int dpll_freq;
>   	long pixel_rate;
>   	int ret;
>   
> @@ -303,7 +310,8 @@ static int max96712_v4l2_register(struct max96712_priv *priv)
>   	 * TODO: Once V4L2_CID_LINK_FREQ is changed from a menu control to an
>   	 * INT64 control it should be used here instead of V4L2_CID_PIXEL_RATE.
>   	 */
> -	pixel_rate = MAX96712_DPLL_FREQ / priv->mipi.num_data_lanes * 1000000;
> +	dpll_freq = priv->max96724 ? MAX96724_DPLL_FREQ : MAX96712_DPLL_FREQ;
> +	pixel_rate = dpll_freq / priv->mipi.num_data_lanes * 1000000;
>   	v4l2_ctrl_new_std(&priv->ctrl_handler, NULL, V4L2_CID_PIXEL_RATE,
>   			  pixel_rate, pixel_rate, 1, pixel_rate);
>   
> @@ -419,8 +427,15 @@ static int max96712_probe(struct i2c_client *client)
>   	if (priv->gpiod_pwdn)
>   		usleep_range(4000, 5000);
>   
> -	if (max96712_read(priv, 0x4a) != MAX96712_ID)
> +	switch (max96712_read(priv, 0x4a)) {
> +	case MAX96712_ID:
> +		break;
> +	case MAX96724_ID:
> +		priv->max96724 = true;
> +		break;
> +	default:
>   		return -ENODEV;
> +	}
>   
>   	max96712_reset(priv);
>   
> @@ -444,6 +459,7 @@ static void max96712_remove(struct i2c_client *client)
>   
>   static const struct of_device_id max96712_of_table[] = {
>   	{ .compatible = "maxim,max96712" },
> +	{ .compatible = "maxim,max96724" },
>   	{ /* sentinel */ },
>   };
>   MODULE_DEVICE_TABLE(of, max96712_of_table);
  
Niklas Söderlund May 28, 2024, 9:37 a.m. UTC | #2
Hi Julien,

On 2024-05-28 10:04:37 +0200, Julien Massot wrote:
> Hi Niklas,
> 
> On 5/27/24 3:34 PM, Niklas Söderlund wrote:
> > The MAX96724 is almost identical to the MAX96712 and can be supported by
> > the same driver, add support for it.
> > 
> > For the staging driver which only supports patter generation the big
> > difference is that the datasheet (rev 4) for MAX96724 do not describe
> > the DEBUG_EXTRA register, which is at offset 0x0009 on MAX96712. It's
> > not clear if this register is removed or moved to a different offset.
> > What is known is writing to register 0x0009 have no effect on MAX96724.
> > 
> > This makes it impossible to increase the test pattern clock frequency
> > from 25 MHz to 75Mhz on MAX96724. To be able to get a stable test
> > pattern the DPLL frequency have to be increase instead to compensate for
> > this. The frequency selected is found by experimentation as the MAX96724
> > datasheet is much sparser then what's available for MAX96712.
> 
> There is a specific User Guide for this chip[1] (under NDA) which describes
> the test pattern
> clock frequency.
> 
> | Debug Extra 0x9 [1:0] | PATGEN_CLK_SRC (0x1dc [7]) | PCLK Frequency |
> |                       |       Pipe 0               |                |
> |-----------------------|----------------------------|----------------|
> | 00                    | x                          | 25  MHz        |
> | 01                    | x                          | 75  MHz        |
> | 1x                    | 0                          | 150 MHz        |
> | 1x                    | 1 (default)                | 375 MHz        |

The same table exists in the MAX96712 users guide, which I do have.

The issue is that the datasheet for MAX96724 I found online do not list 
the Debug Extra (0x9) register that is present on MAX96712. If you have 
access to the full MAX96724 documentation could you check if it contains 
the Debug Extra register and at which offset it is?

I if treat MAX96724 as it is documented for MAX96712 I get an 
unstable/distorted test pattern on MAX96724.

> 
> 
> PATGEN_CLK_SRC
> Pipe 0 0x1dc
> Pipe 1 0x1fc
> Pipe 2 0x21c
> Pipe 3 0x23c
> 
> 
> The document also mention that "This internal Pclk is NOT related to the
> MIPI CSI
> port clock rate" so increasing the dpll should not increase the pattern
> generation
> clock.
> 
> Perhaps increasing the DPLL allows to transmit more data on the CSI port
> because the pattern
> generator is running at a higher clock rate than what we expect.

That is possible. I only have the limited register information found 
online for MAX96724, so I can't compare the setup with MAX96712. But my 
suspicion is that the test pattern clock is running slower on MAX96724 
without the DEBUG_EXTRA write.

For MAX96712 the reset default value for DEBUG_EXTRA is 0x00, which if 
this is also true for MAX96724 means the test pattern clock is running 
at 25Mhz. 

This fits with my observations. As without an increased DPLL I do get 
frames, most only contain zero or a few line. But some contains 80+% of 
the image. There is little distortion in color on each line, but there 
are lines missing.

For example on the check pattern generation. The square "corners" lines 
up perfectly, and the squares are the correct width, but not the correct 
height, most of them are too short. I'm just happy I found a way to 
generate a stable pattern on both devices from the documentation I have.

This driver is in staging as it only supports pattern generation and not 
the GMSL side. My use-case is to have a video source to test the video 
capture pipeline on Renesas SoCs where both MAX96712 and MAX96724 are in 
use.

> 
> Best regards,
> Julien
> 
> [1]: GMSL2_Customers_MAX96724 User Guide (rev2)
> > 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > ---
> >   drivers/staging/media/max96712/max96712.c | 26 ++++++++++++++++++-----
> >   1 file changed, 21 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/staging/media/max96712/max96712.c b/drivers/staging/media/max96712/max96712.c
> > index ea67bcf69c9d..69a0a6a16cf9 100644
> > --- a/drivers/staging/media/max96712/max96712.c
> > +++ b/drivers/staging/media/max96712/max96712.c
> > @@ -17,8 +17,10 @@
> >   #include <media/v4l2-subdev.h>
> >   #define MAX96712_ID 0x20
> > +#define MAX96724_ID 0xA7
> >   #define MAX96712_DPLL_FREQ 1000
> > +#define MAX96724_DPLL_FREQ 1200
> >   enum max96712_pattern {
> >   	MAX96712_PATTERN_CHECKERBOARD = 0,
> > @@ -31,6 +33,7 @@ struct max96712_priv {
> >   	struct gpio_desc *gpiod_pwdn;
> >   	bool cphy;
> > +	bool max96724;
> >   	struct v4l2_mbus_config_mipi_csi2 mipi;
> >   	struct v4l2_subdev sd;
> > @@ -120,6 +123,7 @@ static void max96712_mipi_enable(struct max96712_priv *priv, bool enable)
> >   static void max96712_mipi_configure(struct max96712_priv *priv)
> >   {
> > +	unsigned int dpll_freq;
> >   	unsigned int i;
> >   	u8 phy5 = 0;
> > @@ -152,10 +156,11 @@ static void max96712_mipi_configure(struct max96712_priv *priv)
> >   	max96712_write(priv, 0x8a5, phy5);
> >   	/* Set link frequency for PHY0 and PHY1. */
> > +	dpll_freq = priv->max96724 ? MAX96724_DPLL_FREQ : MAX96712_DPLL_FREQ;
> >   	max96712_update_bits(priv, 0x415, 0x3f,
> > -			     ((MAX96712_DPLL_FREQ / 100) & 0x1f) | BIT(5));
> > +			     ((dpll_freq / 100) & 0x1f) | BIT(5));
> >   	max96712_update_bits(priv, 0x418, 0x3f,
> > -			     ((MAX96712_DPLL_FREQ / 100) & 0x1f) | BIT(5));
> > +			     ((dpll_freq / 100) & 0x1f) | BIT(5));
> >   	/* Enable PHY0 and PHY1 */
> >   	max96712_update_bits(priv, 0x8a2, 0xf0, 0x30);
> > @@ -181,7 +186,8 @@ static void max96712_pattern_enable(struct max96712_priv *priv, bool enable)
> >   	}
> >   	/* PCLK 75MHz. */
> > -	max96712_write(priv, 0x0009, 0x01);
> > +	if (!priv->max96724)
> > +		max96712_write(priv, 0x0009, 0x01);
> >   	/* Configure Video Timing Generator for 1920x1080 @ 30 fps. */
> >   	max96712_write_bulk_value(priv, 0x1052, 0, 3);
> > @@ -290,6 +296,7 @@ static const struct v4l2_ctrl_ops max96712_ctrl_ops = {
> >   static int max96712_v4l2_register(struct max96712_priv *priv)
> >   {
> > +	unsigned int dpll_freq;
> >   	long pixel_rate;
> >   	int ret;
> > @@ -303,7 +310,8 @@ static int max96712_v4l2_register(struct max96712_priv *priv)
> >   	 * TODO: Once V4L2_CID_LINK_FREQ is changed from a menu control to an
> >   	 * INT64 control it should be used here instead of V4L2_CID_PIXEL_RATE.
> >   	 */
> > -	pixel_rate = MAX96712_DPLL_FREQ / priv->mipi.num_data_lanes * 1000000;
> > +	dpll_freq = priv->max96724 ? MAX96724_DPLL_FREQ : MAX96712_DPLL_FREQ;
> > +	pixel_rate = dpll_freq / priv->mipi.num_data_lanes * 1000000;
> >   	v4l2_ctrl_new_std(&priv->ctrl_handler, NULL, V4L2_CID_PIXEL_RATE,
> >   			  pixel_rate, pixel_rate, 1, pixel_rate);
> > @@ -419,8 +427,15 @@ static int max96712_probe(struct i2c_client *client)
> >   	if (priv->gpiod_pwdn)
> >   		usleep_range(4000, 5000);
> > -	if (max96712_read(priv, 0x4a) != MAX96712_ID)
> > +	switch (max96712_read(priv, 0x4a)) {
> > +	case MAX96712_ID:
> > +		break;
> > +	case MAX96724_ID:
> > +		priv->max96724 = true;
> > +		break;
> > +	default:
> >   		return -ENODEV;
> > +	}
> >   	max96712_reset(priv);
> > @@ -444,6 +459,7 @@ static void max96712_remove(struct i2c_client *client)
> >   static const struct of_device_id max96712_of_table[] = {
> >   	{ .compatible = "maxim,max96712" },
> > +	{ .compatible = "maxim,max96724" },
> >   	{ /* sentinel */ },
> >   };
> >   MODULE_DEVICE_TABLE(of, max96712_of_table);
> 
> -- 
> Julien Massot
> Senior Software Engineer
> Collabora Ltd.
> Platinum Building, St John's Innovation Park, Cambridge CB4 0DS, UK
> Registered in England & Wales, no. 5513718
  
Julien Massot May 28, 2024, 12:04 p.m. UTC | #3
On 5/28/24 11:37 AM, Niklas Söderlund wrote:
> Hi Julien,
> 
> On 2024-05-28 10:04:37 +0200, Julien Massot wrote:
>> Hi Niklas,
>>
>> On 5/27/24 3:34 PM, Niklas Söderlund wrote:
>>> The MAX96724 is almost identical to the MAX96712 and can be supported by
>>> the same driver, add support for it.
>>>
>>> For the staging driver which only supports patter generation the big
>>> difference is that the datasheet (rev 4) for MAX96724 do not describe
>>> the DEBUG_EXTRA register, which is at offset 0x0009 on MAX96712. It's
>>> not clear if this register is removed or moved to a different offset.
>>> What is known is writing to register 0x0009 have no effect on MAX96724.
>>>
>>> This makes it impossible to increase the test pattern clock frequency
>>> from 25 MHz to 75Mhz on MAX96724. To be able to get a stable test
>>> pattern the DPLL frequency have to be increase instead to compensate for
>>> this. The frequency selected is found by experimentation as the MAX96724
>>> datasheet is much sparser then what's available for MAX96712.
>>
>> There is a specific User Guide for this chip[1] (under NDA) which describes
>> the test pattern
>> clock frequency.
>>
>> | Debug Extra 0x9 [1:0] | PATGEN_CLK_SRC (0x1dc [7]) | PCLK Frequency |
>> |                       |       Pipe 0               |                |
>> |-----------------------|----------------------------|----------------|
>> | 00                    | x                          | 25  MHz        |
>> | 01                    | x                          | 75  MHz        |
>> | 1x                    | 0                          | 150 MHz        |
>> | 1x                    | 1 (default)                | 375 MHz        |
> 
> The same table exists in the MAX96712 users guide, which I do have.
> 
> The issue is that the datasheet for MAX96724 I found online do not list
> the Debug Extra (0x9) register that is present on MAX96712. If you have
> access to the full MAX96724 documentation could you check if it contains
> the Debug Extra register and at which offset it is?
No the Debug Extra register is not listed in the datasheet (rev4).
Only in the User Guide and the Windows GUI.
> 
> I if treat MAX96724 as it is documented for MAX96712 I get an
> unstable/distorted test pattern on MAX96724.
Ok better to not write this register then :) there might be a reason 
this register disappeared
from the datasheet.
> 
>>
>>
>> PATGEN_CLK_SRC
>> Pipe 0 0x1dc
>> Pipe 1 0x1fc
>> Pipe 2 0x21c
>> Pipe 3 0x23c
>>
>>
>> The document also mention that "This internal Pclk is NOT related to the
>> MIPI CSI
>> port clock rate" so increasing the dpll should not increase the pattern
>> generation
>> clock.
>>
>> Perhaps increasing the DPLL allows to transmit more data on the CSI port
>> because the pattern
>> generator is running at a higher clock rate than what we expect.
> 
> That is possible. I only have the limited register information found
> online for MAX96724, so I can't compare the setup with MAX96712. But my
> suspicion is that the test pattern clock is running slower on MAX96724
> without the DEBUG_EXTRA write.
> 
> For MAX96712 the reset default value for DEBUG_EXTRA is 0x00, which if
> this is also true for MAX96724 means the test pattern clock is running
> at 25Mhz.
> 
> This fits with my observations. As without an increased DPLL I do get
> frames, most only contain zero or a few line. But some contains 80+% of
> the image. There is little distortion in color on each line, but there
> are lines missing.
> 
> For example on the check pattern generation. The square "corners" lines
> up perfectly, and the squares are the correct width, but not the correct
> height, most of them are too short. I'm just happy I found a way to
> generate a stable pattern on both devices from the documentation I have.
> 
> This driver is in staging as it only supports pattern generation and not
> the GMSL side. My use-case is to have a video source to test the video
> capture pipeline on Renesas SoCs where both MAX96712 and MAX96724 are in
> use.
> 
>>
>> Best regards,
>> Julien
>>
>> [1]: GMSL2_Customers_MAX96724 User Guide (rev2)
>>>
>>> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
>>> ---
>>>    drivers/staging/media/max96712/max96712.c | 26 ++++++++++++++++++-----
>>>    1 file changed, 21 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/staging/media/max96712/max96712.c b/drivers/staging/media/max96712/max96712.c
>>> index ea67bcf69c9d..69a0a6a16cf9 100644
>>> --- a/drivers/staging/media/max96712/max96712.c
>>> +++ b/drivers/staging/media/max96712/max96712.c
>>> @@ -17,8 +17,10 @@
>>>    #include <media/v4l2-subdev.h>
>>>    #define MAX96712_ID 0x20
>>> +#define MAX96724_ID 0xA7
These identifiers seems incorrect
#define MAX96712_ID 0xA0 (also valid for MAX96712B)
#define MAX96724_ID 0xA2
#define MAX96724F_ID 0xA3
#define MAX96724R_ID 0xA4


>>>    #define MAX96712_DPLL_FREQ 1000
>>> +#define MAX96724_DPLL_FREQ 1200
>>>    enum max96712_pattern {
>>>    	MAX96712_PATTERN_CHECKERBOARD = 0,
>>> @@ -31,6 +33,7 @@ struct max96712_priv {
>>>    	struct gpio_desc *gpiod_pwdn;
>>>    	bool cphy;
>>> +	bool max96724;
>>>    	struct v4l2_mbus_config_mipi_csi2 mipi;
>>>    	struct v4l2_subdev sd;
>>> @@ -120,6 +123,7 @@ static void max96712_mipi_enable(struct max96712_priv *priv, bool enable)
>>>    static void max96712_mipi_configure(struct max96712_priv *priv)
>>>    {
>>> +	unsigned int dpll_freq;
>>>    	unsigned int i;
>>>    	u8 phy5 = 0;
>>> @@ -152,10 +156,11 @@ static void max96712_mipi_configure(struct max96712_priv *priv)
>>>    	max96712_write(priv, 0x8a5, phy5);
>>>    	/* Set link frequency for PHY0 and PHY1. */
>>> +	dpll_freq = priv->max96724 ? MAX96724_DPLL_FREQ : MAX96712_DPLL_FREQ;
>>>    	max96712_update_bits(priv, 0x415, 0x3f,
>>> -			     ((MAX96712_DPLL_FREQ / 100) & 0x1f) | BIT(5));
>>> +			     ((dpll_freq / 100) & 0x1f) | BIT(5));
>>>    	max96712_update_bits(priv, 0x418, 0x3f,
>>> -			     ((MAX96712_DPLL_FREQ / 100) & 0x1f) | BIT(5));
>>> +			     ((dpll_freq / 100) & 0x1f) | BIT(5));
>>>    	/* Enable PHY0 and PHY1 */
>>>    	max96712_update_bits(priv, 0x8a2, 0xf0, 0x30);
>>> @@ -181,7 +186,8 @@ static void max96712_pattern_enable(struct max96712_priv *priv, bool enable)
>>>    	}
>>>    	/* PCLK 75MHz. */
>>> -	max96712_write(priv, 0x0009, 0x01);
>>> +	if (!priv->max96724)
>>> +		max96712_write(priv, 0x0009, 0x01);
>>>    	/* Configure Video Timing Generator for 1920x1080 @ 30 fps. */
>>>    	max96712_write_bulk_value(priv, 0x1052, 0, 3);
>>> @@ -290,6 +296,7 @@ static const struct v4l2_ctrl_ops max96712_ctrl_ops = {
>>>    static int max96712_v4l2_register(struct max96712_priv *priv)
>>>    {
>>> +	unsigned int dpll_freq;
>>>    	long pixel_rate;
>>>    	int ret;
>>> @@ -303,7 +310,8 @@ static int max96712_v4l2_register(struct max96712_priv *priv)
>>>    	 * TODO: Once V4L2_CID_LINK_FREQ is changed from a menu control to an
>>>    	 * INT64 control it should be used here instead of V4L2_CID_PIXEL_RATE.
>>>    	 */
>>> -	pixel_rate = MAX96712_DPLL_FREQ / priv->mipi.num_data_lanes * 1000000;
>>> +	dpll_freq = priv->max96724 ? MAX96724_DPLL_FREQ : MAX96712_DPLL_FREQ;
>>> +	pixel_rate = dpll_freq / priv->mipi.num_data_lanes * 1000000;
>>>    	v4l2_ctrl_new_std(&priv->ctrl_handler, NULL, V4L2_CID_PIXEL_RATE,
>>>    			  pixel_rate, pixel_rate, 1, pixel_rate);
>>> @@ -419,8 +427,15 @@ static int max96712_probe(struct i2c_client *client)
>>>    	if (priv->gpiod_pwdn)
>>>    		usleep_range(4000, 5000);
>>> -	if (max96712_read(priv, 0x4a) != MAX96712_ID)
>>> +	switch (max96712_read(priv, 0x4a)) {
0x4a is a VDDCMP register you should read 0xd instead (DEV_ID)
switch (max96712_read(priv, 0xd)) {

>>> +	case MAX96712_ID:
>>> +		break;
>>> +	case MAX96724_ID:
>>> +		priv->max96724 = true;
>>> +		break;
>>> +	default:
>>>    		return -ENODEV;
>>> +	}
>>>    	max96712_reset(priv);
>>> @@ -444,6 +459,7 @@ static void max96712_remove(struct i2c_client *client)
>>>    static const struct of_device_id max96712_of_table[] = {
>>>    	{ .compatible = "maxim,max96712" },
>>> +	{ .compatible = "maxim,max96724" },
>>>    	{ /* sentinel */ },
>>>    };
>>>    MODULE_DEVICE_TABLE(of, max96712_of_table);
>>
>> -- 
Best Regards,
Julien
  

Patch

diff --git a/drivers/staging/media/max96712/max96712.c b/drivers/staging/media/max96712/max96712.c
index ea67bcf69c9d..69a0a6a16cf9 100644
--- a/drivers/staging/media/max96712/max96712.c
+++ b/drivers/staging/media/max96712/max96712.c
@@ -17,8 +17,10 @@ 
 #include <media/v4l2-subdev.h>
 
 #define MAX96712_ID 0x20
+#define MAX96724_ID 0xA7
 
 #define MAX96712_DPLL_FREQ 1000
+#define MAX96724_DPLL_FREQ 1200
 
 enum max96712_pattern {
 	MAX96712_PATTERN_CHECKERBOARD = 0,
@@ -31,6 +33,7 @@  struct max96712_priv {
 	struct gpio_desc *gpiod_pwdn;
 
 	bool cphy;
+	bool max96724;
 	struct v4l2_mbus_config_mipi_csi2 mipi;
 
 	struct v4l2_subdev sd;
@@ -120,6 +123,7 @@  static void max96712_mipi_enable(struct max96712_priv *priv, bool enable)
 
 static void max96712_mipi_configure(struct max96712_priv *priv)
 {
+	unsigned int dpll_freq;
 	unsigned int i;
 	u8 phy5 = 0;
 
@@ -152,10 +156,11 @@  static void max96712_mipi_configure(struct max96712_priv *priv)
 	max96712_write(priv, 0x8a5, phy5);
 
 	/* Set link frequency for PHY0 and PHY1. */
+	dpll_freq = priv->max96724 ? MAX96724_DPLL_FREQ : MAX96712_DPLL_FREQ;
 	max96712_update_bits(priv, 0x415, 0x3f,
-			     ((MAX96712_DPLL_FREQ / 100) & 0x1f) | BIT(5));
+			     ((dpll_freq / 100) & 0x1f) | BIT(5));
 	max96712_update_bits(priv, 0x418, 0x3f,
-			     ((MAX96712_DPLL_FREQ / 100) & 0x1f) | BIT(5));
+			     ((dpll_freq / 100) & 0x1f) | BIT(5));
 
 	/* Enable PHY0 and PHY1 */
 	max96712_update_bits(priv, 0x8a2, 0xf0, 0x30);
@@ -181,7 +186,8 @@  static void max96712_pattern_enable(struct max96712_priv *priv, bool enable)
 	}
 
 	/* PCLK 75MHz. */
-	max96712_write(priv, 0x0009, 0x01);
+	if (!priv->max96724)
+		max96712_write(priv, 0x0009, 0x01);
 
 	/* Configure Video Timing Generator for 1920x1080 @ 30 fps. */
 	max96712_write_bulk_value(priv, 0x1052, 0, 3);
@@ -290,6 +296,7 @@  static const struct v4l2_ctrl_ops max96712_ctrl_ops = {
 
 static int max96712_v4l2_register(struct max96712_priv *priv)
 {
+	unsigned int dpll_freq;
 	long pixel_rate;
 	int ret;
 
@@ -303,7 +310,8 @@  static int max96712_v4l2_register(struct max96712_priv *priv)
 	 * TODO: Once V4L2_CID_LINK_FREQ is changed from a menu control to an
 	 * INT64 control it should be used here instead of V4L2_CID_PIXEL_RATE.
 	 */
-	pixel_rate = MAX96712_DPLL_FREQ / priv->mipi.num_data_lanes * 1000000;
+	dpll_freq = priv->max96724 ? MAX96724_DPLL_FREQ : MAX96712_DPLL_FREQ;
+	pixel_rate = dpll_freq / priv->mipi.num_data_lanes * 1000000;
 	v4l2_ctrl_new_std(&priv->ctrl_handler, NULL, V4L2_CID_PIXEL_RATE,
 			  pixel_rate, pixel_rate, 1, pixel_rate);
 
@@ -419,8 +427,15 @@  static int max96712_probe(struct i2c_client *client)
 	if (priv->gpiod_pwdn)
 		usleep_range(4000, 5000);
 
-	if (max96712_read(priv, 0x4a) != MAX96712_ID)
+	switch (max96712_read(priv, 0x4a)) {
+	case MAX96712_ID:
+		break;
+	case MAX96724_ID:
+		priv->max96724 = true;
+		break;
+	default:
 		return -ENODEV;
+	}
 
 	max96712_reset(priv);
 
@@ -444,6 +459,7 @@  static void max96712_remove(struct i2c_client *client)
 
 static const struct of_device_id max96712_of_table[] = {
 	{ .compatible = "maxim,max96712" },
+	{ .compatible = "maxim,max96724" },
 	{ /* sentinel */ },
 };
 MODULE_DEVICE_TABLE(of, max96712_of_table);