Re: [PATCH 4/8] dmaengine: ste_dma40: Do not configure channels during an channel allocation
2013/4/12 Lee Jones : > So I need to devise another way, as this function cannot be called > here either. Using the dmaengine API, allocating a channel and > configuring it are to be completed using different calls. Using the > API correctly, there is no way the driver can setup the channel > with all of the relevant information during allocation time. > > The steps are as follows: > > dma_request_channel() - here we only allot a channel number and > allocate the appropriate resources for the > channel. > > dma_slave_config()- this is where we're meant to configure the > channel, so d40_config_write() needs to be > called here, as only dma_slave_config() will > carry the information required so as > d40_*_cfg() can make the correct decisions. The choice between whether a physical or a logical channel is used is not something that is configurable via dma_slave_config(). And d40_config_write() only needs that information, and that information is available in dma_request_channel(). Therefore no more information relevant to d40_config_write() will be obtained in dma_slave_config(). Hence d40_config_write() can be called in dma_request_channel(). -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/8] dmaengine: ste_dma40: Do not configure channels during an channel allocation
2013/4/12 Lee Jones lee.jo...@linaro.org: So I need to devise another way, as this function cannot be called here either. Using the dmaengine API, allocating a channel and configuring it are to be completed using different calls. Using the API correctly, there is no way the driver can setup the channel with all of the relevant information during allocation time. The steps are as follows: dma_request_channel() - here we only allot a channel number and allocate the appropriate resources for the channel. dma_slave_config()- this is where we're meant to configure the channel, so d40_config_write() needs to be called here, as only dma_slave_config() will carry the information required so as d40_*_cfg() can make the correct decisions. The choice between whether a physical or a logical channel is used is not something that is configurable via dma_slave_config(). And d40_config_write() only needs that information, and that information is available in dma_request_channel(). Therefore no more information relevant to d40_config_write() will be obtained in dma_slave_config(). Hence d40_config_write() can be called in dma_request_channel(). -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/8] dmaengine: ste_dma40: Do not configure channels during an channel allocation
On Fri, 12 Apr 2013, Rabin Vincent wrote:
> 2013/4/9 Lee Jones :
> > diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
> > index 2a0a9d4..9e423ad 100644
> > --- a/drivers/dma/ste_dma40.c
> > +++ b/drivers/dma/ste_dma40.c
> > @@ -2463,16 +2463,10 @@ static int d40_alloc_chan_resources(struct dma_chan
> > *chan)
> > }
> >
> > pm_runtime_get_sync(d40c->base->dev);
> > - /* Fill in basic CFG register values */
> > - d40_phy_cfg(>dma_cfg, >src_def_cfg,
> > - >dst_def_cfg, chan_is_logical(d40c));
> >
> > d40_set_prio_realtime(d40c);
> >
> > if (chan_is_logical(d40c)) {
> > - d40_log_cfg(>dma_cfg,
> > - >log_def.lcsp1, >log_def.lcsp3);
> > -
> > if (d40c->dma_cfg.dir == STEDMA40_PERIPH_TO_MEM)
> > d40c->lcpa = d40c->base->lcpa_base +
> > d40c->dma_cfg.src_dev_type *
> > D40_LCPA_CHAN_SIZE;
> > @@ -2487,14 +2481,6 @@ static int d40_alloc_chan_resources(struct dma_chan
> > *chan)
> > d40c->phy_chan->num,
> > d40c->dma_cfg.use_fixed_channel ? ", fixed" : "");
> >
> > -
> > - /*
> > -* Only write channel configuration to the DMA if the physical
> > -* resource is free. In case of multiple logical channels
> > -* on the same physical resource, only the first write is necessary.
> > -*/
> > - if (is_free_phy)
> > - d40_config_write(d40c);
>
> This function does basic initial configuration which is linked to
> whether the channel is pysical or logical -- something that is not being
> allowed to be runtime configured, so the place to call this function is
> here. Furthermore, it clears the SSLNK/SDLNK which has to be done here,
> when the phy channel is first allocated.
Okay, I see what you're saying.
So I need to devise another way, as this function cannot be called
here either. Using the dmaengine API, allocating a channel and
configuring it are to be completed using different calls. Using the
API correctly, there is no way the driver can setup the channel
with all of the relevant information during allocation time.
The steps are as follows:
dma_request_channel() - here we only allot a channel number and
allocate the appropriate resources for the
channel.
dma_slave_config()- this is where we're meant to configure the
channel, so d40_config_write() needs to be
called here, as only dma_slave_config() will
carry the information required so as
d40_*_cfg() can make the correct decisions.
At the moment calling dma_slave_config() ends up calling
d40_set_runtime_config(), which, taking into consideration what you've
been saying about possibly cutting off other logical channels, is
probably not what we want. I'll take a look at the dmaengine API and
see if there's a more appropriate way to do that.
Thanks again for your input Rabin, it's invaluable.
Kind regards,
Lee
--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/8] dmaengine: ste_dma40: Do not configure channels during an channel allocation
2013/4/9 Lee Jones :
> diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
> index 2a0a9d4..9e423ad 100644
> --- a/drivers/dma/ste_dma40.c
> +++ b/drivers/dma/ste_dma40.c
> @@ -2463,16 +2463,10 @@ static int d40_alloc_chan_resources(struct dma_chan
> *chan)
> }
>
> pm_runtime_get_sync(d40c->base->dev);
> - /* Fill in basic CFG register values */
> - d40_phy_cfg(>dma_cfg, >src_def_cfg,
> - >dst_def_cfg, chan_is_logical(d40c));
>
> d40_set_prio_realtime(d40c);
>
> if (chan_is_logical(d40c)) {
> - d40_log_cfg(>dma_cfg,
> - >log_def.lcsp1, >log_def.lcsp3);
> -
> if (d40c->dma_cfg.dir == STEDMA40_PERIPH_TO_MEM)
> d40c->lcpa = d40c->base->lcpa_base +
> d40c->dma_cfg.src_dev_type *
> D40_LCPA_CHAN_SIZE;
> @@ -2487,14 +2481,6 @@ static int d40_alloc_chan_resources(struct dma_chan
> *chan)
> d40c->phy_chan->num,
> d40c->dma_cfg.use_fixed_channel ? ", fixed" : "");
>
> -
> - /*
> -* Only write channel configuration to the DMA if the physical
> -* resource is free. In case of multiple logical channels
> -* on the same physical resource, only the first write is necessary.
> -*/
> - if (is_free_phy)
> - d40_config_write(d40c);
This function does basic initial configuration which is linked to
whether the channel is pysical or logical -- something that is not being
allowed to be runtime configured, so the place to call this function is
here. Furthermore, it clears the SSLNK/SDLNK which has to be done here,
when the phy channel is first allocated.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/8] dmaengine: ste_dma40: Do not configure channels during an channel allocation
2013/4/9 Lee Jones lee.jo...@linaro.org:
diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index 2a0a9d4..9e423ad 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -2463,16 +2463,10 @@ static int d40_alloc_chan_resources(struct dma_chan
*chan)
}
pm_runtime_get_sync(d40c-base-dev);
- /* Fill in basic CFG register values */
- d40_phy_cfg(d40c-dma_cfg, d40c-src_def_cfg,
- d40c-dst_def_cfg, chan_is_logical(d40c));
d40_set_prio_realtime(d40c);
if (chan_is_logical(d40c)) {
- d40_log_cfg(d40c-dma_cfg,
- d40c-log_def.lcsp1, d40c-log_def.lcsp3);
-
if (d40c-dma_cfg.dir == STEDMA40_PERIPH_TO_MEM)
d40c-lcpa = d40c-base-lcpa_base +
d40c-dma_cfg.src_dev_type *
D40_LCPA_CHAN_SIZE;
@@ -2487,14 +2481,6 @@ static int d40_alloc_chan_resources(struct dma_chan
*chan)
d40c-phy_chan-num,
d40c-dma_cfg.use_fixed_channel ? , fixed : );
-
- /*
-* Only write channel configuration to the DMA if the physical
-* resource is free. In case of multiple logical channels
-* on the same physical resource, only the first write is necessary.
-*/
- if (is_free_phy)
- d40_config_write(d40c);
This function does basic initial configuration which is linked to
whether the channel is pysical or logical -- something that is not being
allowed to be runtime configured, so the place to call this function is
here. Furthermore, it clears the SSLNK/SDLNK which has to be done here,
when the phy channel is first allocated.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/8] dmaengine: ste_dma40: Do not configure channels during an channel allocation
On Fri, 12 Apr 2013, Rabin Vincent wrote:
2013/4/9 Lee Jones lee.jo...@linaro.org:
diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index 2a0a9d4..9e423ad 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -2463,16 +2463,10 @@ static int d40_alloc_chan_resources(struct dma_chan
*chan)
}
pm_runtime_get_sync(d40c-base-dev);
- /* Fill in basic CFG register values */
- d40_phy_cfg(d40c-dma_cfg, d40c-src_def_cfg,
- d40c-dst_def_cfg, chan_is_logical(d40c));
d40_set_prio_realtime(d40c);
if (chan_is_logical(d40c)) {
- d40_log_cfg(d40c-dma_cfg,
- d40c-log_def.lcsp1, d40c-log_def.lcsp3);
-
if (d40c-dma_cfg.dir == STEDMA40_PERIPH_TO_MEM)
d40c-lcpa = d40c-base-lcpa_base +
d40c-dma_cfg.src_dev_type *
D40_LCPA_CHAN_SIZE;
@@ -2487,14 +2481,6 @@ static int d40_alloc_chan_resources(struct dma_chan
*chan)
d40c-phy_chan-num,
d40c-dma_cfg.use_fixed_channel ? , fixed : );
-
- /*
-* Only write channel configuration to the DMA if the physical
-* resource is free. In case of multiple logical channels
-* on the same physical resource, only the first write is necessary.
-*/
- if (is_free_phy)
- d40_config_write(d40c);
This function does basic initial configuration which is linked to
whether the channel is pysical or logical -- something that is not being
allowed to be runtime configured, so the place to call this function is
here. Furthermore, it clears the SSLNK/SDLNK which has to be done here,
when the phy channel is first allocated.
Okay, I see what you're saying.
So I need to devise another way, as this function cannot be called
here either. Using the dmaengine API, allocating a channel and
configuring it are to be completed using different calls. Using the
API correctly, there is no way the driver can setup the channel
with all of the relevant information during allocation time.
The steps are as follows:
dma_request_channel() - here we only allot a channel number and
allocate the appropriate resources for the
channel.
dma_slave_config()- this is where we're meant to configure the
channel, so d40_config_write() needs to be
called here, as only dma_slave_config() will
carry the information required so as
d40_*_cfg() can make the correct decisions.
At the moment calling dma_slave_config() ends up calling
d40_set_runtime_config(), which, taking into consideration what you've
been saying about possibly cutting off other logical channels, is
probably not what we want. I'll take a look at the dmaengine API and
see if there's a more appropriate way to do that.
Thanks again for your input Rabin, it's invaluable.
Kind regards,
Lee
--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 4/8] dmaengine: ste_dma40: Do not configure channels during an channel allocation
According to the DMA documentation allocating a channel and configuring
it are two separate actions. By removing the configuration code from the
channel allocation path we lighten the burden on the information required to
successfully allocate a channel.
Cc: Vinod Koul
Cc: Dan Williams
Cc: Per Forlin
Acked-by: Arnd Bergmann
Signed-off-by: Lee Jones
---
drivers/dma/ste_dma40.c | 14 --
1 file changed, 14 deletions(-)
diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index 2a0a9d4..9e423ad 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -2463,16 +2463,10 @@ static int d40_alloc_chan_resources(struct dma_chan
*chan)
}
pm_runtime_get_sync(d40c->base->dev);
- /* Fill in basic CFG register values */
- d40_phy_cfg(>dma_cfg, >src_def_cfg,
- >dst_def_cfg, chan_is_logical(d40c));
d40_set_prio_realtime(d40c);
if (chan_is_logical(d40c)) {
- d40_log_cfg(>dma_cfg,
- >log_def.lcsp1, >log_def.lcsp3);
-
if (d40c->dma_cfg.dir == STEDMA40_PERIPH_TO_MEM)
d40c->lcpa = d40c->base->lcpa_base +
d40c->dma_cfg.src_dev_type * D40_LCPA_CHAN_SIZE;
@@ -2487,14 +2481,6 @@ static int d40_alloc_chan_resources(struct dma_chan
*chan)
d40c->phy_chan->num,
d40c->dma_cfg.use_fixed_channel ? ", fixed" : "");
-
- /*
-* Only write channel configuration to the DMA if the physical
-* resource is free. In case of multiple logical channels
-* on the same physical resource, only the first write is necessary.
-*/
- if (is_free_phy)
- d40_config_write(d40c);
fail:
pm_runtime_mark_last_busy(d40c->base->dev);
pm_runtime_put_autosuspend(d40c->base->dev);
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 4/8] dmaengine: ste_dma40: Do not configure channels during an channel allocation
According to the DMA documentation allocating a channel and configuring
it are two separate actions. By removing the configuration code from the
channel allocation path we lighten the burden on the information required to
successfully allocate a channel.
Cc: Vinod Koul vinod.k...@intel.com
Cc: Dan Williams d...@fb.com
Cc: Per Forlin per.for...@stericsson.com
Acked-by: Arnd Bergmann a...@arndb.de
Signed-off-by: Lee Jones lee.jo...@linaro.org
---
drivers/dma/ste_dma40.c | 14 --
1 file changed, 14 deletions(-)
diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index 2a0a9d4..9e423ad 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -2463,16 +2463,10 @@ static int d40_alloc_chan_resources(struct dma_chan
*chan)
}
pm_runtime_get_sync(d40c-base-dev);
- /* Fill in basic CFG register values */
- d40_phy_cfg(d40c-dma_cfg, d40c-src_def_cfg,
- d40c-dst_def_cfg, chan_is_logical(d40c));
d40_set_prio_realtime(d40c);
if (chan_is_logical(d40c)) {
- d40_log_cfg(d40c-dma_cfg,
- d40c-log_def.lcsp1, d40c-log_def.lcsp3);
-
if (d40c-dma_cfg.dir == STEDMA40_PERIPH_TO_MEM)
d40c-lcpa = d40c-base-lcpa_base +
d40c-dma_cfg.src_dev_type * D40_LCPA_CHAN_SIZE;
@@ -2487,14 +2481,6 @@ static int d40_alloc_chan_resources(struct dma_chan
*chan)
d40c-phy_chan-num,
d40c-dma_cfg.use_fixed_channel ? , fixed : );
-
- /*
-* Only write channel configuration to the DMA if the physical
-* resource is free. In case of multiple logical channels
-* on the same physical resource, only the first write is necessary.
-*/
- if (is_free_phy)
- d40_config_write(d40c);
fail:
pm_runtime_mark_last_busy(d40c-base-dev);
pm_runtime_put_autosuspend(d40c-base-dev);
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
