linux-kernel - Re: [PATCH v2] media: mt9p031: Refactor format handling for different sensor models

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID:
 <PN3P287MB1829515AEA990C3CC437DC9C8B4A2@PN3P287MB1829.INDP287.PROD.OUTLOOK.COM>
Date: Mon, 28 Oct 2024 12:02:16 +0000
From: Tarang Raval <tarang.raval@...iconsignals.io>
To: Sakari Ailus <sakari.ailus@...ux.intel.com>
CC: "laurent.pinchart@...asonboard.com" <laurent.pinchart@...asonboard.com>,
	Mauro Carvalho Chehab <mchehab@...nel.org>, "linux-media@...r.kernel.org"
	<linux-media@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] media: mt9p031: Refactor format handling for different
 sensor models

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@...ux.intel.com>
Sent: Monday, October 28, 2024 3:14 PM
To: Tarang Raval <tarang.raval@...iconsignals.io>
Cc: laurent.pinchart@...asonboard.com <laurent.pinchart@...asonboard.com>; Mauro Carvalho Chehab <mchehab@...nel.org>; linux-media@...r.kernel.org <linux-media@...r.kernel.org>; linux-kernel@...r.kernel.org <linux-kernel@...r.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@...iconsignals.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