[PATCH 0/6] Blackfin I2C TWI driver updates

[PATCH 0/6] Blackfin I2C TWI driver updates

[Bryan Wu]

Hi Jean:

I reworked this series updates according to you review in previous email.
Could you please apply these patches to 2.6.25, because Blackfin BF54x compiling
will fail without these?

Thanks a lot
-Bryan

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

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

[Jean Delvare]

Hi Bryan,

On Fri, 14 Mar 2008 00:22:34 -0700, Bryan Wu wrote:
> I reworked this series updates according to you review in previous email.
> Could you please apply these patches to 2.6.25, because Blackfin BF54x compiling
> will fail without these?

No, sorry. We're at rc5 already. If you wanted these changes in 2.6.25,
you should have sent them 2 months ago. Plus, none of the 6 patches in
this series says that it fixes a build failure.

At this point of the release cycle I can only accept build fixes, bug
fixes and documentation updates. If anything like this is needed for
the i2c-bfin-twi driver, please send short, clean, documented patches
that do not depend on other patches, and I'll review and apply them.
Anything else will have to wait until 2.6.26.

-- 
Jean Delvare

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

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

[Bryan Wu]

On Thu, Mar 13, 2008 at 2:37 AM, Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org> wrote:
> Hi Bryan,
>
>
>  On Fri, 14 Mar 2008 00:22:34 -0700, Bryan Wu wrote:
>  > I reworked this series updates according to you review in previous email.
>  > Could you please apply these patches to 2.6.25, because Blackfin BF54x compiling
>  > will fail without these?
>
>  No, sorry. We're at rc5 already. If you wanted these changes in 2.6.25,
>  you should have sent them 2 months ago. Plus, none of the 6 patches in
>  this series says that it fixes a build failure.
>
>  At this point of the release cycle I can only accept build fixes, bug
>  fixes and documentation updates. If anything like this is needed for
>  the i2c-bfin-twi driver, please send short, clean, documented patches
>  that do not depend on other patches, and I'll review and apply them.
>  Anything else will have to wait until 2.6.26.
>

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:

--
drivers/i2c/busses/i2c-bfin-twi.c: In function 'bfin_twi_handle_interrupt':
drivers/i2c/busses/i2c-bfin-twi.c:67: error: implicit declaration of
function 'bfin_read_TWI_INT_STAT'
drivers/i2c/busses/i2c-bfin-twi.c:68: error: implicit declaration of
function 'bfin_read_TWI_MASTER_STAT'
drivers/i2c/busses/i2c-bfin-twi.c:73: error: implicit declaration of
function 'bfin_write_TWI_XMT_DATA8'
drivers/i2c/busses/i2c-bfin-twi.c:80: error: implicit declaration of
function 'bfin_write_TWI_MASTER_CTL'
drivers/i2c/busses/i2c-bfin-twi.c:80: error: implicit declaration of
function 'bfin_read_TWI_MASTER_CTL'
drivers/i2c/busses/i2c-bfin-twi.c:87: error: implicit declaration of
function 'bfin_write_TWI_INT_STAT'
drivers/i2c/busses/i2c-bfin-twi.c:93: error: implicit declaration of
function 'bfin_read_TWI_RCV_DATA8'
drivers/i2c/busses/i2c-bfin-twi.c:118: error: implicit declaration of
function 'bfin_write_TWI_INT_MASK'
drivers/i2c/busses/i2c-bfin-twi.c:119: error: implicit declaration of
function 'bfin_write_TWI_MASTER_STAT'
drivers/i2c/busses/i2c-bfin-twi.c: In function 'bfin_twi_master_xfer':
drivers/i2c/busses/i2c-bfin-twi.c:227: error: implicit declaration of
function 'bfin_read_TWI_CONTROL'
drivers/i2c/busses/i2c-bfin-twi.c:251: error: implicit declaration of
function 'bfin_write_TWI_MASTER_ADDR'
drivers/i2c/busses/i2c-bfin-twi.c:256: error: implicit declaration of
function 'bfin_write_TWI_FIFO_CTL'
drivers/i2c/busses/i2c-bfin-twi.c: In function 'i2c_bfin_twi_suspend':
drivers/i2c/busses/i2c-bfin-twi.c:520: error: implicit declaration of
function 'bfin_write_TWI_CONTROL'
drivers/i2c/busses/i2c-bfin-twi.c: In function 'i2c_bfin_twi_probe':
drivers/i2c/busses/i2c-bfin-twi.c:545: error: 'IRQ_TWI' undeclared
(first use in this function)
drivers/i2c/busses/i2c-bfin-twi.c:545: error: (Each undeclared
identifier is reported only once
drivers/i2c/busses/i2c-bfin-twi.c:545: error: for each function it appears in.)
drivers/i2c/busses/i2c-bfin-twi.c:572: error: implicit declaration of
function 'bfin_write_TWI_CLKDIV'
make[3]: *** [drivers/i2c/busses/i2c-bfin-twi.o] Error 1
make[2]: *** [drivers/i2c/busses] Error 2
make[1]: *** [drivers/i2c] Error 2
make: *** [drivers] Error 2

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

