* Re: [PATCH net v2] net: systemport: Fix software statistics for SYSTEMPORT Lite
From: David Miller @ 2017-08-10 5:39 UTC (permalink / raw)
To: f.fainelli; +Cc: netdev
In-Reply-To: <20170808214509.26252-1-f.fainelli@gmail.com>
From: Florian Fainelli <f.fainelli@gmail.com>
Date: Tue, 8 Aug 2017 14:45:09 -0700
> With SYSTEMPORT Lite we have holes in our statistics layout that make us
> skip over the hardware MIB counters, bcm_sysport_get_stats() was not
> taking that into account resulting in reporting 0 for all SW-maintained
> statistics, fix this by skipping accordingly.
>
> Fixes: 44a4524c54af ("net: systemport: Add support for SYSTEMPORT Lite")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Applied, thanks.
^ permalink raw reply
* Regression: Bug 196547 - Since 4.12 - bonding module not working with wireless drivers
From: Kalle Valo @ 2017-08-10 5:39 UTC (permalink / raw)
To: Mahesh Bandewar, Andy Gospodarek
Cc: David Miller, netdev, linux-wireless, James Feeney
Hi Mahesh and Andy,
James Feeney reported that there's a serious regression in bonding
module since v4.12, it doesn't work with wireless drivers anymore as
wireless drivers don't report the link speed via ethtool:
https://bugzilla.kernel.org/show_bug.cgi?id=196547
In the bug report it's said that this commit is the culprit:
3f3c278c94dd bonding: fix active-backup transition
Is there a fix for this or should that commit be reverted? This seems to
be a serious regression as there are multiple reports already and we
should get it fixed for v4.13, and the fix backported to v4.12 stable
release.
--
Kalle Valo
^ permalink raw reply
* Re: [net 1/1] tipc: remove premature ESTABLISH FSM event at link synchronization
From: David Miller @ 2017-08-10 5:38 UTC (permalink / raw)
To: jon.maloy; +Cc: netdev, parthasarathy.bhuvaragan, ying.xue, tipc-discussion
In-Reply-To: <1502223836-3182-1-git-send-email-jon.maloy@ericsson.com>
From: Jon Maloy <jon.maloy@ericsson.com>
Date: Tue, 8 Aug 2017 22:23:56 +0200
> When a link between two nodes come up, both endpoints will initially
> send out a STATE message to the peer, to increase the probability that
> the peer endpoint also is up when the first traffic message arrives.
> Thereafter, if the establishing link is the second link between two
> nodes, this first "traffic" message is a TUNNEL_PROTOCOL/SYNCH message,
> helping the peer to perform initial synchronization between the two
> links.
>
> However, the initial STATE message may be lost, in which case the SYNCH
> message will be the first one arriving at the peer. This should also
> work, as the SYNCH message itself will be used to take up the link
> endpoint before initializing synchronization.
>
> Unfortunately the code for this case is broken. Currently, the link is
> brought up through a tipc_link_fsm_evt(ESTABLISHED) when a SYNCH
> arrives, whereupon __tipc_node_link_up() is called to distribute the
> link slots and take the link into traffic. But, __tipc_node_link_up() is
> itself starting with a test for whether the link is up, and if true,
> returns without action. Clearly, the tipc_link_fsm_evt(ESTABLISHED) call
> is unnecessary, since tipc_node_link_up() is itself issuing such an
> event, but also harmful, since it inhibits tipc_node_link_up() to
> perform the test of its tasks, and the link endpoint in question hence
> is never taken into traffic.
>
> This problem has been exposed when we set up dual links between pre-
> and post-4.4 kernels, because the former ones don't send out the
> initial STATE message described above.
>
> We fix this by removing the unnecessary event call.
>
> Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] ibmvnic: Correct 'unused variable' warning in build.
From: David Miller @ 2017-08-10 5:31 UTC (permalink / raw)
To: nfont; +Cc: netdev, jallen, tlfalcon
In-Reply-To: <20170808202618.32133.49772.stgit@ltcalpine2-lp23.aus.stglabs.ibm.com>
From: Nathan Fontenot <nfont@linux.vnet.ibm.com>
Date: Tue, 08 Aug 2017 15:26:18 -0500
> Commit a248878d7a1d ("ibmvnic: Check for transport event on driver resume")
> removed the loop to kick irqs on driver resume but didn't remove the now
> unused loop variable 'i'.
>
> Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] ibmvnic: Clean up resources on probe failure
From: David Miller @ 2017-08-10 5:31 UTC (permalink / raw)
To: nfont; +Cc: netdev, jallen, tlfalcon
In-Reply-To: <20170808192845.17535.99639.stgit@ltcalpine2-lp23.aus.stglabs.ibm.com>
From: Nathan Fontenot <nfont@linux.vnet.ibm.com>
Date: Tue, 08 Aug 2017 14:28:45 -0500
> Ensure that any resources allocated during probe are released if the
> probe of the driver fails.
>
> Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
Applied, but:
> +
> +ibmvnic_register_fail:
> + device_remove_file(&dev->dev, &dev_attr_failover);
> +
^^^^^^
That last line has trailing whitespace, which I had to fix up.
Please take care of this next time.
^ permalink raw reply
* Re: [PATCH v1 net] TCP_USER_TIMEOUT and tcp_keepalive should conform to RFC5482
From: Jerry Chu @ 2017-08-10 4:59 UTC (permalink / raw)
To: Rao Shoaib
Cc: David Miller, codesoldier1, Yuchung Cheng, Alexey Kuznetsov,
netdev@vger.kernel.org
In-Reply-To: <CAPshTCjs+Hie6xE6P10iB9ftLfhnpNJd0mEVdtSZ70gT6PVevQ@mail.gmail.com>
On Wed, Aug 9, 2017 at 8:32 PM, Jerry Chu <hkchu@google.com> wrote:
> On Wed, Aug 9, 2017 at 5:47 PM, Rao Shoaib <rao.shoaib@oracle.com> wrote:
>>
>>
>> On 08/09/2017 05:30 PM, David Miller wrote:
>>>
>>> From: Joe Smith <codesoldier1@gmail.com>
>>> Date: Wed, 9 Aug 2017 17:20:32 -0700
>>>
>>>> Making Linux conform to standards and behavior that is logical seems
>>>> like a good enough reason.
>>>
>>> That's an awesome attitude to have when we're implementing something
>>> new and don't have the facility already.
>>>
>>> But when we have something already the only important consideration is
>>> not breaking existing apps which rely on that behavior.
>>>
>>> That is much, much, more important than standards compliance.
>>>
>>> If users are confused, just fix the documentation.
>>
>> David,
>>
>> If it was just confusion than sure fixing the documentation is fine. What if
>> the logic is incorrect, does not conform to the standard that is says it is
>
> Not sure what part of logic is "incorrect" when it was a homegrown Linux API
> with no need to conform with any "standard"? Note that the new API was invented
> 7 years ago not out of need for RFC5482. In fact I initially call the option
> TCP_FAILFAST and did not even know the existence of RFC5482 until someone
> around the same time proposed a UTO option specifically for RFC5482 and I
> thought the two can be combined. (This is roughly the memory I can
> recollect so far.)
>
> So you see my focus back then was to devise a "failfast" option whereas RFC5482
> was meant for a "failslow" case. I think that explains why I let the
> option override
> keepalive so a TCP connection can "fail fast" while RFC5482 4.2 tries to prevent
> keepalive failure ahead of UTO, favoring "fail slow".
>
> If we start from a clean slate then perhaps one can argue the semantic
> either way
> but we do not have a clean slate. For that I still slightly favor not
> changing the code
> because the risk of breakage is definitely non-zero and the issue you're having
> seem to be only related to documentation.
One more thing - the proposed patch compares TCP_KEEPIDLE against
TCP_USER_TIMEOUT. But I don't think TCP_KEEPIDLE is what the
"keep-alive
timer" referred to in RFC5482. Linux keepalive implementation seems to use #
of retries (TCP_KEEPCNT) rather than time duration (keep-alive time) to
determine when to quit. If that is the case then your proposed change is not
fully "compliant" either and the best is probably just don't change.
>
> Jerry
>
>> implementing and easy to fix with little or no risk of breakage.
>>
>> The proposed patch changes a feature that no one uses. It also imposes the
>> relation ship between keepalive and timeout values that is required by the
>> RFC and make sense.
>>
>> You are the final authority, if you say we should just fix the documentation
>> than that is fine.
>>
>> Shoaib
>>
^ permalink raw reply
* [PATCH] bonding: require speed/duplex only for 802.3ad, alb and tlb
From: Andreas Born @ 2017-08-10 4:41 UTC (permalink / raw)
To: netdev; +Cc: Mahesh Bandewar, Andreas Born
The patch c4adfc822bf5 ("bonding: make speed, duplex setting consistent
with link state") puts the link state to down if
bond_update_speed_duplex() cannot retrieve speed and duplex settings.
Assumably the patch was written with 802.3ad mode in mind which relies
on link speed/duplex settings. For other modes like active-backup these
settings are not required. Thus, only for these other modes, this patch
reintroduces support for slaves that do not support reporting speed or
duplex such as wireless devices. This fixes the regression reported in
bug 196547 (https://bugzilla.kernel.org/show_bug.cgi?id=196547).
Fixes: c4adfc822bf5 ("bonding: make speed, duplex setting consistent
with link state")
Signed-off-by: Andreas Born <futur.andy@googlemail.com>
---
drivers/net/bonding/bond_main.c | 6 ++++--
include/net/bonding.h | 5 +++++
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 9bee6c1c70cc..85bb272d2a34 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1569,7 +1569,8 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
new_slave->delay = 0;
new_slave->link_failure_count = 0;
- if (bond_update_speed_duplex(new_slave))
+ if (bond_update_speed_duplex(new_slave) &&
+ bond_needs_speed_duplex(bond))
new_slave->link = BOND_LINK_DOWN;
new_slave->last_rx = jiffies -
@@ -2140,7 +2141,8 @@ static void bond_miimon_commit(struct bonding *bond)
continue;
case BOND_LINK_UP:
- if (bond_update_speed_duplex(slave)) {
+ if (bond_update_speed_duplex(slave) &&
+ bond_needs_speed_duplex(bond)) {
slave->link = BOND_LINK_DOWN;
netdev_warn(bond->dev,
"failed to get link speed/duplex for %s\n",
diff --git a/include/net/bonding.h b/include/net/bonding.h
index b00508d22e0a..b2e68657a216 100644
--- a/include/net/bonding.h
+++ b/include/net/bonding.h
@@ -277,6 +277,11 @@ static inline bool bond_is_lb(const struct bonding *bond)
BOND_MODE(bond) == BOND_MODE_ALB;
}
+static inline bool bond_needs_speed_duplex(const struct bonding *bond)
+{
+ return BOND_MODE(bond) == BOND_MODE_8023AD || bond_is_lb(bond);
+}
+
static inline bool bond_is_nondyn_tlb(const struct bonding *bond)
{
return (BOND_MODE(bond) == BOND_MODE_TLB) &&
--
2.14.0
^ permalink raw reply related
* Re: [PATCH v4 05/12] Documentation: net: phy: Add phy-is-internal binding
From: Chen-Yu Tsai @ 2017-08-10 3:58 UTC (permalink / raw)
To: Andrew Lunn
Cc: Florian Fainelli, Mark Rutland, Tao Huang, hwg,
Heiko Stübner, Arnd Bergmann, devicetree, Catalin Marinas,
Will Deacon, Russell King, linux-kernel,
open list:ARM/Rockchip SoC..., Rob Herring, David Wu,
Olof Johansson, Giuseppe Cavallaro, netdev, David Miller,
linux-arm-kernel
In-Reply-To: <20170810002041.GA10776@lunn.ch>
On Thu, Aug 10, 2017 at 8:20 AM, Andrew Lunn <andrew@lunn.ch> wrote:
> On Wed, Aug 09, 2017 at 03:47:34PM -0700, Florian Fainelli wrote:
>> On August 9, 2017 5:10:30 AM PDT, David Wu <david.wu@rock-chips.com> wrote:
>> >Add the documentation for internal phy. A boolean property
>> >indicates that a internal phy will be used.
>> >
>> >Signed-off-by: David Wu <david.wu@rock-chips.com>
>> >---
>> > Documentation/devicetree/bindings/net/phy.txt | 3 +++
>> > 1 file changed, 3 insertions(+)
>> >
>> >diff --git a/Documentation/devicetree/bindings/net/phy.txt
>> >b/Documentation/devicetree/bindings/net/phy.txt
>> >index b558576..942c892 100644
>> >--- a/Documentation/devicetree/bindings/net/phy.txt
>> >+++ b/Documentation/devicetree/bindings/net/phy.txt
>> >@@ -52,6 +52,9 @@ Optional Properties:
>> > Mark the corresponding energy efficient ethernet mode as broken and
>> > request the ethernet to stop advertising it.
>> >
>> >+- phy-is-internal: If set, indicates that phy will connect to the MAC
>> >as a
>> >+ internal phy.
>>
>> Something along the lines of:
>>
>> If set, indicates that the PHY is integrated into the same physical package as the Ethernet MAC.
>
> Hi Florian, David.
>
> I'm happy with the property name. But i think the text needs more
> description. We deal with Ethernet switches with integrated PHYs. Yet
> for us, this property is unneeded.
>
> Seeing this property means some bit of software needs to ensure the
> internal PHY should be used, when given the choice between an internal
> and external PHY. So i would say something like:
>
> If set, indicates that the PHY is integrated into the same
> physical package as the Ethernet MAC. If needed, muxers should be
> configured to ensure the internal PHY is used. The absence of this
> property indicates the muxers should be configured so that the
> external PHY is used.
>
> This last part is important. If the bootloader has set the internal
> PHY to be used, you need to reset it. Otherwise we are going to get
> into a mess sometime later and need to add a phy-is-external property.
Ack.
One other thing. We need to fix our (sunxi) binding which is already
in 4.13-rc1. We'd like to see this new property in netdev, i.e. merged
for 4.13, so we can use it.
Thanks
ChenYu
^ permalink raw reply
* Re:Re: [PATCH net-next 1/1] driver: pptp: Remove unnecessary statements in pptp_sock_destruct
From: Gao Feng @ 2017-08-10 3:54 UTC (permalink / raw)
To: Cong Wang; +Cc: xeb, David Miller, Linux Kernel Network Developers
In-Reply-To: <CAM_iQpUXRV6DamR_U1_bvBUBOk6SQ6jF3xLKBpVZrjYE74LXiw@mail.gmail.com>
At 2017-08-10 02:08:30, "Cong Wang" <xiyou.wangcong@gmail.com> wrote:
>On Wed, Aug 9, 2017 at 8:57 AM, <gfree.wind@vip.163.com> wrote:
>> From: Gao Feng <gfree.wind@vip.163.com>
>>
>> In the commit ddab82821fa6 ("ppp: Fix a scheduling-while-atomic bug in
>> del_chan"), I moved the synchronize_rcu() from del_chan() to pptp_release
>> after del_chan() to avoid one scheduling-while-atomic bug.
>>
>> Actually the del_chan() and pppox_unbind_sock are unneccessary in the
>> pptp_sock_destruct. Because the pptp sock refcnt wouldn't reach zero until
>> sk_state is set as PPPOX_DEAD in pptp_release. By that time, the del_chan()
>> and pppox_unbind_sock() have been invoked already and the condition check
>> "!(sk->sk_state & PPPOX_DEAD)" of this sock must be false in pptp_sock_destruct.
>
>I am not sure. The check for sock->sk in the beginning of pptp_release()
>indicates there could be a case we could skip del_chan() in pptp_release(),
>although I can't figure out how.
>
>Also there is a suspicious sock_put() in pptp_release().
Hi Cong,
Thank you. I also failed to find the case which causes the sock->sk is null in release().
There is a suspicious case in __sock_create following.
err = pf->create(net, sock, protocol, kern);
if (err < 0)
goto out_module_put;
.......
out_module_put:
sock->ops = NULL;
module_put(pf->owner);
out_sock_release:
sock_release(sock);
return err;
In the beginning, I thought when create is failed and the sock->sk is null, then the sock_release is invoked.
It could cause the sk is null in the release().
But I find it has already reset the sock->ops as NULL before sock_release later, so the release() wouldn't be invoked actually.
Best Regards
Feng
^ permalink raw reply
* Re: [PATCH v1 net] TCP_USER_TIMEOUT and tcp_keepalive should conform to RFC5482
From: Jerry Chu @ 2017-08-10 3:32 UTC (permalink / raw)
To: Rao Shoaib
Cc: David Miller, codesoldier1, Yuchung Cheng, Alexey Kuznetsov,
netdev@vger.kernel.org
In-Reply-To: <19024bb3-c06b-d004-5527-e4c54af66003@oracle.com>
On Wed, Aug 9, 2017 at 5:47 PM, Rao Shoaib <rao.shoaib@oracle.com> wrote:
>
>
> On 08/09/2017 05:30 PM, David Miller wrote:
>>
>> From: Joe Smith <codesoldier1@gmail.com>
>> Date: Wed, 9 Aug 2017 17:20:32 -0700
>>
>>> Making Linux conform to standards and behavior that is logical seems
>>> like a good enough reason.
>>
>> That's an awesome attitude to have when we're implementing something
>> new and don't have the facility already.
>>
>> But when we have something already the only important consideration is
>> not breaking existing apps which rely on that behavior.
>>
>> That is much, much, more important than standards compliance.
>>
>> If users are confused, just fix the documentation.
>
> David,
>
> If it was just confusion than sure fixing the documentation is fine. What if
> the logic is incorrect, does not conform to the standard that is says it is
Not sure what part of logic is "incorrect" when it was a homegrown Linux API
with no need to conform with any "standard"? Note that the new API was invented
7 years ago not out of need for RFC5482. In fact I initially call the option
TCP_FAILFAST and did not even know the existence of RFC5482 until someone
around the same time proposed a UTO option specifically for RFC5482 and I
thought the two can be combined. (This is roughly the memory I can
recollect so far.)
So you see my focus back then was to devise a "failfast" option whereas RFC5482
was meant for a "failslow" case. I think that explains why I let the
option override
keepalive so a TCP connection can "fail fast" while RFC5482 4.2 tries to prevent
keepalive failure ahead of UTO, favoring "fail slow".
If we start from a clean slate then perhaps one can argue the semantic
either way
but we do not have a clean slate. For that I still slightly favor not
changing the code
because the risk of breakage is definitely non-zero and the issue you're having
seem to be only related to documentation.
Jerry
> implementing and easy to fix with little or no risk of breakage.
>
> The proposed patch changes a feature that no one uses. It also imposes the
> relation ship between keepalive and timeout values that is required by the
> RFC and make sense.
>
> You are the final authority, if you say we should just fix the documentation
> than that is fine.
>
> Shoaib
>
^ permalink raw reply
* Re:Re: Re:Re: Re:Re: Re: [PATCH net] ppp: Fix a scheduling-while-atomic bug in del_chan
From: Gao Feng @ 2017-08-10 2:41 UTC (permalink / raw)
To: Cong Wang; +Cc: xeb, David Miller, Linux Kernel Network Developers
In-Reply-To: <CAM_iQpWTfV00qcda75f1Oh3LcFt9w8kPsp3HHF18vjO9mv+Dbg@mail.gmail.com>
At 2017-08-10 05:00:19, "Cong Wang" <xiyou.wangcong@gmail.com> wrote:
>On Wed, Aug 9, 2017 at 12:17 AM, Gao Feng <gfree.wind@vip.163.com> wrote:
>> Hi Cong,
>>
>> Actually I have one question about the SOCK_RCU_FREE.
>> I don't think it could resolve the issue you raised even though it exists really.
>>
>> I checked the SOCK_RCU_FREE, it just defer the __sk_destruct after one rcu period.
>> But when it performs, someone still could find this sock by callid during the del_chan period and it may still deference the sock which may freed soon.
>>
>> The right flow should be following.
>> del_chan()
>> wait a rcu period
>> sock_put() ------------ It is safe that someone gets the sock because it already hold sock refcnt.
>>
>> When using SOCK_RCU_FREE, its flow would be following.
>> wait a rcu period
>> del_chan()
>> free the sock directly -------- no sock refcnt check again.
>> Because the del_chan happens after rcu wait, not before, so it isn't helpful with SOCK_RCU_FREE.
>
>
>Yes, good point! With SOCK_RCU_FREE the sock_hold() should
>not be needed. For RCU, unpublish should indeed happen before
>grace period.
Sorry, I couldn't understand why sock_hold() isn't necessary with SOCK_RCU_FREE.
When lookup_chan finds the sock, it would return and reference it later.
If no refcnt, how to protect the sock ?
Best Regards
Feng
>
>
>>
>> I don't know if I misunderstand the SOCK_RCU_FREE usage.
>>
>> But it is a good news that the del_chan is only invoked in pptp_release actually and it would wait a rcu period before sock_put.
>>
>
>Looking at the code again, the reader lookup_chan() is actually
>invoked in BH context, but neither add_chan() nor del_chan()
>actually disables BH...
^ permalink raw reply
* Re: [PATCH v3 05/11] net: stmmac: dwmac-rk: Add internal phy support
From: Chen-Yu Tsai @ 2017-08-10 2:40 UTC (permalink / raw)
To: David.Wu
Cc: Corentin Labbe, Florian Fainelli, Andrew Lunn, David Miller,
Heiko Stübner, Rob Herring, Mark Rutland, Catalin Marinas,
Will Deacon, Olof Johansson, Russell King, Arnd Bergmann,
Tao Huang, hwg-TNX95d0MmH7DzftRWevZcw,
alexandre.torgue-qxv4g6HH51o, devicetree, netdev, linux-kernel,
open list:ARM/Rockchip SoC..., Giuseppe Cavallaro
In-Reply-To: <a9fae87c-9df5-a250-5bc1-230be4af15cd-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
Hi David,
On Wed, Aug 9, 2017 at 5:38 PM, David.Wu <david.wu-TNX95d0MmH7DzftRWevZcw@public.gmane.org> wrote:
> Hello Corentin, Chen-Yu
>
>
> 在 2017/8/9 16:45, Corentin Labbe 写道:
>>
>> On Thu, Aug 03, 2017 at 07:06:33PM +0800, Chen-Yu Tsai wrote:
>>>
>>> On Thu, Aug 3, 2017 at 1:38 AM, Florian Fainelli <f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>> wrote:
>>>>
>>>> On 08/01/2017 11:21 PM, David Wu wrote:
>>>>>
>>>>> To make internal phy work, need to configure the phy_clock,
>>>>> phy cru_reset and related registers.
>>>>>
>>>>> Signed-off-by: David Wu <david.wu-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
>>>>> ---
>>>>> .../devicetree/bindings/net/rockchip-dwmac.txt | 6 +-
>>>>> drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 81
>>>>> ++++++++++++++++++++++
>>>>> 2 files changed, 86 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/net/rockchip-dwmac.txt
>>>>> b/Documentation/devicetree/bindings/net/rockchip-dwmac.txt
>>>>> index 8f42755..ec39b31 100644
>>>>> --- a/Documentation/devicetree/bindings/net/rockchip-dwmac.txt
>>>>> +++ b/Documentation/devicetree/bindings/net/rockchip-dwmac.txt
>>>>> @@ -25,7 +25,8 @@ Required properties:
>>>>> - clock-names: One name for each entry in the clocks property.
>>>>> - phy-mode: See ethernet.txt file in the same directory.
>>>>> - pinctrl-names: Names corresponding to the numbered pinctrl states.
>>>>> - - pinctrl-0: pin-control mode. can be <&rgmii_pins> or <&rmii_pins>.
>>>>> + - pinctrl-0: pin-control mode. can be <&rgmii_pins>, <&rmii_pins> or
>>>>> led pins
>>>>> + for internal phy mode.
>>>>> - clock_in_out: For RGMII, it must be "input", means main
>>>>> clock(125MHz)
>>>>> is not sourced from SoC's PLL, but input from PHY; For RMII,
>>>>> "input" means
>>>>> PHY provides the reference clock(50MHz), "output" means GMAC
>>>>> provides the
>>>>> @@ -40,6 +41,9 @@ Optional properties:
>>>>> - tx_delay: Delay value for TXD timing. Range value is 0~0x7F, 0x30
>>>>> as default.
>>>>> - rx_delay: Delay value for RXD timing. Range value is 0~0x7F, 0x10
>>>>> as default.
>>>>> - phy-supply: phandle to a regulator if the PHY needs one
>>>>> + - clocks: <&cru MAC_PHY>: Clock selector for internal macphy
>>>>> + - phy-is-internal: A boolean property allows us to know that MAC will
>>>>> connect to
>>>>> + internal phy.
>>>>
>>>>
>>>> This is incorrect in two ways:
>>>>
>>>> - this is a property of the PHY, so it should be documented as such in
>>>> Documentation/devicetree/bindings/net/phy.txt so other bindings can
>>>> re-use it
>>>>
>>>> - if it was specific to your MAC you would expect a vendor prefix to
>>>> this property, which is not there
>>>>
>>>> An alternative way to implement the external/internal logic selection
>>>> would be mandate that your Ethernet PHY node have a compatible string
>>>> like this:
>>>>
>>>> phy@0 {
>>>> compatible = "ethernet-phy-id1234.d400",
>>>> "ethernet-phy-802.3-c22";
>>>> };
>>>>
>>>> Then you don't need this phy-is-internal property, because you can
>>>> locate the PHY node by the phy-handle (see more about that in a reply to
>>>> patch 10) and you can determine ahead of time whether this PHY is
>>>> internal or not based on its compatible string.
>>>
>>>
>>> This doesn't really work for us (sunxi). The "internal" PHY of the H3
>>> is also available in the X-Powers AC200 combo chip, which would be
>>> external. Same ID. So if someone were to be stupid and put the two
>>> together, you wouldn't know which one it actually is. Generically
>>> it doesn't make sense to match against the ID either. The PHY is
>>> only integrated or inlined into the SoC, meaning it could be moved
>>> elsewhere or even be a discreet part.
>>>
>>> The way I see it is more like a reversed pinmux. The system should
>>> select either the internal set or external set of xMII pins to use.
>>>
>>> A phy-is-internal property in the PHY node would work for us. We
>>> already need a PHY node to describe its clocks and resets.
>>>
>>> A side note to this solution is that, since the internal PHY is defined
>>> at the .dtsi level, any external PHYs at the same address would need to
>>> make sure to delete the property, which is kind of counterintuitive, but
>>> it is how device tree works. One would want to put the internal PHY's
>>> address, assuming it is configurable, on something that is rarely used.
>>>
>>
>> Hello David, Florian, Andrew
>>
>> Could someone ack this ? or nack if you think that the chance for having
>> two PHY id both internal and external is too low.
>> Anyway, we need a choice.
>>
>
> I think we should be specific to the situation, for us we have the
> possibility that the Mac only picks up internal PHY, so this can be fixed at
> the. DTSi level, also possible INTERNL PHY's Mac can also be used to connect
> external PHY, while cutting off the connection with the internal PHY, so we
> should define the internal PHY at the DTS level, so I think Florian's
> approach is acceptable.
So it looks like you have the clock for the internal/integrated PHY at the
MAC level. I think this lets you define/add the PHY at the board level more
easily without a lot of duplication?
Sunxi has the clock and reset in the PHY node, which is defined in the dtsi
file. (This part is already done.)
ChenYu
--
To unsubscribe from this list: send the line "unsubscribe devicetree" 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 net-next 0/2] zerocopy fixes
From: David Ahern @ 2017-08-10 2:35 UTC (permalink / raw)
To: Willem de Bruijn, netdev; +Cc: davem, Willem de Bruijn
In-Reply-To: <20170809230944.59289-1-willemdebruijn.kernel@gmail.com>
On 8/9/17 5:09 PM, Willem de Bruijn wrote:
> From: Willem de Bruijn <willemb@google.com>
>
> Fix two issues introduced in the msg_zerocopy patchset.
>
> Willem de Bruijn (2):
> sock: fix zerocopy panic in mem accounting
> sock: fix zerocopy_success regression with msg_zerocopy
>
> include/linux/skbuff.h | 9 +++++++--
> net/core/skbuff.c | 4 ++--
> 2 files changed, 9 insertions(+), 4 deletions(-)
>
Fixes the problem I reported. Thanks for the quick turn around.
^ permalink raw reply
* Re: [PATCH 0/3] Fix y2038 issues for security/keys subsystem
From: Baolin Wang @ 2017-08-10 2:00 UTC (permalink / raw)
To: Arnd Bergmann
Cc: David Howells, David Miller, James Morris, Serge E. Hallyn,
marc.dionne, Dan Carpenter, Jason A. Donenfeld, Mark Brown,
keyrings, Linux Kernel Mailing List, LSM List, Networking
In-Reply-To: <CAK8P3a1erR4ToZTH09p_KcNo5cNbOuUpmZG9ststD=gf6zfwsw@mail.gmail.com>
Hi Arnd,
On 9 August 2017 at 16:44, Arnd Bergmann <arnd@arndb.de> wrote:
> On Wed, Aug 9, 2017 at 4:51 AM, Baolin Wang <baolin.wang@linaro.org> wrote:
>> Since 'time_t', 'timeval' and 'timespec' types are not year 2038 safe on
>> 32 bits system, this patchset tries to fix this issues for security/keys
>> subsystem and net/rxrpc subsystem which is connected with security/keys
>> subsystem.
>>
>> Baolin Wang (3):
>> security: keys: Replace time_t/timespec with time64_t
>> security: keys: Replace time_t with time64_t for struct
>> key_preparsed_payload
>> net: rxrpc: Replace time_t type with time64_t type
>
> Hi David,
>
> I did a private review before Baolin posted these patches, this version look
> correct to me, though I would like to see some clarification from you for the
> rxrpc portion, I'll reply there separately.
>
> All three patches
>
> Reviewed-by: Arnd Bergmann <arnd@arndb.de>
Thanks for your reviewed tag.
--
Baolin.wang
Best Regards
^ permalink raw reply
* Re: [PATCH 0/3] Fix y2038 issues for security/keys subsystem
From: Baolin Wang @ 2017-08-10 1:59 UTC (permalink / raw)
To: David Howells
Cc: David Miller, James Morris, Serge E. Hallyn, marc.dionne,
dan.carpenter, Jason, Arnd Bergmann, Mark Brown, keyrings, LKML,
linux-security-module, netdev
In-Reply-To: <4455.1502267323@warthog.procyon.org.uk>
On 9 August 2017 at 16:28, David Howells <dhowells@redhat.com> wrote:
> The rxrpc patch isn't part of the security/keys subsystem. I'll push it
> to the network tree. The other two I'll push to James.
Thanks David.
--
Baolin.wang
Best Regards
^ permalink raw reply
* Re: unregister_netdevice: waiting for eth0 to become free. Usage count = 1
From: Wei Wang @ 2017-08-10 1:36 UTC (permalink / raw)
To: John Stultz
Cc: Cong Wang, lkml, Network Development, Linux USB List,
David S. Miller, Felipe Balbi
In-Reply-To: <CALAqxLXY-kE2N__eDq514cLycR0-QRpyOCMWpdttO6fgovE2rg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Wed, Aug 9, 2017 at 6:26 PM, John Stultz <john.stultz-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> On Wed, Aug 9, 2017 at 5:36 PM, Wei Wang <weiwan-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> wrote:
>> On Wed, Aug 9, 2017 at 4:44 PM, John Stultz <john.stultz-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>>> On Wed, Aug 9, 2017 at 4:34 PM, Cong Wang <xiyou.wangcong-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>> (Cc'ing Wei whose commit was blamed)
>>>>
>>>> On Mon, Aug 7, 2017 at 2:15 PM, John Stultz <john.stultz-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>>>>> On Mon, Aug 7, 2017 at 2:05 PM, John Stultz <john.stultz-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>>>>>> So, with recent testing with my HiKey board, I've been noticing some
>>>>>> quirky behavior with my USB eth adapter.
>>>>>>
>>>>>> Basically, pluging the usb eth adapter in and then removing it, when
>>>>>> plugging it back in I often find that its not detected, and the system
>>>>>> slowly spits out the following message over and over:
>>>>>> unregister_netdevice: waiting for eth0 to become free. Usage count = 1
>>>>>
>>>>> The other bit is that after this starts printing, the board will no
>>>>> longer reboot (it hangs continuing to occasionally print the above
>>>>> message), and I have to manually reset the device.
>>>>>
>>>>
>>>> So this warning is not temporarily shown but lasts until a reboot,
>>>> right? If so it is a dst refcnt leak.
>>>
>>> Correct, once I get into the state it lasts until a reboot.
>>>
>>>> How reproducible is it for you? From my reading, it seems always
>>>> reproduced when you unplug and plug your usb eth interface?
>>>> Is there anything else involved? For example, network namespace.
>>>
>>> So with 4.13-rc3/4 I seem to trigger it easily, often with the first
>>> unplug of the USB eth adapter.
>>>
>>> But as I get back closer to 4.12, it seemingly becomes harder to
>>> trigger, but sometimes still happens.
>>>
>>> So far, I've not been able to trigger it with 4.12.
>>>
>>> I don't think network namespaces are involved? Though its out of my
>>> area, so AOSP may be using them these days. Is there a simple way to
>>> check?
>>>
>>> I'll also do another bisection to see if the bad point moves back any further.
>
> So I went through another bisection around and got 9514528d92d4 ipv6:
> call dst_dev_put() properly as the first bad commit again.
>
>> If you see the problem starts to happen on commit
>> 9514528d92d4cbe086499322370155ed69f5d06c, could you try reverting all
>> the following commits:
>> (from new to old)
>> 1eb04e7c9e63 net: reorder all the dst flags
>> a4c2fd7f7891 net: remove DST_NOCACHE flag
>> b2a9c0ed75a3 net: remove DST_NOGC flag
>> 5b7c9a8ff828 net: remove dst gc related code
>> db916649b5dd ipv6: get rid of icmp6 dst garbage collector
>> 587fea741134 ipv6: mark DST_NOGC and remove the operation of dst_free()
>> ad65a2f05695 ipv6: call dst_hold_safe() properly
>> 9514528d92d4 ipv6: call dst_dev_put() properly
>
>
> And reverting this set off of 4.13-rc4 seems to make the issue go away.
>
> Is there anything I can test to help narrow down the specific problem
> with that patchset?
>
Thanks John for confirming.
Let me spend some time on the commits and I will let you know if I
have some debug image for you to try.
Wei
> thanks
> -john
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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 0/7] rtnetlink: allow selected handlers to run without rtnl
From: David Ahern @ 2017-08-10 1:27 UTC (permalink / raw)
To: David Miller, fw; +Cc: netdev
In-Reply-To: <20170809.172114.233866755819084584.davem@davemloft.net>
On 8/9/17 6:21 PM, David Miller wrote:
>
> Ok series applied, let's see where this goes :-)
>
1 hour in, 1 problem reported
^ permalink raw reply
* Re: unregister_netdevice: waiting for eth0 to become free. Usage count = 1
From: John Stultz @ 2017-08-10 1:26 UTC (permalink / raw)
To: Wei Wang
Cc: Cong Wang, lkml, Network Development, Linux USB List,
David S. Miller, Felipe Balbi
In-Reply-To: <CAEA6p_AL8uUghtZgp4OmqygcFs-Jiy286P5r6tRd+yeSWxR3KA@mail.gmail.com>
On Wed, Aug 9, 2017 at 5:36 PM, Wei Wang <weiwan@google.com> wrote:
> On Wed, Aug 9, 2017 at 4:44 PM, John Stultz <john.stultz@linaro.org> wrote:
>> On Wed, Aug 9, 2017 at 4:34 PM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>>> (Cc'ing Wei whose commit was blamed)
>>>
>>> On Mon, Aug 7, 2017 at 2:15 PM, John Stultz <john.stultz@linaro.org> wrote:
>>>> On Mon, Aug 7, 2017 at 2:05 PM, John Stultz <john.stultz@linaro.org> wrote:
>>>>> So, with recent testing with my HiKey board, I've been noticing some
>>>>> quirky behavior with my USB eth adapter.
>>>>>
>>>>> Basically, pluging the usb eth adapter in and then removing it, when
>>>>> plugging it back in I often find that its not detected, and the system
>>>>> slowly spits out the following message over and over:
>>>>> unregister_netdevice: waiting for eth0 to become free. Usage count = 1
>>>>
>>>> The other bit is that after this starts printing, the board will no
>>>> longer reboot (it hangs continuing to occasionally print the above
>>>> message), and I have to manually reset the device.
>>>>
>>>
>>> So this warning is not temporarily shown but lasts until a reboot,
>>> right? If so it is a dst refcnt leak.
>>
>> Correct, once I get into the state it lasts until a reboot.
>>
>>> How reproducible is it for you? From my reading, it seems always
>>> reproduced when you unplug and plug your usb eth interface?
>>> Is there anything else involved? For example, network namespace.
>>
>> So with 4.13-rc3/4 I seem to trigger it easily, often with the first
>> unplug of the USB eth adapter.
>>
>> But as I get back closer to 4.12, it seemingly becomes harder to
>> trigger, but sometimes still happens.
>>
>> So far, I've not been able to trigger it with 4.12.
>>
>> I don't think network namespaces are involved? Though its out of my
>> area, so AOSP may be using them these days. Is there a simple way to
>> check?
>>
>> I'll also do another bisection to see if the bad point moves back any further.
So I went through another bisection around and got 9514528d92d4 ipv6:
call dst_dev_put() properly as the first bad commit again.
> If you see the problem starts to happen on commit
> 9514528d92d4cbe086499322370155ed69f5d06c, could you try reverting all
> the following commits:
> (from new to old)
> 1eb04e7c9e63 net: reorder all the dst flags
> a4c2fd7f7891 net: remove DST_NOCACHE flag
> b2a9c0ed75a3 net: remove DST_NOGC flag
> 5b7c9a8ff828 net: remove dst gc related code
> db916649b5dd ipv6: get rid of icmp6 dst garbage collector
> 587fea741134 ipv6: mark DST_NOGC and remove the operation of dst_free()
> ad65a2f05695 ipv6: call dst_hold_safe() properly
> 9514528d92d4 ipv6: call dst_dev_put() properly
And reverting this set off of 4.13-rc4 seems to make the issue go away.
Is there anything I can test to help narrow down the specific problem
with that patchset?
thanks
-john
^ permalink raw reply
* Re:Re: Re: Re:Re: Re: [PATCH net] ppp: Fix a scheduling-while-atomic bug in del_chan
From: Gao Feng @ 2017-08-10 1:25 UTC (permalink / raw)
To: Cong Wang; +Cc: xeb, David Miller, Linux Kernel Network Developers
In-Reply-To: <CAM_iQpU3hx-Y7OXTDWARFc08BE6qQ_cEjZb7ciVKrWPFh2j0JA@mail.gmail.com>
At 2017-08-10 02:18:44, "Cong Wang" <xiyou.wangcong@gmail.com> wrote:
>On Tue, Aug 8, 2017 at 10:13 PM, Gao Feng <gfree.wind@vip.163.com> wrote:
>> Maybe I didn't show my explanation clearly.
>> I think it won't happen as I mentioned in the last email.
>> Because the pptp_release invokes the synchronize_rcu to make sure it, and actually there is no one which would invoke del_chan except pptp_release.
>> It is guaranteed by that the pptp_release doesn't put the sock refcnt until complete all cleanup include marking sk_state as PPPOX_DEAD.
>>
>> In other words, even though the pptp_release is not the last user of this sock, the other one wouldn't invoke del_chan in pptp_sock_destruct.
>> Because the condition "!(sk->sk_state & PPPOX_DEAD)" must be false.
>
>Only if sock->sk is always non-NULL for pptp_release(), which
>is what I am not sure. If you look at other ->release(), similar checks
>are there too, so not just for pptp.
Yes. It seems only if the release() is invoked twice, the sock->sk would be NULL.
But I don't find there is any case which could cause it.
>
>>
>> As summary, the del_chan and pppox_unbind_sock in pptp_sock_destruct are unnecessary.
>> And it even brings confusing.
>
>Sorry, I can't draw any conclusion for this.
Thank you all the same, and I have learn a lot from you :)
Wish someone which is familiar with these codes could give more details and explanations.
Best Regards
Feng
^ permalink raw reply
* RTNL: assertion failed at ...
From: David Ahern @ 2017-08-10 1:18 UTC (permalink / raw)
To: Florian Westphal, netdev@vger.kernel.org
Florian:
Booting top of tree on my host with a VRF configured is spewing traces:
[ 24.779911] RTNL: assertion failed at
/home/dsa/kernel.git/net/core/dev.c (5717)
[ 24.779984] CPU: 3 PID: 989 Comm: ip Not tainted
4.13.0-rc4-01020-gcd9cb3890b20 #8
[ 24.779986] Hardware name: Supermicro X9DAi/X9DAi, BIOS 1.0b 10/17/2012
[ 24.779988] Call Trace:
[ 24.780002] dump_stack+0x85/0xc7
[ 24.780009] netdev_master_upper_dev_get+0x5f/0x70
[ 24.780017] if_nlmsg_size+0x158/0x240
[ 24.780021] rtnl_calcit.isra.26+0xa3/0xf0
[ 24.780029] ? dump_rules+0xd0/0xd0
[ 24.780033] rtnetlink_rcv_msg+0x1f0/0x230
[ 24.780037] ? rtnl_calcit.isra.26+0xf0/0xf0
[ 24.780043] netlink_rcv_skb+0xec/0x120
[ 24.780047] rtnetlink_rcv+0x15/0x20
[ 24.780050] netlink_unicast+0x16a/0x210
[ 24.780054] netlink_sendmsg+0x2b8/0x3a0
[ 24.780060] sock_sendmsg+0x38/0x50
[ 24.780063] SYSC_sendto+0x102/0x190
[ 24.780069] ? handle_mm_fault+0xe8/0x210
[ 24.780075] ? __do_page_fault+0x1f6/0x4a0
[ 24.780080] ? trace_hardirqs_on_thunk+0x1a/0x1c
[ 24.780083] SyS_sendto+0xe/0x10
[ 24.780089] entry_SYSCALL_64_fastpath+0x23/0xbd
[ 24.780117] RIP: 0033:0x7fc3972d95ed
[ 24.780119] RSP: 002b:00007fff9fbd4528 EFLAGS: 00000246 ORIG_RAX:
000000000000002c
[ 24.780121] RAX: ffffffffffffffda RBX: 000000000044bd00 RCX:
00007fc3972d95ed
[ 24.780122] RDX: 0000000000000028 RSI: 00007fff9fbd4530 RDI:
0000000000000003
[ 24.780123] RBP: 00007fff9fbd5f77 R08: 0000000000000000 R09:
0000000000000000
[ 24.780124] R10: 0000000000000000 R11: 0000000000000246 R12:
0000000000000002
[ 24.780125] R13: 00007fff9fbd4750 R14: 0000000000000000 R15:
0000000000000000
I did not do a bisect but seems pretty obvious that it has to be your
patch set.
David
^ permalink raw reply
* Re: [PATCH net-next] net: skb_needs_check() removes CHECKSUM_NONE check for tx.
From: Tonghao Zhang @ 2017-08-10 1:02 UTC (permalink / raw)
To: Willem de Bruijn; +Cc: Network Development, Eric Dumazet, Willem de Bruijn
In-Reply-To: <CAF=yD-JjaDFy4pOeJ5Lh2em9MgK6jjNxSfc5MkZZ=L6cu9be4Q@mail.gmail.com>
Thanks for your work.
On Thu, Aug 10, 2017 at 2:30 AM, Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
> On Wed, Aug 9, 2017 at 5:04 AM, Tonghao Zhang <xiangxia.m.yue@gmail.com> wrote:
>> This patch reverts the commit 6e7bc478c9a0
>> ("net: skb_needs_check() accepts CHECKSUM_NONE for tx"),
>> because we removed the UFO support.
>>
>> Cc: Eric Dumazet <edumazet@google.com>
>> Cc: Willem de Bruijn <willemb@google.com>
>> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>
>
> I would wait until net is merged into net-next. This will cause a conflict.
>
> Also, while logically equivalent, it is not a real revert (as in `git
> revert $SHA1`) of that patch.
>
> Aside from those concerns, I agree that the original patch is no
> longer needed now that UFO is reverted.
^ permalink raw reply
* Re: [PATCH v2 3/4] can: m_can: Update documentation to mention new fixed transceiver binding
From: Franklin S Cooper Jr @ 2017-08-10 1:02 UTC (permalink / raw)
To: Rob Herring
Cc: linux-kernel, devicetree, netdev, linux-can, wg, mkl,
quentin.schulz, dev.kurt, andrew, sergei.shtylyov, socketcan
In-Reply-To: <20170803170735.qntnxpx5olghzmbu@rob-hp-laptop>
Hi Rob,
On 08/03/2017 12:07 PM, Rob Herring wrote:
> On Mon, Jul 24, 2017 at 06:05:20PM -0500, Franklin S Cooper Jr wrote:
>> Add information regarding fixed transceiver binding. This is especially
>> important for MCAN since the IP allows CAN FD mode to run significantly
>> faster than what most transceivers are capable of.
>>
>> Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
>> ---
>> Version 2 changes:
>> Drop unit address
>>
>> Documentation/devicetree/bindings/net/can/m_can.txt | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/net/can/m_can.txt b/Documentation/devicetree/bindings/net/can/m_can.txt
>> index 9e33177..e4abd2c 100644
>> --- a/Documentation/devicetree/bindings/net/can/m_can.txt
>> +++ b/Documentation/devicetree/bindings/net/can/m_can.txt
>> @@ -43,6 +43,11 @@ Required properties:
>> Please refer to 2.4.1 Message RAM Configuration in
>> Bosch M_CAN user manual for details.
>>
>> +Optional properties:
>> +- fixed-transceiver : Fixed-transceiver subnode describing maximum speed
>
> This is a node, not a property. Sub nodes should have their own section.
Fixed in my v4 that I just sent.
>
>> + that can be used for CAN and/or CAN-FD modes. See
>> + Documentation/devicetree/bindings/net/can/fixed-transceiver.txt
>> + for details.
>> Example:
>> SoC dtsi:
>> m_can1: can@020e8000 {
>> @@ -64,4 +69,9 @@ Board dts:
>> pinctrl-names = "default";
>> pinctrl-0 = <&pinctrl_m_can1>;
>> status = "enabled";
>> +
>> + fixed-transceiver {
>> + max-arbitration-speed = <1000000>;
>> + max-data-speed = <5000000>;
>> + };
>> };
>> --
>> 2.10.0
>>
^ permalink raw reply
* Re: [PATCH v3 2/4] dt-bindings: can: fixed-transceiver: Add new CAN fixed transceiver bindings
From: Franklin S Cooper Jr @ 2017-08-10 1:00 UTC (permalink / raw)
To: Sergei Shtylyov, linux-kernel, devicetree, netdev, linux-can, wg,
mkl, robh+dt, quentin.schulz, dev.kurt, andrew, socketcan
In-Reply-To: <855a344e-46c4-76c9-331a-848b3a79bf6e@ti.com>
Hi Sergei,
On 08/03/2017 10:38 AM, Franklin S Cooper Jr wrote:
>
>
> On 08/03/2017 07:22 AM, Sergei Shtylyov wrote:
>> On 08/03/2017 12:48 PM, Franklin S Cooper Jr wrote:
>>
>>>>> Add documentation to describe usage of the new fixed transceiver
>>>>> binding.
>>>>> This new binding is applicable for any CAN device therefore it
>>>>> exists as
>>>>> its own document.
>>>>>
>>>>> Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
>>>>> ---
>>>>> .../bindings/net/can/fixed-transceiver.txt | 24
>>>>> ++++++++++++++++++++++
>>>>> 1 file changed, 24 insertions(+)
>>>>> create mode 100644
>>>>> Documentation/devicetree/bindings/net/can/fixed-transceiver.txt
>>>>>
>>>>> diff --git
>>>>> a/Documentation/devicetree/bindings/net/can/fixed-transceiver.txt
>>>>> b/Documentation/devicetree/bindings/net/can/fixed-transceiver.txt
>>>>> new file mode 100644
>>>>> index 0000000..2f58838b
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/net/can/fixed-transceiver.txt
>>>>> @@ -0,0 +1,24 @@
>>>>> +Fixed transceiver Device Tree binding
>>>>> +------------------------------
>>>>> +
>>>>> +CAN transceiver typically limits the max speed in standard CAN and
>>>>> CAN FD
>>>>> +modes. Typically these limitations are static and the transceivers
>>>>> themselves
>>>>> +provide no way to detect this limitation at runtime. For this
>>>>> situation,
>>>>> +the "fixed-transceiver" node can be used.
>>>>> +
>>>>> +Required Properties:
>>>>> + max-bitrate: a positive non 0 value that determines the max
>>>>> + speed that CAN/CAN-FD can run. Any other value
>>>>> + will be ignored.
>>>>> +
>>>>> +Examples:
>>>>> +
>>>>> +Based on Texas Instrument's TCAN1042HGV CAN Transceiver
>>>>> +
>>>>> +m_can0 {
>>>>> + ....
>>>>> + fixed-transceiver@0 {
>>>>
>>>> The <unit-address> (after @) must only be specified if there's "reg"
>>>
>>> Sorry. Fixed this in my v2 and some how it came back. Will fix.
>>>
>>>> prop in the device node. Also, please name the node "can-transceiver@"
>>>> to be more in line with the DT spec. which requires generic node names.
>>>
>>> Its possible for future can transceivers drivers to be created. So I
>>
>> So what? Ah, you are using the node name to match in the CAN drivers...
>>
>>> thought including fixed was important to indicate that this is a "dumb"
>>> transceiver similar to "fixed-link".
>>
>> I'm not sure the "fixed-link" MAC subnode assumed any transceiver at
>> all...
>
> Your right. I wasn't trying to imply that it does. What I meant was that
> having a node named "can-transceiver" may be a bit confusing in the
> future if can transceiver drivers are created. Prefix of "fixed" atleast
> to me makes it clear that this is something unique or a generic
> transceiver with limitations. Similar to "fixed-link" which is for MACs
> not connected to MDIO managed phy. Calling this subnode
> "can-transceiver" to me would be like renaming "fixed-link" to "phy".
>
>>
>>> So would "fixed-can-transceiver" be
>>> ok or do you want to go with can-transceiver?
>>
>> I'm somewhat perplexed at this point...
>
> If my reasoning still didn't change your views then I'll make the switch.
I went ahead and made your suggested switch in my v4. Thanks for taking
the time to review this series.
>>
>> MBR, Sergei
^ permalink raw reply
* [PATCH v4 4/4] can: m_can: Add call to of_can_transceiver
From: Franklin S Cooper Jr @ 2017-08-10 0:59 UTC (permalink / raw)
To: linux-kernel, devicetree, netdev, linux-can, wg, mkl, robh+dt,
quentin.schulz, dev.kurt, andrew, sergei.shtylyov, socketcan
Cc: Franklin S Cooper Jr
In-Reply-To: <20170810005916.27163-1-fcooper@ti.com>
Add call to new generic functions that provides support via a binding
to limit the arbitration rate and/or data rate imposed by the physical
transceiver connected to the MCAN peripheral.
Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
---
drivers/net/can/m_can/m_can.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index f4947a7..f72116e 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -1649,6 +1649,8 @@ static int m_can_plat_probe(struct platform_device *pdev)
devm_can_led_init(dev);
+ of_can_transceiver(dev);
+
dev_info(&pdev->dev, "%s device registered (irq=%d, version=%d)\n",
KBUILD_MODNAME, dev->irq, priv->version);
--
2.9.4.dirty
^ permalink raw reply related
* [PATCH v4 3/4] dt-bindings: can: m_can: Document new can transceiver binding
From: Franklin S Cooper Jr @ 2017-08-10 0:59 UTC (permalink / raw)
To: linux-kernel, devicetree, netdev, linux-can, wg, mkl, robh+dt,
quentin.schulz, dev.kurt, andrew, sergei.shtylyov, socketcan
Cc: Franklin S Cooper Jr
In-Reply-To: <20170810005916.27163-1-fcooper@ti.com>
Add information regarding can-transceiver binding. This is especially
important for MCAN since the IP allows CAN FD mode to run significantly
faster than what most transceivers are capable of.
Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
---
Drop unit address.
Switch from using fixed-transceiver to can-transceiver
Indicate that can-transceiver is an optional subnode not a property.
Documentation/devicetree/bindings/net/can/m_can.txt | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/Documentation/devicetree/bindings/net/can/m_can.txt b/Documentation/devicetree/bindings/net/can/m_can.txt
index 9e33177..ee90aac 100644
--- a/Documentation/devicetree/bindings/net/can/m_can.txt
+++ b/Documentation/devicetree/bindings/net/can/m_can.txt
@@ -43,6 +43,11 @@ Required properties:
Please refer to 2.4.1 Message RAM Configuration in
Bosch M_CAN user manual for details.
+Optional Subnode:
+- can-transceiver : Can-transceiver subnode describing maximum speed
+ that can be used for CAN/CAN-FD modes. See
+ Documentation/devicetree/bindings/net/can/can-transceiver.txt
+ for details.
Example:
SoC dtsi:
m_can1: can@020e8000 {
@@ -64,4 +69,8 @@ Board dts:
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_m_can1>;
status = "enabled";
+
+ can-transceiver@ {
+ max-bitrate = <5000000>;
+ };
};
--
2.9.4.dirty
^ 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