Re: [PATCH] i2c: allow specifying separate wakeup interrupt in device tree

[Wolfram Sang]

On Thu, Jul 30, 2015 at 01:14:31PM -0700, Dmitry Torokhov wrote:
> Instead of having each i2c driver individually parse device tree data in
> case it or platform supports separate wakeup interrupt, and handle
> enabling and disabling wakeup interrupts in their power management
> routines, let's have i2c core do that for us.
> 
> Platforms wishing to specify separate wakeup interrupt for the device
> should use named interrupt syntax in their DTSes:
> 
> 	interrupt-parent = <&intc1>;
> 	interrupts = <5 0>, <6 0>;
> 	interrupt-names = "irq", "wakeup";
> 
> This patch is inspired by work done by Vignesh R <vigneshr@xxxxxx> for
> pixcir_i2c_ts driver.
> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>

I think it is a useful addition. Can someone add a paragraph describing
this handling on top of the new generic i2c binding docs?

http://patchwork.ozlabs.org/patch/505368/

> @@ -659,20 +662,47 @@ static int i2c_device_probe(struct device *dev)
>  	if (!device_can_wakeup(&client->dev))
>  		device_init_wakeup(&client->dev,
>  					client->flags & I2C_CLIENT_WAKE);

I was about to ask if we couldn't combine this and the later if-blocks
with an if-else combination. But now I stumble over the above block in
general: If the device cannot cause wake ups, then we might initialize
it as a wakeup-device depending on client->flags??

> +
> +	if (device_can_wakeup(&client->dev)) {
> +		int wakeirq = -ENOENT;
> +
> +		if (dev->of_node) {
> +			wakeirq = of_irq_get_byname(dev->of_node, "wakeup");
> +			if (wakeirq == -EPROBE_DEFER)
> +				return wakeirq;
> +		}
> +
> +		if (wakeirq > 0 && wakeirq != client->irq)
> +			status = dev_pm_set_dedicated_wake_irq(dev, wakeirq);
> +		else if (client->irq > 0)
> +			status = dev_pm_set_wake_irq(dev, wakeirq);
> +		else
> +			status = 0;
> +
> +		if (status)
> +			dev_warn(&client->dev, "failed to set up wakeup irq");
> +	}
> +
>  	dev_dbg(dev, "probe\n");
>  
>  	status = of_clk_set_defaults(dev->of_node, false);
>  	if (status < 0)
> -		return status;
> +		goto err_clear_wakeup_irq;
>  
>  	status = dev_pm_domain_attach(&client->dev, true);
>  	if (status != -EPROBE_DEFER) {
>  		status = driver->probe(client, i2c_match_id(driver->id_table,
>  					client));
>  		if (status)
> -			dev_pm_domain_detach(&client->dev, true);
> +			goto err_detach_pm_domain;
>  	}
>  
> +	return 0;
> +
> +err_detach_pm_domain:
> +	dev_pm_domain_detach(&client->dev, true);
> +err_clear_wakeup_irq:
> +	dev_pm_clear_wake_irq(&client->dev);
>  	return status;
>  }
>  
> @@ -692,6 +722,10 @@ static int i2c_device_remove(struct device *dev)
>  	}
>  
>  	dev_pm_domain_detach(&client->dev, true);
> +
> +	dev_pm_clear_wake_irq(&client->dev);
> +	device_init_wakeup(&client->dev, 0);
> +
>  	return status;
>  }
>  
> -- 
> 2.5.0.rc2.392.g76e840b
> 
> 
> -- 
> Dmitry

Attachment: signature.asc
Description: Digital signature