| 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
| ||
|
Message-ID: <9b42b402be2b0ed065d176238ac2c41f2ec677ef.camel@mediatek.com>
Date: Thu, 28 Apr 2022 14:48:52 +0800
From: Rex-BC Chen <rex-bc.chen@...iatek.com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
"mturquette@...libre.com" <mturquette@...libre.com>,
"sboyd@...nel.org" <sboyd@...nel.org>,
"matthias.bgg@...il.com" <matthias.bgg@...il.com>,
"robh+dt@...nel.org" <robh+dt@...nel.org>,
"krzysztof.kozlowski+dt@...aro.org"
<krzysztof.kozlowski+dt@...aro.org>
CC: "p.zabel@...gutronix.de" <p.zabel@...gutronix.de>,
"angelogioacchino.delregno@...labora.com"
<angelogioacchino.delregno@...labora.com>,
Chun-Jie Chen (陳浚桀)
<Chun-Jie.Chen@...iatek.com>,
"wenst@...omium.org" <wenst@...omium.org>,
Runyang Chen (陈润洋)
<Runyang.Chen@...iatek.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"linux-clk@...r.kernel.org" <linux-clk@...r.kernel.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"linux-mediatek@...ts.infradead.org"
<linux-mediatek@...ts.infradead.org>,
Project_Global_Chrome_Upstream_Group
<Project_Global_Chrome_Upstream_Group@...iatek.com>
Subject: Re: [PATCH V3 12/17] dt-binding: mt8192: Add infra_ao reset bit
On Thu, 2022-04-28 at 14:40 +0800, Krzysztof Kozlowski wrote:
> On 26/04/2022 10:23, Rex-BC Chen wrote:
> > On Mon, 2022-04-25 at 15:52 +0800, Krzysztof Kozlowski wrote:
> > > On 25/04/2022 07:01, Rex-BC Chen wrote:
> > > > On Sat, 2022-04-23 at 18:28 +0800, Krzysztof Kozlowski wrote:
> > > > > On 22/04/2022 08:01, Rex-BC Chen wrote:
> > > > > > To support reset of infra_ao, add the bit definition for
> > > > > > thermal/PCIe/SVS.
> > > > > >
> > > > > > Signed-off-by: Rex-BC Chen <rex-bc.chen@...iatek.com>
> > > > > > ---
> > > > > > include/dt-bindings/reset/mt8192-resets.h | 10 ++++++++++
> > > > > > 1 file changed, 10 insertions(+)
> > > > > >
> > > > > > diff --git a/include/dt-bindings/reset/mt8192-resets.h
> > > > > > b/include/dt-bindings/reset/mt8192-resets.h
> > > > > > index be9a7ca245b9..d5f3433175c1 100644
> > > > > > --- a/include/dt-bindings/reset/mt8192-resets.h
> > > > > > +++ b/include/dt-bindings/reset/mt8192-resets.h
> > > > > > @@ -27,4 +27,14 @@
> > > > > >
> > > > > > #define MT8192_TOPRGU_SW_RST_NUM
> > > > > > 23
> > > > > >
> > > > > > +/* INFRA RST0 */
> > > > > > +#define MT8192_INFRA_RST0_LVTS_AP_RST
> > > > > >
> > > > > > 0
> > > > > > +/* INFRA RST2 */
> > > > > > +#define MT8192_INFRA_RST2_PCIE_PHY_RST
> > > > > >
> > > > > > 15
> > > > > > +/* INFRA RST3 */
> > > > > > +#define MT8192_INFRA_RST3_PTP_RST
> > > > > > 5
> > > > > > +/* INFRA RST4 */
> > > > > > +#define MT8192_INFRA_RST4_LVTS_MCU
> > > > > > 12
> > > > > > +#define MT8192_INFRA_RST4_PCIE_TOP
> > > > > > 1
> > > > >
> > > > > These should be the IDs of reset, not some register
> > > > > values/offsets.
> > > > > Therefore it is expected to have them incremented by 1.
> > > > >
> > > > >
> > > >
> > > > Hello Krzysztof,
> > > >
> > > > This is define bit.
> > > >
> > > > There is serveral reset set for infra_ao while it's not serial.
> > > > For MT8192, it's 0x120/0x130/0x140/0x150/0x730.
> > > > We are implement #reset-cells = <2>, and we can use this reset
> > > > drive
> > > > more easier.
> > > >
> > > > For example, in dts, we can define
> > > > infra_ao: syscon {
> > > > compatible = "mediatek,mt8192-infracfg", "syscon";
> > > > reg = <0 0x10001000 0 0x1000>;
> > > > #clock-cells = <1>;
> > > > #reset-cells = <2>;
> > > > };
> > > >
> > > > thermal {
> > > > ...
> > > > resets = <&infra_ao 0x730 MT8192_INFRA_RST4_LVTS_MCU>;
> > > > ...
> > > > };
> > > >
> > > > If it's acceptabel, I can update all bit difinition from 0 to
> > > > 15
> > > > for
> > > > all reset set.
> > >
> > > Bits are not acceptable, because you embed specific device
> > > programming
> > > model (register bits) into the binding.
> > >
> > > These should be IDs, so decimal numbers incremented from 0, so:
> > > #define MT8192_INFRA_RST0_LVTS_AP_RST
> > > 0
> > > #define MT8192_INFRA_RST4_LVTS_MCU
> > > 1
> > > #define MT8192_INFRA_RST4_PCIE_TOP
> > > 2
> > >
> > > And what is 0x730 in your example? It does not look like ID of a
> > > reset...
> > >
> > > Entire changeset look wrong from DT point of view.
> > >
> > > Best regards,
> > > Krzysztof
> >
> > Hello Krzysztof,
> >
> > Got it. I will modify them to reset index.
> > And the dts in my next version would somthing like this:
> >
> > ----
> > #define MT8192_INFRA_THERMAL_CTRL_RST 0
> > #define MT8192_INFRA_PEXTP_PHY_RST 79
> > #define MT8192_INFRA_PTP_RST 101
> > #define MT8192_INFRA_RST4_PCIE_TOP 129
> > #define MT8192_INFRA_THERMAL_CTRL_MCU_RST 140
>
> These are still not IDs, incremented by one.
>
> So again from beginning:
> 0
> 1
> 2
> ...
>
> Do not encode hardware register bits into the binding.
>
> Best regards,
> Krzysztof
Hello Krzysztof,
It's not bit definiton, and it's index for our reset.
We have 32*5 reset bits for infra.
But we only use these 5 index currently, I do not list all of them.
The implementation is in [1].
-----
static int mtk_reset_update_set_clr(struct reset_controller_dev *rcdev,
unsigned int deassert_ofs = deassert ? 0x4 : 0;
return regmap_write(data->regmap,
data->desc->rst_bank_ofs[id /
RST_NR_PER_BANK] +
deassert_ofs,
BIT(id % RST_NR_PER_BANK));
}
-----
[1]:
https://lore.kernel.org/all/20220427030950.23395-8-rex-bc.chen@mediatek.com/
BRs,
Rex
Powered by blists - more mailing lists