Re: [Qemu-devel] [PATCH v2 0/7] Misc sm501 improvements

[Philippe Mathieu-Daudé]

Hi David, I'll reply to you using Peter mail :)

On 07/03/2018 06:13 AM, Peter Maydell wrote:
> On 3 July 2018 at 01:28, David Gibson <address@hidden> wrote:
>> On Mon, Jul 02, 2018 at 10:42:07AM +0100, Peter Maydell wrote:
>>> I can have a look, but really I think these should go via the
>>> ppc tree -- the device is used in a ppc machine and an sh4 one,
>>> and the ppc tree is much more active than sh4.
>>
>> Hm, well, if you like.  Although the gradual creep of my
>> maintainership scope into things I know less and less about makes me
>> rather nervous.  I tried to convince Balaton Zoltan to become sm501
>> maintainer, but he didn't bite.
> 
> It's not like I know anything more about the sm501 than you do :-)
> 
> The arm parts of the tree have a lot of board and device code
> I'm not familiar with either. Generally the approach I use is:
>  * eyeball the code to see if it's doing anything that looks
>    "obviously wrong", failing to use correct QEMU APIs, etc

Gerd Hoffmann did a review for the graphic code,
then commented "all look sane":
http://lists.nongnu.org/archive/html/qemu-devel/2018-06/msg06493.html

I don't have a serious understanding of QEMU graphic internals, but
looked at those patches and didn't find anything "obviously wrong".

>  * anything that touches 'core' code or code common to multiple
>    machines gets closer scrutiny

The only SH4 tests I run are using Aurelien Debian:
https://people.debian.org/~aurel32/qemu/sh4/

>  * I might check the data sheet if I'm feeling enthusastic or
>    if the code looks like it's doing weird things, but often
>    not, especially if the code has had review from somebody else

I did a review of the I2C/DDC patch with the specs at hand.

>  * if I have a test image to hand I'll do a smoke test, but often
>    I don't have a test image

Neither do I.

> Basically I think the important thing is to make sure the
> codebase stays maintainable and generally the quality bar
> in terms of using the right APIs and design approaches
> tends to ratchet up rather than down. If our implementation
> of an obscure device isn't actually right, that doesn't
> really affect very many users, so I worry less about that
> side of things.

Zoltan used few deprecated API but said updating "could be done in
separate clean up", which I agree :)
http://lists.nongnu.org/archive/html/qemu-devel/2018-06/msg06911.html

FWIW and using Peter's criteria I think this series is good to go.

Regards,

Phil.