Skip to contentSkip to conversation

Topcliff: Update PCH_IEEE1588 driver to 2.6.35

PATCH In review Masayuki Ohtak 10 participants 25 messages Aug 10, 2010 – Dec 15, 2010 Char and misc drivers

lore
Share & tools
lore

Masayuki Ohtak

author
#0 lore raw

PATCH 1/1 Topcliff: Update PCH_IEEE1588 driver to 2.6.35

IEEE1588 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 IEEE1588 I/F. This driver enables IEEE1588 function on CAN or Ethernet communications.

Signed-off-by Masayuki Ohtake <[email protected]>
---
 drivers/char/Kconfig                     |    9 +
 drivers/char/Makefile                    |    2 +
 drivers/char/pch_ieee1588/Makefile       |    1 +
 drivers/char/pch_ieee1588/pch_ieee1588.c | 3947 ++++++++++++++++++++++++++++++
 drivers/char/pch_ieee1588/pch_ieee1588.h |  528 ++++
 5 files changed, 4487 insertions(+), 0 deletions(-)
 create mode 100755 drivers/char/pch_ieee1588/Makefile
 create mode 100644 drivers/char/pch_ieee1588/pch_ieee1588.c
 create mode 100644 drivers/char/pch_ieee1588/pch_ieee1588.h
drivers/char/Kconfig +9 -0
index 7cfcc62..4784971 100644
@@ -4,6 +4,15 @@
 
 menu "Character devices"
 
+config PCH_IEEE1588
+	tristate "PCH IEEE1588 of Intel Topcliff"
+	depends on PCI
+	help
+	  This driver is for PCH IEEE1588 of Topcliff which is an IOH for x86
+	  embedded processor.
+	  This driver enables IEEE1588 function on CAN or Ethernet
+	  communications.
+
 config VT
 	bool "Virtual terminal" if EMBEDDED
 	depends on !S390
drivers/char/Makefile +2 -0
index 88d6eac..81fb2d0 100644
@@ -113,6 +113,8 @@ obj-$(CONFIG_RAMOOPS)		+= ramoops.o
 obj-$(CONFIG_JS_RTC)		+= js-rtc.o
 js-rtc-y = rtc.o
 
+obj-$(CONFIG_PCH_IEEE1588)		+= pch_ieee1588/
+
 # Files generated that shall be removed upon make clean
 clean-files := consolemap_deftbl.c defkeymap.c
drivers/char/pch_ieee1588/Makefile new file +1 -0
new file mode 100755
index 0000000..e5199ac
@@ -0,0 +1 @@
+obj-$(CONFIG_PCH_IEEE1588) += pch_ieee1588.o
drivers/char/pch_ieee1588/pch_ieee1588.c new file +3947 -0

2,976 lines not on this page · raw message

drivers/char/pch_ieee1588/pch_ieee1588.h new file +528 -0

528 lines not on this page · raw message

The rest of this diff is not shown. View the raw message.

Signature · 1 line
1.6.0.6

Part of the diff is not shown here. View the raw message.

#1 lore raw
Quoted 7 lines · Masayuki Ohtak

IEEE1588 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 IEEE1588 I/F. This driver enables IEEE1588 function on CAN or Ethernet communications.

So it's a CAN or Ethernet driver? If so, why isn't this a network driver like the CAN and Ethernet drivers?

drivers/char/pch_ieee1588/pch_ieee1588.h line 31

Quoted 11 lines
drivers/char/pch_ieee1588/pch_ieee1588.h new file +11 -0
+#ifdef __GNUC__
+#define UNUSED __attribute__ ((unused))
+#define UNUSED_ARG(x)
+#else
+#define UNUSED
+#define UNUSED_ARG(x) (void) x
+#endif
+
+
+#define TRUE 1
+#define FALSE 0

These lines should not be needed at all.

Quoted 1 line

+#define IOC_1588_BASE 0xf8

Do you have documentation for these new ioctls you are creating? It's a lot of them:

Quoted 93 lines
drivers/char/pch_ieee1588/pch_ieee1588.h new file +93 -0
+#define IOCTL_1588_PORT_CONFIG_SET \
+			_IOW(IOC_1588_BASE, 0, struct pch_portcfg_ioctl)
+
+#define IOCTL_1588_PORT_CONFIG_GET \
+			_IOWR(IOC_1588_BASE, 1, struct pch_portcfg_ioctl)
+
+#define IOCTL_1588_RX_POLL _IOWR(IOC_1588_BASE, 2, struct pch_rxtxpoll_ioctl)
+
+#define IOCTL_1588_TX_POLL _IOWR(IOC_1588_BASE, 3, struct pch_rxtxpoll_ioctl)
+
+#define IOCTL_1588_CAN_POLL _IOWR(IOC_1588_BASE, 4, struct pch_canpoll_ioctl)
+
+#define IOCTL_1588_SYS_TIME_GET _IOR(IOC_1588_BASE, 5, struct pch_tim_val)
+
+#define IOCTL_1588_SYS_TIME_SET _IOW(IOC_1588_BASE, 6, struct pch_tim_val)
+
+#define IOCTL_1588_TICK_RATE_SET _IOW(IOC_1588_BASE, 7, __u32)
+
+#define IOCTL_1588_TICK_RATE_GET _IOR(IOC_1588_BASE, 8, __u32)
+
+#define IOCTL_1588_TARG_TIME_INTRPT_ENABLE _IO(IOC_1588_BASE, 9)
+
+#define IOCTL_1588_TARG_TIME_INTRPT_DISABLE _IO(IOC_1588_BASE, 10)
+
+#define IOCTL_1588_TARG_TIME_POLL \
+			_IOR(IOC_1588_BASE, 11, struct pch_timepoll_ioctl)
+
+#define IOCTL_1588_TARG_TIME_SET \
+				_IOW(IOC_1588_BASE, 12, struct pch_tim_val)
+
+#define IOCTL_1588_TARG_TIME_GET \
+				_IOR(IOC_1588_BASE, 13, struct pch_tim_val)
+
+#define IOCTL_1588_AUX_TIME_INTRPT_ENABLE _IOW(IOC_1588_BASE, 14,\
+							enum pch_auxmode)
+
+#define IOCTL_1588_AUX_TIME_INTRPT_DISABLE _IOW(IOC_1588_BASE, 15,\
+							enum pch_auxmode)
+
+#define IOCTL_1588_AUX_TIME_POLL \
+			_IOWR(IOC_1588_BASE, 16, struct pch_timepoll_ioctl)
+
+#define IOCTL_1588_RESET _IO(IOC_1588_BASE, 17)
+
+#define IOCTL_1588_CHNL_RESET _IOW(IOC_1588_BASE, 18, enum pch_ptpport)
+
+#define IOCTL_1588_STATS_GET _IOR(IOC_1588_BASE, 19, struct pch_stats)
+
+#define IOCTL_1588_STATS_RESET _IO(IOC_1588_BASE, 20)
+
+#define IOCTL_1588_SHOW_ALL _IO(IOC_1588_BASE, 21)
+
+#define IOCTL_1588_AUX_TARG_TIME_INTRPT_ENABLE _IO(IOC_1588_BASE, 22)
+
+#define IOCTL_1588_AUX_TARG_TIME_INTRPT_DISABLE _IO(IOC_1588_BASE, 23)
+
+#define IOCTL_1588_AUX_TARG_TIME_POLL \
+			_IOR(IOC_1588_BASE, 24, struct pch_timepoll_ioctl)
+
+#define IOCTL_1588_AUX_TARG_TIME_SET \
+				_IOW(IOC_1588_BASE, 25, struct pch_tim_val)
+
+#define IOCTL_1588_AUX_TARG_TIME_GET \
+				_IOR(IOC_1588_BASE, 26, struct pch_tim_val)
+
+#define IOCTL_1588_PULSE_PER_SEC_INTRPT_ENABLE _IO(IOC_1588_BASE, 27)
+
+#define IOCTL_1588_PULSE_PER_SEC_INTRPT_DISABLE _IO(IOC_1588_BASE, 28)
+
+#define IOCTL_1588_TARG_TIME_NOTIFY \
+				_IOR(IOC_1588_BASE, 29, struct pch_tim_val)
+
+#define IOCTL_1588_AUX_TIME_NOTIFY \
+			_IOR(IOC_1588_BASE, 30, struct pch_auxtimeioctl)
+
+#define IOCTL_1588_AUX_TARG_TIME_NOTIFY \
+				_IOR(IOC_1588_BASE, 31, struct pch_tim_val)
+
+#define IOCTL_1588_PULSE_PER_SEC_NOTIFY _IOR(IOC_1588_BASE, 32, __u32)
+
+#define IOCTL_1588_TARG_TIME_CLR_NOTIFY _IO(IOC_1588_BASE, 33)
+
+#define IOCTL_1588_AUX_TIME_CLR_NOTIFY _IO(IOC_1588_BASE, 34)
+
+#define IOCTL_1588_AUX_TARG_TIME_CLR_NOTIFY _IO(IOC_1588_BASE, 35)
+
+#define IOCTL_1588_PULSE_PER_SEC_CLR_NOTIFY _IO(IOC_1588_BASE, 36)
+
+#define IOCTL_1588_PULSE_PER_SEC_TIME_GET _IOR(IOC_1588_BASE, 37, __u32)
+
+#define IOCTL_1588_PULSE_PER_SEC_TIME_SET _IOW(IOC_1588_BASE, 38, __u32)
+
+#define IOCTL_1588_PORT_VERSION_SET \

drivers/char/pch_ieee1588/pch_ieee1588.h line 139

Quoted 10 lines
drivers/char/pch_ieee1588/pch_ieee1588.h new file +10 -0
+			_IOW(IOC_1588_BASE, 39, struct pch_versionioctl)
+
+#define IOCTL_1588_PORT_VERSION_GET \
+			_IOWR(IOC_1588_BASE, 40, struct pch_versionioctl)
+
+#define IOCTL_1588_PORT_OPERATION_MODE_SET \
+		_IOW(IOC_1588_BASE, 41, struct pch_opemode_ioctl)
+
+#define IOCTL_1588_PORT_OPERATION_MODE_GET \
+		_IOWR(IOC_1588_BASE, 42, struct pch_opemode_ioctl)

Do they all have to be ioctls? What exactly are they doing?

And are they 32/64bit safe?

thanks,

greg k-h

#2 lore raw
Quoted 12 lines · On Tue, 10 Aug 2010 10:13:43 -0700 Greg KH wrote:
Quoted 7 lines · Masayuki Ohtak

IEEE1588 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 IEEE1588 I/F. This driver enables IEEE1588 function on CAN or Ethernet communications.

So it's a CAN or Ethernet driver? If so, why isn't this a network driver like the CAN and Ethernet drivers?

Quoted 15 lines

drivers/char/pch_ieee1588/pch_ieee1588.h line 31

Quoted 11 lines
drivers/char/pch_ieee1588/pch_ieee1588.h new file +11 -0
+#ifdef __GNUC__
+#define UNUSED __attribute__ ((unused))
+#define UNUSED_ARG(x)
+#else
+#define UNUSED
+#define UNUSED_ARG(x) (void) x
+#endif
+
+
+#define TRUE 1
+#define FALSE 0

These lines should not be needed at all.

Quoted 1 line

+#define IOC_1588_BASE 0xf8

and 0xf8 should be added to Documentation/ioctl/ioctl-number.txt, please.

Quoted 110 lines

Do you have documentation for these new ioctls you are creating? It's a lot of them:

Quoted 93 lines
drivers/char/pch_ieee1588/pch_ieee1588.h new file +93 -0
+#define IOCTL_1588_PORT_CONFIG_SET \
+			_IOW(IOC_1588_BASE, 0, struct pch_portcfg_ioctl)
+
+#define IOCTL_1588_PORT_CONFIG_GET \
+			_IOWR(IOC_1588_BASE, 1, struct pch_portcfg_ioctl)
+
+#define IOCTL_1588_RX_POLL _IOWR(IOC_1588_BASE, 2, struct pch_rxtxpoll_ioctl)
+
+#define IOCTL_1588_TX_POLL _IOWR(IOC_1588_BASE, 3, struct pch_rxtxpoll_ioctl)
+
+#define IOCTL_1588_CAN_POLL _IOWR(IOC_1588_BASE, 4, struct pch_canpoll_ioctl)
+
+#define IOCTL_1588_SYS_TIME_GET _IOR(IOC_1588_BASE, 5, struct pch_tim_val)
+
+#define IOCTL_1588_SYS_TIME_SET _IOW(IOC_1588_BASE, 6, struct pch_tim_val)
+
+#define IOCTL_1588_TICK_RATE_SET _IOW(IOC_1588_BASE, 7, __u32)
+
+#define IOCTL_1588_TICK_RATE_GET _IOR(IOC_1588_BASE, 8, __u32)
+
+#define IOCTL_1588_TARG_TIME_INTRPT_ENABLE _IO(IOC_1588_BASE, 9)
+
+#define IOCTL_1588_TARG_TIME_INTRPT_DISABLE _IO(IOC_1588_BASE, 10)
+
+#define IOCTL_1588_TARG_TIME_POLL \
+			_IOR(IOC_1588_BASE, 11, struct pch_timepoll_ioctl)
+
+#define IOCTL_1588_TARG_TIME_SET \
+				_IOW(IOC_1588_BASE, 12, struct pch_tim_val)
+
+#define IOCTL_1588_TARG_TIME_GET \
+				_IOR(IOC_1588_BASE, 13, struct pch_tim_val)
+
+#define IOCTL_1588_AUX_TIME_INTRPT_ENABLE _IOW(IOC_1588_BASE, 14,\
+							enum pch_auxmode)
+
+#define IOCTL_1588_AUX_TIME_INTRPT_DISABLE _IOW(IOC_1588_BASE, 15,\
+							enum pch_auxmode)
+
+#define IOCTL_1588_AUX_TIME_POLL \
+			_IOWR(IOC_1588_BASE, 16, struct pch_timepoll_ioctl)
+
+#define IOCTL_1588_RESET _IO(IOC_1588_BASE, 17)
+
+#define IOCTL_1588_CHNL_RESET _IOW(IOC_1588_BASE, 18, enum pch_ptpport)
+
+#define IOCTL_1588_STATS_GET _IOR(IOC_1588_BASE, 19, struct pch_stats)
+
+#define IOCTL_1588_STATS_RESET _IO(IOC_1588_BASE, 20)
+
+#define IOCTL_1588_SHOW_ALL _IO(IOC_1588_BASE, 21)
+
+#define IOCTL_1588_AUX_TARG_TIME_INTRPT_ENABLE _IO(IOC_1588_BASE, 22)
+
+#define IOCTL_1588_AUX_TARG_TIME_INTRPT_DISABLE _IO(IOC_1588_BASE, 23)
+
+#define IOCTL_1588_AUX_TARG_TIME_POLL \
+			_IOR(IOC_1588_BASE, 24, struct pch_timepoll_ioctl)
+
+#define IOCTL_1588_AUX_TARG_TIME_SET \
+				_IOW(IOC_1588_BASE, 25, struct pch_tim_val)
+
+#define IOCTL_1588_AUX_TARG_TIME_GET \
+				_IOR(IOC_1588_BASE, 26, struct pch_tim_val)
+
+#define IOCTL_1588_PULSE_PER_SEC_INTRPT_ENABLE _IO(IOC_1588_BASE, 27)
+
+#define IOCTL_1588_PULSE_PER_SEC_INTRPT_DISABLE _IO(IOC_1588_BASE, 28)
+
+#define IOCTL_1588_TARG_TIME_NOTIFY \
+				_IOR(IOC_1588_BASE, 29, struct pch_tim_val)
+
+#define IOCTL_1588_AUX_TIME_NOTIFY \
+			_IOR(IOC_1588_BASE, 30, struct pch_auxtimeioctl)
+
+#define IOCTL_1588_AUX_TARG_TIME_NOTIFY \
+				_IOR(IOC_1588_BASE, 31, struct pch_tim_val)
+
+#define IOCTL_1588_PULSE_PER_SEC_NOTIFY _IOR(IOC_1588_BASE, 32, __u32)
+
+#define IOCTL_1588_TARG_TIME_CLR_NOTIFY _IO(IOC_1588_BASE, 33)
+
+#define IOCTL_1588_AUX_TIME_CLR_NOTIFY _IO(IOC_1588_BASE, 34)
+
+#define IOCTL_1588_AUX_TARG_TIME_CLR_NOTIFY _IO(IOC_1588_BASE, 35)
+
+#define IOCTL_1588_PULSE_PER_SEC_CLR_NOTIFY _IO(IOC_1588_BASE, 36)
+
+#define IOCTL_1588_PULSE_PER_SEC_TIME_GET _IOR(IOC_1588_BASE, 37, __u32)
+
+#define IOCTL_1588_PULSE_PER_SEC_TIME_SET _IOW(IOC_1588_BASE, 38, __u32)
+
+#define IOCTL_1588_PORT_VERSION_SET \

drivers/char/pch_ieee1588/pch_ieee1588.h line 139

Quoted 10 lines
drivers/char/pch_ieee1588/pch_ieee1588.h new file +10 -0
+			_IOW(IOC_1588_BASE, 39, struct pch_versionioctl)
+
+#define IOCTL_1588_PORT_VERSION_GET \
+			_IOWR(IOC_1588_BASE, 40, struct pch_versionioctl)
+
+#define IOCTL_1588_PORT_OPERATION_MODE_SET \
+		_IOW(IOC_1588_BASE, 41, struct pch_opemode_ioctl)
+
+#define IOCTL_1588_PORT_OPERATION_MODE_GET \
+		_IOWR(IOC_1588_BASE, 42, struct pch_opemode_ioctl)

Do they all have to be ioctls? What exactly are they doing?

Quoted 1 line

And are they 32/64bit safe?

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

#3 lore raw
In reply to Greg KH
Quoted 16 lines

-----Original Message-----
From: Greg KH [mailto:[email protected]]
Sent: Wednesday, August 11, 2010 1:14 AM
To: Masayuki Ohtak
Cc: [email protected]; LKML; Wang, Qi; Wang, Yong Y; Khor, Andrew Chih Howe; [email protected]
Subject: Re: [MeeGo-Dev][PATCH] Topcliff: Update PCH_IEEE1588 driver to 2.6.35

Quoted 6 lines · Masayuki Ohtak

IEEE1588 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 IEEE1588 I/F. This driver enables IEEE1588 function on CAN

Quoted 5 lines

or

Quoted 1 line

Ethernet communications.

So it's a CAN or Ethernet driver? If so, why isn't this a network driver like the CAN and Ethernet drivers?

This socket CAN drivers which can use Berkeley socket API, and the Linux network stack and implements the CAN device drivers as network interfaces.

Quoted 14 lines

drivers/char/pch_ieee1588/pch_ieee1588.h line 31

Quoted 12 lines
drivers/char/pch_ieee1588/pch_ieee1588.h new file +11 -0
+#ifdef __GNUC__
+#define UNUSED __attribute__ ((unused))
+#define UNUSED_ARG(x)
+#else
+#define UNUSED
+#define UNUSED_ARG(x) (void) x
+#endif
+
+
+#define TRUE 1
+#define FALSE 0

These lines should not be needed at all.

OKI-san, please remove them, Those definition is useless.

Quoted 131 lines
Quoted 2 lines

+#define IOC_1588_BASE 0xf8

Do you have documentation for these new ioctls you are creating? It's a lot of them:

drivers/char/pch_ieee1588/pch_ieee1588.h

Quoted 7 lines
drivers/char/pch_ieee1588/pch_ieee1588.h new file +7 -0
+#define IOCTL_1588_PORT_CONFIG_SET \
+			_IOW(IOC_1588_BASE, 0, struct pch_portcfg_ioctl)
+
+#define IOCTL_1588_PORT_CONFIG_GET \
+			_IOWR(IOC_1588_BASE, 1, struct pch_portcfg_ioctl)
+
+#define IOCTL_1588_RX_POLL _IOWR(IOC_1588_BASE, 2, struct

pch_rxtxpoll_ioctl)

Review

Quoted 2 lines
(quoted diff) +2 -0
+
+#define IOCTL_1588_TX_POLL _IOWR(IOC_1588_BASE, 3, struct

pch_rxtxpoll_ioctl)

Review

Quoted 2 lines
(quoted diff) +2 -0
+
+#define IOCTL_1588_CAN_POLL _IOWR(IOC_1588_BASE, 4, struct

pch_canpoll_ioctl)

Review

Quoted 2 lines
(quoted diff) +2 -0
+
+#define IOCTL_1588_SYS_TIME_GET _IOR(IOC_1588_BASE, 5, struct

pch_tim_val)

Review

Quoted 2 lines
(quoted diff) +2 -0
+
+#define IOCTL_1588_SYS_TIME_SET _IOW(IOC_1588_BASE, 6, struct

pch_tim_val)

drivers/char/pch_ieee1588/pch_ieee1588.h

Quoted 8 lines
drivers/char/pch_ieee1588/pch_ieee1588.h new file +8 -0
+
+#define IOCTL_1588_TICK_RATE_SET _IOW(IOC_1588_BASE, 7, __u32)
+
+#define IOCTL_1588_TICK_RATE_GET _IOR(IOC_1588_BASE, 8, __u32)
+
+#define IOCTL_1588_TARG_TIME_INTRPT_ENABLE _IO(IOC_1588_BASE, 9)
+
+#define IOCTL_1588_TARG_TIME_INTRPT_DISABLE _IO(IOC_1588_BASE,

10)

drivers/char/pch_ieee1588/pch_ieee1588.h

Quoted 11 lines
drivers/char/pch_ieee1588/pch_ieee1588.h new file +11 -0
+
+#define IOCTL_1588_TARG_TIME_POLL \
+			_IOR(IOC_1588_BASE, 11, struct pch_timepoll_ioctl)
+
+#define IOCTL_1588_TARG_TIME_SET \
+				_IOW(IOC_1588_BASE, 12, struct pch_tim_val)
+
+#define IOCTL_1588_TARG_TIME_GET \
+				_IOR(IOC_1588_BASE, 13, struct pch_tim_val)
+
+#define IOCTL_1588_AUX_TIME_INTRPT_ENABLE _IOW(IOC_1588_BASE,

14,\

drivers/char/pch_ieee1588/pch_ieee1588.h

Quoted 3 lines
drivers/char/pch_ieee1588/pch_ieee1588.h new file +3 -0
+							enum pch_auxmode)
+
+#define IOCTL_1588_AUX_TIME_INTRPT_DISABLE _IOW(IOC_1588_BASE,

15,\

drivers/char/pch_ieee1588/pch_ieee1588.h

Quoted 8 lines
drivers/char/pch_ieee1588/pch_ieee1588.h new file +8 -0
+							enum pch_auxmode)
+
+#define IOCTL_1588_AUX_TIME_POLL \
+			_IOWR(IOC_1588_BASE, 16, struct pch_timepoll_ioctl)
+
+#define IOCTL_1588_RESET _IO(IOC_1588_BASE, 17)
+
+#define IOCTL_1588_CHNL_RESET _IOW(IOC_1588_BASE, 18, enum

pch_ptpport)

Review

Quoted 2 lines
(quoted diff) +2 -0
+
+#define IOCTL_1588_STATS_GET _IOR(IOC_1588_BASE, 19, struct

pch_stats)

drivers/char/pch_ieee1588/pch_ieee1588.h

Quoted 6 lines
drivers/char/pch_ieee1588/pch_ieee1588.h new file +6 -0
+
+#define IOCTL_1588_STATS_RESET _IO(IOC_1588_BASE, 20)
+
+#define IOCTL_1588_SHOW_ALL _IO(IOC_1588_BASE, 21)
+
+#define IOCTL_1588_AUX_TARG_TIME_INTRPT_ENABLE

_IO(IOC_1588_BASE, 22)

Review

Quoted 2 lines
(quoted diff) +2 -0
+
+#define IOCTL_1588_AUX_TARG_TIME_INTRPT_DISABLE

_IO(IOC_1588_BASE, 23)

drivers/char/pch_ieee1588/pch_ieee1588.h

Quoted 11 lines
drivers/char/pch_ieee1588/pch_ieee1588.h new file +11 -0
+
+#define IOCTL_1588_AUX_TARG_TIME_POLL \
+			_IOR(IOC_1588_BASE, 24, struct pch_timepoll_ioctl)
+
+#define IOCTL_1588_AUX_TARG_TIME_SET \
+				_IOW(IOC_1588_BASE, 25, struct pch_tim_val)
+
+#define IOCTL_1588_AUX_TARG_TIME_GET \
+				_IOR(IOC_1588_BASE, 26, struct pch_tim_val)
+
+#define IOCTL_1588_PULSE_PER_SEC_INTRPT_ENABLE

_IO(IOC_1588_BASE, 27)

Review

Quoted 2 lines
(quoted diff) +2 -0
+
+#define IOCTL_1588_PULSE_PER_SEC_INTRPT_DISABLE

_IO(IOC_1588_BASE, 28)

drivers/char/pch_ieee1588/pch_ieee1588.h

Quoted 11 lines
drivers/char/pch_ieee1588/pch_ieee1588.h new file +11 -0
+
+#define IOCTL_1588_TARG_TIME_NOTIFY \
+				_IOR(IOC_1588_BASE, 29, struct pch_tim_val)
+
+#define IOCTL_1588_AUX_TIME_NOTIFY \
+			_IOR(IOC_1588_BASE, 30, struct pch_auxtimeioctl)
+
+#define IOCTL_1588_AUX_TARG_TIME_NOTIFY \
+				_IOR(IOC_1588_BASE, 31, struct pch_tim_val)
+
+#define IOCTL_1588_PULSE_PER_SEC_NOTIFY _IOR(IOC_1588_BASE, 32,

__u32)

drivers/char/pch_ieee1588/pch_ieee1588.h

Quoted 6 lines
drivers/char/pch_ieee1588/pch_ieee1588.h new file +6 -0
+
+#define IOCTL_1588_TARG_TIME_CLR_NOTIFY _IO(IOC_1588_BASE, 33)
+
+#define IOCTL_1588_AUX_TIME_CLR_NOTIFY _IO(IOC_1588_BASE, 34)
+
+#define IOCTL_1588_AUX_TARG_TIME_CLR_NOTIFY _IO(IOC_1588_BASE,

35)

Review

Quoted 2 lines
(quoted diff) +2 -0
+
+#define IOCTL_1588_PULSE_PER_SEC_CLR_NOTIFY _IO(IOC_1588_BASE,

36)

Review

Quoted 2 lines
(quoted diff) +2 -0
+
+#define IOCTL_1588_PULSE_PER_SEC_TIME_GET _IOR(IOC_1588_BASE, 37,

__u32)

Review

Quoted 2 lines
(quoted diff) +2 -0
+
+#define IOCTL_1588_PULSE_PER_SEC_TIME_SET _IOW(IOC_1588_BASE, 38,

__u32)

drivers/char/pch_ieee1588/pch_ieee1588.h line 139

Quoted 12 lines
drivers/char/pch_ieee1588/pch_ieee1588.h new file +12 -0
+
+#define IOCTL_1588_PORT_VERSION_SET \
+			_IOW(IOC_1588_BASE, 39, struct pch_versionioctl)
+
+#define IOCTL_1588_PORT_VERSION_GET \
+			_IOWR(IOC_1588_BASE, 40, struct pch_versionioctl)
+
+#define IOCTL_1588_PORT_OPERATION_MODE_SET \
+		_IOW(IOC_1588_BASE, 41, struct pch_opemode_ioctl)
+
+#define IOCTL_1588_PORT_OPERATION_MODE_GET \
+		_IOWR(IOC_1588_BASE, 42, struct pch_opemode_ioctl)

Do they all have to be ioctls? What exactly are they doing?

Quoted 5 lines

And are they 32/64bit safe?

thanks,

greg k-h

Best Regards,
Qi ���{.n�+�������+%�����ݶ��w��{.n�+����{��G�����{ay�ʇڙ�,j��f���h��������z_�(�階�ݢj"���m������G������?����&���~��iO��z��v�^�m���� ������?�I�

author
#4 lore raw
Quoted 5 lines
Quoted 4 lines

drivers/char/pch_ieee1588/pch_ieee1588.h line 31

Quoted 2 lines
drivers/char/pch_ieee1588/pch_ieee1588.h new file +2 -0
+#define TRUE 1
+#define FALSE 0

These lines should not be needed at all.

OKI-san, please remove them, Those definition is useless.

Is the above definition is necessary?
I can't find appropriate header file defines 'TRUE' and 'FALSE'. In addition, some drivers have these definitions.
For example, drivers/char/mwave/smapi.h has the above definitions.

Thanks, Ohtake
----- Original Message -----
From: "Wang, Qi" <[email protected]>
To: "Greg KH" <[email protected]>; "Masayuki Ohtak" <[email protected]>
Cc: <[email protected]>; "LKML" <[email protected]>; "Wang, Yong Y" <[email protected]>; "Khor, Andrew Chih Howe" <[email protected]>; <[email protected]>
Sent: Wednesday, August 11, 2010 10:20 AM
Subject: RE: [MeeGo-Dev][PATCH] Topcliff: Update PCH_IEEE1588 driver to 2.6.35

Quoted 16 lines
Quoted 16 lines

-----Original Message-----
From: Greg KH [mailto:[email protected]]
Sent: Wednesday, August 11, 2010 1:14 AM
To: Masayuki Ohtak
Cc: [email protected]; LKML; Wang, Qi; Wang, Yong Y; Khor, Andrew Chih Howe; [email protected]
Subject: Re: [MeeGo-Dev][PATCH] Topcliff: Update PCH_IEEE1588 driver to 2.6.35

Quoted 6 lines · Masayuki Ohtak

IEEE1588 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 IEEE1588 I/F. This driver enables IEEE1588 function on CAN

Quoted 7 lines
Quoted 5 lines

or

Quoted 1 line

Ethernet communications.

So it's a CAN or Ethernet driver? If so, why isn't this a network driver like the CAN and Ethernet drivers?

This socket CAN drivers which can use Berkeley socket API, and the Linux network stack and implements the CAN device

drivers as network interfaces.

Quoted 155 lines
Quoted 14 lines

drivers/char/pch_ieee1588/pch_ieee1588.h line 31

Quoted 12 lines
drivers/char/pch_ieee1588/pch_ieee1588.h new file +11 -0
+#ifdef __GNUC__
+#define UNUSED __attribute__ ((unused))
+#define UNUSED_ARG(x)
+#else
+#define UNUSED
+#define UNUSED_ARG(x) (void) x
+#endif
+
+
+#define TRUE 1
+#define FALSE 0

These lines should not be needed at all.

OKI-san, please remove them, Those definition is useless.

Quoted 131 lines
Quoted 2 lines

+#define IOC_1588_BASE 0xf8

Do you have documentation for these new ioctls you are creating? It's a lot of them:

drivers/char/pch_ieee1588/pch_ieee1588.h

Quoted 7 lines
drivers/char/pch_ieee1588/pch_ieee1588.h new file +7 -0
+#define IOCTL_1588_PORT_CONFIG_SET \
+ _IOW(IOC_1588_BASE, 0, struct pch_portcfg_ioctl)
+
+#define IOCTL_1588_PORT_CONFIG_GET \
+ _IOWR(IOC_1588_BASE, 1, struct pch_portcfg_ioctl)
+
+#define IOCTL_1588_RX_POLL _IOWR(IOC_1588_BASE, 2, struct

pch_rxtxpoll_ioctl)

Review

Quoted 2 lines
(quoted diff) +2 -0
+
+#define IOCTL_1588_TX_POLL _IOWR(IOC_1588_BASE, 3, struct

pch_rxtxpoll_ioctl)

Review

Quoted 2 lines
(quoted diff) +2 -0
+
+#define IOCTL_1588_CAN_POLL _IOWR(IOC_1588_BASE, 4, struct

pch_canpoll_ioctl)

Review

Quoted 2 lines
(quoted diff) +2 -0
+
+#define IOCTL_1588_SYS_TIME_GET _IOR(IOC_1588_BASE, 5, struct

pch_tim_val)

Review

Quoted 2 lines
(quoted diff) +2 -0
+
+#define IOCTL_1588_SYS_TIME_SET _IOW(IOC_1588_BASE, 6, struct

pch_tim_val)

drivers/char/pch_ieee1588/pch_ieee1588.h

Quoted 8 lines
drivers/char/pch_ieee1588/pch_ieee1588.h new file +8 -0
+
+#define IOCTL_1588_TICK_RATE_SET _IOW(IOC_1588_BASE, 7, __u32)
+
+#define IOCTL_1588_TICK_RATE_GET _IOR(IOC_1588_BASE, 8, __u32)
+
+#define IOCTL_1588_TARG_TIME_INTRPT_ENABLE _IO(IOC_1588_BASE, 9)
+
+#define IOCTL_1588_TARG_TIME_INTRPT_DISABLE _IO(IOC_1588_BASE,

10)

drivers/char/pch_ieee1588/pch_ieee1588.h

Quoted 11 lines
drivers/char/pch_ieee1588/pch_ieee1588.h new file +11 -0
+
+#define IOCTL_1588_TARG_TIME_POLL \
+ _IOR(IOC_1588_BASE, 11, struct pch_timepoll_ioctl)
+
+#define IOCTL_1588_TARG_TIME_SET \
+ _IOW(IOC_1588_BASE, 12, struct pch_tim_val)
+
+#define IOCTL_1588_TARG_TIME_GET \
+ _IOR(IOC_1588_BASE, 13, struct pch_tim_val)
+
+#define IOCTL_1588_AUX_TIME_INTRPT_ENABLE _IOW(IOC_1588_BASE,

14,\

Review

Quoted 3 lines

+ enum pch_auxmode)

(quoted diff) +2 -0
+
+#define IOCTL_1588_AUX_TIME_INTRPT_DISABLE _IOW(IOC_1588_BASE,

15,\

drivers/char/pch_ieee1588/pch_ieee1588.h

Quoted 8 lines

+ enum pch_auxmode)

drivers/char/pch_ieee1588/pch_ieee1588.h new file +7 -0
+
+#define IOCTL_1588_AUX_TIME_POLL \
+ _IOWR(IOC_1588_BASE, 16, struct pch_timepoll_ioctl)
+
+#define IOCTL_1588_RESET _IO(IOC_1588_BASE, 17)
+
+#define IOCTL_1588_CHNL_RESET _IOW(IOC_1588_BASE, 18, enum

pch_ptpport)

Review

Quoted 2 lines
(quoted diff) +2 -0
+
+#define IOCTL_1588_STATS_GET _IOR(IOC_1588_BASE, 19, struct

pch_stats)

drivers/char/pch_ieee1588/pch_ieee1588.h

Quoted 6 lines
drivers/char/pch_ieee1588/pch_ieee1588.h new file +6 -0
+
+#define IOCTL_1588_STATS_RESET _IO(IOC_1588_BASE, 20)
+
+#define IOCTL_1588_SHOW_ALL _IO(IOC_1588_BASE, 21)
+
+#define IOCTL_1588_AUX_TARG_TIME_INTRPT_ENABLE

