diff mbox

[1/1] spi: fsl-dspi: Fixes warning due to devm_kzalloc on unbound device

Message ID

1417723755-5372-1-git-send-email-mirza.krak@hostmobility.com (mailing list archive)

State

New, archived

Headers

show

From: Mirza Krak <mirza.krak@hostmobility.com>
To: linux-spi@vger.kernel.org
Cc: Mirza Krak <mirza.krak@hostmobility.com>
Subject: [PATCH 1/1] spi: fsl-dspi: Fixes warning due to devm_kzalloc on
	unbound device
Date: Thu,  4 Dec 2014 21:09:15 +0100
Message-Id: <1417723755-5372-1-git-send-email-mirza.krak@hostmobility.com>
Sender: linux-spi-owner@vger.kernel.org
Precedence: bulk

Commit Message

Mirza Krak Dec. 4, 2014, 8:09 p.m. UTC

------------[ cut here ]------------
WARNING: at drivers/base/dd.c:286 driver_probe_device+0x1b4/0x22c()
Modules linked in:
Backtrace:
[<80011644>] (dump_backtrace) from [<800117e0>] (show_stack+0x18/0x1c)
r6:0000011e r5:00000009 r4:00000000 r3:00000000
[<800117c8>] (show_stack) from [<805961e8>] (dump_stack+0x20/0x28)
[<805961c8>] (dump_stack) from [<80020e48>]
(warn_slowpath_common+0x6c/0x8c)
[<80020ddc>] (warn_slowpath_common) from [<80020e8c>]
(warn_slowpath_null+0x24/0x2c) r8:00000006 r7:7f005b94 r6:8ea1ae34
r5:8ea1ae00 r4:80837290
[<80020e68>] (warn_slowpath_null) from [<802d9078>]
(driver_probe_device+0x1b4/0x22c)
[<802d8ec4>] (driver_probe_device) from [<802d9184>]
(__driver_attach+0x94/0x98) r8:7f00b000 r7:00000000 r6:8ea1ae34
r5:7f005b94 r4:8ea1ae00 r3:00000000
[<802d90f0>] (__driver_attach) from [<802d778c>]
(bus_for_each_dev+0x5c/0x90) r6:802d90f0 r5:7f005b94 r4:00000000
r3:8ea120bc
[<802d7730>] (bus_for_each_dev) from [<802d8acc>]
(driver_attach+0x24/0x28) r6:807c8428 r5:8e335e80 r4:7f005b94
[<802d8aa8>] (driver_attach) from [<802d874c>]
(bus_add_driver+0xdc/0x1d8)
[<802d8670>] (bus_add_driver) from [<802d9814>]
(driver_register+0x80/0xfc) r7:8e307d00 r6:8e307e00 r5:807a93d8
r4:7f005b94
[<802d9794>] (driver_register) from [<8034f410>]
(spi_register_driver+0x4c/0x60) r5:807a93d8 r4:807a93d8
[<8034f3c4>] (spi_register_driver) from [<7f00b01c>]
(mx4_io_init+0x1c/0x28 [spi_pic])
[<7f00b000>] (mx4_io_init [spi_pic]) from [<8000874c>]
(do_one_initcall+0x88/0x1cc)
[<800086c4>] (do_one_initcall) from [<800638ac>]
(load_module+0x1634/0x1d10) r10:800611c0 r9:7f006174 r8:00000001
r7:8e307d00 r6:00000001 r5:7f006180 r4:8e12bf48
[<80062278>] (load_module) from [<80064068>] (SyS_init_module+0xe0/0xf4)
r10:00000000 r9:8e12a000 r8:8000e9e4 r7:00000080 r6:00884008 r5:00884018
r4:0000cb5d
[<80063f88>] (SyS_init_module) from [<8000e840>]
(ret_fast_syscall+0x0/0x30) r6:7eee4e53 r5:7eee4e53 r4:0000cb5d
---[ end trace 9c516d4cef36cef3 ]---

Inspired by: 10aa5a35e34f ("SPI: fix over-eager devm_xxx()
conversion")

Signed-off-by: Mirza Krak <mirza.krak@hostmobility.com>
---
 drivers/spi/spi-fsl-dspi.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Comments

Mirza Krak Dec. 23, 2014, 8:57 a.m. UTC | #1

Hi!

Any feedback on this?

Best Regards / Mirza

On Thu,  4 Dec 2014 21:09:15 +0100
Mirza Krak <mirza.krak@hostmobility.com> wrote:

> ------------[ cut here ]------------
> WARNING: at drivers/base/dd.c:286 driver_probe_device+0x1b4/0x22c()
> Modules linked in:
> Backtrace:
> [<80011644>] (dump_backtrace) from [<800117e0>] (show_stack+0x18/0x1c)
> r6:0000011e r5:00000009 r4:00000000 r3:00000000
> [<800117c8>] (show_stack) from [<805961e8>] (dump_stack+0x20/0x28)
> [<805961c8>] (dump_stack) from [<80020e48>]
> (warn_slowpath_common+0x6c/0x8c)
> [<80020ddc>] (warn_slowpath_common) from [<80020e8c>]
> (warn_slowpath_null+0x24/0x2c) r8:00000006 r7:7f005b94 r6:8ea1ae34
> r5:8ea1ae00 r4:80837290
> [<80020e68>] (warn_slowpath_null) from [<802d9078>]
> (driver_probe_device+0x1b4/0x22c)
> [<802d8ec4>] (driver_probe_device) from [<802d9184>]
> (__driver_attach+0x94/0x98) r8:7f00b000 r7:00000000 r6:8ea1ae34
> r5:7f005b94 r4:8ea1ae00 r3:00000000
> [<802d90f0>] (__driver_attach) from [<802d778c>]
> (bus_for_each_dev+0x5c/0x90) r6:802d90f0 r5:7f005b94 r4:00000000
> r3:8ea120bc
> [<802d7730>] (bus_for_each_dev) from [<802d8acc>]
> (driver_attach+0x24/0x28) r6:807c8428 r5:8e335e80 r4:7f005b94
> [<802d8aa8>] (driver_attach) from [<802d874c>]
> (bus_add_driver+0xdc/0x1d8)
> [<802d8670>] (bus_add_driver) from [<802d9814>]
> (driver_register+0x80/0xfc) r7:8e307d00 r6:8e307e00 r5:807a93d8
> r4:7f005b94
> [<802d9794>] (driver_register) from [<8034f410>]
> (spi_register_driver+0x4c/0x60) r5:807a93d8 r4:807a93d8
> [<8034f3c4>] (spi_register_driver) from [<7f00b01c>]
> (mx4_io_init+0x1c/0x28 [spi_pic])
> [<7f00b000>] (mx4_io_init [spi_pic]) from [<8000874c>]
> (do_one_initcall+0x88/0x1cc)
> [<800086c4>] (do_one_initcall) from [<800638ac>]
> (load_module+0x1634/0x1d10) r10:800611c0 r9:7f006174 r8:00000001
> r7:8e307d00 r6:00000001 r5:7f006180 r4:8e12bf48
> [<80062278>] (load_module) from [<80064068>]
> (SyS_init_module+0xe0/0xf4) r10:00000000 r9:8e12a000 r8:8000e9e4
> r7:00000080 r6:00884008 r5:00884018 r4:0000cb5d
> [<80063f88>] (SyS_init_module) from [<8000e840>]
> (ret_fast_syscall+0x0/0x30) r6:7eee4e53 r5:7eee4e53 r4:0000cb5d
> ---[ end trace 9c516d4cef36cef3 ]---
> 
> Inspired by: 10aa5a35e34f ("SPI: fix over-eager devm_xxx()
> conversion")
> 
> Signed-off-by: Mirza Krak <mirza.krak@hostmobility.com>
> ---
>  drivers/spi/spi-fsl-dspi.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
> index 831ceb4..78509d6 100644
> --- a/drivers/spi/spi-fsl-dspi.c
> +++ b/drivers/spi/spi-fsl-dspi.c
> @@ -342,7 +342,7 @@ static int dspi_setup_transfer(struct spi_device
> *spi, struct spi_transfer *t) /* Only alloc on first setup */
>  	chip = spi_get_ctldata(spi);
>  	if (chip == NULL) {
> -		chip = devm_kzalloc(&spi->dev, sizeof(struct
> chip_data),
> +		chip = kzalloc(sizeof(struct chip_data), GFP_KERNEL);
>  				    GFP_KERNEL);
>  		if (!chip)
>  			return -ENOMEM;
> @@ -382,6 +382,11 @@ static int dspi_setup(struct spi_device *spi)
>  	return dspi_setup_transfer(spi, NULL);
>  }
>  
> +static void dspi_cleanup(struct spi_device *spi)
> +{
> +	spi_bitbang_cleanup(spi);
> +}
> +
>  static irqreturn_t dspi_interrupt(int irq, void *dev_id)
>  {
>  	struct fsl_dspi *dspi = (struct fsl_dspi *)dev_id;
> @@ -465,6 +470,7 @@ static int dspi_probe(struct platform_device
> *pdev) dspi->bitbang.setup_transfer = dspi_setup_transfer;
>  	dspi->bitbang.txrx_bufs = dspi_txrx_transfer;
>  	dspi->bitbang.master->setup = dspi_setup;
> +	dspi->bitbang.master->cleanup = dspi_cleanup;
>  	dspi->bitbang.master->dev.of_node = pdev->dev.of_node;
>  
>  	master->mode_bits = SPI_CPOL | SPI_CPHA;

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

Mark Brown May 25, 2015, 2:10 p.m. UTC | #2

On Tue, Dec 23, 2014 at 09:57:03AM +0100, Mirza Krak wrote:
> Hi!
> 
> Any feedback on this?

Please don't top post or send content free pings, if you want to submit
patches please always send them to the maintainer as covered in
SubmittingPatches.

Mark Brown May 25, 2015, 2:12 p.m. UTC | #3

On Thu, Dec 04, 2014 at 09:09:15PM +0100, Mirza Krak wrote:

> ------------[ cut here ]------------
> WARNING: at drivers/base/dd.c:286 driver_probe_device+0x1b4/0x22c()
> Modules linked in:
> Backtrace:

Please don't paste entire backtraces into commit messages, they're large
and make it hard to find the content - if one  is useful please paste
the relevant portions only.

> Inspired by: 10aa5a35e34f ("SPI: fix over-eager devm_xxx()
> conversion")
> 
> Signed-off-by: Mirza Krak <mirza.krak@hostmobility.com>

Your commit message should describe what the problem was and how it is
being fixed (ie, describe the change) rather than simply stating that it
is fixing an issue unless the change is really obvious.

> @@ -342,7 +342,7 @@ static int dspi_setup_transfer(struct spi_device *spi, struct spi_transfer *t)
>  	/* Only alloc on first setup */
>  	chip = spi_get_ctldata(spi);
>  	if (chip == NULL) {
> -		chip = devm_kzalloc(&spi->dev, sizeof(struct chip_data),
> +		chip = kzalloc(sizeof(struct chip_data), GFP_KERNEL);
>  				    GFP_KERNEL);
>  		if (!chip)
>  			return -ENOMEM;

This is good.

> @@ -382,6 +382,11 @@ static int dspi_setup(struct spi_device *spi)
>  	return dspi_setup_transfer(spi, NULL);
>  }
>  
> +static void dspi_cleanup(struct spi_device *spi)
> +{
> +	spi_bitbang_cleanup(spi);
> +}
> +
>  static irqreturn_t dspi_interrupt(int irq, void *dev_id)
>  {
>  	struct fsl_dspi *dspi = (struct fsl_dspi *)dev_id;
> @@ -465,6 +470,7 @@ static int dspi_probe(struct platform_device *pdev)
>  	dspi->bitbang.setup_transfer = dspi_setup_transfer;
>  	dspi->bitbang.txrx_bufs = dspi_txrx_transfer;
>  	dspi->bitbang.master->setup = dspi_setup;
> +	dspi->bitbang.master->cleanup = dspi_cleanup;
>  	dspi->bitbang.master->dev.of_node = pdev->dev.of_node;

Why not just assign spi_bitbang_cleanup() directly?

Mirza Krak May 25, 2015, 3 p.m. UTC | #4

2015-05-25 16:10 GMT+02:00 Mark Brown <broonie@kernel.org>:
> On Tue, Dec 23, 2014 at 09:57:03AM +0100, Mirza Krak wrote:
>> Hi!
>>
>> Any feedback on this?
>
> Please don't top post or send content free pings, if you want to submit
> patches please always send them to the maintainer as covered in
> SubmittingPatches.

Realize my error now.

Mirza Krak May 25, 2015, 3:04 p.m. UTC | #5

2015-05-25 16:12 GMT+02:00 Mark Brown <broonie@kernel.org>:
> On Thu, Dec 04, 2014 at 09:09:15PM +0100, Mirza Krak wrote:
>
>> ------------[ cut here ]------------
>> WARNING: at drivers/base/dd.c:286 driver_probe_device+0x1b4/0x22c()
>> Modules linked in:
>> Backtrace:
>
> Please don't paste entire backtraces into commit messages, they're large
> and make it hard to find the content - if one  is useful please paste
> the relevant portions only.
>
>> Inspired by: 10aa5a35e34f ("SPI: fix over-eager devm_xxx()
>> conversion")
>>
>> Signed-off-by: Mirza Krak <mirza.krak@hostmobility.com>
>
> Your commit message should describe what the problem was and how it is
> being fixed (ie, describe the change) rather than simply stating that it
> is fixing an issue unless the change is really obvious.
>
>> @@ -342,7 +342,7 @@ static int dspi_setup_transfer(struct spi_device *spi, struct spi_transfer *t)
>>       /* Only alloc on first setup */
>>       chip = spi_get_ctldata(spi);
>>       if (chip == NULL) {
>> -             chip = devm_kzalloc(&spi->dev, sizeof(struct chip_data),
>> +             chip = kzalloc(sizeof(struct chip_data), GFP_KERNEL);
>>                                   GFP_KERNEL);
>>               if (!chip)
>>                       return -ENOMEM;
>
> This is good.
>
>> @@ -382,6 +382,11 @@ static int dspi_setup(struct spi_device *spi)
>>       return dspi_setup_transfer(spi, NULL);
>>  }
>>
>> +static void dspi_cleanup(struct spi_device *spi)
>> +{
>> +     spi_bitbang_cleanup(spi);
>> +}
>> +
>>  static irqreturn_t dspi_interrupt(int irq, void *dev_id)
>>  {
>>       struct fsl_dspi *dspi = (struct fsl_dspi *)dev_id;
>> @@ -465,6 +470,7 @@ static int dspi_probe(struct platform_device *pdev)
>>       dspi->bitbang.setup_transfer = dspi_setup_transfer;
>>       dspi->bitbang.txrx_bufs = dspi_txrx_transfer;
>>       dspi->bitbang.master->setup = dspi_setup;
>> +     dspi->bitbang.master->cleanup = dspi_cleanup;
>>       dspi->bitbang.master->dev.of_node = pdev->dev.of_node;
>
> Why not just assign spi_bitbang_cleanup() directly?

Thank you for reviewing this, but this has been fixed already [1], he
probably did not top post :). We can NAK this.

[1] https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/spi/spi-fsl-dspi.c?id=973fbce69ed8e79b5fe3ad19cfecb581a7ef8048

diff mbox

Patch

diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index 831ceb4..78509d6 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -342,7 +342,7 @@  static int dspi_setup_transfer(struct spi_device *spi, struct spi_transfer *t)
 	/* Only alloc on first setup */
 	chip = spi_get_ctldata(spi);
 	if (chip == NULL) {
-		chip = devm_kzalloc(&spi->dev, sizeof(struct chip_data),
+		chip = kzalloc(sizeof(struct chip_data), GFP_KERNEL);
 				    GFP_KERNEL);
 		if (!chip)
 			return -ENOMEM;
@@ -382,6 +382,11 @@  static int dspi_setup(struct spi_device *spi)
 	return dspi_setup_transfer(spi, NULL);
 }
 
+static void dspi_cleanup(struct spi_device *spi)
+{
+	spi_bitbang_cleanup(spi);
+}
+
 static irqreturn_t dspi_interrupt(int irq, void *dev_id)
 {
 	struct fsl_dspi *dspi = (struct fsl_dspi *)dev_id;
@@ -465,6 +470,7 @@  static int dspi_probe(struct platform_device *pdev)
 	dspi->bitbang.setup_transfer = dspi_setup_transfer;
 	dspi->bitbang.txrx_bufs = dspi_txrx_transfer;
 	dspi->bitbang.master->setup = dspi_setup;
+	dspi->bitbang.master->cleanup = dspi_cleanup;
 	dspi->bitbang.master->dev.of_node = pdev->dev.of_node;
 
 	master->mode_bits = SPI_CPOL | SPI_CPHA;