linux-kernel - Re: [RFC PATCH 00/16] pinctrl: ralink: fix ABI, improve driver, move to mediatek, improve dt-bindings

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <286c6f85-8c5c-907f-a77b-b0c62e97a18a@arinc9.com>
Date:   Thu, 23 Feb 2023 09:01:59 +0300
From:   Arınç ÜNAL <arinc.unal@...nc9.com>
To:     Sergio Paracuellos <sergio.paracuellos@...il.com>
Cc:     Linus Walleij <linus.walleij@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        linux-mediatek@...ts.infradead.org, linux-mips@...r.kernel.org,
        linux-gpio@...r.kernel.org, devicetree@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        Matthias Brugger <matthias.bgg@...il.com>,
        Sean Wang <sean.wang@...nel.org>,
        William Dean <williamsukatube@...il.com>,
        Daniel Golle <daniel@...rotopia.org>,
        Daniel Santos <daniel.santos@...ox.com>,
        Luiz Angelo Daros de Luca <luizluca@...il.com>,
        Frank Wunderlich <frank-w@...lic-files.de>,
        Landen Chao <Landen.Chao@...iatek.com>,
        DENG Qingfang <dqfext@...il.com>,
        Sean Wang <sean.wang@...iatek.com>, erkin.bozoglu@...ont.com
Subject: Re: [RFC PATCH 00/16] pinctrl: ralink: fix ABI, improve driver, move
 to mediatek, improve dt-bindings

On 23.02.2023 07:58, Sergio Paracuellos wrote:
> Hi Arınç,
> 
> All of this looks pretty good to me. You did a really big effort with
> this series. Thanks for doing this!
> 
> On Wed, Feb 22, 2023 at 7:39 PM <arinc9.unal@...il.com> wrote:
>>
>> This is an ambitious effort I've been wanting to do for months.
>>
>> Straight off the bat, I'm fixing the ABI that I broke a while back, by
>> reintroducing the ralink,rt2880-pinmux compatible string.
>>
>> If you take a look at the schema for mt7620 and rt305x, some functions got
>> multiple lists for groups. Like refclk on mt7620. Because mt7620 and
>> mt7628/mt7688 SoCs use the same compatible string, it's impossible to
>> differentiate on the binding which SoC a devicetree is actually for.
>> Therefore, the binding will allow all groups listed for that function. For
>> example, if the SoC is mt7620, only the refclk function for the mdio group
>> can be used. If one were to put "spi cs1" as the function there, there
>> wouldn't be a warning.
>>
>> I address this by introducing new compatible strings for these SoCs, then
>> split the schemas. I also separate mt7628/mt7688 from mt7620 pinctrl
>> subdriver in the process.
>>
>> I wanted to split the rt305x driver too but too much code would be reused
>> so I backed down from that.
>>
>> Ralink was acquired by MediaTek in 2011. These SoCs have been rebranded as
>> MediaTek. We're moving the Ralink pinctrl driver to MediaTek, and rename
>> the schemas to mediatek.
>>
>> I've renamed the ralink core driver to mtmips. I decided to call the core
>> mtmips as I've seen folks from MediaTek use the same name when they added
>> support for MT7621 pinctrl on U-Boot. Feel free to comment on this.
>>
>> The MTMIPS pinctrl driver requires rt_sysc_membase from
>> arch/mips/ralink/of.c, so, for COMPILE_TEST to be useful, RALINK must be
>> selected. These headers, asm/mach-ralink/ralink_regs.h and
>> asm/mach-ralink/mt7620.h, from arch/mips/include are also required but
>> they can easily be included:
>>
>> ifeq ($(CONFIG_COMPILE_TEST),y)
>> CFLAGS_pinctrl-mtmips.o                 += -I$(srctree)/arch/mips/include
>> endif
>>
>> Sergio, do you see a way to make the pinctrl driver independent of
>> architecture code? At least avoid using rt_sysc_membase.
> 
> The only really dependent architecture code in these drivers now is
> because of the use of
> 'rt_sysc_r32()' and 'rt_sysc_w32()' in 'ralink_pmx_group_enable()'
> function [0]. This is just to set the gpio mode. The read and write
> registers here  SYSC_REG_GPIO_MODE and  SYSC_REG_GPIO_MODE2 are in the
> system controller area. In all single ralink platform 'sysc' nodes
> should be a syscon that can be accessed from the driver side. That way
> you can just get those syscon areas via regmap APIs and properly read
> and write desired registers. For the mt7621.dtsi file, the node is
> already a syscon [1]. Other ralink device tree files should also be
> modified to include this in its 'sysc' node (I think in openWRT dts
> files at least for mt7620 is already included). You have to add that
> in all of them since 'ralink_pmx_group_enable()' is common code for
> all. I think this can be done in a different patch series. I can help
> you to do this after this series is reviewed and accepted.

Awesome, thanks a lot! Note to self, "depends on RALINK" menu entries 
should be changed to "depends on OF" when this happens.

Arınç