[PATCH] mips: Realtek RTL: select NO_EXCEPT_FILL

[PATCH] mips: Realtek RTL: select NO_EXCEPT_FILL

[Sander Vanheule]

The CPUs in these SoCs support MIPS32 R2, and allow ebase relocation.
Even if the default exception base of 0x80000000 is used, the
MIPS_GENERIC load address of 0x80100000 leaves sufficient space to not
need an extra 0x400 bytes of padding.

Suggested-by: Olliver Schinagl <oliver@schinagl.nl>
Signed-off-by: Sander Vanheule <sander@svanheule.net>
---
Olliver has suggested to make this change, in order to reduce the delta
with a fully generic MIPS kernel.
I hope the patch description makes sense, as I based the argumentation
on the behaviour of the code, and similar commits 7d6d28377783 ("MIPS:
Loongson64: select NO_EXCEPT_FILL") and dd54dedd947d ("MIPS: BCM47XX:
select NO_EXCEPT_FILL"). The change was tested on an RTL8380 and an
RTL8393 device, where it appears to work as expected.

Best,
Sander

 arch/mips/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 2db5c853992e..a8895aaa490e 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -627,6 +627,7 @@ config MACH_REALTEK_RTL
 	select IRQ_MIPS_CPU
 	select CEVT_R4K
 	select CSRC_R4K
+	select NO_EXCEPT_FILL
 	select SYS_HAS_CPU_MIPS32_R1
 	select SYS_HAS_CPU_MIPS32_R2
 	select SYS_SUPPORTS_32BIT_KERNEL
-- 
2.39.0

Re: [PATCH] mips: Realtek RTL: select NO_EXCEPT_FILL

[Thomas Bogendoerfer]

On Sun, Jan 15, 2023 at 01:19:22PM +0100, Sander Vanheule wrote:
> The CPUs in these SoCs support MIPS32 R2, and allow ebase relocation.
> Even if the default exception base of 0x80000000 is used, the
> MIPS_GENERIC load address of 0x80100000 leaves sufficient space to not
> need an extra 0x400 bytes of padding.
> 
> Suggested-by: Olliver Schinagl <oliver@schinagl.nl>
> Signed-off-by: Sander Vanheule <sander@svanheule.net>
> ---
> Olliver has suggested to make this change, in order to reduce the delta
> with a fully generic MIPS kernel.
> I hope the patch description makes sense, as I based the argumentation
> on the behaviour of the code, and similar commits 7d6d28377783 ("MIPS:
> Loongson64: select NO_EXCEPT_FILL") and dd54dedd947d ("MIPS: BCM47XX:
> select NO_EXCEPT_FILL"). The change was tested on an RTL8380 and an
> RTL8393 device, where it appears to work as expected.
> 
> Best,
> Sander
> 
>  arch/mips/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> index 2db5c853992e..a8895aaa490e 100644
> --- a/arch/mips/Kconfig
> +++ b/arch/mips/Kconfig
> @@ -627,6 +627,7 @@ config MACH_REALTEK_RTL
>  	select IRQ_MIPS_CPU
>  	select CEVT_R4K
>  	select CSRC_R4K
> +	select NO_EXCEPT_FILL
>  	select SYS_HAS_CPU_MIPS32_R1
>  	select SYS_HAS_CPU_MIPS32_R2
>  	select SYS_SUPPORTS_32BIT_KERNEL
> -- 
> 2.39.0

applied to mips-next.

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]

Re: [PATCH] mips: Realtek RTL: select NO_EXCEPT_FILL

[Olliver Schinagl]

Hey Thomas,

On 27-01-2023 17:27, Thomas Bogendoerfer wrote:
> On Sun, Jan 15, 2023 at 01:19:22PM +0100, Sander Vanheule wrote:
>> Olliver has suggested to make this change, in order to reduce the delta
>> with a fully generic MIPS kernel.
>> <snip>
> applied to mips-next.
>
> Thomas.
>
Thanks Thomas, may I take this moment to poke you about a nother issue I 
don't quite understand how to resolve/figure out.


I made the realtek target work with the GENERIC_MIPS_KERNEL, but only if 
I disable SWAP_IO_SPACE[0].

While I could act all smart and ask what this is for, I better ask, why 
is this concidered 'generic'. The comment in mangle-port [1] mentions 
'sane hardware'. I don't know what is considered sane here, as the 
number of targets following generic mips seem to be around 5.

So any pointers (other then doing SWAP_IO_SPACE if !REALTEK_SOC, which I 
have now) would be appreciated


[0]: https://elixir.bootlin.com/linux/latest/source/arch/mips/Kconfig#L157

[1]: 
https://elixir.bootlin.com/linux/latest/source/arch/mips/include/asm/mach-generic/mangle-port.h#L17

Re: [PATCH] mips: Realtek RTL: select NO_EXCEPT_FILL

[Thomas Bogendoerfer]

On Sat, Jan 28, 2023 at 03:39:59PM +0100, Olliver Schinagl wrote:
> Thanks Thomas, may I take this moment to poke you about a nother issue I
> don't quite understand how to resolve/figure out.
> 
> 
> I made the realtek target work with the GENERIC_MIPS_KERNEL, but only if I
> disable SWAP_IO_SPACE[0].

is this little endian or big endian ? 

> While I could act all smart and ask what this is for, I better ask, why is
> this concidered 'generic'. The comment in mangle-port [1] mentions 'sane
> hardware'. I don't know what is considered sane here, as the number of
> targets following generic mips seem to be around 5.

I always thought that SWAP_IO_SPACE is needed for big endian, but
looking at arch/mips/Kconfig I see a lot of SWAP_IO_SPACE for pure
little endian machines. I need to dig deeper to understand why.

> So any pointers (other then doing SWAP_IO_SPACE if !REALTEK_SOC, which I
> have now) would be appreciated

such a change would defeat the generic part of GENERIC_MIPS_KERNEL,
because it will then only work on REALTEC_SOC and nothing else. 

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]

