| 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: <CAGXv+5FtA7KCPE1FQ1Wp=M_11=20n432zSWTkeBasUa4fdpm8A@mail.gmail.com>
Date: Thu, 16 Jan 2025 16:33:37 +0800
From: Chen-Yu Tsai <wenst@...omium.org>
To: Cathy Xu (许华婷) <ot_cathy.xu@...iatek.com>
Cc: "krzk@...nel.org" <krzk@...nel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-mediatek@...ts.infradead.org" <linux-mediatek@...ts.infradead.org>,
Lei Xue (薛磊) <Lei.Xue@...iatek.com>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
Wenbin Mei (梅文彬) <Wenbin.Mei@...iatek.com>,
"linus.walleij@...aro.org" <linus.walleij@...aro.org>,
Guodong Liu (刘国栋) <Guodong.Liu@...iatek.com>,
"linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>, "conor+dt@...nel.org" <conor+dt@...nel.org>,
"robh@...nel.org" <robh@...nel.org>, "sean.wang@...nel.org" <sean.wang@...nel.org>,
"linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>,
"matthias.bgg@...il.com" <matthias.bgg@...il.com>, "krzk+dt@...nel.org" <krzk+dt@...nel.org>,
AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
Subject: Re: [PATCH v3 1/2] dt-bindings: pinctrl: mediatek: add support for mt8196
On Thu, Jan 16, 2025 at 4:19 PM Cathy Xu (许华婷) <ot_cathy.xu@...iatek.com> wrote:
>
> On Thu, 2025-01-16 at 08:28 +0100, Krzysztof Kozlowski wrote:
> > External email : Please do not click links or open attachments until
> > you have verified the sender or the content.
> >
> >
> > On 16/01/2025 03:20, Cathy Xu (许华婷) wrote:
> > > > > + bias-pull-down:
> > > > > + oneOf:
> > > > > + - type: boolean
> > > > > + - enum: [100, 101, 102, 103]
> > > > > + description: mt8196 pull down PUPD/R0/R1 type
> > > > > define value.
> > > > > + - enum: [200, 201, 202, 203, 204, 205, 206, 207]
> > > > > + description: mt8196 pull down RSEL type define
> > > > > value.
> > > >
> > > > Not much improved.
> > >
> > > I have removed the content related to 'resistance value', we use
> > > 'RSEL' instead of 'resistance value'.
> >
> > So the value in Ohms was removed? I assume above do not have known
> > value
> > in Ohms?
>
> Yes, value in Ohns was removed, no code have knowm value.
That's sad. We went through a lot during the MT8195 cycle to get the
paris driver library to support proper SI unit values [1] to replace
the RSEL values (200 ~ 207). Why can't this be supported anymore?
Also we never got around to getting rid the PUPD/R0/R1 values (100 ~ 103).
> >
> > >
> > > >
> > > >
> > > > > + description: |
> > > > > + For pull down type is normal, it doesn't need
> > > > > add
> > > > > RSEL & R1R0.
> > > > > + For pull down type is PUPD/R0/R1 type, it can
> > > > > add
> > > > > R1R0 define to
> > > > > + set different resistance. It can support
> > > > > "MTK_PUPD_SET_R1R0_00" &
> > > > > + "MTK_PUPD_SET_R1R0_01" & "MTK_PUPD_SET_R1R0_10"
> > > > > &
> > > > > + "MTK_PUPD_SET_R1R0_11" define in mt8196.
> > > > > + For pull down type is PD/RSEL, it can add RSEL
> > > > > define to set
> > > > > + different resistance. It can support
> > > > > + "MTK_PULL_SET_RSEL_000" &
> > > > > "MTK_PULL_SET_RSEL_001" &
> > > > > + "MTK_PULL_SET_RSEL_010" &
> > > > > "MTK_PULL_SET_RSEL_011" &
> > > > > + "MTK_PULL_SET_RSEL_100" &
> > > > > "MTK_PULL_SET_RSEL_101" &
> > > > > + "MTK_PULL_SET_RSEL_110" &
> > > > > "MTK_PULL_SET_RSEL_111"
> > > > > define in
> > > > > + mt8196.
> > > > > diff --git a/include/dt-bindings/pinctrl/mt8196-pinfunc.h
> > > > > b/include/dt-bindings/pinctrl/mt8196-pinfunc.h
> > > > > new file mode 100644
> > > > > index 000000000000..bf0c8374407c
> > > > > --- /dev/null
> > > > > +++ b/include/dt-bindings/pinctrl/mt8196-pinfunc.h
> > > > > @@ -0,0 +1,1572 @@
> > > > > +/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
> > > > > +/*
> > > > > + * Copyright (C) 2025 Mediatek Inc.
> > > > > + * Author: Guodong Liu <Guodong.Liu@...iatek.com>
> > > > > + */
> > > > > +
> > > > > +#ifndef __MT8196_PINFUNC_H
> > > > > +#define __MT8196_PINFUNC_H
> > > > > +
> > > > > +#include <dt-bindings/pinctrl/mt65xx.h>
> > > > > +
> > > > > +#define PINMUX_GPIO0__FUNC_GPIO0 (MTK_PIN_NO(0) | 0)
> > > > > +#define PINMUX_GPIO0__FUNC_DMIC1_CLK (MTK_PIN_NO(0) | 1)
> > > > > +#define PINMUX_GPIO0__FUNC_SPI3_A_MO (MTK_PIN_NO(0) | 3)
> > > > > +#define PINMUX_GPIO0__FUNC_FMI2S_B_LRCK (MTK_PIN_NO(0) | 4)
> > > > > +#define PINMUX_GPIO0__FUNC_SCP_DMIC1_CLK (MTK_PIN_NO(0) | 5)
> > > > > +#define PINMUX_GPIO0__FUNC_TP_GPIO14_AO (MTK_PIN_NO(0) | 6)
> > > >
> > > > I do not see how you resolved my comment from v1. In v2 I
> > > > reminded
> > > > about
> > > > it, so you responded that yopu will change something, but I do
> > > > not
> > > > see
> > > > any changes.
> > > >
> > > > So explain: how did you resolve my comment?
> > > >
> > > > These two examples where you claim you will change something, but
> > > > send
> > > > the same. I skipped the rest of the patch.
> > >
> > > Thank you for your patient response, here is my explanation for
> > > you
> > > question:
> > >
> > > In v1, I undertand that you meant I didn't sent a real binding,
> > > and
> >
> >
> > The comment is under specific lines, so I said these defines are not
> > a
> > real binding. You sent them again, but they are still not bindings,
> > because they are not used in the driver. Maybe the usage is
> > convoluted,
> > so which part of implementation are these connecting with DTS? IOW,
> > which part of driver relies on the binding?
>
> I got you. This binding define many macros, which will be used for
> 'pinmux' setting in the DTS. The usage like this:
>
> adsp_uart_pins: adsp-uart-pins {
> pins-tx-rx {
> pinmux = <PINMUX_GPIO35__FUNC_O_ADSP_UTXD0>,
> <PINMUX_GPIO36__FUNC_I1_ADSP_URXD0>;
> };
> };
The only binding between the DT and the driver is the structure of
the value, given as "(MTK_PIN_NO(<N>) | <function mux value>)".
The whole list of "PINMUX_GPIOxxx__FUNC_xyz" macros is just a convenience
table for developers, and not used by the driver. The driver simply takes
the values from the two bit fields and uses them directly.
That's why Krzysztof is saying the macros are not used in the driver
and therefore not a binding.
Please move the header file to under "arch/arm64/boot/dts/mediatek",
and split it out as a separate commit with a subject like:
arm64: dts: mediatek: mt8196: Add pinmux macro header file
ChenYu
> >
> >
> >
> > > the bindings should be separated from driver. In addition, I should
> > > run scripts/checkpatch.pl and scripts/get_maintainers.pl. So in v2,
> > > I
> > > sent a real binding(mediatek,mt8196-pinctrl.yaml), and sent two
> > > separate patches, one for driver and one for bindings, also ran
> > > scripts/get_maintainers.pl get necessary people and sent to them.
> > >
> > > In v2, I understand that I need refer to git history to modify
> > > the
> > > commit msgs, so I made the changes in v3. Then you asked me about
> > > the
> > > difference between 'RSEL' and 'resistance value'. I replied that
> > > the
> > > 'resistance value' method is no longer use, so in v3, I removed all
> > > content about it(include entire 'rsel-resistance-in-si-unit'
> > > property
> > > and the parts mentioned in bias-pull-up/down).
> >
> > Yes, thank you this I saw, the comments appear under specific places,
> > so
> > only these places are discussed.
>
> ok, thank you, we can discuss if there are any issues.
>
> >
> >
> >
> > Best regards,
> > Krzysztof
Powered by blists - more mailing lists