Re: [PATCH RFC] staging: fieldbus: anybus-s: use proper type for wait_for_completion_timeout

2019-04-27 Thread Nicholas Mc Guire
On Sat, Apr 27, 2019 at 02:17:42AM -0400, Sven Van Asbroeck wrote:
> Hello Nicholas, thank you for your contribution, I really appreciate it !
> See inline comments below.
> 
> On Sat, Apr 27, 2019 at 12:32 AM Nicholas Mc Guire  wrote:
> >
> > wait_for_completion_timeout() returns unsigned long (0 on timeout or
> > remaining jiffies) not int.
> 
> Nice catch !
> 
> > thus there is no negative case to check for
> > here.
> 
> The current code can only break if wait_for_completion_timeout()
> returns an unsigned long large enough to make the "int ret" turn
> negative. This could result in probe() returning a nonsensical error
> value, even though the wait did not time out.
> 
> Fortunately that currently cannot happen, because TIMEOUT
> (2*HZ) won't overflow an int.

ok - thats benign then -  never the less since code tends to get copied 
it would be good to make it comply with API spec

> 
> That being said, this return value type mismatch should definitely
> be fixed up.
> 
> >
> > Signed-off-by: Nicholas Mc Guire 
> > ---
> >
> > Problem located with experimental API conformance checking cocci script
> >
> > Q: It is not really clear if the proposed fix is the right thing here or if
> >this should not be using wait_for_completion_interruptible - which would
> >return -ERESTARTSYS on interruption. Someone that knows the details of
> >this driver would need to check what condition should initiate the
> >goto err_reset; which was actually unreachable in the current code.
> 
> It's used in probe(), so no need for an interruptible wait, AFAIK.
> It can stay as-is.
> 
> >
> > Patch was compile-tested with. x86_64_defconfig + FIELDBUS_DEV=m,
> > HMS_ANYBUSS_BUS=m
> > (some unrelated sparse warnings (cast to restricted __be16))
> 
> That sounds interesting too. Could you provide more details?

make C=1
drivers/staging/fieldbus/anybuss/host.c:1350:25: warning: cast to restricted 
__be16
drivers/staging/fieldbus/anybuss/host.c:1350:25: warning: cast to restricted 
__be16
drivers/staging/fieldbus/anybuss/host.c:1350:25: warning: cast to restricted 
__be16
drivers/staging/fieldbus/anybuss/host.c:1350:25: warning: cast to restricted 
__be16
drivers/staging/fieldbus/anybuss/host.c:1350:25: warning: cast to restricted


> 
> 
> 
> > -   ret = wait_for_completion_timeout(&cd->card_boot, TIMEOUT);
> > -   if (ret == 0)
> > +   time_left = wait_for_completion_timeout(&cd->card_boot, TIMEOUT);
> > +   if (time_left == 0)
> > ret = -ETIMEDOUT;
> > -   if (ret < 0)
> > -   goto err_reset;
> 
> NAK. This does not jump to err_reset on timeout.
> 
> May I suggest the following instead ? (manual formatting)
> 
> - ret = wait_for_completion_timeout(&cd->card_boot, TIMEOUT);
> - if (ret == 0)
> - ret = -ETIMEDOUT;
> - if (ret < 0)
> - goto err_reset;
> + if (!wait_for_completion_timeout(&cd->card_boot, TIMEOUT)) {
> + ret = -ETIMEDOUT;
> + goto err_reset;
> + }

Ah - ok - that was the bit missing for me !
how that goto branch would be reached or if it should be
dropped as it had not been reachable in the past.

I'll send the V2 later today then.

thx!
hofrat

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


Re: [PATCH RFC] staging: fieldbus: anybus-s: use proper type for wait_for_completion_timeout

