RE: [Patch v4 09/11] media: s5p-mfc: Load firmware for each run in MFCv12.

[Aakarsh Jain]

Hi Hans,

> -----Original Message-----
> From: Hans Verkuil <hverkuil-cisco@xxxxxxxxx>
> Sent: 22 November 2023 21:14
> To: Aakarsh Jain <aakarsh.jain@xxxxxxxxxxx>; linux-arm-
> kernel@xxxxxxxxxxxxxxxxxxx; linux-media@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx
> Cc: m.szyprowski@xxxxxxxxxxx; andrzej.hajda@xxxxxxxxx;
> mchehab@xxxxxxxxxx; krzysztof.kozlowski+dt@xxxxxxxxxx;
> dillon.minfei@xxxxxxxxx; david.plowman@xxxxxxxxxxxxxxx;
> mark.rutland@xxxxxxx; robh+dt@xxxxxxxxxx; conor+dt@xxxxxxxxxx; linux-
> samsung-soc@xxxxxxxxxxxxxxx; andi@xxxxxxxxxxx; gost.dev@xxxxxxxxxxx;
> alim.akhtar@xxxxxxxxxxx; aswani.reddy@xxxxxxxxxxx;
> pankaj.dubey@xxxxxxxxxxx; ajaykumar.rs@xxxxxxxxxxx; linux-
> fsd@xxxxxxxxx; Smitha T Murthy <smithatmurthy@xxxxxxxxx>
> Subject: Re: [Patch v4 09/11] media: s5p-mfc: Load firmware for each run in
> MFCv12.
> 
> On 25/10/2023 12:22, Aakarsh Jain wrote:
> > In MFCv12, some section of firmware gets updated at each MFC run.
> > Hence we need to reload original firmware for each run at the start.
> 
> Huh? This is very weird. This definitely deserves a comment in the actual
> code rather than just the commit log.
> 
> Do you know what is going on? What part is updated? Are you sure it isn't a
> driver bug somehow?
> 
> Regards,
> 
> 	Hans
> 
During SYS_INIT command sent to MFC sequentially, firmware is not able to initialize the hardware due to incorrect firmware transfer and in current scenario the firmware is not loaded again in the Reserved memory area.
In this case RET_SYS_INIT response from hardware is failing. So we need to load firmware every time we open the device node.
I will add comment in the code why this change is needed.

Thanks for the review.
> >
> > Cc: linux-fsd@xxxxxxxxx
> > Signed-off-by: Smitha T Murthy <smithatmurthy@xxxxxxxxx>
> > Signed-off-by: Aakarsh Jain <aakarsh.jain@xxxxxxxxxxx>
> > ---
> >  drivers/media/platform/samsung/s5p-mfc/s5p_mfc_ctrl.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/media/platform/samsung/s5p-mfc/s5p_mfc_ctrl.c
> > b/drivers/media/platform/samsung/s5p-mfc/s5p_mfc_ctrl.c
> > index b49159142c53..057088b9d327 100644
> > --- a/drivers/media/platform/samsung/s5p-mfc/s5p_mfc_ctrl.c
> > +++ b/drivers/media/platform/samsung/s5p-mfc/s5p_mfc_ctrl.c
> > @@ -51,8 +51,9 @@ int s5p_mfc_load_firmware(struct s5p_mfc_dev
> *dev)
> >  	 * into kernel. */
> >  	mfc_debug_enter();
> >
> > -	if (dev->fw_get_done)
> > -		return 0;
> > +	if (!IS_MFCV12(dev))
> > +		if (dev->fw_get_done)
> > +			return 0;
> >
> >  	for (i = MFC_FW_MAX_VERSIONS - 1; i >= 0; i--) {
> >  		if (!dev->variant->fw_name[i])