All of lore.kernel.org
 help / color / mirror / Atom feed
From: Timur Tabi <timur@freescale.com>
To: Dan Williams <dan.j.williams@intel.com>
Cc: linux-kernel@vger.kernel.org,
	Andrew Morton <akpm@linux-foundation.org>,
	leoli@freescale.com
Subject: Re: [PATCH] fsl-dma: allow Freescale Elo DMA driver to be compiled as a module
Date: Mon, 22 Sep 2008 15:59:59 -0500	[thread overview]
Message-ID: <48D8074F.9030106@freescale.com> (raw)
In-Reply-To: <e9c3a7c20809201432y31da1e00mf3a20c60b918fab1@mail.gmail.com>

Dan Williams wrote:

> The last three hunks should be broken out into a separate 'fix' patch
> with its own changelog.

Can you be more specific?  I can see how "return 0" -> "return -ENOMEM" could be
separated out, but the Kconfig hunk is integral to the patch, and the "if
(fsl_chan->desc_pool) return 1;" is needed to prevent a memory leak if the
module is unloaded.

>> -static __init int of_fsl_dma_chan_init(void)
>> +static void fsl_dma_chan_remove(struct fsl_dma_chan *fchan)
>>  {
>> -       return of_register_platform_driver(&of_fsl_dma_chan_driver);
>> +       if (fchan) {
>> +               of_device_unregister(fchan->of_dev);
>> +
>> +               free_irq(fchan->irq, fchan);
>> +               list_del(&fchan->common.device_node);
>> +               iounmap(fchan->reg_base);
>> +               kfree(fchan);
>> +       }
>>  }
> 
> removing a NULL fchan should be an error right?  

No, this is a side-effect of not using a linked-list to store the channels found
by the driver.  The driver has a 4-element array to store the channel info.
Normally, all four channels are defined in the OF device tree, so all four
elements are defined.  But it's not a requirement, so in those cases, the
non-defined channels will have NULL for fchan.

I have plans to replace the array with a linked list.  This change will fix a
few other minor problems with the driver.

> Perhaps move the
> check up to of_fsl_dma_remove().

I can do that.

-- 
Timur Tabi
Linux kernel developer at Freescale

  reply	other threads:[~2008-09-22 21:00 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-09-18 16:24 [PATCH] fsl-dma: allow Freescale Elo DMA driver to be compiled as a module Timur Tabi
2008-09-20 21:32 ` Dan Williams
2008-09-22 20:59   ` Timur Tabi [this message]
2008-09-23 20:46     ` Dan Williams
  -- strict thread matches above, loose matches on Subject: below --
2008-09-19 15:11 Timur Tabi
2008-09-24 18:30 ` Kumar Gala
2008-09-24 18:30   ` Kumar Gala
2008-09-24 18:31   ` Timur Tabi
2008-09-24 18:31     ` Timur Tabi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=48D8074F.9030106@freescale.com \
    --to=timur@freescale.com \
    --cc=akpm@linux-foundation.org \
    --cc=dan.j.williams@intel.com \
    --cc=leoli@freescale.com \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.