qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH 1/3] PPC4xx IIC and MAL


From: Salvatore Lionetti
Subject: Re: [Qemu-devel] [PATCH 1/3] PPC4xx IIC and MAL
Date: Wed, 10 Dec 2008 14:13:05 +0000 (GMT)

Ok

1) The line code modified is 80% IIC, only few row about MAL
2) MAL until today (if ever this patch enter in qemu) is only used to rw 
registers, so no malfunction is present.
3) As you can see many ppc* file are to modify since I2C previous support it 
totally not integrated: have sense modify ppc* and never use an IIC device?
4) Is error prone and very tedious to extract some code, expecially if there is 
no modularity advantage: one piece require the other,
5) I already have successfully tested the expected patch.
6) I can't do that directly so every time have 2 see if main line is updated or 
not, merge svn file with current my patch status
7) Who apply the patch only apply one click?

In this moment i have no time to make all this retail and test.
Thanks for punctual feeling.

Salvatore

--- Mar 9/12/08, Hollis Blanchard <address@hidden> ha scritto:

> Da: Hollis Blanchard <address@hidden>
> Oggetto: Re: [Qemu-devel] [PATCH 1/3] PPC4xx IIC and MAL
> A: address@hidden
> Cc: "qemu-devel" <address@hidden>
> Data: Martedì 9 dicembre 2008, 17:57
> On Tue, 2008-12-09 at 01:46 +0000, Salvatore Lionetti wrote:
> > Hi
> > 
> > This set of patch try to full support hw present in
> 'walnut' board.
> > http://elinux.org/DHT-Walnut
> > 
> > The fw used to successfully verify board is
> u-boot-1.3.4.
> > 
> > The only component not tested is PCI: perhaps should
> be a good start
> > point for Hollis 's recent work, which i thanks
> for previous code
> > review.
> > 
> > 1) IIC: new eeprom device, full emulation of 4xx
> master core 
> >    MAL: Moved interface so can be shared with other
> modules
> >    4xx boards: move MAL & IIC common part between
> ppc4xx cpu
> >                add a list of created devices, so
> machine 2 phase
> > creation is allowed,
> > 
> >    Warning: to compile:
> >    - comment line 'ppc4xx_emac_init(...)' in
> module hw/ppc405_boards.c
> >    - rename ppc405ep_init(args) in
> ppc405xp_init("405ep", args, NULL)
> > 
> >    Files changed:
> >    - Makefile.target
> >    - Makefile
> >    - hw/ppc405_uc.c
> >    - hw/ppc4xx_devs.c
> >    - hw/ppc405.h
> >    - hw/i2c.h
> >    - hw/iic_eeprom.c (Added)
> >    - hw/ppc4xx.h
> 
> Please split this into smaller patches which can be more
> easily
> reviewed. For example, you should send one patch that
> *only* moves the
> i2c code (and makes no changes). Then submit a patch that
> modifies the
> code in its new location. You should submit separate
> patches for MAL and
> I2C. You should not change the DEBUG #defines, nor add
> debug calls to
> printf.
> 
> In other words, please remove all extraneous changes from
> your patch,
> and break it up into small self-contained pieces that can
> be committed
> one by one.
> 
> -- 
> Hollis Blanchard
> IBM Linux Technology Center







reply via email to

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