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@xxxxxxxxxx>
> > ---
> > 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
