Linux SPI subsystem development
 help / color / mirror / Atom feed
From: Christophe Leroy <christophe.leroy@csgroup.eu>
To: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
	"Michael Ellerman" <mpe@ellerman.id.au>
Cc: Nicholas Piggin <npiggin@gmail.com>,
	Aneesh Kumar K.V <aneesh.kumar@kernel.org>,
	"Naveen N. Rao" <naveen.n.rao@linux.ibm.com>,
	"linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>,
	"linux-spi@vger.kernel.org" <linux-spi@vger.kernel.org>,
	Mark Brown <broonie@kernel.org>
Subject: Re: Increasing build coverage for drivers/spi/spi-ppc4xx.c
Date: Tue, 27 Feb 2024 10:25:15 +0000	[thread overview]
Message-ID: <6fab09e0-1f21-4ada-b5ae-472bf71a1225@csgroup.eu> (raw)
In-Reply-To: <qvuhez7vrcoui7i6s4yohd4ednneuoejcp6tw6iwzeefgpyvd6@fkwwtwozhakf>



Le 27/02/2024 à 09:46, Uwe Kleine-König a écrit :
> 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.)

What about this ?

diff --git a/arch/powerpc/include/asm/dcr-mmio.h 
b/arch/powerpc/include/asm/dcr-mmio.h
index fc6d93ef4a13..38b515afbffc 100644
--- a/arch/powerpc/include/asm/dcr-mmio.h
+++ b/arch/powerpc/include/asm/dcr-mmio.h
@@ -38,6 +38,11 @@ static inline void dcr_write_mmio(dcr_host_mmio_t host,
  	out_be32(host.token + ((host.base + dcr_n) * host.stride), value);
  }

+static inline void __dcri_clrset(int base_addr, int base_data, int reg,
+				 unsigned clr, unsigned set)
+{
+}
+
  #endif /* __KERNEL__ */
  #endif /* _ASM_POWERPC_DCR_MMIO_H */

diff --git a/arch/powerpc/include/asm/dcr-native.h 
b/arch/powerpc/include/asm/dcr-native.h
index a92059964579..2f6221bf5406 100644
--- a/arch/powerpc/include/asm/dcr-native.h
+++ b/arch/powerpc/include/asm/dcr-native.h
@@ -135,10 +135,6 @@ static inline void __dcri_clrset(int base_addr, int 
base_data, int reg,
  					 DCRN_ ## base ## _CONFIG_DATA,	\
  					 reg, data)

-#define dcri_clrset(base, reg, clr, set)	__dcri_clrset(DCRN_ ## base ## 
_CONFIG_ADDR,	\
-							      DCRN_ ## base ## _CONFIG_DATA,	\
-							      reg, clr, set)
-
  #endif /* __ASSEMBLY__ */
  #endif /* __KERNEL__ */
  #endif /* _ASM_POWERPC_DCR_NATIVE_H */
diff --git a/arch/powerpc/include/asm/dcr.h b/arch/powerpc/include/asm/dcr.h
index 64030e3a1f30..15c123ae38a1 100644
--- a/arch/powerpc/include/asm/dcr.h
+++ b/arch/powerpc/include/asm/dcr.h
@@ -18,6 +18,9 @@
  #include <asm/dcr-mmio.h>
  #endif

+#define dcri_clrset(base, reg, clr, set)	__dcri_clrset(DCRN_ ## base ## 
_CONFIG_ADDR,	\
+							      DCRN_ ## base ## _CONFIG_DATA,	\
+							      reg, clr, set)

  /* Indirection layer for providing both NATIVE and MMIO support. */

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index ddae0fde798e..7b003c5dd613 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -810,7 +810,7 @@ config SPI_PL022

  config SPI_PPC4xx
  	tristate "PPC4xx SPI Controller"
-	depends on PPC32 && 4xx
+	depends on PPC && (4xx || COMPILE_TEST)
  	select SPI_BITBANG
  	help
  	  This selects a driver for the PPC4xx SPI Controller.

  parent reply	other threads:[~2024-02-27 10:25 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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  9:05   ` Uwe Kleine-König
2024-02-27 10:25 ` Christophe Leroy [this message]
2024-02-27 10:58   ` Uwe Kleine-König
2024-02-27 13:52     ` Christophe Leroy
2024-02-27 14:00       ` Uwe Kleine-König
2024-02-27 14:38         ` Christophe Leroy
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=6fab09e0-1f21-4ada-b5ae-472bf71a1225@csgroup.eu \
    --to=christophe.leroy@csgroup.eu \
    --cc=aneesh.kumar@kernel.org \
    --cc=broonie@kernel.org \
    --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 \
    --cc=u.kleine-koenig@pengutronix.de \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox