Re: [PATCH v2 0/1] watchdog: realtek-otto: Make use of regmap API

Hello,
On 2026-06-23 20:44, Sander Vanheule wrote:
> On Tue, 2026-06-23 at 11:44 -0700, Guenter Roeck wrote:
>> On 6/20/26 04:23, Rustam Adilov wrote:
>> > Hello,
>> > 
>> > I have noticed the status for this patch series is "Changes Requested"
>> > in the patchwork site but the maintainer opposes the requested changes
>> > by Sander in [1], so what do i do here? It is just one the roadblocks
>> > to get USB stuff supported for Realtek chips.
>> > 
>> > I have already sent a reply week later in [2] but didn't get a response
>> > back. Maybe this email could be served as a gentle remainder.
>> > 
>> 
>> I said:
>> 	So this is now a NACK if unnecessary error handling is indeed added,
>> 	unless someone convinces me that this would add some benefits that I
>> 	am unable to see.
>> 
>> Your response did not explain the benefits of adding the unnecessary error
>> handling. I did not see a rationale from Sander either.
> 
> I mainly brought it up because I have been asked elsewhere [1] to check the
> regmap return codes. Although that was for a device on an MDIO bus, which I
> suppose is more likely to actually generate errors.
> 
> [1] https://lore.kernel.org/linux-leds/CAMRc=Mdb7CWB9PmzXJyfvGjvG0iwuwUgfLuKJuMweRFvAhAoHg@mail.gmail.com/

Thanks for clarification. I will take it as i don't need to add the error
handling.

As for why my response didn't explain it. I didn't want to speak for Sander so
i decided to just wait on it.

>> On top of that, your patch did not explain the real reason for the patch,
>> as stated in your reply. This means that, for me, it was just a patch making
>> no functional changes for no good reason other than to potentially _increase_
>> code complexity by adding unnecessary error handling while at the same time
>> claiming that code complexity would be reduced.

I can change the commit message to mention this main reason in the next patch
version.

> Given the reason is endianess issues, does the GPIO driver (gpio-realtek-otto.c)
> using ioread32()/iowrite32() still work correctly? If you have the wrong
> endianess there, you would only really see issues with the GPIO interrupt
> handling.
> 
> If GPIO works correctly with CONFIG_SWAP_IO_SPACE enabled, then I suppose the
> watchdog driver needs to be amended. Otherwise perhaps the USB peripheral driver
> should be compensating for its endianess?

Actually, it is other way around. GPIO works correctly when CONFIG_SWAP_IO_SPACE
is not enabled. When i do enable it, i need to patch the driver to make it work.
The dirty patch is here [1], which simply changes ioread32()/iowrite32() to
their __raw variants inside gpio_bank_read and gpio_bank_write the and what is
also important, the GPIO_GENERIC_BIG_ENDIAN_BYTE_ORDER flag needs to be set.
And also, i can't simply use the compatibles without GPIO_PORTS_REVERSED because
the realtek_gpio_line_imr_pos is required for correct functionality.

This patch obviously won't cut as it is going to break rtl9300 without SWAP_IO_SPACE.
Maybe we could make of gpio-regmap to handle swapping and stuff? I don't know of
any other elegant solutions to this problem.

[1] - https://github.com/jameywine/openwrt/blob/bb94712cb6faccf082c5a9fcebfabddf837a16bb/target/linux/realtek/patches-6.18/814-gpio-realtek-otto-change-read-write-functions.patch

>> Also, I do not recall even an attempt to address (or even comment on) the
>> actual problem with the driver as reported by Sashiko.
> 
> I did have a look at this report [2], but I didn't manage to figure out if the
> behavior I see matches the reasoning by the bot. Since I didn't get any
> feedback, I left it as is.
> 
> [2] https://lore.kernel.org/linux-watchdog/[email protected]/

Oh yes, thanks for bumping it back. I'll leave it to Guenter Roeck to leave a
feedback as i am not even nearly qualified to understand it.

> Best,
> Sander

Best,
Rustam