| Message ID | 94a975df64a78bb533c85774a5bbd052c73fa5ba.1312965741.git.viresh.kumar@st.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Wed, Aug 10, 2011 at 02:20:56PM +0530, Viresh Kumar wrote: > In routine configure_dma(), if transfer->len = PAGE_SIZE, then pages is one more > than required. While leads to one more sg getting allocated. > > This is wrong. Correct this to allocate correct number of sg. > > Signed-off-by: Viresh Kumar <viresh.kumar@st.com> > Tested-by: Linus Walleij <linus.walleij@linaro.org> > --- > drivers/spi/spi-pl022.c | 3 ++- > 1 files changed, 2 insertions(+), 1 deletions(-) > > diff --git a/drivers/spi/spi-pl022.c b/drivers/spi/spi-pl022.c > index 80116be..1c8b9ec 100644 > --- a/drivers/spi/spi-pl022.c > +++ b/drivers/spi/spi-pl022.c > @@ -1016,7 +1016,8 @@ static int configure_dma(struct pl022 *pl022) > dmaengine_slave_config(txchan, &tx_conf); > > /* Create sglists for the transfers */ > - pages = (pl022->cur_transfer->len >> PAGE_SHIFT) + 1; > + pages = ((pl022->cur_transfer->len + (1 << PAGE_SHIFT) - 1) > + >> PAGE_SHIFT); It would be far better for this to be: pages = DIV_ROUND_UP(pl022->cur_transfer->len, PAGE_SIZE); The compiler will probably optimize it to the same code anyway. ------------------------------------------------------------------------------ uberSVN's rich system and user administration capabilities and model configuration take the hassle out of deploying and managing Subversion and the tools developers use with it. Learn more about uberSVN and get a free download at: http://p.sf.net/sfu/wandisco-dev2dev
On 08/10/2011 02:24 PM, Russell King - ARM Linux wrote: > It would be far better for this to be: > > pages = DIV_ROUND_UP(pl022->cur_transfer->len, PAGE_SIZE); > > The compiler will probably optimize it to the same code anyway. I thought of it too, but missed to update code. Thanks.
Hello. On 10-08-2011 12:50, Viresh Kumar wrote: > In routine configure_dma(), if transfer->len = PAGE_SIZE, then pages is one more > than required. While leads to one more sg getting allocated. > This is wrong. Correct this to allocate correct number of sg. > Signed-off-by: Viresh Kumar<viresh.kumar@st.com> > Tested-by: Linus Walleij<linus.walleij@linaro.org> > --- > drivers/spi/spi-pl022.c | 3 ++- > 1 files changed, 2 insertions(+), 1 deletions(-) > diff --git a/drivers/spi/spi-pl022.c b/drivers/spi/spi-pl022.c > index 80116be..1c8b9ec 100644 > --- a/drivers/spi/spi-pl022.c > +++ b/drivers/spi/spi-pl022.c > @@ -1016,7 +1016,8 @@ static int configure_dma(struct pl022 *pl022) > dmaengine_slave_config(txchan,&tx_conf); > > /* Create sglists for the transfers */ > - pages = (pl022->cur_transfer->len>> PAGE_SHIFT) + 1; > + pages = ((pl022->cur_transfer->len + (1 << PAGE_SHIFT) - 1) > + >> PAGE_SHIFT); No need for parens around the rvalue. WBR, Sergei ------------------------------------------------------------------------------ uberSVN's rich system and user administration capabilities and model configuration take the hassle out of deploying and managing Subversion and the tools developers use with it. Learn more about uberSVN and get a free download at: http://p.sf.net/sfu/wandisco-dev2dev
On 08/10/2011 05:12 PM, Sergei Shtylyov wrote: >> > - pages = (pl022->cur_transfer->len>> PAGE_SHIFT) + 1; >> > + pages = ((pl022->cur_transfer->len + (1 << PAGE_SHIFT) - 1) >> > + >> PAGE_SHIFT); > No need for parens around the rvalue. Oops, that was a mistake. Anyway i have send V3 for the same and this issue is not present anymore.
diff --git a/drivers/spi/spi-pl022.c b/drivers/spi/spi-pl022.c index 80116be..1c8b9ec 100644 --- a/drivers/spi/spi-pl022.c +++ b/drivers/spi/spi-pl022.c @@ -1016,7 +1016,8 @@ static int configure_dma(struct pl022 *pl022) dmaengine_slave_config(txchan, &tx_conf); /* Create sglists for the transfers */ - pages = (pl022->cur_transfer->len >> PAGE_SHIFT) + 1; + pages = ((pl022->cur_transfer->len + (1 << PAGE_SHIFT) - 1) + >> PAGE_SHIFT); dev_dbg(&pl022->adev->dev, "using %d pages for transfer\n", pages); ret = sg_alloc_table(&pl022->sgt_rx, pages, GFP_ATOMIC);