Re: [PATCH 22/31] staging: mt7621-mmc: Refactor msdc_init_gpd_bd

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

Thanks,
NeilBrown


>  }
>  
>  static int msdc_drv_probe(struct platform_device *pdev)
> -- 
> 2.16.1

Attachment: signature.asc
Description: PGP signature

_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux