[PATCH v3] fbdev: claim Cyber2000 SPARC I/O aperture before ioremap

[PATCH v3] fbdev: claim Cyber2000 SPARC I/O aperture before ioremap

[Chintan Patel]

Claim the memory resource associated with the Cyber2000 SPARC MMIO
aperture before accessing it.

This is part of the effort to request memory regions in fbdev drivers.

Signed-off-by: Chintan Patel <chintanlike@gmail.com>
---
v3:
 - Use pr_err() instead of pci_err()
v2:
- Use pci_err() for error reporting instead of printk().

 drivers/video/fbdev/cyber2000fb.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/cyber2000fb.c b/drivers/video/fbdev/cyber2000fb.c
index 2d12f8e96c7e..9fced6bf6ffd 100644
--- a/drivers/video/fbdev/cyber2000fb.c
+++ b/drivers/video/fbdev/cyber2000fb.c
@@ -47,6 +47,7 @@
 #include <linux/io.h>
 #include <linux/i2c.h>
 #include <linux/i2c-algo-bit.h>
+#include <linux/ioport.h>
 
 #ifdef __arm__
 #include <asm/mach-types.h>
@@ -1620,9 +1621,14 @@ static int cyberpro_pci_enable_mmio(struct cfb_info *cfb)
 	 */
 	unsigned char __iomem *iop;
 
+	if (!request_mem_region(0x3000000, 0x5000, "cyber2000fb iop")) {
+		pr_err("iga5000: cannot reserve I/O area 0x3000000\n");
+		return -EBUSY;
+	}
 	iop = ioremap(0x3000000, 0x5000);
 	if (iop == NULL) {
-		printk(KERN_ERR "iga5000: cannot map I/O\n");
+		pr_err("iga5000: cannot map I/O area\n");
+		release_mem_region(0x3000000, 0x5000);
 		return -ENOMEM;
 	}
 
@@ -1633,6 +1639,7 @@ static int cyberpro_pci_enable_mmio(struct cfb_info *cfb)
 	writeb(EXT_BIU_MISC_LIN_ENABLE, iop + 0x3cf);
 
 	iounmap(iop);
+	release_mem_region(0x3000000, 0x5000);
 #else
 	/*
 	 * Most other machine types are "normal", so
-- 
2.43.0

Re: [PATCH v3] fbdev: claim Cyber2000 SPARC I/O aperture before ioremap

[Helge Deller]

On 6/16/26 06:30, Chintan Patel wrote:
> Claim the memory resource associated with the Cyber2000 SPARC MMIO
> aperture before accessing it.
> 
> This is part of the effort to request memory regions in fbdev drivers.

IMHO this patch doesn't make much sense.
The PCI regions (e.g. for x86) are already requested one level higher and are being kept reserved.
This patch here only touches the SPARC code inside a path which is e.g. being used everytime the machine wakes up from sleep (does sparc even suspends?). So, instead here, I think (if you want to make this robust) the region should be reserved e.g. in cyberpro_common_probe(), but I'm not sure if this is even worth the effort....?

Helge

> 
> Signed-off-by: Chintan Patel <chintanlike@gmail.com>
> ---
> v3:
>   - Use pr_err() instead of pci_err()
> v2:
> - Use pci_err() for error reporting instead of printk().
> 
>   drivers/video/fbdev/cyber2000fb.c | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/video/fbdev/cyber2000fb.c b/drivers/video/fbdev/cyber2000fb.c
> index 2d12f8e96c7e..9fced6bf6ffd 100644
> --- a/drivers/video/fbdev/cyber2000fb.c
> +++ b/drivers/video/fbdev/cyber2000fb.c
> @@ -47,6 +47,7 @@
>   #include <linux/io.h>
>   #include <linux/i2c.h>
>   #include <linux/i2c-algo-bit.h>
> +#include <linux/ioport.h>
>   
>   #ifdef __arm__
>   #include <asm/mach-types.h>
> @@ -1620,9 +1621,14 @@ static int cyberpro_pci_enable_mmio(struct cfb_info *cfb)
>   	 */
>   	unsigned char __iomem *iop;
>   
> +	if (!request_mem_region(0x3000000, 0x5000, "cyber2000fb iop")) {
> +		pr_err("iga5000: cannot reserve I/O area 0x3000000\n");
> +		return -EBUSY;
> +	}
>   	iop = ioremap(0x3000000, 0x5000);
>   	if (iop == NULL) {
> -		printk(KERN_ERR "iga5000: cannot map I/O\n");
> +		pr_err("iga5000: cannot map I/O area\n");
> +		release_mem_region(0x3000000, 0x5000);
>   		return -ENOMEM;
>   	}
>   
> @@ -1633,6 +1639,7 @@ static int cyberpro_pci_enable_mmio(struct cfb_info *cfb)
>   	writeb(EXT_BIU_MISC_LIN_ENABLE, iop + 0x3cf);
>   
>   	iounmap(iop);
> +	release_mem_region(0x3000000, 0x5000);
>   #else
>   	/*
>   	 * Most other machine types are "normal", so

Re: [PATCH v3] fbdev: claim Cyber2000 SPARC I/O aperture before ioremap

[Chintan Patel]

On 6/23/26 00:57, Helge Deller wrote:
> On 6/16/26 06:30, Chintan Patel wrote:
>> Claim the memory resource associated with the Cyber2000 SPARC MMIO
>> aperture before accessing it.
>>
>> This is part of the effort to request memory regions in fbdev drivers.
> 
> IMHO this patch doesn't make much sense.
> The PCI regions (e.g. for x86) are already requested one level higher 
> and are being kept reserved.
> This patch here only touches the SPARC code inside a path which is e.g. 
> being used everytime the machine wakes up from sleep (does sparc even 
> suspends?). So, instead here, I think (if you want to make this robust) 
> the region should be reserved e.g. in cyberpro_common_probe(), but I'm 
> not sure if this is even worth the effort....?
> 
> Helge

Hi Helge,

Thanks for the explanation.

That makes sense. I was following the fbdev TODO item to request memory
regions, but I agree that reserving and releasing the region in
cyberpro_pci_enable_mmio() isn't the right lifetime if the function can
be called multiple times.

I'll take a closer look at whether the reservation belongs in
cyberpro_common_probe(). If not, I'll drop this patch.

Thanks,
Chintan