_IO(IOC_1588_BASE, 22)

Review

Quoted 2 lines
(quoted diff) +2 -0
+
+#define IOCTL_1588_AUX_TARG_TIME_INTRPT_DISABLE

_IO(IOC_1588_BASE, 23)

drivers/char/pch_ieee1588/pch_ieee1588.h

Quoted 11 lines
drivers/char/pch_ieee1588/pch_ieee1588.h new file +11 -0
+
+#define IOCTL_1588_AUX_TARG_TIME_POLL \
+ _IOR(IOC_1588_BASE, 24, struct pch_timepoll_ioctl)
+
+#define IOCTL_1588_AUX_TARG_TIME_SET \
+ _IOW(IOC_1588_BASE, 25, struct pch_tim_val)
+
+#define IOCTL_1588_AUX_TARG_TIME_GET \
+ _IOR(IOC_1588_BASE, 26, struct pch_tim_val)
+
+#define IOCTL_1588_PULSE_PER_SEC_INTRPT_ENABLE

_IO(IOC_1588_BASE, 27)

Review

Quoted 2 lines
(quoted diff) +2 -0
+
+#define IOCTL_1588_PULSE_PER_SEC_INTRPT_DISABLE

_IO(IOC_1588_BASE, 28)

drivers/char/pch_ieee1588/pch_ieee1588.h

Quoted 11 lines
drivers/char/pch_ieee1588/pch_ieee1588.h new file +11 -0
+
+#define IOCTL_1588_TARG_TIME_NOTIFY \
+ _IOR(IOC_1588_BASE, 29, struct pch_tim_val)
+
+#define IOCTL_1588_AUX_TIME_NOTIFY \
+ _IOR(IOC_1588_BASE, 30, struct pch_auxtimeioctl)
+
+#define IOCTL_1588_AUX_TARG_TIME_NOTIFY \
+ _IOR(IOC_1588_BASE, 31, struct pch_tim_val)
+
+#define IOCTL_1588_PULSE_PER_SEC_NOTIFY _IOR(IOC_1588_BASE, 32,

__u32)

drivers/char/pch_ieee1588/pch_ieee1588.h

Quoted 6 lines
drivers/char/pch_ieee1588/pch_ieee1588.h new file +6 -0
+
+#define IOCTL_1588_TARG_TIME_CLR_NOTIFY _IO(IOC_1588_BASE, 33)
+
+#define IOCTL_1588_AUX_TIME_CLR_NOTIFY _IO(IOC_1588_BASE, 34)
+
+#define IOCTL_1588_AUX_TARG_TIME_CLR_NOTIFY _IO(IOC_1588_BASE,

35)

Review

Quoted 2 lines
(quoted diff) +2 -0
+
+#define IOCTL_1588_PULSE_PER_SEC_CLR_NOTIFY _IO(IOC_1588_BASE,

36)

Review

Quoted 2 lines
(quoted diff) +2 -0
+
+#define IOCTL_1588_PULSE_PER_SEC_TIME_GET _IOR(IOC_1588_BASE, 37,

__u32)

Review

Quoted 2 lines
(quoted diff) +2 -0
+
+#define IOCTL_1588_PULSE_PER_SEC_TIME_SET _IOW(IOC_1588_BASE, 38,

__u32)

drivers/char/pch_ieee1588/pch_ieee1588.h line 139

Quoted 12 lines
drivers/char/pch_ieee1588/pch_ieee1588.h new file +12 -0
+
+#define IOCTL_1588_PORT_VERSION_SET \
+ _IOW(IOC_1588_BASE, 39, struct pch_versionioctl)
+
+#define IOCTL_1588_PORT_VERSION_GET \
+ _IOWR(IOC_1588_BASE, 40, struct pch_versionioctl)
+
+#define IOCTL_1588_PORT_OPERATION_MODE_SET \
+ _IOW(IOC_1588_BASE, 41, struct pch_opemode_ioctl)
+
+#define IOCTL_1588_PORT_OPERATION_MODE_GET \
+ _IOWR(IOC_1588_BASE, 42, struct pch_opemode_ioctl)

Do they all have to be ioctls? What exactly are they doing?

Quoted 5 lines

And are they 32/64bit safe?

thanks,

greg k-h

Best Regards,
Qi

#5 lore raw
Quoted 13 lines

-----Original Message-----
From: Masayuki Ohtake [mailto:[email protected]]
Sent: Wednesday, August 11, 2010 10:15 AM
To: Greg KH; Wang, Qi
Cc: [email protected]; Khor, Andrew Chih Howe; Wang, Yong Y; LKML; [email protected]
Subject: Re: [MeeGo-Dev][PATCH] Topcliff: Update PCH_IEEE1588 driver to 2.6.35

Quoted 4 lines
Quoted 4 lines

drivers/char/pch_ieee1588/pch_ieee1588.h line 31

Quoted 2 lines
drivers/char/pch_ieee1588/pch_ieee1588.h new file +2 -0
+#define TRUE 1
+#define FALSE 0

These lines should not be needed at all.

Quoted 6 lines
Quoted 1 line

OKI-san, please remove them, Those definition is useless.

Is the above definition is necessary?
I can't find appropriate header file defines 'TRUE' and 'FALSE'. In addition, some drivers have these definitions.
For example, drivers/char/mwave/smapi.h has the above definitions.

if (a == TRUE).... isn't necessary. Just if (a)... so you needn't 'TRUE'. If (a == FALSE)... isn't necessary also. Just if (!a)..

Best Regards,
Qi.

Quoted 171 lines

Thanks, Ohtake
----- Original Message -----
From: "Wang, Qi" <[email protected]>
To: "Greg KH" <[email protected]>; "Masayuki Ohtak" <[email protected]>
Cc: <[email protected]>; "LKML" <[email protected]>; "Wang, Yong Y" <[email protected]>; "Khor, Andrew
Chih Howe" <[email protected]>; <[email protected]>
Sent: Wednesday, August 11, 2010 10:20 AM
Subject: RE: [MeeGo-Dev][PATCH] Topcliff: Update PCH_IEEE1588 driver to 2.6.35

Quoted 9 lines
Quoted 9 lines

-----Original Message-----
From: Greg KH [mailto:[email protected]]
Sent: Wednesday, August 11, 2010 1:14 AM
To: Masayuki Ohtak
Cc: [email protected]; LKML; Wang, Qi; Wang, Yong Y; Khor, Andrew Chih Howe; [email protected]
Subject: Re: [MeeGo-Dev][PATCH] Topcliff: Update PCH_IEEE1588 driver to 2.6.35

Quoted 7 lines
Quoted 7 lines · Masayuki Ohtak
Quoted 6 lines

IEEE1588 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 IEEE1588 I/F. This driver enables IEEE1588 function on

CAN

Quoted 7 lines
Quoted 5 lines

or

Quoted 1 line

Ethernet communications.

So it's a CAN or Ethernet driver? If so, why isn't this a network driver like the CAN and Ethernet drivers?

This socket CAN drivers which can use Berkeley socket API, and the Linux

network stack and implements the CAN device
drivers as network interfaces.

Quoted 41 lines
Quoted 14 lines

drivers/char/pch_ieee1588/pch_ieee1588.h line 31

Quoted 12 lines
drivers/char/pch_ieee1588/pch_ieee1588.h new file +11 -0
+#ifdef __GNUC__
+#define UNUSED __attribute__ ((unused))
+#define UNUSED_ARG(x)
+#else
+#define UNUSED
+#define UNUSED_ARG(x) (void) x
+#endif
+
+
+#define TRUE 1
+#define FALSE 0

These lines should not be needed at all.

OKI-san, please remove them, Those definition is useless.

Quoted 26 lines
Quoted 2 lines

+#define IOC_1588_BASE 0xf8

Do you have documentation for these new ioctls you are creating? It's a lot of them:

drivers/char/pch_ieee1588/pch_ieee1588.h

Quoted 7 lines
drivers/char/pch_ieee1588/pch_ieee1588.h new file +7 -0
+#define IOCTL_1588_PORT_CONFIG_SET \
+ _IOW(IOC_1588_BASE, 0, struct pch_portcfg_ioctl)
+
+#define IOCTL_1588_PORT_CONFIG_GET \
+ _IOWR(IOC_1588_BASE, 1, struct pch_portcfg_ioctl)
+
+#define IOCTL_1588_RX_POLL _IOWR(IOC_1588_BASE, 2, struct

pch_rxtxpoll_ioctl)

Review

Quoted 2 lines
(quoted diff) +2 -0
+
+#define IOCTL_1588_TX_POLL _IOWR(IOC_1588_BASE, 3, struct

pch_rxtxpoll_ioctl)

Review

Quoted 2 lines
(quoted diff) +2 -0
+
+#define IOCTL_1588_CAN_POLL _IOWR(IOC_1588_BASE, 4, struct

pch_canpoll_ioctl)

Review

Quoted 2 lines
(quoted diff) +2 -0
+
+#define IOCTL_1588_SYS_TIME_GET _IOR(IOC_1588_BASE, 5, struct

pch_tim_val)

Review

Quoted 2 lines
(quoted diff) +2 -0
+
+#define IOCTL_1588_SYS_TIME_SET _IOW(IOC_1588_BASE, 6, struct

pch_tim_val)

Quoted 6 lines
Quoted 6 lines

drivers/char/pch_ieee1588/pch_ieee1588.h

Quoted 6 lines
drivers/char/pch_ieee1588/pch_ieee1588.h new file +6 -0
+
+#define IOCTL_1588_TICK_RATE_SET _IOW(IOC_1588_BASE, 7, __u32)
+
+#define IOCTL_1588_TICK_RATE_GET _IOR(IOC_1588_BASE, 8, __u32)
+
+#define IOCTL_1588_TARG_TIME_INTRPT_ENABLE _IO(IOC_1588_BASE,

9)

Quoted 2 lines
Quoted 2 lines

Review

Quoted 2 lines
(quoted diff) +2 -0
+
+#define IOCTL_1588_TARG_TIME_INTRPT_DISABLE

_IO(IOC_1588_BASE,

Quoted 12 lines
Quoted 12 lines

10)

drivers/char/pch_ieee1588/pch_ieee1588.h

Quoted 11 lines
drivers/char/pch_ieee1588/pch_ieee1588.h new file +11 -0
+
+#define IOCTL_1588_TARG_TIME_POLL \
+ _IOR(IOC_1588_BASE, 11, struct pch_timepoll_ioctl)
+
+#define IOCTL_1588_TARG_TIME_SET \
+ _IOW(IOC_1588_BASE, 12, struct pch_tim_val)
+
+#define IOCTL_1588_TARG_TIME_GET \
+ _IOR(IOC_1588_BASE, 13, struct pch_tim_val)
+
+#define IOCTL_1588_AUX_TIME_INTRPT_ENABLE

_IOW(IOC_1588_BASE,

Quoted 4 lines
Quoted 4 lines

14,\

Review

Quoted 3 lines

+ enum pch_auxmode)

(quoted diff) +2 -0
+
+#define IOCTL_1588_AUX_TIME_INTRPT_DISABLE

_IOW(IOC_1588_BASE,

Quoted 38 lines
Quoted 38 lines

15,\

drivers/char/pch_ieee1588/pch_ieee1588.h

Quoted 8 lines

+ enum pch_auxmode)

drivers/char/pch_ieee1588/pch_ieee1588.h new file +7 -0
+
+#define IOCTL_1588_AUX_TIME_POLL \
+ _IOWR(IOC_1588_BASE, 16, struct pch_timepoll_ioctl)
+
+#define IOCTL_1588_RESET _IO(IOC_1588_BASE, 17)
+
+#define IOCTL_1588_CHNL_RESET _IOW(IOC_1588_BASE, 18, enum

pch_ptpport)

Review

Quoted 2 lines
(quoted diff) +2 -0
+
+#define IOCTL_1588_STATS_GET _IOR(IOC_1588_BASE, 19, struct

