On 13. 11. 23 14:27, Wojciech Drewek wrote:
>
> On 13.11.2023 13:58, Ivan Vecera wrote:
>> Add two helpers i40e_(veb|vsi)_get_by_seid() to find corresponding
>> VEB or VSI by their SEID value and use these helpers to replace
>> existing open-coded loops.
>>
>> Signed-off-by: Ivan Vecera<[email protected]>
>> ---
> Only one nit
> Reviewed-by: Wojciech Drewek<[email protected]>
>
>> drivers/net/ethernet/intel/i40e/i40e.h | 34 +++++++++
>> .../net/ethernet/intel/i40e/i40e_debugfs.c | 38 ++--------
>> drivers/net/ethernet/intel/i40e/i40e_main.c | 76 ++++++-------------
>> 3 files changed, 64 insertions(+), 84 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
>> index 1e9266de270b..220b5ce31519 100644
>> --- a/drivers/net/ethernet/intel/i40e/i40e.h
>> +++ b/drivers/net/ethernet/intel/i40e/i40e.h
>> @@ -1360,4 +1360,38 @@ static inline struct i40e_pf *i40e_hw_to_pf(struct i40e_hw *hw)
>>
>> struct device *i40e_hw_to_dev(struct i40e_hw *hw);
>>
>> +/**
>> + * i40e_vsi_get_by_seid - find VSI by SEID
>> + * @pf: pointer to a PF
>> + **/
>> +static inline struct i40e_vsi *
>> +i40e_vsi_get_by_seid(struct i40e_pf *pf, u16 seid)
>> +{
>> + struct i40e_vsi *vsi;
>> + int i;
>> +
>> + i40e_pf_for_each_vsi(pf, i, vsi)
>> + if (vsi->seid == seid)
>> + return vsi;
>> +
>> + return NULL;
>> +}
>> +
>> +/**
>> + * i40e_veb_get_by_seid - find VEB by SEID
>> + * @pf: pointer to a PF
>> + **/
>> +static inline struct i40e_veb *
>> +i40e_veb_get_by_seid(struct i40e_pf *pf, u16 seid)
>> +{
>> + struct i40e_veb *veb;
>> + int i;
>> +
>> + i40e_pf_for_each_veb(pf, i, veb)
>> + if (veb->seid == seid)
>> + return veb;
>> +
>> + return NULL;
>> +}
> I would prefer i40e_get_{veb|vsi}_by_seid but it's my opinion.
I'd rather use i40e_pf_ prefix...
What about i40e_pf_get_vsi_by_seid() and i40e_pf_get_veb_by_seid() ?
Ivan
On 15.11.2023 18:01, Ivan Vecera wrote:
>
> On 13. 11. 23 14:27, Wojciech Drewek wrote:
>>
>> On 13.11.2023 13:58, Ivan Vecera wrote:
>>> Add two helpers i40e_(veb|vsi)_get_by_seid() to find corresponding
>>> VEB or VSI by their SEID value and use these helpers to replace
>>> existing open-coded loops.
>>>
>>> Signed-off-by: Ivan Vecera<[email protected]>
>>> ---
>> Only one nit
>> Reviewed-by: Wojciech Drewek<[email protected]>
>>
>>> drivers/net/ethernet/intel/i40e/i40e.h | 34 +++++++++
>>> .../net/ethernet/intel/i40e/i40e_debugfs.c | 38 ++--------
>>> drivers/net/ethernet/intel/i40e/i40e_main.c | 76 ++++++-------------
>>> 3 files changed, 64 insertions(+), 84 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
>>> index 1e9266de270b..220b5ce31519 100644
>>> --- a/drivers/net/ethernet/intel/i40e/i40e.h
>>> +++ b/drivers/net/ethernet/intel/i40e/i40e.h
>>> @@ -1360,4 +1360,38 @@ static inline struct i40e_pf *i40e_hw_to_pf(struct i40e_hw *hw)
>>> struct device *i40e_hw_to_dev(struct i40e_hw *hw);
>>> +/**
>>> + * i40e_vsi_get_by_seid - find VSI by SEID
>>> + * @pf: pointer to a PF
>>> + **/
>>> +static inline struct i40e_vsi *
>>> +i40e_vsi_get_by_seid(struct i40e_pf *pf, u16 seid)
>>> +{
>>> + struct i40e_vsi *vsi;
>>> + int i;
>>> +
>>> + i40e_pf_for_each_vsi(pf, i, vsi)
>>> + if (vsi->seid == seid)
>>> + return vsi;
>>> +
>>> + return NULL;
>>> +}
>>> +
>>> +/**
>>> + * i40e_veb_get_by_seid - find VEB by SEID
>>> + * @pf: pointer to a PF
>>> + **/
>>> +static inline struct i40e_veb *
>>> +i40e_veb_get_by_seid(struct i40e_pf *pf, u16 seid)
>>> +{
>>> + struct i40e_veb *veb;
>>> + int i;
>>> +
>>> + i40e_pf_for_each_veb(pf, i, veb)
>>> + if (veb->seid == seid)
>>> + return veb;
>>> +
>>> + return NULL;
>>> +}
>> I would prefer i40e_get_{veb|vsi}_by_seid but it's my opinion.
>
> I'd rather use i40e_pf_ prefix...
>
> What about i40e_pf_get_vsi_by_seid() and i40e_pf_get_veb_by_seid() ?
Sounds good, my point was that I prefer to have "get" before "{veb|vsi}"
>
> Ivan
>
On 16. 11. 23 13:37, Wojciech Drewek wrote:
>
>
> On 15.11.2023 18:01, Ivan Vecera wrote:
>>
>> On 13. 11. 23 14:27, Wojciech Drewek wrote:
>>>
>>> On 13.11.2023 13:58, Ivan Vecera wrote:
>>>> Add two helpers i40e_(veb|vsi)_get_by_seid() to find corresponding
>>>> VEB or VSI by their SEID value and use these helpers to replace
>>>> existing open-coded loops.
>>>>
>>>> Signed-off-by: Ivan Vecera<[email protected]>
>>>> ---
>>> Only one nit
>>> Reviewed-by: Wojciech Drewek<[email protected]>
>>>
>>>> drivers/net/ethernet/intel/i40e/i40e.h | 34 +++++++++
>>>> .../net/ethernet/intel/i40e/i40e_debugfs.c | 38 ++--------
>>>> drivers/net/ethernet/intel/i40e/i40e_main.c | 76 ++++++-------------
>>>> 3 files changed, 64 insertions(+), 84 deletions(-)
>>>>
>>>> diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
>>>> index 1e9266de270b..220b5ce31519 100644
>>>> --- a/drivers/net/ethernet/intel/i40e/i40e.h
>>>> +++ b/drivers/net/ethernet/intel/i40e/i40e.h
>>>> @@ -1360,4 +1360,38 @@ static inline struct i40e_pf *i40e_hw_to_pf(struct i40e_hw *hw)
>>>> struct device *i40e_hw_to_dev(struct i40e_hw *hw);
>>>> +/**
>>>> + * i40e_vsi_get_by_seid - find VSI by SEID
>>>> + * @pf: pointer to a PF
>>>> + **/
>>>> +static inline struct i40e_vsi *
>>>> +i40e_vsi_get_by_seid(struct i40e_pf *pf, u16 seid)
>>>> +{
>>>> + struct i40e_vsi *vsi;
>>>> + int i;
>>>> +
>>>> + i40e_pf_for_each_vsi(pf, i, vsi)
>>>> + if (vsi->seid == seid)
>>>> + return vsi;
>>>> +
>>>> + return NULL;
>>>> +}
>>>> +
>>>> +/**
>>>> + * i40e_veb_get_by_seid - find VEB by SEID
>>>> + * @pf: pointer to a PF
>>>> + **/
>>>> +static inline struct i40e_veb *
>>>> +i40e_veb_get_by_seid(struct i40e_pf *pf, u16 seid)
>>>> +{
>>>> + struct i40e_veb *veb;
>>>> + int i;
>>>> +
>>>> + i40e_pf_for_each_veb(pf, i, veb)
>>>> + if (veb->seid == seid)
>>>> + return veb;
>>>> +
>>>> + return NULL;
>>>> +}
>>> I would prefer i40e_get_{veb|vsi}_by_seid but it's my opinion.
>>
>> I'd rather use i40e_pf_ prefix...
>>
>> What about i40e_pf_get_vsi_by_seid() and i40e_pf_get_veb_by_seid() ?
>
> Sounds good, my point was that I prefer to have "get" before "{veb|vsi}"
OK, got it... Will repost v2 with this change + "too many also..." issue ;-)
Btw. what about the last patch?
Ivan
On 16.11.2023 14:59, Ivan Vecera wrote:
>
> On 16. 11. 23 13:37, Wojciech Drewek wrote:
>>
>>
>> On 15.11.2023 18:01, Ivan Vecera wrote:
>>>
>>> On 13. 11. 23 14:27, Wojciech Drewek wrote:
>>>>
>>>> On 13.11.2023 13:58, Ivan Vecera wrote:
>>>>> Add two helpers i40e_(veb|vsi)_get_by_seid() to find corresponding
>>>>> VEB or VSI by their SEID value and use these helpers to replace
>>>>> existing open-coded loops.
>>>>>
>>>>> Signed-off-by: Ivan Vecera<[email protected]>
>>>>> ---
>>>> Only one nit
>>>> Reviewed-by: Wojciech Drewek<[email protected]>
>>>>
>>>>> drivers/net/ethernet/intel/i40e/i40e.h | 34 +++++++++
>>>>> .../net/ethernet/intel/i40e/i40e_debugfs.c | 38 ++--------
>>>>> drivers/net/ethernet/intel/i40e/i40e_main.c | 76 ++++++-------------
>>>>> 3 files changed, 64 insertions(+), 84 deletions(-)
>>>>>
>>>>> diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
>>>>> index 1e9266de270b..220b5ce31519 100644
>>>>> --- a/drivers/net/ethernet/intel/i40e/i40e.h
>>>>> +++ b/drivers/net/ethernet/intel/i40e/i40e.h
>>>>> @@ -1360,4 +1360,38 @@ static inline struct i40e_pf *i40e_hw_to_pf(struct i40e_hw *hw)
>>>>> struct device *i40e_hw_to_dev(struct i40e_hw *hw);
>>>>> +/**
>>>>> + * i40e_vsi_get_by_seid - find VSI by SEID
>>>>> + * @pf: pointer to a PF
>>>>> + **/
>>>>> +static inline struct i40e_vsi *
>>>>> +i40e_vsi_get_by_seid(struct i40e_pf *pf, u16 seid)
>>>>> +{
>>>>> + struct i40e_vsi *vsi;
>>>>> + int i;
>>>>> +
>>>>> + i40e_pf_for_each_vsi(pf, i, vsi)
>>>>> + if (vsi->seid == seid)
>>>>> + return vsi;
>>>>> +
>>>>> + return NULL;
>>>>> +}
>>>>> +
>>>>> +/**
>>>>> + * i40e_veb_get_by_seid - find VEB by SEID
>>>>> + * @pf: pointer to a PF
>>>>> + **/
>>>>> +static inline struct i40e_veb *
>>>>> +i40e_veb_get_by_seid(struct i40e_pf *pf, u16 seid)
>>>>> +{
>>>>> + struct i40e_veb *veb;
>>>>> + int i;
>>>>> +
>>>>> + i40e_pf_for_each_veb(pf, i, veb)
>>>>> + if (veb->seid == seid)
>>>>> + return veb;
>>>>> +
>>>>> + return NULL;
>>>>> +}
>>>> I would prefer i40e_get_{veb|vsi}_by_seid but it's my opinion.
>>>
>>> I'd rather use i40e_pf_ prefix...
>>>
>>> What about i40e_pf_get_vsi_by_seid() and i40e_pf_get_veb_by_seid() ?
>>
>> Sounds good, my point was that I prefer to have "get" before "{veb|vsi}"
>
> OK, got it... Will repost v2 with this change + "too many also..." issue ;-)
Thanks
>
> Btw. what about the last patch?
Reviewed :)
>
> Ivan
>
On 16. 11. 23 15:21, Wojciech Drewek wrote:
>>> Sounds good, my point was that I prefer to have "get" before "{veb|vsi}"
>> OK, got it... Will repost v2 with this change + "too many also..." issue ????
> Thanks
>
>> Btw. what about the last patch?
> Reviewed ????
Thanks, Wojciech! But I already submitted v2 of the series...but without
your 'Reviewed-by:' tag in patch 5. Could you please "review" the v2 of
patch 5 again?
Thanks,
Ivan