From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752496AbcGONOI (ORCPT ); Fri, 15 Jul 2016 09:14:08 -0400 Received: from mga09.intel.com ([134.134.136.24]:48067 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751032AbcGONOF (ORCPT ); Fri, 15 Jul 2016 09:14:05 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.28,368,1464678000"; d="asc'?scan'208";a="995899055" From: Felipe Balbi To: Oliver Neukum Cc: Bin Gao , Heikki Krogerus , Greg Kroah-Hartman , linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org, Bin Gao , Chandra Sekhar Anagani Subject: Re: [PATCH 1/2] usb: typec: Add USB Power Delivery sink port support In-Reply-To: <87poqfgpmm.fsf@linux.intel.com> References: <20160715021405.GB128987@worksta> <87zipjgy6n.fsf@linux.intel.com> <1468572250.2195.11.camel@suse.com> <87poqfgpmm.fsf@linux.intel.com> User-Agent: Notmuch/0.22+58~g3a45d29 (https://notmuchmail.org) Emacs/25.0.95.2 (x86_64-pc-linux-gnu) Date: Fri, 15 Jul 2016 16:13:01 +0300 Message-ID: <87d1mfgi3m.fsf@linux.intel.com> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha256; protocol="application/pgp-signature" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable Hi again, Felipe Balbi writes: > Oliver Neukum writes: >> On Fri, 2016-07-15 at 10:25 +0300, Felipe Balbi wrote: >>> > +int pd_sink_queue_msg(struct pd_sink_msg *msg) >>> > +{ >>> > + unsigned long flags; >>> > + struct pd_sink_port *port; >>> > + >>> > + if (msg->port < 0 || msg->port >=3D MAX_NR_SINK_PORTS) { >>> > + pr_err("Invalid port number\n"); >>> > + return -EINVAL; >>> > + } >>> > + >>> > + port =3D sink_ports[msg->port]; >>> > + >>> > + spin_lock_irqsave(&port->rx_lock, flags); >>> > + list_add_tail(&msg->list, &port->rx_list); >>> > + spin_unlock_irqrestore(&port->rx_lock, flags); >>> > + >>> > + queue_work(port->rx_wq, &port->rx_work); >>>=20 >>> can we really queue several messages at a time? It seems unfeasible to >>> me. It's not like we can queue several power request in a role. Why do >>> you need this workqueue? Why don't you process message here, in place? >> >> A reset can come at any time. > > right, but that's not how this is being used. IMHO, rx_work is a > misnomer. If you look at how typec_wcove (patch 2 in this series) uses > it, you'll see that pd_sink_queue_msg() is called to queue a reply to a > message that was *already* received. We can't have two replies, right? > > In any case, this is a minor problem. oh wait, it's not a minor problem. If CPU is busy, this workqueue might take longer than 30ms to get scheduled. This is another problem I just reproduced, even after changing that pr_info() in print_message() to a pr_debug(). Everything worked fine when I called rx_msg_worker() directly, instead of queueing it to the workqueue. =2D-=20 balbi --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJXiOFdAAoJEIaOsuA1yqREhBYP/j67RtJhJotIetW5DFUS4yVh sYEV6SFQDsytJLalqfmZKdiOyV0pz5ZBQQ0x37ySIjv/67bM4ENd32ltyAmUPrGH BJolQa85q30LAW6HsRLp8UeoXZHnLFddHo7IWPYLBs9taJN8kdGRYCFZEfIwdWFb BTz+lrRiKEdtaB1LG1LaP2HEyHPwai8o+3GuYgFBPJQFAga/x39U1NjO3DLnM7Pt CyMEJihAWpLOK9C8pardSW4iiXzXgt42dpFmB3gTyweBABKH3CHe1eSt4l8CHn2X pukQkR7WjKESJB3yxRJhlrOwUVNJxUuXMsTVJUrh2S9J7M0p9D/z83bhalWwJIrP CidtaX+e9nrk/TW9E4vffAm8f55Nirark3mq+Tr1mYeMt3aLPUZXd9eYju/5lKfz /+Ujc3zgYKsjGcALWMqWdNHyM44auKy/pf2D3aZE3Eyfd9KfIe882Un9W4gJeOx/ WxugkEupR36mfF0eFwS04Z7mfkNqQILk3SKgu3Y4UViLeqIyYbywqi8BS/2zJAJy oXkr6P8B4LyLe2fqQLhpOFfOem6wHrrp9wk84q2+3RlvfySpc/ujt2ndokU3j0aB YgBgxqZGwBKEDqY7rBdRh2uW2zSumD+E734v8RKGm6lkSgt1byq3gn0CEamMlL0J wqOjbehVI9ONpzlNAutP =ooZd -----END PGP SIGNATURE----- --=-=-=--