On Wed, Apr 25, 2018 at 01:02:31PM +1000, NeilBrown wrote:
> On Wed, Apr 18 2018, Christian Lütke-Stetzkamp wrote:
>
> > The msdc_init_gpd_bd function is currently hard to read, because of
> > old, commented out code and a while loop, where a for loop is much
> > easier to read. Refactor it to make the code more readable.
> >
> > Signed-off-by: Christian Lütke-Stetzkamp <christian@xxxxxxxx>
> > ---
> > drivers/staging/mt7621-mmc/sd.c | 27 ++++++++-------------------
> > 1 file changed, 8 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/staging/mt7621-mmc/sd.c b/drivers/staging/mt7621-mmc/sd.c
> > index 580ac87d9a5f..0da998c74713 100644
> > --- a/drivers/staging/mt7621-mmc/sd.c
> > +++ b/drivers/staging/mt7621-mmc/sd.c
> > @@ -2501,32 +2501,21 @@ static void msdc_init_gpd_bd(struct msdc_host *host, struct msdc_dma *dma)
> > {
> > struct gpd *gpd = dma->gpd;
> > struct bd *bd = dma->bd;
> > - struct bd *ptr, *prev;
> > + int i;
> >
> > - /* we just support one gpd */
> > - int bdlen = MAX_BD_PER_GPD;
> > + /* we just support one gpd, but gpd->next must be set for desc
> > + * DMA. That's why we alloc 2 gpd structurs.
> > + */
> >
> > - /* init the 2 gpd */
> > memset(gpd, 0, sizeof(struct gpd) * 2);
> > - //gpd->next = (void *)virt_to_phys(gpd + 1); /* pointer to a null gpd, bug! kmalloc <-> virt_to_phys */
> > - //gpd->next = (dma->gpd_addr + 1); /* bug */
> > - gpd->next = (void *)((u32)dma->gpd_addr + sizeof(struct gpd));
> >
> > - //gpd->intr = 0;
> > gpd->bdp = 1; /* hwo, cs, bd pointer */
> > - //gpd->ptr = (void*)virt_to_phys(bd);
> > gpd->ptr = (void *)dma->bd_addr; /* physical address */
> > + gpd->next = (void *)((u32)dma->gpd_addr + sizeof(struct gpd));
> >
> > - memset(bd, 0, sizeof(struct bd) * bdlen);
> > - ptr = bd + bdlen - 1;
> > - //ptr->eol = 1; /* 0 or 1 [Fix me]*/
> > - //ptr->next = 0;
> > -
> > - while (ptr != bd) {
> > - prev = ptr - 1;
> > - prev->next = (void *)(dma->bd_addr + sizeof(struct bd) * (ptr - bd));
> > - ptr = prev;
> > - }
> > + memset(bd, 0, sizeof(struct bd) * MAX_BD_NUM);
> > + for (i = 0; i < (MAX_BD_NUM - 1); i++)
> > + bd[i].next = (void *)(dma->bd_addr * sizeof(*bd) * (i + 1));
>
> Sorry, I hadn't actually tested this when I gave my reviewed-by. I have
> now and it didn't work.
> Look at the last line above. Look carefully.
Sorry for that mad typo, I will send a fix as soon as possible.
And again my thanks for reviewing all my patches.
Regards,
Christian
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
