Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Received-SPF: Pass (protection.outlook.com: domain of xilinx.com designates
 149.199.62.198 as permitted sender) receiver=protection.outlook.com;
 client-ip=149.199.62.198; helo=xsj-pvapexch01.xlnx.xilinx.com;
Envelope-to: tanureal@opensource.cirrus.com,
 linux-arm-kernel@lists.infradead.org,
 linux-i2c@vger.kernel.org,
 linux-kernel@vger.kernel.org,
 patches@opensource.cirrus.com
Message-ID: <dd26fa4a-870e-d969-04df-1f42487e2b54@xilinx.com>
Date:   Mon, 21 Mar 2022 16:57:13 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.7.0
Subject: Re: [PATCH] i2c: cadence: Increase timeout per message if necessary
Content-Language: en-US
To:     Lucas Tanure <tanureal@opensource.cirrus.com>,
        Michal Simek <michal.simek@xilinx.com>,
        Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
CC:     <linux-arm-kernel@lists.infradead.org>,
        <linux-i2c@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <patches@opensource.cirrus.com>
References: <20220309093147.102385-1-tanureal@opensource.cirrus.com>
From:   Michal Simek <michal.simek@xilinx.com>
In-Reply-To: <20220309093147.102385-1-tanureal@opensource.cirrus.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 21 Mar 2022 15:57:23.5933
 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: 0c3b160e-5024-420e-8af2-08da0b537d30
X-MS-Exchange-CrossTenant-Id: 657af505-d5df-48d0-8300-c31994686c5c
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=657af505-d5df-48d0-8300-c31994686c5c;Ip=[149.199.62.198];Helo=[xsj-pvapexch01.xlnx.xilinx.com]
X-MS-Exchange-CrossTenant-AuthSource: SN1NAM02FT0022.eop-nam02.prod.protection.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Anonymous
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR0201MB3419
X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,
	DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,
	NICE_REPLY_A,RDNS_NONE,SPF_HELO_NONE,T_SCC_BODY_TEXT_LINE autolearn=no
	autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
	lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

+Shubhrajyoti

On 3/9/22 10:31, Lucas Tanure wrote:
> Timeout as 1 second sets a upper limit on the length of
> the transfer executed, but there is no maximum length of
> a write or read message set in i2c_adapter_quirks for this
> controller.

nit: I would expect that you have run any test and you reached an issue.
Would be good to describe what exactly you have tried on which configuration to 
make it super clear.

> 
> To remove that limitation calculate the minimal time
> necessary, plus some wiggle room, for every message
> and use it instead of the default one second, if
> more than one second.
> 
> Signed-off-by: Lucas Tanure <tanureal@opensource.cirrus.com>
> ---
>   drivers/i2c/busses/i2c-cadence.c | 12 ++++++++++--
>   1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c
> index 805c77143a0f..b4c1ad19cdae 100644
> --- a/drivers/i2c/busses/i2c-cadence.c
> +++ b/drivers/i2c/busses/i2c-cadence.c
> @@ -760,7 +760,7 @@ static void cdns_i2c_master_reset(struct i2c_adapter *adap)
>   static int cdns_i2c_process_msg(struct cdns_i2c *id, struct i2c_msg *msg,
>   		struct i2c_adapter *adap)
>   {
> -	unsigned long time_left;
> +	unsigned long time_left, msg_timeout;
>   	u32 reg;
>   
>   	id->p_msg = msg;
> @@ -785,8 +785,16 @@ static int cdns_i2c_process_msg(struct cdns_i2c *id, struct i2c_msg *msg,
>   	else
>   		cdns_i2c_msend(id);
>   
> +	/* Minimal time to execute this message */
> +	msg_timeout = msecs_to_jiffies((1000 * msg->len * BITS_PER_BYTE) / id->i2c_clk);
> +	/* Plus some wiggle room */
> +	msg_timeout += msecs_to_jiffies(500);
> +
> +	if (msg_timeout < adap->timeout)
> +		msg_timeout = adap->timeout;
> +
>   	/* Wait for the signal of completion */
> -	time_left = wait_for_completion_timeout(&id->xfer_done, adap->timeout);
> +	time_left = wait_for_completion_timeout(&id->xfer_done, msg_timeout);
>   	if (time_left == 0) {
>   		cdns_i2c_master_reset(adap);
>   		dev_err(id->adap.dev.parent,


If my assumption is right and there is any actual issue you had please send v2 
and feel free to add there my:
Acked-by: Michal Simek <michal.simek@xilinx.com>

Thanks,
Michal