From afe2750ef3ee3023de1bac9667f76317b1d3a77b Mon Sep 17 00:00:00 2001 From: Subbaraman Narayanamurthy Date: Tue, 21 Mar 2017 20:38:43 -0700 Subject: [PATCH] leds: qpnp-flash-v2: Fix pinctrl configuration Currently pinctrl is obtained after it is used to obtain strobe_enable and strobe_disable states. This is incorrect and needs to be fixed. Also, when the switch device is disabled, the pinctrl configuration for GPIO that drives the LED output is selected only when the brightness or current is set to a non-zero value in flash device. To keep it simple, move the pinctrl configuration for GPIO that drives the LED output (e.g. flash LED3) to the switch device so that both flash and torch device need not have to control it. Also, fix the pinctrl configuration for GPIOs that are used to drive flash LED3 output on both 8998 and 660 for functional correctness. Change-Id: If6ec27dd39fd750b6d4311a5e370020961f30e08 Signed-off-by: Subbaraman Narayanamurthy --- arch/arm/boot/dts/qcom/msm-pm660l.dtsi | 9 +- arch/arm/boot/dts/qcom/msm-pmi8998.dtsi | 9 +- arch/arm/boot/dts/qcom/msm8998-pinctrl.dtsi | 14 +- arch/arm/boot/dts/qcom/sdm660-pinctrl.dtsi | 14 +- drivers/leds/leds-qpnp-flash-v2.c | 137 +++++++++++--------- 5 files changed, 109 insertions(+), 74 deletions(-) diff --git a/arch/arm/boot/dts/qcom/msm-pm660l.dtsi b/arch/arm/boot/dts/qcom/msm-pm660l.dtsi index bcdbc4ed7c55..e0f5b0b7009d 100644 --- a/arch/arm/boot/dts/qcom/msm-pm660l.dtsi +++ b/arch/arm/boot/dts/qcom/msm-pm660l.dtsi @@ -330,9 +330,6 @@ qcom,ires-ua = <12500>; qcom,hdrm-voltage-mv = <325>; qcom,hdrm-vol-hi-lo-win-mv = <100>; - pinctrl-names = "led_enable","led_disable"; - pinctrl-0 = <&led_enable>; - pinctrl-1 = <&led_disable>; }; pm660l_torch0: qcom,torch_0 { @@ -369,9 +366,6 @@ qcom,ires-ua = <12500>; qcom,hdrm-voltage-mv = <325>; qcom,hdrm-vol-hi-lo-win-mv = <100>; - pinctrl-names = "led_enable","led_disable"; - pinctrl-0 = <&led_enable>; - pinctrl-1 = <&led_disable>; }; pm660l_switch0: qcom,led_switch_0 { @@ -386,6 +380,9 @@ qcom,led-name = "led:switch_1"; qcom,led-mask = <4>; qcom,default-led-trigger = "switch1_trigger"; + pinctrl-names = "led_enable","led_disable"; + pinctrl-0 = <&led_enable>; + pinctrl-1 = <&led_disable>; }; }; diff --git a/arch/arm/boot/dts/qcom/msm-pmi8998.dtsi b/arch/arm/boot/dts/qcom/msm-pmi8998.dtsi index 0cf67dd938e6..2d2b628ca815 100644 --- a/arch/arm/boot/dts/qcom/msm-pmi8998.dtsi +++ b/arch/arm/boot/dts/qcom/msm-pmi8998.dtsi @@ -712,9 +712,6 @@ qcom,ires-ua = <12500>; qcom,hdrm-voltage-mv = <325>; qcom,hdrm-vol-hi-lo-win-mv = <100>; - pinctrl-names = "led_enable","led_disable"; - pinctrl-0 = <&led_enable>; - pinctrl-1 = <&led_disable>; }; pmi8998_torch0: qcom,torch_0 { @@ -751,9 +748,6 @@ qcom,ires-ua = <12500>; qcom,hdrm-voltage-mv = <325>; qcom,hdrm-vol-hi-lo-win-mv = <100>; - pinctrl-names = "led_enable","led_disable"; - pinctrl-0 = <&led_enable>; - pinctrl-1 = <&led_disable>; }; pmi8998_switch0: qcom,led_switch_0 { @@ -768,6 +762,9 @@ qcom,led-name = "led:switch_1"; qcom,led-mask = <4>; qcom,default-led-trigger = "switch1_trigger"; + pinctrl-names = "led_enable","led_disable"; + pinctrl-0 = <&led_enable>; + pinctrl-1 = <&led_disable>; }; }; }; diff --git a/arch/arm/boot/dts/qcom/msm8998-pinctrl.dtsi b/arch/arm/boot/dts/qcom/msm8998-pinctrl.dtsi index d2e18db982ef..220bad31d7f8 100644 --- a/arch/arm/boot/dts/qcom/msm8998-pinctrl.dtsi +++ b/arch/arm/boot/dts/qcom/msm8998-pinctrl.dtsi @@ -1960,16 +1960,28 @@ led_enable: led_enable { mux { pins = "gpio21"; - drive_strength = <16>; + function = "gpio"; + }; + + config { + pins = "gpio21"; + drive_strength = <2>; output-high; + bias-disable; }; }; led_disable: led_disable { mux { + pins = "gpio21"; + function = "gpio"; + }; + + config { pins = "gpio21"; drive_strength = <2>; output-low; + bias-disable; }; }; diff --git a/arch/arm/boot/dts/qcom/sdm660-pinctrl.dtsi b/arch/arm/boot/dts/qcom/sdm660-pinctrl.dtsi index 37a88ea0dcec..efe58563a1f3 100644 --- a/arch/arm/boot/dts/qcom/sdm660-pinctrl.dtsi +++ b/arch/arm/boot/dts/qcom/sdm660-pinctrl.dtsi @@ -36,16 +36,28 @@ led_enable: led_enable { mux { pins = "gpio40"; - drive_strength = <16>; + function = "gpio"; + }; + + config { + pins = "gpio40"; + drive_strength = <2>; output-high; + bias-disable; }; }; led_disable: led_disable { mux { + pins = "gpio40"; + function = "gpio"; + }; + + config { pins = "gpio40"; drive_strength = <2>; output-low; + bias-disable; }; }; diff --git a/drivers/leds/leds-qpnp-flash-v2.c b/drivers/leds/leds-qpnp-flash-v2.c index a8bf5b4b24c5..4c28e0922c84 100644 --- a/drivers/leds/leds-qpnp-flash-v2.c +++ b/drivers/leds/leds-qpnp-flash-v2.c @@ -175,9 +175,7 @@ enum { struct flash_node_data { struct platform_device *pdev; struct led_classdev cdev; - struct pinctrl *pinctrl; - struct pinctrl_state *gpio_state_active; - struct pinctrl_state *gpio_state_suspend; + struct pinctrl *strobe_pinctrl; struct pinctrl_state *hw_strobe_state_active; struct pinctrl_state *hw_strobe_state_suspend; int hw_strobe_gpio; @@ -198,6 +196,9 @@ struct flash_node_data { struct flash_switch_data { struct platform_device *pdev; struct regulator *vreg; + struct pinctrl *led_en_pinctrl; + struct pinctrl_state *gpio_state_active; + struct pinctrl_state *gpio_state_suspend; struct led_classdev cdev; int led_mask; bool regulator_on; @@ -570,9 +571,9 @@ static int qpnp_flash_led_hw_strobe_enable(struct flash_node_data *fnode, if (gpio_is_valid(fnode->hw_strobe_gpio)) { gpio_set_value(fnode->hw_strobe_gpio, on ? 1 : 0); - } else if (fnode->hw_strobe_state_active && + } else if (fnode->strobe_pinctrl && fnode->hw_strobe_state_active && fnode->hw_strobe_state_suspend) { - rc = pinctrl_select_state(fnode->pinctrl, + rc = pinctrl_select_state(fnode->strobe_pinctrl, on ? fnode->hw_strobe_state_active : fnode->hw_strobe_state_suspend); if (rc < 0) { @@ -948,15 +949,6 @@ static int qpnp_flash_led_switch_disable(struct flash_switch_data *snode) led->fnode[i].led_on = false; - if (led->fnode[i].pinctrl) { - rc = pinctrl_select_state(led->fnode[i].pinctrl, - led->fnode[i].gpio_state_suspend); - if (rc < 0) { - pr_err("failed to disable GPIO, rc=%d\n", rc); - return rc; - } - } - if (led->fnode[i].trigger & FLASH_LED_HW_SW_STROBE_SEL_BIT) { rc = qpnp_flash_led_hw_strobe_enable(&led->fnode[i], led->pdata->hw_strobe_option, false); @@ -968,6 +960,17 @@ static int qpnp_flash_led_switch_disable(struct flash_switch_data *snode) } } + if (snode->led_en_pinctrl) { + pr_debug("Selecting suspend state for %s\n", snode->cdev.name); + rc = pinctrl_select_state(snode->led_en_pinctrl, + snode->gpio_state_suspend); + if (rc < 0) { + pr_err("failed to select pinctrl suspend state rc=%d\n", + rc); + return rc; + } + } + snode->enabled = false; return 0; } @@ -1038,15 +1041,6 @@ static int qpnp_flash_led_switch_set(struct flash_switch_data *snode, bool on) val |= FLASH_LED_ENABLE << led->fnode[i].id; - if (led->fnode[i].pinctrl) { - rc = pinctrl_select_state(led->fnode[i].pinctrl, - led->fnode[i].gpio_state_active); - if (rc < 0) { - pr_err("failed to enable GPIO rc=%d\n", rc); - return rc; - } - } - if (led->fnode[i].trigger & FLASH_LED_HW_SW_STROBE_SEL_BIT) { rc = qpnp_flash_led_hw_strobe_enable(&led->fnode[i], led->pdata->hw_strobe_option, true); @@ -1058,6 +1052,17 @@ static int qpnp_flash_led_switch_set(struct flash_switch_data *snode, bool on) } } + if (snode->led_en_pinctrl) { + pr_debug("Selecting active state for %s\n", snode->cdev.name); + rc = pinctrl_select_state(snode->led_en_pinctrl, + snode->gpio_state_active); + if (rc < 0) { + pr_err("failed to select pinctrl active state rc=%d\n", + rc); + return rc; + } + } + if (led->enable == 0) { rc = qpnp_flash_led_masked_write(led, FLASH_LED_REG_MOD_CTRL(led->base), @@ -1460,6 +1465,20 @@ static int qpnp_flash_led_parse_each_led_dt(struct qpnp_flash_led *led, } fnode->trigger = (strobe_sel << 2) | (edge_trigger << 1) | active_high; + rc = led_classdev_register(&led->pdev->dev, &fnode->cdev); + if (rc < 0) { + pr_err("Unable to register led node %d\n", fnode->id); + return rc; + } + + fnode->cdev.dev->of_node = node; + fnode->strobe_pinctrl = devm_pinctrl_get(fnode->cdev.dev); + if (IS_ERR_OR_NULL(fnode->strobe_pinctrl)) { + pr_debug("No pinctrl defined for %s, err=%ld\n", + fnode->cdev.name, PTR_ERR(fnode->strobe_pinctrl)); + fnode->strobe_pinctrl = NULL; + } + if (fnode->trigger & FLASH_LED_HW_SW_STROBE_SEL_BIT) { if (of_find_property(node, "qcom,hw-strobe-gpio", NULL)) { fnode->hw_strobe_gpio = of_get_named_gpio(node, @@ -1469,11 +1488,11 @@ static int qpnp_flash_led_parse_each_led_dt(struct qpnp_flash_led *led, return fnode->hw_strobe_gpio; } gpio_direction_output(fnode->hw_strobe_gpio, 0); - } else { + } else if (fnode->strobe_pinctrl) { fnode->hw_strobe_gpio = -1; fnode->hw_strobe_state_active = - pinctrl_lookup_state(fnode->pinctrl, - "strobe_enable"); + pinctrl_lookup_state(fnode->strobe_pinctrl, + "strobe_enable"); if (IS_ERR_OR_NULL(fnode->hw_strobe_state_active)) { pr_err("No active pin for hardware strobe, rc=%ld\n", PTR_ERR(fnode->hw_strobe_state_active)); @@ -1481,8 +1500,8 @@ static int qpnp_flash_led_parse_each_led_dt(struct qpnp_flash_led *led, } fnode->hw_strobe_state_suspend = - pinctrl_lookup_state(fnode->pinctrl, - "strobe_disable"); + pinctrl_lookup_state(fnode->strobe_pinctrl, + "strobe_disable"); if (IS_ERR_OR_NULL(fnode->hw_strobe_state_suspend)) { pr_err("No suspend pin for hardware strobe, rc=%ld\n", PTR_ERR(fnode->hw_strobe_state_suspend) @@ -1492,38 +1511,6 @@ static int qpnp_flash_led_parse_each_led_dt(struct qpnp_flash_led *led, } } - rc = led_classdev_register(&led->pdev->dev, &fnode->cdev); - if (rc < 0) { - pr_err("Unable to register led node %d\n", fnode->id); - return rc; - } - - fnode->cdev.dev->of_node = node; - - fnode->pinctrl = devm_pinctrl_get(fnode->cdev.dev); - if (IS_ERR_OR_NULL(fnode->pinctrl)) { - pr_debug("No pinctrl defined\n"); - fnode->pinctrl = NULL; - } else { - fnode->gpio_state_active = - pinctrl_lookup_state(fnode->pinctrl, "led_enable"); - if (IS_ERR_OR_NULL(fnode->gpio_state_active)) { - pr_err("Cannot lookup LED active state\n"); - devm_pinctrl_put(fnode->pinctrl); - fnode->pinctrl = NULL; - return PTR_ERR(fnode->gpio_state_active); - } - - fnode->gpio_state_suspend = - pinctrl_lookup_state(fnode->pinctrl, "led_disable"); - if (IS_ERR_OR_NULL(fnode->gpio_state_suspend)) { - pr_err("Cannot lookup LED disable state\n"); - devm_pinctrl_put(fnode->pinctrl); - fnode->pinctrl = NULL; - return PTR_ERR(fnode->gpio_state_suspend); - } - } - return 0; } @@ -1588,6 +1575,36 @@ static int qpnp_flash_led_parse_and_register_switch(struct qpnp_flash_led *led, } snode->cdev.dev->of_node = node; + + snode->led_en_pinctrl = devm_pinctrl_get(snode->cdev.dev); + if (IS_ERR_OR_NULL(snode->led_en_pinctrl)) { + pr_debug("No pinctrl defined for %s, err=%ld\n", + snode->cdev.name, PTR_ERR(snode->led_en_pinctrl)); + snode->led_en_pinctrl = NULL; + } + + if (snode->led_en_pinctrl) { + snode->gpio_state_active = + pinctrl_lookup_state(snode->led_en_pinctrl, + "led_enable"); + if (IS_ERR_OR_NULL(snode->gpio_state_active)) { + pr_err("Cannot lookup LED active state\n"); + devm_pinctrl_put(snode->led_en_pinctrl); + snode->led_en_pinctrl = NULL; + return PTR_ERR(snode->gpio_state_active); + } + + snode->gpio_state_suspend = + pinctrl_lookup_state(snode->led_en_pinctrl, + "led_disable"); + if (IS_ERR_OR_NULL(snode->gpio_state_suspend)) { + pr_err("Cannot lookup LED disable state\n"); + devm_pinctrl_put(snode->led_en_pinctrl); + snode->led_en_pinctrl = NULL; + return PTR_ERR(snode->gpio_state_suspend); + } + } + return 0; }