lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <df304802-bcd9-f241-419a-3345d79bfd1e@quicinc.com>
Date:   Fri, 21 Apr 2023 16:05:22 +0530
From:   Rohit Agarwal <quic_rohiagar@...cinc.com>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        <agross@...nel.org>, <andersson@...nel.org>,
        <konrad.dybcio@...aro.org>, <linus.walleij@...aro.org>,
        <robh+dt@...nel.org>, <krzysztof.kozlowski+dt@...aro.org>,
        <richardcochran@...il.com>, <manivannan.sadhasivam@...aro.org>
CC:     <linux-arm-msm@...r.kernel.org>, <linux-gpio@...r.kernel.org>,
        <devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <netdev@...r.kernel.org>
Subject: Re: [PATCH v2 1/2] dt-bindings: pinctrl: qcom: Add SDX75 pinctrl
 devicetree compatible


On 4/21/2023 3:38 PM, Krzysztof Kozlowski wrote:
> On 21/04/2023 11:43, Rohit Agarwal wrote:
>> Add device tree binding Documentation details for Qualcomm SDX75
>> pinctrl driver.
>>
>> Signed-off-by: Rohit Agarwal <quic_rohiagar@...cinc.com>
> Thank you for your patch. There is something to discuss/improve.
>
>> +properties:
>> +  compatible:
>> +    const: qcom,sdx75-tlmm
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  interrupts: true
>> +  interrupt-controller: true
>> +  "#interrupt-cells": true
>> +  gpio-controller: true
>> +
>> +  gpio-reserved-ranges:
>> +    minItems: 1
>> +    maxItems: 105
>> +
>> +  gpio-line-names:
>> +    maxItems: 133
> If you have 210 GPIOs, then this should be 210.
>
>> +
>> +  "#gpio-cells": true
>> +  gpio-ranges: true
>> +  wakeup-parent: true
>> +
>> +patternProperties:
>> +  "-state$":
>> +    oneOf:
>> +      - $ref: "#/$defs/qcom-sdx75-tlmm-state"
>> +      - patternProperties:
>> +          "-pins$":
>> +            $ref: "#/$defs/qcom-sdx75-tlmm-state"
>> +        additionalProperties: false
>> +
>> +$defs:
>> +  qcom-sdx75-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])$"
> This says you have 210 GPIOs.
>
>> +            - enum: [ ufs_reset, sdc2_clk, sdc2_cmd, sdc2_data ]
> Keep these four enum values sorted alphabetically.
>
>> +        minItems: 1
>> +        maxItems: 36
>> +
>> +      function:
>> +        description:
>> +          Specify the alternative function to be configured for the specified
>> +          pins.
>> +        enum: [ gpio, eth0_mdc, eth0_mdio, eth1_mdc, eth1_mdio,
>> +                qlink0_wmss_reset, qlink1_wmss_reset, rgmii_rxc, rgmii_rxd0,
>> +                rgmii_rxd1, rgmii_rxd2, rgmii_rxd3,rgmii_rx_ctl, rgmii_txc,
>> +                rgmii_txd0, rgmii_txd1, rgmii_txd2, rgmii_txd3, rgmii_tx_ctl,
>> +                adsp_ext_vfr, atest_char_start, atest_char_status0,
>> +                atest_char_status1, atest_char_status2, atest_char_status3,
>> +                audio_ref_clk, bimc_dte_test0, bimc_dte_test1,
>> +                char_exec_pending, char_exec_release, coex_uart2_rx,
>> +                coex_uart2_tx, coex_uart_rx, coex_uart_tx, cri_trng_rosc,
>> +                cri_trng_rosc0, cri_trng_rosc1, dbg_out_clk, ddr_bist_complete,
>> +                ddr_bist_fail, ddr_bist_start, ddr_bist_stop, ddr_pxi0_test,
>> +                ebi0_wrcdc_dq2, ebi0_wrcdc_dq3, ebi2_a_d, ebi2_lcd_cs,
>> +                ebi2_lcd_reset, ebi2_lcd_te, emac0_mcg_pst0, emac0_mcg_pst1,
>> +                emac0_mcg_pst2, emac0_mcg_pst3, emac0_ptp_aux, emac0_ptp_pps,
>> +                emac1_mcg_pst0, emac1_mcg_pst1, emac1_mcg_pst2, emac1_mcg_pst3,
>> +                emac1_ptp_aux0, emac1_ptp_aux1, emac1_ptp_aux2, emac1_ptp_aux3,
>> +                emac1_ptp_pps0, emac1_ptp_pps1, emac1_ptp_pps2, emac1_ptp_pps3,
>> +                emac_cdc_dtest0, emac_cdc_dtest1, emac_pps_in, ext_dbg_uart,
>> +                gcc_125_clk, gcc_gp1_clk, gcc_gp2_clk, gcc_gp3_clk,
>> +                gcc_plltest_bypassnl, gcc_plltest_resetn, i2s_mclk,
>> +                jitter_bist_ref, ldo_en, ldo_update, m_voc_ext, mgpi_clk_req,
>> +                native0, native1, native2, native3, native_char_start,
>> +                native_tsens_osc, native_tsense_pwm1, nav_dr_sync, nav_gpio_0,
>> +                nav_gpio_1, nav_gpio_2, nav_gpio_3, pa_indicator_1, pci_e_rst,
>> +                pcie0_clkreq_n, pcie1_clkreq_n, pcie2_clkreq_n, pll_bist_sync,
>> +                pll_clk_aux, pll_ref_clk, pri_mi2s_data0, pri_mi2s_data1,
>> +                pri_mi2s_sck, pri_mi2s_ws, prng_rosc_test0, prng_rosc_test1,
>> +                prng_rosc_test2, prng_rosc_test3, qdss_cti_trig0,
>> +                qdss_cti_trig1, qdss_gpio_traceclk, qdss_gpio_tracectl,
>> +                qdss_gpio_tracedata0, qdss_gpio_tracedata1,
>> +                qdss_gpio_tracedata10, qdss_gpio_tracedata11,
>> +                qdss_gpio_tracedata12, qdss_gpio_tracedata13,
>> +                qdss_gpio_tracedata14, qdss_gpio_tracedata15,
>> +                qdss_gpio_tracedata2, qdss_gpio_tracedata3,
>> +                qdss_gpio_tracedata4, qdss_gpio_tracedata5,
>> +                qdss_gpio_tracedata6, qdss_gpio_tracedata7,
>> +                qdss_gpio_tracedata8, qdss_gpio_tracedata9, qlink0_b_en,
>> +                qlink0_b_req, qlink0_l_en, qlink0_l_req, qlink1_l_en,
>> +                qlink1_l_req, qup_se0_l0, qup_se0_l1, qup_se0_l2, qup_se0_l3,
>> +                qup_se1_l2, qup_se1_l3, qup_se2_l0, qup_se2_l1, qup_se2_l2,
>> +                qup_se2_l3, qup_se3_l0, qup_se3_l1, qup_se3_l2, qup_se3_l3,
>> +                qup_se4_l2, qup_se4_l3, qup_se5_l0, qup_se5_l1, qup_se6_l0,
>> +                qup_se6_l1, qup_se6_l2, qup_se6_l3, qup_se7_l0, qup_se7_l1,
>> +                qup_se7_l2, qup_se7_l3, qup_se8_l2, qup_se8_l3, sdc1_tb_trig,
>> +                sdc2_tb_trig, sec_mi2s_data0, sec_mi2s_data1, sec_mi2s_sck,
>> +                sec_mi2s_ws, sgmii_phy_intr0, sgmii_phy_intr1, spmi_coex_clk,
>> +                spmi_coex_data, spmi_vgi_hwevent, tgu_ch0_trigout,
>> +                tri_mi2s_data0, tri_mi2s_data1, tri_mi2s_sck, tri_mi2s_ws,
>> +                uim1_clk, uim1_data, uim1_present, uim1_reset, uim2_clk,
>> +                uim2_data, uim2_present, uim2_reset, usb2phy_ac_en,
>> +                vsense_trigger_mirnat]
>> +
>> +      bias-disable: true
>> +      bias-pull-down: true
>> +      bias-pull-up: true
>> +      drive-strength: true
>> +      input-enable: true
> This is not allowed. Please rebase on pinctrl maintainer tree or next.
Will do this.
>
>> +      output-high: true
>> +      output-low: true
>> +
>> +    required:
>> +      - pins
>> +
>> +    additionalProperties: false
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>> +    tlmm: pinctrl@...0000 {
>> +        compatible = "qcom,sdx75-tlmm";
>> +        reg = <0x0f100000 0x300000>;
>> +        gpio-controller;
>> +        #gpio-cells = <2>;
>> +        gpio-ranges = <&tlmm 0 0 134>;
> Wrong number of pins. You have 210, right? This should be number of
> GPIOs + optionally UFS reset.
Thanks for reviewing the patch.
Actually it has 133 pins. Ok. Let me update the above property as well.
And just checked there is no ufs reset pin. So it should be removed 
completely.

Thanks,
Rohit.
>
>
> Best regards,
> Krzysztof
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