[PATCH 0/2] Staging: gasket: Implement apex_get_status() to check driver status

[PATCH 0/2] Staging: gasket: Implement apex_get_status() to check driver status

[Samuil Ivanov]

From the TODO list:
- apex_get_status() should actually check status.

First patch implements the check.
Second removes the TODO comment in the function.
*** BLURB HERE ***

Samuil Ivanov (2):
  Staging: gasket: implement apex_get_status() to check driver status
  Staging: gasket: Clean apex_get_status function of comment

 drivers/staging/gasket/apex_driver.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

-- 
2.17.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 1/2] Staging: gasket: implement apex_get_status() to check driver status

[Samuil Ivanov]

From the TODO:
- apex_get_status() should actually check status

The function now checkes the status of the driver

Signed-off-by: Samuil Ivanov <samuil.ivanovbg@gmail.com>
---
 drivers/staging/gasket/apex_driver.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/staging/gasket/apex_driver.c b/drivers/staging/gasket/apex_driver.c
index 46199c8ca441..a5dd6f3c367d 100644
--- a/drivers/staging/gasket/apex_driver.c
+++ b/drivers/staging/gasket/apex_driver.c
@@ -247,6 +247,9 @@ module_param(bypass_top_level, int, 0644);
 static int apex_get_status(struct gasket_dev *gasket_dev)
 {
 	/* TODO: Check device status. */
+	if (gasket_dev->status == GASKET_STATUS_DEAD)
+		return GASKET_STATUS_DEAD;
+
 	return GASKET_STATUS_ALIVE;
 }
 
-- 
2.17.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 2/2] Staging: gasket: Clean apex_get_status function of comment

[Samuil Ivanov]

After implementing the function to check the status of the driver,
there is no longer need for this comment.

Signed-off-by: Samuil Ivanov <samuil.ivanovbg@gmail.com>
---
 drivers/staging/gasket/apex_driver.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/gasket/apex_driver.c b/drivers/staging/gasket/apex_driver.c
index a5dd6f3c367d..67cdb4f5da8e 100644
--- a/drivers/staging/gasket/apex_driver.c
+++ b/drivers/staging/gasket/apex_driver.c
@@ -246,7 +246,6 @@ module_param(bypass_top_level, int, 0644);
 /* Check the device status registers and return device status ALIVE or DEAD. */
 static int apex_get_status(struct gasket_dev *gasket_dev)
 {
-	/* TODO: Check device status. */
 	if (gasket_dev->status == GASKET_STATUS_DEAD)
 		return GASKET_STATUS_DEAD;
 
-- 
2.17.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/2] Staging: gasket: implement apex_get_status() to check driver status

[Greg KH]

On Tue, Oct 29, 2019 at 12:59:25AM +0200, Samuil Ivanov wrote:
> >From the TODO:
> - apex_get_status() should actually check status
> 
> The function now checkes the status of the driver
> 
> Signed-off-by: Samuil Ivanov <samuil.ivanovbg@gmail.com>
> ---
>  drivers/staging/gasket/apex_driver.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/staging/gasket/apex_driver.c b/drivers/staging/gasket/apex_driver.c
> index 46199c8ca441..a5dd6f3c367d 100644
> --- a/drivers/staging/gasket/apex_driver.c
> +++ b/drivers/staging/gasket/apex_driver.c
> @@ -247,6 +247,9 @@ module_param(bypass_top_level, int, 0644);
>  static int apex_get_status(struct gasket_dev *gasket_dev)
>  {
>  	/* TODO: Check device status. */
> +	if (gasket_dev->status == GASKET_STATUS_DEAD)
> +		return GASKET_STATUS_DEAD;
> +

Have you tested this to verify that this is what is needed here?

thanks,

greg k-h
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/2] Staging: gasket: implement apex_get_status() to check driver status

[Samuil Ivanov]

On Tue, Oct 29, 2019 at 09:10:07AM +0100, Greg KH wrote:
> On Tue, Oct 29, 2019 at 12:59:25AM +0200, Samuil Ivanov wrote:
> > >From the TODO:
> > - apex_get_status() should actually check status
> > 
> > The function now checkes the status of the driver
> > 
> > Signed-off-by: Samuil Ivanov <samuil.ivanovbg@gmail.com>
> > ---
> >  drivers/staging/gasket/apex_driver.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/staging/gasket/apex_driver.c b/drivers/staging/gasket/apex_driver.c
> > index 46199c8ca441..a5dd6f3c367d 100644
> > --- a/drivers/staging/gasket/apex_driver.c
> > +++ b/drivers/staging/gasket/apex_driver.c
> > @@ -247,6 +247,9 @@ module_param(bypass_top_level, int, 0644);
> >  static int apex_get_status(struct gasket_dev *gasket_dev)
> >  {
> >  	/* TODO: Check device status. */
> > +	if (gasket_dev->status == GASKET_STATUS_DEAD)
> > +		return GASKET_STATUS_DEAD;
> > +
> 
> Have you tested this to verify that this is what is needed here?
> 
> thanks,
> 
> greg k-h

Hello Greg,

After going through the code again, I am sure this not what is needed here.

I thought that gasket_dev->status is already set to either
GASKET_STATUS_ALIVE of GASKET_STATUS_DEAD,
and all I needed to do is check it. It turns out that status
has to be set before that.

So what I found is that function gasket_get_hw_status()
in file gasket_core.c is used to determine the health of the Gasket device,
and other function use it to set gasket_dev->status and then check
the device status.

I think it should be something like this.
...
gasket_dev->status = gasket_get_hw_status(gasket_dev);
if (gasket_dev->status == GASKET_STATUS_DEAD) {
        dev_dbg(gasket_dev->dev, "Device reported as dead.\n");
        return -EINVAL;
}
return GASKET_STATUS_ALIVE;
...

I can try this, but I have no means of testing it.

Grt,
Samuil
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel