Skip to content

Asoc runtime pm fix v2 #18

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
nandamajay wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Asoc runtime pm fix v2 #18

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9df0b9d
ASoC: codecs: lpass-wsa-macro: Switch to PM clock framework for runti…
Mar 12, 2026
becd825
ASoC: codecs: lpass-va-macro: Switch to PM clock framework for runtim…
Mar 12, 2026
73dad65
pinctrl: qcom: lpass-lpi: Switch to PM clock framework for runtime PM
Apr 2, 2026
b38abe2
pinctrl: qcom: lpass-lpi: Propagate GPIO register access errors
Apr 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
107 changes: 79 additions & 28 deletions drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,17 @@

#include <linux/pinctrl/pinconf-generic.h>
#include <linux/pinctrl/pinconf.h>
#include <linux/pm_runtime.h>
#include <linux/pinctrl/pinmux.h>

#include "../pinctrl-utils.h"

#include "pinctrl-lpass-lpi.h"
#include <linux/pm_clock.h>


#define MAX_NR_GPIO 32
#define GPIO_FUNC 0
#define MAX_LPI_NUM_CLKS 2

struct lpi_pinctrl {
struct device *dev;
Expand All @@ -31,38 +33,55 @@ struct lpi_pinctrl {
struct pinctrl_desc desc;
char __iomem *tlmm_base;
char __iomem *slew_base;
struct clk_bulk_data clks[MAX_LPI_NUM_CLKS];
/* Protects from concurrent register updates */
struct mutex lock;
DECLARE_BITMAP(ever_gpio, MAX_NR_GPIO);
const struct lpi_pinctrl_variant_data *data;
};

static int lpi_gpio_read(struct lpi_pinctrl *state, unsigned int pin,
unsigned int addr)
unsigned int addr, u32 *val)
{
u32 pin_offset;
int ret;

if (state->data->flags & LPI_FLAG_USE_PREDEFINED_PIN_OFFSET)
pin_offset = state->data->groups[pin].pin_offset;
else
pin_offset = LPI_TLMM_REG_OFFSET * pin;

return ioread32(state->tlmm_base + pin_offset + addr);
ret = pm_runtime_resume_and_get(state->dev);
if (ret < 0)
return ret;

*val = ioread32(state->tlmm_base + pin_offset + addr);

pm_runtime_mark_last_busy(state->dev);
pm_runtime_put_autosuspend(state->dev);
Copy link
Copy Markdown

@Srinivas-Kandagatla Srinivas-Kandagatla Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you need a put?

Copy link
Copy Markdown
Owner Author

@nandamajay nandamajay Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, We need the put_autosuspend() to balance pm_runtime_resume_and_get(). Otherwise the runtime PM reference is leaked and the device will remain permanently active, preventing autosuspend and runtime suspend callbacks from running.


return 0;
}

static int lpi_gpio_write(struct lpi_pinctrl *state, unsigned int pin,
unsigned int addr, unsigned int val)
{
u32 pin_offset;
int ret;

if (state->data->flags & LPI_FLAG_USE_PREDEFINED_PIN_OFFSET)
pin_offset = state->data->groups[pin].pin_offset;
else
pin_offset = LPI_TLMM_REG_OFFSET * pin;

ret = pm_runtime_resume_and_get(state->dev);
if (ret < 0)
return ret;

iowrite32(val, state->tlmm_base + pin_offset + addr);

pm_runtime_mark_last_busy(state->dev);
pm_runtime_put_autosuspend(state->dev);

return 0;
}

