Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <93cc4e4b-4135-8a1f-c7b4-62c5da87bd8b@linaro.org>
Date:   Thu, 24 Nov 2022 14:36:47 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.5.0
Subject: Re: [PATCH v1 2/2] memory: renesas-rpc-if: Add Octal DTR mode support
Content-Language: en-US
To:     JaimeLiao <jaimeliao.tw@gmail.com>, broonie@kernel.org,
        linux-spi@vger.kernel.org, linux-kernel@vger.kernel.org
Cc:     jaimeliao@mxic.com.tw
References: <20221124073957.22471-1-jaimeliao.tw@gmail.com>
 <20221124073957.22471-3-jaimeliao.tw@gmail.com>
From:   Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
In-Reply-To: <20221124073957.22471-3-jaimeliao.tw@gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

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@gmail.com>
> ---
>  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