[v1,1/3] applesmc: cosmetic whitespace and indentation cleanup

Submitted by Gabriel L. Somlo on March 31, 2017, 4:48 p.m.

Details

Message ID 1490978921-3782-2-git-send-email-gsomlo@gmail.com
State New
Headers show

Commit Message

Gabriel L. Somlo March 31, 2017, 4:48 p.m.
Signed-off-by: Gabriel Somlo <gsomlo@gmail.com>
---
 hw/misc/applesmc.c | 100 +++++++++++++++++++++++++++--------------------------
 1 file changed, 51 insertions(+), 49 deletions(-)

Comments

Alexander Graf April 3, 2017, 9:25 a.m.
On 03/31/2017 06:48 PM, Gabriel L. Somlo wrote:
> Signed-off-by: Gabriel Somlo <gsomlo@gmail.com>

Reviewed-by: Alexander Graf <agraf@suse.de>


Alex
Philippe Mathieu-Daudé April 3, 2017, 1:34 p.m.
Hi Gabriel,

On 03/31/2017 01:48 PM, Gabriel L. Somlo wrote:
> Signed-off-by: Gabriel Somlo <gsomlo@gmail.com>
> ---
>  hw/misc/applesmc.c | 100 +++++++++++++++++++++++++++--------------------------
>  1 file changed, 51 insertions(+), 49 deletions(-)
>
> diff --git a/hw/misc/applesmc.c b/hw/misc/applesmc.c
> index 77fab5b..986f2ac 100644
> --- a/hw/misc/applesmc.c
> +++ b/hw/misc/applesmc.c
> @@ -39,24 +39,27 @@
>  /* #define DEBUG_SMC */
>
>  #define APPLESMC_DEFAULT_IOBASE        0x300
> -/* data port used by Apple SMC */
> -#define APPLESMC_DATA_PORT             0x0
> -/* command/status port used by Apple SMC */
> -#define APPLESMC_CMD_PORT              0x4
> -#define APPLESMC_NR_PORTS              32
>
> -#define APPLESMC_READ_CMD              0x10
> -#define APPLESMC_WRITE_CMD             0x11
> -#define APPLESMC_GET_KEY_BY_INDEX_CMD  0x12
> -#define APPLESMC_GET_KEY_TYPE_CMD      0x13
> +enum {
> +    APPLESMC_DATA_PORT               = 0x00,
> +    APPLESMC_CMD_PORT                = 0x04,
> +    APPLESMC_NUM_PORTS               = 0x20,
> +};
> +
> +enum {
> +    APPLESMC_READ_CMD                = 0x10,
> +    APPLESMC_WRITE_CMD               = 0x11,
> +    APPLESMC_GET_KEY_BY_INDEX_CMD    = 0x12,
> +    APPLESMC_GET_KEY_TYPE_CMD        = 0x13,
> +};
>
>  #ifdef DEBUG_SMC
>  #define smc_debug(...) fprintf(stderr, "AppleSMC: " __VA_ARGS__)
>  #else
> -#define smc_debug(...) do { } while(0)
> +#define smc_debug(...) do { } while (0)
>  #endif
>
> -static char default_osk[64] = "This is a dummy key. Enter the real key "
> +static char default_osk[65] = "This is a dummy key. Enter the real key "

Here is more than a cosmetic change.

Since this array is initialized you can declare it without specify the size:
static char default_osk[] = ...

OSK0 + OSK1 is 64, why need an extra byte?

Can yoy add some self-explanatory #define and use them later in this file:

     applesmc_add_key(s, "OSK0", 32, s->osk);
     applesmc_add_key(s, "OSK1", 32, s->osk + 32);

