On Mon, 2025-02-17 at 18:24 +0800, Chen-Yu Tsai wrote:

External email : Please do not click links or open attachments until
you have verified the sender or the content.


On Mon, Feb 17, 2025 at 5:45 PM Cathy Xu (许华婷) <
ot_cathy.xu@mediatek.com> wrote:

quoted

On Tue, 2025-01-21 at 18:03 +0800, Chen-Yu Tsai wrote:

quoted

External email : Please do not click links or open attachments
until
you have verified the sender or the content.


On Tue, Jan 21, 2025 at 5:58 PM Cathy Xu (许华婷) <
ot_cathy.xu@mediatek.com> wrote:

quoted

On Mon, 2025-01-20 at 13:42 +0100, AngeloGioacchino Del Regno
wrote:

quoted

External email : Please do not click links or open
attachments
until
you have verified the sender or the content.


Il 20/01/25 10:17, Cathy Xu (许华婷) ha scritto:

quoted

On Thu, 2025-01-16 at 11:20 +0100, Krzysztof Kozlowski
wrote:

quoted

External email : Please do not click links or open
attachments
until
you have verified the sender or the content.


On 16/01/2025 09:18, Cathy Xu (许华婷) wrote:

quoted

On Thu, 2025-01-16 at 08:28 +0100, Krzysztof Kozlowski
wrote:

quoted

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:

quoted

quoted

quoted

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

This is wrong.

  Sorry, I think I may not have expressed myself clearly. What
I
meant
is that the attribute 'mediatek,rsel-resistance-in-si-unit' is
not
supported. In the dts, can write the resistance value, for
example:
bias-pull-up=<1000>, but can't use 'mediatek,rsel-resistance-
in-si-
unit
= <xxx>'.

`mediatek,rsel-resistance-in-si-unit` is supported in
drivers/pinctrl/mediatek/pinctrl-paris.c

Angelo is requesting that you continue that support and make it
exclusive, i.e. not support the RSEL macro magic numbers, and
_only_ support ohm values, in the device tree.

ChenYu

  Thank you for your response. Can I understand that in next
version, I
should remove the RSEL macro magic numbers and add `mediatek,rsel-
resistance-in-si-unit`?

You should remove the RSEL macro magic numbers, and add an option to

quoted

struct mtk_pin_soc| so that the argument to "bias-pull-up" are
always

treated as values in SI units as if "mediatek,rsel-resistance-in-si-
unit"
was set in the DT.

ChenYu

  Thank you for your support, it will be fixed in next version.

quoted

quoted

quoted

quoted

quoted

quoted

quoted

quoted

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.

Does the hardware have known value in Ohms?

It does.

quoted

   What do you mean by 'hardware'? When writing to the rsel
register,
the value written is 0-7.

Hardware means "the pin controller of the mt8196 SoC" :-)

Anyway.

The RSEL registers' function is to select a specific
resistance
value
to
pullup/down a pin, or a group of pins.

Devicetree bindings require to specify values in known units,
so
in
device tree
you *need* to specify the RSEL resistance in Ohms.

You cannot specify RSEL register value in device-tree. That's
unacceptable.

Regards,
Angelo

 Yes, I understand what you mean. However, I was referring to
writing
the rsel register in the driver, not in dts.

quoted

quoted

quoted

quoted

quoted

quoted

quoted

quoted

+            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@mediatek.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)

quoted

4)
+#define PINMUX_GPIO0__FUNC_SCP_DMIC1_CLK
(MTK_PIN_NO(0) |
5)
+#define PINMUX_GPIO0__FUNC_TP_GPIO14_AO
(MTK_PIN_NO(0)

quoted

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_A
DSP_
URXD0

quoted

;

                 };
         };

That's DTS, not driver, so not a binding. Drop the header
from
bindings.

   Sorry, I don't quite understand the relationship between
binding
and
driver. Driver will parse this macro to get gpio number and
function.