Re: [PATCH v2 2/2] media: imagination: Add E5010 JPEG Encoder driver

[Nicolas Dufresne <nicolas@ndufresne.ca>]

Hi Krzysztof,

Le jeudi 27 juillet 2023 à 14:13 +0200, Krzysztof Kozlowski a écrit :
> On 27/07/2023 13:25, Devarsh Thakkar wrote:
> ...
> 
> > +
> > +static int e5010_release(struct file *file)
> > +{
> > +	struct e5010_dev *dev = video_drvdata(file);
> > +	struct e5010_context *ctx = file->private_data;
> > +
> > +	dprintk(dev, 1, "Releasing instance: 0x%p, m2m_ctx: 0x%p\n", ctx, ctx->fh.m2m_ctx);
> 
> 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 traces
to help future debugging if a bug is seen. These uses v4l2 debug helper, and 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 after
the instance is released, that would be buggy for sure, but I don't think this
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.

. . .