Re: [PATCH 0/6] Blackfin I2C TWI driver updates

[Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>]

Hi Bryan, Kalle,

On Thu, 13 Mar 2008 15:50:59 +0800, Bryan Wu wrote:
> On Thu, Mar 13, 2008 at 1:43 PM, Kalle Pokki <kalle.pokki-jgfpPLbCha8@public.gmane.org> wrote:
> > On Thu, 13 Mar 2008, Bryan Wu wrote:
> >
> >  > I understand. It is too late.
> >  >
> >  > Actually, I plan to send out these for 2.6.26 at first. But found
> >  > following BF54x compiling error:
> >
> >  Please post a fix for the compile error in 2.6.25. As Jean pointed out,
> >  build and bug fixes should always be sent upstream asap, without waiting
> >  for the next merge window.
> 
> [PATCH 2/6]  is to fix this compile error bug.
> So Jean, could you please just merge
> patch 1/6 and patch 2/6 for 2.6.25, while others for 2.6.26?

I can't believe that this patch is the easiest way to fix the build
failure. I seem to understand that the problem is one of the Blackfin
machines missing the proper header definitions? Then I suggest that you
simply exclude this machine in Kconfig. This fixes the build failure in
one line, which is perfectly acceptable at this point of the release
cycle.

> (...)
> >  Actually, patch 1/6 seems to be a real bug fix. In the current code the
> >  i2c-bfin-twi master transmit is broken for all drivers requiring a restart
> >  condition.
> 
> So patch 1/6 and patch 2/6 is ready for 2.6.25, don't they?

The problem is that these are the 2 biggest patches of the series,
summing up to 30 kB. This is really more than I want to take at this
point of the release cycle.

Patch 1/6 looks more like a new feature to me. The bug was not that
some transaction types were not supported, but that the driver
advertised them as being supported. So the proper fix (again, because
we are late in the release cycle) would be to adjust the advertised
functionality bitmap. That being said, I doubt that it will really help
in practice, if a chip driver needs one of the missing transaction
types, it will fail both ways. So I won't insist on changing this for
2.6.25.

For patch 2/6, see what I wrote above, I'd prefer that we adjust
Kconfig to avoid the build breakage in 2.6.25, and add the large patch
in 2.6.26.

On a general note, the patch summaries and header comments should
really tell that they fix build failures or bugs when they do. Here
patches 1/6 and 2/6 both say that they "add" something, not that they
"fix" something; this made it hard for me to figure out what was
needed for 2.6.25 and what wasn't (leaving aside the patch size
considerations for a moment.)

-- 
Jean Delvare

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c