Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <51e4ece5250c3345dae4956fbb4d4dbb5ffdde38.camel@ndufresne.ca>
Subject: Re: [PATCH v2 2/2] media: imagination: Add E5010 JPEG Encoder driver
From:   Nicolas Dufresne <nicolas@ndufresne.ca>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
        Devarsh Thakkar <devarsht@ti.com>, mchehab@kernel.org,
        robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org,
        conor+dt@kernel.org, hverkuil-cisco@xs4all.nl,
        laurent.pinchart@ideasonboard.com, eugen.hristev@collabora.com,
        ezequiel@vanguardiasur.com.ar, u.kleine-koenig@pengutronix.de,
        sakari.ailus@linux.intel.com, linux-media@vger.kernel.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Cc:     praneeth@ti.com, nm@ti.com, vigneshr@ti.com, a-bhatia1@ti.com,
        j-luthra@ti.com, b-brnich@ti.com, detheridge@ti.com,
        p-mantena@ti.com, vijayp@ti.com
Date:   Thu, 27 Jul 2023 10:19:29 -0400
In-Reply-To: <ef4825d6-1016-cbf2-0cd3-94b0fc4165f4@linaro.org>
References: <20230727112546.2201995-1-devarsht@ti.com>
         <20230727112546.2201995-3-devarsht@ti.com>
         <ef4825d6-1016-cbf2-0cd3-94b0fc4165f4@linaro.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.48.4 (3.48.4-1.fc38) 
MIME-Version: 1.0
Precedence: bulk

Hi Krzysztof,

Le jeudi 27 juillet 2023 =C3=A0 14:13 +0200, Krzysztof Kozlowski a =C3=A9cr=
it=C2=A0:
> On 27/07/2023 13:25, Devarsh Thakkar wrote:
> ...
>=20
> > +
> > +static int e5010_release(struct file *file)
> > +{
> > +	struct e5010_dev *dev =3D video_drvdata(file);
> > +	struct e5010_context *ctx =3D file->private_data;
> > +
> > +	dprintk(dev, 1, "Releasing instance: 0x%p, m2m_ctx: 0x%p\n", ctx, ctx=
->fh.m2m_ctx);
>=20
> Why do you print pointers? Looks like code is buggy and you still keep
> debugging it.

Its relatively common practice in linux-media to leave a certain level of t=
races
to help future debugging if a bug is seen. These uses v4l2 debug helper, an=
d are
only going to print if users enable them through the associated sysfs
configuration. I do hope though there isn't any issue with IRQ triggering a=
fter
the instance is released, that would be buggy for sure, but I don't think t=
his
is the case considering the level of documented testing that have been done=
.

I'd be happy to see what others have to say on the subject, as its been a
recurrent subject of confrontation lately. With pretty agressive messages
associated with that.

regards,
Nicolas

p.s. does not invalidate the question, since for this driver, there is only=
 ever
going to be one m2m_ctx, so the question "Why do you print pointers?" is
entirely valid I believe.

. . .