Re: [PATCH v15 2/2] i2c: core: support bus regulator controlling in adapter

[Grygorii Strashko <grygorii.strashko@xxxxxx>]

On 19/05/2020 10:27, Bibby Hsieh wrote:
> Although in the most platforms, the bus power of i2c
> are alway on, some platforms disable the i2c bus power
> in order to meet low power request.
>
> We get and enable bulk regulator in i2c adapter device.
>
> Signed-off-by: Bibby Hsieh <bibby.hsieh@xxxxxxxxxxxx>
> ---
>   drivers/i2c/i2c-core-base.c | 84 +++++++++++++++++++++++++++++++++++++
>   include/linux/i2c.h         |  2 +
>   2 files changed, 86 insertions(+)
>
> diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
> index 5cc0b0ec5570..e1cc8d46bc51 100644
> --- a/drivers/i2c/i2c-core-base.c
> +++ b/drivers/i2c/i2c-core-base.c
> @@ -313,12 +313,14 @@ static int i2c_smbus_host_notify_to_irq(const struct i2c_client *client)
>   static int i2c_device_probe(struct device *dev)
>   {
>   	struct i2c_client	*client = i2c_verify_client(dev);
> +	struct i2c_adapter	*adap;
>   	struct i2c_driver	*driver;
>   	int status;
>   
>   	if (!client)
>   		return 0;
>   
> +	adap = client->adapter;
>   	driver = to_i2c_driver(dev->driver);
>   
>   	client->irq = client->init_irq;
> @@ -378,6 +380,12 @@ static int i2c_device_probe(struct device *dev)
>   
>   	dev_dbg(dev, "probe\n");
>   
> +	status = regulator_enable(adap->bus_regulator);
> +	if (status < 0) {
> +		dev_err(&adap->dev, "Failed to enable power regulator\n");
> +		goto err_clear_wakeup_irq;
> +	}
> +
>   	status = of_clk_set_defaults(dev->of_node, false);
>   	if (status < 0)
>   		goto err_clear_wakeup_irq;
> @@ -414,12 +422,14 @@ static int i2c_device_probe(struct device *dev)
>   static int i2c_device_remove(struct device *dev)
>   {
>   	struct i2c_client	*client = i2c_verify_client(dev);
> +	struct i2c_adapter      *adap;
>   	struct i2c_driver	*driver;
>   	int status = 0;
>   
>   	if (!client || !dev->driver)
>   		return 0;
>   
> +	adap = client->adapter;
>   	driver = to_i2c_driver(dev->driver);
>   	if (driver->remove) {
>   		dev_dbg(dev, "remove\n");
> @@ -427,6 +437,8 @@ static int i2c_device_remove(struct device *dev)
>   	}
>   
>   	dev_pm_domain_detach(&client->dev, true);
> +	if (!pm_runtime_status_suspended(&client->dev))
> +		regulator_disable(adap->bus_regulator);

Not sure this check is correct.

i2c_device_probe()
 - regulator_enable - 1

pm_runtime_get()
 - regulator_enable - 2

i2c_device_remove()
 - pm_runtime_status_suspended() flase
   - regulator_disable() - 1 --> still active?

Sorry, I probably missing smth.

>   
>   	dev_pm_clear_wake_irq(&client->dev);
>   	device_init_wakeup(&client->dev, false);
> @@ -438,6 +450,72 @@ static int i2c_device_remove(struct device *dev)
>   	return status;
>   }
>   

[...]

--
Best regards,
grygorii