All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oliver Hartkopp <socketcan@hartkopp.net>
To: Jakub Kicinski <kuba@kernel.org>
Cc: Marc Kleine-Budde <mkl@pengutronix.de>,
	netdev@vger.kernel.org, davem@davemloft.net,
	linux-can@vger.kernel.org, kernel@pengutronix.de
Subject: Re: [PATCH 08/17] can: add ISO 15765-2:2016 transport protocol
Date: Sat, 10 Oct 2020 18:24:48 +0200	[thread overview]
Message-ID: <10c8b62c-e522-aaa4-2e75-d1c1bd630735@hartkopp.net> (raw)
In-Reply-To: <20201010084421.308645a2@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>



On 10.10.20 17:44, Jakub Kicinski wrote:
> On Sat, 10 Oct 2020 16:29:06 +0200 Oliver Hartkopp wrote:
>>>> diff --git a/net/can/Kconfig b/net/can/Kconfig
>>>> index 25436a715db3..021fe03a8ed6 100644
>>>> --- a/net/can/Kconfig
>>>> +++ b/net/can/Kconfig
>>>> @@ -55,6 +55,19 @@ config CAN_GW
>>>>    
>>>>    source "net/can/j1939/Kconfig"
>>>>    
>>>> +config CAN_ISOTP
>>>> +	tristate "ISO 15765-2:2016 CAN transport protocol"
>>>> +	default y
>>>
>>> default should not be y unless there is a very good reason.
>>> I don't see such reason here. This is new functionality, users
>>> can enable it if they need it.
>>
>> Yes. I agree. But there is a good reason for it.
>> The ISO 15765-2 protocol is used for vehicle diagnosis and is a *very*
>> common CAN bus use case.
> 
> More common than j1939? (Google uses words like 'widely used' and
> 'common' :)) To give you some perspective we don't enable Ethernet
> vlan support by default, vlans are pretty common, too.
> 
>> The config item only shows up when CONFIG_CAN is selected and then ISO
>> 15765-2 should be enabled too. I have implemented and maintained the
>> out-of-tree driver for ~12 years now and the people have real problems
>> using e.g. Ubuntu with signed kernel modules when they need this protocol.
>>
>> Therefore the option should default to 'y' to make sure the common
>> distros (that enable CONFIG_CAN) enable ISO-TP too.
> 
> I understand the motivation, but Linus had pushed back on defaulting to
> 'y' many times over the years, please read this:
> 
> https://lkml.org/lkml/2012/1/6/354
> 
> This really must not pop up on his screen as default 'y' when he does
> an oldconfig after pulling the networking tree..
> 

Ok. Understood.

I'll remove the default=y then. Would it be ok to add some text like

If you want to perfrom automotive vehicle diagnostic services (UDS), say 
'y'.

?

Best regards,
Oliver

  reply	other threads:[~2020-10-10 23:09 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-07 21:31 pull-request: can-next 2020-10-07 Marc Kleine-Budde
2020-10-07 21:31 ` [PATCH 01/17] can: af_can: can_rcv_list_find(): fix kernel doc after variable renaming Marc Kleine-Budde
2020-10-07 21:31 ` [PATCH 02/17] can: softing: softing_card_shutdown(): add braces around empty body in an 'if' statement Marc Kleine-Budde
2020-10-07 21:31 ` [PATCH 03/17] can: c_can: reg_map_{c,d}_can: mark as __maybe_unused Marc Kleine-Budde
2020-10-07 21:31 ` [PATCH 04/17] MAINTAINERS: adjust to mcp251xfd file renaming Marc Kleine-Budde
2020-10-07 21:31 ` [PATCH 05/17] can: raw: add missing error queue support Marc Kleine-Budde
2020-10-07 21:31 ` [PATCH 06/17] can: dev: fix type of get_can_dlc() and get_canfd_dlc() macros Marc Kleine-Budde
2020-10-07 21:31 ` [PATCH 07/17] can: dev: add a helper function to calculate the duration of one bit Marc Kleine-Budde
2020-10-07 21:31 ` [PATCH 08/17] can: add ISO 15765-2:2016 transport protocol Marc Kleine-Budde
2020-10-10  0:57   ` Jakub Kicinski
2020-10-10 14:29     ` Oliver Hartkopp
2020-10-10 15:44       ` Jakub Kicinski
2020-10-10 16:24         ` Oliver Hartkopp [this message]
2020-10-10 16:34           ` Jakub Kicinski
2020-10-07 21:31 ` [PATCH 09/17] dt-bindings: can: rcar_can: Add r8a7742 support Marc Kleine-Budde
2020-10-07 21:31 ` [PATCH 10/17] dt-bindings: can: rcar_canfd: Document r8a774e1 support Marc Kleine-Budde
2020-10-07 21:31 ` [PATCH 11/17] dt-bindings: can: rcar_can: " Marc Kleine-Budde
2020-10-07 21:31 ` [PATCH 12/17] dt-bindings: can: flexcan: list supported processors Marc Kleine-Budde
2020-10-07 21:31 ` [PATCH 13/17] dt-bindings: can: flexcan: remove ack_grp and ack_bit from fsl,stop-mode Marc Kleine-Budde
2020-10-07 21:31 ` [PATCH 14/17] can: flexcan: remove ack_grp and ack_bit handling from driver Marc Kleine-Budde
2020-10-14  8:53   ` Joakim Zhang
2020-10-14 10:17     ` Marc Kleine-Budde
2020-10-07 21:31 ` [PATCH 15/17] can: xilinx_can: Limit CANFD brp to 2 Marc Kleine-Budde
2020-10-07 21:31 ` [PATCH 16/17] can: xilinx_can: Check return value of set_reset_mode Marc Kleine-Budde
2020-10-07 21:31 ` [PATCH 17/17] can: xilinx_can: Fix incorrect variable and initialize with a default value Marc Kleine-Budde
2020-10-10  1:09 ` pull-request: can-next 2020-10-07 Jakub Kicinski
  -- strict thread matches above, loose matches on Subject: below --
2020-10-06 20:37 [RFC]: can-next 2020-10-06 Marc Kleine-Budde
2020-10-06 20:37 ` [PATCH 08/17] can: add ISO 15765-2:2016 transport protocol Marc Kleine-Budde

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=10c8b62c-e522-aaa4-2e75-d1c1bd630735@hartkopp.net \
    --to=socketcan@hartkopp.net \
    --cc=davem@davemloft.net \
    --cc=kernel@pengutronix.de \
    --cc=kuba@kernel.org \
    --cc=linux-can@vger.kernel.org \
    --cc=mkl@pengutronix.de \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.