From:   <Shravan.Chippa@microchip.com>
To:     <sakari.ailus@iki.fi>
CC:     <paul.j.murphy@intel.com>, <daniele.alessandrelli@intel.com>,
        <mchehab@kernel.org>, <robh+dt@kernel.org>,
        <krzysztof.kozlowski+dt@linaro.org>, <shawnguo@kernel.org>,
        <s.hauer@pengutronix.de>, <kernel@pengutronix.de>,
        <festevam@gmail.com>, <linux-imx@nxp.com>,
        <linux-media@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <devicetree@vger.kernel.org>,
        <linux-arm-kernel@lists.infradead.org>
Subject: RE: [PATCH v12 5/5] media: i2c: imx334: update pixel and link
 frequency
Thread-Topic: [PATCH v12 5/5] media: i2c: imx334: update pixel and link
 frequency
Thread-Index: AQHZTBBlW3UhN6JX1k+chzEyKGiizq76t7iAgArC6LA=
Date:   Tue, 21 Mar 2023 15:41:50 +0000
Message-ID: <PH0PR11MB56114161FC09467BC60616F881819@PH0PR11MB5611.namprd11.prod.outlook.com>
References: <20230301073412.1204574-1-shravan.chippa@microchip.com>
 <20230301073412.1204574-6-shravan.chippa@microchip.com>
 <ZBDFcXYeK0eB9i1v@valkosipuli.retiisi.eu>
In-Reply-To: <ZBDFcXYeK0eB9i1v@valkosipuli.retiisi.eu>
Accept-Language: en-GB, en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?9GP3Gh127EKuACBWUpcAsDEc59XdOR0HcloyfJUmVW++CGj1QzzUWqj4PCW8?=
 =?us-ascii?Q?MYhULvvFHUpEQlDEdQe81h8Zi6YRQhCWUx63haNgwI+FI56QHQILrFcdqr8P?=
 =?us-ascii?Q?Cv4W/4sgbW7hgoAuGiHgZcnBLhY13k4zUA+CkM2bmMjDTM3sjZtD9vd0pcMx?=
 =?us-ascii?Q?lMp/14JnEYHHWdkCrn4cHOZ2h5tSCm01f1xcZKeaJZ8sKYm1tTxrwo3Zjp2s?=
 =?us-ascii?Q?8qpWlWlWTkQyp8ZZhQEaxbl5NEAtb+YlSPpxNKcYG8V97YPVRAF1W7VNQhLd?=
 =?us-ascii?Q?4D5os1LiidlNh+AcTA3Ruz8BGAa4wreotbRw1QFqdaVDXf8MoXH1cpawQ96c?=
 =?us-ascii?Q?Vl/mV3Q9ifbc3VcyiObsKWZ0ZmEFptyS+3iom2UQb+MsUrF0d9/ZQxEPvEni?=
 =?us-ascii?Q?U39ZG8qaNIfK5w4perg00/4E3S4NHdJyX92C0OpkkxGxDsBenBp522GGlDCf?=
 =?us-ascii?Q?FbEEFRBttUJuzcFXBFXPZT0RnhxFWg6efsGZd/fJBfE/+DZ0zd8NlppzvAdk?=
 =?us-ascii?Q?ncTY4QzCNV8MEnN5RgHw9oYtw4d9u2HAZWXa7v01fduA00vzio5fnMOruZbW?=
 =?us-ascii?Q?t25uy0TkyL9HnzG10N6/tq8dGUNAJW97lWIqS1ASt/CJ270bkl3tJLJkw2y7?=
 =?us-ascii?Q?GCYhLdNOHmzLB6S2q+Mdw38my46z/apG+WjK/i4Z2+/H2ltcVcifRC/xTWMj?=
 =?us-ascii?Q?ssEe5tEZFpfuz/yScrf0iidJqnKs31NQXZRDC7M2f2toqHJw6MFy3+uBki0Y?=
 =?us-ascii?Q?Ybnp8GFFCSVY4q6wH69iyyFZTK7b75uQY7hsBueRJYMrx4t200/K/st94Ava?=
 =?us-ascii?Q?joOZyUNeCwBu1IYZAD3tbfaqVptFzQuH4RyBqyD+4dhEdXdZp2y1dphnv8td?=
 =?us-ascii?Q?ahd58c/krsaJ2UVBmGjSjShzy774Bzvtr9v9tUvYqHKWjsSiarqIBLDYU77l?=
 =?us-ascii?Q?72uqc+dPVLkRlrVu5b46+7FA+eMJk9HxSpljkb/rYtO/JxsF5L3abCoCqP9D?=
 =?us-ascii?Q?429iBICYCrqet5IkTkww4uuBxD4wzC7IZuA7BH9X+kmPM9jUpKzh6rcQO6VM?=
 =?us-ascii?Q?OJsYAAHuXbXtZj8lqPMPcGRzIyIFGCuHzYMGzOOWtThgYkxA+MoG7gLshkmP?=
 =?us-ascii?Q?WGiS3wqUoBy39Xe20QcZtPuP09Bk1+w+Sx1eJp03smQK1N74UTPE7ckBA+UQ?=
 =?us-ascii?Q?lDvBApmsS8yEYqkgbdILqzDit7jFk7kJEHG5rs5lkmVeT0UbYrZFg6b5yzH0?=
 =?us-ascii?Q?pa9yIJu3amGAPtbzEccuZyjZLbM5243xHEPzBbfyESaBR9fcy4YO+nA0bOjE?=
 =?us-ascii?Q?QO5cRQhCRxNlul6ekEf+To7aymLp3gOc/o3mz15wjnFk52UAPv2QqnvniWTl?=
 =?us-ascii?Q?WyvEKay7jEHPGygFRvsscPqm7UeJw9wMmq2XJOiPKELpCUb/o+XS0uaBSTiS?=
 =?us-ascii?Q?MzOA8uC2Mh7Kjd2JL44kflliyJ8W6Y6kUs8tlKMeO8Y9+jN0NTW2N/xPPVXc?=
 =?us-ascii?Q?EvSD7grRm47o9JdHsEndTK2B661AEbbpPPOX2Mdu15qu6bZrW0wE3KoHcVrq?=
 =?us-ascii?Q?HCjBoMpddIXKqx+otvJpRbM8Z5/4tVfN2gHv8Fhy8yPgCoMHW6ToMPzhReKU?=
 =?us-ascii?Q?6g=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: PH0PR11MB5611.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 9852dbeb-6390-410a-0bfa-08db2a22c98f
