[v1] cache: StarFive: Require a 64-bit system

[PATCH] cache: StarFive: Require a 64-bit system

Posted by Palmer Dabbelt 1 year, 6 months ago

From: Palmer Dabbelt <palmer@rivosinc.com>

This has a bunch of {read,write}q() calls, so it won't work on 32-bit
systems.  I don't think there's any 32-bit StarFive systems, so for now
just require 64-bit.

Fixes: cabff60ca77d ("cache: Add StarFive StarLink cache management")
Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com>
---
 drivers/cache/Kconfig                   | 1 +
 drivers/cache/starfive_starlink_cache.c | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/cache/Kconfig b/drivers/cache/Kconfig
index 94abd8f632a7..db51386c663a 100644
--- a/drivers/cache/Kconfig
+++ b/drivers/cache/Kconfig
@@ -18,6 +18,7 @@ config STARFIVE_STARLINK_CACHE
 	bool "StarFive StarLink Cache controller"
 	depends on RISCV
 	depends on ARCH_STARFIVE
+	depends on 64BIT
 	select RISCV_DMA_NONCOHERENT
 	select RISCV_NONSTANDARD_CACHE_OPS
 	help
diff --git a/drivers/cache/starfive_starlink_cache.c b/drivers/cache/starfive_starlink_cache.c
index 24c7d078ca22..8ee9569771f8 100644
--- a/drivers/cache/starfive_starlink_cache.c
+++ b/drivers/cache/starfive_starlink_cache.c
@@ -19,7 +19,7 @@
 #define STARLINK_CACHE_FLUSH_CTL			0x10
 #define STARLINK_CACHE_ALIGN				0x40
 
-#define STARLINK_CACHE_ADDRESS_RANGE_MASK		GENMASK(39, 0)
+#define STARLINK_CACHE_ADDRESS_RANGE_MASK		GENMASK(39ULL, 0)
 #define STARLINK_CACHE_FLUSH_CTL_MODE_MASK		GENMASK(2, 1)
 #define STARLINK_CACHE_FLUSH_CTL_ENABLE_MASK		BIT(0)
 
-- 
2.45.2

Re: [PATCH] cache: StarFive: Require a 64-bit system

Posted by Conor Dooley 1 year, 6 months ago

On Fri, Jul 19, 2024 at 09:38:41AM -0700, Palmer Dabbelt wrote:
> From: Palmer Dabbelt <palmer@rivosinc.com>
> 
> This has a bunch of {read,write}q() calls, so it won't work on 32-bit
> systems.  I don't think there's any 32-bit StarFive systems, so for now
> just require 64-bit.
> 
> Fixes: cabff60ca77d ("cache: Add StarFive StarLink cache management")
> Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com>

Acked-by: Conor Dooley <conor.dooley@microchip.com>
if you wanna take it - might be worth being explicit about it causing
build issues for rv32 though.

Thanks,
Conor.

> ---
>  drivers/cache/Kconfig                   | 1 +
>  drivers/cache/starfive_starlink_cache.c | 2 +-
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/cache/Kconfig b/drivers/cache/Kconfig
> index 94abd8f632a7..db51386c663a 100644
> --- a/drivers/cache/Kconfig
> +++ b/drivers/cache/Kconfig
> @@ -18,6 +18,7 @@ config STARFIVE_STARLINK_CACHE
>  	bool "StarFive StarLink Cache controller"
>  	depends on RISCV
>  	depends on ARCH_STARFIVE
> +	depends on 64BIT
>  	select RISCV_DMA_NONCOHERENT
>  	select RISCV_NONSTANDARD_CACHE_OPS
>  	help
> diff --git a/drivers/cache/starfive_starlink_cache.c b/drivers/cache/starfive_starlink_cache.c
> index 24c7d078ca22..8ee9569771f8 100644
> --- a/drivers/cache/starfive_starlink_cache.c
> +++ b/drivers/cache/starfive_starlink_cache.c
> @@ -19,7 +19,7 @@
>  #define STARLINK_CACHE_FLUSH_CTL			0x10
>  #define STARLINK_CACHE_ALIGN				0x40
>  
> -#define STARLINK_CACHE_ADDRESS_RANGE_MASK		GENMASK(39, 0)
> +#define STARLINK_CACHE_ADDRESS_RANGE_MASK		GENMASK(39ULL, 0)
>  #define STARLINK_CACHE_FLUSH_CTL_MODE_MASK		GENMASK(2, 1)
>  #define STARLINK_CACHE_FLUSH_CTL_ENABLE_MASK		BIT(0)
>  
> -- 
> 2.45.2
>

Re: [PATCH] cache: StarFive: Require a 64-bit system

Posted by Conor Dooley 1 year, 6 months ago

On Mon, Jul 22, 2024 at 04:33:27PM +0100, Conor Dooley wrote:
> On Fri, Jul 19, 2024 at 09:38:41AM -0700, Palmer Dabbelt wrote:
> > From: Palmer Dabbelt <palmer@rivosinc.com>
> > 
> > This has a bunch of {read,write}q() calls, so it won't work on 32-bit
> > systems.  I don't think there's any 32-bit StarFive systems, so for now
> > just require 64-bit.
> > 
> > Fixes: cabff60ca77d ("cache: Add StarFive StarLink cache management")
> > Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com>
> 
> Acked-by: Conor Dooley <conor.dooley@microchip.com>
> if you wanna take it - might be worth being explicit about it causing
> build issues for rv32 though.
> 
> > ---
> >  drivers/cache/Kconfig                   | 1 +
> >  drivers/cache/starfive_starlink_cache.c | 2 +-
> >  2 files changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/cache/Kconfig b/drivers/cache/Kconfig
> > index 94abd8f632a7..db51386c663a 100644
> > --- a/drivers/cache/Kconfig
> > +++ b/drivers/cache/Kconfig
> > @@ -18,6 +18,7 @@ config STARFIVE_STARLINK_CACHE
> >  	bool "StarFive StarLink Cache controller"
> >  	depends on RISCV
> >  	depends on ARCH_STARFIVE
> > +	depends on 64BIT
> >  	select RISCV_DMA_NONCOHERENT
> >  	select RISCV_NONSTANDARD_CACHE_OPS
> >  	help
> > diff --git a/drivers/cache/starfive_starlink_cache.c b/drivers/cache/starfive_starlink_cache.c
> > index 24c7d078ca22..8ee9569771f8 100644
> > --- a/drivers/cache/starfive_starlink_cache.c
> > +++ b/drivers/cache/starfive_starlink_cache.c
> > @@ -19,7 +19,7 @@
> >  #define STARLINK_CACHE_FLUSH_CTL			0x10
> >  #define STARLINK_CACHE_ALIGN				0x40
> >  
> > -#define STARLINK_CACHE_ADDRESS_RANGE_MASK		GENMASK(39, 0)
> > +#define STARLINK_CACHE_ADDRESS_RANGE_MASK		GENMASK(39ULL, 0)

Actually, Emil pointed out on IRC that this looks wrong. Should probably
be GENMASK_ULL(39, 0), not GENMASK(39ULL, 0)