Re: [dpdk-dev] Q on Support for I217 and I218 Intel chipsets.

[Ravi Kerur <rkerur@gmail.com>]

Thanks Tim, Konstantin and Helin. I will send out revised patch.

Thanks,
Ravi

On Thu, Jan 15, 2015 at 5:14 PM, Zhang, Helin <helin.zhang@intel.com> wrote:

>
>
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ravi Kerur
> > Sent: Monday, January 5, 2015 7:28 AM
> > To: dev@dpdk.org; Neil Horman; Thomas Monjalon
> > Subject: [dpdk-dev] Q on Support for I217 and I218 Intel chipsets.
> >
> > Hi,
> >
> > We have a Gigabyte H97N motherboard which has I217 Intel chipset which
> > uses e100e drivers. I looked into lib/librte_pmd_e1000 directory and I
> do see
> > that e1000e code is integrated but missing some support for read/write
> from
> > flash_address and other minor things. I have made changes shown below and
> > have done some testing with testpmd utility and now have following
> questions
> >
> > 1. What amount of testing is required to qualify patch as successfully
> tested on
> > new chipsets
> >
> > 2. FreeBSD testing, currently we have Ubuntu 14.04 installed on existing
> H97N
> > motherboard and testing is done solely on Linux. We plan to get another
> > motherboard which will have I218 chipset and still deciding whether to
> go with
> > FreeBSD or Ubuntu. So the question I have is what amount of testing
> should be
> > done on FreeBSD? I don't think setup.sh/dpdk_nic_bind.py works on
> FreeBSD
> > yet hence the question on testing.
> >
> > Thanks,
> > Ravi
> >
> >
> >
> > On Sun, Jan 4, 2015 at 3:15 PM, Ravi Kerur <rkerur@gmail.com> wrote:
> >
> > > This patch adds support for I217 and I218 Intel chipsets.
> > > Gigabyte H97N motherboard has I217 Intel chipsets and changes include
> > >
> > > 1. Add relevant device-ids via RTE_PCI_DEV_ID_DECL_EM 2. Add support
> > > for memory mapped flash address read/write
> > >
> > > Basic testing on Ubuntu with testpmd utility.
> > >
> > > Signed-off-by: Ravi Kerur <rkerur@gmail.com>
> > > ---
> > >  lib/librte_eal/common/include/rte_pci_dev_ids.h |  4 ++++
> > >  lib/librte_pmd_e1000/e1000/e1000_api.c          | 21
> > +++++++++++++++++++++
> > >  lib/librte_pmd_e1000/e1000/e1000_api.h          |  1 +
> > >  lib/librte_pmd_e1000/e1000/e1000_osdep.h        | 24
> > > +++++++++++++++++++-----
> > >  lib/librte_pmd_e1000/em_ethdev.c                |  7 +++++++
> > >  5 files changed, 52 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/lib/librte_eal/common/include/rte_pci_dev_ids.h
> > > b/lib/librte_eal/common/include/rte_pci_dev_ids.h
> > > index c922de9..712793a 100644
> > > --- a/lib/librte_eal/common/include/rte_pci_dev_ids.h
> > > +++ b/lib/librte_eal/common/include/rte_pci_dev_ids.h
> > > @@ -274,6 +274,10 @@
> > RTE_PCI_DEV_ID_DECL_EM(PCI_VENDOR_ID_INTEL,
> > > E1000_DEV_ID_82572EI)
> > >  RTE_PCI_DEV_ID_DECL_EM(PCI_VENDOR_ID_INTEL,
> > E1000_DEV_ID_82573L)
> > > RTE_PCI_DEV_ID_DECL_EM(PCI_VENDOR_ID_INTEL,
> > E1000_DEV_ID_82574L)
> > > RTE_PCI_DEV_ID_DECL_EM(PCI_VENDOR_ID_INTEL,
> > E1000_DEV_ID_82574LA)
> > > +RTE_PCI_DEV_ID_DECL_EM(PCI_VENDOR_ID_INTEL,
> > > +E1000_DEV_ID_PCH_LPT_I217_LM)
> > > +RTE_PCI_DEV_ID_DECL_EM(PCI_VENDOR_ID_INTEL,
> > > +E1000_DEV_ID_PCH_LPT_I217_V)
> > > +RTE_PCI_DEV_ID_DECL_EM(PCI_VENDOR_ID_INTEL,
> > > E1000_DEV_ID_PCH_LPTLP_I218_LM)
> > > +RTE_PCI_DEV_ID_DECL_EM(PCI_VENDOR_ID_INTEL,
> > > +E1000_DEV_ID_PCH_LPTLP_I218_V)
> So, you are enabling new Intel(r) NICs. That's great, and thanks!
>
> > >
> > >  /******************** Physical IGB devices from e1000_hw.h
> > > ********************/
> > >
> > > diff --git a/lib/librte_pmd_e1000/e1000/e1000_api.c
> > > b/lib/librte_pmd_e1000/e1000/e1000_api.c
> > > index a064565..30ed1f3 100644
> > > --- a/lib/librte_pmd_e1000/e1000/e1000_api.c
> > > +++ b/lib/librte_pmd_e1000/e1000/e1000_api.c
> > > @@ -1355,3 +1355,24 @@ void e1000_shutdown_fiber_serdes_link(struct
> > > e1000_hw *hw)
> > >                 hw->mac.ops.shutdown_serdes(hw);  }
> > >
> > > +/**
> > > + *  e1000_device_is_ich8 - Check for ICH8 device
> > > + *  @hw: pointer to the HW structure
> > > + *
> > > + *  return TRUE for ICH8, otherwise FALSE  **/ bool
> > > +e1000_device_is_ich8(struct e1000_hw *hw) {
> > > +       DEBUGFUNC("e1000_device_is_ich8");
> > > +
> > > +       switch (hw->device_id) {
> > > +       case E1000_DEV_ID_PCH_LPT_I217_LM:
> > > +       case E1000_DEV_ID_PCH_LPT_I217_V:
> > > +       case E1000_DEV_ID_PCH_LPTLP_I218_LM:
> > > +       case E1000_DEV_ID_PCH_LPTLP_I218_V:
> > > +               return 1;
> > > +
> > > +       default:
> > > +               return 0;
> > > +       }
> > > +}
> As Konstantin indicated, please do not try to modify any code in any
> source files in e1000
> sub-folder, except e1000_osdep.c and e1000_osdep.h. If this piece of code
> is needed, please
> try to move it to em_ethdev.c or e1000_osdep.c files!
>
> > > diff --git a/lib/librte_pmd_e1000/e1000/e1000_api.h
> > > b/lib/librte_pmd_e1000/e1000/e1000_api.h
> > > index 02b16da..f96a674 100644
> > > --- a/lib/librte_pmd_e1000/e1000/e1000_api.h
> > > +++ b/lib/librte_pmd_e1000/e1000/e1000_api.h
> > > @@ -49,6 +49,7 @@ extern void e1000_init_function_pointers_vf(struct
> > > e1000_hw *hw);
> > >  extern void e1000_power_up_fiber_serdes_link(struct e1000_hw *hw);
> > > extern void e1000_shutdown_fiber_serdes_link(struct e1000_hw *hw);
> > > extern void e1000_init_function_pointers_i210(struct e1000_hw *hw);
> > > +extern bool e1000_device_is_ich8(struct e1000_hw *hw);
> > >
> > >  s32 e1000_set_obff_timer(struct e1000_hw *hw, u32 itr);
> > >  s32 e1000_set_mac_type(struct e1000_hw *hw); diff --git
> > > a/lib/librte_pmd_e1000/e1000/e1000_osdep.h
> > > b/lib/librte_pmd_e1000/e1000/e1000_osdep.h
> > > index 438641e..19146a3 100644
> > > --- a/lib/librte_pmd_e1000/e1000/e1000_osdep.h
> > > +++ b/lib/librte_pmd_e1000/e1000/e1000_osdep.h
> > > @@ -95,13 +95,22 @@ typedef int         bool;
> > >
> > >  #define E1000_PCI_REG(reg) (*((volatile uint32_t *)(reg)))
> > >
> > > +#define E1000_PCI_REG16(reg) (*((volatile uint16_t *)(reg)))
> > > +
> > >  #define E1000_PCI_REG_WRITE(reg, value) do { \
> > >         E1000_PCI_REG((reg)) = (value); \  } while (0)
> > >
> > > +#define E1000_PCI_REG_WRITE16(reg, value) do { \
> > > +       E1000_PCI_REG16((reg)) = (value); \ } while (0)
> > > +
> > >  #define E1000_PCI_REG_ADDR(hw, reg) \
> > >         ((volatile uint32_t *)((char *)(hw)->hw_addr + (reg)))
> > >
> > > +#define E1000_PCI_REG_FLASH_ADDR(hw, reg) \
> > > +       ((volatile uint32_t *)((char *)(hw)->flash_address + (reg)))
> > > +
>
> > >  #define E1000_PCI_REG_ARRAY_ADDR(hw, reg, index) \
> > >         E1000_PCI_REG_ADDR((hw), (reg) + ((index) << 2))
> > >
> > > @@ -110,6 +119,11 @@ static inline uint32_t e1000_read_addr(volatile
> > > void*
> > > addr)
> > >         return E1000_PCI_REG(addr);
> > >  }
> > >
> > > +static inline uint32_t e1000_read_addr16(volatile void* addr) {
> > > +       return E1000_PCI_REG16(addr);
> > > +}
> > > +
> > >  /* Necessary defines */
> > >  #define E1000_MRQC_ENABLE_MASK                  0x00000007
> > >  #define E1000_MRQC_RSS_FIELD_IPV6_EX           0x00080000
> > > @@ -155,20 +169,20 @@ static inline uint32_t e1000_read_addr(volatile
> > > void* addr)
> > >         E1000_WRITE_REG(hw, reg, value)
> > >
> > >  /*
> > > - * Not implemented.
> > > + * Tested on I217 chipset.
> > >   */
> > >
> > >  #define E1000_READ_FLASH_REG(hw, reg) \
> > > -       (E1000_ACCESS_PANIC(E1000_READ_FLASH_REG, hw, reg, 0), 0)
> > > +       e1000_read_addr(E1000_PCI_REG_FLASH_ADDR((hw), (reg)))
> > >
> > >  #define E1000_READ_FLASH_REG16(hw, reg)  \
> > > -       (E1000_ACCESS_PANIC(E1000_READ_FLASH_REG16, hw, reg, 0), 0)
> > > +       e1000_read_addr16(E1000_PCI_REG_FLASH_ADDR((hw), (reg)))
> > >
> > >  #define E1000_WRITE_FLASH_REG(hw, reg, value)  \
> > > -       E1000_ACCESS_PANIC(E1000_WRITE_FLASH_REG, hw, reg, value)
> > > +       E1000_PCI_REG_WRITE(E1000_PCI_REG_FLASH_ADDR((hw),
> > (reg)),
> > > + (value))
> > >
> > >  #define E1000_WRITE_FLASH_REG16(hw, reg, value) \
> > > -       E1000_ACCESS_PANIC(E1000_WRITE_FLASH_REG16, hw, reg,
> > value)
> > > +       E1000_PCI_REG_WRITE16(E1000_PCI_REG_FLASH_ADDR((hw),
> > (reg)),
> > > (value))
> > >
> > >  #define STATIC static
> > >
> > > diff --git a/lib/librte_pmd_e1000/em_ethdev.c
> > > b/lib/librte_pmd_e1000/em_ethdev.c
> > > index 3f2897e..643f5cd 100644
> > > --- a/lib/librte_pmd_e1000/em_ethdev.c
> > > +++ b/lib/librte_pmd_e1000/em_ethdev.c
> > > @@ -245,6 +245,9 @@ eth_em_dev_init(__attribute__((unused)) struct
> > > eth_driver *eth_drv,
> > >         hw->device_id = pci_dev->id.device_id;
> > >
> > >         /* For ICH8 support we'll need to map the flash memory BAR */
> > > +       if (e1000_device_is_ich8(hw))
> > > +               hw->flash_address = (void
> > > + *)pci_dev->mem_resource[1].addr;
> > > +
> > >
> > >         if (e1000_setup_init_funcs(hw, TRUE) != E1000_SUCCESS ||
> > >                         em_hw_init(hw) != 0) { @@ -436,6 +439,7 @@
> > > em_set_pba(struct e1000_hw *hw)
> > >                         break;
> > >                 case e1000_pchlan:
> > >                 case e1000_pch2lan:
> > > +               case e1000_pch_lpt:
> > >                         pba = E1000_PBA_26K;
> > >                         break;
> > >                 default:
> > > @@ -678,6 +682,8 @@ em_hardware_init(struct e1000_hw *hw)
> > >         /* Workaround: no TX flow ctrl for PCH */
> > >         if (hw->mac.type == e1000_pchlan)
> > >                 hw->fc.requested_mode = e1000_fc_rx_pause;
> > > +       else if (hw->mac.type == e1000_pch_lpt)
> > > +               hw->fc.requested_mode = e1000_fc_full;
> > >
> > >         /* Override - settings for PCH2LAN, ya its magic :) */
> > >         if (hw->mac.type == e1000_pch2lan) { @@ -845,6 +851,7 @@
> > > em_get_max_pktlen(const struct e1000_hw *hw)
> > >         case e1000_pch2lan:
> > >         case e1000_82574:
> > >         case e1000_80003es2lan: /* 9K Jumbo Frame size */
> > > +       case e1000_pch_lpt:
> > >                 return (0x2412);
> > >         case e1000_pchlan:
> > >                 return (0x1000);
> > > --
> > > 1.9.1
> > >
> > >
>