Hi Stig,
> I have incorporated all your suggestions into this patch, except for
> these two:
>
> * Jean: use of val & 255 in printk statement in pcf_isa_setbyte() - I
> think this is actually required because parameter val is an integer
> quantity and could have been sign-extended? I would prefer to change
> the function parameter to u8, but this violates the types of struct
> i2c_algo_pcf_data.
The "& 255" is done in the debug printk only, iowrite8 is still called
with val unmasked, so if the masking actually did something, something
bas is happening, and the debug trace won't ever show it.
Looking at i2c-algo-pcf, it's quite clear that val will always fit in
an u8, so the masking is actually needless.
> * Greg: I haven't changed the printk and pr_debug calls yet, because
> there are some printk calls before a struct device is initialised (this
> is done when the device is registered to i2c_algo_pcf at the end of
> driver init). What is the policy relating to driver output before
> there is an associated struct device initialised?
pr_debug and pr_info are fine to use in these cases (or even explicit
printk for other cases).
> I took a look at the code with sparse, there were no reported problems.
> However, am I right in thinking that sparse is x86-centric? I was
> running the compilation on an Alpha system. With more than a little
> irony considering the political harangue against gcc in the sparse
> documentation, it didn't compile on my RH8 x86 system: gcc gave an
> internal compiler error...
I had the exact same problem :(
> The attached patch is for code cleanup and is a direct replacement for
> patch 2.
Comments:
> * all printed messages are prefixed by "i2c-elektor: ", via new preprocessor
> definitions DRVNAME and DRVPFX
This is pretty pointless considering that we want to convert all of
these to dev_{dbg,err,warn,info} anyway, and these do not need a prefix.
> * #if __alpha__ converted to #ifdef __alpha__
This is a bug you introduced in patch #1! Don't fix it in patch #2, fix
patch #1 not to introduce it in the first place.
So, here comes a respin of both patches. I'd like to enqueue them and
push them to Greg soon. Can you test them and confirm them work OK for
you?
Thanks,
--
Jean Delvare
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: elektor-alpha-1.patch
Url: http://lists.lm-sensors.org/pipermail/lm-sensors/attachments/20051006/fdab0106/attachment.pl
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: elektor-alpha-2.patch
Url: http://lists.lm-sensors.org/pipermail/lm-sensors/attachments/20051006/fdab0106/attachment-0001.pl
