All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
To: Michael Ellerman <mpe@ellerman.id.au>
Cc: Nicholas Piggin <npiggin@gmail.com>,
	 Christophe Leroy <christophe.leroy@csgroup.eu>,
	"Aneesh Kumar K.V" <aneesh.kumar@kernel.org>,
	 "Naveen N. Rao" <naveen.n.rao@linux.ibm.com>,
	linuxppc-dev@lists.ozlabs.org, linux-spi@vger.kernel.org,
	 Mark Brown <broonie@kernel.org>
Subject: Increasing build coverage for drivers/spi/spi-ppc4xx.c
Date: Tue, 27 Feb 2024 09:46:47 +0100	[thread overview]
Message-ID: <qvuhez7vrcoui7i6s4yohd4ednneuoejcp6tw6iwzeefgpyvd6@fkwwtwozhakf> (raw)

[-- Attachment #1: Type: text/plain, Size: 2390 bytes --]

Hello,

recently the spi-ppc4xx.c driver suffered from build errors and warnings
that were undetected for longer than I expected. I think it would be
very beneficial if this driver was enabled in (at least) a powerpc
allmodconfig build.

The challenge to do so is that spi-ppc4xx.c uses dcri_clrset() which is
only defined for 4xx (as these select PPC_DCR_NATIVE).

I wonder if dcri_clrset() could be defined for the PPC_DCR_MMIO case,
too. I tried and failed. The best I came up without extensive doc
reading is:

diff --git a/arch/powerpc/include/asm/dcr-native.h b/arch/powerpc/include/asm/dcr-native.h
index a92059964579..159ab7abfe46 100644
--- a/arch/powerpc/include/asm/dcr-native.h
+++ b/arch/powerpc/include/asm/dcr-native.h
@@ -115,15 +115,11 @@ static inline void __dcri_clrset(int base_addr, int base_data, int reg,
 	unsigned int val;
 
 	spin_lock_irqsave(&dcr_ind_lock, flags);
-	if (cpu_has_feature(CPU_FTR_INDEXED_DCR)) {
-		mtdcrx(base_addr, reg);
-		val = (mfdcrx(base_data) & ~clr) | set;
-		mtdcrx(base_data, val);
-	} else {
-		__mtdcr(base_addr, reg);
-		val = (__mfdcr(base_data) & ~clr) | set;
-		__mtdcr(base_data, val);
-	}
+
+	mtdcr(base_addr, reg);
+	val = (mfdcr(base_data) & ~clr) | set;
+	mtdcr(base_data, val);
+
 	spin_unlock_irqrestore(&dcr_ind_lock, flags);
 }
 
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index bc7021da2fe9..9a0a5e8c70c8 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -810,7 +810,8 @@ config SPI_PL022
 
 config SPI_PPC4xx
 	tristate "PPC4xx SPI Controller"
-	depends on PPC32 && 4xx
+	depends on 4xx || COMPILE_TEST
+	depends on PPC32 || PPC64
 	select SPI_BITBANG
 	help
 	  This selects a driver for the PPC4xx SPI Controller.

While this is a step in the right direction (I think) it's not enough to
make the driver build (but maybe make it easier to define
dcri_clrset()?)

Could someone with more powerpc knowledge jump in and help (for the
benefit of better compile coverage of the spi driver and so less
breakage)? (If you do so based on my changes above, you don't need to
credit me for my effort, claim it as your's. I'm happy enough if the
situation improves.)

Thanks
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
To: Michael Ellerman <mpe@ellerman.id.au>
Cc: linux-spi@vger.kernel.org,
	"Aneesh Kumar K.V" <aneesh.kumar@kernel.org>,
	Mark Brown <broonie@kernel.org>,
	Nicholas Piggin <npiggin@gmail.com>,
	"Naveen N. Rao" <naveen.n.rao@linux.ibm.com>,
	linuxppc-dev@lists.ozlabs.org
Subject: Increasing build coverage for drivers/spi/spi-ppc4xx.c
Date: Tue, 27 Feb 2024 09:46:47 +0100	[thread overview]
Message-ID: <qvuhez7vrcoui7i6s4yohd4ednneuoejcp6tw6iwzeefgpyvd6@fkwwtwozhakf> (raw)

[-- Attachment #1: Type: text/plain, Size: 2390 bytes --]

Hello,

recently the spi-ppc4xx.c driver suffered from build errors and warnings
that were undetected for longer than I expected. I think it would be
very beneficial if this driver was enabled in (at least) a powerpc
allmodconfig build.

The challenge to do so is that spi-ppc4xx.c uses dcri_clrset() which is
only defined for 4xx (as these select PPC_DCR_NATIVE).

I wonder if dcri_clrset() could be defined for the PPC_DCR_MMIO case,
too. I tried and failed. The best I came up without extensive doc
reading is:

diff --git a/arch/powerpc/include/asm/dcr-native.h b/arch/powerpc/include/asm/dcr-native.h
index a92059964579..159ab7abfe46 100644
--- a/arch/powerpc/include/asm/dcr-native.h
+++ b/arch/powerpc/include/asm/dcr-native.h
@@ -115,15 +115,11 @@ static inline void __dcri_clrset(int base_addr, int base_data, int reg,
 	unsigned int val;
 
 	spin_lock_irqsave(&dcr_ind_lock, flags);
-	if (cpu_has_feature(CPU_FTR_INDEXED_DCR)) {
-		mtdcrx(base_addr, reg);
-		val = (mfdcrx(base_data) & ~clr) | set;
-		mtdcrx(base_data, val);
-	} else {
-		__mtdcr(base_addr, reg);
-		val = (__mfdcr(base_data) & ~clr) | set;
-		__mtdcr(base_data, val);
-	}
+
+	mtdcr(base_addr, reg);
+	val = (mfdcr(base_data) & ~clr) | set;
+	mtdcr(base_data, val);
+
 	spin_unlock_irqrestore(&dcr_ind_lock, flags);
 }
 
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index bc7021da2fe9..9a0a5e8c70c8 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -810,7 +810,8 @@ config SPI_PL022
 
 config SPI_PPC4xx
 	tristate "PPC4xx SPI Controller"
-	depends on PPC32 && 4xx
+	depends on 4xx || COMPILE_TEST
+	depends on PPC32 || PPC64
 	select SPI_BITBANG
 	help
 	  This selects a driver for the PPC4xx SPI Controller.

While this is a step in the right direction (I think) it's not enough to
make the driver build (but maybe make it easier to define
dcri_clrset()?)

Could someone with more powerpc knowledge jump in and help (for the
benefit of better compile coverage of the spi driver and so less
breakage)? (If you do so based on my changes above, you don't need to
credit me for my effort, claim it as your's. I'm happy enough if the
situation improves.)

Thanks
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

             reply	other threads:[~2024-02-27  8:47 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-27  8:46 Uwe Kleine-König [this message]
2024-02-27  8:46 ` Increasing build coverage for drivers/spi/spi-ppc4xx.c Uwe Kleine-König
2024-02-27  8:54 ` Tudor Ambarus
2024-02-27  8:54   ` Tudor Ambarus
2024-02-27  9:05   ` Uwe Kleine-König
2024-02-27  9:05     ` Uwe Kleine-König
2024-02-27 10:25 ` Christophe Leroy
2024-02-27 10:25   ` Christophe Leroy
2024-02-27 10:58   ` Uwe Kleine-König
2024-02-27 10:58     ` Uwe Kleine-König
2024-02-27 13:52     ` Christophe Leroy
2024-02-27 13:52       ` Christophe Leroy
2024-02-27 14:00       ` Uwe Kleine-König
2024-02-27 14:00         ` Uwe Kleine-König
2024-02-27 14:38         ` Christophe Leroy
2024-02-27 14:38           ` Christophe Leroy
2024-02-28  5:37           ` Michael Ellerman
2024-02-28  5:37             ` Michael Ellerman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=qvuhez7vrcoui7i6s4yohd4ednneuoejcp6tw6iwzeefgpyvd6@fkwwtwozhakf \
    --to=u.kleine-koenig@pengutronix.de \
    --cc=aneesh.kumar@kernel.org \
    --cc=broonie@kernel.org \
    --cc=christophe.leroy@csgroup.eu \
    --cc=linux-spi@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mpe@ellerman.id.au \
    --cc=naveen.n.rao@linux.ibm.com \
    --cc=npiggin@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.