Re: [Qemu-devel] [PATCH 05/17] timer: generalize Dallas/Maxim RTC i2c devices

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 1823 bytes --]

On Sun, Dec 03, 2017 at 03:15:10PM -0600, Michael Davidsaver wrote:
> On 11/29/2017 11:13 PM, David Gibson wrote:
> > On Sun, Nov 26, 2017 at 03:59:03PM -0600, Michael Davidsaver wrote:
> >> Support for: ds1307, ds1337, ds1338, ds1339,
> >> ds1340, ds1375, ds1388, and ds3231.
> >>
> >> Tested with ds1338 and ds1375.
> >>
> >> Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
> > 
> > I certainly like the idea of consolidating this code, but reviewing to
> > see that the new code really is a generalization of the old is
> > something I won't have time for for a while.
> > 
> > Also, hw/timer is not within my purview so it'll probably need to go
> > another path to merge.
> 
> Could you be a bit more explicit about what, if anything, I need to do
> to move this forward?

Ugh.. that's pretty tough, since ds1338 doesn't have an activate
maintainer.  You can look at the git history for some possible
candidates of people to ask about it, but it hasn't been touched much
in quite a while.

One approach that could help is to re-order so that your testing
rework goes before the change to ds1338.  If your new generalization
can pass the same set of tests as the original ds1338 code, that's at
least a good start on being convincing that it's a true superset of
the previous functionality.

The other approach is to do the rework in a rather longer series of
patches.  Start by simply moving ds1338.c, then do a mechanical
replacement of the names within it, then start generalizing and
altering.  That's a lot of work for you, but it makes it much easier
to review each step

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]