[Intel-wired-lan] [PATCH S29 1/6] ice: send driver version to firmware
Jeff Kirsher
jeffrey.t.kirsher at intel.com
Fri Sep 6 20:34:53 UTC 2019
On Fri, 2019-09-06 at 09:09 +0200, Paul Menzel wrote:
> Dear Tony, dear Paul,
>
>
> On 05.09.19 17:39, Tony Nguyen wrote:
> > From: Paul M Stillwell Jr <paul.m.stillwell.jr at intel.com>
> >
> > The driver is required to send a version to the firmware
> > to indicate that the driver is up. If the driver doesn't
> > do this the firmware doesn't behave properly.
>
> In what datasheet is that documented?
It is documented in the datasheet, but the datasheet has not been made
available yet because the device(s) have not been released yet. Once
the device(s) get released, a datasheet should be made available
shortly thereafter.
>
> What does “doesn’t behave properly” mean exactly?
What Paul and Tony were getting at that if the DDP pacakge is not
loaded onto the NIC, not all the features that the NIC is capable of
will be made available. The sending of a version to the firmware is
just one step that needs to occur to enable the loading of a DDP
package.
>
> > Signed-off-by: Paul M Stillwell Jr <paul.m.stillwell.jr at intel.com>
> > Signed-off-by: Tony Nguyen <anthony.l.nguyen at intel.com>
> > ---
> > drivers/net/ethernet/intel/ice/ice.h | 1 +
> > .../net/ethernet/intel/ice/ice_adminq_cmd.h | 13 +++++++
> > drivers/net/ethernet/intel/ice/ice_common.c | 37
> > +++++++++++++++++++
> > drivers/net/ethernet/intel/ice/ice_common.h | 3 ++
> > drivers/net/ethernet/intel/ice/ice_main.c | 36
> > +++++++++++++++++-
> > drivers/net/ethernet/intel/ice/ice_type.h | 8 ++++
> > 6 files changed, 97 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/intel/ice/ice.h
> > b/drivers/net/ethernet/intel/ice/ice.h
> > index b36e1cf0e461..4cdedcebb163 100644
> > --- a/drivers/net/ethernet/intel/ice/ice.h
> > +++ b/drivers/net/ethernet/intel/ice/ice.h
> > @@ -29,6 +29,7 @@
> > #include <linux/sctp.h>
> > #include <linux/ipv6.h>
> > #include <linux/if_bridge.h>
> > +#include <linux/ctype.h>
>
> Is the alignment correct? (Or just my mailer messing up?)
Yep, just your mailer screwed it up. The code is aligned correctly in
my tree.
>
> > #include <linux/avf/virtchnl.h>
> > #include <net/ipv6.h>
> > #include "ice_devids.h"
> > diff --git a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
> > b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
> > index 4da0cde9695b..9c9791788610 100644
> > --- a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
> > +++ b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
> > @@ -33,6 +33,17 @@ struct ice_aqc_get_ver {
> > u8 api_patch;
> > };
> >
> > +/* Send driver version (indirect 0x0002) */
> > +struct ice_aqc_driver_ver {
> > + u8 major_ver;
> > + u8 minor_ver;
> > + u8 build_ver;
> > + u8 subbuild_ver;
> > + u8 reserved[4];
> > + __le32 addr_high;
> > + __le32 addr_low;
> > +};
> > +
> > /* Queue Shutdown (direct 0x0003) */
> > struct ice_aqc_q_shutdown {
> > u8 driver_unloading;
> > @@ -1547,6 +1558,7 @@ struct ice_aq_desc {
> > u8 raw[16];
> > struct ice_aqc_generic generic;
> > struct ice_aqc_get_ver get_ver;
> > + struct ice_aqc_driver_ver driver_ver;
> > struct ice_aqc_q_shutdown q_shutdown;
> > struct ice_aqc_req_res res_owner;
> > struct ice_aqc_manage_mac_read mac_read;
> > @@ -1618,6 +1630,7 @@ enum ice_aq_err {
> > enum ice_adminq_opc {
> > /* AQ commands */
> > ice_aqc_opc_get_ver = 0x0001,
> > + ice_aqc_opc_driver_ver = 0x0002,
> > ice_aqc_opc_q_shutdown = 0x0003,
> >
> > /* resource ownership */
> > diff --git a/drivers/net/ethernet/intel/ice/ice_common.c
> > b/drivers/net/ethernet/intel/ice/ice_common.c
> > index 8b2c46615834..db62cc748544 100644
> > --- a/drivers/net/ethernet/intel/ice/ice_common.c
> > +++ b/drivers/net/ethernet/intel/ice/ice_common.c
> > @@ -1258,6 +1258,43 @@ enum ice_status ice_aq_get_fw_ver(struct
> > ice_hw *hw, struct ice_sq_cd *cd)
> > return status;
> > }
> >
> > +/**
> > + * ice_aq_send_driver_ver
> > + * @hw: pointer to the HW struct
> > + * @dv: driver's major, minor version
> > + * @cd: pointer to command details structure or NULL
> > + *
> > + * Send the driver version (0x0002) to the firmware
> > + */
> > +enum ice_status
> > +ice_aq_send_driver_ver(struct ice_hw *hw, struct ice_driver_ver
> > *dv,
> > + struct ice_sq_cd *cd)
> > +{
> > + struct ice_aqc_driver_ver *cmd;
> > + struct ice_aq_desc desc;
> > + u16 len;
>
> Just `unsigned int` or `size_t`?
This change will be based on the current kernel interface that can get
the length of ASCII chars, so yes, Tony will fix this up as well.
>
> > +
> > + cmd = &desc.params.driver_ver;
> > +
> > + if (!dv)
> > + return ICE_ERR_PARAM;
> > +
> > + ice_fill_dflt_direct_cmd_desc(&desc, ice_aqc_opc_driver_ver);
> > +
> > + desc.flags |= cpu_to_le16(ICE_AQ_FLAG_RD);
> > + cmd->major_ver = dv->major_ver;
> > + cmd->minor_ver = dv->minor_ver;
> > + cmd->build_ver = dv->build_ver;
> > + cmd->subbuild_ver = dv->subbuild_ver;
> > +
> > + len = 0;
> > + while (len < sizeof(dv->driver_string) &&
> > + isascii(dv->driver_string[len]) && dv-
> > >driver_string[len])
> > + len++;
>
> Is there such a function for finding the length of ASCII characters
> already somewhere in Linux?
Tony will look into this and use the kernel interface if there is.
>
> > +
> > + return ice_aq_send_cmd(hw, &desc, dv->driver_string, len, cd);
> > +}
> > +
> > /**
> > * ice_aq_q_shutdown
> > * @hw: pointer to the HW struct
> > diff --git a/drivers/net/ethernet/intel/ice/ice_common.h
> > b/drivers/net/ethernet/intel/ice/ice_common.h
> > index e376d1eadba4..e9d77370a17c 100644
> > --- a/drivers/net/ethernet/intel/ice/ice_common.h
> > +++ b/drivers/net/ethernet/intel/ice/ice_common.h
> > @@ -71,6 +71,9 @@ ice_aq_send_cmd(struct ice_hw *hw, struct
> > ice_aq_desc *desc,
> > void *buf, u16 buf_size, struct ice_sq_cd *cd);
> > enum ice_status ice_aq_get_fw_ver(struct ice_hw *hw, struct
> > ice_sq_cd *cd);
> >
> > +enum ice_status
> > +ice_aq_send_driver_ver(struct ice_hw *hw, struct ice_driver_ver
> > *dv,
> > + struct ice_sq_cd *cd);
> > enum ice_status
> > ice_aq_get_phy_caps(struct ice_port_info *pi, bool qual_mods, u8
> > report_mode,
> > struct ice_aqc_get_phy_caps_data *caps,
> > diff --git a/drivers/net/ethernet/intel/ice/ice_main.c
> > b/drivers/net/ethernet/intel/ice/ice_main.c
> > index f8be9ada2447..ea55ec598dac 100644
> > --- a/drivers/net/ethernet/intel/ice/ice_main.c
> > +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> > @@ -9,7 +9,13 @@
> > #include "ice_lib.h"
> > #include "ice_dcb_lib.h"
> >
> > -#define DRV_VERSION "0.7.5-k"
> > +#define DRV_VERSION_MAJOR 0
> > +#define DRV_VERSION_MINOR 7
> > +#define DRV_VERSION_BUILD 5
> > +
> > +#define DRV_VERSION __stringify(DRV_VERSION_MAJOR) "." \
> > + __stringify(DRV_VERSION_MINOR) "." \
> > + __stringify(DRV_VERSION_BUILD) "-k"
> > #define DRV_SUMMARY "Intel(R) Ethernet Connection E800
> > Series Linux Driver"
> > const char ice_drv_ver[] = DRV_VERSION;
> > static const char ice_driver_string[] = DRV_SUMMARY;
> > @@ -2459,6 +2465,25 @@ static void ice_verify_cacheline_size(struct
> > ice_pf *pf)
> > ICE_CACHE_LINE_BYTES);
> > }
> >
> > +/**
> > + * ice_send_version - update firmware with driver version
> > + * @pf: PF struct
> > + *
> > + * Returns ICE_SUCCESS on success, else error code
> > + */
> > +static enum ice_status ice_send_version(struct ice_pf *pf)
> > +{
> > + struct ice_driver_ver dv;
> > +
> > + dv.major_ver = DRV_VERSION_MAJOR;
> > + dv.minor_ver = DRV_VERSION_MINOR;
> > + dv.build_ver = DRV_VERSION_BUILD;
> > + dv.subbuild_ver = 0;
> > + strscpy((char *)dv.driver_string, DRV_VERSION,
> > + sizeof(dv.driver_string));
> > + return ice_aq_send_driver_ver(&pf->hw, &dv, NULL);
> > +}
> > +
> > /**
> > * ice_probe - Device initialization routine
> > * @pdev: PCI device information struct
> > @@ -2612,6 +2637,15 @@ ice_probe(struct pci_dev *pdev, const struct
> > pci_device_id __always_unused *ent)
> >
> > clear_bit(__ICE_SERVICE_DIS, pf->state);
> >
> > + /* tell the firmware we are up */
> > + err = ice_send_version(pf);
> > + if (err) {
> > + dev_err(dev,
> > + "probe failed due to error sending driver
> > version:%d\n",
>
> Space after colon? Maybe also add the driver version in the string?
Tony will fix this up.
>
> > + err);
> > + goto err_alloc_sw_unroll;
> > + }
> > +
> > /* since everything is good, start the service timer */
> > mod_timer(&pf->serv_tmr, round_jiffies(jiffies + pf-
> > >serv_tmr_period));
> >
> > diff --git a/drivers/net/ethernet/intel/ice/ice_type.h
> > b/drivers/net/ethernet/intel/ice/ice_type.h
> > index 4501d50a7dcc..a2676003275a 100644
> > --- a/drivers/net/ethernet/intel/ice/ice_type.h
> > +++ b/drivers/net/ethernet/intel/ice/ice_type.h
> > @@ -53,6 +53,14 @@ enum ice_aq_res_access_type {
> > ICE_RES_WRITE
> > };
> >
> > +struct ice_driver_ver {
> > + u8 major_ver;
> > + u8 minor_ver;
> > + u8 build_ver;
> > + u8 subbuild_ver;
> > + u8 driver_string[32];
> > +};
> > +
> > enum ice_fc_mode {
> > ICE_FC_NONE = 0,
> > ICE_FC_RX_PAUSE,
> >
>
> Kind regards,
>
> Paul
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan at osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part
URL: <http://lists.osuosl.org/pipermail/intel-wired-lan/attachments/20190906/6b062726/attachment.asc>
More information about the Intel-wired-lan
mailing list