2019-04-27 Thread Sven Van Asbroeck
On Sat, Apr 27, 2019 at 3:01 AM Nicholas Mc Guire  wrote:
> > > (some unrelated sparse warnings (cast to restricted __be16))
> >
> > That sounds interesting too. Could you provide more details?
>
> make C=1
> drivers/staging/fieldbus/anybuss/host.c:1350:25: warning: cast to restricted 
> __be16
> drivers/staging/fieldbus/anybuss/host.c:1350:25: warning: cast to restricted 
> __be16
> drivers/staging/fieldbus/anybuss/host.c:1350:25: warning: cast to restricted 
> __be16
> drivers/staging/fieldbus/anybuss/host.c:1350:25: warning: cast to restricted 
> __be16
> drivers/staging/fieldbus/anybuss/host.c:1350:25: warning: cast to restricted

regmap_bulk_read(cd->regmap, REG_FIELDBUS_TYPE, &fieldbus_type,
sizeof(fieldbus_type));
fieldbus_type = be16_to_cpu(fieldbus_type);

Probably because the parameter to be16_to_cpu() should be __be16.
Would you like to spin a separate patch for this too? Or shall I?

> I'll send the V2 later today then.

Thank you !
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH RFC V2] staging: kpc2000: use int for wait_for_completion_interruptible

2019-04-27 Thread Nicholas Mc Guire
weit_for_completion_interruptible returns in (0 on completion and 
-ERESTARTSYS on interruption) - so use an int not long for API conformance
and simplify the logic here a bit: need not check explicitly for == 0 as
this is either -ERESTARTSYS or 0.

Signed-off-by: Nicholas Mc Guire 
---

Problem located with experimental API conformance checking cocci script

V2: kbuild reported a missing closing comment - seems that I managed
send out the the initial version before compile testing/checkpatching
sorry for the noise.

Not sure if making such point-wise fixes makes much sense - this driver has
a number of issues both style-wise and API compliance wise.

Note that kpc_dma_transfer() returns int not long - currently rv (long) is
being returned in most places (some places do return int) - so the return
handling here is a bit inconsistent.

Patch was compile-tested with: x86_64_defconfig + KPC2000=y, KPC2000_DMA=y
(with a number of unrelated sparse warnings about non-declared symbols, and
 smatch warnings about overflowing constants as well as coccicheck warning
 about usless casting)

