From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-qk1-f177.google.com (mail-qk1-f177.google.com [209.85.222.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id DABD233F390 for ; Thu, 15 Jan 2026 17:02:21 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.222.177 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1768496544; cv=none; b=P9KyY9FkWc2Tqrlo52CijKcvRpz1AeNXDAwANUW5Ng8pZCJHAJ+vH2SIQ44Xej9OlYhqj+1pSkrZrmFYfn7iekELAlqzhVnujXZN8TUNMw9rKBYpavXaKEhvC1kbN5l6qt4nQTMQhyWqq51aSRv7dDbg3ZmmPS/xH5eDvzeXl30= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1768496544; c=relaxed/simple; bh=IpwDIvhr5VuqgX4fdAxB6xXe39k/dnAVORQtSR3VM7I=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=a3bK4mGpmD54u8nEBnMqv9bNZnIbpvmRnBNPJ5e4oaSYO0sXMWLDVmnQGpOpNrx82CUpbcXtG/GT3leSIRKU3EHX9a9i1Yj2zpoTDO+9PVJvFelFuWRtAY+FXgoCp52wJiJ50oMiPTuKfZ0OKultl730kaqi1PSHVXs7rRG6dn8= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=riscstar.com; spf=pass smtp.mailfrom=riscstar.com; dkim=pass (2048-bit key) header.d=riscstar-com.20230601.gappssmtp.com header.i=@riscstar-com.20230601.gappssmtp.com header.b=ALNNE+yo; arc=none smtp.client-ip=209.85.222.177 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=riscstar.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=riscstar.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=riscstar-com.20230601.gappssmtp.com header.i=@riscstar-com.20230601.gappssmtp.com header.b="ALNNE+yo" Received: by mail-qk1-f177.google.com with SMTP id af79cd13be357-8c655e0ee70so110986785a.3 for ; Thu, 15 Jan 2026 09:02:21 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=riscstar-com.20230601.gappssmtp.com; s=20230601; t=1768496541; x=1769101341; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=9lmL4wVgyCel4RaZm/YDnYaY71CyzahaKuw46/jDcwA=; b=ALNNE+yovVDKHLBFyd18PDX63XXG8lZgT1V2adakll4e3MdsUUgg5Hm76hLBNYhWnW MhDjSDPAk3KnuEFtKmlRGT4jWJD++PrdhIydUbbRaiWsorDBxJNWXqX68fiB4vR2t6Uu U+cN1wmHBS3qJLD6LGBirPUX6izHFUiMJYUXexRVrwgwJFxTFXi/xYB/FYvbr2XXw1Rm 3UZ4sf29L/BYyrcKk33TqRIhSm4n0S5NjRBB2a/LyzXqMijht+92OHWTqQK8AkvHvHnN QMEbd9CBSHtNucIxbeinikyDZLMYG/e71pjEXBulvKu9YwM0k2jlL3l5l2vhNJ/f6ZtG hDwg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1768496541; x=1769101341; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=9lmL4wVgyCel4RaZm/YDnYaY71CyzahaKuw46/jDcwA=; b=MSfDQ9/7/gs8ceeROzpDJlLpNfvNyivluhQJaoD+gmtMQnQAeZ9WyFNZ2RXOYWKUnD yrpMgQY1lTZUPAkW03x62POWIHvUEgjxW/Idr8OqFEBonwlNMQtIWVya00D7kX8Vz0mi KfMLe8cTRbuVFLwacPAmHkP90klO+IG7o5mhnuYIkQbCYZ8qwJge6Kns0sJlSwQWmw+z DOif4nlHTcr5zqHkE5QYj5BkegrIABRSy5q8+1p5J6ZZ9/20JkyHxrdSm9X8ldqpZmfZ Nq3Q9vT0VQfiMMeJvtSL7ENTAajJxLUmzV4AIAznBGE+Qls5a3Payo2dP6ShQMmGjOow SO3A== X-Forwarded-Encrypted: i=1; AJvYcCU2UNwRnDwBg+YtIlpa+AtWrj39ysv3uYgHT18GhCbF9wH+SkcfFTjNC7PxPgIs4xTE0oAWVvPO3Ao5J8E=@vger.kernel.org X-Gm-Message-State: AOJu0Yycc876mZIuz06DZf643MD6N8Bd7d7HpMPlTS5HXd33U/3A0fwY /BA1rMT7kRnBAibEdqQrWNaHSuYMO9oH2Jpgn9ZM8HERRuU5HpDwsXDrzAVou0yiduRhhuc1EuY kxRKQeqs= X-Gm-Gg: AY/fxX793qdNeO06ivE6LwEVyHHs/eHExBwGoUNWWYdSHAPMN36yHVzpBSns1YR7tyj b0+ncv9WVkILBvXYSFcR9hxyi12FybgJlNmRJm1lK72wTiDIOvRzrVwZoxU3pBcKWBWJzv22LKw aRlLo1YjC5cD63gXNd43jFFi9QhBTfiVRFfB5cSjgbxMtu5zrY/0QM3EmLOVdnK2UEzBDegAvQu 2Xvh9RLIwzMlcmncPUDbppIxt7nawAIoBwx+SsB6jGa8a+bMpnkXA54rc9sT0G3zi4R0i8FS4nV EDrgMDu2gniGD772QqId4/idcxdEMfrPqSxJA7m+YLFJqeDYhb2J2n4BvoBoVaiahFlmgyydnCj ssAIV6EOroiO0EhOL0INcC+6vys4E3gdWH4BLOhx5bBLJjvwEpXVvZ8oNtR+fa4yoQtcw6hQ98j b31pz0oxAhsGvmJxVyi1709CXVZNOE2fOzpvPq+Css9YQDwGTbIOiF X-Received: by 2002:a05:620a:700b:b0:8c5:3376:3326 with SMTP id af79cd13be357-8c6a67c857amr5981285a.80.1768496540568; Thu, 15 Jan 2026 09:02:20 -0800 (PST) Received: from [172.22.22.234] (c-75-72-117-212.hsd1.mn.comcast.net. [75.72.117.212]) by smtp.gmail.com with ESMTPSA id af79cd13be357-8c530b74bc4sm455422485a.32.2026.01.15.09.02.19 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 15 Jan 2026 09:02:20 -0800 (PST) Message-ID: <00f42b6c-857b-4eef-a0da-83053998135a@riscstar.com> Date: Thu, 15 Jan 2026 11:02:17 -0600 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v6 2/2] i2c: spacemit: introduce pio for k1 To: Andi Shyti , Troy Mitchell Cc: Yixun Lan , Aurelien Jarno , Michael Opdenacker , Troy Mitchell , linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org, linux-riscv@lists.infradead.org, spacemit@lists.linux.dev References: <20260108-k1-i2c-atomic-v6-0-41b132b70f68@linux.spacemit.com> <20260108-k1-i2c-atomic-v6-2-41b132b70f68@linux.spacemit.com> Content-Language: en-US From: Alex Elder In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 1/14/26 9:47 AM, Andi Shyti wrote: > Hi Troy, > > ... > >> @@ -171,6 +176,16 @@ static int spacemit_i2c_handle_err(struct spacemit_i2c_dev *i2c) >> return i2c->status & SPACEMIT_SR_ACKNAK ? -ENXIO : -EIO; >> } >> >> +static inline void spacemit_i2c_delay(struct spacemit_i2c_dev *i2c, >> + unsigned int min_us, >> + unsigned int max_us) >> +{ >> + if (i2c->use_pio) >> + udelay(max_us); > > We need some control on how much we want to sleep in atomic. This > can have effects on the whole system. > >> + else >> + usleep_range(min_us, max_us); > > If we assume that max_us = min_us * 2 we don't need to pass it as > a parameter. Even better you can use fsleep here which does it > for you. I agree with both of these comments/suggestions. And if fsleep() were used, spacemit_i2c_delay() might be able to just go away. (The range used in fsleep() isn't quite the same as what you're using, but this is heuristic anyway.) However the delay used in spacemit_i2c_check_bus_release() is 90-150 microseconds, which would lead to sleeping in fsleep(). Is there any chance 10 microseconds (or less) could be used for all delays? Even if not, fsleep() might help here. >> +} > > ... . . . >> - if (i2c->state != SPACEMIT_STATE_IDLE) { >> - val |= SPACEMIT_CR_TB | SPACEMIT_CR_ALDIE; >> - >> - if (spacemit_i2c_is_last_msg(i2c)) { >> - /* trigger next byte with stop */ >> - val |= SPACEMIT_CR_STOP; >> - >> - if (i2c->read) >> - val |= SPACEMIT_CR_ACKNAK; >> - } >> - writel(val, i2c->base + SPACEMIT_ICR); >> - } >> + spacemit_i2c_handle_state(i2c); > > Next time this can be on a separate patch as a preparatory patch > to make the review of this one a bit easier. Yes! It's a next-level skill: beyond just changing the code, breaking the change into good, independent pieces that build on each other, to facilitate review. Thanks! -Alex > >> >> -err_out: >> - spacemit_i2c_err_check(i2c); >> return IRQ_HANDLED; >> } > > Thanks, > Andi