* [1/7] usb: typec: ucsi: add get_fw_info function
@ 2019-01-18 7:06 Greg KH
0 siblings, 0 replies; 6+ messages in thread
From: Greg KH @ 2019-01-18 7:06 UTC (permalink / raw)
To: Ajay Gupta; +Cc: heikki.krogerus, linux-usb
On Thu, Jan 17, 2019 at 05:09:03PM -0800, Ajay Gupta wrote:
> Function is to get the details of ccg firmware and device version.
>
> Signed-off-by: Ajay Gupta <ajayg@nvidia.com>
> ---
> drivers/usb/typec/ucsi/ucsi_ccg.c | 76 ++++++++++++++++++++++++++++++-
> 1 file changed, 74 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/typec/ucsi/ucsi_ccg.c b/drivers/usb/typec/ucsi/ucsi_ccg.c
> index de8a43bdff68..4d35279ab853 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 {
> + u8 fw_mode:2;
> + u8 two_pd_ports:2;
> + u8 row_size_256:2;
> + u8:1; /* reserved */
> + u8 hpi_v2_mode:1;
> + u8 bl_mode:1;
> + u8 cfgtbl_invalid:1;
> + u8 fw1_invalid:1;
> + u8 fw2_invalid:1;
> + u8:4; /* reserved */
> + u16 silicon_id;
> + u16 bl_last_row;
> +} __packed;
Doesn't all of this break into tiny pieces when you read the memory into
a big-endian system? Be very careful when using bitfields as a "raw"
representation of a structure in memory.
> +
> +struct version_format {
> + u16 build;
> + u8 patch;
> + u8 min:4;
> + u8 maj:4;
> +};
Same here.
And not packed? That's dangerous :)
> +
> +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;
> };
>
> -#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,41 @@ static irqreturn_t ccg_irq_handler(int irq, void *data)
> return IRQ_HANDLED;
> }
>
> +static int get_fw_info(struct ucsi_ccg *uc)
> +{
> + struct device *dev = uc->dev;
> + struct version_info version[3];
> + struct version_info *v;
> + int err, i;
> +
> + err = ccg_read(uc, CCGX_RAB_READ_ALL_VER, (u8 *)(&version),
> + sizeof(version));
> + if (err < 0)
> + return err;
> +
> + for (i = 1; i < ARRAY_SIZE(version); i++) {
> + v = &version[i];
> + dev_dbg(dev,
> + "FW%d Version: %c%c v%x.%x%x, [Base %d.%d.%d.%d]\n",
> + i, (v->app.build >> 8), (v->app.build & 0xFF),
> + v->app.patch, v->app.maj, v->app.min,
> + v->base.maj, v->base.min, v->base.patch,
> + v->base.build);
> + }
> +
> + err = ccg_read(uc, CCGX_RAB_DEVICE_MODE, (u8 *)(&uc->info),
> + sizeof(uc->info));
> + if (err < 0)
> + return err;
> +
> + dev_dbg(dev, "fw_mode: %d\n", uc->info.fw_mode);
> + dev_dbg(dev, "fw1_invalid: %d\n", uc->info.fw1_invalid);
> + dev_dbg(dev, "fw2_invalid: %d\n", uc->info.fw2_invalid);
> + dev_dbg(dev, "silicon_id: 0x%04x\n", uc->info.silicon_id);
> +
> + return 0;
> +}
> +
> static int ucsi_ccg_probe(struct i2c_client *client,
> const struct i2c_device_id *id)
> {
> @@ -248,6 +314,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;
Are all devices required to have this information? if not, you just
prevented them from working I think :(
thanks,
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* [1/7] usb: typec: ucsi: add get_fw_info function
@ 2019-01-25 13:55 Heikki Krogerus
0 siblings, 0 replies; 6+ messages in thread
From: Heikki Krogerus @ 2019-01-25 13:55 UTC (permalink / raw)
To: Ajay Gupta; +Cc: linux-usb
On Thu, Jan 24, 2019 at 05:45:48PM +0000, Ajay Gupta wrote:
> Hi Heikki,
>
> > > Function is to get the details of ccg firmware and device version.
> > >
> > > Signed-off-by: Ajay Gupta <ajayg@nvidia.com>
> > > ---
> > > drivers/usb/typec/ucsi/ucsi_ccg.c | 76
> > > ++++++++++++++++++++++++++++++-
> > > 1 file changed, 74 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/usb/typec/ucsi/ucsi_ccg.c
> > > b/drivers/usb/typec/ucsi/ucsi_ccg.c
> > > index de8a43bdff68..4d35279ab853 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 {
> > > + u8 fw_mode:2;
> > > + u8 two_pd_ports:2;
> > > + u8 row_size_256:2;
> > > + u8:1; /* reserved */
> > > + u8 hpi_v2_mode:1;
> > > + u8 bl_mode:1;
> > > + u8 cfgtbl_invalid:1;
> > > + u8 fw1_invalid:1;
> > > + u8 fw2_invalid:1;
> > > + u8:4; /* reserved */
> > > + u16 silicon_id;
> > > + u16 bl_last_row;
> > > +} __packed;
> > > +
> > > +struct version_format {
> > > + u16 build;
> > > + u8 patch;
> > > + u8 min:4;
> > > + u8 maj:4;
> > > +};
> >
> > Don't use bit fields. We shouldn't use them even in the core ucsi driver with the
> > message data structures like we do.
>
> So are you saying to remove bit fields from structure and instead use shift and mask
> on u8 variable to get bit fields?
Yes, exactly.
thanks,
^ permalink raw reply [flat|nested] 6+ messages in thread
* [1/7] usb: typec: ucsi: add get_fw_info function
@ 2019-01-24 17:45 Ajay Gupta
0 siblings, 0 replies; 6+ messages in thread
From: Ajay Gupta @ 2019-01-24 17:45 UTC (permalink / raw)
To: Heikki Krogerus; +Cc: linux-usb
Hi Heikki,
> > Function is to get the details of ccg firmware and device version.
> >
> > Signed-off-by: Ajay Gupta <ajayg@nvidia.com>
> > ---
> > drivers/usb/typec/ucsi/ucsi_ccg.c | 76
> > ++++++++++++++++++++++++++++++-
> > 1 file changed, 74 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/usb/typec/ucsi/ucsi_ccg.c
> > b/drivers/usb/typec/ucsi/ucsi_ccg.c
> > index de8a43bdff68..4d35279ab853 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 {
> > + u8 fw_mode:2;
> > + u8 two_pd_ports:2;
> > + u8 row_size_256:2;
> > + u8:1; /* reserved */
> > + u8 hpi_v2_mode:1;
> > + u8 bl_mode:1;
> > + u8 cfgtbl_invalid:1;
> > + u8 fw1_invalid:1;
> > + u8 fw2_invalid:1;
> > + u8:4; /* reserved */
> > + u16 silicon_id;
> > + u16 bl_last_row;
> > +} __packed;
> > +
> > +struct version_format {
> > + u16 build;
> > + u8 patch;
> > + u8 min:4;
> > + u8 maj:4;
> > +};
>
> Don't use bit fields. We shouldn't use them even in the core ucsi driver with the
> message data structures like we do.
So are you saying to remove bit fields from structure and instead use shift and mask
on u8 variable to get bit fields?
> > +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;
> > };
> >
> > -#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,41 @@ static irqreturn_t ccg_irq_handler(int irq, void *data)
> > return IRQ_HANDLED;
> > }
> >
> > +static int get_fw_info(struct ucsi_ccg *uc) {
> > + struct device *dev = uc->dev;
> > + struct version_info version[3];
> > + struct version_info *v;
> > + int err, i;
> > +
> > + err = ccg_read(uc, CCGX_RAB_READ_ALL_VER, (u8 *)(&version),
> > + sizeof(version));
> > + if (err < 0)
> > + return err;
> > +
> > + for (i = 1; i < ARRAY_SIZE(version); i++) {
> > + v = &version[i];
> > + dev_dbg(dev,
> > + "FW%d Version: %c%c v%x.%x%x, [Base
> %d.%d.%d.%d]\n",
> > + i, (v->app.build >> 8), (v->app.build & 0xFF),
> > + v->app.patch, v->app.maj, v->app.min,
> > + v->base.maj, v->base.min, v->base.patch,
> > + v->base.build);
>
> How about a sysfs file showing the fw version?
I can add that.
>
> > + }
> > +
> > + err = ccg_read(uc, CCGX_RAB_DEVICE_MODE, (u8 *)(&uc->info),
> > + sizeof(uc->info));
> > + if (err < 0)
> > + return err;
> > +
> > + dev_dbg(dev, "fw_mode: %d\n", uc->info.fw_mode);
> > + dev_dbg(dev, "fw1_invalid: %d\n", uc->info.fw1_invalid);
> > + dev_dbg(dev, "fw2_invalid: %d\n", uc->info.fw2_invalid);
> > + dev_dbg(dev, "silicon_id: 0x%04x\n", uc->info.silicon_id);
>
> Are those prints really useful for anybody?
They are mostly informational purpose.
>
> If you need the values from that ccg_dev_info for debugging, then I think
> debugfs is the appropriate place to expose them.
Will remove them.
Thanks
> nvpublic
> > + return 0;
> > +}
> > +
> > static int ucsi_ccg_probe(struct i2c_client *client,
> > const struct i2c_device_id *id)
> > {
> > @@ -248,6 +314,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,
> > --
> > 2.17.1
>
> Br,
>
> --
> heikki
^ permalink raw reply [flat|nested] 6+ messages in thread
* [1/7] usb: typec: ucsi: add get_fw_info function
@ 2019-01-24 17:41 Ajay Gupta
0 siblings, 0 replies; 6+ messages in thread
From: Ajay Gupta @ 2019-01-24 17:41 UTC (permalink / raw)
To: Greg KH; +Cc: heikki.krogerus, linux-usb
Hi Greg
> -----Original Message-----
> From: Greg KH <greg@kroah.com>
> Sent: Thursday, January 17, 2019 11:06 PM
> To: Ajay Gupta <ajayg@nvidia.com>
> Cc: heikki.krogerus@linux.intel.com; linux-usb@vger.kernel.org
> Subject: Re: [PATCH 1/7] usb: typec: ucsi: add get_fw_info function
>
> On Thu, Jan 17, 2019 at 05:09:03PM -0800, Ajay Gupta wrote:
> > Function is to get the details of ccg firmware and device version.
> >
> > Signed-off-by: Ajay Gupta <ajayg@nvidia.com>
> > ---
> > drivers/usb/typec/ucsi/ucsi_ccg.c | 76
> > ++++++++++++++++++++++++++++++-
> > 1 file changed, 74 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/usb/typec/ucsi/ucsi_ccg.c
> > b/drivers/usb/typec/ucsi/ucsi_ccg.c
> > index de8a43bdff68..4d35279ab853 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 {
> > + u8 fw_mode:2;
> > + u8 two_pd_ports:2;
> > + u8 row_size_256:2;
> > + u8:1; /* reserved */
> > + u8 hpi_v2_mode:1;
> > + u8 bl_mode:1;
> > + u8 cfgtbl_invalid:1;
> > + u8 fw1_invalid:1;
> > + u8 fw2_invalid:1;
> > + u8:4; /* reserved */
> > + u16 silicon_id;
> > + u16 bl_last_row;
> > +} __packed;
>
> Doesn't all of this break into tiny pieces when you read the memory into a big-
> endian system? Be very careful when using bitfields as a "raw"
> representation of a structure in memory.
I am planning to drop bit fields itself.
> > +
> > +struct version_format {
> > + u16 build;
> > + u8 patch;
> > + u8 min:4;
> > + u8 maj:4;
> > +};
>
> Same here.
>
> And not packed? That's dangerous :)
Same here.
>
>
> > +
> > +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;
> > };
> >
> > -#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,41 @@ static irqreturn_t ccg_irq_handler(int irq, void *data)
> > return IRQ_HANDLED;
> > }
> >
> > +static int get_fw_info(struct ucsi_ccg *uc) {
> > + struct device *dev = uc->dev;
> > + struct version_info version[3];
> > + struct version_info *v;
> > + int err, i;
> > +
> > + err = ccg_read(uc, CCGX_RAB_READ_ALL_VER, (u8 *)(&version),
> > + sizeof(version));
> > + if (err < 0)
> > + return err;
> > +
> > + for (i = 1; i < ARRAY_SIZE(version); i++) {
> > + v = &version[i];
> > + dev_dbg(dev,
> > + "FW%d Version: %c%c v%x.%x%x, [Base
> %d.%d.%d.%d]\n",
> > + i, (v->app.build >> 8), (v->app.build & 0xFF),
> > + v->app.patch, v->app.maj, v->app.min,
> > + v->base.maj, v->base.min, v->base.patch,
> > + v->base.build);
> > + }
> > +
> > + err = ccg_read(uc, CCGX_RAB_DEVICE_MODE, (u8 *)(&uc->info),
> > + sizeof(uc->info));
> > + if (err < 0)
> > + return err;
> > +
> > + dev_dbg(dev, "fw_mode: %d\n", uc->info.fw_mode);
> > + dev_dbg(dev, "fw1_invalid: %d\n", uc->info.fw1_invalid);
> > + dev_dbg(dev, "fw2_invalid: %d\n", uc->info.fw2_invalid);
> > + dev_dbg(dev, "silicon_id: 0x%04x\n", uc->info.silicon_id);
> > +
> > + return 0;
> > +}
> > +
> > static int ucsi_ccg_probe(struct i2c_client *client,
> > const struct i2c_device_id *id)
> > {
> > @@ -248,6 +314,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;
>
> Are all devices required to have this information? if not, you just prevented
> them from working I think :(
The error is due to ccg_read() failure and in that case we want to stop.
Thanks
> nvpublic
> thanks,
>
> greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* [1/7] usb: typec: ucsi: add get_fw_info function
@ 2019-01-18 14:53 Heikki Krogerus
0 siblings, 0 replies; 6+ messages in thread
From: Heikki Krogerus @ 2019-01-18 14:53 UTC (permalink / raw)
To: Ajay Gupta; +Cc: linux-usb
Hi Ajay,
On Thu, Jan 17, 2019 at 05:09:03PM -0800, Ajay Gupta wrote:
> Function is to get the details of ccg firmware and device version.
>
> Signed-off-by: Ajay Gupta <ajayg@nvidia.com>
> ---
> drivers/usb/typec/ucsi/ucsi_ccg.c | 76 ++++++++++++++++++++++++++++++-
> 1 file changed, 74 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/typec/ucsi/ucsi_ccg.c b/drivers/usb/typec/ucsi/ucsi_ccg.c
> index de8a43bdff68..4d35279ab853 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 {
> + u8 fw_mode:2;
> + u8 two_pd_ports:2;
> + u8 row_size_256:2;
> + u8:1; /* reserved */
> + u8 hpi_v2_mode:1;
> + u8 bl_mode:1;
> + u8 cfgtbl_invalid:1;
> + u8 fw1_invalid:1;
> + u8 fw2_invalid:1;
> + u8:4; /* reserved */
> + u16 silicon_id;
> + u16 bl_last_row;
> +} __packed;
> +
> +struct version_format {
> + u16 build;
> + u8 patch;
> + u8 min:4;
> + u8 maj:4;
> +};
Don't use bit fields. We shouldn't use them even in the core ucsi
driver with the message data structures like we do.
> +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;
> };
>
> -#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,41 @@ static irqreturn_t ccg_irq_handler(int irq, void *data)
> return IRQ_HANDLED;
> }
>
> +static int get_fw_info(struct ucsi_ccg *uc)
> +{
> + struct device *dev = uc->dev;
> + struct version_info version[3];
> + struct version_info *v;
> + int err, i;
> +
> + err = ccg_read(uc, CCGX_RAB_READ_ALL_VER, (u8 *)(&version),
> + sizeof(version));
> + if (err < 0)
> + return err;
> +
> + for (i = 1; i < ARRAY_SIZE(version); i++) {
> + v = &version[i];
> + dev_dbg(dev,
> + "FW%d Version: %c%c v%x.%x%x, [Base %d.%d.%d.%d]\n",
> + i, (v->app.build >> 8), (v->app.build & 0xFF),
> + v->app.patch, v->app.maj, v->app.min,
> + v->base.maj, v->base.min, v->base.patch,
> + v->base.build);
How about a sysfs file showing the fw version?
> + }
> +
> + err = ccg_read(uc, CCGX_RAB_DEVICE_MODE, (u8 *)(&uc->info),
> + sizeof(uc->info));
> + if (err < 0)
> + return err;
> +
> + dev_dbg(dev, "fw_mode: %d\n", uc->info.fw_mode);
> + dev_dbg(dev, "fw1_invalid: %d\n", uc->info.fw1_invalid);
> + dev_dbg(dev, "fw2_invalid: %d\n", uc->info.fw2_invalid);
> + dev_dbg(dev, "silicon_id: 0x%04x\n", uc->info.silicon_id);
Are those prints really useful for anybody?
If you need the values from that ccg_dev_info for debugging, then I
think debugfs is the appropriate place to expose them.
> + return 0;
> +}
> +
> static int ucsi_ccg_probe(struct i2c_client *client,
> const struct i2c_device_id *id)
> {
> @@ -248,6 +314,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,
> --
> 2.17.1
Br,
^ permalink raw reply [flat|nested] 6+ messages in thread
* [1/7] usb: typec: ucsi: add get_fw_info function
@ 2019-01-18 1:09 Ajay Gupta
0 siblings, 0 replies; 6+ messages in thread
From: Ajay Gupta @ 2019-01-18 1:09 UTC (permalink / raw)
To: heikki.krogerus; +Cc: linux-usb, Ajay Gupta
Function is to get the details of ccg firmware and device version.
Signed-off-by: Ajay Gupta <ajayg@nvidia.com>
---
drivers/usb/typec/ucsi/ucsi_ccg.c | 76 ++++++++++++++++++++++++++++++-
1 file changed, 74 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/typec/ucsi/ucsi_ccg.c b/drivers/usb/typec/ucsi/ucsi_ccg.c
index de8a43bdff68..4d35279ab853 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 {
+ u8 fw_mode:2;
+ u8 two_pd_ports:2;
+ u8 row_size_256:2;
+ u8:1; /* reserved */
+ u8 hpi_v2_mode:1;
+ u8 bl_mode:1;
+ u8 cfgtbl_invalid:1;
+ u8 fw1_invalid:1;
+ u8 fw2_invalid:1;
+ u8:4; /* reserved */
+ u16 silicon_id;
+ u16 bl_last_row;
+} __packed;
+
+struct version_format {
+ u16 build;
+ u8 patch;
+ u8 min:4;
+ u8 maj:4;
+};
+
+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;
};
-#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,41 @@ static irqreturn_t ccg_irq_handler(int irq, void *data)
return IRQ_HANDLED;
}
+static int get_fw_info(struct ucsi_ccg *uc)
+{
+ struct device *dev = uc->dev;
+ struct version_info version[3];
+ struct version_info *v;
+ int err, i;
+
+ err = ccg_read(uc, CCGX_RAB_READ_ALL_VER, (u8 *)(&version),
+ sizeof(version));
+ if (err < 0)
+ return err;
+
+ for (i = 1; i < ARRAY_SIZE(version); i++) {
+ v = &version[i];
+ dev_dbg(dev,
+ "FW%d Version: %c%c v%x.%x%x, [Base %d.%d.%d.%d]\n",
+ i, (v->app.build >> 8), (v->app.build & 0xFF),
+ v->app.patch, v->app.maj, v->app.min,
+ v->base.maj, v->base.min, v->base.patch,
+ v->base.build);
+ }
+
+ err = ccg_read(uc, CCGX_RAB_DEVICE_MODE, (u8 *)(&uc->info),
+ sizeof(uc->info));
+ if (err < 0)
+ return err;
+
+ dev_dbg(dev, "fw_mode: %d\n", uc->info.fw_mode);
+ dev_dbg(dev, "fw1_invalid: %d\n", uc->info.fw1_invalid);
+ dev_dbg(dev, "fw2_invalid: %d\n", uc->info.fw2_invalid);
+ dev_dbg(dev, "silicon_id: 0x%04x\n", uc->info.silicon_id);
+
+ return 0;
+}
+
static int ucsi_ccg_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
@@ -248,6 +314,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,
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-01-25 13:55 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-18 7:06 [1/7] usb: typec: ucsi: add get_fw_info function Greg KH
-- strict thread matches above, loose matches on Subject: below --
2019-01-25 13:55 Heikki Krogerus
2019-01-24 17:45 Ajay Gupta
2019-01-24 17:41 Ajay Gupta
2019-01-18 14:53 Heikki Krogerus
2019-01-18 1:09 Ajay Gupta
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.