X-MS-Exchange-CrossTenant-originalarrivaltime: 21 Mar 2023 15:41:50.1750
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 3f4057f3-b418-4d4e-ba84-d55b4e897d88
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: XxNHmhpA5xt+Kb2z90RrNHYvXcPPXNp3Mp3+cgKnHwi9TYVjJ61/Waow8HFe3TwcsMBKnZ/5mlEbwdcc4AhlnK+BSAa99ohNqwKbeecbXv4=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH8PR11MB6901
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Sakari,

> -----Original Message-----
> From: Sakari Ailus <sakari.ailus@iki.fi>
> Sent: 15 March 2023 12:35 AM
> To: shravan Chippa - I35088 <Shravan.Chippa@microchip.com>
> Cc: paul.j.murphy@intel.com; daniele.alessandrelli@intel.com;
> mchehab@kernel.org; robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org=
;
> shawnguo@kernel.org; s.hauer@pengutronix.de; kernel@pengutronix.de;
> festevam@gmail.com; linux-imx@nxp.com; linux-media@vger.kernel.org; linux=
-
> kernel@vger.kernel.org; devicetree@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org
> Subject: Re: [PATCH v12 5/5] media: i2c: imx334: update pixel and link fr=
equency
>=20
> EXTERNAL EMAIL: Do not click links or open attachments unless you know th=
e
> content is safe
>=20
> Hi Shravan,
>=20
> On Wed, Mar 01, 2023 at 01:04:12PM +0530, shravan kumar wrote:
> > @@ -885,7 +895,13 @@ static int imx334_init_pad_cfg(struct v4l2_subdev
> *sd,
> >       struct v4l2_subdev_format fmt =3D { 0 };
> >
> >       fmt.which =3D sd_state ? V4L2_SUBDEV_FORMAT_TRY :
> V4L2_SUBDEV_FORMAT_ACTIVE;
> > -     imx334_fill_pad_format(imx334, &supported_modes[0], &fmt);
> > +     fmt->format.code =3D imx334->cur_code;
>=20
> This does not compile.
>=20
> > +     imx334_fill_pad_format(imx334, imx334->cur_mode, &fmt);
> > +
> > +     __v4l2_ctrl_modify_range(imx334->link_freq_ctrl, 0,
> > +                              __fls(imx334->menu_skip_mask),
> > +                              ~(imx334->menu_skip_mask),
> > +                              __ffs(imx334->menu_skip_mask));
>=20
> You're not holding imx334->mutex here, as you should. Also accessing
> imx334->cur_code should only be done while that mutex is acquired.
>=20
> What's the purpose of calling __v4l2_ctrl_modify_range() here, all these =
values
> are static once probe() function has been called, aren't they?

v4l2_ctrl_new_int_menu() function limiting only upper and lower frequencies=
 max and min, as of now only two frequencies no problem. If we have more fr=
equencies then we may need to use__v4l2_ctrl_modify_range() with skip mask =
value.

>=20
> I'm dropping this patch for now, taking the first four.

I will try to send it as a separate patch.
>=20
> >
> >       return imx334_set_pad_format(sd, sd_state, &fmt);  }
>=20
> --
> Kind regards,
>=20
> Sakari Ailus