Re: [PATCH v3 2/2] i2c: qcom-target: Add driver for Qualcomm I2C target controller

[Viken Dadhaniya <viken.dadhaniya@oss.qualcomm.com>]

On 8/28/2026 5:32 AM, Andi Shyti wrote:
> Hi Viken,
> 
> On Thu, Aug 27, 2026 at 06:44:21PM +0530, Viken Dadhaniya wrote:
>> On 8/26/2026 7:27 PM, Andi Shyti wrote:
>>> ...
>>>
>>>> +static void qcom_i2c_target_hw_reset(struct qcom_i2c_target *target)
>>>> +{
>>>> +	/* Clear error bits before SW_RESET; the reset may not be instantaneous */
>>>> +	writel(BIT(ERR_CONDITION) | BIT(CLOCK_LOW_TIMEOUT),
>>>> +	       target->base + I2C_S_IRQ_CLR);
>>>> +	writel(SW_RESET, target->base + I2C_S_SW_RESET_REG);
>>>> +	/*
>>>> +	 * I2C_S_SW_RESET_REG is write-only so completion cannot be polled.
>>>> +	 * Use a conservative delay to allow the reset to finish before
>>>> +	 * reconfiguring the controller.
>>>> +	 */
>>>> +	usleep_range(10, 20);
>>>
>>> This is also called in atomic context from the irq handler.
>>> Please avoid using usleep_range(), perhaps you can put this in a
>>> thread.
>>>
>>
>> Thanks for the review. The delay is only 20 µs, so we were
>> thinking of replacing usleep_range() with udelay() instead of
>> converting to a threaded IRQ, to avoid the added complexity on
>> all the normal fast paths.
> 
> perhaps before sleeping you could check the context you are
> executing (with an explaining comment).

This helper is reached from the hard IRQ error handler, so it must remain
atomic-safe regardless of the current context. Since the reset wait is
only 20 µs, I will replace usleep_range() with udelay() and add a comment
explaining why a non-sleeping delay is required.

> 
> Thanks,
> Andi