| Message ID | 1398966567-5182-1-git-send-email-broonie@kernel.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | c3676d5ceff213cba35af5ee5f320d2e8dc48cdf |
| Headers | show |
On Thu, May 1, 2014 at 7:49 PM, Mark Brown <broonie@kernel.org> wrote: > From: Mark Brown <broonie@linaro.org> > > If we fail to create the master queue for some reason we should not attempt > to clean it up since attempting to stop a kthread that was not created will > hang and it's just generally bad practice. Unfortunately at present we call > spi_destroy_queue() even in cases where the creation fails. > > Fix this by fixing the error handling in spi_master_initialize_queue() so > that we only flag the master as queued or destroy the queue if creation > succeeded. The change to the flag is done since the general master > cleanup uses this to destroy the queue. > > Reported-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> > Signed-off-by: Mark Brown <broonie@linaro.org> > --- > > This has been compile tested only but it should hopefully be more robust > in the long term than just skipping the queue deletion inside the > destructor. > > drivers/spi/spi.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c > index 38ba75a..bc42e27 100644 > --- a/drivers/spi/spi.c > +++ b/drivers/spi/spi.c > @@ -1151,7 +1151,6 @@ static int spi_master_initialize_queue(struct spi_master *master) > { > int ret; > > - master->queued = true; > master->transfer = spi_queued_transfer; > if (!master->transfer_one_message) > master->transfer_one_message = spi_transfer_one_message; > @@ -1162,6 +1161,7 @@ static int spi_master_initialize_queue(struct spi_master *master) > dev_err(&master->dev, "problem initializing queue\n"); > goto err_init_queue; You can just "return ret;" here... > } > + master->queued = true; > ret = spi_start_queue(master); > if (ret) { > dev_err(&master->dev, "problem starting queue\n"); > @@ -1171,8 +1171,8 @@ static int spi_master_initialize_queue(struct spi_master *master) > return 0; > > err_start_queue: > -err_init_queue: > spi_destroy_queue(master); > +err_init_queue: ... and kill the err_init_queue label. > return ret; > } Apart from that: Acked-by: Geert Uytterhoeven <geert@linux-m68k.org> Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, May 01, 2014 at 08:55:42PM +0200, Geert Uytterhoeven wrote: > On Thu, May 1, 2014 at 7:49 PM, Mark Brown <broonie@kernel.org> wrote: > > From: Mark Brown <broonie@linaro.org> > > @@ -1162,6 +1161,7 @@ static int spi_master_initialize_queue(struct spi_master *master) > > dev_err(&master->dev, "problem initializing queue\n"); > > goto err_init_queue; > You can just "return ret;" here... > > err_start_queue: > > -err_init_queue: > > spi_destroy_queue(master); > > +err_init_queue: > ... and kill the err_init_queue label. Yeah, I know. It's a stylistic thing - I tend to prefer to always use labels if they're used at all in a function even if we could return directly since it's more consistent. Not that I'm 100% consistent about that. > Apart from that: > Acked-by: Geert Uytterhoeven <geert@linux-m68k.org> Thanks.
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 38ba75a..bc42e27 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -1151,7 +1151,6 @@ static int spi_master_initialize_queue(struct spi_master *master) { int ret; - master->queued = true; master->transfer = spi_queued_transfer; if (!master->transfer_one_message) master->transfer_one_message = spi_transfer_one_message; @@ -1162,6 +1161,7 @@ static int spi_master_initialize_queue(struct spi_master *master) dev_err(&master->dev, "problem initializing queue\n"); goto err_init_queue; } + master->queued = true; ret = spi_start_queue(master); if (ret) { dev_err(&master->dev, "problem starting queue\n"); @@ -1171,8 +1171,8 @@ static int spi_master_initialize_queue(struct spi_master *master) return 0; err_start_queue: -err_init_queue: spi_destroy_queue(master); +err_init_queue: return ret; }