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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20150809152255.GA2367@katana>
Date:	Sun, 9 Aug 2015 17:22:55 +0200
From:	Wolfram Sang <wsa@...-dreams.de>
To:	Dmitry Torokhov <dmitry.torokhov@...il.com>
Cc:	Mika Westerberg <mika.westerberg@...ux.intel.com>,
	"Rafael J. Wysocki" <rafael.j.wysocki@...el.com>,
	Ulf Hansson <ulf.hansson@...aro.org>,
	Vignesh R <vigneshr@...com>, Tony Lindgren <tony@...mide.com>,
	Rob Herring <robh+dt@...nel.org>,
	Mark Rutland <mark.rutland@....com>, linux-i2c@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] i2c: allow specifying separate wakeup interrupt in
 device tree

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@...com> for
> pixcir_i2c_ts driver.
> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@...il.com>

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

Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)