[Kalle Pokki]

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.

Although I don't have a BF54x platform I have at times run into a few 
other build failures with BF533 and BF537. These are the most irritating 
bugs for other people to discover in the final releases since they should 
be the simplest to find and fix, often leaving the impression the code 
wasn't even built before submitting. Of course, in reality we cannot 
always build for every platform.

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.


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

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

[Bryan Wu]

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?

>  Although I don't have a BF54x platform I have at times run into a few
>  other build failures with BF533 and BF537. These are the most irritating
>  bugs for other people to discover in the final releases since they should
>  be the simplest to find and fix, often leaving the impression the code
>  wasn't even built before submitting. Of course, in reality we cannot
>  always build for every platform.
>

Actually, we got a daily build machine try to build upstream mainline
git-tree, -mm tree
and other source tree with all Blackfin platform configuration. This
BF54x compile error was
found by this. We will try to fix it before stable version out.

>  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?

Thanks
Regards,
-Bryan

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

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

[Jean Delvare]

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

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

[Bryan Wu]

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

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

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

[Jean Delvare]

On Thu, 13 Mar 2008 09:25:49 -0700, Bryan Wu wrote:
> 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?

Exactly.

Thanks,
-- 
Jean Delvare

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

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

[Jean Delvare]

On Thu, 13 Mar 2008 10:07:07 +0800, Bryan Wu wrote:
> Actually, I plan to send out these for 2.6.26 at first. But found
> following BF54x compiling error:
> 
> --
> drivers/i2c/busses/i2c-bfin-twi.c: In function 'bfin_twi_handle_interrupt':
> drivers/i2c/busses/i2c-bfin-twi.c:67: error: implicit declaration of
> function 'bfin_read_TWI_INT_STAT'
> drivers/i2c/busses/i2c-bfin-twi.c:68: error: implicit declaration of
> function 'bfin_read_TWI_MASTER_STAT'
> drivers/i2c/busses/i2c-bfin-twi.c:73: error: implicit declaration of
> function 'bfin_write_TWI_XMT_DATA8'
> drivers/i2c/busses/i2c-bfin-twi.c:80: error: implicit declaration of
> function 'bfin_write_TWI_MASTER_CTL'
> drivers/i2c/busses/i2c-bfin-twi.c:80: error: implicit declaration of
> function 'bfin_read_TWI_MASTER_CTL'
> drivers/i2c/busses/i2c-bfin-twi.c:87: error: implicit declaration of
> function 'bfin_write_TWI_INT_STAT'
> drivers/i2c/busses/i2c-bfin-twi.c:93: error: implicit declaration of
> function 'bfin_read_TWI_RCV_DATA8'
> drivers/i2c/busses/i2c-bfin-twi.c:118: error: implicit declaration of
> function 'bfin_write_TWI_INT_MASK'
> drivers/i2c/busses/i2c-bfin-twi.c:119: error: implicit declaration of
> function 'bfin_write_TWI_MASTER_STAT'
> drivers/i2c/busses/i2c-bfin-twi.c: In function 'bfin_twi_master_xfer':
> drivers/i2c/busses/i2c-bfin-twi.c:227: error: implicit declaration of
> function 'bfin_read_TWI_CONTROL'
> drivers/i2c/busses/i2c-bfin-twi.c:251: error: implicit declaration of
> function 'bfin_write_TWI_MASTER_ADDR'
> drivers/i2c/busses/i2c-bfin-twi.c:256: error: implicit declaration of
> function 'bfin_write_TWI_FIFO_CTL'
> drivers/i2c/busses/i2c-bfin-twi.c: In function 'i2c_bfin_twi_suspend':
> drivers/i2c/busses/i2c-bfin-twi.c:520: error: implicit declaration of
> function 'bfin_write_TWI_CONTROL'
> drivers/i2c/busses/i2c-bfin-twi.c: In function 'i2c_bfin_twi_probe':
> drivers/i2c/busses/i2c-bfin-twi.c:545: error: 'IRQ_TWI' undeclared
> (first use in this function)
> drivers/i2c/busses/i2c-bfin-twi.c:545: error: (Each undeclared
> identifier is reported only once
> drivers/i2c/busses/i2c-bfin-twi.c:545: error: for each function it appears in.)
> drivers/i2c/busses/i2c-bfin-twi.c:572: error: implicit declaration of
> function 'bfin_write_TWI_CLKDIV'
> make[3]: *** [drivers/i2c/busses/i2c-bfin-twi.o] Error 1
> make[2]: *** [drivers/i2c/busses] Error 2
> make[1]: *** [drivers/i2c] Error 2
> make: *** [drivers] Error 2
> --
> 
> But as you said, it is -rc5 now. If these can be merged in 2.6.26, it
> is OK for me.

Maybe you can provide a fix for just this build problem? This I would
possibly push into 2.6.25, and then the rest of your updates go on top
of it for 2.6.26.

-- 
Jean Delvare

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