* Re: [PATCH net-next v2 1/3] virtio_net: propagate linkspeed/duplex settings from the hypervisor
From: Michael S. Tsirkin @ 2018-01-03 14:01 UTC (permalink / raw)
To: Jason Baron; +Cc: netdev, virtualization, qemu-devel, jasowang
In-Reply-To: <44da522ecee60792ec918234ee4d61a84e4574f0.1513974243.git.jbaron@akamai.com>
On Fri, Dec 22, 2017 at 04:54:01PM -0500, Jason Baron wrote:
> The ability to set speed and duplex for virtio_net in useful in various
> scenarios as described here:
>
> 16032be virtio_net: add ethtool support for set and get of settings
>
> However, it would be nice to be able to set this from the hypervisor,
> such that virtio_net doesn't require custom guest ethtool commands.
>
> Introduce a new feature flag, VIRTIO_NET_F_SPEED_DUPLEX, which allows
> the hypervisor to export a linkspeed and duplex setting. The user can
> subsequently overwrite it later if desired via: 'ethtool -s'.
>
> Signed-off-by: Jason Baron <jbaron@akamai.com>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Jason Wang <jasowang@redhat.com>
> ---
> drivers/net/virtio_net.c | 17 ++++++++++++++++-
> include/uapi/linux/virtio_net.h | 5 +++++
> 2 files changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 511f833..4168d82 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -2621,6 +2621,20 @@ static int virtnet_probe(struct virtio_device *vdev)
> netif_set_real_num_rx_queues(dev, vi->curr_queue_pairs);
>
> virtnet_init_settings(dev);
> + if (virtio_has_feature(vdev, VIRTIO_NET_F_SPEED_DUPLEX)) {
> + u32 speed;
> + u8 duplex;
> +
> + speed = virtio_cread32(vdev, offsetof(struct virtio_net_config,
> + speed));
> + if (ethtool_validate_speed(speed))
> + vi->speed = speed;
> + duplex = virtio_cread8(vdev,
> + offsetof(struct virtio_net_config,
> + duplex));
> + if (ethtool_validate_duplex(duplex))
> + vi->duplex = duplex;
> + }
>
> err = register_netdev(dev);
> if (err) {
It would seem that speed/duplex can change at link up time.
Update it from virtnet_config_changed_work please.
> @@ -2746,7 +2760,8 @@ static struct virtio_device_id id_table[] = {
> VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN, \
> VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ, \
> VIRTIO_NET_F_CTRL_MAC_ADDR, \
> - VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS
> + VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, \
> + VIRTIO_NET_F_SPEED_DUPLEX
>
> static unsigned int features[] = {
> VIRTNET_FEATURES,
> diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
> index fc353b5..0f1548e 100644
> --- a/include/uapi/linux/virtio_net.h
> +++ b/include/uapi/linux/virtio_net.h
> @@ -57,6 +57,8 @@
> * Steering */
> #define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */
>
> +#define VIRTIO_NET_F_SPEED_DUPLEX 63 /* Host set linkspeed and duplex */
> +
Host -> device, to match virtio spec terminology.
> #ifndef VIRTIO_NET_NO_LEGACY
> #define VIRTIO_NET_F_GSO 6 /* Host handles pkts w/ any GSO type */
> #endif /* VIRTIO_NET_NO_LEGACY */
> @@ -76,6 +78,9 @@ struct virtio_net_config {
> __u16 max_virtqueue_pairs;
> /* Default maximum transmit unit advice */
> __u16 mtu;
> + /* Host exported linkspeed and duplex */
I would rather drop this and document each field.
/*
speed, in units of 1Mb. All values 0 to INT_MAX are legal.
Any other value stands for unknown.
*/
> + __u32 speed;
/*
0x00 - half duplex
0x01 - full duplex
Any other value stands for unknown.
*/
> + __u8 duplex;
> } __attribute__((packed));
>
> /*
> --
> 2.6.1
^ permalink raw reply
* Re: [PATCH net-next v2 1/3] virtio_net: propagate linkspeed/duplex settings from the hypervisor
From: Michael S. Tsirkin @ 2018-01-03 13:49 UTC (permalink / raw)
To: Jason Baron; +Cc: qemu-devel, netdev, David Miller, virtualization
In-Reply-To: <93b66082-f37c-0463-e977-d4b430a44008@akamai.com>
On Thu, Dec 28, 2017 at 10:53:54AM -0500, Jason Baron wrote:
>
>
> On 12/27/2017 04:43 PM, David Miller wrote:
> > From: Jason Baron <jbaron@akamai.com>
> > Date: Fri, 22 Dec 2017 16:54:01 -0500
> >
> >> The ability to set speed and duplex for virtio_net in useful in various
> >> scenarios as described here:
> >>
> >> 16032be virtio_net: add ethtool support for set and get of settings
> >>
> >> However, it would be nice to be able to set this from the hypervisor,
> >> such that virtio_net doesn't require custom guest ethtool commands.
> >>
> >> Introduce a new feature flag, VIRTIO_NET_F_SPEED_DUPLEX, which allows
> >> the hypervisor to export a linkspeed and duplex setting. The user can
> >> subsequently overwrite it later if desired via: 'ethtool -s'.
> >>
> >> Signed-off-by: Jason Baron <jbaron@akamai.com>
> >> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> >> Cc: Jason Wang <jasowang@redhat.com>
> >
> > Looks mostly fine to me but need some virtio_net reviewers on this one.
> >
> >> @@ -57,6 +57,8 @@
> >> * Steering */
> >> #define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */
> >>
> >> +#define VIRTIO_NET_F_SPEED_DUPLEX 63 /* Host set linkspeed and duplex */
> >> +
> >
> > Why use a value so far away from the largest existing one?
> >
> > Just curious.
> >
>
> So that came from a discussion with Michael about which bit to use for
> this, and he suggested using 63:
>
> "
> Transports started from bit 24 and are growing up.
> So I would say devices should start from bit 63 and grow down.
> "
>
> https://patchwork.ozlabs.org/patch/848814/#1826669
>
> I will add a comment to explain it.
Maybe in the commit log. I don't think we need it in the header.
> Thanks,
>
> -Jason
^ permalink raw reply
* Re: [net-next: PATCH 0/8] Armada 7k/8k PP2 ACPI support
From: Marcin Wojtas @ 2018-01-03 13:36 UTC (permalink / raw)
To: Andrew Lunn
Cc: Graeme Gregory, Ard Biesheuvel, linux-acpi@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
<netdev@vger.kernel.org>, David S. Miller,
Russell King - ARM Linux, Rafael J. Wysocki, Florian Fainelli,
Antoine Ténart, Thomas Petazzoni, Gregory CLEMENT,
Ezequiel Garcia
In-Reply-To: <20180103133341.GJ15036@lunn.ch>
2018-01-03 14:33 GMT+01:00 Andrew Lunn <andrew@lunn.ch>:
> On Wed, Jan 03, 2018 at 02:13:09PM +0100, Marcin Wojtas wrote:
>> Hi Andrew,
>>
>> 2018-01-03 13:47 GMT+01:00 Andrew Lunn <andrew@lunn.ch>:
>> >> I already agreed with 'reg' being awkward in the later emails.
>> >> Wouldn't _ADR be more appropriate to specify PHY address on MDIO bus?
>> >
>> > Also, how do you specify which MDIO bus the PHY is on. To fully
>> > specify a PHY, you need both bits of information.
>> >
>> > In DT, the phy-handle phandle can point to any PHY anywhere in the
>> > system. This is particularly important when a Ethernet device has two
>> > MDIO busses.
>> >
>>
>> For now, my local MDIO bus description is pretty DT-like, i.e. master
>> bus with children PHYs:
>> Device (MDIO)
>> {
>> Name (_HID, "MRVL0100") //
>> _HID: Hardware ID
>> Name (_UID, 0x00) //
>> _UID: Unique ID
>> Name (_CRS, ResourceTemplate ()
>> {
>> Memory32Fixed (ReadWrite,
>> 0xf212a200, // Address Base
>> 0x00000010, //
>> Address Length
>> )
>> })
>> Device (GPHY)
>> {
>> Name (_ADR, 0x0)
>> }
>> }
>>
>> Device (XSMI)
>> {
>> Name (_HID, "MRVL0101") //
>> _HID: Hardware ID
>> Name (_UID, 0x00) //
>> _UID: Unique ID
>> Name (_CRS, ResourceTemplate ()
>> {
>> Memory32Fixed (ReadWrite,
>> 0xf212a600, // Address Base
>> 0x00000010, //
>> Address Length
>> )
>> })
>> Device (PHY0)
>> {
>> Name (_ADR, 0x0)
>> Name (_CID, "ethernet-phy-ieee802.3-c45")
>> }
>> Device (PHY8)
>> {
>> Name (_ADR, 0x8)
>> Name (_CID, "ethernet-phy-ieee802.3-c45")
>> }
>> }
>>
>> Which is referenced in the port's node:
>>
>> Package () { "phy", Package (){\_SB.XSMI.PHY0}},
>
> Hi Marcin
>
> This reference looks good, giving both the bus and the PHY on the bus.
>
> I assume you can use references like this within the Device (PHY8)
> node?
Yes.
> You need to be able to reference a GPIO used for resetting the
> PHY. And you also need to reference a GPIO at the Device (MDIO) level
> for resetting all the PHYs on the MDIO bus.
>
Yes, for full support of PHYs the GPIO must be supported, as well as
the PHY's IRQs.
Best regards,
Marcin
^ permalink raw reply
* Re: [PATCH net] sctp: fix handling of ICMP Frag Needed for too small MTUs
From: Marcelo Ricardo Leitner @ 2018-01-03 13:35 UTC (permalink / raw)
To: Xin Long; +Cc: network dev, linux-sctp, Vlad Yasevich, Neil Horman
In-Reply-To: <CADvbK_cCftba3-O+Hub29sx6K8jWsvROhMiRLgdHg9tYi6wskg@mail.gmail.com>
On Wed, Jan 03, 2018 at 03:31:00PM +0800, Xin Long wrote:
> On Wed, Jan 3, 2018 at 5:44 AM, Marcelo Ricardo Leitner
> <marcelo.leitner@gmail.com> wrote:
> > syzbot reported a hang involving SCTP, on which it kept flooding dmesg
> > with the message:
> > [ 246.742374] sctp: sctp_transport_update_pmtu: Reported pmtu 508 too
> > low, using default minimum of 512
> >
> > That happened because whenever SCTP hits an ICMP Frag Needed, it tries
> > to adjust to the new MTU and triggers an immediate retransmission. But
> > it didn't consider the fact that MTUs smaller than the SCTP minimum MTU
> > allowed (512) would not cause the PMTU to change, and issued the
> > retransmission anyway (thus leading to another ICMP Frag Needed, and so
> > on).
> >
> > The fix is to disable Path MTU discovery for such transport and to skip
> > the retransmission in such cases. By doing this, SCTP will do the
> > backoff retransmissions as needed and will likely switch to another
> > transport if available.
> >
> > See-also: https://lkml.org/lkml/2017/12/22/811
> > Reported-by: syzbot <syzkaller@googlegroups.com>
> > Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> > ---
> > net/sctp/input.c | 5 ++++-
> > net/sctp/transport.c | 2 ++
> > 2 files changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/sctp/input.c b/net/sctp/input.c
> > index 621b5ca3fd1c17c3d7ef7bb1c7677ab98cebbe77..a24658c6f181e03d85f12dbe929c8bb4abaefcbd 100644
> > --- a/net/sctp/input.c
> > +++ b/net/sctp/input.c
> > @@ -412,8 +412,11 @@ void sctp_icmp_frag_needed(struct sock *sk, struct sctp_association *asoc,
> > * Needed will never be sent, but if a message was sent before
> > * PMTU discovery was disabled that was larger than the PMTU, it
> > * would not be fragmented, so it must be re-transmitted fragmented.
> > + * If the new PMTU is invalid, we will keep getting ICMP Frag
> > + * Needed. In this case, simply avoid the retransmit.
> > */
> > - sctp_retransmit(&asoc->outqueue, t, SCTP_RTXR_PMTUD);
> > + if (pmtu >= SCTP_DEFAULT_MINSEGMENT)
> > + sctp_retransmit(&asoc->outqueue, t, SCTP_RTXR_PMTUD);
> > }
> >
> > void sctp_icmp_redirect(struct sock *sk, struct sctp_transport *t,
> > diff --git a/net/sctp/transport.c b/net/sctp/transport.c
> > index 1e5a22430cf56e40a6f323081beb97836b506384..fbd9fe25764d4d98f93c60a48eccefd9cc6b4165 100644
> > --- a/net/sctp/transport.c
> > +++ b/net/sctp/transport.c
> > @@ -259,6 +259,8 @@ void sctp_transport_update_pmtu(struct sctp_transport *t, u32 pmtu)
> > * pmtu discovery on this transport.
> > */
> > t->pathmtu = SCTP_DEFAULT_MINSEGMENT;
> > + t->param_flags = (t->param_flags & ~SPP_PMTUD) |
> > + SPP_PMTUD_DISABLE;
> It seems that once it hits here, this transport will have the minimum pmtu
> forever, even after t->dst has expired. It means this tx path will not come
> back to normal any more even when it gets a needfrag with reasonable
> pmtu. is it too harsh to this transport ?
That was the idea. That is what the comment above these lines is
describing already. Though I missed 06ad391919b2 ("[SCTP] Don't
disable PMTU discovery when mtu is small") and yes, too harsh.
>
> Another thing is on sctp_sendmsg, it also checks pmtu_pending that may
> be set by needfrag, and goes to sctp_assoc_sync_pmtu to trigger this
> warning again.
That is true but that's not an issue, is it? We are not trying to get
ride of the warning, instead we want to not cause a flood of
bogus retransmissions (which led to the flood of warnings).
By not disabling PMTU discovery (as above) we will have such warning
every now and then again for the same transport. We may add
_ratelimited to it, that would help in the case of we have like a
thousand transports suddenly being affected by such small MTU, but
won't omit it completely.
I'll spin a v2, thanks.
>
> > } else {
> > t->pathmtu = pmtu;
> > }
> > --
> > 2.14.3
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [net-next: PATCH 0/8] Armada 7k/8k PP2 ACPI support
From: Andrew Lunn @ 2018-01-03 13:33 UTC (permalink / raw)
To: Marcin Wojtas
Cc: Graeme Gregory, Ard Biesheuvel, linux-acpi@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
<netdev@vger.kernel.org>, David S. Miller,
Russell King - ARM Linux, Rafael J. Wysocki, Florian Fainelli,
Antoine Ténart, Thomas Petazzoni, Gregory CLEMENT,
Ezequiel Garcia
In-Reply-To: <CAPv3WKfZN+=9wFJ5ct+8nttd=bikaZ=Ei741pyTVXjQz2zq2dg@mail.gmail.com>
On Wed, Jan 03, 2018 at 02:13:09PM +0100, Marcin Wojtas wrote:
> Hi Andrew,
>
> 2018-01-03 13:47 GMT+01:00 Andrew Lunn <andrew@lunn.ch>:
> >> I already agreed with 'reg' being awkward in the later emails.
> >> Wouldn't _ADR be more appropriate to specify PHY address on MDIO bus?
> >
> > Also, how do you specify which MDIO bus the PHY is on. To fully
> > specify a PHY, you need both bits of information.
> >
> > In DT, the phy-handle phandle can point to any PHY anywhere in the
> > system. This is particularly important when a Ethernet device has two
> > MDIO busses.
> >
>
> For now, my local MDIO bus description is pretty DT-like, i.e. master
> bus with children PHYs:
> Device (MDIO)
> {
> Name (_HID, "MRVL0100") //
> _HID: Hardware ID
> Name (_UID, 0x00) //
> _UID: Unique ID
> Name (_CRS, ResourceTemplate ()
> {
> Memory32Fixed (ReadWrite,
> 0xf212a200, // Address Base
> 0x00000010, //
> Address Length
> )
> })
> Device (GPHY)
> {
> Name (_ADR, 0x0)
> }
> }
>
> Device (XSMI)
> {
> Name (_HID, "MRVL0101") //
> _HID: Hardware ID
> Name (_UID, 0x00) //
> _UID: Unique ID
> Name (_CRS, ResourceTemplate ()
> {
> Memory32Fixed (ReadWrite,
> 0xf212a600, // Address Base
> 0x00000010, //
> Address Length
> )
> })
> Device (PHY0)
> {
> Name (_ADR, 0x0)
> Name (_CID, "ethernet-phy-ieee802.3-c45")
> }
> Device (PHY8)
> {
> Name (_ADR, 0x8)
> Name (_CID, "ethernet-phy-ieee802.3-c45")
> }
> }
>
> Which is referenced in the port's node:
>
> Package () { "phy", Package (){\_SB.XSMI.PHY0}},
Hi Marcin
This reference looks good, giving both the bus and the PHY on the bus.
I assume you can use references like this within the Device (PHY8)
node? You need to be able to reference a GPIO used for resetting the
PHY. And you also need to reference a GPIO at the Device (MDIO) level
for resetting all the PHYs on the MDIO bus.
Andrew
^ permalink raw reply
* Re: [PATCHv1 1/6] net: dsa: Support internal phy on 'cpu' port
From: Andrew Lunn @ 2018-01-03 13:21 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Vivien Didelot, Florian Fainelli, Shawn Guo, Sascha Hauer,
Fabio Estevam, Ian Ray, Nandor Han, Rob Herring, David S. Miller,
netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20180103122609.5482-2-sebastian.reichel-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
On Wed, Jan 03, 2018 at 01:26:04PM +0100, Sebastian Reichel wrote:
> This adds support for enabling the internal phy for a 'cpu' port.
> It has been tested on GE B850v3 and B650v3, which have a built-in
> MV88E6240 switch connected to a PCIe based network card. Without
> this patch the link does not come up and no traffic can be routed
> through the switch.
>
> Signed-off-by: Sebastian Reichel <sebastian.reichel-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
> ---
> net/dsa/port.c | 26 ++++++++++++++++++++++----
> 1 file changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/net/dsa/port.c b/net/dsa/port.c
> index bb4be2679904..f99c1d34416c 100644
> --- a/net/dsa/port.c
> +++ b/net/dsa/port.c
> @@ -282,6 +282,10 @@ int dsa_port_fixed_link_register_of(struct dsa_port *dp)
> int mode;
> int err;
>
> + mode = of_get_phy_mode(dn);
> + if (mode < 0)
> + mode = PHY_INTERFACE_MODE_NA;
> +
> if (of_phy_is_fixed_link(dn)) {
> err = of_phy_register_fixed_link(dn);
> if (err) {
> @@ -292,10 +296,6 @@ int dsa_port_fixed_link_register_of(struct dsa_port *dp)
> }
>
> phydev = of_phy_find_device(dn);
> -
> - mode = of_get_phy_mode(dn);
> - if (mode < 0)
> - mode = PHY_INTERFACE_MODE_NA;
> phydev->interface = mode;
>
> genphy_config_init(phydev);
> @@ -305,6 +305,24 @@ int dsa_port_fixed_link_register_of(struct dsa_port *dp)
> ds->ops->adjust_link(ds, port, phydev);
>
> put_device(&phydev->mdio.dev);
> + } else if (mode == PHY_INTERFACE_MODE_INTERNAL ||
> + mode == PHY_INTERFACE_MODE_NA) {
Hi Sebastian
I understand what you are trying to do, i've got boards which also
have back-to-back PHYs for the CPU port. These boards however have the
strapping correct, so nothing needs doing in software.
But the way you are doing it is wrong. PHY_INTERFACE_MODE_NA means
something else has already setup the interface mode, leave it alone.
PHY_INTERFACE_MODE_INTERNAL means there is some other sort of bus
between the MAC and the PHY than the normal MII.
What you want to say is that there is a PHY on this port, and that you
want to configure it to a given fixed configuration, probably 1000
Full, with auto-neg turned off. This is something completely different
to a fixed phy, which is used when there is no PHY at all.
What state is the PHY in, if you don't have this patch? Is it powered
down?
Andrew
--
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: [net-next: PATCH 0/8] Armada 7k/8k PP2 ACPI support
From: Marcin Wojtas @ 2018-01-03 13:13 UTC (permalink / raw)
To: Andrew Lunn
Cc: Graeme Gregory, Ard Biesheuvel, linux-acpi@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
<netdev@vger.kernel.org>, David S. Miller,
Russell King - ARM Linux, Rafael J. Wysocki, Florian Fainelli,
Antoine Ténart, Thomas Petazzoni, Gregory CLEMENT,
Ezequiel Garcia
In-Reply-To: <20180103124720.GG15036@lunn.ch>
Hi Andrew,
2018-01-03 13:47 GMT+01:00 Andrew Lunn <andrew@lunn.ch>:
>> I already agreed with 'reg' being awkward in the later emails.
>> Wouldn't _ADR be more appropriate to specify PHY address on MDIO bus?
>
> Also, how do you specify which MDIO bus the PHY is on. To fully
> specify a PHY, you need both bits of information.
>
> In DT, the phy-handle phandle can point to any PHY anywhere in the
> system. This is particularly important when a Ethernet device has two
> MDIO busses.
>
For now, my local MDIO bus description is pretty DT-like, i.e. master
bus with children PHYs:
Device (MDIO)
{
Name (_HID, "MRVL0100") //
_HID: Hardware ID
Name (_UID, 0x00) //
_UID: Unique ID
Name (_CRS, ResourceTemplate ()
{
Memory32Fixed (ReadWrite,
0xf212a200, // Address Base
0x00000010, //
Address Length
)
})
Device (GPHY)
{
Name (_ADR, 0x0)
}
}
Device (XSMI)
{
Name (_HID, "MRVL0101") //
_HID: Hardware ID
Name (_UID, 0x00) //
_UID: Unique ID
Name (_CRS, ResourceTemplate ()
{
Memory32Fixed (ReadWrite,
0xf212a600, // Address Base
0x00000010, //
Address Length
)
})
Device (PHY0)
{
Name (_ADR, 0x0)
Name (_CID, "ethernet-phy-ieee802.3-c45")
}
Device (PHY8)
{
Name (_ADR, 0x8)
Name (_CID, "ethernet-phy-ieee802.3-c45")
}
}
Which is referenced in the port's node:
Package () { "phy", Package (){\_SB.XSMI.PHY0}},
I'm studying an alternatives with graphs, as suggested by Tomasz
Nowicki, but to me above is pretty natural and not complicated.
Best regards,
Marcin
^ permalink raw reply
* Re: [PATCHv1 2/6] net: dsa: mv88e6xxx: add 88E6240 DT compatible
From: Andrew Lunn @ 2018-01-03 12:55 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Vivien Didelot, Florian Fainelli, Shawn Guo, Sascha Hauer,
Fabio Estevam, Ian Ray, Nandor Han, Rob Herring, David S. Miller,
netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20180103122609.5482-3-sebastian.reichel-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
On Wed, Jan 03, 2018 at 01:26:05PM +0100, Sebastian Reichel wrote:
> Add compatible for Marvell 88E6240 switch.
>
> Signed-off-by: Sebastian Reichel <sebastian.reichel-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
> ---
> Documentation/devicetree/bindings/net/dsa/marvell.txt | 6 ++++--
> drivers/net/dsa/mv88e6xxx/chip.c | 4 ++++
> 2 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/dsa/marvell.txt b/Documentation/devicetree/bindings/net/dsa/marvell.txt
> index 1d4d0f49c9d0..cf437b526f7f 100644
> --- a/Documentation/devicetree/bindings/net/dsa/marvell.txt
> +++ b/Documentation/devicetree/bindings/net/dsa/marvell.txt
> @@ -14,8 +14,10 @@ The properties described here are those specific to Marvell devices.
> Additional required and optional properties can be found in dsa.txt.
>
> Required properties:
> -- compatible : Should be one of "marvell,mv88e6085" or
> - "marvell,mv88e6190"
> +- compatible : Should be one of the following
> + * "marvell,mv88e6085"
> + * "marvell,mv88e6190"
> + * "marvell,mv88e6240"
Hi Sebastian
This is not required. The 6240 is compatible with the 6085, so please
use "marvell,mv88e6085". We don't add compatible strings per
device. All the compatible string is used for is to find the ID
register in the device. Nothing more.
Andrew
--
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 v6 4/6] can: m_can: Support higher speed CAN-FD bitrates
From: Faiz Abbas @ 2018-01-03 12:55 UTC (permalink / raw)
To: Marc Kleine-Budde, wg, robh+dt, mark.rutland
Cc: linux-can, netdev, devicetree, linux-kernel, nsekhar, fcooper,
robh, Wenyou.Yang, sergei.shtylyov
In-Reply-To: <f2a063cd-9456-c21d-2544-02179e0f4f27@pengutronix.de>
Hi,
On Tuesday 02 January 2018 07:05 PM, Marc Kleine-Budde wrote:
> On 12/22/2017 02:31 PM, Faiz Abbas wrote:
>> From: Franklin S Cooper Jr <fcooper@ti.com>
>>
>> During test transmitting using CAN-FD at high bitrates (> 2 Mbps)
>> would fail. Scoping the signals I noticed that only a single bit
>> was being transmitted and with a bit more investigation realized the actual
>> MCAN IP would go back to initialization mode automatically.
>>
>> It appears this issue is due to the MCAN needing to use the Transmitter
>> Delay Compensation Mode with the correct value for the transmitter delay
>> compensation offset (tdco). What impacts the tdco value isn't 100% clear
>> but to calculate it you use an equation defined in the MCAN User's Guide.
>>
>> The user guide mentions that this register needs to be set based on clock
>> values, secondary sample point and the data bitrate. One of the key
>> variables that can't automatically be determined is the secondary
>> sample point (ssp). This ssp is similar to the sp but is specific to this
>> transmitter delay compensation mode. The guidelines for configuring
>> ssp is rather vague but via some CAN test it appears for DRA76x that putting
>> the value same as data sampling point works.
>>
>> The CAN-CIA's "Bit Time Requirements for CAN FD" paper presented at
>> the International CAN Conference 2013 indicates that this TDC mode is
>> only needed for data bit rates above 2.5 Mbps. Therefore, only enable
>> this mode when the data bit rate is above 2.5 Mbps.
>>
>> Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
>> Signed-off-by: Sekhar Nori <nsekhar@ti.com>
>> Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
>> ---
>> drivers/net/can/m_can/m_can.c | 41 ++++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 40 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
>> index 53e764f..371ffc1 100644
>> --- a/drivers/net/can/m_can/m_can.c
>> +++ b/drivers/net/can/m_can/m_can.c
>> @@ -127,6 +127,12 @@ enum m_can_mram_cfg {
>> #define DBTP_DSJW_SHIFT 0
>> #define DBTP_DSJW_MASK (0xf << DBTP_DSJW_SHIFT)
>>
>> +/* Transmitter Delay Compensation Register (TDCR) */
>> +#define TDCR_TDCO_SHIFT 8
>> +#define TDCR_TDCO_MASK (0x7F << TDCR_TDCO_SHIFT)
>> +#define TDCR_TDCF_SHIFT 0
>> +#define TDCR_TDCF_MASK (0x7F << TDCR_TDCF_SHIFT)
>> +
>> /* Test Register (TEST) */
>> #define TEST_LBCK BIT(4)
>>
>> @@ -991,7 +997,8 @@ static int m_can_set_bittiming(struct net_device *dev)
>> const struct can_bittiming *bt = &priv->can.bittiming;
>> const struct can_bittiming *dbt = &priv->can.data_bittiming;
>> u16 brp, sjw, tseg1, tseg2;
>> - u32 reg_btp;
>> + u32 reg_btp, tdco, ssp;
>
> Please move the tdco and the ssp into "if (dbt->bitrate > 2500000)" scope.
Ok.
>
> Initialize "reg_btp = 0", see below.
>
>> + bool enable_tdc = false;
>
> Please remove, see below.
>
>>
>> brp = bt->brp - 1;
>> sjw = bt->sjw - 1;
>> @@ -1006,9 +1013,41 @@ static int m_can_set_bittiming(struct net_device *dev)
>> sjw = dbt->sjw - 1;
>> tseg1 = dbt->prop_seg + dbt->phase_seg1 - 1;
>> tseg2 = dbt->phase_seg2 - 1;
>> +
>> + /* TDC is only needed for bitrates beyond 2.5 MBit/s.
>> + * This is mentioned in the "Bit Time Requirements for CAN FD"
>> + * paper presented at the International CAN Conference 2013
>> + */
>> + if (dbt->bitrate > 2500000) {
>> + /* Use the same value of secondary sampling point
>> + * as the data sampling point
>> + */
>> + ssp = dbt->sample_point;
>> +
>> + /* Equation based on Bosch's M_CAN User Manual's
>> + * Transmitter Delay Compensation Section
>> + */
>> + tdco = (priv->can.clock.freq / 1000) *
>> + ssp / dbt->bitrate;
>> +
>> + /* Max valid TDCO value is 127 */
>> + if (tdco > 127) {
>> + netdev_warn(dev, "TDCO value of %u is beyond maximum limit. Disabling Transmitter Delay Compensation mode\n",
>
> "maximum limit"? Either "maximum" or "limit" should be enough. If the
> value is above 127, does it make sense to disable the tdco completely?
I guess we can put the closest possible value (127) so that the ssp
calculated by the IP is as close to the chosen one as possible.
>
>> + tdco);
>> + } else {
>> + enable_tdc = true;
>
> Why not set "reg_btp |= DBTP_TDC;" here directly?
>
>> + m_can_write(priv, M_CAN_TDCR,
>> + tdco << TDCR_TDCO_SHIFT);
>> + }
>> + }
>> +
>> reg_btp = (brp << DBTP_DBRP_SHIFT) | (sjw << DBTP_DSJW_SHIFT) |
>> (tseg1 << DBTP_DTSEG1_SHIFT) |
>> (tseg2 << DBTP_DTSEG2_SHIFT);
>
> Adjust this to "reg_btp |=".
>
>> +
>> + if (enable_tdc)
>> + reg_btp |= DBTP_TDC;
>
> Please remove.
Sure, will remove.
Thanks,
Faiz
^ permalink raw reply
* Re: [net-next: PATCH 0/8] Armada 7k/8k PP2 ACPI support
From: Andrew Lunn @ 2018-01-03 12:47 UTC (permalink / raw)
To: Marcin Wojtas
Cc: Graeme Gregory, Ard Biesheuvel, linux-acpi@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
<netdev@vger.kernel.org>, David S. Miller,
Russell King - ARM Linux, Rafael J. Wysocki, Florian Fainelli,
Antoine Ténart, Thomas Petazzoni, Gregory CLEMENT,
Ezequiel Garcia
In-Reply-To: <CAPv3WKcM8WWvh64A=FTtuWkEr6V_QSKjBpNpWnMpp_P=cMU9sw@mail.gmail.com>
> I already agreed with 'reg' being awkward in the later emails.
> Wouldn't _ADR be more appropriate to specify PHY address on MDIO bus?
Also, how do you specify which MDIO bus the PHY is on. To fully
specify a PHY, you need both bits of information.
In DT, the phy-handle phandle can point to any PHY anywhere in the
system. This is particularly important when a Ethernet device has two
MDIO busses.
Andrew
^ permalink raw reply
* Re: [PATCH v6 3/6] can: m_can: Add PM Runtime
From: Faiz Abbas @ 2018-01-03 12:39 UTC (permalink / raw)
To: Marc Kleine-Budde, wg-5Yr1BZd7O62+XT7JhA+gdA,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8
Cc: linux-can-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, nsekhar-l0cyMroinI0,
fcooper-l0cyMroinI0, robh-DgEjT+Ai2ygdnm+yROfE0A,
Wenyou.Yang-UWL1GkI3JZL3oGB3hsPCZA,
sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8
In-Reply-To: <aea8638a-c847-b55e-ff2d-37999980ba7b-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Hi,
On Tuesday 02 January 2018 09:37 PM, Marc Kleine-Budde wrote:
> On 12/22/2017 02:31 PM, Faiz Abbas wrote:
>> From: Franklin S Cooper Jr <fcooper-l0cyMroinI0@public.gmane.org>
>>
>> Add support for PM Runtime which is the new way to handle managing clocks.
>> However, to avoid breaking SoCs not using PM_RUNTIME leave the old clk
>> management approach in place.
>
> There is no PM_RUNTIME anymore since 464ed18ebdb6 ("PM: Eliminate
> CONFIG_PM_RUNTIME")
Ok. Will change the commit message.
>
> Have a look at the discussion: https://patchwork.kernel.org/patch/9436507/ :
>
>>> Well, I admit it would be nicer if drivers didn't have to worry about
>>> whether or not CONFIG_PM was enabled. A slightly cleaner approach
>>> from the one outlined above would have the probe routine do this:
>>>
>>> my_power_up(dev);
>>> pm_runtime_set_active(dev);
>>> pm_runtime_get_noresume(dev);
>>> pm_runtime_enable(dev);
This discussion seems to be about cases in which CONFIG_PM is not
enabled. CONFIG_PM is always selected in the case of omap devices.
>
>> PM_RUNTIME is required by OMAP based devices to handle clock management.
>> Therefore, this allows future Texas Instruments SoCs that have the MCAN IP
>> to work with this driver.
>
> Who will set the SET_RUNTIME_PM_OPS in this case?
It is set with a common SET_RUNTIME_PM_OPS in the case of omap at
arch/arm/mach-omap2/omap_device.c:632
struct dev_pm_domain omap_device_pm_domain = {
.ops = {
SET_RUNTIME_PM_OPS(_od_runtime_suspend, _od_runtime_resume,
NULL)
USE_PLATFORM_PM_SLEEP_OPS
SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(_od_suspend_noirq,
_od_resume_noirq)
}
};
>
>> Signed-off-by: Franklin S Cooper Jr <fcooper-l0cyMroinI0@public.gmane.org>
>> [nsekhar-l0cyMroinI0@public.gmane.org: handle pm_runtime_get_sync() failure, fix some bugs]
>> Signed-off-by: Sekhar Nori <nsekhar-l0cyMroinI0@public.gmane.org>
>> Signed-off-by: Faiz Abbas <faiz_abbas-l0cyMroinI0@public.gmane.org>
>> ---
>> drivers/net/can/m_can/m_can.c | 38 ++++++++++++++++++++++++++++++++++----
>> 1 file changed, 34 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
>> index f72116e..53e764f 100644
>> --- a/drivers/net/can/m_can/m_can.c
>> +++ b/drivers/net/can/m_can/m_can.c
>> @@ -23,6 +23,7 @@
>> #include <linux/of.h>
>> #include <linux/of_device.h>
>> #include <linux/platform_device.h>
>> +#include <linux/pm_runtime.h>
>> #include <linux/iopoll.h>
>> #include <linux/can/dev.h>
>>
>> @@ -625,19 +626,33 @@ static int m_can_clk_start(struct m_can_priv *priv)
>> {
>> int err;
>>
>> + err = pm_runtime_get_sync(priv->device);
>> + if (err) {
>> + pm_runtime_put_noidle(priv->device);
>
> Why do you call this in case of an error?
pm_runtime_get_sync() increments the usage count of the device before
any error is returned. This needs to be decremented using
pm_runtime_put_noidle().
>
>> + return err;
>> + }
>> +
>> err = clk_prepare_enable(priv->hclk);
>> if (err)
>> - return err;
>> + goto pm_runtime_put;
>>
>> err = clk_prepare_enable(priv->cclk);
>> if (err)
>> - clk_disable_unprepare(priv->hclk);
>> + goto disable_hclk;
>>
>> return err;
>> +
>> +disable_hclk:
>> + clk_disable_unprepare(priv->hclk);
>> +pm_runtime_put:
>> + pm_runtime_put_sync(priv->device);
>> + return err;
>> }
>>
>> static void m_can_clk_stop(struct m_can_priv *priv)
>> {
>> + pm_runtime_put_sync(priv->device);
>> +
>> clk_disable_unprepare(priv->cclk);
>> clk_disable_unprepare(priv->hclk);
>> }
>> @@ -1577,13 +1592,20 @@ static int m_can_plat_probe(struct platform_device *pdev)
>> /* Enable clocks. Necessary to read Core Release in order to determine
>> * M_CAN version
>> */
>> + pm_runtime_enable(&pdev->dev);
>> + ret = pm_runtime_get_sync(&pdev->dev);
>> + if (ret) {
>> + pm_runtime_put_noidle(&pdev->dev);
>
> Why do you call this in case of error?
Same here.
Thanks,
Faiz
--
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] rt2x00: Delete an error message for a failed memory allocation in rt2x00queue_allocate()
From: Stanislaw Gruszka @ 2018-01-03 12:26 UTC (permalink / raw)
To: SF Markus Elfring
Cc: linux-wireless, netdev, Helmut Schaa, Kalle Valo, LKML,
kernel-janitors
In-Reply-To: <cdeaa8a2-06bb-10e9-05d7-f531c00eef5d@users.sourceforge.net>
On Fri, Dec 29, 2017 at 10:18:14PM +0100, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Fri, 29 Dec 2017 22:11:42 +0100
>
> Omit an extra message for a memory allocation failure in this function.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
Acked-by: Stanislaw Gruszka <sgruszka@redhat.com>
^ permalink raw reply
* [PATCHv1 6/6] ARM: dts: imx6q-b450v3: Add switch port configuration
From: Sebastian Reichel @ 2018-01-03 12:26 UTC (permalink / raw)
To: Andrew Lunn, Vivien Didelot, Florian Fainelli, Shawn Guo,
Sascha Hauer, Fabio Estevam
Cc: Ian Ray, Nandor Han, Rob Herring, David S. Miller, netdev,
devicetree, linux-kernel, Sebastian Reichel
In-Reply-To: <20180103122609.5482-1-sebastian.reichel@collabora.co.uk>
This adds support for the Marvell switch and names the network
ports according to the labels, that can be found next to the
connectors. The switch is connected to the host system using a
PCI based network card.
The PCI bus configuration has been written using the following
information:
root@b450v3# lspci -tv
-[0000:00]---00.0-[01]----00.0 Intel Corporation I210 Gigabit Network Connection
root@b450v3# lspci -nn
00:00.0 PCI bridge [0604]: Synopsys, Inc. Device [16c3:abcd] (rev 01)
01:00.0 Ethernet controller [0200]: Intel Corporation I210 Gigabit Network Connection [8086:1533] (rev 03)
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
arch/arm/boot/dts/imx6q-b450v3.dts | 47 ++++++++++++++++++++++++++++++++++++++
1 file changed, 47 insertions(+)
diff --git a/arch/arm/boot/dts/imx6q-b450v3.dts b/arch/arm/boot/dts/imx6q-b450v3.dts
index 404a93d9596b..693dfa7d751d 100644
--- a/arch/arm/boot/dts/imx6q-b450v3.dts
+++ b/arch/arm/boot/dts/imx6q-b450v3.dts
@@ -112,3 +112,50 @@
line-name = "PCA9539-P07";
};
};
+
+&pci_root {
+ /* Intel Corporation I210 Gigabit Network Connection */
+ switch_nic: ethernet@3,0 {
+ compatible = "pci8086,1533";
+ reg = <0x00010000 0 0 0 0>;
+ };
+};
+
+&switch_ports {
+ port@0 {
+ reg = <0>;
+ label = "enacq";
+ };
+
+ port@1 {
+ reg = <1>;
+ label = "eneport1";
+ };
+
+ port@2 {
+ reg = <2>;
+ label = "enix";
+ };
+
+ port@3 {
+ reg = <3>;
+ label = "enid";
+ };
+
+ port@4 {
+ reg = <4>;
+ label = "cpu";
+ ethernet = <&switch_nic>;
+ };
+
+ port@5 {
+ reg = <5>;
+ label = "enembc";
+
+ /* connected to Ethernet MAC of AT91RM9200 in MII mode */
+ fixed-link {
+ speed = <100>;
+ full-duplex;
+ };
+ };
+};
--
2.15.1
^ permalink raw reply related
* [PATCHv1 5/6] ARM: dts: imx6q-b650v3: Add switch port configuration
From: Sebastian Reichel @ 2018-01-03 12:26 UTC (permalink / raw)
To: Andrew Lunn, Vivien Didelot, Florian Fainelli, Shawn Guo,
Sascha Hauer, Fabio Estevam
Cc: Ian Ray, Nandor Han, Rob Herring, David S. Miller, netdev,
devicetree, linux-kernel, Sebastian Reichel
In-Reply-To: <20180103122609.5482-1-sebastian.reichel@collabora.co.uk>
This adds support for the Marvell switch and names the network
ports according to the labels, that can be found next to the
connectors. The switch is connected to the host system using a
PCI based network card.
The PCI bus configuration has been written using the following
information:
root@b650v3# lspci -tv
-[0000:00]---00.0-[01]----00.0 Intel Corporation I210 Gigabit Network Connection
root@b650v3# lspci -nn
00:00.0 PCI bridge [0604]: Synopsys, Inc. Device [16c3:abcd] (rev 01)
01:00.0 Ethernet controller [0200]: Intel Corporation I210 Gigabit Network Connection [8086:1533] (rev 03)
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
arch/arm/boot/dts/imx6q-b650v3.dts | 47 ++++++++++++++++++++++++++++++++++++++
1 file changed, 47 insertions(+)
diff --git a/arch/arm/boot/dts/imx6q-b650v3.dts b/arch/arm/boot/dts/imx6q-b650v3.dts
index 7f9f176901d4..928f6cd8d5ae 100644
--- a/arch/arm/boot/dts/imx6q-b650v3.dts
+++ b/arch/arm/boot/dts/imx6q-b650v3.dts
@@ -111,3 +111,50 @@
fsl,tx-cal-45-dp-ohms = <55>;
fsl,tx-d-cal = <100>;
};
+
+&pci_root {
+ /* Intel Corporation I210 Gigabit Network Connection */
+ switch_nic: ethernet@3,0 {
+ compatible = "pci8086,1533";
+ reg = <0x00010000 0 0 0 0>;
+ };
+};
+
+&switch_ports {
+ port@0 {
+ reg = <0>;
+ label = "enacq";
+ };
+
+ port@1 {
+ reg = <1>;
+ label = "eneport1";
+ };
+
+ port@2 {
+ reg = <2>;
+ label = "enix";
+ };
+
+ port@3 {
+ reg = <3>;
+ label = "enid";
+ };
+
+ port@4 {
+ reg = <4>;
+ label = "cpu";
+ ethernet = <&switch_nic>;
+ };
+
+ port@5 {
+ reg = <5>;
+ label = "enembc";
+
+ /* connected to Ethernet MAC of AT91RM9200 in MII mode */
+ fixed-link {
+ speed = <100>;
+ full-duplex;
+ };
+ };
+};
--
2.15.1
^ permalink raw reply related
* [PATCHv1 4/6] ARM: dts: imx6q-b850v3: Add switch port configuration
From: Sebastian Reichel @ 2018-01-03 12:26 UTC (permalink / raw)
To: Andrew Lunn, Vivien Didelot, Florian Fainelli, Shawn Guo,
Sascha Hauer, Fabio Estevam
Cc: Ian Ray, Nandor Han, Rob Herring, David S. Miller, netdev,
devicetree, linux-kernel, Sebastian Reichel
In-Reply-To: <20180103122609.5482-1-sebastian.reichel@collabora.co.uk>
This adds support for the Marvell switch and names the network
ports according to the labels, that can be found next to the
connectors ("ID", "IX", "ePort 1", "ePort 2"). The switch is
connected to the host system using a PCI based network card.
The PCI bus configuration has been written using the following
information:
root@b850v3# lspci -tv
-[0000:00]---00.0-[01]----00.0-[02-05]--+-01.0-[03]----00.0 Intel Corporation I210 Gigabit Network Connection
+-02.0-[04]----00.0 Intel Corporation I210 Gigabit Network Connection
\-03.0-[05]--
root@b850v3# lspci -nn
00:00.0 PCI bridge [0604]: Synopsys, Inc. Device [16c3:abcd] (rev 01)
01:00.0 PCI bridge [0604]: PLX Technology, Inc. PEX 8605 PCI Express 4-port Gen2 Switch [10b5:8605] (rev ab)
02:01.0 PCI bridge [0604]: PLX Technology, Inc. PEX 8605 PCI Express 4-port Gen2 Switch [10b5:8605] (rev ab)
02:02.0 PCI bridge [0604]: PLX Technology, Inc. PEX 8605 PCI Express 4-port Gen2 Switch [10b5:8605] (rev ab)
02:03.0 PCI bridge [0604]: PLX Technology, Inc. PEX 8605 PCI Express 4-port Gen2 Switch [10b5:8605] (rev ab)
03:00.0 Ethernet controller [0200]: Intel Corporation I210 Gigabit Network Connection [8086:1533] (rev 03)
04:00.0 Ethernet controller [0200]: Intel Corporation I210 Gigabit Network Connection [8086:1533] (rev 03)
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
arch/arm/boot/dts/imx6q-b850v3.dts | 70 ++++++++++++++++++++++++++++++++++++++
1 file changed, 70 insertions(+)
diff --git a/arch/arm/boot/dts/imx6q-b850v3.dts b/arch/arm/boot/dts/imx6q-b850v3.dts
index 46bdc6722715..a55ccdfb341c 100644
--- a/arch/arm/boot/dts/imx6q-b850v3.dts
+++ b/arch/arm/boot/dts/imx6q-b850v3.dts
@@ -212,3 +212,73 @@
};
};
};
+
+&pci_root {
+ /* PLX Technology, Inc. PEX 8605 PCI Express 4-port Gen2 Switch */
+ bridge@1,0 {
+ compatible = "pci10b5,8605";
+ reg = <0x00010000 0 0 0 0>;
+
+ #address-cells = <3>;
+ #size-cells = <2>;
+ #interrupt-cells = <1>;
+
+ bridge@2,1 {
+ compatible = "pci10b5,8605";
+ reg = <0x00020800 0 0 0 0>;
+
+ #address-cells = <3>;
+ #size-cells = <2>;
+ #interrupt-cells = <1>;
+
+ /* Intel Corporation I210 Gigabit Network Connection */
+ ethernet@3,0 {
+ compatible = "pci8086,1533";
+ reg = <0x00030000 0 0 0 0>;
+ };
+ };
+
+ bridge@2,2 {
+ compatible = "pci10b5,8605";
+ reg = <0x00021000 0 0 0 0>;
+
+ #address-cells = <3>;
+ #size-cells = <2>;
+ #interrupt-cells = <1>;
+
+ /* Intel Corporation I210 Gigabit Network Connection */
+ switch_nic: ethernet@4,0 {
+ compatible = "pci8086,1533";
+ reg = <0x00040000 0 0 0 0>;
+ };
+ };
+ };
+};
+
+&switch_ports {
+ port@0 {
+ reg = <0>;
+ label = "eneport1";
+ };
+
+ port@1 {
+ reg = <1>;
+ label = "eneport2";
+ };
+
+ port@2 {
+ reg = <2>;
+ label = "enix";
+ };
+
+ port@3 {
+ reg = <3>;
+ label = "enid";
+ };
+
+ port@4 {
+ reg = <4>;
+ label = "cpu";
+ ethernet = <&switch_nic>;
+ };
+};
--
2.15.1
^ permalink raw reply related
* [PATCHv1 3/6] ARM: dts: imx6q-bx50v3: Add internal switch
From: Sebastian Reichel @ 2018-01-03 12:26 UTC (permalink / raw)
To: Andrew Lunn, Vivien Didelot, Florian Fainelli, Shawn Guo,
Sascha Hauer, Fabio Estevam
Cc: Ian Ray, Nandor Han, Rob Herring, David S. Miller, netdev,
devicetree, linux-kernel, Sebastian Reichel
In-Reply-To: <20180103122609.5482-1-sebastian.reichel@collabora.co.uk>
B850v3, B650v3 and B450v3 all have a GPIO bit banged MDIO bus to
communicate with a Marvell switch. On all devices the switch is
connected to a PCI based network card, which needs to be referenced
by DT, so this also adds the common PCI root node.
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
arch/arm/boot/dts/imx6q-bx50v3.dtsi | 37 +++++++++++++++++++++++++++++++++++++
1 file changed, 37 insertions(+)
diff --git a/arch/arm/boot/dts/imx6q-bx50v3.dtsi b/arch/arm/boot/dts/imx6q-bx50v3.dtsi
index b915837bbb5f..689981e90e68 100644
--- a/arch/arm/boot/dts/imx6q-bx50v3.dtsi
+++ b/arch/arm/boot/dts/imx6q-bx50v3.dtsi
@@ -92,6 +92,31 @@
mux-int-port = <1>;
mux-ext-port = <4>;
};
+
+ aliases {
+ mdio-gpio0 = &mdio0;
+ };
+
+ mdio0: mdio-gpio {
+ compatible = "virtual,mdio-gpio";
+ gpios = <&gpio2 5 GPIO_ACTIVE_HIGH>, /* mdc */
+ <&gpio2 7 GPIO_ACTIVE_HIGH>; /* mdio */
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ switch@0 {
+ compatible = "marvell,mv88e6240";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0>;
+
+ switch_ports: ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+ };
+ };
};
&ecspi5 {
@@ -326,3 +351,15 @@
tcxo-clock-frequency = <26000000>;
};
};
+
+&pcie {
+ /* Synopsys, Inc. Device */
+ pci_root: root@0,0 {
+ compatible = "pci16c3,abcd";
+ reg = <0x00000000 0 0 0 0>;
+
+ #address-cells = <3>;
+ #size-cells = <2>;
+ #interrupt-cells = <1>;
+ };
+};
--
2.15.1
^ permalink raw reply related
* [PATCHv1 2/6] net: dsa: mv88e6xxx: add 88E6240 DT compatible
From: Sebastian Reichel @ 2018-01-03 12:26 UTC (permalink / raw)
To: Andrew Lunn, Vivien Didelot, Florian Fainelli, Shawn Guo,
Sascha Hauer, Fabio Estevam
Cc: Ian Ray, Nandor Han, Rob Herring, David S. Miller, netdev,
devicetree, linux-kernel, Sebastian Reichel
In-Reply-To: <20180103122609.5482-1-sebastian.reichel@collabora.co.uk>
Add compatible for Marvell 88E6240 switch.
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
Documentation/devicetree/bindings/net/dsa/marvell.txt | 6 ++++--
drivers/net/dsa/mv88e6xxx/chip.c | 4 ++++
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/dsa/marvell.txt b/Documentation/devicetree/bindings/net/dsa/marvell.txt
index 1d4d0f49c9d0..cf437b526f7f 100644
--- a/Documentation/devicetree/bindings/net/dsa/marvell.txt
+++ b/Documentation/devicetree/bindings/net/dsa/marvell.txt
@@ -14,8 +14,10 @@ The properties described here are those specific to Marvell devices.
Additional required and optional properties can be found in dsa.txt.
Required properties:
-- compatible : Should be one of "marvell,mv88e6085" or
- "marvell,mv88e6190"
+- compatible : Should be one of the following
+ * "marvell,mv88e6085"
+ * "marvell,mv88e6190"
+ * "marvell,mv88e6240"
- reg : Address on the MII bus for the switch.
Optional properties:
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 66d33e97cbc5..78ff06239b58 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -4012,6 +4012,10 @@ static const struct of_device_id mv88e6xxx_of_match[] = {
.compatible = "marvell,mv88e6190",
.data = &mv88e6xxx_table[MV88E6190],
},
+ {
+ .compatible = "marvell,mv88e6240",
+ .data = &mv88e6xxx_table[MV88E6240],
+ },
{ /* sentinel */ },
};
--
2.15.1
^ permalink raw reply related
* [PATCHv1 1/6] net: dsa: Support internal phy on 'cpu' port
From: Sebastian Reichel @ 2018-01-03 12:26 UTC (permalink / raw)
To: Andrew Lunn, Vivien Didelot, Florian Fainelli, Shawn Guo,
Sascha Hauer, Fabio Estevam
Cc: Ian Ray, Nandor Han, Rob Herring, David S. Miller, netdev,
devicetree, linux-kernel, Sebastian Reichel
In-Reply-To: <20180103122609.5482-1-sebastian.reichel@collabora.co.uk>
This adds support for enabling the internal phy for a 'cpu' port.
It has been tested on GE B850v3 and B650v3, which have a built-in
MV88E6240 switch connected to a PCIe based network card. Without
this patch the link does not come up and no traffic can be routed
through the switch.
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
net/dsa/port.c | 26 ++++++++++++++++++++++----
1 file changed, 22 insertions(+), 4 deletions(-)
diff --git a/net/dsa/port.c b/net/dsa/port.c
index bb4be2679904..f99c1d34416c 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -282,6 +282,10 @@ int dsa_port_fixed_link_register_of(struct dsa_port *dp)
int mode;
int err;
+ mode = of_get_phy_mode(dn);
+ if (mode < 0)
+ mode = PHY_INTERFACE_MODE_NA;
+
if (of_phy_is_fixed_link(dn)) {
err = of_phy_register_fixed_link(dn);
if (err) {
@@ -292,10 +296,6 @@ int dsa_port_fixed_link_register_of(struct dsa_port *dp)
}
phydev = of_phy_find_device(dn);
-
- mode = of_get_phy_mode(dn);
- if (mode < 0)
- mode = PHY_INTERFACE_MODE_NA;
phydev->interface = mode;
genphy_config_init(phydev);
@@ -305,6 +305,24 @@ int dsa_port_fixed_link_register_of(struct dsa_port *dp)
ds->ops->adjust_link(ds, port, phydev);
put_device(&phydev->mdio.dev);
+ } else if (mode == PHY_INTERFACE_MODE_INTERNAL ||
+ mode == PHY_INTERFACE_MODE_NA) {
+ phydev = mdiobus_get_phy(ds->slave_mii_bus, port);
+ if (phydev) {
+ genphy_config_init(phydev);
+ genphy_resume(phydev);
+ genphy_read_status(phydev);
+
+ if (ds->ops->adjust_link)
+ ds->ops->adjust_link(ds, port, phydev);
+
+ dev_dbg(ds->dev, "enabled cpu port's phy: %s",
+ phydev_name(phydev));
+ } else {
+ dev_warn(ds->dev, "cpu port has no internal phy and no fixed linked has been configured!");
+ }
+ } else {
+ dev_err(ds->dev, "unsupported phy mode!");
}
return 0;
--
2.15.1
^ permalink raw reply related
* [PATCHv1 0/6] GEHC Bx50 Switch Support
From: Sebastian Reichel @ 2018-01-03 12:26 UTC (permalink / raw)
To: Andrew Lunn, Vivien Didelot, Florian Fainelli, Shawn Guo,
Sascha Hauer, Fabio Estevam
Cc: Ian Ray, Nandor Han, Rob Herring, David S. Miller, netdev,
devicetree, linux-kernel, Sebastian Reichel
Hi,
This adds support for the internal switch found in GE Healthcare
B450v3, B650v3 and B850v3. All devices use a GPIO bitbanged MDIO
bus to communicate with the switch and a PCIe based network card
for exchanging network data. The cpu network data link requires,
that the switch's internal phy interface is enabled, so support
for that is added by the first patch in this series.
The patch series is based on v4.15-rc6.
-- Sebastian
Sebastian Reichel (6):
net: dsa: Support internal phy on 'cpu' port
net: dsa: mv88e6xxx: add 88E6240 DT compatible
ARM: dts: imx6q-bx50v3: Add internal switch
ARM: dts: imx6q-b850v3: Add switch port configuration
ARM: dts: imx6q-b650v3: Add switch port configuration
ARM: dts: imx6q-b450v3: Add switch port configuration
.../devicetree/bindings/net/dsa/marvell.txt | 6 +-
arch/arm/boot/dts/imx6q-b450v3.dts | 47 +++++++++++++++
arch/arm/boot/dts/imx6q-b650v3.dts | 47 +++++++++++++++
arch/arm/boot/dts/imx6q-b850v3.dts | 70 ++++++++++++++++++++++
arch/arm/boot/dts/imx6q-bx50v3.dtsi | 37 ++++++++++++
drivers/net/dsa/mv88e6xxx/chip.c | 4 ++
net/dsa/port.c | 26 ++++++--
7 files changed, 231 insertions(+), 6 deletions(-)
--
2.15.1
^ permalink raw reply
* Re: [PATCH v6 1/6] can: dev: Add support for limiting configured bitrate
From: Faiz Abbas @ 2018-01-03 12:21 UTC (permalink / raw)
To: Marc Kleine-Budde, wg-5Yr1BZd7O62+XT7JhA+gdA,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8
Cc: linux-can-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, nsekhar-l0cyMroinI0,
fcooper-l0cyMroinI0, robh-DgEjT+Ai2ygdnm+yROfE0A,
Wenyou.Yang-UWL1GkI3JZL3oGB3hsPCZA,
sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8
In-Reply-To: <8d973da8-df40-e37e-377f-50666db382fe-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Hi,
On Tuesday 02 January 2018 09:45 PM, Marc Kleine-Budde wrote:
> On 12/22/2017 02:31 PM, Faiz Abbas wrote:
>> From: Franklin S Cooper Jr <fcooper-l0cyMroinI0@public.gmane.org>
>>
>> Various CAN or CAN-FD IP may be able to run at a faster rate than
>> what the transceiver the CAN node is connected to. This can lead to
>> unexpected errors. However, CAN transceivers typically have fixed
>> limitations and provide no means to discover these limitations at
>> runtime. Therefore, add support for a can-transceiver node that
>> can be reused by other CAN peripheral drivers to determine for both
>> CAN and CAN-FD what the max bitrate that can be used. If the user
>> tries to configure CAN to pass these maximum bitrates it will throw
>> an error.
>
> Please add support to read the maximum bitrate via netlink. Have a look
> at 12a6075cabc0 ("can: dev: add CAN interface termination API"). I think
> you need to extend the following functions: can_get_size() and
> can_fill_info().
Will add in the next version.
Thanks,
Faiz
>
> Marc
>
--
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 v6 1/6] can: dev: Add support for limiting configured bitrate
From: Faiz Abbas @ 2018-01-03 12:19 UTC (permalink / raw)
To: Marc Kleine-Budde, wg, robh+dt, mark.rutland
Cc: linux-can, netdev, devicetree, linux-kernel, nsekhar, fcooper,
robh, Wenyou.Yang, sergei.shtylyov
In-Reply-To: <b3cb6058-f61f-b643-a26f-c8330d9a3ffa@pengutronix.de>
Hi Marc,
On Tuesday 02 January 2018 06:30 PM, Marc Kleine-Budde wrote:
> On 12/22/2017 02:31 PM, Faiz Abbas wrote:
>> From: Franklin S Cooper Jr <fcooper@ti.com>
>>
>> Various CAN or CAN-FD IP may be able to run at a faster rate than
>> what the transceiver the CAN node is connected to. This can lead to
>> unexpected errors. However, CAN transceivers typically have fixed
>> limitations and provide no means to discover these limitations at
>> runtime. Therefore, add support for a can-transceiver node that
>> can be reused by other CAN peripheral drivers to determine for both
>> CAN and CAN-FD what the max bitrate that can be used. If the user
>> tries to configure CAN to pass these maximum bitrates it will throw
>> an error.
>>
>> Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
>> [nsekhar@ti.com: fix build error with !CONFIG_OF]
>> Signed-off-by: Sekhar Nori <nsekhar@ti.com>
>> Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
>> ---
>> v6 changes:
>> fix build error with !CONFIG_OF
>>
>> drivers/net/can/dev.c | 39 +++++++++++++++++++++++++++++++++++++++
>> include/linux/can/dev.h | 8 ++++++++
>> 2 files changed, 47 insertions(+)
>>
>> diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
>> index 365a8cc..007cfc0 100644
>> --- a/drivers/net/can/dev.c
>> +++ b/drivers/net/can/dev.c
>> @@ -27,6 +27,7 @@
>> #include <linux/can/skb.h>
>> #include <linux/can/netlink.h>
>> #include <linux/can/led.h>
>> +#include <linux/of.h>
>> #include <net/rtnetlink.h>
>>
>> #define MOD_DESC "CAN device driver interface"
>> @@ -814,6 +815,30 @@ int open_candev(struct net_device *dev)
>> }
>> EXPORT_SYMBOL_GPL(open_candev);
>>
>> +#ifdef CONFIG_OF
>> +/*
>> + * Common function that can be used to understand the limitation of
>> + * a transceiver when it provides no means to determine these limitations
>> + * at runtime.
>> + */
>> +void of_can_transceiver(struct net_device *dev)
>> +{
>> + struct device_node *dn;
>> + struct can_priv *priv = netdev_priv(dev);
>> + struct device_node *np = dev->dev.parent->of_node;
>> + int ret;
>> +
>> + dn = of_get_child_by_name(np, "can-transceiver");
>> + if (!dn)
>> + return;
>> +
>> + ret = of_property_read_u32(dn, "max-bitrate", &priv->max_bitrate);
>> + if ((ret && ret != -EINVAL) || (!ret && !priv->max_bitrate))
>> + netdev_warn(dev, "Invalid value for transceiver max bitrate. Ignoring bitrate limit.\n");
>> +}
>> +EXPORT_SYMBOL_GPL(of_can_transceiver);
>> +#endif
>> +
>> /*
>> * Common close function for cleanup before the device gets closed.
>> *
>> @@ -913,6 +938,13 @@ static int can_changelink(struct net_device *dev, struct nlattr *tb[],
>> priv->bitrate_const_cnt);
>> if (err)
>> return err;
>> +
>> + if (priv->max_bitrate && bt.bitrate > priv->max_bitrate) {
>> + netdev_err(dev, "arbitration bitrate surpasses transceiver capabilities of %d bps\n",
>> + priv->max_bitrate);
>> + return -EINVAL;
>> + }
>
> What about movong this check into can_get_bittiming()? Although we loose
> the "arbitration" vs "canfd data".
I would prefer to keep the distinction.
>
>> +
>> memcpy(&priv->bittiming, &bt, sizeof(bt));
>>
>> if (priv->do_set_bittiming) {
>> @@ -997,6 +1029,13 @@ static int can_changelink(struct net_device *dev, struct nlattr *tb[],
>> priv->data_bitrate_const_cnt);
>> if (err)
>> return err;
>> +
>> + if (priv->max_bitrate && dbt.bitrate > priv->max_bitrate) {
>> + netdev_err(dev, "canfd data bitrate surpasses transceiver capabilities of %d bps\n",
>> + priv->max_bitrate);
>> + return -EINVAL;
>> + }
>> +
>> memcpy(&priv->data_bittiming, &dbt, sizeof(dbt));
>>
>> if (priv->do_set_data_bittiming) {
>> diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h
>> index 61f1cf2..fbb7810 100644
>> --- a/include/linux/can/dev.h
>> +++ b/include/linux/can/dev.h
>> @@ -48,6 +48,8 @@ struct can_priv {
>> unsigned int data_bitrate_const_cnt;
>> struct can_clock clock;
>>
>> + unsigned int max_bitrate;
>
> Please make it an u32, name it "bitrate_max" and ...
Sure.
>
>>> struct can_priv {
>>> struct net_device *dev;
>>> struct can_device_stats can_stats;
>>>
>>> struct can_bittiming bittiming, data_bittiming;
>>> const struct can_bittiming_const *bittiming_const,
>>> *data_bittiming_const;
>>> const u16 *termination_const;
>>> unsigned int termination_const_cnt;
>>> u16 termination;
>>> const u32 *bitrate_const;
>>> unsigned int bitrate_const_cnt;
>>> const u32 *data_bitrate_const;
>>> unsigned int data_bitrate_const_cnt;
>
> ...move it here.
Ok.
Thanks,
Faiz
^ permalink raw reply
* Re: [PATCH ipsec-next 0/7]: Support multiple VTIs with the same src+dst pair
From: Steffen Klassert @ 2018-01-03 12:10 UTC (permalink / raw)
To: Lorenzo Colitti; +Cc: netdev, subashab, nharold, davem
In-Reply-To: <20171220170607.41516-1-lorenzo@google.com>
On Thu, Dec 21, 2017 at 02:06:00AM +0900, Lorenzo Colitti wrote:
> When using IPsec tunnel mode, VTIs provide many benefits compared
> to direct configuration of xfrm policies / states. However, one
> limitation is that there can only be one VTI between a given pair
> of IP addresses. This does not allow configuring multiple IPsec
> tunnels to the same security gateway. This is required by some
> deployments, for example I-WLAN [3GPP TS 24.327].
>
> This patchset introduces a new VTI_KEYED flag that allows
> configuration of multiple VTIs between the same IP address
> pairs. The semantics are as follows:
>
> - The output path is the same as current VTI behaviour, where a
> routing lookup selects a VTI interface, and the VTI's okey
> specifies the mark to use in the XFRM lookup.
> - The input and ICMP error paths instead work by first looking up
> an SA with a loose match that ignores the mark. That mark is
> then used to find the tunnel by ikey (for input packets) or
> okey (for ICMP errors).
>
> In order for ICMP errors to work, flags are added to the common
> IP lookup functions to ignore the tunnel ikey and to look up
> tunnels by okey instead of ikey.
>
> On the same IP address pair, keyed VTIs can coexist with each
> other (as long as the ikeys are different), but cannot coexist
> with keyless VTIs. This is because the existing keyless VTI
> behaviour (which this series does not change) is to always accept
> packets matching an input policy, regardless of whether there is
> any matching XFRM state. Thus, the keyless VTI would accept the
> traffic for the keyed tunnel and drop it because it would not
> match the keyed tunnel's state.
I think this is not the way we should go. Instead of creating some
new sort of VTI interfaces that have incompatibilities with the old
ones, we should create something new.
The fact that you need new keyed VTIs looks a bit like a workaround
of the design limitations the VTI interfaces have. Unfortunately
this is not the only limitation of VTI and I think we don't get what
we really want by changing VTI without breaking existing userspace.
The problem is that VTI interfaces are IP tunnels, and this is
not the thing we need. The tunnel is already implemented in the
generic xfrm code. All we need is some interface we can route
through. In particular we need something that can work with
transport mode too.
I showed already some ideas on creating xfrm interfaces at the
IPsec workshop in Seoul and my plan is to discuss this at the
upcomming IPsec workshop, so that we get something everybody is
happy with. In particular I want to have feedback from the
userspace IPsec IKE developers before we change/create something.
^ permalink raw reply
* Re: ACPI issues on cold power on [bisected]
From: Rafael J. Wysocki @ 2018-01-03 11:29 UTC (permalink / raw)
To: Jonathan McDowell
Cc: Joonsoo Kim, Rafael J. Wysocki, ACPI Devel Maling List,
Linux Kernel Mailing List, Linux Memory Management List, netdev
In-Reply-To: <20180103103812.klxncszqtq3lj3rr@earth.li>
On Wednesday, January 3, 2018 11:38:12 AM CET Jonathan McDowell wrote:
> On Wed, Jan 03, 2018 at 11:11:29AM +0900, Joonsoo Kim wrote:
> > On Tue, Jan 02, 2018 at 11:25:01AM +0100, Rafael J. Wysocki wrote:
> > > On Tue, Jan 2, 2018 at 3:54 AM, Joonsoo Kim <iamjoonsoo.kim@lge.com> wrote:
> > > > On Fri, Dec 29, 2017 at 04:36:59PM +0000, Jonathan McDowell wrote:
> > > >> On Fri, Dec 22, 2017 at 09:21:09AM +0900, Joonsoo Kim wrote:
> > > >> > On Fri, Dec 08, 2017 at 03:11:59PM +0000, Jonathan McDowell wrote:
> > > >> > > I've been sitting on this for a while and should have spent time to
> > > >> > > investigate sooner, but it's been an odd failure mode that wasn't quite
> > > >> > > obvious.
> > > >> > >
> > > >> > > In 4.9 if I cold power on my laptop (Dell E7240) it fails to boot - I
> > > >> > > don't see anything after grub says its booting. In 4.10 onwards the
> > > >> > > laptop boots, but I get an Oops as part of the boot and ACPI is unhappy
> > > >> > > (no suspend, no clean poweroff, no ACPI buttons). The Oops is below;
> > > >> > > taken from 4.12 as that's the most recent error dmesg I have saved but
> > > >> > > also seen back in 4.10. It's always address 0x30 for the dereference.
> > > >> > >
> > > >> > > Rebooting the laptop does not lead to these problems; it's *only* from a
> > > >> > > complete cold boot that they arise (which didn't help me in terms of
> > > >> > > being able to reliably bisect). Once I realised that I was able to
> > > >> > > bisect, but it leads me to an odd commit:
> > > >> > >
> > > >> > > 86d9f48534e800e4d62cdc1b5aaf539f4c1d47d6
> > > >> > > (mm/slab: fix kmemcg cache creation delayed issue)
> > > >> > >
> > > >> > > If I revert this then I can cold boot without problems.
> > > >> > >
> > > >> > > Also I don't see the problem with a stock Debian kernel, I think because
> > > >> > > the ACPI support is modularised.
> > > >> >
> > > >> > Sorry for late response. I was on a long vacation.
> > > >>
> > > >> No problem. I've been trying to get around to diagnosing this for a
> > > >> while now anyway and this isn't a great time of year for fast responses.
> > > >>
> > > >> > I have tried to solve the problem however I don't find any clue yet.
> > > >> >
> > > >> > >From my analysis, oops report shows that 'struct sock *ssk' passed to
> > > >> > netlink_broadcast_filtered() is NULL. It means that some of
> > > >> > netlink_kernel_create() returns NULL. Maybe, it is due to slab
> > > >> > allocation failure. Could you check it by inserting some log on that
> > > >> > part? The issue cannot be reproducible in my side so I need your help.
> > > >>
> > > >> I've added some debug in acpi_bus_generate_netlink_event +
> > > >> genlmsg_multicast and the problem seems to be that genlmsg_multicast is
> > > >> getting called when init_net.genl_sock has not yet been initialised,
> > > >> leading to the NULL deference.
> > > >>
> > > >> Full dmesg output from a cold 4.14.8 boot at:
> > > >>
> > > >> https://the.earth.li/~noodles/acpi-problem/dmesg-4.14.8-broken
> > > >>
> > > >> And the same kernel after a reboot ("shutdown -r now"):
> > > >>
> > > >> https://the.earth.li/~noodles/acpi-problem/dmesg-4.14.8-working
> > > >>
> > > >> Patch that I've applied is at
> > > >>
> > > >> https://the.earth.li/~noodles/acpi-problem/debug-acpi.diff
> > > >>
> > > >
> > > > Thanks for testing! It's very helpful.
> > > >
> > > >> The interesting difference seems to be:
> > > >>
> > > >> PCI: Using ACPI for IRQ routing
> > > >> +ACPI: Generating event type 208 (:9DBB5994-A997-11DA-B012-B622A1EF5492)
> > > >> +ERROR: init_net.genl_sock is NULL
> > > >> +BUG: unable to handle kernel NULL pointer dereference at 0000000000000030
> > > >> +IP: netlink_broadcast_filtered+0x20/0x3d0
> > > >> +PGD 0 P4D 0
> > > >> +Oops: 0000 [#1] SMP
> > > >> +Modules linked in:
> > > >> +CPU: 0 PID: 29 Comm: kworker/0:1 Not tainted 4.14.8+ #1
> > > >> +Hardware name: Dell Inc. Latitude E7240/07RPNV, BIOS A22 10/18/2017
> > > >> +Workqueue: kacpi_notify acpi_os_execute_deferred
> > > >>
> > > >> 9DBB5994-A997-11DA-B012-B622A1EF5492 is the Dell WMI event GUID and
> > > >> there's no visible event for it on a reboot, just on a cold power on.
> > > >> Some sort of ordering issues such that genl_sock is being initialised
> > > >> later with the slab change?
> > > >
> > > > I have checked that there is an ordering issue.
> > > >
> > > > genl_init() which initializes init_net->genl_sock is called on
> > > > subsys_initcall().
> > > >
> > > > acpi_wmi_init() which schedules acpi_wmi_notify_handler() to the
> > > > workqueue is called on subsys_initcall(), too.
> > > > (acpi_wmi_notify_handler() -> acpi_bus_generate_netlink_event() ->
> > > > netlink_broadcast())
> > > >
> > > > In my system, acpi_wmi_init() is called before the genl_init().
> > > > Therefore, if the worker is scheduled before genl_init() is done, NULL
> > > > derefence would happen.
> > >
> > > Does it help to change the subsys_initcall() in wmi.c to subsys_initcall_sync()?
> >
> > I guess that it would work. I cannot reproduce the issue so it needs
> > to be checked by Jonathan. Jonathan, could you check the problem
> > is disappeared with above change?
>
> I have confirmed that the problem also occurs when using SLUB instead of
> SLAB, and that switching drivers/platform/x86/wmi.c to use
> subsys_initcall_sync() instead of subsys_initcall() fixes the problem
> for both. Weirdly I don't see the ACPI 208 event at boot time being
> raised once that patch is in place.
Interesting.
Anyway, let me send this change as a proper patch.
Thanks,
Rafael
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [net-next: PATCH v2 2/5] device property: Introduce fwnode_get_phy_mode()
From: Rafael J. Wysocki @ 2018-01-03 11:27 UTC (permalink / raw)
To: Marcin Wojtas
Cc: linux-kernel, linux-arm-kernel, netdev, linux-acpi,
graeme.gregory, davem, linux, rafael.j.wysocki, andrew,
f.fainelli, antoine.tenart, thomas.petazzoni, gregory.clement,
ezequiel.garcia, nadavh, neta, ard.biesheuvel, jaz, tn
In-Reply-To: <1514721520-18964-3-git-send-email-mw@semihalf.com>
On Sunday, December 31, 2017 12:58:37 PM CET Marcin Wojtas wrote:
> Until now there were two almost identical functions for
> obtaining network PHY mode - of_get_phy_mode() and,
> more generic, device_get_phy_mode(). However it is not uncommon,
> that the network interface is represented as a child
> of the actual controller, hence it is not associated
> directly to any struct device, required by the latter
> routine.
>
> This commit allows for getting the PHY mode for
> children nodes in the ACPI world by introducing a new function -
> fwnode_get_phy_mode(). This commit also changes
> device_get_phy_mode() routine to be its wrapper, in order
> to prevent unnecessary duplication.
>
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> drivers/base/property.c | 24 ++++++++++++++++----
> include/linux/property.h | 1 +
> 2 files changed, 20 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/base/property.c b/drivers/base/property.c
> index f261d1a..7c4a53d 100644
> --- a/drivers/base/property.c
> +++ b/drivers/base/property.c
> @@ -1126,21 +1126,21 @@ enum dev_dma_attr device_get_dma_attr(struct device *dev)
> EXPORT_SYMBOL_GPL(device_get_dma_attr);
>
> /**
> - * device_get_phy_mode - Get phy mode for given device
> - * @dev: Pointer to the given device
> + * fwnode_get_phy_mode - Get phy mode for given firmware node
> + * @fwnode: Pointer to the given node
> *
> * The function gets phy interface string from property 'phy-mode' or
> * 'phy-connection-type', and return its index in phy_modes table, or errno in
> * error case.
> */
> -int device_get_phy_mode(struct device *dev)
> +int fwnode_get_phy_mode(struct fwnode_handle *fwnode)
> {
> const char *pm;
> int err, i;
>
> - err = device_property_read_string(dev, "phy-mode", &pm);
> + err = fwnode_property_read_string(fwnode, "phy-mode", &pm);
> if (err < 0)
> - err = device_property_read_string(dev,
> + err = fwnode_property_read_string(fwnode,
> "phy-connection-type", &pm);
> if (err < 0)
> return err;
> @@ -1151,6 +1151,20 @@ int device_get_phy_mode(struct device *dev)
>
> return -ENODEV;
> }
> +EXPORT_SYMBOL_GPL(fwnode_get_phy_mode);
> +
> +/**
> + * device_get_phy_mode - Get phy mode for given device
> + * @dev: Pointer to the given device
> + *
> + * The function gets phy interface string from property 'phy-mode' or
> + * 'phy-connection-type', and return its index in phy_modes table, or errno in
> + * error case.
> + */
> +int device_get_phy_mode(struct device *dev)
> +{
> + return fwnode_get_phy_mode(dev_fwnode(dev));
> +}
> EXPORT_SYMBOL_GPL(device_get_phy_mode);
>
> static void *fwnode_get_mac_addr(struct fwnode_handle *fwnode,
> diff --git a/include/linux/property.h b/include/linux/property.h
> index 35620e0..9b13332 100644
> --- a/include/linux/property.h
> +++ b/include/linux/property.h
> @@ -279,6 +279,7 @@ int device_get_phy_mode(struct device *dev);
>
> void *device_get_mac_address(struct device *dev, char *addr, int alen);
>
> +int fwnode_get_phy_mode(struct fwnode_handle *fwnode);
> void *fwnode_get_mac_address(struct fwnode_handle *fwnode,
> char *addr, int alen);
> struct fwnode_handle *fwnode_graph_get_next_endpoint(
>
^ permalink raw reply
* Re: [net-next: PATCH v2 1/5] device property: Introduce fwnode_get_mac_address()
From: Rafael J. Wysocki @ 2018-01-03 11:27 UTC (permalink / raw)
To: Marcin Wojtas
Cc: linux-kernel, linux-arm-kernel, netdev, linux-acpi,
graeme.gregory, davem, linux, rafael.j.wysocki, andrew,
f.fainelli, antoine.tenart, thomas.petazzoni, gregory.clement,
ezequiel.garcia, nadavh, neta, ard.biesheuvel, jaz, tn
In-Reply-To: <1514721520-18964-2-git-send-email-mw@semihalf.com>
On Sunday, December 31, 2017 12:58:36 PM CET Marcin Wojtas wrote:
> Until now there were two almost identical functions for
> obtaining MAC address - of_get_mac_address() and, more generic,
> device_get_mac_address(). However it is not uncommon,
> that the network interface is represented as a child
> of the actual controller, hence it is not associated
> directly to any struct device, required by the latter
> routine.
>
> This commit allows for getting the MAC address for
> children nodes in the ACPI world by introducing a new function -
> fwnode_get_mac_address(). This commit also changes
> device_get_mac_address() routine to be its wrapper, in order
> to prevent unnecessary duplication.
>
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
The changes look reasonable to me, so
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> drivers/base/property.c | 28 ++++++++++++++------
> include/linux/property.h | 2 ++
> 2 files changed, 22 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/base/property.c b/drivers/base/property.c
> index 851b1b6..f261d1a 100644
> --- a/drivers/base/property.c
> +++ b/drivers/base/property.c
> @@ -1153,11 +1153,11 @@ int device_get_phy_mode(struct device *dev)
> }
> EXPORT_SYMBOL_GPL(device_get_phy_mode);
>
> -static void *device_get_mac_addr(struct device *dev,
> +static void *fwnode_get_mac_addr(struct fwnode_handle *fwnode,
> const char *name, char *addr,
> int alen)
> {
> - int ret = device_property_read_u8_array(dev, name, addr, alen);
> + int ret = fwnode_property_read_u8_array(fwnode, name, addr, alen);
>
> if (ret == 0 && alen == ETH_ALEN && is_valid_ether_addr(addr))
> return addr;
> @@ -1165,8 +1165,8 @@ static void *device_get_mac_addr(struct device *dev,
> }
>
> /**
> - * device_get_mac_address - Get the MAC for a given device
> - * @dev: Pointer to the device
> + * fwnode_get_mac_address - Get the MAC from the firmware node
> + * @fwnode: Pointer to the firmware node
> * @addr: Address of buffer to store the MAC in
> * @alen: Length of the buffer pointed to by addr, should be ETH_ALEN
> *
> @@ -1187,19 +1187,31 @@ static void *device_get_mac_addr(struct device *dev,
> * In this case, the real MAC is in 'local-mac-address', and 'mac-address'
> * exists but is all zeros.
> */
> -void *device_get_mac_address(struct device *dev, char *addr, int alen)
> +void *fwnode_get_mac_address(struct fwnode_handle *fwnode, char *addr, int alen)
> {
> char *res;
>
> - res = device_get_mac_addr(dev, "mac-address", addr, alen);
> + res = fwnode_get_mac_addr(fwnode, "mac-address", addr, alen);
> if (res)
> return res;
>
> - res = device_get_mac_addr(dev, "local-mac-address", addr, alen);
> + res = fwnode_get_mac_addr(fwnode, "local-mac-address", addr, alen);
> if (res)
> return res;
>
> - return device_get_mac_addr(dev, "address", addr, alen);
> + return fwnode_get_mac_addr(fwnode, "address", addr, alen);
> +}
> +EXPORT_SYMBOL(fwnode_get_mac_address);
> +
> +/**
> + * device_get_mac_address - Get the MAC for a given device
> + * @dev: Pointer to the device
> + * @addr: Address of buffer to store the MAC in
> + * @alen: Length of the buffer pointed to by addr, should be ETH_ALEN
> + */
> +void *device_get_mac_address(struct device *dev, char *addr, int alen)
> +{
> + return fwnode_get_mac_address(dev_fwnode(dev), addr, alen);
> }
> EXPORT_SYMBOL(device_get_mac_address);
>
> diff --git a/include/linux/property.h b/include/linux/property.h
> index f6189a3..35620e0 100644
> --- a/include/linux/property.h
> +++ b/include/linux/property.h
> @@ -279,6 +279,8 @@ int device_get_phy_mode(struct device *dev);
>
> void *device_get_mac_address(struct device *dev, char *addr, int alen);
>
> +void *fwnode_get_mac_address(struct fwnode_handle *fwnode,
> + char *addr, int alen);
> struct fwnode_handle *fwnode_graph_get_next_endpoint(
> const struct fwnode_handle *fwnode, struct fwnode_handle *prev);
> struct fwnode_handle *
>
^ permalink raw reply
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