pch_stats)

drivers/char/pch_ieee1588/pch_ieee1588.h

Quoted 6 lines
drivers/char/pch_ieee1588/pch_ieee1588.h new file +6 -0
+
+#define IOCTL_1588_STATS_RESET _IO(IOC_1588_BASE, 20)
+
+#define IOCTL_1588_SHOW_ALL _IO(IOC_1588_BASE, 21)
+
+#define IOCTL_1588_AUX_TARG_TIME_INTRPT_ENABLE

_IO(IOC_1588_BASE, 22)

Review

Quoted 2 lines
(quoted diff) +2 -0
+
+#define IOCTL_1588_AUX_TARG_TIME_INTRPT_DISABLE

_IO(IOC_1588_BASE, 23)

drivers/char/pch_ieee1588/pch_ieee1588.h

Quoted 11 lines
drivers/char/pch_ieee1588/pch_ieee1588.h new file +11 -0
+
+#define IOCTL_1588_AUX_TARG_TIME_POLL \
+ _IOR(IOC_1588_BASE, 24, struct pch_timepoll_ioctl)
+
+#define IOCTL_1588_AUX_TARG_TIME_SET \
+ _IOW(IOC_1588_BASE, 25, struct pch_tim_val)
+
+#define IOCTL_1588_AUX_TARG_TIME_GET \
+ _IOR(IOC_1588_BASE, 26, struct pch_tim_val)
+
+#define IOCTL_1588_PULSE_PER_SEC_INTRPT_ENABLE

_IO(IOC_1588_BASE, 27)

Review

Quoted 2 lines
(quoted diff) +2 -0
+
+#define IOCTL_1588_PULSE_PER_SEC_INTRPT_DISABLE

_IO(IOC_1588_BASE, 28)

Quoted 11 lines
Quoted 11 lines

drivers/char/pch_ieee1588/pch_ieee1588.h

Quoted 11 lines
drivers/char/pch_ieee1588/pch_ieee1588.h new file +11 -0
+
+#define IOCTL_1588_TARG_TIME_NOTIFY \
+ _IOR(IOC_1588_BASE, 29, struct pch_tim_val)
+
+#define IOCTL_1588_AUX_TIME_NOTIFY \
+ _IOR(IOC_1588_BASE, 30, struct pch_auxtimeioctl)
+
+#define IOCTL_1588_AUX_TARG_TIME_NOTIFY \
+ _IOR(IOC_1588_BASE, 31, struct pch_tim_val)
+
+#define IOCTL_1588_PULSE_PER_SEC_NOTIFY _IOR(IOC_1588_BASE,

32,

Quoted 7 lines
Quoted 7 lines

__u32)

drivers/char/pch_ieee1588/pch_ieee1588.h

Quoted 6 lines
drivers/char/pch_ieee1588/pch_ieee1588.h new file +6 -0
+
+#define IOCTL_1588_TARG_TIME_CLR_NOTIFY _IO(IOC_1588_BASE, 33)
+
+#define IOCTL_1588_AUX_TIME_CLR_NOTIFY _IO(IOC_1588_BASE, 34)
+
+#define IOCTL_1588_AUX_TARG_TIME_CLR_NOTIFY

_IO(IOC_1588_BASE,

Quoted 3 lines
Quoted 3 lines

35)

Review

Quoted 2 lines
(quoted diff) +2 -0
+
+#define IOCTL_1588_PULSE_PER_SEC_CLR_NOTIFY

_IO(IOC_1588_BASE,

Quoted 34 lines
Quoted 3 lines
Quoted 3 lines

36)

Review

Quoted 2 lines
(quoted diff) +2 -0
+
+#define IOCTL_1588_PULSE_PER_SEC_TIME_GET _IOR(IOC_1588_BASE,

37,

Quoted 3 lines
Quoted 3 lines

__u32)

Review

Quoted 2 lines
(quoted diff) +2 -0
+
+#define IOCTL_1588_PULSE_PER_SEC_TIME_SET _IOW(IOC_1588_BASE,

38,

Quoted 25 lines
Quoted 16 lines

__u32)

drivers/char/pch_ieee1588/pch_ieee1588.h line 139

Quoted 12 lines
drivers/char/pch_ieee1588/pch_ieee1588.h new file +12 -0
+
+#define IOCTL_1588_PORT_VERSION_SET \
+ _IOW(IOC_1588_BASE, 39, struct pch_versionioctl)
+
+#define IOCTL_1588_PORT_VERSION_GET \
+ _IOWR(IOC_1588_BASE, 40, struct pch_versionioctl)
+
+#define IOCTL_1588_PORT_OPERATION_MODE_SET \
+ _IOW(IOC_1588_BASE, 41, struct pch_opemode_ioctl)
+
+#define IOCTL_1588_PORT_OPERATION_MODE_GET \
+ _IOWR(IOC_1588_BASE, 42, struct pch_opemode_ioctl)

Do they all have to be ioctls? What exactly are they doing?

Quoted 5 lines

And are they 32/64bit safe?

thanks,

greg k-h

Best Regards,
Qi

���{.n�+�������+%�����ݶ��w��{.n�+����{��G�����{ay�ʇڙ�,j��f���h��������z_�(�階�ݢj"���m������G������?����&���~��iO��z��v�^�m���� ������?�I�

author
#6 lore raw

Hi Qi,

Quoted 2 lines

if (a == TRUE).... isn't necessary. Just if (a)... so you needn't 'TRUE'. If (a == FALSE)... isn't necessary also. Just if (!a)..

I understand.
I will delete TRUE/FALSE definition.

Thanks, Ohtake.

----- Original Message -----
From: "Wang, Qi" <[email protected]>
To: "Masayuki Ohtake" <[email protected]>; "Greg KH" <[email protected]>
Cc: <[email protected]>; "Khor, Andrew Chih Howe" <[email protected]>; "Wang, Yong Y" <[email protected]>; "LKML" <[email protected]>; <[email protected]>
Sent: Wednesday, August 11, 2010 11:18 AM
Subject: RE: [MeeGo-Dev][PATCH] Topcliff: Update PCH_IEEE1588 driver to 2.6.35

Quoted 232 lines
Quoted 13 lines

-----Original Message-----
From: Masayuki Ohtake [mailto:[email protected]]
Sent: Wednesday, August 11, 2010 10:15 AM
To: Greg KH; Wang, Qi
Cc: [email protected]; Khor, Andrew Chih Howe; Wang, Yong Y; LKML; [email protected]
Subject: Re: [MeeGo-Dev][PATCH] Topcliff: Update PCH_IEEE1588 driver to 2.6.35

Quoted 4 lines
Quoted 4 lines

drivers/char/pch_ieee1588/pch_ieee1588.h line 31

Quoted 2 lines
drivers/char/pch_ieee1588/pch_ieee1588.h new file +2 -0
+#define TRUE 1
+#define FALSE 0

These lines should not be needed at all.

Quoted 6 lines
Quoted 1 line

OKI-san, please remove them, Those definition is useless.

Is the above definition is necessary?
I can't find appropriate header file defines 'TRUE' and 'FALSE'. In addition, some drivers have these definitions.
For example, drivers/char/mwave/smapi.h has the above definitions.

if (a == TRUE).... isn't necessary. Just if (a)... so you needn't 'TRUE'. If (a == FALSE)... isn't necessary also. Just if (!a)..

Best Regards,
Qi.

Quoted 205 lines

Thanks, Ohtake
----- Original Message -----
From: "Wang, Qi" <[email protected]>
To: "Greg KH" <[email protected]>; "Masayuki Ohtak" <[email protected]>
Cc: <[email protected]>; "LKML" <[email protected]>; "Wang, Yong Y" <[email protected]>; "Khor, Andrew
Chih Howe" <[email protected]>; <[email protected]>
Sent: Wednesday, August 11, 2010 10:20 AM
Subject: RE: [MeeGo-Dev][PATCH] Topcliff: Update PCH_IEEE1588 driver to 2.6.35

Quoted 9 lines
Quoted 9 lines

-----Original Message-----
From: Greg KH [mailto:[email protected]]
Sent: Wednesday, August 11, 2010 1:14 AM
To: Masayuki Ohtak
Cc: [email protected]; LKML; Wang, Qi; Wang, Yong Y; Khor, Andrew Chih Howe; [email protected]
Subject: Re: [MeeGo-Dev][PATCH] Topcliff: Update PCH_IEEE1588 driver to 2.6.35

Quoted 7 lines
Quoted 7 lines · Masayuki Ohtak
Quoted 6 lines

IEEE1588 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 IEEE1588 I/F. This driver enables IEEE1588 function on

CAN

Quoted 7 lines
Quoted 5 lines

or

Quoted 1 line

Ethernet communications.

So it's a CAN or Ethernet driver? If so, why isn't this a network driver like the CAN and Ethernet drivers?

This socket CAN drivers which can use Berkeley socket API, and the Linux

network stack and implements the CAN device
drivers as network interfaces.

Quoted 41 lines
Quoted 14 lines

drivers/char/pch_ieee1588/pch_ieee1588.h line 31

Quoted 12 lines
drivers/char/pch_ieee1588/pch_ieee1588.h new file +11 -0
+#ifdef __GNUC__
+#define UNUSED __attribute__ ((unused))
+#define UNUSED_ARG(x)
+#else
+#define UNUSED
+#define UNUSED_ARG(x) (void) x
+#endif
+
+
+#define TRUE 1
+#define FALSE 0

These lines should not be needed at all.

OKI-san, please remove them, Those definition is useless.

Quoted 26 lines
Quoted 2 lines

+#define IOC_1588_BASE 0xf8

Do you have documentation for these new ioctls you are creating? It's a lot of them:

drivers/char/pch_ieee1588/pch_ieee1588.h

Quoted 7 lines
drivers/char/pch_ieee1588/pch_ieee1588.h new file +7 -0
+#define IOCTL_1588_PORT_CONFIG_SET \
+ _IOW(IOC_1588_BASE, 0, struct pch_portcfg_ioctl)
+
+#define IOCTL_1588_PORT_CONFIG_GET \
+ _IOWR(IOC_1588_BASE, 1, struct pch_portcfg_ioctl)
+
+#define IOCTL_1588_RX_POLL _IOWR(IOC_1588_BASE, 2, struct

pch_rxtxpoll_ioctl)

Review

Quoted 2 lines
(quoted diff) +2 -0
+
+#define IOCTL_1588_TX_POLL _IOWR(IOC_1588_BASE, 3, struct

pch_rxtxpoll_ioctl)

Review

Quoted 2 lines
(quoted diff) +2 -0
+
+#define IOCTL_1588_CAN_POLL _IOWR(IOC_1588_BASE, 4, struct

pch_canpoll_ioctl)

Review

Quoted 2 lines
(quoted diff) +2 -0
+
+#define IOCTL_1588_SYS_TIME_GET _IOR(IOC_1588_BASE, 5, struct

pch_tim_val)

Review

Quoted 2 lines
(quoted diff) +2 -0
+
+#define IOCTL_1588_SYS_TIME_SET _IOW(IOC_1588_BASE, 6, struct

pch_tim_val)

Quoted 6 lines
Quoted 6 lines

drivers/char/pch_ieee1588/pch_ieee1588.h

Quoted 6 lines
drivers/char/pch_ieee1588/pch_ieee1588.h new file +6 -0
+
+#define IOCTL_1588_TICK_RATE_SET _IOW(IOC_1588_BASE, 7, __u32)
+
+#define IOCTL_1588_TICK_RATE_GET _IOR(IOC_1588_BASE, 8, __u32)
+
+#define IOCTL_1588_TARG_TIME_INTRPT_ENABLE _IO(IOC_1588_BASE,

9)

Quoted 2 lines
Quoted 2 lines

Review

