Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 20 May 2022 13:13:05 +0400
From:   Yassine Oudjana <yassine.oudjana@gmail.com>
Subject: Re: [PATCH v2 2/4] dt-bindings: reset: Add MT6735 reset bindings
To:     AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@collabora.com>
Cc:     Michael Turquette <mturquette@baylibre.com>,
        Stephen Boyd <sboyd@kernel.org>,
        Matthias Brugger <matthias.bgg@gmail.com>,
        Philipp Zabel <p.zabel@pengutronix.de>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Yassine Oudjana <y.oudjana@protonmail.com>,
        Tinghan Shen <tinghan.shen@mediatek.com>,
        Chun-Jie Chen <chun-jie.chen@mediatek.com>,
        Weiyi Lu <weiyi.lu@mediatek.com>,
        Ikjoon Jang <ikjn@chromium.org>,
        Miles Chen <miles.chen@mediatek.com>,
        Sam Shih <sam.shih@mediatek.com>,
        Chen-Yu Tsai <wenst@chromium.org>,
        Bartosz Golaszewski <bgolaszewski@baylibre.com>,
        devicetree@vger.kernel.org, linux-mediatek@lists.infradead.org,
        linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        linux-kernel@vger.kernel.org,
        ~postmarketos/upstreaming@lists.sr.ht,
        Rob Herring <robh@kernel.org>
Message-Id: <T9C6CR.ZDXJ22Q377EZ1@gmail.com>
In-Reply-To: <d54ea1d3-903f-0b2a-3db4-99f1adfadce0@collabora.com>
References: <20220519142211.458336-1-y.oudjana@protonmail.com>
        <20220519142211.458336-3-y.oudjana@protonmail.com>
        <d54ea1d3-903f-0b2a-3db4-99f1adfadce0@collabora.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii; format=flowed
Precedence: bulk


On Fri, May 20 2022 at 10:55:24 +0200, AngeloGioacchino Del Regno 
<angelogioacchino.delregno@collabora.com> wrote:
> Il 19/05/22 16:22, Yassine Oudjana ha scritto:
>> From: Yassine Oudjana <y.oudjana@protonmail.com>
>> 
>> Add reset definitions for Mediatek MT6735 resets provided by
>> infracfg and pericfg.
>> 
>> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
>> Acked-by: Rob Herring <robh@kernel.org>
>> ---
>>   MAINTAINERS                                   |  2 ++
>>   .../reset/mediatek,mt6735-infracfg.h          | 31 
>> +++++++++++++++++++
>>   .../reset/mediatek,mt6735-pericfg.h           | 31 
>> +++++++++++++++++++
>>   3 files changed, 64 insertions(+)
>>   create mode 100644 
>> include/dt-bindings/reset/mediatek,mt6735-infracfg.h
>>   create mode 100644 
>> include/dt-bindings/reset/mediatek,mt6735-pericfg.h
>> 
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index a59069263cfb..1c0af554a7b6 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -12503,6 +12503,8 @@ 
>> F:	include/dt-bindings/clock/mediatek,mt6735-apmixedsys.h
>>   F:	include/dt-bindings/clock/mediatek,mt6735-infracfg.h
>>   F:	include/dt-bindings/clock/mediatek,mt6735-pericfg.h
>>   F:	include/dt-bindings/clock/mediatek,mt6735-topckgen.h
>> +F:	include/dt-bindings/reset/mediatek,mt6735-infracfg.h
>> +F:	include/dt-bindings/reset/mediatek,mt6735-pericfg.h
>>     MEDIATEK MT76 WIRELESS LAN DRIVER
>>   M:	Felix Fietkau <nbd@nbd.name>
> 
> ..snip..
> 
>> diff --git a/include/dt-bindings/reset/mediatek,mt6735-pericfg.h 
>> b/include/dt-bindings/reset/mediatek,mt6735-pericfg.h
>> new file mode 100644
>> index 000000000000..6cdfaa7ddadf
>> --- /dev/null
>> +++ b/include/dt-bindings/reset/mediatek,mt6735-pericfg.h
>> @@ -0,0 +1,31 @@
>> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
>> +
>> +#ifndef _DT_BINDINGS_RST_MT6735_PERICFG_H
>> +#define _DT_BINDINGS_RST_MT6735_PERICFG_H
>> +
>> +#define UART0_SW_RST			0
>> +#define UART1_SW_RST			1
>> +#define UART2_SW_RST			2
>> +#define UART3_SW_RST			3
>> +#define UART4_SW_RST			4
> 
> where's number 5?
> 
>> +#define BTIF_SW_RST			6
>> +#define DISP_PWM_SW_RST			7
>> +#define PWM_SW_RST			8
> 
> ...and where's 9?
> 
>> +#define AUXADC_SW_RST			10
>> +#define DMA_SW_RST			11
>> +#define IRDA_SW_RST			12
>> +#define IRTX_SW_RST			13
> 
> and 14, 15?
> 
>> +#define THERM_SW_RST			16
>> +#define MSDC2_SW_RST			17
>> +#define MSDC3_SW_RST			17
> 
> MSDC 2 and 3 are both 17?! :-)
> 
>> +#define MSDC0_SW_RST			19
>> +#define MSDC1_SW_RST			20
> 
> 21?
> 
>> +#define I2C0_SW_RST			22
>> +#define I2C1_SW_RST			23
>> +#define I2C2_SW_RST			24
>> +#define I2C3_SW_RST			25
>> +#define USB_SW_RST			28
>> +
> 
> and 29-32?
> 
>> +#define SPI0_SW_RST			33
>> +
>> +#endif
> 
> I have a hunch that you've misunderstood the changes in the resets...
> 
> What Rex-BC has done in his reset cleanup is exactly to stop directly
> mapping these to the actual bits that we're using... so the 
> definitions
> in there will simply be sequential, and the actual mapping is done in
> your clk-mt6735-pericfg.c driver.

I did notice that, but reading the documentation in reset.h:

 * @rst_idx_map:Pointer to an array containing ids if input argument is 
index.
 *		This array is not necessary if our input argument does not mean 
index.

I thought that it wasn't necessary to use it. Thinking
about it now however, I guess that was to maintain compatibility
with old device trees. I'll change it next time.
Maybe a note should be put there to avoid confusion
in the future.

Thanks,
Yassine