* [PATCH v2] media: mt9p031: Refactor format handling for different sensor models
@ 2024-10-25 22:15 Tarang Raval
2024-10-26 17:36 ` Laurent Pinchart
2024-10-28 9:44 ` Sakari Ailus
0 siblings, 2 replies; 5+ messages in thread
From: Tarang Raval @ 2024-10-25 22:15 UTC (permalink / raw)
To: laurent.pinchart
Cc: Tarang Raval, Sakari Ailus, Mauro Carvalho Chehab, linux-media,
linux-kernel
Add new structure 'mt9p031_model_info' to encapsulate format codes for
the mt9p031 camera sensor family. This approach enhances code clarity
and maintainability.
Signed-off-by: Tarang Raval <tarang.raval@siliconsignals.io>
---
drivers/media/i2c/mt9p031.c | 31 ++++++++++++++++++++++++++++---
1 file changed, 28 insertions(+), 3 deletions(-)
diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
index f2f52f484044..3576d3066738 100644
--- a/drivers/media/i2c/mt9p031.c
+++ b/drivers/media/i2c/mt9p031.c
@@ -112,6 +112,24 @@
#define MT9P031_TEST_PATTERN_RED 0xa2
#define MT9P031_TEST_PATTERN_BLUE 0xa3
+struct mt9p031_model_info {
+ u32 code;
+};
+
+enum mt9p031_model {
+ MT9P031_MODEL_BAYER,
+ MT9P031_MODEL_MONO,
+};
+
+static const struct mt9p031_model_info mt9p031_models[] = {
+ [MT9P031_MODEL_BAYER] = {
+ .code = MEDIA_BUS_FMT_SGRBG12_1X12,
+ },
+ [MT9P031_MODEL_MONO] = {
+ .code = MEDIA_BUS_FMT_Y12_1X12,
+ },
+};
+
struct mt9p031 {
struct v4l2_subdev subdev;
struct media_pad pad;
@@ -1209,9 +1227,16 @@ static void mt9p031_remove(struct i2c_client *client)
}
static const struct of_device_id mt9p031_of_match[] = {
- { .compatible = "aptina,mt9p006", .data = (void *)MEDIA_BUS_FMT_SGRBG12_1X12 },
- { .compatible = "aptina,mt9p031", .data = (void *)MEDIA_BUS_FMT_SGRBG12_1X12 },
- { .compatible = "aptina,mt9p031m", .data = (void *)MEDIA_BUS_FMT_Y12_1X12 },
+ {
+ .compatible = "aptina,mt9p006",
+ .data = &mt9p031_models[MT9P031_MODEL_BAYER]
+ }, {
+ .compatible = "aptina,mt9p031",
+ .data = &mt9p031_models[MT9P031_MODEL_BAYER]
+ }, {
+ .compatible = "aptina,mt9p031m",
+ .data = &mt9p031_models[MT9P031_MODEL_MONO]
+ },
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, mt9p031_of_match);
--
2.43.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] media: mt9p031: Refactor format handling for different sensor models
2024-10-25 22:15 [PATCH v2] media: mt9p031: Refactor format handling for different sensor models Tarang Raval
@ 2024-10-26 17:36 ` Laurent Pinchart
2024-10-28 9:44 ` Sakari Ailus
1 sibling, 0 replies; 5+ messages in thread
From: Laurent Pinchart @ 2024-10-26 17:36 UTC (permalink / raw)
To: Tarang Raval
Cc: Sakari Ailus, Mauro Carvalho Chehab, linux-media, linux-kernel
Hi Tarang,
Thank you for the patch.
On Sat, Oct 26, 2024 at 03:45:40AM +0530, Tarang Raval wrote:
> Add new structure 'mt9p031_model_info' to encapsulate format codes for
> the mt9p031 camera sensor family. This approach enhances code clarity
> and maintainability.
>
> Signed-off-by: Tarang Raval <tarang.raval@siliconsignals.io>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Sakari, this applies on top of "[PATCH 0/2] media: mt9p031: Drop legacy
platform data" (https://lore.kernel.org/r/20241025181708.20648-1-laurent.pinchart@ideasonboard.com).
Tarang, feel free to review that series to accelerate integration of the
patches upstream :-)
> ---
> drivers/media/i2c/mt9p031.c | 31 ++++++++++++++++++++++++++++---
> 1 file changed, 28 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
> index f2f52f484044..3576d3066738 100644
> --- a/drivers/media/i2c/mt9p031.c
> +++ b/drivers/media/i2c/mt9p031.c
> @@ -112,6 +112,24 @@
> #define MT9P031_TEST_PATTERN_RED 0xa2
> #define MT9P031_TEST_PATTERN_BLUE 0xa3
>
> +struct mt9p031_model_info {
> + u32 code;
> +};
> +
> +enum mt9p031_model {
> + MT9P031_MODEL_BAYER,
> + MT9P031_MODEL_MONO,
> +};
> +
> +static const struct mt9p031_model_info mt9p031_models[] = {
> + [MT9P031_MODEL_BAYER] = {
> + .code = MEDIA_BUS_FMT_SGRBG12_1X12,
> + },
> + [MT9P031_MODEL_MONO] = {
> + .code = MEDIA_BUS_FMT_Y12_1X12,
> + },
> +};
> +
> struct mt9p031 {
> struct v4l2_subdev subdev;
> struct media_pad pad;
> @@ -1209,9 +1227,16 @@ static void mt9p031_remove(struct i2c_client *client)
> }
>
> static const struct of_device_id mt9p031_of_match[] = {
> - { .compatible = "aptina,mt9p006", .data = (void *)MEDIA_BUS_FMT_SGRBG12_1X12 },
> - { .compatible = "aptina,mt9p031", .data = (void *)MEDIA_BUS_FMT_SGRBG12_1X12 },
> - { .compatible = "aptina,mt9p031m", .data = (void *)MEDIA_BUS_FMT_Y12_1X12 },
> + {
> + .compatible = "aptina,mt9p006",
> + .data = &mt9p031_models[MT9P031_MODEL_BAYER]
> + }, {
> + .compatible = "aptina,mt9p031",
> + .data = &mt9p031_models[MT9P031_MODEL_BAYER]
> + }, {
> + .compatible = "aptina,mt9p031m",
> + .data = &mt9p031_models[MT9P031_MODEL_MONO]
> + },
> { /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(of, mt9p031_of_match);
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] media: mt9p031: Refactor format handling for different sensor models
2024-10-25 22:15 [PATCH v2] media: mt9p031: Refactor format handling for different sensor models Tarang Raval
2024-10-26 17:36 ` Laurent Pinchart
@ 2024-10-28 9:44 ` Sakari Ailus
2024-10-28 12:02 ` Tarang Raval
2024-10-28 13:29 ` Laurent Pinchart
1 sibling, 2 replies; 5+ messages in thread
From: Sakari Ailus @ 2024-10-28 9:44 UTC (permalink / raw)
To: Tarang Raval
Cc: laurent.pinchart, Mauro Carvalho Chehab, linux-media, linux-kernel
Hi Tarang,
On Sat, Oct 26, 2024 at 03:45:40AM +0530, Tarang Raval wrote:
> Add new structure 'mt9p031_model_info' to encapsulate format codes for
> the mt9p031 camera sensor family. This approach enhances code clarity
> and maintainability.
>
> Signed-off-by: Tarang Raval <tarang.raval@siliconsignals.io>
> ---
> drivers/media/i2c/mt9p031.c | 31 ++++++++++++++++++++++++++++---
> 1 file changed, 28 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
> index f2f52f484044..3576d3066738 100644
> --- a/drivers/media/i2c/mt9p031.c
> +++ b/drivers/media/i2c/mt9p031.c
> @@ -112,6 +112,24 @@
> #define MT9P031_TEST_PATTERN_RED 0xa2
> #define MT9P031_TEST_PATTERN_BLUE 0xa3
>
> +struct mt9p031_model_info {
> + u32 code;
> +};
> +
> +enum mt9p031_model {
> + MT9P031_MODEL_BAYER,
> + MT9P031_MODEL_MONO,
> +};
> +
> +static const struct mt9p031_model_info mt9p031_models[] = {
> + [MT9P031_MODEL_BAYER] = {
> + .code = MEDIA_BUS_FMT_SGRBG12_1X12,
> + },
> + [MT9P031_MODEL_MONO] = {
> + .code = MEDIA_BUS_FMT_Y12_1X12,
> + },
> +};
> +
> struct mt9p031 {
> struct v4l2_subdev subdev;
> struct media_pad pad;
> @@ -1209,9 +1227,16 @@ static void mt9p031_remove(struct i2c_client *client)
> }
>
> static const struct of_device_id mt9p031_of_match[] = {
> - { .compatible = "aptina,mt9p006", .data = (void *)MEDIA_BUS_FMT_SGRBG12_1X12 },
> - { .compatible = "aptina,mt9p031", .data = (void *)MEDIA_BUS_FMT_SGRBG12_1X12 },
> - { .compatible = "aptina,mt9p031m", .data = (void *)MEDIA_BUS_FMT_Y12_1X12 },
> + {
> + .compatible = "aptina,mt9p006",
> + .data = &mt9p031_models[MT9P031_MODEL_BAYER]
> + }, {
> + .compatible = "aptina,mt9p031",
> + .data = &mt9p031_models[MT9P031_MODEL_BAYER]
> + }, {
> + .compatible = "aptina,mt9p031m",
> + .data = &mt9p031_models[MT9P031_MODEL_MONO]
Instead using an index into an array, could you add structs for describing
both separately? See e.g. drivers/media/i2c/ccs/ccs-core.c for an example.
> + },
> { /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(of, mt9p031_of_match);
--
Kind regards,
Sakari Ailus
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] media: mt9p031: Refactor format handling for different sensor models
2024-10-28 9:44 ` Sakari Ailus
@ 2024-10-28 12:02 ` Tarang Raval
2024-10-28 13:29 ` Laurent Pinchart
1 sibling, 0 replies; 5+ messages in thread
From: Tarang Raval @ 2024-10-28 12:02 UTC (permalink / raw)
To: Sakari Ailus
Cc: laurent.pinchart, Mauro Carvalho Chehab, linux-media, linux-kernel
Hi Sakari,
Thanks for suggention
>> + }, {
>> + .compatible = "aptina,mt9p031",
>> + .data = &mt9p031_models[MT9P031_MODEL_BAYER]
>> + }, {
>> + .compatible = "aptina,mt9p031m",
>> + .data = &mt9p031_models[MT9P031_MODEL_MONO]
>
>Instead using an index into an array, could you add structs for describing
>both separately? See e.g. drivers/media/i2c/ccs/ccs-core.c for an example.
Sure, I will send v3 with the above approach.
Best Regards,
Tarang
________________________________________
From: Sakari Ailus <sakari.ailus@linux.intel.com>
Sent: Monday, October 28, 2024 3:14 PM
To: Tarang Raval <tarang.raval@siliconsignals.io>
Cc: laurent.pinchart@ideasonboard.com <laurent.pinchart@ideasonboard.com>; Mauro Carvalho Chehab <mchehab@kernel.org>; linux-media@vger.kernel.org <linux-media@vger.kernel.org>; linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2] media: mt9p031: Refactor format handling for different sensor models
CAUTION: This email originated from outside the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.
Hi Tarang,
On Sat, Oct 26, 2024 at 03:45:40AM +0530, Tarang Raval wrote:
> Add new structure 'mt9p031_model_info' to encapsulate format codes for
> the mt9p031 camera sensor family. This approach enhances code clarity
> and maintainability.
>
> Signed-off-by: Tarang Raval <tarang.raval@siliconsignals.io>
> ---
> drivers/media/i2c/mt9p031.c | 31 ++++++++++++++++++++++++++++---
> 1 file changed, 28 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
> index f2f52f484044..3576d3066738 100644
> --- a/drivers/media/i2c/mt9p031.c
> +++ b/drivers/media/i2c/mt9p031.c
> @@ -112,6 +112,24 @@
> #define MT9P031_TEST_PATTERN_RED 0xa2
> #define MT9P031_TEST_PATTERN_BLUE 0xa3
>
> +struct mt9p031_model_info {
> + u32 code;
> +};
> +
> +enum mt9p031_model {
> + MT9P031_MODEL_BAYER,
> + MT9P031_MODEL_MONO,
> +};
> +
> +static const struct mt9p031_model_info mt9p031_models[] = {
> + [MT9P031_MODEL_BAYER] = {
> + .code = MEDIA_BUS_FMT_SGRBG12_1X12,
> + },
> + [MT9P031_MODEL_MONO] = {
> + .code = MEDIA_BUS_FMT_Y12_1X12,
> + },
> +};
> +
> struct mt9p031 {
> struct v4l2_subdev subdev;
> struct media_pad pad;
> @@ -1209,9 +1227,16 @@ static void mt9p031_remove(struct i2c_client *client)
> }
>
> static const struct of_device_id mt9p031_of_match[] = {
> - { .compatible = "aptina,mt9p006", .data = (void *)MEDIA_BUS_FMT_SGRBG12_1X12 },
> - { .compatible = "aptina,mt9p031", .data = (void *)MEDIA_BUS_FMT_SGRBG12_1X12 },
> - { .compatible = "aptina,mt9p031m", .data = (void *)MEDIA_BUS_FMT_Y12_1X12 },
> + {
> + .compatible = "aptina,mt9p006",
> + .data = &mt9p031_models[MT9P031_MODEL_BAYER]
> + }, {
> + .compatible = "aptina,mt9p031",
> + .data = &mt9p031_models[MT9P031_MODEL_BAYER]
> + }, {
> + .compatible = "aptina,mt9p031m",
> + .data = &mt9p031_models[MT9P031_MODEL_MONO]
Instead using an index into an array, could you add structs for describing
both separately? See e.g. drivers/media/i2c/ccs/ccs-core.c for an example.
> + },
> { /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(of, mt9p031_of_match);
--
Kind regards,
Sakari Ailus
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] media: mt9p031: Refactor format handling for different sensor models
2024-10-28 9:44 ` Sakari Ailus
2024-10-28 12:02 ` Tarang Raval
@ 2024-10-28 13:29 ` Laurent Pinchart
1 sibling, 0 replies; 5+ messages in thread
From: Laurent Pinchart @ 2024-10-28 13:29 UTC (permalink / raw)
To: Sakari Ailus
Cc: Tarang Raval, Mauro Carvalho Chehab, linux-media, linux-kernel
On Mon, Oct 28, 2024 at 09:44:18AM +0000, Sakari Ailus wrote:
> Hi Tarang,
>
> On Sat, Oct 26, 2024 at 03:45:40AM +0530, Tarang Raval wrote:
> > Add new structure 'mt9p031_model_info' to encapsulate format codes for
> > the mt9p031 camera sensor family. This approach enhances code clarity
> > and maintainability.
> >
> > Signed-off-by: Tarang Raval <tarang.raval@siliconsignals.io>
> > ---
> > drivers/media/i2c/mt9p031.c | 31 ++++++++++++++++++++++++++++---
> > 1 file changed, 28 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
> > index f2f52f484044..3576d3066738 100644
> > --- a/drivers/media/i2c/mt9p031.c
> > +++ b/drivers/media/i2c/mt9p031.c
> > @@ -112,6 +112,24 @@
> > #define MT9P031_TEST_PATTERN_RED 0xa2
> > #define MT9P031_TEST_PATTERN_BLUE 0xa3
> >
> > +struct mt9p031_model_info {
> > + u32 code;
> > +};
> > +
> > +enum mt9p031_model {
> > + MT9P031_MODEL_BAYER,
> > + MT9P031_MODEL_MONO,
> > +};
> > +
> > +static const struct mt9p031_model_info mt9p031_models[] = {
> > + [MT9P031_MODEL_BAYER] = {
> > + .code = MEDIA_BUS_FMT_SGRBG12_1X12,
> > + },
> > + [MT9P031_MODEL_MONO] = {
> > + .code = MEDIA_BUS_FMT_Y12_1X12,
> > + },
> > +};
> > +
> > struct mt9p031 {
> > struct v4l2_subdev subdev;
> > struct media_pad pad;
> > @@ -1209,9 +1227,16 @@ static void mt9p031_remove(struct i2c_client *client)
> > }
> >
> > static const struct of_device_id mt9p031_of_match[] = {
> > - { .compatible = "aptina,mt9p006", .data = (void *)MEDIA_BUS_FMT_SGRBG12_1X12 },
> > - { .compatible = "aptina,mt9p031", .data = (void *)MEDIA_BUS_FMT_SGRBG12_1X12 },
> > - { .compatible = "aptina,mt9p031m", .data = (void *)MEDIA_BUS_FMT_Y12_1X12 },
> > + {
> > + .compatible = "aptina,mt9p006",
> > + .data = &mt9p031_models[MT9P031_MODEL_BAYER]
> > + }, {
> > + .compatible = "aptina,mt9p031",
> > + .data = &mt9p031_models[MT9P031_MODEL_BAYER]
> > + }, {
> > + .compatible = "aptina,mt9p031m",
> > + .data = &mt9p031_models[MT9P031_MODEL_MONO]
>
> Instead using an index into an array, could you add structs for describing
> both separately? See e.g. drivers/media/i2c/ccs/ccs-core.c for an example.
For what it's worth, I prefer the array and indices. I don't care too
much though.
> > + },
> > { /* sentinel */ }
> > };
> > MODULE_DEVICE_TABLE(of, mt9p031_of_match);
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-10-28 13:30 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-10-25 22:15 [PATCH v2] media: mt9p031: Refactor format handling for different sensor models Tarang Raval
2024-10-26 17:36 ` Laurent Pinchart
2024-10-28 9:44 ` Sakari Ailus
2024-10-28 12:02 ` Tarang Raval
2024-10-28 13:29 ` Laurent Pinchart
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®