Hi Wolfram, Wolfram Sang wrote: > 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.
If you are heading for kernel inclusion, just go ahead, especially if you have a good chance to get it in. This also may result in valuable feedback from the linuxppc-dev and devicetree-discuss ML. >>> 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. Why "intermediate"? I think the clock should be selectable via 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... For the MPC512x it's simple. For the MPC5200 it might be more tricky, indeed. Anyhow, it's a requirement for mainline inclusion. Wolfgang. _______________________________________________ Socketcan-core mailing list [email protected] https://lists.berlios.de/mailman/listinfo/socketcan-core
