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

[Bartosz Golaszewski]

On Sun, 19 Jul 2026 06:17:01 +0200, Rustam Adilov <adilov@xxxxxxxxxxx> said:
> On 2026-07-17 10:39, Bartosz Golaszewski wrote:
>> On Wed, 15 Jul 2026 21:00:45 +0200, Rustam Adilov <adilov@xxxxxxxxxxx> said:
>>> Hello,
>>> On 2026-07-15 09:39, Bartosz Golaszewski wrote:
>>>> On Fri, 10 Jul 2026 20:34:39 +0200, Rustam Adilov <adilov@xxxxxxxxxxx> 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@xxxxxxxxxxx>
>>>>> ---
>>>>>  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.
>

I don't know the details of this use case. In general: devicetree support is
considered stable ABI. If a driver ever supported a certain property in any
release it must do so indefinitely. Any changes you propose must retain the
same behavior for existing users - even if they are missing certain DT flags,
even if they're out of tree. We don't care about breaking the code of
out-of-tree drivers but we *do* care about breaking out-of-tree devicetrees.

Bart