Re: [PATCH] i2c: Renesas Highlander FPGA SMBus support.

[Jean Delvare <khali@linux-fr.org>]

On Fri, 25 Apr 2008 15:22:08 +0900, Paul Mundt wrote:
> On Thu, Apr 24, 2008 at 11:12:07PM -0700, Andrew Morton wrote:
> > > On Fri, 25 Apr 2008 10:30:11 +0900 Paul Mundt <lethal@linux-sh.org> wrote:
> > > On Wed, Apr 23, 2008 at 08:31:04PM +0200, Jean Delvare wrote:
> > > > On Wed, 23 Apr 2008 20:11:01 +0200, Manuel Lauss wrote:
> > > > > I don't think I'm qualified to review other peoples' code (it looks
> > > > > fine to me).
> > 
> > I looked through it when I merged it - believe it or not, I always do
> > (well, except for some dopey mechanical code transformation patches where
> > I'll just believe the changelog).  I saw nothing worth commenting on.  As
> > is always the case when I don't comment ;)
> > 
> > So here's a
> > Reviewed-by: Andrew Morton <akpm@linux-foundation.org>
> > 
> > Although that is of course of limited use, coming from a person
> > who isn't terribly sure what an i2c is.
> 
> This is the root of the issue, none of the people asked to review the
> code are i2c people either. This is a pretty sad state for the subsystem
> if the subsystem maintainer needs to defer to people with little to no
> knowledge of the subsystem to "review" a driver before it can be merged.
>
> While Manuel, Magnus, and I can easily review and ack our patches, none
> of this changes the fact that outside of the platform and architecture
> specific bits in the driver, there's very little we can generally comment
> on. The reason for soliciting feedback from the i2c list in the first
> place was to get review and comments on the subsystem-specific bits from
> the people who are obviously far more familiar with these things. I
> understand that Jean isn't an embedded person and therefore isn't
> comfortable reviewing those sorts of drivers, but in these cases it's the
> bus-specific stuff where the review really matters, which obviously the
> rest of us aren't in the best position to self-review.

OK, I just reviewed your driver. I had 20 comments, only 6 of them
required knowledge about the i2c subsystem. The 14 remaining comments
could have been made by about anybody with some experience with Linux
kernel development.

Given the limited time I have to review new i2c drivers, my hope was
some other people could take care of the first review catching most of
the non-i2c-related issues, and then I could just focus on the i2c side
of things. But I guess I'm asking for too much.

> If it's not possible to get a subsystem maintainer to review a patch,
> what's the point of having a centralized subsystem in the first place?

I don't even understand your question, but I doubt it deserves an
answer anyway.

-- 
Jean Delvare