[PATCH] clocksource: rtl-otto: Change driver to use __raw reads and writes

[PATCH] clocksource: rtl-otto: Change driver to use __raw reads and writes

[Rustam Adilov]

As it stands, the driver uses ioread32 and iowrite32 for register
access and it works fine. However this stops working when the
SWAP_IO_SPACE config is enabled as this drivers expects ioread32 and
iowrite32 to be in native endian (that is big endian for currently
supported SoCs). RTL9607C is a big endian MIPS SoC that has identical
timer as the already supported chips but needs to have SWAP_IO_SPACE
to have a functioning little endian USB host.

Fix this by replacing all instances of ioread32 and iowrite32 with
__raw_readl and __raw_writel variants. Since they essentially do
the same register access, this shouldn't affect anything on other
machines.

Signed-off-by: Rustam Adilov <adilov@disroot.org>
---
 drivers/clocksource/timer-rtl-otto.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/clocksource/timer-rtl-otto.c b/drivers/clocksource/timer-rtl-otto.c
index 6113d2fdd4de..bcb449ef0481 100644
--- a/drivers/clocksource/timer-rtl-otto.c
+++ b/drivers/clocksource/timer-rtl-otto.c
@@ -56,37 +56,37 @@ struct rttm_cs {
 /* Simple internal register functions */
 static inline unsigned int rttm_get_counter(void __iomem *base)
 {
-	return ioread32(base + RTTM_CNT);
+	return __raw_readl(base + RTTM_CNT);
 }
 
 static inline void rttm_set_period(void __iomem *base, unsigned int period)
 {
-	iowrite32(period, base + RTTM_DATA);
+	__raw_writel(period, base + RTTM_DATA);
 }
 
 static inline void rttm_disable_timer(void __iomem *base)
 {
-	iowrite32(0, base + RTTM_CTRL);
+	__raw_writel(0, base + RTTM_CTRL);
 }
 
 static inline void rttm_enable_timer(void __iomem *base, u32 mode, u32 divisor)
 {
-	iowrite32(RTTM_CTRL_ENABLE | mode | divisor, base + RTTM_CTRL);
+	__raw_writel(RTTM_CTRL_ENABLE | mode | divisor, base + RTTM_CTRL);
 }
 
 static inline void rttm_ack_irq(void __iomem *base)
 {
-	iowrite32(ioread32(base + RTTM_INT) | RTTM_INT_PENDING, base + RTTM_INT);
+	__raw_writel(__raw_readl(base + RTTM_INT) | RTTM_INT_PENDING, base + RTTM_INT);
 }
 
 static inline void rttm_enable_irq(void __iomem *base)
 {
-	iowrite32(RTTM_INT_ENABLE, base + RTTM_INT);
+	__raw_writel(RTTM_INT_ENABLE, base + RTTM_INT);
 }
 
 static inline void rttm_disable_irq(void __iomem *base)
 {
-	iowrite32(0, base + RTTM_INT);
+	__raw_writel(0, base + RTTM_INT);
 }
 
 /* Aggregated control functions for kernel clock framework */
-- 
2.54.0

Re: [PATCH] clocksource: rtl-otto: Change driver to use __raw reads and writes

[Rustam Adilov]

On 2026-05-11 13:15, Rustam Adilov wrote:
> As it stands, the driver uses ioread32 and iowrite32 for register
> access and it works fine. However this stops working when the
> SWAP_IO_SPACE config is enabled as this drivers expects ioread32 and
> iowrite32 to be in native endian (that is big endian for currently
> supported SoCs). RTL9607C is a big endian MIPS SoC that has identical
> timer as the already supported chips but needs to have SWAP_IO_SPACE
> to have a functioning little endian USB host.
> 
> Fix this by replacing all instances of ioread32 and iowrite32 with
> __raw_readl and __raw_writel variants. Since they essentially do
> the same register access, this shouldn't affect anything on other
> machines.
> 
> Signed-off-by: Rustam Adilov <adilov@disroot.org>
> ---
>  drivers/clocksource/timer-rtl-otto.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/clocksource/timer-rtl-otto.c b/drivers/clocksource/timer-rtl-otto.c
> index 6113d2fdd4de..bcb449ef0481 100644
> --- a/drivers/clocksource/timer-rtl-otto.c
> +++ b/drivers/clocksource/timer-rtl-otto.c
> @@ -56,37 +56,37 @@ struct rttm_cs {
>  /* Simple internal register functions */
>  static inline unsigned int rttm_get_counter(void __iomem *base)
>  {
> -	return ioread32(base + RTTM_CNT);
> +	return __raw_readl(base + RTTM_CNT);
>  }
>  
>  static inline void rttm_set_period(void __iomem *base, unsigned int period)
>  {
> -	iowrite32(period, base + RTTM_DATA);
> +	__raw_writel(period, base + RTTM_DATA);
>  }
>  
>  static inline void rttm_disable_timer(void __iomem *base)
>  {
> -	iowrite32(0, base + RTTM_CTRL);
> +	__raw_writel(0, base + RTTM_CTRL);
>  }
>  
>  static inline void rttm_enable_timer(void __iomem *base, u32 mode, u32 divisor)
>  {
> -	iowrite32(RTTM_CTRL_ENABLE | mode | divisor, base + RTTM_CTRL);
> +	__raw_writel(RTTM_CTRL_ENABLE | mode | divisor, base + RTTM_CTRL);
>  }
>  
>  static inline void rttm_ack_irq(void __iomem *base)
>  {
> -	iowrite32(ioread32(base + RTTM_INT) | RTTM_INT_PENDING, base + RTTM_INT);
> +	__raw_writel(__raw_readl(base + RTTM_INT) | RTTM_INT_PENDING, base + RTTM_INT);
>  }
>  
>  static inline void rttm_enable_irq(void __iomem *base)
>  {
> -	iowrite32(RTTM_INT_ENABLE, base + RTTM_INT);
> +	__raw_writel(RTTM_INT_ENABLE, base + RTTM_INT);
>  }
>  
>  static inline void rttm_disable_irq(void __iomem *base)
>  {
> -	iowrite32(0, base + RTTM_INT);
> +	__raw_writel(0, base + RTTM_INT);
>  }
>  
>  /* Aggregated control functions for kernel clock framework */

It appears the get_maintainer.pl script didn't catch the Chris Packham, presumably
because i didn't touch the device bindings file. Added them to CC for review.

Re: [PATCH] clocksource: rtl-otto: Change driver to use __raw reads and writes

[Chris Packham]

Hi Rustam,

On 14/05/2026 06:31, Rustam Adilov wrote:
> On 2026-05-11 13:15, Rustam Adilov wrote:
>> As it stands, the driver uses ioread32 and iowrite32 for register
>> access and it works fine. However this stops working when the
>> SWAP_IO_SPACE config is enabled as this drivers expects ioread32 and
>> iowrite32 to be in native endian (that is big endian for currently
>> supported SoCs). RTL9607C is a big endian MIPS SoC that has identical
>> timer as the already supported chips but needs to have SWAP_IO_SPACE
>> to have a functioning little endian USB host.
>>
>> Fix this by replacing all instances of ioread32 and iowrite32 with
>> __raw_readl and __raw_writel variants. Since they essentially do
>> the same register access, this shouldn't affect anything on other
>> machines.
>>
>> Signed-off-by: Rustam Adilov <adilov@disroot.org>
>> ---
>>   drivers/clocksource/timer-rtl-otto.c | 14 +++++++-------
>>   1 file changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/clocksource/timer-rtl-otto.c b/drivers/clocksource/timer-rtl-otto.c
>> index 6113d2fdd4de..bcb449ef0481 100644
>> --- a/drivers/clocksource/timer-rtl-otto.c
>> +++ b/drivers/clocksource/timer-rtl-otto.c
>> @@ -56,37 +56,37 @@ struct rttm_cs {
>>   /* Simple internal register functions */
>>   static inline unsigned int rttm_get_counter(void __iomem *base)
>>   {
>> -	return ioread32(base + RTTM_CNT);
>> +	return __raw_readl(base + RTTM_CNT);
>>   }
>>   
>>   static inline void rttm_set_period(void __iomem *base, unsigned int period)
>>   {
>> -	iowrite32(period, base + RTTM_DATA);
>> +	__raw_writel(period, base + RTTM_DATA);
>>   }
>>   
>>   static inline void rttm_disable_timer(void __iomem *base)
>>   {
>> -	iowrite32(0, base + RTTM_CTRL);
>> +	__raw_writel(0, base + RTTM_CTRL);
>>   }
>>   
>>   static inline void rttm_enable_timer(void __iomem *base, u32 mode, u32 divisor)
>>   {
>> -	iowrite32(RTTM_CTRL_ENABLE | mode | divisor, base + RTTM_CTRL);
>> +	__raw_writel(RTTM_CTRL_ENABLE | mode | divisor, base + RTTM_CTRL);
>>   }
>>   
>>   static inline void rttm_ack_irq(void __iomem *base)
>>   {
>> -	iowrite32(ioread32(base + RTTM_INT) | RTTM_INT_PENDING, base + RTTM_INT);
>> +	__raw_writel(__raw_readl(base + RTTM_INT) | RTTM_INT_PENDING, base + RTTM_INT);
>>   }
>>   
>>   static inline void rttm_enable_irq(void __iomem *base)
>>   {
>> -	iowrite32(RTTM_INT_ENABLE, base + RTTM_INT);
>> +	__raw_writel(RTTM_INT_ENABLE, base + RTTM_INT);
>>   }
>>   
>>   static inline void rttm_disable_irq(void __iomem *base)
>>   {
>> -	iowrite32(0, base + RTTM_INT);
>> +	__raw_writel(0, base + RTTM_INT);
>>   }
>>   
>>   /* Aggregated control functions for kernel clock framework */
> It appears the get_maintainer.pl script didn't catch the Chris Packham, presumably
> because i didn't touch the device bindings file. Added them to CC for review.

No problem.  Has --git been dropped by default from get_maintainer.pl? 
It doesn't actually help as even when explicitly using it that doesn't 
seem to get me (initial commit ignored?), it does find Markus who I've 
added to the Cc.

Reviewed-by: Chris Packham <chris.packham@alliedtelesis.co.nz>

AW: [PATCH] clocksource: rtl-otto: Change driver to use __raw reads and writes

[markus.stockhausen]

> Von: Chris Packham <Chris.Packham@alliedtelesis.co.nz> 
> Gesendet: Mittwoch, 13. Mai 2026 23:05
> Betreff: Re: [PATCH] clocksource: rtl-otto: Change driver to use __raw reads and writes
> ...
>> Fix this by replacing all instances of ioread32 and iowrite32 with
>> __raw_readl and __raw_writel variants. Since they essentially do
>> the same register access, this shouldn't affect anything on other
>> machines.
>>
>> Signed-off-by: Rustam Adilov <adilov@disroot.org>

IIRC the current functions use memory barriers while the
new ones do not. Timers are critical and cost me a lot
of time in the past [1]. We nearly lost support for several
devices last year. So I like to have at least some stability 
confirmation from downstream.

Markus

[1] https://lkml.org/lkml/2025/8/4/240

Re: AW: [PATCH] clocksource: rtl-otto: Change driver to use __raw reads and writes

[Rustam Adilov]

Hi Markus,
On 2026-05-14 20:50, markus.stockhausen@gmx.de wrote:
>> Von: Chris Packham <Chris.Packham@alliedtelesis.co.nz> 
>> Gesendet: Mittwoch, 13. Mai 2026 23:05
>> Betreff: Re: [PATCH] clocksource: rtl-otto: Change driver to use __raw reads and writes
>> ...
>>> Fix this by replacing all instances of ioread32 and iowrite32 with
>>> __raw_readl and __raw_writel variants. Since they essentially do
>>> the same register access, this shouldn't affect anything on other
>>> machines.
>>>
>>> Signed-off-by: Rustam Adilov <adilov@disroot.org>
> 
> IIRC the current functions use memory barriers while the
> new ones do not. Timers are critical and cost me a lot
> of time in the past [1]. We nearly lost support for several
> devices last year. So I like to have at least some stability 
> confirmation from downstream.

As far as i can see, if you untangle the the BUILDIO_MEM(l, u32) in [1]
you should see they do have barriers as denoted by barrier argument being set.
So that means, in MIPS, both readl/writel are the same as their __raw variants.
And both ioread32/iowrite32 are using readl/writel under the hood so...

[1] - https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/mips/include/asm/io.h

> Markus
> 
> [1] https://lkml.org/lkml/2025/8/4/240

Best,
Rustam

AW: AW: [PATCH] clocksource: rtl-otto: Change driver to use __raw reads and writes

[markus.stockhausen]

> Von: Rustam Adilov <adilov@disroot.org> 
> Gesendet: Freitag, 15. Mai 2026 16:33
> An: markus.stockhausen@gmx.de
> Betreff: Re: AW: [PATCH] clocksource: rtl-otto: Change driver to use __raw
reads and writes
> ...
> > IIRC the current functions use memory barriers while the
> > new ones do not. Timers are critical and cost me a lot
> > of time in the past [1]. We nearly lost support for several
> > devices last year. So I like to have at least some stability 
> > confirmation from downstream.
>
> As far as i can see, if you untangle the the BUILDIO_MEM(l, u32) in [1]
> you should see they do have barriers as denoted by barrier argument being
set.
> So that means, in MIPS, both readl/writel are the same as their __raw
variants.
> And both ioread32/iowrite32 are using readl/writel under the hood so...

Hi,

will the __relaxed functions serve your IO_SWAP scenario too?

Markus

Re: AW: AW: [PATCH] clocksource: rtl-otto: Change driver to use __raw reads and writes

[Rustam Adilov]

On 2026-05-15 17:18, markus.stockhausen@gmx.de wrote:
>> Von: Rustam Adilov <adilov@disroot.org> 
>> Gesendet: Freitag, 15. Mai 2026 16:33
>> An: markus.stockhausen@gmx.de
>> Betreff: Re: AW: [PATCH] clocksource: rtl-otto: Change driver to use __raw
> reads and writes
>> ...
>> > IIRC the current functions use memory barriers while the
>> > new ones do not. Timers are critical and cost me a lot
>> > of time in the past [1]. We nearly lost support for several
>> > devices last year. So I like to have at least some stability 
>> > confirmation from downstream.
>>
>> As far as i can see, if you untangle the the BUILDIO_MEM(l, u32) in [1]
>> you should see they do have barriers as denoted by barrier argument being
> set.
>> So that means, in MIPS, both readl/writel are the same as their __raw
> variants.
>> And both ioread32/iowrite32 are using readl/writel under the hood so...
> 
> Hi,
> 
> will the __relaxed functions serve your IO_SWAP scenario too?
> 
> Markus

Not really because the readl_relaxed and readl appear to be both mapped to ioswabl
which changes behavior as per IO_SWAP config [1].

[1] - https://elixir.bootlin.com/linux/v7.0.6/source/arch/mips/include/asm/io.h#L49

AW: AW: AW: [PATCH] clocksource: rtl-otto: Change driver to use __raw reads and writes

[markus.stockhausen]

> Von: Rustam Adilov <adilov@disroot.org> 
> Gesendet: Freitag, 15. Mai 2026 20:21
> An: markus.stockhausen@gmx.de
> Betreff: Re: AW: AW: [PATCH] clocksource: rtl-otto: Change driver to use
__raw reads and writes
>
> On 2026-05-15 17:18, markus.stockhausen@gmx.de wrote:
> >> Von: Rustam Adilov <adilov@disroot.org> 
> >> Gesendet: Freitag, 15. Mai 2026 16:33
> >> An: markus.stockhausen@gmx.de
> >> Betreff: Re: AW: [PATCH] clocksource: rtl-otto: Change driver to use
__raw
> > reads and writes
> >> ...
> >> > IIRC the current functions use memory barriers while the
> >> > new ones do not. Timers are critical and cost me a lot
> >> > of time in the past [1]. We nearly lost support for several
> >> > devices last year. So I like to have at least some stability 
> >> > confirmation from downstream.
> >>
> >> As far as i can see, if you untangle the the BUILDIO_MEM(l, u32) in [1]
> >> you should see they do have barriers as denoted by barrier argument
being
> > set.
> >> So that means, in MIPS, both readl/writel are the same as their __raw
> > variants.
> >> And both ioread32/iowrite32 are using readl/writel under the hood so...
> > 
> > Hi,
> > 
> > will the __relaxed functions serve your IO_SWAP scenario too?
> > 
> > Markus
>
> Not really because the readl_relaxed and readl appear to be both mapped to
ioswabl
> which changes behavior as per IO_SWAP config [1].

My two cents regarding the real issue behind this discussion. 
I'm still convinced that CONFIG_SWAP_IO_SPACE is not the
right way to get an USB driver working. Rewriting every other
driver around that just feels wrong.

Coming back to this. If it produces the same assembler code
for the big endian Realtek Otto platform then I'm ok with this.
I expect an immediate downstream backport if it gets
accepted upstream.

Markus 

Re: AW: AW: AW: [PATCH] clocksource: rtl-otto: Change driver to use __raw reads and writes

[Rustam Adilov]

On 2026-05-15 18:39, markus.stockhausen@gmx.de wrote:
>> Von: Rustam Adilov <adilov@disroot.org> 
>> Gesendet: Freitag, 15. Mai 2026 20:21
>> An: markus.stockhausen@gmx.de
>> Betreff: Re: AW: AW: [PATCH] clocksource: rtl-otto: Change driver to use
> __raw reads and writes
>>
>> On 2026-05-15 17:18, markus.stockhausen@gmx.de wrote:
>> >> Von: Rustam Adilov <adilov@disroot.org> 
>> >> Gesendet: Freitag, 15. Mai 2026 16:33
>> >> An: markus.stockhausen@gmx.de
>> >> Betreff: Re: AW: [PATCH] clocksource: rtl-otto: Change driver to use
> __raw
>> > reads and writes
>> >> ...
>> >> > IIRC the current functions use memory barriers while the
>> >> > new ones do not. Timers are critical and cost me a lot
>> >> > of time in the past [1]. We nearly lost support for several
>> >> > devices last year. So I like to have at least some stability 
>> >> > confirmation from downstream.
>> >>
>> >> As far as i can see, if you untangle the the BUILDIO_MEM(l, u32) in [1]
>> >> you should see they do have barriers as denoted by barrier argument
> being
>> > set.
>> >> So that means, in MIPS, both readl/writel are the same as their __raw
>> > variants.
>> >> And both ioread32/iowrite32 are using readl/writel under the hood so...
>> > 
>> > Hi,
>> > 
>> > will the __relaxed functions serve your IO_SWAP scenario too?
>> > 
>> > Markus
>>
>> Not really because the readl_relaxed and readl appear to be both mapped to
> ioswabl
>> which changes behavior as per IO_SWAP config [1].
> 
> My two cents regarding the real issue behind this discussion. 
> I'm still convinced that CONFIG_SWAP_IO_SPACE is not the
> right way to get an USB driver working. Rewriting every other
> driver around that just feels wrong.

I mean, it is either this or hacking up the general usb reads and writes
functions with this atrociousness:

 static inline unsigned int ehci_readl(const struct ehci_hcd *ehci,
 		__u32 __iomem *regs)
 {
+#if defined(CONFIG_RTK_MIPS_SOC)
+	return (le32_to_cpu((*(volatile unsigned long *)(regs))));
+#else
 #ifdef CONFIG_USB_EHCI_BIG_ENDIAN_MMIO
 	return ehci_big_endian_mmio(ehci) ?
 		readl_be(regs) :
@@ -746,6 +749,7 @@
 #else
 	return readl(regs);
 #endif
+#endif
 }

 static inline void ehci_writel(const struct ehci_hcd *ehci,
 		const unsigned int val, __u32 __iomem *regs)
 {
+#if defined(CONFIG_RTK_MIPS_SOC)
+	((*(volatile unsigned long *)(regs))=cpu_to_le32(val));
+#else
 #ifdef CONFIG_USB_EHCI_BIG_ENDIAN_MMIO
 	ehci_big_endian_mmio(ehci) ?
 		writel_be(val, regs) :
@@ -773,6 +780,7 @@
 	else
 		writel(val, regs);
 #endif
+#endif
 }

And i don't think anyone upstream is gonna be okay with it.
Obviously that's straight from Realtek code and we can slightly
improve it but the underlying issue is still gonna be there.

With all that, I believe CONFIG_SWAP_IO_SPACE is a good long term
solution to this endian mess and multiple big endian MIPS SoCs use
it too like Lantiq and Broadcom. The happy side effect is rewriting
existing drivers, which i'll call as a good thing personally.

> Coming back to this. If it produces the same assembler code
> for the big endian Realtek Otto platform then I'm ok with this.
> I expect an immediate downstream backport if it gets
> accepted upstream.
> 
> Markus

Best,
Rustam