All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v15 1/3] can: etas_es58x: add core support for ETAS ES58X CAN USB interfaces
@ 2021-04-10 13:50 kernel test robot
  0 siblings, 0 replies; 2+ messages in thread
From: kernel test robot @ 2021-04-10 13:50 UTC (permalink / raw)
  To: kbuild

[-- Attachment #1: Type: text/plain, Size: 5229 bytes --]

CC: kbuild-all(a)lists.01.org
In-Reply-To: <20210410095948.233305-2-mailhol.vincent@wanadoo.fr>
References: <20210410095948.233305-2-mailhol.vincent@wanadoo.fr>
TO: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
TO: "Marc Kleine-Budde" <mkl@pengutronix.de>
TO: linux-can(a)vger.kernel.org
CC: Jimmy Assarsson <extja@kvaser.com>
CC: Masahiro Yamada <masahiroy@kernel.org>
CC: linux-kernel(a)vger.kernel.org
CC: netdev(a)vger.kernel.org
CC: Arunachalam Santhanam <arunachalam.santhanam@in.bosch.com>
CC: Vincent Mailhol <mailhol.vincent@wanadoo.fr>

Hi Vincent,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on mkl-can-next/testing]
[also build test WARNING on next-20210409]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Vincent-Mailhol/Introducing-ETAS-ES58X-CAN-USB-interfaces/20210410-180245
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git testing
:::::: branch date: 4 hours ago
:::::: commit date: 4 hours ago
config: x86_64-allyesconfig (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Julia Lawall <julia.lawall@lip6.fr>


cocci warnings: (new ones prefixed by >>)
>> drivers/net/can/usb/etas_es58x/es58x_core.c:1802:5-24: atomic_dec_and_test variation before object free at line 1803.
   drivers/net/can/usb/etas_es58x/es58x_core.c:1837:5-24: atomic_dec_and_test variation before object free at line 1838.

vim +1802 drivers/net/can/usb/etas_es58x/es58x_core.c

1eb17b66e055c7 Vincent Mailhol 2021-04-10  1764  
1eb17b66e055c7 Vincent Mailhol 2021-04-10  1765  /**
1eb17b66e055c7 Vincent Mailhol 2021-04-10  1766   * es58x_open() - Enable the network device.
1eb17b66e055c7 Vincent Mailhol 2021-04-10  1767   * @netdev: CAN network device.
1eb17b66e055c7 Vincent Mailhol 2021-04-10  1768   *
1eb17b66e055c7 Vincent Mailhol 2021-04-10  1769   * Called when the network transitions to the up state. Allocate the
1eb17b66e055c7 Vincent Mailhol 2021-04-10  1770   * URB resources if needed and open the channel.
1eb17b66e055c7 Vincent Mailhol 2021-04-10  1771   *
1eb17b66e055c7 Vincent Mailhol 2021-04-10  1772   * Return: zero on success, errno when any error occurs.
1eb17b66e055c7 Vincent Mailhol 2021-04-10  1773   */
1eb17b66e055c7 Vincent Mailhol 2021-04-10  1774  static int es58x_open(struct net_device *netdev)
1eb17b66e055c7 Vincent Mailhol 2021-04-10  1775  {
1eb17b66e055c7 Vincent Mailhol 2021-04-10  1776  	struct es58x_device *es58x_dev = es58x_priv(netdev)->es58x_dev;
1eb17b66e055c7 Vincent Mailhol 2021-04-10  1777  	int ret;
1eb17b66e055c7 Vincent Mailhol 2021-04-10  1778  
1eb17b66e055c7 Vincent Mailhol 2021-04-10  1779  	if (atomic_inc_return(&es58x_dev->opened_channel_cnt) == 1) {
1eb17b66e055c7 Vincent Mailhol 2021-04-10  1780  		ret = es58x_alloc_rx_urbs(es58x_dev);
1eb17b66e055c7 Vincent Mailhol 2021-04-10  1781  		if (ret)
1eb17b66e055c7 Vincent Mailhol 2021-04-10  1782  			return ret;
1eb17b66e055c7 Vincent Mailhol 2021-04-10  1783  
1eb17b66e055c7 Vincent Mailhol 2021-04-10  1784  		ret = es58x_set_realtime_diff_ns(es58x_dev);
1eb17b66e055c7 Vincent Mailhol 2021-04-10  1785  		if (ret)
1eb17b66e055c7 Vincent Mailhol 2021-04-10  1786  			goto free_urbs;
1eb17b66e055c7 Vincent Mailhol 2021-04-10  1787  	}
1eb17b66e055c7 Vincent Mailhol 2021-04-10  1788  
1eb17b66e055c7 Vincent Mailhol 2021-04-10  1789  	ret = open_candev(netdev);
1eb17b66e055c7 Vincent Mailhol 2021-04-10  1790  	if (ret)
1eb17b66e055c7 Vincent Mailhol 2021-04-10  1791  		goto free_urbs;
1eb17b66e055c7 Vincent Mailhol 2021-04-10  1792  
1eb17b66e055c7 Vincent Mailhol 2021-04-10  1793  	ret = es58x_dev->ops->enable_channel(es58x_priv(netdev));
1eb17b66e055c7 Vincent Mailhol 2021-04-10  1794  	if (ret)
1eb17b66e055c7 Vincent Mailhol 2021-04-10  1795  		goto free_urbs;
1eb17b66e055c7 Vincent Mailhol 2021-04-10  1796  
1eb17b66e055c7 Vincent Mailhol 2021-04-10  1797  	netif_start_queue(netdev);
1eb17b66e055c7 Vincent Mailhol 2021-04-10  1798  
1eb17b66e055c7 Vincent Mailhol 2021-04-10  1799  	return ret;
1eb17b66e055c7 Vincent Mailhol 2021-04-10  1800  
1eb17b66e055c7 Vincent Mailhol 2021-04-10  1801   free_urbs:
1eb17b66e055c7 Vincent Mailhol 2021-04-10 @1802  	if (atomic_dec_and_test(&es58x_dev->opened_channel_cnt))
1eb17b66e055c7 Vincent Mailhol 2021-04-10 @1803  		es58x_free_urbs(es58x_dev);
1eb17b66e055c7 Vincent Mailhol 2021-04-10  1804  	netdev_err(netdev, "%s: Could not open the network device: %pe\n",
1eb17b66e055c7 Vincent Mailhol 2021-04-10  1805  		   __func__, ERR_PTR(ret));
1eb17b66e055c7 Vincent Mailhol 2021-04-10  1806  
1eb17b66e055c7 Vincent Mailhol 2021-04-10  1807  	return ret;
1eb17b66e055c7 Vincent Mailhol 2021-04-10  1808  }
1eb17b66e055c7 Vincent Mailhol 2021-04-10  1809  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 65474 bytes --]

