From patchwork Wed Mar 6 15:59:40 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andy Shevchenko X-Patchwork-Id: 13584269 Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.13]) (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 C4C4913F014; Wed, 6 Mar 2024 16:01:26 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=198.175.65.13 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1709740888; cv=none; b=HGpjG410fgGIFO1Rj0DQr2RgbpCUyccm2OJyW9hWTcnfIx1i7hnToqrYesk9ni0uaAZ28bqMkzxvvOthJTTdYqZiMehnnZAkVuwq1WqzHv4fDFBHCZz/PjYp4GcGe/4Bh9eZc5wMlVnzAwcE2Ul4XBp16zivNqILzrfvn+2+9RA= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1709740888; c=relaxed/simple; bh=P2l9CR4aujI1O60I6dNzMO2e/QBRzTm+RhT0rIrrIPk=; h=From:To:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=OK/P81Ge9dJqUlU8572StTqZGk7qQxpkb1Ot/5xVdgXhh6sbAILMalwTzkHv06Kkvx7CjLjQyz6Duz+xNJLxx9onaoSdJpiMXlXb2GAow6N88H8oTadP9QU6TsNe7NtTd+/fKCNIAL23BEhcaImVveldvHzkuAzALlLVxT5H/kE= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.intel.com; spf=none smtp.mailfrom=linux.intel.com; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b=knfgAfjm; arc=none smtp.client-ip=198.175.65.13 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.intel.com Authentication-Results: smtp.subspace.kernel.org; spf=none smtp.mailfrom=linux.intel.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b="knfgAfjm" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1709740887; x=1741276887; h=from:to:subject:date:message-id:in-reply-to:references: mime-version:content-transfer-encoding; bh=P2l9CR4aujI1O60I6dNzMO2e/QBRzTm+RhT0rIrrIPk=; b=knfgAfjmwELWSJzOFPU6nLrIx+t03LS2NkKIjuTGO7BBMVGFNuKfcQUW jvVcDJXnZZZrVjt45VrCOY5ZQCt4J0xLpyiclAnhoGYYLwt+9bFB4UyHA IpPUOn4Era+UBPMkuv08kf1stQesQxa9gg7ckpAuKdXe1cJiTlns6Cno9 a2e07Cxdez8iMYJ66Eg+iIpvV7jcuzxSQ1WGvVdYUo5wdutYtJiD6tvNm 5q7tZAK29SXikY4bvJvvPWFFOYVsGKBJHK0K/f2gfg//MIgwnRWaokbUY N/P5tk0rCGHJ6TIFAvtVIOKREbnPB+HXwHicAEyn6/+Ui1G3S6Vz+494x Q==; X-IronPort-AV: E=McAfee;i="6600,9927,11005"; a="15504038" X-IronPort-AV: E=Sophos;i="6.06,208,1705392000"; d="scan'208";a="15504038" Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orvoesa105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 06 Mar 2024 08:01:26 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,11005"; a="937045130" X-IronPort-AV: E=Sophos;i="6.06,208,1705392000"; d="scan'208";a="937045130" Received: from black.fi.intel.com ([10.237.72.28]) by fmsmga001.fm.intel.com with ESMTP; 06 Mar 2024 08:01:24 -0800 Received: by black.fi.intel.com (Postfix, from userid 1003) id EAA003C0; Wed, 6 Mar 2024 18:01:23 +0200 (EET) From: Andy Shevchenko To: Mark Brown , Andy Shevchenko , linux-spi@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH v1 1/3] spi: Exctract spi_set_all_cs_unused() helper Date: Wed, 6 Mar 2024 17:59:40 +0200 Message-ID: <20240306160114.3471398-2-andriy.shevchenko@linux.intel.com> X-Mailer: git-send-email 2.43.0.rc1.1.gbec44491f096 In-Reply-To: <20240306160114.3471398-1-andriy.shevchenko@linux.intel.com> References: <20240306160114.3471398-1-andriy.shevchenko@linux.intel.com> Precedence: bulk X-Mailing-List: linux-spi@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 It seems a few functions implement the similar for-loop to mark all chip select pins unused. Let's deduplicate that code in order to have a single place of that for better maintenance. Signed-off-by: Andy Shevchenko --- drivers/spi/spi.c | 74 +++++++++++++++-------------------------------- 1 file changed, 24 insertions(+), 50 deletions(-) diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index ba4d3fde2054..5e530fa790b0 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -745,6 +745,23 @@ int spi_add_device(struct spi_device *spi) } EXPORT_SYMBOL_GPL(spi_add_device); +static void spi_set_all_cs_unused(struct spi_device *spi) +{ + u8 idx; + + /* + * Zero(0) is a valid physical CS value and can be located at any + * logical CS in the spi->chip_select[]. If all the physical CS + * are initialized to 0 then It would be difficult to differentiate + * between a valid physical CS 0 & an unused logical CS whose physical + * CS can be 0. As a solution to this issue initialize all the CS to 0xFF. + * Now all the unused logical CS will have 0xFF physical CS value & can be + * ignore while performing physical CS validity checks. + */ + for (idx = 0; idx < SPI_CS_CNT_MAX; idx++) + spi_set_chipselect(spi, idx, 0xFF); +} + /** * spi_new_device - instantiate one new SPI device * @ctlr: Controller to which device is connected @@ -764,7 +781,6 @@ struct spi_device *spi_new_device(struct spi_controller *ctlr, { struct spi_device *proxy; int status; - u8 idx; /* * NOTE: caller did any chip->bus_num checks necessary. @@ -780,19 +796,10 @@ struct spi_device *spi_new_device(struct spi_controller *ctlr, WARN_ON(strlen(chip->modalias) >= sizeof(proxy->modalias)); - /* - * Zero(0) is a valid physical CS value and can be located at any - * logical CS in the spi->chip_select[]. If all the physical CS - * are initialized to 0 then It would be difficult to differentiate - * between a valid physical CS 0 & an unused logical CS whose physical - * CS can be 0. As a solution to this issue initialize all the CS to 0xFF. - * Now all the unused logical CS will have 0xFF physical CS value & can be - * ignore while performing physical CS validity checks. - */ - for (idx = 0; idx < SPI_CS_CNT_MAX; idx++) - spi_set_chipselect(proxy, idx, 0xFF); - + /* Use provided chip-select for proxy device */ + spi_set_all_cs_unused(proxy); spi_set_chipselect(proxy, 0, chip->chip_select); + proxy->max_speed_hz = chip->max_speed_hz; proxy->mode = chip->mode; proxy->irq = chip->irq; @@ -2418,17 +2425,7 @@ static int of_spi_parse_dt(struct spi_controller *ctlr, struct spi_device *spi, return -EINVAL; } - /* - * Zero(0) is a valid physical CS value and can be located at any - * logical CS in the spi->chip_select[]. If all the physical CS - * are initialized to 0 then It would be difficult to differentiate - * between a valid physical CS 0 & an unused logical CS whose physical - * CS can be 0. As a solution to this issue initialize all the CS to 0xFF. - * Now all the unused logical CS will have 0xFF physical CS value & can be - * ignore while performing physical CS validity checks. - */ - for (idx = 0; idx < SPI_CS_CNT_MAX; idx++) - spi_set_chipselect(spi, idx, 0xFF); + spi_set_all_cs_unused(spi); /* Device address */ rc = of_property_read_variable_u32_array(nc, "reg", &cs[0], 1, @@ -2565,7 +2562,6 @@ struct spi_device *spi_new_ancillary_device(struct spi_device *spi, struct spi_controller *ctlr = spi->controller; struct spi_device *ancillary; int rc = 0; - u8 idx; /* Alloc an spi_device */ ancillary = spi_alloc_device(ctlr); @@ -2576,19 +2572,8 @@ struct spi_device *spi_new_ancillary_device(struct spi_device *spi, strscpy(ancillary->modalias, "dummy", sizeof(ancillary->modalias)); - /* - * Zero(0) is a valid physical CS value and can be located at any - * logical CS in the spi->chip_select[]. If all the physical CS - * are initialized to 0 then It would be difficult to differentiate - * between a valid physical CS 0 & an unused logical CS whose physical - * CS can be 0. As a solution to this issue initialize all the CS to 0xFF. - * Now all the unused logical CS will have 0xFF physical CS value & can be - * ignore while performing physical CS validity checks. - */ - for (idx = 0; idx < SPI_CS_CNT_MAX; idx++) - spi_set_chipselect(ancillary, idx, 0xFF); - /* Use provided chip-select for ancillary device */ + spi_set_all_cs_unused(ancillary); spi_set_chipselect(ancillary, 0, chip_select); /* Take over SPI mode/speed from SPI main device */ @@ -2805,7 +2790,6 @@ struct spi_device *acpi_spi_device_alloc(struct spi_controller *ctlr, struct acpi_spi_lookup lookup = {}; struct spi_device *spi; int ret; - u8 idx; if (!ctlr && index == -1) return ERR_PTR(-EINVAL); @@ -2841,24 +2825,14 @@ struct spi_device *acpi_spi_device_alloc(struct spi_controller *ctlr, return ERR_PTR(-ENOMEM); } - /* - * Zero(0) is a valid physical CS value and can be located at any - * logical CS in the spi->chip_select[]. If all the physical CS - * are initialized to 0 then It would be difficult to differentiate - * between a valid physical CS 0 & an unused logical CS whose physical - * CS can be 0. As a solution to this issue initialize all the CS to 0xFF. - * Now all the unused logical CS will have 0xFF physical CS value & can be - * ignore while performing physical CS validity checks. - */ - for (idx = 0; idx < SPI_CS_CNT_MAX; idx++) - spi_set_chipselect(spi, idx, 0xFF); + spi_set_all_cs_unused(spi); + spi_set_chipselect(spi, 0, lookup.chip_select); ACPI_COMPANION_SET(&spi->dev, adev); spi->max_speed_hz = lookup.max_speed_hz; spi->mode |= lookup.mode; spi->irq = lookup.irq; spi->bits_per_word = lookup.bits_per_word; - spi_set_chipselect(spi, 0, lookup.chip_select); /* * spi->chip_select[i] gives the corresponding physical CS for logical CS i * logical CS number is represented by setting the ith bit in spi->cs_index_mask From patchwork Wed Mar 6 15:59:41 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andy Shevchenko X-Patchwork-Id: 13584270 Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.13]) (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 5407A13F42A; Wed, 6 Mar 2024 16:01:27 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=198.175.65.13 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1709740888; cv=none; b=rxb2WX+Rras0eoQSSnXSTA3lXNYP8OnLuDdPipPqj2AFt50eds+vxOefHnbDBBF4gysh/ZmU2J2VQOPWpP9JkXAo4xmoznK71NGaL4yryXluc0cSebqUUFRaKZYkvszotvCdUTDakJfMIr32yJy6itAZ5uz4R0TkAscIQ7Gacz8= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1709740888; c=relaxed/simple; bh=ONiNnvJDMYm2St92CefVKZ/wRrZ/XLhtQQJUC7bzd/0=; h=From:To:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=Q8X9NyQynru8HhK6hrNt1ZHAEAK8mYTMqR+XNp7aCzvsJ+Bblr0KwjwuJg/NVuBUnUkvS3AIbb4+le9gxkhMjA0FSEM/O+2eBgsBsg3S/K++J1R9RGKvWC2f42yGzLwK1oVm5MKBu/1Q7vfdHJaRGjrrxfBfb2fM0+qqK5tDO4Q= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.intel.com; spf=none smtp.mailfrom=linux.intel.com; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b=g9pmez6x; arc=none smtp.client-ip=198.175.65.13 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.intel.com Authentication-Results: smtp.subspace.kernel.org; spf=none smtp.mailfrom=linux.intel.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b="g9pmez6x" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1709740888; x=1741276888; h=from:to:subject:date:message-id:in-reply-to:references: mime-version:content-transfer-encoding; bh=ONiNnvJDMYm2St92CefVKZ/wRrZ/XLhtQQJUC7bzd/0=; b=g9pmez6xwjys9uI+TmsIkVWJGmUWvCZCB4RZkKLAJVXFDNBegQQT1Lgw p55ooJYtAz04wvQPgKdowd5/jYNgR7EwXfgu8JfEfUjdD7C/ONkuR8EdG tje06NijfQsSkmiiWi9wIskYjnyJMx1pRUdo8bGtn0rT0TS0TP2fmzQVI EKquEcyySc4rEG0ScT5qP2BPyClCCPHz9UaQtLTKCj7pRuW5Q8Lhw9HJS LKMPF6AutUh5HKnMVPRnK9eVsAaD8oyM072lu69cDSd+MiOZOKbPa5hlr bEW35YyV97NcbTuf5vVa6va0s31z7TFNVJ3uNK4d4PDwHWLAaR9KIdMj1 Q==; X-IronPort-AV: E=McAfee;i="6600,9927,11005"; a="15504045" X-IronPort-AV: E=Sophos;i="6.06,208,1705392000"; d="scan'208";a="15504045" Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orvoesa105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 06 Mar 2024 08:01:27 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,11005"; a="937045131" X-IronPort-AV: E=Sophos;i="6.06,208,1705392000"; d="scan'208";a="937045131" Received: from black.fi.intel.com ([10.237.72.28]) by fmsmga001.fm.intel.com with ESMTP; 06 Mar 2024 08:01:25 -0800 Received: by black.fi.intel.com (Postfix, from userid 1003) id 963AA3F1; Wed, 6 Mar 2024 18:01:24 +0200 (EET) From: Andy Shevchenko To: Mark Brown , Andy Shevchenko , linux-spi@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH v1 2/3] spi: Exctract spi_dev_check_cs() helper Date: Wed, 6 Mar 2024 17:59:41 +0200 Message-ID: <20240306160114.3471398-3-andriy.shevchenko@linux.intel.com> X-Mailer: git-send-email 2.43.0.rc1.1.gbec44491f096 In-Reply-To: <20240306160114.3471398-1-andriy.shevchenko@linux.intel.com> References: <20240306160114.3471398-1-andriy.shevchenko@linux.intel.com> Precedence: bulk X-Mailing-List: linux-spi@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 It seems a few functions implement the similar for-loop to validate chip select pins for uniqueness. Let's deduplicate that code in order to have a single place of that for better maintenance. Signed-off-by: Andy Shevchenko --- drivers/spi/spi.c | 47 +++++++++++++++++++++++++++-------------------- 1 file changed, 27 insertions(+), 20 deletions(-) diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 5e530fa790b0..eb7e3ddf909b 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -608,23 +608,35 @@ static void spi_dev_set_name(struct spi_device *spi) spi_get_chipselect(spi, 0)); } +static inline int spi_dev_check_cs(struct device *dev, + struct spi_device *spi, u8 idx, + struct spi_device *new_spi, u8 new_idx) +{ + u8 cs, cs_new; + u8 idx_new; + + cs = spi_get_chipselect(spi, idx); + for (idx_new = new_idx; idx_new < SPI_CS_CNT_MAX; idx_new++) { + cs_new = spi_get_chipselect(new_spi, idx_new); + if (cs != 0xFF && cs_new != 0xFF && cs == cs_new) { + dev_err(dev, "chipselect %u already in use\n", cs_new); + return -EBUSY; + } + } + return 0; +} + static int spi_dev_check(struct device *dev, void *data) { struct spi_device *spi = to_spi_device(dev); struct spi_device *new_spi = data; - int idx, nw_idx; - u8 cs, cs_nw; + int status, idx; if (spi->controller == new_spi->controller) { for (idx = 0; idx < SPI_CS_CNT_MAX; idx++) { - cs = spi_get_chipselect(spi, idx); - for (nw_idx = 0; nw_idx < SPI_CS_CNT_MAX; nw_idx++) { - cs_nw = spi_get_chipselect(new_spi, nw_idx); - if (cs != 0xFF && cs_nw != 0xFF && cs == cs_nw) { - dev_err(dev, "chipselect %d already in use\n", cs_nw); - return -EBUSY; - } - } + status = spi_dev_check_cs(dev, spi, idx, new_spi, 0); + if (status) + return status; } } return 0; @@ -640,8 +652,8 @@ static int __spi_add_device(struct spi_device *spi) { struct spi_controller *ctlr = spi->controller; struct device *dev = ctlr->dev.parent; - int status, idx, nw_idx; - u8 cs, nw_cs; + int status, idx; + u8 cs; for (idx = 0; idx < SPI_CS_CNT_MAX; idx++) { /* Chipselects are numbered 0..max; validate. */ @@ -658,14 +670,9 @@ static int __spi_add_device(struct spi_device *spi) * For example, spi->chip_select[0] != spi->chip_select[1] and so on. */ for (idx = 0; idx < SPI_CS_CNT_MAX; idx++) { - cs = spi_get_chipselect(spi, idx); - for (nw_idx = idx + 1; nw_idx < SPI_CS_CNT_MAX; nw_idx++) { - nw_cs = spi_get_chipselect(spi, nw_idx); - if (cs != 0xFF && nw_cs != 0xFF && cs == nw_cs) { - dev_err(dev, "chipselect %d already in use\n", nw_cs); - return -EBUSY; - } - } + status = spi_dev_check_cs(dev, spi, idx, spi, idx + 1); + if (status) + return status; } /* Set the bus ID string */ From patchwork Wed Mar 6 15:59:42 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andy Shevchenko X-Patchwork-Id: 13584271 Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.13]) (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 24E2C13F44D; Wed, 6 Mar 2024 16:01:27 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=198.175.65.13 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1709740889; cv=none; b=NB+4NzUMKioz79OC0JaV2btXJrJd+HKFLlQsHObRDQ312/qRQ23yBjX61ENvR898LctE6uMh101OIil2um55+73Rw7BWPLvDI1YVM/T0ayWrV+QhDu3FjZX6Rb9orrO9rq81AIm76GmsNamG43ve7pjdpqJLpeWQY93+QHlRmIk= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1709740889; c=relaxed/simple; bh=2P5SJN0mZupkQUYJMPK1FJ8A0y8TZeDvivA+SQJ6m7M=; h=From:To:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=WIqYDld+8x1I/QJEbN78tBT9nDdlDtB/zaPgJXLNvHB2gX34I55OvQIkNvtWtgR0ybhK8craZRAXl34WQxQ5WtBOsujQyN5fU0ui1XIp93C4qXexXiLrBTnbv94HunQ2B16n586poa7vQQUuvaS5xdd1OhDOaSLN1O2esWESqT0= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.intel.com; spf=none smtp.mailfrom=linux.intel.com; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b=cyVfXuFz; arc=none smtp.client-ip=198.175.65.13 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.intel.com Authentication-Results: smtp.subspace.kernel.org; spf=none smtp.mailfrom=linux.intel.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b="cyVfXuFz" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1709740888; x=1741276888; h=from:to:subject:date:message-id:in-reply-to:references: mime-version:content-transfer-encoding; bh=2P5SJN0mZupkQUYJMPK1FJ8A0y8TZeDvivA+SQJ6m7M=; b=cyVfXuFzZLJuOZXDxjRN0ni+ABVzu3Zy3f69kAgEjKSb/mxj50hdfL80 0expzLn/8CxKpXhHHGtDPjR8PEsag+VT27Z6y6yYWFLnBGWse4AvoqI6L Sz/RwstgjuHnIx1xhKMkeK+Foa6c+/tJROxd8fUpP7DMdRZFGpY8a7dLH zp7ITFF+g5Wglnm+ss7LKnMR2GyY15Fara6aeHHa6db9Jk0dl0vwOVGo9 r/rXaDqEgUzsqH3ek4rxv2j6Q1mLxch7uyQbC1ilnOanLo2CnVdiW+NK/ ioouy+AQNDnYV5elQxUWuyZDMGRdczYLP6R+unoEupMMLqU7/OVhY+dXT g==; X-IronPort-AV: E=McAfee;i="6600,9927,11005"; a="15504050" X-IronPort-AV: E=Sophos;i="6.06,208,1705392000"; d="scan'208";a="15504050" Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orvoesa105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 06 Mar 2024 08:01:28 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,11005"; a="937045132" X-IronPort-AV: E=Sophos;i="6.06,208,1705392000"; d="scan'208";a="937045132" Received: from black.fi.intel.com ([10.237.72.28]) by fmsmga001.fm.intel.com with ESMTP; 06 Mar 2024 08:01:26 -0800 Received: by black.fi.intel.com (Postfix, from userid 1003) id 25F10184; Wed, 6 Mar 2024 18:01:25 +0200 (EET) From: Andy Shevchenko To: Mark Brown , Andy Shevchenko , linux-spi@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH v1 3/3] spi: Fix multiple issues with Chip Select variables and comments Date: Wed, 6 Mar 2024 17:59:42 +0200 Message-ID: <20240306160114.3471398-4-andriy.shevchenko@linux.intel.com> X-Mailer: git-send-email 2.43.0.rc1.1.gbec44491f096 In-Reply-To: <20240306160114.3471398-1-andriy.shevchenko@linux.intel.com> References: <20240306160114.3471398-1-andriy.shevchenko@linux.intel.com> Precedence: bulk X-Mailing-List: linux-spi@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 There are the following issues with the current code: - inconsistent use of 0xFF and -1 for invalid chip select pin - inconsistent plain or BIT() use - wrong types used for last_cs_* fields - wrong multi-line comment style - misleading or hard-to-understand comments Fix all of these here. Signed-off-by: Andy Shevchenko --- drivers/spi/spi.c | 74 +++++++++++++++++++---------------------- include/linux/spi/spi.h | 15 +++++---- 2 files changed, 42 insertions(+), 47 deletions(-) diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index eb7e3ddf909b..f18738ae95f8 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -608,6 +608,22 @@ static void spi_dev_set_name(struct spi_device *spi) spi_get_chipselect(spi, 0)); } +/* + * Zero(0) is a valid physical CS value and can be located at any + * logical CS in the spi->chip_select[]. If all the physical CS + * are initialized to 0 then It would be difficult to differentiate + * between a valid physical CS 0 & an unused logical CS whose physical + * CS can be 0. As a solution to this issue initialize all the CS to -1. + * Now all the unused logical CS will have -1 physical CS value & can be + * ignored while performing physical CS validity checks. + */ +#define SPI_INVALID_CS ((s8)-1) + +static inline bool is_valid_cs(s8 chip_select) +{ + return chip_select != SPI_INVALID_CS; +} + static inline int spi_dev_check_cs(struct device *dev, struct spi_device *spi, u8 idx, struct spi_device *new_spi, u8 new_idx) @@ -618,7 +634,7 @@ static inline int spi_dev_check_cs(struct device *dev, cs = spi_get_chipselect(spi, idx); for (idx_new = new_idx; idx_new < SPI_CS_CNT_MAX; idx_new++) { cs_new = spi_get_chipselect(new_spi, idx_new); - if (cs != 0xFF && cs_new != 0xFF && cs == cs_new) { + if (is_valid_cs(cs) && is_valid_cs(cs_new) && cs == cs_new) { dev_err(dev, "chipselect %u already in use\n", cs_new); return -EBUSY; } @@ -658,7 +674,7 @@ static int __spi_add_device(struct spi_device *spi) for (idx = 0; idx < SPI_CS_CNT_MAX; idx++) { /* Chipselects are numbered 0..max; validate. */ cs = spi_get_chipselect(spi, idx); - if (cs != 0xFF && cs >= ctlr->num_chipselect) { + if (is_valid_cs(cs) && cs >= ctlr->num_chipselect) { dev_err(dev, "cs%d >= max %d\n", spi_get_chipselect(spi, idx), ctlr->num_chipselect); return -EINVAL; @@ -698,7 +714,7 @@ static int __spi_add_device(struct spi_device *spi) for (idx = 0; idx < SPI_CS_CNT_MAX; idx++) { cs = spi_get_chipselect(spi, idx); - if (cs != 0xFF) + if (is_valid_cs(cs)) spi_set_csgpiod(spi, idx, ctlr->cs_gpiods[cs]); } } @@ -756,17 +772,8 @@ static void spi_set_all_cs_unused(struct spi_device *spi) { u8 idx; - /* - * Zero(0) is a valid physical CS value and can be located at any - * logical CS in the spi->chip_select[]. If all the physical CS - * are initialized to 0 then It would be difficult to differentiate - * between a valid physical CS 0 & an unused logical CS whose physical - * CS can be 0. As a solution to this issue initialize all the CS to 0xFF. - * Now all the unused logical CS will have 0xFF physical CS value & can be - * ignore while performing physical CS validity checks. - */ for (idx = 0; idx < SPI_CS_CNT_MAX; idx++) - spi_set_chipselect(spi, idx, 0xFF); + spi_set_chipselect(spi, idx, SPI_INVALID_CS); } /** @@ -1021,7 +1028,7 @@ static inline bool spi_is_last_cs(struct spi_device *spi) bool last = false; for (idx = 0; idx < SPI_CS_CNT_MAX; idx++) { - if ((spi->cs_index_mask >> idx) & 0x01) { + if (spi->cs_index_mask & BIT(idx)) { if (spi->controller->last_cs[idx] == spi_get_chipselect(spi, idx)) last = true; } @@ -1050,7 +1057,7 @@ static void spi_set_cs(struct spi_device *spi, bool enable, bool force) spi->controller->last_cs_index_mask = spi->cs_index_mask; for (idx = 0; idx < SPI_CS_CNT_MAX; idx++) - spi->controller->last_cs[idx] = enable ? spi_get_chipselect(spi, 0) : -1; + spi->controller->last_cs[idx] = enable ? spi_get_chipselect(spi, 0) : SPI_INVALID_CS; spi->controller->last_cs_mode_high = spi->mode & SPI_CS_HIGH; if (spi->mode & SPI_CS_HIGH) @@ -1072,8 +1079,7 @@ static void spi_set_cs(struct spi_device *spi, bool enable, bool force) * into account. */ for (idx = 0; idx < SPI_CS_CNT_MAX; idx++) { - if (((spi->cs_index_mask >> idx) & 0x01) && - spi_get_csgpiod(spi, idx)) { + if ((spi->cs_index_mask & BIT(idx)) && spi_get_csgpiod(spi, idx)) { if (has_acpi_companion(&spi->dev)) gpiod_set_value_cansleep(spi_get_csgpiod(spi, idx), !enable); @@ -2456,14 +2462,10 @@ static int of_spi_parse_dt(struct spi_controller *ctlr, struct spi_device *spi, spi_set_chipselect(spi, idx, cs[idx]); /* - * spi->chip_select[i] gives the corresponding physical CS for logical CS i - * logical CS number is represented by setting the ith bit in spi->cs_index_mask - * So, for example, if spi->cs_index_mask = 0x01 then logical CS number is 0 and - * spi->chip_select[0] will give the physical CS. - * By default spi->chip_select[0] will hold the physical CS number so, set - * spi->cs_index_mask as 0x01. + * By default spi->chip_select[0] will hold the physical CS number, + * so set bit 0 in spi->cs_index_mask. */ - spi->cs_index_mask = 0x01; + spi->cs_index_mask = BIT(0); /* Device speed */ if (!of_property_read_u32(nc, "spi-max-frequency", &value)) @@ -2587,14 +2589,10 @@ struct spi_device *spi_new_ancillary_device(struct spi_device *spi, ancillary->max_speed_hz = spi->max_speed_hz; ancillary->mode = spi->mode; /* - * spi->chip_select[i] gives the corresponding physical CS for logical CS i - * logical CS number is represented by setting the ith bit in spi->cs_index_mask - * So, for example, if spi->cs_index_mask = 0x01 then logical CS number is 0 and - * spi->chip_select[0] will give the physical CS. - * By default spi->chip_select[0] will hold the physical CS number so, set - * spi->cs_index_mask as 0x01. + * By default spi->chip_select[0] will hold the physical CS number, + * so set bit 0 in spi->cs_index_mask. */ - ancillary->cs_index_mask = 0x01; + ancillary->cs_index_mask = BIT(0); WARN_ON(!mutex_is_locked(&ctlr->add_lock)); @@ -2841,14 +2839,10 @@ struct spi_device *acpi_spi_device_alloc(struct spi_controller *ctlr, spi->irq = lookup.irq; spi->bits_per_word = lookup.bits_per_word; /* - * spi->chip_select[i] gives the corresponding physical CS for logical CS i - * logical CS number is represented by setting the ith bit in spi->cs_index_mask - * So, for example, if spi->cs_index_mask = 0x01 then logical CS number is 0 and - * spi->chip_select[0] will give the physical CS. - * By default spi->chip_select[0] will hold the physical CS number so, set - * spi->cs_index_mask as 0x01. + * By default spi->chip_select[0] will hold the physical CS number, + * so set bit 0 in spi->cs_index_mask. */ - spi->cs_index_mask = 0x01; + spi->cs_index_mask = BIT(0); return spi; } @@ -3346,9 +3340,9 @@ int spi_register_controller(struct spi_controller *ctlr) goto free_bus_id; } - /* Setting last_cs to -1 means no chip selected */ + /* Setting last_cs to SPI_INVALID_CS means no chip selected */ for (idx = 0; idx < SPI_CS_CNT_MAX; idx++) - ctlr->last_cs[idx] = -1; + ctlr->last_cs[idx] = SPI_INVALID_CS; status = device_add(&ctlr->dev); if (status < 0) diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h index e400d454b3f0..81ca62e608c1 100644 --- a/include/linux/spi/spi.h +++ b/include/linux/spi/spi.h @@ -226,7 +226,8 @@ struct spi_device { /* The statistics */ struct spi_statistics __percpu *pcpu_statistics; - /* Bit mask of the chipselect(s) that the driver need to use from + /* + * Bit mask of the chipselect(s) that the driver need to use from * the chipselect array.When the controller is capable to handle * multiple chip selects & memories are connected in parallel * then more than one bit need to be set in cs_index_mask. @@ -448,9 +449,11 @@ extern struct spi_device *spi_new_ancillary_device(struct spi_device *spi, u8 ch * the @cur_msg_completion. This flag is used to signal the context that * is running spi_finalize_current_message() that it needs to complete() * @cur_msg_mapped: message has been mapped for DMA + * @fallback: fallback to PIO if DMA transfer return failure with + * SPI_TRANS_FAIL_NO_START. + * @last_cs_mode_high: was (mode & SPI_CS_HIGH) true on the last call to set_cs. * @last_cs: the last chip_select that is recorded by set_cs, -1 on non chip * selected - * @last_cs_mode_high: was (mode & SPI_CS_HIGH) true on the last call to set_cs. * @xfer_completion: used by core transfer_one_message() * @busy: message pump is busy * @running: message pump is running @@ -527,8 +530,6 @@ extern struct spi_device *spi_new_ancillary_device(struct spi_device *spi, u8 ch * If the driver does not set this, the SPI core takes the snapshot as * close to the driver hand-over as possible. * @irq_flags: Interrupt enable state during PTP system timestamping - * @fallback: fallback to PIO if DMA transfer return failure with - * SPI_TRANS_FAIL_NO_START. * @queue_empty: signal green light for opportunistically skipping the queue * for spi_sync transfers. * @must_async: disable all fast paths in the core @@ -708,10 +709,10 @@ struct spi_controller { bool rt; bool auto_runtime_pm; bool cur_msg_mapped; - char last_cs[SPI_CS_CNT_MAX]; - char last_cs_index_mask; - bool last_cs_mode_high; bool fallback; + bool last_cs_mode_high; + s8 last_cs[SPI_CS_CNT_MAX]; + u32 last_cs_index_mask : SPI_CS_CNT_MAX; struct completion xfer_completion; size_t max_dma_len;