From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.9]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 0997039AD2A for ; Wed, 24 Jun 2026 12:22:52 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=fail smtp.client-ip=198.175.65.9 ARC-Seal:i=2; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1782303776; cv=fail; b=UQ6SsbKt0SlF8rzu8wGrRnHzJtCsq4qWYn/7/WUdH90DzXxS4drOnb33MdX6gq/n/0ywqZRutVwJwipm8IP4uOk5QwB+OMMZZ7wxnpQIOEP37nGs7+dYy4NFSbcwZGPlwPTvtrQSseiZX/+DklluqjmLaldoFhzmPy/HkXDvjGs= ARC-Message-Signature:i=2; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1782303776; c=relaxed/simple; bh=+cPS7610MpV/DIguzS3aPM4WipN75Q4kRvVpZBzgO48=; h=Message-ID:Date:Subject:To:CC:References:From:In-Reply-To: Content-Type:MIME-Version; b=nyd6kHNkzRGo+p9LT+pd/A+h73mqbF1VFEkncMho4X1qqw0BRZqYrmmKWkrf3C+MziuqCm5le8J17NWd2sgg3rkOlfmlzoW13k9pQ89SAEDJLxt6JrE/shwlIYBqWq5TT645gloUSNsBuiiWl90gxC5BGfV9HI2pCBrM2FQhvu4= ARC-Authentication-Results:i=2; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=intel.com; spf=pass smtp.mailfrom=intel.com; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b=Q6/5YA+5; arc=fail smtp.client-ip=198.175.65.9 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=intel.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=intel.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b="Q6/5YA+5" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1782303773; x=1813839773; h=message-id:date:subject:to:cc:references:from: in-reply-to:content-transfer-encoding:mime-version; bh=+cPS7610MpV/DIguzS3aPM4WipN75Q4kRvVpZBzgO48=; b=Q6/5YA+51d+e3EuKODdxrsQWzbmdGtkzdyyzAoPWF8+eDOYuH3Q2PBGW ZF02bmPi5XmDMH/F/LgMBgiwolspmzxfEXSS3yVtH6Vji/6Z68Z8QHsTT UME8U78ogxhvqA5JLauyD4ydR2zUDnzd/wIXFf8Ly59lUK1JEuxv63NMz x7F+ZKumi/NpL+JeGsRQ+UC0CHqoCIlxYMIsQIGjPZN6dDX2YU/vACbhC Gwz4J0RgUaXgv/hBC78QBr+/Z1YqIVBiAlgaA+X3BITZejRvWO9417iov 59MOhB5YOCAF+p3aNp3ZGM3GYhPe4/JyJg+ipxd+6TpPAJyS5aX0gGCAc Q==; X-CSE-ConnectionGUID: gJvclO62RKyb16u8sH0qWw== X-CSE-MsgGUID: VForYjafS/uA3oSSL9n2Ng== X-IronPort-AV: E=McAfee;i="6800,10657,11826"; a="105855228" X-IronPort-AV: E=Sophos;i="6.24,222,1774335600"; d="scan'208";a="105855228" Received: from fmviesa008.fm.intel.com ([10.60.135.148]) by orvoesa101.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 24 Jun 2026 05:22:52 -0700 X-CSE-ConnectionGUID: DC6TmTO+TcaZ9sWSUsCl9g== X-CSE-MsgGUID: KKcnPf/4Rg6tKprDK9C/7g== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.24,222,1774335600"; d="scan'208";a="247464686" Received: from fmsmsx902.amr.corp.intel.com ([10.18.126.91]) by fmviesa008.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 24 Jun 2026 05:22:52 -0700 Received: from FMSMSX902.amr.corp.intel.com (10.18.126.91) by fmsmsx902.amr.corp.intel.com (10.18.126.91) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.2562.37; Wed, 24 Jun 2026 05:22:51 -0700 Received: from fmsedg901.ED.cps.intel.com (10.1.192.143) by FMSMSX902.amr.corp.intel.com (10.18.126.91) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.2562.37 via Frontend Transport; Wed, 24 Jun 2026 05:22:51 -0700 Received: from BYAPR05CU005.outbound.protection.outlook.com (52.101.85.30) by edgegateway.intel.com (192.55.55.81) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.2562.37; Wed, 24 Jun 2026 05:22:50 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=mTgWL1ArHKHVFYgVrVSVrDBmNGq4bFAeLjDpI2bpJzF+pHskaxsj77LoxIiE+Sj2m9tJ91o799q22S4L0tWqAdnlP7DQT9U8sD6P5oxsnOJGg02DD6E9EHSpS917/JFEf4ZU3Hc8W49y0bSQTFYc7Z8Pg7V9xWtsXmC4YmmRpXKAJW22jmntefG+gXlucLacde5Eq1B7EbptGoNbDdQOIWjx0SVhEZfZvvynLr8VBFA+/b733KVuA3N9biiHrxiHl9jov9aDi3fvD71ao6WsM6ROI53XSt6+3PkWgc/T+9z67wyoD1QevO62v4gRSDnoEdz+bVcGgrYKDHPtkogs1Q== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector10001; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=8ulgaGWfdDWJFeq4RB3uzNAvj7/BEtBZZ5D2D4PyDWo=; b=kez0Pf9CaWhfifi9CcggmpPiS3gb8tgAOx+Y+zeOKyOX73EV1U5Q3Bxv7nL/zw5g+xOixA6noKGo2ZMJCnrUULj19E5P3vNQnX1kyw9iNWkw8U5gWCJLK8Qqpe/syZKd2kZs7M1ZgyNKTI4hJQrDnQp6ioLVjKLaC9C3jN91oheoHYoq7TnNdM3+d4mlCOkCFqFG+ThQwHrFMPlCw2dYdSWpros5RRgxD6AwWFqgoG1oeBSN25GxV6XIMo8dJ+NVRXrHk75XvcTYwuRbCMGDko0VSPFl7Eleh8kUKHgNmw4UQh9mVpLtk2i3VbItoIgTggFCB4GyRpgc6MeixkrPkA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=intel.com; Received: from IA1PR11MB7198.namprd11.prod.outlook.com (2603:10b6:208:419::15) by DSWPR11MB9929.namprd11.prod.outlook.com (2603:10b6:8:39d::8) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.21.159.12; Wed, 24 Jun 2026 12:22:49 +0000 Received: from IA1PR11MB7198.namprd11.prod.outlook.com ([fe80::2c4e:e92a:4fa:a456]) by IA1PR11MB7198.namprd11.prod.outlook.com ([fe80::2c4e:e92a:4fa:a456%3]) with mapi id 15.21.0159.012; Wed, 24 Jun 2026 12:22:41 +0000 Message-ID: <382a7c2d-2e9e-4a83-94d1-b917921d91f1@intel.com> Date: Wed, 24 Jun 2026 15:22:36 +0300 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v3 3/3] i3c: master: Validate GET CCC payload length and retry M0/M2 once To: "NG, TZE YEE" CC: Alexandre Belloni , Frank Li , "NG, ADRIAN HO YIN" , "Felix Gu" , Wolfram Sang , Manikanta Guntupalli , Jorge Marques , Sakari Ailus , "linux-i3c@lists.infradead.org" , "linux-kernel@vger.kernel.org" References: <29a3da29-d67c-4596-9a50-e42355726368@intel.com> <4e172a71-1d3f-497d-a277-0ce8cffc474b@altera.com> <14571ae8-bd6f-42f3-8892-287aab82611b@intel.com> Content-Language: en-US From: Adrian Hunter Organization: Intel Finland Oy, Registered Address: c/o Alberga Business Park, 6 krs, Bertel Jungin Aukio 5, 02600 Espoo, Business Identity Code: 0357606 - 4, Domiciled in Helsinki In-Reply-To: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit X-ClientProxiedBy: DUZPR01CA0326.eurprd01.prod.exchangelabs.com (2603:10a6:10:4ba::25) To IA1PR11MB7198.namprd11.prod.outlook.com (2603:10b6:208:419::15) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: IA1PR11MB7198:EE_|DSWPR11MB9929:EE_ X-MS-Office365-Filtering-Correlation-Id: b299ad3a-9dd0-4669-6916-08ded1eb498d X-LD-Processed: 46c98d88-e344-4ed4-8496-4ed7712e255d,ExtAddr X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0;ARA:13230040|366016|23010399003|1800799024|376014|7416014|18002099003|22082099003|4143699003|11063799006|56012099006|3023799007; X-Microsoft-Antispam-Message-Info: cYiuSl2OgUz0RDk9+i4F4bRWmUuOtdR+7fdLHD6S5109vNtqJjJBjV6wbhYcqhqwgZmGDMfBwYfHQSite3M7RF3HYT8A3KYmvMhCitJKm+cxCbCrP4wpC64mXgBeZJEXMvnOzFedOcAznz6FuM+8VN6rSQyxK37+tVF4cl2ISUPmmuer1Pyr2HJcp3jZGT+GRRU1d3cnq/RoBdmcAz0bOUnzoOVDq6uALNDbgTEGK9BIHiajRyU3BQnAn+k2ELyZYh3FTLFcYXXfkiesUHpecW9uM5Y+3Hxd7axMswqsUu1JZmRp9BhS68qavXglYIUHW1p4Mo0aL8J0Hc5Kw0oZbCqyPZJEjw6G1lusQy6/+8PhsHNayRngAZ2rQX2FneebnBTyfz8kbjU09InF+uou7NSbDIl2OOIX926JHRK3B7Z8Gmq4qGaGZWgxyCkXOKdbFZ8jHk/SXlq7buHwQtLnxRNSc0IzCgEA4XPv+qM+odUU0SAWKPDXCwMpgFreB2M+WdQK14h5IqxdZmYxTzXjrUIs4Qgnlvl7NfUjHNxWvDphaozfYEO1nFIqRtnSXxzHeQjBKY4xAzFKiDCabU3lJDZna1TgPBT1ay6enQbGTehtqVnV/ogMAMJPRXuk29AmRc2AKSxfRIvEj7shnW2JSBGT/DWfOAYphIzf4Vy2xx8= X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:IA1PR11MB7198.namprd11.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230040)(366016)(23010399003)(1800799024)(376014)(7416014)(18002099003)(22082099003)(4143699003)(11063799006)(56012099006)(3023799007);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?SWQ3T0V3REEwcjdsWVU0Um9PMkcxZVVpR2pjT2YrQjB2Y2JGeEJJdHQwZEFX?= =?utf-8?B?WkhKRlhMcGJOUlMzcnVMVmpUYW9YVVF6OGxnY2tmb0JKeUJYYUUwNk9lSUEr?= =?utf-8?B?Njlqb05Pb2dhSDZ4SUNJZUFFSkhHTVVvT2xzSGJnd2xiSk9JM0tVMmlVUnhO?= =?utf-8?B?TUhyV3NEWmxSdG1ONjczU240d3d0S24yN05TVDVUUWZ6R05NajFhNS9SSExm?= =?utf-8?B?VU9pSGFCZWZRUnd1QkVva1JYRy94MVBYaXlJNDNPbWxWSUU2Y1RTeERYelc4?= =?utf-8?B?MklLaEJWVzMyc0d1QWZ2VCtpR0VRTURCUUNIRGJsWTZNMEF0OEkvT1RlcDdR?= =?utf-8?B?V1hLaWladk5SVTI2WUJZVWY0R1dicTVMWXZLMi9hc056b2FMTEdSd0JTU2JL?= =?utf-8?B?OGJRbkl0cnd0VkxvejRibzJ6cnVDaVJLdjFWbzFCczRWWnZDaEt5NGJOQmJs?= =?utf-8?B?Q2pucmxVSjhLVGE2UG9GdElmMDFQSzNHTk44NUtUSEp1OHBySmg3Ym9uNUpq?= =?utf-8?B?clloRGJMWXI5N0VkTURvdzRrYkdSd2dpclU4UmoyZTVLdER2SHp2WXZhS2sy?= =?utf-8?B?QWxHeERPbWJTd21YYkwxUms2RmRFMm5BZlc2clg1WE51ODc4MXZPcXd0cFh1?= =?utf-8?B?MURhb0dMNEMySGlwazRRbUdyY3BmM3kyZmZKQWtyQ2pUOEFXZUFEVVIrSmt6?= =?utf-8?B?SGcvbXVFai9qNlIrb3lVWkdrWXBSb2tHc2Yva2VodjZNUkhTOXEyK1hVdkQ4?= =?utf-8?B?YTNZTFRrbFlEUVM1LzVhWmJWcW9CUlFOdmlOTEh5dFZBaUhSVFBqUXJOUHhI?= =?utf-8?B?Q3VHWGYrOXUxQ1Vzb1FrZEk1dXl1N3lEOXprRTc4NXIzc2hndnpCcVN2aVNG?= =?utf-8?B?Q09XU0JlbEd3TjNaQ1VsWmU5WFpYV1N3Z1JielVLam9EUUZqRS9qWGs2djE2?= =?utf-8?B?SGdTd0JrSEo2dW95SWx0c2pPU3B3ckVPbnV5VmhrODlHM1cvRklFeHByS0N3?= =?utf-8?B?dFE1UHQ4bWVlbGg4MHEvbzRnY01mY3ZDTjlVTmEzaXgyVzA1L2t5VmtJR3Mr?= =?utf-8?B?Uy9VN3FSRzhEMnd3SDNsNFlIQW5YT2llUzZjZjZ4RmVIOTFkUUZKcWJUSWdH?= =?utf-8?B?QzVPazNRcmNucVovN1BTdjdIWnJjbjFPM2JoZU04ZysvSG85L1hrYTU1dGQv?= =?utf-8?B?SG02cThpY3M5YzJmc3N6OHBxaHh5MC9XK1hLN3BNOEdFVHVoNFdBakw2bmEv?= =?utf-8?B?ak1LejZYc0FYUDZ0dmxLTU0vc3JjTmQ4ZlZzTnVxVE13WmNsSmlwNUxlSXdN?= =?utf-8?B?VUUraXh4MkFrQWI4RGVseDhiTFFTQnJqeW9Yc0tSUmhDZkpQZktaVkxjL0lY?= =?utf-8?B?VHA2K0drSG1NMFNUbTJHUUdWdVN4WW5xUTJjUWs1NndSRzBGQ1lFR2psRkYy?= =?utf-8?B?d2RuYncxc04yYWJ3cmtxanptUGM2V1NJejhhQ2VXRVlqQXZEN1YvNHoxd2s1?= =?utf-8?B?UWt0MnRvUDZBc2V6bDJtMDF2ZDlabGF4SlZkRXNGNnNrNndpN3pDMDAyNkhH?= =?utf-8?B?WUdHbHdXU2tlZ3EvdURpblZYVkdSYmJlUmdzZHV5WTRtVWl4bG1acmUzYU93?= =?utf-8?B?eTN6TkZtaUhyblRBdlRxdVA0cVkrZnpUcDZxTURlWFgweDhDWjIyQnVVSHNs?= =?utf-8?B?NWZYbnRWM1VkMWtyQTc1YjdCMStUQUk1dk5mS2FUWGVicUFNTlFSNTU2dS8v?= =?utf-8?B?WkFEVHhBWi9zV2dIZDByYjY4bW9RS2Y0T0pqd1lrVlRoWk5WRGpDRXh2U2dU?= =?utf-8?B?SjNRQU9UU1dOUGpMK21sVCtCWjNsQ0hRRTVQaHhQTTJ6V0hPU2VaTGlGcTJ5?= =?utf-8?B?SFkxQjlJWkFuTzg0SkU2ZEFMYVhFNkd3ekVvSVU1bVZXS25qYWZXb2htUk92?= =?utf-8?B?bGYzbEVQbTdFdmFySjVGMVVLbEY1U01sUDZtdTFoK200VS9RSzExbEs2RnB1?= =?utf-8?B?aXpkWDBhRmNjQU1BYy9EZ3p4bFVDYU52aTNoMy9xTW92Rlk5aDVoUS9CMHpT?= =?utf-8?B?b2JvbWswUkw4bWlFb0RYejhUTmhtY2ZaZy9pNWxFZ1dxMXYweVBpMlhHcDZG?= =?utf-8?B?ampFbUZzTjVkK0VTRk0vY3pyNmE0dGQrUVBsR0NpVkV2WkFaVHlSd0Y0QVZk?= =?utf-8?B?anFxMkErR24zN1JTY2QvRy9UbEI3Y0RuK1d1YUFQeXJMbCtmS3p4T21ialJO?= =?utf-8?B?c2xMQjdGWVhZb1gyRVdWUXZrQjl3WWIvOTBPOGdaaGs0NXJGTDNTdFdsZHgv?= =?utf-8?B?MEJVR25VY0dGOVNJTVNOVGZMNmhMT2t5aUVIZUxTa3pEczJrKzgvTkpOOTdp?= =?utf-8?Q?aTXi8iLd4aHSHEhk=3D?= X-Exchange-RoutingPolicyChecked: T2gXOqYfQUu7KTK3fM6uo86todVvj3Ywn6z9CePvIn0bILKWagJRC1BdkntWuBZuj3ITfnXbTOgQzAQz5eVwrRRmb7uzGqlgaHbZnxmtxrGLoQLOhWNqdBSJ1ocSgJ6pTT8Grn4SddWgAQecjoxhFCpDwrujKXurEIVucdYAzWazInaT0R0yJPGvtMq9qd1l/h3w8JmCbo00SzfvpVzcIZVTMfusRV2viN7WtPHu/AjyNKs4BjCK/DLz4EXpzYkH8pUiTEj/gMxXEqqTs5BBvqvI2yFlEGdfs/aZlXSFSuTUuA165F3x5SdwSQPcg4WCYU6wenpv54JI+PedZ4ZxGw== X-MS-Exchange-CrossTenant-Network-Message-Id: b299ad3a-9dd0-4669-6916-08ded1eb498d X-MS-Exchange-CrossTenant-AuthSource: IA1PR11MB7198.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 24 Jun 2026 12:22:41.8165 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: N6+9QO3xRmUh1ps99OayasrrvCfqS7GuVt230ZVBnfzWkk5m/ooAyRNSs4vT08uwhVSiebZojqc2j4PnmeWiRQ== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DSWPR11MB9929 X-OriginatorOrg: intel.com On 24/06/2026 11:28, NG, TZE YEE wrote: > On 23/6/2026 8:29 pm, Adrian Hunter wrote: >> On 19/06/2026 11:09, NG, TZE YEE wrote: >>> On 16/6/2026 3:41 pm, Adrian Hunter wrote: >>>> [You don't often get email from adrian.hunter@intel.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ] >>>> >>>> On 11/06/2026 04:54, tze.yee.ng@altera.com wrote: >>>>> From: Adrian Ng Ho Yin >>>>> >>>>> Validate GET CCC payload length after a successful transfer. Treat a >>>>> short read as I3C_ERROR_M0 and return -EIO. GETMRL accepts exactly 2 or >>>>> 3 bytes per the I3C spec defined formats. GETMXDS may return 2 bytes >>>>> (format 1) or 5 bytes (format 2) per I3C spec. >>>>> >>>>> Retry GET CCCs once on retriable errors: I3C_ERROR_M0 (frame error) and >>>>> I3C_ERROR_M2 (address-header NACK, e.g. IBI or Controller Role Request >>>> Some controller drivers do not set I3C_ERROR_M2 correctly: >>>> svc-i3c-master seems to set I3C_ERROR_M2 on all errors >>>> mipi-i3c-hci sets it also on error status 0x5: NACK: Address was NACK’ed >>>> >>>> Others need to be checked >>>> >>> I checked the same paths: >>> - svc-i3c-master sets I3C_ERROR_M2 on any CCC failure. >>> - mipi-i3c-hci maps RESP_ERR_NACK (0x5) to I3C_ERROR_M2. >>> So a generic core retry on I3C_ERROR_M2 is only as good as each driver's >>> error reporting. Our DW series maps specific hardware status bits to >>> M0/M2, but we agree the wider driver behaviour should be reviewed before >>> relying on M2 retry across all masters. >>> >>> We can either narrow this series to DW-only behaviour for now, or follow >>> up with a separate audit/fix of M2 reporting in other drivers. Happy to >>> take your preference. >> >> I am wondering if we should ignore the nature of the error and just >> retry after any error. >> >> Also it seems like the number of retries should be passed by the caller, >> so that different commands could be treated differently. There is now >> i3c_master_i3c_dev_present() which does its own retries. >> > On retry: agreed — we will drop M0/M2-based retry and retry on any error > where a caller decides it is appropriate. > For retry count, we would like ask your preference between two approaches: > (i) Sysfs ccc_retry_count on the I3C master (similar to the existing > dev_nack_retry_count): default 1, user-tunable via sysfs. Callers that > need retries would read this from the master controller (or a shared > helper would). Let's not add sysfs knobs to start with. If a platform or device needs special treatment, it is better to configure that via device properties or DMI quirks or some such. > (ii) Caller-local constants (like your i3c_master_i3c_dev_present() > pattern : each helper that needs retries defines its own MAX_CNT / > backoff and loops around i3c_master_send_ccc_cmd_locked() — no new sysfs > knob. > Which do you prefer? Could define a default: #define I3C_CCC_RETRIES 1 Perhaps add i3c_ccc_cmd_init_retries() like i3c_ccc_cmd_init() but with a retries parameter, and then i3c_ccc_cmd_init() just calls i3c_ccc_cmd_init_retries(..., I3C_CCC_RETRIES) Call sites that need a different number of retries can call i3c_ccc_cmd_init_retries() instead of i3c_ccc_cmd_init() While we are only doing 1 retry anyway, maybe let's not bother with a backoff. >>>>> arbitration per I3C spec section 5.1.2.2.3). SET CCCs are not retried >>>> >>>> What has section 5.1.2.2.3 got to do with I3C_ERROR_M2? >>>> >>> Section 5.1.2.2.3 describes the recovery behaviour for a transient >>> address-header NACK (e.g. IBI/CRR arbitration), not the definition of >>> I3C_ERROR_M2 itself. >>> >>> M2 is the address-header NACK error code; §5.1.2.2.3 is one case where >>> the spec says software should re-issue the transfer. We will reword the >>> commit message to make that distinction clear and drop the implication >>> that M2 is defined in that section. >> >> The spec. has a different definition of M2: >> >> "If the Master does not receive an ACK of a transmitted Broadcast Address (7’h7E)," >> >> Whereas section 5.1.2.2.3 is about Frames starting with the target address. >> That could be covered by DEV_NACK_RETRY_CNT (which we should change to >> default to 1 not 0 by the way) >> > Agreed. M2 in spec is the broadcast-address (7'h7E) NACK case, not > address-header NACK error code. I will reword the commit message in v4. > > For the DEV_NACK_RETRY_CNT default value, we will update it in separate > patch.>>>> to avoid repeating side-effecting commands. Restore > dests[].payload.len >>>>> to the originally requested length before each attempt and again before >>>>> returning an error, so callers that adjust the length on failure (e.g. >>>>> i3c_master_getmxds_locked()) do not underflow a shortened value. >>>>> >>>>> Use a stack buffer for the common single-destination GET case and only >>>>> kmalloc when ndests > 1. >>>>> >>>>> Signed-off-by: Adrian Ng Ho Yin >>>>> Signed-off-by: Tze Yee Ng >>>>> --- >>>>> Changes in v3: >>>>> - Drop the change that moves RESPONSE_ERROR_ADDRESS_NACK to default case >>>>> in dw_i3c_master_end_xfer_locked(). Now dw_i3c_master_end_xfer_locked() >>>>> returns -EIO for RESPONSE_ERROR_ADDRESS_NACK. >>>>> --- >>>>> drivers/i3c/master.c | 111 ++++++++++++++++++++++++++++++++++++++++++- >>>>> 1 file changed, 110 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c >>>>> index 5cd4e5da2233..c94d37cd8b3f 100644 >>>>> --- a/drivers/i3c/master.c >>>>> +++ b/drivers/i3c/master.c >>>>> @@ -26,6 +26,12 @@ static DEFINE_MUTEX(i3c_core_lock); >>>>> static int __i3c_first_dynamic_bus_num; >>>>> static BLOCKING_NOTIFIER_HEAD(i3c_bus_notifier); >>>>> >>>>> +#define I3C_CCC_GETMRL_LEN_SHORT 2 >>>>> +#define I3C_CCC_GETMRL_LEN_FULL 3 >>>>> +#define I3C_CCC_GETMXDS_LEN_SHORT 2 >>>>> +#define I3C_CCC_GETMXDS_LEN_FULL 5 >>>>> +#define I3C_CCC_MAX_RETRIES 2 >>>>> + >>>>> /** >>>>> * i3c_bus_maintenance_lock - Lock the bus for a maintenance operation >>>>> * @bus: I3C bus to take the lock on >>>>> @@ -925,6 +931,61 @@ static void i3c_ccc_cmd_init(struct i3c_ccc_cmd *cmd, bool rnw, u8 id, >>>>> cmd->err = I3C_ERROR_UNKNOWN; >>>>> } >>>>> >>>>> +static bool i3c_ccc_get_payload_ok(u8 id, u16 req_len, u16 actual_len) >>>>> +{ >>>>> + if (actual_len > req_len) >>>>> + return false; >>>>> + >>>>> + if (!req_len) >>>>> + return actual_len == 0; >>>>> + >>>>> + if (id == I3C_CCC_GETMRL) >>>>> + return actual_len == I3C_CCC_GETMRL_LEN_SHORT || >>>>> + actual_len == I3C_CCC_GETMRL_LEN_FULL; >>>>> + >>>>> + if (id == I3C_CCC_GETMXDS) >>>>> + return actual_len == I3C_CCC_GETMXDS_LEN_SHORT || >>>>> + actual_len == I3C_CCC_GETMXDS_LEN_FULL; >>>> >>>> It would be better to contain individual CCC information in >>>> the caller of i3c_master_send_ccc_cmd_locked(). Perhaps >>>> add optional_bytes to struct i3c_ccc_cmd_payload: >>>> For I3C_CCC_GETMRL, optional_bytes = 1 >>>> For I3C_CCC_GETMXDS, optional_bytes = 3 >>>> >>> Agreed. The GETMRL/GETMXDS length rules are caller-specific and fit >>> better at the call site than as hardcoded CCC IDs in >>> i3c_master_send_ccc_cmd_locked(). >>> >>> Setting optional_bytes in the caller (e.g. getmrl_locked() → 1, >>> getmxds_locked() → 3). We can respin v4 with that approach.>> + >>>>> + return actual_len == req_len; >>>>> +} >>>>> + >>>>> +static int i3c_ccc_validate_payload_len(struct i3c_ccc_cmd *cmd, >>>>> + const u16 *req_lens) >>>>> +{ >>>>> + unsigned int i; >>>>> + >>>>> + if (!cmd->rnw) >>>>> + return 0; >>>>> + >>>>> + for (i = 0; i < cmd->ndests; i++) { >>>>> + u16 actual = cmd->dests[i].payload.len; >>>>> + u16 req = req_lens[i]; >>>>> + >>>>> + if (!i3c_ccc_get_payload_ok(cmd->id, req, actual)) { >>>>> + cmd->err = I3C_ERROR_M0; >>>>> + return -EIO; >>>>> + } >>>>> + } >>>>> + >>>>> + return 0; >>>>> +} >>>>> + >>>>> +/* >>>>> + * M0: transient frame errors. >>>>> + * M2: address-header NACK (I3C spec section 5.1.2.2.3), e.g. when a target >>>>> + * simultaneously asserts an IBI or Controller Role Request and neither >>>>> + * side ACKs. Software should re-issue the transfer; the controller wins >>>>> + * arbitration after Repeated START. >>>>> + * >>>>> + * Retries apply to GET CCCs only; SET CCCs are not retried to avoid >>>>> + * repeating side-effecting commands. >>>>> + */ >>>>> +static bool i3c_ccc_err_retriable(enum i3c_error_code err) >>>>> +{ >>>>> + return err == I3C_ERROR_M0 || err == I3C_ERROR_M2; >>>>> +} >>>>> + >>>>> /** >>>>> * i3c_master_send_ccc_cmd_locked() - send a CCC (Common Command Codes) >>>>> * @master: master used to send frames on the bus >>>>> @@ -936,9 +997,17 @@ static void i3c_ccc_cmd_init(struct i3c_ccc_cmd *cmd, bool rnw, u8 id, >>>>> static int i3c_master_send_ccc_cmd_locked(struct i3c_master_controller *master, >>>>> struct i3c_ccc_cmd *cmd) >>>>> { >>>>> + u16 req_len; >>>>> + u16 *req_lens = NULL; >>>>> + u16 *req_lens_alloc = NULL; >>>>> + unsigned int i; >>>>> + int ret, retries; >>>>> + >>>>> if (!cmd || !master) >>>>> return -EINVAL; >>>>> >>>>> + retries = cmd->rnw ? I3C_CCC_MAX_RETRIES : 1; >>>>> + >>>>> if (WARN_ON(master->init_done && >>>>> !rwsem_is_locked(&master->bus.lock))) >>>>> return -EINVAL; >>>>> @@ -953,7 +1022,47 @@ static int i3c_master_send_ccc_cmd_locked(struct i3c_master_controller *master, >>>>> !master->ops->supports_ccc_cmd(master, cmd)) >>>>> return -EOPNOTSUPP; >>>>> >>>>> - return master->ops->send_ccc_cmd(master, cmd); >>>>> + if (cmd->rnw && cmd->dests && cmd->ndests) { >>>>> + if (cmd->ndests == 1) { >>>>> + req_len = cmd->dests[0].payload.len; >>>>> + req_lens = &req_len; >>>>> + } else { >>>>> + req_lens_alloc = kmalloc_array(cmd->ndests, >>>>> + sizeof(*req_lens_alloc), >>>>> + GFP_KERNEL); >>>> >>>> Simpler to add actual_len to struct i3c_ccc_cmd_payload and >>>> amend controller drivers to use that. >>>> >>> Agreed. We will add actual_len to struct i3c_ccc_cmd_payload in v4.>> + >>> if (!req_lens_alloc) >>>>> + return -ENOMEM; >>>>> + >>>>> + req_lens = req_lens_alloc; >>>>> + for (i = 0; i < cmd->ndests; i++) >>>>> + req_lens[i] = cmd->dests[i].payload.len; >>>>> + } >>>>> + } >>>>> + >>>>> + do { >>>>> + cmd->err = I3C_ERROR_UNKNOWN; >>>>> + if (req_lens) { >>>>> + for (i = 0; i < cmd->ndests; i++) >>>>> + cmd->dests[i].payload.len = req_lens[i]; >>>>> + } >>>>> + ret = master->ops->send_ccc_cmd(master, cmd); >>>>> + if (!ret && req_lens) >>>>> + ret = i3c_ccc_validate_payload_len(cmd, req_lens); >>>>> + } while (--retries && ret && i3c_ccc_err_retriable(cmd->err)); >>>>> + >>>>> + if (ret && req_lens) { >>>>> + /* >>>>> + * Drivers may update payload.len to the actual RX count; >>>>> + * restore the requested length so callers can safely adjust >>>>> + * it on error (e.g. i3c_master_getmxds_locked()). >>>>> + */ >>>>> + for (i = 0; i < cmd->ndests; i++) >>>>> + cmd->dests[i].payload.len = req_lens[i]; >>>>> + } >>>>> + >>>>> + kfree(req_lens_alloc); >>>>> + >>>>> + return ret; >>>>> } >>>>> >>>>> static struct i2c_dev_desc * >>>> >>> >> >