Quoted 2 lines
(quoted diff) +2 -0
+
+#define IOCTL_1588_TARG_TIME_INTRPT_DISABLE

_IO(IOC_1588_BASE,

Quoted 12 lines
Quoted 12 lines

10)

drivers/char/pch_ieee1588/pch_ieee1588.h

Quoted 11 lines
drivers/char/pch_ieee1588/pch_ieee1588.h new file +11 -0
+
+#define IOCTL_1588_TARG_TIME_POLL \
+ _IOR(IOC_1588_BASE, 11, struct pch_timepoll_ioctl)
+
+#define IOCTL_1588_TARG_TIME_SET \
+ _IOW(IOC_1588_BASE, 12, struct pch_tim_val)
+
+#define IOCTL_1588_TARG_TIME_GET \
+ _IOR(IOC_1588_BASE, 13, struct pch_tim_val)
+
+#define IOCTL_1588_AUX_TIME_INTRPT_ENABLE

_IOW(IOC_1588_BASE,

Quoted 4 lines
Quoted 4 lines

14,\

Review

Quoted 3 lines

+ enum pch_auxmode)

(quoted diff) +2 -0
+
+#define IOCTL_1588_AUX_TIME_INTRPT_DISABLE

_IOW(IOC_1588_BASE,

Quoted 38 lines
Quoted 38 lines

15,\

drivers/char/pch_ieee1588/pch_ieee1588.h

Quoted 8 lines

+ enum pch_auxmode)

drivers/char/pch_ieee1588/pch_ieee1588.h new file +7 -0
+
+#define IOCTL_1588_AUX_TIME_POLL \
+ _IOWR(IOC_1588_BASE, 16, struct pch_timepoll_ioctl)
+
+#define IOCTL_1588_RESET _IO(IOC_1588_BASE, 17)
+
+#define IOCTL_1588_CHNL_RESET _IOW(IOC_1588_BASE, 18, enum

pch_ptpport)

Review

Quoted 2 lines
(quoted diff) +2 -0
+
+#define IOCTL_1588_STATS_GET _IOR(IOC_1588_BASE, 19, struct

pch_stats)

drivers/char/pch_ieee1588/pch_ieee1588.h

Quoted 6 lines
drivers/char/pch_ieee1588/pch_ieee1588.h new file +6 -0
+
+#define IOCTL_1588_STATS_RESET _IO(IOC_1588_BASE, 20)
+
+#define IOCTL_1588_SHOW_ALL _IO(IOC_1588_BASE, 21)
+
+#define IOCTL_1588_AUX_TARG_TIME_INTRPT_ENABLE

_IO(IOC_1588_BASE, 22)

Review

Quoted 2 lines
(quoted diff) +2 -0
+
+#define IOCTL_1588_AUX_TARG_TIME_INTRPT_DISABLE

_IO(IOC_1588_BASE, 23)

drivers/char/pch_ieee1588/pch_ieee1588.h

Quoted 11 lines
drivers/char/pch_ieee1588/pch_ieee1588.h new file +11 -0
+
+#define IOCTL_1588_AUX_TARG_TIME_POLL \
+ _IOR(IOC_1588_BASE, 24, struct pch_timepoll_ioctl)
+
+#define IOCTL_1588_AUX_TARG_TIME_SET \
+ _IOW(IOC_1588_BASE, 25, struct pch_tim_val)
+
+#define IOCTL_1588_AUX_TARG_TIME_GET \
+ _IOR(IOC_1588_BASE, 26, struct pch_tim_val)
+
+#define IOCTL_1588_PULSE_PER_SEC_INTRPT_ENABLE

_IO(IOC_1588_BASE, 27)

Review

Quoted 2 lines
(quoted diff) +2 -0
+
+#define IOCTL_1588_PULSE_PER_SEC_INTRPT_DISABLE

_IO(IOC_1588_BASE, 28)

Quoted 11 lines
Quoted 11 lines

drivers/char/pch_ieee1588/pch_ieee1588.h

Quoted 11 lines
drivers/char/pch_ieee1588/pch_ieee1588.h new file +11 -0
+
+#define IOCTL_1588_TARG_TIME_NOTIFY \
+ _IOR(IOC_1588_BASE, 29, struct pch_tim_val)
+
+#define IOCTL_1588_AUX_TIME_NOTIFY \
+ _IOR(IOC_1588_BASE, 30, struct pch_auxtimeioctl)
+
+#define IOCTL_1588_AUX_TARG_TIME_NOTIFY \
+ _IOR(IOC_1588_BASE, 31, struct pch_tim_val)
+
+#define IOCTL_1588_PULSE_PER_SEC_NOTIFY _IOR(IOC_1588_BASE,

32,

Quoted 7 lines
Quoted 7 lines

__u32)

drivers/char/pch_ieee1588/pch_ieee1588.h

Quoted 6 lines
drivers/char/pch_ieee1588/pch_ieee1588.h new file +6 -0
+
+#define IOCTL_1588_TARG_TIME_CLR_NOTIFY _IO(IOC_1588_BASE, 33)
+
+#define IOCTL_1588_AUX_TIME_CLR_NOTIFY _IO(IOC_1588_BASE, 34)
+
+#define IOCTL_1588_AUX_TARG_TIME_CLR_NOTIFY

_IO(IOC_1588_BASE,

Quoted 3 lines
Quoted 3 lines

35)

Review

Quoted 2 lines
(quoted diff) +2 -0
+
+#define IOCTL_1588_PULSE_PER_SEC_CLR_NOTIFY

_IO(IOC_1588_BASE,

Quoted 3 lines
Quoted 3 lines

36)

Review

Quoted 2 lines
(quoted diff) +2 -0
+
+#define IOCTL_1588_PULSE_PER_SEC_TIME_GET _IOR(IOC_1588_BASE,

37,

Quoted 3 lines
Quoted 3 lines

__u32)

Review

Quoted 2 lines
(quoted diff) +2 -0
+
+#define IOCTL_1588_PULSE_PER_SEC_TIME_SET _IOW(IOC_1588_BASE,

38,

Quoted 25 lines
Quoted 16 lines

__u32)

drivers/char/pch_ieee1588/pch_ieee1588.h line 139

Quoted 12 lines
drivers/char/pch_ieee1588/pch_ieee1588.h new file +12 -0
+
+#define IOCTL_1588_PORT_VERSION_SET \
+ _IOW(IOC_1588_BASE, 39, struct pch_versionioctl)
+
+#define IOCTL_1588_PORT_VERSION_GET \
+ _IOWR(IOC_1588_BASE, 40, struct pch_versionioctl)
+
+#define IOCTL_1588_PORT_OPERATION_MODE_SET \
+ _IOW(IOC_1588_BASE, 41, struct pch_opemode_ioctl)
+
+#define IOCTL_1588_PORT_OPERATION_MODE_GET \
+ _IOWR(IOC_1588_BASE, 42, struct pch_opemode_ioctl)

Do they all have to be ioctls? What exactly are they doing?

Quoted 5 lines

And are they 32/64bit safe?

thanks,

greg k-h

Best Regards,
Qi

#7 lore raw
In reply to Masayuki Ohtake
Quoted 8 lines · ext Masayuki Ohtake
Quoted 5 lines
Quoted 4 lines

drivers/char/pch_ieee1588/pch_ieee1588.h line 31

Quoted 2 lines
drivers/char/pch_ieee1588/pch_ieee1588.h new file +2 -0
+#define TRUE 1
+#define FALSE 0

These lines should not be needed at all.

OKI-san, please remove them, Those definition is useless.

Is the above definition is necessary?
I can't find appropriate header file defines 'TRUE' and 'FALSE'.

it's true and false. Lowercase. UNUSED is __unused.

Signature · 3 lines
balbi

DefectiveByDesign.org
author
#8 lore raw

Hi Balbi

I can do well.

Thanks! Ohtake.

----- Original Message -----
From: "Felipe Balbi" <[email protected]>
To: "ext Masayuki Ohtake" <[email protected]>
Cc: "Greg KH" <[email protected]>; "Wang, Qi" <[email protected]>; <[email protected]>; "Khor, Andrew Chih Howe" <[email protected]>; "Wang, Yong Y" <[email protected]>; "LKML" <[email protected]>; <[email protected]>
Sent: Wednesday, August 11, 2010 2:49 PM
Subject: Re: [MeeGo-Dev][PATCH] Topcliff: Update PCH_IEEE1588 driver to 2.6.35

Quoted 17 lines · ext Masayuki Ohtake
Quoted 8 lines
Quoted 5 lines
Quoted 4 lines

drivers/char/pch_ieee1588/pch_ieee1588.h line 31

Quoted 2 lines
drivers/char/pch_ieee1588/pch_ieee1588.h new file +2 -0
+#define TRUE 1
+#define FALSE 0

These lines should not be needed at all.

OKI-san, please remove them, Those definition is useless.

Is the above definition is necessary?
I can't find appropriate header file defines 'TRUE' and 'FALSE'.

it's true and false. Lowercase. UNUSED is __unused.

Signature · 3 lines
balbi

DefectiveByDesign.org
author
#9 lore raw
In reply to Greg KH

Hi Greg,

Quoted 1 line

Do they all have to be ioctls? What exactly are they doing?

I think using ioctl is common for this patch.
Do you think that using ioctl is NOT appropriate for this patch? Let me know your intension in more detail.

Quoted 2 lines

And are they 32/64bit safe?

Only 32bit support.

Thanks, Ohtake
----- Original Message -----
From: "Greg KH" <[email protected]>
To: "Masayuki Ohtak" <[email protected]>
Cc: <[email protected]>; "LKML" <[email protected]>; <[email protected]>; <[email protected]>; <[email protected]>; <[email protected]>
Sent: Wednesday, August 11, 2010 2:13 AM
Subject: Re: [MeeGo-Dev][PATCH] Topcliff: Update PCH_IEEE1588 driver to 2.6.35

Quoted 143 lines · Masayuki Ohtak
Quoted 7 lines

IEEE1588 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 IEEE1588 I/F. This driver enables IEEE1588 function on CAN or Ethernet communications.

So it's a CAN or Ethernet driver? If so, why isn't this a network driver like the CAN and Ethernet drivers?

drivers/char/pch_ieee1588/pch_ieee1588.h line 31

Quoted 11 lines
drivers/char/pch_ieee1588/pch_ieee1588.h new file +11 -0
+#ifdef __GNUC__
+#define UNUSED __attribute__ ((unused))
+#define UNUSED_ARG(x)
+#else
+#define UNUSED
+#define UNUSED_ARG(x) (void) x
+#endif
+
+
+#define TRUE 1
+#define FALSE 0

These lines should not be needed at all.

Quoted 1 line

+#define IOC_1588_BASE 0xf8

Do you have documentation for these new ioctls you are creating? It's a lot of them:

