On 10/10/2020 10:45 AM, Jiawen Wu wrote:
On 10/9/2020 5:47 PM, Ferruh Yigit wrote:
On 10/9/2020 4:03 AM, [email protected] wrote:
Hi Ferruh,

For the syntax/style check issue, should I fix all the errors and warnings or
just fix the errors?
It seems to be a lot of warnings.


[Please don't top post, it makes archives un-readable]

Please fix all, but beware that there may be false positive in the checkpatch
warnings, so you need to process the output first.
This is a new PMD, if the syntax is not put correct at first place, very 
unlikely
that it will be fixed later, so lets try to fix them as much as possible.

For some drivers, the base code is shared in multiple platforms, like Linux,
FreeBSD, Windows etc..., for them we are more flexible and we allow to keep
the original syntax of that shared code, *as long as it is consistent within 
itself*.
Do you have similar case in the base folder files?

The code for the DPDK should follow the DPDK coding convention [1] and should
have as less checkpatch warnings/errors as possible.

[1] https://doc.dpdk.org/guides/contributing/coding_style.html

Thanks,
ferruh



There are some 'checks' show that macro argument reused will possible take 
side-effects.
Like this:

CHECK:MACRO_ARG_REUSE: Macro argument reuse 'y' - possible side-effects?
#56: FILE: drivers/net/txgbe/base/txgbe_regs.h:35:
+#define ROUND_UP(x, y)          (((x) + (y) - 1) / (y) * (y))

But the example given in the DPDK coding convention is:

#define MACRO(x, y) do {      \
                variable = (x) + (y);   \
                (y) += 2;            \
} while(0)

It seems to reuse argument, too.
Should I fix this 'check', or treat it as a false positive?


That is a check, please double check if the usage is safe, and if it is you can ignore it.



-----Original Message-----
From: Ferruh Yigit <[email protected]>
Sent: Tuesday, October 6, 2020 7:03 PM
To: Jiawen Wu <[email protected]>; [email protected]
Subject: Re: [dpdk-dev] [PATCH v2 00/56] net: txgbe PMD

On 10/5/2020 1:08 PM, Jiawen Wu wrote:
v2: re-order patches and fix some known problems
v1: introduce txgbe PMD

jiawenwu (56):
     net/txgbe: add build and doc infrastructure
     net/txgbe: add ethdev probe and remove
     net/txgbe: add device init and uninit
     net/txgbe: add error types and registers
     net/txgbe: add mac type and bus lan id
     net/txgbe: add HW infrastructure and dummy function
     net/txgbe: add EEPROM functions
     net/txgbe: add HW init and reset operation
     net/txgbe: add PHY init
     net/txgbe: add module identify
     net/txgbe: add PHY reset
     net/txgbe: add info get operation
     net/txgbe: add interrupt operation
     net/txgbe: add device configure operation
     net/txgbe: add link status change
     net/txgbe: add multi-speed link setup
     net/txgbe: add autoc read and write
     net/txgbe: add MAC address operations
     net/txgbe: add unicast hash bitmap
     net/txgbe: add RX and TX init
     net/txgbe: add RX and TX queues setup and release
     net/txgbe: add RX and TX start and stop
     net/txgbe: add packet type
     net/txgbe: fill simple transmit function
     net/txgbe: fill transmit function with hardware offload
     net/txgbe: fill TX prepare funtion
     net/txgbe: fill receive functions
     net/txgbe: add device start operation
     net/txgbe: add RX and TX data path start and stop
     net/txgbe: add device stop and close operations
     net/txgbe: support RX interrupt
     net/txgbe: add RX and TX queue info get
     net/txgbe: add device stats get
     net/txgbe: add device xstats get
     net/txgbe: add queue stats mapping
     net/txgbe: add VLAN handle support
     net/txgbe: add SWFW semaphore and lock
     net/txgbe: add PF module init and uninit for SRIOV
     net/txgbe: add process mailbox operation
     net/txgbe: add PF module configure for SRIOV
     net/txgbe: add VMDq configure
     net/txgbe: add RSS support
     net/txgbe: add DCB support
     net/txgbe: add flow control support
     net/txgbe: add FC auto negotiation support
     net/txgbe: add priority flow control support
     net/txgbe: add device promiscuous and allmulticast mode
     net/txgbe: add MTU set operation
     net/txgbe: add FW version get operation
     net/txgbe: add EEPROM info get operation
     net/txgbe: add register dump support
     net/txgbe: support device LED on and off
     net/txgbe: add mirror rule operations
     net/txgbe: add PTP support
     net/txgbe: add DCB info get operation
     net/txgbe: add Rx and Tx descriptor status


Hi Jiawen,

Before going into more detailed reviews, the patchset conflicts with some
recent changes in the main repo [1], can you please rebase on top of the latest
head of the repo?

Also DPDK syntax/style check scripts are giving errors, can you please fix
them too? You should run following to check:
./devtools/checkpatches.sh -n56
./devtools/check-git-log.sh -n56
(This one needs codespell package to show spelling errors)



[1] mainly the list is:
1) PMD close behavior change,
      - .dev_close changes
      - RTE_ETH_DEV_CLOSE_REMOVE flag removed

2) Some dev_ops moved to ethdev struct
      - .rx_queue_count
      - .rx_descriptor_done
      - .rx_descriptor_status
      - .tx_descriptor_status









Reply via email to