qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH target-arm v1 07/15] net: cadence_gem: Split sta


From: Peter Crosthwaite
Subject: Re: [Qemu-devel] [PATCH target-arm v1 07/15] net: cadence_gem: Split state struct and type into header
Date: Mon, 2 Mar 2015 14:24:51 -0800

On Thu, Feb 26, 2015 at 7:12 PM, Alistair Francis
<address@hidden> wrote:
> On Tue, Feb 24, 2015 at 9:04 AM, Peter Crosthwaite
> <address@hidden> wrote:
>> To allow using the device with modern SoC programming conventions. The
>> state struct needs to be visible to embed the device in SoC containers.
>>
>> Signed-off-by: Peter Crosthwaite <address@hidden>
>> ---
>>  hw/net/cadence_gem.c         | 43 +-------------------------------------
>>  include/hw/net/cadence_gem.h | 49 
>> ++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 50 insertions(+), 42 deletions(-)
>>  create mode 100644 include/hw/net/cadence_gem.h
>>
>> diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
>> index 5994306..dafe914 100644
>> --- a/hw/net/cadence_gem.c
>> +++ b/hw/net/cadence_gem.c
>> @@ -24,8 +24,7 @@
>>
>>  #include <zlib.h> /* For crc32 */
>>
>> -#include "hw/sysbus.h"
>> -#include "net/net.h"
>> +#include "hw/net/cadence_gem.h"
>>  #include "net/checksum.h"
>>
>>  #ifdef CADENCE_GEM_ERR_DEBUG
>> @@ -141,8 +140,6 @@
>>  #define GEM_DESCONF6      (0x00000294/4)
>>  #define GEM_DESCONF7      (0x00000298/4)
>>
>> -#define CADENCE_GEM_MAXREG        (0x00000640/4) /* Last valid GEM address 
>> */
>> -
>>  /*****************************************/
>>  #define GEM_NWCTRL_TXSTART     0x00000200 /* Transmit Enable */
>>  #define GEM_NWCTRL_TXENA       0x00000008 /* Transmit Enable */
>> @@ -349,44 +346,6 @@ static inline void rx_desc_set_sar(unsigned *desc, int 
>> sar_idx)
>>      desc[1] |= R_DESC_1_RX_SAR_MATCH;
>>  }
>>
>> -#define TYPE_CADENCE_GEM "cadence_gem"
>> -#define CADENCE_GEM(obj) OBJECT_CHECK(CadenceGEMState, (obj), 
>> TYPE_CADENCE_GEM)
>> -
>> -typedef struct CadenceGEMState {
>> -    SysBusDevice parent_obj;
>> -
>> -    MemoryRegion iomem;
>> -    NICState *nic;
>> -    NICConf conf;
>> -    qemu_irq irq;
>> -
>> -    /* GEM registers backing store */
>> -    uint32_t regs[CADENCE_GEM_MAXREG];
>> -    /* Mask of register bits which are write only */
>> -    uint32_t regs_wo[CADENCE_GEM_MAXREG];
>> -    /* Mask of register bits which are read only */
>> -    uint32_t regs_ro[CADENCE_GEM_MAXREG];
>> -    /* Mask of register bits which are clear on read */
>> -    uint32_t regs_rtc[CADENCE_GEM_MAXREG];
>> -    /* Mask of register bits which are write 1 to clear */
>> -    uint32_t regs_w1c[CADENCE_GEM_MAXREG];
>> -
>> -    /* PHY registers backing store */
>> -    uint16_t phy_regs[32];
>> -
>> -    uint8_t phy_loop; /* Are we in phy loopback? */
>> -
>> -    /* The current DMA descriptor pointers */
>> -    uint32_t rx_desc_addr;
>> -    uint32_t tx_desc_addr;
>> -
>> -    uint8_t can_rx_state; /* Debug only */
>> -
>> -    unsigned rx_desc[2];
>> -
>> -    bool sar_active[4];
>> -} CadenceGEMState;
>> -
>>  /* The broadcast MAC address: 0xFFFFFFFFFFFF */
>>  static const uint8_t broadcast_addr[] = { 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 
>> 0xFF };
>>
>> diff --git a/include/hw/net/cadence_gem.h b/include/hw/net/cadence_gem.h
>> new file mode 100644
>> index 0000000..e6413ff
>> --- /dev/null
>> +++ b/include/hw/net/cadence_gem.h
>> @@ -0,0 +1,49 @@
>> +#ifndef CADENCE_GEM_H_
>> +
>> +#define TYPE_CADENCE_GEM "cadence_gem"
>> +#define CADENCE_GEM(obj) OBJECT_CHECK(CadenceGEMState, (obj), 
>> TYPE_CADENCE_GEM)
>> +
>> +#include "net/net.h"
>> +#include "hw/sysbus.h"
>> +
>> +#define CADENCE_GEM_MAXREG        (0x00000640/4) /* Last valid GEM address 
>> */
>> +
>> +typedef struct CadenceGEMState {
>> +    /*< private >*/
>> +    SysBusDevice parent_obj;
>> +    /*< public >*/
>> +
>> +    MemoryRegion iomem;
>
> Nit pic, I think it looks cleaner like this:
>     /*< private >*/
>     SysBusDevice parent_obj;
>
>     /*< public >*/
>     MemoryRegion iomem;
>

Fixed.

> but it doesn't really matter
>
> Reviewed-by: Alistair Francis <address@hidden>
>

Thanks.

Regards,
Peter

> Thanks,
>
> Alistair
>
>> +    NICState *nic;
>> +    NICConf conf;
>> +    qemu_irq irq;
>> +
>> +    /* GEM registers backing store */
>> +    uint32_t regs[CADENCE_GEM_MAXREG];
>> +    /* Mask of register bits which are write only */
>> +    uint32_t regs_wo[CADENCE_GEM_MAXREG];
>> +    /* Mask of register bits which are read only */
>> +    uint32_t regs_ro[CADENCE_GEM_MAXREG];
>> +    /* Mask of register bits which are clear on read */
>> +    uint32_t regs_rtc[CADENCE_GEM_MAXREG];
>> +    /* Mask of register bits which are write 1 to clear */
>> +    uint32_t regs_w1c[CADENCE_GEM_MAXREG];
>> +
>> +    /* PHY registers backing store */
>> +    uint16_t phy_regs[32];
>> +
>> +    uint8_t phy_loop; /* Are we in phy loopback? */
>> +
>> +    /* The current DMA descriptor pointers */
>> +    uint32_t rx_desc_addr;
>> +    uint32_t tx_desc_addr;
>> +
>> +    uint8_t can_rx_state; /* Debug only */
>> +
>> +    unsigned rx_desc[2];
>> +
>> +    bool sar_active[4];
>> +} CadenceGEMState;
>> +
>> +#define CADENCE_GEM_H_
>> +#endif
>> --
>> 2.3.0.1.g27a12f1
>>
>>
>



reply via email to

[Prev in Thread] Current Thread [Next in Thread]