On Thu, Jan 24, 2019 at 07:15:35PM +0000, Ajay Gupta wrote:
> Hi Heikki
>
> > > Used to send command to ccg controller
> > >
> > > Signed-off-by: Ajay Gupta <ajayg@xxxxxxxxxx>
> > > ---
> > > drivers/usb/typec/ucsi/ucsi_ccg.c | 252
> > > ++++++++++++++++++++++++++++--
> > > 1 file changed, 243 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/drivers/usb/typec/ucsi/ucsi_ccg.c
> > > b/drivers/usb/typec/ucsi/ucsi_ccg.c
> > > index 4d35279ab853..dce9126b6a37 100644
> > > --- a/drivers/usb/typec/ucsi/ucsi_ccg.c
> > > +++ b/drivers/usb/typec/ucsi/ucsi_ccg.c
> > > @@ -17,6 +17,29 @@
> > > #include <asm/unaligned.h>
> > > #include "ucsi.h"
> > >
> > > +#define CCGX_RAB_DEVICE_MODE 0x0000
> > > +#define CCGX_RAB_INTR_REG 0x0006
> > > +#define DEV_INT BIT(0)
> > > +#define PORT0_INT BIT(1)
> > > +#define PORT1_INT BIT(2)
> > > +#define UCSI_READ_INT BIT(7)
> > > +#define CCGX_RAB_READ_ALL_VER 0x0010
> > > +#define CCGX_RAB_READ_FW2_VER 0x0020
> > > +#define CCGX_RAB_UCSI_CONTROL 0x0039
> > > +#define CCGX_RAB_UCSI_CONTROL_START BIT(0)
> > > +#define CCGX_RAB_UCSI_CONTROL_STOP BIT(1)
> > > +#define CCGX_RAB_UCSI_DATA_BLOCK(offset) (0xf000 | ((offset) &
> > 0xff))
> > > +#define DEV_REG_IDX
> > CCGX_RAB_DEVICE_MODE
> > > +#define CCGX_RAB_RESPONSE 0x007E
> > > +#define ASYNC_EVENT BIT(7)
> > > +
> > > +/* CCGx events & async msg codes */
> > > +#define RESET_COMPLETE 0x80
> > > +#define EVENT_INDEX RESET_COMPLETE
> > > +#define PORT_CONNECT_DET 0x84
> > > +#define PORT_DISCONNECT_DET 0x85
> > > +#define ROLE_SWAP_COMPELETE 0x87
> > > +
> > > struct ccg_dev_info {
> > > u8 fw_mode:2;
> > > u8 two_pd_ports:2;
> > > @@ -44,23 +67,113 @@ struct version_info {
> > > struct version_format app;
> > > };
> > >
> > > +/* CCGx response codes */
> > > +enum ccg_resp_code {
> > > + CMD_NO_RESP = 0x00,
> > > + CMD_SUCCESS = 0x02,
> > > + FLASH_DATA_AVAILABLE = 0x03,
> > > + CMD_INVALID = 0x05,
> > > + FLASH_UPDATE_FAIL = 0x07,
> > > + INVALID_FW = 0x08,
> > > + INVALID_ARG = 0x09,
> > > + CMD_NOT_SUPPORT = 0x0A,
> > > + TRANSACTION_FAIL = 0x0C,
> > > + PD_CMD_FAIL = 0x0D,
> > > + UNDEF_ERROR = 0x0F,
> > > + INVALID_RESP = 0x10,
> > > +};
> > > +
> > > +static const char * const ccg_resp_strs[] = {
> > > + /* 0x00 */ "No Response.",
> > > + /* 0x01 */ "0x01",
> > > + /* 0x02 */ "HPI Command Success.",
> > > + /* 0x03 */ "Flash Data Available in data memory.",
> > > + /* 0x04 */ "0x04",
> > > + /* 0x05 */ "Invalid Command.",
> > > + /* 0x06 */ "0x06",
> > > + /* 0x07 */ "Flash write operation failed.",
> > > + /* 0x08 */ "Firmware validity check failed.",
> > > + /* 0x09 */ "Command failed due to invalid arguments.",
> > > + /* 0x0A */ "Command not supported in the current mode.",
> > > + /* 0x0B */ "0x0B",
> > > + /* 0x0C */ "Transaction Failed. GOOD_CRC was not received.",
> > > + /* 0x0D */ "PD Command Failed.",
> > > + /* 0x0E */ "0x0E",
> > > + /* 0x0F */ "Undefined Error",
> > > +};
> > > +
> > > +static const char * const ccg_evt_strs[] = {
> > > + /* 0x80 */ "Reset Complete.",
> > > + /* 0x81 */ "Message queue overflow detected.",
> > > + /* 0x82 */ "Overcurrent Detected",
> > > + /* 0x83 */ "Overvoltage Detected",
> > > + /* 0x84 */ "Type-C Port Connect Detected",
> > > + /* 0x85 */ "Type-C Port Disconnect Detected",
> > > + /* 0x86 */ "PD Contract Negotiation Complete",
> > > + /* 0x87 */ "SWAP Complete",
> > > + /* 0x88 */ "0x88",
> > > + /* 0x89 */ "0x89",
> > > + /* 0x8A */ "PS_RDY Message Received",
> > > + /* 0x8B */ "GotoMin Message Received.",
> > > + /* 0x8C */ "Accept Message Received",
> > > + /* 0x8D */ "Reject Message Received",
> > > + /* 0x8E */ "Wait Message Received",
> > > + /* 0x8F */ "Hard Reset Received",
> > > + /* 0x90 */ "VDM Received",
> > > + /* 0x91 */ "Source Capabilities Message Received",
> > > + /* 0x92 */ "Sink Capabilities Message Received",
> > > + /* 0x93 */ "Display Port Alternate Mode entered",
> > > + /* 0x94 */ "Display Port device connected at UFP_U",
> > > + /* 0x95 */ "Display port device not connected at UFP_U",
> > > + /* 0x96 */ "Display port SID not found in Discover SID process",
> > > + /* 0x97 */ "Multiple SVIDs discovered along with DisplayPort SID",
> > > + /* 0x98 */ "DP Functionality not supported by Cable",
> > > + /* 0x99 */ "Display Port Configuration not supported by UFP",
> > > + /* 0x9A */ "Hard Reset Sent to Port Partner",
> > > + /* 0x9B */ "Soft Reset Sent to Port Partner",
> > > + /* 0x9C */ "Cable Reset Sent to EMCA",
> > > + /* 0x9D */ "Source Disabled State Entered",
> > > + /* 0x9E */ "Sender Response Timer Timeout",
> > > + /* 0x9F */ "No VDM Response Received",
> > > + /* 0xA0 */ "Unexpected Voltage on Vbus",
> > > + /* 0xA1 */ "Type-C Error Recovery",
> > > + /* 0xA2 */ "0xA2",
> > > + /* 0xA3 */ "0xA3",
> > > + /* 0xA4 */ "0xA4",
> > > + /* 0xA5 */ "0xA5",
> > > + /* 0xA6 */ "EMCA Detected",
> > > + /* 0xA7 */ "0xA7",
> > > + /* 0xA8 */ "0xA8",
> > > + /* 0xA9 */ "0xA9",
> > > + /* 0xAA */ "Rp Change Detected",
> > > +};
> >
> > Those looks like something that you could use in a tracepoint.
> These are various events sent from ccg controller in response to a cmd. We use it
> to display message from any error events.
>
> > Can you check how much would it take to implement tracepoints for this driver.
> > They would be more useful compared to the dev_dbg() you are using now.
> Is it okay to add new tracepoint for ucsi_ccg driver or to reuse drivers/usb/typec/ucsi/trace.c, trace.h ?
I don't know actually what is the best approach.
Can you skip those debug messages for now. They need to be added in a
separate patch in any case. Or patches actually.
I think it would be better that you first introduce the feature
without any debugging prints. Add the debugging after the feature is
in.
So in your next version, drop all dev_dbg() prints from all patches in
the series.
thanks,
--
heikki
