The properties `mediatek,pull-up-adv` and `mediatek,pull-down-adv`
were using incorrect values like `<10>` and `<01>`. These values
are parsed as decimal (10 and 1 respectively), not binary.
However, the driver interprets these as bitfields:
- BIT(0): R0
- BIT(1): R1
So valid values are:
- 0 => no pull
- 1 => enable R0
- 2 => enable R1
- 3 => enable R0 + R1
Using `<10>` is invalid as it exceeds the accepted range.
It was likely intended as binary `0b10` (i.e., `2`),
to enable R1 only.
This patch replaces incorrect values with the correct ones
and removes the leading zero from `<01>` to avoid confusion
with bitfield notation.
Signed-off-by: Julien Massot <julien.massot@collabora.com>
---
arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
index 400c61d1103561db6ee0fb2d2e1c157529d03206..02bdfdb8e53c87dba0ba0024e0c69fcee825552b 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
@@ -574,19 +574,19 @@ pins-cmd-dat {
<PINMUX_GPIO122__FUNC_MSDC0_CMD>;
input-enable;
drive-strength = <MTK_DRIVE_14mA>;
- mediatek,pull-up-adv = <01>;
+ mediatek,pull-up-adv = <1>;
};
pins-clk {
pinmux = <PINMUX_GPIO124__FUNC_MSDC0_CLK>;
drive-strength = <MTK_DRIVE_14mA>;
- mediatek,pull-down-adv = <10>;
+ mediatek,pull-down-adv = <2>;
};
pins-rst {
pinmux = <PINMUX_GPIO133__FUNC_MSDC0_RSTB>;
drive-strength = <MTK_DRIVE_14mA>;
- mediatek,pull-down-adv = <01>;
+ mediatek,pull-down-adv = <1>;
};
};
@@ -603,25 +603,25 @@ pins-cmd-dat {
<PINMUX_GPIO122__FUNC_MSDC0_CMD>;
input-enable;
drive-strength = <MTK_DRIVE_14mA>;
- mediatek,pull-up-adv = <01>;
+ mediatek,pull-up-adv = <1>;
};
pins-clk {
pinmux = <PINMUX_GPIO124__FUNC_MSDC0_CLK>;
drive-strength = <MTK_DRIVE_14mA>;
- mediatek,pull-down-adv = <10>;
+ mediatek,pull-down-adv = <2>;
};
pins-ds {
pinmux = <PINMUX_GPIO131__FUNC_MSDC0_DSL>;
drive-strength = <MTK_DRIVE_14mA>;
- mediatek,pull-down-adv = <10>;
+ mediatek,pull-down-adv = <2>;
};
pins-rst {
pinmux = <PINMUX_GPIO133__FUNC_MSDC0_RSTB>;
drive-strength = <MTK_DRIVE_14mA>;
- mediatek,pull-up-adv = <01>;
+ mediatek,pull-up-adv = <1>;
};
};
@@ -633,13 +633,13 @@ pins-cmd-dat {
<PINMUX_GPIO33__FUNC_MSDC1_DAT2>,
<PINMUX_GPIO30__FUNC_MSDC1_DAT3>;
input-enable;
- mediatek,pull-up-adv = <10>;
+ mediatek,pull-up-adv = <2>;
};
pins-clk {
pinmux = <PINMUX_GPIO29__FUNC_MSDC1_CLK>;
input-enable;
- mediatek,pull-down-adv = <10>;
+ mediatek,pull-down-adv = <2>;
};
};
@@ -652,13 +652,13 @@ pins-cmd-dat {
<PINMUX_GPIO30__FUNC_MSDC1_DAT3>;
drive-strength = <6>;
input-enable;
- mediatek,pull-up-adv = <10>;
+ mediatek,pull-up-adv = <2>;
};
pins-clk {
pinmux = <PINMUX_GPIO29__FUNC_MSDC1_CLK>;
drive-strength = <8>;
- mediatek,pull-down-adv = <10>;
+ mediatek,pull-down-adv = <2>;
input-enable;
};
};
--
2.50.1Il 01/08/25 13:18, Julien Massot ha scritto: > The properties `mediatek,pull-up-adv` and `mediatek,pull-down-adv` > were using incorrect values like `<10>` and `<01>`. These values > are parsed as decimal (10 and 1 respectively), not binary. > > However, the driver interprets these as bitfields: > - BIT(0): R0 > - BIT(1): R1 > > So valid values are: > - 0 => no pull > - 1 => enable R0 > - 2 => enable R1 > - 3 => enable R0 + R1 > > Using `<10>` is invalid as it exceeds the accepted range. > It was likely intended as binary `0b10` (i.e., `2`), > to enable R1 only. > > This patch replaces incorrect values with the correct ones > and removes the leading zero from `<01>` to avoid confusion > with bitfield notation. > > Signed-off-by: Julien Massot <julien.massot@collabora.com> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
On Fri, Aug 1, 2025 at 7:18 PM Julien Massot
<julien.massot@collabora.com> wrote:
>
> The properties `mediatek,pull-up-adv` and `mediatek,pull-down-adv`
> were using incorrect values like `<10>` and `<01>`. These values
> are parsed as decimal (10 and 1 respectively), not binary.
>
> However, the driver interprets these as bitfields:
> - BIT(0): R0
> - BIT(1): R1
>
> So valid values are:
> - 0 => no pull
> - 1 => enable R0
> - 2 => enable R1
> - 3 => enable R0 + R1
>
> Using `<10>` is invalid as it exceeds the accepted range.
> It was likely intended as binary `0b10` (i.e., `2`),
> to enable R1 only.
>
> This patch replaces incorrect values with the correct ones
> and removes the leading zero from `<01>` to avoid confusion
> with bitfield notation.
>
> Signed-off-by: Julien Massot <julien.massot@collabora.com>
> ---
> arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi | 22 +++++++++++-----------
> 1 file changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
> index 400c61d1103561db6ee0fb2d2e1c157529d03206..02bdfdb8e53c87dba0ba0024e0c69fcee825552b 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
> @@ -574,19 +574,19 @@ pins-cmd-dat {
> <PINMUX_GPIO122__FUNC_MSDC0_CMD>;
> input-enable;
> drive-strength = <MTK_DRIVE_14mA>;
> - mediatek,pull-up-adv = <01>;
> + mediatek,pull-up-adv = <1>;
I suggest we just convert them to the standard bias-* properties:
bias-pull-up = <MTK_PUPD_SET_R1R0_01>;
> };
>
> pins-clk {
> pinmux = <PINMUX_GPIO124__FUNC_MSDC0_CLK>;
> drive-strength = <MTK_DRIVE_14mA>;
> - mediatek,pull-down-adv = <10>;
> + mediatek,pull-down-adv = <2>;
bias-pull-down = <MTK_PUPD_SET_R1R0_10>;
and so on.
ChenYu
> };
>
> pins-rst {
> pinmux = <PINMUX_GPIO133__FUNC_MSDC0_RSTB>;
> drive-strength = <MTK_DRIVE_14mA>;
> - mediatek,pull-down-adv = <01>;
> + mediatek,pull-down-adv = <1>;
> };
> };
>
> @@ -603,25 +603,25 @@ pins-cmd-dat {
> <PINMUX_GPIO122__FUNC_MSDC0_CMD>;
> input-enable;
> drive-strength = <MTK_DRIVE_14mA>;
> - mediatek,pull-up-adv = <01>;
> + mediatek,pull-up-adv = <1>;
> };
>
> pins-clk {
> pinmux = <PINMUX_GPIO124__FUNC_MSDC0_CLK>;
> drive-strength = <MTK_DRIVE_14mA>;
> - mediatek,pull-down-adv = <10>;
> + mediatek,pull-down-adv = <2>;
> };
>
> pins-ds {
> pinmux = <PINMUX_GPIO131__FUNC_MSDC0_DSL>;
> drive-strength = <MTK_DRIVE_14mA>;
> - mediatek,pull-down-adv = <10>;
> + mediatek,pull-down-adv = <2>;
> };
>
> pins-rst {
> pinmux = <PINMUX_GPIO133__FUNC_MSDC0_RSTB>;
> drive-strength = <MTK_DRIVE_14mA>;
> - mediatek,pull-up-adv = <01>;
> + mediatek,pull-up-adv = <1>;
> };
> };
>
> @@ -633,13 +633,13 @@ pins-cmd-dat {
> <PINMUX_GPIO33__FUNC_MSDC1_DAT2>,
> <PINMUX_GPIO30__FUNC_MSDC1_DAT3>;
> input-enable;
> - mediatek,pull-up-adv = <10>;
> + mediatek,pull-up-adv = <2>;
> };
>
> pins-clk {
> pinmux = <PINMUX_GPIO29__FUNC_MSDC1_CLK>;
> input-enable;
> - mediatek,pull-down-adv = <10>;
> + mediatek,pull-down-adv = <2>;
> };
> };
>
> @@ -652,13 +652,13 @@ pins-cmd-dat {
> <PINMUX_GPIO30__FUNC_MSDC1_DAT3>;
> drive-strength = <6>;
> input-enable;
> - mediatek,pull-up-adv = <10>;
> + mediatek,pull-up-adv = <2>;
> };
>
> pins-clk {
> pinmux = <PINMUX_GPIO29__FUNC_MSDC1_CLK>;
> drive-strength = <8>;
> - mediatek,pull-down-adv = <10>;
> + mediatek,pull-down-adv = <2>;
> input-enable;
> };
> };
>
> --
> 2.50.1
>
On Wed, Aug 6, 2025 at 8:38 AM Chen-Yu Tsai <wenst@chromium.org> wrote:
> On Fri, Aug 1, 2025 at 7:18 PM Julien Massot wrote
> > pins-clk {
> > pinmux = <PINMUX_GPIO124__FUNC_MSDC0_CLK>;
> > drive-strength = <MTK_DRIVE_14mA>;
> > - mediatek,pull-down-adv = <10>;
> > + mediatek,pull-down-adv = <2>;
>
> bias-pull-down = <MTK_PUPD_SET_R1R0_10>;
>
> and so on.
>
> ChenYu
I agree with ChenYu, the more standardized properties are the better it is.
All the custom properties makes sense for an engineer working with just
that one SoC (like the SoC vendor...) but for field engineers who have
to use different SoCs every day this is just a big mess for the mind.
The standard properties are clear, concise and tell you exactly what
they are about.
The argument should be in Ohms though, according to the standard
bindings, but maybe the value of MTK_PUPD_SET_R1R0_10 is
something like that?
Yours,
Linus Walleij
On Mon, Aug 18, 2025 at 11:22 PM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Wed, Aug 6, 2025 at 8:38 AM Chen-Yu Tsai <wenst@chromium.org> wrote:
> > On Fri, Aug 1, 2025 at 7:18 PM Julien Massot wrote
>
> > > pins-clk {
> > > pinmux = <PINMUX_GPIO124__FUNC_MSDC0_CLK>;
> > > drive-strength = <MTK_DRIVE_14mA>;
> > > - mediatek,pull-down-adv = <10>;
> > > + mediatek,pull-down-adv = <2>;
> >
> > bias-pull-down = <MTK_PUPD_SET_R1R0_10>;
> >
> > and so on.
> >
> > ChenYu
>
> I agree with ChenYu, the more standardized properties are the better it is.
>
> All the custom properties makes sense for an engineer working with just
> that one SoC (like the SoC vendor...) but for field engineers who have
> to use different SoCs every day this is just a big mess for the mind.
>
> The standard properties are clear, concise and tell you exactly what
> they are about.
>
> The argument should be in Ohms though, according to the standard
> bindings, but maybe the value of MTK_PUPD_SET_R1R0_10 is
> something like that?
For reasons I can't recall clearly these are just placeholder values
that the driver then maps to the R1 and R0 settings. But at least they
use the standard properties.
The reason was either one of the following or both:
a. not every group of pins had the same resistance values for R1 & R0
b. there are no known precise values; the values depend on the process
and batch
ChenYu
On Tue, Aug 19, 2025 at 1:27 PM Chen-Yu Tsai <wenst@chromium.org> wrote:
>
> On Mon, Aug 18, 2025 at 11:22 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> >
> > On Wed, Aug 6, 2025 at 8:38 AM Chen-Yu Tsai <wenst@chromium.org> wrote:
> > > On Fri, Aug 1, 2025 at 7:18 PM Julien Massot wrote
> >
> > > > pins-clk {
> > > > pinmux = <PINMUX_GPIO124__FUNC_MSDC0_CLK>;
> > > > drive-strength = <MTK_DRIVE_14mA>;
> > > > - mediatek,pull-down-adv = <10>;
> > > > + mediatek,pull-down-adv = <2>;
> > >
> > > bias-pull-down = <MTK_PUPD_SET_R1R0_10>;
> > >
> > > and so on.
> > >
> > > ChenYu
> >
> > I agree with ChenYu, the more standardized properties are the better it is.
> >
> > All the custom properties makes sense for an engineer working with just
> > that one SoC (like the SoC vendor...) but for field engineers who have
> > to use different SoCs every day this is just a big mess for the mind.
> >
> > The standard properties are clear, concise and tell you exactly what
> > they are about.
> >
> > The argument should be in Ohms though, according to the standard
> > bindings, but maybe the value of MTK_PUPD_SET_R1R0_10 is
> > something like that?
>
> For reasons I can't recall clearly these are just placeholder values
> that the driver then maps to the R1 and R0 settings. But at least they
> use the standard properties.
>
> The reason was either one of the following or both:
>
> a. not every group of pins had the same resistance values for R1 & R0
> b. there are no known precise values; the values depend on the process
> and batch
Also, their customers seemed more accustomed to dealing with toggling
R1 & R0 vs setting some actual value. I presume that comes with the
uncertainty of the actual hardware value, and they just try which
combination works better.
ChenYu
Hi,
On Tue, 2025-08-19 at 13:29 +0800, Chen-Yu Tsai wrote:
> On Tue, Aug 19, 2025 at 1:27 PM Chen-Yu Tsai <wenst@chromium.org> wrote:
> >
> > On Mon, Aug 18, 2025 at 11:22 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> > >
> > > On Wed, Aug 6, 2025 at 8:38 AM Chen-Yu Tsai <wenst@chromium.org> wrote:
> > > > On Fri, Aug 1, 2025 at 7:18 PM Julien Massot wrote
> > >
> > > > > pins-clk {
> > > > > pinmux = <PINMUX_GPIO124__FUNC_MSDC0_CLK>;
> > > > > drive-strength = <MTK_DRIVE_14mA>;
> > > > > - mediatek,pull-down-adv = <10>;
> > > > > + mediatek,pull-down-adv = <2>;
> > > >
> > > > bias-pull-down = <MTK_PUPD_SET_R1R0_10>;
> > > >
> > > > and so on.
> > > >
> > > > ChenYu
> > >
> > > I agree with ChenYu, the more standardized properties are the better it is.
> > >
> > > All the custom properties makes sense for an engineer working with just
> > > that one SoC (like the SoC vendor...) but for field engineers who have
> > > to use different SoCs every day this is just a big mess for the mind.
> > >
> > > The standard properties are clear, concise and tell you exactly what
> > > they are about.
> > >
> > > The argument should be in Ohms though, according to the standard
> > > bindings, but maybe the value of MTK_PUPD_SET_R1R0_10 is
> > > something like that?
> >
> > For reasons I can't recall clearly these are just placeholder values
> > that the driver then maps to the R1 and R0 settings. But at least they
> > use the standard properties.
> >
> > The reason was either one of the following or both:
> >
> > a. not every group of pins had the same resistance values for R1 & R0
> > b. there are no known precise values; the values depend on the process
> > and batch
>
> I don't know for (b), but no there is a lot of different values for R1 & R0
>
From what I saw in the register table
We can have for the pull up resistors
75K / 200K
2K / 75K
5K / 20K
50k / 10K
And for the pull down ones:
75k/2k
75k/75k
10k/50k
And we can have a combination of both resistors that will give odd values
(e.g 1948 Ohm for 2k/75K, 545454 Ohm for 75k/200k) to express in the device tree
>
> Also, their customers seemed more accustomed to dealing with toggling
> R1 & R0 vs setting some actual value. I presume that comes with the
> uncertainty of the actual hardware value, and they just try which
> combination works better.
>
> ChenYu
Regards,
Julien
© 2016 - 2025 Red Hat, Inc.