Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: arnd@arndb.de (Arnd Bergmann)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] ARM: SAMSUNG: dma: Remove unnecessary code
Date: Tue, 5 Feb 2013 11:13:22 +0000	[thread overview]
Message-ID: <201302051113.22720.arnd@arndb.de> (raw)
In-Reply-To: <CAAgF-BfaYo7FoYyFHo1eW-bUGOYdvcfskf7_7UhmCOzASoreYg@mail.gmail.com>

On Tuesday 05 February 2013, Padma Venkat wrote:
> On Mon, Feb 4, 2013 at 11:13 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Monday 04 February 2013, Padmavathi Venna wrote:
> >> diff --git a/arch/arm/plat-samsung/dma-ops.c b/arch/arm/plat-samsung/dma-ops.c
> >> index 71d58dd..ec0d731 100644
> >> --- a/arch/arm/plat-samsung/dma-ops.c
> >> +++ b/arch/arm/plat-samsung/dma-ops.c
> >> @@ -23,23 +23,15 @@ static unsigned samsung_dmadev_request(enum dma_ch dma_ch,
> >>                               struct device *dev, char *ch_name)
> >>  {
> >>       dma_cap_mask_t mask;
> >> -     void *filter_param;
> >>
> >>       dma_cap_zero(mask);
> >>       dma_cap_set(param->cap, mask);
> >>
> >> -     /*
> >> -      * If a dma channel property of a device node from device tree is
> >> -      * specified, use that as the fliter parameter.
> >> -      */
> >> -     filter_param = (dma_ch == DMACH_DT_PROP) ?
> >> -             (void *)param->dt_dmach_prop : (void *)dma_ch;
> >> -
> >>       if (dev->of_node)
> >>               return (unsigned)dma_request_slave_channel(dev, ch_name);
> >>       else
> >>               return (unsigned)dma_request_channel(mask, pl330_filter,
> >> -                                                     filter_param);
> >> +                                                     (void *)dma_ch);
> >>  }
> >
> > This still looks wrong to me, because the pl330_filter function now tkes
> > a struct dma_pl330_filter_args pointer argument, not dma_ch name.
> 
> Below is my understanding about generic dma and our discussion on
> previous versions of my patches.
> 
> I can?t pass single dma channel number(may be not dma_ch name in your
> comment above) as void* argument to pl330_filter.  Because I also need
> to compare against the dma controller device node, as my requested
> channel can belong to any of the available dma controller on SoC.  So
> I either need to pass pointer to dma_spec as void* argument which
> holds the dma controller node and required channel number or I can
> pass pointer to dma_pl330_filter_args as per your dw_dmac patches.

Right.

> If I pass pointer to dma_spec I can have a check like below in my
> filter function
> return ((chan->private == dma_spec->np) && (chan->chan_id == dma_spec->args[0]))
> 
> Or if I pass dma_pl330_filter_args I can have a check like below.
> return ((chan->device == &fargs->pdmac->ddma) && (chan->chan_id ==
> fargs->chan_id));
> 
> I modified  the pl330_filter function based on your dw_dmac patches.
> Indeed I don?t need to pass pointer to pdmac object as 3rd arg in
> of_dma_controller_register .  Even I pass NULL here works for me.
> Can I pass NULL here as the third argument in of_dma_controller_register?

These are all not the issues I am referring to in my comment above.
I think it works either way, even if you pass NULL to
of_dma_controller_register, although using it for the pdmac object
seems cleaner to me.

> Please clarify me which is best way of doing this and correct me if my
> understanding is wrong.

My point was that in the samsung_dmadev_request quoted above, you
refer to the same pl330_filter filter function, but the argument there
is a pointer to 'enum dma_ch', which is not compatible with any of
the methods you list, neither the dma_pl330_filter_args nor the
raw property.

Also, if you change the calling conventions for the pl330_filter
function, you should change both the caller and the function in the
same patch.

	Arnd

  reply	other threads:[~2013-02-05 11:13 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-02-04  8:47 [PATCH] ARM: SAMSUNG: dma: Remove unnecessary code Padmavathi Venna
2013-02-04 17:43 ` Arnd Bergmann
2013-02-05  8:04   ` Padma Venkat
2013-02-05 11:13     ` Arnd Bergmann [this message]
2013-02-05 15:54       ` Padma Venkat
2013-02-05 23:51         ` Arnd Bergmann
  -- strict thread matches above, loose matches on Subject: below --
2013-02-14  3:47 Padmavathi Venna

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=201302051113.22720.arnd@arndb.de \
    --to=arnd@arndb.de \
    --cc=linux-arm-kernel@lists.infradead.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox