2022-03-09 00:00:17

by Usyskin, Alexander

[permalink] [raw]
Subject: [PATCH v10 3/5] mei: gsc: setup char driver alive in spite of firmware handshake failure

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.

Signed-off-by: Alexander Usyskin <[email protected]>
Signed-off-by: Tomas Winkler <[email protected]>
---
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);
--
2.32.0


2022-03-10 04:11:07

by Daniele Ceraolo Spurio

[permalink] [raw]
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.

>
> Signed-off-by: Alexander Usyskin <[email protected]>
> Signed-off-by: Tomas Winkler <[email protected]>

Reviewed-by: Daniele Ceraolo Spurio <[email protected]>

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);

2022-03-14 12:05:24

by Usyskin, Alexander

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



> -----Original Message-----
> From: Ceraolo Spurio, Daniele <[email protected]>
> Sent: Thursday, March 10, 2022 02:28
> To: Usyskin, Alexander <[email protected]>; Greg Kroah-
> Hartman <[email protected]>; Jani Nikula
> <[email protected]>; Joonas Lahtinen
> <[email protected]>; Vivi, Rodrigo <[email protected]>;
> David Airlie <[email protected]>; Daniel Vetter <[email protected]>; Tvrtko
> Ursulin <[email protected]>
> Cc: [email protected]; Winkler, Tomas
> <[email protected]>; Lubart, Vitaly <[email protected]>; intel-
> [email protected]
> 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 <[email protected]>
> > Signed-off-by: Tomas Winkler <[email protected]>
>
> Reviewed-by: Daniele Ceraolo Spurio <[email protected]>
>
> 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);