mbox series

[0/3] pinctl: qcom: Add SM4450 pinctrl driver

Message ID 20230908063843.26835-1-quic_tengfan@quicinc.com
Headers show
Series pinctl: qcom: Add SM4450 pinctrl driver | expand

Message

Tengfei Fan Sept. 8, 2023, 6:38 a.m. UTC
Add Sm4450 pinctrl driver for support enable uart console.

Tengfei Fan (3):
  dt-bindings: pinctrl: qcom: Add SM4450 pinctrl
  pinctrl: qcom: Add SM4450 pinctrl driver
  arm64: defconfig: Enable pinctrl for SM4450

 .../bindings/pinctrl/qcom,sm4450-tlmm.yaml    |  129 ++
 arch/arm64/configs/defconfig                  |    1 +
 drivers/pinctrl/qcom/Kconfig.msm              |    8 +
 drivers/pinctrl/qcom/Makefile                 |    1 +
 drivers/pinctrl/qcom/pinctrl-sm4450.c         | 1528 +++++++++++++++++
 5 files changed, 1667 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,sm4450-tlmm.yaml
 create mode 100644 drivers/pinctrl/qcom/pinctrl-sm4450.c


base-commit: a47fc304d2b678db1a5d760a7d644dac9b067752

Comments

Rob Herring (Arm) Sept. 8, 2023, 7:22 a.m. UTC | #1
On Fri, 08 Sep 2023 14:38:41 +0800, Tengfei Fan wrote:
> Add device tree binding Documentation details for Qualcomm SM4450
> TLMM device.
> 
> Signed-off-by: Tengfei Fan <quic_tengfan@quicinc.com>
> ---
>  .../bindings/pinctrl/qcom,sm4450-tlmm.yaml    | 129 ++++++++++++++++++
>  1 file changed, 129 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,sm4450-tlmm.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pinctrl/qcom,sm4450-tlmm.yaml:
Error in referenced schema matching $id: http://devicetree.org/schemas/pinctrl/qcom,tlmm-common.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230908063843.26835-2-quic_tengfan@quicinc.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
Krzysztof Kozlowski Sept. 8, 2023, 7:53 a.m. UTC | #2
On 08/09/2023 08:38, Tengfei Fan wrote:
> Add device tree binding Documentation details for Qualcomm SM4450
> TLMM device.
> 
> Signed-off-by: Tengfei Fan <quic_tengfan@quicinc.com>
> ---
>  .../bindings/pinctrl/qcom,sm4450-tlmm.yaml    | 129 ++++++++++++++++++
>  1 file changed, 129 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,sm4450-tlmm.yaml
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,sm4450-tlmm.yaml b/Documentation/devicetree/bindings/pinctrl/qcom,sm4450-tlmm.yaml
> new file mode 100644
> index 000000000000..51735604b3c3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/qcom,sm4450-tlmm.yaml
> @@ -0,0 +1,129 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/bindings/pinctrl/qcom,sm4450-tlmm.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm Technologies, Inc. RAVELIN TLMM block

SM4450, not ravelin

> +
> +maintainers:
> +  - Tengfei Fan <quic_tengfan@quicinc.com>
> +
> +description:
> +  Top Level Mode Multiplexer pin controller in Qualcomm SM4450 SoC.
> +
> +allOf:
> +  - $ref: /schemas/pinctrl/qcom,tlmm-common.yaml#
> +
> +properties:
> +  compatible:
> +    const: qcom,sm4450-pinctrl
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts: true
> +  interrupt-controller: true
> +  "#interrupt-cells": true
> +  gpio-controller: true
> +
> +  gpio-reserved-ranges:
> +    minItems: 1
> +    maxItems: 105

That's not true, you cannot have 105 ranges.

> +
> +  gpio-line-names:
> +    maxItems: 210

No, your driver tells something entirely different.

> +
> +  "#gpio-cells": true
> +  gpio-ranges: true
> +  wakeup-parent: true
> +
> +patternProperties:
> +  "-state$":
> +    oneOf:
> +      - $ref: "#/$defs/qcom-sm4450-tlmm-state"
> +      - patternProperties:
> +          "-pins$":
> +            $ref: "#/$defs/qcom-sm4450-tlmm-state"
> +        additionalProperties: false
> +
> +$defs:
> +  qcom-sm4450-tlmm-state:
> +    type: object
> +    description:
> +      Pinctrl node's client devices use subnodes for desired pin configuration.
> +      Client device subnodes use below standard properties.
> +    $ref: qcom,tlmm-common.yaml#/$defs/qcom-tlmm-state
> +    unevaluatedProperties: false
> +
> +    properties:
> +      pins:
> +        description:
> +          List of gpio pins affected by the properties specified in this
> +          subnode.
> +        items:
> +          oneOf:
> +            - pattern: "^gpio([0-9]|[1-9][0-9]|1[0-9][0-9]|20[0-9])$"

You do not have 210 GPIOs. Narrow it to real values.

> +            - enum: [ sdc2_clk, sdc2_cmd, sdc2_data, ufs_reset ]

Not tested. Missing min/maxItems.

