From: Thomas Zimmermann <tzimmermann@suse.de>
To: Arnd Bergmann <arnd@arndb.de>, kernel test robot <lkp@intel.com>,
Helge Deller <deller@gmx.de>,
Geert Uytterhoeven <geert@linux-m68k.org>,
Javier Martinez Canillas <javierm@redhat.com>,
Daniel Vetter <daniel@ffwll.ch>, Vineet Gupta <vgupta@kernel.org>,
Huacai Chen <chenhuacai@kernel.org>,
WANG Xuerui <kernel@xen0n.name>,
"David S . Miller" <davem@davemloft.net>,
"James E . J . Bottomley" <James.Bottomley@hansenpartnership.com>,
Sam Ravnborg <sam@ravnborg.org>,
suijingfeng@loongson.cn
Cc: Linux-Arch <linux-arch@vger.kernel.org>,
linux-fbdev@vger.kernel.org, linux-ia64@vger.kernel.org,
linux-parisc@vger.kernel.org, linux-kernel@vger.kernel.org,
dri-devel@lists.freedesktop.org, linux-m68k@lists.linux-m68k.org,
loongarch@lists.linux.dev, oe-kbuild-all@lists.linux.dev,
sparclinux@vger.kernel.org, linux-snps-arc@lists.infradead.org,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v6 5/6] fbdev: Move framebuffer I/O helpers into <asm/fb.h>
Date: Thu, 11 May 2023 15:12:06 +0200 [thread overview]
Message-ID: <cee24e80-cc41-00bc-06a2-37f1e2cad8ef@suse.de> (raw)
In-Reply-To: <743d2b1e-c843-4fb2-b252-0006be2e2bd8@app.fastmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 2375 bytes --]
Hi
Am 10.05.23 um 17:54 schrieb Arnd Bergmann:
> On Wed, May 10, 2023, at 16:27, Thomas Zimmermann wrote:
>> Am 10.05.23 um 16:15 schrieb Arnd Bergmann:
>>> On Wed, May 10, 2023, at 16:03, kernel test robot wrote:
>
>>> I think that's a preexisting bug and I have no idea what the
>>> correct solution is. Looking for HD64461 shows it being used
>>> both with inw/outw and readw/writew, so there is no way to have
>>> the correct type. The sh __raw_readw() definition hides this bug,
>>> but that is a problem with arch/sh and it probably hides others
>>> as well.
>>
>> The constant HD64461_IOBASE is defined as integer at
>>
>>
>> https://elixir.bootlin.com/linux/latest/source/arch/sh/include/asm/hd64461.h#L17
>>
>> but fb_readw() expects a volatile-void pointer. I guess we could add a
>> cast somewhere to silence the problem. In the current upstream code,
>> that appears to be done by sh's __raw_readw() internally:
>>
>>
>> https://elixir.bootlin.com/linux/latest/source/arch/sh/include/asm/io.h#L35
>
> Sure, that would make it build again, but that still doesn't make the
> code correct, since it's completely unclear what base address the
> HD64461_IOBASE is relative to. The hp6xx platform code only passes it
> through inw()/outw(), which take an offset relative to sh_io_port_base,
> but that is not initialized on hp6xx. I tried to find in the history
> when it broke, apparently that was in 2007 commit 34a780a0afeb ("sh:
> hp6xx pata_platform support."), which removed the custom inw/outw
> implementations.
It just occured to me that these fb_read and fb_write calls are probably
all wrong. The fb_ interfaces are for framebuffer I/O memory. The driver
uses them to access the regular state registers. The writew() on sh is
definitely different. [1]
I assume that it only works because CONFIG_SWAP_IO_SPACE [2] is not set
in hp6xx_defconfig.
Best regards
Thomas
[1]
https://elixir.bootlin.com/linux/latest/source/arch/sh/include/asm/io.h#L55
[2]
https://elixir.bootlin.com/linux/latest/source/arch/sh/include/mach-common/mach/mangle-port.h#L22
>
> Arnd
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]
WARNING: multiple messages have this Message-ID (diff)
From: Thomas Zimmermann <tzimmermann@suse.de>
To: Arnd Bergmann <arnd@arndb.de>, kernel test robot <lkp@intel.com>,
Helge Deller <deller@gmx.de>,
Geert Uytterhoeven <geert@linux-m68k.org>,
Javier Martinez Canillas <javierm@redhat.com>,
Daniel Vetter <daniel@ffwll.ch>, Vineet Gupta <vgupta@kernel.org>,
Huacai Chen <chenhuacai@kernel.org>,
WANG Xuerui <kernel@xen0n.name>,
"David S . Miller" <davem@davemloft.net>,
"James E . J . Bottomley" <James.Bottomley@hansenpartnership.com>,
Sam Ravnborg <sam@ravnborg.org>,
suijingfeng@loongson.cn
Cc: Linux-Arch <linux-arch@vger.kernel.org>,
linux-fbdev@vger.kernel.org, linux-ia64@vger.kernel.org,
linux-parisc@vger.kernel.org, linux-kernel@vger.kernel.org,
dri-devel@lists.freedesktop.org, linux-m68k@lists.linux-m68k.org,
loongarch@lists.linux.dev, oe-kbuild-all@lists.linux.dev,
sparclinux@vger.kernel.org, linux-snps-arc@lists.infradead.org,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v6 5/6] fbdev: Move framebuffer I/O helpers into <asm/fb.h>
Date: Thu, 11 May 2023 15:12:06 +0200 [thread overview]
Message-ID: <cee24e80-cc41-00bc-06a2-37f1e2cad8ef@suse.de> (raw)
In-Reply-To: <743d2b1e-c843-4fb2-b252-0006be2e2bd8@app.fastmail.com>
[-- Attachment #1.1.1: Type: text/plain, Size: 2375 bytes --]
Hi
Am 10.05.23 um 17:54 schrieb Arnd Bergmann:
> On Wed, May 10, 2023, at 16:27, Thomas Zimmermann wrote:
>> Am 10.05.23 um 16:15 schrieb Arnd Bergmann:
>>> On Wed, May 10, 2023, at 16:03, kernel test robot wrote:
>
>>> I think that's a preexisting bug and I have no idea what the
>>> correct solution is. Looking for HD64461 shows it being used
>>> both with inw/outw and readw/writew, so there is no way to have
>>> the correct type. The sh __raw_readw() definition hides this bug,
>>> but that is a problem with arch/sh and it probably hides others
>>> as well.
>>
>> The constant HD64461_IOBASE is defined as integer at
>>
>>
>> https://elixir.bootlin.com/linux/latest/source/arch/sh/include/asm/hd64461.h#L17
>>
>> but fb_readw() expects a volatile-void pointer. I guess we could add a
>> cast somewhere to silence the problem. In the current upstream code,
>> that appears to be done by sh's __raw_readw() internally:
>>
>>
>> https://elixir.bootlin.com/linux/latest/source/arch/sh/include/asm/io.h#L35
>
> Sure, that would make it build again, but that still doesn't make the
> code correct, since it's completely unclear what base address the
> HD64461_IOBASE is relative to. The hp6xx platform code only passes it
> through inw()/outw(), which take an offset relative to sh_io_port_base,
> but that is not initialized on hp6xx. I tried to find in the history
> when it broke, apparently that was in 2007 commit 34a780a0afeb ("sh:
> hp6xx pata_platform support."), which removed the custom inw/outw
> implementations.
It just occured to me that these fb_read and fb_write calls are probably
all wrong. The fb_ interfaces are for framebuffer I/O memory. The driver
uses them to access the regular state registers. The writew() on sh is
definitely different. [1]
I assume that it only works because CONFIG_SWAP_IO_SPACE [2] is not set
in hp6xx_defconfig.
Best regards
Thomas
[1]
https://elixir.bootlin.com/linux/latest/source/arch/sh/include/asm/io.h#L55
[2]
https://elixir.bootlin.com/linux/latest/source/arch/sh/include/mach-common/mach/mangle-port.h#L22
>
> Arnd
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]
[-- Attachment #2: Type: text/plain, Size: 170 bytes --]
_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
WARNING: multiple messages have this Message-ID (diff)
From: Thomas Zimmermann <tzimmermann@suse.de>
To: Arnd Bergmann <arnd@arndb.de>, kernel test robot <lkp@intel.com>,
Helge Deller <deller@gmx.de>,
Geert Uytterhoeven <geert@linux-m68k.org>,
Javier Martinez Canillas <javierm@redhat.com>,
Daniel Vetter <daniel@ffwll.ch>, Vineet Gupta <vgupta@kernel.org>,
Huacai Chen <chenhuacai@kernel.org>,
WANG Xuerui <kernel@xen0n.name>,
"David S . Miller" <davem@davemloft.net>,
"James E . J . Bottomley" <James.Bottomley@hansenpartnership.com>,
Sam Ravnborg <sam@ravnborg.org>,
suijingfeng@loongson.cn
Cc: Linux-Arch <linux-arch@vger.kernel.org>,
linux-fbdev@vger.kernel.org, linux-ia64@vger.kernel.org,
linux-parisc@vger.kernel.org, linux-kernel@vger.kernel.org,
dri-devel@lists.freedesktop.org, linux-m68k@lists.linux-m68k.org,
loongarch@lists.linux.dev, oe-kbuild-all@lists.linux.dev,
sparclinux@vger.kernel.org, linux-snps-arc@lists.infradead.org,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v6 5/6] fbdev: Move framebuffer I/O helpers into <asm/fb.h>
Date: Thu, 11 May 2023 13:12:06 +0000 [thread overview]
Message-ID: <cee24e80-cc41-00bc-06a2-37f1e2cad8ef@suse.de> (raw)
In-Reply-To: <743d2b1e-c843-4fb2-b252-0006be2e2bd8@app.fastmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 2375 bytes --]
Hi
Am 10.05.23 um 17:54 schrieb Arnd Bergmann:
> On Wed, May 10, 2023, at 16:27, Thomas Zimmermann wrote:
>> Am 10.05.23 um 16:15 schrieb Arnd Bergmann:
>>> On Wed, May 10, 2023, at 16:03, kernel test robot wrote:
>
>>> I think that's a preexisting bug and I have no idea what the
>>> correct solution is. Looking for HD64461 shows it being used
>>> both with inw/outw and readw/writew, so there is no way to have
>>> the correct type. The sh __raw_readw() definition hides this bug,
>>> but that is a problem with arch/sh and it probably hides others
>>> as well.
>>
>> The constant HD64461_IOBASE is defined as integer at
>>
>>
>> https://elixir.bootlin.com/linux/latest/source/arch/sh/include/asm/hd64461.h#L17
>>
>> but fb_readw() expects a volatile-void pointer. I guess we could add a
>> cast somewhere to silence the problem. In the current upstream code,
>> that appears to be done by sh's __raw_readw() internally:
>>
>>
>> https://elixir.bootlin.com/linux/latest/source/arch/sh/include/asm/io.h#L35
>
> Sure, that would make it build again, but that still doesn't make the
> code correct, since it's completely unclear what base address the
> HD64461_IOBASE is relative to. The hp6xx platform code only passes it
> through inw()/outw(), which take an offset relative to sh_io_port_base,
> but that is not initialized on hp6xx. I tried to find in the history
> when it broke, apparently that was in 2007 commit 34a780a0afeb ("sh:
> hp6xx pata_platform support."), which removed the custom inw/outw
> implementations.
It just occured to me that these fb_read and fb_write calls are probably
all wrong. The fb_ interfaces are for framebuffer I/O memory. The driver
uses them to access the regular state registers. The writew() on sh is
definitely different. [1]
I assume that it only works because CONFIG_SWAP_IO_SPACE [2] is not set
in hp6xx_defconfig.
Best regards
Thomas
[1]
https://elixir.bootlin.com/linux/latest/source/arch/sh/include/asm/io.h#L55
[2]
https://elixir.bootlin.com/linux/latest/source/arch/sh/include/mach-common/mach/mangle-port.h#L22
>
> Arnd
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]
next prev parent reply other threads:[~2023-05-11 13:12 UTC|newest]
Thread overview: 120+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-10 11:05 [PATCH v6 0/6] fbdev: Move framebuffer I/O helpers to <asm/fb.h> Thomas Zimmermann
2023-05-10 11:05 ` Thomas Zimmermann
2023-05-10 11:05 ` Thomas Zimmermann
2023-05-10 11:05 ` Thomas Zimmermann
2023-05-10 11:05 ` [PATCH v6 1/6] fbdev/matrox: Remove trailing whitespaces Thomas Zimmermann
2023-05-10 11:05 ` Thomas Zimmermann
2023-05-10 11:05 ` Thomas Zimmermann
2023-05-10 11:05 ` Thomas Zimmermann
2023-05-10 18:20 ` Sui Jingfeng
2023-05-10 18:20 ` Sui Jingfeng
2023-05-10 18:20 ` Sui Jingfeng
2023-05-11 7:55 ` Thomas Zimmermann
2023-05-11 7:55 ` Thomas Zimmermann
2023-05-11 7:55 ` Thomas Zimmermann
2023-05-11 7:55 ` Thomas Zimmermann
2023-05-11 9:24 ` Sui Jingfeng
2023-05-11 9:24 ` Sui Jingfeng
2023-05-11 9:24 ` Sui Jingfeng
2023-05-11 13:05 ` Helge Deller
2023-05-11 13:05 ` Helge Deller
2023-05-11 13:05 ` Helge Deller
2023-05-11 13:05 ` Helge Deller
2023-05-11 13:10 ` Geert Uytterhoeven
2023-05-11 13:10 ` Geert Uytterhoeven
2023-05-11 13:10 ` Geert Uytterhoeven
2023-05-11 13:10 ` Geert Uytterhoeven
2023-05-11 16:23 ` Helge Deller
2023-05-11 16:23 ` Helge Deller
2023-05-11 16:23 ` Helge Deller
2023-05-11 16:23 ` Helge Deller
2023-05-11 14:27 ` Thomas Zimmermann
2023-05-11 14:27 ` Thomas Zimmermann
2023-05-11 14:27 ` Thomas Zimmermann
2023-05-11 14:27 ` Thomas Zimmermann
2023-05-11 17:02 ` Helge Deller
2023-05-11 17:02 ` Helge Deller
2023-05-11 17:02 ` Helge Deller
2023-05-11 17:02 ` Helge Deller
2023-05-12 7:14 ` Thomas Zimmermann
2023-05-12 7:14 ` Thomas Zimmermann
2023-05-12 7:14 ` Thomas Zimmermann
2023-05-12 7:14 ` Thomas Zimmermann
2023-05-12 10:04 ` Finn Thain
2023-05-12 10:04 ` Finn Thain
2023-05-12 10:04 ` Finn Thain
2023-05-12 10:04 ` Finn Thain
2023-05-10 11:05 ` [PATCH v6 2/6] ipu-v3: Include <linux/io.h> Thomas Zimmermann
2023-05-10 11:05 ` Thomas Zimmermann
2023-05-10 11:05 ` Thomas Zimmermann
2023-05-10 11:05 ` Thomas Zimmermann
2023-05-10 11:05 ` [PATCH v6 3/6] fbdev: Include <linux/io.h> in various drivers Thomas Zimmermann
2023-05-10 11:05 ` Thomas Zimmermann
2023-05-10 11:05 ` Thomas Zimmermann
2023-05-10 11:05 ` Thomas Zimmermann
2023-05-10 11:05 ` [PATCH v6 4/6] fbdev: Include <linux/fb.h> instead of <asm/fb.h> Thomas Zimmermann
2023-05-10 11:05 ` Thomas Zimmermann
2023-05-10 11:05 ` Thomas Zimmermann
2023-05-10 11:05 ` Thomas Zimmermann
2023-05-10 11:05 ` [PATCH v6 5/6] fbdev: Move framebuffer I/O helpers into <asm/fb.h> Thomas Zimmermann
2023-05-10 11:05 ` Thomas Zimmermann
2023-05-10 11:05 ` Thomas Zimmermann
2023-05-10 11:05 ` Thomas Zimmermann
2023-05-10 12:34 ` Geert Uytterhoeven
2023-05-10 12:34 ` Geert Uytterhoeven
2023-05-10 12:34 ` Geert Uytterhoeven
2023-05-10 12:34 ` Geert Uytterhoeven
2023-05-10 14:20 ` Thomas Zimmermann
2023-05-10 14:20 ` Thomas Zimmermann
2023-05-10 14:20 ` Thomas Zimmermann
2023-05-10 14:20 ` Thomas Zimmermann
2023-05-10 14:34 ` Geert Uytterhoeven
2023-05-10 14:34 ` Geert Uytterhoeven
2023-05-10 14:34 ` Geert Uytterhoeven
2023-05-10 14:34 ` Geert Uytterhoeven
2023-05-10 15:11 ` Thomas Zimmermann
2023-05-10 15:11 ` Thomas Zimmermann
2023-05-10 15:11 ` Thomas Zimmermann
2023-05-10 15:11 ` Thomas Zimmermann
2023-05-10 14:03 ` kernel test robot
2023-05-10 14:03 ` kernel test robot
2023-05-10 14:03 ` kernel test robot
2023-05-10 14:03 ` kernel test robot
2023-05-10 14:15 ` Arnd Bergmann
2023-05-10 14:15 ` Arnd Bergmann
2023-05-10 14:15 ` Arnd Bergmann
2023-05-10 14:15 ` Arnd Bergmann
2023-05-10 14:27 ` Thomas Zimmermann
2023-05-10 14:27 ` Thomas Zimmermann
2023-05-10 14:27 ` Thomas Zimmermann
2023-05-10 14:27 ` Thomas Zimmermann
2023-05-10 15:54 ` Arnd Bergmann
2023-05-10 15:54 ` Arnd Bergmann
2023-05-10 15:54 ` Arnd Bergmann
2023-05-10 15:54 ` Arnd Bergmann
2023-05-11 10:53 ` Thomas Zimmermann
2023-05-11 10:53 ` Thomas Zimmermann
2023-05-11 10:53 ` Thomas Zimmermann
2023-05-11 12:35 ` Geert Uytterhoeven
2023-05-11 12:35 ` Geert Uytterhoeven
2023-05-11 12:35 ` Geert Uytterhoeven
2023-05-11 12:35 ` Geert Uytterhoeven
2023-05-11 12:48 ` Arnd Bergmann
2023-05-11 12:48 ` Arnd Bergmann
2023-05-11 12:48 ` Arnd Bergmann
2023-05-11 12:48 ` Arnd Bergmann
2023-05-11 13:22 ` Artur Rojek
2023-05-11 13:22 ` Artur Rojek
2023-05-11 13:22 ` Artur Rojek
2023-05-11 13:22 ` Artur Rojek
2023-05-11 13:40 ` Arnd Bergmann
2023-05-11 13:40 ` Arnd Bergmann
2023-05-11 13:40 ` Arnd Bergmann
2023-05-11 13:40 ` Arnd Bergmann
2023-05-11 13:12 ` Thomas Zimmermann [this message]
2023-05-11 13:12 ` Thomas Zimmermann
2023-05-11 13:12 ` Thomas Zimmermann
2023-05-10 11:05 ` [PATCH v6 6/6] fbdev: Rename fb_mem*() helpers Thomas Zimmermann
2023-05-10 11:05 ` Thomas Zimmermann
2023-05-10 11:05 ` Thomas Zimmermann
2023-05-10 11:05 ` Thomas Zimmermann
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=cee24e80-cc41-00bc-06a2-37f1e2cad8ef@suse.de \
--to=tzimmermann@suse.de \
--cc=James.Bottomley@hansenpartnership.com \
--cc=arnd@arndb.de \
--cc=chenhuacai@kernel.org \
--cc=daniel@ffwll.ch \
--cc=davem@davemloft.net \
--cc=deller@gmx.de \
--cc=dri-devel@lists.freedesktop.org \
--cc=geert@linux-m68k.org \
--cc=javierm@redhat.com \
--cc=kernel@xen0n.name \
--cc=linux-arch@vger.kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-fbdev@vger.kernel.org \
--cc=linux-ia64@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-m68k@lists.linux-m68k.org \
--cc=linux-parisc@vger.kernel.org \
--cc=linux-snps-arc@lists.infradead.org \
--cc=lkp@intel.com \
--cc=loongarch@lists.linux.dev \
--cc=oe-kbuild-all@lists.linux.dev \
--cc=sam@ravnborg.org \
--cc=sparclinux@vger.kernel.org \
--cc=suijingfeng@loongson.cn \
--cc=vgupta@kernel.org \
/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.