diff mbox series

[net-next,v3,1/2] can-isotp: implement cleanups / improvements from review

Message ID 20201012074354.25839-1-socketcan@hartkopp.net
State Awaiting Upstream
Delegated to: David Miller
Headers show
Series [net-next,v3,1/2] can-isotp: implement cleanups / improvements from review | expand

Commit Message

Oliver Hartkopp Oct. 12, 2020, 7:43 a.m. UTC
As pointed out by Jakub Kicinski here:
http://lore.kernel.org/r/20201009175751.5c54097f@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com
this patch addresses the remarked issues:

- remove empty line in comment
- remove default=y for CAN_ISOTP in Kconfig
- make use of pr_notice_once()
- use GFP_ATOMIC 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 <socketcan@hartkopp.net>
---
 include/uapi/linux/can/isotp.h |  1 -
 net/can/Kconfig                |  3 ++-
 net/can/isotp.c                | 14 +++++++-------
 3 files changed, 9 insertions(+), 9 deletions(-)

Comments

Marc Kleine-Budde Oct. 12, 2020, 8:05 a.m. UTC | #1
On 10/12/20 9:43 AM, Oliver Hartkopp wrote:
> As pointed out by Jakub Kicinski here:
> http://lore.kernel.org/r/20201009175751.5c54097f@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com
> this patch addresses the remarked issues:
> 
> - remove empty line in comment
> - remove default=y for CAN_ISOTP in Kconfig
> - make use of pr_notice_once()
> - use GFP_ATOMIC 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 <socketcan@hartkopp.net>
> ---

Added both to linux-can-next.

tnx,
Marc
Oliver Hartkopp Oct. 12, 2020, 8:11 a.m. UTC | #2
On 12.10.20 10:05, Marc Kleine-Budde wrote:
> On 10/12/20 9:43 AM, Oliver Hartkopp wrote:
>> As pointed out by Jakub Kicinski here:
>> http://lore.kernel.org/r/20201009175751.5c54097f@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com
>> this patch addresses the remarked issues:
>>
>> - remove empty line in comment
>> - remove default=y for CAN_ISOTP in Kconfig
>> - make use of pr_notice_once()
>> - use GFP_ATOMIC 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 <socketcan@hartkopp.net>
>> ---
> 
> Added both to linux-can-next.

The patches were intended for 'net-next' as the merge window will open 
*very* soon.

I'm not sure if there would be another pull from linux-can-next until 
then, therefore I would suggest that Jakub takes the patches himself.

Thanks,
Oliver
Marc Kleine-Budde Oct. 12, 2020, 8:30 a.m. UTC | #3
On 10/12/20 10:11 AM, Oliver Hartkopp wrote:
> 
> 
> On 12.10.20 10:05, Marc Kleine-Budde wrote:
>> On 10/12/20 9:43 AM, Oliver Hartkopp wrote:
>>> As pointed out by Jakub Kicinski here:
>>> http://lore.kernel.org/r/20201009175751.5c54097f@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com
>>> this patch addresses the remarked issues:
>>>
>>> - remove empty line in comment
>>> - remove default=y for CAN_ISOTP in Kconfig
>>> - make use of pr_notice_once()
>>> - use GFP_ATOMIC 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 <socketcan@hartkopp.net>
>>> ---
>>
>> Added both to linux-can-next.
> 
> The patches were intended for 'net-next' as the merge window will open 
> *very* soon.
> 
> I'm not sure if there would be another pull from linux-can-next until 
> then, therefore I would suggest that Jakub takes the patches himself.

I don't mind. FWIW, here's the pull request:

    http://lore.kernel.org/r/20201012082727.2338859-1-mkl@pengutronix.de

    git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git tags/linux-can-next-for-5.10-20201012

Marc
Oliver Hartkopp Oct. 12, 2020, 8:35 a.m. UTC | #4
On 12.10.20 10:30, Marc Kleine-Budde wrote:
> On 10/12/20 10:11 AM, Oliver Hartkopp wrote:
>>
>>
>> On 12.10.20 10:05, Marc Kleine-Budde wrote:
>>> On 10/12/20 9:43 AM, Oliver Hartkopp wrote:
>>>> As pointed out by Jakub Kicinski here:
>>>> http://lore.kernel.org/r/20201009175751.5c54097f@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com
>>>> this patch addresses the remarked issues:
>>>>
>>>> - remove empty line in comment
>>>> - remove default=y for CAN_ISOTP in Kconfig
>>>> - make use of pr_notice_once()
>>>> - use GFP_ATOMIC 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 <socketcan@hartkopp.net>
>>>> ---
>>>
>>> Added both to linux-can-next.
>>
>> The patches were intended for 'net-next' as the merge window will open
>> *very* soon.
>>
>> I'm not sure if there would be another pull from linux-can-next until
>> then, therefore I would suggest that Jakub takes the patches himself.
> 
> I don't mind. FWIW, here's the pull request:
> 
>      http://lore.kernel.org/r/20201012082727.2338859-1-mkl@pengutronix.de
> 
>      git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git tags/linux-can-next-for-5.10-20201012
> 
> Marc

:-D

Well thanks!

I just got the feeling that especially the
"remove default=y for CAN_ISOTP in Kconfig"
issue is urgent in net-next.

So now any way to get the patches is fine.

Thanks & BR,
Oliver
diff mbox series

Patch

diff --git a/include/uapi/linux/can/isotp.h b/include/uapi/linux/can/isotp.h
index 553006509f4e..7793b26aa154 100644
--- a/include/uapi/linux/can/isotp.h
+++ b/include/uapi/linux/can/isotp.h
@@ -160,7 +160,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..ca63061bb932 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);
 
 	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_ATOMIC);
 		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;
 	}