* Re: [PATCH net 0/3] bonding: void calling rtmsg_ifinfo for netlink notifications
From: David Miller @ 2017-10-19 12:03 UTC (permalink / raw)
To: lucien.xin; +Cc: jiri, netdev
In-Reply-To: <CADvbK_c7R2OJNq=DyXOCSNVzD2aU2qxBO6UiitGbCYS=icwxTQ@mail.gmail.com>
From: Xin Long <lucien.xin@gmail.com>
Date: Tue, 17 Oct 2017 18:28:45 +0800
> On Tue, Oct 17, 2017 at 5:59 PM, Jiri Pirko <jiri@resnulli.us> wrote:
>> Tue, Oct 17, 2017 at 11:39:38AM CEST, lucien.xin@gmail.com wrote:
>>>It's better to send notifications to userspace by the events
>>>in rtnetlink_event, instead of calling rtmsg_ifinfo directly.
>>>
>>>This patcheset is to remove rtmsg_ifinfo called in bonding,
>>>the notifications can be handled by NETDEV_CHANGEUPPER and
>>>NETDEV_CHANGELOWERSTATE events in rtnetlink_event.
>>>
>>>It could also fix some redundant notifications from bonding.
>>
>> This should go to net-next.
>
> NETDEV_CHANGEUPPER is not yet in rtnetlink_event in net-next tree.
> patches can only work on net tree by now.
>
> Hi, David, you want me to hold them until the patches for NETDEV_CHANGEUPPER
> are copied to net-next, or you would apply them to net ?
Please hold until net is next merged into net-next, thank you.
^ permalink raw reply
* Re: [PATCH net-next v12] openvswitch: enable NSH support
From: Yang, Yi @ 2017-10-19 11:40 UTC (permalink / raw)
To: Jiri Benc
Cc: netdev@vger.kernel.org, dev@openvswitch.org, e@erig.me,
pshelar@ovn.org, davem@davemloft.net
In-Reply-To: <20171018231955.058ec5c8@griffin>
On Thu, Oct 19, 2017 at 05:19:55AM +0800, Jiri Benc wrote:
> On Mon, 16 Oct 2017 21:53:29 +0800, Yi Yang wrote:
> > +static int set_nsh(struct sk_buff *skb, struct sw_flow_key *flow_key,
> > + const struct nlattr *a)
> > +{
> > + struct nshhdr *nh;
> > + size_t length;
> > + int err;
> > + u8 flags;
> > + u8 ttl;
> > + int i;
> > +
> > + struct ovs_key_nsh key;
> > + struct ovs_key_nsh mask;
> > +
> > + err = nsh_key_from_nlattr(a, &key, &mask);
> > + if (err)
> > + return err;
> > +
> > + /* Make sure the NSH base header is there */
> > + err = skb_ensure_writable(skb, skb_network_offset(skb) +
> > + NSH_BASE_HDR_LEN);
> > + if (unlikely(err))
> > + return err;
> > +
> > + nh = nsh_hdr(skb);
> > + length = nsh_hdr_len(nh);
> > +
> > + /* Make sure the whole NSH header is there */
> > + err = skb_ensure_writable(skb, skb_network_offset(skb) +
> > + length);
>
> Calling skb_ensure_writable twice is an unnecessary waste in the fast
> path. If anything, the first call should be changed to pskb_may_pull.
>
> But what we really should do here is to move the header only once. We
> know how much data we're going to write, we have everything stored in
> the key and can calculate it from there.
Actually mdtype can't be set, only push_nsh can set mdtype, so set_nsh
won't have mdtype flow key, we can't get it from flow_key in set_nsh,
only ttl, flags and path_hdr can be set in set_nsh as you can see in code.
I understand your concern is calling skb_ensure_writable twice, so
changing the first one to pskb_may_pull is more appropriate for set_nsh.
>
> length = NSH_BASE_HDR_LEN;
> switch (flow_key->nsh.base.mdtype) {
> case NSH_MD_TYPE1:
> length += sizeof(struct ovs_nsh_key_md1);
> break;
> case NSH_MD_TYPE2:
> length += whatever_way_we_store_the_tlvs_in_flow_key;
> break;
> }
> err = skb_ensure_writable(skb, skb_network_offset(skb) + length);
>
> However, set_nsh does not support MD type 2, thus the second case is a
> dead code. In both switches in this function. As such, it should be
> removed and added only when MD type 2 is introduced. I'd still keep the
> overall logic to ease the future addition, though. This boils down to:
>
> length = NSH_BASE_HDR_LEN;
> /* Assume MD type 1. This function cannot be called for anything
> * else currently. When MD type 2 is added, the line below will
> * have to be turned into a switch on flow_key->nsh.base.mdtype.
> */
> length += sizeof(struct ovs_nsh_key_md1);
> err = skb_ensure_writable(skb, skb_network_offset(skb) + length);
> ...
> flow_key->nsh.base.path_hdr = nh->path_hdr;
> /* Only MD type 1, see the comment above. */
> for (i = 0; i < NSH_MD1_CONTEXT_SIZE; i++) {
> ...
>
> Please verify I'm not missing something.
>
> It seems we also rely on the user space checking first whether the
> packet is indeed compatible with the pushed key/mask. Most importantly,
> that it's of the same mdtype and (in the future) that the MD type 2
> TLVs being written actually fit. Seems this is done the same way in the
> other set_* actions, thus fine with me.
>
> Jiri
^ permalink raw reply
* Re: [PATCH net-next 0/6] update ENA driver to releawse 1.3.0
From: David Miller @ 2017-10-19 11:52 UTC (permalink / raw)
To: netanel; +Cc: netdev, dwmw, zorik, matua, saeedb, msw, aliguori, nafea, evgenys
In-Reply-To: <1508225641-106366-1-git-send-email-netanel@amazon.com>
From: <netanel@amazon.com>
Date: Tue, 17 Oct 2017 07:33:55 +0000
> From: Netanel Belgazal <netanel@amazon.com>
>
> Netanel Belgazal (6):
> net: ena: improve ENA driver boot time.
> net: ena: remove legacy suspend suspend/resume support
> net: ena: add power management ops to the ENA driver
> net: ena: add statistics for missed tx packets
> net: ena: add new admin define for future support of IPv6 RSS
> net: ena: increase ena driver version to 1.3.0
Series applied, but this is a terrible header posting.
You should have real content here. You need to provide text
explaining, at a high level, what the patch series is doing,
how it is doing it, and why it is doing it that way.
Please don't provide an empty header posting like this in
the future.
Thank you.
^ permalink raw reply
* Re: [PATCH net 0/3] ENA ethernet driver bug fixes
From: David Miller @ 2017-10-19 11:49 UTC (permalink / raw)
To: netanel; +Cc: netdev, dwmw, zorik, matua, saeedb, msw, aliguori, nafea, evgenys
In-Reply-To: <1508225585-106167-1-git-send-email-netanel@amazon.com>
From: <netanel@amazon.com>
Date: Tue, 17 Oct 2017 07:33:02 +0000
> From: Netanel Belgazal <netanel@amazon.com>
>
> Some fixes for ENA ethernet driver
Series applied.
^ permalink raw reply
* Re: [PATCH v2 net-next 0/7] net: speedup netns create/delete time
From: Tariq Toukan @ 2017-10-19 11:48 UTC (permalink / raw)
To: Eric Dumazet, Dmitry Torokhov
Cc: Tariq Toukan, David S . Miller, netdev, Eric W . Biederman,
Eric Dumazet, Majd Dibbiny, Yonatan Cohen, Eran Ben Elisha
In-Reply-To: <CANn89iKq86_FRaL7Y3Ekhf_JD8NY2s8PWibGN+WdmJL1TsPXJA@mail.gmail.com>
On 26/09/2017 6:30 PM, Eric Dumazet wrote:
> On Tue, Sep 26, 2017 at 8:22 AM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
>
>> It is in Greg's tree where all kobject patches should go through as far as I know.
>
> Yes, I will fix this, adding a second memmove()
>
Hi Eric,
I just wanted to check if this is solved already, as I don't want to
keep an unnecessary revert patch in our internal branches.
According to my check bug still exists.
Thanks,
Tariq
^ permalink raw reply
* Re: [PATCH net-next] MAINTAINERS: change ENA driver maintainers email domain
From: David Miller @ 2017-10-19 11:47 UTC (permalink / raw)
To: netanel
Cc: linux-kernel, netdev, dwmw, zorik, matua, saeedb, msw, aliguori,
nafea, evgenys
In-Reply-To: <1508225434-105787-1-git-send-email-netanel@amazon.com>
From: <netanel@amazon.com>
Date: Tue, 17 Oct 2017 07:30:34 +0000
> From: Netanel Belgazal <netanel@amazon.com>
>
> ENA driver was developed by developers from Annapurna Labs.
> Annapurna Labs was acquired by Amazon and the company's domain
> (@annapurnalabs.com) will become deprecated soon.
>
> Update the email addresses of the maintainers to the alternative amazon
> emails (@amazon.com)
>
> Signed-off-by: Netanel Belgazal <netanel@amazon.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] qed: Fix iWARP out of order flow
From: David Miller @ 2017-10-19 11:47 UTC (permalink / raw)
To: Michal.Kalderon-YGCgFSpz5w/QT0dZR+AlfA
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
dledford-H+wXaHxf7aLQT0dZR+AlfA,
Manish.Rangankar-YGCgFSpz5w/QT0dZR+AlfA,
Ariel.Elior-YGCgFSpz5w/QT0dZR+AlfA
In-Reply-To: <1508225005-3572-1-git-send-email-Michal.Kalderon-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
From: Michal Kalderon <Michal.Kalderon-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
Date: Tue, 17 Oct 2017 10:23:25 +0300
> Out of order flow is not working for iWARP.
> This patch got cut out from initial series that added out
> of order support for iWARP.
>
> Make out of order code common for iWARP and iSCSI.
> Add new configuration option CONFIG_QED_OOO. Set by
> qedr and qedi Kconfigs.
>
> Fixes: d1abfd0b4ee2 ("qed: Add iWARP out of order support")
>
> Signed-off-by: Michal Kalderon <Michal.Kalderon-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Manish Rangankar <Manish.Rangankar-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Ariel Elior <Ariel.Elior-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
Applied.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 net-next] net: hns3: Add mqprio hardware offload support in hns3 driver
From: David Miller @ 2017-10-19 11:45 UTC (permalink / raw)
To: linyunsheng
Cc: huangdaode, xuwei5, liguozhu, Yisen.Zhuang, gabriele.paoloni,
john.garry, linuxarm, salil.mehta, lipeng321, netdev,
linux-kernel
In-Reply-To: <1508223090-37907-1-git-send-email-linyunsheng@huawei.com>
From: Yunsheng Lin <linyunsheng@huawei.com>
Date: Tue, 17 Oct 2017 14:51:30 +0800
> When using tc qdisc, dcb_ops->setup_tc is used to tell hclge_dcb
> module to do the tm related setup. Only TC_MQPRIO_MODE_CHANNEL
> offload mode is supported.
>
> Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
Applied.
^ permalink raw reply
* Re: [net-next PATCH] macvlan/macvtap: Add support for L2 forwarding offloads with macvtap
From: David Miller @ 2017-10-19 11:44 UTC (permalink / raw)
To: alexander.h.duyck; +Cc: netdev
In-Reply-To: <20171017024428.28589.73728.stgit@ahduyck-green-test.jf.intel.com>
From: Alexander Duyck <alexander.h.duyck@intel.com>
Date: Mon, 16 Oct 2017 22:44:44 -0400
> This patch reverts earlier commit b13ba1b83f52 ("macvlan: forbid L2
> fowarding offload for macvtap"). The reason for reverting this is because
> the original patch no longer fixes what it previously did as the
> underlying structure has changed for macvtap. Specifically macvtap
> originally pulled packets directly off of the lowerdev. However in commit
> 6acf54f1cf0a ("macvtap: Add support of packet capture on macvtap device.")
> that code was changed and instead macvtap would listen directly on the
> macvtap device itself instead of the lower device. As such, the L2
> forwarding offload should now be able to provide a performance advantage of
> skipping the checks on the lower dev while not introducing any sort of
> regression.
>
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Applied.
^ permalink raw reply
* Re: Linux 4.12+ memory leak on router with i40e NICs
From: Pavlos Parissis @ 2017-10-19 11:41 UTC (permalink / raw)
To: Paweł Staszewski
Cc: Alexander Duyck, Anders K. Pedersen | Cohaesio,
netdev@vger.kernel.org, intel-wired-lan@lists.osuosl.org,
alexander.h.duyck@intel.com
In-Reply-To: <57579746-77e1-4603-12ed-7d999fdfeabf@itcare.pl>
On 19 October 2017 at 01:40, Paweł Staszewski <pstaszewski@itcare.pl> wrote:
>
>
> W dniu 2017-10-19 o 01:29, Alexander Duyck pisze:
>
>> On Mon, Oct 16, 2017 at 10:51 PM, Vitezslav Samel <vitezslav@samel.cz>
>> wrote:
>>>
>>> On Tue, Oct 17, 2017 at 01:34:29AM +0200, Paweł Staszewski wrote:
>>>>
>>>> W dniu 2017-10-16 o 18:26, Paweł Staszewski pisze:
>>>>>
>>>>> W dniu 2017-10-16 o 13:20, Pavlos Parissis pisze:
>>>>>>
>>>>>> On 15/10/2017 02:58 πμ, Alexander Duyck wrote:
>>>>>>>
>>>>>>> Hi Pawel,
>>>>>>>
>>>>>>> To clarify is that Dave Miller's tree or Linus's that you are talking
>>>>>>> about? If it is Dave's tree how long ago was it you pulled it since I
>>>>>>> think the fix was just pushed by Jeff Kirsher a few days ago.
>>>>>>>
>>>>>>> The issue should be fixed in the following commit:
>>>>>>>
>>>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git/commit/drivers/net/ethernet/intel/i40e/i40e_txrx.c?id=2b9478ffc550f17c6cd8c69057234e91150f5972
>>>>>>
>>>>>> Do you know when it is going to be available on net-next and
>>>>>> linux-stable repos?
>>>>>>
>>>>>> Cheers,
>>>>>> Pavlos
>>>>>>
>>>>>>
>>>>> I will make some tests today night with "net" git tree where this patch
>>>>> is included.
>>>>> Starting from 0:00 CET
>>>>> :)
>>>>>
>>>>>
>>>> Upgraded and looks like problem is not solved with that patch
>>>> Currently running system with
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git/
>>>> kernel
>>>>
>>>> Still about 0.5GB of memory is leaking somewhere
>>>>
>>>> Also can confirm that the latest kernel where memory is not leaking
>>>> (with
>>>> use i40e driver intel 710 cards) is 4.11.12
>>>> With kernel 4.11.12 - after hour no change in memory usage.
>>>>
>>>> also checked that with ixgbe instead of i40e with same net.git kernel
>>>> there
>>>> is no memleak - after hour same memory usage - so for 100% this is i40e
>>>> driver problem.
>>>
>>> I have (probably) the same problem here but with X520 cards: booting
>>> 4.12.x gives me oops after circa 20 minutes of our workload. Booting
>>> 4.9.y is OK. This machine is in production so any testing is very
>>> limited.
>>>
>>> Machine was stable for >2 months (on the desk before got to
>>> production) with 4.12.8 but with no traffic on X520 cards.
>>>
>>> Cheers,
>>>
>>> Vita
>>
>> Sorry but it can't be the same issue since we are discussing a
>> different driver (i40e) running different hardware (X710 or XL170).
>> You might want to start a new thread for your issue, and/or if
>> possible file a bug on e1000.sf.net.
>>
>> Thanks.
>>
>> - Alex
>>
> sorry but bugs reported on e1000.sf.net are delayed - some after about 6 or
> more months - when i reported first bug there iv got reply after a year
> about no activity :):) haha - and reported there bug is still actrive :)
> better for me is now to change nics (for sure cheaper from the perspective
> of clients :) ) to mellanox or just to replace and use ixgbe - that have no
> this bug (mellanox and ixgbe have no such bug - have many servers with them
> with same conf - and only one with i40e where is same conf and memleak)
>
> If nobody from Intel wants to reproduce this - qool - this is not my problem
> but intels :) - there is now many good nics to use - like mellanox or just
> stick with many 10G based on ixgbe that is really good driver - but really ?
> intel guys have no XL710 cards ? i dont want to buy another buggy cards to
> do only kernel bisects .... sorry ....
> To do good bisects with this bug You need to spend maybee 200/300 bisects -
> and to confirm each - You need maybee 30minutes so count how much time You
> need - more that 100 cards in price from mellanox maybee :)
>
I have similar issues with you in regards to the stability of i40e
driver. I will need to open another thread about them, but I would
like to mention that you are not the only one who suffers from
problems related to i40e driver. In my case I can't simply change
NICs..so it is even worse.
Cheers,
Pavlos
^ permalink raw reply
* [PATCH net-next] geneve: Get rid of is_all_zero(), streamline is_tnl_info_zero()
From: Stefano Brivio @ 2017-10-19 11:31 UTC (permalink / raw)
To: David S . Miller, netdev; +Cc: pravin shelar
No need to re-invent memchr_inv() with !is_all_zero(). While at
it, replace conditional and return clauses with a single return
clause in is_tnl_info_zero().
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
drivers/net/geneve.c | 19 +++----------------
1 file changed, 3 insertions(+), 16 deletions(-)
diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
index f6404074b7b0..869726660b83 100644
--- a/drivers/net/geneve.c
+++ b/drivers/net/geneve.c
@@ -1140,24 +1140,11 @@ static struct geneve_dev *geneve_find_dev(struct geneve_net *gn,
return t;
}
-static bool is_all_zero(const u8 *fp, size_t size)
-{
- int i;
-
- for (i = 0; i < size; i++)
- if (fp[i])
- return false;
- return true;
-}
-
static bool is_tnl_info_zero(const struct ip_tunnel_info *info)
{
- if (info->key.tun_id || info->key.tun_flags || info->key.tos ||
- info->key.ttl || info->key.label || info->key.tp_src ||
- !is_all_zero((const u8 *)&info->key.u, sizeof(info->key.u)))
- return false;
- else
- return true;
+ return !(info->key.tun_id || !info->key.tun_flags || !info->key.tos ||
+ info->key.ttl || !info->key.label || !info->key.tp_src ||
+ memchr_inv(&info->key.u, 0, sizeof(info->key.u)));
}
static bool geneve_dst_addr_equal(struct ip_tunnel_info *a,
--
2.9.4
^ permalink raw reply related
* [PATCH net] geneve: Fix function matching VNI and tunnel ID on big-endian
From: Stefano Brivio @ 2017-10-19 11:31 UTC (permalink / raw)
To: David S . Miller, netdev
Cc: pravin shelar, Girish Moodalbail, Sabrina Dubroca
On big-endian machines, functions converting between tunnel ID
and VNI use the three LSBs of tunnel ID storage to map VNI.
The comparison function eq_tun_id_and_vni(), on the other hand,
attempted to map the VNI from the three MSBs. Fix it by using
the same check implemented on LE, which maps VNI from the three
LSBs of tunnel ID.
Fixes: 2e0b26e10352 ("geneve: Optimize geneve device lookup.")
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
I guess this should be queued up for -stable, back to 4.10
drivers/net/geneve.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
index f6404074b7b0..ed51018a813e 100644
--- a/drivers/net/geneve.c
+++ b/drivers/net/geneve.c
@@ -113,13 +113,7 @@ static void tunnel_id_to_vni(__be64 tun_id, __u8 *vni)
static bool eq_tun_id_and_vni(u8 *tun_id, u8 *vni)
{
-#ifdef __BIG_ENDIAN
- return (vni[0] == tun_id[2]) &&
- (vni[1] == tun_id[1]) &&
- (vni[2] == tun_id[0]);
-#else
return !memcmp(vni, &tun_id[5], 3);
-#endif
}
static sa_family_t geneve_get_sk_family(struct geneve_sock *gs)
--
2.9.4
^ permalink raw reply related
* Re: Netlink XFRM socket subsystem NULL pointer dereference
From: Timo Teras @ 2017-10-19 11:33 UTC (permalink / raw)
To: Herbert Xu; +Cc: Eric Dumazet, Steffen Klassert, netdev
In-Reply-To: <20171019095704.GA13459@gondor.apana.org.au>
On Thu, 19 Oct 2017 17:57:04 +0800
Herbert Xu <herbert@gondor.apana.org.au> wrote:
> On Thu, Oct 19, 2017 at 05:26:25PM +0800, Herbert Xu wrote:
> >
> > So it's an netlink API issue. It is possible for cb->done to be
> > called without cb->dump ever being called. And xfrm_user doesn't
> > deal with that. Let me survey the others to see whether we should
> > fix this in netlink, xfrm, or both.
>
> OK it looks like an XFRM bug pure and simple. Funnily enough
> the xfrm sa dump code which got changed by the same commit that
> added this bug isn't buggy.
>
> ---8<---
> Subject: ipsec: Fix aborted xfrm policy dump crash
>
> An independent security researcher, Mohamed Ghannam, has reported
> this vulnerability to Beyond Security's SecuriTeam Secure Disclosure
> program.
>
> The xfrm_dump_policy_done function expects xfrm_dump_policy to
> have been called at least once or it will crash. This can be
> triggered if a dump fails because the target socket's receive
> buffer is full.
>
> This patch fixes it by using the cb->start mechanism to ensure that
> the initialisation is always done regardless of the buffer situation.
>
> Fixes: 4c563f7669c1 ("[XFRM]: Speed up xfrm_policy and xfrm_state...")
This is not correct. The original commit works just fine.
The bug was introduced later. And SA side had the same issue, it got
fixed in commit:
1ba5bf993c6a3142e18e "xfrm: fix crash in XFRM_MSG_GETSA netlink handler"
The commit introducing these issues is
12a169e7d8f4b1c95252 "ipsec: Put dumpers on the dump list".
where the walker state was modified to contain list entry instead of
pointer reference.
At that time there was no .start which got added just few years ago. I
suggest to do the same fix for SA side since it had same issue fixed on
the other commit. Your approach with defining the .start is cleaner.
With updated Fixed line, and preferably applying the same to SA side,
you can add:
Acked-by: Timo Teräs <timo.teras@iki.fi>
Thanks,
Timo
^ permalink raw reply
* Re: [RFC PATCH] can: m_can: Support higher speed CAN-FD bitrates
From: Marc Kleine-Budde @ 2017-10-19 11:32 UTC (permalink / raw)
To: Sekhar Nori, Franklin S Cooper Jr, Mario Hüttel,
Yang, Wenyou, wg, socketcan, quentin.schulz, edumazet, linux-can,
netdev, linux-kernel
Cc: Wenyou Yang, Dong Aisheng, Quadros, Roger
In-Reply-To: <4c5b5e9f-44dc-0bf5-5131-b3caf54f390e@ti.com>
[-- Attachment #1.1: Type: text/plain, Size: 5205 bytes --]
On 10/19/2017 01:09 PM, Sekhar Nori wrote:
> On Thursday 19 October 2017 02:43 PM, Marc Kleine-Budde wrote:
>> On 10/19/2017 07:07 AM, Sekhar Nori wrote:
>>>>>> Sounds reasonable. What's the status of this series?
>>>>>
>>>>> I have had some offline discussions with Franklin on this, and I am not
>>>>> fully convinced that DT is the way to go here (although I don't have the
>>>>> agreement with Franklin there).
>>>>
>>>> Probably the fundamental area where we disagree is what "default" SSP
>>>> value should be used. Based on a short (< 1 ft) point to point test
>>>> using a SSP of 50% worked fine. However, I'm not convinced that this
>>>> default value of 50% will work in a more "traditional" CAN bus at higher
>>>> speeds. Nor am I convinced that a SSP of 50% will work on every MCAN
>>>> board in even the simplest test cases.
>>>>
>>>> I believe that this default SSP should be a DT property that allows any
>>>> board to determine what default value works best in general.
>>>
>>> With that, I think, we are taking DT from describing board/hardware
>>> characteristics to providing default values that software should use.
>>
>> If the default value is board specific and cannot be calculated in
>> general or from other values present in the DT, then it's from my point
>> of view describing the hardware.
>>
>>> In any case, if Marc and/or Wolfgang are okay with it, binding
>>> documentation for such a property should be sent to DT maintainers for
>>> review.
>>>
>>>>>
>>>>> There are two components in configuring the secondary sample point. It
>>>>> is the transceiver loopback delay and an offset (example half of the bit
>>>>> time in data phase).
>>>>>
>>>>> While the transceiver loopback delay is pretty board dependent (and thus
>>>>> amenable to DT encoding), I am not quite sure the offset can be
>>>>> configured in DT because its not really board dependent.
>>>>>
>>>>> Unfortunately, offset calculation does not seem to be an exact science.
>>>>> There are recommendations ranging from using 50% of bit time to making
>>>>> it same as the sample point configured. This means users who need to
>>>>> change the SSP due to offset variations need to change their DT even
>>>>> without anything changing on their board.
>>>>>
>>>>> Since we have a netlink socket interface to configure sample point, I
>>>>> wonder if that should be extended to configure SSP too (or at least the
>>>>> offset part of SSP)?
>>>>
>>>> Sekhar is right that ideally the user should be able to set the SSP at
>>>> runtime. However, my issue is that based on my experience CAN users
>>>> expect the driver to just work the majority of times. For unique use
>>>> cases where the driver calculated values don't work then the user should
>>>> be able to override it. This should only be done for a very small
>>>> percentage of CAN users. Unless you allow DT to provide a default SSP
>>>> many users of MCAN may find that the default SSP doesn't work and must
>>>> always use runtime overrides to get anything to work. I don't think that
>>>> is a good user experience which is why I don't like the idea.
>>>
>>> Fair enough. But not quite sure if CAN users expect CAN-FD to "just
>>> work" without doing any bittiming related setup.
>>
>> From my point of view I'd rather buy a board from a HW vendor where
>> CAN-FD works, rather than a board where I have to tweak the bit-timing
>> for a simple CAN-FD test setup.
>>
>> As far as I see for the m_can driver it's a single tuple: "bitrate > 2.5
>> MBit/s -> 50%". Do we need an array of tuples in general?
Do we need more than one tuple here?
>> If good default values are transceiver and board specific, they can go
>> into the DT. We need a generic (this means driver agnostic) binding for
>> this. If this table needs to be tweaked for special purpose, then we can
>> add a netlink interface for this as well.
>>
>> Comments?
>
> I dont know how a good default (other than 50% as the starting point)
> can be arrived at without doing any actual measurements on the actual
> network. Since we do know that the value has to be tweaked, agree that
> netlink interface has to be provided.
>
> I wonder whether even if a DT binding for default is provided, everyone
> will end up setting it to 50% (since there is no way for them to predict
> any better). In effect, I am suggesting using a hardcoded value of 50%
> instead of introducing a binding without a clear need for it.
Ok, if the value is network and not board specific it doesn't belong
into the DT.
> Note that I am only talking about there "offset" part of SSP here. The
> transceiver loopback delay is calculated automatically by Bosch's MCAN
> IP. But if there are other IPs which don't do that, then yes, that
> should be a DT property IMO.
Ok, but let's wait for such an IP core to show up here :)
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [RFC PATCH] can: m_can: Support higher speed CAN-FD bitrates
From: Marc Kleine-Budde @ 2017-10-19 11:26 UTC (permalink / raw)
To: Oliver Hartkopp, Sekhar Nori, Franklin S Cooper Jr,
Mario Hüttel, Yang, Wenyou, wg, quentin.schulz, edumazet,
linux-can, netdev, linux-kernel
Cc: Wenyou Yang, Dong Aisheng, Quadros, Roger
In-Reply-To: <a972cec1-3386-3ca3-ace3-175e1cabd286@hartkopp.net>
[-- Attachment #1.1: Type: text/plain, Size: 2523 bytes --]
On 10/19/2017 01:14 PM, Oliver Hartkopp wrote:
>>>>> Since we have a netlink socket interface to configure sample point, I
>>>>> wonder if that should be extended to configure SSP too (or at least the
>>>>> offset part of SSP)?
>
> +1 too
The struct can_bittiming in defined in uapi, so we have to keep ABI
compatibility in mind.
>>>> Sekhar is right that ideally the user should be able to set the SSP at
>>>> runtime. However, my issue is that based on my experience CAN users
>>>> expect the driver to just work the majority of times. For unique use
>>>> cases where the driver calculated values don't work then the user should
>>>> be able to override it. This should only be done for a very small
>>>> percentage of CAN users. Unless you allow DT to provide a default SSP
>>>> many users of MCAN may find that the default SSP doesn't work and must
>>>> always use runtime overrides to get anything to work. I don't think that
>>>> is a good user experience which is why I don't like the idea.
>>>
>>> Fair enough. But not quite sure if CAN users expect CAN-FD to "just
>>> work" without doing any bittiming related setup.
>>
>> From my point of view I'd rather buy a board from a HW vendor where
>> CAN-FD works, rather than a board where I have to tweak the bit-timing
>> for a simple CAN-FD test setup.
>>
>> As far as I see for the m_can driver it's a single tuple: "bitrate > 2.5
>> MBit/s -> 50%". Do we need an array of tuples in general?
>>
>> If good default values are transceiver and board specific, they can go
>> into the DT. We need a generic (this means driver agnostic) binding for
>> this. If this table needs to be tweaked for special purpose, then we can
>> add a netlink interface for this as well. >
>> Comments?
>
> By now we calculate reasonable default values (e.g. for SP and SJW), you
> can override by setting alternative values via netlink configuration.
>
> I would tend to stay on this approach and not hide these things in DTs -
> just because of someone wants to initialize his specific interface 'easier'.
If the values are not board specific, then it makes no sense to put them
into the DT.
> One tool, one place is IMHO still the best option.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* [PATCH 10/11] can: esd_usb2: Fix can_dlc value for received RTR, frames
From: Marc Kleine-Budde @ 2017-10-19 11:19 UTC (permalink / raw)
To: netdev
Cc: davem, linux-can, kernel, Stefan Mätje, Stefan Mätje,
linux-stable, Marc Kleine-Budde
In-Reply-To: <20171019111945.13580-1-mkl@pengutronix.de>
From: Stefan Mätje <Stefan.Maetje@esd.eu>
The dlc member of the struct rx_msg contains also the ESD_RTR flag to
mark received RTR frames. Without the fix the can_dlc value for received
RTR frames would always be set to 8 by get_can_dlc() instead of the
received value.
Fixes: 96d8e90382dc ("can: Add driver for esd CAN-USB/2 device")
Signed-off-by: Stefan Mätje <stefan.maetje@esd.eu>
Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
drivers/net/can/usb/esd_usb2.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/can/usb/esd_usb2.c b/drivers/net/can/usb/esd_usb2.c
index be928ce62d32..9fdb0f0bfa06 100644
--- a/drivers/net/can/usb/esd_usb2.c
+++ b/drivers/net/can/usb/esd_usb2.c
@@ -333,7 +333,7 @@ static void esd_usb2_rx_can_msg(struct esd_usb2_net_priv *priv,
}
cf->can_id = id & ESD_IDMASK;
- cf->can_dlc = get_can_dlc(msg->msg.rx.dlc);
+ cf->can_dlc = get_can_dlc(msg->msg.rx.dlc & ~ESD_RTR);
if (id & ESD_EXTID)
cf->can_id |= CAN_EFF_FLAG;
--
2.14.2
^ permalink raw reply related
* [PATCH 07/11] can: bcm: check for null sk before deferencing it via the call to sock_net
From: Marc Kleine-Budde @ 2017-10-19 11:19 UTC (permalink / raw)
To: netdev; +Cc: davem, linux-can, kernel, Colin Ian King, Marc Kleine-Budde
In-Reply-To: <20171019111945.13580-1-mkl@pengutronix.de>
From: Colin Ian King <colin.king@canonical.com>
The assignment of net via call sock_net will dereference sk. This
is performed before a sanity null check on sk, so there could be
a potential null dereference on the sock_net call if sk is null.
Fix this by assigning net after the sk null check. Also replace
the sk == NULL with the more usual !sk idiom.
Detected by CoverityScan CID#1431862 ("Dereference before null check")
Fixes: 384317ef4187 ("can: network namespace support for CAN_BCM protocol")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
Acked-by: Oliver Hartkopp <socketcan@hartkopp.net>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
net/can/bcm.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/net/can/bcm.c b/net/can/bcm.c
index 47a8748d953a..13690334efa3 100644
--- a/net/can/bcm.c
+++ b/net/can/bcm.c
@@ -1493,13 +1493,14 @@ static int bcm_init(struct sock *sk)
static int bcm_release(struct socket *sock)
{
struct sock *sk = sock->sk;
- struct net *net = sock_net(sk);
+ struct net *net;
struct bcm_sock *bo;
struct bcm_op *op, *next;
- if (sk == NULL)
+ if (!sk)
return 0;
+ net = sock_net(sk);
bo = bcm_sk(sk);
/* remove bcm_ops, timer, rx_unregister(), etc. */
--
2.14.2
^ permalink raw reply related
* [PATCH 08/11] can: af_can: do not access proto_tab directly use rcu_access_pointer instead
From: Marc Kleine-Budde @ 2017-10-19 11:19 UTC (permalink / raw)
To: netdev; +Cc: davem, linux-can, kernel, Marc Kleine-Budde
In-Reply-To: <20171019111945.13580-1-mkl@pengutronix.de>
"proto_tab" is a RCU protected array, when directly accessing the array,
sparse throws these warnings:
CHECK /srv/work/frogger/socketcan/linux/net/can/af_can.c
net/can/af_can.c:115:14: error: incompatible types in comparison expression (different address spaces)
net/can/af_can.c:795:17: error: incompatible types in comparison expression (different address spaces)
net/can/af_can.c:816:9: error: incompatible types in comparison expression (different address spaces)
This patch fixes the problem by using rcu_access_pointer() and
annotating "proto_tab" array as __rcu.
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
net/can/af_can.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/can/af_can.c b/net/can/af_can.c
index 88edac0f3e36..eb1ad74b40f4 100644
--- a/net/can/af_can.c
+++ b/net/can/af_can.c
@@ -78,7 +78,7 @@ MODULE_PARM_DESC(stats_timer, "enable timer for statistics (default:on)");
static struct kmem_cache *rcv_cache __read_mostly;
/* table of registered CAN protocols */
-static const struct can_proto *proto_tab[CAN_NPROTO] __read_mostly;
+static const struct can_proto __rcu *proto_tab[CAN_NPROTO] __read_mostly;
static DEFINE_MUTEX(proto_tab_lock);
static atomic_t skbcounter = ATOMIC_INIT(0);
@@ -788,7 +788,7 @@ int can_proto_register(const struct can_proto *cp)
mutex_lock(&proto_tab_lock);
- if (proto_tab[proto]) {
+ if (rcu_access_pointer(proto_tab[proto])) {
pr_err("can: protocol %d already registered\n", proto);
err = -EBUSY;
} else
@@ -812,7 +812,7 @@ void can_proto_unregister(const struct can_proto *cp)
int proto = cp->protocol;
mutex_lock(&proto_tab_lock);
- BUG_ON(proto_tab[proto] != cp);
+ BUG_ON(rcu_access_pointer(proto_tab[proto]) != cp);
RCU_INIT_POINTER(proto_tab[proto], NULL);
mutex_unlock(&proto_tab_lock);
--
2.14.2
^ permalink raw reply related
* [PATCH 05/11] can: flexcan: fix i.MX28 state transition issue
From: Marc Kleine-Budde @ 2017-10-19 11:19 UTC (permalink / raw)
To: netdev
Cc: davem, linux-can, kernel, ZHU Yi (ST-FIR/ENG1-Zhu),
Wolfgang Grandegger, Zhu Yi, Mark Jonas, linux-stable,
Marc Kleine-Budde
In-Reply-To: <20171019111945.13580-1-mkl@pengutronix.de>
From: "ZHU Yi (ST-FIR/ENG1-Zhu)" <Yi.Zhu5@cn.bosch.com>
Enable FLEXCAN_QUIRK_BROKEN_PERR_STATE for i.MX28 to report correct
state transitions, especially to error passive.
Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
Signed-off-by: Zhu Yi <yi.zhu5@cn.bosch.com>
Signed-off-by: Mark Jonas <mark.jonas@de.bosch.com>
Cc: linux-stable <stable@vger.kernel.org> # >= v4.11
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
drivers/net/can/flexcan.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index d6ad12744ff1..ed544c44848f 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -285,7 +285,9 @@ static const struct flexcan_devtype_data fsl_p1010_devtype_data = {
.quirks = FLEXCAN_QUIRK_BROKEN_WERR_STATE,
};
-static const struct flexcan_devtype_data fsl_imx28_devtype_data;
+static const struct flexcan_devtype_data fsl_imx28_devtype_data = {
+ .quirks = FLEXCAN_QUIRK_BROKEN_PERR_STATE,
+};
static const struct flexcan_devtype_data fsl_imx6q_devtype_data = {
.quirks = FLEXCAN_QUIRK_DISABLE_RXFG | FLEXCAN_QUIRK_ENABLE_EACEN_RRS |
--
2.14.2
^ permalink raw reply related
* pull-request: can 2017-10-19
From: Marc Kleine-Budde @ 2017-10-19 11:19 UTC (permalink / raw)
To: netdev; +Cc: davem, linux-can, kernel
Hello David,
this is a pull request of 11 patches for the upcoming 4.14 release.
There are 6 patches by ZHU Yi for the flexcan driver, that work around
the CAN error handling state transition problems found in various
incarnations of the flexcan IP core.
The patch by Colin Ian King fixes a potential NULL pointer deref in the
CAN broad cast manager (bcm). One patch by me replaces a direct deref of a RCU
protected pointer by rcu_access_pointer. My second patch adds missing
OOM error handling in af_can. A patch by Stefan Mätje for the esd_usb2
driver fixes the dlc in received RTR frames. And the last patch is by
Wolfgang Grandegger, it fixes a busy loop in the gs_usb driver in case
it runs out of TX contexts.
regards,
Marc
---
The following changes since commit 28e33f9d78eefe98ea86673ab31e988b37a9a738:
bpf: disallow arithmetic operations on context pointer (2017-10-18 13:21:13 +0100)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can.git tags/linux-can-fixes-for-4.14-20171019
for you to fetch changes up to 97819f943063b622eca44d3644067c190dc75039:
can: gs_usb: fix busy loop if no more TX context is available (2017-10-19 13:05:54 +0200)
----------------------------------------------------------------
linux-can-fixes-for-4.14-20171019
----------------------------------------------------------------
Colin Ian King (1):
can: bcm: check for null sk before deferencing it via the call to sock_net
Marc Kleine-Budde (2):
can: af_can: do not access proto_tab directly use rcu_access_pointer instead
can: af_can: can_pernet_init(): add missing error handling for kzalloc returning NULL
Stefan Mätje (1):
can: esd_usb2: Fix can_dlc value for received RTR, frames
Wolfgang Grandegger (1):
can: gs_usb: fix busy loop if no more TX context is available
ZHU Yi (ST-FIR/ENG1-Zhu) (6):
can: flexcan: fix state transition regression
can: flexcan: rename legacy error state quirk
can: flexcan: implement error passive state quirk
can: flexcan: fix i.MX6 state transition issue
can: flexcan: fix i.MX28 state transition issue
can: flexcan: fix p1010 state transition issue
drivers/net/can/flexcan.c | 91 +++++++++++++++++++++++++++++++++++-------
drivers/net/can/usb/esd_usb2.c | 2 +-
drivers/net/can/usb/gs_usb.c | 10 +----
net/can/af_can.c | 20 ++++++++--
net/can/bcm.c | 5 ++-
5 files changed, 98 insertions(+), 30 deletions(-)
^ permalink raw reply
* [PATCH 11/11] can: gs_usb: fix busy loop if no more TX context is available
From: Marc Kleine-Budde @ 2017-10-19 11:19 UTC (permalink / raw)
To: netdev
Cc: davem, linux-can, kernel, Wolfgang Grandegger, linux-stable,
Marc Kleine-Budde
In-Reply-To: <20171019111945.13580-1-mkl@pengutronix.de>
From: Wolfgang Grandegger <wg@grandegger.com>
If sending messages with no cable connected, it quickly happens that
there is no more TX context available. Then "gs_can_start_xmit()"
returns with "NETDEV_TX_BUSY" and the upper layer does retry
immediately keeping the CPU busy. To fix that issue, I moved
"atomic_dec(&dev->active_tx_urbs)" from "gs_usb_xmit_callback()" to
the TX done handling in "gs_usb_receive_bulk_callback()". Renaming
"active_tx_urbs" to "active_tx_contexts" and moving it into
"gs_[alloc|free]_tx_context()" would also make sense.
Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
drivers/net/can/usb/gs_usb.c | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c
index afcc1312dbaf..68ac3e88a8ce 100644
--- a/drivers/net/can/usb/gs_usb.c
+++ b/drivers/net/can/usb/gs_usb.c
@@ -375,6 +375,8 @@ static void gs_usb_receive_bulk_callback(struct urb *urb)
gs_free_tx_context(txc);
+ atomic_dec(&dev->active_tx_urbs);
+
netif_wake_queue(netdev);
}
@@ -463,14 +465,6 @@ static void gs_usb_xmit_callback(struct urb *urb)
urb->transfer_buffer_length,
urb->transfer_buffer,
urb->transfer_dma);
-
- atomic_dec(&dev->active_tx_urbs);
-
- if (!netif_device_present(netdev))
- return;
-
- if (netif_queue_stopped(netdev))
- netif_wake_queue(netdev);
}
static netdev_tx_t gs_can_start_xmit(struct sk_buff *skb,
--
2.14.2
^ permalink raw reply related
* [PATCH 09/11] can: af_can: can_pernet_init(): add missing error handling for kzalloc returning NULL
From: Marc Kleine-Budde @ 2017-10-19 11:19 UTC (permalink / raw)
To: netdev; +Cc: davem, linux-can, kernel, Marc Kleine-Budde, linux-stable
In-Reply-To: <20171019111945.13580-1-mkl@pengutronix.de>
This patch adds the missing check and error handling for out-of-memory
situations, when kzalloc cannot allocate memory.
Fixes: cb5635a36776 ("can: complete initial namespace support")
Acked-by: Oliver Hartkopp <socketcan@hartkopp.net>
Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
net/can/af_can.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/net/can/af_can.c b/net/can/af_can.c
index eb1ad74b40f4..ecd5c703d11e 100644
--- a/net/can/af_can.c
+++ b/net/can/af_can.c
@@ -875,9 +875,14 @@ static int can_pernet_init(struct net *net)
spin_lock_init(&net->can.can_rcvlists_lock);
net->can.can_rx_alldev_list =
kzalloc(sizeof(struct dev_rcv_lists), GFP_KERNEL);
-
+ if (!net->can.can_rx_alldev_list)
+ goto out;
net->can.can_stats = kzalloc(sizeof(struct s_stats), GFP_KERNEL);
+ if (!net->can.can_stats)
+ goto out_free_alldev_list;
net->can.can_pstats = kzalloc(sizeof(struct s_pstats), GFP_KERNEL);
+ if (!net->can.can_pstats)
+ goto out_free_can_stats;
if (IS_ENABLED(CONFIG_PROC_FS)) {
/* the statistics are updated every second (timer triggered) */
@@ -892,6 +897,13 @@ static int can_pernet_init(struct net *net)
}
return 0;
+
+ out_free_can_stats:
+ kfree(net->can.can_stats);
+ out_free_alldev_list:
+ kfree(net->can.can_rx_alldev_list);
+ out:
+ return -ENOMEM;
}
static void can_pernet_exit(struct net *net)
--
2.14.2
^ permalink raw reply related
* [PATCH 06/11] can: flexcan: fix p1010 state transition issue
From: Marc Kleine-Budde @ 2017-10-19 11:19 UTC (permalink / raw)
To: netdev
Cc: davem, linux-can, kernel, ZHU Yi (ST-FIR/ENG1-Zhu), Zhu Yi,
Mark Jonas, linux-stable, Marc Kleine-Budde
In-Reply-To: <20171019111945.13580-1-mkl@pengutronix.de>
From: "ZHU Yi (ST-FIR/ENG1-Zhu)" <Yi.Zhu5@cn.bosch.com>
Enable FLEXCAN_QUIRK_BROKEN_WERR_STATE and
FLEXCAN_QUIRK_BROKEN_PERR_STATE for p1010 to report correct state
transitions.
Signed-off-by: Zhu Yi <yi.zhu5@cn.bosch.com>
Signed-off-by: Mark Jonas <mark.jonas@de.bosch.com>
Acked-by: Wolfgang Grandegger <wg@grandegger.com>
Cc: linux-stable <stable@vger.kernel.org> # >= v4.11
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
drivers/net/can/flexcan.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index ed544c44848f..a13a4896a8bd 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -282,7 +282,8 @@ struct flexcan_priv {
};
static const struct flexcan_devtype_data fsl_p1010_devtype_data = {
- .quirks = FLEXCAN_QUIRK_BROKEN_WERR_STATE,
+ .quirks = FLEXCAN_QUIRK_BROKEN_WERR_STATE |
+ FLEXCAN_QUIRK_BROKEN_PERR_STATE,
};
static const struct flexcan_devtype_data fsl_imx28_devtype_data = {
--
2.14.2
^ permalink raw reply related
* [PATCH 04/11] can: flexcan: fix i.MX6 state transition issue
From: Marc Kleine-Budde @ 2017-10-19 11:19 UTC (permalink / raw)
To: netdev
Cc: davem, linux-can, kernel, ZHU Yi (ST-FIR/ENG1-Zhu), Zhu Yi,
Mark Jonas, linux-stable, Marc Kleine-Budde
In-Reply-To: <20171019111945.13580-1-mkl@pengutronix.de>
From: "ZHU Yi (ST-FIR/ENG1-Zhu)" <Yi.Zhu5@cn.bosch.com>
Enable FLEXCAN_QUIRK_BROKEN_PERR_STATE for i.MX6 to report correct state
transitions.
Signed-off-by: Zhu Yi <yi.zhu5@cn.bosch.com>
Signed-off-by: Mark Jonas <mark.jonas@de.bosch.com>
Acked-by: Wolfgang Grandegger <wg@grandegger.com>
Cc: linux-stable <stable@vger.kernel.org> # >= v4.11
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
drivers/net/can/flexcan.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index c83a09fa4166..d6ad12744ff1 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -289,7 +289,7 @@ static const struct flexcan_devtype_data fsl_imx28_devtype_data;
static const struct flexcan_devtype_data fsl_imx6q_devtype_data = {
.quirks = FLEXCAN_QUIRK_DISABLE_RXFG | FLEXCAN_QUIRK_ENABLE_EACEN_RRS |
- FLEXCAN_QUIRK_USE_OFF_TIMESTAMP,
+ FLEXCAN_QUIRK_USE_OFF_TIMESTAMP | FLEXCAN_QUIRK_BROKEN_PERR_STATE,
};
static const struct flexcan_devtype_data fsl_vf610_devtype_data = {
--
2.14.2
^ permalink raw reply related
* [PATCH 03/11] can: flexcan: implement error passive state quirk
From: Marc Kleine-Budde @ 2017-10-19 11:19 UTC (permalink / raw)
To: netdev
Cc: davem, linux-can, kernel, ZHU Yi (ST-FIR/ENG1-Zhu), Zhu Yi,
Mark Jonas, linux-stable, Marc Kleine-Budde
In-Reply-To: <20171019111945.13580-1-mkl@pengutronix.de>
From: "ZHU Yi (ST-FIR/ENG1-Zhu)" <Yi.Zhu5@cn.bosch.com>
Add FLEXCAN_QUIRK_BROKEN_PERR_STATE for better description of the
missing error passive interrupt quirk.
Error interrupt flooding may happen if the broken error state quirk fix
is enabled. For example, in case there is singled out node on the bus
and the node sends a frame, then error interrupt flooding happens and
will not stop because the node cannot go to bus off. The flooding will
stop after another node connected to the bus again.
If high bitrate configured on the low end system, then the flooding
may causes performance issue, hence, this patch mitigates this by:
1. disable error interrupt upon error passive state transition
2. re-enable error interrupt upon error warning state transition
3. disable/enable error interrupt upon error active state transition
depends on FLEXCAN_QUIRK_BROKEN_WERR_STATE
In this way, the driver is still able to report correct state
transitions without additional latency. When there are bus problems,
flooding of error interrupts is limited to the number of frames required
to change state from error warning to error passive if the core has
[TR]WRN_INT connected (FLEXCAN_QUIRK_BROKEN_WERR_STATE is not enabled),
otherwise, the flooding is limited to the number of frames required to
change state from error active to error passive.
Signed-off-by: Zhu Yi <yi.zhu5@cn.bosch.com>
Signed-off-by: Mark Jonas <mark.jonas@de.bosch.com>
Acked-by: Wolfgang Grandegger <wg@grandegger.com>
Cc: linux-stable <stable@vger.kernel.org> # >= v4.11
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
drivers/net/can/flexcan.c | 75 +++++++++++++++++++++++++++++++++++++++++------
1 file changed, 66 insertions(+), 9 deletions(-)
diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index e163c55e737b..c83a09fa4166 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -182,14 +182,14 @@
/* FLEXCAN hardware feature flags
*
* Below is some version info we got:
- * SOC Version IP-Version Glitch- [TR]WRN_INT Memory err RTR re-
- * Filter? connected? detection ception in MB
- * MX25 FlexCAN2 03.00.00.00 no no no no
- * MX28 FlexCAN2 03.00.04.00 yes yes no no
- * MX35 FlexCAN2 03.00.00.00 no no no no
- * MX53 FlexCAN2 03.00.00.00 yes no no no
- * MX6s FlexCAN3 10.00.12.00 yes yes no yes
- * VF610 FlexCAN3 ? no yes yes yes?
+ * SOC Version IP-Version Glitch- [TR]WRN_INT IRQ Err Memory err RTR re-
+ * Filter? connected? Passive detection ception in MB
+ * MX25 FlexCAN2 03.00.00.00 no no ? no no
+ * MX28 FlexCAN2 03.00.04.00 yes yes no no no
+ * MX35 FlexCAN2 03.00.00.00 no no ? no no
+ * MX53 FlexCAN2 03.00.00.00 yes no no no no
+ * MX6s FlexCAN3 10.00.12.00 yes yes no no yes
+ * VF610 FlexCAN3 ? no yes ? yes yes?
*
* Some SOCs do not have the RX_WARN & TX_WARN interrupt line connected.
*/
@@ -198,6 +198,7 @@
#define FLEXCAN_QUIRK_ENABLE_EACEN_RRS BIT(3) /* Enable EACEN and RRS bit in ctrl2 */
#define FLEXCAN_QUIRK_DISABLE_MECR BIT(4) /* Disable Memory error detection */
#define FLEXCAN_QUIRK_USE_OFF_TIMESTAMP BIT(5) /* Use timestamp based offloading */
+#define FLEXCAN_QUIRK_BROKEN_PERR_STATE BIT(6) /* No interrupt for error passive */
/* Structure of the message buffer */
struct flexcan_mb {
@@ -335,6 +336,22 @@ static inline void flexcan_write(u32 val, void __iomem *addr)
}
#endif
+static inline void flexcan_error_irq_enable(const struct flexcan_priv *priv)
+{
+ struct flexcan_regs __iomem *regs = priv->regs;
+ u32 reg_ctrl = (priv->reg_ctrl_default | FLEXCAN_CTRL_ERR_MSK);
+
+ flexcan_write(reg_ctrl, ®s->ctrl);
+}
+
+static inline void flexcan_error_irq_disable(const struct flexcan_priv *priv)
+{
+ struct flexcan_regs __iomem *regs = priv->regs;
+ u32 reg_ctrl = (priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_MSK);
+
+ flexcan_write(reg_ctrl, ®s->ctrl);
+}
+
static inline int flexcan_transceiver_enable(const struct flexcan_priv *priv)
{
if (!priv->reg_xceiver)
@@ -713,6 +730,7 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
struct flexcan_regs __iomem *regs = priv->regs;
irqreturn_t handled = IRQ_NONE;
u32 reg_iflag1, reg_esr;
+ enum can_state last_state = priv->can.state;
reg_iflag1 = flexcan_read(®s->iflag1);
@@ -767,7 +785,8 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
/* state change interrupt or broken error state quirk fix is enabled */
if ((reg_esr & FLEXCAN_ESR_ERR_STATE) ||
- (priv->devtype_data->quirks & FLEXCAN_QUIRK_BROKEN_WERR_STATE))
+ (priv->devtype_data->quirks & (FLEXCAN_QUIRK_BROKEN_WERR_STATE |
+ FLEXCAN_QUIRK_BROKEN_PERR_STATE)))
flexcan_irq_state(dev, reg_esr);
/* bus error IRQ - handle if bus error reporting is activated */
@@ -775,6 +794,44 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
(priv->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING))
flexcan_irq_bus_err(dev, reg_esr);
+ /* availability of error interrupt among state transitions in case
+ * bus error reporting is de-activated and
+ * FLEXCAN_QUIRK_BROKEN_PERR_STATE is enabled:
+ * +--------------------------------------------------------------+
+ * | +----------------------------------------------+ [stopped / |
+ * | | | sleeping] -+
+ * +-+-> active <-> warning <-> passive -> bus off -+
+ * ___________^^^^^^^^^^^^_______________________________
+ * disabled(1) enabled disabled
+ *
+ * (1): enabled if FLEXCAN_QUIRK_BROKEN_WERR_STATE is enabled
+ */
+ if ((last_state != priv->can.state) &&
+ (priv->devtype_data->quirks & FLEXCAN_QUIRK_BROKEN_PERR_STATE) &&
+ !(priv->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING)) {
+ switch (priv->can.state) {
+ case CAN_STATE_ERROR_ACTIVE:
+ if (priv->devtype_data->quirks &
+ FLEXCAN_QUIRK_BROKEN_WERR_STATE)
+ flexcan_error_irq_enable(priv);
+ else
+ flexcan_error_irq_disable(priv);
+ break;
+
+ case CAN_STATE_ERROR_WARNING:
+ flexcan_error_irq_enable(priv);
+ break;
+
+ case CAN_STATE_ERROR_PASSIVE:
+ case CAN_STATE_BUS_OFF:
+ flexcan_error_irq_disable(priv);
+ break;
+
+ default:
+ break;
+ }
+ }
+
return handled;
}
--
2.14.2
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox