From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757346AbcG0R2C (ORCPT ); Wed, 27 Jul 2016 13:28:02 -0400 Received: from mga11.intel.com ([192.55.52.93]:21064 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753276AbcG0R2A (ORCPT ); Wed, 27 Jul 2016 13:28:00 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.28,430,1464678000"; d="scan'208";a="1003652747" Date: Wed, 27 Jul 2016 10:32:27 -0700 From: Bin Gao To: Felipe Balbi Cc: Heikki Krogerus , Greg Kroah-Hartman , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, Bin Gao , Chandra Sekhar Anagani , Pranav Tipnis Subject: Re: [PATCH v2 1/2] usb: typec: Add USB Power Delivery sink port support Message-ID: <20160727173227.GB169011@worksta> References: <20160726183722.GE211765@worksta> <87wpk7fqh4.fsf@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87wpk7fqh4.fsf@linux.intel.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Jul 27, 2016 at 11:13:43AM +0300, Felipe Balbi wrote: > > Hi, > > Bin Gao writes: > > This patch implements a simple USB Power Delivery sink port state machine. > > It assumes the hardware only handles PD packet transmitting and receiving > > over the CC line of the USB Type-C connector. The state transition is > > completely controlled by software. This patch only implement the sink port > > function and it doesn't support source port and port swap yet. > > > > This patch depends on these two patches: > > https://lkml.org/lkml/2016/6/29/349 > > https://lkml.org/lkml/2016/6/29/350 > > > > Signed-off-by: Bin Gao > > Changes in v2: > > - Removed work queue so messages are directly handled in phy driver's interrupt context > > - used pr_debug instead of pr_info for message dump > > - Converted PD driver to tristate and typec driver is independent of it > > this should be after the tearline (---) below. We don't want this in > changelog ;-) > > > +static void handle_source_cap(struct pd_sink_port *port, u8 msg_revision, > > + u8 nr_objs, u8 *buf) > > +{ > > + int i; > > + u32 *obj; > > + u8 type; > > + struct pd_source_cap *cap = port->source_caps; > > + > > + /* > > + * The PD spec revision included in SOURCE_CAPABILITY message is the > > + * highest revision that the Source supports. > > + */ > > + port->pd_rev = msg_revision; > > + > > + /* > > + * First we need to save all PDOs - they may be used in the future. > > + * USB PD spec says we must use PDOs in the most recent > > + * SOURCE_CAPABILITY message. Here we replace old PDOs with new ones. > > + */ > > + port->nr_source_caps = 0; > > + for (i = 0; i < nr_objs; i++) { > > + obj = (u32 *)(buf + i * PD_OBJ_SIZE); > > + type = (*obj >> SOURCE_CAP_TYPE_BIT) & SOURCE_CAP_TYPE_MASK; > > + switch (type) { > > + case PS_TYPE_FIXED: > > + cap->ps_type = PS_TYPE_FIXED; > > + cap->fixed = *(struct pd_pdo_src_fixed *)obj; > > + break; > > + case PS_TYPE_VARIABLE: > > + cap->ps_type = PS_TYPE_VARIABLE; > > + cap->variable = *(struct pd_pdo_variable *)obj; > > + break; > > + case PS_TYPE_BATTERY: > > + cap->ps_type = PS_TYPE_BATTERY; > > + cap->battery = *(struct pd_pdo_battery *)obj; > > + break; > > + default: /* shouldn't come here */ > > + pr_err("Invalid Source Capability type: %u.\n", type); > > + continue; > > + } > > + port->nr_source_caps++; > > + cap++; > > + } > > + > > + if (port->nr_source_caps == 0) { > > + pr_err("There is no valid PDOs in SOURCE_CAPABILITY message\n"); > > + return; > > + } > > + > > + /* If a contract is not established, we need send a REQUEST message */ > > + if (port->state == PD_SINK_STATE_WAIT_FOR_SRC_CAP) { > > this is wrong. Read the fluxchart in figure 8-42. Source can decide to > send another Source Capability before receiving our Good CRC and we need > to work with that. This state check is, at a minimum, wrong. I'd > actually just go ahead and remove it. > > > + if (!send_request(port)) > > + port->state = PD_SINK_STATE_REQUEST_SENT; > > + } > > +} > > -- > balbi I'll fix these in next revision. Thanks for your review. -Bin