Re: [Intel-wired-lan] [PATCH 03/15] ice: Start hardware initialization

2018-03-15 Thread Venkataramanan, Anirudh 
On Thu, 2018-03-15 at 10:00 -0700, Shannon Nelson wrote:
> On 3/14/2018 3:05 PM, Venkataramanan, Anirudh wrote:
> > On Mon, 2018-03-12 at 19:05 -0700, Shannon Nelson wrote:
> > > On 3/9/2018 9:21 AM, Anirudh Venkataramanan wrote:
> 
> 
> > > > +
> > > > +/**
> > > > + * ice_read_sr_aq - Read Shadow RAM.
> > > > + * @hw: pointer to the HW structure
> > > > + * @offset: offset in words from module start
> > > > + * @words: number of words to read
> > > > + * @data: buffer for words reads from Shadow RAM
> > > > + * @last_command: tells the AdminQ that this is the last
> > > > command
> > > > + *
> > > > + * Reads 16-bit word buffers from the Shadow RAM using the
> > > > admin
> > > > command.
> > > > + */
> > > > +static enum ice_status
> > > > +ice_read_sr_aq(struct ice_hw *hw, u32 offset, u16 words, u16
> > > > *data,
> > > > +  bool last_command)
> > > > +{
> > > > +   enum ice_status status;
> > > > +
> > > > +   status = ice_check_sr_access_params(hw, offset,
> > > > words);
> > > > +   if (!status)
> > > > +   status = ice_aq_read_nvm(hw, 0, 2 * offset, 2
> > > > *
> > > > words, data,
> > > 
> > > Why the doubling of offset and words?  If this is some general
> > > adjustment made for the AQ interface, it should be made in
> > > ice_aq_read_nvm().  If not, then some explanation is needed here.
> > 
> > ice_read_sr_aq expects a word offset and size in words. The
> > ice_aq_read_nvm interface expects offset and size in bytes. The
> > doubling is a conversion from word offset/size to byte offset/size.
> 
> In that case, this might be a good place for a small comment for
> readers 
> like me who don't have the spec available.

Sure thing! :-)

> 
> sln
> 

smime.p7s
Description: S/MIME cryptographic signature

Re: [Intel-wired-lan] [PATCH 03/15] ice: Start hardware initialization

2018-03-15 Thread Shannon Nelson 

On 3/14/2018 3:05 PM, Venkataramanan, Anirudh wrote:

On Mon, 2018-03-12 at 19:05 -0700, Shannon Nelson wrote:

On 3/9/2018 9:21 AM, Anirudh Venkataramanan wrote:




+
+/**
+ * ice_read_sr_aq - Read Shadow RAM.
+ * @hw: pointer to the HW structure
+ * @offset: offset in words from module start
+ * @words: number of words to read
+ * @data: buffer for words reads from Shadow RAM
+ * @last_command: tells the AdminQ that this is the last command
+ *
+ * Reads 16-bit word buffers from the Shadow RAM using the admin
command.
+ */
+static enum ice_status
+ice_read_sr_aq(struct ice_hw *hw, u32 offset, u16 words, u16
*data,
+  bool last_command)
+{
+   enum ice_status status;
+
+   status = ice_check_sr_access_params(hw, offset, words);
+   if (!status)
+   status = ice_aq_read_nvm(hw, 0, 2 * offset, 2 *
words, data,


Why the doubling of offset and words?  If this is some general
adjustment made for the AQ interface, it should be made in
ice_aq_read_nvm().  If not, then some explanation is needed here.


ice_read_sr_aq expects a word offset and size in words. The
ice_aq_read_nvm interface expects offset and size in bytes. The
doubling is a conversion from word offset/size to byte offset/size.


In that case, this might be a good place for a small comment for readers 
like me who don't have the spec available.


sln

Re: [Intel-wired-lan] [PATCH 03/15] ice: Start hardware initialization

2018-03-14 Thread Venkataramanan, Anirudh 
On Mon, 2018-03-12 at 19:05 -0700, Shannon Nelson wrote:
> On 3/9/2018 9:21 AM, Anirudh Venkataramanan wrote:
> > This patch implements multiple pieces of the initialization flow
> > as follows:
> > 
> > 1) A reset is issued to ensure a clean device state, followed
> > by initialization of admin queue interface.
> > 
> > 2) Once the admin queue interface is up, clear the PF config
> > and transition the device to non-PXE mode.
> > 
> > 3) Get the NVM configuration stored in the device's non-volatile
> > memory (NVM) using ice_init_nvm.
> > 
> > Signed-off-by: Anirudh Venkataramanan  > .com>
> > ---
> >   drivers/net/ethernet/intel/ice/Makefile |   3 +-
> >   drivers/net/ethernet/intel/ice/ice.h|   2 +
> >   drivers/net/ethernet/intel/ice/ice_adminq_cmd.h |  79 +
> >   drivers/net/ethernet/intel/ice/ice_common.c | 410
> > 
> >   drivers/net/ethernet/intel/ice/ice_common.h |  11 +
> >   drivers/net/ethernet/intel/ice/ice_controlq.h   |   3 +
> >   drivers/net/ethernet/intel/ice/ice_hw_autogen.h |  30 ++
> >   drivers/net/ethernet/intel/ice/ice_main.c   |  31 ++
> >   drivers/net/ethernet/intel/ice/ice_nvm.c| 245
> > ++
> >   drivers/net/ethernet/intel/ice/ice_osdep.h  |   1 +
> >   drivers/net/ethernet/intel/ice/ice_status.h |   5 +
> >   drivers/net/ethernet/intel/ice/ice_type.h   |  49 +++
> >   12 files changed, 868 insertions(+), 1 deletion(-)
> >   create mode 100644 drivers/net/ethernet/intel/ice/ice_nvm.c
> > 
> > diff --git a/drivers/net/ethernet/intel/ice/Makefile
> > b/drivers/net/ethernet/intel/ice/Makefile
> > index eebf619e84a8..373d481dbb25 100644
> > --- a/drivers/net/ethernet/intel/ice/Makefile
> > +++ b/drivers/net/ethernet/intel/ice/Makefile
> > @@ -26,4 +26,5 @@ obj-$(CONFIG_ICE) += ice.o
> >   
> >   ice-y := ice_main.o   \
> >  ice_controlq.o \
> > -ice_common.o
> > +ice_common.o   \
> > +ice_nvm.o
> > diff --git a/drivers/net/ethernet/intel/ice/ice.h
> > b/drivers/net/ethernet/intel/ice/ice.h
> > index ea2fb63bb095..ab2800c31906 100644
> > --- a/drivers/net/ethernet/intel/ice/ice.h
> > +++ b/drivers/net/ethernet/intel/ice/ice.h
> > @@ -30,8 +30,10 @@
> >   #include 
> >   #include "ice_devids.h"
> >   #include "ice_type.h"
> > +#include "ice_common.h"
> >   
> >   #define ICE_BAR0  0
> > +#define ICE_AQ_LEN 64
> >   
> >   #define ICE_DFLT_NETIF_M (NETIF_MSG_DRV | NETIF_MSG_PROBE |
> > NETIF_MSG_LINK)
> >   
> > diff --git a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
> > b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
> > index 885fa3c6fec4..05b22a1ffd70 100644
> > --- a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
> > +++ b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
> > @@ -50,6 +50,67 @@ struct ice_aqc_q_shutdown {
> > u8 reserved[12];
> >   };
> >   
> > +/* Request resource ownership (direct 0x0008)
> > + * Release resource ownership (direct 0x0009)
> > + */
> > +struct ice_aqc_req_res {
> > +   __le16 res_id;
> > +#define ICE_AQC_RES_ID_NVM 1
> > +#define ICE_AQC_RES_ID_SDP 2
> > +#define ICE_AQC_RES_ID_CHNG_LOCK   3
> > +#define ICE_AQC_RES_ID_GLBL_LOCK   4
> > +   __le16 access_type;
> > +#define ICE_AQC_RES_ACCESS_READ1
> > +#define ICE_AQC_RES_ACCESS_WRITE   2
> > +
> > +   /* Upon successful completion, FW writes this value and
> > driver is
> > +* expected to release resource before timeout. This value
> > is provided
> > +* in milliseconds.
> > +*/
> > +   __le32 timeout;
> > +#define ICE_AQ_RES_NVM_READ_DFLT_TIMEOUT_MS3000
> > +#define ICE_AQ_RES_NVM_WRITE_DFLT_TIMEOUT_MS   18
> > +#define ICE_AQ_RES_CHNG_LOCK_DFLT_TIMEOUT_MS   1000
> > +#define ICE_AQ_RES_GLBL_LOCK_DFLT_TIMEOUT_MS   3000
> > +   /* For SDP: pin id of the SDP */
> > +   __le32 res_number;
> > +   /* Status is only used for ICE_AQC_RES_ID_GLBL_LOCK */
> > +   __le16 status;
> > +#define ICE_AQ_RES_GLBL_SUCCESS0
> > +#define ICE_AQ_RES_GLBL_IN_PROG1
> > +#define ICE_AQ_RES_GLBL_DONE   2
> > +   u8 reserved[2];
> 
> Since these structs all become part of the descriptor's param union, 
> perhaps adding reserved space to the end is not necessary.
> 
> > +};
> > +
> > +/* Clear PXE Command and response (direct 0x0110) */
> > +struct ice_aqc_clear_pxe {
> > +   u8 rx_cnt;
> > +#define ICE_AQC_CLEAR_PXE_RX_CNT   0x2
> > +   u8 reserved[15];
> > +};
> > +
> > +/* NVM Read command (indirect 0x0701)
> > + * NVM Erase commands (direct 0x0702)
> > + * NVM Update commands (indirect 0x0703)
> > + */
> > +struct ice_aqc_nvm {
> > +   u8  cmd_flags;
> > +#define ICE_AQC_NVM_LAST_CMD   BIT(0)
> > +#define ICE_AQC_NVM_PCIR_REQ   BIT(0)  /* Used
> > by NVM Update reply */
> > +#define ICE_AQC_NVM_PRESERVATION_S 1
> > +#define ICE_AQC_NVM_PRESERVATION_M (3 <<
> > CSR_AQ_NVM_PRESERVATION_S)
> > +#define