Expand Down Expand Up @@ -107,7 +126,9 @@ static int lpi_gpio_set_mux(struct pinctrl_dev *pctldev, unsigned int function,
{
struct lpi_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
const struct lpi_pingroup *g = &pctrl->data->groups[group];
u32 val;
u32 val, io_val;
int ret;

int i, pin = g->pin;

for (i = 0; i < g->nfuncs; i++) {
Expand All @@ -119,7 +140,9 @@ static int lpi_gpio_set_mux(struct pinctrl_dev *pctldev, unsigned int function,
return -EINVAL;

mutex_lock(&pctrl->lock);
val = lpi_gpio_read(pctrl, pin, LPI_GPIO_CFG_REG);
ret = lpi_gpio_read(pctrl, pin, LPI_GPIO_CFG_REG, &val);
if (ret)
goto out_unlock;

/*
* If this is the first time muxing to GPIO and the direction is
Expand All @@ -129,24 +152,28 @@ static int lpi_gpio_set_mux(struct pinctrl_dev *pctldev, unsigned int function,
*/
if (i == GPIO_FUNC && (val & LPI_GPIO_OE_MASK) &&
!test_and_set_bit(group, pctrl->ever_gpio)) {
u32 io_val = lpi_gpio_read(pctrl, group, LPI_GPIO_VALUE_REG);
ret = lpi_gpio_read(pctrl, group, LPI_GPIO_VALUE_REG, &io_val);
if (ret)
goto out_unlock;

if (io_val & LPI_GPIO_VALUE_IN_MASK) {
if (!(io_val & LPI_GPIO_VALUE_OUT_MASK))
lpi_gpio_write(pctrl, group, LPI_GPIO_VALUE_REG,
ret = lpi_gpio_write(pctrl, group, LPI_GPIO_VALUE_REG,
io_val | LPI_GPIO_VALUE_OUT_MASK);
} else {
if (io_val & LPI_GPIO_VALUE_OUT_MASK)
lpi_gpio_write(pctrl, group, LPI_GPIO_VALUE_REG,
ret = lpi_gpio_write(pctrl, group, LPI_GPIO_VALUE_REG,
io_val & ~LPI_GPIO_VALUE_OUT_MASK);
}
}

u32p_replace_bits(&val, i, LPI_GPIO_FUNCTION_MASK);
lpi_gpio_write(pctrl, pin, LPI_GPIO_CFG_REG, val);
ret = lpi_gpio_write(pctrl, pin, LPI_GPIO_CFG_REG, val);

out_unlock:
mutex_unlock(&pctrl->lock);

return 0;
return ret;
}

static const struct pinmux_ops lpi_gpio_pinmux_ops = {
Expand All @@ -165,8 +192,11 @@ static int lpi_config_get(struct pinctrl_dev *pctldev,
int is_out;
int pull;
u32 ctl_reg;
int ret;

ctl_reg = lpi_gpio_read(state, pin, LPI_GPIO_CFG_REG);
ret = lpi_gpio_read(state, pin, LPI_GPIO_CFG_REG, &ctl_reg);
if (ret)
return ret;
is_out = ctl_reg & LPI_GPIO_OE_MASK;
pull = FIELD_GET(LPI_GPIO_PULL_MASK, ctl_reg);

Expand Down Expand Up @@ -293,17 +323,22 @@ static int lpi_config_set(struct pinctrl_dev *pctldev, unsigned int group,
}

mutex_lock(&pctrl->lock);
val = lpi_gpio_read(pctrl, group, LPI_GPIO_CFG_REG);
ret = lpi_gpio_read(pctrl, group, LPI_GPIO_CFG_REG, &val);
if (ret) {
mutex_unlock(&pctrl->lock);
goto out_unlock;
}

u32p_replace_bits(&val, pullup, LPI_GPIO_PULL_MASK);
u32p_replace_bits(&val, LPI_GPIO_DS_TO_VAL(strength),
LPI_GPIO_OUT_STRENGTH_MASK);
u32p_replace_bits(&val, output_enabled, LPI_GPIO_OE_MASK);

lpi_gpio_write(pctrl, group, LPI_GPIO_CFG_REG, val);
mutex_unlock(&pctrl->lock);
ret = lpi_gpio_write(pctrl, group, LPI_GPIO_CFG_REG, val);

return 0;
out_unlock:
mutex_unlock(&pctrl->lock);
return ret;
}

static const struct pinconf_ops lpi_gpio_pinconf_ops = {
Expand Down Expand Up @@ -352,9 +387,13 @@ static int lpi_gpio_direction_output(struct gpio_chip *chip,
static int lpi_gpio_get(struct gpio_chip *chip, unsigned int pin)
{
struct lpi_pinctrl *state = gpiochip_get_data(chip);
u32 val;
int ret;

return lpi_gpio_read(state, pin, LPI_GPIO_VALUE_REG) &
LPI_GPIO_VALUE_IN_MASK;
ret = lpi_gpio_read(state, pin, LPI_GPIO_VALUE_REG, &val);
if (ret)
return ret;
return val & LPI_GPIO_VALUE_IN_MASK;
Copy link
Copy Markdown

@Srinivas-Kandagatla Srinivas-Kandagatla Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated changes in this patch, please separate such changes.

Copy link
Copy Markdown
Owner Author

@nandamajay nandamajay Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK, kept as seperate patch

}

static int lpi_gpio_set(struct gpio_chip *chip, unsigned int pin, int value)
Expand Down Expand Up @@ -387,6 +426,7 @@ static void lpi_gpio_dbg_show_one(struct seq_file *s,
int drive;
int pull;
u32 ctl_reg;
int ret;

static const char * const pulls[] = {
"no pull",
Expand All @@ -397,7 +437,11 @@ static void lpi_gpio_dbg_show_one(struct seq_file *s,

pctldev = pctldev ? : state->ctrl;
pindesc = pctldev->desc->pins[offset];
ctl_reg = lpi_gpio_read(state, offset, LPI_GPIO_CFG_REG);
ret = lpi_gpio_read(state, offset, LPI_GPIO_CFG_REG, &ctl_reg);
if (ret) {
seq_printf(s, " %-8s: <read error %d>", pindesc.name, ret);
return;
Copy link
Copy Markdown

@Srinivas-Kandagatla Srinivas-Kandagatla Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not belong to this patch?

Copy link
Copy Markdown
Owner Author

@nandamajay nandamajay Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK, Made into a seperate patch

}
is_out = ctl_reg & LPI_GPIO_OE_MASK;

func = FIELD_GET(LPI_GPIO_FUNCTION_MASK, ctl_reg);
Expand Down Expand Up @@ -480,9 +524,6 @@ int lpi_pinctrl_probe(struct platform_device *pdev)
pctrl->data = data;
pctrl->dev = &pdev->dev;

pctrl->clks[0].id = "core";
pctrl->clks[1].id = "audio";

pctrl->tlmm_base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(pctrl->tlmm_base))
return dev_err_probe(dev, PTR_ERR(pctrl->tlmm_base),
Expand All @@ -495,13 +536,22 @@ int lpi_pinctrl_probe(struct platform_device *pdev)
"Slew resource not provided\n");
}

ret = devm_clk_bulk_get_optional(dev, MAX_LPI_NUM_CLKS, pctrl->clks);

Copy link
Copy Markdown

@Srinivas-Kandagatla Srinivas-Kandagatla Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we addding this here?

Copy link
Copy Markdown
Owner Author

@nandamajay nandamajay Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Acknowledged

ret = devm_pm_clk_create(dev);
if (ret)
return ret;

ret = clk_bulk_prepare_enable(MAX_LPI_NUM_CLKS, pctrl->clks);
if (ret)
return dev_err_probe(dev, ret, "Can't enable clocks\n");
ret = pm_clk_add(dev, "core");
if (ret < 0)
dev_err(dev, "failed to add PM clock 'core': %d\n", ret);

ret = pm_clk_add(dev, "audio");
if (ret < 0)
dev_err(dev, "failed to add PM clock 'audio': %d\n", ret);

pm_runtime_set_autosuspend_delay(dev, 100);
pm_runtime_use_autosuspend(dev);
pm_runtime_enable(dev);

pctrl->desc.pctlops = &lpi_gpio_pinctrl_ops;
pctrl->desc.pmxops = &lpi_gpio_pinmux_ops;
Expand Down Expand Up @@ -539,8 +589,8 @@ int lpi_pinctrl_probe(struct platform_device *pdev)
return 0;

err_pinctrl:
pm_runtime_disable(dev);
mutex_destroy(&pctrl->lock);
clk_bulk_disable_unprepare(MAX_LPI_NUM_CLKS, pctrl->clks);

return ret;
}
Expand All @@ -551,8 +601,9 @@ void lpi_pinctrl_remove(struct platform_device *pdev)
struct lpi_pinctrl *pctrl = platform_get_drvdata(pdev);
int i;

pm_runtime_disable(pctrl->dev);

mutex_destroy(&pctrl->lock);
clk_bulk_disable_unprepare(MAX_LPI_NUM_CLKS, pctrl->clks);

for (i = 0; i < pctrl->data->npins; i++)
pinctrl_generic_remove_group(pctrl->ctrl, i);
Expand Down
Loading