^ permalink raw reply	[flat|nested] 2+ messages in thread
* [PATCH v15 0/3] Introducing ETAS ES58X CAN USB interfaces
@ 2021-04-10  9:59 Vincent Mailhol
  2021-04-10  9:59 ` [PATCH v15 1/3] can: etas_es58x: add core support for " Vincent Mailhol
  0 siblings, 1 reply; 2+ messages in thread
From: Vincent Mailhol @ 2021-04-10  9:59 UTC (permalink / raw)
  To: Marc Kleine-Budde, linux-can
  Cc: Jimmy Assarsson, Masahiro Yamada, linux-kernel, netdev,
	Arunachalam Santhanam, Vincent Mailhol

Here comes the 15th iteration of the patch. This new version addresses
the comments received from Marc (thanks again for the review!) and
simplify the device probing by using .driver_info.

** Changelog **

Changes in v15 (2021-04-10):
  - Use of .driver_info to keep track of each device quirks (c.f. enum
    es58x_driver_info).
  - Replace es58x_netdev_queue_set_dql_min_limit() by
    netdev_queue_set_dql_min_limit() which was recently added in
    net-next.
  - es58x_start_xmit: remove the recursive call.
  - es58x_start_xmit: remove the memset zero of the urb
    transfer_buffer in es58x_xmit(). Adjust the
    es58{1_4,x_fd}_tx_can_msg() glue code accordingly to make sure
    that all relevant fields are correctly initialised.
  - es58x_start_xmit: directly update tx_head each time a packet is
    queued in can.echo_skb[].
  - Remove es58x_set_bittiming() and es58x_set_data_bittiming(). The
    bittiming is now set when opening the channel.
  - Shutdown the channel when a bus off event occurs. Remove the
    es58x_reset() function which is not needed anymore after this
    change.
  - Cleanup of the {net,}dev_dbg() calls to make the driver less
    verbose.
  - Modify es58x_open(), es58x_close() and es58x_probe() in order to
    release driver resources as much as possible when all the network
    interfaces of the device are down.
  - Other trivial changes (c.f. below link for details)
Reference: https://lore.kernel.org/linux-can/20210321104103.213308-1-mailhol.vincent@wanadoo.fr/T/#m8ca804e9f53584b79acd236602403d5b82db1e6c

Changes in v14 (2021-03-21):
  - Rework the split into core support, es581_4, es58x_fd (cosmetic
    change) so that es581_4.h and es58x_fd.h headers can be added in
    one block.
  - Add a fourth patch to introduce a helper function
    es58x_netdev_queue_set_dql_min_limit() to set up dql minimum limit
    (in parallel, I will try to have this merged in the network device
    header)
  - Remove unused function es58x_add_skb_idx(): leftover from the old
    FIFO logic, should have been removed in v11.
  - Fix memory leak in es58x_get_product_info(): buffer was not freed
    in case of error in usb_sting().
  - s/loopback/echo and s/self reception/echo: in the driver the terms
    "loopback, self reception, and echo" were all used to designated
    the same thing. Renamed structures, variables and comments
    accordingly to make it more consistent.
  - Remove CAN_CTRLMODE_LOOPBACK. The driver never supported this
    feature. It was added due to a confusion with the echo skb.
  - Use the new can_free_echo_skb() which return the frame_len.
  - Do the statistics handling in es58x_rx_err_msg() even if kalloc()
    fails.
  - Replace array es58x_cmd_ret_desc[] by helper function
    es58x_cmd_ret_desc().
  - Other trivial changes (c.f. below link for details)
Reference: https://lore.kernel.org/linux-can/50850e5f-87c6-505e-4398-babce3facb97@pengutronix.de/T/#mbcace9c13b19a504cd28d81591f983b95eb66657

Changes in v13 (2021-03-19 by Marc Kleine-Budde):
  - split the driver into 3 patches, so that it can be send via
    mailing lists (core support, es581_4, es58x_fd)
  - Remove the dql.min_limit settings
  - typo and kernel doc fixes
Reference: https://lore.kernel.org/linux-can/50850e5f-87c6-505e-4398-babce3facb97@pengutronix.de/T/#t

Changes in v12 (2021-03-09):
  - Rework the queue stop/wake management so that spinlocks are not
    needed anymore.
  - es58x_start_xmit(): check for valid SKB using
    can_dropped_invalid_skb().
  - Implemented TDC according to latest patches in linux-can-next.
Reference: https://lore.kernel.org/linux-can/20210224002008.4158-1-mailhol.vincent@wanadoo.fr/T/#t

Changes in v11 (2021-01-23):
  - Remove all WARN_ON() calls: these were use during development,
    relevant tests are done not to trigger these.
  - es58x_start_xmit(): added net_ratelimit() condition to prevent
    spamming dmesg.
  - add a new es58x_xmit_more() function and simplify the code of
    es58x_start_xmit().
  - Removed functions {es581_4,es58x_fd}_print_conf() which were only
    there for debug.
  - Additional comment for es58x_fd_param.bitrate_max.
  - Make the device FIFO size a power of two and modify the echo_skb
    indexes logic to prevent the use of spinlocks.

Changes in v10 (2021-01-12):
  - Rebased on linux-can-next/testing and modified according to latest
    BQL patches.
Reference: https://lore.kernel.org/linux-can/20210111141930.693847-1-mkl@pengutronix.de/T/#m5f99d4da8e8934a75f9481ecc3137b59f3762413
  - Replaced __netdev_sent_queue() by netdev_sent_queue().

Changes in v9 (2021-01-09):
  - es58x_start_xmit(): do not use skb anymore after the call of
    can_put_echo_skb(). Rationale: can_put_echo_skb() calls
    skb_clone() and thus the original skb gets consumed (i.e. use
    after free issue).
  - es58x_start_xmit(): Add a "drop_skb" label to free the skb when
    errors occur.

Changes in v8 (2021-01-04):
  - The driver requires CRC16. Modified Kconfig accordingly.

Changes in v7 (2020-11-17):
  - Fix compilation issue if CONFIG_BQL is not set.
Reference: https://lkml.org/lkml/2020/11/15/163

Changes in v6 (2020-11-15):
  - Rebase the patch on the testing branch of linux-can-next.
  - Rename the helper functions according latest changes
    (e.g. can_cc_get_len() -> can_cc_dlc2len())
  - Fix comments of enum es58x_physical_layer and enum
    es58x_sync_edge.

Changes in v5 (2020-11-07):
  - Add support for DLC greater than 8.
  - All other patches from the previous series were either accepted or
    dismissed. As such, this is not a series any more but a single
    patch.

Changes in v4 (2020-10-17):
  - Remove struct es58x_abstracted_can_frame.
  - Fix formatting (spaces, comment style).
  - Transform macros into static inline functions when possible.
  - Fix the ctrlmode_supported flags in es581_4.c and removed
    misleading comments in enum es58x_samples_per_bit.
  - Rename enums according to the type.
  - Remove function es58x_can_put_echo_skb().
Reference: https://lkml.org/lkml/2020/10/10/53

Changes in v3 (2020-10-03):
  - Remove all the calls to likely() and unlikely().
Reference: https://lkml.org/lkml/2020/9/30/995

Changes in v2 (2020-09-30):
  - Fixed -W1 warnings (v1 was tested with GCC -WExtra but not with -W1).

v1 (2020-09-27):
  - First release

Vincent Mailhol (3):
  can: etas_es58x: add core support for ETAS ES58X CAN USB interfaces
  can: etas_es58x: add support for ETAS ES581.4 CAN USB interface
  can: etas_es58x: add support for the ETAS ES58X_FD CAN USB interfaces

 drivers/net/can/usb/Kconfig                 |   10 +
 drivers/net/can/usb/Makefile                |    1 +
 drivers/net/can/usb/etas_es58x/Makefile     |    3 +
 drivers/net/can/usb/etas_es58x/es581_4.c    |  507 ++++
 drivers/net/can/usb/etas_es58x/es581_4.h    |  207 ++
 drivers/net/can/usb/etas_es58x/es58x_core.c | 2301 +++++++++++++++++++
 drivers/net/can/usb/etas_es58x/es58x_core.h |  700 ++++++
 drivers/net/can/usb/etas_es58x/es58x_fd.c   |  562 +++++
 drivers/net/can/usb/etas_es58x/es58x_fd.h   |  243 ++
 9 files changed, 4534 insertions(+)
 create mode 100644 drivers/net/can/usb/etas_es58x/Makefile
 create mode 100644 drivers/net/can/usb/etas_es58x/es581_4.c
 create mode 100644 drivers/net/can/usb/etas_es58x/es581_4.h
 create mode 100644 drivers/net/can/usb/etas_es58x/es58x_core.c
 create mode 100644 drivers/net/can/usb/etas_es58x/es58x_core.h
 create mode 100644 drivers/net/can/usb/etas_es58x/es58x_fd.c
 create mode 100644 drivers/net/can/usb/etas_es58x/es58x_fd.h

-- 
2.26.3


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2021-04-10 13:50 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-10 13:50 [PATCH v15 1/3] can: etas_es58x: add core support for ETAS ES58X CAN USB interfaces kernel test robot
  -- strict thread matches above, loose matches on Subject: below --
2021-04-10  9:59 [PATCH v15 0/3] Introducing " Vincent Mailhol
2021-04-10  9:59 ` [PATCH v15 1/3] can: etas_es58x: add core support for " Vincent Mailhol

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.