* [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®