Re: [PATCH v3 1/2] dt-bindings: pinctrl: mediatek: add support for mt8196
From: AngeloGioacchino Del Regno
Date: Mon Jan 20 2025 - 07:42:21 EST
Il 20/01/25 10:17, Cathy Xu (许华婷) ha scritto:
On Thu, 2025-01-16 at 11:20 +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 09:18, Cathy Xu (许华婷) 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'.
This is wrong.
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.
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
+ 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@xxxxxxxxxxxx>
+ */
+
+#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
;
};
};
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.