Messages in this thread | | | Date | Mon, 13 Sep 2010 21:47:59 +0300 | Subject | Re: [MeeGo-Dev][PATCH v2] Topcliff: Update PCH_CAN driver to 2.6.35 | From | Daniel Baluta <> |
| |
On Mon, Sep 13, 2010 at 3:22 PM, Masayuki Ohtak <masa-korg@dsn.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@dsn.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 linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |