Hi Wolfgang, > what is your plan now? Provide patches for mainline inclusion > (net-next-2.6) or for the Socket-CAN SVN trunk? Please don't send > patches for both. We can do the backport to the SVN trunk later.
Okay, I didn't know that this was acceptable. I thought we get the driver straight first in socketcan-trunk and then put it to net-next. From the development process, I'd like it a _lot_ more if I just could create my git-branch and then post to the according mailing-lists, like ppc and netdev. I didn't really dare to do this for this first round, as I didn't know who is working on what and if there are already other plans for mscan and I didn't want to interfere. > > Note 1: It depends on changes to the clock-implementation of the MPC512x. > > Those patches need to be discussed and approved on the ppc list and can be > > found here: > > http://patchwork.ozlabs.org/patch/37417/ > > http://patchwork.ozlabs.org/patch/37427/ > > (If they work for you, I'd be happy about Acked-by-tags.) > > My impression is that they will not be accepted as already a new clock > interface is available (via patches). As those patches you mentioned are in an early state, I think my additions could be accepted for now. Also, for the mpc5xxx_can-part, there shouldn't be much change, as it is still clk_get() and clk_enable() there. > > > Note 2: If this approach is accepted, I will next do the remaining cleanup > > work (giving the functions the proper 5xxx names, ...) as follow up patches > > to > > hopefully get this driver into mainline soonish. > > Would be nice to have that fixed a.s.a.p. Yeah, in case I can use git and a the top-of-tree kernel version, this should go more smoothly. > > Kconfig | 8 +++--- > > mscan/mpc52xx_can.c | 64 > > +++++++++++++++++++++++++++++++++++++++++----------- > > mscan/mscan.h | 3 +- > > 3 files changed, 57 insertions(+), 18 deletions(-) > > I assume this patch is for the SVN trunk. Please provide patches against > top of tree, and not a subdirectory to ease testing. Uh, ouch! I'm very sorry, seems like my svn-fu has dramatically dropped since git. > > Index: mscan/mpc52xx_can.c > > As said above, please do s/mpc52xx/mpc5xxx/ immediately. It does not > harm if we have both, the old mpc52xx_can.c and the new mpc5xxx_can.c > in the SVN trunk including the related CONFIG_CAN_* parameters. The > mpc5xxx_can.c should not include the legacy driver stuff (pre-OF), of > course. OK. > > + cellp = of_get_property(ofdev->node, "cell-index", NULL); > > + if (!cellp) > > + return 0; > > Hm, the mscan node does not have a cell-index any more because it's > deprecated (removed by the device tree police), IIRC. Using Grant's suggestion, this becomes obsolete. > > + if (port_clk) > > + return clk_get_rate(port_clk); > > + else > > + return clk_get_rate(mscan_clk); > > +} > > I have not looked into your clock patches, but how can the user select > the clock source and the clock frequency (via clock divider)? This > should be selectable via DTS file, I think. This is not possible yet. For that, I wanted to wait until the clock-representation in the device-tree (the patches you mentioned, I suppose) stabilizes. If this matures, we can add the proper way to do it. For now, "clock-ipb" is supported, the rest has to be done by the bootloader or additional patches. I wouldn't like to introduce "intermediate" properties to the device tree. > > What's also missing is the proper bus-off recovery method for the > MCP512x. Unfortunately, it's also not yet OK for the MPC5200. It should > be implemented as listed below: > > - if possible, automatic bus-off recovery should be turned off, which > is possible on the MPC512x (but not the default for compatibility with > the MPC5200. > > - if bus-off recovery cannot be switched of, the driver should handle it > as follows: > > - if restart_ms == 0, the device should be stopped on bus-off to > suppress automatic recovery. > > - if restart_ms > 0, the hardware should do the automatic recovery and > a RESTARTED error message should be sent when it re-enters the > error-active state (or leaves the bus-off state). Uh, I didn't find this in the TODO. That might beat my timeframe for this project, as it surely needs lots of testing :( Maybe Fu can help me here, but not sure... Thanks for the feedback! Wolfram -- Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ |
signature.asc
Description: Digital signature
_______________________________________________ Socketcan-core mailing list [email protected] https://lists.berlios.de/mailman/listinfo/socketcan-core
