Re: [PATCH v1 2/2] memory: renesas-rpc-if: Add Octal DTR mode support

[Krzysztof Kozlowski]

On 24/11/2022 08:39, JaimeLiao wrote:
> 1. Support Octal DTR manual mode (Program & Erase & register)
> 2. Support diramap read (enable external address space read mode)

If these are two separate features, you need two separate commits.

> 
> Signed-off-by: JaimeLiao <jaimeliao.tw@xxxxxxxxx>
> ---
>  drivers/memory/renesas-rpc-if.c | 56 +++++++++++++++++++++++++++++++++
>  1 file changed, 56 insertions(+)
> 
> diff --git a/drivers/memory/renesas-rpc-if.c b/drivers/memory/renesas-rpc-if.c
> index bdf0a7f68ff2..db5019a7190e 100644
> --- a/drivers/memory/renesas-rpc-if.c
> +++ b/drivers/memory/renesas-rpc-if.c
> @@ -462,6 +462,43 @@ void rpcif_prepare(struct rpcif *rpc, const struct rpcif_op *op, u64 *offs,
>  
>  		rpc->enable |= RPCIF_SMENR_SPIDB(rpcif_bit_size(op->data.buswidth));
>  	}
> +       /* Fixup in Octal DTR mode */
> +	if (op->cmd.buswidth == 8 && op->cmd.ddr) {
> +		rpc->bus_size = 2;
> +
> +		regmap_update_bits(rpc->regmap, RPCIF_PHYCNT,
> +				   RPCIF_PHYCNT_OCTA(0x2) | RPCIF_PHYCNT_OCT |
> +				   RPCIF_PHYCNT_PHYMEM(0x1) ,
> +				   RPCIF_PHYCNT_OCTA(0x2) | RPCIF_PHYCNT_OCT |
> +				   RPCIF_PHYCNT_PHYMEM(0x1));
> +		regmap_update_bits(rpc->regmap, RPCIF_CMNCR, RPCIF_CMNCR_BSZ(1),
> +				   RPCIF_CMNCR_BSZ(1));
> +
> +		regmap_update_bits(rpc->regmap, RPCIF_PHYOFFSET1,
> +				   RPCIF_PHYOFFSET1_DDRTMG(3),
> +				   RPCIF_PHYOFFSET1_DDRTMG(2));
> +		regmap_update_bits(rpc->regmap, RPCIF_PHYOFFSET2,
> +				   RPCIF_PHYOFFSET2_OCTTMG(7),
> +				   RPCIF_PHYOFFSET2_OCTTMG(3));
> +
> +		if (op->ocmd.buswidth == 8 && RPCIF_SMENR_OCDE)
> +			rpc->enable &= ~RPCIF_SMENR_OCDE;
> +
> +		if (op->addr.ddr)
> +			rpc->enable &= ~RPCIF_SMENR_ADE(0xF) |
> +					RPCIF_SMENR_ADE(0xc);
> +
> +		if (op->dummy.buswidth == 8)
> +			rpc->dummy =
> +				RPCIF_SMDMCR_DMCYC(op->dummy.ncycles / 2);
> +
> +		if (op->data.dir == RPCIF_DATA_IN)
> +			/* Set Extenal Address space Read mode */
> +			if (op->data.buswidth == 8 && op->data.ddr) {
> +				regmap_update_bits(rpc->regmap, RPCIF_PHYCNT,
> +					RPCIF_PHYCNT_EXDS, RPCIF_PHYCNT_EXDS);
> +			}

Wrap it in {}. It's quite error-prone in such syntax.


Best regards,
Krzysztof