| Message ID | 94a975df64a78bb533c85774a5bbd052c73fa5ba.1312965741.git.viresh.kumar@st.com (mailing list archive) |
|---|---|
| State | New, 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.
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
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);