Re: [Qemu-devel] [PATCH v1 1/3] ssi: Move ssi.h into a seperate directory

[Alistair Francis]

On Thu, Oct 1, 2015 at 10:39 AM, Peter Crosthwaite
<address@hidden> wrote:
> On Thu, Oct 1, 2015 at 10:19 AM, Alistair Francis
> <address@hidden> wrote:
>> On Wed, Sep 30, 2015 at 2:06 PM, Peter Crosthwaite
>> <address@hidden> wrote:
>>> On Tue, Sep 29, 2015 at 4:03 PM, Alistair Francis
>>> <address@hidden> wrote:
>>>> Move the ssi.h include file into the ssi directory.
>>>>
>>>> Signed-off-by: Alistair Francis <address@hidden>
>>>> ---
>>>>
>>>>  hw/arm/pxa2xx.c                     |  2 +-
>>>>  hw/arm/spitz.c                      |  2 +-
>>>>  hw/arm/stellaris.c                  |  2 +-
>>>>  hw/arm/strongarm.c                  |  2 +-
>>>>  hw/arm/tosa.c                       |  2 +-
>>>>  hw/arm/xilinx_zynq.c                |  2 +-
>>>>  hw/arm/z2.c                         |  2 +-
>>>>  hw/block/m25p80.c                   |  2 +-
>>>>  hw/display/ads7846.c                |  2 +-
>>>>  hw/display/ssd0323.c                |  2 +-
>>>>  hw/microblaze/petalogix_ml605_mmu.c |  2 +-
>>>>  hw/misc/max111x.c                   |  2 +-
>>>>  hw/sd/ssi-sd.c                      |  2 +-
>>>>  hw/ssi/pl022.c                      |  2 +-
>>>>  hw/ssi/ssi.c                        |  2 +-
>>>>  hw/ssi/xilinx_spi.c                 |  2 +-
>>>>  hw/ssi/xilinx_spips.c               |  2 +-
>>>>  include/hw/ssi.h                    | 94 
>>>> ------------------------------------
>>>>  include/hw/ssi/ssi.h                | 96 
>>>> +++++++++++++++++++++++++++++++++++++
>>>
>>> Curious as to why git didn't pick this up as a rename (I think it
>>> should handle small diffs in the renaming). Do you have rename
>>> detection turned on?
>>
>> I do have rename detection turned on. I think the reason it didn't is
>> because I tiddied up the code a little bit when I moved it. Checkpatch
>> was reproting errors on the struct typedeffs so I had to change that.
>>
>
> Can we do it in two steps and take the hit on the checkpatch fails on
> P1? Checkpatch fails are acceptable for pure mass-code movements.
> There is also probably some way to set your rename detction to higher
> level that should pick it up.

I managed to change the rename detection (-M for git format-patch) and
now the diff passes checkpatch and indicates a rename.

Thanks,

Alistair

