Re: [Qemu-devel] [PATCH v2 01/20] arm: add Faraday a36x SoC platform support

[Kuo-Jung Su]

Hi Andreas:

Thanks for the information, and sorry for the mess I've done.
I'll one-by-one re-send all the patches.

However because most of my patches are new files,
should I send-out the patches with detail change log?

For example:

[PATCH] dumb timer
... [PATCH v2 0/2] dumb timer (Cover letter)
    [PATCH v2 1/2] dumb timer (The one in Patch V1)
    [PATCH v2 2/2] dumb timer: coding style update (Change log for V2)
...... [PATCH v3 0/2] dumb timer (Cover letter)
       [PATCH v3 1/2] dumb timer (The merged file in Patch V1 & v2)
       [PATCH v3 2/2] dumb timer: bug fix (Change log for V3)


Best Wishes
Dante

2013/2/1 Andreas Färber <address@hidden>:
> Hi,
>
> Am 01.02.2013 02:39, schrieb Kuo-Jung Su:
>> 2013/2/1 Igor Mitsyanko <address@hidden>
>>>
>>> On 01/25/2013 12:19 PM, Kuo-Jung Su wrote:
>>>>
>>>> +/* Board init. */
>>>> +static void
>>>> +a360_device_init(a360_state *s)
>>>> +{
>>>> +    qemu_irq *pic;
>>>> +    qemu_irq ack, req;
>>>> +    qemu_irq cs_line;
>>>> +    DeviceState *ds;
>>>> +    int i, done_nic = 0, nr_flash = 1;
>>>> +    SSIBus *spi;
>>>> +    DeviceState *fl;
>>>> +
>>>> +    /* Interrupt Controller */
>>>> +    pic = ftintc020_init(0x98800000, s->cpu);
>>>
>>>
>>>
>>> You haven't introduced this interrupt controller yet, patches should be 
>>> arranged in such an order that they at least wouldn't break a build.
>>> Same goes for ftintc020_init and ftgmac100_init.
>>
>>
>> I thought that's why patch set is designed for.
>> And I susposed to split up each component and send out patches one by one?
> [...]
>>>> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
>>>> index 6d049e7..c7bb10e 100644
>>>> --- a/hw/arm/Makefile.objs
>>>> +++ b/hw/arm/Makefile.objs
>>>> @@ -1,4 +1,10 @@
>>>>   obj-y = integratorcp.o versatilepb.o arm_pic.o
>>>> +obj-y += a360.o a369.o \
>>>> +                               rom.o ftdmac020.o ftapbbrg020.o \
>>>> +                               ftintc020.o fttmr010.o ftpwmtmr010.o \
>>>> +                               ftspi020.o ftssp010.o fti2c010.o \
>>>> +                               ftrtc011.o ftwdt010.o ftmac110.o 
>>>> ftgmac100.o ftlcdc200.o \
>>>> +                               fttsc010.o ftkbc010.o ftnandc021.o 
>>>> ftsdc010.o
>>>
>>>
>>> No such files exist at this point, you should add them here one by one in a 
>>> corresponding patch.
>>> And tabs should be replaced with spaces.
>>
>> It looks like that I really have to split up each of one by one.
>> That's good to me, because I don't have any common sense about patch process,
>> Each component in one patch would be much more easier to me.
>>
>>>>   obj-y += arm_boot.o
>>>>   obj-y += xilinx_zynq.o zynq_slcr.o
>>>>   obj-y += xilinx_spips.o
>>>> diff --git a/hw/faraday.h b/hw/faraday.h
>>>> new file mode 100644
>>>> index 0000000..f4fe0cc
>>>> --- /dev/null
>>>> +++ b/hw/faraday.h
>>>
>>>
>>>
>>> None of three function prototyped in this file exists at this point, I 
>>> think you should add this file later in the patch set.
>>
>> Same issue.
>> I'm studying patchwork now, because I don't want to [act] like a idiot,
>> I'll send out the new patch when I 'though' I'm ready.
>
> The criteria is that every patch must be compilable on its own, we say
> 'bisectable' because it allows to work with git-bisect command for error
> searching. Someone who is debugging an x86 problem might land on an arm
> a36x commit and otherwise not be able to compile.
>
> You can compare my Tegra patchset here (not polished yet):
> http://repo.or.cz/w/qemu/afaerber.git/shortlog/refs/heads/tegra
>
> You can add a stub version of your machine and step by step add devices
> to it until it is complete. My Tegra2 SoC modeling with a QOM object is
> not final yet but might serve as design inspiration. I.e., all A36x SoC
> devices should have their state structs either in their own header or in
> a SoC-specific header, so that they can be embedded as fields within a
> SoC state struct - be it within your patchset or as a later step.
>
> Regards,
> Andreas
>
> --
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



-- 
Best wishes,
Kuo-Jung Su