Re: [PATCH] mips: Realtek RTL: select NO_EXCEPT_FILL

[Olliver Schinagl]

Hey Thomas,

On 05-02-2023 17:19, Thomas Bogendoerfer wrote:
> On Sat, Jan 28, 2023 at 03:39:59PM +0100, Olliver Schinagl wrote:
>> Thanks Thomas, may I take this moment to poke you about a nother issue I
>> don't quite understand how to resolve/figure out.
>>
>>
>> I made the realtek target work with the GENERIC_MIPS_KERNEL, but only if I
>> disable SWAP_IO_SPACE[0].
> 
> is this little endian or big endian ?
The kernel config currently states Big Endian

> 
>> While I could act all smart and ask what this is for, I better ask, why is
>> this concidered 'generic'. The comment in mangle-port [1] mentions 'sane
>> hardware'. I don't know what is considered sane here, as the number of
>> targets following generic mips seem to be around 5.
> 
> I always thought that SWAP_IO_SPACE is needed for big endian, but
> looking at arch/mips/Kconfig I see a lot of SWAP_IO_SPACE for pure
> little endian machines. I need to dig deeper to understand why.
Thank you!

> 
>> So any pointers (other then doing SWAP_IO_SPACE if !REALTEK_SOC, which I
>> have now) would be appreciated
> 
> such a change would defeat the generic part of GENERIC_MIPS_KERNEL,
> because it will then only work on REALTEC_SOC and nothing else.
Ignoring the potential incorrect detail here, obviously I would prefer 
to use the GENERIC_MIPS_KERNEL, but having to copy the whole config just 
to leave out that one option also is kinda meh. So i'm hoping we can 
find a solution of course :)

Olliver

> 
> Thomas.
> 

Re: [PATCH] mips: Realtek RTL: select NO_EXCEPT_FILL

[Thomas Bogendoerfer]

On Sun, Feb 05, 2023 at 07:33:46PM +0100, Olliver Schinagl wrote:
> > I always thought that SWAP_IO_SPACE is needed for big endian, but
> > looking at arch/mips/Kconfig I see a lot of SWAP_IO_SPACE for pure
> > little endian machines. I need to dig deeper to understand why.
> Thank you!

