Re: [PATCH 1/2] gpio: realtek-otto: use __raw_readl/writel in realtek_gpio_update_line_imr()

[Rustam Adilov]

Hello,
On 2026-07-20 16:32, Sander Vanheule wrote:
> Hi,
> 
> Adding linux-mips as SWAP_IO_SPACE is mostly a MIPS thing, and some lists/people
> for your other pending patches.

I think linux-mips would have been enough but that's fine.

> On Fri, 2026-07-10 at 23:34 +0500, Rustam Adilov wrote:
>> In preparation for upcoming changes to how bank reads and writes
>> are defined in this driver, change the ioread32 and iowrite32 to
>> their __raw variants. The realtek_gpio_update_line_imr() function
>> is used by all devices regardless of GPIO_PORTS_REVERSED flag and
>> thus this is the only place where there shouldn't be any byte
>> swapping whether SWAP_IO_SPACE config is enabled or not and that
>> is only possible with __raw_readl and __raw_writel.
>> 
>> Signed-off-by: Rustam Adilov <adilov@xxxxxxxxxxx>
>> ---
>>  drivers/gpio/gpio-realtek-otto.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/gpio/gpio-realtek-otto.c b/drivers/gpio/gpio-realtek-
>> otto.c
>> index 4a606bad5848..491fde846d46 100644
>> --- a/drivers/gpio/gpio-realtek-otto.c
>> +++ b/drivers/gpio/gpio-realtek-otto.c
>> @@ -176,10 +176,10 @@ static void realtek_gpio_update_line_imr(struct
>> realtek_gpio_ctrl *ctrl, unsigne
>>  	u32 reg_val;
>>  
>>  	reg += 4 * (line_shift / 32);
>> -	reg_val = ioread32(reg);
>> +	reg_val = __raw_readl(reg);
>>  	reg_val &= ~(REALTEK_GPIO_IMR_LINE_MASK << shift);
>>  	reg_val |= (irq_type & irq_mask & REALTEK_GPIO_IMR_LINE_MASK) <<
>> shift;
>> -	iowrite32(reg_val, reg);
>> +	__raw_writel(reg_val, reg);
>>  }
>>  
>>  static void realtek_gpio_irq_ack(struct irq_data *data)
> 
> As per my earlier message on your watchdog patch [1], I'm still not convinced
> converting all existing drivers [2, 3] to work around an issue with the USB
> framework is the right thing to do. You claim the required USB changes are not
> upstreamable, but I have not seen you make an attempt at doing so.

Yes, i did claim it and the lack of attempt is my fault which i should have done
right away when i sent out the patches to phy-rtk-usb2.c [5] back in March.
I've just set the expectations that to me seem very logical but didn't get to
check them in practice. Though really my reasoning comes down to "We are trying
to recreate what readl() and writel() should supposedly do and SWAP_IO_SPACE is
only way to make them work".

I don't think it is necessary specific to USB as the same would have happened if
some other peripheral were to be in "little endian" mode. That is because our
realtek chips are big endian MIPS and kernel has an interesting ways of going
about their io.

There is however an ongoing patch series in linux-usb [6] that could be of use
if SWAP_IO_SPACE turns out to be a wrong approach after all.

[5] https://lore.kernel.org/linux-phy/20260326193419.48419-1-adilov@xxxxxxxxxxx/
[6] https://lore.kernel.org/linux-usb/20260713122205.1350933-1-daniel@xxxxxxxxx/

> Can the people from linux-mips shine their light on whether using __raw_*() to
> work around the effects of SWAP_IO_SPACE is reasonable? My gut feeling is this
> indicates SWAP_IO_SPACE shouldn't be enabled. Markus has apparently made the
> same remark [4].
> 
> [1] https://lore.kernel.org/all/ebdcb8c5563ceff723f3e4a3c4fdfe9bf87d42fa.camel@xxxxxxxxxxxxx/
> [2] https://lore.kernel.org/linux-watchdog/20260710074316.46643-1-adilov@xxxxxxxxxxx/
> [3] https://lore.kernel.org/all/20260511131520.98420-1-adilov@xxxxxxxxxxx/
> [4] https://lore.kernel.org/all/016d01dce49a$36b7bd60$a4273820$@gmx.de/
> Best,
> Sander

Best,
Rustam