On 10/10/20 10:49 PM, Oliver Hartkopp wrote: > As pointed out by Jakub Kicinski here: > https://marc.info/?l=linux-can&m=160229286216008 > this patch addresses the remarked issues: > > - remove empty lines in comment > - remove default=y for CAN_ISOTP in Kconfig > - make use of pr_notice_once() > - use GFP_KERNEL instead of gfp_any() in soft hrtimer context > > The version strings in the CAN subsystem are removed by a separate patch. > > Signed-off-by: Oliver Hartkopp <socket...@hartkopp.net> > --- > include/uapi/linux/can/isotp.h | 4 +--- > net/can/Kconfig | 3 ++- > net/can/isotp.c | 14 +++++++------- > 3 files changed, 10 insertions(+), 11 deletions(-) > > diff --git a/include/uapi/linux/can/isotp.h b/include/uapi/linux/can/isotp.h > index 553006509f4e..accf0efa46f4 100644 > --- a/include/uapi/linux/can/isotp.h > +++ b/include/uapi/linux/can/isotp.h > @@ -151,8 +151,7 @@ struct can_isotp_ll_options { > #define CAN_ISOTP_DEFAULT_LL_TX_DL CAN_MAX_DLEN > #define CAN_ISOTP_DEFAULT_LL_TX_FLAGS 0 > > -/* > - * Remark on CAN_ISOTP_DEFAULT_RECV_* values: > +/* Remark on CAN_ISOTP_DEFAULT_RECV_* values:
I think for uapi headers we use the default commenting style, not the netdev
one.
> *
> * We can strongly assume, that the Linux Kernel implementation of
> * CAN_ISOTP is capable to run with BS=0, STmin=0 and WFTmax=0.
> @@ -160,7 +159,6 @@ struct can_isotp_ll_options {
> * these default settings can be changed via sockopts.
> * For that reason the STmin value is intentionally _not_ checked for
> * consistency and copied directly into the flow control (FC) frame.
> - *
> */
>
> #endif /* !_UAPI_CAN_ISOTP_H */
> diff --git a/net/can/Kconfig b/net/can/Kconfig
> index 021fe03a8ed6..224e5e0283a9 100644
> --- a/net/can/Kconfig
> +++ b/net/can/Kconfig
> @@ -57,7 +57,6 @@ source "net/can/j1939/Kconfig"
>
> config CAN_ISOTP
> tristate "ISO 15765-2:2016 CAN transport protocol"
> - default y
> help
> CAN Transport Protocols offer support for segmented Point-to-Point
> communication between CAN nodes via two defined CAN Identifiers.
> @@ -67,6 +66,8 @@ config CAN_ISOTP
> vehicle diagnosis (UDS, ISO 14229) or IP-over-CAN traffic.
> This protocol driver implements data transfers according to
> ISO 15765-2:2016 for 'classic' CAN and CAN FD frame types.
> + If you want to perform automotive vehicle diagnostic services (UDS),
> + say 'y'.
>
> source "drivers/net/can/Kconfig"
>
> diff --git a/net/can/isotp.c b/net/can/isotp.c
> index e6ff032b5426..bc3a722c200b 100644
> --- a/net/can/isotp.c
> +++ b/net/can/isotp.c
> @@ -222,8 +222,8 @@ static int isotp_send_fc(struct sock *sk, int ae, u8
> flowstatus)
>
> can_send_ret = can_send(nskb, 1);
> if (can_send_ret)
> - printk_once(KERN_NOTICE "can-isotp: %s: can_send_ret %d\n",
> - __func__, can_send_ret);
> + pr_notice_once("can-isotp: %s: can_send_ret %d\n",
> + __func__, can_send_ret);
please define a pr_fmt for the "can-isotp: " prefix.
>
> dev_put(dev);
>
> @@ -769,7 +769,7 @@ static enum hrtimer_restart isotp_tx_timer_handler(struct
> hrtimer *hrtimer)
>
> isotp_tx_burst:
> skb = alloc_skb(so->ll.mtu + sizeof(struct can_skb_priv),
> - gfp_any());
> + GFP_KERNEL);
> if (!skb) {
> dev_put(dev);
> break;
> @@ -798,8 +798,8 @@ static enum hrtimer_restart isotp_tx_timer_handler(struct
> hrtimer *hrtimer)
>
> can_send_ret = can_send(skb, 1);
> if (can_send_ret)
> - printk_once(KERN_NOTICE "can-isotp: %s: can_send_ret
> %d\n",
> - __func__, can_send_ret);
> + pr_notice_once("can-isotp: %s: can_send_ret %d\n",
> + __func__, can_send_ret);
>
> if (so->tx.idx >= so->tx.len) {
> /* we are done */
> @@ -942,8 +942,8 @@ static int isotp_sendmsg(struct socket *sock, struct
> msghdr *msg, size_t size)
> err = can_send(skb, 1);
> dev_put(dev);
> if (err) {
> - printk_once(KERN_NOTICE "can-isotp: %s: can_send_ret %d\n",
> - __func__, err);
> + pr_notice_once("can-isotp: %s: can_send_ret %d\n",
> + __func__, err);
> return err;
> }
>
>
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung West/Dortmund | Phone: +49-231-2826-924 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
signature.asc
Description: OpenPGP digital signature
