[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 4/5] vmstateify tsc2005
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [Qemu-devel] [PATCH v2 4/5] vmstateify tsc2005 |
Date: |
Thu, 22 Sep 2016 16:53:39 +0100 |
User-agent: |
Mutt/1.7.0 (2016-08-17) |
* Peter Maydell (address@hidden) wrote:
> On 24 August 2016 at 11:40, Dr. David Alan Gilbert (git)
> <address@hidden> wrote:
> > From: "Dr. David Alan Gilbert" <address@hidden>
> >
> > I've converted the fields in it's main data structure
> > to fixed size types in ways that look sane, but I don't actually
> > know the details of this hardware.
>
> This is the kind of thing that should go below the '---',
> not in the commit message proper (at least not in this phrasing,
> in my opinion).
I can fix that.
> The TSC2005 datasheet is http://www.ti.com/lit/ds/symlink/tsc2005.pdf .
>
> > Signed-off-by: Dr. David Alan Gilbert <address@hidden>
> > ---
> > hw/input/tsc2005.c | 154
> > ++++++++++++++++++++---------------------------------
> > 1 file changed, 57 insertions(+), 97 deletions(-)
> >
> > diff --git a/hw/input/tsc2005.c b/hw/input/tsc2005.c
> > index 9b359aa..b66dc50 100644
> > --- a/hw/input/tsc2005.c
> > +++ b/hw/input/tsc2005.c
> > @@ -31,30 +31,31 @@ typedef struct {
> > QEMUTimer *timer;
> > uint16_t model;
> >
> > - int x, y;
> > - int pressure;
> > + int32_t x, y;
> > + bool pressure;
> >
> > - int state, reg, irq, command;
> > + uint8_t reg, state;
> > + bool irq, command;
> > uint16_t data, dav;
> >
> > - int busy;
> > - int enabled;
> > - int host_mode;
> > - int function;
> > - int nextfunction;
> > - int precision;
> > - int nextprecision;
> > - int filter;
> > - int pin_func;
> > - int timing[2];
> > - int noise;
> > - int reset;
> > - int pdst;
> > - int pnd0;
> > + bool busy;
> > + bool enabled;
>
> These changes mean the code is now doing bitwise-logic on
> bool variables, for instance:
> s->busy &= s->enabled;
> We also assign '0' and '1' to them rather than 'true' and 'false'.
Yes, I went through and as far as I can tell they're always
used as booleans already.
> > + bool host_mode;
> > + int8_t function;
> > + int8_t nextfunction;
> > + bool precision;
> > + bool nextprecision;
> > + uint16_t filter;
> > + uint8_t pin_func;
> > + int16_t timing[2];
>
> Why not uint16_t ?
Yep, I'll fix that.
> > + uint8_t noise;
> > + bool reset;
> > + bool pdst;
> > + bool pnd0;
> > uint16_t temp_thr[2];
> > uint16_t aux_thr[2];
> >
> > - int tr[8];
> > + int32_t tr[8];
> > } TSC2005State;
>
> Looks ok otherwise.
>
> thanks
> -- PMM
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK