[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <d7e9d9f8-ac81-4cbb-897c-585741ca00c9@ti.com>
Date: Wed, 4 Dec 2024 07:53:30 -0600
From: Andrew Davis <afd@...com>
To: Thomas Richard <thomas.richard@...tlin.com>, Peter Rosin <peda@...ntia.se>
CC: <linux-kernel@...r.kernel.org>, <gregory.clement@...tlin.com>,
<theo.lebrun@...tlin.com>, <thomas.petazzoni@...tlin.com>,
<u-kumar1@...com>
Subject: Re: [PATCH v2] mux: mmio: Add suspend and resume support
On 9/11/24 3:53 AM, Thomas Richard wrote:
> The status of each mux is read during suspend and stored in the private
> memory of the mux_chip.
> Then the state is restored during the resume.
>
> Signed-off-by: Thomas Richard <thomas.richard@...tlin.com>
> ---
> In this second version, as discussed with Peter, everything is done in the
> mmio-mux driver.
> A mux_mmio_set() function was added, and used during suspend stage to get
> the status of the of the muxes.
> This status is stored in the private memory of the mux_chip.
> ---
> Changes in v2:
> - Remove all modifications done in the mux subsystem
> - Add a mux_mmio_set()
> - Read the status of muxes during suspend and store in the private memory
> of the mux_chip.
Do you need this private memory? Since this is already using regmap, why
not use the regmap cache, then you can restore all the values on resume
with a simple call to regcache_sync().
Andrew
> - Use this status to restore muxes during resume.
> - Link to v1: https://lore.kernel.org/r/20240613-mux-mmio-resume-support-v1-0-4525bf56024a@bootlin.com
> ---
> drivers/mux/mmio.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 73 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/mux/mmio.c b/drivers/mux/mmio.c
> index 30a952c34365..30b84382637f 100644
> --- a/drivers/mux/mmio.c
> +++ b/drivers/mux/mmio.c
> @@ -15,11 +15,25 @@
> #include <linux/property.h>
> #include <linux/regmap.h>
>
> +struct mux_mmio {
> + struct regmap_field **fields;
> + unsigned int *hardware_states;
> +};
> +
> +static int mux_mmio_get(struct mux_control *mux, int *state)
> +{
> + struct mux_mmio *mux_mmio = mux_chip_priv(mux->chip);
> + unsigned int index = mux_control_get_index(mux);
> +
> + return regmap_field_read(mux_mmio->fields[index], state);
> +}
> +
> static int mux_mmio_set(struct mux_control *mux, int state)
> {
> - struct regmap_field **fields = mux_chip_priv(mux->chip);
> + struct mux_mmio *mux_mmio = mux_chip_priv(mux->chip);
> + unsigned int index = mux_control_get_index(mux);
>
> - return regmap_field_write(fields[mux_control_get_index(mux)], state);
> + return regmap_field_write(mux_mmio->fields[index], state);
> }
>
> static const struct mux_control_ops mux_mmio_ops = {
> @@ -37,8 +51,8 @@ static int mux_mmio_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> struct device_node *np = dev->of_node;
> - struct regmap_field **fields;
> struct mux_chip *mux_chip;
> + struct mux_mmio *mux_mmio;
> struct regmap *regmap;
> int num_fields;
> int ret;
> @@ -69,12 +83,20 @@ static int mux_mmio_probe(struct platform_device *pdev)
> }
> num_fields = ret / 2;
>
> - mux_chip = devm_mux_chip_alloc(dev, num_fields, num_fields *
> - sizeof(*fields));
> + mux_chip = devm_mux_chip_alloc(dev, num_fields, sizeof(struct mux_mmio));
> if (IS_ERR(mux_chip))
> return PTR_ERR(mux_chip);
>
> - fields = mux_chip_priv(mux_chip);
> + mux_mmio = mux_chip_priv(mux_chip);
> +
> + mux_mmio->fields = devm_kmalloc(dev, num_fields * sizeof(*mux_mmio->fields), GFP_KERNEL);
> + if (IS_ERR(mux_mmio->fields))
> + return PTR_ERR(mux_mmio->fields);
> +
> + mux_mmio->hardware_states = devm_kmalloc(dev, num_fields *
> + sizeof(*mux_mmio->hardware_states), GFP_KERNEL);
> + if (IS_ERR(mux_mmio->hardware_states))
> + return PTR_ERR(mux_mmio->hardware_states);
>
> for (i = 0; i < num_fields; i++) {
> struct mux_control *mux = &mux_chip->mux[i];
> @@ -104,9 +126,9 @@ static int mux_mmio_probe(struct platform_device *pdev)
> return -EINVAL;
> }
>
> - fields[i] = devm_regmap_field_alloc(dev, regmap, field);
> - if (IS_ERR(fields[i])) {
> - ret = PTR_ERR(fields[i]);
> + mux_mmio->fields[i] = devm_regmap_field_alloc(dev, regmap, field);
> + if (IS_ERR(mux_mmio->fields[i])) {
> + ret = PTR_ERR(mux_mmio->fields[i]);
> dev_err(dev, "bitfield %d: failed allocate: %d\n",
> i, ret);
> return ret;
> @@ -130,13 +152,55 @@ static int mux_mmio_probe(struct platform_device *pdev)
>
> mux_chip->ops = &mux_mmio_ops;
>
> + dev_set_drvdata(dev, mux_chip);
> +
> return devm_mux_chip_register(dev, mux_chip);
> }
>
> +static int mux_mmio_suspend_noirq(struct device *dev)
> +{
> + struct mux_chip *mux_chip = dev_get_drvdata(dev);
> + struct mux_mmio *mux_mmio = mux_chip_priv(mux_chip);
> + unsigned int state;
> + int ret, i;
> +
> + for (i = 0; i < mux_chip->controllers; i++) {
> + ret = mux_mmio_get(&mux_chip->mux[i], &state);
> + if (ret) {
> + dev_err(dev, "control %u: error saving mux: %d\n", i, ret);
> + return ret;
> + }
> +
> + mux_mmio->hardware_states[i] = state;
> + }
> +
> + return 0;
> +}
> +
> +static int mux_mmio_resume_noirq(struct device *dev)
> +{
> + struct mux_chip *mux_chip = dev_get_drvdata(dev);
> + struct mux_mmio *mux_mmio = mux_chip_priv(mux_chip);
> + int ret, i;
> +
> + for (i = 0; i < mux_chip->controllers; i++) {
> + ret = mux_mmio_set(&mux_chip->mux[i], mux_mmio->hardware_states[i]);
> + if (ret) {
> + dev_err(dev, "control %u: error restoring mux: %d\n", i, ret);
> + return ret;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static DEFINE_NOIRQ_DEV_PM_OPS(mux_mmio_pm_ops, mux_mmio_suspend_noirq, mux_mmio_resume_noirq);
> +
> static struct platform_driver mux_mmio_driver = {
> .driver = {
> .name = "mmio-mux",
> .of_match_table = mux_mmio_dt_ids,
> + .pm = pm_sleep_ptr(&mux_mmio_pm_ops),
> },
> .probe = mux_mmio_probe,
> };
>
> ---
> base-commit: caf614bf68351c7e9e38dd37e07539417c757813
> change-id: 20240613-mux-mmio-resume-support-4f3b2a34a32a
>
> Best regards,
Powered by blists - more mailing lists