and

     if (!s->osk || (strlen(s->osk) != 64)) {
         fprintf(stderr, "WARNING: Using AppleSMC with invalid key\n");


>                                "using the -osk parameter";
>
>  struct AppleSMCData {
> @@ -77,12 +80,11 @@ struct AppleSMCState {
>      uint32_t iobase;
>      uint8_t cmd;
>      uint8_t status;
> -    uint8_t key[4];
> +    char key[4];
>      uint8_t read_pos;
>      uint8_t data_len;
>      uint8_t data_pos;
>      uint8_t data[255];
> -    uint8_t charactic[4];
>      char *osk;
>      QLIST_HEAD(, AppleSMCData) data_def;
>  };
> @@ -93,10 +95,10 @@ static void applesmc_io_cmd_write(void *opaque, hwaddr addr, uint64_t val,
>      AppleSMCState *s = opaque;
>
>      smc_debug("CMD Write B: %#x = %#x\n", addr, val);
> -    switch(val) {
> -        case APPLESMC_READ_CMD:
> -            s->status = 0x0c;
> -            break;
> +    switch (val) {
> +    case APPLESMC_READ_CMD:
> +        s->status = 0x0c;
> +        break;
>      }
>      s->cmd = val;
>      s->read_pos = 0;
> @@ -123,54 +125,54 @@ static void applesmc_io_data_write(void *opaque, hwaddr addr, uint64_t val,
>      AppleSMCState *s = opaque;
>
>      smc_debug("DATA Write B: %#x = %#x\n", addr, val);
> -    switch(s->cmd) {
> -        case APPLESMC_READ_CMD:
> -            if(s->read_pos < 4) {
> -                s->key[s->read_pos] = val;
> -                s->status = 0x04;
> -            } else if(s->read_pos == 4) {
> -                s->data_len = val;
> -                s->status = 0x05;
> -                s->data_pos = 0;
> -                smc_debug("Key = %c%c%c%c Len = %d\n", s->key[0],
> -                          s->key[1], s->key[2], s->key[3], val);
> -                applesmc_fill_data(s);
> -            }
> -            s->read_pos++;
> -            break;
> +    switch (s->cmd) {
> +    case APPLESMC_READ_CMD:
> +        if (s->read_pos < 4) {
> +            s->key[s->read_pos] = val;
> +            s->status = 0x04;
> +        } else if (s->read_pos == 4) {
> +            s->data_len = val;
> +            s->status = 0x05;
> +            s->data_pos = 0;
> +            smc_debug("Key = %c%c%c%c Len = %d\n", s->key[0],
> +                      s->key[1], s->key[2], s->key[3], val);
> +            applesmc_fill_data(s);
> +        }
> +        s->read_pos++;
> +        break;
>      }
>  }
>
> -static uint64_t applesmc_io_data_read(void *opaque, hwaddr addr1,
> -                                      unsigned size)
> +static uint64_t applesmc_io_data_read(void *opaque, hwaddr addr, unsigned size)
>  {
>      AppleSMCState *s = opaque;
>      uint8_t retval = 0;
>
> -    switch(s->cmd) {
> -        case APPLESMC_READ_CMD:
> -            if(s->data_pos < s->data_len) {
> -                retval = s->data[s->data_pos];
> -                smc_debug("READ_DATA[%d] = %#hhx\n", s->data_pos,
> -                          retval);
> -                s->data_pos++;
> -                if(s->data_pos == s->data_len) {
> -                    s->status = 0x00;
> -                    smc_debug("EOF\n");
> -                } else
> -                    s->status = 0x05;
> +    switch (s->cmd) {
> +    case APPLESMC_READ_CMD:
> +        if (s->data_pos < s->data_len) {
> +            retval = s->data[s->data_pos];
> +            smc_debug("READ_DATA[%d] = %#hhx\n", s->data_pos,
> +                      retval);
> +            s->data_pos++;
> +            if (s->data_pos == s->data_len) {
> +                s->status = 0x00;
> +                smc_debug("EOF\n");
> +            } else {
> +                s->status = 0x05;
>              }
> +        }
>      }
> -    smc_debug("DATA Read b: %#x = %#x\n", addr1, retval);
> +    smc_debug("DATA Read b: %#x = %#x\n", addr, retval);
>
>      return retval;
>  }
>
> -static uint64_t applesmc_io_cmd_read(void *opaque, hwaddr addr1, unsigned size)
> +static uint64_t applesmc_io_cmd_read(void *opaque, hwaddr addr, unsigned size)
>  {
>      AppleSMCState *s = opaque;
>
> -    smc_debug("CMD Read B: %#x\n", addr1);
> +    smc_debug("CMD Read B: %#x\n", addr);
>      return s->status;
>  }
>
>
Gabriel L. Somlo April 3, 2017, 9:12 p.m.
On Mon, Apr 03, 2017 at 10:34:09AM -0300, Philippe Mathieu-Daudé wrote:
> Hi Gabriel,
> 
> On 03/31/2017 01:48 PM, Gabriel L. Somlo wrote:
> > Signed-off-by: Gabriel Somlo <gsomlo@gmail.com>
> > ---
> >  hw/misc/applesmc.c | 100 +++++++++++++++++++++++++++--------------------------
> >  1 file changed, 51 insertions(+), 49 deletions(-)
> > 
> > diff --git a/hw/misc/applesmc.c b/hw/misc/applesmc.c
> > index 77fab5b..986f2ac 100644
> > --- a/hw/misc/applesmc.c
> > +++ b/hw/misc/applesmc.c
> > @@ -39,24 +39,27 @@
> >  /* #define DEBUG_SMC */
> > 
> >  #define APPLESMC_DEFAULT_IOBASE        0x300
> > -/* data port used by Apple SMC */
> > -#define APPLESMC_DATA_PORT             0x0
> > -/* command/status port used by Apple SMC */
> > -#define APPLESMC_CMD_PORT              0x4
> > -#define APPLESMC_NR_PORTS              32
> > 
> > -#define APPLESMC_READ_CMD              0x10
> > -#define APPLESMC_WRITE_CMD             0x11
> > -#define APPLESMC_GET_KEY_BY_INDEX_CMD  0x12
> > -#define APPLESMC_GET_KEY_TYPE_CMD      0x13
> > +enum {
> > +    APPLESMC_DATA_PORT               = 0x00,
> > +    APPLESMC_CMD_PORT                = 0x04,
> > +    APPLESMC_NUM_PORTS               = 0x20,
> > +};
> > +
> > +enum {
> > +    APPLESMC_READ_CMD                = 0x10,
> > +    APPLESMC_WRITE_CMD               = 0x11,
> > +    APPLESMC_GET_KEY_BY_INDEX_CMD    = 0x12,
> > +    APPLESMC_GET_KEY_TYPE_CMD        = 0x13,
> > +};
> > 
> >  #ifdef DEBUG_SMC
> >  #define smc_debug(...) fprintf(stderr, "AppleSMC: " __VA_ARGS__)
> >  #else
> > -#define smc_debug(...) do { } while(0)
> > +#define smc_debug(...) do { } while (0)
> >  #endif
> > 
> > -static char default_osk[64] = "This is a dummy key. Enter the real key "
> > +static char default_osk[65] = "This is a dummy key. Enter the real key "
> 
> Here is more than a cosmetic change.

You're right, that was my mistake (should have left it 64.
> 
> Since this array is initialized you can declare it without specify the size:
> static char default_osk[] = ...

It's initialized to something that's not necessarily 64 characters, so
(I assume) the size specification is there to reserve the appropriate
amount of space, which is precisely 64 bytes.

> OSK0 + OSK1 is 64, why need an extra byte?

Right, I undid that part of the patch, will resubmit everything minus
that hunk as part of v2.

> Can yoy add some self-explanatory #define and use them later in this file:
> 
>     applesmc_add_key(s, "OSK0", 32, s->osk);
>     applesmc_add_key(s, "OSK1", 32, s->osk + 32);
> 
> and
> 
>     if (!s->osk || (strlen(s->osk) != 64)) {
>         fprintf(stderr, "WARNING: Using AppleSMC with invalid key\n");

Do I have to, now that I'm staying away from touching that bit in the
first place ? :)

The original patch Eric Shelton proposed (inspired by FakeSMC)
completely reworks how keys are initialized, adds write support (and
access permissions) for select keys, and generally implements a much
more complete emulation of the AppleSMC. Since this is just about
getting OS X to keep working, I figured I'd keep changes to a minimum.

Reworking key initialization, adding write support, etc. could
(should?) be part of a separate series, if we decide we want a more
realistically emulated AppleSMC. In that case, minimizing churn and
leaving things as-is in this particular case would be preferable.

Let me know what you think.

Thanks,
--Gabriel
 
> 
> >                                "using the -osk parameter";
> > 
> >  struct AppleSMCData {
> > @@ -77,12 +80,11 @@ struct AppleSMCState {
> >      uint32_t iobase;
> >      uint8_t cmd;
> >      uint8_t status;
> > -    uint8_t key[4];
> > +    char key[4];
> >      uint8_t read_pos;
> >      uint8_t data_len;
> >      uint8_t data_pos;
> >      uint8_t data[255];
> > -    uint8_t charactic[4];
> >      char *osk;
> >      QLIST_HEAD(, AppleSMCData) data_def;
> >  };
> > @@ -93,10 +95,10 @@ static void applesmc_io_cmd_write(void *opaque, hwaddr addr, uint64_t val,
> >      AppleSMCState *s = opaque;
> > 
> >      smc_debug("CMD Write B: %#x = %#x\n", addr, val);
> > -    switch(val) {
> > -        case APPLESMC_READ_CMD:
> > -            s->status = 0x0c;
> > -            break;
> > +    switch (val) {
> > +    case APPLESMC_READ_CMD:
> > +        s->status = 0x0c;
> > +        break;
> >      }
> >      s->cmd = val;
> >      s->read_pos = 0;
> > @@ -123,54 +125,54 @@ static void applesmc_io_data_write(void *opaque, hwaddr addr, uint64_t val,
> >      AppleSMCState *s = opaque;
> > 
> >      smc_debug("DATA Write B: %#x = %#x\n", addr, val);
> > -    switch(s->cmd) {
> > -        case APPLESMC_READ_CMD:
> > -            if(s->read_pos < 4) {
> > -                s->key[s->read_pos] = val;
> > -                s->status = 0x04;
> > -            } else if(s->read_pos == 4) {
> > -                s->data_len = val;
> > -                s->status = 0x05;
> > -                s->data_pos = 0;
> > -                smc_debug("Key = %c%c%c%c Len = %d\n", s->key[0],
> > -                          s->key[1], s->key[2], s->key[3], val);
> > -                applesmc_fill_data(s);
> > -            }
> > -            s->read_pos++;
> > -            break;
> > +    switch (s->cmd) {
> > +    case APPLESMC_READ_CMD:
> > +        if (s->read_pos < 4) {
> > +            s->key[s->read_pos] = val;
> > +            s->status = 0x04;
> > +        } else if (s->read_pos == 4) {
> > +            s->data_len = val;
> > +            s->status = 0x05;
> > +            s->data_pos = 0;
> > +            smc_debug("Key = %c%c%c%c Len = %d\n", s->key[0],
> > +                      s->key[1], s->key[2], s->key[3], val);
> > +            applesmc_fill_data(s);
> > +        }
> > +        s->read_pos++;
> > +        break;
> >      }
> >  }
> > 
> > -static uint64_t applesmc_io_data_read(void *opaque, hwaddr addr1,
> > -                                      unsigned size)
> > +static uint64_t applesmc_io_data_read(void *opaque, hwaddr addr, unsigned size)
> >  {
> >      AppleSMCState *s = opaque;
> >      uint8_t retval = 0;
> > 
> > -    switch(s->cmd) {
> > -        case APPLESMC_READ_CMD:
> > -            if(s->data_pos < s->data_len) {
> > -                retval = s->data[s->data_pos];
> > -                smc_debug("READ_DATA[%d] = %#hhx\n", s->data_pos,
> > -                          retval);
> > -                s->data_pos++;
> > -                if(s->data_pos == s->data_len) {
> > -                    s->status = 0x00;
> > -                    smc_debug("EOF\n");
> > -                } else
> > -                    s->status = 0x05;
> > +    switch (s->cmd) {
> > +    case APPLESMC_READ_CMD:
> > +        if (s->data_pos < s->data_len) {
> > +            retval = s->data[s->data_pos];
> > +            smc_debug("READ_DATA[%d] = %#hhx\n", s->data_pos,
> > +                      retval);
> > +            s->data_pos++;
> > +            if (s->data_pos == s->data_len) {
> > +                s->status = 0x00;
> > +                smc_debug("EOF\n");
> > +            } else {
> > +                s->status = 0x05;
> >              }
> > +        }
> >      }
> > -    smc_debug("DATA Read b: %#x = %#x\n", addr1, retval);
> > +    smc_debug("DATA Read b: %#x = %#x\n", addr, retval);
> > 
> >      return retval;
> >  }
> > 
> > -static uint64_t applesmc_io_cmd_read(void *opaque, hwaddr addr1, unsigned size)
> > +static uint64_t applesmc_io_cmd_read(void *opaque, hwaddr addr, unsigned size)
> >  {
> >      AppleSMCState *s = opaque;
> > 
> > -    smc_debug("CMD Read B: %#x\n", addr1);
> > +    smc_debug("CMD Read B: %#x\n", addr);
> >      return s->status;
> >  }
> > 
> >
Philippe Mathieu-Daudé April 3, 2017, 9:37 p.m.
On 04/03/2017 06:12 PM, Gabriel L. Somlo wrote:
> On Mon, Apr 03, 2017 at 10:34:09AM -0300, Philippe Mathieu-Daudé wrote:
>> Hi Gabriel,
>>
>> On 03/31/2017 01:48 PM, Gabriel L. Somlo wrote:
>>> Signed-off-by: Gabriel Somlo <gsomlo@gmail.com>
>>> ---
>>>  hw/misc/applesmc.c | 100 +++++++++++++++++++++++++++--------------------------
>>>  1 file changed, 51 insertions(+), 49 deletions(-)
>>>
>>> diff --git a/hw/misc/applesmc.c b/hw/misc/applesmc.c
>>> index 77fab5b..986f2ac 100644
>>> --- a/hw/misc/applesmc.c
>>> +++ b/hw/misc/applesmc.c
>>> @@ -39,24 +39,27 @@
>>>  /* #define DEBUG_SMC */
>>>
>>>  #define APPLESMC_DEFAULT_IOBASE        0x300
>>> -/* data port used by Apple SMC */
>>> -#define APPLESMC_DATA_PORT             0x0
>>> -/* command/status port used by Apple SMC */
>>> -#define APPLESMC_CMD_PORT              0x4
>>> -#define APPLESMC_NR_PORTS              32
>>>
>>> -#define APPLESMC_READ_CMD              0x10
>>> -#define APPLESMC_WRITE_CMD             0x11
>>> -#define APPLESMC_GET_KEY_BY_INDEX_CMD  0x12
>>> -#define APPLESMC_GET_KEY_TYPE_CMD      0x13
>>> +enum {
>>> +    APPLESMC_DATA_PORT               = 0x00,
>>> +    APPLESMC_CMD_PORT                = 0x04,
>>> +    APPLESMC_NUM_PORTS               = 0x20,
>>> +};
>>> +
>>> +enum {
>>> +    APPLESMC_READ_CMD                = 0x10,
>>> +    APPLESMC_WRITE_CMD               = 0x11,
>>> +    APPLESMC_GET_KEY_BY_INDEX_CMD    = 0x12,
>>> +    APPLESMC_GET_KEY_TYPE_CMD        = 0x13,
>>> +};
>>>
>>>  #ifdef DEBUG_SMC
>>>  #define smc_debug(...) fprintf(stderr, "AppleSMC: " __VA_ARGS__)
>>>  #else
>>> -#define smc_debug(...) do { } while(0)
>>> +#define smc_debug(...) do { } while (0)
>>>  #endif
>>>
>>> -static char default_osk[64] = "This is a dummy key. Enter the real key "
>>> +static char default_osk[65] = "This is a dummy key. Enter the real key "
>>
>> Here is more than a cosmetic change.
>
> You're right, that was my mistake (should have left it 64.
>>
>> Since this array is initialized you can declare it without specify the size:
>> static char default_osk[] = ...
>
> It's initialized to something that's not necessarily 64 characters, so
> (I assume) the size specification is there to reserve the appropriate
> amount of space, which is precisely 64 bytes.

Since 'default_osk' is used read-only, it could be 'const' but since it 
is assigned as `s->osk = default_osk` which is a PROP_STRING it can not 
be const. For this read-only reason I think it is safer to let the cpp 
calculate the size to allocate to this buffer (without specifying 64 
which lead to confusion).

>
>> OSK0 + OSK1 is 64, why need an extra byte?
>
> Right, I undid that part of the patch, will resubmit everything minus
> that hunk as part of v2.
>
>> Can yoy add some self-explanatory #define and use them later in this file:
>>
>>     applesmc_add_key(s, "OSK0", 32, s->osk);
>>     applesmc_add_key(s, "OSK1", 32, s->osk + 32);
>>
>> and
>>
>>     if (!s->osk || (strlen(s->osk) != 64)) {
>>         fprintf(stderr, "WARNING: Using AppleSMC with invalid key\n");
>
> Do I have to, now that I'm staying away from touching that bit in the
> first place ? :)
>
> The original patch Eric Shelton proposed (inspired by FakeSMC)
> completely reworks how keys are initialized, adds write support (and
> access permissions) for select keys, and generally implements a much
> more complete emulation of the AppleSMC. Since this is just about
> getting OS X to keep working, I figured I'd keep changes to a minimum.
>
> Reworking key initialization, adding write support, etc. could
> (should?) be part of a separate series, if we decide we want a more
> realistically emulated AppleSMC. In that case, minimizing churn and
> leaving things as-is in this particular case would be preferable.
>
> Let me know what you think.

Let keep it simple enough for this serie :)

Either with the 64B array size back or without:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

>
> Thanks,
> --Gabriel
>
>>
>>>                                "using the -osk parameter";
>>>
>>>  struct AppleSMCData {
>>> @@ -77,12 +80,11 @@ struct AppleSMCState {
>>>      uint32_t iobase;
>>>      uint8_t cmd;
>>>      uint8_t status;
>>> -    uint8_t key[4];
>>> +    char key[4];
>>>      uint8_t read_pos;
>>>      uint8_t data_len;
>>>      uint8_t data_pos;
>>>      uint8_t data[255];
>>> -    uint8_t charactic[4];
>>>      char *osk;
>>>      QLIST_HEAD(, AppleSMCData) data_def;
>>>  };
>>> @@ -93,10 +95,10 @@ static void applesmc_io_cmd_write(void *opaque, hwaddr addr, uint64_t val,
>>>      AppleSMCState *s = opaque;
>>>
>>>      smc_debug("CMD Write B: %#x = %#x\n", addr, val);
>>> -    switch(val) {
>>> -        case APPLESMC_READ_CMD:
>>> -            s->status = 0x0c;
>>> -            break;
>>> +    switch (val) {
>>> +    case APPLESMC_READ_CMD:
>>> +        s->status = 0x0c;
>>> +        break;
>>>      }
>>>      s->cmd = val;
>>>      s->read_pos = 0;
>>> @@ -123,54 +125,54 @@ static void applesmc_io_data_write(void *opaque, hwaddr addr, uint64_t val,
>>>      AppleSMCState *s = opaque;
>>>
>>>      smc_debug("DATA Write B: %#x = %#x\n", addr, val);
>>> -    switch(s->cmd) {
>>> -        case APPLESMC_READ_CMD:
>>> -            if(s->read_pos < 4) {
>>> -                s->key[s->read_pos] = val;
>>> -                s->status = 0x04;
>>> -            } else if(s->read_pos == 4) {
>>> -                s->data_len = val;
>>> -                s->status = 0x05;
>>> -                s->data_pos = 0;
>>> -                smc_debug("Key = %c%c%c%c Len = %d\n", s->key[0],
>>> -                          s->key[1], s->key[2], s->key[3], val);
>>> -                applesmc_fill_data(s);
>>> -            }
>>> -            s->read_pos++;
>>> -            break;
>>> +    switch (s->cmd) {
>>> +    case APPLESMC_READ_CMD:
>>> +        if (s->read_pos < 4) {
>>> +            s->key[s->read_pos] = val;
>>> +            s->status = 0x04;
>>> +        } else if (s->read_pos == 4) {
>>> +            s->data_len = val;
>>> +            s->status = 0x05;
>>> +            s->data_pos = 0;
>>> +            smc_debug("Key = %c%c%c%c Len = %d\n", s->key[0],
>>> +                      s->key[1], s->key[2], s->key[3], val);
>>> +            applesmc_fill_data(s);
>>> +        }
>>> +        s->read_pos++;
>>> +        break;
>>>      }
>>>  }
>>>
>>> -static uint64_t applesmc_io_data_read(void *opaque, hwaddr addr1,
>>> -                                      unsigned size)
>>> +static uint64_t applesmc_io_data_read(void *opaque, hwaddr addr, unsigned size)
>>>  {
>>>      AppleSMCState *s = opaque;
>>>      uint8_t retval = 0;
>>>
>>> -    switch(s->cmd) {
>>> -        case APPLESMC_READ_CMD:
>>> -            if(s->data_pos < s->data_len) {
>>> -                retval = s->data[s->data_pos];
>>> -                smc_debug("READ_DATA[%d] = %#hhx\n", s->data_pos,
>>> -                          retval);
>>> -                s->data_pos++;
>>> -                if(s->data_pos == s->data_len) {
>>> -                    s->status = 0x00;
>>> -                    smc_debug("EOF\n");
>>> -                } else
>>> -                    s->status = 0x05;
>>> +    switch (s->cmd) {
>>> +    case APPLESMC_READ_CMD:
>>> +        if (s->data_pos < s->data_len) {
>>> +            retval = s->data[s->data_pos];
>>> +            smc_debug("READ_DATA[%d] = %#hhx\n", s->data_pos,
>>> +                      retval);
>>> +            s->data_pos++;
>>> +            if (s->data_pos == s->data_len) {
>>> +                s->status = 0x00;
>>> +                smc_debug("EOF\n");
>>> +            } else {
>>> +                s->status = 0x05;
>>>              }
>>> +        }
>>>      }
>>> -    smc_debug("DATA Read b: %#x = %#x\n", addr1, retval);
>>> +    smc_debug("DATA Read b: %#x = %#x\n", addr, retval);
>>>
>>>      return retval;
>>>  }
>>>
>>> -static uint64_t applesmc_io_cmd_read(void *opaque, hwaddr addr1, unsigned size)
>>> +static uint64_t applesmc_io_cmd_read(void *opaque, hwaddr addr, unsigned size)
>>>  {
>>>      AppleSMCState *s = opaque;
>>>
>>> -    smc_debug("CMD Read B: %#x\n", addr1);
>>> +    smc_debug("CMD Read B: %#x\n", addr);
>>>      return s->status;
>>>  }
>>>
>>>

Patch hide | download patch | download mbox

diff --git a/hw/misc/applesmc.c b/hw/misc/applesmc.c
index 77fab5b..986f2ac 100644
--- a/hw/misc/applesmc.c
+++ b/hw/misc/applesmc.c
@@ -39,24 +39,27 @@ 
 /* #define DEBUG_SMC */
 
 #define APPLESMC_DEFAULT_IOBASE        0x300
-/* data port used by Apple SMC */
-#define APPLESMC_DATA_PORT             0x0
-/* command/status port used by Apple SMC */
-#define APPLESMC_CMD_PORT              0x4
-#define APPLESMC_NR_PORTS              32
 
-#define APPLESMC_READ_CMD              0x10
-#define APPLESMC_WRITE_CMD             0x11
-#define APPLESMC_GET_KEY_BY_INDEX_CMD  0x12
-#define APPLESMC_GET_KEY_TYPE_CMD      0x13
+enum {
+    APPLESMC_DATA_PORT               = 0x00,
+    APPLESMC_CMD_PORT                = 0x04,
+    APPLESMC_NUM_PORTS               = 0x20,
+};
+
+enum {
+    APPLESMC_READ_CMD                = 0x10,
+    APPLESMC_WRITE_CMD               = 0x11,
+    APPLESMC_GET_KEY_BY_INDEX_CMD    = 0x12,
+    APPLESMC_GET_KEY_TYPE_CMD        = 0x13,
+};
 
 #ifdef DEBUG_SMC
 #define smc_debug(...) fprintf(stderr, "AppleSMC: " __VA_ARGS__)
 #else
-#define smc_debug(...) do { } while(0)
+#define smc_debug(...) do { } while (0)
 #endif
 
-static char default_osk[64] = "This is a dummy key. Enter the real key "
+static char default_osk[65] = "This is a dummy key. Enter the real key "
                               "using the -osk parameter";
 
 struct AppleSMCData {
@@ -77,12 +80,11 @@  struct AppleSMCState {
     uint32_t iobase;
     uint8_t cmd;
     uint8_t status;
-    uint8_t key[4];
+    char key[4];
     uint8_t read_pos;
     uint8_t data_len;
     uint8_t data_pos;
     uint8_t data[255];
-    uint8_t charactic[4];
     char *osk;
     QLIST_HEAD(, AppleSMCData) data_def;
 };
@@ -93,10 +95,10 @@  static void applesmc_io_cmd_write(void *opaque, hwaddr addr, uint64_t val,
     AppleSMCState *s = opaque;
 
     smc_debug("CMD Write B: %#x = %#x\n", addr, val);
-    switch(val) {
-        case APPLESMC_READ_CMD:
-            s->status = 0x0c;
-            break;
+    switch (val) {
+    case APPLESMC_READ_CMD:
+        s->status = 0x0c;
+        break;
     }
     s->cmd = val;
     s->read_pos = 0;
@@ -123,54 +125,54 @@  static void applesmc_io_data_write(void *opaque, hwaddr addr, uint64_t val,
     AppleSMCState *s = opaque;
 
     smc_debug("DATA Write B: %#x = %#x\n", addr, val);
-    switch(s->cmd) {
-        case APPLESMC_READ_CMD:
-            if(s->read_pos < 4) {
-                s->key[s->read_pos] = val;
-                s->status = 0x04;
-            } else if(s->read_pos == 4) {
-                s->data_len = val;
-                s->status = 0x05;
-                s->data_pos = 0;
-                smc_debug("Key = %c%c%c%c Len = %d\n", s->key[0],
-                          s->key[1], s->key[2], s->key[3], val);
-                applesmc_fill_data(s);
-            }
-            s->read_pos++;
-            break;
+    switch (s->cmd) {
+    case APPLESMC_READ_CMD:
+        if (s->read_pos < 4) {
+            s->key[s->read_pos] = val;
+            s->status = 0x04;
+        } else if (s->read_pos == 4) {
+            s->data_len = val;
+            s->status = 0x05;
+            s->data_pos = 0;
+            smc_debug("Key = %c%c%c%c Len = %d\n", s->key[0],
+                      s->key[1], s->key[2], s->key[3], val);
+            applesmc_fill_data(s);
+        }
+        s->read_pos++;
+        break;
     }
 }
 
-static uint64_t applesmc_io_data_read(void *opaque, hwaddr addr1,
-                                      unsigned size)
+static uint64_t applesmc_io_data_read(void *opaque, hwaddr addr, unsigned size)
 {
     AppleSMCState *s = opaque;
     uint8_t retval = 0;
 
-    switch(s->cmd) {
-        case APPLESMC_READ_CMD:
-            if(s->data_pos < s->data_len) {
-                retval = s->data[s->data_pos];
-                smc_debug("READ_DATA[%d] = %#hhx\n", s->data_pos,
-                          retval);
-                s->data_pos++;
-                if(s->data_pos == s->data_len) {
-                    s->status = 0x00;
-                    smc_debug("EOF\n");
-                } else
-                    s->status = 0x05;
+    switch (s->cmd) {
+    case APPLESMC_READ_CMD:
+        if (s->data_pos < s->data_len) {
+            retval = s->data[s->data_pos];
+            smc_debug("READ_DATA[%d] = %#hhx\n", s->data_pos,
+                      retval);
+            s->data_pos++;
+            if (s->data_pos == s->data_len) {
+                s->status = 0x00;
+                smc_debug("EOF\n");
+            } else {
+                s->status = 0x05;
             }
+        }
     }
-    smc_debug("DATA Read b: %#x = %#x\n", addr1, retval);
+    smc_debug("DATA Read b: %#x = %#x\n", addr, retval);
 
     return retval;
 }
 
-static uint64_t applesmc_io_cmd_read(void *opaque, hwaddr addr1, unsigned size)
+static uint64_t applesmc_io_cmd_read(void *opaque, hwaddr addr, unsigned size)
 {
     AppleSMCState *s = opaque;
 
-    smc_debug("CMD Read B: %#x\n", addr1);
+    smc_debug("CMD Read B: %#x\n", addr);
     return s->status;
 }