lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Mon, 13 Sep 2010 21:47:59 +0300 From: Daniel Baluta <daniel.baluta@...il.com> To: Masayuki Ohtak <masa-korg@....okisemi.com> Cc: Wolfgang Grandegger <wg@...ndegger.com>, "David S. Miller" <davem@...emloft.net>, Wolfram Sang <w.sang@...gutronix.de>, Christian Pellegrin <chripell@...e.org>, Barry Song <21cnbao@...il.com>, Samuel Ortiz <sameo@...ux.intel.com>, socketcan-core@...ts.berlios.de, netdev@...r.kernel.org, linux-kernel@...r.kernel.org, meego-dev@...go.com, "Andrew\"" <andrew.chih.howe.khor@...el.com>, "Wang Qi\"" <qi.wang@...el.com>, gregkh@...e.de, "Wang Yong Y\"" <yong.y.wang@...el.com>, Tomoya MORINAGA <morinaga526@....okisemi.com>, arjan@...ux.intel.com Subject: Re: [MeeGo-Dev][PATCH v2] Topcliff: Update PCH_CAN driver to 2.6.35 On Mon, Sep 13, 2010 at 3:22 PM, Masayuki Ohtak <masa-korg@....okisemi.com> 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. > Topcliff PCH has CAN I/F. This driver enables CAN function. > > Signed-off-by: Masayuki Ohtake <masa-korg@....okisemi.com> > +#define MAX_CAN_DEVICES 1 Where is this used? > +#define MAX_BITRATE 0x3e8 > +#define NUM_NODES 2000 /* Maximum number of > + Software FIFO nodes. */ I think you don't need NUM_NODES. > +/** > + * struct pch_can_msg - CAN message structure > + * @ide: Standard/extended msg > + * @id: 11 or 29 bit msg id > + * @dlc: Size of data > + * @data: Message pay load > + * @rtr: RTR message > + */ > +struct pch_can_msg { > + unsigned short ide; > + unsigned int id; > + unsigned short dlc; > + unsigned char data[PCH_CAN_MSG_DATA_LEN]; > + unsigned short rtr; > +}; I think you should use can_frame, and try to get rid of pch_can hardware abstraction. > +struct can_hw { > + void __iomem *io_base; > +}; Why do you need can_hw? > +static int pch_can_get_rx_enable(struct can_hw *can, u32 buff_num, u32 *enable, > + struct net_device *ndev) Here ndev is used only for debugging purposes. It doesn't seem a natural approach. > +static irqreturn_t pch_can_handler(int irq, void *dev_id) > +{ > + irqreturn_t retval = IRQ_NONE; > + u32 int_stat; > + struct pch_can_os *can_os; > + struct net_device *ndev = (struct net_device *)dev_id; > + struct pch_can_priv *priv = netdev_priv(ndev); > + dev_dbg(&can_os->ndev->dev, "%s -> Invoked.\n", __func__); > + > + can_os = &priv->pch_can_os_p; > + int_stat = pch_can_int_pending(can_os->can); Where are the interrupt status bit(s) cleared? > + dev_dbg(&can_os->ndev->dev, > + "%s -> pch_can_int_pending returned value: %x\n", __func__, int_stat); > + As someone stated before I think you are trying to port an old fashion CAN character device based driver. Although, having the old source code, should be helpful I think you should read the existing CAN drivers (drivers/net/can) and start from scratch trying to follow the ideas implemented there. I also have a character driver very similar with yours (ICP CAN for EP80579) and I struggle for some time to get a working version with Socket CAN interface. I don't have any 'official' position towards Linux Socket CAN so please consider my comments as being advisory. thanks, Daniel. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists