linux-kernel - Re: [PATCH v3 1/2] dt-bindings: pinctrl: mediatek: add support for mt8196

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: <d11076d3eb2f92018fd3e26cae665a47f71ca838.camel@mediatek.com>
Date: Thu, 16 Jan 2025 08:18:17 +0000
From: Cathy Xu (许华婷) <ot_cathy.xu@...iatek.com>
To: "krzk@...nel.org" <krzk@...nel.org>
CC: "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, 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.

> 
> > 
> > > 
> > > 
> > > > +            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 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