Re: [Intel-wired-lan] [PATCH 03/15] ice: Start hardware initialization

2018-03-12 Thread Shannon Nelson 

On 3/9/2018 9:21 AM, Anirudh Venkataramanan wrote:

This patch implements multiple pieces of the initialization flow
as follows:

1) A reset is issued to ensure a clean device state, followed
by initialization of admin queue interface.

2) Once the admin queue interface is up, clear the PF config
and transition the device to non-PXE mode.

3) Get the NVM configuration stored in the device's non-volatile
memory (NVM) using ice_init_nvm.

Signed-off-by: Anirudh Venkataramanan 
---
  drivers/net/ethernet/intel/ice/Makefile |   3 +-
  drivers/net/ethernet/intel/ice/ice.h|   2 +
  drivers/net/ethernet/intel/ice/ice_adminq_cmd.h |  79 +
  drivers/net/ethernet/intel/ice/ice_common.c | 410 
  drivers/net/ethernet/intel/ice/ice_common.h |  11 +
  drivers/net/ethernet/intel/ice/ice_controlq.h   |   3 +
  drivers/net/ethernet/intel/ice/ice_hw_autogen.h |  30 ++
  drivers/net/ethernet/intel/ice/ice_main.c   |  31 ++
  drivers/net/ethernet/intel/ice/ice_nvm.c| 245 ++
  drivers/net/ethernet/intel/ice/ice_osdep.h  |   1 +
  drivers/net/ethernet/intel/ice/ice_status.h |   5 +
  drivers/net/ethernet/intel/ice/ice_type.h   |  49 +++
  12 files changed, 868 insertions(+), 1 deletion(-)
  create mode 100644 drivers/net/ethernet/intel/ice/ice_nvm.c

