Re: [PATCH v2] mux: mmio: Add suspend and resume support

[Andrew Davis <afd@ti.com>]

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@bootlin.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,