The macros hnae3_dev_XXX_supported just can be used in hclge layer, but
hns3_enet layer may need to use, so this serial redefine these macros.
Guangbin Huang (14):
net: hns3: modify macro hnae3_dev_fec_supported
net: hns3: modify macro hnae3_dev_udp_gso_supported
net: hns3: modify macro hnae3_dev_qb_supported
net: hns3: modify macro hnae3_dev_fd_forward_tc_supported
net: hns3: modify macro hnae3_dev_ptp_supported
net: hns3: modify macro hnae3_dev_int_ql_supported
net: hns3: modify macro hnae3_dev_hw_csum_supported
net: hns3: modify macro hnae3_dev_tx_push_supported
net: hns3: modify macro hnae3_dev_phy_imp_supported
net: hns3: modify macro hnae3_dev_ras_imp_supported
net: hns3: delete redundant macro hnae3_dev_tqp_txrx_indep_supported
net: hns3: modify macro hnae3_dev_hw_pad_supported
net: hns3: modify macro hnae3_dev_stash_supported
net: hns3: modify macro hnae3_dev_pause_supported
drivers/net/ethernet/hisilicon/hns3/hnae3.h | 55 +++++++++----------
.../hns3/hns3_common/hclge_comm_cmd.c | 2 +-
.../hns3/hns3_common/hclge_comm_cmd.h | 3 -
.../ethernet/hisilicon/hns3/hns3_debugfs.c | 2 +-
.../net/ethernet/hisilicon/hns3/hns3_enet.c | 10 ++--
.../ethernet/hisilicon/hns3/hns3_ethtool.c | 14 ++---
.../hisilicon/hns3/hns3pf/hclge_debugfs.c | 2 +-
.../hisilicon/hns3/hns3pf/hclge_main.c | 38 ++++++-------
.../hisilicon/hns3/hns3pf/hclge_ptp.c | 2 +-
.../hisilicon/hns3/hns3vf/hclgevf_main.c | 2 +-
10 files changed, 62 insertions(+), 68 deletions(-)
--
2.33.0
Redefine macro hnae3_dev_hw_pad_supported(hdev) to
hnae3_ae_dev_hw_pad_supported(ae_dev), so it can be
used in both hclge and hns3_enet layer.
Signed-off-by: Guangbin Huang <[email protected]>
---
drivers/net/ethernet/hisilicon/hns3/hnae3.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hnae3.h b/drivers/net/ethernet/hisilicon/hns3/hnae3.h
index 12730cbc3bfe..99140a7617f3 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hnae3.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hnae3.h
@@ -137,8 +137,8 @@ enum HNAE3_DEV_CAP_BITS {
#define hnae3_ae_dev_ras_imp_supported(ae_dev) \
test_bit(HNAE3_DEV_SUPPORT_RAS_IMP_B, (ae_dev)->caps)
-#define hnae3_dev_hw_pad_supported(hdev) \
- test_bit(HNAE3_DEV_SUPPORT_HW_PAD_B, (hdev)->ae_dev->caps)
+#define hnae3_ae_dev_hw_pad_supported(ae_dev) \
+ test_bit(HNAE3_DEV_SUPPORT_HW_PAD_B, (ae_dev)->caps)
#define hnae3_dev_stash_supported(hdev) \
test_bit(HNAE3_DEV_SUPPORT_STASH_B, (hdev)->ae_dev->caps)
--
2.33.0
Redefine macro hnae3_dev_ptp_supported(hdev) to
hnae3_ae_dev_ptp_supported(ae_dev), so it can be
used in both hclge and hns3_enet layer.
Signed-off-by: Guangbin Huang <[email protected]>
---
drivers/net/ethernet/hisilicon/hns3/hnae3.h | 4 ++--
drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c | 2 +-
drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_ptp.c | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hnae3.h b/drivers/net/ethernet/hisilicon/hns3/hnae3.h
index 60fda66e08bd..dd2dd085ab3d 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hnae3.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hnae3.h
@@ -119,8 +119,8 @@ enum HNAE3_DEV_CAP_BITS {
#define hnae3_ae_dev_fd_forward_tc_supported(ae_dev) \
test_bit(HNAE3_DEV_SUPPORT_FD_FORWARD_TC_B, (ae_dev)->caps)
-#define hnae3_dev_ptp_supported(hdev) \
- test_bit(HNAE3_DEV_SUPPORT_PTP_B, (hdev)->ae_dev->caps)
+#define hnae3_ae_dev_ptp_supported(ae_dev) \
+ test_bit(HNAE3_DEV_SUPPORT_PTP_B, (ae_dev)->caps)
#define hnae3_dev_int_ql_supported(hdev) \
test_bit(HNAE3_DEV_SUPPORT_INT_QL_B, (hdev)->ae_dev->caps)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c b/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c
index 66feb23f7b7b..7de1f91c4877 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c
@@ -1367,7 +1367,7 @@ int hns3_dbg_init(struct hnae3_handle *handle)
if ((hns3_dbg_cmd[i].cmd == HNAE3_DBG_CMD_TM_NODES &&
ae_dev->dev_version <= HNAE3_DEVICE_VERSION_V2) ||
(hns3_dbg_cmd[i].cmd == HNAE3_DBG_CMD_PTP_INFO &&
- !test_bit(HNAE3_DEV_SUPPORT_PTP_B, ae_dev->caps)))
+ !hnae3_ae_dev_ptp_supported(ae_dev)))
continue;
if (!hns3_dbg_cmd[i].init) {
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_ptp.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_ptp.c
index a40b1583f114..aebd98586e62 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_ptp.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_ptp.c
@@ -486,7 +486,7 @@ int hclge_ptp_init(struct hclge_dev *hdev)
struct timespec64 ts;
int ret;
- if (!test_bit(HNAE3_DEV_SUPPORT_PTP_B, ae_dev->caps))
+ if (!hnae3_ae_dev_ptp_supported(ae_dev))
return 0;
if (!hdev->ptp) {
--
2.33.0
On Sat, Sep 24, 2022 at 10:30:10AM +0800, Guangbin Huang wrote:
> The macros hnae3_dev_XXX_supported just can be used in hclge layer, but
> hns3_enet layer may need to use, so this serial redefine these macros.
IMHO, you shouldn't add new obfuscated code, but delete it.
Jakub,
The more drivers authors will obfuscate in-kernel primitives and reinvent
their own names, macros e.t.c, the less external reviewers you will be able
to attract.
IMHO, netdev should have more active position do not allow obfuscated code.
Thanks
>
> Guangbin Huang (14):
> net: hns3: modify macro hnae3_dev_fec_supported
> net: hns3: modify macro hnae3_dev_udp_gso_supported
> net: hns3: modify macro hnae3_dev_qb_supported
> net: hns3: modify macro hnae3_dev_fd_forward_tc_supported
> net: hns3: modify macro hnae3_dev_ptp_supported
> net: hns3: modify macro hnae3_dev_int_ql_supported
> net: hns3: modify macro hnae3_dev_hw_csum_supported
> net: hns3: modify macro hnae3_dev_tx_push_supported
> net: hns3: modify macro hnae3_dev_phy_imp_supported
> net: hns3: modify macro hnae3_dev_ras_imp_supported
> net: hns3: delete redundant macro hnae3_dev_tqp_txrx_indep_supported
> net: hns3: modify macro hnae3_dev_hw_pad_supported
> net: hns3: modify macro hnae3_dev_stash_supported
> net: hns3: modify macro hnae3_dev_pause_supported
>
> drivers/net/ethernet/hisilicon/hns3/hnae3.h | 55 +++++++++----------
> .../hns3/hns3_common/hclge_comm_cmd.c | 2 +-
> .../hns3/hns3_common/hclge_comm_cmd.h | 3 -
> .../ethernet/hisilicon/hns3/hns3_debugfs.c | 2 +-
> .../net/ethernet/hisilicon/hns3/hns3_enet.c | 10 ++--
> .../ethernet/hisilicon/hns3/hns3_ethtool.c | 14 ++---
> .../hisilicon/hns3/hns3pf/hclge_debugfs.c | 2 +-
> .../hisilicon/hns3/hns3pf/hclge_main.c | 38 ++++++-------
> .../hisilicon/hns3/hns3pf/hclge_ptp.c | 2 +-
> .../hisilicon/hns3/hns3vf/hclgevf_main.c | 2 +-
> 10 files changed, 62 insertions(+), 68 deletions(-)
>
> --
> 2.33.0
>
On 2022/9/24 19:27, Leon Romanovsky wrote:
> On Sat, Sep 24, 2022 at 10:30:10AM +0800, Guangbin Huang wrote:
>> The macros hnae3_dev_XXX_supported just can be used in hclge layer, but
>> hns3_enet layer may need to use, so this serial redefine these macros.
>
> IMHO, you shouldn't add new obfuscated code, but delete it.
>
> Jakub,
>
> The more drivers authors will obfuscate in-kernel primitives and reinvent
> their own names, macros e.t.c, the less external reviewers you will be able
> to attract.
>
> IMHO, netdev should have more active position do not allow obfuscated code.
>
> Thanks
>
Hi, Leon
I'm sorry, I can not get your point. Can you explain in more detail?
Do you mean the name "macro" should not be used?
On Mon, 26 Sep 2022 20:56:26 +0800 huangguangbin (A) wrote:
> On 2022/9/24 19:27, Leon Romanovsky wrote:
> > On Sat, Sep 24, 2022 at 10:30:10AM +0800, Guangbin Huang wrote:
> >> The macros hnae3_dev_XXX_supported just can be used in hclge layer, but
> >> hns3_enet layer may need to use, so this serial redefine these macros.
> >
> > IMHO, you shouldn't add new obfuscated code, but delete it.
> >
> > Jakub,
> >
> > The more drivers authors will obfuscate in-kernel primitives and reinvent
> > their own names, macros e.t.c, the less external reviewers you will be able
> > to attract.
> >
> > IMHO, netdev should have more active position do not allow obfuscated code.
> >
> > Thanks
> >
>
> Hi, Leon
> I'm sorry, I can not get your point. Can you explain in more detail?
> Do you mean the name "macro" should not be used?
He is saying that you should try to remove those macros rather than
touch them up. The macros may seem obvious to people working on the
driver but to upstream reviewers any local conventions obfuscate the
code and require looking up definitions.
For example the first patch is better off as:
diff --git a/drivers/net/ethernet/hisilicon/hns3/hnae3.h b/drivers/net/ethernet/hisilicon/hns3/hnae3.h
index 0179fc288f5f..449d496b824b 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hnae3.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hnae3.h
@@ -107,9 +107,6 @@ enum HNAE3_DEV_CAP_BITS {
#define hnae3_ae_dev_gro_supported(ae_dev) \
test_bit(HNAE3_DEV_SUPPORT_GRO_B, (ae_dev)->caps)
-#define hnae3_dev_fec_supported(hdev) \
- test_bit(HNAE3_DEV_SUPPORT_FEC_B, (hdev)->ae_dev->caps)
-
#define hnae3_dev_udp_gso_supported(hdev) \
test_bit(HNAE3_DEV_SUPPORT_UDP_GSO_B, (hdev)->ae_dev->caps)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index 6962a9d69cf8..ded92f7dbd79 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -1179,7 +1179,7 @@ static void hclge_parse_fiber_link_mode(struct hclge_dev *hdev,
hclge_convert_setting_sr(speed_ability, mac->supported);
hclge_convert_setting_lr(speed_ability, mac->supported);
hclge_convert_setting_cr(speed_ability, mac->supported);
- if (hnae3_dev_fec_supported(hdev))
+ if (test_bit(HNAE3_DEV_SUPPORT_FEC_B, hdev->caps))
hclge_convert_setting_fec(mac);
if (hnae3_dev_pause_supported(hdev))
@@ -1195,7 +1195,7 @@ static void hclge_parse_backplane_link_mode(struct hclge_dev *hdev,
struct hclge_mac *mac = &hdev->hw.mac;
hclge_convert_setting_kr(speed_ability, mac->supported);
- if (hnae3_dev_fec_supported(hdev))
+ if (test_bit(HNAE3_DEV_SUPPORT_FEC_B, hdev->caps))
hclge_convert_setting_fec(mac);
if (hnae3_dev_pause_supported(hdev))
@@ -3232,7 +3232,7 @@ static void hclge_update_advertising(struct hclge_dev *hdev)
static void hclge_update_port_capability(struct hclge_dev *hdev,
struct hclge_mac *mac)
{
- if (hnae3_dev_fec_supported(hdev))
+ if (test_bit(HNAE3_DEV_SUPPORT_FEC_B, hdev->caps))
hclge_convert_setting_fec(mac);
/* firmware can not identify back plane type, the media type
On 2022/9/27 1:11, Jakub Kicinski wrote:
> On Mon, 26 Sep 2022 20:56:26 +0800 huangguangbin (A) wrote:
>> On 2022/9/24 19:27, Leon Romanovsky wrote:
>>> On Sat, Sep 24, 2022 at 10:30:10AM +0800, Guangbin Huang wrote:
>>>> The macros hnae3_dev_XXX_supported just can be used in hclge layer, but
>>>> hns3_enet layer may need to use, so this serial redefine these macros.
>>>
>>> IMHO, you shouldn't add new obfuscated code, but delete it.
>>>
>>> Jakub,
>>>
>>> The more drivers authors will obfuscate in-kernel primitives and reinvent
>>> their own names, macros e.t.c, the less external reviewers you will be able
>>> to attract.
>>>
>>> IMHO, netdev should have more active position do not allow obfuscated code.
>>>
>>> Thanks
>>>
>>
>> Hi, Leon
>> I'm sorry, I can not get your point. Can you explain in more detail?
>> Do you mean the name "macro" should not be used?
>
> He is saying that you should try to remove those macros rather than
> touch them up. The macros may seem obvious to people working on the
> driver but to upstream reviewers any local conventions obfuscate the
> code and require looking up definitions.
>
> For example the first patch is better off as:
>
> diff --git a/drivers/net/ethernet/hisilicon/hns3/hnae3.h b/drivers/net/ethernet/hisilicon/hns3/hnae3.h
> index 0179fc288f5f..449d496b824b 100644
> --- a/drivers/net/ethernet/hisilicon/hns3/hnae3.h
> +++ b/drivers/net/ethernet/hisilicon/hns3/hnae3.h
> @@ -107,9 +107,6 @@ enum HNAE3_DEV_CAP_BITS {
> #define hnae3_ae_dev_gro_supported(ae_dev) \
> test_bit(HNAE3_DEV_SUPPORT_GRO_B, (ae_dev)->caps)
>
> -#define hnae3_dev_fec_supported(hdev) \
> - test_bit(HNAE3_DEV_SUPPORT_FEC_B, (hdev)->ae_dev->caps)
> -
> #define hnae3_dev_udp_gso_supported(hdev) \
> test_bit(HNAE3_DEV_SUPPORT_UDP_GSO_B, (hdev)->ae_dev->caps)
>
> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
> index 6962a9d69cf8..ded92f7dbd79 100644
> --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
> @@ -1179,7 +1179,7 @@ static void hclge_parse_fiber_link_mode(struct hclge_dev *hdev,
> hclge_convert_setting_sr(speed_ability, mac->supported);
> hclge_convert_setting_lr(speed_ability, mac->supported);
> hclge_convert_setting_cr(speed_ability, mac->supported);
> - if (hnae3_dev_fec_supported(hdev))
> + if (test_bit(HNAE3_DEV_SUPPORT_FEC_B, hdev->caps))
> hclge_convert_setting_fec(mac);
>
> if (hnae3_dev_pause_supported(hdev))
> @@ -1195,7 +1195,7 @@ static void hclge_parse_backplane_link_mode(struct hclge_dev *hdev,
> struct hclge_mac *mac = &hdev->hw.mac;
>
> hclge_convert_setting_kr(speed_ability, mac->supported);
> - if (hnae3_dev_fec_supported(hdev))
> + if (test_bit(HNAE3_DEV_SUPPORT_FEC_B, hdev->caps))
> hclge_convert_setting_fec(mac);
>
> if (hnae3_dev_pause_supported(hdev))
> @@ -3232,7 +3232,7 @@ static void hclge_update_advertising(struct hclge_dev *hdev)
> static void hclge_update_port_capability(struct hclge_dev *hdev,
> struct hclge_mac *mac)
> {
> - if (hnae3_dev_fec_supported(hdev))
> + if (test_bit(HNAE3_DEV_SUPPORT_FEC_B, hdev->caps))
> hclge_convert_setting_fec(mac);
>
> /* firmware can not identify back plane type, the media type
> .
>
Ok, I see, thanks!
On Mon, Sep 26, 2022 at 10:11:35AM -0700, Jakub Kicinski wrote:
> On Mon, 26 Sep 2022 20:56:26 +0800 huangguangbin (A) wrote:
> > On 2022/9/24 19:27, Leon Romanovsky wrote:
> > > On Sat, Sep 24, 2022 at 10:30:10AM +0800, Guangbin Huang wrote:
> > >> The macros hnae3_dev_XXX_supported just can be used in hclge layer, but
> > >> hns3_enet layer may need to use, so this serial redefine these macros.
> > >
> > > IMHO, you shouldn't add new obfuscated code, but delete it.
> > >
> > > Jakub,
> > >
> > > The more drivers authors will obfuscate in-kernel primitives and reinvent
> > > their own names, macros e.t.c, the less external reviewers you will be able
> > > to attract.
> > >
> > > IMHO, netdev should have more active position do not allow obfuscated code.
> > >
> > > Thanks
> > >
> >
> > Hi, Leon
> > I'm sorry, I can not get your point. Can you explain in more detail?
> > Do you mean the name "macro" should not be used?
>
> He is saying that you should try to remove those macros rather than
> touch them up.
Exactly, thanks Jakub.