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

[Rustam Adilov <adilov@disroot.org>]

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