diff --git a/drivers/net/ethernet/intel/ice/Makefile 
b/drivers/net/ethernet/intel/ice/Makefile
index eebf619e84a8..373d481dbb25 100644
--- a/drivers/net/ethernet/intel/ice/Makefile
+++ b/drivers/net/ethernet/intel/ice/Makefile
@@ -26,4 +26,5 @@ obj-$(CONFIG_ICE) += ice.o
  
  ice-y := ice_main.o	\

 ice_controlq.o \
-ice_common.o
+ice_common.o   \
+ice_nvm.o
diff --git a/drivers/net/ethernet/intel/ice/ice.h 
b/drivers/net/ethernet/intel/ice/ice.h
index ea2fb63bb095..ab2800c31906 100644
--- a/drivers/net/ethernet/intel/ice/ice.h
+++ b/drivers/net/ethernet/intel/ice/ice.h
@@ -30,8 +30,10 @@
  #include 
  #include "ice_devids.h"
  #include "ice_type.h"
+#include "ice_common.h"
  
  #define ICE_BAR0		0

+#define ICE_AQ_LEN 64
  
  #define ICE_DFLT_NETIF_M (NETIF_MSG_DRV | NETIF_MSG_PROBE | NETIF_MSG_LINK)
  
diff --git a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h

index 885fa3c6fec4..05b22a1ffd70 100644
--- a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
+++ b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
@@ -50,6 +50,67 @@ struct ice_aqc_q_shutdown {
u8 reserved[12];
  };
  
