[PATCH v2 03/10] arm64: dts: qcom: msm8916-pins: keep cdc_dmic pins in suspend mode
This node was the only one that didn't have the same set of pins in active and suspend mode. Signed-off-by: Damien Riegel --- arch/arm64/boot/dts/qcom/msm8916-pins.dtsi | 8 1 file changed, 8 insertions(+) diff --git a/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi b/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi index c67ad8ed8b60..10fc1fc90682 100644 --- a/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi +++ b/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi @@ -687,6 +687,14 @@ }; }; cdc_dmic_lines_sus: dmic_lines_off { + pinmux_dmic0_clk { + function = "dmic0_clk"; + pins = "gpio0"; + }; + pinmux_dmic0_data { + function = "dmic0_data"; + pins = "gpio1"; + }; pinconf { pins = "gpio0", "gpio1"; drive-strength = <2>; -- 2.15.0
[PATCH v2 10/10] arm64: dts: qcom: msm8916: add nodes for i2c1, i2c3, i2c5
Signed-off-by: Damien Riegel --- Changes in v2: - Reworded commit title - Changed size to 0x500 arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi | 48 ++ arch/arm64/boot/dts/qcom/msm8916-pins.dtsi | 42 ++ arch/arm64/boot/dts/qcom/msm8916.dtsi | 45 3 files changed, 135 insertions(+) diff --git a/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi b/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi index 53c1ddd281a4..11305015ba0b 100644 --- a/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi +++ b/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi @@ -630,6 +630,22 @@ }; }; +&i2c1_default { + pinconf { + pins = "gpio2", "gpio3"; + drive-strength = <16>; + bias-disable; + }; +}; + +&i2c1_sleep { + pinconf { + pins = "gpio2", "gpio3"; + drive-strength = <2>; + bias-disable; + }; +}; + &i2c2_default { pinconf { pins = "gpio6", "gpio7"; @@ -646,6 +662,22 @@ }; }; +&i2c3_default { + pinconf { + pins = "gpio10", "gpio11"; + drive-strength = <16>; + bias-disable; + }; +}; + +&i2c3_sleep { + pinconf { + pins = "gpio10", "gpio11"; + drive-strength = <2>; + bias-disable; + }; +}; + &i2c4_default { pinconf { pins = "gpio14", "gpio15"; @@ -662,6 +694,22 @@ }; }; +&i2c5_default { + pinconf { + pins = "gpio18", "gpio19"; + drive-strength = <16>; + bias-disable; + }; +}; + +&i2c5_sleep { + pinconf { + pins = "gpio18", "gpio19"; + drive-strength = <2>; + bias-disable; + }; +}; + &i2c6_default { pinconf { pins = "gpio22", "gpio23"; diff --git a/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi b/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi index 7704ddecb6c4..44e68860fc8c 100644 --- a/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi +++ b/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi @@ -152,6 +152,20 @@ }; }; + i2c1_default: i2c1_default { + pinmux { + function = "blsp_i2c1"; + pins = "gpio2", "gpio3"; + }; + }; + + i2c1_sleep: i2c1_sleep { + pinmux { + function = "gpio"; + pins = "gpio2", "gpio3"; + }; + }; + i2c2_default: i2c2_default { pinmux { function = "blsp_i2c2"; @@ -166,6 +180,20 @@ }; }; + i2c3_default: i2c3_default { + pinmux { + function = "blsp_i2c3"; + pins = "gpio10", "gpio11"; + }; + }; + + i2c3_sleep: i2c3_sleep { + pinmux { + function = "gpio"; + pins = "gpio10", "gpio11"; + }; + }; + i2c4_default: i2c4_default { pinmux { function = "blsp_i2c4"; @@ -180,6 +208,20 @@ }; }; + i2c5_default: i2c5_default { + pinmux { + function = "blsp_i2c5"; + pins = "gpio18", "gpio19"; + }; + }; + + i2c5_sleep: i2c5_sleep { + pinmux { + function = "gpio"; + pins = "gpio18", "gpio19"; + }; + }; + i2c6_default: i2c6_default { pinmux { function = "blsp_i2c6"; diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi index ac440f287633..7478c7337995 100644 --- a/arch/arm64/boot/dts/qcom/msm8916.dtsi +++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi @@ -455,6 +455,21 @@ status = "disabled"; }; + blsp_i2c1: i2c@78b5000 { + compatible = "qcom,i2c-qup-v2.2.1"; + reg = <0x078b5000 0x500>; + interrupts = ; + clocks = <&gcc GCC_BLSP1_AHB_CLK>, +<&gcc GCC_BLSP1_QUP1_I2C_APPS_CLK>; + clock-names = "iface", "core"; +
[PATCH v2 06/10] arm64: dts: qcom: msm8916: move pinconfs to board files
Following a suggestion from Bjorn Andersson [1], this commit moves electrical specifications which were defined in mms8916-pins.dtsi to board files, where they actually belong. Pinmuxing is kept in the platform file because there are no alternative pins on which all these functions could be routed, so this part is indeed common to all boards using this SoC. [1] https://www.spinics.net/lists/devicetree/msg201764.html Signed-off-by: Damien Riegel Suggested-by: Bjorn Andersson --- arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi | 386 + arch/arm64/boot/dts/qcom/msm8916-mtp.dtsi | 17 ++ arch/arm64/boot/dts/qcom/msm8916-pins.dtsi | 258 --- 3 files changed, 403 insertions(+), 258 deletions(-) diff --git a/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi b/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi index 981450f50e10..53c1ddd281a4 100644 --- a/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi +++ b/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi @@ -544,6 +544,384 @@ }; }; +&blsp1_uart1_default { + pinconf { + pins = "gpio0", "gpio1", + "gpio2", "gpio3"; + drive-strength = <16>; + bias-disable; + }; +}; + +&blsp1_uart1_sleep { + pinconf { + pins = "gpio0", "gpio1", + "gpio2", "gpio3"; + drive-strength = <2>; + bias-pull-down; + }; +}; + +&blsp1_uart2_default { + pinconf { + pins = "gpio4", "gpio5"; + drive-strength = <16>; + bias-disable; + }; +}; + +&blsp1_uart2_sleep { + pinconf { + pins = "gpio4", "gpio5"; + drive-strength = <2>; + bias-pull-down; + }; +}; + +&cdc_pdm_lines_act { + pinconf { + pins = "gpio63", "gpio64", "gpio65", "gpio66", + "gpio67", "gpio68"; + drive-strength = <8>; + bias-pull-none; + }; +}; + +&cdc_pdm_lines_sus { + pinconf { + pins = "gpio63", "gpio64", "gpio65", "gpio66", + "gpio67", "gpio68"; + drive-strength = <2>; + bias-disable; + }; +}; + +&ext_mclk_tlmm_lines_act { + pinconf { + pins = "gpio116"; + drive-strength = <8>; + bias-pull-none; + }; +}; + +&ext_mclk_tlmm_lines_sus { + pinconf { + pins = "gpio116"; + drive-strength = <2>; + bias-disable; + }; +}; + +&ext_sec_tlmm_lines_act { + pinconf { + pins = "gpio112", "gpio117", "gpio118", + "gpio119"; + drive-strength = <8>; + bias-pull-none; + }; +}; + +&ext_sec_tlmm_lines_sus { + pinconf { + pins = "gpio112", "gpio117", "gpio118", + "gpio119"; + drive-strength = <2>; + bias-disable; + }; +}; + +&i2c2_default { + pinconf { + pins = "gpio6", "gpio7"; + drive-strength = <16>; + bias-disable; + }; +}; + +&i2c2_sleep { + pinconf { + pins = "gpio6", "gpio7"; + drive-strength = <2>; + bias-disable; + }; +}; + +&i2c4_default { + pinconf { + pins = "gpio14", "gpio15"; + drive-strength = <16>; + bias-disable; + }; +}; + +&i2c4_sleep { + pinconf { + pins = "gpio14", "gpio15"; + drive-strength = <2>; + bias-disable; + }; +}; + +&i2c6_default { + pinconf { + pins = "gpio22", "gpio23"; + drive-strength = <16>; + bias-disable; + }; +}; + +&i2c6_sleep { + pinconf { + pins = "gpio22", "gpio23"; + drive-strength = <2>; + bias-disable; + }; +}; + +&sdc1_clk_on { + pinconf { + pins = "sdc1_clk"; + bias-disable; + drive-strength = <16>; + }; +}; + +&sdc1_clk_off { + pinconf { + pins = "sdc1_clk"; + bias-disable; + drive-strength = <2>; + }; +
[PATCH v2 04/10] arm64: dts: qcom: msm8916: drop unused board-specific nodes
These nodes reserve and configure some pins as GPIOs. They are not generic pinctrls, they actually belong to board files but they are not used by any other node, so just drop them altogether. Signed-off-by: Damien Riegel --- arch/arm64/boot/dts/qcom/msm8916-pins.dtsi | 52 -- 1 file changed, 52 deletions(-) diff --git a/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi b/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi index 10fc1fc90682..21f144c55638 100644 --- a/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi +++ b/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi @@ -505,32 +505,6 @@ }; }; - ext-codec-lines { - ext_codec_lines_act: lines_on { - pinmux { - function = "gpio"; - pins = "gpio67"; - }; - pinconf { - pins = "gpio67"; - drive-strength = <8>; - bias-disable; - output-high; - }; - }; - ext_codec_lines_sus: lines_off { - pinmux { - function = "gpio"; - pins = "gpio67"; - }; - pinconf { - pins = "gpio67"; - drive-strength = <2>; - bias-disable; - }; - }; - }; - cdc-pdm-lines { cdc_pdm_lines_act: pdm_lines_on { pinmux { @@ -703,32 +677,6 @@ }; }; - cross-conn-det { - cross_conn_det_act: lines_on { - pinmux { - function = "gpio"; - pins = "gpio120"; - }; - pinconf { - pins = "gpio120"; - drive-strength = <8>; - output-low; - bias-pull-down; - }; - }; - cross_conn_det_sus: lines_off { - pinmux { - function = "gpio"; - pins = "gpio120"; - }; - pinconf { - pins = "gpio120"; - drive-strength = <2>; - bias-disable; - }; - }; - }; - wcnss_pin_a: wcnss-active { pinmux { pins = "gpio40", "gpio41", "gpio42", "gpio43", "gpio44"; -- 2.15.0
[PATCH v2 09/10] arm64: dts: qcom: msm8916: normalize I2C and SPI nodes
The QUP core can be used either for I2C or SPI, so the same IP is mapped by a driver or the other. SPI bindings use a leading 0 for the start address and a size of 0x600, I2C bindings don't have the leading 0 and have a size 0x1000. To make them more similar, add the leading 0 to I2C bindings and changes the size to 0x500 for all of them, as this is the actual size of these blocks. Also align the second entry of the clocks array. Signed-off-by: Damien Riegel --- Changes in v2: - Set size to 0x500 arch/arm64/boot/dts/qcom/msm8916.dtsi | 24 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi index e16ba8334518..ac440f287633 100644 --- a/arch/arm64/boot/dts/qcom/msm8916.dtsi +++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi @@ -355,7 +355,7 @@ blsp_spi1: spi@78b5000 { compatible = "qcom,spi-qup-v2.2.1"; - reg = <0x078b5000 0x600>; + reg = <0x078b5000 0x500>; interrupts = ; clocks = <&gcc GCC_BLSP1_QUP1_SPI_APPS_CLK>, <&gcc GCC_BLSP1_AHB_CLK>; @@ -372,7 +372,7 @@ blsp_spi2: spi@78b6000 { compatible = "qcom,spi-qup-v2.2.1"; - reg = <0x078b6000 0x600>; + reg = <0x078b6000 0x500>; interrupts = ; clocks = <&gcc GCC_BLSP1_QUP2_SPI_APPS_CLK>, <&gcc GCC_BLSP1_AHB_CLK>; @@ -389,7 +389,7 @@ blsp_spi3: spi@78b7000 { compatible = "qcom,spi-qup-v2.2.1"; - reg = <0x078b7000 0x600>; + reg = <0x078b7000 0x500>; interrupts = ; clocks = <&gcc GCC_BLSP1_QUP3_SPI_APPS_CLK>, <&gcc GCC_BLSP1_AHB_CLK>; @@ -406,7 +406,7 @@ blsp_spi4: spi@78b8000 { compatible = "qcom,spi-qup-v2.2.1"; - reg = <0x078b8000 0x600>; + reg = <0x078b8000 0x500>; interrupts = ; clocks = <&gcc GCC_BLSP1_QUP4_SPI_APPS_CLK>, <&gcc GCC_BLSP1_AHB_CLK>; @@ -423,7 +423,7 @@ blsp_spi5: spi@78b9000 { compatible = "qcom,spi-qup-v2.2.1"; - reg = <0x078b9000 0x600>; + reg = <0x078b9000 0x500>; interrupts = ; clocks = <&gcc GCC_BLSP1_QUP5_SPI_APPS_CLK>, <&gcc GCC_BLSP1_AHB_CLK>; @@ -440,7 +440,7 @@ blsp_spi6: spi@78ba000 { compatible = "qcom,spi-qup-v2.2.1"; - reg = <0x078ba000 0x600>; + reg = <0x078ba000 0x500>; interrupts = ; clocks = <&gcc GCC_BLSP1_QUP6_SPI_APPS_CLK>, <&gcc GCC_BLSP1_AHB_CLK>; @@ -457,10 +457,10 @@ blsp_i2c2: i2c@78b6000 { compatible = "qcom,i2c-qup-v2.2.1"; - reg = <0x78b6000 0x1000>; + reg = <0x078b6000 0x500>; interrupts = ; clocks = <&gcc GCC_BLSP1_AHB_CLK>, - <&gcc GCC_BLSP1_QUP2_I2C_APPS_CLK>; +<&gcc GCC_BLSP1_QUP2_I2C_APPS_CLK>; clock-names = "iface", "core"; pinctrl-names = "default", "sleep"; pinctrl-0 = <&i2c2_default>; @@ -472,10 +472,10 @@ blsp_i2c4: i2c@78b8000 { compatible = "qcom,i2c-qup-v2.2.1"; - reg = <0x78b8000 0x1000>; + reg = <0x078b8000 0x500>; interrupts = ; clocks = <&gcc GCC_BLSP1_AHB_CLK>, - <&gcc GCC_BLSP1_QUP4_I2C_APPS_CLK>; +<&gcc GCC_BLSP1_QUP4_I2C_APPS_CLK>; clock-names = "iface", "core"; pinctrl-names = "default", "sleep"; pinctrl-0 = <&i2c4_default>; @@ -487,10 +487,10 @@ blsp_i2c6: i2c@78ba000 { co
[PATCH v2 01/10] arm64: dts: qcom: pm8916: fix wcd_codec indentation
Indentation did not respect kernel standards, so fix that for the usual indent with tabs, align with spaces. While at it, remove some empty lines before and after the closing parenthesis of this block. Signed-off-by: Damien Riegel Reviewed-by: Bjorn Andersson --- Changes in v2: - Added Reviewed-by Bjorn Andersson arch/arm64/boot/dts/qcom/pm8916.dtsi | 82 ++-- 1 file changed, 40 insertions(+), 42 deletions(-) diff --git a/arch/arm64/boot/dts/qcom/pm8916.dtsi b/arch/arm64/boot/dts/qcom/pm8916.dtsi index 53deebf9f515..d19f4cb9a5f3 100644 --- a/arch/arm64/boot/dts/qcom/pm8916.dtsi +++ b/arch/arm64/boot/dts/qcom/pm8916.dtsi @@ -96,47 +96,45 @@ #address-cells = <1>; #size-cells = <0>; -wcd_codec: codec@f000 { - compatible = "qcom,pm8916-wcd-analog-codec"; - reg = <0xf000 0x200>; - reg-names = "pmic-codec-core"; - clocks = <&gcc GCC_CODEC_DIGCODEC_CLK>; - clock-names = "mclk"; - interrupt-parent = <&spmi_bus>; - interrupts = <0x1 0xf0 0x0 IRQ_TYPE_NONE>, - <0x1 0xf0 0x1 IRQ_TYPE_NONE>, - <0x1 0xf0 0x2 IRQ_TYPE_NONE>, - <0x1 0xf0 0x3 IRQ_TYPE_NONE>, - <0x1 0xf0 0x4 IRQ_TYPE_NONE>, - <0x1 0xf0 0x5 IRQ_TYPE_NONE>, - <0x1 0xf0 0x6 IRQ_TYPE_NONE>, - <0x1 0xf0 0x7 IRQ_TYPE_NONE>, - <0x1 0xf1 0x0 IRQ_TYPE_NONE>, - <0x1 0xf1 0x1 IRQ_TYPE_NONE>, - <0x1 0xf1 0x2 IRQ_TYPE_NONE>, - <0x1 0xf1 0x3 IRQ_TYPE_NONE>, - <0x1 0xf1 0x4 IRQ_TYPE_NONE>, - <0x1 0xf1 0x5 IRQ_TYPE_NONE>; - interrupt-names = "cdc_spk_cnp_int", -"cdc_spk_clip_int", -"cdc_spk_ocp_int", -"mbhc_ins_rem_det1", -"mbhc_but_rel_det", -"mbhc_but_press_det", -"mbhc_ins_rem_det", -"mbhc_switch_int", -"cdc_ear_ocp_int", -"cdc_hphr_ocp_int", -"cdc_hphl_ocp_det", -"cdc_ear_cnp_int", -"cdc_hphr_cnp_int", -"cdc_hphl_cnp_int"; - vdd-cdc-io-supply = <&pm8916_l5>; - vdd-cdc-tx-rx-cx-supply = <&pm8916_l5>; - vdd-micbias-supply = <&pm8916_l13>; - #sound-dai-cells = <1>; - -}; - + wcd_codec: codec@f000 { + compatible = "qcom,pm8916-wcd-analog-codec"; + reg = <0xf000 0x200>; + reg-names = "pmic-codec-core"; + clocks = <&gcc GCC_CODEC_DIGCODEC_CLK>; + clock-names = "mclk"; + interrupt-parent = <&spmi_bus>; + interrupts = <0x1 0xf0 0x0 IRQ_TYPE_NONE>, +<0x1 0xf0 0x1 IRQ_TYPE_NONE>, +<0x1 0xf0 0x2 IRQ_TYPE_NONE>, +<0x1 0xf0 0x3 IRQ_TYPE_NONE>, +<0x1 0xf0 0x4 IRQ_TYPE_NONE>, +<0x1 0xf0 0x5 IRQ_TYPE_NONE>, +<0x1 0xf0 0x6 IRQ_TYPE_NONE>, +<0x1 0xf0 0x7 IRQ_TYPE_NONE>, +<0x1 0xf1 0x0 IRQ_TYPE_NONE>, +<0x1 0xf1 0x1 IRQ_TYPE_NONE>, +<0x1 0xf1 0x2 IRQ_TYPE_NONE>, +<0x1 0xf1 0x3 IRQ_TYPE_NONE>, +<0x1 0xf1 0x4 IRQ_TYPE_NONE>, +<0x1 0xf1 0x5 IRQ_TYPE_NONE>; + i
[PATCH v2 00/10] arm64: dts: qcom: dts improvements
The goal of this series was to add missing I2C bindings for BLSP_I2C1, BLSP_I2C3, and BLSP_I2C5. But while working on this, I noticed some styling issues and decided to tackle them in the same patchset, mostly because they touch the same files and the same people will be involved for the review. In this second version, I followed the recommandation of Bjorn Andersson and splitted pinmuxing and pinconf. The reason behind that is that the pinmuxing is common as functions cannot be routed to other pins, but pinconfs are board-specific. Damien Riegel (10): arm64: dts: qcom: pm8916: fix wcd_codec indentation arm64: dts: qcom: msm8916-pins: remove assignments to bias-disable arm64: dts: qcom: msm8916-pins: keep cdc_dmic pins in suspend mode arm64: dts: qcom: msm8916: drop unused board-specific nodes arm64: dts: qcom: apq8016-sbc: sort nodes alphabetically arm64: dts: qcom: msm8916: move pinconfs to board files arm64: dts: qcom: msm8916: drop remaining unused pinconfs arm64: dts: qcom: msm8916-pins: move sdhc2 cd node with its siblings arm64: dts: qcom: msm8916: normalize I2C and SPI nodes arm64: dts: qcom: msm8916: add nodes for i2c1, i2c3, i2c5 arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi | 446 - arch/arm64/boot/dts/qcom/msm8916-mtp.dtsi | 17 ++ arch/arm64/boot/dts/qcom/msm8916-pins.dtsi | 393 - arch/arm64/boot/dts/qcom/msm8916.dtsi | 69 - arch/arm64/boot/dts/qcom/pm8916.dtsi | 82 +++--- 5 files changed, 604 insertions(+), 403 deletions(-) -- 2.15.0
[PATCH v2 02/10] arm64: dts: qcom: msm8916-pins: remove assignments to bias-disable
Drop assignments to bias-disable as the documentation [1] states that this property doesn't take a value. Other occurrences of this property respect that. [1] Documentation/devicetree/bindings/pinctrl/qcom,msm8916-pinctrl.txt Signed-off-by: Damien Riegel Reviewed-by: Bjorn Andersson --- Changes in v2: - Added Reviewed-by Bjorn Andersson arch/arm64/boot/dts/qcom/msm8916-pins.dtsi | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi b/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi index 4cb0b5834143..c67ad8ed8b60 100644 --- a/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi +++ b/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi @@ -278,7 +278,7 @@ pinconf { pins = "gpio6", "gpio7"; drive-strength = <16>; - bias-disable = <0>; + bias-disable; }; }; @@ -290,7 +290,7 @@ pinconf { pins = "gpio6", "gpio7"; drive-strength = <2>; - bias-disable = <0>; + bias-disable; }; }; @@ -302,7 +302,7 @@ pinconf { pins = "gpio14", "gpio15"; drive-strength = <16>; - bias-disable = <0>; + bias-disable; }; }; @@ -314,7 +314,7 @@ pinconf { pins = "gpio14", "gpio15"; drive-strength = <2>; - bias-disable = <0>; + bias-disable; }; }; @@ -326,7 +326,7 @@ pinconf { pins = "gpio22", "gpio23"; drive-strength = <16>; - bias-disable = <0>; + bias-disable; }; }; @@ -338,7 +338,7 @@ pinconf { pins = "gpio22", "gpio23"; drive-strength = <2>; - bias-disable = <0>; + bias-disable; }; }; -- 2.15.0
[PATCH v2 05/10] arm64: dts: qcom: apq8016-sbc: sort nodes alphabetically
Also, it was using whitespaces for indentation on some lines, fix that while moving it. Signed-off-by: Damien Riegel --- arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi b/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi index d4b35d81a282..981450f50e10 100644 --- a/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi +++ b/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi @@ -544,14 +544,6 @@ }; }; -&wcd_codec { -status = "okay"; -clocks = <&gcc GCC_CODEC_DIGCODEC_CLK>; -clock-names = "mclk"; - qcom,mbhc-vthreshold-low = <75 150 237 450 500>; - qcom,mbhc-vthreshold-high = <75 150 237 450 500>; -}; - &smd_rpm_regulators { vdd_l1_l2_l3-supply = <&pm8916_s3>; vdd_l5-supply = <&pm8916_s3>; @@ -671,3 +663,11 @@ regulator-max-microvolt = <3337000>; }; }; + +&wcd_codec { + status = "okay"; + clocks = <&gcc GCC_CODEC_DIGCODEC_CLK>; + clock-names = "mclk"; + qcom,mbhc-vthreshold-low = <75 150 237 450 500>; + qcom,mbhc-vthreshold-high = <75 150 237 450 500>; +}; -- 2.15.0
[PATCH v2 08/10] arm64: dts: qcom: msm8916-pins: move sdhc2 cd node with its siblings
Nodes relative to the first sdhc node were interlaced with node of the second sdhc. Move sdhc2_cd_pin with its siblings to prevent that. Also rename the grouping node from sdhc2_cd_pin to pmx_sdc2_cd_pin, as "pmx_sdc" is the prefix used by other nodes. Signed-off-by: Damien Riegel --- arch/arm64/boot/dts/qcom/msm8916-pins.dtsi | 30 +++--- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi b/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi index c79301f204b7..7704ddecb6c4 100644 --- a/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi +++ b/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi @@ -194,21 +194,6 @@ }; }; - sdhc2_cd_pin { - sdc2_cd_on: cd_on { - pinmux { - function = "gpio"; - pins = "gpio38"; - }; - }; - sdc2_cd_off: cd_off { - pinmux { - function = "gpio"; - pins = "gpio38"; - }; - }; - }; - pmx_sdc1_clk { sdc1_clk_on: clk_on { pinmux { @@ -287,6 +272,21 @@ }; }; + pmx_sdc2_cd_pin { + sdc2_cd_on: cd_on { + pinmux { + function = "gpio"; + pins = "gpio38"; + }; + }; + sdc2_cd_off: cd_off { + pinmux { + function = "gpio"; + pins = "gpio38"; + }; + }; + }; + cdc-pdm-lines { cdc_pdm_lines_act: pdm_lines_on { pinmux { -- 2.15.0
[PATCH v2 07/10] arm64: dts: qcom: msm8916: drop remaining unused pinconfs
This commit drops pin configs that cannot be moved to board files as no boards use them. Signed-off-by: Damien Riegel --- arch/arm64/boot/dts/qcom/msm8916-pins.dtsi | 33 -- 1 file changed, 33 deletions(-) diff --git a/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi b/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi index 0790232c4654..c79301f204b7 100644 --- a/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi +++ b/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi @@ -311,26 +311,13 @@ pins = "gpio113", "gpio114", "gpio115", "gpio116"; }; - pinconf { - pins = "gpio113", "gpio114", "gpio115", - "gpio116"; - drive-strength = <8>; - bias-pull-none; - }; }; - ext_pri_tlmm_lines_sus: ext_pa_off { pinmux { function = "pri_mi2s"; pins = "gpio113", "gpio114", "gpio115", "gpio116"; }; - pinconf { - pins = "gpio113", "gpio114", "gpio115", - "gpio116"; - drive-strength = <2>; - bias-disable; - }; }; }; @@ -340,23 +327,12 @@ function = "pri_mi2s_ws"; pins = "gpio110"; }; - pinconf { - pins = "gpio110"; - drive-strength = <8>; - bias-pull-none; - }; }; - ext_pri_ws_sus: ext_pa_off { pinmux { function = "pri_mi2s_ws"; pins = "gpio110"; }; - pinconf { - pins = "gpio110"; - drive-strength = <2>; - bias-disable; - }; }; }; @@ -403,10 +379,6 @@ function = "dmic0_data"; pins = "gpio1"; }; - pinconf { - pins = "gpio0", "gpio1"; - drive-strength = <8>; - }; }; cdc_dmic_lines_sus: dmic_lines_off { pinmux_dmic0_clk { @@ -417,11 +389,6 @@ function = "dmic0_data"; pins = "gpio1"; }; - pinconf { - pins = "gpio0", "gpio1"; - drive-strength = <2>; - bias-disable; - }; }; }; -- 2.15.0
Re: [PATCH 4/4] arm64: dts: qcom: msm8916: add bindings for i2c1, i2c3, i2c5
On Fri, Nov 10, 2017 at 11:56:12PM -0800, Bjorn Andersson wrote: > On Thu 09 Nov 09:14 PST 2017, Damien Riegel wrote: > > > Hi Bjorn, > > > > > > On Thu, Nov 09, 2017 at 09:00:16AM -0800, Bjorn Andersson wrote: > > > On Wed 01 Nov 10:53 PDT 2017, Damien Riegel wrote: > > > > > > I think it's better to use the word "nodes" (add nodes...) > > > > Will reword that. > > > > > > > > > Signed-off-by: Damien Riegel > > > > --- > > > > arch/arm64/boot/dts/qcom/msm8916-pins.dtsi | 72 > > > > ++ > > > > arch/arm64/boot/dts/qcom/msm8916.dtsi | 45 +++ > > > > 2 files changed, 117 insertions(+) > > > > > > > > diff --git a/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi > > > > b/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi > > > > index c67ad8ed8b60..1cec5b30ed6e 100644 > > > > --- a/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi > > > > +++ b/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi > > > > @@ -270,6 +270,30 @@ > > > > }; > > > > }; > > > > > > > > + i2c1_default: i2c1_default { > > > > + pinmux { > > > > + function = "blsp_i2c1"; > > > > + pins = "gpio2", "gpio3"; > > > > + }; > > > > + pinconf { > > > > + pins = "gpio2", "gpio3"; > > > > + drive-strength = <16>; > > > > + bias-disable; > > > > + }; > > > > > > pinconf is typically board specific, so please leave these out from the > > > base dtsi. > > > > I don't mind dropping that, but pinconf for i2c{2,4,6} is already in > > msm8916-pins.dtsi, so we should either drop them all, or add pinconf for > > i2c{1,3,5} for consistency. > > You're right, this needs to be consistent. So I would like us to update > the existing nodes, > > > And if I read the pinctrl driver correctly, > > I2C cannot be routed to other pins, the only properties that users may > > want to modify are drive-strength and bias. Let me know which option you > > prefer. > > > > I discussed the matter with my colleagues and we concluded that we want > to keep the muxing in the platform.dtsi and move the specification of > the electrical properties (pinconf) to the board.dts(i). > > We did discuss having some "default values" in the platform file, but > pushing these down to the board file gives an indication to others that > these values are board specific. Okay that makes sense. I'll work on a patchset to do that. Regards, -- Damien
Re: [PATCH 4/4] arm64: dts: qcom: msm8916: add bindings for i2c1, i2c3, i2c5
Hi Bjorn, On Thu, Nov 09, 2017 at 09:00:16AM -0800, Bjorn Andersson wrote: > On Wed 01 Nov 10:53 PDT 2017, Damien Riegel wrote: > > I think it's better to use the word "nodes" (add nodes...) Will reword that. > > > Signed-off-by: Damien Riegel > > --- > > arch/arm64/boot/dts/qcom/msm8916-pins.dtsi | 72 > > ++ > > arch/arm64/boot/dts/qcom/msm8916.dtsi | 45 +++ > > 2 files changed, 117 insertions(+) > > > > diff --git a/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi > > b/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi > > index c67ad8ed8b60..1cec5b30ed6e 100644 > > --- a/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi > > +++ b/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi > > @@ -270,6 +270,30 @@ > > }; > > }; > > > > + i2c1_default: i2c1_default { > > + pinmux { > > + function = "blsp_i2c1"; > > + pins = "gpio2", "gpio3"; > > + }; > > + pinconf { > > + pins = "gpio2", "gpio3"; > > + drive-strength = <16>; > > + bias-disable; > > + }; > > pinconf is typically board specific, so please leave these out from the > base dtsi. I don't mind dropping that, but pinconf for i2c{2,4,6} is already in msm8916-pins.dtsi, so we should either drop them all, or add pinconf for i2c{1,3,5} for consistency. And if I read the pinctrl driver correctly, I2C cannot be routed to other pins, the only properties that users may want to modify are drive-strength and bias. Let me know which option you prefer. > > diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi > > b/arch/arm64/boot/dts/qcom/msm8916.dtsi > > index de25bd6070f5..bdc4cb6f66d4 100644 > > --- a/arch/arm64/boot/dts/qcom/msm8916.dtsi > > +++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi > > @@ -455,6 +455,21 @@ > > status = "disabled"; > > }; > > > > + blsp_i2c1: i2c@78b5000 { > > + compatible = "qcom,i2c-qup-v2.2.1"; > > + reg = <0x078b5000 0x600>; > > Size is 0x500. Will update that (and will also do that for patch 3) > > + interrupts = ; > > + clocks = <&gcc GCC_BLSP1_AHB_CLK>, > > +<&gcc GCC_BLSP1_QUP1_I2C_APPS_CLK>; > > + clock-names = "iface", "core"; > > + pinctrl-names = "default", "sleep"; > > + pinctrl-0 = <&i2c1_default>; > > + pinctrl-1 = <&i2c1_sleep>; > > Please omit the pinctrl-* properties from the base dtsi (when it's not > hard coded things for the platform). > > > + #address-cells = <1>; > > + #size-cells = <0>; > > + status = "disabled"; > > + }; > > + > > blsp_i2c2: i2c@78b6000 { > > compatible = "qcom,i2c-qup-v2.2.1"; > > reg = <0x078b6000 0x600>; > > Otherwise this looks good! Thank you for the review, -- Damien
[PATCH 0/4] arm64: dts: qcom: dts improvements
The goal of this series was to add missing I2C bindings for BLSP_I2C1, BLSP_I2C3, and BLSP_I2C5. But while working on this, I noticed some styling issues and decided to tackle them in the same patchset, mostly because they touch the same files and the same people will be involved for the review. Let me know if you'd rather see them sent separately. First patch reindents the wcd_codec block in pm8916.dtsi with respect to the kernel standards. Second patch and third patch makes existing I2C more in sync with the rest of the bindings by dropping assignments to a property that doesn't take a value, and by making their style matches thus of SPI bindings. Last patch add bindings of I2C cores 1, 3, and 5. Damien Riegel (4): arm64: dts: qcom: pm8916: fix wcd_codec indentation arm64: dts: qcom: msm8916-pins: remove assignments to bias-disable arm64: dts: qcom: msm8916: normalize I2C bindings arm64: dts: qcom: msm8916: add bindings for i2c1, i2c3, i2c5 arch/arm64/boot/dts/qcom/msm8916-pins.dtsi | 84 +++--- arch/arm64/boot/dts/qcom/msm8916.dtsi | 57 +--- arch/arm64/boot/dts/qcom/pm8916.dtsi | 82 ++--- 3 files changed, 169 insertions(+), 54 deletions(-) -- 2.15.0
[PATCH 1/4] arm64: dts: qcom: pm8916: fix wcd_codec indentation
Indentation did not respect kernel standards, so fix that for the usual indent with tabs, align with spaces. While at it, remove some empty lines before and after the closing parenthesis of this block. Signed-off-by: Damien Riegel --- arch/arm64/boot/dts/qcom/pm8916.dtsi | 82 ++-- 1 file changed, 40 insertions(+), 42 deletions(-) diff --git a/arch/arm64/boot/dts/qcom/pm8916.dtsi b/arch/arm64/boot/dts/qcom/pm8916.dtsi index 53deebf9f515..d19f4cb9a5f3 100644 --- a/arch/arm64/boot/dts/qcom/pm8916.dtsi +++ b/arch/arm64/boot/dts/qcom/pm8916.dtsi @@ -96,47 +96,45 @@ #address-cells = <1>; #size-cells = <0>; -wcd_codec: codec@f000 { - compatible = "qcom,pm8916-wcd-analog-codec"; - reg = <0xf000 0x200>; - reg-names = "pmic-codec-core"; - clocks = <&gcc GCC_CODEC_DIGCODEC_CLK>; - clock-names = "mclk"; - interrupt-parent = <&spmi_bus>; - interrupts = <0x1 0xf0 0x0 IRQ_TYPE_NONE>, - <0x1 0xf0 0x1 IRQ_TYPE_NONE>, - <0x1 0xf0 0x2 IRQ_TYPE_NONE>, - <0x1 0xf0 0x3 IRQ_TYPE_NONE>, - <0x1 0xf0 0x4 IRQ_TYPE_NONE>, - <0x1 0xf0 0x5 IRQ_TYPE_NONE>, - <0x1 0xf0 0x6 IRQ_TYPE_NONE>, - <0x1 0xf0 0x7 IRQ_TYPE_NONE>, - <0x1 0xf1 0x0 IRQ_TYPE_NONE>, - <0x1 0xf1 0x1 IRQ_TYPE_NONE>, - <0x1 0xf1 0x2 IRQ_TYPE_NONE>, - <0x1 0xf1 0x3 IRQ_TYPE_NONE>, - <0x1 0xf1 0x4 IRQ_TYPE_NONE>, - <0x1 0xf1 0x5 IRQ_TYPE_NONE>; - interrupt-names = "cdc_spk_cnp_int", -"cdc_spk_clip_int", -"cdc_spk_ocp_int", -"mbhc_ins_rem_det1", -"mbhc_but_rel_det", -"mbhc_but_press_det", -"mbhc_ins_rem_det", -"mbhc_switch_int", -"cdc_ear_ocp_int", -"cdc_hphr_ocp_int", -"cdc_hphl_ocp_det", -"cdc_ear_cnp_int", -"cdc_hphr_cnp_int", -"cdc_hphl_cnp_int"; - vdd-cdc-io-supply = <&pm8916_l5>; - vdd-cdc-tx-rx-cx-supply = <&pm8916_l5>; - vdd-micbias-supply = <&pm8916_l13>; - #sound-dai-cells = <1>; - -}; - + wcd_codec: codec@f000 { + compatible = "qcom,pm8916-wcd-analog-codec"; + reg = <0xf000 0x200>; + reg-names = "pmic-codec-core"; + clocks = <&gcc GCC_CODEC_DIGCODEC_CLK>; + clock-names = "mclk"; + interrupt-parent = <&spmi_bus>; + interrupts = <0x1 0xf0 0x0 IRQ_TYPE_NONE>, +<0x1 0xf0 0x1 IRQ_TYPE_NONE>, +<0x1 0xf0 0x2 IRQ_TYPE_NONE>, +<0x1 0xf0 0x3 IRQ_TYPE_NONE>, +<0x1 0xf0 0x4 IRQ_TYPE_NONE>, +<0x1 0xf0 0x5 IRQ_TYPE_NONE>, +<0x1 0xf0 0x6 IRQ_TYPE_NONE>, +<0x1 0xf0 0x7 IRQ_TYPE_NONE>, +<0x1 0xf1 0x0 IRQ_TYPE_NONE>, +<0x1 0xf1 0x1 IRQ_TYPE_NONE>, +<0x1 0xf1 0x2 IRQ_TYPE_NONE>, +<0x1 0xf1 0x3 IRQ_TYPE_NONE>, +<0x1 0xf1 0x4 IRQ_TYPE_NONE>, +<0x1 0xf1 0x5 IRQ_TYPE_NONE>; + interrupt-names = "cdc_spk_cnp_int", +
[PATCH 4/4] arm64: dts: qcom: msm8916: add bindings for i2c1, i2c3, i2c5
Signed-off-by: Damien Riegel --- arch/arm64/boot/dts/qcom/msm8916-pins.dtsi | 72 ++ arch/arm64/boot/dts/qcom/msm8916.dtsi | 45 +++ 2 files changed, 117 insertions(+) diff --git a/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi b/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi index c67ad8ed8b60..1cec5b30ed6e 100644 --- a/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi +++ b/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi @@ -270,6 +270,30 @@ }; }; + i2c1_default: i2c1_default { + pinmux { + function = "blsp_i2c1"; + pins = "gpio2", "gpio3"; + }; + pinconf { + pins = "gpio2", "gpio3"; + drive-strength = <16>; + bias-disable; + }; + }; + + i2c1_sleep: i2c1_sleep { + pinmux { + function = "gpio"; + pins = "gpio2", "gpio3"; + }; + pinconf { + pins = "gpio2", "gpio3"; + drive-strength = <2>; + bias-disable; + }; + }; + i2c2_default: i2c2_default { pinmux { function = "blsp_i2c2"; @@ -294,6 +318,30 @@ }; }; + i2c3_default: i2c3_default { + pinmux { + function = "blsp_i2c3"; + pins = "gpio10", "gpio11"; + }; + pinconf { + pins = "gpio10", "gpio11"; + drive-strength = <16>; + bias-disable; + }; + }; + + i2c3_sleep: i2c3_sleep { + pinmux { + function = "gpio"; + pins = "gpio10", "gpio11"; + }; + pinconf { + pins = "gpio10", "gpio11"; + drive-strength = <2>; + bias-disable; + }; + }; + i2c4_default: i2c4_default { pinmux { function = "blsp_i2c4"; @@ -318,6 +366,30 @@ }; }; + i2c5_default: i2c5_default { + pinmux { + function = "blsp_i2c5"; + pins = "gpio18", "gpio19"; + }; + pinconf { + pins = "gpio18", "gpio19"; + drive-strength = <16>; + bias-disable; + }; + }; + + i2c5_sleep: i2c5_sleep { + pinmux { + function = "gpio"; + pins = "gpio18", "gpio19"; + }; + pinconf { + pins = "gpio18", "gpio19"; + drive-strength = <2>; + bias-disable; + }; + }; + i2c6_default: i2c6_default { pinmux { function = "blsp_i2c6"; diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi index de25bd6070f5..bdc4cb6f66d4 100644 --- a/arch/arm64/boot/dts/qcom/msm8916.dtsi +++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi @@ -455,6 +455,21 @@ status = "disabled"; }; + blsp_i2c1: i2c@78b5000 { + compatible = "qcom,i2c-qup-v2.2.1"; + reg = <0x078b5000 0x600>; + interrupts = ; + clocks = <&gcc GCC_BLSP1_AHB_CLK>, +<&gcc GCC_BLSP1_QUP1_I2C_APPS_CLK>; + clock-names = "iface", "core"; + pinctrl-names = "default", "sleep"; + pinctrl-0 = <&i2c1_default>; + pinctrl-1 = <&i2c1_sleep>; + #address-cells = <1>; + #size-cells = <0>; + status = "disabled"; + }; + blsp_i2c2: i2c@78b6000 { compatible = "qcom,i2c-qup-v2.2.1"; reg = <0x078b6000 0x600>; @@ -470,6 +485,21 @@ status = "disabled"; }; + blsp_i2c3: i2c@78b7000 { +
[PATCH 3/4] arm64: dts: qcom: msm8916: normalize I2C bindings
The QUP core can be used either for I2C or SPI, so the same IP is mapped by a driver or the other. SPI bindings use a leading 0 for the start address and a size of 0x600, I2C bindings don't have the leading 0 and have a size 0x1000. To make them more similar, add the leading 0 to I2C bindings and changes the size to 0x600, as the driver only accesses registers up to address 0x408. Also align the second entry of the clocks array. Signed-off-by: Damien Riegel --- arch/arm64/boot/dts/qcom/msm8916.dtsi | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi index e16ba8334518..de25bd6070f5 100644 --- a/arch/arm64/boot/dts/qcom/msm8916.dtsi +++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi @@ -457,10 +457,10 @@ blsp_i2c2: i2c@78b6000 { compatible = "qcom,i2c-qup-v2.2.1"; - reg = <0x78b6000 0x1000>; + reg = <0x078b6000 0x600>; interrupts = ; clocks = <&gcc GCC_BLSP1_AHB_CLK>, - <&gcc GCC_BLSP1_QUP2_I2C_APPS_CLK>; +<&gcc GCC_BLSP1_QUP2_I2C_APPS_CLK>; clock-names = "iface", "core"; pinctrl-names = "default", "sleep"; pinctrl-0 = <&i2c2_default>; @@ -472,10 +472,10 @@ blsp_i2c4: i2c@78b8000 { compatible = "qcom,i2c-qup-v2.2.1"; - reg = <0x78b8000 0x1000>; + reg = <0x078b8000 0x600>; interrupts = ; clocks = <&gcc GCC_BLSP1_AHB_CLK>, - <&gcc GCC_BLSP1_QUP4_I2C_APPS_CLK>; +<&gcc GCC_BLSP1_QUP4_I2C_APPS_CLK>; clock-names = "iface", "core"; pinctrl-names = "default", "sleep"; pinctrl-0 = <&i2c4_default>; @@ -487,10 +487,10 @@ blsp_i2c6: i2c@78ba000 { compatible = "qcom,i2c-qup-v2.2.1"; - reg = <0x78ba000 0x1000>; + reg = <0x078ba000 0x600>; interrupts = ; clocks = <&gcc GCC_BLSP1_AHB_CLK>, - <&gcc GCC_BLSP1_QUP6_I2C_APPS_CLK>; +<&gcc GCC_BLSP1_QUP6_I2C_APPS_CLK>; clock-names = "iface", "core"; pinctrl-names = "default", "sleep"; pinctrl-0 = <&i2c6_default>; -- 2.15.0
[PATCH 2/4] arm64: dts: qcom: msm8916-pins: remove assignments to bias-disable
Drop assignments to bias-disable as the documentation [1] states that this property doesn't take a value. Other occurrences of this property respect that. [1] Documentation/devicetree/bindings/pinctrl/qcom,msm8916-pinctrl.txt Signed-off-by: Damien Riegel --- arch/arm64/boot/dts/qcom/msm8916-pins.dtsi | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi b/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi index 4cb0b5834143..c67ad8ed8b60 100644 --- a/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi +++ b/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi @@ -278,7 +278,7 @@ pinconf { pins = "gpio6", "gpio7"; drive-strength = <16>; - bias-disable = <0>; + bias-disable; }; }; @@ -290,7 +290,7 @@ pinconf { pins = "gpio6", "gpio7"; drive-strength = <2>; - bias-disable = <0>; + bias-disable; }; }; @@ -302,7 +302,7 @@ pinconf { pins = "gpio14", "gpio15"; drive-strength = <16>; - bias-disable = <0>; + bias-disable; }; }; @@ -314,7 +314,7 @@ pinconf { pins = "gpio14", "gpio15"; drive-strength = <2>; - bias-disable = <0>; + bias-disable; }; }; @@ -326,7 +326,7 @@ pinconf { pins = "gpio22", "gpio23"; drive-strength = <16>; - bias-disable = <0>; + bias-disable; }; }; @@ -338,7 +338,7 @@ pinconf { pins = "gpio22", "gpio23"; drive-strength = <2>; - bias-disable = <0>; + bias-disable; }; }; -- 2.15.0
i2c: qup: missing stop bit and messages
Hi, While testing a gas gauge, we encountered intermittent i2c issues with the APQ8016. Sometimes, the i2c controller seems to send incomplete messages, or doesn't send some messages. To read values from the gas gauge, this group of two messages is sent: - one write message of 1 byte, with the gas gauge register to be read - one read message of 2 bytes, for the reply from the device Once in a while, there is an odd sequence: - First, the very last tick of the clock of the read message is shorter, and the stop bit is missing. - The following group of messages is incomplete, only the write message is sent. So on the scope, we observe the following timeline: ... [ok] [ok] [ok] [missing stop bit] [only write message] Here are captures of what we see. These are three consecutive queries of the gas gauge: - successful query: https://postimg.org/gallery/215zrvd9o/ - missing stop bit: https://postimg.org/gallery/26xn101d8/ - incomplete group: https://postimg.org/image/2cvghepi0b/ (a start is indicated by a green marker, a stop by a red one) As we use i2c3, for which dts bindings are not yet upstreamed, below are the additions in our device tree. It's slightly incomplete, but note that blsp_i2c3 is enabled and blsp_spi3 disabled. Maybe there is an issue with that particular i2c core? Regards, Damien diff --git a/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi b/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi index 899f2b28a9c9..a303a4c8cad4 100644 --- a/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi +++ b/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi @@ -294,6 +294,30 @@ }; }; + i2c3_default: i2c3_default { + pinmux { + function = "blsp_i2c3"; + pins = "gpio10", "gpio11"; + }; + pinconf { + pins = "gpio10", "gpio11"; + drive-strength = <16>; + bias-disable = <0>; + }; + }; + + i2c3_sleep: i2c3_sleep { + pinmux { + function = "gpio"; + pins = "gpio10", "gpio11"; + }; + pinconf { + pins = "gpio10", "gpio11"; + drive-strength = <2>; + bias-disable = <0>; + }; + }; + i2c4_default: i2c4_default { pinmux { function = "blsp_i2c4"; diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi index f6091aafc984..a08e2b4894fd 100644 --- a/arch/arm64/boot/dts/qcom/msm8916.dtsi +++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi @@ -618,6 +618,21 @@ status = "disabled"; }; + blsp_i2c3: i2c@78b7000 { + compatible = "qcom,i2c-qup-v2.2.1"; + reg = <0x78b7000 0x1000>; + interrupts = ; + clocks = <&gcc GCC_BLSP1_AHB_CLK>, + <&gcc GCC_BLSP1_QUP3_I2C_APPS_CLK>; + clock-names = "iface", "core"; + pinctrl-names = "default", "sleep"; + pinctrl-0 = <&i2c3_default>; + pinctrl-1 = <&i2c3_sleep>; + #address-cells = <1>; + #size-cells = <0>; + status = "disabled"; + }; + blsp_i2c4: i2c@78b8000 { compatible = "qcom,i2c-qup-v2.2.1"; reg = <0x78b8000 0x1000>; diff --git a/arch/arm64/boot/dts/qcom/apq8016-sbc-centaur.dts b/arch/arm64/boot/dts/qcom/apq8016-sbc-centaur.dts index c96aebcb046c..10928bf31189 100644 --- a/arch/arm64/boot/dts/qcom/apq8016-sbc-centaur.dts +++ b/arch/arm64/boot/dts/qcom/apq8016-sbc-centaur.dts @@ -29,6 +29,13 @@ /* use i2c3 instead of spi3, for gas gauge */ &blsp_i2c3 { status = "okay"; + + bq27510@55 { + compatible = "ti,bq27510g2"; + reg = <0x55>; + pinctrl-names = "default"; + pinctrl-0 = <&fuelgauge_default>; + }; }; &blsp_i2c4 { @@ -155,6 +162,18 @@ }; }; + fuelgauge_default: fuelgauge_default { + pinmux { + function = "gpio"; + pins = "gpio9"; + }; + pinconf { + pins = "gpio9"; + input-enable; + bias-pull-up; + }; + }; + /* overwrite this node to use gpio 13 instead of 110 */ keypad_default: keypad_default { pinmux {
wcn36xx: connection monitoring issue
Hi, I'm working on a board derived from the DragonBoard-410c (apq8016 + wcn3620) and noticed an issue with WiFi connection monitoring. When I get out of the range of the access point, the device stays associated, no matter how far I go or how long I wait. The chip on this board is the WCN3620, and the driver sets IEEE80211_HW_CONNECTION_MONITOR at drivers/net/wireless/ath/wcn36xx/main.c:1127, so the hardware is supposed to do the monitoring, but it looks like it doesn't. If I comment out this flag (snippet at the end of the email), the correct behaviour is observed. Has anyone else noticed that issue? Thanks, Damien diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c index 35bd50bcbbd5..2aa1643d15bf 100644 --- a/drivers/net/wireless/ath/wcn36xx/main.c +++ b/drivers/net/wireless/ath/wcn36xx/main.c @@ -1124,7 +1124,6 @@ static int wcn36xx_init_ieee80211(struct wcn36xx *wcn) ieee80211_hw_set(wcn->hw, TIMING_BEACON_ONLY); ieee80211_hw_set(wcn->hw, AMPDU_AGGREGATION); - ieee80211_hw_set(wcn->hw, CONNECTION_MONITOR); ieee80211_hw_set(wcn->hw, SUPPORTS_PS); ieee80211_hw_set(wcn->hw, SIGNAL_DBM); ieee80211_hw_set(wcn->hw, HAS_RATE_CONTROL);
[PATCH] Input: tca8418 - enable interrupt after it has been requested
Currently, enabling keypad interrupts is one of the first operations done on the keypad, even before the interrupt is requested, so there is a small time window where the keypad can fire interrupts but the driver is not yet ready to handle them. It's fine for level interrupts because they will be handled anyway, but not so much for edge ones. This commit modifies and moves the function in charge of configuring the keypad. Enabling interrupts is now the last thing done on the keypad, and after the interrupt has been requested by the driver. Writing to the config register was also used to determine if the device was indeed present on the bus or not, this has been replaced by reading the lock/event count register to keep the same functionality. Signed-off-by: Damien Riegel --- I originally made this patch on top of: 800e3b9a6801 Input: drop owner assignment from i2c_driver But I only compile-tested it on top of input/next. The issue was spotted thanks to a bootloader that left the keypad configured, only with interrupts disabled. So any button pressed or released during Linux's boot was queued and triggered an interrupt as soon as the driver enabled interrupts. It would really be appreciated if someone could give this patch a try. drivers/input/keyboard/tca8418_keypad.c | 29 + 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/drivers/input/keyboard/tca8418_keypad.c b/drivers/input/keyboard/tca8418_keypad.c index e37e335e406f..6da607d3b811 100644 --- a/drivers/input/keyboard/tca8418_keypad.c +++ b/drivers/input/keyboard/tca8418_keypad.c @@ -234,14 +234,7 @@ static irqreturn_t tca8418_irq_handler(int irq, void *dev_id) static int tca8418_configure(struct tca8418_keypad *keypad_data, u32 rows, u32 cols) { - int reg, error; - - /* Write config register, if this fails assume device not present */ - error = tca8418_write_byte(keypad_data, REG_CFG, - CFG_INT_CFG | CFG_OVR_FLOW_IEN | CFG_KE_IEN); - if (error < 0) - return -ENODEV; - + int reg, error = 0; /* Assemble a mask for row and column registers */ reg = ~(~0 << rows); @@ -257,6 +250,12 @@ static int tca8418_configure(struct tca8418_keypad *keypad_data, error |= tca8418_write_byte(keypad_data, REG_DEBOUNCE_DIS2, reg >> 8); error |= tca8418_write_byte(keypad_data, REG_DEBOUNCE_DIS3, reg >> 16); + if (error) + return error; + + error = tca8418_write_byte(keypad_data, REG_CFG, + CFG_INT_CFG | CFG_OVR_FLOW_IEN | CFG_KE_IEN); + return error; } @@ -268,6 +267,7 @@ static int tca8418_keypad_probe(struct i2c_client *client, struct input_dev *input; u32 rows = 0, cols = 0; int error, row_shift, max_keys; + u8 reg; /* Check i2c driver capabilities */ if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE)) { @@ -301,10 +301,10 @@ static int tca8418_keypad_probe(struct i2c_client *client, keypad_data->client = client; keypad_data->row_shift = row_shift; - /* Initialize the chip or fail if chip isn't present */ - error = tca8418_configure(keypad_data, rows, cols); - if (error < 0) - return error; + /* Read key lock register, if this fails assume device not present */ + error = tca8418_read_byte(keypad_data, REG_KEY_LCK_EC, ®); + if (error) + return -ENODEV; /* Configure input device */ input = devm_input_allocate_device(dev); @@ -340,6 +340,11 @@ static int tca8418_keypad_probe(struct i2c_client *client, return error; } + /* Initialize the chip */ + error = tca8418_configure(keypad_data, rows, cols); + if (error < 0) + return error; + error = input_register_device(input); if (error) { dev_err(dev, "Unable to register input device, error: %d\n", -- 2.14.2
Re: [PATCH v2] ASoC: codecs: msm8916-wcd-analog: configure micbias in mbhc setup
On Tue, Oct 03, 2017 at 09:42:18PM +0100, Srinivas Kandagatla wrote: > > > On 03/10/17 14:27, Damien Riegel wrote: > > The very first time a headset is plugged in, detection is unreliable > > because bias hasn't been configured yet, it's done once a mechanical > > insertion interrupt has been triggered, so following insertions (and > > thus detections) are not affected. > > > > To fix the very first detection, the bias must also be configured in the > > function that setup the MBHC. Move pm8916_wcd_setup_mbhc after > > pm8916_mbhc_configure_bias to avoid a forward declaration. > > > > Signed-off-by: Damien Riegel > > --- > > Changes in v2: > > - squash the two patches in one patch only > > I do not see the squashed patches here?? > It looks like its just the first patch. pm8916_mbhc_configure_bias is now called in pm8916_wcd_setup_mbhc. See inlined comment. > Thanks, > srini > > > > sound/soc/codecs/msm8916-wcd-analog.c | 94 > > +++ > > 1 file changed, 50 insertions(+), 44 deletions(-) > > > > diff --git a/sound/soc/codecs/msm8916-wcd-analog.c > > b/sound/soc/codecs/msm8916-wcd-analog.c > > index f562f2d86907..71494e9dbdcb 100644 > > --- a/sound/soc/codecs/msm8916-wcd-analog.c > > +++ b/sound/soc/codecs/msm8916-wcd-analog.c > > @@ -443,50 +443,6 @@ static int pm8916_wcd_analog_enable_micbias_int1(struct > > wcd->micbias1_cap_mode); > > } > > -static void pm8916_wcd_setup_mbhc(struct pm8916_wcd_analog_priv *wcd) > > -{ > > - struct snd_soc_codec *codec = wcd->codec; > > - u32 plug_type = 0; > > - u32 int_en_mask; > > - > > - snd_soc_write(codec, CDC_A_MBHC_DET_CTL_1, > > - CDC_A_MBHC_DET_CTL_L_DET_EN | > > - CDC_A_MBHC_DET_CTL_MECH_DET_TYPE_INSERTION | > > - CDC_A_MBHC_DET_CTL_MIC_CLAMP_CTL_AUTO | > > - CDC_A_MBHC_DET_CTL_MBHC_BIAS_EN); > > - > > - if (wcd->hphl_jack_type_normally_open) > > - plug_type |= CDC_A_HPHL_PLUG_TYPE_NO; > > - > > - if (wcd->gnd_jack_type_normally_open) > > - plug_type |= CDC_A_GND_PLUG_TYPE_NO; > > - > > - snd_soc_write(codec, CDC_A_MBHC_DET_CTL_2, > > - CDC_A_MBHC_DET_CTL_HS_L_DET_PULL_UP_CTRL_I_3P0 | > > - CDC_A_MBHC_DET_CTL_HS_L_DET_COMPA_CTRL_V0P9_VDD | > > - plug_type | > > - CDC_A_MBHC_DET_CTL_HPHL_100K_TO_GND_EN); > > - > > - > > - snd_soc_write(codec, CDC_A_MBHC_DBNC_TIMER, > > - CDC_A_MBHC_DBNC_TIMER_INSREM_DBNC_T_256_MS | > > - CDC_A_MBHC_DBNC_TIMER_BTN_DBNC_T_16MS); > > - > > - /* enable MBHC clock */ > > - snd_soc_update_bits(codec, CDC_D_CDC_DIG_CLK_CTL, > > - DIG_CLK_CTL_D_MBHC_CLK_EN_MASK, > > - DIG_CLK_CTL_D_MBHC_CLK_EN); > > - > > - int_en_mask = MBHC_SWITCH_INT; > > - if (wcd->mbhc_btn_enabled) > > - int_en_mask |= MBHC_BUTTON_PRESS_DET | MBHC_BUTTON_RELEASE_DET; > > - > > - snd_soc_update_bits(codec, CDC_D_INT_EN_CLR, int_en_mask, 0); > > - snd_soc_update_bits(codec, CDC_D_INT_EN_SET, int_en_mask, int_en_mask); > > - wcd->mbhc_btn0_released = false; > > - wcd->detect_accessory_type = true; > > -} > > - > > static int pm8916_mbhc_configure_bias(struct pm8916_wcd_analog_priv *priv, > > bool micbias2_enabled) > > { > > @@ -534,6 +490,56 @@ static int pm8916_mbhc_configure_bias(struct > > pm8916_wcd_analog_priv *priv, > > return 0; > > } > > +static void pm8916_wcd_setup_mbhc(struct pm8916_wcd_analog_priv *wcd) > > +{ > > + struct snd_soc_codec *codec = wcd->codec; > > + bool micbias_enabled = false; > > + u32 plug_type = 0; > > + u32 int_en_mask; > > + > > + snd_soc_write(codec, CDC_A_MBHC_DET_CTL_1, > > + CDC_A_MBHC_DET_CTL_L_DET_EN | > > + CDC_A_MBHC_DET_CTL_MECH_DET_TYPE_INSERTION | > > + CDC_A_MBHC_DET_CTL_MIC_CLAMP_CTL_AUTO | > > + CDC_A_MBHC_DET_CTL_MBHC_BIAS_EN); > > + > > + if (wcd->hphl_jack_type_normally_open) > > + plug_type |= CDC_A_HPHL_PLUG_TYPE_NO; > > + > > + if (wcd->gnd_jack_type_normally_open) > > + plug_type |= CDC_A_GND_PLUG_TYPE_NO; > > + > > + snd_soc_write(codec, CDC_A_MBHC_DET_CTL_2, > > +
[PATCH v2] ASoC: codecs: msm8916-wcd-analog: configure micbias in mbhc setup
The very first time a headset is plugged in, detection is unreliable because bias hasn't been configured yet, it's done once a mechanical insertion interrupt has been triggered, so following insertions (and thus detections) are not affected. To fix the very first detection, the bias must also be configured in the function that setup the MBHC. Move pm8916_wcd_setup_mbhc after pm8916_mbhc_configure_bias to avoid a forward declaration. Signed-off-by: Damien Riegel --- Changes in v2: - squash the two patches in one patch only sound/soc/codecs/msm8916-wcd-analog.c | 94 +++ 1 file changed, 50 insertions(+), 44 deletions(-) diff --git a/sound/soc/codecs/msm8916-wcd-analog.c b/sound/soc/codecs/msm8916-wcd-analog.c index f562f2d86907..71494e9dbdcb 100644 --- a/sound/soc/codecs/msm8916-wcd-analog.c +++ b/sound/soc/codecs/msm8916-wcd-analog.c @@ -443,50 +443,6 @@ static int pm8916_wcd_analog_enable_micbias_int1(struct wcd->micbias1_cap_mode); } -static void pm8916_wcd_setup_mbhc(struct pm8916_wcd_analog_priv *wcd) -{ - struct snd_soc_codec *codec = wcd->codec; - u32 plug_type = 0; - u32 int_en_mask; - - snd_soc_write(codec, CDC_A_MBHC_DET_CTL_1, - CDC_A_MBHC_DET_CTL_L_DET_EN | - CDC_A_MBHC_DET_CTL_MECH_DET_TYPE_INSERTION | - CDC_A_MBHC_DET_CTL_MIC_CLAMP_CTL_AUTO | - CDC_A_MBHC_DET_CTL_MBHC_BIAS_EN); - - if (wcd->hphl_jack_type_normally_open) - plug_type |= CDC_A_HPHL_PLUG_TYPE_NO; - - if (wcd->gnd_jack_type_normally_open) - plug_type |= CDC_A_GND_PLUG_TYPE_NO; - - snd_soc_write(codec, CDC_A_MBHC_DET_CTL_2, - CDC_A_MBHC_DET_CTL_HS_L_DET_PULL_UP_CTRL_I_3P0 | - CDC_A_MBHC_DET_CTL_HS_L_DET_COMPA_CTRL_V0P9_VDD | - plug_type | - CDC_A_MBHC_DET_CTL_HPHL_100K_TO_GND_EN); - - - snd_soc_write(codec, CDC_A_MBHC_DBNC_TIMER, - CDC_A_MBHC_DBNC_TIMER_INSREM_DBNC_T_256_MS | - CDC_A_MBHC_DBNC_TIMER_BTN_DBNC_T_16MS); - - /* enable MBHC clock */ - snd_soc_update_bits(codec, CDC_D_CDC_DIG_CLK_CTL, - DIG_CLK_CTL_D_MBHC_CLK_EN_MASK, - DIG_CLK_CTL_D_MBHC_CLK_EN); - - int_en_mask = MBHC_SWITCH_INT; - if (wcd->mbhc_btn_enabled) - int_en_mask |= MBHC_BUTTON_PRESS_DET | MBHC_BUTTON_RELEASE_DET; - - snd_soc_update_bits(codec, CDC_D_INT_EN_CLR, int_en_mask, 0); - snd_soc_update_bits(codec, CDC_D_INT_EN_SET, int_en_mask, int_en_mask); - wcd->mbhc_btn0_released = false; - wcd->detect_accessory_type = true; -} - static int pm8916_mbhc_configure_bias(struct pm8916_wcd_analog_priv *priv, bool micbias2_enabled) { @@ -534,6 +490,56 @@ static int pm8916_mbhc_configure_bias(struct pm8916_wcd_analog_priv *priv, return 0; } +static void pm8916_wcd_setup_mbhc(struct pm8916_wcd_analog_priv *wcd) +{ + struct snd_soc_codec *codec = wcd->codec; + bool micbias_enabled = false; + u32 plug_type = 0; + u32 int_en_mask; + + snd_soc_write(codec, CDC_A_MBHC_DET_CTL_1, + CDC_A_MBHC_DET_CTL_L_DET_EN | + CDC_A_MBHC_DET_CTL_MECH_DET_TYPE_INSERTION | + CDC_A_MBHC_DET_CTL_MIC_CLAMP_CTL_AUTO | + CDC_A_MBHC_DET_CTL_MBHC_BIAS_EN); + + if (wcd->hphl_jack_type_normally_open) + plug_type |= CDC_A_HPHL_PLUG_TYPE_NO; + + if (wcd->gnd_jack_type_normally_open) + plug_type |= CDC_A_GND_PLUG_TYPE_NO; + + snd_soc_write(codec, CDC_A_MBHC_DET_CTL_2, + CDC_A_MBHC_DET_CTL_HS_L_DET_PULL_UP_CTRL_I_3P0 | + CDC_A_MBHC_DET_CTL_HS_L_DET_COMPA_CTRL_V0P9_VDD | + plug_type | + CDC_A_MBHC_DET_CTL_HPHL_100K_TO_GND_EN); + + + snd_soc_write(codec, CDC_A_MBHC_DBNC_TIMER, + CDC_A_MBHC_DBNC_TIMER_INSREM_DBNC_T_256_MS | + CDC_A_MBHC_DBNC_TIMER_BTN_DBNC_T_16MS); + + /* enable MBHC clock */ + snd_soc_update_bits(codec, CDC_D_CDC_DIG_CLK_CTL, + DIG_CLK_CTL_D_MBHC_CLK_EN_MASK, + DIG_CLK_CTL_D_MBHC_CLK_EN); + + if (snd_soc_read(codec, CDC_A_MICB_2_EN) & CDC_A_MICB_2_EN_ENABLE) + micbias_enabled = true; + + pm8916_mbhc_configure_bias(wcd, micbias_enabled); + + int_en_mask = MBHC_SWITCH_INT; + if (wcd->mbhc_btn_enabled) + int_en_mask |= MBHC_BUTTON_PRESS_DET | MBHC_BUTTON_RELEASE_DET; + + snd_soc_update_bits(codec, CDC_D_INT_EN_CLR, int_en_mask, 0); + snd_soc_update_bits(codec, CDC_D_INT_EN_SET, int_en_mask, int_en_mask); +
[PATCH v1 0/2] Fix first detection issue with headset
This series fix an issue with headset being misidentified on the very first time a device is plugged in by calling the function that configures the bias when the MBHC is setup. I've split this series into two patches because pm8916_wcd_setup_mbhc needs to be moved a few lines below to avoid forward declaring it. It is overkill but that was not part of the code snippet that Srinivas acked, and I wanted to be extra sure not to misattribute an Acked-by. Damien Riegel (2): ASoC: codecs: msm8916-wcd-analog: move pm8916_wcd_setup_mbhc ASoC: codecs: msm8916-wcd-analog: configure micbias in mbhc setup sound/soc/codecs/msm8916-wcd-analog.c | 94 +++ 1 file changed, 50 insertions(+), 44 deletions(-) -- 2.14.1
[PATCH v1 1/2] ASoC: codecs: msm8916-wcd-analog: move pm8916_wcd_setup_mbhc
In preparation of a fix for the initial detection of headset, pm8916_wcd_setup_mbhc is moved to be defined after pm8916_mbhc_configure_bias instead of before. Signed-off-by: Damien Riegel --- sound/soc/codecs/msm8916-wcd-analog.c | 88 +-- 1 file changed, 44 insertions(+), 44 deletions(-) diff --git a/sound/soc/codecs/msm8916-wcd-analog.c b/sound/soc/codecs/msm8916-wcd-analog.c index f562f2d86907..9f08d998a79b 100644 --- a/sound/soc/codecs/msm8916-wcd-analog.c +++ b/sound/soc/codecs/msm8916-wcd-analog.c @@ -443,50 +443,6 @@ static int pm8916_wcd_analog_enable_micbias_int1(struct wcd->micbias1_cap_mode); } -static void pm8916_wcd_setup_mbhc(struct pm8916_wcd_analog_priv *wcd) -{ - struct snd_soc_codec *codec = wcd->codec; - u32 plug_type = 0; - u32 int_en_mask; - - snd_soc_write(codec, CDC_A_MBHC_DET_CTL_1, - CDC_A_MBHC_DET_CTL_L_DET_EN | - CDC_A_MBHC_DET_CTL_MECH_DET_TYPE_INSERTION | - CDC_A_MBHC_DET_CTL_MIC_CLAMP_CTL_AUTO | - CDC_A_MBHC_DET_CTL_MBHC_BIAS_EN); - - if (wcd->hphl_jack_type_normally_open) - plug_type |= CDC_A_HPHL_PLUG_TYPE_NO; - - if (wcd->gnd_jack_type_normally_open) - plug_type |= CDC_A_GND_PLUG_TYPE_NO; - - snd_soc_write(codec, CDC_A_MBHC_DET_CTL_2, - CDC_A_MBHC_DET_CTL_HS_L_DET_PULL_UP_CTRL_I_3P0 | - CDC_A_MBHC_DET_CTL_HS_L_DET_COMPA_CTRL_V0P9_VDD | - plug_type | - CDC_A_MBHC_DET_CTL_HPHL_100K_TO_GND_EN); - - - snd_soc_write(codec, CDC_A_MBHC_DBNC_TIMER, - CDC_A_MBHC_DBNC_TIMER_INSREM_DBNC_T_256_MS | - CDC_A_MBHC_DBNC_TIMER_BTN_DBNC_T_16MS); - - /* enable MBHC clock */ - snd_soc_update_bits(codec, CDC_D_CDC_DIG_CLK_CTL, - DIG_CLK_CTL_D_MBHC_CLK_EN_MASK, - DIG_CLK_CTL_D_MBHC_CLK_EN); - - int_en_mask = MBHC_SWITCH_INT; - if (wcd->mbhc_btn_enabled) - int_en_mask |= MBHC_BUTTON_PRESS_DET | MBHC_BUTTON_RELEASE_DET; - - snd_soc_update_bits(codec, CDC_D_INT_EN_CLR, int_en_mask, 0); - snd_soc_update_bits(codec, CDC_D_INT_EN_SET, int_en_mask, int_en_mask); - wcd->mbhc_btn0_released = false; - wcd->detect_accessory_type = true; -} - static int pm8916_mbhc_configure_bias(struct pm8916_wcd_analog_priv *priv, bool micbias2_enabled) { @@ -534,6 +490,50 @@ static int pm8916_mbhc_configure_bias(struct pm8916_wcd_analog_priv *priv, return 0; } +static void pm8916_wcd_setup_mbhc(struct pm8916_wcd_analog_priv *wcd) +{ + struct snd_soc_codec *codec = wcd->codec; + u32 plug_type = 0; + u32 int_en_mask; + + snd_soc_write(codec, CDC_A_MBHC_DET_CTL_1, + CDC_A_MBHC_DET_CTL_L_DET_EN | + CDC_A_MBHC_DET_CTL_MECH_DET_TYPE_INSERTION | + CDC_A_MBHC_DET_CTL_MIC_CLAMP_CTL_AUTO | + CDC_A_MBHC_DET_CTL_MBHC_BIAS_EN); + + if (wcd->hphl_jack_type_normally_open) + plug_type |= CDC_A_HPHL_PLUG_TYPE_NO; + + if (wcd->gnd_jack_type_normally_open) + plug_type |= CDC_A_GND_PLUG_TYPE_NO; + + snd_soc_write(codec, CDC_A_MBHC_DET_CTL_2, + CDC_A_MBHC_DET_CTL_HS_L_DET_PULL_UP_CTRL_I_3P0 | + CDC_A_MBHC_DET_CTL_HS_L_DET_COMPA_CTRL_V0P9_VDD | + plug_type | + CDC_A_MBHC_DET_CTL_HPHL_100K_TO_GND_EN); + + + snd_soc_write(codec, CDC_A_MBHC_DBNC_TIMER, + CDC_A_MBHC_DBNC_TIMER_INSREM_DBNC_T_256_MS | + CDC_A_MBHC_DBNC_TIMER_BTN_DBNC_T_16MS); + + /* enable MBHC clock */ + snd_soc_update_bits(codec, CDC_D_CDC_DIG_CLK_CTL, + DIG_CLK_CTL_D_MBHC_CLK_EN_MASK, + DIG_CLK_CTL_D_MBHC_CLK_EN); + + int_en_mask = MBHC_SWITCH_INT; + if (wcd->mbhc_btn_enabled) + int_en_mask |= MBHC_BUTTON_PRESS_DET | MBHC_BUTTON_RELEASE_DET; + + snd_soc_update_bits(codec, CDC_D_INT_EN_CLR, int_en_mask, 0); + snd_soc_update_bits(codec, CDC_D_INT_EN_SET, int_en_mask, int_en_mask); + wcd->mbhc_btn0_released = false; + wcd->detect_accessory_type = true; +} + static int pm8916_wcd_analog_enable_micbias_int2(struct snd_soc_dapm_widget *w, struct snd_kcontrol -- 2.14.1
[PATCH v1 2/2] ASoC: codecs: msm8916-wcd-analog: configure micbias in mbhc setup
The very first time a headset is plugged in, detection is unreliable because bias hasn't been configured yet, it's done once a mechanical insertion interrupt has been triggered, so following insertions (and thus detections) are not affected. To fix the very first detection, the bias must also be configured in the function that setup the MBHC. Signed-off-by: Damien Riegel Acked-by: Srinivas Kandagatla --- sound/soc/codecs/msm8916-wcd-analog.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/sound/soc/codecs/msm8916-wcd-analog.c b/sound/soc/codecs/msm8916-wcd-analog.c index 9f08d998a79b..71494e9dbdcb 100644 --- a/sound/soc/codecs/msm8916-wcd-analog.c +++ b/sound/soc/codecs/msm8916-wcd-analog.c @@ -493,6 +493,7 @@ static int pm8916_mbhc_configure_bias(struct pm8916_wcd_analog_priv *priv, static void pm8916_wcd_setup_mbhc(struct pm8916_wcd_analog_priv *wcd) { struct snd_soc_codec *codec = wcd->codec; + bool micbias_enabled = false; u32 plug_type = 0; u32 int_en_mask; @@ -524,6 +525,11 @@ static void pm8916_wcd_setup_mbhc(struct pm8916_wcd_analog_priv *wcd) DIG_CLK_CTL_D_MBHC_CLK_EN_MASK, DIG_CLK_CTL_D_MBHC_CLK_EN); + if (snd_soc_read(codec, CDC_A_MICB_2_EN) & CDC_A_MICB_2_EN_ENABLE) + micbias_enabled = true; + + pm8916_mbhc_configure_bias(wcd, micbias_enabled); + int_en_mask = MBHC_SWITCH_INT; if (wcd->mbhc_btn_enabled) int_en_mask |= MBHC_BUTTON_PRESS_DET | MBHC_BUTTON_RELEASE_DET; -- 2.14.1
Re: [RFC] ASoC: codecs: msm8916-wcd-analog: use btn0 released detection
Mark, I think Srinivas' Ack was only for the snippet quoted below, not for the original patch of this thread. I don't know if you still want to take that patch without his Ack or not. On Thu, Sep 28, 2017 at 11:18:26AM -0700, Srinivas Kandagatla wrote: > > > On 19/09/17 06:22, Damien Riegel wrote: > > Definitely. I also noticed an issue with the first very detection > > because the micbias will only be set the first time the driver gets a > > mechanical insertion interrupt. Following snippet seems to solve the > > problem: > Does that mean that you do not need the "ASoC: codecs: msm8916-wcd-analog: > use btn0 released detection" patch anymore? No, configuring the bias in setup_mbhc only fixes an issue with the very first detection, we are dealing with two different issues. But I haven't had the time recently to capture the signal on our board to show you how our hardware behaves. > Below changes seems to fine as it is. > > Acked-by: Srinivas Kandagatla Thanks, I'll resend a proper patch with a real commit message then. -- Damien > > > > diff --git a/sound/soc/codecs/msm8916-wcd-analog.c > > b/sound/soc/codecs/msm8916-wcd-analog.c > > index f562f2d86907..5045dabd9ea9 100644 > > --- a/sound/soc/codecs/msm8916-wcd-analog.c > > +++ b/sound/soc/codecs/msm8916-wcd-analog.c > > @@ -446,6 +446,7 @@ static int pm8916_wcd_analog_enable_micbias_int1(struct > > static void pm8916_wcd_setup_mbhc(struct pm8916_wcd_analog_priv *wcd) > > { > > struct snd_soc_codec *codec = wcd->codec; > > + bool micbias_enabled = false; > > u32 plug_type = 0; > > u32 int_en_mask; > > > > @@ -477,6 +478,11 @@ static void pm8916_wcd_setup_mbhc(struct > > pm8916_wcd_analog_priv *wcd) > > DIG_CLK_CTL_D_MBHC_CLK_EN_MASK, > > DIG_CLK_CTL_D_MBHC_CLK_EN); > > > > + if (snd_soc_read(codec, CDC_A_MICB_2_EN) & CDC_A_MICB_2_EN_ENABLE) > > + micbias_enabled = true; > > + > > + pm8916_mbhc_configure_bias(priv, micbias_enabled); > > + > > int_en_mask = MBHC_SWITCH_INT; > > if (wcd->mbhc_btn_enabled) > > int_en_mask |= MBHC_BUTTON_PRESS_DET | > > MBHC_BUTTON_RELEASE_DET; > >
Re: [RFC] ASoC: codecs: msm8916-wcd-analog: use btn0 released detection
On Tue, Sep 19, 2017 at 01:11:47PM +0100, Mark Brown wrote: > On Mon, Sep 18, 2017 at 10:08:04AM +0100, Srinivas Kandagatla wrote: > > On 13/09/17 21:43, Damien Riegel wrote: > > > msm8916-wcd-analog uses button0 to differentiate between headphone and > > > headset. Under some circumstances, button pressed and released > > > interrupts are not fired as the driver expects it. > > > This is what we need to understand to find a right solution, > > I would like to understand on what is the difference in the hw layout. I asked the hardware team and the design is exactly the same, but we use different mechanical parts (ie. the jack connector). We've started comparing electrical signals between "Android on Intrinsyc's eval kit" vs. "Linux on our device", just to get a broad idea of what might be happening, and today we'll compare the two hardware with the same software on it. I'll get back to you as soon as I have more information. > These sorts of problems can also occur because of differences in test > process - pressure at different angles, different speeds of insertion > and a million other things. Definitely. I also noticed an issue with the first very detection because the micbias will only be set the first time the driver gets a mechanical insertion interrupt. Following snippet seems to solve the problem: diff --git a/sound/soc/codecs/msm8916-wcd-analog.c b/sound/soc/codecs/msm8916-wcd-analog.c index f562f2d86907..5045dabd9ea9 100644 --- a/sound/soc/codecs/msm8916-wcd-analog.c +++ b/sound/soc/codecs/msm8916-wcd-analog.c @@ -446,6 +446,7 @@ static int pm8916_wcd_analog_enable_micbias_int1(struct static void pm8916_wcd_setup_mbhc(struct pm8916_wcd_analog_priv *wcd) { struct snd_soc_codec *codec = wcd->codec; + bool micbias_enabled = false; u32 plug_type = 0; u32 int_en_mask; @@ -477,6 +478,11 @@ static void pm8916_wcd_setup_mbhc(struct pm8916_wcd_analog_priv *wcd) DIG_CLK_CTL_D_MBHC_CLK_EN_MASK, DIG_CLK_CTL_D_MBHC_CLK_EN); + if (snd_soc_read(codec, CDC_A_MICB_2_EN) & CDC_A_MICB_2_EN_ENABLE) + micbias_enabled = true; + + pm8916_mbhc_configure_bias(priv, micbias_enabled); + int_en_mask = MBHC_SWITCH_INT; if (wcd->mbhc_btn_enabled) int_en_mask |= MBHC_BUTTON_PRESS_DET | MBHC_BUTTON_RELEASE_DET;
[RFC] ASoC: codecs: msm8916-wcd-analog: use btn0 released detection
msm8916-wcd-analog uses button0 to differentiate between headphone and headset. Under some circumstances, button pressed and released interrupts are not fired as the driver expects it. For instance, with some connectors, there are spurious button-pressed interrupts when unplugging a headphone, without the corresponding button-released interrupt. But the codec always alternates between button pressed and released interrupts, it cannot fire two interrupts of the same kind in a row. That means that when the headphone is plugged back, only a button-released interrupt will be fired instead of pressed then released. This causes the driver to report headphone as headset. By changing the logic and relying on button 0 release interrupt, the driver could be made more robust for connectors that differ from the one used on the Dragonboard's audio mezzanine. Signed-off-by: Damien Riegel --- sound/soc/codecs/msm8916-wcd-analog.c | 18 -- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/sound/soc/codecs/msm8916-wcd-analog.c b/sound/soc/codecs/msm8916-wcd-analog.c index 549c269acc7d..f562f2d86907 100644 --- a/sound/soc/codecs/msm8916-wcd-analog.c +++ b/sound/soc/codecs/msm8916-wcd-analog.c @@ -285,7 +285,7 @@ struct pm8916_wcd_analog_priv { u16 codec_version; boolmbhc_btn_enabled; /* special event to detect accessory type */ - boolmbhc_btn0_pressed; + int mbhc_btn0_released; booldetect_accessory_type; struct clk *mclk; struct snd_soc_codec *codec; @@ -483,7 +483,7 @@ static void pm8916_wcd_setup_mbhc(struct pm8916_wcd_analog_priv *wcd) snd_soc_update_bits(codec, CDC_D_INT_EN_CLR, int_en_mask, 0); snd_soc_update_bits(codec, CDC_D_INT_EN_SET, int_en_mask, int_en_mask); - wcd->mbhc_btn0_pressed = false; + wcd->mbhc_btn0_released = false; wcd->detect_accessory_type = true; } @@ -950,7 +950,7 @@ static irqreturn_t mbhc_btn_release_irq_handler(int irq, void *arg) /* check if its BTN0 thats released */ if ((val != -1) && !(val & CDC_A_MBHC_RESULT_1_BTN_RESULT_MASK)) - priv->mbhc_btn0_pressed = false; + priv->mbhc_btn0_released = true; } else { snd_soc_jack_report(priv->jack, 0, btn_mask); @@ -983,9 +983,7 @@ static irqreturn_t mbhc_btn_press_irq_handler(int irq, void *arg) break; case 0x0: /* handle BTN_0 specially for type detection */ - if (priv->detect_accessory_type) - priv->mbhc_btn0_pressed = true; - else + if (!priv->detect_accessory_type) snd_soc_jack_report(priv->jack, SND_JACK_BTN_0, btn_mask); break; @@ -1029,19 +1027,19 @@ static irqreturn_t pm8916_mbhc_switch_irq_handler(int irq, void *arg) * both press and release event received then its * a headset. */ - if (priv->mbhc_btn0_pressed) + if (priv->mbhc_btn0_released) snd_soc_jack_report(priv->jack, - SND_JACK_HEADPHONE, hs_jack_mask); + SND_JACK_HEADSET, hs_jack_mask); else snd_soc_jack_report(priv->jack, - SND_JACK_HEADSET, hs_jack_mask); + SND_JACK_HEADPHONE, hs_jack_mask); priv->detect_accessory_type = false; } else { /* removal */ snd_soc_jack_report(priv->jack, 0, hs_jack_mask); priv->detect_accessory_type = true; - priv->mbhc_btn0_pressed = false; + priv->mbhc_btn0_released = false; } return IRQ_HANDLED; -- 2.14.1
Re: [alsa-devel] [PATCH v3 3/5] ASoC: codecs: msm8916-wcd-analog: add MBHC support
On Mon, Aug 14, 2017 at 05:34:53PM +0100, Srinivas Kandagatla wrote: > > > On 14/08/17 15:12, Damien Riegel wrote: > > Hi, > > > > > > On Thu, Aug 10, 2017 at 09:33:29AM -0400, Damien Riegel wrote: > > > Hi, > > > > > > On Thu, Aug 10, 2017 at 11:02:34AM +0100, Srinivas Kandagatla wrote: > > > > Hi Damien, > > > > Thanks for testing. > > > > > > > > On 09/08/17 22:10, Damien Riegel wrote: > > > > > Hi Srinivas, > > > > > > > > > > On Wed, Aug 09, 2017 at 06:49:25PM +0200, > > > > > srinivas.kandaga...@linaro.org wrote: > > > > > > From: Srinivas Kandagatla > > > > > > > > > > > > MBHC (MultiButton Headset Control) support is available in pm8921 > > > > > > in two > > > > > > blocks, one to detect mechanical headset insertion and removal and > > > > > > other > > > > > > block to support headset type detection and 5 button detection and > > > > > > othe > > > > > > features like impedance calculation. > > > > > > > > > > > > This patch adds support to: > > > > > > 1> Support to NC and NO type of headset Jacks. > > > > > > 2> Mechanical insertion and detection of headset jack. > > > > > > 3> Detect a 3 pole Headphone and a 4 pole Headset. > > > > > > 4> Detect 5 buttons. > > > > > > > > > > > > Tested it on DB410c with Audio Mezz board with 4 pole and 3 pole > > > > > > headset/headphones. > > > > > > > > > > I have the same issue with this patchset, KEY_MEDIA is being reported > > > > > when unplugging a headset. I added a few traces and what I observe is > > > > Am unable to reproduce the same issue, I tried atleast 6 different mix > > > > of > > > > headset/headphones. > > > > here is my evtest log: http://paste.ubuntu.com/25282592/ > > > > > > > > Could you explain bit more about your setup, so that I can try to > > > > reproduce > > > > the same issue. > > > > > > > > My setup is DB410c + Audio Mezz board > > > > https://www.arrow.com/en/products/audiomezz/seeed-technology-limited > > > > > > I'm using a hardware based on Intrinsyc Open-Q 410 Development Kit: > > > https://www.intrinsyc.com/snapdragon-embedded-development-kits/snapdragon-410-development-kit/ > > > > > > We use the same SoM with a different carrier board, but connections to > > > the jack connector are routed the same way on both carrier boards. > > > > > > > > that the "button pressed" irq is fired when unplugging, but no "button > > > > > released" irq follows. To get a "button released" irq, I need to > > > > > either > > > > > plug a headset, or plug-and-unplug headphones. > > > > > > > > > > So basically, detection fails because we don't get the "button > > > > > pressed" > > > > > irq prior to the mechanical switch irq. Do you know what could explain > > > > > the MBHC not firing the "button released" irq when unplugging headset? > > > > > > > > > Can you also share output of evetest and /proc/interrupts so that i can > > > > see > > > > if the extra logic of masking btn0 is creating the issue, we can also > > > > try > > > > removing the btn0 accessory detect logic and see if we get correct > > > > button > > > > press/releases. > > > > > > It's definitely what's causing the headphones to be reported as headset, > > > but not what's causing the interrupt not to be triggered in the first > > > place. I don't have access to the hardware right now, so I'll provide > > > you with logs next week. > > > > I did the test, and addded the content of /proc/interrupts. > > > > https://paste.ubuntu.com/25312443/ > > > > As you can see, after "[INSERT HEADPHONE]", we only get "mbhc switch > > irq", but not "mbhc btn press irq". Let me know if you need more > > details. > > > > As I said before, I don't see that 100% of the time, but I don't do > > anything special to trigger it either (like pulling the cable e
Re: [alsa-devel] [PATCH v3 3/5] ASoC: codecs: msm8916-wcd-analog: add MBHC support
Hi, On Thu, Aug 10, 2017 at 09:33:29AM -0400, Damien Riegel wrote: > Hi, > > On Thu, Aug 10, 2017 at 11:02:34AM +0100, Srinivas Kandagatla wrote: > > Hi Damien, > > Thanks for testing. > > > > On 09/08/17 22:10, Damien Riegel wrote: > > > Hi Srinivas, > > > > > > On Wed, Aug 09, 2017 at 06:49:25PM +0200, srinivas.kandaga...@linaro.org > > > wrote: > > > > From: Srinivas Kandagatla > > > > > > > > MBHC (MultiButton Headset Control) support is available in pm8921 in two > > > > blocks, one to detect mechanical headset insertion and removal and other > > > > block to support headset type detection and 5 button detection and othe > > > > features like impedance calculation. > > > > > > > > This patch adds support to: > > > > 1> Support to NC and NO type of headset Jacks. > > > > 2> Mechanical insertion and detection of headset jack. > > > > 3> Detect a 3 pole Headphone and a 4 pole Headset. > > > > 4> Detect 5 buttons. > > > > > > > > Tested it on DB410c with Audio Mezz board with 4 pole and 3 pole > > > > headset/headphones. > > > > > > I have the same issue with this patchset, KEY_MEDIA is being reported > > > when unplugging a headset. I added a few traces and what I observe is > > Am unable to reproduce the same issue, I tried atleast 6 different mix of > > headset/headphones. > > here is my evtest log: http://paste.ubuntu.com/25282592/ > > > > Could you explain bit more about your setup, so that I can try to reproduce > > the same issue. > > > > My setup is DB410c + Audio Mezz board > > https://www.arrow.com/en/products/audiomezz/seeed-technology-limited > > I'm using a hardware based on Intrinsyc Open-Q 410 Development Kit: > https://www.intrinsyc.com/snapdragon-embedded-development-kits/snapdragon-410-development-kit/ > > We use the same SoM with a different carrier board, but connections to > the jack connector are routed the same way on both carrier boards. > > > > that the "button pressed" irq is fired when unplugging, but no "button > > > released" irq follows. To get a "button released" irq, I need to either > > > plug a headset, or plug-and-unplug headphones. > > > > > > So basically, detection fails because we don't get the "button pressed" > > > irq prior to the mechanical switch irq. Do you know what could explain > > > the MBHC not firing the "button released" irq when unplugging headset? > > > > > Can you also share output of evetest and /proc/interrupts so that i can see > > if the extra logic of masking btn0 is creating the issue, we can also try > > removing the btn0 accessory detect logic and see if we get correct button > > press/releases. > > It's definitely what's causing the headphones to be reported as headset, > but not what's causing the interrupt not to be triggered in the first > place. I don't have access to the hardware right now, so I'll provide > you with logs next week. I did the test, and addded the content of /proc/interrupts. https://paste.ubuntu.com/25312443/ As you can see, after "[INSERT HEADPHONE]", we only get "mbhc switch irq", but not "mbhc btn press irq". Let me know if you need more details. As I said before, I don't see that 100% of the time, but I don't do anything special to trigger it either (like pulling the cable extra slowly or whatnot), it just happens. Thank you, -- Damien
Re: [PATCH v3 3/5] ASoC: codecs: msm8916-wcd-analog: add MBHC support
Hi, On Thu, Aug 10, 2017 at 11:02:34AM +0100, Srinivas Kandagatla wrote: > Hi Damien, > Thanks for testing. > > On 09/08/17 22:10, Damien Riegel wrote: > > Hi Srinivas, > > > > On Wed, Aug 09, 2017 at 06:49:25PM +0200, srinivas.kandaga...@linaro.org > > wrote: > > > From: Srinivas Kandagatla > > > > > > MBHC (MultiButton Headset Control) support is available in pm8921 in two > > > blocks, one to detect mechanical headset insertion and removal and other > > > block to support headset type detection and 5 button detection and othe > > > features like impedance calculation. > > > > > > This patch adds support to: > > > 1> Support to NC and NO type of headset Jacks. > > > 2> Mechanical insertion and detection of headset jack. > > > 3> Detect a 3 pole Headphone and a 4 pole Headset. > > > 4> Detect 5 buttons. > > > > > > Tested it on DB410c with Audio Mezz board with 4 pole and 3 pole > > > headset/headphones. > > > > I have the same issue with this patchset, KEY_MEDIA is being reported > > when unplugging a headset. I added a few traces and what I observe is > Am unable to reproduce the same issue, I tried atleast 6 different mix of > headset/headphones. > here is my evtest log: http://paste.ubuntu.com/25282592/ > > Could you explain bit more about your setup, so that I can try to reproduce > the same issue. > > My setup is DB410c + Audio Mezz board > https://www.arrow.com/en/products/audiomezz/seeed-technology-limited I'm using a hardware based on Intrinsyc Open-Q 410 Development Kit: https://www.intrinsyc.com/snapdragon-embedded-development-kits/snapdragon-410-development-kit/ We use the same SoM with a different carrier board, but connections to the jack connector are routed the same way on both carrier boards. > > that the "button pressed" irq is fired when unplugging, but no "button > > released" irq follows. To get a "button released" irq, I need to either > > plug a headset, or plug-and-unplug headphones. > > > > So basically, detection fails because we don't get the "button pressed" > > irq prior to the mechanical switch irq. Do you know what could explain > > the MBHC not firing the "button released" irq when unplugging headset? > > > Can you also share output of evetest and /proc/interrupts so that i can see > if the extra logic of masking btn0 is creating the issue, we can also try > removing the btn0 accessory detect logic and see if we get correct button > press/releases. It's definitely what's causing the headphones to be reported as headset, but not what's causing the interrupt not to be triggered in the first place. I don't have access to the hardware right now, so I'll provide you with logs next week. Thanks, -- Damien
Re: [PATCH v3 3/5] ASoC: codecs: msm8916-wcd-analog: add MBHC support
Hi Srinivas, On Wed, Aug 09, 2017 at 06:49:25PM +0200, srinivas.kandaga...@linaro.org wrote: > From: Srinivas Kandagatla > > MBHC (MultiButton Headset Control) support is available in pm8921 in two > blocks, one to detect mechanical headset insertion and removal and other > block to support headset type detection and 5 button detection and othe > features like impedance calculation. > > This patch adds support to: > 1> Support to NC and NO type of headset Jacks. > 2> Mechanical insertion and detection of headset jack. > 3> Detect a 3 pole Headphone and a 4 pole Headset. > 4> Detect 5 buttons. > > Tested it on DB410c with Audio Mezz board with 4 pole and 3 pole > headset/headphones. I have the same issue with this patchset, KEY_MEDIA is being reported when unplugging a headset. I added a few traces and what I observe is that the "button pressed" irq is fired when unplugging, but no "button released" irq follows. To get a "button released" irq, I need to either plug a headset, or plug-and-unplug headphones. So basically, detection fails because we don't get the "button pressed" irq prior to the mechanical switch irq. Do you know what could explain the MBHC not firing the "button released" irq when unplugging headset? Thanks, -- Damien
Re: [PATCH v2 4/6] ASoC: codecs: msm8916-wcd-analog: add MBHC support
On Thu, Aug 03, 2017 at 12:11:04PM +0100, Srinivas Kandagatla wrote: > > > On 03/08/17 00:33, Damien Riegel wrote: > > Hi Srinivas, > > > > > > On Wed, Aug 02, 2017 at 07:09:28PM +0200, srinivas.kandaga...@linaro.org > > wrote: > > > From: Srinivas Kandagatla > > > > > > MBHC (MultiButton Headset Control) support is available in pm8921 in two > > > blocks, one to detect mechanical headset insertion and removal and other > > > block to support headset type detection and 5 button detection and othe > > > features like impedance calculation. > > > > > > This patch adds support to: > > > 1> Support to NC and NO type of headset Jacks. > > > 2> Mechanical insertion and detection of headset jack. > > > 3> Detect a 3 pole Headphone and a 4 pole Headset. > > > 4> Detect 5 buttons. > > > > > > Tested it on DB410c with Audio Mezz board with 4 pole and 3 pole > > > headset/headphones. > > > > > Good news! This version has better results. I still have an issue where > > a headphone is reported as a headset when I do this sequence: > > Hi Damien, > > Thanks for testing.. > > > > 1. connect headset > > 2. disconnect headset > > 3. connect headphone > > > Following headphone connections/disconnections are reported correctly. > > Note that I don't press the media key, it's wrongly detected when I pull > > off the cable. > > If you do step 3 slowly there is a chance that we get KEY_MEDIA events, as > headphone is a 3 pole, during hp inserting we would end up shorting the MIC > and GND lines on the connector, as 3-pole connectors have longer GND > pole/connector length, this action is same as pressing MEDIA KEY. > > The logic in the driver uses this MEDIA KEY press release as advantage to > detect the accessory type before hs insertion, but this logic could break if > we are inserting headphone slowly, which is not a normal usecase. > > Any solution to this issue can be always break if we are doing slow > insertion of hs or we endup making the driver complex. I will think about > this once again and see how other drivers handle this usecase. Understood, and you're right we should keep this driver as simple as possible. I get the KEY_MEDIA event when doing step 2 (disconnecting headset), and I understand why it's doing that (and I think it's okay if we don't filter out that spurious event), but we should at least get a KEY_MEDIA release event at the same time as SW_{MICROPHONE,HEADPHONE}_INSERT removal. It doesn't really matter if I disconnect it fast or slowly, sometimes I'll get the KEY_MEDIA, some other times I won't. Shouldn't mbhc_btn0_pressed be reset to false when physical removal is detected? I think that would solve the issue of headphone being detected as headset. Regards, -- Damien
Re: [PATCH v2 4/6] ASoC: codecs: msm8916-wcd-analog: add MBHC support
Hi Srinivas, On Wed, Aug 02, 2017 at 07:09:28PM +0200, srinivas.kandaga...@linaro.org wrote: > From: Srinivas Kandagatla > > MBHC (MultiButton Headset Control) support is available in pm8921 in two > blocks, one to detect mechanical headset insertion and removal and other > block to support headset type detection and 5 button detection and othe > features like impedance calculation. > > This patch adds support to: > 1> Support to NC and NO type of headset Jacks. > 2> Mechanical insertion and detection of headset jack. > 3> Detect a 3 pole Headphone and a 4 pole Headset. > 4> Detect 5 buttons. > > Tested it on DB410c with Audio Mezz board with 4 pole and 3 pole > headset/headphones. Good news! This version has better results. I still have an issue where a headphone is reported as a headset when I do this sequence: 1. connect headset 2. disconnect headset 3. connect headphone Following headphone connections/disconnections are reported correctly. Note that I don't press the media key, it's wrongly detected when I pull off the cable. evtest logs: Plugging headset: Event: time 10181.936746, type 5 (EV_SW), code 2 (SW_HEADPHONE_INSERT), value 1 Event: time 10181.936746, type 5 (EV_SW), code 4 (SW_MICROPHONE_INSERT), value 1 Event: time 10181.936746, -- SYN_REPORT Unplugging headset: Event: time 10183.432797, type 1 (EV_KEY), code 226 (KEY_MEDIA), value 1 Event: time 10183.432797, -- SYN_REPORT Event: time 10183.871029, type 1 (EV_KEY), code 226 (KEY_MEDIA), value 0 Event: time 10183.871029, -- SYN_REPORT Event: time 10183.971521, type 1 (EV_KEY), code 226 (KEY_MEDIA), value 1 Event: time 10183.971521, -- SYN_REPORT Event: time 10184.249429, type 5 (EV_SW), code 2 (SW_HEADPHONE_INSERT), value 0 Event: time 10184.249429, type 5 (EV_SW), code 4 (SW_MICROPHONE_INSERT), value 0 Event: time 10184.249429, -- SYN_REPORT Plugin headphone: Event: time 10190.033748, type 5 (EV_SW), code 2 (SW_HEADPHONE_INSERT), value 1 Event: time 10190.033748, type 5 (EV_SW), code 4 (SW_MICROPHONE_INSERT), value 1 Event: time 10190.033748, -- SYN_REPORT Unplugging headphone: Event: time 10191.267473, type 1 (EV_KEY), code 226 (KEY_MEDIA), value 0 Event: time 10191.267473, -- SYN_REPORT Event: time 10191.548238, type 5 (EV_SW), code 2 (SW_HEADPHONE_INSERT), value 0 Event: time 10191.548238, type 5 (EV_SW), code 4 (SW_MICROPHONE_INSERT), value 0 Event: time 10191.548238, -- SYN_REPORT Plugging headphone: Event: time 10194.342217, type 5 (EV_SW), code 2 (SW_HEADPHONE_INSERT), value 1 Event: time 10194.342217, -- SYN_REPORT Unplugging headphone: Event: time 10195.446049, type 5 (EV_SW), code 2 (SW_HEADPHONE_INSERT), value 0 Event: time 10195.446049, -- SYN_REPORT --- Also, the microphone is not reported when cable is connected and a button is pressed down. That might be a bit of a corner case, so I don't think it's worth complexifying the driver to handle that. A few additional comments inline. > Signed-off-by: Srinivas Kandagatla > --- > .../bindings/sound/qcom,msm8916-wcd-analog.txt | 17 +- > sound/soc/codecs/msm8916-wcd-analog.c | 378 > + > 2 files changed, 394 insertions(+), 1 deletion(-) > > diff --git > a/Documentation/devicetree/bindings/sound/qcom,msm8916-wcd-analog.txt > b/Documentation/devicetree/bindings/sound/qcom,msm8916-wcd-analog.txt > index 05b67a1..38668ed 100644 > --- a/Documentation/devicetree/bindings/sound/qcom,msm8916-wcd-analog.txt > +++ b/Documentation/devicetree/bindings/sound/qcom,msm8916-wcd-analog.txt > @@ -31,9 +31,22 @@ Required properties > - vdd-cdc-io-supply: phandle to VDD_CDC_IO regulator DT node. > - vdd-cdc-tx-rx-cx-supply: phandle to VDD_CDC_TX/RX/CX regulator DT node. > - vdd-micbias-supply: phandle of VDD_MICBIAS supply's regulator DT node. > - > Optional Properties: > + - qcom,mbhc-vthreshold-low: Array of 5 threshold voltages in mV for 5 > buttons > + detection on headset when the mbhc is powered up ^ git am complains about the leading space > + by internal current source, this is a low power. > + - qcom,mbhc-vthreshold-high: Array of 5 thresold voltages in mV for 5 > buttons > +detection on headset when mbhc is powered up ^ same here > diff --git a/sound/soc/codecs/msm8916-wcd-analog.c > b/sound/soc/codecs/msm8916-wcd-analog.c > index dec4978..04a2112 100644 > --- a/sound/soc/codecs/msm8916-wcd-analog.c > +++ b/sound/soc/codecs/msm8916-wcd-analog.c [...] > +int btn_mask = SND_JACK_BTN_0 | SND_JACK_BTN_1 | > +SND_JACK_BTN_2 | SND_JACK_BTN_3 | SND_JACK_BTN_4; > +int hs_jack_mask = SND_JACK_HEADPHONE | SND_JACK_HEADSE
Re: [PATCH v1 2/3] ASoC: codecs: msm8916-analog: support jack detection
On Wed, Jul 26, 2017 at 05:44:14PM +0100, Srinivas Kandagatla wrote: > > > On 25/07/17 18:51, Damien Riegel wrote: > > The audio codec in the PM8916 has a feature called Multi-Button Headset > > Control (MBHC). It can support of up to five buttons on a headset, and > > jack insertion/removal detection. > > > > This patch only supports the jack detection. A complete implementation > > is available in the Android kernel [1] for reference. > > > > [1] > > https://source.codeaurora.org/quic/la/kernel/msm-4.4/tree/sound/soc/codecs/wcd-mbhc-v2.c?h=LA.BR.1.2.4-00310-8x16.0 > > > > Signed-off-by: Damien Riegel > > If you have noticed it or not, but in the dai_shutdown patch the codec is > put in reset state, so this code only works for one time. > Once you do a playback session and end it this code will stop working. Right, I haven't tested that workflow, thanks. > > > > --- > > sound/soc/codecs/msm8916-wcd-analog.c | 103 > > ++ > > sound/soc/codecs/msm8916-wcd-analog.h | 18 ++ > > 2 files changed, 121 insertions(+) > > create mode 100644 sound/soc/codecs/msm8916-wcd-analog.h > > > > diff --git a/sound/soc/codecs/msm8916-wcd-analog.c > > b/sound/soc/codecs/msm8916-wcd-analog.c > > > ... > > + > > + > > +/** > > + * pm8916_wcd_analog_jack_detect - Enable jack detection. > > + * > > + * @codec: msm8916 codec > > + * @jack: jack to report detection events on > > + * > > + * Enables jack detection of the pm8916-analog. It is capable of reporting > > + * mechanical insertion. > > + */ > > +int pm8916_wcd_analog_jack_detect(struct snd_soc_codec *codec, > > + struct snd_soc_jack *jack) > > +{ > > + struct pm8916_wcd_analog_priv *priv = snd_soc_codec_get_drvdata(codec); > > + > > + priv->jack = jack; > > + > > + snd_soc_update_bits(codec, CDC_D_CDC_RST_CTL, > > + RST_CTL_DIG_SW_RST_N_MASK, > > + RST_CTL_DIG_SW_RST_N_REMOVE_RESET); > Why do you need this? > > > + snd_soc_write(codec, CDC_A_MBHC_DET_CTL_1, 0xB5); > > > + snd_soc_write(codec, CDC_A_MBHC_DET_CTL_2, 0xE1); > > > + snd_soc_write(codec, CDC_A_MBHC_DBNC_TIMER, 0x98); > What do these values mean? Right, define would definitely be better. > > + snd_soc_update_bits(codec, CDC_D_CDC_DIG_CLK_CTL, > > + DIG_CLK_CTL_MBHC_EN_MASK, > > + DIG_CLK_CTL_MBHC_EN); > > + > > + return 0; > > +} > > +EXPORT_SYMBOL_GPL(pm8916_wcd_analog_jack_detect); > > + > > static const struct snd_soc_dapm_route pm8916_wcd_analog_audio_map[] = { > > {"PDM_RX1", NULL, "PDM Playback"}, > > @@ -799,6 +873,14 @@ static struct snd_soc_codec_driver pm8916_wcd_analog = > > { > > }, > > }; > > +static struct jack_detect_irq { > > + const char *name; > > + irqreturn_t (*handler)(int, void *); > > +} jack_detect_irqs[] = { > > + { "mbhc_switch_int", pm8916_wcd_analog_mbhc_switch_handler }, > > + /* other MBHC related interrupts are not handled yet */ > > +}; > > + > > static int pm8916_wcd_analog_parse_dt(struct device *dev, > >struct pm8916_wcd_analog_priv *priv) > > { > > @@ -846,6 +928,27 @@ static int pm8916_wcd_analog_spmi_probe(struct > > platform_device *pdev) > > return ret; > > } > > + for (i = 0; i < ARRAY_SIZE(jack_detect_irqs); i++) { > > + int irq; > > + > > + irq = platform_get_irq_byname(pdev, jack_detect_irqs[i].name); > > + if (irq < 0) { > > + dev_warn(dev, "failed to get irq '%s', jack insertion > > detection disabled\n", > > +jack_detect_irqs[i].name); > > + break; > > + } > > + > > + ret = devm_request_threaded_irq(dev, irq, NULL, > > + jack_detect_irqs[i].handler, > > + IRQF_ONESHOT, > > + jack_detect_irqs[i].name, priv); > > + if (ret) { > > + dev_err(dev, "failed to request irq '%s': %d\n", > > + jack_detect_irqs[i].name, ret); > > + return ret; > > + } > > + } > > + > Not sure how it would work, Where are these interrupts enabled in the pmic? Not sure I understand the question as this is very similar to what you did in your patches. Anyway, as you sent a version that is more feature complete than mine, I don't think there is much value in reviewing my series of patches. Regards, -- Damien
Re: [alsa-devel] [PATCH v1 5/6] ASoC: codecs: msm8916-wcd-analog: add MBHC support
[Sorry for breaking the thread, but I don't have the initial message to reply to.] I tried your patchset and faced some issues regarding removal detection: the driver detects headphone and microphone removal as soon as I press a button on the headset. evtest logs: [Headset insertion] Event: time 10207.841157, type 1 (EV_KEY), code 226 (KEY_MEDIA), value 1 Event: time 10207.841157, -- SYN_REPORT Event: time 10208.039105, type 1 (EV_KEY), code 226 (KEY_MEDIA), value 0 Event: time 10208.039105, -- SYN_REPORT Event: time 10208.075519, type 5 (EV_SW), code 2 (SW_HEADPHONE_INSERT), value 1 Event: time 10208.075519, type 5 (EV_SW), code 4 (SW_MICROPHONE_INSERT), value 1 Event: time 10208.075519, -- SYN_REPORT [Pressing Volume up Button] Event: time 10210.590403, type 1 (EV_KEY), code 115 (KEY_VOLUMEUP), value 1 Event: time 10210.590403, type 5 (EV_SW), code 2 (SW_HEADPHONE_INSERT), value 0 Event: time 10210.590403, type 5 (EV_SW), code 4 (SW_MICROPHONE_INSERT), value 0 Event: time 10210.590403, -- SYN_REPORT Event: time 10210.719560, type 1 (EV_KEY), code 115 (KEY_VOLUMEUP), value 0 Event: time 10210.719560, -- SYN_REPORT [Unplugging] Event: time 10221.926156, type 1 (EV_KEY), code 226 (KEY_MEDIA), value 1 Event: time 10221.926156, -- SYN_REPORT Event: time 10222.026466, type 1 (EV_KEY), code 226 (KEY_MEDIA), value 0 Event: time 10222.026466, -- SYN_REPORT Also, the KEY_MEDIA is wrongly reported but I don't see that every time, I think it depends on how fast I plug in the headset. Anyway, I think that can easily be filtered out by testing the presence of the microphone. Regards, -- Damien
[PATCH v1 2/3] ASoC: codecs: msm8916-analog: support jack detection
The audio codec in the PM8916 has a feature called Multi-Button Headset Control (MBHC). It can support of up to five buttons on a headset, and jack insertion/removal detection. This patch only supports the jack detection. A complete implementation is available in the Android kernel [1] for reference. [1] https://source.codeaurora.org/quic/la/kernel/msm-4.4/tree/sound/soc/codecs/wcd-mbhc-v2.c?h=LA.BR.1.2.4-00310-8x16.0 Signed-off-by: Damien Riegel --- sound/soc/codecs/msm8916-wcd-analog.c | 103 ++ sound/soc/codecs/msm8916-wcd-analog.h | 18 ++ 2 files changed, 121 insertions(+) create mode 100644 sound/soc/codecs/msm8916-wcd-analog.h diff --git a/sound/soc/codecs/msm8916-wcd-analog.c b/sound/soc/codecs/msm8916-wcd-analog.c index 0c351700044a..b9e11012f763 100644 --- a/sound/soc/codecs/msm8916-wcd-analog.c +++ b/sound/soc/codecs/msm8916-wcd-analog.c @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -37,6 +38,8 @@ #define DIG_CLK_CTL_RXD1_CLK_ENBIT(0) #define DIG_CLK_CTL_RXD2_CLK_ENBIT(1) #define DIG_CLK_CTL_RXD3_CLK_ENBIT(2) +#define DIG_CLK_CTL_MBHC_EN_MASK BIT(3) +#define DIG_CLK_CTL_MBHC_ENBIT(3) #define DIG_CLK_CTL_TXD_CLK_EN BIT(4) #define DIG_CLK_CTL_NCP_CLK_EN_MASKBIT(6) #define DIG_CLK_CTL_NCP_CLK_EN BIT(6) @@ -130,6 +133,10 @@ #define CDC_A_MICB_2_EN(0xf144) #define CDC_A_TX_1_2_ATEST_CTL_2 (0xf145) #define CDC_A_MASTER_BIAS_CTL (0xf146) +#define CDC_A_MBHC_DET_CTL_1 (0xf147) +#define MHBC_DET_CTL_1_DET_TYPEBIT(5) +#define CDC_A_MBHC_DET_CTL_2 (0xf150) +#define CDC_A_MBHC_DBNC_TIMER (0xf152) #define CDC_A_TX_1_EN (0xf160) #define CDC_A_TX_2_EN (0xf161) #define CDC_A_TX_1_2_TEST_CTL_1(0xf162) @@ -221,8 +228,10 @@ static const char * const supply_names[] = { struct pm8916_wcd_analog_priv { u16 pmic_rev; u16 codec_version; + struct snd_soc_codec *codec; struct clk *mclk; struct regulator_bulk_data supplies[ARRAY_SIZE(supply_names)]; + struct snd_soc_jack *jack; unsigned int micbias1_cap_mode; unsigned int micbias2_cap_mode; }; @@ -522,6 +531,7 @@ static int pm8916_wcd_analog_probe(struct snd_soc_codec *codec) return err; } + priv->codec = codec; snd_soc_codec_set_drvdata(codec, priv); priv->pmic_rev = snd_soc_read(codec, CDC_D_REVISION1); priv->codec_version = snd_soc_read(codec, CDC_D_PERPH_SUBTYPE); @@ -547,6 +557,70 @@ static int pm8916_wcd_analog_remove(struct snd_soc_codec *codec) priv->supplies); } +static irqreturn_t pm8916_wcd_analog_mbhc_switch_handler(int irq, void *data) +{ + struct pm8916_wcd_analog_priv *priv = data; + struct snd_soc_codec *codec = priv->codec; + unsigned int reg; + int mask, status; + bool insert; + + if (!codec || !priv->jack) + return IRQ_HANDLED; + + reg = snd_soc_read(codec, CDC_A_MBHC_DET_CTL_1); + insert = reg & MHBC_DET_CTL_1_DET_TYPE; + mask = SND_JACK_MECHANICAL; + + dev_dbg(codec->dev, "detected jack %s\n", + insert ? "insertion" : "removal"); + + /* switch between insertion and removal detection */ + snd_soc_update_bits(codec, CDC_A_MBHC_DET_CTL_1, + MHBC_DET_CTL_1_DET_TYPE, + reg ^ MHBC_DET_CTL_1_DET_TYPE); + + if (insert) + status = SND_JACK_MECHANICAL; + else + status = 0; + + snd_soc_jack_report(priv->jack, status, mask); + + return IRQ_HANDLED; +} + + +/** + * pm8916_wcd_analog_jack_detect - Enable jack detection. + * + * @codec: msm8916 codec + * @jack: jack to report detection events on + * + * Enables jack detection of the pm8916-analog. It is capable of reporting + * mechanical insertion. + */ +int pm8916_wcd_analog_jack_detect(struct snd_soc_codec *codec, + struct snd_soc_jack *jack) +{ + struct pm8916_wcd_analog_priv *priv = snd_soc_codec_get_drvdata(codec); + + priv->jack = jack; + + snd_soc_update_bits(codec, CDC_D_CDC_RST_CTL, + RST_CTL_DIG_SW_RST_N_MASK, + RST_CTL_DIG_SW_RST_N_REMOVE_RESET); + snd_soc_write(codec, CDC_A_MBHC_DET_CTL_1, 0xB5); + snd_soc_write(codec, CDC_A_MBHC_DET_CTL_2, 0xE1); + snd_soc_write(codec, CDC_A_MBHC_DBNC_TIMER, 0x98); + snd_soc_update_bits(codec, CDC_D_CDC_DIG_CLK_CTL, + DIG_CLK_CTL_MBHC_EN_MASK, + DIG_CLK_CTL_MBHC_EN); + + return 0; +} +EXPORT_SYMBOL_GPL(pm8916_wcd_a
[PATCH v1 3/3] ASoC: qcom: apq8016-sbc: enable jack detection
Now that the pm8916 audio codec has support for jack detection, let the sound card driver use it. Signed-off-by: Damien Riegel --- sound/soc/qcom/apq8016_sbc.c | 16 1 file changed, 16 insertions(+) diff --git a/sound/soc/qcom/apq8016_sbc.c b/sound/soc/qcom/apq8016_sbc.c index d084d7468299..5f03f6249397 100644 --- a/sound/soc/qcom/apq8016_sbc.c +++ b/sound/soc/qcom/apq8016_sbc.c @@ -19,11 +19,14 @@ #include #include #include +#include #include #include #include #include +#include "../codecs/msm8916-wcd-analog.h" + struct apq8016_sbc_data { void __iomem *mic_iomux; void __iomem *spkr_iomux; @@ -35,13 +38,26 @@ struct apq8016_sbc_data { #define MIC_CTRL_TLMM_SCLK_EN BIT(1) #defineSPKR_CTL_PRI_WS_SLAVE_SEL_11(BIT(17) | BIT(16)) +static struct snd_soc_jack apq8016_jack; + static int apq8016_sbc_dai_init(struct snd_soc_pcm_runtime *rtd) { struct snd_soc_dai *cpu_dai = rtd->cpu_dai; struct snd_soc_card *card = rtd->card; + struct snd_soc_codec *codec = rtd->codec; struct apq8016_sbc_data *pdata = snd_soc_card_get_drvdata(card); int rval = 0; + if (!apq8016_jack.jack) { + rval = snd_soc_card_jack_new(card, "headset", +SND_JACK_MECHANICAL, +&apq8016_jack, NULL, 0); + if (rval) + return rval; + + pm8916_wcd_analog_jack_detect(codec, &apq8016_jack); + } + switch (cpu_dai->id) { case MI2S_PRIMARY: writel(readl(pdata->spkr_iomux) | SPKR_CTL_PRI_WS_SLAVE_SEL_11, -- 2.13.3
[PATCH v1 1/3] ASoC: codecs: msm8916-analog: fix DIG_CLK_CTL_RXD3_CLK_EN define
The wrong bit is assigned to DIG_CLK_CTL_RXD3_CLK_EN, change it for the correct one. Signed-off-by: Damien Riegel --- sound/soc/codecs/msm8916-wcd-analog.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sound/soc/codecs/msm8916-wcd-analog.c b/sound/soc/codecs/msm8916-wcd-analog.c index aec1e1626993..0c351700044a 100644 --- a/sound/soc/codecs/msm8916-wcd-analog.c +++ b/sound/soc/codecs/msm8916-wcd-analog.c @@ -36,7 +36,7 @@ #define CDC_D_CDC_DIG_CLK_CTL (0xf04A) #define DIG_CLK_CTL_RXD1_CLK_ENBIT(0) #define DIG_CLK_CTL_RXD2_CLK_ENBIT(1) -#define DIG_CLK_CTL_RXD3_CLK_ENBIT(3) +#define DIG_CLK_CTL_RXD3_CLK_ENBIT(2) #define DIG_CLK_CTL_TXD_CLK_EN BIT(4) #define DIG_CLK_CTL_NCP_CLK_EN_MASKBIT(6) #define DIG_CLK_CTL_NCP_CLK_EN BIT(6) -- 2.13.3
[PATCH v1 0/3] add jack detection to MSM8916 analog
This series adds support for the MBHC (Multi-Button Headset Control) of the PM8916's analog codec. This IP is capable to identify up to five buttons on a headset, but for now only the jack detection is supported. A complete implementation exists in the Android kernel, but it's quite complex and I'd rather go step by step. First patch fixes a define that is squatting a bit used by the MBHC. Second patch is the actual implementation of the jack detection, and third patch plugs the jack detection in the sound card driver. Damien Riegel (3): ASoC: codecs: msm8916-analog: fix DIG_CLK_CTL_RXD3_CLK_EN define ASoC: codecs: msm8916-analog: support jack detection ASoC: qcom: apq8016-sbc: enable jack detection sound/soc/codecs/msm8916-wcd-analog.c | 105 +- sound/soc/codecs/msm8916-wcd-analog.h | 18 ++ sound/soc/qcom/apq8016_sbc.c | 16 ++ 3 files changed, 138 insertions(+), 1 deletion(-) create mode 100644 sound/soc/codecs/msm8916-wcd-analog.h -- 2.13.3
Re: [RFC][PATCH 0/3] add EXTCON_CHG_USB_* cables to MSM USB phy
Hi, On Fri, Apr 14, 2017 at 02:43:27PM -0400, Damien Riegel wrote: > This patchset adds a way for the MSM USB phy to notify a power supply > when the charging state changes. It achieves that using the extcon > subsystem. > > The first patch makes sure msm_otg_notify_charger is called after the > charger attributes have been set. > The second one makes sure that function is called when unplugging a > "in-the-wall" charger. > The last one adds EXTCON_CHG_USB_* cables to the phy. > > > I send this patchset as RFC because it seems a bit peculiar to have > different drivers that generate the EXTCON_USB_* and EXTCON_CHG_USB_* > events, so I want to make sure to get things right. > > As far as I can tell, all the drivers in the kernel that have USB > charger events also have the EXTCON_USB one. In this case, this patchset > would make things a bit different for the MSM phy: > >+--+ +--+ >| gpio | | PMIC |<-+ >+--+ +--+ | >|| | >`+ | EXTCON_CHG_USB_* > | | events > EXTCON_USB | | > events| | >\|/ | > +--+ | > | USB PHY|--+ > +--+ > > Text version: EXTCON_USB comes from a GPIO or a PMIC, that triggers a > notifier in the USB phy. That notifier will determine the new > EXTCON_CHG_USB_XXX state and the PMIC will be notified about it and > determine how much current it can use to charge a battery. > > Please let me know if this is the correct way to go. I wanted to know if someone has any comment to make on this patchset? I'm currently working on the PMIC driver and it uses the EXTCON notifications, so I just want to make sure it makes sense to do that. Thanks, -- Damien
[RFC][PATCH 0/3] add EXTCON_CHG_USB_* cables to MSM USB phy
This patchset adds a way for the MSM USB phy to notify a power supply when the charging state changes. It achieves that using the extcon subsystem. The first patch makes sure msm_otg_notify_charger is called after the charger attributes have been set. The second one makes sure that function is called when unplugging a "in-the-wall" charger. The last one adds EXTCON_CHG_USB_* cables to the phy. I send this patchset as RFC because it seems a bit peculiar to have different drivers that generate the EXTCON_USB_* and EXTCON_CHG_USB_* events, so I want to make sure to get things right. As far as I can tell, all the drivers in the kernel that have USB charger events also have the EXTCON_USB one. In this case, this patchset would make things a bit different for the MSM phy: +--+ +--+ | gpio | | PMIC |<-+ +--+ +--+ | || | `+ | EXTCON_CHG_USB_* | | events EXTCON_USB | | events| | \|/ | +--+ | | USB PHY|--+ +--+ Text version: EXTCON_USB comes from a GPIO or a PMIC, that triggers a notifier in the USB phy. That notifier will determine the new EXTCON_CHG_USB_XXX state and the PMIC will be notified about it and determine how much current it can use to charge a battery. Please let me know if this is the correct way to go. Damien Riegel (3): usb: phy: msm: notify charger after setting charger info usb: phy: msm: notify charger when power supply is unplugged usb: phy: msm: use extcon to notify charger drivers/usb/phy/phy-msm-usb.c | 47 --- 1 file changed, 44 insertions(+), 3 deletions(-) -- 2.12.2
[RFC][PATCH 2/3] usb: phy: msm: notify charger when power supply is unplugged
With the current code, msm_otg_notify_charger doesn't get called when a power supply identified as a DCP is unplugged. To work around that, update charger info and call the notify function when switching from idle to host. Signed-off-by: Damien Riegel --- drivers/usb/phy/phy-msm-usb.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c index c1460182bc56..f89a2a540f71 100644 --- a/drivers/usb/phy/phy-msm-usb.c +++ b/drivers/usb/phy/phy-msm-usb.c @@ -1349,6 +1349,9 @@ static void msm_otg_sm_work(struct work_struct *w) writel(readl(USB_OTGSC) & ~OTGSC_BSVIE, USB_OTGSC); msm_otg_start_host(otg->usb_phy, 1); otg->state = OTG_STATE_A_HOST; + motg->chg_state = USB_CHG_STATE_UNDEFINED; + motg->chg_type = USB_INVALID_CHARGER; + msm_otg_notify_charger(motg, 0); } else if (test_bit(B_SESS_VLD, &motg->inputs)) { switch (motg->chg_state) { case USB_CHG_STATE_UNDEFINED: -- 2.12.2
[RFC][PATCH 3/3] usb: phy: msm: use extcon to notify charger
Phy already keeps track of the USB charger mode it is in, that information could be useful to a power supply to let it know how much current it can draw. So in this case when DCP or CDP is set maximum current available is 1500mA, and 100mA when SDP is set. This is a bit peculiar in that this driver only handle EXTCON_CHG_* states, EXTCON_USB_* events come from another driver. Signed-off-by: Damien Riegel --- drivers/usb/phy/phy-msm-usb.c | 38 ++ 1 file changed, 38 insertions(+) diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c index f89a2a540f71..f264d5a5ad76 100644 --- a/drivers/usb/phy/phy-msm-usb.c +++ b/drivers/usb/phy/phy-msm-usb.c @@ -217,6 +217,7 @@ struct msm_otg { struct msm_usb_cable vbus; struct msm_usb_cable id; + struct extcon_dev *edev; struct gpio_desc *switch_gpio; struct notifier_block reboot; @@ -248,6 +249,13 @@ enum vdd_levels { VDD_LEVEL_MAX, }; +static const unsigned int msm_extcon_cables[] = { + EXTCON_CHG_USB_SDP, + EXTCON_CHG_USB_DCP, + EXTCON_CHG_USB_CDP, + EXTCON_NONE, +}; + static int msm_hsusb_init_vddcx(struct msm_otg *motg, int init) { int ret = 0; @@ -834,6 +842,21 @@ static int msm_otg_resume(struct msm_otg *motg) static void msm_otg_notify_charger(struct msm_otg *motg, unsigned mA) { + const unsigned int extcon_cables[][2] = { + { EXTCON_CHG_USB_SDP, USB_SDP_CHARGER }, + { EXTCON_CHG_USB_DCP, USB_DCP_CHARGER }, + { EXTCON_CHG_USB_CDP, USB_CDP_CHARGER }, + }; + int i; + + for (i = 0; i < ARRAY_SIZE(extcon_cables); i++) { + unsigned int cable = extcon_cables[i][0]; + unsigned int chg_type = extcon_cables[i][1]; + + extcon_set_cable_state_(motg->edev, cable, + motg->chg_type == chg_type); + } + if (motg->cur_power == mA) return; @@ -1861,6 +1884,21 @@ static int msm_otg_probe(struct platform_device *pdev) } /* +* Documentation in extcon.h states that EXTCONG_CHG_USB_SDP should +* always appear together with EXTCON_USB, so register extcon cables +* only if we successfully got the vbus extcon. +*/ + if (!IS_ERR(motg->vbus.extcon)) { + motg->edev = devm_extcon_dev_allocate(&pdev->dev, msm_extcon_cables); + if (IS_ERR(motg->edev)) + return PTR_ERR(motg->edev); + + ret = devm_extcon_dev_register(&pdev->dev, motg->edev); + if (ret) + return ret; + } + + /* * NOTE: The PHYs can be multiplexed between the chipidea controller * and the dwc3 controller, using a single bit. It is important that * the dwc3 driver does not set this bit in an incompatible way. -- 2.12.2
[RFC][PATCH 1/3] usb: phy: msm: notify charger after setting charger info
Move calls to msm_otg_notify_charger after attributes chg_state and chg_type have been set. That way the function can use them and not rely only on the "mA" parameter. Signed-off-by: Damien Riegel --- drivers/usb/phy/phy-msm-usb.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c index 93d9aaad2994..c1460182bc56 100644 --- a/drivers/usb/phy/phy-msm-usb.c +++ b/drivers/usb/phy/phy-msm-usb.c @@ -1392,9 +1392,9 @@ static void msm_otg_sm_work(struct work_struct *w) pm_runtime_put_sync(otg->usb_phy->dev); msm_otg_reset(otg->usb_phy); } - msm_otg_notify_charger(motg, 0); motg->chg_state = USB_CHG_STATE_UNDEFINED; motg->chg_type = USB_INVALID_CHARGER; + msm_otg_notify_charger(motg, 0); } if (otg->state == OTG_STATE_B_IDLE) @@ -1404,10 +1404,10 @@ static void msm_otg_sm_work(struct work_struct *w) dev_dbg(otg->usb_phy->dev, "OTG_STATE_B_PERIPHERAL state\n"); if (!test_bit(B_SESS_VLD, &motg->inputs) || !test_bit(ID, &motg->inputs)) { - msm_otg_notify_charger(motg, 0); - msm_otg_start_peripheral(otg->usb_phy, 0); motg->chg_state = USB_CHG_STATE_UNDEFINED; motg->chg_type = USB_INVALID_CHARGER; + msm_otg_notify_charger(motg, 0); + msm_otg_start_peripheral(otg->usb_phy, 0); otg->state = OTG_STATE_B_IDLE; msm_otg_reset(otg->usb_phy); schedule_work(w); -- 2.12.2
[PATCH v2 2/2] ARM: dts: TS-4800: add CAN support
This enables support for the CAN controller located in the FPGA. Signed-off-by: Damien Riegel --- Changes in v2: - Get rid of "0," in unit-address - Move node to keep them sorted by unit-address arch/arm/boot/dts/imx51-ts4800.dts | 10 ++ 1 file changed, 10 insertions(+) diff --git a/arch/arm/boot/dts/imx51-ts4800.dts b/arch/arm/boot/dts/imx51-ts4800.dts index 0e80fb7..984c71b 100644 --- a/arch/arm/boot/dts/imx51-ts4800.dts +++ b/arch/arm/boot/dts/imx51-ts4800.dts @@ -176,6 +176,16 @@ interrupt-controller; #interrupt-cells = <1>; }; + + can@1a000 { + compatible = "technologic,sja1000"; + reg = <0x1a000 0x100>; + interrupt-parent = <&fpga_irqc>; + interrupts = <1>; + reg-io-width = <2>; + nxp,tx-output-config = <0x06>; + nxp,external-clock-frequency = <2400>; + }; }; }; -- 2.5.0
[PATCH v2 1/2] ARM: dts: TS-4800: add FPGA's IRQ controller support
Enable FPGA's IRQ controller. It is in charge of dispatching interrupts generated by IPs in the FPGA. The SoC is notified that an interrupt occurred through a GPIO. Signed-off-by: Damien Riegel --- Changes in v2: - Remove new lines - Use hyphen rather than underscore in node name - Get rid of "0," in unit-address - Move node to keep them sorted by unit-address arch/arm/boot/dts/imx51-ts4800.dts | 17 + 1 file changed, 17 insertions(+) diff --git a/arch/arm/boot/dts/imx51-ts4800.dts b/arch/arm/boot/dts/imx51-ts4800.dts index 0ff76a1..0e80fb7 100644 --- a/arch/arm/boot/dts/imx51-ts4800.dts +++ b/arch/arm/boot/dts/imx51-ts4800.dts @@ -165,6 +165,17 @@ reg = <0x12000 0x1000>; syscon = <&syscon 0x10 6>; }; + + fpga_irqc: fpga-irqc@15000 { + compatible = "technologic,ts4800-irqc"; + reg = <0x15000 0x1000>; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_interrupt_fpga>; + interrupt-parent = <&gpio2>; + interrupts= <9 IRQ_TYPE_LEVEL_HIGH>; + interrupt-controller; + #interrupt-cells = <1>; + }; }; }; @@ -228,6 +239,12 @@ >; }; + pinctrl_interrupt_fpga: fpgaicgrp { + fsl,pins = < + MX51_PAD_EIM_D27__GPIO2_9 0xe5 + >; + }; + pinctrl_lcd: lcdgrp { fsl,pins = < MX51_PAD_DISP1_DAT0__DISP1_DAT0 0x5 -- 2.5.0
[PATCH 2/2] ARM: dts: TS-4800: add CAN support
This enables support for the CAN controller located in the FPGA. Signed-off-by: Damien Riegel --- arch/arm/boot/dts/imx51-ts4800.dts | 10 ++ 1 file changed, 10 insertions(+) diff --git a/arch/arm/boot/dts/imx51-ts4800.dts b/arch/arm/boot/dts/imx51-ts4800.dts index 2c89988..41be82e 100644 --- a/arch/arm/boot/dts/imx51-ts4800.dts +++ b/arch/arm/boot/dts/imx51-ts4800.dts @@ -149,6 +149,16 @@ #size-cells = <1>; ranges = <0 0 0 0x1d000>; + can@0,1a000 { + compatible = "technologic,sja1000"; + reg = <0x1a000 0x0100>; + interrupt-parent = <&fpga_irqc>; + interrupts = <1>; + reg-io-width = <2>; + nxp,tx-output-config = <0x06>; + nxp,external-clock-frequency = <2400>; + }; + fpga_irqc: fpga_irqc@0,15000 { compatible = "technologic,ts4800-irqc"; reg = <0x15000 0x1000>; -- 2.5.0
[PATCH 1/2] ARM: dts: TS-4800: add FPGA's IRQ controller support
Enable FPGA's IRQ controller. It is in charge of dispatching interrupts generated by IPs in the FPGA. The SoC is notified that an interrupt occurred through a GPIO. Signed-off-by: Damien Riegel --- arch/arm/boot/dts/imx51-ts4800.dts | 19 +++ 1 file changed, 19 insertions(+) diff --git a/arch/arm/boot/dts/imx51-ts4800.dts b/arch/arm/boot/dts/imx51-ts4800.dts index 0ff76a1..2c89988 100644 --- a/arch/arm/boot/dts/imx51-ts4800.dts +++ b/arch/arm/boot/dts/imx51-ts4800.dts @@ -149,6 +149,19 @@ #size-cells = <1>; ranges = <0 0 0 0x1d000>; + fpga_irqc: fpga_irqc@0,15000 { + compatible = "technologic,ts4800-irqc"; + reg = <0x15000 0x1000>; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_interrupt_fpga>; + + interrupt-parent = <&gpio2>; + interrupts= <9 IRQ_TYPE_LEVEL_HIGH>; + + interrupt-controller; + #interrupt-cells = <1>; + }; + syscon: syscon@b001 { compatible = "syscon", "simple-mfd"; reg = <0x1 0x3d>; @@ -228,6 +241,12 @@ >; }; + pinctrl_interrupt_fpga: fpgaicgrp { + fsl,pins = < + MX51_PAD_EIM_D27__GPIO2_9 0xe5 + >; + }; + pinctrl_lcd: lcdgrp { fsl,pins = < MX51_PAD_DISP1_DAT0__DISP1_DAT0 0x5 -- 2.5.0
Re: [PATCH] irqchip/ts4800: Make ts4800_ic_ops static const
On Sun, Feb 14, 2016 at 09:50:04PM +0800, Axel Lin wrote: > ts4800_ic_ops is only referenced in this driver, so make it static. > In additional, it's never get modified thus also make it const. > > Signed-off-by: Axel Lin Reviewed-by: Damien Riegel Thanks for catching this.
Re: [PATCH 0/3] Add hardware dependency to TS-4800 board drivers
Hi Jean, On Tue, Feb 09, 2016 at 11:15:49AM +0100, Jean Delvare wrote: > Hi all, > > This adds hardware dependency to 3 drivers for the Technologic Systems > TS-4800 board. Thanks to these dependencies, users of other systems > will not accidentally enable these drivers. These drivers are for IPs implemented in an FPGA. For now it is true that they are used only on one board with an iMX.51 but this is not a strong dependency and it might change in the future if Technologic Systems decide to reuse these IPs on boards based on different SoCs. As the FPGA and the SoC are two separate chips, I don't know how such dependency should be expressed in kconfig. Thanks, Damien
[tip:irq/core] irqchip/ts4800: Add TS-4800 interrupt controller
Commit-ID: d01f8633d52e4dac5ee598b87d49fd23346ccfd6 Gitweb: http://git.kernel.org/tip/d01f8633d52e4dac5ee598b87d49fd23346ccfd6 Author: Damien Riegel AuthorDate: Mon, 21 Dec 2015 15:11:23 -0500 Committer: Thomas Gleixner CommitDate: Tue, 29 Dec 2015 11:58:53 +0100 irqchip/ts4800: Add TS-4800 interrupt controller This commit adds support for the TS-4800 interrupt controller. This controller is instantiated in a companion FPGA, and multiplex interrupts for other FPGA IPs. As this component is external to the SoC, the SoC might need to reserve pins, so this controller is implemented as a platform driver and doesn't use the IRQCHIP_DECLARE construct. Signed-off-by: Damien Riegel Cc: Jason Cooper Cc: Marc Zyngier Cc: Rob Herring Cc: Pawel Moll Cc: Mark Rutland Cc: Ian Campbell Cc: Kumar Gala Cc: ker...@savoirfairelinux.com Link: http://lkml.kernel.org/r/1450728683-31416-2-git-send-email-damien.rie...@savoirfairelinux.com Signed-off-by: Thomas Gleixner --- drivers/irqchip/Kconfig | 6 ++ drivers/irqchip/Makefile | 1 + drivers/irqchip/irq-ts4800.c | 163 +++ 3 files changed, 170 insertions(+) diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig index b5f5133..11fc2a2 100644 --- a/drivers/irqchip/Kconfig +++ b/drivers/irqchip/Kconfig @@ -151,6 +151,12 @@ config TB10X_IRQC select IRQ_DOMAIN select GENERIC_IRQ_CHIP +config TS4800_IRQ + tristate "TS-4800 IRQ controller" + select IRQ_DOMAIN + help + Support for the TS-4800 FPGA IRQ controller + config VERSATILE_FPGA_IRQ bool select IRQ_DOMAIN diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile index 83d1fce..d4c2e4e 100644 --- a/drivers/irqchip/Makefile +++ b/drivers/irqchip/Makefile @@ -41,6 +41,7 @@ obj-$(CONFIG_ARCH_NSPIRE) += irq-zevio.o obj-$(CONFIG_ARCH_VT8500) += irq-vt8500.o obj-$(CONFIG_ST_IRQCHIP) += irq-st.o obj-$(CONFIG_TB10X_IRQC) += irq-tb10x.o +obj-$(CONFIG_TS4800_IRQ) += irq-ts4800.o obj-$(CONFIG_XTENSA) += irq-xtensa-pic.o obj-$(CONFIG_XTENSA_MX)+= irq-xtensa-mx.o obj-$(CONFIG_IRQ_CROSSBAR) += irq-crossbar.o diff --git a/drivers/irqchip/irq-ts4800.c b/drivers/irqchip/irq-ts4800.c new file mode 100644 index 000..4192bdc --- /dev/null +++ b/drivers/irqchip/irq-ts4800.c @@ -0,0 +1,163 @@ +/* + * Multiplexed-IRQs driver for TS-4800's FPGA + * + * Copyright (c) 2015 - Savoir-faire Linux + * + * This file is licensed under the terms of the GNU General Public + * License version 2. This program is licensed "as is" without any + * warranty of any kind, whether express or implied. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define IRQ_MASK0x4 +#define IRQ_STATUS 0x8 + +struct ts4800_irq_data { + void __iomem*base; + struct irq_domain *domain; + struct irq_chip irq_chip; +}; + +static void ts4800_irq_mask(struct irq_data *d) +{ + struct ts4800_irq_data *data = irq_data_get_irq_chip_data(d); + u16 reg = readw(data->base + IRQ_MASK); + u16 mask = 1 << d->hwirq; + + writew(reg | mask, data->base + IRQ_MASK); +} + +static void ts4800_irq_unmask(struct irq_data *d) +{ + struct ts4800_irq_data *data = irq_data_get_irq_chip_data(d); + u16 reg = readw(data->base + IRQ_MASK); + u16 mask = 1 << d->hwirq; + + writew(reg & ~mask, data->base + IRQ_MASK); +} + +static int ts4800_irqdomain_map(struct irq_domain *d, unsigned int irq, + irq_hw_number_t hwirq) +{ + struct ts4800_irq_data *data = d->host_data; + + irq_set_chip_and_handler(irq, &data->irq_chip, handle_simple_irq); + irq_set_chip_data(irq, data); + irq_set_noprobe(irq); + + return 0; +} + +struct irq_domain_ops ts4800_ic_ops = { + .map = ts4800_irqdomain_map, + .xlate = irq_domain_xlate_onecell, +}; + +static void ts4800_ic_chained_handle_irq(struct irq_desc *desc) +{ + struct ts4800_irq_data *data = irq_desc_get_handler_data(desc); + struct irq_chip *chip = irq_desc_get_chip(desc); + u16 status = readw(data->base + IRQ_STATUS); + + chained_irq_enter(chip, desc); + + if (unlikely(status == 0)) { + handle_bad_irq(desc); + goto out; + } + + do { + unsigned int bit = __ffs(status); + int irq = irq_find_mapping(data->domain, bit); + + status &= ~(1 << bit); + generic_handle_irq(irq); + } while (status); + +out: + chained_irq_exit(chip, desc); +} + +static int ts4800_ic_probe(struct platform_device *pdev) +{ + struct device_node *node =
[tip:irq/core] irqchip/ts4800: Add documentation for TS-4800 interrupt controller
Commit-ID: 0f6d785c847eeff55ae19546f5885156394be569 Gitweb: http://git.kernel.org/tip/0f6d785c847eeff55ae19546f5885156394be569 Author: Damien Riegel AuthorDate: Mon, 21 Dec 2015 15:11:22 -0500 Committer: Thomas Gleixner CommitDate: Tue, 29 Dec 2015 11:58:53 +0100 irqchip/ts4800: Add documentation for TS-4800 interrupt controller This is an interrupt-controller implemented in an FPGA, to multiplex interrupts generated from other IPs. The FPGA usually uses a GPIO as a parent interrupt controller to notify that one of the multiplexed interrupts has triggered. Signed-off-by: Damien Riegel Acked-by: Rob Herring Cc: Jason Cooper Cc: Marc Zyngier Cc: Rob Herring Cc: Pawel Moll Cc: Mark Rutland Cc: Ian Campbell Cc: Kumar Gala Cc: ker...@savoirfairelinux.com Link: http://lkml.kernel.org/r/1450728683-31416-1-git-send-email-damien.rie...@savoirfairelinux.com Signed-off-by: Thomas Gleixner --- .../bindings/interrupt-controller/technologic,ts4800.txt | 16 1 file changed, 16 insertions(+) diff --git a/Documentation/devicetree/bindings/interrupt-controller/technologic,ts4800.txt b/Documentation/devicetree/bindings/interrupt-controller/technologic,ts4800.txt new file mode 100644 index 000..7f15f1b --- /dev/null +++ b/Documentation/devicetree/bindings/interrupt-controller/technologic,ts4800.txt @@ -0,0 +1,16 @@ +TS-4800 FPGA interrupt controller + +TS-4800 FPGA has an internal interrupt controller. When one of the +interrupts is triggered, the SoC is notified, usually using a GPIO as +parent interrupt source. + +Required properties: +- compatible: should be "technologic,ts4800-irqc" +- interrupt-controller: identifies the node as an interrupt controller +- reg: physical base address of the controller and length of memory mapped + region +- #interrupt-cells: specifies the number of cells needed to encode an interrupt + source, should be 1. +- interrupt-parent: phandle to the parent interrupt controller this one is + cascaded from +- interrupts: specifies the interrupt line in the interrupt-parent controller -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 3/3] can: sja1000: of: add compatibility with Technologic Systems version
Technologic Systems provides an IP compatible with the SJA1000, instantiated in an FPGA. Because of some bus widths issue, access to registers is made through a "window" that works like this: base + 0x0: address to read/write base + 0x2: 8-bit register value This commit adds a new compatible device, "technologic,sja1000", with read and write functions using the window mechanism. Signed-off-by: Damien Riegel --- drivers/net/can/sja1000/sja1000_platform.c | 47 ++ 1 file changed, 47 insertions(+) diff --git a/drivers/net/can/sja1000/sja1000_platform.c b/drivers/net/can/sja1000/sja1000_platform.c index e0572d0..bfb124d 100644 --- a/drivers/net/can/sja1000/sja1000_platform.c +++ b/drivers/net/can/sja1000/sja1000_platform.c @@ -45,6 +45,10 @@ struct sja1000_of_data { int (*init)(struct sja1000_priv *priv, struct device_node *of); }; +struct technologic_priv { + spinlock_t io_lock; +}; + static u8 sp_read_reg8(const struct sja1000_priv *priv, int reg) { return ioread8(priv->reg_base + reg); @@ -75,6 +79,43 @@ static void sp_write_reg32(const struct sja1000_priv *priv, int reg, u8 val) iowrite8(val, priv->reg_base + reg * 4); } +static u8 technologic_read_reg16(const struct sja1000_priv *priv, int reg) +{ + struct technologic_priv *tp = priv->priv; + unsigned long flags; + u8 val; + + spin_lock_irqsave(&tp->io_lock, flags); + iowrite16(reg, priv->reg_base + 0); + val = ioread16(priv->reg_base + 2); + spin_unlock_irqrestore(&tp->io_lock, flags); + + return val; +} + +static void technologic_write_reg16(const struct sja1000_priv *priv, + int reg, u8 val) +{ + struct technologic_priv *tp = priv->priv; + unsigned long flags; + + spin_lock_irqsave(&tp->io_lock, flags); + iowrite16(reg, priv->reg_base + 0); + iowrite16(val, priv->reg_base + 2); + spin_unlock_irqrestore(&tp->io_lock, flags); +} + +static int technologic_init(struct sja1000_priv *priv, struct device_node *of) +{ + struct technologic_priv *tp = priv->priv; + + priv->read_reg = technologic_read_reg16; + priv->write_reg = technologic_write_reg16; + spin_lock_init(&tp->io_lock); + + return 0; +} + static void sp_populate(struct sja1000_priv *priv, struct sja1000_platform_data *pdata, unsigned long resource_mem_flags) @@ -255,8 +296,14 @@ static int sp_remove(struct platform_device *pdev) return 0; } +struct sja1000_of_data technologic_data = { + .priv_sz = sizeof(struct technologic_priv), + .init = technologic_init, +}; + static const struct of_device_id sp_of_table[] = { {.compatible = "nxp,sja1000"}, + {.compatible = "technologic,sja1000", .data = &technologic_data}, {}, }; MODULE_DEVICE_TABLE(of, sp_of_table); -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 1/3] can: sja1000: of: add per-compatible init hook
This commit adds the capability to allocate and init private data embedded in the sja1000_priv structure on a per-compatible basis. The device node is passed as a parameter of the init callback to allow parsing of custom device tree properties. Signed-off-by: Damien Riegel --- drivers/net/can/sja1000/sja1000_platform.c | 39 -- 1 file changed, 37 insertions(+), 2 deletions(-) diff --git a/drivers/net/can/sja1000/sja1000_platform.c b/drivers/net/can/sja1000/sja1000_platform.c index 0552ed4..e0572d0 100644 --- a/drivers/net/can/sja1000/sja1000_platform.c +++ b/drivers/net/can/sja1000/sja1000_platform.c @@ -40,6 +40,11 @@ MODULE_DESCRIPTION("Socket-CAN driver for SJA1000 on the platform bus"); MODULE_ALIAS("platform:" DRV_NAME); MODULE_LICENSE("GPL v2"); +struct sja1000_of_data { + size_t priv_sz; + int (*init)(struct sja1000_priv *priv, struct device_node *of); +}; + static u8 sp_read_reg8(const struct sja1000_priv *priv, int reg) { return ioread8(priv->reg_base + reg); @@ -154,7 +159,8 @@ static void sp_populate_of(struct sja1000_priv *priv, struct device_node *of) priv->cdr |= CDR_CBP; /* default */ } -static int sp_probe(struct platform_device *pdev) +static int __sp_probe(struct platform_device *pdev, + const struct sja1000_of_data *of_data) { int err, irq = 0; void __iomem *addr; @@ -163,6 +169,7 @@ static int sp_probe(struct platform_device *pdev) struct resource *res_mem, *res_irq = NULL; struct sja1000_platform_data *pdata; struct device_node *of = pdev->dev.of_node; + size_t priv_sz = of_data ? of_data->priv_sz : 0; pdata = dev_get_platdata(&pdev->dev); if (!pdata && !of) { @@ -191,7 +198,7 @@ static int sp_probe(struct platform_device *pdev) if (!irq && !res_irq) return -ENODEV; - dev = alloc_sja1000dev(0); + dev = alloc_sja1000dev(priv_sz); if (!dev) return -ENOMEM; priv = netdev_priv(dev); @@ -213,6 +220,12 @@ static int sp_probe(struct platform_device *pdev) else sp_populate(priv, pdata, res_mem->flags); + if (of_data && of_data->init) { + err = of_data->init(priv, of); + if (err) + goto exit_free; + } + platform_set_drvdata(pdev, dev); SET_NETDEV_DEV(dev, &pdev->dev); @@ -248,6 +261,28 @@ static const struct of_device_id sp_of_table[] = { }; MODULE_DEVICE_TABLE(of, sp_of_table); +static const struct sja1000_of_data *sp_get_of_data(struct device_node *of) +{ + const struct of_device_id *id; + + if (!of) + return NULL; + + id = of_match_node(sp_of_table, of); + if (!id) + return NULL; + + return id->data; +} + +static int sp_probe(struct platform_device *pdev) +{ + struct device_node *of = pdev->dev.of_node; + const struct sja1000_of_data *of_data = sp_get_of_data(of); + + return __sp_probe(pdev, of_data); +} + static struct platform_driver sp_driver = { .probe = sp_probe, .remove = sp_remove, -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 2/3] can: sja1000: add documentation for Technologic Systems version
This commit adds documentation for the Technologic Systems version of SJA1000. The difference with the NXP version is in the way the registers are accessed. Signed-off-by: Damien Riegel --- Documentation/devicetree/bindings/net/can/sja1000.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/net/can/sja1000.txt b/Documentation/devicetree/bindings/net/can/sja1000.txt index b4a6d53..ac3160e 100644 --- a/Documentation/devicetree/bindings/net/can/sja1000.txt +++ b/Documentation/devicetree/bindings/net/can/sja1000.txt @@ -2,7 +2,7 @@ Memory mapped SJA1000 CAN controller from NXP (formerly Philips) Required properties: -- compatible : should be "nxp,sja1000". +- compatible : should be one of "nxp,sja1000", "technologic,sja1000". - reg : should specify the chip select, address offset and size required to map the registers of the SJA1000. The size is usually 0x80. @@ -14,6 +14,7 @@ Optional properties: - reg-io-width : Specify the size (in bytes) of the IO accesses that should be performed on the device. Valid value is 1, 2 or 4. + This property is ignored for technologic version. Default to 1 (8 bits). - nxp,external-clock-frequency : Frequency of the external oscillator -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 0/3] can: sja1000: support for technologic version
This patchset introduces support for the technologic version of the SJA1000. Access to IP's registers are proxied through a window, requiring two bus accesses to read or write a register. These accesses must be protected by a spinlock to prevent race conditions. Currently, there is no easy way to allocate and initialize this spinlock. SJA1000 already provides a way to allocate private data, but sja1000_platform.c makes no use of it. Patch 1 adds the capability to allocate and initialize private data on a per-compatible basis in sja1000_platform.c. Patch 2 updates device tree documentation to add the technologic version. Patch 3 updates the driver to implement the technologic version Changes in v2: - added a patch to allocate and initialize private data - changed device tree documentation - added a spinlock to protect bus accesses - changed sp_{read,write}_reg16 to io{read,write}16 Damien Riegel (3): can: sja1000: of: add per-compatible init hook can: sja1000: add documentation for Technologic Systems version can: sja1000: of: add compatibility with Technologic Systems version .../devicetree/bindings/net/can/sja1000.txt| 3 +- drivers/net/can/sja1000/sja1000_platform.c | 86 +- 2 files changed, 86 insertions(+), 3 deletions(-) -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/5] watchdog: Do not use 'dev' from watchdog_device in watchdog drivers
On Wed, Dec 23, 2015 at 09:11:28PM -0800, Guenter Roeck wrote: > The 'dev' variable in watchdog drivers has a different lifetime than the > watchdog character device and should therefore not be used by watchdog > drivers. > > Some of the drivers use the variable to print kernel messages. Those are > either dropped or converted to use pr_ functions. One driver sets the > variable during initialization to the watchdog driver's parent device, > which is wrong and also removed. Hi Guenter, For gpio_wdt and mena21_wdt, wdd->parent is set and could be used for dev_* printings. Do you prefer to keep this variable only for watchdog core internal usage? Otherwise, the serie looks good. Thanks, Damien -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v8] watchdog: ts4800: add driver for TS-4800 watchdog
On Tue, Dec 08, 2015 at 11:37:28AM -0500, Damien Riegel wrote: > This watchdog is instantiated in a FPGA that is memory mapped. It is > made of only one register, called the feed register. Writing to this > register will re-arm the watchdog for a given time (and enable it if it > was disable). It can be disabled by writing a special value into it. > > It is part of a syscon block, and the watchdog register offset in this > block varies from board to board. This offset is passed in the syscon > property after the phandle to the syscon node. > > Signed-off-by: Damien Riegel > Acked-by: Rob Herring > Reviewed-by: Guenter Roeck Hi Guenter, You have reviewed this patch but not picked it up in your tree. Shall I expect Wim to pick it up directly for the next merge window? The board would be quite useless without its watchdog driver. Thanks, Damien > --- > Changes in v8: > - Split the serie into two parts: watchdog and dts as there are no >build dependencies between the two > - Added Reviewed-by Guenter Roeck and Acked-by by Rob Herring (for >the dts bindings). > > Changes in v7: > - syscon: change bus-width DT property to reg-io-width > - watchdog: add dependency on HAS_IOMEM (spotted by a 0-day build) > > Changes in v6: > - vendor prefix: reorder to sort alphabetically (wrong order since v3) > - split commit adding device tree into two patches: one for the doc, one for >the bindings > > Changes in v5: > - watchdog: changed iteration stop condition in set_timeout to be less >error prone > > Changes in v4: > - syscon: rewrite DT property reading to be clearer > - watchdog: made fixes suggested by Guenter (now uses >watchdog_init_timeout, u32 instead of u16, fixed error checking in >probe, cleaned set_timeout) > > Changes in v3: > - Rebased on v4.3 > - Changed vendor prefix from "ts" to "technologic" > - Added a DT option to generic syscon driver to allow regmap configuration > - Dropped custom mfd driver, use generic syscon driver instead. > > Changes in v2: > - Added a mfd driver to handle syscon registers > - The watchdog driver now uses the regmap (created by the mfd driver) >to access the feed register > - Remove watchdog's dependency on SOC_IMX51 > > .../devicetree/bindings/watchdog/ts4800-wdt.txt| 25 +++ > drivers/watchdog/Kconfig | 10 + > drivers/watchdog/Makefile | 1 + > drivers/watchdog/ts4800_wdt.c | 215 > + > 4 files changed, 251 insertions(+) > create mode 100644 Documentation/devicetree/bindings/watchdog/ts4800-wdt.txt > create mode 100644 drivers/watchdog/ts4800_wdt.c > > diff --git a/Documentation/devicetree/bindings/watchdog/ts4800-wdt.txt > b/Documentation/devicetree/bindings/watchdog/ts4800-wdt.txt > new file mode 100644 > index 000..8f6caad > --- /dev/null > +++ b/Documentation/devicetree/bindings/watchdog/ts4800-wdt.txt > @@ -0,0 +1,25 @@ > +Technologic Systems Watchdog > + > +Required properties: > +- compatible: must be "technologic,ts4800-wdt" > +- syscon: phandle / integer array that points to the syscon node which > + describes the FPGA's syscon registers. > + - phandle to FPGA's syscon > + - offset to the watchdog register > + > +Optional property: > +- timeout-sec: contains the watchdog timeout in seconds. > + > +Example: > + > +syscon: syscon@b001 { > + compatible = "syscon", "simple-mfd"; > + reg = <0xb001 0x3d>; > + reg-io-width = <2>; > + > + wdt@e { > + compatible = "technologic,ts4800-wdt"; > + syscon = <&syscon 0xe>; > + timeout-sec = <10>; > + }; > +} > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig > index 79e1aa1..bb624d2 100644 > --- a/drivers/watchdog/Kconfig > +++ b/drivers/watchdog/Kconfig > @@ -426,6 +426,16 @@ config NUC900_WATCHDOG > To compile this driver as a module, choose M here: the > module will be called nuc900_wdt. > > +config TS4800_WATCHDOG > + tristate "TS-4800 Watchdog" > + depends on HAS_IOMEM && OF > + select WATCHDOG_CORE > + select MFD_SYSCON > + help > + Technologic Systems TS-4800 has watchdog timer implemented in > + an external FPGA. Say Y here if you want to support for the > + watchdog timer on TS-4800 board. > + > config TS72XX_WATCHDOG > tristate "TS-72XX SBC Watchdog" > depends on MACH_TS72XX > diff --git a/drivers/watchdog/Make
Re: [PATCH 2/5] watchdog: Separate and maintain variables based on variable lifetime
On Tue, Dec 22, 2015 at 08:22:40AM -0800, Guenter Roeck wrote: > On 12/22/2015 08:09 AM, Damien Riegel wrote: > >On Mon, Dec 21, 2015 at 05:10:58PM -0800, Guenter Roeck wrote: > >>On 12/21/2015 09:28 AM, Damien Riegel wrote: > >>>On Sun, Dec 20, 2015 at 01:05:00PM -0800, Guenter Roeck wrote: > >>>>All variables required by the watchdog core to manage a watchdog are > >>>>currently stored in struct watchdog_device. The lifetime of those > >>>>variables is determined by the watchdog driver. However, the lifetime > >>>>of variables used by the watchdog core differs from the lifetime of > >>>>struct watchdog_device. To remedy this situation, watchdog drivers > >>>>can implement ref and unref callbacks, to be used by the watchdog > >>>>core to lock struct watchdog_device in memory. > >>>> > >>>>While this solves the immediate problem, it depends on watchdog drivers > >>>>to actually implement the ref/unref callbacks. This is error prone, > >>>>often not implemented in the first place, or not implemented correctly. > >>>> > >>>>To solve the problem without requiring driver support, split the variables > >>>>in struct watchdog_device into two data structures - one for variables > >>>>associated with the watchdog driver, one for variables associated with > >>>>the watchdog core. With this approach, the watchdog core can keep track > >>>>of its variable lifetime and no longer depends on ref/unref callbacks > >>>>in the driver. As a side effect, some of the variables originally in > >>>>struct watchdog_driver are now private to the watchdog core and no longer > >>>>visible in watchdog drivers. > >>>> > >>>>The 'ref' and 'unref' callbacks in struct watchdog_driver are no longer > >>>>used and marked as deprecated. > >>> > >>>Two comments below. It's great to see that unbinding a driver no longer > >>>triggers a kernel panic. > >>> > >>It should not have caused a panic to start with, but the ref/unref functions > >>for the most part were either not or wrongly implemented. Not really > >>surprising - it took me a while to understand the problem. > > > >I tested on a driver which did not implement ref/unref. When ping is > >called, it tries to dereference a freed 'struct watchdog_device' in > >watchdog_get_drvdata, leading to a panic. > > > Yes, that will happen. Problem here is that the driver is buggy - > pretty much all drivers which dynamically allocate struct watchdog_device > have this problem. > > This is the ultimate reason for coming up with this patch. > > >> > >>[ ... ] > >> > >>>> > >>>> /* > >>>>+ * struct _watchdog_device - watchdog core internal data > >>> > >>>Think it should be /**. Anyway, I find it confusing to have both > >>>_watchdog_device and watchdog_device, but I can't think of a better > >>>name right now. > >> > >>I renamed the data structure to watchdog_data and moved it into > >>watchdog_dev.c > >>since it is only used there. No '**', though, because it is not a published > >>API, but just an internal data structure. > >> > >>I also renamed the matching variable name to 'wd_data' (from '_wdd'). > > > >Okay. Also, why didn't you use the explicit type for 'wdd_data' in > >'struct watchdog_device' instead of a void*? > > > > This is to hide the data type, since the structure is not exported > to drivers. > > I could pre-declare the structure with 'struct watchdog_data;', but > then I'd have to use a different structure name (watchdog_cdev_data, > maybe, or watchdog_core_data) to make it less generic. Any opinion ? > Would that be better / preferred ? I am 50/50 about it. My personal preference would be to be explicit. That makes code nagivation easier and it might help the compiler catch some mistakes. Plus, as you moved the structure definition in watchdog_dev.c, it is very clear that drivers aren't supposed to use it. Thanks, Damien -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/5] watchdog: Separate and maintain variables based on variable lifetime
On Mon, Dec 21, 2015 at 05:10:58PM -0800, Guenter Roeck wrote: > On 12/21/2015 09:28 AM, Damien Riegel wrote: > >On Sun, Dec 20, 2015 at 01:05:00PM -0800, Guenter Roeck wrote: > >>All variables required by the watchdog core to manage a watchdog are > >>currently stored in struct watchdog_device. The lifetime of those > >>variables is determined by the watchdog driver. However, the lifetime > >>of variables used by the watchdog core differs from the lifetime of > >>struct watchdog_device. To remedy this situation, watchdog drivers > >>can implement ref and unref callbacks, to be used by the watchdog > >>core to lock struct watchdog_device in memory. > >> > >>While this solves the immediate problem, it depends on watchdog drivers > >>to actually implement the ref/unref callbacks. This is error prone, > >>often not implemented in the first place, or not implemented correctly. > >> > >>To solve the problem without requiring driver support, split the variables > >>in struct watchdog_device into two data structures - one for variables > >>associated with the watchdog driver, one for variables associated with > >>the watchdog core. With this approach, the watchdog core can keep track > >>of its variable lifetime and no longer depends on ref/unref callbacks > >>in the driver. As a side effect, some of the variables originally in > >>struct watchdog_driver are now private to the watchdog core and no longer > >>visible in watchdog drivers. > >> > >>The 'ref' and 'unref' callbacks in struct watchdog_driver are no longer > >>used and marked as deprecated. > > > >Two comments below. It's great to see that unbinding a driver no longer > >triggers a kernel panic. > > > It should not have caused a panic to start with, but the ref/unref functions > for the most part were either not or wrongly implemented. Not really > surprising - it took me a while to understand the problem. I tested on a driver which did not implement ref/unref. When ping is called, it tries to dereference a freed 'struct watchdog_device' in watchdog_get_drvdata, leading to a panic. > > [ ... ] > > >> > >> /* > >>+ * struct _watchdog_device - watchdog core internal data > > > >Think it should be /**. Anyway, I find it confusing to have both > >_watchdog_device and watchdog_device, but I can't think of a better > >name right now. > > I renamed the data structure to watchdog_data and moved it into watchdog_dev.c > since it is only used there. No '**', though, because it is not a published > API, but just an internal data structure. > > I also renamed the matching variable name to 'wd_data' (from '_wdd'). Okay. Also, why didn't you use the explicit type for 'wdd_data' in 'struct watchdog_device' instead of a void*? > > >> > >> static void watchdog_cdev_unregister(struct watchdog_device *wdd) > >> { > >>- mutex_lock(&wdd->lock); > >>- set_bit(WDOG_UNREGISTERED, &wdd->status); > >>- mutex_unlock(&wdd->lock); > >>+ struct _watchdog_device *_wdd = wdd->wdd_data; > >> > >>- cdev_del(&wdd->cdev); > >>+ cdev_del(&_wdd->cdev); > >>if (wdd->id == 0) { > >>misc_deregister(&watchdog_miscdev); > >>- old_wdd = NULL; > >>+ _old_wdd = NULL; > >>} > >>+ > >>+ if (watchdog_active(wdd)) > >>+ pr_crit("watchdog%d: watchdog still running!\n", wdd->id); > > > >As it is now safe to unbind and rebind a driver, it means that a > >watchdog driver probe function can now be called with a running > >watchdog. Some drivers handle this situation, but I think that most of > >them expect the watchdog to be off at this point. > > > No semantics change, though, and no change in behavior. Drivers _should_ > handle that situation today. Sure, many don't, but that is a different issue. All right, that's what confused me. It was, and still will be, driver responsiblity to handle this situation. > > I'll address handling an already-running watchdog by the watchdog core until > the character device is opened in a separate patch set, but we'll have to have > this series accepted before I re-introduce that. Even with that, it will still > be the driver's responsibility to detect and report that/if a watchdog is > already running. > > Thanks, > Guenter > Thanks, Damien -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/5] watchdog: Create watchdog device in watchdog_dev.c
On Mon, Dec 21, 2015 at 03:28:39PM -0800, Guenter Roeck wrote: > On 12/21/2015 09:31 AM, Damien Riegel wrote: > >On Sun, Dec 20, 2015 at 01:04:59PM -0800, Guenter Roeck wrote: > >>The watchdog character device is currently created in watchdog_dev.c, > >>and the watchdog device in watchdog_core.c. This results in > >>cross-dependencies, since device creation needs to know the watchdog > >>character device number as well as the watchdog class, both of which > >>reside in watchdog_dev.c. > >> > >>Create the watchdog device in watchdog_dev.c to simplify the code. > >> > >>Inspired by earlier patch set from Damien Riegel. > > > >Hi Guenter, > > > >The main purpose of my patch was to inverse the device creation and the > >cdev registration to avoid a racy situation, bu you have dropped that in > >this version. Is there a reason for that? > > > Every other driver I looked at does it in the same order (cdev first, device > second). I don't really know if doing it differently has any undesired > side effect, so I wanted to play safe. > > It would help a lot if someone listening to this exchange can confirm > that it is ok to create the device first, followed by the character device. The issue is that some drivers use watchdog_device->dev in their watchdog_ops functions. With a quick grep, I could spot 3 examples: - bcm2835_wdt_stop in bcm2835_wdt.c - gpio_wdt_hwping in gpio_wdt.c - a21_wdt_set_timeout in mena21_wdt.c Maybe we should simply fix these drivers and keep watchdog_device->dev for core internal usage? Thanks, Damien -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 1/2] irqchip: add documentation for TS-4800 interrupt controller
This is an interrupt-controller implemented in an FPGA, to multiplex interrupts generated from other IPs. The FPGA usually uses a GPIO as a parent interrupt controller to notify that one of the multiplexed interrupts has triggered. Signed-off-by: Damien Riegel Acked-by: Rob Herring --- Changes in v2: - Added Acked-by Rob Herring. .../bindings/interrupt-controller/technologic,ts4800.txt | 16 1 file changed, 16 insertions(+) create mode 100644 Documentation/devicetree/bindings/interrupt-controller/technologic,ts4800.txt diff --git a/Documentation/devicetree/bindings/interrupt-controller/technologic,ts4800.txt b/Documentation/devicetree/bindings/interrupt-controller/technologic,ts4800.txt new file mode 100644 index 000..7f15f1b --- /dev/null +++ b/Documentation/devicetree/bindings/interrupt-controller/technologic,ts4800.txt @@ -0,0 +1,16 @@ +TS-4800 FPGA interrupt controller + +TS-4800 FPGA has an internal interrupt controller. When one of the +interrupts is triggered, the SoC is notified, usually using a GPIO as +parent interrupt source. + +Required properties: +- compatible: should be "technologic,ts4800-irqc" +- interrupt-controller: identifies the node as an interrupt controller +- reg: physical base address of the controller and length of memory mapped + region +- #interrupt-cells: specifies the number of cells needed to encode an interrupt + source, should be 1. +- interrupt-parent: phandle to the parent interrupt controller this one is + cascaded from +- interrupts: specifies the interrupt line in the interrupt-parent controller -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 2/2] irqchip: add TS-4800 interrupt controller
This commit adds support for the TS-4800 interrupt controller. This controller is instantiated in a companion FPGA, and multiplex interrupts for other FPGA IPs. As this component is external to the SoC, the SoC might need to reserve pins, so this controller is implemented as a platform driver and doesn't use the IRQCHIP_DECLARE construct. Signed-off-by: Damien Riegel --- Changes in v2: - Added chained_irq_{enter,exit} in interrupt handler. drivers/irqchip/Kconfig | 6 ++ drivers/irqchip/Makefile | 1 + drivers/irqchip/irq-ts4800.c | 163 +++ 3 files changed, 170 insertions(+) create mode 100644 drivers/irqchip/irq-ts4800.c diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig index 27b52c8..e734772 100644 --- a/drivers/irqchip/Kconfig +++ b/drivers/irqchip/Kconfig @@ -137,6 +137,12 @@ config TB10X_IRQC select IRQ_DOMAIN select GENERIC_IRQ_CHIP +config TS4800_IRQ + tristate "TS-4800 IRQ controller" + select IRQ_DOMAIN + help + Support for the TS-4800 FPGA IRQ controller + config VERSATILE_FPGA_IRQ bool select IRQ_DOMAIN diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile index bb3048f..ca9de01 100644 --- a/drivers/irqchip/Makefile +++ b/drivers/irqchip/Makefile @@ -39,6 +39,7 @@ obj-$(CONFIG_ARCH_NSPIRE) += irq-zevio.o obj-$(CONFIG_ARCH_VT8500) += irq-vt8500.o obj-$(CONFIG_ST_IRQCHIP) += irq-st.o obj-$(CONFIG_TB10X_IRQC) += irq-tb10x.o +obj-$(CONFIG_TS4800_IRQ) += irq-ts4800.o obj-$(CONFIG_XTENSA) += irq-xtensa-pic.o obj-$(CONFIG_XTENSA_MX)+= irq-xtensa-mx.o obj-$(CONFIG_IRQ_CROSSBAR) += irq-crossbar.o diff --git a/drivers/irqchip/irq-ts4800.c b/drivers/irqchip/irq-ts4800.c new file mode 100644 index 000..1dc5f3d --- /dev/null +++ b/drivers/irqchip/irq-ts4800.c @@ -0,0 +1,163 @@ +/* + * Multiplexed-IRQs driver for TS-4800's FPGA + * + * Copyright (c) 2015 - Savoir-faire Linux + * + * This file is licensed under the terms of the GNU General Public + * License version 2. This program is licensed "as is" without any + * warranty of any kind, whether express or implied. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define IRQ_MASK0x4 +#define IRQ_STATUS 0x8 + +struct ts4800_irq_data { + void __iomem*base; + struct irq_domain *domain; + struct irq_chip irq_chip; +}; + +static void ts4800_irq_mask(struct irq_data *d) +{ + struct ts4800_irq_data *data = irq_data_get_irq_chip_data(d); + u16 mask = 1 << d->hwirq; + u16 reg = readw(data->base + IRQ_MASK); + + writew(reg | mask, data->base + IRQ_MASK); +} + +static void ts4800_irq_unmask(struct irq_data *d) +{ + struct ts4800_irq_data *data = irq_data_get_irq_chip_data(d); + u16 mask = 1 << d->hwirq; + u16 reg = readw(data->base + IRQ_MASK); + + writew(reg & ~mask, data->base + IRQ_MASK); +} + +static int ts4800_irqdomain_map(struct irq_domain *d, unsigned int irq, + irq_hw_number_t hwirq) +{ + struct ts4800_irq_data *data = d->host_data; + + irq_set_chip_and_handler(irq, &data->irq_chip, handle_simple_irq); + irq_set_chip_data(irq, data); + irq_set_noprobe(irq); + + return 0; +} + +struct irq_domain_ops ts4800_ic_ops = { + .map = ts4800_irqdomain_map, + .xlate = irq_domain_xlate_onecell, +}; + +static void ts4800_ic_chained_handle_irq(struct irq_desc *desc) +{ + struct ts4800_irq_data *data = irq_desc_get_handler_data(desc); + struct irq_chip *chip = irq_desc_get_chip(desc); + u16 status = readw(data->base + IRQ_STATUS); + + chained_irq_enter(chip, desc); + + if (unlikely(status == 0)) { + handle_bad_irq(desc); + goto out; + } + + do { + unsigned int bit = __ffs(status); + int irq = irq_find_mapping(data->domain, bit); + + status &= ~(1 << bit); + generic_handle_irq(irq); + } while (status); + +out: + chained_irq_exit(chip, desc); +} + +static int ts4800_ic_probe(struct platform_device *pdev) +{ + struct device_node *node = pdev->dev.of_node; + struct ts4800_irq_data *data; + struct irq_chip *irq_chip; + struct resource *res; + int parent_irq; + + data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL); + if (!data) + return -ENOMEM; + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + data->base = devm_ioremap_resource(&pdev->dev, res); + if (IS_ERR(data->base)) + return PTR_ERR(data->base
Re: [PATCH 1/2] ARM: dts: ts-4800: Add LCD support
On Mon, Dec 21, 2015 at 09:35:19PM +0800, Shawn Guo wrote: > On Fri, Dec 18, 2015 at 12:00:32PM -0500, Damien Riegel wrote: > > This commit adds LCD support for the TS-4800. The panel is an Okaya > > RS800480T-7X0WQ and the timings have been extracted from Technologic > > Systems' tree. > > > > Signed-off-by: Damien Riegel > > --- > > arch/arm/boot/dts/imx51-ts4800.dts | 109 > > + > > 1 file changed, 109 insertions(+) > > > > diff --git a/arch/arm/boot/dts/imx51-ts4800.dts > > b/arch/arm/boot/dts/imx51-ts4800.dts > > index 83352cb..4951ebd 100644 > > --- a/arch/arm/boot/dts/imx51-ts4800.dts > > +++ b/arch/arm/boot/dts/imx51-ts4800.dts > > @@ -30,6 +30,60 @@ > > clock-frequency = <24576000>; > > }; > > }; > > + > > + regulators { > > + compatible = "simple-bus"; > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + backlight_reg: regulator@0 { > > + compatible = "regulator-fixed"; > > + reg = <0>; > > + pinctrl-names = "default"; > > + pinctrl-0 = <&pinctrl_enable_lcd>; > > + regulator-name = "enable_lcd_reg"; > > + regulator-min-microvolt = <330>; > > + regulator-max-microvolt = <330>; > > + gpio = <&gpio4 9 GPIO_ACTIVE_HIGH>; > > + enable-active-high; > > + }; > > + }; > > DT maintainers dislike this fake simple-bus container. Let's put the > regulator directly under root like below. > > backlight_reg: regulator-backlight { > compatible = "regulator-fixed"; > pinctrl-names = "default"; > pinctrl-0 = <&pinctrl_enable_lcd>; > regulator-name = "enable_lcd_reg"; > regulator-min-microvolt = <330>; > regulator-max-microvolt = <330>; > gpio = <&gpio4 9 GPIO_ACTIVE_HIGH>; > enable-active-high; > }; > > I fixed it up and applied both patches. Thank you. I am using the imx51-*.dts as examples, maybe they should be updated to be consistent with the newer device tree style? I'm willing to do that but I don't want to make changes on dts files that I can't test. Sidenote: your Signed-off-by is missing on these two patches. Thanks, Damien > > Shawn > > > + > > + backlight: backlight { > > + compatible = "pwm-backlight"; > > + pwms = <&pwm1 0 78770>; > > + brightness-levels = <0 150 200 255>; > > + default-brightness-level = <1>; > > + power-supply = <&backlight_reg>; > > + }; > > + > > + display0: display@di0 { > > + compatible = "fsl,imx-parallel-display"; > > + interface-pix-fmt = "rgb24"; > > + pinctrl-names = "default"; > > + pinctrl-0 = <&pinctrl_lcd>; > > + > > + display-timings { > > + 800x480p60 { > > + native-mode; > > + clock-frequency = <30066000>; > > + hactive = <800>; > > + vactive = <480>; > > + hfront-porch = <50>; > > + hback-porch = <70>; > > + hsync-len = <50>; > > + vback-porch = <0>; > > + vfront-porch = <0>; > > + vsync-len = <50>; > > + }; > > + }; > > + > > + port@0 { > > + display0_in: endpoint { > > + remote-endpoint = <&ipu_di0_disp0>; > > + }; > > + }; > > + }; > > }; > > > > &esdhc1 { > > @@ -60,6 +114,16 @@ > > }; > > }; > > > > +&ipu_di0_disp0 { > > + remote-endpoint = <&display0_in>; > > +}; > > + > > +&pwm1 { > > + pinctrl-names = "default"; >
Re: [PATCH 2/5] watchdog: Separate and maintain variables based on variable lifetime
On Sun, Dec 20, 2015 at 01:05:00PM -0800, Guenter Roeck wrote: > All variables required by the watchdog core to manage a watchdog are > currently stored in struct watchdog_device. The lifetime of those > variables is determined by the watchdog driver. However, the lifetime > of variables used by the watchdog core differs from the lifetime of > struct watchdog_device. To remedy this situation, watchdog drivers > can implement ref and unref callbacks, to be used by the watchdog > core to lock struct watchdog_device in memory. > > While this solves the immediate problem, it depends on watchdog drivers > to actually implement the ref/unref callbacks. This is error prone, > often not implemented in the first place, or not implemented correctly. > > To solve the problem without requiring driver support, split the variables > in struct watchdog_device into two data structures - one for variables > associated with the watchdog driver, one for variables associated with > the watchdog core. With this approach, the watchdog core can keep track > of its variable lifetime and no longer depends on ref/unref callbacks > in the driver. As a side effect, some of the variables originally in > struct watchdog_driver are now private to the watchdog core and no longer > visible in watchdog drivers. > > The 'ref' and 'unref' callbacks in struct watchdog_driver are no longer > used and marked as deprecated. Two comments below. It's great to see that unbinding a driver no longer triggers a kernel panic. > > Signed-off-by: Guenter Roeck > --- > Documentation/watchdog/watchdog-kernel-api.txt | 45 +-- > drivers/watchdog/watchdog_core.c | 2 - > drivers/watchdog/watchdog_core.h | 23 ++ > drivers/watchdog/watchdog_dev.c| 377 > + > include/linux/watchdog.h | 21 +- > 5 files changed, 239 insertions(+), 229 deletions(-) > > diff --git a/Documentation/watchdog/watchdog-kernel-api.txt > b/Documentation/watchdog/watchdog-kernel-api.txt > index 0a37da76acef..3db5092924e5 100644 > --- a/Documentation/watchdog/watchdog-kernel-api.txt > +++ b/Documentation/watchdog/watchdog-kernel-api.txt > @@ -44,7 +44,6 @@ The watchdog device structure looks like this: > > struct watchdog_device { > int id; > - struct cdev cdev; > struct device *dev; > struct device *parent; > const struct watchdog_info *info; > @@ -56,7 +55,7 @@ struct watchdog_device { > struct notifier_block reboot_nb; > struct notifier_block restart_nb; > void *driver_data; > - struct mutex lock; > + void *wdd_data; > unsigned long status; > struct list_head deferred; > }; > @@ -66,8 +65,6 @@ It contains following fields: >/dev/watchdog0 cdev (dynamic major, minor 0) as well as the old >/dev/watchdog miscdev. The id is set automatically when calling >watchdog_register_device. > -* cdev: cdev for the dynamic /dev/watchdog device nodes. This > - field is also populated by watchdog_register_device. > * dev: device under the watchdog class (created by watchdog_register_device). > * parent: set this to the parent device (or NULL) before calling >watchdog_register_device. > @@ -89,11 +86,10 @@ It contains following fields: > * driver_data: a pointer to the drivers private data of a watchdog device. >This data should only be accessed via the watchdog_set_drvdata and >watchdog_get_drvdata routines. > -* lock: Mutex for WatchDog Timer Driver Core internal use only. > +* wdd_data: a pointer to watchdog core internal data. > * status: this field contains a number of status bits that give extra >information about the status of the device (Like: is the watchdog timer > - running/active, is the nowayout bit set, is the device opened via > - the /dev/watchdog interface or not, ...). > + running/active, or is the nowayout bit set). > * deferred: entry in wtd_deferred_reg_list which is used to >register early initialized watchdogs. > > @@ -110,8 +106,8 @@ struct watchdog_ops { > int (*set_timeout)(struct watchdog_device *, unsigned int); > unsigned int (*get_timeleft)(struct watchdog_device *); > int (*restart)(struct watchdog_device *); > - void (*ref)(struct watchdog_device *); > - void (*unref)(struct watchdog_device *); > + void (*ref)(struct watchdog_device *) __deprecated; > + void (*unref)(struct watchdog_device *) __deprecated; > long (*ioctl)(struct watchdog_device *, unsigned int, unsigned long); > }; > > @@ -120,20 +116,6 @@ driver's operations. This module owner will be used to > lock the module when > the watchdog is active. (This to avoid a system crash when you unload the > module and /dev/watchdog is still open). > > -If the watchdog_device struct is dynamically allocated, just locking the > module > -is not enough and a driver also needs to define the ref and unref operations > to > -ensure the structure holding the watc
Re: [PATCH 1/5] watchdog: Create watchdog device in watchdog_dev.c
On Sun, Dec 20, 2015 at 01:04:59PM -0800, Guenter Roeck wrote: > The watchdog character device is currently created in watchdog_dev.c, > and the watchdog device in watchdog_core.c. This results in > cross-dependencies, since device creation needs to know the watchdog > character device number as well as the watchdog class, both of which > reside in watchdog_dev.c. > > Create the watchdog device in watchdog_dev.c to simplify the code. > > Inspired by earlier patch set from Damien Riegel. Hi Guenter, The main purpose of my patch was to inverse the device creation and the cdev registration to avoid a racy situation, bu you have dropped that in this version. Is there a reason for that? Thanks, Damien > > Cc: Damien Riegel > Signed-off-by: Guenter Roeck > --- > drivers/watchdog/watchdog_core.c | 33 -- > drivers/watchdog/watchdog_core.h | 4 +-- > drivers/watchdog/watchdog_dev.c | 73 > +--- > 3 files changed, 69 insertions(+), 41 deletions(-) > > diff --git a/drivers/watchdog/watchdog_core.c > b/drivers/watchdog/watchdog_core.c > index 551af042867c..f0293f7d2b80 100644 > --- a/drivers/watchdog/watchdog_core.c > +++ b/drivers/watchdog/watchdog_core.c > @@ -42,7 +42,6 @@ > #include "watchdog_core.h" /* For watchdog_dev_register/... */ > > static DEFINE_IDA(watchdog_ida); > -static struct class *watchdog_class; > > /* > * Deferred Registration infrastructure. > @@ -194,7 +193,7 @@ EXPORT_SYMBOL_GPL(watchdog_set_restart_priority); > > static int __watchdog_register_device(struct watchdog_device *wdd) > { > - int ret, id = -1, devno; > + int ret, id = -1; > > if (wdd == NULL || wdd->info == NULL || wdd->ops == NULL) > return -EINVAL; > @@ -247,16 +246,6 @@ static int __watchdog_register_device(struct > watchdog_device *wdd) > } > } > > - devno = wdd->cdev.dev; > - wdd->dev = device_create(watchdog_class, wdd->parent, devno, > - wdd, "watchdog%d", wdd->id); > - if (IS_ERR(wdd->dev)) { > - watchdog_dev_unregister(wdd); > - ida_simple_remove(&watchdog_ida, id); > - ret = PTR_ERR(wdd->dev); > - return ret; > - } > - > if (test_bit(WDOG_STOP_ON_REBOOT, &wdd->status)) { > wdd->reboot_nb.notifier_call = watchdog_reboot_notifier; > > @@ -265,9 +254,7 @@ static int __watchdog_register_device(struct > watchdog_device *wdd) > dev_err(wdd->dev, "Cannot register reboot notifier > (%d)\n", > ret); > watchdog_dev_unregister(wdd); > - device_destroy(watchdog_class, devno); > ida_simple_remove(&watchdog_ida, wdd->id); > - wdd->dev = NULL; > return ret; > } > } > @@ -311,9 +298,6 @@ EXPORT_SYMBOL_GPL(watchdog_register_device); > > static void __watchdog_unregister_device(struct watchdog_device *wdd) > { > - int ret; > - int devno; > - > if (wdd == NULL) > return; > > @@ -323,13 +307,8 @@ static void __watchdog_unregister_device(struct > watchdog_device *wdd) > if (test_bit(WDOG_STOP_ON_REBOOT, &wdd->status)) > unregister_reboot_notifier(&wdd->reboot_nb); > > - devno = wdd->cdev.dev; > - ret = watchdog_dev_unregister(wdd); > - if (ret) > - pr_err("error unregistering /dev/watchdog (err=%d)\n", ret); > - device_destroy(watchdog_class, devno); > + watchdog_dev_unregister(wdd); > ida_simple_remove(&watchdog_ida, wdd->id); > - wdd->dev = NULL; > } > > /** > @@ -370,9 +349,11 @@ static int __init watchdog_deferred_registration(void) > > static int __init watchdog_init(void) > { > - watchdog_class = watchdog_dev_init(); > - if (IS_ERR(watchdog_class)) > - return PTR_ERR(watchdog_class); > + int err; > + > + err = watchdog_dev_init(); > + if (err < 0) > + return err; > > watchdog_deferred_registration(); > return 0; > diff --git a/drivers/watchdog/watchdog_core.h > b/drivers/watchdog/watchdog_core.h > index 1c8d6b0e68c7..86ff962d1e15 100644 > --- a/drivers/watchdog/watchdog_core.h > +++ b/drivers/watchdog/watchdog_core.h > @@ -32,6 +32,6 @@ > * Functions/procedures to be called by the core > */ > extern int watchdog_dev_register(struct watchdog_device *);
Re: [PATCH 1/2] can: sja1000: add documentation for Technologic Systems version
On Sat, Dec 19, 2015 at 09:37:42PM -0600, Rob Herring wrote: > On Fri, Dec 18, 2015 at 03:17:24PM -0500, Damien Riegel wrote: > > This commit adds documentation for the Technologic Systems version of > > SJA1000. The difference with the NXP version is in the way the registers > > are accessed. > > > > Signed-off-by: Damien Riegel > > --- > > Documentation/devicetree/bindings/net/can/sja1000.txt | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/Documentation/devicetree/bindings/net/can/sja1000.txt > > b/Documentation/devicetree/bindings/net/can/sja1000.txt > > index b4a6d53..7a158d5 100644 > > --- a/Documentation/devicetree/bindings/net/can/sja1000.txt > > +++ b/Documentation/devicetree/bindings/net/can/sja1000.txt > > @@ -2,7 +2,7 @@ Memory mapped SJA1000 CAN controller from NXP (formerly > > Philips) > > > > Required properties: > > > > -- compatible : should be "nxp,sja1000". > > +- compatible : should be one of "nxp,sja1000", "technologic,sja1000". > > > > - reg : should specify the chip select, address offset and size required > > to map the registers of the SJA1000. The size is usually 0x80. > > @@ -14,6 +14,7 @@ Optional properties: > > > > - reg-io-width : Specify the size (in bytes) of the IO accesses that > > should be performed on the device. Valid value is 1, 2 or 4. > > + Must be set to 2 for technologic version. > > Default to 1 (8 bits). > > Really, this should default to 2 for technologic version and not be > required. Would something along the line of "This property is for technologic version." be more appropriate? Thanks, Damien -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC PATCH] can: sja1000: of: add per-compatible init hook
This commit adds the capability to allocate and init private data embedded in the sja1000_priv structure on a per-compatible basis. The device node is passed as a parameter of the init callback to allow parsing of custom device tree properties. Signed-off-by: Damien Riegel --- Hi Marc, I am sending this patch as RFC as I have only compile-tested it, but I would like your feedback on it. The idea of this patch is to ease integration of the Technologic Systems version of this IP in the most generic way, providing a facility that could be used by others. Thanks, Damien drivers/net/can/sja1000/sja1000_platform.c | 44 +- 1 file changed, 37 insertions(+), 7 deletions(-) diff --git a/drivers/net/can/sja1000/sja1000_platform.c b/drivers/net/can/sja1000/sja1000_platform.c index 0552ed4..33581b0 100644 --- a/drivers/net/can/sja1000/sja1000_platform.c +++ b/drivers/net/can/sja1000/sja1000_platform.c @@ -40,6 +40,11 @@ MODULE_DESCRIPTION("Socket-CAN driver for SJA1000 on the platform bus"); MODULE_ALIAS("platform:" DRV_NAME); MODULE_LICENSE("GPL v2"); +struct sja1000_of_data { + size_t priv_sz; + int (*init)(struct sja1000_priv *priv, struct device_node *of); +}; + static u8 sp_read_reg8(const struct sja1000_priv *priv, int reg) { return ioread8(priv->reg_base + reg); @@ -154,6 +159,26 @@ static void sp_populate_of(struct sja1000_priv *priv, struct device_node *of) priv->cdr |= CDR_CBP; /* default */ } +static const struct of_device_id sp_of_table[] = { + {.compatible = "nxp,sja1000"}, + {}, +}; +MODULE_DEVICE_TABLE(of, sp_of_table); + +static const struct sja1000_of_data *sp_get_of_data(struct device_node *of) +{ + const struct of_device_id *id; + + if (!of) + return NULL; + + id = of_match_node(sp_of_table, of); + if (!id) + return NULL; + + return id->data; +} + static int sp_probe(struct platform_device *pdev) { int err, irq = 0; @@ -163,6 +188,8 @@ static int sp_probe(struct platform_device *pdev) struct resource *res_mem, *res_irq = NULL; struct sja1000_platform_data *pdata; struct device_node *of = pdev->dev.of_node; + const struct sja1000_of_data *of_data = sp_get_of_data(of); + size_t priv_sz = 0; pdata = dev_get_platdata(&pdev->dev); if (!pdata && !of) { @@ -191,7 +218,10 @@ static int sp_probe(struct platform_device *pdev) if (!irq && !res_irq) return -ENODEV; - dev = alloc_sja1000dev(0); + if (of_data) + priv_sz = of_data->priv_sz; + + dev = alloc_sja1000dev(priv_sz); if (!dev) return -ENOMEM; priv = netdev_priv(dev); @@ -213,6 +243,12 @@ static int sp_probe(struct platform_device *pdev) else sp_populate(priv, pdata, res_mem->flags); + if (of_data && of_data->init) { + err = of_data->init(priv, of); + if (err) + goto exit_free; + } + platform_set_drvdata(pdev, dev); SET_NETDEV_DEV(dev, &pdev->dev); @@ -242,12 +278,6 @@ static int sp_remove(struct platform_device *pdev) return 0; } -static const struct of_device_id sp_of_table[] = { - {.compatible = "nxp,sja1000"}, - {}, -}; -MODULE_DEVICE_TABLE(of, sp_of_table); - static struct platform_driver sp_driver = { .probe = sp_probe, .remove = sp_remove, -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] can: sja1000: of: add compatibility with Technologic Systems version
On Fri, Dec 18, 2015 at 09:41:47PM +0100, Marc Kleine-Budde wrote: > On 12/18/2015 09:17 PM, Damien Riegel wrote: > > Technologic Systems provides an IP compatible with the SJA1000, > > instantiated in an FPGA. Because of some bus widths issue, access to > > registers is made through a "window" that works like this: > > > > base + 0x0: address to read/write > > base + 0x2: 8-bit register value > > > > This commit adds a new compatible device, "technologic,sja1000", with > > read and write functions using the window mechanism. > > > > Signed-off-by: Damien Riegel > > --- > > drivers/net/can/sja1000/sja1000_platform.c | 30 > > -- > > 1 file changed, 28 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/net/can/sja1000/sja1000_platform.c > > b/drivers/net/can/sja1000/sja1000_platform.c > > index 0552ed4..6cbf251 100644 > > --- a/drivers/net/can/sja1000/sja1000_platform.c > > +++ b/drivers/net/can/sja1000/sja1000_platform.c > > @@ -70,6 +70,18 @@ static void sp_write_reg32(const struct sja1000_priv > > *priv, int reg, u8 val) > > iowrite8(val, priv->reg_base + reg * 4); > > } > > > > +static u8 ts4800_read_reg16(const struct sja1000_priv *priv, int reg) > > +{ > > + sp_write_reg16(priv, 0, reg); > > + return sp_read_reg16(priv, 2); > > This is racy, please add a spinlock. > > > +} > > + > > +static void ts4800_write_reg16(const struct sja1000_priv *priv, int reg, > > u8 val) > > +{ > > + sp_write_reg16(priv, 0, reg); > > + sp_write_reg16(priv, 2, val); > > This is racy, too. > > Have a look at https://marc.info/?l=linux-can&m=137149497403825&w=2 Thank you for the link. In my situation, I don't think there is a race issue at the bus level, so a per-device spinlock should be enough. Would that be an acceptable patch? It would look a lot like the patch you suggested in the thread you linked, just rebased on current version. Thanks, Damien -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] can: sja1000: add documentation for Technologic Systems version
This commit adds documentation for the Technologic Systems version of SJA1000. The difference with the NXP version is in the way the registers are accessed. Signed-off-by: Damien Riegel --- Documentation/devicetree/bindings/net/can/sja1000.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/net/can/sja1000.txt b/Documentation/devicetree/bindings/net/can/sja1000.txt index b4a6d53..7a158d5 100644 --- a/Documentation/devicetree/bindings/net/can/sja1000.txt +++ b/Documentation/devicetree/bindings/net/can/sja1000.txt @@ -2,7 +2,7 @@ Memory mapped SJA1000 CAN controller from NXP (formerly Philips) Required properties: -- compatible : should be "nxp,sja1000". +- compatible : should be one of "nxp,sja1000", "technologic,sja1000". - reg : should specify the chip select, address offset and size required to map the registers of the SJA1000. The size is usually 0x80. @@ -14,6 +14,7 @@ Optional properties: - reg-io-width : Specify the size (in bytes) of the IO accesses that should be performed on the device. Valid value is 1, 2 or 4. + Must be set to 2 for technologic version. Default to 1 (8 bits). - nxp,external-clock-frequency : Frequency of the external oscillator -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/2] can: sja1000: of: add compatibility with Technologic Systems version
Technologic Systems provides an IP compatible with the SJA1000, instantiated in an FPGA. Because of some bus widths issue, access to registers is made through a "window" that works like this: base + 0x0: address to read/write base + 0x2: 8-bit register value This commit adds a new compatible device, "technologic,sja1000", with read and write functions using the window mechanism. Signed-off-by: Damien Riegel --- drivers/net/can/sja1000/sja1000_platform.c | 30 -- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/drivers/net/can/sja1000/sja1000_platform.c b/drivers/net/can/sja1000/sja1000_platform.c index 0552ed4..6cbf251 100644 --- a/drivers/net/can/sja1000/sja1000_platform.c +++ b/drivers/net/can/sja1000/sja1000_platform.c @@ -70,6 +70,18 @@ static void sp_write_reg32(const struct sja1000_priv *priv, int reg, u8 val) iowrite8(val, priv->reg_base + reg * 4); } +static u8 ts4800_read_reg16(const struct sja1000_priv *priv, int reg) +{ + sp_write_reg16(priv, 0, reg); + return sp_read_reg16(priv, 2); +} + +static void ts4800_write_reg16(const struct sja1000_priv *priv, int reg, u8 val) +{ + sp_write_reg16(priv, 0, reg); + sp_write_reg16(priv, 2, val); +} + static void sp_populate(struct sja1000_priv *priv, struct sja1000_platform_data *pdata, unsigned long resource_mem_flags) @@ -98,21 +110,34 @@ static void sp_populate(struct sja1000_priv *priv, static void sp_populate_of(struct sja1000_priv *priv, struct device_node *of) { + int is_technologic; int err; u32 prop; + is_technologic = of_device_is_compatible(of, "technologic,sja1000"); + err = of_property_read_u32(of, "reg-io-width", &prop); if (err) prop = 1; /* 8 bit is default */ + if (is_technologic && prop != 2) { + netdev_warn(priv->dev, "forcing reg-io-width to 2\n"); + prop = 2; + } + switch (prop) { case 4: priv->read_reg = sp_read_reg32; priv->write_reg = sp_write_reg32; break; case 2: - priv->read_reg = sp_read_reg16; - priv->write_reg = sp_write_reg16; + if (is_technologic) { + priv->read_reg = ts4800_read_reg16; + priv->write_reg = ts4800_write_reg16; + } else { + priv->read_reg = sp_read_reg16; + priv->write_reg = sp_write_reg16; + } break; case 1: /* fallthrough */ default: @@ -244,6 +269,7 @@ static int sp_remove(struct platform_device *pdev) static const struct of_device_id sp_of_table[] = { {.compatible = "nxp,sja1000"}, + {.compatible = "technologic,sja1000"}, {}, }; MODULE_DEVICE_TABLE(of, sp_of_table); -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] irqchip: add documentation for TS-4800 interrupt controller
This is an interrupt-controller implemented in an FPGA, to multiplex interrupts generated from other IPs. The FPGA usually uses a GPIO as a parent interrupt controller to notify that one of the multiplexed interrupts has triggered. Signed-off-by: Damien Riegel --- .../bindings/interrupt-controller/technologic,ts4800.txt | 16 1 file changed, 16 insertions(+) create mode 100644 Documentation/devicetree/bindings/interrupt-controller/technologic,ts4800.txt diff --git a/Documentation/devicetree/bindings/interrupt-controller/technologic,ts4800.txt b/Documentation/devicetree/bindings/interrupt-controller/technologic,ts4800.txt new file mode 100644 index 000..7f15f1b --- /dev/null +++ b/Documentation/devicetree/bindings/interrupt-controller/technologic,ts4800.txt @@ -0,0 +1,16 @@ +TS-4800 FPGA interrupt controller + +TS-4800 FPGA has an internal interrupt controller. When one of the +interrupts is triggered, the SoC is notified, usually using a GPIO as +parent interrupt source. + +Required properties: +- compatible: should be "technologic,ts4800-irqc" +- interrupt-controller: identifies the node as an interrupt controller +- reg: physical base address of the controller and length of memory mapped + region +- #interrupt-cells: specifies the number of cells needed to encode an interrupt + source, should be 1. +- interrupt-parent: phandle to the parent interrupt controller this one is + cascaded from +- interrupts: specifies the interrupt line in the interrupt-parent controller -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/2] irqchip: add TS-4800 interrupt controller
This commit adds support for the TS-4800 interrupt controller. This controller is instantiated in a companion FPGA, and multiplex interrupts for other FPGA IPs. As this component is external to the SoC, the SoC might need to reserve pins, so this controller is implemented as a platform driver and doesn't use the IRQCHIP_DECLARE construct. Signed-off-by: Damien Riegel --- drivers/irqchip/Kconfig | 6 ++ drivers/irqchip/Makefile | 1 + drivers/irqchip/irq-ts4800.c | 156 +++ 3 files changed, 163 insertions(+) create mode 100644 drivers/irqchip/irq-ts4800.c diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig index 27b52c8..e734772 100644 --- a/drivers/irqchip/Kconfig +++ b/drivers/irqchip/Kconfig @@ -137,6 +137,12 @@ config TB10X_IRQC select IRQ_DOMAIN select GENERIC_IRQ_CHIP +config TS4800_IRQ + tristate "TS-4800 IRQ controller" + select IRQ_DOMAIN + help + Support for the TS-4800 FPGA IRQ controller + config VERSATILE_FPGA_IRQ bool select IRQ_DOMAIN diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile index bb3048f..ca9de01 100644 --- a/drivers/irqchip/Makefile +++ b/drivers/irqchip/Makefile @@ -39,6 +39,7 @@ obj-$(CONFIG_ARCH_NSPIRE) += irq-zevio.o obj-$(CONFIG_ARCH_VT8500) += irq-vt8500.o obj-$(CONFIG_ST_IRQCHIP) += irq-st.o obj-$(CONFIG_TB10X_IRQC) += irq-tb10x.o +obj-$(CONFIG_TS4800_IRQ) += irq-ts4800.o obj-$(CONFIG_XTENSA) += irq-xtensa-pic.o obj-$(CONFIG_XTENSA_MX)+= irq-xtensa-mx.o obj-$(CONFIG_IRQ_CROSSBAR) += irq-crossbar.o diff --git a/drivers/irqchip/irq-ts4800.c b/drivers/irqchip/irq-ts4800.c new file mode 100644 index 000..f5a4d5b --- /dev/null +++ b/drivers/irqchip/irq-ts4800.c @@ -0,0 +1,156 @@ +/* + * Multiplexed-IRQs driver for TS-4800's FPGA + * + * Copyright (c) 2015 - Savoir-faire Linux + * + * This file is licensed under the terms of the GNU General Public + * License version 2. This program is licensed "as is" without any + * warranty of any kind, whether express or implied. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define IRQ_MASK0x4 +#define IRQ_STATUS 0x8 + +struct ts4800_irq_data { + void __iomem*base; + struct irq_domain *domain; + struct irq_chip irq_chip; +}; + +static void ts4800_irq_mask(struct irq_data *d) +{ + struct ts4800_irq_data *data = irq_data_get_irq_chip_data(d); + u16 mask = 1 << d->hwirq; + u16 reg = readw(data->base + IRQ_MASK); + + writew(reg | mask, data->base + IRQ_MASK); +} + +static void ts4800_irq_unmask(struct irq_data *d) +{ + struct ts4800_irq_data *data = irq_data_get_irq_chip_data(d); + u16 mask = 1 << d->hwirq; + u16 reg = readw(data->base + IRQ_MASK); + + writew(reg & ~mask, data->base + IRQ_MASK); +} + +static int ts4800_irqdomain_map(struct irq_domain *d, unsigned int irq, + irq_hw_number_t hwirq) +{ + struct ts4800_irq_data *data = d->host_data; + + irq_set_chip_and_handler(irq, &data->irq_chip, handle_simple_irq); + irq_set_chip_data(irq, data); + irq_set_noprobe(irq); + + return 0; +} + +struct irq_domain_ops ts4800_ic_ops = { + .map = ts4800_irqdomain_map, + .xlate = irq_domain_xlate_onecell, +}; + +static void ts4800_ic_chained_handle_irq(struct irq_desc *desc) +{ + struct ts4800_irq_data *data = irq_desc_get_handler_data(desc); + u16 status = readw(data->base + IRQ_STATUS); + + if (unlikely(status == 0)) { + handle_bad_irq(desc); + return; + } + + do { + unsigned int bit = __ffs(status); + int irq = irq_find_mapping(data->domain, bit); + + status &= ~(1 << bit); + generic_handle_irq(irq); + } while (status); +} + +static int ts4800_ic_probe(struct platform_device *pdev) +{ + struct device_node *node = pdev->dev.of_node; + struct ts4800_irq_data *data; + struct irq_chip *irq_chip; + struct resource *res; + int parent_irq; + + data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL); + if (!data) + return -ENOMEM; + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + data->base = devm_ioremap_resource(&pdev->dev, res); + if (IS_ERR(data->base)) + return PTR_ERR(data->base); + + writew(0x, data->base + IRQ_MASK); + + parent_irq = irq_of_parse_and_map(node, 0); + if (!parent_irq) { + dev_err(&pdev->dev, "failed to get parent IRQ\n"); +
[PATCH 2/2] ARM: dts: TS-4800: add touchscreen support
This commit enables the touchscreen on TS-4800, using the ts4800-ts driver. Signed-off-by: Damien Riegel --- arch/arm/boot/dts/imx51-ts4800.dts | 6 ++ 1 file changed, 6 insertions(+) diff --git a/arch/arm/boot/dts/imx51-ts4800.dts b/arch/arm/boot/dts/imx51-ts4800.dts index 4951ebd..e5e9bd1 100644 --- a/arch/arm/boot/dts/imx51-ts4800.dts +++ b/arch/arm/boot/dts/imx51-ts4800.dts @@ -166,6 +166,12 @@ syscon = <&syscon 0xe>; }; }; + + touchscreen { + compatible = "technologic,ts4800-ts"; + reg = <0x12000 0x1000>; + syscon = <&syscon 0x10 6>; + }; }; }; -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] ARM: dts: ts-4800: Add LCD support
This commit adds LCD support for the TS-4800. The panel is an Okaya RS800480T-7X0WQ and the timings have been extracted from Technologic Systems' tree. Signed-off-by: Damien Riegel --- arch/arm/boot/dts/imx51-ts4800.dts | 109 + 1 file changed, 109 insertions(+) diff --git a/arch/arm/boot/dts/imx51-ts4800.dts b/arch/arm/boot/dts/imx51-ts4800.dts index 83352cb..4951ebd 100644 --- a/arch/arm/boot/dts/imx51-ts4800.dts +++ b/arch/arm/boot/dts/imx51-ts4800.dts @@ -30,6 +30,60 @@ clock-frequency = <24576000>; }; }; + + regulators { + compatible = "simple-bus"; + #address-cells = <1>; + #size-cells = <0>; + + backlight_reg: regulator@0 { + compatible = "regulator-fixed"; + reg = <0>; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_enable_lcd>; + regulator-name = "enable_lcd_reg"; + regulator-min-microvolt = <330>; + regulator-max-microvolt = <330>; + gpio = <&gpio4 9 GPIO_ACTIVE_HIGH>; + enable-active-high; + }; + }; + + backlight: backlight { + compatible = "pwm-backlight"; + pwms = <&pwm1 0 78770>; + brightness-levels = <0 150 200 255>; + default-brightness-level = <1>; + power-supply = <&backlight_reg>; + }; + + display0: display@di0 { + compatible = "fsl,imx-parallel-display"; + interface-pix-fmt = "rgb24"; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_lcd>; + + display-timings { + 800x480p60 { + native-mode; + clock-frequency = <30066000>; + hactive = <800>; + vactive = <480>; + hfront-porch = <50>; + hback-porch = <70>; + hsync-len = <50>; + vback-porch = <0>; + vfront-porch = <0>; + vsync-len = <50>; + }; + }; + + port@0 { + display0_in: endpoint { + remote-endpoint = <&ipu_di0_disp0>; + }; + }; + }; }; &esdhc1 { @@ -60,6 +114,16 @@ }; }; +&ipu_di0_disp0 { + remote-endpoint = <&display0_in>; +}; + +&pwm1 { + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_pwm_backlight>; + status = "okay"; +}; + &uart1 { pinctrl-names = "default"; pinctrl-0 = <&pinctrl_uart1>; @@ -115,6 +179,12 @@ >; }; + pinctrl_enable_lcd: enablelcdgrp { + fsl,pins = < + MX51_PAD_CSI2_D12__GPIO4_9 0x1c5 + >; + }; + pinctrl_esdhc1: esdhc1grp { fsl,pins = < MX51_PAD_SD1_CMD__SD1_CMD 0x400020d5 @@ -159,6 +229,45 @@ >; }; + pinctrl_lcd: lcdgrp { + fsl,pins = < + MX51_PAD_DISP1_DAT0__DISP1_DAT0 0x5 + MX51_PAD_DISP1_DAT1__DISP1_DAT1 0x5 + MX51_PAD_DISP1_DAT2__DISP1_DAT2 0x5 + MX51_PAD_DISP1_DAT3__DISP1_DAT3 0x5 + MX51_PAD_DISP1_DAT4__DISP1_DAT4 0x5 + MX51_PAD_DISP1_DAT5__DISP1_DAT5 0x5 + MX51_PAD_DISP1_DAT6__DISP1_DAT6 0x5 + MX51_PAD_DISP1_DAT7__DISP1_DAT7 0x5 + MX51_PAD_DISP1_DAT8__DISP1_DAT8 0x5 + MX51_PAD_DISP1_DAT9__DISP1_DAT9 0x5 + MX51_PAD_DISP1_DAT10__DISP1_DAT10 0x5 + MX51_PAD_DISP1_DAT11__DISP1_DAT11 0x5 + MX51_PAD_DISP1_DAT12__DISP1_DAT12 0x5 + MX51_PAD_DISP1_DAT13__DISP1_DAT13 0x5 + MX51_PAD_DISP1_DAT14__DISP1_DAT14 0x5 + MX51_PAD_DISP1_DAT15__DISP1_DAT15 0x5 + MX51_PAD_DISP1_DAT16__DISP1_DAT16 0x5 + MX51_PAD_DISP1_DAT17__DISP1_DAT1
[PATCH 1/2] ARM: dts: TS-4800: drop uart rts/cts pin reservations
These pins are actually not routed for UARTs, they should not be reserved. Signed-off-by: Damien Riegel --- arch/arm/boot/dts/imx51-ts4800.dts | 4 1 file changed, 4 deletions(-) diff --git a/arch/arm/boot/dts/imx51-ts4800.dts b/arch/arm/boot/dts/imx51-ts4800.dts index f1317f7..64ac55c 100644 --- a/arch/arm/boot/dts/imx51-ts4800.dts +++ b/arch/arm/boot/dts/imx51-ts4800.dts @@ -157,8 +157,6 @@ fsl,pins = < MX51_PAD_UART1_RXD__UART1_RXD 0x1c5 MX51_PAD_UART1_TXD__UART1_TXD 0x1c5 - MX51_PAD_UART1_RTS__UART1_RTS 0x1c5 - MX51_PAD_UART1_CTS__UART1_CTS 0x1c5 >; }; @@ -173,8 +171,6 @@ fsl,pins = < MX51_PAD_EIM_D25__UART3_RXD 0x1c5 MX51_PAD_EIM_D26__UART3_TXD 0x1c5 - MX51_PAD_EIM_D27__UART3_RTS 0x1c5 - MX51_PAD_EIM_D24__UART3_CTS 0x1c5 >; }; }; -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/2] ARM: dts: TS-4800: use weim IP to map the FPGA
Previously, the device tree mapped the FPGA like any other IPs inside the SoC, but it is actually mapped through the WEIM (Wireless External Interface Module). This patch updates the device tree to make use of it. About the timings: in the image provided by the manufacturer, only CS0GCR1 is changed. The other values are the default ones, but the WEIM bindings expect them to be all explicitly set in the device tree, so I just put the default values in the dt. Signed-off-by: Damien Riegel --- arch/arm/boot/dts/imx51-ts4800.dts | 60 +- 1 file changed, 39 insertions(+), 21 deletions(-) diff --git a/arch/arm/boot/dts/imx51-ts4800.dts b/arch/arm/boot/dts/imx51-ts4800.dts index 64ac55c..83352cb 100644 --- a/arch/arm/boot/dts/imx51-ts4800.dts +++ b/arch/arm/boot/dts/imx51-ts4800.dts @@ -21,27 +21,6 @@ reg = <0x9000 0x1000>; }; - soc { - fpga { - compatible = "simple-bus"; - reg = <0xb000 0x1d000>; - #address-cells = <1>; - #size-cells = <1>; - ranges; - - syscon: syscon@b001 { - compatible = "syscon", "simple-mfd"; - reg = <0xb001 0x3d>; - reg-io-width = <2>; - - wdt@e { - compatible = "technologic,ts4800-wdt"; - syscon = <&syscon 0xe>; - }; - }; - }; - }; - clocks { ckih1 { clock-frequency = <22579200>; @@ -99,6 +78,33 @@ status = "okay"; }; +&weim { + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_weim>; + status = "okay"; + + fpga@0 { + compatible = "simple-bus"; + fsl,weim-cs-timing = <0x0061008F 0x0002 0x1c022000 + 0x 0x1c092480 0x>; + reg = <0 0x000 0x1d000>; + #address-cells = <1>; + #size-cells = <1>; + ranges = <0 0 0 0x1d000>; + + syscon: syscon@b001 { + compatible = "syscon", "simple-mfd"; + reg = <0x1 0x3d>; + reg-io-width = <2>; + + wdt@e { + compatible = "technologic,ts4800-wdt"; + syscon = <&syscon 0xe>; + }; + }; + }; +}; + &iomuxc { pinctrl_ecspi1: ecspi1grp { fsl,pins = < @@ -173,4 +179,16 @@ MX51_PAD_EIM_D26__UART3_TXD 0x1c5 >; }; + + pinctrl_weim: weimgrp { + fsl,pins = < + MX51_PAD_EIM_DTACK__EIM_DTACK 0x85 + MX51_PAD_EIM_CS0__EIM_CS0 0x0 + MX51_PAD_EIM_CS1__EIM_CS1 0x0 + MX51_PAD_EIM_EB0__EIM_EB0 0x85 + MX51_PAD_EIM_EB1__EIM_EB1 0x85 + MX51_PAD_EIM_OE__EIM_OE 0x85 + MX51_PAD_EIM_LBA__EIM_LBA 0x85 + >; + }; }; -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC/RFT PATCH] watchdog: Move watchdog device creation to watchdog_dev.c
On Mon, Dec 07, 2015 at 09:41:03PM +0100, Wim Van Sebroeck wrote: > Hi All, > > > On 12/07/2015 08:15 AM, Damien Riegel wrote: > > >On Sun, Dec 06, 2015 at 11:51:41AM -0800, Guenter Roeck wrote: > > >>The watchdog character device s currently created in > > >>watchdog_dev.c, and the watchdog device in watchdog_core.c. This > > >>results in cross-dependencies, as the device creation needs to > > >>know the watchdog character device number. > > >> > > >>On top of that, the watchdog character device is created before > > >>the watchdog device is created. This can result in race conditions > > >>if the watchdog device node is accessed before the watchdog device > > >>has been created. > > >> > > >>To solve the problem, move watchdog device creation into > > >>watchdog_dev.c, and create the watchdog device prior to creating > > >>its device node. Also move device class creation into > > >>watchdog_dev.c, since this is now the only place where the > > >>watchdog class is needed. > > >> > > >>Inspired by an earlier patch set from Damien Riegel. > > >> > > >>Cc: Damien Riegel > > >>Signed-off-by: Guenter Roeck --- Hi Damien, > > >> > > >>I think this approach would be a bit better. The watchdog device > > >>isn't really used in the watchdog core code, so it is better > > >>created in watchdog_dev.c. That also fits well with other pending > > >>changes, such as sysfs attribute support, and my attempts to move > > >>the ref/unref functions completely into the watchdog core. As a > > >>side effect, it also cleans up the error path in > > >>__watchdog_register_device(). > > >> > > >>What do you think ? > > > > > >Hi Guenter, > > > > > >Like the idea, but I don't really get the separation. For instance, > > >you move watchdog_class in watchdog_dev.c but you keep watchdog_ida > > >in watchdog_core.c whereas it is only used for device > > >creation/deletion. > > > > > The class is watchdog driver internal, and it is device related, so > > I think it made sense to move it to watchdog_dev.c. On top of that, > > it will be needed there if/when we introduce sysfs attributes. > > > > The watchdog id can be determined by obtaining an id using ida, or > > it can be provided through the watchdog alias. The operation to get > > it is not device related, and it is not straightforward to obtain > > it, so I thought it makes sense to keep the code in watchdog_core.c. > > > > Of course a lot of it is personal preference. > > > > Let me go back to how I saw the design when I created the generic > watchdog framework: When using watchdog device drivers we need to be > able to support the /dev/watchdog system. I also foresaw that we > should have a sysfs interface and I saw the future for watchdog > devices that you should be able to choose between the 2 different > systems. You should be able to use only the /dev/watchdog interfacing, > but you should also be able to use both a sysfs interface and a > /dev/watchdog interface and it should even be possible to have only a > sysfs interface in certain embedded devices. So that's why I split the > watchdog framework over 3 files: core code, the /dev/watchdog > interfacing and the sysfs code. Since I want to have compiled code > small enough when choosing either /Dev/watchdog or sysfs or both this > sounded the most logical thing to do (Unless you have a single file > full of #ifdef-ery that becomes unreadable). > > So I do not agree to have sysfs code in watchdog_dev.c . It belongs in > watchdog_sysfs.c imho. If someone has a better idea, I'll be glad to > listen to it and see what the benefits are. But I want a clean system > for excluding both /dev/ (current watchdog_dev.c) and/or sysfs > (watchdog_sysfs.c) in the future. Off-course the current behaviour is > to have the /dev/ interface and have the option to add sysfs > attributes. I agree that keeping sysfs code separate makes sense, as someone might want to not use it. The question is: can we make the /dev/watchdog entries optional ? That would break the compatibility, right? Imho, it would be saner to keep only one way to interact with watchdogs (ie. keep /dev/watchdog as is and don't make it optional, and sysfs read-only and eventually optional). I think that question should be answered before we can decide how we want to split the code between watchdog_dev.c and watchdog_core.c Thanks, Damien > > Kind regards, Wim. > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] Input: add touchscreen support for TS-4800
Hi Dmitry On Sat, Dec 12, 2015 at 08:58:58PM -0800, Dmitry Torokhov wrote: > Hi Damien, > > On Thu, Dec 10, 2015 at 11:11:12AM -0500, Damien Riegel wrote: > > On this board, the touchscreen, an ads7843, is not handled directly by > > Linux but by a companion FPGA. This FPGA is memory-mapped and the IP > > design is very similar to the mk712. > > ... > > > + > > + poll_dev = devm_input_allocate_polled_device(&pdev->dev); > > + if (!poll_dev) > > + return -ENOMEM; > > I wonder how useful touchscreen implemented as polling device is. Isn't > there an interrupt line for it? There is an interrupt line between the touchscreen and the FGPA, but not between the FPGA and the SoC. I agree it is a bit peculiar. Hopefully on more recent boards, they changed their design to handle the touchscreen directly with Linux. Thanks, Damien > Thanks. > > -- > Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v9 2/3] of: documentation: add bindings documentation for TS-4800
This adds the documentation for the TS-4800 by Technologic Systems. Signed-off-by: Damien Riegel Acked-by: Rob Herring --- Documentation/devicetree/bindings/arm/technologic.txt | 6 ++ 1 file changed, 6 insertions(+) create mode 100644 Documentation/devicetree/bindings/arm/technologic.txt diff --git a/Documentation/devicetree/bindings/arm/technologic.txt b/Documentation/devicetree/bindings/arm/technologic.txt new file mode 100644 index 000..8422988 --- /dev/null +++ b/Documentation/devicetree/bindings/arm/technologic.txt @@ -0,0 +1,6 @@ +Technologic Systems Platforms Device Tree Bindings +-- + +TS-4800 board +Required root node properties: + - compatible = "technologic,imx51-ts4800", "fsl,imx51"; -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v9 1/3] of: add vendor prefix for Technologic Systems
Signed-off-by: Damien Riegel Acked-by: Lee Jones Acked-by: Rob Herring --- Documentation/devicetree/bindings/vendor-prefixes.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt index 82d2ac9..d3a206d 100644 --- a/Documentation/devicetree/bindings/vendor-prefixes.txt +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt @@ -215,6 +215,7 @@ stericsson ST-Ericsson synology Synology, Inc. tbsTBS Technologies tclToby Churchill Ltd. +technologicTechnologic Systems thine THine Electronics, Inc. ti Texas Instruments tlmTrusted Logic Mobility -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v9 3/3] ARM: dts: TS-4800: add basic device tree
This device tree adds support for TS-4800 by Technologic Systems. This board is based on MX51-babbage, but there are some subtle differences in the pins used, and there is an additional FPGA that is memory-mapped. More details here: http://wiki.embeddedarm.com/wiki/TS-4800 Signed-off-by: Damien Riegel --- arch/arm/boot/dts/Makefile | 3 +- arch/arm/boot/dts/imx51-ts4800.dts | 180 + 2 files changed, 182 insertions(+), 1 deletion(-) create mode 100644 arch/arm/boot/dts/imx51-ts4800.dts diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile index bb8fa02..41b9985 100644 --- a/arch/arm/boot/dts/Makefile +++ b/arch/arm/boot/dts/Makefile @@ -258,7 +258,8 @@ dtb-$(CONFIG_SOC_IMX51) += \ imx51-apf51dev.dtb \ imx51-babbage.dtb \ imx51-digi-connectcore-jsk.dtb \ - imx51-eukrea-mbimxsd51-baseboard.dtb + imx51-eukrea-mbimxsd51-baseboard.dtb \ + imx51-ts4800.dtb dtb-$(CONFIG_SOC_IMX53) += \ imx53-ard.dtb \ imx53-m53evk.dtb \ diff --git a/arch/arm/boot/dts/imx51-ts4800.dts b/arch/arm/boot/dts/imx51-ts4800.dts new file mode 100644 index 000..f1317f7 --- /dev/null +++ b/arch/arm/boot/dts/imx51-ts4800.dts @@ -0,0 +1,180 @@ +/* + * Copyright 2015 Savoir-faire Linux + * + * This device tree is based on imx51-babbage.dts + * + * Licensed under the X11 license or the GPL v2 (or later) + */ + +/dts-v1/; +#include "imx51.dtsi" + +/ { + model = "Technologic Systems TS-4800"; + compatible = "technologic,imx51-ts4800", "fsl,imx51"; + + chosen { + stdout-path = &uart1; + }; + + memory { + reg = <0x9000 0x1000>; + }; + + soc { + fpga { + compatible = "simple-bus"; + reg = <0xb000 0x1d000>; + #address-cells = <1>; + #size-cells = <1>; + ranges; + + syscon: syscon@b001 { + compatible = "syscon", "simple-mfd"; + reg = <0xb001 0x3d>; + reg-io-width = <2>; + + wdt@e { + compatible = "technologic,ts4800-wdt"; + syscon = <&syscon 0xe>; + }; + }; + }; + }; + + clocks { + ckih1 { + clock-frequency = <22579200>; + }; + + ckih2 { + clock-frequency = <24576000>; + }; + }; +}; + +&esdhc1 { + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_esdhc1>; + cd-gpios = <&gpio1 0 GPIO_ACTIVE_LOW>; + wp-gpios = <&gpio1 1 GPIO_ACTIVE_HIGH>; + status = "okay"; +}; + +&fec { + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_fec>; + phy-mode = "mii"; + phy-reset-gpios = <&gpio2 14 GPIO_ACTIVE_LOW>; + phy-reset-duration = <1>; + status = "okay"; +}; + +&i2c2 { + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_i2c2>; + status = "okay"; + + rtc: m41t00@68 { + compatible = "stm,m41t00"; + reg = <0x68>; + }; +}; + +&uart1 { + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_uart1>; + status = "okay"; +}; + +&uart2 { + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_uart2>; + status = "okay"; +}; + +&uart3 { + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_uart3>; + status = "okay"; +}; + +&iomuxc { + pinctrl_ecspi1: ecspi1grp { + fsl,pins = < + MX51_PAD_CSPI1_MISO__ECSPI1_MISO0x185 + MX51_PAD_CSPI1_MOSI__ECSPI1_MOSI0x185 + MX51_PAD_CSPI1_SCLK__ECSPI1_SCLK0x185 + MX51_PAD_CSPI1_SS0__GPIO4_240x85 /* CS0 */ + >; + }; + + pinctrl_esdhc1: esdhc1grp { + fsl,pins = < + MX51_PAD_SD1_CMD__SD1_CMD 0x400020d5 + MX51_PAD_SD1_CLK__SD1_CLK 0x20d5 + MX51_PAD_SD1_DATA0__SD1_DATA0 0x20d5 + MX51_PAD_SD1_DATA1__SD1_DATA1 0x20d5 + MX51_PAD_SD1_DATA2__SD1_DATA2 0x20d5 + MX51_P
[PATCH v9 0/3] Add board support for TS-4800
This patch serie adds support for TS-4800 board. This board, manufactured by Technologic Systems, is based on an IMX515. The first stage bootloader, called TS-BOOTROM, enables the watchdog, so a watchdog driver is required to prevent board from rebooting. It is handled in a separate patchset. The current device tree is minimal but it allows to get a shell on the board. Changes in v9: - Changed dts file license to X11/GPL - Reordered i2c2 node to be alphabetically sorted - Dropped container node in iomuxc Changes in v8: - Split the serie into two parts, watchdog and dts, because there are no build dependencies between the two, and the syscon patch has been separately applied in Lee Jones' tree. - As a consequence, I dropped the patch which enabled the watchdog in imx_v6_v7_defconfig to ease the integration. Changes in v7: - syscon: change bus-width DT property to reg-io-width - watchdog: add dependency on HAS_IOMEM (spotted by a 0-day build) Changes in v6: - vendor prefix: reorder to sort alphabetically (wrong order since v3) - split commit adding device tree into two patches: one for the doc, one for the bindings Changes in v5: - watchdog: changed iteration stop condition in set_timeout to be less error prone Changes in v4: - syscon: rewrite DT property reading to be clearer - watchdog: made fixes suggested by Guenter (now uses watchdog_init_timeout, u32 instead of u16, fixed error checking in probe, cleaned set_timeout) Changes in v3: - Rebased on v4.3 - Changed vendor prefix from "ts" to "technologic" - Added a DT option to generic syscon driver to allow regmap configuration - Dropped custom mfd driver, use generic syscon driver instead. Changes in v2: - Added a mfd driver to handle syscon registers - The watchdog driver now uses the regmap (created by the mfd driver) to access the feed register - Remove watchdog's dependency on SOC_IMX51 Damien Riegel (3): of: add vendor prefix for Technologic Systems of: documentation: add bindings documentation for TS-4800 ARM: dts: TS-4800: add basic device tree .../devicetree/bindings/arm/technologic.txt| 6 + .../devicetree/bindings/vendor-prefixes.txt| 1 + arch/arm/boot/dts/Makefile | 3 +- arch/arm/boot/dts/imx51-ts4800.dts | 180 + 4 files changed, 189 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/arm/technologic.txt create mode 100644 arch/arm/boot/dts/imx51-ts4800.dts -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] Input: add touchscreen bindings for TS-4800
Add bindings for the TS-4800 touchscreen. Signed-off-by: Damien Riegel --- .../devicetree/bindings/input/touchscreen/ts4800-ts.txt | 12 1 file changed, 12 insertions(+) create mode 100644 Documentation/devicetree/bindings/input/touchscreen/ts4800-ts.txt diff --git a/Documentation/devicetree/bindings/input/touchscreen/ts4800-ts.txt b/Documentation/devicetree/bindings/input/touchscreen/ts4800-ts.txt new file mode 100644 index 000..63282fa --- /dev/null +++ b/Documentation/devicetree/bindings/input/touchscreen/ts4800-ts.txt @@ -0,0 +1,12 @@ +* TS-4800 Touchscreen bindings + +Required properties: +- compatible: must be "technologic,ts4800-ts" +- reg: physical base address of the controller and length of memory mapped + region. +- syscon: phandle / integers array that points to the syscon node which + describes the FPGA's syscon registers. + - phandle to FPGA's syscon + - offset to the touchscreen register + - offset to the touchscreen enable bit + -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/2] Input: add touchscreen support for TS-4800
On this board, the touchscreen, an ads7843, is not handled directly by Linux but by a companion FPGA. This FPGA is memory-mapped and the IP design is very similar to the mk712. This commit adds the support for this IP. Signed-off-by: Damien Riegel --- drivers/input/touchscreen/Kconfig | 15 +++ drivers/input/touchscreen/Makefile| 1 + drivers/input/touchscreen/ts4800-ts.c | 238 ++ 3 files changed, 254 insertions(+) create mode 100644 drivers/input/touchscreen/ts4800-ts.c diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig index deb14c1..2d3b2f2 100644 --- a/drivers/input/touchscreen/Kconfig +++ b/drivers/input/touchscreen/Kconfig @@ -914,6 +914,21 @@ config TOUCHSCREEN_TOUCHIT213 To compile this driver as a module, choose M here: the module will be called touchit213. +config TOUCHSCREEN_TS4800 + tristate "TS-4800 touchscreen" + depends on HAS_IOMEM && OF + select MFD_SYSCON + help + Say Y here if you have a touchscreen on a TS-4800 board. + + On TS-4800, the touchscreen is not handled directly by Linux but by + a companion FPGA. + + If unsure, say N. + + To compile this driver as a module, choose M here: the + module will be called ts4800_ts. + config TOUCHSCREEN_TSC_SERIO tristate "TSC-10/25/40 serial touchscreen support" select SERIO diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile index 1b79cc0..5d81ba8c 100644 --- a/drivers/input/touchscreen/Makefile +++ b/drivers/input/touchscreen/Makefile @@ -67,6 +67,7 @@ obj-$(CONFIG_TOUCHSCREEN_TI_AM335X_TSC) += ti_am335x_tsc.o obj-$(CONFIG_TOUCHSCREEN_TOUCHIT213) += touchit213.o obj-$(CONFIG_TOUCHSCREEN_TOUCHRIGHT) += touchright.o obj-$(CONFIG_TOUCHSCREEN_TOUCHWIN) += touchwin.o +obj-$(CONFIG_TOUCHSCREEN_TS4800) += ts4800-ts.o obj-$(CONFIG_TOUCHSCREEN_TSC_SERIO)+= tsc40.o obj-$(CONFIG_TOUCHSCREEN_TSC2005) += tsc2005.o obj-$(CONFIG_TOUCHSCREEN_TSC2007) += tsc2007.o diff --git a/drivers/input/touchscreen/ts4800-ts.c b/drivers/input/touchscreen/ts4800-ts.c new file mode 100644 index 000..1e81b17 --- /dev/null +++ b/drivers/input/touchscreen/ts4800-ts.c @@ -0,0 +1,238 @@ +/* + * Touchscreen driver for the TS-4800 board + * + * Copyright (c) 2015 - Savoir-faire Linux + * + * This file is licensed under the terms of the GNU General Public + * License version 2. This program is licensed "as is" without any + * warranty of any kind, whether express or implied. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +/* polling interval in ms*/ +#define POLL_INTERVAL 3 + +/* sensor values are 12-bit wide */ +#define MAX_12BIT ((1 << 12) - 1) + +#define PENDOWN_MASK0x1 + +#define X_OFFSET0x0 +#define Y_OFFSET0x2 + +struct ts4800_ts { + struct input_polled_dev *poll_dev; + struct device *dev; + charphys[32]; + + void __iomem*base; + struct regmap *regmap; + unsigned intreg; + unsigned intbit; + + boolpendown; + int debounce; +}; + +static void ts4800_ts_open(struct input_polled_dev *dev) +{ + struct ts4800_ts *ts = dev->private; + int ret; + + ret = regmap_update_bits(ts->regmap, ts->reg, +1 << ts->bit, 1 << ts->bit); + if (ret) + dev_warn(ts->dev, "Failed to enable touchscreen\n"); +} + +static void ts4800_ts_close(struct input_polled_dev *dev) +{ + struct ts4800_ts *ts = dev->private; + int ret; + + ret = regmap_update_bits(ts->regmap, ts->reg, +1 << ts->bit, 0); + if (ret) + dev_warn(ts->dev, "Failed to disable touchscreen\n"); + +} + +static void ts4800_ts_poll(struct input_polled_dev *dev) +{ + struct input_dev *input_dev = dev->input; + struct ts4800_ts *ts = dev->private; + u16 last_x = readw(ts->base + X_OFFSET); + u16 last_y = readw(ts->base + Y_OFFSET); + bool pendown = last_x & PENDOWN_MASK; + + if (!pendown && ts->pendown) { + ts->pendown = false; + ts->debounce = 1; + input_report_key(input_dev, BTN_TOUCH, 0); + input_sync(input_dev); + } + + if (pendown) { + if (ts->debounce) { + ts->debounce = 0; + return; + } + + if (!ts->pendown) { + input_report_key(input_dev, BTN_TOUCH, 1); +
[PATCH v8 2/3] of: documentation: add bindings documentation for TS-4800
This adds the documentation for the TS-4800 by Technologic Systems. Signed-off-by: Damien Riegel Acked-by: Rob Herring --- Documentation/devicetree/bindings/arm/technologic.txt | 6 ++ 1 file changed, 6 insertions(+) create mode 100644 Documentation/devicetree/bindings/arm/technologic.txt diff --git a/Documentation/devicetree/bindings/arm/technologic.txt b/Documentation/devicetree/bindings/arm/technologic.txt new file mode 100644 index 000..8422988 --- /dev/null +++ b/Documentation/devicetree/bindings/arm/technologic.txt @@ -0,0 +1,6 @@ +Technologic Systems Platforms Device Tree Bindings +-- + +TS-4800 board +Required root node properties: + - compatible = "technologic,imx51-ts4800", "fsl,imx51"; -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v8 0/3] Add board support for TS-4800
This patch serie adds support for TS-4800 board. This board, manufactured by Technologic Systems, is based on an IMX515. The first stage bootloader, called TS-BOOTROM, enables the watchdog, so a watchdog driver is required to prevent board from rebooting. It is handled in a separate patchset. The current device tree is minimal but it allows to get a shell on the board. Changes in v8: - Split the serie into two parts, watchdog and dts, because there are no build dependencies between the two, and the syscon patch has been separately applied in Lee Jones' tree. - As a consequence, I dropped the patch which enabled the watchdog in imx_v6_v7_defconfig to ease the integration. Changes in v7: - syscon: change bus-width DT property to reg-io-width - watchdog: add dependency on HAS_IOMEM (spotted by a 0-day build) Changes in v6: - vendor prefix: reorder to sort alphabetically (wrong order since v3) - split commit adding device tree into two patches: one for the doc, one for the bindings Changes in v5: - watchdog: changed iteration stop condition in set_timeout to be less error prone Changes in v4: - syscon: rewrite DT property reading to be clearer - watchdog: made fixes suggested by Guenter (now uses watchdog_init_timeout, u32 instead of u16, fixed error checking in probe, cleaned set_timeout) Changes in v3: - Rebased on v4.3 - Changed vendor prefix from "ts" to "technologic" - Added a DT option to generic syscon driver to allow regmap configuration - Dropped custom mfd driver, use generic syscon driver instead. Changes in v2: - Added a mfd driver to handle syscon registers - The watchdog driver now uses the regmap (created by the mfd driver) to access the feed register - Remove watchdog's dependency on SOC_IMX51 Damien Riegel (3): of: add vendor prefix for Technologic Systems of: documentation: add bindings documentation for TS-4800 ARM: dts: TS-4800: add basic device tree .../devicetree/bindings/arm/technologic.txt| 6 + .../devicetree/bindings/vendor-prefixes.txt| 1 + arch/arm/boot/dts/Makefile | 3 +- arch/arm/boot/dts/imx51-ts4800.dts | 190 + 4 files changed, 199 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/arm/technologic.txt create mode 100644 arch/arm/boot/dts/imx51-ts4800.dts -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v8 1/3] of: add vendor prefix for Technologic Systems
Signed-off-by: Damien Riegel Acked-by: Lee Jones Acked-by: Rob Herring --- Documentation/devicetree/bindings/vendor-prefixes.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt index 82d2ac9..d3a206d 100644 --- a/Documentation/devicetree/bindings/vendor-prefixes.txt +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt @@ -215,6 +215,7 @@ stericsson ST-Ericsson synology Synology, Inc. tbsTBS Technologies tclToby Churchill Ltd. +technologicTechnologic Systems thine THine Electronics, Inc. ti Texas Instruments tlmTrusted Logic Mobility -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v8 3/3] ARM: dts: TS-4800: add basic device tree
This device tree adds support for TS-4800 by Technologic Systems. This board is based on MX51-babbage, but there are some subtle differences in the pins used, and there is an additional FPGA that is memory-mapped. More details here: http://wiki.embeddedarm.com/wiki/TS-4800 Signed-off-by: Damien Riegel --- arch/arm/boot/dts/Makefile | 3 +- arch/arm/boot/dts/imx51-ts4800.dts | 190 + 2 files changed, 192 insertions(+), 1 deletion(-) create mode 100644 arch/arm/boot/dts/imx51-ts4800.dts diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile index bb8fa02..41b9985 100644 --- a/arch/arm/boot/dts/Makefile +++ b/arch/arm/boot/dts/Makefile @@ -258,7 +258,8 @@ dtb-$(CONFIG_SOC_IMX51) += \ imx51-apf51dev.dtb \ imx51-babbage.dtb \ imx51-digi-connectcore-jsk.dtb \ - imx51-eukrea-mbimxsd51-baseboard.dtb + imx51-eukrea-mbimxsd51-baseboard.dtb \ + imx51-ts4800.dtb dtb-$(CONFIG_SOC_IMX53) += \ imx53-ard.dtb \ imx53-m53evk.dtb \ diff --git a/arch/arm/boot/dts/imx51-ts4800.dts b/arch/arm/boot/dts/imx51-ts4800.dts new file mode 100644 index 000..a0b2c6f --- /dev/null +++ b/arch/arm/boot/dts/imx51-ts4800.dts @@ -0,0 +1,190 @@ +/* + * Copyright 2015 Savoir-faire Linux + * + * This device tree is based on imx51-babbage.dts + * + * The code contained herein is licensed under the GNU General Public + * License. You may obtain a copy of the GNU General Public License + * Version 2 at the following locations: + * + * http://www.opensource.org/licenses/gpl-license.html + * http://www.gnu.org/copyleft/gpl.html + */ + +/dts-v1/; +#include "imx51.dtsi" + +/ { + model = "Technologic Systems TS-4800"; + compatible = "technologic,imx51-ts4800", "fsl,imx51"; + + chosen { + stdout-path = &uart1; + }; + + memory { + reg = <0x9000 0x1000>; + }; + + soc { + fpga { + compatible = "simple-bus"; + reg = <0xb000 0x1d000>; + #address-cells = <1>; + #size-cells = <1>; + ranges; + + syscon: syscon@b001 { + compatible = "syscon", "simple-mfd"; + reg = <0xb001 0x3d>; + reg-io-width = <2>; + + wdt@e { + compatible = "technologic,ts4800-wdt"; + syscon = <&syscon 0xe>; + }; + }; + }; + }; + + clocks { + ckih1 { + clock-frequency = <22579200>; + }; + + ckih2 { + clock-frequency = <24576000>; + }; + }; +}; + +&esdhc1 { + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_esdhc1>; + cd-gpios = <&gpio1 0 GPIO_ACTIVE_LOW>; + wp-gpios = <&gpio1 1 GPIO_ACTIVE_HIGH>; + status = "okay"; +}; + +&fec { + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_fec>; + phy-mode = "mii"; + phy-reset-gpios = <&gpio2 14 GPIO_ACTIVE_LOW>; + phy-reset-duration = <1>; + status = "okay"; +}; + +&uart1 { + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_uart1>; + status = "okay"; +}; + +&uart2 { + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_uart2>; + status = "okay"; +}; + +&uart3 { + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_uart3>; + status = "okay"; +}; + +&i2c2 { + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_i2c2>; + status = "okay"; + + rtc: m41t00@68 { + compatible = "stm,m41t00"; + reg = <0x68>; + }; +}; + + +&iomuxc { + imx51-ts4800 { + + pinctrl_ecspi1: ecspi1grp { + fsl,pins = < + MX51_PAD_CSPI1_MISO__ECSPI1_MISO0x185 + MX51_PAD_CSPI1_MOSI__ECSPI1_MOSI0x185 + MX51_PAD_CSPI1_SCLK__ECSPI1_SCLK0x185 + MX51_PAD_CSPI1_SS0__GPIO4_240x85 /* CS0 */ + >; + }; + + pinctrl_esdhc1: esdhc1grp { + fsl,pins = < + MX51_PAD_SD1_CMD__SD1_CMD
[PATCH v8] watchdog: ts4800: add driver for TS-4800 watchdog
This watchdog is instantiated in a FPGA that is memory mapped. It is made of only one register, called the feed register. Writing to this register will re-arm the watchdog for a given time (and enable it if it was disable). It can be disabled by writing a special value into it. It is part of a syscon block, and the watchdog register offset in this block varies from board to board. This offset is passed in the syscon property after the phandle to the syscon node. Signed-off-by: Damien Riegel Acked-by: Rob Herring Reviewed-by: Guenter Roeck --- Changes in v8: - Split the serie into two parts: watchdog and dts as there are no build dependencies between the two - Added Reviewed-by Guenter Roeck and Acked-by by Rob Herring (for the dts bindings). Changes in v7: - syscon: change bus-width DT property to reg-io-width - watchdog: add dependency on HAS_IOMEM (spotted by a 0-day build) Changes in v6: - vendor prefix: reorder to sort alphabetically (wrong order since v3) - split commit adding device tree into two patches: one for the doc, one for the bindings Changes in v5: - watchdog: changed iteration stop condition in set_timeout to be less error prone Changes in v4: - syscon: rewrite DT property reading to be clearer - watchdog: made fixes suggested by Guenter (now uses watchdog_init_timeout, u32 instead of u16, fixed error checking in probe, cleaned set_timeout) Changes in v3: - Rebased on v4.3 - Changed vendor prefix from "ts" to "technologic" - Added a DT option to generic syscon driver to allow regmap configuration - Dropped custom mfd driver, use generic syscon driver instead. Changes in v2: - Added a mfd driver to handle syscon registers - The watchdog driver now uses the regmap (created by the mfd driver) to access the feed register - Remove watchdog's dependency on SOC_IMX51 .../devicetree/bindings/watchdog/ts4800-wdt.txt| 25 +++ drivers/watchdog/Kconfig | 10 + drivers/watchdog/Makefile | 1 + drivers/watchdog/ts4800_wdt.c | 215 + 4 files changed, 251 insertions(+) create mode 100644 Documentation/devicetree/bindings/watchdog/ts4800-wdt.txt create mode 100644 drivers/watchdog/ts4800_wdt.c diff --git a/Documentation/devicetree/bindings/watchdog/ts4800-wdt.txt b/Documentation/devicetree/bindings/watchdog/ts4800-wdt.txt new file mode 100644 index 000..8f6caad --- /dev/null +++ b/Documentation/devicetree/bindings/watchdog/ts4800-wdt.txt @@ -0,0 +1,25 @@ +Technologic Systems Watchdog + +Required properties: +- compatible: must be "technologic,ts4800-wdt" +- syscon: phandle / integer array that points to the syscon node which + describes the FPGA's syscon registers. + - phandle to FPGA's syscon + - offset to the watchdog register + +Optional property: +- timeout-sec: contains the watchdog timeout in seconds. + +Example: + +syscon: syscon@b001 { + compatible = "syscon", "simple-mfd"; + reg = <0xb001 0x3d>; + reg-io-width = <2>; + + wdt@e { + compatible = "technologic,ts4800-wdt"; + syscon = <&syscon 0xe>; + timeout-sec = <10>; + }; +} diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index 79e1aa1..bb624d2 100644 --- a/drivers/watchdog/Kconfig +++ b/drivers/watchdog/Kconfig @@ -426,6 +426,16 @@ config NUC900_WATCHDOG To compile this driver as a module, choose M here: the module will be called nuc900_wdt. +config TS4800_WATCHDOG + tristate "TS-4800 Watchdog" + depends on HAS_IOMEM && OF + select WATCHDOG_CORE + select MFD_SYSCON + help + Technologic Systems TS-4800 has watchdog timer implemented in + an external FPGA. Say Y here if you want to support for the + watchdog timer on TS-4800 board. + config TS72XX_WATCHDOG tristate "TS-72XX SBC Watchdog" depends on MACH_TS72XX diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile index 0c616e3..3863ce0 100644 --- a/drivers/watchdog/Makefile +++ b/drivers/watchdog/Makefile @@ -53,6 +53,7 @@ obj-$(CONFIG_RN5T618_WATCHDOG) += rn5t618_wdt.o obj-$(CONFIG_COH901327_WATCHDOG) += coh901327_wdt.o obj-$(CONFIG_STMP3XXX_RTC_WATCHDOG) += stmp3xxx_rtc_wdt.o obj-$(CONFIG_NUC900_WATCHDOG) += nuc900_wdt.o +obj-$(CONFIG_TS4800_WATCHDOG) += ts4800_wdt.o obj-$(CONFIG_TS72XX_WATCHDOG) += ts72xx_wdt.o obj-$(CONFIG_IMX2_WDT) += imx2_wdt.o obj-$(CONFIG_UX500_WATCHDOG) += ux500_wdt.o diff --git a/drivers/watchdog/ts4800_wdt.c b/drivers/watchdog/ts4800_wdt.c new file mode 100644 index 000..2b8de86 --- /dev/null +++ b/drivers/watchdog/ts4800_wdt.c @@ -0,0 +1,215 @@ +/* + * Watchdog driver for TS-4800 based boards + * + * Copyright (c) 2015 - Savoir-faire Linux + * + *
Re: [PATCH v7 2/6] mfd: syscon: add a DT property to set value width
On Mon, Dec 07, 2015 at 09:40:20AM +, Lee Jones wrote: > On Mon, 30 Nov 2015, Damien Riegel wrote: > > > Currently syscon has a fixed configuration of 32 bits for register and > > values widths. In some cases, it would be desirable to be able to > > customize the value width. > > > > For example, certain boards (like the ones manufactured by Technologic > > Systems) have a FPGA that is memory-mapped, but its registers are only > > 16-bit wide. > > > > This patch adds an optional "reg-io-width" DT binding for syscon that > > allows to change the width for the data bus (i.e. val_bits). If this > > property is provided, it will also set the register stride to > > reg-io-width's value. If not provided, the default configuration is > > used. > > > > Signed-off-by: Damien Riegel > > --- > > Documentation/devicetree/bindings/mfd/syscon.txt | 4 > > drivers/mfd/syscon.c | 13 + > > 2 files changed, 17 insertions(+) > > Applied, thanks. > Hi Lee, Good to see this patch applied. What's going on now with the other patches of this serie ? How should I handle them ? Thanks, Damien > > diff --git a/Documentation/devicetree/bindings/mfd/syscon.txt > > b/Documentation/devicetree/bindings/mfd/syscon.txt > > index fe8150b..408f768 100644 > > --- a/Documentation/devicetree/bindings/mfd/syscon.txt > > +++ b/Documentation/devicetree/bindings/mfd/syscon.txt > > @@ -13,6 +13,10 @@ Required properties: > > - compatible: Should contain "syscon". > > - reg: the register region can be accessed from syscon > > > > +Optional property: > > +- reg-io-width: the size (in bytes) of the IO accesses that should be > > + performed on the device. > > + > > Examples: > > gpr: iomuxc-gpr@020e { > > compatible = "fsl,imx6q-iomuxc-gpr", "syscon"; > > diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c > > index 176bf0f..b7aabee 100644 > > --- a/drivers/mfd/syscon.c > > +++ b/drivers/mfd/syscon.c > > @@ -47,6 +47,7 @@ static struct syscon *of_syscon_register(struct > > device_node *np) > > struct syscon *syscon; > > struct regmap *regmap; > > void __iomem *base; > > + u32 reg_io_width; > > int ret; > > struct regmap_config syscon_config = syscon_regmap_config; > > > > @@ -69,6 +70,18 @@ static struct syscon *of_syscon_register(struct > > device_node *np) > > else if (of_property_read_bool(np, "little-endian")) > > syscon_config.val_format_endian = REGMAP_ENDIAN_LITTLE; > > > > + /* > > +* search for reg-io-width property in DT. If it is not provided, > > +* default to 4 bytes. regmap_init_mmio will return an error if values > > +* are invalid so there is no need to check them here. > > +*/ > > + ret = of_property_read_u32(np, "reg-io-width", ®_io_width); > > + if (ret) > > + reg_io_width = 4; > > + > > + syscon_config.reg_stride = reg_io_width; > > + syscon_config.val_bits = reg_io_width * 8; > > + > > regmap = regmap_init_mmio(NULL, base, &syscon_config); > > if (IS_ERR(regmap)) { > > pr_err("regmap init failed\n"); > > -- > Lee Jones > Linaro STMicroelectronics Landing Team Lead > Linaro.org │ Open source software for ARM SoCs > Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC/RFT PATCH] watchdog: Move watchdog device creation to watchdog_dev.c
On Sun, Dec 06, 2015 at 11:51:41AM -0800, Guenter Roeck wrote: > The watchdog character device s currently created in watchdog_dev.c, > and the watchdog device in watchdog_core.c. This results in > cross-dependencies, as the device creation needs to know the watchdog > character device number. > > On top of that, the watchdog character device is created before the > watchdog device is created. This can result in race conditions if the > watchdog device node is accessed before the watchdog device has been > created. > > To solve the problem, move watchdog device creation into watchdog_dev.c, > and create the watchdog device prior to creating its device node. > Also move device class creation into watchdog_dev.c, since this is now > the only place where the watchdog class is needed. > > Inspired by an earlier patch set from Damien Riegel. > > Cc: Damien Riegel > Signed-off-by: Guenter Roeck > --- > Hi Damien, > > I think this approach would be a bit better. The watchdog device isn't > really used in the watchdog core code, so it is better created in > watchdog_dev.c. That also fits well with other pending changes, such as > sysfs attribute support, and my attempts to move the ref/unref functions > completely into the watchdog core. As a side effect, it also cleans up > the error path in __watchdog_register_device(). > > What do you think ? Hi Guenter, Like the idea, but I don't really get the separation. For instance, you move watchdog_class in watchdog_dev.c but you keep watchdog_ida in watchdog_core.c whereas it is only used for device creation/deletion. Also a few minor comments below. > The code has been compile tested only so far. I'll try to test it later > today, but I wanted to get it out for discussion. I'll give it a try but I have only one board to test it. > > Thanks, > Guenter > > drivers/watchdog/watchdog_core.c | 37 ++-- > drivers/watchdog/watchdog_core.h | 2 +- > drivers/watchdog/watchdog_dev.c | 61 > > 3 files changed, 54 insertions(+), 46 deletions(-) > > diff --git a/drivers/watchdog/watchdog_core.c > b/drivers/watchdog/watchdog_core.c > index 873f13972cf4..089e930fce19 100644 > --- a/drivers/watchdog/watchdog_core.c > +++ b/drivers/watchdog/watchdog_core.c > @@ -41,7 +41,6 @@ > #include "watchdog_core.h" /* For watchdog_dev_register/... */ > > static DEFINE_IDA(watchdog_ida); > -static struct class *watchdog_class; > > /* > * Deferred Registration infrastructure. > @@ -139,7 +138,7 @@ EXPORT_SYMBOL_GPL(watchdog_init_timeout); > > static int __watchdog_register_device(struct watchdog_device *wdd) > { > - int ret, id = -1, devno; > + int ret, id = -1; > > if (wdd == NULL || wdd->info == NULL || wdd->ops == NULL) > return -EINVAL; > @@ -192,16 +191,6 @@ static int __watchdog_register_device(struct > watchdog_device *wdd) > } > } > > - devno = wdd->cdev.dev; > - wdd->dev = device_create(watchdog_class, wdd->parent, devno, > - NULL, "watchdog%d", wdd->id); > - if (IS_ERR(wdd->dev)) { > - watchdog_dev_unregister(wdd); > - ida_simple_remove(&watchdog_ida, id); > - ret = PTR_ERR(wdd->dev); > - return ret; > - } > - > return 0; > } > > @@ -232,19 +221,8 @@ EXPORT_SYMBOL_GPL(watchdog_register_device); > > static void __watchdog_unregister_device(struct watchdog_device *wdd) > { > - int ret; > - int devno; > - > - if (wdd == NULL) > - return; > - > - devno = wdd->cdev.dev; > - ret = watchdog_dev_unregister(wdd); > - if (ret) > - pr_err("error unregistering /dev/watchdog (err=%d)\n", ret); > - device_destroy(watchdog_class, devno); > + watchdog_dev_unregister(wdd); > ida_simple_remove(&watchdog_ida, wdd->id); > - wdd->dev = NULL; > } > > /** > @@ -287,17 +265,9 @@ static int __init watchdog_init(void) > { > int err; > > - watchdog_class = class_create(THIS_MODULE, "watchdog"); > - if (IS_ERR(watchdog_class)) { > - pr_err("couldn't create class\n"); > - return PTR_ERR(watchdog_class); > - } > - > err = watchdog_dev_init(); > - if (err < 0) { > - class_destroy(watchdog_class); > + if (err < 0) > return err; > - } > > watchdog_deferred_registration(); > return 0; > @@ -306,7
Re: [PATCH v7 3/6] watchdog: ts4800: add driver for TS-4800 watchdog
Hi Wim, This patch is part of a serie and Lee Jones was willing to handle the serie (at least the first three patches) but he needs your Ack on the watchdog as you are the maintainer of this subsystem. This patch has already been reviewed by Guenter. Could you please review this patch ? Thanks, Damien On Mon, Nov 30, 2015 at 08:14:48AM -0800, Guenter Roeck wrote: > On 11/30/2015 07:59 AM, Damien Riegel wrote: > >This watchdog is instantiated in a FPGA that is memory mapped. It is > >made of only one register, called the feed register. Writing to this > >register will re-arm the watchdog for a given time (and enable it if it > >was disable). It can be disabled by writing a special value into it. > > > >It is part of a syscon block, and the watchdog register offset in this > >block varies from board to board. This offset is passed in the syscon > >property after the phandle to the syscon node. > > > >Signed-off-by: Damien Riegel > > Reviewed-by: Guenter Roeck > > >--- > > .../devicetree/bindings/watchdog/ts4800-wdt.txt| 25 +++ > > drivers/watchdog/Kconfig | 10 + > > drivers/watchdog/Makefile | 1 + > > drivers/watchdog/ts4800_wdt.c | 215 > > + > > 4 files changed, 251 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/watchdog/ts4800-wdt.txt > > create mode 100644 drivers/watchdog/ts4800_wdt.c > > > >diff --git a/Documentation/devicetree/bindings/watchdog/ts4800-wdt.txt > >b/Documentation/devicetree/bindings/watchdog/ts4800-wdt.txt > >new file mode 100644 > >index 000..8f6caad > >--- /dev/null > >+++ b/Documentation/devicetree/bindings/watchdog/ts4800-wdt.txt > >@@ -0,0 +1,25 @@ > >+Technologic Systems Watchdog > >+ > >+Required properties: > >+- compatible: must be "technologic,ts4800-wdt" > >+- syscon: phandle / integer array that points to the syscon node which > >+ describes the FPGA's syscon registers. > >+ - phandle to FPGA's syscon > >+ - offset to the watchdog register > >+ > >+Optional property: > >+- timeout-sec: contains the watchdog timeout in seconds. > >+ > >+Example: > >+ > >+syscon: syscon@b001 { > >+compatible = "syscon", "simple-mfd"; > >+reg = <0xb001 0x3d>; > >+reg-io-width = <2>; > >+ > >+wdt@e { > >+compatible = "technologic,ts4800-wdt"; > >+syscon = <&syscon 0xe>; > >+timeout-sec = <10>; > >+}; > >+} > >diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig > >index 79e1aa1..bb624d2 100644 > >--- a/drivers/watchdog/Kconfig > >+++ b/drivers/watchdog/Kconfig > >@@ -426,6 +426,16 @@ config NUC900_WATCHDOG > > To compile this driver as a module, choose M here: the > > module will be called nuc900_wdt. > > > >+config TS4800_WATCHDOG > >+tristate "TS-4800 Watchdog" > >+depends on HAS_IOMEM && OF > >+select WATCHDOG_CORE > >+select MFD_SYSCON > >+help > >+ Technologic Systems TS-4800 has watchdog timer implemented in > >+ an external FPGA. Say Y here if you want to support for the > >+ watchdog timer on TS-4800 board. > >+ > > config TS72XX_WATCHDOG > > tristate "TS-72XX SBC Watchdog" > > depends on MACH_TS72XX > >diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile > >index 0c616e3..3863ce0 100644 > >--- a/drivers/watchdog/Makefile > >+++ b/drivers/watchdog/Makefile > >@@ -53,6 +53,7 @@ obj-$(CONFIG_RN5T618_WATCHDOG) += rn5t618_wdt.o > > obj-$(CONFIG_COH901327_WATCHDOG) += coh901327_wdt.o > > obj-$(CONFIG_STMP3XXX_RTC_WATCHDOG) += stmp3xxx_rtc_wdt.o > > obj-$(CONFIG_NUC900_WATCHDOG) += nuc900_wdt.o > >+obj-$(CONFIG_TS4800_WATCHDOG) += ts4800_wdt.o > > obj-$(CONFIG_TS72XX_WATCHDOG) += ts72xx_wdt.o > > obj-$(CONFIG_IMX2_WDT) += imx2_wdt.o > > obj-$(CONFIG_UX500_WATCHDOG) += ux500_wdt.o > >diff --git a/drivers/watchdog/ts4800_wdt.c b/drivers/watchdog/ts4800_wdt.c > >new file mode 100644 > >index 000..2b8de86 > >--- /dev/null > >+++ b/drivers/watchdog/ts4800_wdt.c > >@@ -0,0 +1,215 @@ > >+/* > >+ * Watchdog driver for TS-4800 based boards > >+ * > >+ * Copyright (c) 2015 - Savoir-faire Linux > >+ * > >+ * This file is licensed under the terms of the GNU General Public > &g
Re: [PATCH v7 2/6] mfd: syscon: add a DT property to set value width
Lee, Arnd, I had to make changes in this patch to address Rob's comments. reg-io-width is now in bytes while bus-width was a value in bits. Could you please review this patch ? Thanks, Damien On Mon, Nov 30, 2015 at 01:00:15PM -0600, Rob Herring wrote: > On Mon, Nov 30, 2015 at 10:59:47AM -0500, Damien Riegel wrote: > > Currently syscon has a fixed configuration of 32 bits for register and > > values widths. In some cases, it would be desirable to be able to > > customize the value width. > > > > For example, certain boards (like the ones manufactured by Technologic > > Systems) have a FPGA that is memory-mapped, but its registers are only > > 16-bit wide. > > > > This patch adds an optional "reg-io-width" DT binding for syscon that > > allows to change the width for the data bus (i.e. val_bits). If this > > property is provided, it will also set the register stride to > > reg-io-width's value. If not provided, the default configuration is > > used. > > > > Signed-off-by: Damien Riegel > > Acked-by: Rob Herring > > > --- > > Documentation/devicetree/bindings/mfd/syscon.txt | 4 > > drivers/mfd/syscon.c | 13 + > > 2 files changed, 17 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/mfd/syscon.txt > > b/Documentation/devicetree/bindings/mfd/syscon.txt > > index fe8150b..408f768 100644 > > --- a/Documentation/devicetree/bindings/mfd/syscon.txt > > +++ b/Documentation/devicetree/bindings/mfd/syscon.txt > > @@ -13,6 +13,10 @@ Required properties: > > - compatible: Should contain "syscon". > > - reg: the register region can be accessed from syscon > > > > +Optional property: > > +- reg-io-width: the size (in bytes) of the IO accesses that should be > > + performed on the device. > > + > > Examples: > > gpr: iomuxc-gpr@020e { > > compatible = "fsl,imx6q-iomuxc-gpr", "syscon"; > > diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c > > index 176bf0f..b7aabee 100644 > > --- a/drivers/mfd/syscon.c > > +++ b/drivers/mfd/syscon.c > > @@ -47,6 +47,7 @@ static struct syscon *of_syscon_register(struct > > device_node *np) > > struct syscon *syscon; > > struct regmap *regmap; > > void __iomem *base; > > + u32 reg_io_width; > > int ret; > > struct regmap_config syscon_config = syscon_regmap_config; > > > > @@ -69,6 +70,18 @@ static struct syscon *of_syscon_register(struct > > device_node *np) > > else if (of_property_read_bool(np, "little-endian")) > > syscon_config.val_format_endian = REGMAP_ENDIAN_LITTLE; > > > > + /* > > +* search for reg-io-width property in DT. If it is not provided, > > +* default to 4 bytes. regmap_init_mmio will return an error if values > > +* are invalid so there is no need to check them here. > > +*/ > > + ret = of_property_read_u32(np, "reg-io-width", ®_io_width); > > + if (ret) > > + reg_io_width = 4; > > + > > + syscon_config.reg_stride = reg_io_width; > > + syscon_config.val_bits = reg_io_width * 8; > > + > > regmap = regmap_init_mmio(NULL, base, &syscon_config); > > if (IS_ERR(regmap)) { > > pr_err("regmap init failed\n"); > > -- > > 2.5.0 > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v7 0/6] Add board support for TS-4800
This patch serie adds support for TS-4800 board. This board, manufactured by Technologic Systems, is based on an IMX515. The first stage bootloader, called TS-BOOTROM, enables the watchdog, so a watchdog driver is required to prevent board from rebooting. The current device tree is minimal but it allows to get a shell on the board. Changes in v7: - syscon: change bus-width DT property to reg-io-width - watchdog: add dependency on HAS_IOMEM (spotted by a 0-day build) Changes in v6: - vendor prefix: reorder to sort alphabetically (wrong order since v3) - split commit adding device tree into two patches: one for the doc, one for the bindings Changes in v5: - watchdog: changed iteration stop condition in set_timeout to be less error prone Changes in v4: - syscon: rewrite DT property reading to be clearer - watchdog: made fixes suggested by Guenter (now uses watchdog_init_timeout, u32 instead of u16, fixed error checking in probe, cleaned set_timeout) Changes in v3: - Rebased on v4.3 - Changed vendor prefix from "ts" to "technologic" - Added a DT option to generic syscon driver to allow regmap configuration - Dropped custom mfd driver, use generic syscon driver instead. Changes in v2: - Added a mfd driver to handle syscon registers - The watchdog driver now uses the regmap (created by the mfd driver) to access the feed register - Remove watchdog's dependency on SOC_IMX51 Damien Riegel (6): of: add vendor prefix for Technologic Systems mfd: syscon: add a DT property to set value width watchdog: ts4800: add driver for TS-4800 watchdog ARM: imx_v6_v7_defconfig: add TS-4800 watchdog of: documentation: add bindings documentation for TS-4800 ARM: dts: TS-4800: add basic device tree .../devicetree/bindings/arm/technologic.txt| 6 + Documentation/devicetree/bindings/mfd/syscon.txt | 4 + .../devicetree/bindings/vendor-prefixes.txt| 1 + .../devicetree/bindings/watchdog/ts4800-wdt.txt| 25 +++ arch/arm/boot/dts/Makefile | 3 +- arch/arm/boot/dts/imx51-ts4800.dts | 190 ++ arch/arm/configs/imx_v6_v7_defconfig | 1 + drivers/mfd/syscon.c | 13 ++ drivers/watchdog/Kconfig | 10 + drivers/watchdog/Makefile | 1 + drivers/watchdog/ts4800_wdt.c | 215 + 11 files changed, 468 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/arm/technologic.txt create mode 100644 Documentation/devicetree/bindings/watchdog/ts4800-wdt.txt create mode 100644 arch/arm/boot/dts/imx51-ts4800.dts create mode 100644 drivers/watchdog/ts4800_wdt.c -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v7 2/6] mfd: syscon: add a DT property to set value width
Currently syscon has a fixed configuration of 32 bits for register and values widths. In some cases, it would be desirable to be able to customize the value width. For example, certain boards (like the ones manufactured by Technologic Systems) have a FPGA that is memory-mapped, but its registers are only 16-bit wide. This patch adds an optional "reg-io-width" DT binding for syscon that allows to change the width for the data bus (i.e. val_bits). If this property is provided, it will also set the register stride to reg-io-width's value. If not provided, the default configuration is used. Signed-off-by: Damien Riegel --- Documentation/devicetree/bindings/mfd/syscon.txt | 4 drivers/mfd/syscon.c | 13 + 2 files changed, 17 insertions(+) diff --git a/Documentation/devicetree/bindings/mfd/syscon.txt b/Documentation/devicetree/bindings/mfd/syscon.txt index fe8150b..408f768 100644 --- a/Documentation/devicetree/bindings/mfd/syscon.txt +++ b/Documentation/devicetree/bindings/mfd/syscon.txt @@ -13,6 +13,10 @@ Required properties: - compatible: Should contain "syscon". - reg: the register region can be accessed from syscon +Optional property: +- reg-io-width: the size (in bytes) of the IO accesses that should be + performed on the device. + Examples: gpr: iomuxc-gpr@020e { compatible = "fsl,imx6q-iomuxc-gpr", "syscon"; diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c index 176bf0f..b7aabee 100644 --- a/drivers/mfd/syscon.c +++ b/drivers/mfd/syscon.c @@ -47,6 +47,7 @@ static struct syscon *of_syscon_register(struct device_node *np) struct syscon *syscon; struct regmap *regmap; void __iomem *base; + u32 reg_io_width; int ret; struct regmap_config syscon_config = syscon_regmap_config; @@ -69,6 +70,18 @@ static struct syscon *of_syscon_register(struct device_node *np) else if (of_property_read_bool(np, "little-endian")) syscon_config.val_format_endian = REGMAP_ENDIAN_LITTLE; + /* +* search for reg-io-width property in DT. If it is not provided, +* default to 4 bytes. regmap_init_mmio will return an error if values +* are invalid so there is no need to check them here. +*/ + ret = of_property_read_u32(np, "reg-io-width", ®_io_width); + if (ret) + reg_io_width = 4; + + syscon_config.reg_stride = reg_io_width; + syscon_config.val_bits = reg_io_width * 8; + regmap = regmap_init_mmio(NULL, base, &syscon_config); if (IS_ERR(regmap)) { pr_err("regmap init failed\n"); -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v7 1/6] of: add vendor prefix for Technologic Systems
Signed-off-by: Damien Riegel Acked-by: Lee Jones Acked-by: Rob Herring --- Documentation/devicetree/bindings/vendor-prefixes.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt index 82d2ac9..d3a206d 100644 --- a/Documentation/devicetree/bindings/vendor-prefixes.txt +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt @@ -215,6 +215,7 @@ stericsson ST-Ericsson synology Synology, Inc. tbsTBS Technologies tclToby Churchill Ltd. +technologicTechnologic Systems thine THine Electronics, Inc. ti Texas Instruments tlmTrusted Logic Mobility -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v7 3/6] watchdog: ts4800: add driver for TS-4800 watchdog
This watchdog is instantiated in a FPGA that is memory mapped. It is made of only one register, called the feed register. Writing to this register will re-arm the watchdog for a given time (and enable it if it was disable). It can be disabled by writing a special value into it. It is part of a syscon block, and the watchdog register offset in this block varies from board to board. This offset is passed in the syscon property after the phandle to the syscon node. Signed-off-by: Damien Riegel --- .../devicetree/bindings/watchdog/ts4800-wdt.txt| 25 +++ drivers/watchdog/Kconfig | 10 + drivers/watchdog/Makefile | 1 + drivers/watchdog/ts4800_wdt.c | 215 + 4 files changed, 251 insertions(+) create mode 100644 Documentation/devicetree/bindings/watchdog/ts4800-wdt.txt create mode 100644 drivers/watchdog/ts4800_wdt.c diff --git a/Documentation/devicetree/bindings/watchdog/ts4800-wdt.txt b/Documentation/devicetree/bindings/watchdog/ts4800-wdt.txt new file mode 100644 index 000..8f6caad --- /dev/null +++ b/Documentation/devicetree/bindings/watchdog/ts4800-wdt.txt @@ -0,0 +1,25 @@ +Technologic Systems Watchdog + +Required properties: +- compatible: must be "technologic,ts4800-wdt" +- syscon: phandle / integer array that points to the syscon node which + describes the FPGA's syscon registers. + - phandle to FPGA's syscon + - offset to the watchdog register + +Optional property: +- timeout-sec: contains the watchdog timeout in seconds. + +Example: + +syscon: syscon@b001 { + compatible = "syscon", "simple-mfd"; + reg = <0xb001 0x3d>; + reg-io-width = <2>; + + wdt@e { + compatible = "technologic,ts4800-wdt"; + syscon = <&syscon 0xe>; + timeout-sec = <10>; + }; +} diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index 79e1aa1..bb624d2 100644 --- a/drivers/watchdog/Kconfig +++ b/drivers/watchdog/Kconfig @@ -426,6 +426,16 @@ config NUC900_WATCHDOG To compile this driver as a module, choose M here: the module will be called nuc900_wdt. +config TS4800_WATCHDOG + tristate "TS-4800 Watchdog" + depends on HAS_IOMEM && OF + select WATCHDOG_CORE + select MFD_SYSCON + help + Technologic Systems TS-4800 has watchdog timer implemented in + an external FPGA. Say Y here if you want to support for the + watchdog timer on TS-4800 board. + config TS72XX_WATCHDOG tristate "TS-72XX SBC Watchdog" depends on MACH_TS72XX diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile index 0c616e3..3863ce0 100644 --- a/drivers/watchdog/Makefile +++ b/drivers/watchdog/Makefile @@ -53,6 +53,7 @@ obj-$(CONFIG_RN5T618_WATCHDOG) += rn5t618_wdt.o obj-$(CONFIG_COH901327_WATCHDOG) += coh901327_wdt.o obj-$(CONFIG_STMP3XXX_RTC_WATCHDOG) += stmp3xxx_rtc_wdt.o obj-$(CONFIG_NUC900_WATCHDOG) += nuc900_wdt.o +obj-$(CONFIG_TS4800_WATCHDOG) += ts4800_wdt.o obj-$(CONFIG_TS72XX_WATCHDOG) += ts72xx_wdt.o obj-$(CONFIG_IMX2_WDT) += imx2_wdt.o obj-$(CONFIG_UX500_WATCHDOG) += ux500_wdt.o diff --git a/drivers/watchdog/ts4800_wdt.c b/drivers/watchdog/ts4800_wdt.c new file mode 100644 index 000..2b8de86 --- /dev/null +++ b/drivers/watchdog/ts4800_wdt.c @@ -0,0 +1,215 @@ +/* + * Watchdog driver for TS-4800 based boards + * + * Copyright (c) 2015 - Savoir-faire Linux + * + * This file is licensed under the terms of the GNU General Public + * License version 2. This program is licensed "as is" without any + * warranty of any kind, whether express or implied. + */ + +#include +#include +#include +#include +#include +#include +#include + +static bool nowayout = WATCHDOG_NOWAYOUT; +module_param(nowayout, bool, 0); +MODULE_PARM_DESC(nowayout, + "Watchdog cannot be stopped once started (default=" + __MODULE_STRING(WATCHDOG_NOWAYOUT) ")"); + +/* possible feed values */ +#define TS4800_WDT_FEED_2S 0x1 +#define TS4800_WDT_FEED_10S 0x2 +#define TS4800_WDT_DISABLE 0x3 + +struct ts4800_wdt { + struct watchdog_device wdd; + struct regmap *regmap; + u32 feed_offset; + u32 feed_val; +}; + +/* + * TS-4800 supports the following timeout values: + * + * value desc + * - + * 0feed for 338ms + * 1feed for 2.706s + * 2feed for 10.824s + * 3disable watchdog + * + * Keep the regmap/timeout map ordered by timeout + */ +static const struct { + const int timeout; + const int regval; +} ts4800_wdt_map[] = { + { 2, TS4800_WDT_FEED_2S }, + { 10, TS4800_WDT_FEED_10S }, +}; + +#define MAX_TIMEOUT_INDEX (ARRAY_SIZE(ts4800_wd