and the reason why this works is simple, CONFIG_SWAP_IO_SPACE is a nop
for little endian.

> > such a change would defeat the generic part of GENERIC_MIPS_KERNEL,
> > because it will then only work on REALTEC_SOC and nothing else.
> Ignoring the potential incorrect detail here, obviously I would prefer to
> use the GENERIC_MIPS_KERNEL, but having to copy the whole config just to
> leave out that one option also is kinda meh. So i'm hoping we can find a
> solution of course :)

what SOC is this exactly ? Do you have a programmers manual for it,
which contains details about the PCI bridge ? Most of the PCI bridges
used for MIPS contain a bit to select the endianess of the access
to PCI spaces. If there is such a config just changing the current
setting, should solve your problem.

If this isn't possible you could use something similair to the
INGENIC approach, which could use a generic kernel but also an
customized kernel. And in such a customized kernel it should be
possible to remove SWAP_IO_SPACE.

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]

Re: [PATCH] mips: Realtek RTL: select NO_EXCEPT_FILL

[Olliver Schinagl]

Hey Thomas,

On 17-02-2023 18:37, Thomas Bogendoerfer wrote:
> On Sun, Feb 05, 2023 at 07:33:46PM +0100, Olliver Schinagl wrote:
>>> I always thought that SWAP_IO_SPACE is needed for big endian, but
>>> looking at arch/mips/Kconfig I see a lot of SWAP_IO_SPACE for pure
>>> little endian machines. I need to dig deeper to understand why.
>> Thank you!
> 
> and the reason why this works is simple, CONFIG_SWAP_IO_SPACE is a nop
> for little endian.

That makes life easy :p

> 
>>> such a change would defeat the generic part of GENERIC_MIPS_KERNEL,
>>> because it will then only work on REALTEC_SOC and nothing else.
>> Ignoring the potential incorrect detail here, obviously I would prefer to
>> use the GENERIC_MIPS_KERNEL, but having to copy the whole config just to
>> leave out that one option also is kinda meh. So i'm hoping we can find a
>> solution of course :)
> 
> what SOC is this exactly ?

It is the Realtek series of SoC, specifically in my case, the RTL9302b, 
which doesn't have a PCI peripherial, at all :) Nor configured in the 
devicetree, but of course generic_mips_kernel enables the drivers, which 
should be noop. I don't see anything related to PCI during boot.

Having said that, the RTL930x series take their heritage from the 
RTL819x (and probably older) wifi SoC series from realtek, which did 
contain a PCIe peripherial, as that is where the (external) wifi chip 
was connected too.

> Do you have a programmers manual for it,
> which contains details about the PCI bridge ?

If only. There are some (leaked) datasheets, that do contain the PCI 
registers, mostly (obviously) the rtl819x datasheets. 
https://github.com/libc0607/Realtek_switch_hacking/blob/files/REALTEK-RTL8196E.pdf 
is one such example that contains the PCIe registers.

> Most of the PCI bridges
> used for MIPS contain a bit to select the endianess of the access
> to PCI spaces. If there is such a config just changing the current
> setting, should solve your problem.

I really want to solve this obviously, but without the PCI registers ... 
It could be that those registers DO exist of course!

I'll double check tomorrow, where abouts booting fails, what driver is 
the last to output anything to give a hint to where things are failing...

> 
> If this isn't possible you could use something similair to the
> INGENIC approach, which could use a generic kernel but also an
> customized kernel. And in such a customized kernel it should be
> possible to remove SWAP_IO_SPACE.

Yeah, I just disabled that flag; I'll look into INGENIC in more detail, 
but would hope we can do this in a clean way, because without the flag, 
generic_mips_kernel works fine ...

Olliver

> 
> Thomas.
> 

Re: [PATCH] mips: Realtek RTL: select NO_EXCEPT_FILL

[Thomas Bogendoerfer]

