From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heikki Krogerus Subject: Re: [PATCH v4 4/7] typec: tcpm: Add core support for sink side PPS Date: Tue, 6 Feb 2018 16:53:57 +0200 Message-ID: <20180206145357.GA25136@kuha.fi.intel.com> References: <26195a73c6f2f379b47dd39f23db9c2feec20371.1514904982.git.Adam.Thomson.Opensource@diasemi.com> <20180130124653.GD14922@kuha.fi.intel.com> <2E89032DDAA8B9408CB92943514A0337014C1C1538@SW-EX-MBX01.diasemi.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Received: from mga07.intel.com ([134.134.136.100]:23896 "EHLO mga07.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752116AbeBFOyD (ORCPT ); Tue, 6 Feb 2018 09:54:03 -0500 Content-Disposition: inline In-Reply-To: <2E89032DDAA8B9408CB92943514A0337014C1C1538@SW-EX-MBX01.diasemi.com> Sender: linux-pm-owner@vger.kernel.org List-Id: linux-pm@vger.kernel.org To: Adam Thomson Cc: Guenter Roeck , Greg Kroah-Hartman , Sebastian Reichel , Hans de Goede , Yueyao Zhu , Rui Miguel Silva , "linux-usb@vger.kernel.org" , "linux-pm@vger.kernel.org" , "linux-kernel@vger.kernel.org" , Support Opensource On Tue, Feb 06, 2018 at 02:33:08PM +0000, Adam Thomson wrote: > On 30 January 2018 12:47, Heikki Krogerus wrote: > > > > +static int tcpm_pps_set_op_curr(struct tcpm_port *port, u16 op_curr) > > > +{ > > > + unsigned int target_mw; > > > + int ret = 0; > > > + > > > + mutex_lock(&port->swap_lock); > > > + mutex_lock(&port->lock); > > > + > > > + if (!port->pps_data.active) { > > > + ret = -EOPNOTSUPP; > > > + goto port_unlock; > > > + } > > > + > > > + if (port->state != SNK_READY) { > > > + ret = -EAGAIN; > > > + goto port_unlock; > > > + } > > > + > > > + if (op_curr > port->pps_data.max_curr) { > > > + ret = -EINVAL; > > > + goto port_unlock; > > > + } > > > + > > > + target_mw = (op_curr * port->pps_data.out_volt) / 1000; > > > + if (target_mw < port->operating_snk_mw) { > > > + ret = -EINVAL; > > > + goto port_unlock; > > > + } > > > + > > > + reinit_completion(&port->pps_complete); > > > + port->pps_data.op_curr = op_curr; > > > + port->pps_status = 0; > > > + port->pps_pending = true; > > > + tcpm_set_state(port, SNK_NEGOTIATE_PPS_CAPABILITIES, 0); > > > > Why not just take the swap_lock here.. > > I believe this would result in deadlock. All of the existing uses of swap_lock > acquire it first before the port->lock is then acquired (and vice-versa for > unlock). We don't want the power role to change during this procedure, so we > hold the swap_lock for the whole process. Have a look at tcpm_dr_set() and > tcpm_pr_set() as examples of existing usage. OK. Then I'm fine with this patch as well. FWIW: Acked-by: Heikki Krogerus -- heikki From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Subject: [v4,4/7] typec: tcpm: Add core support for sink side PPS From: Heikki Krogerus Message-Id: <20180206145357.GA25136@kuha.fi.intel.com> Date: Tue, 6 Feb 2018 16:53:57 +0200 To: Adam Thomson Cc: Guenter Roeck , Greg Kroah-Hartman , Sebastian Reichel , Hans de Goede , Yueyao Zhu , Rui Miguel Silva , "linux-usb@vger.kernel.org" , "linux-pm@vger.kernel.org" , "linux-kernel@vger.kernel.org" , Support Opensource List-ID: T24gVHVlLCBGZWIgMDYsIDIwMTggYXQgMDI6MzM6MDhQTSArMDAwMCwgQWRhbSBUaG9tc29uIHdy b3RlOgo+IE9uIDMwIEphbnVhcnkgMjAxOCAxMjo0NywgSGVpa2tpIEtyb2dlcnVzIHdyb3RlOgo+ IAo+ID4gPiArc3RhdGljIGludCB0Y3BtX3Bwc19zZXRfb3BfY3VycihzdHJ1Y3QgdGNwbV9wb3J0 ICpwb3J0LCB1MTYgb3BfY3VycikKPiA+ID4gK3sKPiA+ID4gKwl1bnNpZ25lZCBpbnQgdGFyZ2V0 X213Owo+ID4gPiArCWludCByZXQgPSAwOwo+ID4gPiArCj4gPiA+ICsJbXV0ZXhfbG9jaygmcG9y dC0+c3dhcF9sb2NrKTsKPiA+ID4gKwltdXRleF9sb2NrKCZwb3J0LT5sb2NrKTsKPiA+ID4gKwo+ ID4gPiArCWlmICghcG9ydC0+cHBzX2RhdGEuYWN0aXZlKSB7Cj4gPiA+ICsJCXJldCA9IC1FT1BO T1RTVVBQOwo+ID4gPiArCQlnb3RvIHBvcnRfdW5sb2NrOwo+ID4gPiArCX0KPiA+ID4gKwo+ID4g PiArCWlmIChwb3J0LT5zdGF0ZSAhPSBTTktfUkVBRFkpIHsKPiA+ID4gKwkJcmV0ID0gLUVBR0FJ TjsKPiA+ID4gKwkJZ290byBwb3J0X3VubG9jazsKPiA+ID4gKwl9Cj4gPiA+ICsKPiA+ID4gKwlp ZiAob3BfY3VyciA+IHBvcnQtPnBwc19kYXRhLm1heF9jdXJyKSB7Cj4gPiA+ICsJCXJldCA9IC1F SU5WQUw7Cj4gPiA+ICsJCWdvdG8gcG9ydF91bmxvY2s7Cj4gPiA+ICsJfQo+ID4gPiArCj4gPiA+ ICsJdGFyZ2V0X213ID0gKG9wX2N1cnIgKiBwb3J0LT5wcHNfZGF0YS5vdXRfdm9sdCkgLyAxMDAw Owo+ID4gPiArCWlmICh0YXJnZXRfbXcgPCBwb3J0LT5vcGVyYXRpbmdfc25rX213KSB7Cj4gPiA+ ICsJCXJldCA9IC1FSU5WQUw7Cj4gPiA+ICsJCWdvdG8gcG9ydF91bmxvY2s7Cj4gPiA+ICsJfQo+ ID4gPiArCj4gPiA+ICsJcmVpbml0X2NvbXBsZXRpb24oJnBvcnQtPnBwc19jb21wbGV0ZSk7Cj4g PiA+ICsJcG9ydC0+cHBzX2RhdGEub3BfY3VyciA9IG9wX2N1cnI7Cj4gPiA+ICsJcG9ydC0+cHBz X3N0YXR1cyA9IDA7Cj4gPiA+ICsJcG9ydC0+cHBzX3BlbmRpbmcgPSB0cnVlOwo+ID4gPiArCXRj cG1fc2V0X3N0YXRlKHBvcnQsIFNOS19ORUdPVElBVEVfUFBTX0NBUEFCSUxJVElFUywgMCk7Cj4g Pgo+ID4gV2h5IG5vdCBqdXN0IHRha2UgdGhlIHN3YXBfbG9jayBoZXJlLi4KPiAKPiBJIGJlbGll dmUgdGhpcyB3b3VsZCByZXN1bHQgaW4gZGVhZGxvY2suIEFsbCBvZiB0aGUgZXhpc3RpbmcgdXNl cyBvZiBzd2FwX2xvY2sKPiBhY3F1aXJlIGl0IGZpcnN0IGJlZm9yZSB0aGUgcG9ydC0+bG9jayBp cyB0aGVuIGFjcXVpcmVkIChhbmQgdmljZS12ZXJzYSBmb3IKPiB1bmxvY2spLiBXZSBkb24ndCB3 YW50IHRoZSBwb3dlciByb2xlIHRvIGNoYW5nZSBkdXJpbmcgdGhpcyBwcm9jZWR1cmUsIHNvIHdl Cj4gaG9sZCB0aGUgc3dhcF9sb2NrIGZvciB0aGUgd2hvbGUgcHJvY2Vzcy4gSGF2ZSBhIGxvb2sg YXQgdGNwbV9kcl9zZXQoKSBhbmQKPiB0Y3BtX3ByX3NldCgpIGFzIGV4YW1wbGVzIG9mIGV4aXN0 aW5nIHVzYWdlLgoKT0suIFRoZW4gSSdtIGZpbmUgd2l0aCB0aGlzIHBhdGNoIGFzIHdlbGwuIEZX SVc6CgpBY2tlZC1ieTogSGVpa2tpIEtyb2dlcnVzIDxoZWlra2kua3JvZ2VydXNAbGludXguaW50 ZWwuY29tPgo=