From patchwork Wed Mar 20 04:20:07 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sam Edwards X-Patchwork-Id: 781696 Received: from mail-pf1-f178.google.com (mail-pf1-f178.google.com [209.85.210.178]) (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 C808A14007 for ; Wed, 20 Mar 2024 04:20:18 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.210.178 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710908420; cv=none; b=H3YcB8G0V4eNiTCnAN87DbDkjZsgUXVbTmr0LHfDMn9mqKQpEN/H+rnNCttnpM/NzLSt8suTV4+nQbjB2UBs5hQP9Dy//4RT/IjjUnEe7kjcZHz6R8b+/MazE6UpioIspRRTKK4SykDUUC+onOopY5RBigpe8ymwEUlMyhuYlmk= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710908420; c=relaxed/simple; bh=H/oWAmb3qdH6GqOGd7i1JdAe4XTSZAqLaJD9s7aHPtk=; h=MIME-Version:From:Date:Message-ID:Subject:To:Cc:Content-Type; b=ue+4c7onulOaIdMcmkHfw24UeFgxK7aupIA57xjfBSntuYb3a73O7bmGv7PJnn7hFbL7Vm2UuK4a4wDXU4ro7wyxiVChjH8k28nZQ/o9RDLa+xvbdqrRXehk782Ztt+z8ikiK1JLNTgyOPOQdYwiAlMO1jmBq3FbTlXMMholzbQ= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=turingpi.com; spf=pass smtp.mailfrom=turingpi.com; dkim=pass (2048-bit key) header.d=turingpi.com header.i=@turingpi.com header.b=jW35wPlH; arc=none smtp.client-ip=209.85.210.178 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=turingpi.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=turingpi.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=turingpi.com header.i=@turingpi.com header.b="jW35wPlH" Received: by mail-pf1-f178.google.com with SMTP id d2e1a72fcca58-6e6c0098328so5164176b3a.3 for ; Tue, 19 Mar 2024 21:20:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=turingpi.com; s=google; t=1710908418; x=1711513218; darn=vger.kernel.org; h=cc:to:subject:message-id:date:from:mime-version:from:to:cc:subject :date:message-id:reply-to; bh=x/ne4R6QNYBHc5s1C1wG8NSPymgeCplkIkbc0sb+CmI=; b=jW35wPlHXg3qlupWIcHLVQjTLb6Vz+uFu8/YqZrbFSYh0rq7qTfBFO3tYiDxLZSqYN nYMzj8lBwk8KWhFB280YimQ9i35wxHBj2kfE3PG31VsRi9C38y4JqlUVq3uzA5uOM2f6 hjLZon7MlgTRXyInBNZEBFeDuQ0rMFq6eyIlo1wJpg1VHPDt1q2ZOu3qGQADw/XJjut1 KDCQ0EPQ9F1uHw60tZKYEC8NzDE2fB5WEkNK1AeyeXqxtkW3yX7NCmVMSkkxn4vV5Ofv EpEGdvCmgycDYAprthqMPOALOp1XPDjEEb/Od2fQ9ISiuEY7lFBPUVW0vJl1nEhD5qQy QUPQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1710908418; x=1711513218; h=cc:to:subject:message-id:date:from:mime-version:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=x/ne4R6QNYBHc5s1C1wG8NSPymgeCplkIkbc0sb+CmI=; b=GhnXpnWE+IXvyNN4+iteCLgvq8u8TWMigVOLaKeVFo5sH4FRECeyPw7IoLoYgagbpB uVLTTkFvz44hw1dyVdOCcWznHoTkai9qO2M+pyeyBTUGmPMVo7P3qRWJU4bWHcnskGFw uL2y8T6FgivbnhQ5wV5ARB6AmeBDLZPD34WUOCU3RyjxPY2cUosS9MDLwFut8s3GBRF5 E37iJ8Ftbinxro18LEHGAVgC/LfshmLrgAi+EmWi202SW+wnyu38Oglm1/iuvazYGOXZ EFWZnojdBHYeEx9fZw1+K+T4Zndy9wlxhoXEVL/9d09X1gGsUgyUAD+C3lorx6El6FI6 JCNw== X-Gm-Message-State: AOJu0YziYq87YA63e6ZsT7Tc2r69lAj8zaZlnkTf44110Jlj0/JCinAJ O1UkrPzsBv1qYqOtmBj0mzMcDCYqg5T7vO+84uUOeTjdz+CMbbiogO70bx7j+cx97rBs19N+OyG 6IAJnpxsC18GirnF15H2hAcsyHbDXJE+xlaDKc8McED2U7KCpXmY= X-Google-Smtp-Source: AGHT+IHff6ZCZ3a77kNIs1sj3YhQ2PuSoZWxg+6jNDPEOpMuOBgzBIEK6UEkagTBVQCJexujrskf7gcN6CxbhnGJtxg= X-Received: by 2002:a05:6a20:7351:b0:1a3:6833:1ccb with SMTP id v17-20020a056a20735100b001a368331ccbmr1039918pzc.40.1710908418227; Tue, 19 Mar 2024 21:20:18 -0700 (PDT) Precedence: bulk X-Mailing-List: linux-i2c@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: Sam Edwards Date: Tue, 19 Mar 2024 22:20:07 -0600 Message-ID: Subject: [RESEND RFC PATCH 1/5] i2c: mv64xxx: Clear bus errors before transfer To: Gregory CLEMENT , Andi Shyti Cc: linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org The MV64XXX hardware can act as either a bus controller or target device. In order to protect target devices from a glitching bus (apparently), the core listens on the lines even when idle and moves the hardware FSM to the "BUS_ERR" state if an invalid transition is detected. The hardware then does not exit this state until reset. This feature is actually counterproductive when using the hardware as a controller (as this driver does): we do not actually *care* what happened on the bus previously, as long as it's ready for use when the new transfer starts. However, the controller will remember a previous glitch and trip up the driver after it attempts to send the start command. The driver logs and error and resets the controller, recovering from the BUS_ERR state, but not without erroring back the transfer with errno EAGAIN. Clients generally do not handle this gracefully. This is easily fixed by checking for the BUS_ERR condition upfront and issuing the hardware reset before beginning the transfer. This patch does NOT also call i2c_recover_bus(): the assumption is that the bus is fine, just the hardware is upset; if the bus is also in a bad state, this should not pass silently. Signed-off-by: Sam Edwards --- drivers/i2c/busses/i2c-mv64xxx.c | 6 ++++++ 1 file changed, 6 insertions(+) if (rc) @@ -762,6 +763,11 @@ mv64xxx_i2c_xfer_core(struct i2c_adapter *adap, struct i2c_msg msgs[], int num) drv_data->msgs = msgs; drv_data->num_msgs = num; + /* Calm down the hardware if it was upset by a bus glitch while idle */ + status = readl(drv_data->reg_base + drv_data->reg_offsets.status); + if (status == MV64XXX_I2C_STATUS_BUS_ERR) + mv64xxx_i2c_hw_init(drv_data); + if (mv64xxx_i2c_can_offload(drv_data) && !drv_data->atomic) rc = mv64xxx_i2c_offload_xfer(drv_data); else -- 2.43.2 diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c index fd8403b07fa6..cfc16909fba3 100644 --- a/drivers/i2c/busses/i2c-mv64xxx.c +++ b/drivers/i2c/busses/i2c-mv64xxx.c @@ -753,6 +753,7 @@ mv64xxx_i2c_xfer_core(struct i2c_adapter *adap, struct i2c_msg msgs[], int num) { struct mv64xxx_i2c_data *drv_data = i2c_get_adapdata(adap); int rc, ret = num; + u32 status; rc = pm_runtime_resume_and_get(&adap->dev); From patchwork Wed Mar 20 04:20:20 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sam Edwards X-Patchwork-Id: 781432 Received: from mail-oo1-f46.google.com (mail-oo1-f46.google.com [209.85.161.46]) (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 9D79917996 for ; Wed, 20 Mar 2024 04:20:31 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.161.46 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710908433; cv=none; b=tZBA21kdjnqGi425YwqcrYyow8Rxq3fsac6olK50bNUDmg6xXplbHScYJoYRc4mfTMkBT1iyxY8a5Wnla1N6BsGKR6SYJ5IOpHSmqF07N4W18vNt6hj+NsvTMUwu0PKEgEJiLjpV6b60tWI9FVWOO9YICTTCc95r+fKp3AUg7H0= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710908433; c=relaxed/simple; bh=BxX5FQpfU7ryyCmToAmFws1m7NS1nntuj2W5QNchcRU=; h=MIME-Version:From:Date:Message-ID:Subject:To:Cc:Content-Type; b=jOUE3k41UXgkOjdE3RMs5xcRQjxNuPV6TwspFGsZ0cGN9Ys0UWV8Mjy4iSl5Ezhna3oHOBCG+g+wZbrYH9l2SjC9bmVQBG2XjR0ZCV01LWsHah0mGje34QFKEwQtm+q2gGrCGyDmu8OfNqpKMIr015cpnI3b9QMpiCTSPKWx6hk= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=turingpi.com; spf=pass smtp.mailfrom=turingpi.com; dkim=pass (2048-bit key) header.d=turingpi.com header.i=@turingpi.com header.b=YJewdRFN; arc=none smtp.client-ip=209.85.161.46 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=turingpi.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=turingpi.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=turingpi.com header.i=@turingpi.com header.b="YJewdRFN" Received: by mail-oo1-f46.google.com with SMTP id 006d021491bc7-5a46ec4050eso3400041eaf.2 for ; Tue, 19 Mar 2024 21:20:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=turingpi.com; s=google; t=1710908430; x=1711513230; darn=vger.kernel.org; h=cc:to:subject:message-id:date:from:mime-version:from:to:cc:subject :date:message-id:reply-to; bh=3fWtKmJ3MfVCnV+E74sqM2ekSX6lKZRkdLbepWZrZ6o=; b=YJewdRFN25r7nvfh0g5ZDPxVjTfWAY5ZgO/HlBsKOXYUpZmDZUqwMwGFTgdHTvyeTS ozUM/vGMhx1btZLeAoiDefT2N0aNMeLmTfhOMohTsbMKV2WCZCZK5+tgrP4uXWF+p9NL JNrE+Z1xkVNaJ1+FjnUTe5uZGL/x0bns1Fxu8Mos4kHkO2bYiJnrFAU6BYuqhe1XCJWJ GC0mFQdD4ljmiVa8bY72SpRlxt+X/wbp20A9q812r3b8WADpfArqHBLqJe7/k/bieyFi WFuwnhuRZMHtMM2ANir0xpUOyGAha8i5Dm7GFIm4HqHBt5C+soeCr2iNaxVWL6E6vXbQ 3R/g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1710908431; x=1711513231; h=cc:to:subject:message-id:date:from:mime-version:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=3fWtKmJ3MfVCnV+E74sqM2ekSX6lKZRkdLbepWZrZ6o=; b=Ccrgf2iTLiMQv1AR88SGFEcHNXyqVTtVwooF6Yh9/tl088JRjTlKLz6PETZDuxeiEv aOt+4HMyKSOE1ngRPQOzZGbiRdbVEaODF5C7FTxDToux84zVZC5BZ9pY8fqxCNcD0XHZ lh52wxGR7rvMcjoER9AjBdviyQU0b6b0OyLe6LPI/Jx0DieM1BWOYBhlzrxjPheF9sqq 0sVaj/6kry2tEinmTCZnm2j/v6n/qxesnB9L9cxZ17qd2pIdeTzXxGkHDN62VwjXI/ly bqFoHleFCkasFn7aXkrZCjDZ+YudxVK8e2X/agyrdw1x9mAvhaXSs1gVjF+aipyMmAGj 2qPA== X-Gm-Message-State: AOJu0YxQxX0BfEKRQLxr+Lx2YO4FkLe9gia4d70YdhlQGDG2zWfyFlSb dDUuHvQ7vSzvY9ktR/y7YApKJsZqeQBR2/pGSiFDaNEEnrs4zJEbttHyGdkhoCWF8wpW4E+Gw7x t3UWcOq3Aba6PMmX2Oo2wJj9p6fD/g2Ub11PbxQ== X-Google-Smtp-Source: AGHT+IFN6M8w1apocywAYQoQVW8GClTgKjY7u+bmQrz7JBQVn0gQDUfr8+LYrMWU5Bh08mcydgynecAP9NjVEwOKlto= X-Received: by 2002:a05:6358:938f:b0:17e:69cf:2105 with SMTP id h15-20020a056358938f00b0017e69cf2105mr16163747rwb.29.1710908430736; Tue, 19 Mar 2024 21:20:30 -0700 (PDT) Precedence: bulk X-Mailing-List: linux-i2c@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: Sam Edwards Date: Tue, 19 Mar 2024 22:20:20 -0600 Message-ID: Subject: [RESEND RFC PATCH 2/5] i2c: mv64xxx: Clean up the private data struct To: Gregory CLEMENT , Andi Shyti Cc: linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org There are many fields in `struct mv64xxx_i2c_data` with `u32` type despite this not being the correct type for those fields. Change the types to accurately reflect what is being kept in each field. Signed-off-by: Sam Edwards --- drivers/i2c/busses/i2c-mv64xxx.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) -- 2.43.2 diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c index cfc16909fba3..bb048e655be7 100644 --- a/drivers/i2c/busses/i2c-mv64xxx.c +++ b/drivers/i2c/busses/i2c-mv64xxx.c @@ -83,7 +83,7 @@ #define MV64XXX_I2C_BRIDGE_STATUS_ERROR BIT(0) /* Driver states */ -enum { +enum mv64xxx_i2c_state { MV64XXX_I2C_STATE_INVALID, MV64XXX_I2C_STATE_IDLE, MV64XXX_I2C_STATE_WAITING_FOR_START_COND, @@ -95,7 +95,7 @@ enum { }; /* Driver actions */ -enum { +enum mv64xxx_i2c_action { MV64XXX_I2C_ACTION_INVALID, MV64XXX_I2C_ACTION_CONTINUE, MV64XXX_I2C_ACTION_SEND_RESTART, @@ -121,21 +121,21 @@ struct mv64xxx_i2c_data { struct i2c_msg *msgs; int num_msgs; int irq; - u32 state; - u32 action; - u32 aborting; + enum mv64xxx_i2c_state state; + enum mv64xxx_i2c_action action; + bool aborting; u32 cntl_bits; void __iomem *reg_base; struct mv64xxx_i2c_regs reg_offsets; - u32 addr1; - u32 addr2; - u32 bytes_left; - u32 byte_posn; - u32 send_stop; - u32 block; + u8 addr1; + u8 addr2; + size_t bytes_left; + size_t byte_posn; + bool send_stop; + bool block; int rc; - u32 freq_m; - u32 freq_n; + u8 freq_m; + u8 freq_n; struct clk *clk; struct clk *reg_clk; wait_queue_head_t waitq; From patchwork Wed Mar 20 04:20:29 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sam Edwards X-Patchwork-Id: 781695 Received: from mail-pf1-f179.google.com (mail-pf1-f179.google.com [209.85.210.179]) (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 E34C62134B for ; Wed, 20 Mar 2024 04:20:40 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.210.179 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710908442; cv=none; b=KzenRJEoeSd0YwqBiOxkyL6hbCYC1E8WoWecN+r2Em95Rc2SuzKQ+XefRtgsBXmFzT58mLH2l/1TUNJ/HrB02z/wlOWSMP3ZVPXCLF4X4WEDUiQO10+1ClGYw1f//QRZfsi4jF6kmhb9se8EvZX8q1Be5RJzCPtfV0+S/FDt5Go= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710908442; c=relaxed/simple; bh=SjbEN4fkrrBL1ck5gPJrGqik62t0NXeQF6UqdJMndQI=; h=MIME-Version:From:Date:Message-ID:Subject:To:Cc:Content-Type; b=Pud5hR344Eq1IO3puF+k9mKLlauIHysdAjcUakhxH0JHzYfV2vToYYLCL1l8sWM5T6seLl7Z0pMqL/zsPXctQ8ywA+HEcKE6YhkMyirER50kk7Vaci6Pyy+MmUKPpacM3Yafw9aUEy5YzqeLvvagAhpu+X6Gvt9yGxA51Bux7i0= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=turingpi.com; spf=pass smtp.mailfrom=turingpi.com; dkim=pass (2048-bit key) header.d=turingpi.com header.i=@turingpi.com header.b=WzM3oORq; arc=none smtp.client-ip=209.85.210.179 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=turingpi.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=turingpi.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=turingpi.com header.i=@turingpi.com header.b="WzM3oORq" Received: by mail-pf1-f179.google.com with SMTP id d2e1a72fcca58-6e6f4ad4c57so4330827b3a.2 for ; Tue, 19 Mar 2024 21:20:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=turingpi.com; s=google; t=1710908440; x=1711513240; darn=vger.kernel.org; h=cc:to:subject:message-id:date:from:mime-version:from:to:cc:subject :date:message-id:reply-to; bh=zXzIlG5TYdMxur/TeuDOa7DGRuUn8c/2Q48Teo2gnXY=; b=WzM3oORqQlMQOKcZLF3NzBZM4MY5wThfEGquETU6NBC7qcxi30OCAn1dipgE8zsPVx MIR4e7YryUG2sya3aZnGtBqJiTlnANuZenUtYVq835TsOObCV20EgL6KoH604EXTzj+y 2WI2ccJo0OvxGb7NR/SiYPIAjVMNdJ7zsAP3wAUdvUc/vVI/PDy+S1LEQZN34YrUYg/5 x3Dgw3W9YCOZvwUNKsci0XHrn/OXrIIvqTsUMgy2lkHigl65WvvjnMulEfhKsp0hkztB aYpV1dLSGcw3QvOOZo+NJMZDoNJnOcyVFOnV7DPs8g0hMd/Sel+wpA7yxnS/pc1IhzhW ByHA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1710908440; x=1711513240; h=cc:to:subject:message-id:date:from:mime-version:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=zXzIlG5TYdMxur/TeuDOa7DGRuUn8c/2Q48Teo2gnXY=; b=iwCgqm0FOwtZ7LLtySVb693Nk/6Fy5g/pMzaMSU9ViRRsqG/GA0y2FqBZsWw6UoOVd uU/VwC8qI6LpOOvlQUWra4p4T6MKTEkIHGrSBR6JvrOdArpKiHJnsItSYvZ4d1AGr58b v0gi8s8huUnYcmc5VgZM3NTu2dKannl6si3m9GqYblazOJinuH/IaCIrEQAIpHjPnNVU PjyU0IT8gNV70hUjxsxlnWFVOLxvpJDZpNPD1REThZxvDW8cllLrWLRoKimCRbLjT9PE X1ztUwUZXUQEn/a3958znYb1nBJNgiuYS2cnwuTF5CVxGfjmqRZWRfVeaQhP0OePFjcr +aHw== X-Gm-Message-State: AOJu0YxCJLUjh1Ab8QK7NTpiy3Dc3X9TotD7w4UCXKfyR2PXOUObaMWI 7zTCHxaB4V0ez7bwuY6vlHdo934DakMj0sylgCRW7VFTV/pIfwt3q5YnErf/Bv2weiiMJ8YMMXd y3EQhFzf9i3qWGfrjD2Q+AqM7De7drD5/5vTYFQ== X-Google-Smtp-Source: AGHT+IHLc/sHqe1BPJZKiM9d8WGXJt3ykzdFPowhPBIUGdabOE1ph8nHAaE8apaaQ5wtFd5++lXLpeaXEcj08Ikb60M= X-Received: by 2002:a05:6a20:3c9e:b0:1a3:4e19:1831 with SMTP id b30-20020a056a203c9e00b001a34e191831mr5564805pzj.13.1710908440204; Tue, 19 Mar 2024 21:20:40 -0700 (PDT) Precedence: bulk X-Mailing-List: linux-i2c@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: Sam Edwards Date: Tue, 19 Mar 2024 22:20:29 -0600 Message-ID: Subject: [RESEND RFC PATCH 3/5] i2c: mv64xxx: Refactor FSM To: Gregory CLEMENT , Andi Shyti Cc: linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org Conceptually, there are two FSMs here: a hardware FSM in the MV64XXX itself, and a software FSM in the driver. The current software FSM is not a "real" FSM: it is just using the hardware status to decide what to do next, and the `state` is mostly unused. There are two drawbacks to this approach: 1) If the hardware returns an unexpected status, the driver will accept it blindly, allowing bugs to go unnoticed and complicating testing. 2) The driver FSM cannot have states/transitions not represented in hardware. Rework this by making the hardware status decoder state-aware, and introducing an enum of "events" which can be fed to the driver FSM that reflect the hardware events deduced by the status decoder. Any unexpected status results in an "invalid" event, which triggers the driver's error recovery. The state machine logic is otherwise the same: the sequence of actions emitted by the FSM is unchanged by this patch. Note: The meaning of bytes_left in reads is now the number of byte reads left to *start* on the hardware, not the number of bytes left to be read from the data register. Signed-off-by: Sam Edwards --- drivers/i2c/busses/i2c-mv64xxx.c | 270 +++++++++++++++++++++---------- 1 file changed, 185 insertions(+), 85 deletions(-) + /* * If state is idle, then this is likely the remnants of an old * operation that driver has given up on or the user has killed. @@ -245,99 +323,121 @@ mv64xxx_i2c_fsm(struct mv64xxx_i2c_data *drv_data, u32 status) return; } - /* The status from the ctlr [mostly] tells us what to do next */ - switch (status) { - /* Start condition interrupt */ - case MV64XXX_I2C_STATUS_MAST_START: /* 0x08 */ - case MV64XXX_I2C_STATUS_MAST_REPEAT_START: /* 0x10 */ - drv_data->action = MV64XXX_I2C_ACTION_SEND_ADDR_1; - drv_data->state = MV64XXX_I2C_STATE_WAITING_FOR_ADDR_1_ACK; + /* + * The FSM is broken into 3 parts: + * 1) Decode `status` to determine the underlying hardware event + * 2) Handle hardware event driven state transitions + * 3) Perform internal state transitions and action emission + */ + event = mv64xxx_i2c_decode_status(drv_data, status); + + /* Handle event; determine state transition */ + switch (event) { + case MV64XXX_I2C_EVENT_STARTED: + drv_data->state = MV64XXX_I2C_STATE_SEND_ADDR_1; break; - /* Performing a write */ - case MV64XXX_I2C_STATUS_MAST_WR_ADDR_ACK: /* 0x18 */ - if (drv_data->msg->flags & I2C_M_TEN) { - drv_data->action = MV64XXX_I2C_ACTION_SEND_ADDR_2; - drv_data->state = - MV64XXX_I2C_STATE_WAITING_FOR_ADDR_2_ACK; - break; - } - fallthrough; - case MV64XXX_I2C_STATUS_MAST_WR_ADDR_2_ACK: /* 0xd0 */ - case MV64XXX_I2C_STATUS_MAST_WR_ACK: /* 0x28 */ - if ((drv_data->bytes_left == 0) - || (drv_data->aborting - && (drv_data->byte_posn != 0))) { - if (drv_data->send_stop || drv_data->aborting) { - drv_data->action = MV64XXX_I2C_ACTION_SEND_STOP; - drv_data->state = MV64XXX_I2C_STATE_IDLE; - } else { - drv_data->action = - MV64XXX_I2C_ACTION_SEND_RESTART; - drv_data->state = - MV64XXX_I2C_STATE_WAITING_FOR_RESTART; - } - } else { - drv_data->action = MV64XXX_I2C_ACTION_SEND_DATA; - drv_data->state = - MV64XXX_I2C_STATE_WAITING_FOR_SLAVE_ACK; - drv_data->bytes_left--; - } + case MV64XXX_I2C_EVENT_ADDR_ACK: + if ((drv_data->state == MV64XXX_I2C_STATE_SEND_ADDR_1) + && (drv_data->msg->flags & I2C_M_TEN)) + drv_data->state = MV64XXX_I2C_STATE_SEND_ADDR_2; + else if (drv_data->msg->flags & I2C_M_RD) + drv_data->state = MV64XXX_I2C_STATE_READ; + else + drv_data->state = MV64XXX_I2C_STATE_WRITE; break; - /* Performing a read */ - case MV64XXX_I2C_STATUS_MAST_RD_ADDR_ACK: /* 40 */ - if (drv_data->msg->flags & I2C_M_TEN) { - drv_data->action = MV64XXX_I2C_ACTION_SEND_ADDR_2; - drv_data->state = - MV64XXX_I2C_STATE_WAITING_FOR_ADDR_2_ACK; - break; - } - fallthrough; - case MV64XXX_I2C_STATUS_MAST_RD_ADDR_2_ACK: /* 0xe0 */ - if (drv_data->bytes_left == 0) { - drv_data->action = MV64XXX_I2C_ACTION_SEND_STOP; - drv_data->state = MV64XXX_I2C_STATE_IDLE; - break; - } - fallthrough; - case MV64XXX_I2C_STATUS_MAST_RD_DATA_ACK: /* 0x50 */ - if (status != MV64XXX_I2C_STATUS_MAST_RD_DATA_ACK) - drv_data->action = MV64XXX_I2C_ACTION_CONTINUE; - else { - drv_data->action = MV64XXX_I2C_ACTION_RCV_DATA; - drv_data->bytes_left--; - } - drv_data->state = MV64XXX_I2C_STATE_WAITING_FOR_SLAVE_DATA; + case MV64XXX_I2C_EVENT_ADDR_NO_ACK: + case MV64XXX_I2C_EVENT_WR_NO_ACK: + /* Doesn't seem to be a device at other end */ + drv_data->state = MV64XXX_I2C_STATE_IDLE; + break; - if ((drv_data->bytes_left == 1) || drv_data->aborting) - drv_data->cntl_bits &= ~MV64XXX_I2C_REG_CONTROL_ACK; + case MV64XXX_I2C_EVENT_WR_ACK: break; - case MV64XXX_I2C_STATUS_MAST_RD_DATA_NO_ACK: /* 0x58 */ - drv_data->action = MV64XXX_I2C_ACTION_RCV_DATA_STOP; - drv_data->state = MV64XXX_I2C_STATE_IDLE; + case MV64XXX_I2C_EVENT_RD_ACKED: + BUG_ON(drv_data->bytes_left == 0); break; - case MV64XXX_I2C_STATUS_MAST_WR_ADDR_NO_ACK: /* 0x20 */ - case MV64XXX_I2C_STATUS_MAST_WR_NO_ACK: /* 30 */ - case MV64XXX_I2C_STATUS_MAST_RD_ADDR_NO_ACK: /* 48 */ - /* Doesn't seem to be a device at other end */ - drv_data->action = MV64XXX_I2C_ACTION_SEND_STOP; - drv_data->state = MV64XXX_I2C_STATE_IDLE; - drv_data->rc = -ENXIO; + case MV64XXX_I2C_EVENT_RD_UNACKED: + BUG_ON(drv_data->bytes_left != 0); break; + case MV64XXX_I2C_EVENT_INVALID: default: dev_err(&drv_data->adapter.dev, "mv64xxx_i2c_fsm: Ctlr Error -- state: 0x%x, " - "status: 0x%x, addr: 0x%x, flags: 0x%x\n", - drv_data->state, status, drv_data->msg->addr, + "status: 0x%x, event: 0x%x, addr: 0x%x, flags: 0x%x\n", + drv_data->state, status, event, drv_data->msg->addr, drv_data->msg->flags); drv_data->action = MV64XXX_I2C_ACTION_SEND_STOP; mv64xxx_i2c_hw_init(drv_data); i2c_recover_bus(&drv_data->adapter); drv_data->rc = -EAGAIN; + return; + } + + /* Internal FSM transitions and action emission */ + switch (drv_data->state) { + case MV64XXX_I2C_STATE_IDLE: + drv_data->action = MV64XXX_I2C_ACTION_SEND_STOP; + drv_data->rc = -ENXIO; + break; + + case MV64XXX_I2C_STATE_SEND_ADDR_1: + drv_data->action = MV64XXX_I2C_ACTION_SEND_ADDR_1; + break; + + case MV64XXX_I2C_STATE_SEND_ADDR_2: + drv_data->action = MV64XXX_I2C_ACTION_SEND_ADDR_2; + break; + + case MV64XXX_I2C_STATE_READ: + if (drv_data->bytes_left == 0) { + if (prev_state == MV64XXX_I2C_STATE_READ) + drv_data->action = MV64XXX_I2C_ACTION_RCV_DATA_STOP; + else + drv_data->action = MV64XXX_I2C_ACTION_SEND_STOP; + drv_data->state = MV64XXX_I2C_STATE_IDLE; + } else { + if (prev_state == MV64XXX_I2C_STATE_READ) + drv_data->action = MV64XXX_I2C_ACTION_RCV_DATA; + else + drv_data->action = MV64XXX_I2C_ACTION_CONTINUE; + + /* + * bytes_left counts the remaining read actions to send + * to the hardware, not the remaining bytes to be + * retrieved from the data register + */ + if (drv_data->aborting) + drv_data->bytes_left = 0; + else + drv_data->bytes_left--; + + if (drv_data->bytes_left == 0) + drv_data->cntl_bits &= ~MV64XXX_I2C_REG_CONTROL_ACK; + } + break; + + case MV64XXX_I2C_STATE_WRITE: + if ((drv_data->bytes_left == 0) + || (drv_data->aborting && (drv_data->byte_posn != 0))) { + if (drv_data->send_stop || drv_data->aborting) { + drv_data->action = MV64XXX_I2C_ACTION_SEND_STOP; + drv_data->state = MV64XXX_I2C_STATE_IDLE; + } else { + drv_data->action = MV64XXX_I2C_ACTION_SEND_RESTART; + drv_data->state = MV64XXX_I2C_STATE_RESTART; + } + } else { + drv_data->action = MV64XXX_I2C_ACTION_SEND_DATA; + drv_data->bytes_left--; + } + break; + + default: } } @@ -611,7 +711,7 @@ mv64xxx_i2c_execute_msg(struct mv64xxx_i2c_data *drv_data, struct i2c_msg *msg, spin_lock_irqsave(&drv_data->lock, flags); - drv_data->state = MV64XXX_I2C_STATE_WAITING_FOR_START_COND; + drv_data->state = MV64XXX_I2C_STATE_START; drv_data->send_stop = is_last; drv_data->block = 1; -- 2.43.2 diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c index bb048e655be7..3ae74160001d 100644 --- a/drivers/i2c/busses/i2c-mv64xxx.c +++ b/drivers/i2c/busses/i2c-mv64xxx.c @@ -4,10 +4,12 @@ * * Author: Mark A. Greer * - * 2005 (c) MontaVista, Software, Inc. This file is licensed under - * the terms of the GNU General Public License version 2. This program - * is licensed "as is" without any warranty of any kind, whether express - * or implied. + * 2005 (c) MontaVista, Software, Inc. + * Copyright (c) 2024 Turing Machines, Inc. + * + * This file is licensed under the terms of the GNU General Public License v2. + * This program is licensed "as is" without any warranty of any kind, whether + * express or implied. */ #include #include @@ -86,12 +88,24 @@ enum mv64xxx_i2c_state { MV64XXX_I2C_STATE_INVALID, MV64XXX_I2C_STATE_IDLE, - MV64XXX_I2C_STATE_WAITING_FOR_START_COND, - MV64XXX_I2C_STATE_WAITING_FOR_RESTART, - MV64XXX_I2C_STATE_WAITING_FOR_ADDR_1_ACK, - MV64XXX_I2C_STATE_WAITING_FOR_ADDR_2_ACK, - MV64XXX_I2C_STATE_WAITING_FOR_SLAVE_ACK, - MV64XXX_I2C_STATE_WAITING_FOR_SLAVE_DATA, + MV64XXX_I2C_STATE_START, + MV64XXX_I2C_STATE_RESTART, + MV64XXX_I2C_STATE_SEND_ADDR_1, + MV64XXX_I2C_STATE_SEND_ADDR_2, + MV64XXX_I2C_STATE_WRITE, + MV64XXX_I2C_STATE_READ, +}; + +/* Driver events */ +enum mv64xxx_i2c_event { + MV64XXX_I2C_EVENT_INVALID, + MV64XXX_I2C_EVENT_STARTED, + MV64XXX_I2C_EVENT_ADDR_ACK, + MV64XXX_I2C_EVENT_ADDR_NO_ACK, + MV64XXX_I2C_EVENT_WR_ACK, + MV64XXX_I2C_EVENT_WR_NO_ACK, + MV64XXX_I2C_EVENT_RD_ACKED, + MV64XXX_I2C_EVENT_RD_UNACKED, }; /* Driver actions */ @@ -232,9 +246,73 @@ mv64xxx_i2c_hw_init(struct mv64xxx_i2c_data *drv_data) drv_data->state = MV64XXX_I2C_STATE_IDLE; } +static enum mv64xxx_i2c_event +mv64xxx_i2c_decode_status(struct mv64xxx_i2c_data *drv_data, u32 status) +{ + /* Decode status to event (state-driven; catches unexpected status) */ + switch (drv_data->state) { + case MV64XXX_I2C_STATE_RESTART: + case MV64XXX_I2C_STATE_START: + if (status == MV64XXX_I2C_STATUS_MAST_START || + status == MV64XXX_I2C_STATUS_MAST_REPEAT_START) + return MV64XXX_I2C_EVENT_STARTED; + return MV64XXX_I2C_EVENT_INVALID; + + case MV64XXX_I2C_STATE_SEND_ADDR_1: + if (drv_data->msg->flags & I2C_M_RD) { + if (status == MV64XXX_I2C_STATUS_MAST_RD_ADDR_ACK) + return MV64XXX_I2C_EVENT_ADDR_ACK; + else if (status == MV64XXX_I2C_STATUS_MAST_RD_ADDR_NO_ACK) + return MV64XXX_I2C_EVENT_ADDR_NO_ACK; + } else { + if (status == MV64XXX_I2C_STATUS_MAST_WR_ADDR_ACK) + return MV64XXX_I2C_EVENT_ADDR_ACK; + else if (status == MV64XXX_I2C_STATUS_MAST_WR_ADDR_NO_ACK) + return MV64XXX_I2C_EVENT_ADDR_NO_ACK; + } + return MV64XXX_I2C_EVENT_INVALID; + + case MV64XXX_I2C_STATE_SEND_ADDR_2: + if (drv_data->msg->flags & I2C_M_RD) { + if (status == MV64XXX_I2C_STATUS_MAST_RD_ADDR_2_ACK) + return MV64XXX_I2C_EVENT_ADDR_ACK; + else if (status == MV64XXX_I2C_STATUS_MAST_RD_ADDR_2_NO_ACK) + return MV64XXX_I2C_EVENT_ADDR_NO_ACK; + } else { + if (status == MV64XXX_I2C_STATUS_MAST_WR_ADDR_2_ACK) + return MV64XXX_I2C_EVENT_ADDR_ACK; + else if (status == MV64XXX_I2C_STATUS_MAST_WR_ADDR_2_NO_ACK) + return MV64XXX_I2C_EVENT_ADDR_NO_ACK; + } + return MV64XXX_I2C_EVENT_INVALID; + + case MV64XXX_I2C_STATE_WRITE: + if (status == MV64XXX_I2C_STATUS_MAST_WR_ACK) + return MV64XXX_I2C_EVENT_WR_ACK; + else if (status == MV64XXX_I2C_STATUS_MAST_WR_NO_ACK) + return MV64XXX_I2C_EVENT_ADDR_NO_ACK; + return MV64XXX_I2C_EVENT_INVALID; + + case MV64XXX_I2C_STATE_READ: + if (status == MV64XXX_I2C_STATUS_MAST_RD_DATA_ACK) + return MV64XXX_I2C_EVENT_RD_ACKED; + else if (status == MV64XXX_I2C_STATUS_MAST_RD_DATA_NO_ACK) + return MV64XXX_I2C_EVENT_RD_UNACKED; + return MV64XXX_I2C_EVENT_INVALID; + + default: + return MV64XXX_I2C_EVENT_INVALID; + } +} + static void mv64xxx_i2c_fsm(struct mv64xxx_i2c_data *drv_data, u32 status) { + enum mv64xxx_i2c_event event; + enum mv64xxx_i2c_state prev_state = drv_data->state; + + drv_data->action = MV64XXX_I2C_ACTION_INVALID; From patchwork Wed Mar 20 04:20:39 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sam Edwards X-Patchwork-Id: 781431 Received: from mail-pj1-f53.google.com (mail-pj1-f53.google.com [209.85.216.53]) (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 B985412E48 for ; Wed, 20 Mar 2024 04:20:50 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.216.53 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710908452; cv=none; b=Hbe5mqUHr+f47A74VePikfGBX0w1kUUYQQnJ3yxRwLvlqkp3CVnQzkr0Zl+eRARZKSVDLn9gmWqCeMsyMOHJF2whpr139pi0GtOUM7lCwUMuG0JgE6MKOgfld+9kwR520WIueiDd1KNeqMB2ozp1s07QBdcOeafGOWBIxtC/oBA= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710908452; c=relaxed/simple; bh=UlPOs3tIIxZhbkUElypu4cny/3BJ5z+E4k+yUP6J+Oo=; h=MIME-Version:From:Date:Message-ID:Subject:To:Cc:Content-Type; b=eSfjiHRaHr16u+PjSfho7xirlsrdIKzlhA+/rSrvRN8JU4fpm8clbTwxysncqpL9JJN0oytkzKu+piaT2oJg1WuXAk93KDdk3/ejwjU92koTnqsM3A10c7LlCclZWtf2qjLbwqPkblT1PGdThODihBqIOeu+WgWRGHY92cMl8NQ= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=turingpi.com; spf=pass smtp.mailfrom=turingpi.com; dkim=pass (2048-bit key) header.d=turingpi.com header.i=@turingpi.com header.b=MXUEz6/r; arc=none smtp.client-ip=209.85.216.53 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=turingpi.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=turingpi.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=turingpi.com header.i=@turingpi.com header.b="MXUEz6/r" Received: by mail-pj1-f53.google.com with SMTP id 98e67ed59e1d1-29fb7e52731so1991911a91.2 for ; Tue, 19 Mar 2024 21:20:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=turingpi.com; s=google; t=1710908450; x=1711513250; darn=vger.kernel.org; h=cc:to:subject:message-id:date:from:mime-version:from:to:cc:subject :date:message-id:reply-to; bh=W3pFWuhAHC9co53c1lcHeStcZG/8RoQn2TUU6VVtrFc=; b=MXUEz6/rSatSZ/i5E/BQSAI6FK00FcgCThel+7C9nM9FwWzLhYYhyjREgSOBby+kjK 1rRUgQlBtAqPd43Lf1hSxTzPwavSc79m7xubHI9MwFiJFJPEE5iUDvAEj8JUxuFf5Xqu M66Waf7XjG3gUHpXhCrcEknb5CaEsouzj1Y4ZR36C65vzaDJdVkBh/E4k7dAyphBvB3v Sy0NcAz1ucU7pcBgmWdOKFPLbo0kFeK1J4MxgEiy6SAnLX7KaMis4gSMINA1SoBGVytV +eribLdpEsJHuJvau5y3bpbv/pxTtNdI3E0xtCZiJ5h3rtvh2v0kn3/801pQvoDNvSax qdmA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1710908450; x=1711513250; h=cc:to:subject:message-id:date:from:mime-version:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=W3pFWuhAHC9co53c1lcHeStcZG/8RoQn2TUU6VVtrFc=; b=dId3h5A80lGLCckNcBMy2FxvREswEs+duf0GHBQPyGUZrrVoDiYa225jKA7SWm9nU6 dAq0iTRP0Mo2hmsKNAJbU+uu7Rgj7OOguRK94h7EaUk2diSaJk+4Egyuve2IRSW3Bi3E 3tjoXOfeOA36t+vtMPXfRr3OLDHDjMrTQa12lvg/vta9y+iDAbW6bgH9V3eCZkTvdPFK k5Q+OdZ9gPQ4ZxcbpRXLdUKmItl9nGGo5ckS7fqIk+vGUfy5YuYWmFPexgrfkKFIJKd3 ABSPgyMYTPX5Az4Xodu80CZEtEuikcagTVdkndjuaMeqQaFBmvmXFr3YgFRlgBH2zuQf uBxg== X-Gm-Message-State: AOJu0Yz0Enk8/JXUSmJIJbzs0Qtb5pC1gLlpjxqKlkF+Q6IfGvqoWFFa Ym4hLiPesyLcCYBebo8nGo33t00+RUunkr2F9hAV8PZCaXM1pPEcAEKqq8unf/So91dLSNHp2iv kErvBZVUhB4f/iVpO82OW+/w7pBAe3D8EbFMF6U8eujPOKl7a8uc= X-Google-Smtp-Source: AGHT+IFk5PDxWwRKK7+uq9PFy7yBCr9PGqYEewLxuuXFT9HM6SkCFkeNAFShZf1ykJJGazkzJG+dUcAvHTYhM3S9rdA= X-Received: by 2002:a17:90b:1056:b0:29b:acc6:c54f with SMTP id gq22-20020a17090b105600b0029bacc6c54fmr4441532pjb.35.1710908450007; Tue, 19 Mar 2024 21:20:50 -0700 (PDT) Precedence: bulk X-Mailing-List: linux-i2c@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: Sam Edwards Date: Tue, 19 Mar 2024 22:20:39 -0600 Message-ID: Subject: [RESEND RFC PATCH 4/5] i2c: mv64xxx: Allow continuing after read To: Gregory CLEMENT , Andi Shyti Cc: linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org The current FSM does not check the `send_stop` flag when completing a read message; it just assumes any read message is always the end of the transfer. This means subsequent messages go unprocessed, with no error code to indicate otherwise. Fixing this requires that the FSM check the `send_stop` flag and issue the `SEND_RESTART` action instead of stop, but there are only two RCV_* variants, one for receive-and-continue and one for receive-and-stop. We could add another variant, however, the new FSM from the previous patch makes it pretty clear that the RCV_* variants aren't really full actions in their own respect, since they both implement the same observable functionality as another action, just with an added read from the data register first. Therefore, convert the receive actions to a flag that can be set, allowing any action to have an "...and also read" variant. The FSM can then just use the plain SEND_RESTART action, but OR-in the flag, to represent "read a byte, finish this message, go to the next message." Signed-off-by: Sam Edwards --- drivers/i2c/busses/i2c-mv64xxx.c | 47 +++++++++++--------------------- 1 file changed, 16 insertions(+), 31 deletions(-) /* * bytes_left counts the remaining read actions to send @@ -419,6 +418,8 @@ mv64xxx_i2c_fsm(struct mv64xxx_i2c_data *drv_data, u32 status) if (drv_data->bytes_left == 0) drv_data->cntl_bits &= ~MV64XXX_I2C_REG_CONTROL_ACK; } + if (prev_state == MV64XXX_I2C_STATE_READ) + drv_data->action |= MV64XXX_I2C_ACTION_RECEIVE; break; case MV64XXX_I2C_STATE_WRITE: @@ -457,6 +458,11 @@ static void mv64xxx_i2c_send_start(struct mv64xxx_i2c_data *drv_data) static void mv64xxx_i2c_do_action(struct mv64xxx_i2c_data *drv_data) { + if (drv_data->action & MV64XXX_I2C_ACTION_RECEIVE) + drv_data->msg->buf[drv_data->byte_posn++] = + readl(drv_data->reg_base + drv_data->reg_offsets.data); + drv_data->action &= ~MV64XXX_I2C_ACTION_RECEIVE; + switch(drv_data->action) { case MV64XXX_I2C_ACTION_SEND_RESTART: /* We should only get here if we have further messages */ @@ -503,27 +509,6 @@ mv64xxx_i2c_do_action(struct mv64xxx_i2c_data *drv_data) drv_data->reg_base + drv_data->reg_offsets.control); break; - case MV64XXX_I2C_ACTION_RCV_DATA: - drv_data->msg->buf[drv_data->byte_posn++] = - readl(drv_data->reg_base + drv_data->reg_offsets.data); - writel(drv_data->cntl_bits, - drv_data->reg_base + drv_data->reg_offsets.control); - break; - - case MV64XXX_I2C_ACTION_RCV_DATA_STOP: - drv_data->msg->buf[drv_data->byte_posn++] = - readl(drv_data->reg_base + drv_data->reg_offsets.data); - if (!drv_data->atomic) - drv_data->cntl_bits &= ~MV64XXX_I2C_REG_CONTROL_INTEN; - writel(drv_data->cntl_bits | MV64XXX_I2C_REG_CONTROL_STOP, - drv_data->reg_base + drv_data->reg_offsets.control); - drv_data->block = 0; - if (drv_data->errata_delay) - udelay(5); - - wake_up(&drv_data->waitq); - break; - case MV64XXX_I2C_ACTION_INVALID: default: dev_err(&drv_data->adapter.dev, -- 2.43.2 diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c index 3ae74160001d..6a205cca603a 100644 --- a/drivers/i2c/busses/i2c-mv64xxx.c +++ b/drivers/i2c/busses/i2c-mv64xxx.c @@ -116,9 +116,9 @@ enum mv64xxx_i2c_action { MV64XXX_I2C_ACTION_SEND_ADDR_1, MV64XXX_I2C_ACTION_SEND_ADDR_2, MV64XXX_I2C_ACTION_SEND_DATA, - MV64XXX_I2C_ACTION_RCV_DATA, - MV64XXX_I2C_ACTION_RCV_DATA_STOP, MV64XXX_I2C_ACTION_SEND_STOP, + + MV64XXX_I2C_ACTION_RECEIVE = 0x80, }; struct mv64xxx_i2c_regs { @@ -395,16 +395,15 @@ mv64xxx_i2c_fsm(struct mv64xxx_i2c_data *drv_data, u32 status) case MV64XXX_I2C_STATE_READ: if (drv_data->bytes_left == 0) { - if (prev_state == MV64XXX_I2C_STATE_READ) - drv_data->action = MV64XXX_I2C_ACTION_RCV_DATA_STOP; - else + if (drv_data->send_stop || drv_data->aborting) { drv_data->action = MV64XXX_I2C_ACTION_SEND_STOP; - drv_data->state = MV64XXX_I2C_STATE_IDLE; + drv_data->state = MV64XXX_I2C_STATE_IDLE; + } else { + drv_data->action = MV64XXX_I2C_ACTION_SEND_RESTART; + drv_data->state = MV64XXX_I2C_STATE_RESTART; + } } else { - if (prev_state == MV64XXX_I2C_STATE_READ) - drv_data->action = MV64XXX_I2C_ACTION_RCV_DATA; - else - drv_data->action = MV64XXX_I2C_ACTION_CONTINUE; + drv_data->action = MV64XXX_I2C_ACTION_CONTINUE; From patchwork Wed Mar 20 04:20:47 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sam Edwards X-Patchwork-Id: 781694 Received: from mail-pf1-f170.google.com (mail-pf1-f170.google.com [209.85.210.170]) (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 37E25171A5 for ; Wed, 20 Mar 2024 04:20:58 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.210.170 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710908459; cv=none; b=jk+97RVhGGhWtccGCyd5KQuuggK3dwMnlid0/3QXm0nD6s8K3J7bpidtJOt1mcQlUi3q0ZusTY7wCUlGtaHSksz8iUsljJYoeFyJkW89PBkTfZTb4RHed6ZBlijKxRrspG51p8vj0YAlcmsChgdfigyjTLXrloFP37k4bQCoBxE= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710908459; c=relaxed/simple; bh=PrvF/T8vamkKirx2Ed0QZ95luLCroeOaj40bsFQsGrs=; h=MIME-Version:From:Date:Message-ID:Subject:To:Cc:Content-Type; b=BS9BMyVvoGSAD1rXdWJOkrPv/IRC6Exqrtpw2HVon502KaDTkMDmvshalYiNoXsGtuoO1yiJvL2ZHkFNzYoOSCRXkIpvQYQBZ12xxLlnqcoaL6I/u6ZLBJOwS1IoVLfeBircwJbtmeplit86MaGMwEaxbZkL2AwBGcHKYP/leQI= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=turingpi.com; spf=pass smtp.mailfrom=turingpi.com; dkim=pass (2048-bit key) header.d=turingpi.com header.i=@turingpi.com header.b=TEyf7xLL; arc=none smtp.client-ip=209.85.210.170 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=turingpi.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=turingpi.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=turingpi.com header.i=@turingpi.com header.b="TEyf7xLL" Received: by mail-pf1-f170.google.com with SMTP id d2e1a72fcca58-6e6f69e850bso5268745b3a.0 for ; Tue, 19 Mar 2024 21:20:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=turingpi.com; s=google; t=1710908457; x=1711513257; darn=vger.kernel.org; h=cc:to:subject:message-id:date:from:mime-version:from:to:cc:subject :date:message-id:reply-to; bh=5N4n+mF0sQfZVR1O8pkqc3ga6bp8MYBRjMBDjAzSasA=; b=TEyf7xLL9TpQpDxlHmpLpNyRH4GEaVl3ewGjdoDx5LJG80sKlRjC5bjrH5pUu64PsL xoo4E3gNzzEMQowB11db2dlJHJhJuxrUIeGrHwtXkSfwVTUhPsSSc7VaOIsqVT0Ltw+U SEB4pbb0Y1UbV0jweNEHxHMdvOK81yXwvrVjpHWJ+L4J/boilgpM1p0FdEsWq2E/mSj7 Dks90XYXXuuy6wqJF7NUhfzxMslV59cbFM0bfMVyMvCDpccPojpwPEqjObMHvreifiaU v0iTke3vUGRS1X9aspDF6nXumlXLXpScjmyOjSazD3FVfn9iPgAUVJ3kNAbVCvRHpW/5 3KdQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1710908457; x=1711513257; h=cc:to:subject:message-id:date:from:mime-version:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=5N4n+mF0sQfZVR1O8pkqc3ga6bp8MYBRjMBDjAzSasA=; b=Yp7z6DjyciLh3w6Y5Pkz5QiIh3OYE8LlCz+0l+boqr0rz3Co2j3KlzQW4wFCg2XEeF 1kpOiaLPcF4sla0CPUzS9MWdMBWADLjlf5KoT4Q23envx6292Rj71ItUC59O0VAKXPkn vANkJmU8L6olZT1QQtYgGgCHHLkgBanEQ1PgYZVKkQdrFotd/DgM0Kga90RsciQurEn9 SoIwbw84YH6EcxFU9UyhiVRbBwOFBntAEqMWOOHsJXBLpQI9IbM+xSZZ7jMItBngrQbS 9y2kIzoISzyV8+obv+e5XNx7Ktu2tYST2JeAnn0X4P96Lovrk8OmxCs3QIRZbQrRp6Z3 QJxA== X-Gm-Message-State: AOJu0Yw2KKwoSFiDsOCpPUiED42VMXzmA+980STMFKbchJn6JMZ3jZjo CYFOAnc6PQmtTs+1jL/JzbhCvNLxJkEMzkJ1DhAwtwiMIPGckNxcIQAJkhyNu4Q2FHKdlSHUWw0 J1mG/sQtdlSQAOpF7CdHOU6wbgUj3/tGH1YWfHcCnvvsMXxiSve4= X-Google-Smtp-Source: AGHT+IHBr0yhP5f6enGUJQ4kL+Fl6ugvO8zbzcifbubCn8BjFdgtROsuURc338iJGIFQQutamscOtz8zUkM4vHQaVto= X-Received: by 2002:a05:6a20:9e4d:b0:1a3:8984:b334 with SMTP id mt13-20020a056a209e4d00b001a38984b334mr65141pzb.22.1710908457706; Tue, 19 Mar 2024 21:20:57 -0700 (PDT) Precedence: bulk X-Mailing-List: linux-i2c@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: Sam Edwards Date: Tue, 19 Mar 2024 22:20:47 -0600 Message-ID: Subject: [RESEND RFC PATCH 5/5] i2c: mv64xxx: Implement I2C_FUNC_NOSTART To: Gregory CLEMENT , Andi Shyti Cc: linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org There are certain I2C devices [1] that require some extra parameter bytes in any read operation, after the start byte but before the bus turnaround, in contrast to the "conventional" approach of using a normal write to parameterize the upcoming read. The (Allwinner variant of) mv64xxx supports this kind of transfer, for up to 3 bytes of additional data, through a register called "TWI_EFR" ("Enhance Feature Register"). These devices can be supported by setting this register appropriately when beginning the read operation. In absence of a specialized flag to represent this in Linux's I2C subsystem, clients use I2C_M_NOSTART to achieve this. In fact, this appears to be the most common use of I2C_M_NOSTART in the Linux codebase, with the second most common being to implement "gather" I/O by chaining a series of NOSTART writes. This patch implements both of these use cases when the EFR is present: 1) Continuing a write message with further write bytes, as a sort of "gather" operation. 2) Inserting extra parameter bytes after the address of a read operation, using a zero-byte read, a small (<= 3 bytes) NOSTART write, and then a NOSTART read. ...the hardware is likely too strict to allow any other uses, so the message bundle is checked for proper use of NOSTART before it begins. That being said, there's a good chance that these are the only two uses of NOSTART "in the wild," which would mean that this isn't much of a limitation. The implementation redesigns the hardware event handler slightly, so that the FSM can be invoked in a loop if the do_action function generates follow-up events. The NEXT_MESSAGE (formerly SEND_RESTART) action now results in either a restart, or (for NOSTART) a follow-up NOSTART event to the FSM, which allows it to bypass the entire start sequence and jump straight to the data. [1]: See e.g. `as5011_i2c_read`, `ivch_read`, `maven_get_reg` Signed-off-by: Sam Edwards --- drivers/i2c/busses/i2c-mv64xxx.c | 105 +++++++++++++++++++++++++++---- 1 file changed, 94 insertions(+), 11 deletions(-) drv_data->action = MV64XXX_I2C_ACTION_INVALID; @@ -329,7 +332,6 @@ mv64xxx_i2c_fsm(struct mv64xxx_i2c_data *drv_data, u32 status) * 2) Handle hardware event driven state transitions * 3) Perform internal state transitions and action emission */ - event = mv64xxx_i2c_decode_status(drv_data, status); /* Handle event; determine state transition */ switch (event) { @@ -337,6 +339,7 @@ mv64xxx_i2c_fsm(struct mv64xxx_i2c_data *drv_data, u32 status) drv_data->state = MV64XXX_I2C_STATE_SEND_ADDR_1; break; + case MV64XXX_I2C_EVENT_NOSTART: case MV64XXX_I2C_EVENT_ADDR_ACK: if ((drv_data->state == MV64XXX_I2C_STATE_SEND_ADDR_1) && (drv_data->msg->flags & I2C_M_TEN)) @@ -399,7 +402,7 @@ mv64xxx_i2c_fsm(struct mv64xxx_i2c_data *drv_data, u32 status) drv_data->action = MV64XXX_I2C_ACTION_SEND_STOP; drv_data->state = MV64XXX_I2C_STATE_IDLE; } else { - drv_data->action = MV64XXX_I2C_ACTION_SEND_RESTART; + drv_data->action = MV64XXX_I2C_ACTION_NEXT_MESSAGE; drv_data->state = MV64XXX_I2C_STATE_RESTART; } } else { @@ -429,7 +432,7 @@ mv64xxx_i2c_fsm(struct mv64xxx_i2c_data *drv_data, u32 status) drv_data->action = MV64XXX_I2C_ACTION_SEND_STOP; drv_data->state = MV64XXX_I2C_STATE_IDLE; } else { - drv_data->action = MV64XXX_I2C_ACTION_SEND_RESTART; + drv_data->action = MV64XXX_I2C_ACTION_NEXT_MESSAGE; drv_data->state = MV64XXX_I2C_STATE_RESTART; } } else { @@ -444,18 +447,38 @@ mv64xxx_i2c_fsm(struct mv64xxx_i2c_data *drv_data, u32 status) static void mv64xxx_i2c_send_start(struct mv64xxx_i2c_data *drv_data) { + u8 extra_bytes; + drv_data->msg = drv_data->msgs; drv_data->byte_posn = 0; drv_data->bytes_left = drv_data->msg->len; drv_data->aborting = 0; drv_data->rc = 0; + if (drv_data->msg->flags & I2C_M_NOSTART) + return; + + /* + * If this is a zero-length read, and the next message is a NOSTART + * write, the client driver is trying to insert extra bytes after the + * address but before the read proper. + */ + if ((drv_data->msg->len == 0) && (drv_data->msg->flags & I2C_M_RD) && + (drv_data->num_msgs > 1) && (drv_data->msgs[1].flags == I2C_M_NOSTART)) + extra_bytes = drv_data->msgs[1].len; + else + extra_bytes = 0; + + if (drv_data->reg_offsets.enh_feat != 0) + writel(extra_bytes, + drv_data->reg_base + drv_data->reg_offsets.enh_feat); + mv64xxx_i2c_prepare_for_io(drv_data, drv_data->msgs); writel(drv_data->cntl_bits | MV64XXX_I2C_REG_CONTROL_START, drv_data->reg_base + drv_data->reg_offsets.control); } -static void +static enum mv64xxx_i2c_event mv64xxx_i2c_do_action(struct mv64xxx_i2c_data *drv_data) { if (drv_data->action & MV64XXX_I2C_ACTION_RECEIVE) @@ -464,7 +487,7 @@ mv64xxx_i2c_do_action(struct mv64xxx_i2c_data *drv_data) drv_data->action &= ~MV64XXX_I2C_ACTION_RECEIVE; switch(drv_data->action) { - case MV64XXX_I2C_ACTION_SEND_RESTART: + case MV64XXX_I2C_ACTION_NEXT_MESSAGE: /* We should only get here if we have further messages */ BUG_ON(drv_data->num_msgs == 0); @@ -481,6 +504,10 @@ mv64xxx_i2c_do_action(struct mv64xxx_i2c_data *drv_data) * Thankfully, do not advertise support for that feature. */ drv_data->send_stop = drv_data->num_msgs == 1; + + if (drv_data->msg->flags & I2C_M_NOSTART) + return MV64XXX_I2C_EVENT_NOSTART; + break; case MV64XXX_I2C_ACTION_CONTINUE: @@ -525,6 +552,8 @@ mv64xxx_i2c_do_action(struct mv64xxx_i2c_data *drv_data) wake_up(&drv_data->waitq); break; } + + return MV64XXX_I2C_EVENT_INVALID; } static void @@ -595,6 +624,7 @@ static irqreturn_t mv64xxx_i2c_intr(int irq, void *dev_id) { struct mv64xxx_i2c_data *drv_data = dev_id; + enum mv64xxx_i2c_event event; u32 status; irqreturn_t rc = IRQ_NONE; @@ -617,8 +647,11 @@ mv64xxx_i2c_intr(int irq, void *dev_id) ndelay(100); status = readl(drv_data->reg_base + drv_data->reg_offsets.status); - mv64xxx_i2c_fsm(drv_data, status); - mv64xxx_i2c_do_action(drv_data); + event = mv64xxx_i2c_decode_status(drv_data, status); + do { + mv64xxx_i2c_fsm(drv_data, event, status); + event = mv64xxx_i2c_do_action(drv_data); + } while (event != MV64XXX_I2C_EVENT_INVALID); if (drv_data->irq_clear_inverted) writel(drv_data->cntl_bits | MV64XXX_I2C_REG_CONTROL_IFLG, @@ -830,7 +863,54 @@ mv64xxx_i2c_can_offload(struct mv64xxx_i2c_data *drv_data) static u32 mv64xxx_i2c_functionality(struct i2c_adapter *adap) { - return I2C_FUNC_I2C | I2C_FUNC_10BIT_ADDR | I2C_FUNC_SMBUS_EMUL; + struct mv64xxx_i2c_data *drv_data = i2c_get_adapdata(adap); + u32 func = I2C_FUNC_I2C | I2C_FUNC_10BIT_ADDR | I2C_FUNC_SMBUS_EMUL; + + if (drv_data->reg_offsets.enh_feat != 0) + func |= I2C_FUNC_NOSTART; + + return func; +} + +static bool +mv64xxx_i2c_check_msgs(struct i2c_msg msgs[], int num) +{ + int i; + bool is_write, prev_is_write; + + /* + * The I2C hardware is pretty strict about ensuring that the protocol + * is followed properly, and doesn't allow a lot of "creativity" how + * transfers are composed. This driver advertises I2C_FUNC_NOSTART, but + * can only support the two most common patterns for use of NOSTART: + * 1) Continuing a write message with further write bytes, as a sort of + * "gather" operation. + * 2) Inserting extra parameter bytes after the address of a read + * operation, using a zero-byte read, a small (<= 3 bytes) NOSTART + * write, and then a NOSTART read. + */ + + for (i = 0; i < num; i++) { + /* Check for case 1 */ + if (msgs[i].flags & I2C_M_NOSTART) { + if (i == 0) + return false; + is_write = !(msgs[i].flags & I2C_M_RD); + prev_is_write = !(msgs[i-1].flags & I2C_M_RD); + if (!is_write || !prev_is_write) + return false; + } + + /* Check for case 2 */ + if (i + 2 < num) { + if ((msgs[i].flags == I2C_M_RD) && (msgs[i].len == 0) && + (msgs[i+1].flags == I2C_M_NOSTART) && (msgs[i+1].len <= 3) && + (msgs[i+2].flags == (I2C_M_NOSTART|I2C_M_RD))) + i += 2; + } + } + + return true; } static int @@ -840,6 +920,9 @@ mv64xxx_i2c_xfer_core(struct i2c_adapter *adap, struct i2c_msg msgs[], int num) int rc, ret = num; u32 status; + if (!mv64xxx_i2c_check_msgs(msgs, num)) + return -ENOTSUPP; + rc = pm_runtime_resume_and_get(&adap->dev); if (rc) return rc; -- 2.43.2 diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c index 6a205cca603a..f09f23404784 100644 --- a/drivers/i2c/busses/i2c-mv64xxx.c +++ b/drivers/i2c/busses/i2c-mv64xxx.c @@ -100,6 +100,7 @@ enum mv64xxx_i2c_state { enum mv64xxx_i2c_event { MV64XXX_I2C_EVENT_INVALID, MV64XXX_I2C_EVENT_STARTED, + MV64XXX_I2C_EVENT_NOSTART, MV64XXX_I2C_EVENT_ADDR_ACK, MV64XXX_I2C_EVENT_ADDR_NO_ACK, MV64XXX_I2C_EVENT_WR_ACK, @@ -112,7 +113,7 @@ enum mv64xxx_i2c_event { enum mv64xxx_i2c_action { MV64XXX_I2C_ACTION_INVALID, MV64XXX_I2C_ACTION_CONTINUE, - MV64XXX_I2C_ACTION_SEND_RESTART, + MV64XXX_I2C_ACTION_NEXT_MESSAGE, MV64XXX_I2C_ACTION_SEND_ADDR_1, MV64XXX_I2C_ACTION_SEND_ADDR_2, MV64XXX_I2C_ACTION_SEND_DATA, @@ -129,6 +130,7 @@ struct mv64xxx_i2c_regs { u8 status; u8 clock; u8 soft_reset; + u8 enh_feat; }; struct mv64xxx_i2c_data { @@ -185,6 +187,7 @@ static struct mv64xxx_i2c_regs mv64xxx_i2c_regs_sun4i = { .status = 0x10, .clock = 0x14, .soft_reset = 0x18, + .enh_feat = 0x1c, }; static void @@ -306,9 +309,9 @@ mv64xxx_i2c_decode_status(struct mv64xxx_i2c_data *drv_data, u32 status) } static void -mv64xxx_i2c_fsm(struct mv64xxx_i2c_data *drv_data, u32 status) +mv64xxx_i2c_fsm(struct mv64xxx_i2c_data *drv_data, + enum mv64xxx_i2c_event event, u32 status) { - enum mv64xxx_i2c_event event; enum mv64xxx_i2c_state prev_state = drv_data->state;