RE: [PATCH v5 4/8] thunderbolt: Communication with the ICM (firmware)

[Levy, Amir (Jer)]

On Sat, Jul 30 2016, 12:03 AM, Stephen Hemminger wrote:
> On Thu, 28 Jul 2016 11:15:17 +0300
> Amir Levy  wrote:
> 
> > +int nhi_send_message(struct tbt_nhi_ctxt *nhi_ctxt, enum pdf_value
> pdf,
> > +u32 msg_len, const u8 *msg, bool ignore_icm_resp) {
> 
> Why not make msg a void * and not have to do so many casts?

I couldn't agree more.

RE: [PATCH v5 4/8] thunderbolt: Communication with the ICM (firmware)

[Levy, Amir (Jer)]

On Sat, Jul 30 2016, 12:48 AM, Greg KH wrote:
> On Fri, Jul 29, 2016 at 02:02:24PM -0700, Stephen Hemminger wrote:
> > On Thu, 28 Jul 2016 11:15:17 +0300
> > Amir Levy  wrote:
> >
> > > +static LIST_HEAD(controllers_list); static
> > > +DECLARE_RWSEM(controllers_list_rwsem);
> >
> > Why use a semaphore when simple spinlock or mutex would be better?
> 
> And never use a RW semaphore unless you can benchmark the difference
> from a normal lock.  If you can't benchmark it, then don't use it...

I used RW semaphore since most of the time the list will be accessed for read.
Since it is used in non-time sensitive flows, I'll change it to mutex.

Re: [PATCH v5 4/8] thunderbolt: Communication with the ICM (firmware)

[Greg KH]

On Fri, Jul 29, 2016 at 02:02:24PM -0700, Stephen Hemminger wrote:
> On Thu, 28 Jul 2016 11:15:17 +0300
> Amir Levy  wrote:
> 
> > +static LIST_HEAD(controllers_list);
> > +static DECLARE_RWSEM(controllers_list_rwsem);
> 
> Why use a semaphore when simple spinlock or mutex would be better?

And never use a RW semaphore unless you can benchmark the difference
from a normal lock.  If you can't benchmark it, then don't use it...

Re: [PATCH v5 4/8] thunderbolt: Communication with the ICM (firmware)

[Stephen Hemminger]

On Thu, 28 Jul 2016 11:15:17 +0300
Amir Levy  wrote:

> +int nhi_send_message(struct tbt_nhi_ctxt *nhi_ctxt, enum pdf_value pdf,
> +  u32 msg_len, const u8 *msg, bool ignore_icm_resp)
> +{

Why not make msg a void * and not have to do so many casts?

Re: [PATCH v5 4/8] thunderbolt: Communication with the ICM (firmware)

[Stephen Hemminger]

On Thu, 28 Jul 2016 11:15:17 +0300
Amir Levy  wrote:

> +static LIST_HEAD(controllers_list);
> +static DECLARE_RWSEM(controllers_list_rwsem);

Why use a semaphore when simple spinlock or mutex would be better?

[PATCH v5 4/8] thunderbolt: Communication with the ICM (firmware)

[Amir Levy]

Firmware-based (a.k.a ICM - Intel Connection Manager) controller is
used for establishing and maintaining the Thunderbolt Networking
connection. We need to be able to communicate with it.

Signed-off-by: Amir Levy 
---
 drivers/thunderbolt/Makefile  |1 +
 drivers/thunderbolt/icm/Makefile  |   28 +
 drivers/thunderbolt/icm/icm_nhi.c | 1343 +
 drivers/thunderbolt/icm/icm_nhi.h |   93 +++
 drivers/thunderbolt/icm/net.h |  200 ++
 5 files changed, 1665 insertions(+)
 create mode 100644 drivers/thunderbolt/icm/Makefile
 create mode 100644 drivers/thunderbolt/icm/icm_nhi.c
 create mode 100644 drivers/thunderbolt/icm/icm_nhi.h
 create mode 100644 drivers/thunderbolt/icm/net.h

diff --git a/drivers/thunderbolt/Makefile b/drivers/thunderbolt/Makefile
index 7a85bd1..b6aa6a3 100644
--- a/drivers/thunderbolt/Makefile
+++ b/drivers/thunderbolt/Makefile
@@ -1,3 +1,4 @@
 obj-${CONFIG_THUNDERBOLT_APPLE} := thunderbolt.o
 thunderbolt-objs := nhi.o ctl.o tb.o switch.o cap.o path.o tunnel_pci.o 
eeprom.o
 
+obj-${CONFIG_THUNDERBOLT_ICM} += icm/
diff --git a/drivers/thunderbolt/icm/Makefile b/drivers/thunderbolt/icm/Makefile
new file mode 100644
index 000..3adfc35
--- /dev/null
+++ b/drivers/thunderbolt/icm/Makefile
@@ -0,0 +1,28 @@
+
+#
+# Intel Thunderbolt(TM) driver
+# Copyright(c) 2014 - 2016 Intel Corporation.
+#
+# This program is free software; you can redistribute it and/or modify it
+# under the terms and conditions of the GNU General Public License,
+# version 2, as published by the Free Software Foundation.
+#
+# This program is distributed in the hope it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+# FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+# more details.
+#
+# You should have received a copy of the GNU General Public License along
+# with this program.  If not, see .
+#
+# The full GNU General Public License is included in this distribution in
+# the file called "COPYING".
+#
+# Contact Information:
+# Intel Thunderbolt Mailing List 
+# Intel Corporation, 5200 N.E. Elam Young Parkway, Hillsboro, OR 97124-6497
+#
+
+
+obj-${CONFIG_THUNDERBOLT_ICM} += thunderbolt-icm.o
+thunderbolt-icm-objs := icm_nhi.o
diff --git a/drivers/thunderbolt/icm/icm_nhi.c 
b/drivers/thunderbolt/icm/icm_nhi.c
new file mode 100644
index 000..2106dea
--- /dev/null
+++ b/drivers/thunderbolt/icm/icm_nhi.c
@@ -0,0 +1,1343 @@
+/***
+ *
+ * Intel Thunderbolt(TM) driver
+ * Copyright(c) 2014 - 2016 Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program.  If not, see .
+ *
+ * The full GNU General Public License is included in this distribution in
+ * the file called "COPYING".
+ *
+ * Contact Information:
+ * Intel Thunderbolt Mailing List 
+ * Intel Corporation, 5200 N.E. Elam Young Parkway, Hillsboro, OR 97124-6497
+ *
+ 
**/
+
+#include 
+#include 
+#include 
+#include 
+#include "icm_nhi.h"
+#include "net.h"
+
+#define NHI_GENL_VERSION 1
+#define NHI_GENL_NAME DRV_NAME
+
+#define DEVICE_DATA(num_ports, dma_port, nvm_ver_offset, nvm_auth_on_boot,\
+   support_full_e2e) \
+   ((num_ports) | ((dma_port) << 4) | ((nvm_ver_offset) << 10) | \
+((nvm_auth_on_boot) << 22) | ((support_full_e2e) << 23))
+#define DEVICE_DATA_NUM_PORTS(device_data) ((device_data) & 0xf)
+#define DEVICE_DATA_DMA_PORT(device_data) (((device_data) >> 4) & 0x3f)
+#define DEVICE_DATA_NVM_VER_OFFSET(device_data) (((device_data) >> 10) & 0xfff)
+#define DEVICE_DATA_NVM_AUTH_ON_BOOT(device_data) (((device_data) >> 22) & 0x1)
+#define DEVICE_DATA_SUPPORT_FULL_E2E(device_data) (((device_data) >> 23) & 0x1)
+
+#define USEC_TO_256_NSECS(usec) DIV_ROUND_UP((usec) * NSEC_PER_USEC, 256)
+
+/*
+ * FW->SW responses
+ * RC = response code
+ */
+enum {
+   RC_GET_TBT_TOPOLOGY = 1,
+   RC_GET_VIDEO_RESOURCES_DATA,
+   RC_DRV_READY,
+   RC_APPROVE_PCI_CONNEXION,
+   RC_CHALLENGE_PCI_CONNEXION,
+