Message ID | 20240126-spi-omap2-mcspi-multi-mode-v1-2-d143d33f0fe0@bootlin.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel+bounces-54636-ouuuleilei=gmail.com@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:7301:168b:b0:106:860b:bbdd with SMTP id ma11csp1432991dyb; Tue, 6 Feb 2024 02:01:53 -0800 (PST) X-Google-Smtp-Source: AGHT+IGj7MhhAPhL1vYhiSziN4Lp4m1j5jBhtPVI4BScxN1ra/CN1R7b7d9c8TAvEfA7l/rFdUVy X-Received: by 2002:a92:b750:0:b0:363:b6c3:b484 with SMTP id c16-20020a92b750000000b00363b6c3b484mr2726620ilm.28.1707213713321; Tue, 06 Feb 2024 02:01:53 -0800 (PST) ARC-Seal: i=2; a=rsa-sha256; t=1707213713; cv=pass; d=google.com; s=arc-20160816; b=CzNRndxvM4CY8REpxtiww44OIUnxnocjLZY4hQ597s0+Ab2ibQ9ZTRVNz1VAnfxj0O 5ZzFMLipen22cOlyUVDkHUFJgL4Xfl/5EvbZKFSQFYvx8BglSmQC/jzfKMMjho1gWE1x np4j2k1YY4OCUnI9V6J3Oh4xoNzmyRpXjcDT4ykHbi1WXxFoQOKmpN2sgitwGA8c9I1V h37SRFq3a8s3O8HdJS0qvQ/K9TDfSYKPuh5uy6sx6YCsbTjH44ImwPC0WWJ08QpDCkXO P5c8BJ7gEikcW6O1lu5RHUa/qkYaXEqd3TJL/aoGYOuvvSlAW7M7YQcvSLpKymKXRbFW jT+w== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=cc:to:in-reply-to:references:message-id:content-transfer-encoding :mime-version:list-unsubscribe:list-subscribe:list-id:precedence :subject:date:from:dkim-signature; bh=7ZyyNB/CZxx9/DUWNjxPsDT3HiAzuHwEEv/w7sNe/Cg=; fh=UN2bpMdhOePpGnzEa9aaJJGeCWgKsD76dEMoBjU/rPU=; b=G9VowNQbqYshnB5SOD7l999mA24mZEI8LBpY8symQT4MWLqKLAG12z0lvVTOF2iXcP FRCxRpJWHYLtzOrKv7cVXN/9CMu40ifxo8evewE+Y9ppp1aPQIJwqtdED5UxNFaBMTcY jI0uUkod7JStuuVNFjgSHD0dwahGlaGsTkEZxvAAXD4Foz0ka6Vhm8sf1Tbs3q1vC6YM xZuM7SoNmk185FRf/nzj8E14yY/czXxiv8LjF6Cn7hDgEY26hmQpoibMth8XW0sQn9tL EWpcmSUckNpZGG9t/cMfRZhG8/zZealU79/1/rku5s+knhT4blR5AX5xnm4N8x8YO0Sw yNLg==; dara=google.com ARC-Authentication-Results: i=2; mx.google.com; dkim=pass header.i=@bootlin.com header.s=gm1 header.b=J+oeDK55; arc=pass (i=1 spf=pass spfdomain=bootlin.com dkim=pass dkdomain=bootlin.com dmarc=pass fromdomain=bootlin.com); spf=pass (google.com: domain of linux-kernel+bounces-54636-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) smtp.mailfrom="linux-kernel+bounces-54636-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=bootlin.com X-Forwarded-Encrypted: i=1; AJvYcCWZXxWeDc1EcSYoiQX8qwCxV1ggxJ0lVs4OGMR28abk+JrhKrRZ8RHWGaptVKBZ86S5tbi1+UT6NDQDAlEi5f1Kc2IsJg== Received: from sv.mirrors.kernel.org (sv.mirrors.kernel.org. [2604:1380:45e3:2400::1]) by mx.google.com with ESMTPS id k1-20020a636f01000000b005dbf27229edsi1362476pgc.290.2024.02.06.02.01.53 for <ouuuleilei@gmail.com> (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 06 Feb 2024 02:01:53 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel+bounces-54636-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1; Authentication-Results: mx.google.com; dkim=pass header.i=@bootlin.com header.s=gm1 header.b=J+oeDK55; arc=pass (i=1 spf=pass spfdomain=bootlin.com dkim=pass dkdomain=bootlin.com dmarc=pass fromdomain=bootlin.com); spf=pass (google.com: domain of linux-kernel+bounces-54636-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) smtp.mailfrom="linux-kernel+bounces-54636-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=bootlin.com Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by sv.mirrors.kernel.org (Postfix) with ESMTPS id 0DEEB280E2B for <ouuuleilei@gmail.com>; Tue, 6 Feb 2024 10:01:53 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 2C05612E1F8; Tue, 6 Feb 2024 10:01:14 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=bootlin.com header.i=@bootlin.com header.b="J+oeDK55" Received: from relay1-d.mail.gandi.net (relay1-d.mail.gandi.net [217.70.183.193]) (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 7297F12D76D; Tue, 6 Feb 2024 10:01:08 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.70.183.193 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1707213671; cv=none; b=sxQgZ/anhhZ8fIQ2jHWN1uubm51RhaRUsqQQdF+5CQmsab5vvt53AKh3XTwGc4HviE2Ctg7/JhrCfDfkGdYQAR30SeBrF71CWZVoylpU/y6WmKPoB1Z7wQ3fw4xCjobWay+uVsrtGQhgaZrQ+pr2cwGJnA3OV0f83zin5RBPlTc= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1707213671; c=relaxed/simple; bh=dLeoHdat5+2yokZ57ZMb1U/V1RdRR+uIGAJcxMGtu2w=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=tXMfIgqAv9PCADsR/RFCbEOQZ7U6oya9xcPZ3/cxWRtChTB707i05sqzQe1I1dU/Vx1xniMRhF8fZTpvK/m5vNTw1YjKTjTswx6o3o0NxfPAREfFmChIlzC+r5JbZM8/EukekvnaJtJbwEtOKrvuUezj/cY69xsw8kbBrFBtcBU= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=bootlin.com; spf=pass smtp.mailfrom=bootlin.com; dkim=pass (2048-bit key) header.d=bootlin.com header.i=@bootlin.com header.b=J+oeDK55; arc=none smtp.client-ip=217.70.183.193 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=bootlin.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=bootlin.com Received: by mail.gandi.net (Postfix) with ESMTPSA id 3893B240012; Tue, 6 Feb 2024 10:01:01 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bootlin.com; s=gm1; t=1707213661; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=7ZyyNB/CZxx9/DUWNjxPsDT3HiAzuHwEEv/w7sNe/Cg=; b=J+oeDK55buHO+8CcYynTWmMMg+qfgizl8HUCfvSWyp4iSLtf04Hrv1xBFwTLYw7usBX6hW bxBKB0AAWZVRQJl0OFoTuTdayGkVo8gonZRhgf6MVdEBDnuAWxJAQv3cF6I7ORwJVnpCga GVbs1PyXfU/2cFsGuhwy4+f0mbQIXImEwfyEeI2Ng7dfIxDJpC8nWXcfyrRyv34ThBp3Yc 85Dv1RIAnDczpOLW7kaNqDFy+eYAQuT0ZRkwZuDln+TME8QSuQ05mYXu9aRfVSljWv9jWB WNaxMsrFcVh7GkKRze01RKD61HwtXvIG0ryznNkL5KfgCA4umFyR9BNR0eJtiA== From: Louis Chauvet <louis.chauvet@bootlin.com> Date: Tue, 06 Feb 2024 11:00:50 +0100 Subject: [PATCH 2/2] spi: omap2-mcspi: Add support for MULTI-mode Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: <linux-kernel.vger.kernel.org> List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org> List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Message-Id: <20240126-spi-omap2-mcspi-multi-mode-v1-2-d143d33f0fe0@bootlin.com> References: <20240126-spi-omap2-mcspi-multi-mode-v1-0-d143d33f0fe0@bootlin.com> In-Reply-To: <20240126-spi-omap2-mcspi-multi-mode-v1-0-d143d33f0fe0@bootlin.com> To: Mark Brown <broonie@kernel.org> Cc: linux-spi@vger.kernel.org, linux-kernel@vger.kernel.org, thomas.petazzoni@bootlin.com, Miquel Raynal <miquel.raynal@bootlin.com>, yen-mei.goh@keysight.com, koon-kee.lie@keysight.com, Louis Chauvet <louis.chauvet@bootlin.com> X-Mailer: b4 0.12.1 X-Developer-Signature: v=1; a=openpgp-sha256; l=4015; i=louis.chauvet@bootlin.com; h=from:subject:message-id; bh=dLeoHdat5+2yokZ57ZMb1U/V1RdRR+uIGAJcxMGtu2w=; b=owEBbQKS/ZANAwAIASCtLsZbECziAcsmYgBlwgNc490SgPJdYZBCh8N73jiZS1KgRhcdKvOucQHs mmRLqHWJAjMEAAEIAB0WIQRPj7g/vng8MQxQWQQgrS7GWxAs4gUCZcIDXAAKCRAgrS7GWxAs4rhwD/ 9jG8LYFgLIO0pYCUnLTGlca5mn6PEjqk+gZKi2JAcQ5/p1YlnV6w3ZSuFqOY+FgNXAPc0voIN/P9T2 yJztrL12vfJf90d5D4SptCK0Ncmv+bJguUiY9ELbwqa6y67EoecbKnb3kIbGJ1foqbJaq1Qw3gZGtY WMF7Mkz2fngaQeAOctrnBT8Fs1y4am7TZ+bhD5ffr/b2v6Fi9vSPPXo6/nVk79OXkt1XYcktDlUvF4 3wHCDyeGfjw4E2sdmF3ZLXhC+SsEFbhqJZZ5m/gc3hItN9+pQ08jiYFX1QUjfx1SICnWL1j1Am5Moq qm82fjr/iekD3yBIYFYAQUojmR2PmGEXN2ieuwBMcRw1WXPX9bfaWj0wSzV70LNoNd4hmmLxu5C9MF KS8ydETXSG4HxoAtbLnLDc0+V+1YQUrIN0W63yX5fg6p1ZJ0NEGJfa8sRloViKKXBoqMqM/rYmVRnI i5St0q9kznSlgnBDDV8CR6cyu3EaDTbugGZz/yzoFQGyCALN+RaX7M1ELJMUl/9FiLF7Oo0rj5yJxr WA/ZCmfEs8jVNQhWeZwYPMHAgYRJ+762tML1CQBlFqbw3bdFbpAR1xziT3F1YsQL59zMKbPoNP+p60 0ZEr008aPriW0QFXDtNRTkFnap9K2598aoRc14UNYKJm4xkIavAj5M7JkNqQ== X-Developer-Key: i=louis.chauvet@bootlin.com; a=openpgp; fpr=8B7104AE9A272D6693F527F2EC1883F55E0B40A5 X-GND-Sasl: louis.chauvet@bootlin.com X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1790143326385555785 X-GMAIL-MSGID: 1790143326385555785 |
Series |
Add multi mode support for omap-mcspi
|
|
Commit Message
Louis Chauvet
Feb. 6, 2024, 10 a.m. UTC
Introduce support for MULTI-mode in the OMAP2 MCSPI driver. Currently, the
driver always uses SINGLE mode to handle the chip select (CS). With this
enhancement, MULTI-mode is enabled for specific messages, allowing for a
shorter delay between CS enable and the message (some FPGA devices are
sensitive to this delay).
The drawback of multi-mode is that it's not possible to keep the CS
enabled between each words. Therefore, this patch enables multi-mode only
for specific messages: the spi_message must contain only spi_transfer of 1
word (of any size) with cs_change enabled.
A new member is introduced in the omap2_mcspi structure to keep track of
the current used mode.
Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
---
drivers/spi/spi-omap2-mcspi.c | 57 ++++++++++++++++++++++++++++++++++++++-----
1 file changed, 51 insertions(+), 6 deletions(-)
Comments
On Tue, Feb 06, 2024 at 11:00:50AM +0100, Louis Chauvet wrote: > Introduce support for MULTI-mode in the OMAP2 MCSPI driver. Currently, the > driver always uses SINGLE mode to handle the chip select (CS). With this > enhancement, MULTI-mode is enabled for specific messages, allowing for a > shorter delay between CS enable and the message (some FPGA devices are > sensitive to this delay). I have no idea based on this commit message what either of these modes is or how this shorter delay would be achieved, these terms are specific to the OMAP hardware AFAICT. Please clarify this, it's hard to follow what the change does. It looks like this is just a CS per word thing? Note that you may not have to tell the hardware the same word length as the transfer specifies, so long as the wire result is the same it doesn't matter.
Hi Louis, kernel test robot noticed the following build errors: [auto build test ERROR on 41bccc98fb7931d63d03f326a746ac4d429c1dd3] url: https://github.com/intel-lab-lkp/linux/commits/Louis-Chauvet/Revert-spi-spi-omap2-mcspi-c-Toggle-CS-after-each-word/20240206-180243 base: 41bccc98fb7931d63d03f326a746ac4d429c1dd3 patch link: https://lore.kernel.org/r/20240126-spi-omap2-mcspi-multi-mode-v1-2-d143d33f0fe0%40bootlin.com patch subject: [PATCH 2/2] spi: omap2-mcspi: Add support for MULTI-mode config: i386-buildonly-randconfig-003-20240207 (https://download.01.org/0day-ci/archive/20240207/202402070919.C08LpTkR-lkp@intel.com/config) compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240207/202402070919.C08LpTkR-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202402070919.C08LpTkR-lkp@intel.com/ All errors (new ones prefixed by >>): >> drivers/spi/spi-omap2-mcspi.c:1415:23: error: use of undeclared identifier 'master' 1415 | omap2_mcspi_set_mode(master); | ^ 1 error generated. vim +/master +1415 drivers/spi/spi-omap2-mcspi.c 1379 1380 static int omap2_mcspi_prepare_message(struct spi_controller *ctlr, 1381 struct spi_message *msg) 1382 { 1383 struct omap2_mcspi *mcspi = spi_controller_get_devdata(ctlr); 1384 struct omap2_mcspi_regs *ctx = &mcspi->ctx; 1385 struct omap2_mcspi_cs *cs; 1386 struct spi_transfer *tr; 1387 u8 bits_per_word; 1388 u32 speed_hz; 1389 1390 /* 1391 * The conditions are strict, it is mandatory to check each transfer of the list to see if 1392 * multi-mode is applicable. 1393 */ 1394 mcspi->use_multi_mode = true; 1395 list_for_each_entry(tr, &msg->transfers, transfer_list) { 1396 if (!tr->bits_per_word) 1397 bits_per_word = msg->spi->bits_per_word; 1398 else 1399 bits_per_word = tr->bits_per_word; 1400 1401 /* Check if the transfer content is only one word */ 1402 if ((bits_per_word < 8 && tr->len > 1) || 1403 (bits_per_word >= 8 && tr->len > bits_per_word / 8)) 1404 mcspi->use_multi_mode = false; 1405 1406 /* Check if transfer asks to change the CS status after the transfer */ 1407 if (!tr->cs_change) 1408 mcspi->use_multi_mode = false; 1409 1410 /* If at least one message is not compatible, switch back to single mode */ 1411 if (!mcspi->use_multi_mode) 1412 break; 1413 } 1414 > 1415 omap2_mcspi_set_mode(master); 1416 1417 /* In single mode only a single channel can have the FORCE bit enabled 1418 * in its chconf0 register. 1419 * Scan all channels and disable them except the current one. 1420 * A FORCE can remain from a last transfer having cs_change enabled 1421 * 1422 * In multi mode all FORCE bits must be disabled. 1423 */ 1424 list_for_each_entry(cs, &ctx->cs, node) { 1425 if (msg->spi->controller_state == cs && !mcspi->use_multi_mode) { 1426 continue; 1427 } 1428 1429 if ((cs->chconf0 & OMAP2_MCSPI_CHCONF_FORCE)) { 1430 cs->chconf0 &= ~OMAP2_MCSPI_CHCONF_FORCE; 1431 writel_relaxed(cs->chconf0, 1432 cs->base + OMAP2_MCSPI_CHCONF0); 1433 readl_relaxed(cs->base + OMAP2_MCSPI_CHCONF0); 1434 } 1435 } 1436 1437 return 0; 1438 } 1439
Hi Louis, kernel test robot noticed the following build errors: [auto build test ERROR on 41bccc98fb7931d63d03f326a746ac4d429c1dd3] url: https://github.com/intel-lab-lkp/linux/commits/Louis-Chauvet/Revert-spi-spi-omap2-mcspi-c-Toggle-CS-after-each-word/20240206-180243 base: 41bccc98fb7931d63d03f326a746ac4d429c1dd3 patch link: https://lore.kernel.org/r/20240126-spi-omap2-mcspi-multi-mode-v1-2-d143d33f0fe0%40bootlin.com patch subject: [PATCH 2/2] spi: omap2-mcspi: Add support for MULTI-mode config: arm64-defconfig (https://download.01.org/0day-ci/archive/20240207/202402071719.e1p4tUge-lkp@intel.com/config) compiler: aarch64-linux-gcc (GCC) 13.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240207/202402071719.e1p4tUge-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202402071719.e1p4tUge-lkp@intel.com/ All error/warnings (new ones prefixed by >>): drivers/spi/spi-omap2-mcspi.c: In function 'omap2_mcspi_prepare_message': >> drivers/spi/spi-omap2-mcspi.c:1415:30: error: 'master' undeclared (first use in this function) 1415 | omap2_mcspi_set_mode(master); | ^~~~~~ drivers/spi/spi-omap2-mcspi.c:1415:30: note: each undeclared identifier is reported only once for each function it appears in >> drivers/spi/spi-omap2-mcspi.c:1388:13: warning: unused variable 'speed_hz' [-Wunused-variable] 1388 | u32 speed_hz; | ^~~~~~~~ vim +/master +1415 drivers/spi/spi-omap2-mcspi.c 1379 1380 static int omap2_mcspi_prepare_message(struct spi_controller *ctlr, 1381 struct spi_message *msg) 1382 { 1383 struct omap2_mcspi *mcspi = spi_controller_get_devdata(ctlr); 1384 struct omap2_mcspi_regs *ctx = &mcspi->ctx; 1385 struct omap2_mcspi_cs *cs; 1386 struct spi_transfer *tr; 1387 u8 bits_per_word; > 1388 u32 speed_hz; 1389 1390 /* 1391 * The conditions are strict, it is mandatory to check each transfer of the list to see if 1392 * multi-mode is applicable. 1393 */ 1394 mcspi->use_multi_mode = true; 1395 list_for_each_entry(tr, &msg->transfers, transfer_list) { 1396 if (!tr->bits_per_word) 1397 bits_per_word = msg->spi->bits_per_word; 1398 else 1399 bits_per_word = tr->bits_per_word; 1400 1401 /* Check if the transfer content is only one word */ 1402 if ((bits_per_word < 8 && tr->len > 1) || 1403 (bits_per_word >= 8 && tr->len > bits_per_word / 8)) 1404 mcspi->use_multi_mode = false; 1405 1406 /* Check if transfer asks to change the CS status after the transfer */ 1407 if (!tr->cs_change) 1408 mcspi->use_multi_mode = false; 1409 1410 /* If at least one message is not compatible, switch back to single mode */ 1411 if (!mcspi->use_multi_mode) 1412 break; 1413 } 1414 > 1415 omap2_mcspi_set_mode(master); 1416 1417 /* In single mode only a single channel can have the FORCE bit enabled 1418 * in its chconf0 register. 1419 * Scan all channels and disable them except the current one. 1420 * A FORCE can remain from a last transfer having cs_change enabled 1421 * 1422 * In multi mode all FORCE bits must be disabled. 1423 */ 1424 list_for_each_entry(cs, &ctx->cs, node) { 1425 if (msg->spi->controller_state == cs && !mcspi->use_multi_mode) { 1426 continue; 1427 } 1428 1429 if ((cs->chconf0 & OMAP2_MCSPI_CHCONF_FORCE)) { 1430 cs->chconf0 &= ~OMAP2_MCSPI_CHCONF_FORCE; 1431 writel_relaxed(cs->chconf0, 1432 cs->base + OMAP2_MCSPI_CHCONF0); 1433 readl_relaxed(cs->base + OMAP2_MCSPI_CHCONF0); 1434 } 1435 } 1436 1437 return 0; 1438 } 1439
Le 06/02/24 - 10:56, Mark Brown a écrit : > On Tue, Feb 06, 2024 at 11:00:50AM +0100, Louis Chauvet wrote: > > > Introduce support for MULTI-mode in the OMAP2 MCSPI driver. Currently, the > > driver always uses SINGLE mode to handle the chip select (CS). With this > > enhancement, MULTI-mode is enabled for specific messages, allowing for a > > shorter delay between CS enable and the message (some FPGA devices are > > sensitive to this delay). > > I have no idea based on this commit message what either of these modes > is or how this shorter delay would be achieved, these terms are specific > to the OMAP hardware AFAICT. Please clarify this, it's hard to follow > what the change does. It looks like this is just a CS per word thing? Indeed, you're right, the wording is probably very OMAP specific, I didn't realize that earlier. I'll try to explain better. What about this addition following the above paragraph, would it be clearer? [...] this delay). The OMAP2 MCSPI device can use two different mode to send messages: SINGLE and MULTI: In SINGLE mode, the controller only leverages one single FIFO, and the host system has to manually select the CS it wants to enable. In MULTI mode, each CS is bound to a FIFO, the host system then writes the data to the relevant FIFO, as the hardware will take care of the CS The drawback [...] > Note that you may not have to tell the hardware the same word length as > the transfer specifies, so long as the wire result is the same it > doesn't matter. If I understand correclty what you want is: given a message, containing 2 transfers of 4 bits, with cs_change disabled, use the multi mode and send only one 8 bits transfer instead of two 4 bits transfer? This seems very complex to implement, and will only benefit in very niche cases. If I have to add this, I have to: - detect the very particular pattern "message of multiple transfer and those transfer can be packed in bigger transfer" - reimplement the transfer_one_message method to merge multiple transfer into one; - manage the rx buffer to "unmerge" the answer; - take care of timings if requested; - probably other issues I don't see I agree this kind of optimisation can be nice, and may benefit for this spi controller, but I don't have the time to work on it. If someone want to do it, it could be a nice improvement. I just see that I miss my rebase, I will push a v2. This v2 will also change the commit name for patch 1/2. Have a nice day, Louis Chauvet
On Wed, Feb 07, 2024 at 04:25:16PM +0100, Louis Chauvet wrote: > Le 06/02/24 - 10:56, Mark Brown a écrit : > this addition following the above paragraph, would it be clearer? > > [...] this delay). > > The OMAP2 MCSPI device can use two different mode to send messages: > SINGLE and MULTI: > In SINGLE mode, the controller only leverages one single FIFO, and the > host system has to manually select the CS it wants to enable. > In MULTI mode, each CS is bound to a FIFO, the host system then writes > the data to the relevant FIFO, as the hardware will take care of the CS > > The drawback [...] Yes. > > Note that you may not have to tell the hardware the same word length as > > the transfer specifies, so long as the wire result is the same it > > doesn't matter. > If I understand correclty what you want is: given a message, containing 2 > transfers of 4 bits, with cs_change disabled, use the multi mode and send > only one 8 bits transfer instead of two 4 bits transfer? > This seems very complex to implement, and will only benefit in very > niche cases. I was hoping that the hardware supports more than 8 bit words, in that case then it gets useful for common operations like 8 bit register 8 bit data register writes (and more for larger word sizes) which are relatively simple. If it's just 8 bit words then yes, totally not worth the effort. > If I have to add this, I have to: > - detect the very particular pattern "message of multiple transfer and > those transfer can be packed in bigger transfer" Or just a single transfer with two words, it's trivial cases that don't involve rewriting anything beyond lying about the word lengths that I was thinking of. Anything more involved should go in the core.
diff --git a/drivers/spi/spi-omap2-mcspi.c b/drivers/spi/spi-omap2-mcspi.c index fc7f69973334..ab22b1b062f3 100644 --- a/drivers/spi/spi-omap2-mcspi.c +++ b/drivers/spi/spi-omap2-mcspi.c @@ -133,6 +133,8 @@ struct omap2_mcspi { unsigned int pin_dir:1; size_t max_xfer_len; u32 ref_clk_hz; + + bool use_multi_mode; }; struct omap2_mcspi_cs { @@ -258,10 +260,15 @@ static void omap2_mcspi_set_cs(struct spi_device *spi, bool enable) l = mcspi_cached_chconf0(spi); - if (enable) + /* Only enable chip select manually if single mode is used */ + if (mcspi->use_multi_mode) { l &= ~OMAP2_MCSPI_CHCONF_FORCE; - else - l |= OMAP2_MCSPI_CHCONF_FORCE; + } else { + if (enable) + l &= ~OMAP2_MCSPI_CHCONF_FORCE; + else + l |= OMAP2_MCSPI_CHCONF_FORCE; + } mcspi_write_chconf0(spi, l); @@ -285,7 +292,12 @@ static void omap2_mcspi_set_mode(struct spi_controller *ctlr) l |= (OMAP2_MCSPI_MODULCTRL_MS); } else { l &= ~(OMAP2_MCSPI_MODULCTRL_MS); - l |= OMAP2_MCSPI_MODULCTRL_SINGLE; + + /* Enable single mode if needed */ + if (mcspi->use_multi_mode) + l &= ~OMAP2_MCSPI_MODULCTRL_SINGLE; + else + l |= OMAP2_MCSPI_MODULCTRL_SINGLE; } mcspi_write_reg(ctlr, OMAP2_MCSPI_MODULCTRL, l); @@ -1371,15 +1383,48 @@ static int omap2_mcspi_prepare_message(struct spi_controller *ctlr, struct omap2_mcspi *mcspi = spi_controller_get_devdata(ctlr); struct omap2_mcspi_regs *ctx = &mcspi->ctx; struct omap2_mcspi_cs *cs; + struct spi_transfer *tr; + u8 bits_per_word; + u32 speed_hz; - /* Only a single channel can have the FORCE bit enabled + /* + * The conditions are strict, it is mandatory to check each transfer of the list to see if + * multi-mode is applicable. + */ + mcspi->use_multi_mode = true; + list_for_each_entry(tr, &msg->transfers, transfer_list) { + if (!tr->bits_per_word) + bits_per_word = msg->spi->bits_per_word; + else + bits_per_word = tr->bits_per_word; + + /* Check if the transfer content is only one word */ + if ((bits_per_word < 8 && tr->len > 1) || + (bits_per_word >= 8 && tr->len > bits_per_word / 8)) + mcspi->use_multi_mode = false; + + /* Check if transfer asks to change the CS status after the transfer */ + if (!tr->cs_change) + mcspi->use_multi_mode = false; + + /* If at least one message is not compatible, switch back to single mode */ + if (!mcspi->use_multi_mode) + break; + } + + omap2_mcspi_set_mode(master); + + /* In single mode only a single channel can have the FORCE bit enabled * in its chconf0 register. * Scan all channels and disable them except the current one. * A FORCE can remain from a last transfer having cs_change enabled + * + * In multi mode all FORCE bits must be disabled. */ list_for_each_entry(cs, &ctx->cs, node) { - if (msg->spi->controller_state == cs) + if (msg->spi->controller_state == cs && !mcspi->use_multi_mode) { continue; + } if ((cs->chconf0 & OMAP2_MCSPI_CHCONF_FORCE)) { cs->chconf0 &= ~OMAP2_MCSPI_CHCONF_FORCE;