Quoted 93 lines
drivers/char/pch_ieee1588/pch_ieee1588.h new file +93 -0
+#define IOCTL_1588_PORT_CONFIG_SET \
+ _IOW(IOC_1588_BASE, 0, struct pch_portcfg_ioctl)
+
+#define IOCTL_1588_PORT_CONFIG_GET \
+ _IOWR(IOC_1588_BASE, 1, struct pch_portcfg_ioctl)
+
+#define IOCTL_1588_RX_POLL _IOWR(IOC_1588_BASE, 2, struct pch_rxtxpoll_ioctl)
+
+#define IOCTL_1588_TX_POLL _IOWR(IOC_1588_BASE, 3, struct pch_rxtxpoll_ioctl)
+
+#define IOCTL_1588_CAN_POLL _IOWR(IOC_1588_BASE, 4, struct pch_canpoll_ioctl)
+
+#define IOCTL_1588_SYS_TIME_GET _IOR(IOC_1588_BASE, 5, struct pch_tim_val)
+
+#define IOCTL_1588_SYS_TIME_SET _IOW(IOC_1588_BASE, 6, struct pch_tim_val)
+
+#define IOCTL_1588_TICK_RATE_SET _IOW(IOC_1588_BASE, 7, __u32)
+
+#define IOCTL_1588_TICK_RATE_GET _IOR(IOC_1588_BASE, 8, __u32)
+
+#define IOCTL_1588_TARG_TIME_INTRPT_ENABLE _IO(IOC_1588_BASE, 9)
+
+#define IOCTL_1588_TARG_TIME_INTRPT_DISABLE _IO(IOC_1588_BASE, 10)
+
+#define IOCTL_1588_TARG_TIME_POLL \
+ _IOR(IOC_1588_BASE, 11, struct pch_timepoll_ioctl)
+
+#define IOCTL_1588_TARG_TIME_SET \
+ _IOW(IOC_1588_BASE, 12, struct pch_tim_val)
+
+#define IOCTL_1588_TARG_TIME_GET \
+ _IOR(IOC_1588_BASE, 13, struct pch_tim_val)
+
+#define IOCTL_1588_AUX_TIME_INTRPT_ENABLE _IOW(IOC_1588_BASE, 14,\
+ enum pch_auxmode)
+
+#define IOCTL_1588_AUX_TIME_INTRPT_DISABLE _IOW(IOC_1588_BASE, 15,\
+ enum pch_auxmode)
+
+#define IOCTL_1588_AUX_TIME_POLL \
+ _IOWR(IOC_1588_BASE, 16, struct pch_timepoll_ioctl)
+
+#define IOCTL_1588_RESET _IO(IOC_1588_BASE, 17)
+
+#define IOCTL_1588_CHNL_RESET _IOW(IOC_1588_BASE, 18, enum pch_ptpport)
+
+#define IOCTL_1588_STATS_GET _IOR(IOC_1588_BASE, 19, struct pch_stats)
+
+#define IOCTL_1588_STATS_RESET _IO(IOC_1588_BASE, 20)
+
+#define IOCTL_1588_SHOW_ALL _IO(IOC_1588_BASE, 21)
+
+#define IOCTL_1588_AUX_TARG_TIME_INTRPT_ENABLE _IO(IOC_1588_BASE, 22)
+
+#define IOCTL_1588_AUX_TARG_TIME_INTRPT_DISABLE _IO(IOC_1588_BASE, 23)
+
+#define IOCTL_1588_AUX_TARG_TIME_POLL \
+ _IOR(IOC_1588_BASE, 24, struct pch_timepoll_ioctl)
+
+#define IOCTL_1588_AUX_TARG_TIME_SET \
+ _IOW(IOC_1588_BASE, 25, struct pch_tim_val)
+
+#define IOCTL_1588_AUX_TARG_TIME_GET \
+ _IOR(IOC_1588_BASE, 26, struct pch_tim_val)
+
+#define IOCTL_1588_PULSE_PER_SEC_INTRPT_ENABLE _IO(IOC_1588_BASE, 27)
+
+#define IOCTL_1588_PULSE_PER_SEC_INTRPT_DISABLE _IO(IOC_1588_BASE, 28)
+
+#define IOCTL_1588_TARG_TIME_NOTIFY \
+ _IOR(IOC_1588_BASE, 29, struct pch_tim_val)
+
+#define IOCTL_1588_AUX_TIME_NOTIFY \
+ _IOR(IOC_1588_BASE, 30, struct pch_auxtimeioctl)
+
+#define IOCTL_1588_AUX_TARG_TIME_NOTIFY \
+ _IOR(IOC_1588_BASE, 31, struct pch_tim_val)
+
+#define IOCTL_1588_PULSE_PER_SEC_NOTIFY _IOR(IOC_1588_BASE, 32, __u32)
+
+#define IOCTL_1588_TARG_TIME_CLR_NOTIFY _IO(IOC_1588_BASE, 33)
+
+#define IOCTL_1588_AUX_TIME_CLR_NOTIFY _IO(IOC_1588_BASE, 34)
+
+#define IOCTL_1588_AUX_TARG_TIME_CLR_NOTIFY _IO(IOC_1588_BASE, 35)
+
+#define IOCTL_1588_PULSE_PER_SEC_CLR_NOTIFY _IO(IOC_1588_BASE, 36)
+
+#define IOCTL_1588_PULSE_PER_SEC_TIME_GET _IOR(IOC_1588_BASE, 37, __u32)
+
+#define IOCTL_1588_PULSE_PER_SEC_TIME_SET _IOW(IOC_1588_BASE, 38, __u32)
+
+#define IOCTL_1588_PORT_VERSION_SET \

drivers/char/pch_ieee1588/pch_ieee1588.h line 139

Quoted 10 lines
drivers/char/pch_ieee1588/pch_ieee1588.h new file +10 -0
+ _IOW(IOC_1588_BASE, 39, struct pch_versionioctl)
+
+#define IOCTL_1588_PORT_VERSION_GET \
+ _IOWR(IOC_1588_BASE, 40, struct pch_versionioctl)
+
+#define IOCTL_1588_PORT_OPERATION_MODE_SET \
+ _IOW(IOC_1588_BASE, 41, struct pch_opemode_ioctl)
+
+#define IOCTL_1588_PORT_OPERATION_MODE_GET \
+ _IOWR(IOC_1588_BASE, 42, struct pch_opemode_ioctl)

Do they all have to be ioctls? What exactly are they doing?

And are they 32/64bit safe?

thanks,

greg k-h

#10 lore raw
Quoted 4 lines · Masayuki Ohtake

Hi Greg,

Quoted 1 line

Do they all have to be ioctls? What exactly are they doing?

I think using ioctl is common for this patch.

"Common" is not ok, right?

Quoted 1 line

Do you think that using ioctl is NOT appropriate for this patch?

Yes.

Quoted 1 line

Let me know your intension in more detail.

Please express yours. Why do you feel you need these to be ioctls? What exactly are they doing? Where is the documentation for them all?

Quoted 2 lines
Quoted 1 line

And are they 32/64bit safe?

Only 32bit support.

That's an obvious problem that needs to be resolved, right?

thanks,

greg k-h

#11 lore raw
Quoted 23 lines

-----Original Message-----
From: Greg KH [mailto:[email protected]]
Sent: Wednesday, August 11, 2010 11:15 PM
To: Masayuki Ohtake
Cc: [email protected]; LKML; Wang, Qi; Wang, Yong Y; Khor, Andrew Chih Howe; [email protected]
Subject: Re: [MeeGo-Dev][PATCH] Topcliff: Update PCH_IEEE1588 driver to 2.6.35

Quoted 4 lines · Masayuki Ohtake

Hi Greg,

Quoted 1 line

Do they all have to be ioctls? What exactly are they doing?

I think using ioctl is common for this patch.

"Common" is not ok, right?

Quoted 1 line

Do you think that using ioctl is NOT appropriate for this patch?

Yes.

Quoted 1 line

Let me know your intension in more detail.

Quoted 7 lines

Please express yours. Why do you feel you need these to be ioctls? What exactly are they doing? Where is the documentation for them all?

Quoted 2 lines
Quoted 1 line

And are they 32/64bit safe?

Only 32bit support.

That's an obvious problem that needs to be resolved, right?

The Tunnelcreak processor (A kind of Atom-based processor) only supports 32b, so PCH drivers only support 32b. They can make it 32/64 compatible, but it's difficult to test them, because there're no 64b Atom-based platform have those PCH devices.

Best Regards,
QI.

Quoted 4 lines

thanks,

greg k-h

