Thread (16 messages) 16 messages, 4 authors, 2025-02-19

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

From: Cathy Xu (许华婷) <hidden>
Date: 2025-01-21 09:56:37
Also in: linux-devicetree, linux-gpio, linux-mediatek, lkml

On Mon, 2025-01-20 at 13:42 +0100, AngeloGioacchino Del Regno wrote:
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>'.
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
+            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 [off-list ref]
+ */
+
+#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
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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help