> +
> +      function:
> +        description:
> +          Specify the alternative function to be configured for the specified
> +          pins.
> +        enum: [ gpio, atest_char, atest_char0, atest_char1, atest_char2,
> +                atest_char3, atest_usb0, atest_usb00, atest_usb01, atest_usb02,
> +                atest_usb03, audio_ref, cam_mclk, cci_async, cci_i2c,
> +                cci_timer0, cci_timer1, cci_timer2, cci_timer3, cci_timer4,
> +                cmu_rng0, cmu_rng1, cmu_rng2, cmu_rng3, coex_uart1, cri_trng,
> +                cri_trng0, cri_trng1, dbg_out, ddr_bist, ddr_pxi0, ddr_pxi1,
> +                dp0_hot, gcc_gp1, gcc_gp2, gcc_gp3, host2wlan_sol, ibi_i3c,
> +                jitter_bist, mdp_vsync, mdp_vsync0, mdp_vsync1, mdp_vsync2,
> +                mdp_vsync3, mi2s0_data0, mi2s0_data1, mi2s0_sck, mi2s0_ws,
> +                mi2s2_data0, mi2s2_data1, mi2s2_sck, mi2s2_ws, mi2s_mclk0,
> +                mi2s_mclk1, nav_gpio0, nav_gpio1, nav_gpio2, pcie0_clk,
> +                phase_flag0, phase_flag1, phase_flag10, phase_flag11,
> +                phase_flag12, phase_flag13, phase_flag14, phase_flag15,
> +                phase_flag16, phase_flag17, phase_flag18, phase_flag19,
> +                phase_flag2, phase_flag20, phase_flag21, phase_flag22,
> +                phase_flag23, phase_flag24, phase_flag25, phase_flag26,
> +                phase_flag27, phase_flag28, phase_flag29, phase_flag3,
> +                phase_flag30, phase_flag31, phase_flag4, phase_flag5,
> +                phase_flag6, phase_flag7, phase_flag8, phase_flag9,
> +                pll_bist, pll_clk, prng_rosc0, prng_rosc1, prng_rosc2,
> +                prng_rosc3, qdss_cti, qdss_gpio, qdss_gpio0, qdss_gpio1,
> +                qdss_gpio10, qdss_gpio11, qdss_gpio12, qdss_gpio13, qdss_gpio14,
> +                qdss_gpio15, qdss_gpio2, qdss_gpio3, qdss_gpio4, qdss_gpio5,
> +                qdss_gpio6, qdss_gpio7, qdss_gpio8, qdss_gpio9, qlink0_enable,
> +                qlink0_request, qlink0_wmss, qlink1_enable, qlink1_request,
> +                qlink1_wmss, qlink2_enable, qlink2_request, qlink2_wmss,
> +                qup0_se0, qup0_se1, qup0_se2, qup0_se3, qup0_se4, qup0_se5,
> +                qup0_se6, qup0_se7, qup1_se0, qup1_se1, qup1_se2, qup1_se3,
> +                qup1_se4, qup1_se5, qup1_se6, sd_write, tb_trig, tgu_ch0,
> +                tgu_ch1, tgu_ch2, tgu_ch3, tmess_prng0, tmess_prng1,
> +                tmess_prng2, tmess_prng3, tsense_pwm1, tsense_pwm2, uim0_clk,
> +                uim0_data, uim0_present, uim0_reset, uim1_clk, uim1_data,
> +                uim1_present, uim1_reset, usb0_hs, usb0_phy, vfr_0, vfr_1,
> +                vsense_trigger ]
> +
> +        required:
> +          - pins
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    tlmm: pinctrl@f100000 {
> +      compatible = "qcom,sm4450-tlmm";
> +      reg = <0x0f100000 0x300000>;
> +      gpio-controller;
> +      #gpio-cells = <2>;
> +      interrupt-controller;
> +      #interrupt-cells = <2>;
> +      interrupts = <GIC_SPI 208 IRQ_TYPE_LEVEL_HIGH>;

Example pin config, gpio-ranges.

> +    };
> +...

Best regards,
Krzysztof
Konrad Dybcio Sept. 8, 2023, 8:45 a.m. UTC | #3
On 8.09.2023 08:38, Tengfei Fan wrote:
> Add pinctrl driver for TLMM block found in SM4450 SoC.
> 
> Signed-off-by: Tengfei Fan <quic_tengfan@quicinc.com>
> ---
[...]

> +/* Every pin is maintained as a single group, and missing or non-existing pin
/*
 * Every pin

> + * would be maintained as dummy group to synchronize pin group index with
> + * pin descriptor registered with pinctrl core.
> + * Clients would not be able to request these dummy pin groups.
> + */
[...]

> +static const int sm4450_acpi_reserved_gpios[] = {
> +	0, 1, 2, 3, 136, -1
> +};
Are you ever going to boot with ACPI on this platform?

Why reserve UFS_RESET?

Why are 0-3 reserved? FP reader? Please leave a comment. Or
delete this.

Konrad
Tengfei Fan Sept. 11, 2023, 1:23 a.m. UTC | #4
在 9/8/2023 4:45 PM, Konrad Dybcio 写道:
> On 8.09.2023 08:38, Tengfei Fan wrote:
>> Add pinctrl driver for TLMM block found in SM4450 SoC.
>>
>> Signed-off-by: Tengfei Fan <quic_tengfan@quicinc.com>
>> ---
> [...]
> 
>> +/* Every pin is maintained as a single group, and missing or non-existing pin
> /*
>   * Every pin
> 
>> + * would be maintained as dummy group to synchronize pin group index with
>> + * pin descriptor registered with pinctrl core.
>> + * Clients would not be able to request these dummy pin groups.
>> + */
> [...]
> 
>> +static const int sm4450_acpi_reserved_gpios[] = {
>> +	0, 1, 2, 3, 136, -1
>> +};
> Are you ever going to boot with ACPI on this platform?
> 
> Why reserve UFS_RESET?
> 
> Why are 0-3 reserved? FP reader? Please leave a comment. Or
> delete this.
> 
> Konrad
Thanks Konrad reviewed this patch, will do more test and disscuss about 
this reserve gpio setting, and will remove this in next patch if possible.