diff mbox series

[2/2] media: i2c: imx412: Add imx577 compatible string

Message ID 20220607134057.2427663-3-bryan.odonoghue@linaro.org (mailing list archive)
State Superseded
Headers
Series Add imx577 compatible to imx412 |

Commit Message

Bryan O'Donoghue June 7, 2022, 1:40 p.m. UTC 
  The Sony IMX577 uses the same silicon enabling reference code from Sony in
the available examples provided.

Add an imx577 compatible string and re-use the existing imx412 code.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/media/i2c/imx412.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Kieran Bingham June 14, 2022, 7:47 a.m. UTC | #1 
Hi Bryan,

Quoting Bryan O'Donoghue (2022-06-07 14:40:57)
> The Sony IMX577 uses the same silicon enabling reference code from Sony in
> the available examples provided.
> 
> Add an imx577 compatible string and re-use the existing imx412 code.
> 

As discussed in the cover letter, I think this is a reasonable way to
accomodate the imx577 (and certainly better than letting any real
hardware be defined as if it's an imx412).

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

> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
>  drivers/media/i2c/imx412.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/media/i2c/imx412.c b/drivers/media/i2c/imx412.c
> index a1394d6c1432..3b7011ab0a8f 100644
> --- a/drivers/media/i2c/imx412.c
> +++ b/drivers/media/i2c/imx412.c
> @@ -1282,6 +1282,7 @@ static const struct dev_pm_ops imx412_pm_ops = {
>  
>  static const struct of_device_id imx412_of_match[] = {
>         { .compatible = "sony,imx412" },
> +       { .compatible = "sony,imx577" },
>         { }
>  };
>  
> -- 
> 2.36.1
>
Alessandrelli, Daniele June 14, 2022, 8:02 a.m. UTC | #2 
On Tue, 2022-06-07 at 14:40 +0100, Bryan O'Donoghue wrote:
> The Sony IMX577 uses the same silicon enabling reference code from Sony in
> the available examples provided.
> 
> Add an imx577 compatible string and re-use the existing imx412 code.
> 
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

I've read the discussion following the cover letter and I also tend to
agree that this change makes sense, at least for now.

If in the future somebody wants to add IMX577-specific features, they
can always create a separate driver for IMX577, if that's preferable.

Reviewed-by: Daniele Alessandrelli <daniele.alessandrelli@intel.com>

> ---
>  drivers/media/i2c/imx412.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/media/i2c/imx412.c b/drivers/media/i2c/imx412.c
> index a1394d6c1432..3b7011ab0a8f 100644
> --- a/drivers/media/i2c/imx412.c
> +++ b/drivers/media/i2c/imx412.c
> @@ -1282,6 +1282,7 @@ static const struct dev_pm_ops imx412_pm_ops = {
>  
>  static const struct of_device_id imx412_of_match[] = {
>         { .compatible = "sony,imx412" },
> +       { .compatible = "sony,imx577" },
>         { }
>  };
>
Sakari Ailus June 14, 2022, noon UTC | #3 
Hi Bryan,

On Tue, Jun 07, 2022 at 02:40:57PM +0100, Bryan O'Donoghue wrote:
> The Sony IMX577 uses the same silicon enabling reference code from Sony in
> the available examples provided.
> 
> Add an imx577 compatible string and re-use the existing imx412 code.
> 
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
>  drivers/media/i2c/imx412.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/media/i2c/imx412.c b/drivers/media/i2c/imx412.c
> index a1394d6c1432..3b7011ab0a8f 100644
> --- a/drivers/media/i2c/imx412.c
> +++ b/drivers/media/i2c/imx412.c
> @@ -1282,6 +1282,7 @@ static const struct dev_pm_ops imx412_pm_ops = {
>  
>  static const struct of_device_id imx412_of_match[] = {
>  	{ .compatible = "sony,imx412" },
> +	{ .compatible = "sony,imx577" },

Thanks for the patch.

Surely the sensors still have some differences.

Even if the same registers would work as-is (the imx577 might still benefit
from different MSRs?), the user should know which sensor it is. I.e. please
set the media entity name accordingly. See e.g. the CCS driver for an
example.

>  	{ }
>  };
>
Bryan O'Donoghue June 14, 2022, 2:43 p.m. UTC | #4 
On 14/06/2022 05:00, Sakari Ailus wrote:
> Thanks for the patch.
> 
> Surely the sensors still have some differences.

They do, they absolutely do, the imx577 has a whole bunch of extra modes.

We don't have any reference code or access to documentation for those modes.

My reference is the qualcomm camx code for the rb5 board, which includes 
a imx577 sensor. That stack uses the same init code as for the 412.

So for that baseline mode, the imx412 driver works perfectly.

> Even if the same registers would work as-is (the imx577 might still benefit
> from different MSRs?), the user should know which sensor it is. I.e. please
> set the media entity name accordingly. See e.g. the CCS driver for an
> example.

Agreed, I'll do that.

---
bod
diff mbox series

Patch

diff --git a/drivers/media/i2c/imx412.c b/drivers/media/i2c/imx412.c
index a1394d6c1432..3b7011ab0a8f 100644
--- a/drivers/media/i2c/imx412.c
+++ b/drivers/media/i2c/imx412.c
@@ -1282,6 +1282,7 @@  static const struct dev_pm_ops imx412_pm_ops = {
 
 static const struct of_device_id imx412_of_match[] = {
 	{ .compatible = "sony,imx412" },
+	{ .compatible = "sony,imx577" },
 	{ }
 };