On Fri, Feb 17, 2023 at 08:27:05PM +0100, Olliver Schinagl wrote:
> It is the Realtek series of SoC, specifically in my case, the RTL9302b,
> which doesn't have a PCI peripherial, at all :) Nor configured in the
> devicetree, but of course generic_mips_kernel enables the drivers, which
> should be noop. I don't see anything related to PCI during boot.
> 
> Having said that, the RTL930x series take their heritage from the RTL819x
> (and probably older) wifi SoC series from realtek, which did contain a PCIe
> peripherial, as that is where the (external) wifi chip was connected too.

I see, and you want to use the already existing wifi driver ?

> > Do you have a programmers manual for it,
> > which contains details about the PCI bridge ?
> 
> If only. There are some (leaked) datasheets, that do contain the PCI
> registers, mostly (obviously) the rtl819x datasheets. https://github.com/libc0607/Realtek_switch_hacking/blob/files/REALTEK-RTL8196E.pdf
> is one such example that contains the PCIe registers.

that's just the PCIe PHY part, but not the localbus<->PCIe bridge
part. But if the wifi part of ypur SOC is directly connected to
the localbus, having the some PCI register won't help anyway.
I am afraid you need to go the route via extra Kconfig section.

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]

Re: [PATCH] mips: Realtek RTL: select NO_EXCEPT_FILL

[Olliver Schinagl]

On 19-02-2023 10:02, Thomas Bogendoerfer wrote:
> On Fri, Feb 17, 2023 at 08:27:05PM +0100, Olliver Schinagl wrote:
>> It is the Realtek series of SoC, specifically in my case, the RTL9302b,
>> which doesn't have a PCI peripherial, at all :) Nor configured in the
>> devicetree, but of course generic_mips_kernel enables the drivers, which
>> should be noop. I don't see anything related to PCI during boot.
>>
>> Having said that, the RTL930x series take their heritage from the RTL819x
>> (and probably older) wifi SoC series from realtek, which did contain a PCIe
>> peripherial, as that is where the (external) wifi chip was connected too.
> I see, and you want to use the already existing wifi driver ?
Not at all, the rtl930x is a pure switch chip (10 serdes connecting to 
multiple PHY's yielding 24 - 48 (+ some SFP port) configurations, all 
without anything PCI related.
>
>>> Do you have a programmers manual for it,
>>> which contains details about the PCI bridge ?
>> If only. There are some (leaked) datasheets, that do contain the PCI
>> registers, mostly (obviously) the rtl819x datasheets. https://github.com/libc0607/Realtek_switch_hacking/blob/files/REALTEK-RTL8196E.pdf
>> is one such example that contains the PCIe registers.
> that's just the PCIe PHY part, but not the localbus<->PCIe bridge
> part. But if the wifi part of ypur SOC is directly connected to
> the localbus, having the some PCI register won't help anyway.
> I am afraid you need to go the route via extra Kconfig section.

It's still odd though; as we do not have _anything_ PCI, but it 
SWAP_IO_SPACE causes the crash.

So in that case, we'll wend up with a duplicate of generic_mips_kernel - 
SWAP_IO_SPACE, any suggestions of doing this cleanly in kconfig? copy 
pasting the whole shebang (and maintaining it) seems wasteful.

I suppose the chip is generic, except one thing makes it not generic :(


What makes SWAP_IO_SPACE generic then? :)

>
> Thomas.
>

Re: [PATCH] mips: Realtek RTL: select NO_EXCEPT_FILL

[Thomas Bogendoerfer]

On Sun, Feb 19, 2023 at 10:27:17AM +0100, Olliver Schinagl wrote:
> It's still odd though; as we do not have _anything_ PCI, but it
> SWAP_IO_SPACE causes the crash.

but something uses readX/write() calls. If you aren't using any driver
existing driver but only newly written dedicated for that SOC
you could use raw_read/raw_writeX() instead. These type of functions
are always using native endianess.

> What makes SWAP_IO_SPACE generic then? :)

