On Wed, Jul 27, 2016 at 11:13:43AM +0300, Felipe Balbi wrote: > > Hi, > > Bin Gao <bin.gao@xxxxxxxxxxxxxxx> 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 <bin.gao@xxxxxxxxx> > > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html