���{.n�+�������+%�����ݶ��w��{.n�+����{��G�����{ay�ʇڙ�,j��f���h��������z_�(�階�ݢj"���m������G������?����&���~��iO��z��v�^�m���� ������?�I�

#12 lore raw
Quoted 17 lines · Qi
Quoted 17 lines

-----Original Message-----
From: Greg KH [mailto:[email protected]]
Sent: Wednesday, August 11, 2010 11:15 PM
To: Masayuki Ohtake
Cc: [email protected]; LKML; Wang, Qi; Wang, Yong Y; Khor, Andrew Chih Howe; [email protected]
Subject: Re: [MeeGo-Dev][PATCH] Topcliff: Update PCH_IEEE1588 driver to 2.6.35

Quoted 4 lines · Masayuki Ohtake

Hi Greg,

Quoted 1 line

Do they all have to be ioctls? What exactly are they doing?

I think using ioctl is common for this patch.

"Common" is not ok, right?

Quoted 8 lines
Quoted 8 lines
Quoted 1 line

Do you think that using ioctl is NOT appropriate for this patch?

Yes.

Quoted 1 line

Let me know your intension in more detail.

Please express yours. Why do you feel you need these to be ioctls? What exactly are they doing? Where is the documentation for them all?

You forgot to answer this.

Quoted 8 lines
Quoted 4 lines
Quoted 2 lines
Quoted 1 line

And are they 32/64bit safe?

Only 32bit support.

That's an obvious problem that needs to be resolved, right?

The Tunnelcreak processor (A kind of Atom-based processor) only supports 32b, so PCH drivers only support 32b.
They can make it 32/64 compatible, but it's difficult to test them, because there're no 64b Atom-based platform have those PCH devices.

But that does not mean you should not be using the proper types and other things necessary for proper ioctl usage.

As you failed to describe what any of these ioctls do, I can't recommend that you have any of them in the driver.

sorry,

greg k-h

#13 lore raw
Quoted 29 lines

-----Original Message-----
From: Greg KH [mailto:[email protected]]
Sent: Thursday, August 12, 2010 10:00 AM
To: Wang, Qi
Cc: Masayuki Ohtake; [email protected]; LKML; Wang, Yong Y; Khor, Andrew Chih Howe; [email protected]
Subject: Re: [MeeGo-Dev][PATCH] Topcliff: Update PCH_IEEE1588 driver to 2.6.35

Quoted 19 lines · Qi
Quoted 19 lines

-----Original Message-----
From: Greg KH [mailto:[email protected]]
Sent: Wednesday, August 11, 2010 11:15 PM
To: Masayuki Ohtake
Cc: [email protected]; LKML; Wang, Qi; Wang, Yong Y; Khor, Andrew Chih Howe; [email protected]
Subject: Re: [MeeGo-Dev][PATCH] Topcliff: Update PCH_IEEE1588 driver to 2.6.35

Quoted 4 lines · Masayuki Ohtake

Hi Greg,

Quoted 1 line

Do they all have to be ioctls? What exactly are they doing?

I think using ioctl is common for this patch.

"Common" is not ok, right?

Quoted 1 line

Do you think that using ioctl is NOT appropriate for this patch?

Quoted 8 lines
Quoted 6 lines
Quoted 6 lines

Yes.

Quoted 1 line

Let me know your intension in more detail.

Please express yours. Why do you feel you need these to be ioctls? What exactly are they doing? Where is the documentation for them all?

You forgot to answer this.

Ohtake-san,

Please answer this question asap.

Quoted 10 lines
Quoted 9 lines
Quoted 5 lines
Quoted 3 lines
Quoted 2 lines

And are they 32/64bit safe?

Only 32bit support.

That's an obvious problem that needs to be resolved, right?

The Tunnelcreak processor (A kind of Atom-based processor) only supports 32b, so PCH drivers only support 32b.
They can make it 32/64 compatible, but it's difficult to test them, because there're no 64b Atom-based platform have those PCH devices.

Quoted 5 lines

But that does not mean you should not be using the proper types and other things necessary for proper ioctl usage.

As you failed to describe what any of these ioctls do, I can't recommend that you have any of them in the driver.

Ohtake-san, Please solve 32b/64b safe issue asap.

Quoted 4 lines

sorry,

greg k-h

���{.n�+�������+%�����ݶ��w��{.n�+����{��G�����{ay�ʇڙ�,j��f���h��������z_�(�階�ݢj"���m������G������?����&���~��iO��z��v�^�m���� ������?�I�

author
#14 lore raw

Sorry, for late response.
Please see out inline comments.

Thanks, Ohtake

----- Original Message -----
From: "Wang, Qi" <[email protected]>
To: "Masayuki Ohtake" <[email protected]>
Cc: <[email protected]>; "LKML" <[email protected]>; "Wang, Yong Y" <[email protected]>; "Khor, Andrew Chih Howe" <[email protected]>; <[email protected]>; "Greg KH" <[email protected]>
Sent: Thursday, August 12, 2010 11:15 AM
Subject: RE: [MeeGo-Dev][PATCH] Topcliff: Update PCH_IEEE1588 driver to 2.6.35

Quoted 33 lines
Quoted 33 lines

-----Original Message-----
From: Greg KH [mailto:[email protected]]
Sent: Thursday, August 12, 2010 10:00 AM
To: Wang, Qi
Cc: Masayuki Ohtake; [email protected]; LKML; Wang, Yong Y; Khor, Andrew Chih Howe; [email protected]
Subject: Re: [MeeGo-Dev][PATCH] Topcliff: Update PCH_IEEE1588 driver to 2.6.35

Quoted 23 lines · Qi
Quoted 23 lines

-----Original Message-----
From: Greg KH [mailto:[email protected]]
Sent: Wednesday, August 11, 2010 11:15 PM
To: Masayuki Ohtake
Cc: [email protected]; LKML; Wang, Qi; Wang, Yong Y; Khor, Andrew Chih Howe; [email protected]
Subject: Re: [MeeGo-Dev][PATCH] Topcliff: Update PCH_IEEE1588 driver to 2.6.35

Quoted 4 lines · Masayuki Ohtake

Hi Greg,

Quoted 1 line

Do they all have to be ioctls? What exactly are they doing?

I think using ioctl is common for this patch.

"Common" is not ok, right?

Quoted 1 line

Do you think that using ioctl is NOT appropriate for this patch?

Yes.

Quoted 1 line

Let me know your intension in more detail.

Quoted 7 lines
Quoted 4 lines
Quoted 2 lines
Quoted 2 lines

Please express yours. Why do you feel you need these to be ioctls? What exactly are they doing? Where is the documentation for them all?

You forgot to answer this.

Ohtake-san,

Please answer this question asap.

Intel Topcliff IEEE1588 device driver API is developed according to Intel Tolapai 1588 device driver.
Both APIs are the same.

Quoted 21 lines
Quoted 10 lines
Quoted 9 lines
Quoted 5 lines
Quoted 3 lines
Quoted 2 lines

And are they 32/64bit safe?

Only 32bit support.

That's an obvious problem that needs to be resolved, right?

The Tunnelcreak processor (A kind of Atom-based processor) only supports 32b, so PCH drivers only support 32b.
They can make it 32/64 compatible, but it's difficult to test them, because there're no 64b Atom-based platform have those PCH devices.

Quoted 5 lines

But that does not mean you should not be using the proper types and other things necessary for proper ioctl usage.

As you failed to describe what any of these ioctls do, I can't recommend that you have any of them in the driver.

Ohtake-san, Please solve 32b/64b safe issue asap.

Quoted 4 lines

sorry,

greg k-h

#15 lore raw
Quoted 3 lines · Masayuki Ohtake

Intel Topcliff IEEE1588 device driver API is developed according to Intel Tolapai 1588 device driver.
Both APIs are the same.

But what is the "Intel Tolapai 1588 device driver?"

Is that a Linux driver?

If so, where can I find it?

Richard

maintainer
#16 lore raw
Quoted 10 lines · Richard Cochran
Quoted 3 lines · Masayuki Ohtake

Intel Topcliff IEEE1588 device driver API is developed according to Intel Tolapai 1588 device driver.
Both APIs are the same.

But what is the "Intel Tolapai 1588 device driver?"

Is that a Linux driver?

If so, where can I find it?

I found it under
http://downloadcenter.intel.com/Detail_Desc.aspx?agr=Y&DwnldID=17182

That driver was never submitted for inclusion anywhere, so it is not really relevant here. The APIs are similar, but not identical, most importantly they use a different ioctl command base number, but also some of the structures are laid out slightly differently.

Ohtake-san, Richard's code already contains an abstraction for multiple ieee1588 sources, which looks like it should perfectly fit your purposes. I don't see room for another user interface, so please try to integrate into version 5 of his ptp code.

Arnd

#17 lore raw
Quoted 4 lines · Arnd Bergmann

Ohtake-san, Richard's code already contains an abstraction for multiple ieee1588 sources, which looks like it should perfectly fit your purposes. I don't see room for another user interface, so please try to integrate into version 5 of his ptp code.

We want to implement IEEE1588 driver for Intel EG20T PCH (Topcliff) using PTP core according to you saying.
However, it seems PTP core module is not accepted by upstream.
How about your PTP status ?

Thanks,
---
Tomoya MORINAGA
OKI SEMICONDUCTOR CO., LTD.

----- Original Message ----- From: "Arnd Bergmann" <[email protected]>
To: "Richard Cochran" <[email protected]>
Cc: "Masayuki Ohtake" <[email protected]>; <[email protected]>; "LKML" <[email protected]>
Sent: Tuesday, August 17, 2010 8:25 PM
Subject: Re: [MeeGo-dev] [MeeGo-Dev][PATCH] Topcliff: Update PCH_IEEE1588 driver to 2.6.35

Quoted 27 lines · Richard Cochran
Quoted 10 lines · Masayuki Ohtake
Quoted 3 lines

Intel Topcliff IEEE1588 device driver API is developed according to Intel Tolapai 1588 device driver.
Both APIs are the same.

But what is the "Intel Tolapai 1588 device driver?"

Is that a Linux driver?

If so, where can I find it?

I found it under
http://downloadcenter.intel.com/Detail_Desc.aspx?agr=Y&DwnldID=17182

That driver was never submitted for inclusion anywhere, so it is not really relevant here. The APIs are similar, but not identical, most importantly they use a different ioctl command base number, but also some of the structures are laid out slightly differently.

Ohtake-san, Richard's code already contains an abstraction for multiple ieee1588 sources, which looks like it should perfectly fit your purposes. I don't see room for another user interface, so please try to integrate into version 5 of his ptp code.

Arnd

#18 lore raw
Quoted 2 lines · Tomoya MORINAGA

We want to implement IEEE1588 driver for Intel EG20T PCH (Topcliff) using PTP core according to you saying.

That is good news.

Quoted 2 lines

However, it seems PTP core module is not accepted by upstream. How about your PTP status ?

I posted v6 back on September 23. Based on the comments, I think the patch set is almost good enough for mainline inclusion (at least, I hope so).

I am just getting ready to post v7, and the kernel interface will change a bit. I will post it before Christmas, so perhaps you can wait a bit. You can take look at the v6 code, since the general ideas will not change very much.

Thanks,
Richard

#19 lore raw

Hi Richard,

Thank you for your PTP core's status information. Accepted by upstream, we will start our development.

Thanks,
---
Tomoya MORINAGA
OKI SEMICONDUCTOR CO., LTD.

----- Original Message ----- From: "Richard Cochran" <[email protected]>
To: "Tomoya MORINAGA" <[email protected]>
Cc: "Arnd Bergmann" <[email protected]>; "LKML" <[email protected]>
Sent: Wednesday, December 15, 2010 1:21 AM
Subject: Re: [MeeGo-dev][PATCH] Topcliff: Update PCH_IEEE1588 driver to 2.6.35

Quoted 26 lines · Tomoya MORINAGA
Quoted 2 lines

We want to implement IEEE1588 driver for Intel EG20T PCH (Topcliff) using PTP core according to you saying.

That is good news.

Quoted 2 lines

However, it seems PTP core module is not accepted by upstream. How about your PTP status ?

I posted v6 back on September 23. Based on the comments, I think the patch set is almost good enough for mainline inclusion (at least, I hope so).

I am just getting ready to post v7, and the kernel interface will change a bit. I will post it before Christmas, so perhaps you can wait a bit. You can take look at the v6 code, since the general ideas will not change very much.

Thanks,
Richard
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

#20 lore raw
In reply to Masayuki Ohtake
Quoted 17 lines · Masayuki Ohtake
Quoted 17 lines
Quoted 17 lines
Quoted 16 lines
Quoted 16 lines · Masayuki Ohtake
Quoted 4 lines

Hi Greg,

Quoted 1 line

Do they all have to be ioctls? What exactly are they doing?

I think using ioctl is common for this patch.

"Common" is not ok, right?

Quoted 1 line

Do you think that using ioctl is NOT appropriate for this patch?

Yes.

Quoted 1 line

Let me know your intension in more detail.

Please express yours. Why do you feel you need these to be ioctls? What exactly are they doing? Where is the documentation for them all?

Quoted 8 lines
Quoted 4 lines
Quoted 1 line

You forgot to answer this.

Ohtake-san,

Please answer this question asap.

Intel Topcliff IEEE1588 device driver API is developed according to Intel Tolapai 1588 device driver.
Both APIs are the same.

That _still_ did not answer the questions. Please answer them.

greg k-h

#21 lore raw
In reply to Greg KH
Quoted 5 lines · Greg KH
Quoted 2 lines · Qi
Quoted 2 lines

Please express yours. Why do you feel you need these to be ioctls? What exactly are they doing? Where is the documentation for them all?

You forgot to answer this.

I recently introduced a PTP clock infrastructure on the netdev list. While the patch set is still under review, some necessary background patches have already been accepted, and the response to the API has been positive.

One major goal of the framework is to reduce the amount of driver code need to support a new PTP clock. Another goal is remove the need for MAC drivers to add a bunch of new, private ioctls just for PTP.

Please take a look at that patch set (latest was v4) and also submit any PTP code on the netdev list for review.

In any case, a MAC driver that offers time stamping on network packets must implement the SO_TIMESTAMPING API, which has been a part of Linux since 2.6.30.

Richard

#22 lore raw
In reply to Masayuki Ohtak
Quoted 1 line · Masayuki Ohtak

IEEE1588 driver of Topcliff PCH

Are you aware of Richard Cochran's 1588 support patches? I'm not sure about the integration status, but Richard (set on Cc:) can surely say more.

rsc

Signature · 4 lines
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
author
#23 lore raw

Re: [MeeGo-Dev][PATCH] Topcliff: Update PCH_IEEE1588 driver to2.6.35

Hi Robert

My specified CC of IEEE1588 patch e-mail is not enough. I will re-submit the patch again.

Thanks, Ohtake.
----- Original Message -----
From: "Robert Schwebel" <[email protected]>
To: "Masayuki Ohtak" <[email protected]>
Cc: <[email protected]>; "LKML" <[email protected]>; <[email protected]>; <[email protected]>; <[email protected]>; <[email protected]>; <[email protected]>; "Richard Cochran" <[email protected]>
Sent: Wednesday, August 11, 2010 6:08 PM
Subject: Re: [MeeGo-Dev][PATCH] Topcliff: Update PCH_IEEE1588 driver to2.6.35

Quoted 14 lines · Masayuki Ohtak
Quoted 1 line

IEEE1588 driver of Topcliff PCH

Are you aware of Richard Cochran's 1588 support patches? I'm not sure about the integration status, but Richard (set on Cc:) can surely say more.

rsc

Signature · 4 lines
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
#24 lore raw

RE: [MeeGo-Dev][PATCH] Topcliff: Update PCH_IEEE1588 driver to2.6.35

Quoted 2 lines

My specified CC of IEEE1588 patch e-mail is not enough. I will re-submit the patch again.

Please post anything with PTP or hardware time stamping on net-next for review.

Quoted 3 lines
Quoted 3 lines

Are you aware of Richard Cochran's 1588 support patches? I'm not sure about the integration status, but Richard (set on Cc:) can surely say more.

The background work need for PHY time stamping has been accepted.

For the rest, I am preparing to post another patch set soon...

Thanks,
Richard