Re: [PATCH 2/2] gpio: realtek-otto: decide bank_read/write by device endianness

[Rustam Adilov <adilov@disroot.org>]

On 2026-07-17 10:39, Bartosz Golaszewski wrote:
> On Wed, 15 Jul 2026 21:00:45 +0200, Rustam Adilov <adilov@disroot.org> said:
>> Hello,
>> On 2026-07-15 09:39, Bartosz Golaszewski wrote:
>>> On Fri, 10 Jul 2026 20:34:39 +0200, Rustam Adilov <adilov@disroot.org> said:
>>>> In order to have a working gpio controller with SWAP_IO_SPACE,
>>>> the way bank_read and bank_write are defined must be changed and
>>>> separated from GPIO_PORTS_REVERSED flag. That also includes the
>>>> flags parameter used by gpio_generic_chip_config.
>>>>
>>>> To achieve it, use the device_is_big_endian() to decide the
>>>> bank_read/write parameter and the flag.
>>>>
>>>> Signed-off-by: Rustam Adilov <adilov@disroot.org>
>>>> ---
>>>>  drivers/gpio/gpio-realtek-otto.c | 15 +++++++++------
>>>>  1 file changed, 9 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/drivers/gpio/gpio-realtek-otto.c b/drivers/gpio/gpio-realtek-otto.c
>>>> index 491fde846d46..f96072fbce92 100644
>>>> --- a/drivers/gpio/gpio-realtek-otto.c
>>>> +++ b/drivers/gpio/gpio-realtek-otto.c
>>>> @@ -393,16 +393,19 @@ static int realtek_gpio_probe(struct platform_device *pdev)
>>>>
>>>>  	raw_spin_lock_init(&ctrl->lock);
>>>>
>>>> -	if (dev_flags & GPIO_PORTS_REVERSED) {
>>>> -		gen_gc_flags = 0;
>>>> -		ctrl->bank_read = realtek_gpio_bank_read;
>>>> -		ctrl->bank_write = realtek_gpio_bank_write;
>>>> +	if (dev_flags & GPIO_PORTS_REVERSED)
>>>>  		ctrl->line_imr_pos = realtek_gpio_line_imr_pos;
>>>> -	} else {
>>>> +	else
>>>> +		ctrl->line_imr_pos = realtek_gpio_line_imr_pos_swapped;
>>>> +
>>>> +	if (device_is_big_endian(dev)) {
>>>>  		gen_gc_flags = GPIO_GENERIC_BIG_ENDIAN_BYTE_ORDER;
>>>>  		ctrl->bank_read = realtek_gpio_bank_read_swapped;
>>>>  		ctrl->bank_write = realtek_gpio_bank_write_swapped;
>>>> -		ctrl->line_imr_pos = realtek_gpio_line_imr_pos_swapped;
>>>> +	} else {
>>>> +		gen_gc_flags = 0;
>>>> +		ctrl->bank_read = realtek_gpio_bank_read;
>>>> +		ctrl->bank_write = realtek_gpio_bank_write;
>>>>  	}
>>>>
>>>>  	config = (struct gpio_generic_chip_config) {
>>>> --
>>>> 2.55.0
>>>>
>>>>
>>>
>>> Can you look at the sashiko report? I think it's right about this change
>>> possibly breaking existing devicetrees. Can we keep big-endian as the
>>> default?
>>>
>>> Bart
>>
>> Oh i didn't know sashiko was looking after this subsystem too.
>> But yes, it does require the rtl8380-gpio, rtl8390-gpio, rtl9310-gpio
>> to append big-endian or native-endian (as they are all big endian chips).
>>
>> I was thinking earlier whatever it is appropriate to make changes to
>> the arch/mips/boot/dts/realtek in this commit because it is different
>> subsystem entirely but i guess this (previous sentence) answers the
>> question, doesn't it.
>>
> 
> That's irrelevant as there may be devicetrees out in the wild and we can't
> just break them.

So, what is the solution then? Cause relying on device tree property the
driver did not rely on before would break stuff, though realistically it
is only relevant to OpenWrt and here. If we instead just create a flag
in the driver like PORTS_REVERSED, then we would have to patch it again
when SWAP_IO_SPACE is selected in the future and that is more hassle than
simply changing device tree property. Unless i am missing something, these
are the only possible ways of going about it that i can think of.

>> Also, what exactly do you mean by "Can we keep big-endian as the
>> default?" ? My only reading off is to change the if statement
>> so that the big endian flag and the _swapped functions to be
>> under the else part, is it correct?
>>
> 
> Yes, just retain the original default behavior.
> 
> Bart

Best,
Rustam