| Message ID | 20231025103315.1149589-2-ivecera@redhat.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Remove VF MAC types and move helpers from i40e_type.h | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Guessing tree name failed - patch did not apply |
On 25.10.2023 12:33, Ivan Vecera wrote: > The i40e_hw.mac.type cannot to be equal to I40E_MAC_VF or > I40E_MAC_X722_VF so remove helper i40e_is_vf(), simplify > i40e_adminq_init_regs() and remove enums for these VF MAC types. > > Signed-off-by: Ivan Vecera <ivecera@redhat.com> > --- > drivers/net/ethernet/intel/i40e/i40e_adminq.c | 33 ++++++------------- > drivers/net/ethernet/intel/i40e/i40e_type.h | 8 ----- > 2 files changed, 10 insertions(+), 31 deletions(-) > > diff --git a/drivers/net/ethernet/intel/i40e/i40e_adminq.c b/drivers/net/ethernet/intel/i40e/i40e_adminq.c > index 29fc46abf690..896c43905309 100644 > --- a/drivers/net/ethernet/intel/i40e/i40e_adminq.c > +++ b/drivers/net/ethernet/intel/i40e/i40e_adminq.c > @@ -17,29 +17,16 @@ static void i40e_resume_aq(struct i40e_hw *hw); > static void i40e_adminq_init_regs(struct i40e_hw *hw) > { > /* set head and tail registers in our local struct */ > - if (i40e_is_vf(hw)) { > - hw->aq.asq.tail = I40E_VF_ATQT1; > - hw->aq.asq.head = I40E_VF_ATQH1; > - hw->aq.asq.len = I40E_VF_ATQLEN1; > - hw->aq.asq.bal = I40E_VF_ATQBAL1; > - hw->aq.asq.bah = I40E_VF_ATQBAH1; > - hw->aq.arq.tail = I40E_VF_ARQT1; > - hw->aq.arq.head = I40E_VF_ARQH1; > - hw->aq.arq.len = I40E_VF_ARQLEN1; > - hw->aq.arq.bal = I40E_VF_ARQBAL1; > - hw->aq.arq.bah = I40E_VF_ARQBAH1; What about removing those I40E_VF_* defines? This is their only usage here, right? > - } else { > - hw->aq.asq.tail = I40E_PF_ATQT; > - hw->aq.asq.head = I40E_PF_ATQH; > - hw->aq.asq.len = I40E_PF_ATQLEN; > - hw->aq.asq.bal = I40E_PF_ATQBAL; > - hw->aq.asq.bah = I40E_PF_ATQBAH; > - hw->aq.arq.tail = I40E_PF_ARQT; > - hw->aq.arq.head = I40E_PF_ARQH; > - hw->aq.arq.len = I40E_PF_ARQLEN; > - hw->aq.arq.bal = I40E_PF_ARQBAL; > - hw->aq.arq.bah = I40E_PF_ARQBAH; > - } > + hw->aq.asq.tail = I40E_PF_ATQT; > + hw->aq.asq.head = I40E_PF_ATQH; > + hw->aq.asq.len = I40E_PF_ATQLEN; > + hw->aq.asq.bal = I40E_PF_ATQBAL; > + hw->aq.asq.bah = I40E_PF_ATQBAH; > + hw->aq.arq.tail = I40E_PF_ARQT; > + hw->aq.arq.head = I40E_PF_ARQH; > + hw->aq.arq.len = I40E_PF_ARQLEN; > + hw->aq.arq.bal = I40E_PF_ARQBAL; > + hw->aq.arq.bah = I40E_PF_ARQBAH; > } > > /** > diff --git a/drivers/net/ethernet/intel/i40e/i40e_type.h b/drivers/net/ethernet/intel/i40e/i40e_type.h > index 9fda0cb6bdbe..7eaf8b013125 100644 > --- a/drivers/net/ethernet/intel/i40e/i40e_type.h > +++ b/drivers/net/ethernet/intel/i40e/i40e_type.h > @@ -64,9 +64,7 @@ typedef void (*I40E_ADMINQ_CALLBACK)(struct i40e_hw *, struct i40e_aq_desc *); > enum i40e_mac_type { > I40E_MAC_UNKNOWN = 0, > I40E_MAC_XL710, > - I40E_MAC_VF, > I40E_MAC_X722, > - I40E_MAC_X722_VF, > I40E_MAC_GENERIC, > }; > > @@ -588,12 +586,6 @@ struct i40e_hw { > char err_str[16]; > }; > > -static inline bool i40e_is_vf(struct i40e_hw *hw) > -{ > - return (hw->mac.type == I40E_MAC_VF || > - hw->mac.type == I40E_MAC_X722_VF); > -} > - > /** > * i40e_is_aq_api_ver_ge > * @hw: pointer to i40e_hw structure
Wed, Oct 25, 2023 at 12:48:37PM CEST, wojciech.drewek@intel.com wrote: > > >On 25.10.2023 12:33, Ivan Vecera wrote: >> The i40e_hw.mac.type cannot to be equal to I40E_MAC_VF or >> I40E_MAC_X722_VF so remove helper i40e_is_vf(), simplify >> i40e_adminq_init_regs() and remove enums for these VF MAC types. >> >> Signed-off-by: Ivan Vecera <ivecera@redhat.com> >> --- >> drivers/net/ethernet/intel/i40e/i40e_adminq.c | 33 ++++++------------- >> drivers/net/ethernet/intel/i40e/i40e_type.h | 8 ----- >> 2 files changed, 10 insertions(+), 31 deletions(-) >> >> diff --git a/drivers/net/ethernet/intel/i40e/i40e_adminq.c b/drivers/net/ethernet/intel/i40e/i40e_adminq.c >> index 29fc46abf690..896c43905309 100644 >> --- a/drivers/net/ethernet/intel/i40e/i40e_adminq.c >> +++ b/drivers/net/ethernet/intel/i40e/i40e_adminq.c >> @@ -17,29 +17,16 @@ static void i40e_resume_aq(struct i40e_hw *hw); >> static void i40e_adminq_init_regs(struct i40e_hw *hw) >> { >> /* set head and tail registers in our local struct */ >> - if (i40e_is_vf(hw)) { >> - hw->aq.asq.tail = I40E_VF_ATQT1; >> - hw->aq.asq.head = I40E_VF_ATQH1; >> - hw->aq.asq.len = I40E_VF_ATQLEN1; >> - hw->aq.asq.bal = I40E_VF_ATQBAL1; >> - hw->aq.asq.bah = I40E_VF_ATQBAH1; >> - hw->aq.arq.tail = I40E_VF_ARQT1; >> - hw->aq.arq.head = I40E_VF_ARQH1; >> - hw->aq.arq.len = I40E_VF_ARQLEN1; >> - hw->aq.arq.bal = I40E_VF_ARQBAL1; >> - hw->aq.arq.bah = I40E_VF_ARQBAH1; > >What about removing those I40E_VF_* defines? >This is their only usage here, right? Wait, do you suggest to use the values directly? That would be wild even for i40e :) > >> - } else { >> - hw->aq.asq.tail = I40E_PF_ATQT; >> - hw->aq.asq.head = I40E_PF_ATQH; >> - hw->aq.asq.len = I40E_PF_ATQLEN; >> - hw->aq.asq.bal = I40E_PF_ATQBAL; >> - hw->aq.asq.bah = I40E_PF_ATQBAH; >> - hw->aq.arq.tail = I40E_PF_ARQT; >> - hw->aq.arq.head = I40E_PF_ARQH; >> - hw->aq.arq.len = I40E_PF_ARQLEN; >> - hw->aq.arq.bal = I40E_PF_ARQBAL; >> - hw->aq.arq.bah = I40E_PF_ARQBAH; >> - } >> + hw->aq.asq.tail = I40E_PF_ATQT; >> + hw->aq.asq.head = I40E_PF_ATQH; >> + hw->aq.asq.len = I40E_PF_ATQLEN; >> + hw->aq.asq.bal = I40E_PF_ATQBAL; >> + hw->aq.asq.bah = I40E_PF_ATQBAH; >> + hw->aq.arq.tail = I40E_PF_ARQT; >> + hw->aq.arq.head = I40E_PF_ARQH; >> + hw->aq.arq.len = I40E_PF_ARQLEN; >> + hw->aq.arq.bal = I40E_PF_ARQBAL; >> + hw->aq.arq.bah = I40E_PF_ARQBAH; >> } >> >> /** >> diff --git a/drivers/net/ethernet/intel/i40e/i40e_type.h b/drivers/net/ethernet/intel/i40e/i40e_type.h >> index 9fda0cb6bdbe..7eaf8b013125 100644 >> --- a/drivers/net/ethernet/intel/i40e/i40e_type.h >> +++ b/drivers/net/ethernet/intel/i40e/i40e_type.h >> @@ -64,9 +64,7 @@ typedef void (*I40E_ADMINQ_CALLBACK)(struct i40e_hw *, struct i40e_aq_desc *); >> enum i40e_mac_type { >> I40E_MAC_UNKNOWN = 0, >> I40E_MAC_XL710, >> - I40E_MAC_VF, >> I40E_MAC_X722, >> - I40E_MAC_X722_VF, >> I40E_MAC_GENERIC, >> }; >> >> @@ -588,12 +586,6 @@ struct i40e_hw { >> char err_str[16]; >> }; >> >> -static inline bool i40e_is_vf(struct i40e_hw *hw) >> -{ >> - return (hw->mac.type == I40E_MAC_VF || >> - hw->mac.type == I40E_MAC_X722_VF); >> -} >> - >> /** >> * i40e_is_aq_api_ver_ge >> * @hw: pointer to i40e_hw structure >
Wed, Oct 25, 2023 at 02:16:39PM CEST, jiri@resnulli.us wrote: >Wed, Oct 25, 2023 at 12:48:37PM CEST, wojciech.drewek@intel.com wrote: >> >> >>On 25.10.2023 12:33, Ivan Vecera wrote: >>> The i40e_hw.mac.type cannot to be equal to I40E_MAC_VF or >>> I40E_MAC_X722_VF so remove helper i40e_is_vf(), simplify >>> i40e_adminq_init_regs() and remove enums for these VF MAC types. >>> >>> Signed-off-by: Ivan Vecera <ivecera@redhat.com> >>> --- >>> drivers/net/ethernet/intel/i40e/i40e_adminq.c | 33 ++++++------------- >>> drivers/net/ethernet/intel/i40e/i40e_type.h | 8 ----- >>> 2 files changed, 10 insertions(+), 31 deletions(-) >>> >>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_adminq.c b/drivers/net/ethernet/intel/i40e/i40e_adminq.c >>> index 29fc46abf690..896c43905309 100644 >>> --- a/drivers/net/ethernet/intel/i40e/i40e_adminq.c >>> +++ b/drivers/net/ethernet/intel/i40e/i40e_adminq.c >>> @@ -17,29 +17,16 @@ static void i40e_resume_aq(struct i40e_hw *hw); >>> static void i40e_adminq_init_regs(struct i40e_hw *hw) >>> { >>> /* set head and tail registers in our local struct */ >>> - if (i40e_is_vf(hw)) { >>> - hw->aq.asq.tail = I40E_VF_ATQT1; >>> - hw->aq.asq.head = I40E_VF_ATQH1; >>> - hw->aq.asq.len = I40E_VF_ATQLEN1; >>> - hw->aq.asq.bal = I40E_VF_ATQBAL1; >>> - hw->aq.asq.bah = I40E_VF_ATQBAH1; >>> - hw->aq.arq.tail = I40E_VF_ARQT1; >>> - hw->aq.arq.head = I40E_VF_ARQH1; >>> - hw->aq.arq.len = I40E_VF_ARQLEN1; >>> - hw->aq.arq.bal = I40E_VF_ARQBAL1; >>> - hw->aq.arq.bah = I40E_VF_ARQBAH1; >> >>What about removing those I40E_VF_* defines? >>This is their only usage here, right? > >Wait, do you suggest to use the values directly? That would be >wild even for i40e :) Ah, sec. This is duplicated in drivers/net/ethernet/intel/iavf/iavf_register.h. That confused me. > > >> >>> - } else { >>> - hw->aq.asq.tail = I40E_PF_ATQT; >>> - hw->aq.asq.head = I40E_PF_ATQH; >>> - hw->aq.asq.len = I40E_PF_ATQLEN; >>> - hw->aq.asq.bal = I40E_PF_ATQBAL; >>> - hw->aq.asq.bah = I40E_PF_ATQBAH; >>> - hw->aq.arq.tail = I40E_PF_ARQT; >>> - hw->aq.arq.head = I40E_PF_ARQH; >>> - hw->aq.arq.len = I40E_PF_ARQLEN; >>> - hw->aq.arq.bal = I40E_PF_ARQBAL; >>> - hw->aq.arq.bah = I40E_PF_ARQBAH; >>> - } >>> + hw->aq.asq.tail = I40E_PF_ATQT; >>> + hw->aq.asq.head = I40E_PF_ATQH; >>> + hw->aq.asq.len = I40E_PF_ATQLEN; >>> + hw->aq.asq.bal = I40E_PF_ATQBAL; >>> + hw->aq.asq.bah = I40E_PF_ATQBAH; >>> + hw->aq.arq.tail = I40E_PF_ARQT; >>> + hw->aq.arq.head = I40E_PF_ARQH; >>> + hw->aq.arq.len = I40E_PF_ARQLEN; >>> + hw->aq.arq.bal = I40E_PF_ARQBAL; >>> + hw->aq.arq.bah = I40E_PF_ARQBAH; >>> } >>> >>> /** >>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_type.h b/drivers/net/ethernet/intel/i40e/i40e_type.h >>> index 9fda0cb6bdbe..7eaf8b013125 100644 >>> --- a/drivers/net/ethernet/intel/i40e/i40e_type.h >>> +++ b/drivers/net/ethernet/intel/i40e/i40e_type.h >>> @@ -64,9 +64,7 @@ typedef void (*I40E_ADMINQ_CALLBACK)(struct i40e_hw *, struct i40e_aq_desc *); >>> enum i40e_mac_type { >>> I40E_MAC_UNKNOWN = 0, >>> I40E_MAC_XL710, >>> - I40E_MAC_VF, >>> I40E_MAC_X722, >>> - I40E_MAC_X722_VF, >>> I40E_MAC_GENERIC, >>> }; >>> >>> @@ -588,12 +586,6 @@ struct i40e_hw { >>> char err_str[16]; >>> }; >>> >>> -static inline bool i40e_is_vf(struct i40e_hw *hw) >>> -{ >>> - return (hw->mac.type == I40E_MAC_VF || >>> - hw->mac.type == I40E_MAC_X722_VF); >>> -} >>> - >>> /** >>> * i40e_is_aq_api_ver_ge >>> * @hw: pointer to i40e_hw structure >>
On 25.10.2023 14:20, Jiri Pirko wrote: > Wed, Oct 25, 2023 at 02:16:39PM CEST, jiri@resnulli.us wrote: >> Wed, Oct 25, 2023 at 12:48:37PM CEST, wojciech.drewek@intel.com wrote: >>> >>> >>> On 25.10.2023 12:33, Ivan Vecera wrote: >>>> The i40e_hw.mac.type cannot to be equal to I40E_MAC_VF or >>>> I40E_MAC_X722_VF so remove helper i40e_is_vf(), simplify >>>> i40e_adminq_init_regs() and remove enums for these VF MAC types. >>>> >>>> Signed-off-by: Ivan Vecera <ivecera@redhat.com> >>>> --- >>>> drivers/net/ethernet/intel/i40e/i40e_adminq.c | 33 ++++++------------- >>>> drivers/net/ethernet/intel/i40e/i40e_type.h | 8 ----- >>>> 2 files changed, 10 insertions(+), 31 deletions(-) >>>> >>>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_adminq.c b/drivers/net/ethernet/intel/i40e/i40e_adminq.c >>>> index 29fc46abf690..896c43905309 100644 >>>> --- a/drivers/net/ethernet/intel/i40e/i40e_adminq.c >>>> +++ b/drivers/net/ethernet/intel/i40e/i40e_adminq.c >>>> @@ -17,29 +17,16 @@ static void i40e_resume_aq(struct i40e_hw *hw); >>>> static void i40e_adminq_init_regs(struct i40e_hw *hw) >>>> { >>>> /* set head and tail registers in our local struct */ >>>> - if (i40e_is_vf(hw)) { >>>> - hw->aq.asq.tail = I40E_VF_ATQT1; >>>> - hw->aq.asq.head = I40E_VF_ATQH1; >>>> - hw->aq.asq.len = I40E_VF_ATQLEN1; >>>> - hw->aq.asq.bal = I40E_VF_ATQBAL1; >>>> - hw->aq.asq.bah = I40E_VF_ATQBAH1; >>>> - hw->aq.arq.tail = I40E_VF_ARQT1; >>>> - hw->aq.arq.head = I40E_VF_ARQH1; >>>> - hw->aq.arq.len = I40E_VF_ARQLEN1; >>>> - hw->aq.arq.bal = I40E_VF_ARQBAL1; >>>> - hw->aq.arq.bah = I40E_VF_ARQBAH1; >>> >>> What about removing those I40E_VF_* defines? >>> This is their only usage here, right? >> >> Wait, do you suggest to use the values directly? That would be >> wild even for i40e :) > > Ah, sec. This is duplicated in > drivers/net/ethernet/intel/iavf/iavf_register.h. That confused me. Indeed, in case of i40e they're going be unused after this patch so there is no point in keeping them, I think. > > > >> >> >>> >>>> - } else { >>>> - hw->aq.asq.tail = I40E_PF_ATQT; >>>> - hw->aq.asq.head = I40E_PF_ATQH; >>>> - hw->aq.asq.len = I40E_PF_ATQLEN; >>>> - hw->aq.asq.bal = I40E_PF_ATQBAL; >>>> - hw->aq.asq.bah = I40E_PF_ATQBAH; >>>> - hw->aq.arq.tail = I40E_PF_ARQT; >>>> - hw->aq.arq.head = I40E_PF_ARQH; >>>> - hw->aq.arq.len = I40E_PF_ARQLEN; >>>> - hw->aq.arq.bal = I40E_PF_ARQBAL; >>>> - hw->aq.arq.bah = I40E_PF_ARQBAH; >>>> - } >>>> + hw->aq.asq.tail = I40E_PF_ATQT; >>>> + hw->aq.asq.head = I40E_PF_ATQH; >>>> + hw->aq.asq.len = I40E_PF_ATQLEN; >>>> + hw->aq.asq.bal = I40E_PF_ATQBAL; >>>> + hw->aq.asq.bah = I40E_PF_ATQBAH; >>>> + hw->aq.arq.tail = I40E_PF_ARQT; >>>> + hw->aq.arq.head = I40E_PF_ARQH; >>>> + hw->aq.arq.len = I40E_PF_ARQLEN; >>>> + hw->aq.arq.bal = I40E_PF_ARQBAL; >>>> + hw->aq.arq.bah = I40E_PF_ARQBAH; >>>> } >>>> >>>> /** >>>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_type.h b/drivers/net/ethernet/intel/i40e/i40e_type.h >>>> index 9fda0cb6bdbe..7eaf8b013125 100644 >>>> --- a/drivers/net/ethernet/intel/i40e/i40e_type.h >>>> +++ b/drivers/net/ethernet/intel/i40e/i40e_type.h >>>> @@ -64,9 +64,7 @@ typedef void (*I40E_ADMINQ_CALLBACK)(struct i40e_hw *, struct i40e_aq_desc *); >>>> enum i40e_mac_type { >>>> I40E_MAC_UNKNOWN = 0, >>>> I40E_MAC_XL710, >>>> - I40E_MAC_VF, >>>> I40E_MAC_X722, >>>> - I40E_MAC_X722_VF, >>>> I40E_MAC_GENERIC, >>>> }; >>>> >>>> @@ -588,12 +586,6 @@ struct i40e_hw { >>>> char err_str[16]; >>>> }; >>>> >>>> -static inline bool i40e_is_vf(struct i40e_hw *hw) >>>> -{ >>>> - return (hw->mac.type == I40E_MAC_VF || >>>> - hw->mac.type == I40E_MAC_X722_VF); >>>> -} >>>> - >>>> /** >>>> * i40e_is_aq_api_ver_ge >>>> * @hw: pointer to i40e_hw structure >>>
On 25. 10. 23 12:48, Wojciech Drewek wrote: > > On 25.10.2023 12:33, Ivan Vecera wrote: >> The i40e_hw.mac.type cannot to be equal to I40E_MAC_VF or >> I40E_MAC_X722_VF so remove helper i40e_is_vf(), simplify >> i40e_adminq_init_regs() and remove enums for these VF MAC types. >> >> Signed-off-by: Ivan Vecera<ivecera@redhat.com> >> --- >> drivers/net/ethernet/intel/i40e/i40e_adminq.c | 33 ++++++------------- >> drivers/net/ethernet/intel/i40e/i40e_type.h | 8 ----- >> 2 files changed, 10 insertions(+), 31 deletions(-) >> >> diff --git a/drivers/net/ethernet/intel/i40e/i40e_adminq.c b/drivers/net/ethernet/intel/i40e/i40e_adminq.c >> index 29fc46abf690..896c43905309 100644 >> --- a/drivers/net/ethernet/intel/i40e/i40e_adminq.c >> +++ b/drivers/net/ethernet/intel/i40e/i40e_adminq.c >> @@ -17,29 +17,16 @@ static void i40e_resume_aq(struct i40e_hw *hw); >> static void i40e_adminq_init_regs(struct i40e_hw *hw) >> { >> /* set head and tail registers in our local struct */ >> - if (i40e_is_vf(hw)) { >> - hw->aq.asq.tail = I40E_VF_ATQT1; >> - hw->aq.asq.head = I40E_VF_ATQH1; >> - hw->aq.asq.len = I40E_VF_ATQLEN1; >> - hw->aq.asq.bal = I40E_VF_ATQBAL1; >> - hw->aq.asq.bah = I40E_VF_ATQBAH1; >> - hw->aq.arq.tail = I40E_VF_ARQT1; >> - hw->aq.arq.head = I40E_VF_ARQH1; >> - hw->aq.arq.len = I40E_VF_ARQLEN1; >> - hw->aq.arq.bal = I40E_VF_ARQBAL1; >> - hw->aq.arq.bah = I40E_VF_ARQBAH1; > What about removing those I40E_VF_* defines? > This is their only usage here, right? Yes, do you want to remove them in this patch? Or follow-up is sufficient? Ivan
On 10/25/2023 5:27 AM, Wojciech Drewek wrote: > > > On 25.10.2023 14:20, Jiri Pirko wrote: >> Wed, Oct 25, 2023 at 02:16:39PM CEST, jiri@resnulli.us wrote: >>> Wed, Oct 25, 2023 at 12:48:37PM CEST, wojciech.drewek@intel.com wrote: >>>> >>>> >>>> On 25.10.2023 12:33, Ivan Vecera wrote: >>>>> The i40e_hw.mac.type cannot to be equal to I40E_MAC_VF or >>>>> I40E_MAC_X722_VF so remove helper i40e_is_vf(), simplify >>>>> i40e_adminq_init_regs() and remove enums for these VF MAC types. >>>>> >>>>> Signed-off-by: Ivan Vecera <ivecera@redhat.com> >>>>> --- >>>>> drivers/net/ethernet/intel/i40e/i40e_adminq.c | 33 ++++++------------- >>>>> drivers/net/ethernet/intel/i40e/i40e_type.h | 8 ----- >>>>> 2 files changed, 10 insertions(+), 31 deletions(-) >>>>> >>>>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_adminq.c b/drivers/net/ethernet/intel/i40e/i40e_adminq.c >>>>> index 29fc46abf690..896c43905309 100644 >>>>> --- a/drivers/net/ethernet/intel/i40e/i40e_adminq.c >>>>> +++ b/drivers/net/ethernet/intel/i40e/i40e_adminq.c >>>>> @@ -17,29 +17,16 @@ static void i40e_resume_aq(struct i40e_hw *hw); >>>>> static void i40e_adminq_init_regs(struct i40e_hw *hw) >>>>> { >>>>> /* set head and tail registers in our local struct */ >>>>> - if (i40e_is_vf(hw)) { >>>>> - hw->aq.asq.tail = I40E_VF_ATQT1; >>>>> - hw->aq.asq.head = I40E_VF_ATQH1; >>>>> - hw->aq.asq.len = I40E_VF_ATQLEN1; >>>>> - hw->aq.asq.bal = I40E_VF_ATQBAL1; >>>>> - hw->aq.asq.bah = I40E_VF_ATQBAH1; >>>>> - hw->aq.arq.tail = I40E_VF_ARQT1; >>>>> - hw->aq.arq.head = I40E_VF_ARQH1; >>>>> - hw->aq.arq.len = I40E_VF_ARQLEN1; >>>>> - hw->aq.arq.bal = I40E_VF_ARQBAL1; >>>>> - hw->aq.arq.bah = I40E_VF_ARQBAH1; >>>> >>>> What about removing those I40E_VF_* defines? >>>> This is their only usage here, right? >>> >>> Wait, do you suggest to use the values directly? That would be >>> wild even for i40e :) >> >> Ah, sec. This is duplicated in >> drivers/net/ethernet/intel/iavf/iavf_register.h. That confused me. > > Indeed, in case of i40e they're going be unused after this patch so > there is no point in keeping them, I think. > Ya, this is all a relic from early days when i40e planned to share code with i40evf.
On 10/25/2023 7:39 AM, Ivan Vecera wrote: > > > On 25. 10. 23 12:48, Wojciech Drewek wrote: >> >> On 25.10.2023 12:33, Ivan Vecera wrote: >>> The i40e_hw.mac.type cannot to be equal to I40E_MAC_VF or >>> I40E_MAC_X722_VF so remove helper i40e_is_vf(), simplify >>> i40e_adminq_init_regs() and remove enums for these VF MAC types. >>> >>> Signed-off-by: Ivan Vecera<ivecera@redhat.com> >>> --- >>> drivers/net/ethernet/intel/i40e/i40e_adminq.c | 33 ++++++------------- >>> drivers/net/ethernet/intel/i40e/i40e_type.h | 8 ----- >>> 2 files changed, 10 insertions(+), 31 deletions(-) >>> >>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_adminq.c b/drivers/net/ethernet/intel/i40e/i40e_adminq.c >>> index 29fc46abf690..896c43905309 100644 >>> --- a/drivers/net/ethernet/intel/i40e/i40e_adminq.c >>> +++ b/drivers/net/ethernet/intel/i40e/i40e_adminq.c >>> @@ -17,29 +17,16 @@ static void i40e_resume_aq(struct i40e_hw *hw); >>> static void i40e_adminq_init_regs(struct i40e_hw *hw) >>> { >>> /* set head and tail registers in our local struct */ >>> - if (i40e_is_vf(hw)) { >>> - hw->aq.asq.tail = I40E_VF_ATQT1; >>> - hw->aq.asq.head = I40E_VF_ATQH1; >>> - hw->aq.asq.len = I40E_VF_ATQLEN1; >>> - hw->aq.asq.bal = I40E_VF_ATQBAL1; >>> - hw->aq.asq.bah = I40E_VF_ATQBAH1; >>> - hw->aq.arq.tail = I40E_VF_ARQT1; >>> - hw->aq.arq.head = I40E_VF_ARQH1; >>> - hw->aq.arq.len = I40E_VF_ARQLEN1; >>> - hw->aq.arq.bal = I40E_VF_ARQBAL1; >>> - hw->aq.arq.bah = I40E_VF_ARQBAH1; >> What about removing those I40E_VF_* defines? >> This is their only usage here, right? > > Yes, do you want to remove them in this patch? Or follow-up is sufficient? > > Ivan > > I'm fine with a follow up. Thanks, Jake
On 10/25/2023 5:20 AM, Jiri Pirko wrote: > Wed, Oct 25, 2023 at 02:16:39PM CEST, jiri@resnulli.us wrote: >> Wed, Oct 25, 2023 at 12:48:37PM CEST, wojciech.drewek@intel.com wrote: >>> >>> >>> On 25.10.2023 12:33, Ivan Vecera wrote: >>>> The i40e_hw.mac.type cannot to be equal to I40E_MAC_VF or >>>> I40E_MAC_X722_VF so remove helper i40e_is_vf(), simplify >>>> i40e_adminq_init_regs() and remove enums for these VF MAC types. >>>> >>>> Signed-off-by: Ivan Vecera <ivecera@redhat.com> >>>> --- >>>> drivers/net/ethernet/intel/i40e/i40e_adminq.c | 33 ++++++------------- >>>> drivers/net/ethernet/intel/i40e/i40e_type.h | 8 ----- >>>> 2 files changed, 10 insertions(+), 31 deletions(-) >>>> >>>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_adminq.c b/drivers/net/ethernet/intel/i40e/i40e_adminq.c >>>> index 29fc46abf690..896c43905309 100644 >>>> --- a/drivers/net/ethernet/intel/i40e/i40e_adminq.c >>>> +++ b/drivers/net/ethernet/intel/i40e/i40e_adminq.c >>>> @@ -17,29 +17,16 @@ static void i40e_resume_aq(struct i40e_hw *hw); >>>> static void i40e_adminq_init_regs(struct i40e_hw *hw) >>>> { >>>> /* set head and tail registers in our local struct */ >>>> - if (i40e_is_vf(hw)) { >>>> - hw->aq.asq.tail = I40E_VF_ATQT1; >>>> - hw->aq.asq.head = I40E_VF_ATQH1; >>>> - hw->aq.asq.len = I40E_VF_ATQLEN1; >>>> - hw->aq.asq.bal = I40E_VF_ATQBAL1; >>>> - hw->aq.asq.bah = I40E_VF_ATQBAH1; >>>> - hw->aq.arq.tail = I40E_VF_ARQT1; >>>> - hw->aq.arq.head = I40E_VF_ARQH1; >>>> - hw->aq.arq.len = I40E_VF_ARQLEN1; >>>> - hw->aq.arq.bal = I40E_VF_ARQBAL1; >>>> - hw->aq.arq.bah = I40E_VF_ARQBAH1; >>> >>> What about removing those I40E_VF_* defines? >>> This is their only usage here, right? >> >> Wait, do you suggest to use the values directly? That would be >> wild even for i40e :) > > Ah, sec. This is duplicated in > drivers/net/ethernet/intel/iavf/iavf_register.h. That confused me. > Its possible the iAVF code could be cleaned up too... Historically the i40e and i40evf duplicated quite some code.
On 25.10.2023 19:44, Jacob Keller wrote: > > > On 10/25/2023 7:39 AM, Ivan Vecera wrote: >> >> >> On 25. 10. 23 12:48, Wojciech Drewek wrote: >>> >>> On 25.10.2023 12:33, Ivan Vecera wrote: >>>> The i40e_hw.mac.type cannot to be equal to I40E_MAC_VF or >>>> I40E_MAC_X722_VF so remove helper i40e_is_vf(), simplify >>>> i40e_adminq_init_regs() and remove enums for these VF MAC types. >>>> >>>> Signed-off-by: Ivan Vecera<ivecera@redhat.com> >>>> --- >>>> drivers/net/ethernet/intel/i40e/i40e_adminq.c | 33 ++++++------------- >>>> drivers/net/ethernet/intel/i40e/i40e_type.h | 8 ----- >>>> 2 files changed, 10 insertions(+), 31 deletions(-) >>>> >>>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_adminq.c b/drivers/net/ethernet/intel/i40e/i40e_adminq.c >>>> index 29fc46abf690..896c43905309 100644 >>>> --- a/drivers/net/ethernet/intel/i40e/i40e_adminq.c >>>> +++ b/drivers/net/ethernet/intel/i40e/i40e_adminq.c >>>> @@ -17,29 +17,16 @@ static void i40e_resume_aq(struct i40e_hw *hw); >>>> static void i40e_adminq_init_regs(struct i40e_hw *hw) >>>> { >>>> /* set head and tail registers in our local struct */ >>>> - if (i40e_is_vf(hw)) { >>>> - hw->aq.asq.tail = I40E_VF_ATQT1; >>>> - hw->aq.asq.head = I40E_VF_ATQH1; >>>> - hw->aq.asq.len = I40E_VF_ATQLEN1; >>>> - hw->aq.asq.bal = I40E_VF_ATQBAL1; >>>> - hw->aq.asq.bah = I40E_VF_ATQBAH1; >>>> - hw->aq.arq.tail = I40E_VF_ARQT1; >>>> - hw->aq.arq.head = I40E_VF_ARQH1; >>>> - hw->aq.arq.len = I40E_VF_ARQLEN1; >>>> - hw->aq.arq.bal = I40E_VF_ARQBAL1; >>>> - hw->aq.arq.bah = I40E_VF_ARQBAH1; >>> What about removing those I40E_VF_* defines? >>> This is their only usage here, right? >> >> Yes, do you want to remove them in this patch? Or follow-up is sufficient? >> >> Ivan >> >> > > I'm fine with a follow up. Me too > > Thanks, > Jake
> -----Original Message----- > From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of Ivan Vecera > Sent: Wednesday, October 25, 2023 4:03 PM > To: netdev@vger.kernel.org > Cc: intel-wired-lan@lists.osuosl.org; Brandeburg, Jesse <jesse.brandeburg@intel.com>; linux-kernel@vger.kernel.org; Eric Dumazet <edumazet@google.com>; Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Keller, Jacob E <jacob.e.keller@intel.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; David S. Miller <davem@davemloft.net> > Subject: [Intel-wired-lan] [PATCH iwl-next 1/2] i40e: Remove VF MAC types > > The i40e_hw.mac.type cannot to be equal to I40E_MAC_VF or > I40E_MAC_X722_VF so remove helper i40e_is_vf(), simplify > i40e_adminq_init_regs() and remove enums for these VF MAC types. > > Signed-off-by: Ivan Vecera <ivecera@redhat.com> > --- > drivers/net/ethernet/intel/i40e/i40e_adminq.c | 33 ++++++------------- > drivers/net/ethernet/intel/i40e/i40e_type.h | 8 ----- > 2 files changed, 10 insertions(+), 31 deletions(-) > Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@intel.com> (A Contingent worker at Intel)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_adminq.c b/drivers/net/ethernet/intel/i40e/i40e_adminq.c index 29fc46abf690..896c43905309 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_adminq.c +++ b/drivers/net/ethernet/intel/i40e/i40e_adminq.c @@ -17,29 +17,16 @@ static void i40e_resume_aq(struct i40e_hw *hw); static void i40e_adminq_init_regs(struct i40e_hw *hw) { /* set head and tail registers in our local struct */ - if (i40e_is_vf(hw)) { - hw->aq.asq.tail = I40E_VF_ATQT1; - hw->aq.asq.head = I40E_VF_ATQH1; - hw->aq.asq.len = I40E_VF_ATQLEN1; - hw->aq.asq.bal = I40E_VF_ATQBAL1; - hw->aq.asq.bah = I40E_VF_ATQBAH1; - hw->aq.arq.tail = I40E_VF_ARQT1; - hw->aq.arq.head = I40E_VF_ARQH1; - hw->aq.arq.len = I40E_VF_ARQLEN1; - hw->aq.arq.bal = I40E_VF_ARQBAL1; - hw->aq.arq.bah = I40E_VF_ARQBAH1; - } else { - hw->aq.asq.tail = I40E_PF_ATQT; - hw->aq.asq.head = I40E_PF_ATQH; - hw->aq.asq.len = I40E_PF_ATQLEN; - hw->aq.asq.bal = I40E_PF_ATQBAL; - hw->aq.asq.bah = I40E_PF_ATQBAH; - hw->aq.arq.tail = I40E_PF_ARQT; - hw->aq.arq.head = I40E_PF_ARQH; - hw->aq.arq.len = I40E_PF_ARQLEN; - hw->aq.arq.bal = I40E_PF_ARQBAL; - hw->aq.arq.bah = I40E_PF_ARQBAH; - } + hw->aq.asq.tail = I40E_PF_ATQT; + hw->aq.asq.head = I40E_PF_ATQH; + hw->aq.asq.len = I40E_PF_ATQLEN; + hw->aq.asq.bal = I40E_PF_ATQBAL; + hw->aq.asq.bah = I40E_PF_ATQBAH; + hw->aq.arq.tail = I40E_PF_ARQT; + hw->aq.arq.head = I40E_PF_ARQH; + hw->aq.arq.len = I40E_PF_ARQLEN; + hw->aq.arq.bal = I40E_PF_ARQBAL; + hw->aq.arq.bah = I40E_PF_ARQBAH; } /** diff --git a/drivers/net/ethernet/intel/i40e/i40e_type.h b/drivers/net/ethernet/intel/i40e/i40e_type.h index 9fda0cb6bdbe..7eaf8b013125 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_type.h +++ b/drivers/net/ethernet/intel/i40e/i40e_type.h @@ -64,9 +64,7 @@ typedef void (*I40E_ADMINQ_CALLBACK)(struct i40e_hw *, struct i40e_aq_desc *); enum i40e_mac_type { I40E_MAC_UNKNOWN = 0, I40E_MAC_XL710, - I40E_MAC_VF, I40E_MAC_X722, - I40E_MAC_X722_VF, I40E_MAC_GENERIC, }; @@ -588,12 +586,6 @@ struct i40e_hw { char err_str[16]; }; -static inline bool i40e_is_vf(struct i40e_hw *hw) -{ - return (hw->mac.type == I40E_MAC_VF || - hw->mac.type == I40E_MAC_X722_VF); -} - /** * i40e_is_aq_api_ver_ge * @hw: pointer to i40e_hw structure
The i40e_hw.mac.type cannot to be equal to I40E_MAC_VF or I40E_MAC_X722_VF so remove helper i40e_is_vf(), simplify i40e_adminq_init_regs() and remove enums for these VF MAC types. Signed-off-by: Ivan Vecera <ivecera@redhat.com> --- drivers/net/ethernet/intel/i40e/i40e_adminq.c | 33 ++++++------------- drivers/net/ethernet/intel/i40e/i40e_type.h | 8 ----- 2 files changed, 10 insertions(+), 31 deletions(-)