diff mbox series

[V2,07/14] net/hns3: fix device capabilities for copper media type

Message ID 1614693534-27620-8-git-send-email-oulijun@huawei.com (mailing list archive)
State Changes Requested, archived
Delegated to: Ferruh Yigit
Headers
Series Features and bugfixes for hns3 |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Lijun Ou March 2, 2021, 1:58 p.m. UTC 
  From: Huisong Li <lihuisong@huawei.com>

The configuration operation for PHY is implemented by firmware. And
a capability flag will be report to driver, which means the firmware
supports the PHY driver.  However, the current implementation only
supports obtaining the capability bit, but some basic functions of
copper ports in driver, such as, the query of link status and link
info, are not supported.

Therefore, it is necessary for driver to set the copper capability
bit to zero when the firmware supports the configuration of the PHY.

Fixes: 438752358158 ("net/hns3: get device capability from firmware")
Fixes: 95e50325864c ("net/hns3: support copper media type")
Cc: stable@dpdk.org

Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
 drivers/net/hns3/hns3_cmd.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Comments

Ferruh Yigit March 3, 2021, 1:27 p.m. UTC | #1 
On 3/2/2021 1:58 PM, Lijun Ou wrote:
> From: Huisong Li <lihuisong@huawei.com>
> 
> The configuration operation for PHY is implemented by firmware. And
> a capability flag will be report to driver, which means the firmware
> supports the PHY driver.  However, the current implementation only
> supports obtaining the capability bit, but some basic functions of
> copper ports in driver, such as, the query of link status and link
> info, are not supported.
> 
> Therefore, it is necessary for driver to set the copper capability
> bit to zero when the firmware supports the configuration of the PHY.
> 
> Fixes: 438752358158 ("net/hns3: get device capability from firmware")
> Fixes: 95e50325864c ("net/hns3: support copper media type")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> ---
>   drivers/net/hns3/hns3_cmd.c | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/hns3/hns3_cmd.c b/drivers/net/hns3/hns3_cmd.c
> index 32cd56b..ec34615 100644
> --- a/drivers/net/hns3/hns3_cmd.c
> +++ b/drivers/net/hns3/hns3_cmd.c
> @@ -423,8 +423,14 @@ static void hns3_parse_capability(struct hns3_hw *hw,
>   		hns3_set_bit(hw->capability, HNS3_DEV_SUPPORT_PTP_B, 1);
>   	if (hns3_get_bit(caps, HNS3_CAPS_TX_PUSH_B))	
>   		hns3_set_bit(hw->capability, HNS3_DEV_SUPPORT_TX_PUSH_B, 1);
> +	/*
> +	 * Currently, the query of link status and link info on copper ports
> +	 * are not supported. So it is necessary for driver to set the copper
> +	 * capability bit to zero when the firmware supports the configuration
> +	 * of the PHY.
> +	 */
>   	if (hns3_get_bit(caps, HNS3_CAPS_PHY_IMP_B))
> -		hns3_set_bit(hw->capability, HNS3_DEV_SUPPORT_COPPER_B, 1);
> +		hns3_set_bit(hw->capability, HNS3_DEV_SUPPORT_COPPER_B, 0);
>   	if (hns3_get_bit(caps, HNS3_CAPS_TQP_TXRX_INDEP_B))
>   		hns3_set_bit(hw->capability, HNS3_DEV_SUPPORT_INDEP_TXRX_B, 1);
>   	if (hns3_get_bit(caps, HNS3_CAPS_STASH_B))
> 

Hi Lijun,

I see this patch is to be accurate, but in next patch the link status is 
implemented and this patch practically reverted back completely, so I guess this 
patch can be dropped, what do you think?
Lijun Ou March 3, 2021, 1:51 p.m. UTC | #2 
在 2021/3/3 21:27, Ferruh Yigit 写道:
> On 3/2/2021 1:58 PM, Lijun Ou wrote:
>> From: Huisong Li <lihuisong@huawei.com>
>>
>> The configuration operation for PHY is implemented by firmware. And
>> a capability flag will be report to driver, which means the firmware
>> supports the PHY driver.  However, the current implementation only
>> supports obtaining the capability bit, but some basic functions of
>> copper ports in driver, such as, the query of link status and link
>> info, are not supported.
>>
>> Therefore, it is necessary for driver to set the copper capability
>> bit to zero when the firmware supports the configuration of the PHY.
>>
>> Fixes: 438752358158 ("net/hns3: get device capability from firmware")
>> Fixes: 95e50325864c ("net/hns3: support copper media type")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>> ---
>>   drivers/net/hns3/hns3_cmd.c | 8 +++++++-
>>   1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/hns3/hns3_cmd.c b/drivers/net/hns3/hns3_cmd.c
>> index 32cd56b..ec34615 100644
>> --- a/drivers/net/hns3/hns3_cmd.c
>> +++ b/drivers/net/hns3/hns3_cmd.c
>> @@ -423,8 +423,14 @@ static void hns3_parse_capability(struct hns3_hw 
>> *hw,
>>           hns3_set_bit(hw->capability, HNS3_DEV_SUPPORT_PTP_B, 1);
>>       if (hns3_get_bit(caps, HNS3_CAPS_TX_PUSH_B))
>>           hns3_set_bit(hw->capability, HNS3_DEV_SUPPORT_TX_PUSH_B, 1);
>> +    /*
>> +     * Currently, the query of link status and link info on copper ports
>> +     * are not supported. So it is necessary for driver to set the 
>> copper
>> +     * capability bit to zero when the firmware supports the 
>> configuration
>> +     * of the PHY.
>> +     */
>>       if (hns3_get_bit(caps, HNS3_CAPS_PHY_IMP_B))
>> -        hns3_set_bit(hw->capability, HNS3_DEV_SUPPORT_COPPER_B, 1);
>> +        hns3_set_bit(hw->capability, HNS3_DEV_SUPPORT_COPPER_B, 0);
>>       if (hns3_get_bit(caps, HNS3_CAPS_TQP_TXRX_INDEP_B))
>>           hns3_set_bit(hw->capability, HNS3_DEV_SUPPORT_INDEP_TXRX_B, 1);
>>       if (hns3_get_bit(caps, HNS3_CAPS_STASH_B))
>>
> 
> Hi Lijun,
> 
> I see this patch is to be accurate, but in next patch the link status is 
> implemented and this patch practically reverted back completely, so I 
> guess this patch can be dropped, what do you think?
Yes, I am.  I split up on purpose. We find that if patch [8/14] is not 
integrated, the COPPER function should not be supported in 
hw->capability.That is, in 20.11 and 21.02, hw->capability should not 
support copper,this patch needs to be backported to 20.11 and 21.02.
For the above reasons, we'll split it up.
> .
>
Ferruh Yigit March 3, 2021, 1:58 p.m. UTC | #3 
On 3/3/2021 1:51 PM, oulijun wrote:
> 
> 
> 在 2021/3/3 21:27, Ferruh Yigit 写道:
>> On 3/2/2021 1:58 PM, Lijun Ou wrote:
>>> From: Huisong Li <lihuisong@huawei.com>
>>>
>>> The configuration operation for PHY is implemented by firmware. And
>>> a capability flag will be report to driver, which means the firmware
>>> supports the PHY driver.  However, the current implementation only
>>> supports obtaining the capability bit, but some basic functions of
>>> copper ports in driver, such as, the query of link status and link
>>> info, are not supported.
>>>
>>> Therefore, it is necessary for driver to set the copper capability
>>> bit to zero when the firmware supports the configuration of the PHY.
>>>
>>> Fixes: 438752358158 ("net/hns3: get device capability from firmware")
>>> Fixes: 95e50325864c ("net/hns3: support copper media type")
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>>> ---
>>>   drivers/net/hns3/hns3_cmd.c | 8 +++++++-
>>>   1 file changed, 7 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/hns3/hns3_cmd.c b/drivers/net/hns3/hns3_cmd.c
>>> index 32cd56b..ec34615 100644
>>> --- a/drivers/net/hns3/hns3_cmd.c
>>> +++ b/drivers/net/hns3/hns3_cmd.c
>>> @@ -423,8 +423,14 @@ static void hns3_parse_capability(struct hns3_hw *hw,
>>>           hns3_set_bit(hw->capability, HNS3_DEV_SUPPORT_PTP_B, 1);
>>>       if (hns3_get_bit(caps, HNS3_CAPS_TX_PUSH_B))
>>>           hns3_set_bit(hw->capability, HNS3_DEV_SUPPORT_TX_PUSH_B, 1);
>>> +    /*
>>> +     * Currently, the query of link status and link info on copper ports
>>> +     * are not supported. So it is necessary for driver to set the copper
>>> +     * capability bit to zero when the firmware supports the configuration
>>> +     * of the PHY.
>>> +     */
>>>       if (hns3_get_bit(caps, HNS3_CAPS_PHY_IMP_B))
>>> -        hns3_set_bit(hw->capability, HNS3_DEV_SUPPORT_COPPER_B, 1);
>>> +        hns3_set_bit(hw->capability, HNS3_DEV_SUPPORT_COPPER_B, 0);
>>>       if (hns3_get_bit(caps, HNS3_CAPS_TQP_TXRX_INDEP_B))
>>>           hns3_set_bit(hw->capability, HNS3_DEV_SUPPORT_INDEP_TXRX_B, 1);
>>>       if (hns3_get_bit(caps, HNS3_CAPS_STASH_B))
>>>
>>
>> Hi Lijun,
>>
>> I see this patch is to be accurate, but in next patch the link status is 
>> implemented and this patch practically reverted back completely, so I guess 
>> this patch can be dropped, what do you think?
> Yes, I am.  I split up on purpose. We find that if patch [8/14] is not 
> integrated, the COPPER function should not be supported in hw->capability.That 
> is, in 20.11 and 21.02, hw->capability should not support copper,this patch 
> needs to be backported to 20.11 and 21.02.
> For the above reasons, we'll split it up.

make sense from backporting point of view, OK to continue as it is.
diff mbox series

Patch

diff --git a/drivers/net/hns3/hns3_cmd.c b/drivers/net/hns3/hns3_cmd.c
index 32cd56b..ec34615 100644
--- a/drivers/net/hns3/hns3_cmd.c
+++ b/drivers/net/hns3/hns3_cmd.c
@@ -423,8 +423,14 @@  static void hns3_parse_capability(struct hns3_hw *hw,
 		hns3_set_bit(hw->capability, HNS3_DEV_SUPPORT_PTP_B, 1);
 	if (hns3_get_bit(caps, HNS3_CAPS_TX_PUSH_B))
 		hns3_set_bit(hw->capability, HNS3_DEV_SUPPORT_TX_PUSH_B, 1);
+	/*
+	 * Currently, the query of link status and link info on copper ports
+	 * are not supported. So it is necessary for driver to set the copper
+	 * capability bit to zero when the firmware supports the configuration
+	 * of the PHY.
+	 */
 	if (hns3_get_bit(caps, HNS3_CAPS_PHY_IMP_B))
-		hns3_set_bit(hw->capability, HNS3_DEV_SUPPORT_COPPER_B, 1);
+		hns3_set_bit(hw->capability, HNS3_DEV_SUPPORT_COPPER_B, 0);
 	if (hns3_get_bit(caps, HNS3_CAPS_TQP_TXRX_INDEP_B))
 		hns3_set_bit(hw->capability, HNS3_DEV_SUPPORT_INDEP_TXRX_B, 1);
 	if (hns3_get_bit(caps, HNS3_CAPS_STASH_B))