Sorry, for late response. ----- Original Message ----- From: "Wang, Qi" <[email protected]> To: "Masayuki Ohtak" <[email protected]> Cc: "Khor, Andrew Chih Howe" <[email protected]>; <[email protected]>; <[email protected]>; "Wang, Yong Y" <[email protected]>; "Wolfgang Grandegger" <[email protected]> Sent: Thursday, August 12, 2010 6:00 PM Subject: RE: [MeeGo-Dev][PATCH] Topcliff: Update PCH_CAN driver to 2.6.35
> Hi Ohtak-san, > > Would you please answer the question from Wolfgang? > > Best Regards, > Qi. > > > -----Original Message----- > > From: Wolfgang Grandegger [mailto:[email protected]] > > Sent: Thursday, August 12, 2010 4:55 PM > > To: Wang, Qi > > Cc: Masayuki Ohtak; Khor, Andrew Chih Howe; [email protected]; > > [email protected]; Wang, Yong Y > > Subject: Re: [MeeGo-Dev][PATCH] Topcliff: Update PCH_CAN driver to 2.6.35 > > > > On 08/12/2010 03:29 AM, Wang, Qi wrote: > > >> -----Original Message----- > > >> From: Wolfgang Grandegger [mailto:[email protected]] > > >> Sent: Wednesday, August 11, 2010 8:31 PM > > >> To: Masayuki Ohtak > > >> Cc: [email protected]; [email protected]; > > >> [email protected]; Khor, Andrew Chih Howe; [email protected]; > > >> [email protected]; Wang, Qi; Wang, Yong Y > > >> Subject: Re: [MeeGo-Dev][PATCH] Topcliff: Update PCH_CAN driver to 2.6.35 > > >> > > >> Hello, > > >> > > >> On 08/11/2010 02:25 AM, Masayuki Ohtak wrote: > > >>> CAN driver of Topcliff PCH > > >>> > > >>> Topcliff PCH is the platform controller hub that is going to be used in > > >>> Intel's upcoming general embedded platform. All IO peripherals in > > >>> Topcliff PCH are actually devices sitting on AMBA bus. > > >> > > >> That's interesting. Where can I/we find more information about this CAN > > >> controller, e.g. data-sheets. It seems to have a few interesting > > >> features (message scheduler, etc.). > > > > > > I remove all the maillist, and show you something about this platform. > > > > Like Greg, I also prefer the public discussion. > > > > > > > > Topcliff PCH is connected with Tunnelcreak (A kind of ATOM-based > > > processor, > > which integrate Memory controller, GFX and RC). And Topcliff is a kind of > > stand-alone ARM-based processor, so it has AMBA-based peripherals, such as > > CAN, GBE, I2C. > > > > > > Topcliff connected with Tunnelcreak via PCIe x1 lane. And PCH_PHUB works > > as a gateway, which transform PCIe transaction to AMBA transaction, and vice > > versa. > > > > > > The datasheet of those two chips aren't open now. > > > > Thanks for the info. > > > > >>> Topcliff PCH has CAN I/F. This driver enables CAN function. > > >>> > > >>> Signed-off-by: Masayuki Ohtake <[email protected]> > > >> > > >> Thanks for your contribution. Unfortunately, there are many issues, > > >> especially the driver is not yet conform with the Socket-CAN driver API: > > >> > > >> - My first observation was: > > >> > > >> $ wc -l pch_can.c > > >> 4076 pch_can.c > > >> $ grep dev_dbg pch_can.c | wc -l > > >> 143 > > >> > > >> That's a lot of code, mainly debugging code, I think. This needs to > > >> be cleaned up sooner than later. dev_dbg's should be restricted to a > > >> few useful for the real users. We will reduce dev_dbg code. > > >> > > >> - The values for the hw-specific bit-timing registers should be derived > > >> from the calculated values in "priv->can.bittiming": > > >> > > >> http://lxr.linux.no/#linux+v2.6.35/include/linux/can/netlink.h#L17 > > >> I show current pch_can code below. +static int pch_set_bittiming(struct net_device *ndev) +{ + struct pch_can_priv *priv = netdev_priv(ndev); + struct pch_can_os *dev_can_os = priv->pch_can_os_p; + const struct can_bittiming *bt = &priv->can.bittiming; Is the above TRUE, isn't it ? > > >> - The driver should handle state changes and communicate them to the > > >> user space via error messages, if possible. > > >> What's "state chage" mean ? > > >> - The driver should report errors to the user space via error messages. > > >> Is the above mean using alloc_can_err_skb and set error info and notify to kernel with netif_rx ? > > >> - Bus errors seem not to be handled properly.I'm missing can_bus_off(). > > >> Does the controller recover from bus-off automatically? No. CAN driver recovers from Bus-off state. > > >> > > >> - I see that the driver uses many TX and RX objects. How do you avoid > > >> out-of-order transmission and reception? > > > What do you mean out-of-order RX and TX? > > > Atom processor only supports in-order execution, and PCIe-based > > > peripherals > > can solve it with consumer-producer model. Actually IC designer will take > > care > > of out-of-order PCIe CPLD transaction. > > > > I mean out-of-order transmission to or from the CAN bus. This is handled > > by the CAN controller hardware. It has nothing to to with the processor. Cannot avoid occurring rx or tx our-of-order. Thanks, Ohtake(OKISemi) _______________________________________________ Socketcan-core mailing list [email protected] https://lists.berlios.de/mailman/listinfo/socketcan-core
