Re: [Qemu-devel] [PATCH v02] add 1394 bus support

[Andreas Färber]

Hi,

Am 19.04.2015 um 18:38 schrieb Itamar Tal:
> I've set it just after the subject field in the patch? Should I add it
> somewhere else and resubmit?

Please compare other QEMU or Linux patches: It needs to go before ---
into the commit message. git commit --amend -s will place it correctly.

BTW I'm missing a type_init() and type_register_static() in the bottom
of hcd-ohci.c and wonder how you managed to test it without.

Did you copy this code from some other codebase? For one, hcd-ohci.c has
"address@hidden", which looks like you copied it from some mailing list
archive (then you need to replicate those authors' Signed-off-bys), and
for another the Coding Style does not exactly match that of QEMU, in
particular you are using a lot of custom _t types whereas QEMU uses
CamelCase type names. BTW if you consistently used typedefs for your
unions, you could avoid those macros declaring phy etc. unions inline, no?

Please also drop the empty last line in the new files you add. Did you
run scripts/checkpatch.pl? Running git-am on a clean branch can also
help highlight whitespace errors.

hcd-ohci.h is missing a license/copyright header.

hcd_pci_exit() has commented-out code - please fix or remove.
hcd_pci_init() has the opening brace placed wrong.

It looks like a generic PCI device, so why are you placing the config
option into i386 and x86_64 configs only rather than pci.mak?

Regards,
Andreas

P.S. Please don't top-post.

> On Apr 19, 2015 6:47 PM, "Andreas Färber" <address@hidden
> <mailto:address@hidden>> wrote:
> 
>     Am 19.04.2015 um 12:52 schrieb address@hidden
>     <mailto:address@hidden>:
>     > From: Itamar Tal <address@hidden
>     <mailto:address@hidden>>
>     >
>     > ---
> 
>     Still no Signed-off-by...
> 
>     Regards,
>     Andreas
> 
>     --
>     SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
>     GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
>     Graham Norton; HRB 21284 (AG Nürnberg)
> 


-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)