| Message ID | 20190128203731.12681-2-ajayg@nvidia.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add support for firmware update on Cypres CCGx | expand |
On Mon, Jan 28, 2019 at 12:37:26PM -0800, Ajay Gupta wrote: > Function is to get the details of ccg firmware and device version. > It will be useful in debugging and also during firmware update. > > Signed-off-by: Ajay Gupta <ajayg@nvidia.com> > --- > Changes from v1: > - Updated commit message > - Dropped uses of bitfield > - Removed dev_dbg prints > > drivers/usb/typec/ucsi/ucsi_ccg.c | 58 +++++++++++++++++++++++++++++-- > 1 file changed, 56 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/typec/ucsi/ucsi_ccg.c b/drivers/usb/typec/ucsi/ucsi_ccg.c > index de8a43bdff68..76cf772872db 100644 > --- a/drivers/usb/typec/ucsi/ucsi_ccg.c > +++ b/drivers/usb/typec/ucsi/ucsi_ccg.c > @@ -17,15 +17,46 @@ > #include <asm/unaligned.h> > #include "ucsi.h" > > +struct ccg_dev_info { > +#define CCG_DEVINFO_FWMODE_SHIFT (0) > +#define CCG_DEVINFO_FWMODE_MASK (0x3 << CCG_DEVINFO_FWMODE_SHIFT) > +#define CCG_DEVINFO_PDPORTS_SHIFT (2) > +#define CCG_DEVINFO_PDPORTS_MASK (0x3 << CCG_DEVINFO_PDPORTS_SHIFT) > + u8 mode; > + u8 bl_mode; > + u16 silicon_id; > + u16 bl_last_row; __le16 I am guessing for both of these? > +}; Should this be a packed structure? > + > +struct version_format { > + u16 build; __le16? > + u8 patch; > + u8 ver; > +#define CCG_VERSION_MIN_SHIFT (0) > +#define CCG_VERSION_MIN_MASK (0xf << CCG_VERSION_MIN_SHIFT) > +#define CCG_VERSION_MAJ_SHIFT (4) > +#define CCG_VERSION_MAJ_MASK (0xf << CCG_VERSION_MAJ_SHIFT) > +}; Also, packed? > + > +struct version_info { > + struct version_format base; > + struct version_format app; > +}; > + > struct ucsi_ccg { > struct device *dev; > struct ucsi *ucsi; > struct ucsi_ppm ppm; > struct i2c_client *client; > + struct ccg_dev_info info; > + struct version_info version[3]; 3 versions? Any hint as to what they are all "called"? thanks, greg k-h
On Mon, Jan 28, 2019 at 12:37:26PM -0800, Ajay Gupta wrote: > Function is to get the details of ccg firmware and device version. > It will be useful in debugging and also during firmware update. > > Signed-off-by: Ajay Gupta <ajayg@nvidia.com> Your "From:" and Signed-off-by: lines do not match :(
Hi Greg, > On Mon, Jan 28, 2019 at 12:37:26PM -0800, Ajay Gupta wrote: > > Function is to get the details of ccg firmware and device version. > > It will be useful in debugging and also during firmware update. > > > > Signed-off-by: Ajay Gupta <ajayg@nvidia.com> > > --- > > Changes from v1: > > - Updated commit message > > - Dropped uses of bitfield > > - Removed dev_dbg prints > > > > drivers/usb/typec/ucsi/ucsi_ccg.c | 58 > > +++++++++++++++++++++++++++++-- > > 1 file changed, 56 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/usb/typec/ucsi/ucsi_ccg.c > > b/drivers/usb/typec/ucsi/ucsi_ccg.c > > index de8a43bdff68..76cf772872db 100644 > > --- a/drivers/usb/typec/ucsi/ucsi_ccg.c > > +++ b/drivers/usb/typec/ucsi/ucsi_ccg.c > > @@ -17,15 +17,46 @@ > > #include <asm/unaligned.h> > > #include "ucsi.h" > > > > +struct ccg_dev_info { > > +#define CCG_DEVINFO_FWMODE_SHIFT (0) > > +#define CCG_DEVINFO_FWMODE_MASK (0x3 << > CCG_DEVINFO_FWMODE_SHIFT) > > +#define CCG_DEVINFO_PDPORTS_SHIFT (2) #define > > +CCG_DEVINFO_PDPORTS_MASK (0x3 << CCG_DEVINFO_PDPORTS_SHIFT) > > + u8 mode; > > + u8 bl_mode; > > + u16 silicon_id; > > + u16 bl_last_row; > > __le16 I am guessing for both of these? Ok. > > > +}; > > Should this be a packed structure? Yes, will do. > > > > + > > +struct version_format { > > + u16 build; > > __le16? Ok > > > + u8 patch; > > + u8 ver; > > +#define CCG_VERSION_MIN_SHIFT (0) > > +#define CCG_VERSION_MIN_MASK (0xf << CCG_VERSION_MIN_SHIFT) > #define > > +CCG_VERSION_MAJ_SHIFT (4) #define CCG_VERSION_MAJ_MASK (0xf << > > +CCG_VERSION_MAJ_SHIFT) }; > > Also, packed? Ok > > > + > > +struct version_info { > > + struct version_format base; > > + struct version_format app; > > +}; > > + > > struct ucsi_ccg { > > struct device *dev; > > struct ucsi *ucsi; > > struct ucsi_ppm ppm; > > struct i2c_client *client; > > + struct ccg_dev_info info; > > + struct version_info version[3]; > > 3 versions? Any hint as to what they are all "called"? Those three are for versions of three different firmware, boot, primary and secondary. Will fix it my moving enum enum_fw_mode from patch 5/6 here and use it as struct version_info version[FW2 + 1]; +enum enum_fw_mode { + BOOT, /* bootloader */ + FW1, /* FW partition-1 (contains secondary fw) */ + FW2, /* FW partition-2 (contains primary fw) */ + FW_INVALID, +}; thanks > nvpublic > thanks, > > greg k-h
Hi Greg > Subject: Re: [PATCH v2 1/6] usb: typec: ucsi: add get_fw_info function > > On Mon, Jan 28, 2019 at 12:37:26PM -0800, Ajay Gupta wrote: > > Function is to get the details of ccg firmware and device version. > > It will be useful in debugging and also during firmware update. > > > > Signed-off-by: Ajay Gupta <ajayg@nvidia.com> > > Your "From:" and Signed-off-by: lines do not match :( Will fix. Thanks > nvpublic
diff --git a/drivers/usb/typec/ucsi/ucsi_ccg.c b/drivers/usb/typec/ucsi/ucsi_ccg.c index de8a43bdff68..76cf772872db 100644 --- a/drivers/usb/typec/ucsi/ucsi_ccg.c +++ b/drivers/usb/typec/ucsi/ucsi_ccg.c @@ -17,15 +17,46 @@ #include <asm/unaligned.h> #include "ucsi.h" +struct ccg_dev_info { +#define CCG_DEVINFO_FWMODE_SHIFT (0) +#define CCG_DEVINFO_FWMODE_MASK (0x3 << CCG_DEVINFO_FWMODE_SHIFT) +#define CCG_DEVINFO_PDPORTS_SHIFT (2) +#define CCG_DEVINFO_PDPORTS_MASK (0x3 << CCG_DEVINFO_PDPORTS_SHIFT) + u8 mode; + u8 bl_mode; + u16 silicon_id; + u16 bl_last_row; +}; + +struct version_format { + u16 build; + u8 patch; + u8 ver; +#define CCG_VERSION_MIN_SHIFT (0) +#define CCG_VERSION_MIN_MASK (0xf << CCG_VERSION_MIN_SHIFT) +#define CCG_VERSION_MAJ_SHIFT (4) +#define CCG_VERSION_MAJ_MASK (0xf << CCG_VERSION_MAJ_SHIFT) +}; + +struct version_info { + struct version_format base; + struct version_format app; +}; + struct ucsi_ccg { struct device *dev; struct ucsi *ucsi; struct ucsi_ppm ppm; struct i2c_client *client; + struct ccg_dev_info info; + struct version_info version[3]; }; -#define CCGX_RAB_INTR_REG 0x06 -#define CCGX_RAB_UCSI_CONTROL 0x39 +#define CCGX_RAB_DEVICE_MODE 0x0000 +#define CCGX_RAB_INTR_REG 0x0006 +#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)) @@ -220,6 +251,23 @@ static irqreturn_t ccg_irq_handler(int irq, void *data) return IRQ_HANDLED; } +static int get_fw_info(struct ucsi_ccg *uc) +{ + int err; + + err = ccg_read(uc, CCGX_RAB_READ_ALL_VER, (u8 *)(&uc->version), + sizeof(uc->version)); + if (err < 0) + return err; + + err = ccg_read(uc, CCGX_RAB_DEVICE_MODE, (u8 *)(&uc->info), + sizeof(uc->info)); + if (err < 0) + return err; + + return 0; +} + static int ucsi_ccg_probe(struct i2c_client *client, const struct i2c_device_id *id) { @@ -248,6 +296,12 @@ static int ucsi_ccg_probe(struct i2c_client *client, return status; } + status = get_fw_info(uc); + if (status < 0) { + dev_err(uc->dev, "get_fw_info failed - %d\n", status); + return status; + } + status = devm_request_threaded_irq(dev, client->irq, NULL, ccg_irq_handler, IRQF_ONESHOT | IRQF_TRIGGER_HIGH,
Function is to get the details of ccg firmware and device version. It will be useful in debugging and also during firmware update. Signed-off-by: Ajay Gupta <ajayg@nvidia.com> --- Changes from v1: - Updated commit message - Dropped uses of bitfield - Removed dev_dbg prints drivers/usb/typec/ucsi/ucsi_ccg.c | 58 +++++++++++++++++++++++++++++-- 1 file changed, 56 insertions(+), 2 deletions(-)