From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757222AbcG0R1I (ORCPT ); Wed, 27 Jul 2016 13:27:08 -0400 Received: from mga14.intel.com ([192.55.52.115]:25935 "EHLO mga14.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753276AbcG0R1G (ORCPT ); Wed, 27 Jul 2016 13:27:06 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.28,430,1464678000"; d="scan'208";a="1003652340" Date: Wed, 27 Jul 2016 10:31:32 -0700 From: Bin Gao To: Oliver Neukum Cc: Felipe Balbi , Heikki Krogerus , Greg Kroah-Hartman , linux-kernel@vger.kernel.org, linux-usb@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: <20160727173132.GA169011@worksta> References: <20160726183722.GE211765@worksta> <1469611273.2408.2.camel@suse.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1469611273.2408.2.camel@suse.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:21:13AM +0200, Oliver Neukum wrote: > On Tue, 2016-07-26 at 11:37 -0700, Bin Gao wrote: > > +#define MAKE_HEADER(port, header, msg, objs) \ > > +do { \ > > + header->type = msg; \ > > + header->data_role = PD_DATA_ROLE_UFP; \ > > + header->revision = port->pd_rev; \ > > + header->power_role = PD_POWER_ROLE_SINK; \ > > + header->id = roll_msg_id(port); \ > > + header->nr_objs = objs; \ > > + header->extended = PD_MSG_NOT_EXTENDED; \ > > +} while (0) > > + > > +static struct pd_sink_port *sink_ports[MAX_NR_SINK_PORTS]; > > +static int nr_ports; > > + > > +BLOCKING_NOTIFIER_HEAD(pd_sink_notifier_list); > > + > > +static char *state_strings[] = { > > + "WAIT_FOR_SOURCE_CAPABILITY", > > + "REQUEST_SENT", > > + "ACCEPT_RECEIVED", > > + "POWER_SUPPLY_READY", > > +}; > > + > > +/* Control messages */ > > +static char *cmsg_strings[] = { > > + "GOODCRC", /* 1 */ > > + "GOTOMIN", /* 2 */ > > + "ACCEPT", /* 3 */ > > + "REJECT", /* 4 */ > > + "PING", /* 5 */ > > + "PS_RDY", /* 6 */ > > + "GET_SRC_CAP", /* 7 */ > > + "GET_SINK_CAP", /* 8 */ > > + "DR_SWAP", /* 9 */ > > + "PR_SWAP", /* 10 */ > > + "VCONN_SWAP", /* 11 */ > > + "WAIT", /* 12 */ > > + "SOFT_RESET", /* 13 */ > > + "RESERVED", /* 14 */ > > + "RESERVED", /* 15 */ > > + "NOT_SUPPORTED", /* 16 */ > > + "GET_SOURCE_CAP_EXTENDED", /* 17 */ > > + "GET_STATUS", /* 18 */ > > + "FR_SWAP", /* 19 */ > > + /* RESERVED 20 - 31 */ > > +}; > > + > > +/* Data messages */ > > +static char *dmsg_strings[] = { > > + "SOURCE_CAP", /* 1 */ > > + "REQUEST", /* 2 */ > > + "BIST", /* 3 */ > > + "SINK_CAP", /* 4 */ > > + "BATTERY_STATUS", /* 5 */ > > + "ALERT", /* 6 */ > > + "RESERVED", /* 7 */ > > + "RESERVED", /* 8 */ > > + "RESERVED", /* 9 */ > > + "RESERVED", /* 10 */ > > + "RESERVED", /* 11 */ > > + "RESERVED", /* 12 */ > > + "RESERVED", /* 13 */ > > + "RESERVED", /* 14 */ > > + "VENDOR_DEFINED", /* 15 */ > > + /* RESERVED 16 - 31 */ > > +}; > > + > > +static char *state_to_string(enum pd_sink_state state) > > +{ > > + if (state < ARRAY_SIZE(state_strings)) > > + return state_strings[state]; > > + else > > + return NULL; > > +} > > + > > +static char *msg_to_string(bool is_cmsg, u8 msg) > > +{ > > + int nr = is_cmsg ? ARRAY_SIZE(cmsg_strings) : > > ARRAY_SIZE(dmsg_strings); > > + > > + if (msg <= nr) > > + return is_cmsg ? cmsg_strings[msg - 1] : > > dmsg_strings[msg - 1]; > > + else > > + return "RESERVED"; > > +} > > + > > +static void print_message(int port, bool is_cmsg, u8 msg, bool recv) > > +{ > > + pr_debug("sink port %d: %s message %s %s\n", port, > > + is_cmsg ? "Control" : "Data", > > + msg_to_string(is_cmsg, msg), > > + recv ? "received" : "sent)"); > > +} > > + > > +static inline bool fixed_ps_equal(struct sink_ps *p1, > > + struct sink_ps *p2) > > +{ > > + return p1->ps_type == p2->ps_type && > > + p1->ps_fixed.voltage_fixed == > > p2->ps_fixed.voltage_fixed && > > + p1->ps_fixed.current_default == > > p2->ps_fixed.current_default; > > +} > > + > > +/* The message ID increments each time we send out a new message */ > > +static u8 roll_msg_id(struct pd_sink_port *port) > > +{ > > + u8 msg_id = port->msg_id; > > + > > + if (msg_id == PD_MSG_ID_MAX) > > + msg_id = PD_MSG_ID_MIN; > > + else > > + msg_id++; > > + > > + port->msg_id = msg_id; > > + return msg_id; > > +} > > + > > These pieces of code are completely generic. They should be shared > among drivers. Please move them into the include files. > > Regards > Oliver > > They are only internally used by this driver (USB PD state machine driver). And they are not used by drivers (typicall USB Type-C phy drivers) which use APIs exposed by this driver. So I think it's not appropriate to move to include files.