+/* Request resource ownership (direct 0x0008)

+ * Release resource ownership (direct 0x0009)
+ */
+struct ice_aqc_req_res {
+   __le16 res_id;
+#define ICE_AQC_RES_ID_NVM 1
+#define ICE_AQC_RES_ID_SDP 2
+#define ICE_AQC_RES_ID_CHNG_LOCK   3
+#define ICE_AQC_RES_ID_GLBL_LOCK   4
+   __le16 access_type;
+#define ICE_AQC_RES_ACCESS_READ1
+#define ICE_AQC_RES_ACCESS_WRITE   2
+
+   /* Upon successful completion, FW writes this value and driver is
+* expected to release resource before timeout. This value is provided
+* in milliseconds.
+*/
+   __le32 timeout;
+#define ICE_AQ_RES_NVM_READ_DFLT_TIMEOUT_MS3000
+#define ICE_AQ_RES_NVM_WRITE_DFLT_TIMEOUT_MS   18
+#define ICE_AQ_RES_CHNG_LOCK_DFLT_TIMEOUT_MS   1000
+#define ICE_AQ_RES_GLBL_LOCK_DFLT_TIMEOUT_MS   3000
+   /* For SDP: pin id of the SDP */
+   __le32 res_number;
+   /* Status is only used for ICE_AQC_RES_ID_GLBL_LOCK */
+   __le16 status;
+#define ICE_AQ_RES_GLBL_SUCCESS0
+#define ICE_AQ_RES_GLBL_IN_PROG1
+#define ICE_AQ_RES_GLBL_DONE   2
+   u8 reserved[2];


Since these structs all become part of the descriptor's param union, 
perhaps adding reserved space to the end is not necessary.



+};
+
+/* Clear PXE Command and response (direct 0x0110) */
+struct ice_aqc_clear_pxe {
+   u8 rx_cnt;
+#define ICE_AQC_CLEAR_PXE_RX_CNT   0x2
+   u8 reserved[15];
+};
+
+/* NVM Read command (indirect 0x0701)
+ * NVM Erase commands (direct 0x0702)
+ * NVM Update commands (indirect 0x0703)
+ */
+struct ice_aqc_nvm {
+   u8  cmd_flags;
+#define ICE_AQC_NVM_LAST_CMD   BIT(0)
+#define ICE_AQC_NVM_PCIR_REQ   BIT(0)  /* Used by NVM Update reply */
+#define ICE_AQC_NVM_PRESERVATION_S 1
+#define ICE_AQC_NVM_PRESERVATION_M (3 << CSR_AQ_NVM_PRESERVATION_S)
+#define ICE_AQC_NVM_NO_PRESERVATION(0 << CSR_AQ_NVM_PRESERVATION_S)
+#define ICE_AQC_NVM_PRESERVE_ALL   BIT(1)
+#define ICE_AQC_NVM_PRESERVE_SELECTED  (3 << CSR_AQ_NVM_PRESERVATION_S)
+#define ICE_AQC_NVM_FLASH_ONLY BIT(7)
+   u8  module_typeid;
+   __le16  length;
+#define ICE_AQC_NVM_ERASE_LEN  0x
+   __le32  offset;
+   __le32  addr_high;
+   __le32  addr_low;
+};
+
  /**
   * struct ice_aq_desc - Admin Queue (AQ) descriptor