>
> Regards,
> Peter
>
>> Thanks,
>>
>> Alistair
>>
>>>
>>> Regards,
>>> Peter
>>>
>>>>  19 files changed, 113 insertions(+), 111 deletions(-)
>>>>  delete mode 100644 include/hw/ssi.h
>>>>  create mode 100644 include/hw/ssi/ssi.h
>>>>
>>>> diff --git a/hw/arm/pxa2xx.c b/hw/arm/pxa2xx.c
>>>> index 164260a..534c06f 100644
>>>> --- a/hw/arm/pxa2xx.c
>>>> +++ b/hw/arm/pxa2xx.c
>>>> @@ -12,7 +12,7 @@
>>>>  #include "sysemu/sysemu.h"
>>>>  #include "hw/char/serial.h"
>>>>  #include "hw/i2c/i2c.h"
>>>> -#include "hw/ssi.h"
>>>> +#include "hw/ssi/ssi.h"
>>>>  #include "sysemu/char.h"
>>>>  #include "sysemu/block-backend.h"
>>>>  #include "sysemu/blockdev.h"
>>>> diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
>>>> index 2af03be..c9405af 100644
>>>> --- a/hw/arm/spitz.c
>>>> +++ b/hw/arm/spitz.c
>>>> @@ -16,7 +16,7 @@
>>>>  #include "sysemu/sysemu.h"
>>>>  #include "hw/pcmcia.h"
>>>>  #include "hw/i2c/i2c.h"
>>>> -#include "hw/ssi.h"
>>>> +#include "hw/ssi/ssi.h"
>>>>  #include "hw/block/flash.h"
>>>>  #include "qemu/timer.h"
>>>>  #include "hw/devices.h"
>>>> diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c
>>>> index 3d6486f..c785e90 100644
>>>> --- a/hw/arm/stellaris.c
>>>> +++ b/hw/arm/stellaris.c
>>>> @@ -8,7 +8,7 @@
>>>>   */
>>>>
>>>>  #include "hw/sysbus.h"
>>>> -#include "hw/ssi.h"
>>>> +#include "hw/ssi/ssi.h"
>>>>  #include "hw/arm/arm.h"
>>>>  #include "hw/devices.h"
>>>>  #include "qemu/timer.h"
>>>> diff --git a/hw/arm/strongarm.c b/hw/arm/strongarm.c
>>>> index 9624ecb..4d2ba02 100644
>>>> --- a/hw/arm/strongarm.c
>>>> +++ b/hw/arm/strongarm.c
>>>> @@ -34,7 +34,7 @@
>>>>  #include "hw/arm/arm.h"
>>>>  #include "sysemu/char.h"
>>>>  #include "sysemu/sysemu.h"
>>>> -#include "hw/ssi.h"
>>>> +#include "hw/ssi/ssi.h"
>>>>
>>>>  //#define DEBUG
>>>>
>>>> diff --git a/hw/arm/tosa.c b/hw/arm/tosa.c
>>>> index 51d0b89..6b210a6 100644
>>>> --- a/hw/arm/tosa.c
>>>> +++ b/hw/arm/tosa.c
>>>> @@ -19,7 +19,7 @@
>>>>  #include "hw/pcmcia.h"
>>>>  #include "hw/boards.h"
>>>>  #include "hw/i2c/i2c.h"
>>>> -#include "hw/ssi.h"
>>>> +#include "hw/ssi/ssi.h"
>>>>  #include "sysemu/block-backend.h"
>>>>  #include "hw/sysbus.h"
>>>>  #include "exec/address-spaces.h"
>>>> diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
>>>> index 9f89483..9db9602 100644
>>>> --- a/hw/arm/xilinx_zynq.c
>>>> +++ b/hw/arm/xilinx_zynq.c
>>>> @@ -24,7 +24,7 @@
>>>>  #include "hw/block/flash.h"
>>>>  #include "sysemu/block-backend.h"
>>>>  #include "hw/loader.h"
>>>> -#include "hw/ssi.h"
>>>> +#include "hw/ssi/ssi.h"
>>>>  #include "qemu/error-report.h"
>>>>
>>>>  #define NUM_SPI_FLASHES 4
>>>> diff --git a/hw/arm/z2.c b/hw/arm/z2.c
>>>> index b44eb76..c82fe2c 100644
>>>> --- a/hw/arm/z2.c
>>>> +++ b/hw/arm/z2.c
>>>> @@ -16,7 +16,7 @@
>>>>  #include "hw/arm/arm.h"
>>>>  #include "hw/devices.h"
>>>>  #include "hw/i2c/i2c.h"
>>>> -#include "hw/ssi.h"
>>>> +#include "hw/ssi/ssi.h"
>>>>  #include "hw/boards.h"
>>>>  #include "sysemu/sysemu.h"
>>>>  #include "hw/block/flash.h"
>>>> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
>>>> index efc43dd..44830c7 100644
>>>> --- a/hw/block/m25p80.c
>>>> +++ b/hw/block/m25p80.c
>>>> @@ -24,7 +24,7 @@
>>>>  #include "hw/hw.h"
>>>>  #include "sysemu/block-backend.h"
>>>>  #include "sysemu/blockdev.h"
>>>> -#include "hw/ssi.h"
>>>> +#include "hw/ssi/ssi.h"
>>>>
>>>>  #ifndef M25P80_ERR_DEBUG
>>>>  #define M25P80_ERR_DEBUG 0
>>>> diff --git a/hw/display/ads7846.c b/hw/display/ads7846.c
>>>> index 3f35369..cb82317 100644
>>>> --- a/hw/display/ads7846.c
>>>> +++ b/hw/display/ads7846.c
>>>> @@ -10,7 +10,7 @@
>>>>   * GNU GPL, version 2 or (at your option) any later version.
>>>>   */
>>>>
>>>> -#include "hw/ssi.h"
>>>> +#include "hw/ssi/ssi.h"
>>>>  #include "ui/console.h"
>>>>
>>>>  typedef struct {
>>>> diff --git a/hw/display/ssd0323.c b/hw/display/ssd0323.c
>>>> index 9727007..7545da8 100644
>>>> --- a/hw/display/ssd0323.c
>>>> +++ b/hw/display/ssd0323.c
>>>> @@ -10,7 +10,7 @@
>>>>  /* The controller can support a variety of different displays, but we only
>>>>     implement one.  Most of the commends relating to brightness and 
>>>> geometry
>>>>     setup are ignored. */
>>>> -#include "hw/ssi.h"
>>>> +#include "hw/ssi/ssi.h"
>>>>  #include "ui/console.h"
>>>>
>>>>  //#define DEBUG_SSD0323 1
>>>> diff --git a/hw/microblaze/petalogix_ml605_mmu.c 
>>>> b/hw/microblaze/petalogix_ml605_mmu.c
>>>> index 462060f..5366cec 100644
>>>> --- a/hw/microblaze/petalogix_ml605_mmu.c
>>>> +++ b/hw/microblaze/petalogix_ml605_mmu.c
>>>> @@ -35,7 +35,7 @@
>>>>  #include "sysemu/block-backend.h"
>>>>  #include "hw/char/serial.h"
>>>>  #include "exec/address-spaces.h"
>>>> -#include "hw/ssi.h"
>>>> +#include "hw/ssi/ssi.h"
>>>>
>>>>  #include "boot.h"
>>>>
>>>> diff --git a/hw/misc/max111x.c b/hw/misc/max111x.c
>>>> index bef3651..d619d61 100644
>>>> --- a/hw/misc/max111x.c
>>>> +++ b/hw/misc/max111x.c
>>>> @@ -10,7 +10,7 @@
>>>>   * GNU GPL, version 2 or (at your option) any later version.
>>>>   */
>>>>
>>>> -#include "hw/ssi.h"
>>>> +#include "hw/ssi/ssi.h"
>>>>
>>>>  typedef struct {
>>>>      SSISlave parent_obj;
>>>> diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
>>>> index e4b2d4f..4fe00fd 100644
>>>> --- a/hw/sd/ssi-sd.c
>>>> +++ b/hw/sd/ssi-sd.c
>>>> @@ -12,7 +12,7 @@
>>>>
>>>>  #include "sysemu/block-backend.h"
>>>>  #include "sysemu/blockdev.h"
>>>> -#include "hw/ssi.h"
>>>> +#include "hw/ssi/ssi.h"
>>>>  #include "hw/sd.h"
>>>>
>>>>  //#define DEBUG_SSI_SD 1
>>>> diff --git a/hw/ssi/pl022.c b/hw/ssi/pl022.c
>>>> index 61d568f..0bbf633 100644
>>>> --- a/hw/ssi/pl022.c
>>>> +++ b/hw/ssi/pl022.c
>>>> @@ -8,7 +8,7 @@
>>>>   */
>>>>
>>>>  #include "hw/sysbus.h"
>>>> -#include "hw/ssi.h"
>>>> +#include "hw/ssi/ssi.h"
>>>>
>>>>  //#define DEBUG_PL022 1
>>>>
>>>> diff --git a/hw/ssi/ssi.c b/hw/ssi/ssi.c
>>>> index 2aab79b..a0f57c0 100644
>>>> --- a/hw/ssi/ssi.c
>>>> +++ b/hw/ssi/ssi.c
>>>> @@ -12,7 +12,7 @@
>>>>   * GNU GPL, version 2 or (at your option) any later version.
>>>>   */
>>>>
>>>> -#include "hw/ssi.h"
>>>> +#include "hw/ssi/ssi.h"
>>>>
>>>>  struct SSIBus {
>>>>      BusState parent_obj;
>>>> diff --git a/hw/ssi/xilinx_spi.c b/hw/ssi/xilinx_spi.c
>>>> index 620573c..94bb2a7 100644
>>>> --- a/hw/ssi/xilinx_spi.c
>>>> +++ b/hw/ssi/xilinx_spi.c
>>>> @@ -29,7 +29,7 @@
>>>>  #include "qemu/log.h"
>>>>  #include "qemu/fifo8.h"
>>>>
>>>> -#include "hw/ssi.h"
>>>> +#include "hw/ssi/ssi.h"
>>>>
>>>>  #ifdef XILINX_SPI_ERR_DEBUG
>>>>  #define DB_PRINT(...) do { \
>>>> diff --git a/hw/ssi/xilinx_spips.c b/hw/ssi/xilinx_spips.c
>>>> index 0910f54..e9471ff 100644
>>>> --- a/hw/ssi/xilinx_spips.c
>>>> +++ b/hw/ssi/xilinx_spips.c
>>>> @@ -27,7 +27,7 @@
>>>>  #include "hw/ptimer.h"
>>>>  #include "qemu/log.h"
>>>>  #include "qemu/fifo8.h"
>>>> -#include "hw/ssi.h"
>>>> +#include "hw/ssi/ssi.h"
>>>>  #include "qemu/bitops.h"
>>>>
>>>>  #ifndef XILINX_SPIPS_ERR_DEBUG
>>>> diff --git a/include/hw/ssi.h b/include/hw/ssi.h
>>>> deleted file mode 100644
>>>> index df0f838..0000000
>>>> --- a/include/hw/ssi.h
>>>> +++ /dev/null
>>>> @@ -1,94 +0,0 @@
>>>> -/* QEMU Synchronous Serial Interface support.  */
>>>> -
>>>> -/* In principle SSI is a point-point interface.  As such the qemu
>>>> -   implementation has a single slave device on a "bus".
>>>> -   However it is fairly common for boards to have multiple slaves
>>>> -   connected to a single master, and select devices with an external
>>>> -   chip select.  This is implemented in qemu by having an explicit mux 
>>>> device.
>>>> -   It is assumed that master and slave are both using the same transfer 
>>>> width.
>>>> -   */
>>>> -
>>>> -#ifndef QEMU_SSI_H
>>>> -#define QEMU_SSI_H
>>>> -
>>>> -#include "hw/qdev.h"
>>>> -
>>>> -typedef struct SSISlave SSISlave;
>>>> -
>>>> -#define TYPE_SSI_SLAVE "ssi-slave"
>>>> -#define SSI_SLAVE(obj) \
>>>> -     OBJECT_CHECK(SSISlave, (obj), TYPE_SSI_SLAVE)
>>>> -#define SSI_SLAVE_CLASS(klass) \
>>>> -     OBJECT_CLASS_CHECK(SSISlaveClass, (klass), TYPE_SSI_SLAVE)
>>>> -#define SSI_SLAVE_GET_CLASS(obj) \
>>>> -     OBJECT_GET_CLASS(SSISlaveClass, (obj), TYPE_SSI_SLAVE)
>>>> -
>>>> -#define SSI_GPIO_CS "ssi-gpio-cs"
>>>> -
>>>> -typedef enum {
>>>> -    SSI_CS_NONE = 0,
>>>> -    SSI_CS_LOW,
>>>> -    SSI_CS_HIGH,
>>>> -} SSICSMode;
>>>> -
>>>> -/* Slave devices.  */
>>>> -typedef struct SSISlaveClass {
>>>> -    DeviceClass parent_class;
>>>> -
>>>> -    int (*init)(SSISlave *dev);
>>>> -
>>>> -    /* if you have standard or no CS behaviour, just override transfer.
>>>> -     * This is called when the device cs is active (true by default).
>>>> -     */
>>>> -    uint32_t (*transfer)(SSISlave *dev, uint32_t val);
>>>> -    /* called when the CS line changes. Optional, devices only need to 
>>>> implement
>>>> -     * this if they have side effects associated with the cs line (beyond
>>>> -     * tristating the txrx lines).
>>>> -     */
>>>> -    int (*set_cs)(SSISlave *dev, bool select);
>>>> -    /* define whether or not CS exists and is active low/high */
>>>> -    SSICSMode cs_polarity;
>>>> -
>>>> -    /* if you have non-standard CS behaviour override this to take control
>>>> -     * of the CS behaviour at the device level. transfer, set_cs, and
>>>> -     * cs_polarity are unused if this is overwritten. Transfer_raw will
>>>> -     * always be called for the device for every txrx access to the 
>>>> parent bus
>>>> -     */
>>>> -    uint32_t (*transfer_raw)(SSISlave *dev, uint32_t val);
>>>> -} SSISlaveClass;
>>>> -
>>>> -struct SSISlave {
>>>> -    DeviceState parent_obj;
>>>> -
>>>> -    /* Chip select state */
>>>> -    bool cs;
>>>> -};
>>>> -
>>>> -#define FROM_SSI_SLAVE(type, dev) DO_UPCAST(type, ssidev, dev)
>>>> -
>>>> -extern const VMStateDescription vmstate_ssi_slave;
>>>> -
>>>> -#define VMSTATE_SSI_SLAVE(_field, _state) {                          \
>>>> -    .name       = (stringify(_field)),                               \
>>>> -    .size       = sizeof(SSISlave),                                  \
>>>> -    .vmsd       = &vmstate_ssi_slave,                                \
>>>> -    .flags      = VMS_STRUCT,                                        \
>>>> -    .offset     = vmstate_offset_value(_state, _field, SSISlave),    \
>>>> -}
>>>> -
>>>> -DeviceState *ssi_create_slave(SSIBus *bus, const char *name);
>>>> -DeviceState *ssi_create_slave_no_init(SSIBus *bus, const char *name);
>>>> -
>>>> -/* Master interface.  */
>>>> -SSIBus *ssi_create_bus(DeviceState *parent, const char *name);
>>>> -
>>>> -uint32_t ssi_transfer(SSIBus *bus, uint32_t val);
>>>> -
>>>> -/* Automatically connect all children nodes a spi controller as slaves */
>>>> -void ssi_auto_connect_slaves(DeviceState *parent, qemu_irq *cs_lines,
>>>> -                             SSIBus *bus);
>>>> -
>>>> -/* max111x.c */
>>>> -void max111x_set_input(DeviceState *dev, int line, uint8_t value);
>>>> -
>>>> -#endif
>>>> diff --git a/include/hw/ssi/ssi.h b/include/hw/ssi/ssi.h
>>>> new file mode 100644
>>>> index 0000000..4a0a539
>>>> --- /dev/null
>>>> +++ b/include/hw/ssi/ssi.h
>>>> @@ -0,0 +1,96 @@
>>>> +/* QEMU Synchronous Serial Interface support.  */
>>>> +
>>>> +/* In principle SSI is a point-point interface.  As such the qemu
>>>> +   implementation has a single slave device on a "bus".
>>>> +   However it is fairly common for boards to have multiple slaves
>>>> +   connected to a single master, and select devices with an external
>>>> +   chip select.  This is implemented in qemu by having an explicit mux 
>>>> device.
>>>> +   It is assumed that master and slave are both using the same transfer 
>>>> width.
>>>> +   */
>>>> +
>>>> +#ifndef QEMU_SSI_H
>>>> +#define QEMU_SSI_H
>>>> +
>>>> +#include "hw/qdev.h"
>>>> +
>>>> +typedef struct SSISlave SSISlave;
>>>> +typedef struct SSISlaveClass SSISlaveClass;
>>>> +typedef enum SSICSMode SSICSMode;
>>>> +
>>>> +#define TYPE_SSI_SLAVE "ssi-slave"
>>>> +#define SSI_SLAVE(obj) \
>>>> +     OBJECT_CHECK(SSISlave, (obj), TYPE_SSI_SLAVE)
>>>> +#define SSI_SLAVE_CLASS(klass) \
>>>> +     OBJECT_CLASS_CHECK(SSISlaveClass, (klass), TYPE_SSI_SLAVE)
>>>> +#define SSI_SLAVE_GET_CLASS(obj) \
>>>> +     OBJECT_GET_CLASS(SSISlaveClass, (obj), TYPE_SSI_SLAVE)
>>>> +
>>>> +#define SSI_GPIO_CS "ssi-gpio-cs"
>>>> +
>>>> +enum SSICSMode {
>>>> +    SSI_CS_NONE = 0,
>>>> +    SSI_CS_LOW,
>>>> +    SSI_CS_HIGH,
>>>> +};
>>>> +
>>>> +/* Slave devices.  */
>>>> +struct SSISlaveClass {
>>>> +    DeviceClass parent_class;
>>>> +
>>>> +    int (*init)(SSISlave *dev);
>>>> +
>>>> +    /* if you have standard or no CS behaviour, just override transfer.
>>>> +     * This is called when the device cs is active (true by default).
>>>> +     */
>>>> +    uint32_t (*transfer)(SSISlave *dev, uint32_t val);
>>>> +    /* called when the CS line changes. Optional, devices only need to 
>>>> implement
>>>> +     * this if they have side effects associated with the cs line (beyond
>>>> +     * tristating the txrx lines).
>>>> +     */
>>>> +    int (*set_cs)(SSISlave *dev, bool select);
>>>> +    /* define whether or not CS exists and is active low/high */
>>>> +    SSICSMode cs_polarity;
>>>> +
>>>> +    /* if you have non-standard CS behaviour override this to take control
>>>> +     * of the CS behaviour at the device level. transfer, set_cs, and
>>>> +     * cs_polarity are unused if this is overwritten. Transfer_raw will
>>>> +     * always be called for the device for every txrx access to the 
>>>> parent bus
>>>> +     */
>>>> +    uint32_t (*transfer_raw)(SSISlave *dev, uint32_t val);
>>>> +};
>>>> +
>>>> +struct SSISlave {
>>>> +    DeviceState parent_obj;
>>>> +
>>>> +    /* Chip select state */
>>>> +    bool cs;
>>>> +};
>>>> +
>>>> +#define FROM_SSI_SLAVE(type, dev) DO_UPCAST(type, ssidev, dev)
>>>> +
>>>> +extern const VMStateDescription vmstate_ssi_slave;
>>>> +
>>>> +#define VMSTATE_SSI_SLAVE(_field, _state) {                          \
>>>> +    .name       = (stringify(_field)),                               \
>>>> +    .size       = sizeof(SSISlave),                                  \
>>>> +    .vmsd       = &vmstate_ssi_slave,                                \
>>>> +    .flags      = VMS_STRUCT,                                        \
>>>> +    .offset     = vmstate_offset_value(_state, _field, SSISlave),    \
>>>> +}
>>>> +
>>>> +DeviceState *ssi_create_slave(SSIBus *bus, const char *name);
>>>> +DeviceState *ssi_create_slave_no_init(SSIBus *bus, const char *name);
>>>> +
>>>> +/* Master interface.  */
>>>> +SSIBus *ssi_create_bus(DeviceState *parent, const char *name);
>>>> +
>>>> +uint32_t ssi_transfer(SSIBus *bus, uint32_t val);
>>>> +
>>>> +/* Automatically connect all children nodes a spi controller as slaves */
>>>> +void ssi_auto_connect_slaves(DeviceState *parent, qemu_irq *cs_lines,
>>>> +                             SSIBus *bus);
>>>> +
>>>> +/* max111x.c */
>>>> +void max111x_set_input(DeviceState *dev, int line, uint8_t value);
>>>> +
>>>> +#endif
>>>> --
>>>> 1.9.1
>>>>
>>>
>