From 9c8eaec8ebe41198b10599f2586750d8900afd97 Mon Sep 17 00:00:00 2001 From: Charles Keepax Date: Mon, 9 Dec 2019 11:09:07 +0000 Subject: extcon: arizona: Correct clean up if arizona_identify_headphone fails In the error path of arizona_identify_headphone, neither the clamp nor the PM runtime are cleaned up. Add calls to clean up both of these. Signed-off-by: Charles Keepax Signed-off-by: Chanwoo Choi --- drivers/extcon/extcon-arizona.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c index e970134c95fa..79e9a2410182 100644 --- a/drivers/extcon/extcon-arizona.c +++ b/drivers/extcon/extcon-arizona.c @@ -724,6 +724,9 @@ static void arizona_identify_headphone(struct arizona_extcon_info *info) return; err: + arizona_extcon_hp_clamp(info, false); + pm_runtime_put_autosuspend(info->dev); + regmap_update_bits(arizona->regmap, ARIZONA_ACCESSORY_DETECT_MODE_1, ARIZONA_ACCDET_MODE_MASK, ARIZONA_ACCDET_MODE_MIC); -- cgit v1.2.3 From b82f871a335a8b0de178ace526c847183d17f66d Mon Sep 17 00:00:00 2001 From: Charles Keepax Date: Mon, 9 Dec 2019 11:09:08 +0000 Subject: extcon: arizona: Make rev A register sequences atomic The special register sequences that are applied for rev A of wm5102 should be applied atomically with respect to any other register writes. Use regmap_multi_reg_write to ensure all writes happen under the regmap lock. Signed-off-by: Charles Keepax Signed-off-by: Chanwoo Choi --- drivers/extcon/extcon-arizona.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c index 79e9a2410182..a8b0bc2d4323 100644 --- a/drivers/extcon/extcon-arizona.c +++ b/drivers/extcon/extcon-arizona.c @@ -310,9 +310,13 @@ static void arizona_start_mic(struct arizona_extcon_info *info) } if (info->micd_reva) { - regmap_write(arizona->regmap, 0x80, 0x3); - regmap_write(arizona->regmap, 0x294, 0); - regmap_write(arizona->regmap, 0x80, 0x0); + const struct reg_sequence reva[] = { + { 0x80, 0x3 }, + { 0x294, 0x0 }, + { 0x80, 0x0 }, + }; + + regmap_multi_reg_write(arizona->regmap, reva, ARRAY_SIZE(reva)); } if (info->detecting && arizona->pdata.micd_software_compare) @@ -361,9 +365,13 @@ static void arizona_stop_mic(struct arizona_extcon_info *info) snd_soc_dapm_sync(dapm); if (info->micd_reva) { - regmap_write(arizona->regmap, 0x80, 0x3); - regmap_write(arizona->regmap, 0x294, 2); - regmap_write(arizona->regmap, 0x80, 0x0); + const struct reg_sequence reva[] = { + { 0x80, 0x3 }, + { 0x294, 0x2 }, + { 0x80, 0x0 }, + }; + + regmap_multi_reg_write(arizona->regmap, reva, ARRAY_SIZE(reva)); } ret = regulator_allow_bypass(info->micvdd, true); -- cgit v1.2.3 From be87cb72bf7513fb92fa0e9e4ae83f958c73c042 Mon Sep 17 00:00:00 2001 From: Charles Keepax Date: Mon, 9 Dec 2019 11:09:09 +0000 Subject: extcon: arizona: Move pdata extraction to probe It makes no sense to be extracting values from pdata for the first time in the jack detection handler function, move this to probe time where it belongs. Signed-off-by: Charles Keepax Signed-off-by: Chanwoo Choi --- drivers/extcon/extcon-arizona.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c index a8b0bc2d4323..121c41706947 100644 --- a/drivers/extcon/extcon-arizona.c +++ b/drivers/extcon/extcon-arizona.c @@ -77,8 +77,6 @@ struct arizona_extcon_info { const struct arizona_micd_range *micd_ranges; int num_micd_ranges; - int micd_timeout; - bool micd_reva; bool micd_clamp; @@ -1016,7 +1014,7 @@ handled: queue_delayed_work(system_power_efficient_wq, &info->micd_timeout_work, - msecs_to_jiffies(info->micd_timeout)); + msecs_to_jiffies(arizona->pdata.micd_timeout)); } pm_runtime_mark_last_busy(info->dev); @@ -1136,7 +1134,7 @@ static irqreturn_t arizona_jackdet(int irq, void *data) msecs_to_jiffies(HPDET_DEBOUNCE)); if (cancelled_mic) { - int micd_timeout = info->micd_timeout; + int micd_timeout = arizona->pdata.micd_timeout; queue_delayed_work(system_power_efficient_wq, &info->micd_timeout_work, @@ -1213,11 +1211,6 @@ static irqreturn_t arizona_jackdet(int irq, void *data) ARIZONA_MICD_CLAMP_DB | ARIZONA_JD1_DB); } - if (arizona->pdata.micd_timeout) - info->micd_timeout = arizona->pdata.micd_timeout; - else - info->micd_timeout = DEFAULT_MICD_TIMEOUT; - out: /* Clear trig_sts to make sure DCVDD is not forced up */ regmap_write(arizona->regmap, ARIZONA_AOD_WKUP_AND_TRIG, @@ -1446,6 +1439,9 @@ static int arizona_extcon_probe(struct platform_device *pdev) info->input->name = "Headset"; info->input->phys = "arizona/extcon"; + if (!pdata->micd_timeout) + pdata->micd_timeout = DEFAULT_MICD_TIMEOUT; + if (pdata->num_micd_configs) { info->micd_modes = pdata->micd_configs; info->micd_num_modes = pdata->num_micd_configs; -- cgit v1.2.3 From ac7614fab9dd1054ec6bd082f02a436bb5cb082f Mon Sep 17 00:00:00 2001 From: Charles Keepax Date: Mon, 9 Dec 2019 11:09:10 +0000 Subject: extcon: arizona: Clear jack status regardless of detection type It makes sense to clear the internal state of the jack detection regardless of if the headphone detect based accessory detection or the normal microphone detect based flow is used. No issues are currently known because of this but the change makes more logical sense and eases future refactoring of the code. Signed-off-by: Charles Keepax Signed-off-by: Chanwoo Choi --- drivers/extcon/extcon-arizona.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c index 121c41706947..11f1d753aa10 100644 --- a/drivers/extcon/extcon-arizona.c +++ b/drivers/extcon/extcon-arizona.c @@ -1154,11 +1154,11 @@ static irqreturn_t arizona_jackdet(int irq, void *data) dev_err(arizona->dev, "Mechanical report failed: %d\n", ret); - if (!arizona->pdata.hpdet_acc_id) { - info->detecting = true; - info->mic = false; - info->jack_flips = 0; + info->detecting = true; + info->mic = false; + info->jack_flips = 0; + if (!arizona->pdata.hpdet_acc_id) { arizona_start_mic(info); } else { queue_delayed_work(system_power_efficient_wq, -- cgit v1.2.3 From d5aa46ddf9ed7cf0d7ea0b86bec20412733ac870 Mon Sep 17 00:00:00 2001 From: Charles Keepax Date: Mon, 9 Dec 2019 11:09:11 +0000 Subject: extcon: arizona: Tidy up transition from mic to headphone detect Moving from microphone detection to headphone detection is done fairly haphazardly at the moment, sometimes calling arizona_stop_mic at the call site sometimes relying on a call inside arizona_identify_headphone. Simplify all this and always call arizona_stop_mic at the top of arizona_identify_headphone. Signed-off-by: Charles Keepax Signed-off-by: Chanwoo Choi --- drivers/extcon/extcon-arizona.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c index 11f1d753aa10..5ae111ee3d63 100644 --- a/drivers/extcon/extcon-arizona.c +++ b/drivers/extcon/extcon-arizona.c @@ -705,8 +705,7 @@ static void arizona_identify_headphone(struct arizona_extcon_info *info) info->hpdet_active = true; - if (info->mic) - arizona_stop_mic(info); + arizona_stop_mic(info); arizona_extcon_hp_clamp(info, true); @@ -815,8 +814,6 @@ static void arizona_micd_timeout_work(struct work_struct *work) arizona_identify_headphone(info); - arizona_stop_mic(info); - mutex_unlock(&info->lock); } @@ -906,7 +903,6 @@ static void arizona_micd_detect(struct work_struct *work) if (!(val & ARIZONA_MICD_STS)) { dev_warn(arizona->dev, "Detected open circuit\n"); info->mic = false; - arizona_stop_mic(info); info->detecting = false; arizona_identify_headphone(info); goto handled; @@ -948,8 +944,6 @@ static void arizona_micd_detect(struct work_struct *work) info->detecting = false; arizona_identify_headphone(info); - - arizona_stop_mic(info); } else { info->micd_mode++; if (info->micd_mode == info->micd_num_modes) @@ -988,7 +982,6 @@ static void arizona_micd_detect(struct work_struct *work) } else if (info->detecting) { dev_dbg(arizona->dev, "Headphone detected\n"); info->detecting = false; - arizona_stop_mic(info); arizona_identify_headphone(info); } else { -- cgit v1.2.3 From f4ba6c0ba762e10b0bad42b5d28f7e2ab1fe5ff9 Mon Sep 17 00:00:00 2001 From: Charles Keepax Date: Mon, 9 Dec 2019 11:09:12 +0000 Subject: extcon: arizona: Remove unnecessary sets of ACCDET_MODE arizona_start_mic sets ACCDET_MODE as required for the microphone detection as such it is redundant to set this outside of this function. Signed-off-by: Charles Keepax Signed-off-by: Chanwoo Choi --- drivers/extcon/extcon-arizona.c | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c index 5ae111ee3d63..e7c198e798e2 100644 --- a/drivers/extcon/extcon-arizona.c +++ b/drivers/extcon/extcon-arizona.c @@ -668,11 +668,6 @@ done: if (id_gpio) gpio_set_value_cansleep(id_gpio, 0); - /* Revert back to MICDET mode */ - regmap_update_bits(arizona->regmap, - ARIZONA_ACCESSORY_DETECT_MODE_1, - ARIZONA_ACCDET_MODE_MASK, ARIZONA_ACCDET_MODE_MIC); - /* If we have a mic then reenable MICDET */ if (mic || info->mic) arizona_start_mic(info); @@ -732,9 +727,6 @@ err: arizona_extcon_hp_clamp(info, false); pm_runtime_put_autosuspend(info->dev); - regmap_update_bits(arizona->regmap, ARIZONA_ACCESSORY_DETECT_MODE_1, - ARIZONA_ACCDET_MODE_MASK, ARIZONA_ACCDET_MODE_MIC); - /* Just report headphone */ ret = extcon_set_state_sync(info->edev, EXTCON_JACK_HEADPHONE, true); if (ret != 0) @@ -789,9 +781,6 @@ static void arizona_start_hpdet_acc_id(struct arizona_extcon_info *info) return; err: - regmap_update_bits(arizona->regmap, ARIZONA_ACCESSORY_DETECT_MODE_1, - ARIZONA_ACCDET_MODE_MASK, ARIZONA_ACCDET_MODE_MIC); - /* Just report headphone */ ret = extcon_set_state_sync(info->edev, EXTCON_JACK_HEADPHONE, true); if (ret != 0) -- cgit v1.2.3 From 8267ebcc46176d90b7b169440ebbc890e09a5010 Mon Sep 17 00:00:00 2001 From: Charles Keepax Date: Mon, 9 Dec 2019 11:09:13 +0000 Subject: extcon: arizona: Remove excessive WARN_ON A WARN_ON is very strong for simply finding a button that is out of range, downgrade this to a simple error message in the log. Signed-off-by: Charles Keepax Signed-off-by: Chanwoo Choi --- drivers/extcon/extcon-arizona.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c index e7c198e798e2..3f7ced35e0b8 100644 --- a/drivers/extcon/extcon-arizona.c +++ b/drivers/extcon/extcon-arizona.c @@ -960,14 +960,13 @@ static void arizona_micd_detect(struct work_struct *work) input_report_key(info->input, info->micd_ranges[i].key, 0); - WARN_ON(!lvl); - WARN_ON(ffs(lvl) - 1 >= info->num_micd_ranges); if (lvl && ffs(lvl) - 1 < info->num_micd_ranges) { key = info->micd_ranges[ffs(lvl) - 1].key; input_report_key(info->input, key, 1); input_sync(info->input); + } else { + dev_err(arizona->dev, "Button out of range\n"); } - } else if (info->detecting) { dev_dbg(arizona->dev, "Headphone detected\n"); info->detecting = false; -- cgit v1.2.3 From 3dfa743dcd2e39064475ff7f622d05db61b65375 Mon Sep 17 00:00:00 2001 From: Charles Keepax Date: Mon, 9 Dec 2019 11:09:14 +0000 Subject: extcon: arizona: Invert logic of check in arizona_hpdet_do_id Invert the check of hpdet_acc_id at the top of arizona_hpdet_do_id to reduce the identation within the function. Signed-off-by: Charles Keepax Signed-off-by: Chanwoo Choi --- drivers/extcon/extcon-arizona.c | 92 ++++++++++++++++++++--------------------- 1 file changed, 45 insertions(+), 47 deletions(-) diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c index 3f7ced35e0b8..a0135f44cba1 100644 --- a/drivers/extcon/extcon-arizona.c +++ b/drivers/extcon/extcon-arizona.c @@ -533,67 +533,65 @@ static int arizona_hpdet_do_id(struct arizona_extcon_info *info, int *reading, struct arizona *arizona = info->arizona; int id_gpio = arizona->pdata.hpdet_id_gpio; + if (!arizona->pdata.hpdet_acc_id) + return 0; + /* * If we're using HPDET for accessory identification we need * to take multiple measurements, step through them in sequence. */ - if (arizona->pdata.hpdet_acc_id) { - info->hpdet_res[info->num_hpdet_res++] = *reading; + info->hpdet_res[info->num_hpdet_res++] = *reading; - /* Only check the mic directly if we didn't already ID it */ - if (id_gpio && info->num_hpdet_res == 1) { - dev_dbg(arizona->dev, "Measuring mic\n"); - - regmap_update_bits(arizona->regmap, - ARIZONA_ACCESSORY_DETECT_MODE_1, - ARIZONA_ACCDET_MODE_MASK | - ARIZONA_ACCDET_SRC, - ARIZONA_ACCDET_MODE_HPR | - info->micd_modes[0].src); + /* Only check the mic directly if we didn't already ID it */ + if (id_gpio && info->num_hpdet_res == 1) { + dev_dbg(arizona->dev, "Measuring mic\n"); - gpio_set_value_cansleep(id_gpio, 1); + regmap_update_bits(arizona->regmap, + ARIZONA_ACCESSORY_DETECT_MODE_1, + ARIZONA_ACCDET_MODE_MASK | + ARIZONA_ACCDET_SRC, + ARIZONA_ACCDET_MODE_HPR | + info->micd_modes[0].src); - regmap_update_bits(arizona->regmap, - ARIZONA_HEADPHONE_DETECT_1, - ARIZONA_HP_POLL, ARIZONA_HP_POLL); - return -EAGAIN; - } + gpio_set_value_cansleep(id_gpio, 1); - /* OK, got both. Now, compare... */ - dev_dbg(arizona->dev, "HPDET measured %d %d\n", - info->hpdet_res[0], info->hpdet_res[1]); + regmap_update_bits(arizona->regmap, ARIZONA_HEADPHONE_DETECT_1, + ARIZONA_HP_POLL, ARIZONA_HP_POLL); + return -EAGAIN; + } - /* Take the headphone impedance for the main report */ - *reading = info->hpdet_res[0]; + /* OK, got both. Now, compare... */ + dev_dbg(arizona->dev, "HPDET measured %d %d\n", + info->hpdet_res[0], info->hpdet_res[1]); - /* Sometimes we get false readings due to slow insert */ - if (*reading >= ARIZONA_HPDET_MAX && !info->hpdet_retried) { - dev_dbg(arizona->dev, "Retrying high impedance\n"); - info->num_hpdet_res = 0; - info->hpdet_retried = true; - arizona_start_hpdet_acc_id(info); - pm_runtime_put(info->dev); - return -EAGAIN; - } + /* Take the headphone impedance for the main report */ + *reading = info->hpdet_res[0]; - /* - * If we measure the mic as high impedance - */ - if (!id_gpio || info->hpdet_res[1] > 50) { - dev_dbg(arizona->dev, "Detected mic\n"); - *mic = true; - info->detecting = true; - } else { - dev_dbg(arizona->dev, "Detected headphone\n"); - } + /* Sometimes we get false readings due to slow insert */ + if (*reading >= ARIZONA_HPDET_MAX && !info->hpdet_retried) { + dev_dbg(arizona->dev, "Retrying high impedance\n"); + info->num_hpdet_res = 0; + info->hpdet_retried = true; + arizona_start_hpdet_acc_id(info); + pm_runtime_put(info->dev); + return -EAGAIN; + } - /* Make sure everything is reset back to the real polarity */ - regmap_update_bits(arizona->regmap, - ARIZONA_ACCESSORY_DETECT_MODE_1, - ARIZONA_ACCDET_SRC, - info->micd_modes[0].src); + /* + * If we measure the mic as high impedance + */ + if (!id_gpio || info->hpdet_res[1] > 50) { + dev_dbg(arizona->dev, "Detected mic\n"); + *mic = true; + info->detecting = true; + } else { + dev_dbg(arizona->dev, "Detected headphone\n"); } + /* Make sure everything is reset back to the real polarity */ + regmap_update_bits(arizona->regmap, ARIZONA_ACCESSORY_DETECT_MODE_1, + ARIZONA_ACCDET_SRC, info->micd_modes[0].src); + return 0; } -- cgit v1.2.3 From 7e14fc437c8144d1d5587d7b3a8d33e732144227 Mon Sep 17 00:00:00 2001 From: Charles Keepax Date: Mon, 9 Dec 2019 11:09:15 +0000 Subject: extcon: arizona: Factor out microphone impedance into a function The microphone detection handler is very long, start breaking it up by factoring out the actual reading of the impedance value into a separate functions. Additionally, this also fixes a minor bug and ensures that the microphone timeout will be rescheduled in all error cases. Signed-off-by: Charles Keepax Signed-off-by: Chanwoo Choi --- drivers/extcon/extcon-arizona.c | 125 +++++++++++++++++++++++----------------- 1 file changed, 73 insertions(+), 52 deletions(-) diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c index a0135f44cba1..b09a9a8ce98b 100644 --- a/drivers/extcon/extcon-arizona.c +++ b/drivers/extcon/extcon-arizona.c @@ -804,70 +804,55 @@ static void arizona_micd_timeout_work(struct work_struct *work) mutex_unlock(&info->lock); } -static void arizona_micd_detect(struct work_struct *work) +static int arizona_micd_adc_read(struct arizona_extcon_info *info) { - struct arizona_extcon_info *info = container_of(work, - struct arizona_extcon_info, - micd_detect_work.work); struct arizona *arizona = info->arizona; - unsigned int val = 0, lvl; - int ret, i, key; - - cancel_delayed_work_sync(&info->micd_timeout_work); + unsigned int val; + int ret; - mutex_lock(&info->lock); + /* Must disable MICD before we read the ADCVAL */ + regmap_update_bits(arizona->regmap, ARIZONA_MIC_DETECT_1, + ARIZONA_MICD_ENA, 0); - /* If the cable was removed while measuring ignore the result */ - ret = extcon_get_state(info->edev, EXTCON_MECHANICAL); - if (ret < 0) { - dev_err(arizona->dev, "Failed to check cable state: %d\n", - ret); - mutex_unlock(&info->lock); - return; - } else if (!ret) { - dev_dbg(arizona->dev, "Ignoring MICDET for removed cable\n"); - mutex_unlock(&info->lock); - return; + ret = regmap_read(arizona->regmap, ARIZONA_MIC_DETECT_4, &val); + if (ret != 0) { + dev_err(arizona->dev, + "Failed to read MICDET_ADCVAL: %d\n", ret); + return ret; } - if (info->detecting && arizona->pdata.micd_software_compare) { - /* Must disable MICD before we read the ADCVAL */ - regmap_update_bits(arizona->regmap, ARIZONA_MIC_DETECT_1, - ARIZONA_MICD_ENA, 0); - ret = regmap_read(arizona->regmap, ARIZONA_MIC_DETECT_4, &val); - if (ret != 0) { - dev_err(arizona->dev, - "Failed to read MICDET_ADCVAL: %d\n", - ret); - mutex_unlock(&info->lock); - return; - } + dev_dbg(arizona->dev, "MICDET_ADCVAL: %x\n", val); - dev_dbg(arizona->dev, "MICDET_ADCVAL: %x\n", val); + val &= ARIZONA_MICDET_ADCVAL_MASK; + if (val < ARRAY_SIZE(arizona_micd_levels)) + val = arizona_micd_levels[val]; + else + val = INT_MAX; + + if (val <= QUICK_HEADPHONE_MAX_OHM) + val = ARIZONA_MICD_STS | ARIZONA_MICD_LVL_0; + else if (val <= MICROPHONE_MIN_OHM) + val = ARIZONA_MICD_STS | ARIZONA_MICD_LVL_1; + else if (val <= MICROPHONE_MAX_OHM) + val = ARIZONA_MICD_STS | ARIZONA_MICD_LVL_8; + else + val = ARIZONA_MICD_LVL_8; - val &= ARIZONA_MICDET_ADCVAL_MASK; - if (val < ARRAY_SIZE(arizona_micd_levels)) - val = arizona_micd_levels[val]; - else - val = INT_MAX; - - if (val <= QUICK_HEADPHONE_MAX_OHM) - val = ARIZONA_MICD_STS | ARIZONA_MICD_LVL_0; - else if (val <= MICROPHONE_MIN_OHM) - val = ARIZONA_MICD_STS | ARIZONA_MICD_LVL_1; - else if (val <= MICROPHONE_MAX_OHM) - val = ARIZONA_MICD_STS | ARIZONA_MICD_LVL_8; - else - val = ARIZONA_MICD_LVL_8; - } + return val; +} + +static int arizona_micd_read(struct arizona_extcon_info *info) +{ + struct arizona *arizona = info->arizona; + unsigned int val = 0; + int ret, i; for (i = 0; i < 10 && !(val & MICD_LVL_0_TO_8); i++) { ret = regmap_read(arizona->regmap, ARIZONA_MIC_DETECT_3, &val); if (ret != 0) { dev_err(arizona->dev, "Failed to read MICDET: %d\n", ret); - mutex_unlock(&info->lock); - return; + return ret; } dev_dbg(arizona->dev, "MICDET: %x\n", val); @@ -875,17 +860,53 @@ static void arizona_micd_detect(struct work_struct *work) if (!(val & ARIZONA_MICD_VALID)) { dev_warn(arizona->dev, "Microphone detection state invalid\n"); - mutex_unlock(&info->lock); - return; + return -EINVAL; } } if (i == 10 && !(val & MICD_LVL_0_TO_8)) { dev_err(arizona->dev, "Failed to get valid MICDET value\n"); + return -EINVAL; + } + + return val; +} + +static void arizona_micd_detect(struct work_struct *work) +{ + struct arizona_extcon_info *info = container_of(work, + struct arizona_extcon_info, + micd_detect_work.work); + struct arizona *arizona = info->arizona; + unsigned int val = 0, lvl; + int ret, i, key; + + cancel_delayed_work_sync(&info->micd_timeout_work); + + mutex_lock(&info->lock); + + /* If the cable was removed while measuring ignore the result */ + ret = extcon_get_state(info->edev, EXTCON_MECHANICAL); + if (ret < 0) { + dev_err(arizona->dev, "Failed to check cable state: %d\n", + ret); + mutex_unlock(&info->lock); + return; + } else if (!ret) { + dev_dbg(arizona->dev, "Ignoring MICDET for removed cable\n"); mutex_unlock(&info->lock); return; } + if (info->detecting && arizona->pdata.micd_software_compare) + ret = arizona_micd_adc_read(info); + else + ret = arizona_micd_read(info); + if (ret < 0) + goto handled; + + val = ret; + /* Due to jack detect this should never happen */ if (!(val & ARIZONA_MICD_STS)) { dev_warn(arizona->dev, "Detected open circuit\n"); -- cgit v1.2.3 From 4b28b25c3062002eebdef1a4e6bab2f8293308f1 Mon Sep 17 00:00:00 2001 From: Charles Keepax Date: Mon, 9 Dec 2019 11:09:16 +0000 Subject: extcon: arizona: Factor out microphone and button detection Continue refactoring the microphone detect handling by factoring out the handling for microphone detection and button detection into separate functions. This both makes the code a little clearer and prepares for some planned future refactoring to make the state handling in the driver more explicit. Signed-off-by: Charles Keepax Signed-off-by: Chanwoo Choi --- drivers/extcon/extcon-arizona.c | 115 +++++++++++++++++++++++++--------------- 1 file changed, 71 insertions(+), 44 deletions(-) diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c index b09a9a8ce98b..7401733db08b 100644 --- a/drivers/extcon/extcon-arizona.c +++ b/drivers/extcon/extcon-arizona.c @@ -872,38 +872,18 @@ static int arizona_micd_read(struct arizona_extcon_info *info) return val; } -static void arizona_micd_detect(struct work_struct *work) +static int arizona_micdet_reading(void *priv) { - struct arizona_extcon_info *info = container_of(work, - struct arizona_extcon_info, - micd_detect_work.work); + struct arizona_extcon_info *info = priv; struct arizona *arizona = info->arizona; - unsigned int val = 0, lvl; - int ret, i, key; - - cancel_delayed_work_sync(&info->micd_timeout_work); - - mutex_lock(&info->lock); - - /* If the cable was removed while measuring ignore the result */ - ret = extcon_get_state(info->edev, EXTCON_MECHANICAL); - if (ret < 0) { - dev_err(arizona->dev, "Failed to check cable state: %d\n", - ret); - mutex_unlock(&info->lock); - return; - } else if (!ret) { - dev_dbg(arizona->dev, "Ignoring MICDET for removed cable\n"); - mutex_unlock(&info->lock); - return; - } + int ret, val; if (info->detecting && arizona->pdata.micd_software_compare) ret = arizona_micd_adc_read(info); else ret = arizona_micd_read(info); if (ret < 0) - goto handled; + return ret; val = ret; @@ -913,11 +893,11 @@ static void arizona_micd_detect(struct work_struct *work) info->mic = false; info->detecting = false; arizona_identify_headphone(info); - goto handled; + return 0; } /* If we got a high impedence we should have a headset, report it. */ - if (info->detecting && (val & ARIZONA_MICD_LVL_8)) { + if (val & ARIZONA_MICD_LVL_8) { info->mic = true; info->detecting = false; @@ -936,7 +916,7 @@ static void arizona_micd_detect(struct work_struct *work) ret); } - goto handled; + return 0; } /* If we detected a lower impedence during initial startup @@ -945,7 +925,7 @@ static void arizona_micd_detect(struct work_struct *work) * plain headphones. If both polarities report a low * impedence then give up and report headphones. */ - if (info->detecting && (val & MICD_LVL_1_TO_7)) { + if (val & MICD_LVL_1_TO_7) { if (info->jack_flips >= info->micd_num_modes * 10) { dev_dbg(arizona->dev, "Detected HP/line\n"); @@ -959,11 +939,43 @@ static void arizona_micd_detect(struct work_struct *work) arizona_extcon_set_mode(info, info->micd_mode); info->jack_flips++; + + if (arizona->pdata.micd_software_compare) + regmap_update_bits(arizona->regmap, + ARIZONA_MIC_DETECT_1, + ARIZONA_MICD_ENA, + ARIZONA_MICD_ENA); + + queue_delayed_work(system_power_efficient_wq, + &info->micd_timeout_work, + msecs_to_jiffies(arizona->pdata.micd_timeout)); } - goto handled; + return 0; } + /* + * If we're still detecting and we detect a short then we've + * got a headphone. + */ + dev_dbg(arizona->dev, "Headphone detected\n"); + info->detecting = false; + + arizona_identify_headphone(info); + + return 0; +} + +static int arizona_button_reading(void *priv) +{ + struct arizona_extcon_info *info = priv; + struct arizona *arizona = info->arizona; + int val, key, lvl, i; + + val = arizona_micd_read(info); + if (val < 0) + return val; + /* * If we're still detecting and we detect a short then we've * got a headphone. Otherwise it's a button press. @@ -986,11 +998,6 @@ static void arizona_micd_detect(struct work_struct *work) } else { dev_err(arizona->dev, "Button out of range\n"); } - } else if (info->detecting) { - dev_dbg(arizona->dev, "Headphone detected\n"); - info->detecting = false; - - arizona_identify_headphone(info); } else { dev_warn(arizona->dev, "Button with no mic: %x\n", val); @@ -1004,19 +1011,39 @@ static void arizona_micd_detect(struct work_struct *work) arizona_extcon_pulse_micbias(info); } -handled: - if (info->detecting) { - if (arizona->pdata.micd_software_compare) - regmap_update_bits(arizona->regmap, - ARIZONA_MIC_DETECT_1, - ARIZONA_MICD_ENA, - ARIZONA_MICD_ENA); + return 0; +} - queue_delayed_work(system_power_efficient_wq, - &info->micd_timeout_work, - msecs_to_jiffies(arizona->pdata.micd_timeout)); +static void arizona_micd_detect(struct work_struct *work) +{ + struct arizona_extcon_info *info = container_of(work, + struct arizona_extcon_info, + micd_detect_work.work); + struct arizona *arizona = info->arizona; + int ret; + + cancel_delayed_work_sync(&info->micd_timeout_work); + + mutex_lock(&info->lock); + + /* If the cable was removed while measuring ignore the result */ + ret = extcon_get_state(info->edev, EXTCON_MECHANICAL); + if (ret < 0) { + dev_err(arizona->dev, "Failed to check cable state: %d\n", + ret); + mutex_unlock(&info->lock); + return; + } else if (!ret) { + dev_dbg(arizona->dev, "Ignoring MICDET for removed cable\n"); + mutex_unlock(&info->lock); + return; } + if (info->detecting) + arizona_micdet_reading(info); + else + arizona_button_reading(info); + pm_runtime_mark_last_busy(info->dev); mutex_unlock(&info->lock); } -- cgit v1.2.3 From 2ddf50a75dab49eeb534dbdad92972f56a84046d Mon Sep 17 00:00:00 2001 From: Xu Wang Date: Fri, 13 Dec 2019 09:48:34 +0000 Subject: extcon: sm5502: Remove unneeded semicolon Remove unneeded semicolon reported by coccinelle. Signed-off-by: Xu Wang [cw00.choi: Edit patch title and description] Signed-off-by: Chanwoo Choi --- drivers/extcon/extcon-sm5502.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/extcon/extcon-sm5502.c b/drivers/extcon/extcon-sm5502.c index bcf65aaca5d2..106d4da647bd 100644 --- a/drivers/extcon/extcon-sm5502.c +++ b/drivers/extcon/extcon-sm5502.c @@ -249,7 +249,7 @@ static int sm5502_muic_set_path(struct sm5502_muic_info *info, dev_err(info->dev, "Unknown DM_CON/DP_CON switch type (%d)\n", con_sw); return -EINVAL; - }; + } switch (vbus_sw) { case VBUSIN_SWITCH_OPEN: @@ -268,7 +268,7 @@ static int sm5502_muic_set_path(struct sm5502_muic_info *info, default: dev_err(info->dev, "Unknown VBUS switch type (%d)\n", vbus_sw); return -EINVAL; - }; + } return 0; } @@ -357,13 +357,13 @@ static unsigned int sm5502_muic_get_cable_type(struct sm5502_muic_info *info) "cannot identify the cable type: adc(0x%x)\n", adc); return -EINVAL; - }; + } break; default: dev_err(info->dev, "failed to identify the cable type: adc(0x%x)\n", adc); return -EINVAL; - }; + } return cable_type; } @@ -405,7 +405,7 @@ static int sm5502_muic_cable_handler(struct sm5502_muic_info *info, dev_dbg(info->dev, "cannot handle this cable_type (0x%x)\n", cable_type); return 0; - }; + } /* Change internal hardware path(DM_CON/DP_CON, VBUSIN) */ ret = sm5502_muic_set_path(info, con_sw, vbus_sw, attached); -- cgit v1.2.3 From 3cce2c6fa70c768e516bff011d77db72e2f38a15 Mon Sep 17 00:00:00 2001 From: Georgi Djakov Date: Mon, 2 Dec 2019 18:21:32 +0200 Subject: interconnect: Add a common helper for removing all nodes The removal of all nodes from a provider seem to be a common functionality for all existing users and it would make sense to factor out this into a a common helper function. Suggested-by: Dmitry Osipenko Reviewed-by: Bjorn Andersson Signed-off-by: Georgi Djakov --- drivers/interconnect/core.c | 22 ++++++++++++++++++++++ include/linux/interconnect-provider.h | 6 ++++++ 2 files changed, 28 insertions(+) diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c index c498796adc07..1b811423020a 100644 --- a/drivers/interconnect/core.c +++ b/drivers/interconnect/core.c @@ -742,6 +742,28 @@ void icc_node_del(struct icc_node *node) } EXPORT_SYMBOL_GPL(icc_node_del); +/** + * icc_nodes_remove() - remove all previously added nodes from provider + * @provider: the interconnect provider we are removing nodes from + * + * Return: 0 on success, or an error code otherwise + */ +int icc_nodes_remove(struct icc_provider *provider) +{ + struct icc_node *n, *tmp; + + if (WARN_ON(IS_ERR_OR_NULL(provider))) + return -EINVAL; + + list_for_each_entry_safe_reverse(n, tmp, &provider->nodes, node_list) { + icc_node_del(n); + icc_node_destroy(n->id); + } + + return 0; +} +EXPORT_SYMBOL_GPL(icc_nodes_remove); + /** * icc_provider_add() - add a new interconnect provider * @provider: the interconnect provider that will be added into topology diff --git a/include/linux/interconnect-provider.h b/include/linux/interconnect-provider.h index b16f9effa555..31440c921216 100644 --- a/include/linux/interconnect-provider.h +++ b/include/linux/interconnect-provider.h @@ -98,6 +98,7 @@ int icc_link_create(struct icc_node *node, const int dst_id); int icc_link_destroy(struct icc_node *src, struct icc_node *dst); void icc_node_add(struct icc_node *node, struct icc_provider *provider); void icc_node_del(struct icc_node *node); +int icc_nodes_remove(struct icc_provider *provider); int icc_provider_add(struct icc_provider *provider); int icc_provider_del(struct icc_provider *provider); @@ -130,6 +131,11 @@ void icc_node_del(struct icc_node *node) { } +static inline int icc_nodes_remove(struct icc_provider *provider) +{ + return -ENOTSUPP; +} + static inline int icc_provider_add(struct icc_provider *provider) { return -ENOTSUPP; -- cgit v1.2.3 From ad3703ac24e7d6d2c5ca2ce7abeb1fd0b0afc01e Mon Sep 17 00:00:00 2001 From: Georgi Djakov Date: Mon, 2 Dec 2019 18:21:33 +0200 Subject: interconnect: qcom: Use the new common helper for node removal There is a new helper function for removing all nodes. Let's use it instead of duplicating the code. Reviewed-by: Bjorn Andersson Signed-off-by: Georgi Djakov --- drivers/interconnect/qcom/msm8974.c | 17 ++++------------- drivers/interconnect/qcom/qcs404.c | 17 ++++------------- drivers/interconnect/qcom/sdm845.c | 16 +++------------- 3 files changed, 11 insertions(+), 39 deletions(-) diff --git a/drivers/interconnect/qcom/msm8974.c b/drivers/interconnect/qcom/msm8974.c index bf8bd1aee358..e669a1f726d2 100644 --- a/drivers/interconnect/qcom/msm8974.c +++ b/drivers/interconnect/qcom/msm8974.c @@ -652,7 +652,7 @@ static int msm8974_icc_probe(struct platform_device *pdev) struct device *dev = &pdev->dev; struct icc_onecell_data *data; struct icc_provider *provider; - struct icc_node *node, *tmp; + struct icc_node *node; size_t num_nodes, i; int ret; @@ -732,10 +732,7 @@ static int msm8974_icc_probe(struct platform_device *pdev) return 0; err_del_icc: - list_for_each_entry_safe(node, tmp, &provider->nodes, node_list) { - icc_node_del(node); - icc_node_destroy(node->id); - } + icc_nodes_remove(provider); icc_provider_del(provider); err_disable_clks: @@ -747,16 +744,10 @@ err_disable_clks: static int msm8974_icc_remove(struct platform_device *pdev) { struct msm8974_icc_provider *qp = platform_get_drvdata(pdev); - struct icc_provider *provider = &qp->provider; - struct icc_node *n, *tmp; - list_for_each_entry_safe(n, tmp, &provider->nodes, node_list) { - icc_node_del(n); - icc_node_destroy(n->id); - } + icc_nodes_remove(&qp->provider); clk_bulk_disable_unprepare(qp->num_clks, qp->bus_clks); - - return icc_provider_del(provider); + return icc_provider_del(&qp->provider); } static const struct of_device_id msm8974_noc_of_match[] = { diff --git a/drivers/interconnect/qcom/qcs404.c b/drivers/interconnect/qcom/qcs404.c index 8e0735a87040..c8eb1276cce8 100644 --- a/drivers/interconnect/qcom/qcs404.c +++ b/drivers/interconnect/qcom/qcs404.c @@ -414,7 +414,7 @@ static int qnoc_probe(struct platform_device *pdev) struct icc_provider *provider; struct qcom_icc_node **qnodes; struct qcom_icc_provider *qp; - struct icc_node *node, *tmp; + struct icc_node *node; size_t num_nodes, i; int ret; @@ -494,10 +494,7 @@ static int qnoc_probe(struct platform_device *pdev) return 0; err: - list_for_each_entry_safe(node, tmp, &provider->nodes, node_list) { - icc_node_del(node); - icc_node_destroy(node->id); - } + icc_nodes_remove(provider); clk_bulk_disable_unprepare(qp->num_clks, qp->bus_clks); icc_provider_del(provider); @@ -507,16 +504,10 @@ err: static int qnoc_remove(struct platform_device *pdev) { struct qcom_icc_provider *qp = platform_get_drvdata(pdev); - struct icc_provider *provider = &qp->provider; - struct icc_node *n, *tmp; - list_for_each_entry_safe(n, tmp, &provider->nodes, node_list) { - icc_node_del(n); - icc_node_destroy(n->id); - } + icc_nodes_remove(&qp->provider); clk_bulk_disable_unprepare(qp->num_clks, qp->bus_clks); - - return icc_provider_del(provider); + return icc_provider_del(&qp->provider); } static const struct of_device_id qcs404_noc_of_match[] = { diff --git a/drivers/interconnect/qcom/sdm845.c b/drivers/interconnect/qcom/sdm845.c index 387267ee9648..f078cf0fce56 100644 --- a/drivers/interconnect/qcom/sdm845.c +++ b/drivers/interconnect/qcom/sdm845.c @@ -855,11 +855,7 @@ static int qnoc_probe(struct platform_device *pdev) return ret; err: - list_for_each_entry(node, &provider->nodes, node_list) { - icc_node_del(node); - icc_node_destroy(node->id); - } - + icc_nodes_remove(provider); icc_provider_del(provider); return ret; } @@ -867,15 +863,9 @@ err: static int qnoc_remove(struct platform_device *pdev) { struct qcom_icc_provider *qp = platform_get_drvdata(pdev); - struct icc_provider *provider = &qp->provider; - struct icc_node *n, *tmp; - - list_for_each_entry_safe(n, tmp, &provider->nodes, node_list) { - icc_node_del(n); - icc_node_destroy(n->id); - } - return icc_provider_del(provider); + icc_nodes_remove(&qp->provider); + return icc_provider_del(&qp->provider); } static const struct of_device_id qnoc_of_match[] = { -- cgit v1.2.3 From dd018a9cf9108f9c7d924f6fe09aed745e78a67e Mon Sep 17 00:00:00 2001 From: Georgi Djakov Date: Thu, 28 Nov 2019 16:18:16 +0200 Subject: interconnect: Move internal structs into a separate file Move the interconnect framework internal structs into a separate file, so that it can be included and used by ftrace code. This will allow us to expose some more useful information in the traces. Reviewed-by: Bjorn Andersson Signed-off-by: Georgi Djakov --- drivers/interconnect/core.c | 30 ++---------------------------- drivers/interconnect/internal.h | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 28 deletions(-) create mode 100644 drivers/interconnect/internal.h diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c index 1b811423020a..f30a326dc7ce 100644 --- a/drivers/interconnect/core.c +++ b/drivers/interconnect/core.c @@ -19,39 +19,13 @@ #include #include +#include "internal.h" + static DEFINE_IDR(icc_idr); static LIST_HEAD(icc_providers); static DEFINE_MUTEX(icc_lock); static struct dentry *icc_debugfs_dir; -/** - * struct icc_req - constraints that are attached to each node - * @req_node: entry in list of requests for the particular @node - * @node: the interconnect node to which this constraint applies - * @dev: reference to the device that sets the constraints - * @tag: path tag (optional) - * @avg_bw: an integer describing the average bandwidth in kBps - * @peak_bw: an integer describing the peak bandwidth in kBps - */ -struct icc_req { - struct hlist_node req_node; - struct icc_node *node; - struct device *dev; - u32 tag; - u32 avg_bw; - u32 peak_bw; -}; - -/** - * struct icc_path - interconnect path structure - * @num_nodes: number of hops (nodes) - * @reqs: array of the requests applicable to this path of nodes - */ -struct icc_path { - size_t num_nodes; - struct icc_req reqs[]; -}; - static void icc_summary_show_one(struct seq_file *s, struct icc_node *n) { if (!n) diff --git a/drivers/interconnect/internal.h b/drivers/interconnect/internal.h new file mode 100644 index 000000000000..5853e8faf223 --- /dev/null +++ b/drivers/interconnect/internal.h @@ -0,0 +1,40 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Interconnect framework internal structs + * + * Copyright (c) 2019, Linaro Ltd. + * Author: Georgi Djakov + */ + +#ifndef __DRIVERS_INTERCONNECT_INTERNAL_H +#define __DRIVERS_INTERCONNECT_INTERNAL_H + +/** + * struct icc_req - constraints that are attached to each node + * @req_node: entry in list of requests for the particular @node + * @node: the interconnect node to which this constraint applies + * @dev: reference to the device that sets the constraints + * @tag: path tag (optional) + * @avg_bw: an integer describing the average bandwidth in kBps + * @peak_bw: an integer describing the peak bandwidth in kBps + */ +struct icc_req { + struct hlist_node req_node; + struct icc_node *node; + struct device *dev; + u32 tag; + u32 avg_bw; + u32 peak_bw; +}; + +/** + * struct icc_path - interconnect path structure + * @num_nodes: number of hops (nodes) + * @reqs: array of the requests applicable to this path of nodes + */ +struct icc_path { + size_t num_nodes; + struct icc_req reqs[]; +}; + +#endif -- cgit v1.2.3 From 05309830e1f869f939e283576dd3684313390062 Mon Sep 17 00:00:00 2001 From: Georgi Djakov Date: Thu, 28 Nov 2019 16:18:17 +0200 Subject: interconnect: Add a name to struct icc_path When debugging interconnect things, it turned out that saving the path name and including it in the traces is quite useful, especially for devices with multiple paths. For the path name we use the one specified in DT, or if we use platform data, the name is based on the source and destination node names. Suggested-by: Bjorn Andersson Reviewed-by: Bjorn Andersson Signed-off-by: Georgi Djakov --- drivers/interconnect/core.c | 18 +++++++++++++++--- drivers/interconnect/internal.h | 2 ++ 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c index f30a326dc7ce..4f9bdb7f9165 100644 --- a/drivers/interconnect/core.c +++ b/drivers/interconnect/core.c @@ -356,9 +356,17 @@ struct icc_path *of_icc_get(struct device *dev, const char *name) mutex_lock(&icc_lock); path = path_find(dev, src_node, dst_node); - if (IS_ERR(path)) - dev_err(dev, "%s: invalid path=%ld\n", __func__, PTR_ERR(path)); mutex_unlock(&icc_lock); + if (IS_ERR(path)) { + dev_err(dev, "%s: invalid path=%ld\n", __func__, PTR_ERR(path)); + return path; + } + + if (name) + path->name = kstrdup_const(name, GFP_KERNEL); + else + path->name = kasprintf(GFP_KERNEL, "%s-%s", + src_node->name, dst_node->name); return path; } @@ -481,9 +489,12 @@ struct icc_path *icc_get(struct device *dev, const int src_id, const int dst_id) goto out; path = path_find(dev, src, dst); - if (IS_ERR(path)) + if (IS_ERR(path)) { dev_err(dev, "%s: invalid path=%ld\n", __func__, PTR_ERR(path)); + goto out; + } + path->name = kasprintf(GFP_KERNEL, "%s-%s", src->name, dst->name); out: mutex_unlock(&icc_lock); return path; @@ -519,6 +530,7 @@ void icc_put(struct icc_path *path) } mutex_unlock(&icc_lock); + kfree_const(path->name); kfree(path); } EXPORT_SYMBOL_GPL(icc_put); diff --git a/drivers/interconnect/internal.h b/drivers/interconnect/internal.h index 5853e8faf223..bf18cb7239df 100644 --- a/drivers/interconnect/internal.h +++ b/drivers/interconnect/internal.h @@ -29,10 +29,12 @@ struct icc_req { /** * struct icc_path - interconnect path structure + * @name: a string name of the path (useful for ftrace) * @num_nodes: number of hops (nodes) * @reqs: array of the requests applicable to this path of nodes */ struct icc_path { + const char *name; size_t num_nodes; struct icc_req reqs[]; }; -- cgit v1.2.3 From c46ab9db64979b0875fff79e1b00013343ca8286 Mon Sep 17 00:00:00 2001 From: Georgi Djakov Date: Thu, 28 Nov 2019 16:18:18 +0200 Subject: interconnect: Add basic tracepoints The tracepoints can help with understanding the system behavior of a given interconnect path when the consumer drivers change their bandwidth demands. This might be interesting when we want to monitor the requested interconnect bandwidth for each client driver. The paths may share the same nodes and this will help to understand "who and when is requesting what". All this is useful for subsystem drivers developers and may also provide hints when optimizing the power and performance profile of the system. Reviewed-by: Steven Rostedt (VMware) Reviewed-by: Bjorn Andersson Signed-off-by: Georgi Djakov --- drivers/interconnect/Makefile | 1 + drivers/interconnect/core.c | 7 ++++ drivers/interconnect/trace.h | 88 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 96 insertions(+) create mode 100644 drivers/interconnect/trace.h diff --git a/drivers/interconnect/Makefile b/drivers/interconnect/Makefile index 28f2ab0824d5..725029ae7a2c 100644 --- a/drivers/interconnect/Makefile +++ b/drivers/interconnect/Makefile @@ -1,5 +1,6 @@ # SPDX-License-Identifier: GPL-2.0 +CFLAGS_core.o := -I$(src) icc-core-objs := core.o obj-$(CONFIG_INTERCONNECT) += icc-core.o diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c index 4f9bdb7f9165..fbec2e4fdfeb 100644 --- a/drivers/interconnect/core.c +++ b/drivers/interconnect/core.c @@ -21,6 +21,9 @@ #include "internal.h" +#define CREATE_TRACE_POINTS +#include "trace.h" + static DEFINE_IDR(icc_idr); static LIST_HEAD(icc_providers); static DEFINE_MUTEX(icc_lock); @@ -435,6 +438,8 @@ int icc_set_bw(struct icc_path *path, u32 avg_bw, u32 peak_bw) /* aggregate requests for this node */ aggregate_requests(node); + + trace_icc_set_bw(path, node, i, avg_bw, peak_bw); } ret = apply_constraints(path); @@ -453,6 +458,8 @@ int icc_set_bw(struct icc_path *path, u32 avg_bw, u32 peak_bw) mutex_unlock(&icc_lock); + trace_icc_set_bw_end(path, ret); + return ret; } EXPORT_SYMBOL_GPL(icc_set_bw); diff --git a/drivers/interconnect/trace.h b/drivers/interconnect/trace.h new file mode 100644 index 000000000000..3d668ff566bf --- /dev/null +++ b/drivers/interconnect/trace.h @@ -0,0 +1,88 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Interconnect framework tracepoints + * Copyright (c) 2019, Linaro Ltd. + * Author: Georgi Djakov + */ + +#undef TRACE_SYSTEM +#define TRACE_SYSTEM interconnect + +#if !defined(_TRACE_INTERCONNECT_H) || defined(TRACE_HEADER_MULTI_READ) +#define _TRACE_INTERCONNECT_H + +#include +#include + +TRACE_EVENT(icc_set_bw, + + TP_PROTO(struct icc_path *p, struct icc_node *n, int i, + u32 avg_bw, u32 peak_bw), + + TP_ARGS(p, n, i, avg_bw, peak_bw), + + TP_STRUCT__entry( + __string(path_name, p->name) + __string(dev, dev_name(p->reqs[i].dev)) + __string(node_name, n->name) + __field(u32, avg_bw) + __field(u32, peak_bw) + __field(u32, node_avg_bw) + __field(u32, node_peak_bw) + ), + + TP_fast_assign( + __assign_str(path_name, p->name); + __assign_str(dev, dev_name(p->reqs[i].dev)); + __assign_str(node_name, n->name); + __entry->avg_bw = avg_bw; + __entry->peak_bw = peak_bw; + __entry->node_avg_bw = n->avg_bw; + __entry->node_peak_bw = n->peak_bw; + ), + + TP_printk("path=%s dev=%s node=%s avg_bw=%u peak_bw=%u agg_avg=%u agg_peak=%u", + __get_str(path_name), + __get_str(dev), + __get_str(node_name), + __entry->avg_bw, + __entry->peak_bw, + __entry->node_avg_bw, + __entry->node_peak_bw) +); + +TRACE_EVENT(icc_set_bw_end, + + TP_PROTO(struct icc_path *p, int ret), + + TP_ARGS(p, ret), + + TP_STRUCT__entry( + __string(path_name, p->name) + __string(dev, dev_name(p->reqs[0].dev)) + __field(int, ret) + ), + + TP_fast_assign( + __assign_str(path_name, p->name); + __assign_str(dev, dev_name(p->reqs[0].dev)); + __entry->ret = ret; + ), + + TP_printk("path=%s dev=%s ret=%d", + __get_str(path_name), + __get_str(dev), + __entry->ret) +); + +#endif /* _TRACE_INTERCONNECT_H */ + +/* This part must be outside protection */ + +#undef TRACE_INCLUDE_PATH +#define TRACE_INCLUDE_PATH . + +#undef TRACE_INCLUDE_FILE +#define TRACE_INCLUDE_FILE trace + +#include -- cgit v1.2.3 From 3172e4d276315afa82c12b14c8dd0db526c7aff1 Mon Sep 17 00:00:00 2001 From: Georgi Djakov Date: Thu, 28 Nov 2019 15:48:38 +0200 Subject: interconnect: Add a common standard aggregate function Currently there is one very standard aggregation method that is used by several drivers. Let's add this as a common function, so that drivers could just point to it, instead of copy/pasting code. Suggested-by: Evan Green Reviewed-by: Brian Masney Reviewed-by: Bjorn Andersson Reviewed-by: Evan Green Signed-off-by: Georgi Djakov --- drivers/interconnect/core.c | 10 ++++++++++ include/linux/interconnect-provider.h | 8 ++++++++ 2 files changed, 18 insertions(+) diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c index fbec2e4fdfeb..03625406c69f 100644 --- a/drivers/interconnect/core.c +++ b/drivers/interconnect/core.c @@ -221,6 +221,16 @@ out: return ret; } +int icc_std_aggregate(struct icc_node *node, u32 tag, u32 avg_bw, + u32 peak_bw, u32 *agg_avg, u32 *agg_peak) +{ + *agg_avg += avg_bw; + *agg_peak = max(*agg_peak, peak_bw); + + return 0; +} +EXPORT_SYMBOL_GPL(icc_std_aggregate); + /* of_icc_xlate_onecell() - Translate function using a single index. * @spec: OF phandle args to map into an interconnect node. * @data: private data (pointer to struct icc_onecell_data) diff --git a/include/linux/interconnect-provider.h b/include/linux/interconnect-provider.h index 31440c921216..0c494534b4d3 100644 --- a/include/linux/interconnect-provider.h +++ b/include/linux/interconnect-provider.h @@ -92,6 +92,8 @@ struct icc_node { #if IS_ENABLED(CONFIG_INTERCONNECT) +int icc_std_aggregate(struct icc_node *node, u32 tag, u32 avg_bw, + u32 peak_bw, u32 *agg_avg, u32 *agg_peak); struct icc_node *icc_node_create(int id); void icc_node_destroy(int id); int icc_link_create(struct icc_node *node, const int dst_id); @@ -104,6 +106,12 @@ int icc_provider_del(struct icc_provider *provider); #else +static inline int icc_std_aggregate(struct icc_node *node, u32 tag, u32 avg_bw, + u32 peak_bw, u32 *agg_avg, u32 *agg_peak) +{ + return -ENOTSUPP; +} + static inline struct icc_node *icc_node_create(int id) { return ERR_PTR(-ENOTSUPP); -- cgit v1.2.3 From b92c35e1b9c9211635df9e8fb060c69311871865 Mon Sep 17 00:00:00 2001 From: Georgi Djakov Date: Thu, 28 Nov 2019 15:48:39 +0200 Subject: interconnect: qcom: Use the standard aggregate function Now we have a common function for standard aggregation, so let's use it, instead of duplicating the code. Reviewed-by: Brian Masney Reviewed-by: Bjorn Andersson Reviewed-by: Evan Green Signed-off-by: Georgi Djakov --- drivers/interconnect/qcom/msm8974.c | 15 +++------------ drivers/interconnect/qcom/qcs404.c | 15 +++------------ 2 files changed, 6 insertions(+), 24 deletions(-) diff --git a/drivers/interconnect/qcom/msm8974.c b/drivers/interconnect/qcom/msm8974.c index e669a1f726d2..3a313e11e73d 100644 --- a/drivers/interconnect/qcom/msm8974.c +++ b/drivers/interconnect/qcom/msm8974.c @@ -550,15 +550,6 @@ static struct msm8974_icc_desc msm8974_snoc = { .num_nodes = ARRAY_SIZE(msm8974_snoc_nodes), }; -static int msm8974_icc_aggregate(struct icc_node *node, u32 tag, u32 avg_bw, - u32 peak_bw, u32 *agg_avg, u32 *agg_peak) -{ - *agg_avg += avg_bw; - *agg_peak = max(*agg_peak, peak_bw); - - return 0; -} - static void msm8974_icc_rpm_smd_send(struct device *dev, int rsc_type, char *name, int id, u64 val) { @@ -603,8 +594,8 @@ static int msm8974_icc_set(struct icc_node *src, struct icc_node *dst) qp = to_msm8974_icc_provider(provider); list_for_each_entry(n, &provider->nodes, node_list) - msm8974_icc_aggregate(n, 0, n->avg_bw, n->peak_bw, - &agg_avg, &agg_peak); + provider->aggregate(n, 0, n->avg_bw, n->peak_bw, + &agg_avg, &agg_peak); sum_bw = icc_units_to_bps(agg_avg); max_peak_bw = icc_units_to_bps(agg_peak); @@ -694,7 +685,7 @@ static int msm8974_icc_probe(struct platform_device *pdev) INIT_LIST_HEAD(&provider->nodes); provider->dev = dev; provider->set = msm8974_icc_set; - provider->aggregate = msm8974_icc_aggregate; + provider->aggregate = icc_std_aggregate; provider->xlate = of_icc_xlate_onecell; provider->data = data; diff --git a/drivers/interconnect/qcom/qcs404.c b/drivers/interconnect/qcom/qcs404.c index c8eb1276cce8..d4769a5ea182 100644 --- a/drivers/interconnect/qcom/qcs404.c +++ b/drivers/interconnect/qcom/qcs404.c @@ -327,15 +327,6 @@ static struct qcom_icc_desc qcs404_snoc = { .num_nodes = ARRAY_SIZE(qcs404_snoc_nodes), }; -static int qcom_icc_aggregate(struct icc_node *node, u32 tag, u32 avg_bw, - u32 peak_bw, u32 *agg_avg, u32 *agg_peak) -{ - *agg_avg += avg_bw; - *agg_peak = max(*agg_peak, peak_bw); - - return 0; -} - static int qcom_icc_set(struct icc_node *src, struct icc_node *dst) { struct qcom_icc_provider *qp; @@ -354,8 +345,8 @@ static int qcom_icc_set(struct icc_node *src, struct icc_node *dst) qp = to_qcom_provider(provider); list_for_each_entry(n, &provider->nodes, node_list) - qcom_icc_aggregate(n, 0, n->avg_bw, n->peak_bw, - &agg_avg, &agg_peak); + provider->aggregate(n, 0, n->avg_bw, n->peak_bw, + &agg_avg, &agg_peak); sum_bw = icc_units_to_bps(agg_avg); max_peak_bw = icc_units_to_bps(agg_peak); @@ -456,7 +447,7 @@ static int qnoc_probe(struct platform_device *pdev) INIT_LIST_HEAD(&provider->nodes); provider->dev = dev; provider->set = qcom_icc_set; - provider->aggregate = qcom_icc_aggregate; + provider->aggregate = icc_std_aggregate; provider->xlate = of_icc_xlate_onecell; provider->data = data; -- cgit v1.2.3 From 1a0013c62b33158dcb67a3c11872a03be50711a3 Mon Sep 17 00:00:00 2001 From: Leonard Crestez Date: Tue, 19 Nov 2019 00:34:01 +0200 Subject: interconnect: Add interconnect_graph file to debugfs The interconnect graphs can be difficult to understand and the current "interconnect_summary" file doesn't even display links in any way. Add a new "interconnect_graph" file to debugfs in the graphviz "dot" format which describes interconnect providers, nodes and links. The file is human-readable and can be visualized by piping through graphviz. Example: ssh $TARGET cat /sys/kernel/debug/interconnect/interconnect_graph \ | dot -Tsvg > interconnect_graph.svg Signed-off-by: Leonard Crestez Reviewed-by: Greg Kroah-Hartman Reviewed-by: Bjorn Andersson Signed-off-by: Georgi Djakov --- Documentation/driver-api/interconnect.rst | 22 +++++++++++ drivers/interconnect/core.c | 66 +++++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+) diff --git a/Documentation/driver-api/interconnect.rst b/Documentation/driver-api/interconnect.rst index cdeb5825f314..5ed4f57a6bac 100644 --- a/Documentation/driver-api/interconnect.rst +++ b/Documentation/driver-api/interconnect.rst @@ -91,3 +91,25 @@ Interconnect consumers are the clients which use the interconnect APIs to get paths between endpoints and set their bandwidth/latency/QoS requirements for these interconnect paths. These interfaces are not currently documented. + +Interconnect debugfs interfaces +------------------------------- + +Like several other subsystems interconnect will create some files for debugging +and introspection. Files in debugfs are not considered ABI so application +software shouldn't rely on format details change between kernel versions. + +``/sys/kernel/debug/interconnect/interconnect_summary``: + +Show all interconnect nodes in the system with their aggregated bandwidth +request. Indented under each node show bandwidth requests from each device. + +``/sys/kernel/debug/interconnect/interconnect_graph``: + +Show the interconnect graph in the graphviz dot format. It shows all +interconnect nodes and links in the system and groups together nodes from the +same provider as subgraphs. The format is human-readable and can also be piped +through dot to generate diagrams in many graphical formats:: + + $ cat /sys/kernel/debug/interconnect/interconnect_graph | \ + dot -Tsvg > interconnect_graph.svg diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c index 03625406c69f..63c164264b73 100644 --- a/drivers/interconnect/core.c +++ b/drivers/interconnect/core.c @@ -71,6 +71,70 @@ static int icc_summary_show(struct seq_file *s, void *data) } DEFINE_SHOW_ATTRIBUTE(icc_summary); +static void icc_graph_show_link(struct seq_file *s, int level, + struct icc_node *n, struct icc_node *m) +{ + seq_printf(s, "%s\"%d:%s\" -> \"%d:%s\"\n", + level == 2 ? "\t\t" : "\t", + n->id, n->name, m->id, m->name); +} + +static void icc_graph_show_node(struct seq_file *s, struct icc_node *n) +{ + seq_printf(s, "\t\t\"%d:%s\" [label=\"%d:%s", + n->id, n->name, n->id, n->name); + seq_printf(s, "\n\t\t\t|avg_bw=%ukBps", n->avg_bw); + seq_printf(s, "\n\t\t\t|peak_bw=%ukBps", n->peak_bw); + seq_puts(s, "\"]\n"); +} + +static int icc_graph_show(struct seq_file *s, void *data) +{ + struct icc_provider *provider; + struct icc_node *n; + int cluster_index = 0; + int i; + + seq_puts(s, "digraph {\n\trankdir = LR\n\tnode [shape = record]\n"); + mutex_lock(&icc_lock); + + /* draw providers as cluster subgraphs */ + cluster_index = 0; + list_for_each_entry(provider, &icc_providers, provider_list) { + seq_printf(s, "\tsubgraph cluster_%d {\n", ++cluster_index); + if (provider->dev) + seq_printf(s, "\t\tlabel = \"%s\"\n", + dev_name(provider->dev)); + + /* draw nodes */ + list_for_each_entry(n, &provider->nodes, node_list) + icc_graph_show_node(s, n); + + /* draw internal links */ + list_for_each_entry(n, &provider->nodes, node_list) + for (i = 0; i < n->num_links; ++i) + if (n->provider == n->links[i]->provider) + icc_graph_show_link(s, 2, n, + n->links[i]); + + seq_puts(s, "\t}\n"); + } + + /* draw external links */ + list_for_each_entry(provider, &icc_providers, provider_list) + list_for_each_entry(n, &provider->nodes, node_list) + for (i = 0; i < n->num_links; ++i) + if (n->provider != n->links[i]->provider) + icc_graph_show_link(s, 1, n, + n->links[i]); + + mutex_unlock(&icc_lock); + seq_puts(s, "}"); + + return 0; +} +DEFINE_SHOW_ATTRIBUTE(icc_graph); + static struct icc_node *node_find(const int id) { return idr_find(&icc_idr, id); @@ -827,6 +891,8 @@ static int __init icc_init(void) icc_debugfs_dir = debugfs_create_dir("interconnect", NULL); debugfs_create_file("interconnect_summary", 0444, icc_debugfs_dir, NULL, &icc_summary_fops); + debugfs_create_file("interconnect_graph", 0444, + icc_debugfs_dir, NULL, &icc_graph_fops); return 0; } -- cgit v1.2.3 From 8082c51ac34d05258ac80de47eab1a5d727bb5d2 Mon Sep 17 00:00:00 2001 From: yu kuai Date: Thu, 26 Dec 2019 20:16:38 +0800 Subject: fpga: dfl: fme: remove set but not used variable 'fme' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes gcc '-Wunused-but-set-variable' warning: drivers/fpga/dfl-fme-main.c: In function ‘fme_dev_destroy’: drivers/fpga/dfl-fme-main.c:678:18: warning: variable ‘fme’ set but not used [-Wunused-but-set-variable] It is never used and so can be removed. Acked-by: Wu Hao Signed-off-by: yu kuai Signed-off-by: Moritz Fischer --- drivers/fpga/dfl-fme-main.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/fpga/dfl-fme-main.c b/drivers/fpga/dfl-fme-main.c index 7c930e6b314d..1d4690c99268 100644 --- a/drivers/fpga/dfl-fme-main.c +++ b/drivers/fpga/dfl-fme-main.c @@ -675,10 +675,8 @@ static int fme_dev_init(struct platform_device *pdev) static void fme_dev_destroy(struct platform_device *pdev) { struct dfl_feature_platform_data *pdata = dev_get_platdata(&pdev->dev); - struct dfl_fme *fme; mutex_lock(&pdata->lock); - fme = dfl_fpga_pdata_get_private(pdata); dfl_fpga_pdata_set_private(pdata, NULL); mutex_unlock(&pdata->lock); } -- cgit v1.2.3 From 9bc65970bb037d3197083b205d27855ffd0f14f2 Mon Sep 17 00:00:00 2001 From: yu kuai Date: Thu, 26 Dec 2019 20:15:33 +0800 Subject: fpga: dfl: afu: remove set but not used variable 'afu' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes gcc '-Wunused-but-set-variable' warning: drivers/fpga/dfl-afu-main.c: In function ‘afu_dev_destroy’: drivers/fpga/dfl-afu-main.c:816:18: warning: variable ‘afu’ set but not used [-Wunused-but-set-variable] It is never used, and so can be removed. Acked-by: Wu Hao Signed-off-by: yu kuai Signed-off-by: Moritz Fischer --- drivers/fpga/dfl-afu-main.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/fpga/dfl-afu-main.c b/drivers/fpga/dfl-afu-main.c index e4a34dc7947f..65437b6a6842 100644 --- a/drivers/fpga/dfl-afu-main.c +++ b/drivers/fpga/dfl-afu-main.c @@ -813,10 +813,8 @@ static int afu_dev_init(struct platform_device *pdev) static int afu_dev_destroy(struct platform_device *pdev) { struct dfl_feature_platform_data *pdata = dev_get_platdata(&pdev->dev); - struct dfl_afu *afu; mutex_lock(&pdata->lock); - afu = dfl_fpga_pdata_get_private(pdata); afu_mmio_region_destroy(pdata); afu_dma_region_destroy(pdata); dfl_fpga_pdata_set_private(pdata, NULL); -- cgit v1.2.3 From 1d39387ce8594988a079a1f2617ea137702c7b2f Mon Sep 17 00:00:00 2001 From: Ding Xiang Date: Tue, 10 Sep 2019 17:26:56 +0800 Subject: fpga: remove redundant dev_err message devm_ioremap_resource already contains error message, so remove the redundant dev_err message Signed-off-by: Ding Xiang Signed-off-by: Moritz Fischer --- drivers/fpga/ts73xx-fpga.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/fpga/ts73xx-fpga.c b/drivers/fpga/ts73xx-fpga.c index 9a17fe98c1b0..2888ff000e4d 100644 --- a/drivers/fpga/ts73xx-fpga.c +++ b/drivers/fpga/ts73xx-fpga.c @@ -119,10 +119,8 @@ static int ts73xx_fpga_probe(struct platform_device *pdev) res = platform_get_resource(pdev, IORESOURCE_MEM, 0); priv->io_base = devm_ioremap_resource(kdev, res); - if (IS_ERR(priv->io_base)) { - dev_err(kdev, "unable to remap registers\n"); + if (IS_ERR(priv->io_base)) return PTR_ERR(priv->io_base); - } mgr = devm_fpga_mgr_create(kdev, "TS-73xx FPGA Manager", &ts73xx_fpga_ops, priv); -- cgit v1.2.3 From 2c5127a7fa0369801e623e8c6e58193f86d31d73 Mon Sep 17 00:00:00 2001 From: Georgi Djakov Date: Fri, 20 Dec 2019 18:38:46 +0200 Subject: interconnect: Print the tag in the debugfs summary Now we can have a tag associated with the path. Add this information to the interconnect_summary file, as the current information in debugfs is incomplete. Reviewed-by: Bjorn Andersson Signed-off-by: Georgi Djakov --- drivers/interconnect/core.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c index 63c164264b73..10dde5df9251 100644 --- a/drivers/interconnect/core.c +++ b/drivers/interconnect/core.c @@ -34,7 +34,7 @@ static void icc_summary_show_one(struct seq_file *s, struct icc_node *n) if (!n) return; - seq_printf(s, "%-30s %12u %12u\n", + seq_printf(s, "%-42s %12u %12u\n", n->name, n->avg_bw, n->peak_bw); } @@ -42,8 +42,8 @@ static int icc_summary_show(struct seq_file *s, void *data) { struct icc_provider *provider; - seq_puts(s, " node avg peak\n"); - seq_puts(s, "--------------------------------------------------------\n"); + seq_puts(s, " node tag avg peak\n"); + seq_puts(s, "--------------------------------------------------------------------\n"); mutex_lock(&icc_lock); @@ -58,8 +58,8 @@ static int icc_summary_show(struct seq_file *s, void *data) if (!r->dev) continue; - seq_printf(s, " %-26s %12u %12u\n", - dev_name(r->dev), r->avg_bw, + seq_printf(s, " %-27s %12u %12u %12u\n", + dev_name(r->dev), r->tag, r->avg_bw, r->peak_bw); } } -- cgit v1.2.3 From 7d7899c5297be8cd6095bcddecbbedccb34dd161 Mon Sep 17 00:00:00 2001 From: Georgi Djakov Date: Mon, 6 Jan 2020 19:27:46 +0200 Subject: interconnect: Check for valid path in icc_set_bw() Use IS_ERR() to ensure that the path passed to icc_set_bw() is valid. Reviewed-by: Evan Green Reviewed-by: Bjorn Andersson Signed-off-by: Georgi Djakov --- drivers/interconnect/core.c