RE: [Intel-gfx] [PATCH v10 3/5] mei: gsc: setup char driver alive in spite of firmware handshake failure

[Usyskin, Alexander]

> -----Original Message-----
> From: Ceraolo Spurio, Daniele <daniele.ceraolospurio@xxxxxxxxx>
> Sent: Thursday, March 10, 2022 02:28
> To: Usyskin, Alexander <alexander.usyskin@xxxxxxxxx>; Greg Kroah-
> Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>; Jani Nikula
> <jani.nikula@xxxxxxxxxxxxxxx>; Joonas Lahtinen
> <joonas.lahtinen@xxxxxxxxxxxxxxx>; Vivi, Rodrigo <rodrigo.vivi@xxxxxxxxx>;
> David Airlie <airlied@xxxxxxxx>; Daniel Vetter <daniel@xxxxxxxx>; Tvrtko
> Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx>
> Cc: linux-kernel@xxxxxxxxxxxxxxx; Winkler, Tomas
> <tomas.winkler@xxxxxxxxx>; Lubart, Vitaly <vitaly.lubart@xxxxxxxxx>; intel-
> gfx@xxxxxxxxxxxxxxxxxxxxx
> Subject: Re: [Intel-gfx] [PATCH v10 3/5] mei: gsc: setup char driver alive in
> spite of firmware handshake failure
> 
> 
> 
> On 3/8/2022 8:36 AM, Alexander Usyskin wrote:
> > Setup char device in spite of firmware handshake failure.
> > In order to provide host access to the firmware status registers and other
> > information required for the manufacturing process.
> 
> IMO this patch should be moved to after the patch that adds the logic to
> fetch the FW version, as that is interesting info for sysfs. Not a blocker.
> 

Actually, the FW version is filled only if there is an established channel with FW.
Firmware status registers are the crucial information for debug, and it filled
in previous patches.

-- 
Thanks,
Sasha


> >
> > Signed-off-by: Alexander Usyskin <alexander.usyskin@xxxxxxxxx>
> > Signed-off-by: Tomas Winkler <tomas.winkler@xxxxxxxxx>
> 
> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
> 
> Daniele
> 
> > ---
> >   drivers/misc/mei/gsc-me.c | 11 ++++++-----
> >   1 file changed, 6 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/misc/mei/gsc-me.c b/drivers/misc/mei/gsc-me.c
> > index 0afae70e0609..cf427f6fdec9 100644
> > --- a/drivers/misc/mei/gsc-me.c
> > +++ b/drivers/misc/mei/gsc-me.c
> > @@ -79,11 +79,12 @@ static int mei_gsc_probe(struct auxiliary_device
> *aux_dev,
> >   	pm_runtime_set_active(device);
> >   	pm_runtime_enable(device);
> >
> > -	if (mei_start(dev)) {
> > -		dev_err(device, "init hw failure.\n");
> > -		ret = -ENODEV;
> > -		goto err;
> > -	}
> > +	/* Continue to char device setup in spite of firmware handshake
> failure.
> > +	 * In order to provide access to the firmware status registers to the
> user
> > +	 * space via sysfs.
> > +	 */
> > +	if (mei_start(dev))
> > +		dev_warn(device, "init hw failure.\n");
> >
> >   	pm_runtime_set_autosuspend_delay(device,
> MEI_GSC_RPM_TIMEOUT);
> >   	pm_runtime_use_autosuspend(device);