Thread (17 messages) 17 messages, 4 authors, 2010-11-15

Re: [PATCH net-next-2.6 v2] can: Topcliff: PCH_CAN driver: Fix build warnings

From: Tomoya MORINAGA <hidden>
Date: 2010-11-09 12:26:49
Also in: lkml

Possibly related (same subject, not in this thread)

Sorry, for late response.
On Saturday, October 30, 2010 1:46 AM,  Oliver Hartkopp wrote:
It's just a question if the driver for an Intel Chipset should/could ignore
the endian problematic.

If this is intended the driver should only appear in Kconfig depending on X86
or little endian systems.
This driver is for only x86 processor.
I have no intention to support processor except x86.
Besides this remark, the struct pch_can_regs could also be defined in a way
that every single CAN payload data byte could be accessed directly to allow
e.g. this code in pch_can_rx_normal():

cf->data[0] = ioread8(&priv->regs->if1_data0);
cf->data[1] = ioread8(&priv->regs->if1_data1);
cf->data[2] = ioread8(&priv->regs->if1_data2);
(..)
cf->data[6] = ioread8(&priv->regs->if1_data6);
cf->data[7] = ioread8(&priv->regs->if1_data7);

This is easy to understand and additionally endian aware.
I agree. Thease codes are very simple.
I will modify like above.
In opposite to this:

+ if (cf->can_dlc > 2) {
+ u32 data1 = *((u16 *)&cf->data[2]);
+ iowrite32(data1, &priv->regs->if2_dataa2);
+ }

Which puts a little? endian u16 into the big endian register layout.
Sorry i'm just getting curious on this register access implementation.
I think cf->data is like below
MSB----------LSB
D3-D2-D1-D0
D7-D6-D5-D4

*((u16 *)&cf->data[2]) means "D3-D2".
This order coprresponds to register order.
data register is like below
MSB----------LSB
**-**-D3-D2

** means reserved area.

Thanks,

Tomoya MORINAGA
OKI SEMICONDUCTOR CO., LTD.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help