als long as hardware presents memory used with readX/writeX is
seen as little endian independant from CPU endianess it's generic.

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]

Re: [PATCH] mips: Realtek RTL: select NO_EXCEPT_FILL

[Olliver Schinagl]

Hey Thomas,

On 19-02-2023 11:31, Thomas Bogendoerfer wrote:
> On Sun, Feb 19, 2023 at 10:27:17AM +0100, Olliver Schinagl wrote:
>> It's still odd though; as we do not have _anything_ PCI, but it
>> SWAP_IO_SPACE causes the crash.
> 
> but something uses readX/write() calls. If you aren't using any driver
> existing driver but only newly written dedicated for that SOC
> you could use raw_read/raw_writeX() instead. These type of functions
> are always using native endianess.

Ok that is valueable information. I think currently the only driver 
(that I can think of right now) that we use 'off the shelf' is the uart 
driver, so I hope that's written cleanly :)

As for readX/writeX, for sure that is being used, and I intended to 
refactor that while going to proper and clean drivers (the realtek 
support in openwrt is a big mess right now).

So you say raw_readX, but what about ioread32 which I thought was preferred?

I'll meanwhile read into what readX vs raw_readX to learn more with 
regards to endianess.

Thank you so mcuh so far!

> 
>> What makes SWAP_IO_SPACE generic then? :)
> 
> als long as hardware presents memory used with readX/writeX is
> seen as little endian independant from CPU endianess it's generic.

'little endian independent'? What does that mean, that the register maps 
follow the CPU endianess?

> 
> Thomas.
> 

Re: [PATCH] mips: Realtek RTL: select NO_EXCEPT_FILL

[Olliver Schinagl]

Hey Thomas,

I'm digging deeper into this and understand less of it :)

For one, we currently use both ioread32 (our timer driver for example) 
and ioread32be (our GPIO driver) and thus both variants of register 
access I would think without problems.

I also see that a few devices override? (is that possible at all) 
mangle-port.h, though I didn't dig into this too much, just noticed some 
other boards with those headers. Is this an option at all? just supply a 
SoC specific mangle-port.h?

Loonking at mangle-port.h (and only at the 32 bit case), basically, 
there's ioswabl and __mem_ioswabl that I should be concerned about. 
Grepping for those functions however, only yields me one result, where  
__relaxed_ioswabl gets defined to  __ioswabl, __mem_iosawbl is unused. 
Even more frustrating, is that git grep also doesn't find a user for 
__relaxed_ioswabl. I'm sure there's some non-greppable magic missing :) 
as the failures of setting this define of course are real.

So to further my search, how does this work then?

Olliver

On 19-02-2023 17:07, Olliver Schinagl wrote:
> Hey Thomas,
>
> On 19-02-2023 11:31, Thomas Bogendoerfer wrote:
>> On Sun, Feb 19, 2023 at 10:27:17AM +0100, Olliver Schinagl wrote:
>>> It's still odd though; as we do not have _anything_ PCI, but it
>>> SWAP_IO_SPACE causes the crash.
>> but something uses readX/write() calls. If you aren't using any driver
>> existing driver but only newly written dedicated for that SOC
>> you could use raw_read/raw_writeX() instead. These type of functions
>> are always using native endianess.
> Ok that is valueable information. I think currently the only driver
> (that I can think of right now) that we use 'off the shelf' is the uart
> driver, so I hope that's written cleanly :)
>
> As for readX/writeX, for sure that is being used, and I intended to
> refactor that while going to proper and clean drivers (the realtek
> support in openwrt is a big mess right now).
>
> So you say raw_readX, but what about ioread32 which I thought was preferred?
>
> I'll meanwhile read into what readX vs raw_readX to learn more with
> regards to endianess.
>
> Thank you so mcuh so far!
>
>>> What makes SWAP_IO_SPACE generic then? :)
>> als long as hardware presents memory used with readX/writeX is
>> seen as little endian independant from CPU endianess it's generic.
> 'little endian independent'? What does that mean, that the register maps
> follow the CPU endianess?
>
>> Thomas.
>>