Patch is against 5.1-rc6 (localversion-next is next-20190426)

 drivers/staging/kpc2000/kpc_dma/fileops.c | 16 +---
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/kpc2000/kpc_dma/fileops.c 
b/drivers/staging/kpc2000/kpc_dma/fileops.c
index 5741d2b..66f0d5a 100644
--- a/drivers/staging/kpc2000/kpc_dma/fileops.c
+++ b/drivers/staging/kpc2000/kpc_dma/fileops.c
@@ -38,6 +38,7 @@ int  kpc_dma_transfer(struct dev_private_data *priv, struct 
kiocb *kcb, unsigned
 {
unsigned int i = 0;
long rv = 0;
+   int ret = 0;
struct kpc_dma_device *ldev;
struct aio_cb_data *acd;
DECLARE_COMPLETION_ONSTACK(done);
@@ -180,16 +181,17 @@ int  kpc_dma_transfer(struct dev_private_data *priv, 
struct kiocb *kcb, unsigned

// If this is a synchronous kiocb, we need to put the calling process 
to sleep until the transfer is complete
if (kcb == NULL || is_sync_kiocb(kcb)){
-   rv = wait_for_completion_interruptible(&done);
-   // If the user aborted (rv == -ERESTARTSYS), we're no longer 
responsible for cleaning up the acd
-   if (rv == -ERESTARTSYS){
+   ret = wait_for_completion_interruptible(&done);
+   /* If the user aborted (ret == -ERESTARTSYS), we're
+* no longer responsible for cleaning up the acd
+*/
+   if (ret) {
acd->cpl = NULL;
-   }
-   if (rv == 0){
-   rv = acd->len;
+   } else {
+   ret = acd->len;
kfree(acd);
}
-   return rv;
+   return ret;
}

return -EIOCBQUEUED;
-- 
2.1.4

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


Re: [PATCH RFC] staging: fieldbus: anybus-s: use proper type for wait_for_completion_timeout

2019-04-27 Thread Nicholas Mc Guire
On Sat, Apr 27, 2019 at 03:20:54AM -0400, Sven Van Asbroeck wrote:
> On Sat, Apr 27, 2019 at 3:01 AM Nicholas Mc Guire  wrote:
> > > > (some unrelated sparse warnings (cast to restricted __be16))
> > >
> > > That sounds interesting too. Could you provide more details?
> >
> > make C=1
> > drivers/staging/fieldbus/anybuss/host.c:1350:25: warning: cast to 
> > restricted __be16
> > drivers/staging/fieldbus/anybuss/host.c:1350:25: warning: cast to 
> > restricted __be16
> > drivers/staging/fieldbus/anybuss/host.c:1350:25: warning: cast to 
> > restricted __be16
> > drivers/staging/fieldbus/anybuss/host.c:1350:25: warning: cast to 
> > restricted __be16
> > drivers/staging/fieldbus/anybuss/host.c:1350:25: warning: cast to restricted
> 
> regmap_bulk_read(cd->regmap, REG_FIELDBUS_TYPE, &fieldbus_type,
i> sizeof(fieldbus_type));
> fieldbus_type = be16_to_cpu(fieldbus_type);
> 
> Probably because the parameter to be16_to_cpu() should be __be16.
> Would you like to spin a separate patch for this too? Or shall I?

so the issue is simply that the endiannes anotatoin is missing even 
though the conversion is being done - with other words there is no code
lvel funcitonal bug here but rather sparse needs the anotation to verify
correctness and that is missing. Just want to make sure I understand
this before I try to "fix" a sparse warning.

thx!
hofrat
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: comedi: comedi_isadma: Use a non-NULL device for DMA API

2019-04-27 Thread Greg Kroah-Hartman
On Fri, Apr 26, 2019 at 10:41:20AM +0100, Ian Abbott wrote:
> On 25/04/2019 18:13, Greg Kroah-Hartman wrote:
> > On Thu, Apr 25, 2019 at 05:26:44PM +0100, Ian Abbott wrote:
> > > The "comedi_isadma" module calls `dma_alloc_coherent()` and
> > > `dma_free_coherent()` with a NULL device pointer which is no longer
> > > allowed.  If the `hw_dev` member of the `struct comedi_device` has been
> > > set to a valid device, that can be used instead.  Unfortunately, all the
> > > current users of the "comedi_isadma" module leave the `hw_dev` member
> > > set to NULL.  In that case, use a static dummy fallback device structure
> > > with the coherent DMA mask set to the ISA bus limit of 16MB.
> > > 
> > > Signed-off-by: Ian Abbott 
> > > ---
> > >   drivers/staging/comedi/drivers/comedi_isadma.c | 15 +--
> > >   drivers/staging/comedi/drivers/comedi_isadma.h |  3 +++
> > >   2 files changed, 16 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/staging/comedi/drivers/comedi_isadma.c 
> > > b/drivers/staging/comedi/drivers/comedi_isadma.c
> > > index b77dc8d5d3ff..8929952516a1 100644
> > > --- a/drivers/staging/comedi/drivers/comedi_isadma.c
> > > +++ b/drivers/staging/comedi/drivers/comedi_isadma.c
> > > @@ -14,6 +14,16 @@
> > >   #include "comedi_isadma.h"
> > > +/*
> > > + * Fallback device used when hardware device is NULL.
> > > + * This can be removed after drivers have been converted to use 
> > > isa_driver.
> > > + */
> > > +static struct device fallback_dev = {
> > > + .init_name = "comedi_isadma fallback device",
> > > + .coherent_dma_mask = DMA_BIT_MASK(24),
> > > + .dma_mask = &fallback_dev.coherent_dma_mask,
> > > +};
> > 
> > Ick, no, static struct device are a very bad idea as this is a reference
> > counted structure and making it static can cause odd problems.
> 
> This was based on the use of `struct device x86_dma_fallback_dev` in
> "arch/x86/kernel/pci-dma.c", and `static struct device isa_dma_dev` in
> "arch/arm/kernel/dma-isa.c", but perhaps it is not appropriate in non-arch
> code.

No, those are probably broken as well :)

Ah, I see why, ugh, ISA.

> > Why not just create a "real" one?  Or better yet, use the real device
> > for the comedi device as all of these drivers should have one now.
> 
> I suppose I could use the comedi class device pointed to by the `class_dev`
> member of `struct comedi_device` (although that could also be NULL because
> the comedi core does not currently treat `device_create()` failures as
> fatal).

Why would device_create() fail?  If it does, the driver should not have
been bound to anything.

And no, this shouldn't be the class device, but the "real" hardware
device that does dma, which is what is needed anyway here to allow the
DMA to happen properly.

I guess the problem is with the ISA drivers, right?  And I doubt that is
ever going to get fixed up, hopefully just deleted :)

Your second patch is "good enough", I'll go queue that up now.

thanks,

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


Re: [PATCH RFC] staging: fieldbus: anybus-s: use proper type for wait_for_completion_timeout

2019-04-27 Thread Sven Van Asbroeck
On Sat, Apr 27, 2019 at 7:18 AM Nicholas Mc Guire  wrote:
>
> so the issue is simply that the endiannes anotatoin is missing even
> though the conversion is being done - with other words there is no code
> lvel funcitonal bug here but rather sparse needs the anotation to verify
> correctness and that is missing. Just want to make sure I understand
> this before I try to "fix" a sparse warning.

Correct.

Thanks,
Sven
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] staging: vc04_services: bcm2835-camera: Modify return statement.

2019-04-27 Thread Vatsala Narang
Modify return statement and remove the respective assignment.

Issue found by Coccinelle.

Signed-off-by: Vatsala Narang 
---
 drivers/staging/vc04_services/bcm2835-camera/controls.c | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/vc04_services/bcm2835-camera/controls.c 
b/drivers/staging/vc04_services/bcm2835-camera/controls.c
index 2e0a422cdf3e..9841c30450ce 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/controls.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/controls.c
@@ -270,11 +270,9 @@ static int ctrl_set_rotate(struct bm2835_mmal_dev *dev,
if (ret < 0)
return ret;
 
-   ret = vchiq_mmal_port_parameter_set(dev->instance, &camera->output[2],
+   return vchiq_mmal_port_parameter_set(dev->instance, &camera->output[2],
mmal_ctrl->mmal_id,
&u32_value, sizeof(u32_value));
-
-   return ret;
 }
 
 static int ctrl_set_flip(struct bm2835_mmal_dev *dev,
@@ -313,11 +311,9 @@ static int ctrl_set_flip(struct bm2835_mmal_dev *dev,
if (ret < 0)
return ret;
 
-   ret = vchiq_mmal_port_parameter_set(dev->instance, &camera->output[2],
+   return vchiq_mmal_port_parameter_set(dev->instance, &camera->output[2],
mmal_ctrl->mmal_id,
&u32_value, sizeof(u32_value));
-
-   return ret;
 }
 
 static int ctrl_set_exposure(struct bm2835_mmal_dev *dev,
-- 
2.17.1

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


[PATCH] Staging: sm750fb: Change *array into *const array

2019-04-27 Thread Kelsey Skunberg
Resolve checkpatch warning for static const char * array by using const
pointers.

Checkpatch Warning in sm750.c:
static const char * array should probably be static const char * const

Signed-off-by: Kelsey Skunberg 
---
 drivers/staging/sm750fb/sm750.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index 105089b97bf5..59568d18ce23 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -748,7 +748,7 @@ static int lynxfb_set_fbinfo(struct fb_info *info, int 
index)
lynx750_ext, NULL, vesa_modes,
};
int cdb[] = {ARRAY_SIZE(lynx750_ext), 0, VESA_MODEDB_SIZE};
-   static const char *mdb_desc[] = {
+   static const char * const mdb_desc[] = {
"driver prepared modes",
"kernel prepared default modedb",
"kernel HELPERS prepared vesa_modes",
-- 
2.20.1

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


[PATCH 2/2] staging: kpc2000_spi: eliminated duplicate initialization of master local variable.

2019-04-27 Thread Jeremy Sowden
master was being initialized to a particular value and then having the
same value assigned to it immediately afterwards.  Removed the
initializer.

Since the value assigned to master was dynamically allocated, this fixes
a memory-leak.

Signed-off-by: Jeremy Sowden 
---
 drivers/staging/kpc2000/kpc_spi/spi_driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/kpc2000/kpc_spi/spi_driver.c 
b/drivers/staging/kpc2000/kpc_spi/spi_driver.c
index e77f04bf02d9..c0999e080577 100644
--- a/drivers/staging/kpc2000/kpc_spi/spi_driver.c
+++ b/drivers/staging/kpc2000/kpc_spi/spi_driver.c
@@ -408,7 +408,7 @@ static int
 kp_spi_probe(struct platform_device *pldev)
 {
 struct kpc_core_device_platdata *drvdata;
-struct spi_master *master = spi_alloc_master(&pldev->dev, sizeof(struct 
kp_spi));
+struct spi_master *master;
 struct kp_spi *kpspi;
 struct resource *r;
 int status = 0;
-- 
2.20.1

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


[PATCH 0/2] staging: kpc2000_spi: eliminated duplicate initialization of two local variables.

2019-04-27 Thread Jeremy Sowden
A couple of the local variables in kp_spi_probe had initializers which were
being overwritten by immediate assignment of the same values.  One of them was
dynamically allocated and so would leak memory.

Checkpatch whinges about the formatting, but fixing that would be rather a large
job.

Jeremy Sowden (2):
  staging: kpc2000_spi: eliminated duplicate initialization of drvdata local
variable.
  staging: kpc2000_spi: eliminated duplicate initialization of master local
variable.

 drivers/staging/kpc2000/kpc_spi/spi_driver.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

-- 
2.20.1

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


[PATCH 1/2] staging: kpc2000_spi: eliminated duplicate initialization of drvdata local variable.

2019-04-27 Thread Jeremy Sowden
drvdata was being initialized to a particular value and then
having the same value assigned to it immediately afterwards.  Removed
the initializer.

Since the value assigned, pldev->dev.platform_data, is a pointer-to-
void, removed superfluous cast.

Signed-off-by: Jeremy Sowden 
---
 drivers/staging/kpc2000/kpc_spi/spi_driver.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/kpc2000/kpc_spi/spi_driver.c 
b/drivers/staging/kpc2000/kpc_spi/spi_driver.c
index 63b4616bf538..e77f04bf02d9 100644
--- a/drivers/staging/kpc2000/kpc_spi/spi_driver.c
+++ b/drivers/staging/kpc2000/kpc_spi/spi_driver.c
@@ -407,14 +407,14 @@ kp_spi_cleanup(struct spi_device *spidev)
 static int
 kp_spi_probe(struct platform_device *pldev)
 {
-struct kpc_core_device_platdata *drvdata = (struct 
kpc_core_device_platdata *)pldev->dev.platform_data;
+struct kpc_core_device_platdata *drvdata;
 struct spi_master *master = spi_alloc_master(&pldev->dev, sizeof(struct 
kp_spi));
 struct kp_spi *kpspi;
 struct resource *r;
 int status = 0;
 int i;
 
-drvdata = (struct kpc_core_device_platdata *)pldev->dev.platform_data;
+drvdata = pldev->dev.platform_data;
 if (!drvdata){
 dev_err(&pldev->dev, "kp_spi_probe: platform_data is NULL!\n");
 return -ENODEV;
-- 
2.20.1

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


Re: [PATCH] staging: vc04_services: bcm2835-camera: Modify return statement.

2019-04-27 Thread Stefan Wahren
Am 27.04.19 um 18:07 schrieb Vatsala Narang:
> Modify return statement and remove the respective assignment.
>
> Issue found by Coccinelle.
>
> Signed-off-by: Vatsala Narang 
Acked-by: Stefan Wahren 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v3] Staging: vc04_services: Cleanup in ctrl_set_bitrate()

2019-04-27 Thread Stefan Wahren
Am 23.04.19 um 16:47 schrieb Madhumitha Prabakaran:
> Remove unnecessary variable from the function and make a corresponding
> change w.r.t the variable. In addition to that align the parameters in
> the parentheses to maintain Linux kernel coding style
>
> Issue suggested by Coccinelle.
>
> Signed-off-by: Madhumitha Prabakaran 
Acked-by: Stefan Wahren 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] staging: fieldbus: anybus-s: force endiannes annotation

2019-04-27 Thread Nicholas Mc Guire
While the endiannes is being handled correctly sparse was unhappy with
the missing annotation as be16_to_cpu() expects a __be16. 

Signed-off-by: Nicholas Mc Guire 
---

Problem reported by sparse

As far as I understand sparse here the __force is actually the only 
solution possible to inform sparse that the endiannes handling is ok

Patch was compile-tested with. x86_64_defconfig + FIELDBUS_DEV=m,
HMS_ANYBUSS_BUS=m

Patch is against 5.1-rc6 (localversion-next is next-20190426)

 drivers/staging/fieldbus/anybuss/host.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/fieldbus/anybuss/host.c 
b/drivers/staging/fieldbus/anybuss/host.c
index 6227daf..278acac 100644
--- a/drivers/staging/fieldbus/anybuss/host.c
+++ b/drivers/staging/fieldbus/anybuss/host.c
@@ -1348,7 +1348,7 @@ anybuss_host_common_probe(struct device *dev,
add_device_randomness(&val, 4);
regmap_bulk_read(cd->regmap, REG_FIELDBUS_TYPE, &fieldbus_type,
 sizeof(fieldbus_type));
-   fieldbus_type = be16_to_cpu(fieldbus_type);
+   fieldbus_type = be16_to_cpu((__force __be16)fieldbus_type);
dev_info(dev, "Fieldbus type: %04X", fieldbus_type);
regmap_bulk_read(cd->regmap, REG_MODULE_SW_V, val, 2);
dev_info(dev, "Module SW version: %02X%02X",
-- 
2.1.4

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


[PATCH V2] staging: fieldbus: anybus-s: consolidate wait_for_completion_timeout return handling

2019-04-27 Thread Nicholas Mc Guire
wait_for_completion_timeout() returns unsigned long (0 on timeout or
remaining jiffies) not int - so rather than introducing an additional
variable simply wrap the completion into an if().

Signed-off-by: Nicholas Mc Guire 
---

Problem located with experimental API conformance checking cocci script

V2: The original patch's logic was wrong as it was skipping the 
fall-through if so using the fix proposed by Sven Van Asbroeck 
 - This solution also eliminates the need
to introduce an additional variable - Thanks !

Patch was compile-tested with. x86_64_defconfig + FIELDBUS_DEV=m,
HMS_ANYBUSS_BUS=m
(with an unrelated sparse warnings (cast to restricted __be16))

Patch is against 5.1-rc6 (localversion-next is next-20190426)

 drivers/staging/fieldbus/anybuss/host.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/fieldbus/anybuss/host.c 
b/drivers/staging/fieldbus/anybuss/host.c
index e34d424..6227daf 100644
--- a/drivers/staging/fieldbus/anybuss/host.c
+++ b/drivers/staging/fieldbus/anybuss/host.c
@@ -1325,11 +1325,11 @@ anybuss_host_common_probe(struct device *dev,
 *   interrupt came in: ready to go !
 */
reset_deassert(cd);
-   ret = wait_for_completion_timeout(&cd->card_boot, TIMEOUT);
-   if (ret == 0)
+   if (!wait_for_completion_timeout(&cd->card_boot, TIMEOUT)) {
ret = -ETIMEDOUT;
-   if (ret < 0)
goto err_reset;
+   }
+
/*
 * according to the anybus docs, we're allowed to read these
 * without handshaking / reserving the area
-- 
2.1.4

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