[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <T9C6CR.ZDXJ22Q377EZ1@gmail.com>
Date: Fri, 20 May 2022 13:13:05 +0400
From: Yassine Oudjana <yassine.oudjana@...il.com>
To: AngeloGioacchino Del Regno
<angelogioacchino.delregno@...labora.com>
Cc: Michael Turquette <mturquette@...libre.com>,
Stephen Boyd <sboyd@...nel.org>,
Matthias Brugger <matthias.bgg@...il.com>,
Philipp Zabel <p.zabel@...gutronix.de>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Yassine Oudjana <y.oudjana@...tonmail.com>,
Tinghan Shen <tinghan.shen@...iatek.com>,
Chun-Jie Chen <chun-jie.chen@...iatek.com>,
Weiyi Lu <weiyi.lu@...iatek.com>,
Ikjoon Jang <ikjn@...omium.org>,
Miles Chen <miles.chen@...iatek.com>,
Sam Shih <sam.shih@...iatek.com>,
Chen-Yu Tsai <wenst@...omium.org>,
Bartosz Golaszewski <bgolaszewski@...libre.com>,
devicetree@...r.kernel.org, linux-mediatek@...ts.infradead.org,
linux-clk@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org,
~postmarketos/upstreaming@...ts.sr.ht,
Rob Herring <robh@...nel.org>
Subject: Re: [PATCH v2 2/4] dt-bindings: reset: Add MT6735 reset bindings
On Fri, May 20 2022 at 10:55:24 +0200, AngeloGioacchino Del Regno
<angelogioacchino.delregno@...labora.com> wrote:
> Il 19/05/22 16:22, Yassine Oudjana ha scritto:
>> From: Yassine Oudjana <y.oudjana@...tonmail.com>
>>
>> Add reset definitions for Mediatek MT6735 resets provided by
>> infracfg and pericfg.
>>
>> Signed-off-by: Yassine Oudjana <y.oudjana@...tonmail.com>
>> Acked-by: Rob Herring <robh@...nel.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@....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
Powered by blists - more mailing lists