Re: [PATCH v2] memory: renesas-rpc-if: Fix HF/OSPI data transfer in Manual mode

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

 



Hi Geert,

thank you for the patch!

> Lightly (read-only) tested on:
>   - Salvator-XS with R-Car M3-W ES1.0 with HyperFLASH,
>   - Falcon with R-Car V3U ES1.0 with QSPI.

I (also lightly) tested writing on the V3U and it seems to still work,
even with the problematic cases we needed to fix earlier.

>       Note: Alternatively, rpcif_manual_xfer() could bypass regmap and
>       use {read,write}[bwl]() directly, cfr. commit 0d37f69cacb33435
>       ("memory: renesas-rpc-if: Correct QSPI data transfer in Manual
>       mode") in the BSP.

As discuseed via IRC, I don't like bypassing regmap. I prefer your
solution here.

> @@ -171,18 +171,32 @@ static int rpcif_reg_read(void *context, unsigned int reg, unsigned int *val)

Before this function is a comment which needs to be updated (away from
SMENR_SPIDE to xfer_size).

> +		switch (rpc->xfer_size) {
> +		case 1:
>  			*val = readb(rpc->base + reg);
>  			return 0;
> -		} else if (spide == 0xC) {
> +
> +		case 2:
>  			*val = readw(rpc->base + reg);
>  			return 0;
> -		} else if (spide != 0xF) {
> +
> +		case 4:
> +		case 8:

I think it is more readable to add here:

  			*val = readl(rpc->base + reg);
  			return 0;

> +			break;

... and remove this break ...


> +
> +		default:
>  			return -EILSEQ;
>  		}
> +		break;

... and this one. Same for the write function. I am not strict on this
one, but I think this make following the logic easier.

Rest looks good. I like the additional checks!

All the best,

   Wolfram

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux