Re: [PATCH v2 3/4] arm64: dts: nuvoton: Add pinctrl support for ma35d1

From: Jacky Huang
Date: Wed Nov 29 2023 - 05:14:16 EST



Dear Krzysztof,


On 2023/11/29 下午 06:02, Krzysztof Kozlowski wrote:
On 29/11/2023 10:41, Jacky Huang wrote:
Dear Krzysztof,


On 2023/11/29 下午 04:11, Krzysztof Kozlowski wrote:
On 29/11/2023 04:35, Jacky Huang wrote:
Best regards,
Krzysztof

Yes, it did pass the 'dtbs_check'. I guess the tool does not detect such
issues.
Anyway, I will fix it in the next version.
Hm, I see your bindings indeed allow pin-.* and unit addresses, so it is
the binding issue.

The examples you used as reference - xlnx,zynqmp-pinctrl.yaml and
realtek,rtd1315e-pinctrl.yaml - do not mix these as you do.

I don't understand why do you need them yet. I don't see any populate of
children. There are no compatibles, either.

Which part of your driver uses them exactly?

Best regards,
Krzysztof

I will move the 'pcfg_default: pin-default' from dtsi to dts, like this:

&pinctrl {
    pcfg_default: pin-default {
        slew-rate = <0>;
        input-schmitt-disable;
        bias-disable;
        power-source = <1>;
        drive-strength = <17100>;
    };
This solves nothing. It's the same placement.


Best regards,
Krzysztof

OK, it stil be the binding issues.
For "^pin-[a-z0-9]+$", I reference to the "pcfg-[a-z0-9-]+$" of
rockchip,pinctrl.yaml.

My intention is to describe a generic pin configuration, aiming to make
the pin
description more concise. In actual testing, it proves to be effective.
Can you instead respond to my actual questions?

Best regards,
Krzysztof


The the last one item of nuvoton,pins is a phandle, which can refer to '&pin-default'. The following code of driver pinctrl-ma35.c parse "nuvoton,pins", including the node reference by phandle. list = of_get_property(np, "nuvoton,pins", &size); size /= sizeof(*list); if (!size || size % 4) { dev_err(npctl->dev, "wrong setting!\n"); return -EINVAL; } grp->npins = size / 4; grp->pins = devm_kzalloc(npctl->dev, grp->npins * sizeof(*grp->pins), GFP_KERNEL); if (!grp->pins) return -ENOMEM; pin = grp->settings = devm_kzalloc(npctl->dev, grp->npins * sizeof(*grp->settings), GFP_KERNEL); if (!grp->settings) return -ENOMEM; for (i = 0, j = 0; i < size; i += 4, j++) { struct device_node *np_config; const __be32 *phandle; pin->offset = be32_to_cpu(*list++) * MA35_MFP_REG_SZ_PER_BANK + MA35_MFP_REG_BASE; pin->shift = (be32_to_cpu(*list++) * MA35_MFP_BITS_PER_PORT) % 32; pin->muxval = be32_to_cpu(*list++); phandle = list++; if (!phandle) return -EINVAL; np_config = of_find_node_by_phandle(be32_to_cpup(phandle)); ret = pinconf_generic_parse_dt_config(np_config, NULL, &pin->configs, &pin->nconfigs); if (ret) return ret; grp->pins[j] = npctl->info->get_pin_num(pin->offset, pin->shift); pin++; } Best Regards, Jacky Huang