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

Bryan Wu
2008-03-13T16:25:49Z

On Thu, Mar 13, 2008 at 7:34 AM, Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org> wrote:
> 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.
>

OK, Killing BF54x from drivers/i2c/busses/Kconfig is acceptable.
And the whole i2c function of BF54x will be disabled in 2.6.25, it
will be fixed in 2.6.26, right?

> > (...)
>
> > > 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.
>

In fact, this patch set was sent out before. While some BF54x i2c header
change was merged, patch 2/6 was queued for a long. So it introduced
this compile error.

> 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.)
>

Yes, originally they are fuction-adding patches, not bug fixing. I
agree with to workaround in 2.6.25
and added this patches to 2.6.26 to both adding new functions and
fixing compile bug.

I will send out the Kconfig fixing patch soon.

Thanks a lot
-Bryan Wu