Wolfgang Grandegger wrote: > Hello, > > we have a nasty mistake in the clock setup code of the SJA1000 platform > driver. The member "clock" of "struct sja1000_platform_data" is > documented as "CAN bus oscillator frequency in Hz" but in the probe > function we assign: > > priv->can.clock.freq = pdata->clock; > > In fact, it should be "pdata->clock/2", but that would require fixing > platform code as well. To avoild confusion, we should at least fix the > documentation. A clean fix would rename the member to "osc_freq", assign > "pdata->osc_freq/2" and fix the relevant platform code. > > What do you think?
There's two in tree users of sja1000_platform
"arch/arm/mach-mx3/mach-pcm037.c"
"arch/arm/mach-mx2/pcm970-baseboard.c"
and they both use platform data like this:
struct sja1000_platform_data pcm970_sja1000_platform_data = {
.clock = 16000000 / 2,
.ocr = 0x40 | 0x18,
.cdr = 0x40,
};
This means it's at least _used_ consistent in the kernel.
Hmmm...it would be more consistent with the other sja users, e.g. the of
driver, to do the full fix. Renaming the pdata member should compile
time break it instead of run-time. Thus the non mainline users will
notice it, too.
cheers, Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Socketcan-core mailing list [email protected] https://lists.berlios.de/mailman/listinfo/socketcan-core
