* RE: [EXT] Re: [PATCH net-next 4/4] qed: Add devlink support for configuration attributes.
From: Sudarsana Reddy Kalluru @ 2019-07-04 5:49 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Jiri Pirko, davem@davemloft.net, netdev@vger.kernel.org,
Michal Kalderon, Ariel Elior
In-Reply-To: <20190703104244.7d26e734@cakuba.netronome.com>
> -----Original Message-----
> From: Jakub Kicinski <jakub.kicinski@netronome.com>
> Sent: Wednesday, July 3, 2019 11:13 PM
> To: Sudarsana Reddy Kalluru <skalluru@marvell.com>
> Cc: Jiri Pirko <jiri@resnulli.us>; davem@davemloft.net;
> netdev@vger.kernel.org; Michal Kalderon <mkalderon@marvell.com>; Ariel
> Elior <aelior@marvell.com>
> Subject: Re: [EXT] Re: [PATCH net-next 4/4] qed: Add devlink support for
> configuration attributes.
>
> On Wed, 3 Jul 2019 12:56:39 +0000, Sudarsana Reddy Kalluru wrote:
> > Apologies for bringing this topic again. From the driver(s) code
> > paths/'devlink man pages', I understood that devlink-port object is an
> > entity on top of the PCI bus device. Some drivers say NFP represents
> > vnics (on pci-dev) as a devlink-ports and, some represents
> > (virtual?) ports on the PF/device as devlink-ports. In the case of
> > Marvell NIC driver, we don't have [port] partitioning of the PCI
> > device. And the config attributes are specific to PCI-device (not the
> > vports/vnics of PF). Hence I didn't see a need for creating
> > devlink-port objects in the system for Marvell NICs. And planning to
> > add the config attributes to 'devlink-dev' object. Please let me know
> > if my understanding and the proposal is ok?
>
> I understand where you're coming from.
>
> We want to make that judgement call on attribute-by-attribute basis.
> We want consistency across vendors (and, frankly, multiple drivers of the
> same vendor). If attribute looks like it belongs to the port, rather than the
> entire device/ASIC operation, we should make it a port attribute.
Thanks for your mail. I'll go with creating PCI-dev/0 (i.e., port-id 0) for port based attributes as there's no port partitioning for Marvell NICs.
^ permalink raw reply
* RE: [PATCH v1 net-next] net: stmmac: enable clause 45 mdio support
From: Voon, Weifeng @ 2019-07-04 6:05 UTC (permalink / raw)
To: Andrew Lunn
Cc: David S. Miller, Maxime Coquelin, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, Jose Abreu, Giuseppe Cavallaro,
Florian Fainelli, Alexandre Torgue, biao huang, Ong, Boon Leong,
Kweh, Hock Leong
In-Reply-To: <20190704033038.GA6276@lunn.ch>
> > > > @@ -155,22 +171,26 @@ static int stmmac_mdio_read(struct mii_bus
> > > > *bus,
> > > int phyaddr, int phyreg)
> > > > struct stmmac_priv *priv = netdev_priv(ndev);
> > > > unsigned int mii_address = priv->hw->mii.addr;
> > > > unsigned int mii_data = priv->hw->mii.data;
> > > > - u32 v;
> > > > - int data;
> > > > u32 value = MII_BUSY;
> > > > + int data = 0;
> > > > + u32 v;
> > > >
> > > > value |= (phyaddr << priv->hw->mii.addr_shift)
> > > > & priv->hw->mii.addr_mask;
> > > > value |= (phyreg << priv->hw->mii.reg_shift) & priv->hw-
> > > >mii.reg_mask;
> > > > value |= (priv->clk_csr << priv->hw->mii.clk_csr_shift)
> > > > & priv->hw->mii.clk_csr_mask;
> > > > - if (priv->plat->has_gmac4)
> > > > + if (priv->plat->has_gmac4) {
> > > > value |= MII_GMAC4_READ;
> > > > + if (phyreg & MII_ADDR_C45)
> > > > + stmmac_mdio_c45_setup(priv, phyreg, &value, &data);
> > > > + }
> > > >
> > > > if (readl_poll_timeout(priv->ioaddr + mii_address, v, !(v &
> > > MII_BUSY),
> > > > 100, 10000))
> > > > return -EBUSY;
> > > >
> > > > + writel(data, priv->ioaddr + mii_data);
> > >
> > > That looks odd. Could you explain why it is needed.
> > >
> > > Thanks
> > > Andrew
> >
> > Hi Andrew,
> > This mdio c45 support needed to access DWC xPCS which is a Clause-45
>
> I mean it looks odd doing a write to the data register in the middle of
> stmmac_mdio_read().
MAC is using an indirect access to access mdio devices. In order to read,
the driver needs to write into both mii_data and mii_address to select
c45, read/write command, phy address, address to read, and etc.
Weifeng
^ permalink raw reply
* Re: [RFC v2] vhost: introduce mdev based hardware vhost backend
From: Tiwei Bie @ 2019-07-04 6:21 UTC (permalink / raw)
To: Jason Wang
Cc: mst, alex.williamson, maxime.coquelin, linux-kernel, kvm,
virtualization, netdev, dan.daly, cunming.liang, zhihong.wang
In-Reply-To: <b01b8e28-8d96-31dd-56f4-ca7793498c55@redhat.com>
On Thu, Jul 04, 2019 at 12:31:48PM +0800, Jason Wang wrote:
> On 2019/7/3 下午9:08, Tiwei Bie wrote:
> > On Wed, Jul 03, 2019 at 08:16:23PM +0800, Jason Wang wrote:
> > > On 2019/7/3 下午7:52, Tiwei Bie wrote:
> > > > On Wed, Jul 03, 2019 at 06:09:51PM +0800, Jason Wang wrote:
> > > > > On 2019/7/3 下午5:13, Tiwei Bie wrote:
> > > > > > Details about this can be found here:
> > > > > >
> > > > > > https://lwn.net/Articles/750770/
> > > > > >
> > > > > > What's new in this version
> > > > > > ==========================
> > > > > >
> > > > > > A new VFIO device type is introduced - vfio-vhost. This addressed
> > > > > > some comments from here:https://patchwork.ozlabs.org/cover/984763/
> > > > > >
> > > > > > Below is the updated device interface:
> > > > > >
> > > > > > Currently, there are two regions of this device: 1) CONFIG_REGION
> > > > > > (VFIO_VHOST_CONFIG_REGION_INDEX), which can be used to setup the
> > > > > > device; 2) NOTIFY_REGION (VFIO_VHOST_NOTIFY_REGION_INDEX), which
> > > > > > can be used to notify the device.
> > > > > >
> > > > > > 1. CONFIG_REGION
> > > > > >
> > > > > > The region described by CONFIG_REGION is the main control interface.
> > > > > > Messages will be written to or read from this region.
> > > > > >
> > > > > > The message type is determined by the `request` field in message
> > > > > > header. The message size is encoded in the message header too.
> > > > > > The message format looks like this:
> > > > > >
> > > > > > struct vhost_vfio_op {
> > > > > > __u64 request;
> > > > > > __u32 flags;
> > > > > > /* Flag values: */
> > > > > > #define VHOST_VFIO_NEED_REPLY 0x1 /* Whether need reply */
> > > > > > __u32 size;
> > > > > > union {
> > > > > > __u64 u64;
> > > > > > struct vhost_vring_state state;
> > > > > > struct vhost_vring_addr addr;
> > > > > > } payload;
> > > > > > };
> > > > > >
> > > > > > The existing vhost-kernel ioctl cmds are reused as the message
> > > > > > requests in above structure.
> > > > > Still a comments like V1. What's the advantage of inventing a new protocol?
> > > > I'm trying to make it work in VFIO's way..
> > > >
> > > > > I believe either of the following should be better:
> > > > >
> > > > > - using vhost ioctl, we can start from SET_VRING_KICK/SET_VRING_CALL and
> > > > > extend it with e.g notify region. The advantages is that all exist userspace
> > > > > program could be reused without modification (or minimal modification). And
> > > > > vhost API hides lots of details that is not necessary to be understood by
> > > > > application (e.g in the case of container).
> > > > Do you mean reusing vhost's ioctl on VFIO device fd directly,
> > > > or introducing another mdev driver (i.e. vhost_mdev instead of
> > > > using the existing vfio_mdev) for mdev device?
> > > Can we simply add them into ioctl of mdev_parent_ops?
> > Right, either way, these ioctls have to be and just need to be
> > added in the ioctl of the mdev_parent_ops. But another thing we
> > also need to consider is that which file descriptor the userspace
> > will do the ioctl() on. So I'm wondering do you mean let the
> > userspace do the ioctl() on the VFIO device fd of the mdev
> > device?
> >
>
> Yes.
Got it! I'm not sure what's Alex opinion on this. If we all
agree with this, I can do it in this way.
> Is there any other way btw?
Just a quick thought.. Maybe totally a bad idea. I was thinking
whether it would be odd to do non-VFIO's ioctls on VFIO's device
fd. So I was wondering whether it's possible to allow binding
another mdev driver (e.g. vhost_mdev) to the supported mdev
devices. The new mdev driver, vhost_mdev, can provide similar
ways to let userspace open the mdev device and do the vhost ioctls
on it. To distinguish with the vfio_mdev compatible mdev devices,
the device API of the new vhost_mdev compatible mdev devices
might be e.g. "vhost-net" for net?
So in VFIO case, the device will be for passthru directly. And
in VHOST case, the device can be used to accelerate the existing
virtualized devices.
How do you think?
Thanks,
Tiwei
>
> Thanks
>
^ permalink raw reply
* Re: [RFC v2] vhost: introduce mdev based hardware vhost backend
From: Jason Wang @ 2019-07-04 6:35 UTC (permalink / raw)
To: Tiwei Bie
Cc: mst, alex.williamson, maxime.coquelin, linux-kernel, kvm,
virtualization, netdev, dan.daly, cunming.liang, zhihong.wang
In-Reply-To: <20190704062134.GA21116@___>
On 2019/7/4 下午2:21, Tiwei Bie wrote:
> On Thu, Jul 04, 2019 at 12:31:48PM +0800, Jason Wang wrote:
>> On 2019/7/3 下午9:08, Tiwei Bie wrote:
>>> On Wed, Jul 03, 2019 at 08:16:23PM +0800, Jason Wang wrote:
>>>> On 2019/7/3 下午7:52, Tiwei Bie wrote:
>>>>> On Wed, Jul 03, 2019 at 06:09:51PM +0800, Jason Wang wrote:
>>>>>> On 2019/7/3 下午5:13, Tiwei Bie wrote:
>>>>>>> Details about this can be found here:
>>>>>>>
>>>>>>> https://lwn.net/Articles/750770/
>>>>>>>
>>>>>>> What's new in this version
>>>>>>> ==========================
>>>>>>>
>>>>>>> A new VFIO device type is introduced - vfio-vhost. This addressed
>>>>>>> some comments from here:https://patchwork.ozlabs.org/cover/984763/
>>>>>>>
>>>>>>> Below is the updated device interface:
>>>>>>>
>>>>>>> Currently, there are two regions of this device: 1) CONFIG_REGION
>>>>>>> (VFIO_VHOST_CONFIG_REGION_INDEX), which can be used to setup the
>>>>>>> device; 2) NOTIFY_REGION (VFIO_VHOST_NOTIFY_REGION_INDEX), which
>>>>>>> can be used to notify the device.
>>>>>>>
>>>>>>> 1. CONFIG_REGION
>>>>>>>
>>>>>>> The region described by CONFIG_REGION is the main control interface.
>>>>>>> Messages will be written to or read from this region.
>>>>>>>
>>>>>>> The message type is determined by the `request` field in message
>>>>>>> header. The message size is encoded in the message header too.
>>>>>>> The message format looks like this:
>>>>>>>
>>>>>>> struct vhost_vfio_op {
>>>>>>> __u64 request;
>>>>>>> __u32 flags;
>>>>>>> /* Flag values: */
>>>>>>> #define VHOST_VFIO_NEED_REPLY 0x1 /* Whether need reply */
>>>>>>> __u32 size;
>>>>>>> union {
>>>>>>> __u64 u64;
>>>>>>> struct vhost_vring_state state;
>>>>>>> struct vhost_vring_addr addr;
>>>>>>> } payload;
>>>>>>> };
>>>>>>>
>>>>>>> The existing vhost-kernel ioctl cmds are reused as the message
>>>>>>> requests in above structure.
>>>>>> Still a comments like V1. What's the advantage of inventing a new protocol?
>>>>> I'm trying to make it work in VFIO's way..
>>>>>
>>>>>> I believe either of the following should be better:
>>>>>>
>>>>>> - using vhost ioctl, we can start from SET_VRING_KICK/SET_VRING_CALL and
>>>>>> extend it with e.g notify region. The advantages is that all exist userspace
>>>>>> program could be reused without modification (or minimal modification). And
>>>>>> vhost API hides lots of details that is not necessary to be understood by
>>>>>> application (e.g in the case of container).
>>>>> Do you mean reusing vhost's ioctl on VFIO device fd directly,
>>>>> or introducing another mdev driver (i.e. vhost_mdev instead of
>>>>> using the existing vfio_mdev) for mdev device?
>>>> Can we simply add them into ioctl of mdev_parent_ops?
>>> Right, either way, these ioctls have to be and just need to be
>>> added in the ioctl of the mdev_parent_ops. But another thing we
>>> also need to consider is that which file descriptor the userspace
>>> will do the ioctl() on. So I'm wondering do you mean let the
>>> userspace do the ioctl() on the VFIO device fd of the mdev
>>> device?
>>>
>> Yes.
> Got it! I'm not sure what's Alex opinion on this. If we all
> agree with this, I can do it in this way.
>
>> Is there any other way btw?
> Just a quick thought.. Maybe totally a bad idea.
It's not for sure :)
> I was thinking
> whether it would be odd to do non-VFIO's ioctls on VFIO's device
> fd. So I was wondering whether it's possible to allow binding
> another mdev driver (e.g. vhost_mdev) to the supported mdev
> devices. The new mdev driver, vhost_mdev, can provide similar
> ways to let userspace open the mdev device and do the vhost ioctls
> on it. To distinguish with the vfio_mdev compatible mdev devices,
> the device API of the new vhost_mdev compatible mdev devices
> might be e.g. "vhost-net" for net?
>
> So in VFIO case, the device will be for passthru directly. And
> in VHOST case, the device can be used to accelerate the existing
> virtualized devices.
>
> How do you think?
If my understanding is correct, there will be no VFIO ioctl if we go for
vhost_mdev?
Thanks
>
> Thanks,
> Tiwei
>> Thanks
>>
^ permalink raw reply
* Re: [RFC v2] vhost: introduce mdev based hardware vhost backend
From: Tiwei Bie @ 2019-07-04 7:02 UTC (permalink / raw)
To: Jason Wang
Cc: mst, alex.williamson, maxime.coquelin, linux-kernel, kvm,
virtualization, netdev, dan.daly, cunming.liang, zhihong.wang
In-Reply-To: <c67f628f-e0c1-9a41-6c5d-b6bbda31467d@redhat.com>
On Thu, Jul 04, 2019 at 02:35:20PM +0800, Jason Wang wrote:
> On 2019/7/4 下午2:21, Tiwei Bie wrote:
> > On Thu, Jul 04, 2019 at 12:31:48PM +0800, Jason Wang wrote:
> > > On 2019/7/3 下午9:08, Tiwei Bie wrote:
> > > > On Wed, Jul 03, 2019 at 08:16:23PM +0800, Jason Wang wrote:
> > > > > On 2019/7/3 下午7:52, Tiwei Bie wrote:
> > > > > > On Wed, Jul 03, 2019 at 06:09:51PM +0800, Jason Wang wrote:
> > > > > > > On 2019/7/3 下午5:13, Tiwei Bie wrote:
> > > > > > > > Details about this can be found here:
> > > > > > > >
> > > > > > > > https://lwn.net/Articles/750770/
> > > > > > > >
> > > > > > > > What's new in this version
> > > > > > > > ==========================
> > > > > > > >
> > > > > > > > A new VFIO device type is introduced - vfio-vhost. This addressed
> > > > > > > > some comments from here:https://patchwork.ozlabs.org/cover/984763/
> > > > > > > >
> > > > > > > > Below is the updated device interface:
> > > > > > > >
> > > > > > > > Currently, there are two regions of this device: 1) CONFIG_REGION
> > > > > > > > (VFIO_VHOST_CONFIG_REGION_INDEX), which can be used to setup the
> > > > > > > > device; 2) NOTIFY_REGION (VFIO_VHOST_NOTIFY_REGION_INDEX), which
> > > > > > > > can be used to notify the device.
> > > > > > > >
> > > > > > > > 1. CONFIG_REGION
> > > > > > > >
> > > > > > > > The region described by CONFIG_REGION is the main control interface.
> > > > > > > > Messages will be written to or read from this region.
> > > > > > > >
> > > > > > > > The message type is determined by the `request` field in message
> > > > > > > > header. The message size is encoded in the message header too.
> > > > > > > > The message format looks like this:
> > > > > > > >
> > > > > > > > struct vhost_vfio_op {
> > > > > > > > __u64 request;
> > > > > > > > __u32 flags;
> > > > > > > > /* Flag values: */
> > > > > > > > #define VHOST_VFIO_NEED_REPLY 0x1 /* Whether need reply */
> > > > > > > > __u32 size;
> > > > > > > > union {
> > > > > > > > __u64 u64;
> > > > > > > > struct vhost_vring_state state;
> > > > > > > > struct vhost_vring_addr addr;
> > > > > > > > } payload;
> > > > > > > > };
> > > > > > > >
> > > > > > > > The existing vhost-kernel ioctl cmds are reused as the message
> > > > > > > > requests in above structure.
> > > > > > > Still a comments like V1. What's the advantage of inventing a new protocol?
> > > > > > I'm trying to make it work in VFIO's way..
> > > > > >
> > > > > > > I believe either of the following should be better:
> > > > > > >
> > > > > > > - using vhost ioctl, we can start from SET_VRING_KICK/SET_VRING_CALL and
> > > > > > > extend it with e.g notify region. The advantages is that all exist userspace
> > > > > > > program could be reused without modification (or minimal modification). And
> > > > > > > vhost API hides lots of details that is not necessary to be understood by
> > > > > > > application (e.g in the case of container).
> > > > > > Do you mean reusing vhost's ioctl on VFIO device fd directly,
> > > > > > or introducing another mdev driver (i.e. vhost_mdev instead of
> > > > > > using the existing vfio_mdev) for mdev device?
> > > > > Can we simply add them into ioctl of mdev_parent_ops?
> > > > Right, either way, these ioctls have to be and just need to be
> > > > added in the ioctl of the mdev_parent_ops. But another thing we
> > > > also need to consider is that which file descriptor the userspace
> > > > will do the ioctl() on. So I'm wondering do you mean let the
> > > > userspace do the ioctl() on the VFIO device fd of the mdev
> > > > device?
> > > >
> > > Yes.
> > Got it! I'm not sure what's Alex opinion on this. If we all
> > agree with this, I can do it in this way.
> >
> > > Is there any other way btw?
> > Just a quick thought.. Maybe totally a bad idea.
>
>
> It's not for sure :)
Thanks!
>
>
> > I was thinking
> > whether it would be odd to do non-VFIO's ioctls on VFIO's device
> > fd. So I was wondering whether it's possible to allow binding
> > another mdev driver (e.g. vhost_mdev) to the supported mdev
> > devices. The new mdev driver, vhost_mdev, can provide similar
> > ways to let userspace open the mdev device and do the vhost ioctls
> > on it. To distinguish with the vfio_mdev compatible mdev devices,
> > the device API of the new vhost_mdev compatible mdev devices
> > might be e.g. "vhost-net" for net?
> >
> > So in VFIO case, the device will be for passthru directly. And
> > in VHOST case, the device can be used to accelerate the existing
> > virtualized devices.
> >
> > How do you think?
>
>
> If my understanding is correct, there will be no VFIO ioctl if we go for
> vhost_mdev?
Yeah, exactly. If we go for vhost_mdev, we may have some vhost nodes
in /dev similar to what /dev/vfio/* does to handle the $UUID and open
the device (e.g. similar to VFIO_GROUP_GET_DEVICE_FD in VFIO). And
to setup the device, we can try to reuse the ioctls of the existing
kernel vhost as much as possible.
Thanks,
Tiwei
>
> Thanks
>
>
> >
> > Thanks,
> > Tiwei
> > > Thanks
> > >
^ permalink raw reply
* Re: [PATCH v2 1/4] net: dsa: Change DT bindings for Vitesse VSC73xx switches
From: Linus Walleij @ 2019-07-04 7:05 UTC (permalink / raw)
To: Pawel Dembicki
Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, David S. Miller,
Rob Herring, Mark Rutland, netdev,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
linux-kernel@vger.kernel.org
In-Reply-To: <20190703171924.31801-2-paweldembicki@gmail.com>
On Wed, Jul 3, 2019 at 7:21 PM Pawel Dembicki <paweldembicki@gmail.com> wrote:
> This commit introduce how to use vsc73xx platform driver.
>
> Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>
Nice!
> +If Platform driver is used, the device tree node is an platform device so it
> +must reside inside a platform bus device tree node.
I would write something like "when connected to a memory bus, and
used in memory-mapped I/O mode, a platform device is used to represent
the vsc73xx" so it is clear what is going on.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH v2 2/4] net: dsa: vsc73xx: Split vsc73xx driver
From: Linus Walleij @ 2019-07-04 7:08 UTC (permalink / raw)
To: Pawel Dembicki
Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, David S. Miller,
Rob Herring, Mark Rutland, netdev,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
linux-kernel@vger.kernel.org
In-Reply-To: <20190703171924.31801-3-paweldembicki@gmail.com>
On Wed, Jul 3, 2019 at 7:21 PM Pawel Dembicki <paweldembicki@gmail.com> wrote:
> This driver (currently) only takes control of the switch chip over
> SPI and configures it to route packages around when connected to a
> CPU port. But Vitesse chip support also parallel interface.
>
> This patch split driver into two parts: core and spi. It is required
> for add support to another managing interface.
>
> Tested-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>
That's a nice and clean split.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH net-next 0/8] mlxsw: Enable/disable PTP shapers
From: Ido Schimmel @ 2019-07-04 7:07 UTC (permalink / raw)
To: netdev; +Cc: davem, richardcochran, jiri, petrm, shalomt, mlxsw, Ido Schimmel
From: Ido Schimmel <idosch@mellanox.com>
Shalom says:
In order to get more accurate hardware time stamping in Spectrum-1, the
driver needs to apply a shaper on the port for speeds lower than 40Gbps.
This shaper is called a PTP shaper and it is applied on hierarchy 0,
which is the port hierarchy. This shaper may affect the shaper rates of
all hierarchies.
This patchset adds the ability to enable or disable the PTP shaper on
the port in two scenarios:
1. When the user wants to enable/disable the hardware time stamping
2. When the port is brought up or down (including port speed change)
Patch #1 adds the QEEC.ptps field that is used for enabling or disabling
the PTP shaper on a port.
Patch #2 adds a note about disabling the PTP shaper when calling to
mlxsw_sp_port_ets_maxrate_set().
Patch #3 adds the QPSC register that is responsible for configuring the
PTP shaper parameters per speed.
Patch #4 sets the PTP shaper parameters during the ptp_init().
Patch #5 adds new operation for getting the port's speed.
Patch #6 enables/disables the PTP shaper when turning on or off the
hardware time stamping.
Patch #7 enables/disables the PTP shaper when the port's status has
changed (including port speed change).
Patch #8 applies the PTP shaper enable/disable logic by filling the PTP
shaper parameters array.
Shalom Toledo (8):
mlxsw: reg: Add ptps field in QoS ETS Element Configuration Register
mlxsw: spectrum: Add note about the PTP shaper
mlxsw: reg: Add QoS PTP Shaper Configuration Register
mlxsw: spectrum_ptp: Set the PTP shaper parameters
mlxsw: spectrum: Add new operation for getting the port's speed
mlxsw: spectrum_ptp: Enable/disable PTP shaper on a port when getting
HWTSTAMP on/off
mlxsw: spectrum: Set up PTP shaper when port status has changed
mlxsw: spectrum_ptp: Apply the PTP shaper enable/disable logic
drivers/net/ethernet/mellanox/mlxsw/reg.h | 129 ++++++++++++++
.../net/ethernet/mellanox/mlxsw/spectrum.c | 81 +++++----
.../net/ethernet/mellanox/mlxsw/spectrum.h | 2 +
.../ethernet/mellanox/mlxsw/spectrum_ptp.c | 158 ++++++++++++++++++
.../ethernet/mellanox/mlxsw/spectrum_ptp.h | 10 ++
5 files changed, 350 insertions(+), 30 deletions(-)
--
2.20.1
^ permalink raw reply
* [PATCH net-next 2/8] mlxsw: spectrum: Add note about the PTP shaper
From: Ido Schimmel @ 2019-07-04 7:07 UTC (permalink / raw)
To: netdev; +Cc: davem, richardcochran, jiri, petrm, shalomt, mlxsw, Ido Schimmel
In-Reply-To: <20190704070740.302-1-idosch@idosch.org>
From: Shalom Toledo <shalomt@mellanox.com>
Add note about disabling the PTP shaper when calling to
mlxsw_sp_port_ets_maxrate_set().
Signed-off-by: Shalom Toledo <shalomt@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
Reviewed-by: Petr Machata <petrm@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 755b14b82c8f..67133ba53015 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -3457,8 +3457,9 @@ static int mlxsw_sp_port_ets_init(struct mlxsw_sp_port *mlxsw_sp_port)
return err;
}
- /* Make sure the max shaper is disabled in all hierarchies that
- * support it.
+ /* Make sure the max shaper is disabled in all hierarchies that support
+ * it. Note that this disables ptps (PTP shaper), but that is intended
+ * for the initial configuration.
*/
err = mlxsw_sp_port_ets_maxrate_set(mlxsw_sp_port,
MLXSW_REG_QEEC_HIERARCY_PORT, 0, 0,
--
2.20.1
^ permalink raw reply related
* [PATCH net-next 1/8] mlxsw: reg: Add ptps field in QoS ETS Element Configuration Register
From: Ido Schimmel @ 2019-07-04 7:07 UTC (permalink / raw)
To: netdev; +Cc: davem, richardcochran, jiri, petrm, shalomt, mlxsw, Ido Schimmel
In-Reply-To: <20190704070740.302-1-idosch@idosch.org>
From: Shalom Toledo <shalomt@mellanox.com>
The PTP Shaper field is used for enabling and disabling of port-rate based
shaper which is slightly lower than port rate.
Signed-off-by: Shalom Toledo <shalomt@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
Reviewed-by: Petr Machata <petrm@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
drivers/net/ethernet/mellanox/mlxsw/reg.h | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/reg.h b/drivers/net/ethernet/mellanox/mlxsw/reg.h
index 76ff5b217c04..d2e2a75f7983 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/reg.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/reg.h
@@ -3515,6 +3515,18 @@ MLXSW_ITEM32(reg, qeec, next_element_index, 0x08, 0, 8);
*/
MLXSW_ITEM32(reg, qeec, mise, 0x0C, 31, 1);
+/* reg_qeec_ptps
+ * PTP shaper
+ * 0: regular shaper mode
+ * 1: PTP oriented shaper
+ * Allowed only for hierarchy 0
+ * Not supported for CPU port
+ * Note that ptps mode may affect the shaper rates of all hierarchies
+ * Supported only on Spectrum-1
+ * Access: RW
+ */
+MLXSW_ITEM32(reg, qeec, ptps, 0x0C, 29, 1);
+
enum {
MLXSW_REG_QEEC_BYTES_MODE,
MLXSW_REG_QEEC_PACKETS_MODE,
@@ -3601,6 +3613,16 @@ static inline void mlxsw_reg_qeec_pack(char *payload, u8 local_port,
mlxsw_reg_qeec_next_element_index_set(payload, next_index);
}
+static inline void mlxsw_reg_qeec_ptps_pack(char *payload, u8 local_port,
+ bool ptps)
+{
+ MLXSW_REG_ZERO(qeec, payload);
+ mlxsw_reg_qeec_local_port_set(payload, local_port);
+ mlxsw_reg_qeec_element_hierarchy_set(payload,
+ MLXSW_REG_QEEC_HIERARCY_PORT);
+ mlxsw_reg_qeec_ptps_set(payload, ptps);
+}
+
/* QRWE - QoS ReWrite Enable
* -------------------------
* This register configures the rewrite enable per receive port.
--
2.20.1
^ permalink raw reply related
* [PATCH net-next 3/8] mlxsw: reg: Add QoS PTP Shaper Configuration Register
From: Ido Schimmel @ 2019-07-04 7:07 UTC (permalink / raw)
To: netdev; +Cc: davem, richardcochran, jiri, petrm, shalomt, mlxsw, Ido Schimmel
In-Reply-To: <20190704070740.302-1-idosch@idosch.org>
From: Shalom Toledo <shalomt@mellanox.com>
The QPSC allows advanced configuration of the PTP shapers.
Signed-off-by: Shalom Toledo <shalomt@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
Reviewed-by: Petr Machata <petrm@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
drivers/net/ethernet/mellanox/mlxsw/reg.h | 107 ++++++++++++++++++++++
1 file changed, 107 insertions(+)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/reg.h b/drivers/net/ethernet/mellanox/mlxsw/reg.h
index d2e2a75f7983..ead36702549a 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/reg.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/reg.h
@@ -3836,6 +3836,112 @@ mlxsw_reg_qtctm_pack(char *payload, u8 local_port, bool mc)
mlxsw_reg_qtctm_mc_set(payload, mc);
}
+/* QPSC - QoS PTP Shaper Configuration Register
+ * --------------------------------------------
+ * The QPSC allows advanced configuration of the shapers when QEEC.ptps=1.
+ * Supported only on Spectrum-1.
+ */
+#define MLXSW_REG_QPSC_ID 0x401B
+#define MLXSW_REG_QPSC_LEN 0x28
+
+MLXSW_REG_DEFINE(qpsc, MLXSW_REG_QPSC_ID, MLXSW_REG_QPSC_LEN);
+
+enum mlxsw_reg_qpsc_port_speed {
+ MLXSW_REG_QPSC_PORT_SPEED_100M,
+ MLXSW_REG_QPSC_PORT_SPEED_1G,
+ MLXSW_REG_QPSC_PORT_SPEED_10G,
+ MLXSW_REG_QPSC_PORT_SPEED_25G,
+};
+
+/* reg_qpsc_port_speed
+ * Port speed.
+ * Access: Index
+ */
+MLXSW_ITEM32(reg, qpsc, port_speed, 0x00, 0, 4);
+
+/* reg_qpsc_shaper_time_exp
+ * The base-time-interval for updating the shapers tokens (for all hierarchies).
+ * shaper_update_rate = 2 ^ shaper_time_exp * (1 + shaper_time_mantissa) * 32nSec
+ * shaper_rate = 64bit * shaper_inc / shaper_update_rate
+ * Access: RW
+ */
+MLXSW_ITEM32(reg, qpsc, shaper_time_exp, 0x04, 16, 4);
+
+/* reg_qpsc_shaper_time_mantissa
+ * The base-time-interval for updating the shapers tokens (for all hierarchies).
+ * shaper_update_rate = 2 ^ shaper_time_exp * (1 + shaper_time_mantissa) * 32nSec
+ * shaper_rate = 64bit * shaper_inc / shaper_update_rate
+ * Access: RW
+ */
+MLXSW_ITEM32(reg, qpsc, shaper_time_mantissa, 0x04, 0, 5);
+
+/* reg_qpsc_shaper_inc
+ * Number of tokens added to shaper on each update.
+ * Units of 8B.
+ * Access: RW
+ */
+MLXSW_ITEM32(reg, qpsc, shaper_inc, 0x08, 0, 5);
+
+/* reg_qpsc_shaper_bs
+ * Max shaper Burst size.
+ * Burst size is 2 ^ max_shaper_bs * 512 [bits]
+ * Range is: 5..25 (from 2KB..2GB)
+ * Access: RW
+ */
+MLXSW_ITEM32(reg, qpsc, shaper_bs, 0x0C, 0, 6);
+
+/* reg_qpsc_ptsc_we
+ * Write enable to port_to_shaper_credits.
+ * Access: WO
+ */
+MLXSW_ITEM32(reg, qpsc, ptsc_we, 0x10, 31, 1);
+
+/* reg_qpsc_port_to_shaper_credits
+ * For split ports: range 1..57
+ * For non-split ports: range 1..112
+ * Written only when ptsc_we is set.
+ * Access: RW
+ */
+MLXSW_ITEM32(reg, qpsc, port_to_shaper_credits, 0x10, 0, 8);
+
+/* reg_qpsc_ing_timestamp_inc
+ * Ingress timestamp increment.
+ * 2's complement.
+ * The timestamp of MTPPTR at ingress will be incremented by this value. Global
+ * value for all ports.
+ * Same units as used by MTPPTR.
+ * Access: RW
+ */
+MLXSW_ITEM32(reg, qpsc, ing_timestamp_inc, 0x20, 0, 32);
+
+/* reg_qpsc_egr_timestamp_inc
+ * Egress timestamp increment.
+ * 2's complement.
+ * The timestamp of MTPPTR at egress will be incremented by this value. Global
+ * value for all ports.
+ * Same units as used by MTPPTR.
+ * Access: RW
+ */
+MLXSW_ITEM32(reg, qpsc, egr_timestamp_inc, 0x24, 0, 32);
+
+static inline void
+mlxsw_reg_qpsc_pack(char *payload, enum mlxsw_reg_qpsc_port_speed port_speed,
+ u8 shaper_time_exp, u8 shaper_time_mantissa, u8 shaper_inc,
+ u8 shaper_bs, u8 port_to_shaper_credits,
+ int ing_timestamp_inc, int egr_timestamp_inc)
+{
+ MLXSW_REG_ZERO(qpsc, payload);
+ mlxsw_reg_qpsc_port_speed_set(payload, port_speed);
+ mlxsw_reg_qpsc_shaper_time_exp_set(payload, shaper_time_exp);
+ mlxsw_reg_qpsc_shaper_time_mantissa_set(payload, shaper_time_mantissa);
+ mlxsw_reg_qpsc_shaper_inc_set(payload, shaper_inc);
+ mlxsw_reg_qpsc_shaper_bs_set(payload, shaper_bs);
+ mlxsw_reg_qpsc_ptsc_we_set(payload, true);
+ mlxsw_reg_qpsc_port_to_shaper_credits_set(payload, port_to_shaper_credits);
+ mlxsw_reg_qpsc_ing_timestamp_inc_set(payload, ing_timestamp_inc);
+ mlxsw_reg_qpsc_egr_timestamp_inc_set(payload, egr_timestamp_inc);
+}
+
/* PMLP - Ports Module to Local Port Register
* ------------------------------------------
* Configures the assignment of modules to local ports.
@@ -10396,6 +10502,7 @@ static const struct mlxsw_reg_info *mlxsw_reg_infos[] = {
MLXSW_REG(qpdsm),
MLXSW_REG(qpdpm),
MLXSW_REG(qtctm),
+ MLXSW_REG(qpsc),
MLXSW_REG(pmlp),
MLXSW_REG(pmtu),
MLXSW_REG(ptys),
--
2.20.1
^ permalink raw reply related
* [PATCH net-next 4/8] mlxsw: spectrum_ptp: Set the PTP shaper parameters
From: Ido Schimmel @ 2019-07-04 7:07 UTC (permalink / raw)
To: netdev; +Cc: davem, richardcochran, jiri, petrm, shalomt, mlxsw, Ido Schimmel
In-Reply-To: <20190704070740.302-1-idosch@idosch.org>
From: Shalom Toledo <shalomt@mellanox.com>
Set the PTP shaper parameters during the ptp_init(). For different
speeds, there are different parameters.
When the port's speed changes and PTP shaper is enabled, the firmware
changes the ETS shaper values according to the PTP shaper parameters for
this new speed.
The PTP shaper parameters array is left empty for now, will be filled in
a follow-up patch.
Signed-off-by: Shalom Toledo <shalomt@mellanox.com>
Reviewed-by: Petr Machata <petrm@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
.../ethernet/mellanox/mlxsw/spectrum_ptp.c | 45 +++++++++++++++++++
1 file changed, 45 insertions(+)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c
index 437023d67a3b..08e62cb3d7f2 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c
@@ -753,12 +753,57 @@ static int mlxsw_sp1_ptp_mtpppc_set(struct mlxsw_sp *mlxsw_sp,
return mlxsw_reg_write(mlxsw_sp->core, MLXSW_REG(mtpppc), mtpppc_pl);
}
+struct mlxsw_sp1_ptp_shaper_params {
+ u32 ethtool_speed;
+ enum mlxsw_reg_qpsc_port_speed port_speed;
+ u8 shaper_time_exp;
+ u8 shaper_time_mantissa;
+ u8 shaper_inc;
+ u8 shaper_bs;
+ u8 port_to_shaper_credits;
+ int ing_timestamp_inc;
+ int egr_timestamp_inc;
+};
+
+static const struct mlxsw_sp1_ptp_shaper_params
+mlxsw_sp1_ptp_shaper_params[] = {
+};
+
+#define MLXSW_SP1_PTP_SHAPER_PARAMS_LEN ARRAY_SIZE(mlxsw_sp1_ptp_shaper_params)
+
+static int mlxsw_sp1_ptp_shaper_params_set(struct mlxsw_sp *mlxsw_sp)
+{
+ const struct mlxsw_sp1_ptp_shaper_params *params;
+ char qpsc_pl[MLXSW_REG_QPSC_LEN];
+ int i, err;
+
+ for (i = 0; i < MLXSW_SP1_PTP_SHAPER_PARAMS_LEN; i++) {
+ params = &mlxsw_sp1_ptp_shaper_params[i];
+ mlxsw_reg_qpsc_pack(qpsc_pl, params->port_speed,
+ params->shaper_time_exp,
+ params->shaper_time_mantissa,
+ params->shaper_inc, params->shaper_bs,
+ params->port_to_shaper_credits,
+ params->ing_timestamp_inc,
+ params->egr_timestamp_inc);
+ err = mlxsw_reg_write(mlxsw_sp->core, MLXSW_REG(qpsc), qpsc_pl);
+ if (err)
+ return err;
+ }
+
+ return 0;
+}
+
struct mlxsw_sp_ptp_state *mlxsw_sp1_ptp_init(struct mlxsw_sp *mlxsw_sp)
{
struct mlxsw_sp_ptp_state *ptp_state;
u16 message_type;
int err;
+ err = mlxsw_sp1_ptp_shaper_params_set(mlxsw_sp);
+ if (err)
+ return ERR_PTR(err);
+
ptp_state = kzalloc(sizeof(*ptp_state), GFP_KERNEL);
if (!ptp_state)
return ERR_PTR(-ENOMEM);
--
2.20.1
^ permalink raw reply related
* [PATCH net-next 6/8] mlxsw: spectrum_ptp: Enable/disable PTP shaper on a port when getting HWTSTAMP on/off
From: Ido Schimmel @ 2019-07-04 7:07 UTC (permalink / raw)
To: netdev; +Cc: davem, richardcochran, jiri, petrm, shalomt, mlxsw, Ido Schimmel
In-Reply-To: <20190704070740.302-1-idosch@idosch.org>
From: Shalom Toledo <shalomt@mellanox.com>
In order to get more accurate hardware time stamping, the driver needs to
enable PTP shaper on the port, for speeds lower than 40 Gbps.
Enable the PTP shaper on the port when the user turns on the hardware
time stamping, and disable it when the user turns off the hardware time
stamping.
Signed-off-by: Shalom Toledo <shalomt@mellanox.com>
Reviewed-by: Petr Machata <petrm@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
.../ethernet/mellanox/mlxsw/spectrum_ptp.c | 52 +++++++++++++++++++
1 file changed, 52 insertions(+)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c
index 08e62cb3d7f2..49aba0ce896b 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c
@@ -952,6 +952,54 @@ static int mlxsw_sp1_ptp_mtpppc_update(struct mlxsw_sp_port *mlxsw_sp_port,
ing_types, egr_types);
}
+static bool mlxsw_sp1_ptp_hwtstamp_enabled(struct mlxsw_sp_port *mlxsw_sp_port)
+{
+ return mlxsw_sp_port->ptp.ing_types || mlxsw_sp_port->ptp.egr_types;
+}
+
+static int
+mlxsw_sp1_ptp_port_shaper_set(struct mlxsw_sp_port *mlxsw_sp_port, bool enable)
+{
+ struct mlxsw_sp *mlxsw_sp = mlxsw_sp_port->mlxsw_sp;
+ char qeec_pl[MLXSW_REG_QEEC_LEN];
+
+ mlxsw_reg_qeec_ptps_pack(qeec_pl, mlxsw_sp_port->local_port, enable);
+ return mlxsw_reg_write(mlxsw_sp->core, MLXSW_REG(qeec), qeec_pl);
+}
+
+static int mlxsw_sp1_ptp_port_shaper_check(struct mlxsw_sp_port *mlxsw_sp_port)
+{
+ const struct mlxsw_sp_port_type_speed_ops *port_type_speed_ops;
+ struct mlxsw_sp *mlxsw_sp = mlxsw_sp_port->mlxsw_sp;
+ char ptys_pl[MLXSW_REG_PTYS_LEN];
+ u32 eth_proto_oper, speed;
+ bool ptps = false;
+ int err, i;
+
+ if (!mlxsw_sp1_ptp_hwtstamp_enabled(mlxsw_sp_port))
+ return mlxsw_sp1_ptp_port_shaper_set(mlxsw_sp_port, false);
+
+ port_type_speed_ops = mlxsw_sp->port_type_speed_ops;
+ port_type_speed_ops->reg_ptys_eth_pack(mlxsw_sp, ptys_pl,
+ mlxsw_sp_port->local_port, 0,
+ false);
+ err = mlxsw_reg_query(mlxsw_sp->core, MLXSW_REG(ptys), ptys_pl);
+ if (err)
+ return err;
+ port_type_speed_ops->reg_ptys_eth_unpack(mlxsw_sp, ptys_pl, NULL, NULL,
+ ð_proto_oper);
+
+ speed = port_type_speed_ops->from_ptys_speed(mlxsw_sp, eth_proto_oper);
+ for (i = 0; i < MLXSW_SP1_PTP_SHAPER_PARAMS_LEN; i++) {
+ if (mlxsw_sp1_ptp_shaper_params[i].ethtool_speed == speed) {
+ ptps = true;
+ break;
+ }
+ }
+
+ return mlxsw_sp1_ptp_port_shaper_set(mlxsw_sp_port, ptps);
+}
+
int mlxsw_sp1_ptp_hwtstamp_set(struct mlxsw_sp_port *mlxsw_sp_port,
struct hwtstamp_config *config)
{
@@ -973,6 +1021,10 @@ int mlxsw_sp1_ptp_hwtstamp_set(struct mlxsw_sp_port *mlxsw_sp_port,
mlxsw_sp_port->ptp.ing_types = ing_types;
mlxsw_sp_port->ptp.egr_types = egr_types;
+ err = mlxsw_sp1_ptp_port_shaper_check(mlxsw_sp_port);
+ if (err)
+ return err;
+
/* Notify the ioctl caller what we are actually timestamping. */
config->rx_filter = rx_filter;
--
2.20.1
^ permalink raw reply related
* [PATCH net-next 5/8] mlxsw: spectrum: Add new operation for getting the port's speed
From: Ido Schimmel @ 2019-07-04 7:07 UTC (permalink / raw)
To: netdev; +Cc: davem, richardcochran, jiri, petrm, shalomt, mlxsw, Ido Schimmel
In-Reply-To: <20190704070740.302-1-idosch@idosch.org>
From: Shalom Toledo <shalomt@mellanox.com>
New operation for getting the port's speed as part of port-type-speed
operations.
Signed-off-by: Shalom Toledo <shalomt@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
Reviewed-by: Petr Machata <petrm@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
.../net/ethernet/mellanox/mlxsw/spectrum.c | 68 +++++++++++--------
.../net/ethernet/mellanox/mlxsw/spectrum.h | 1 +
2 files changed, 41 insertions(+), 28 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 67133ba53015..7cfca2be09fc 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -2655,28 +2655,33 @@ mlxsw_sp1_from_ptys_link(struct mlxsw_sp *mlxsw_sp, u32 ptys_eth_proto,
}
}
+static u32
+mlxsw_sp1_from_ptys_speed(struct mlxsw_sp *mlxsw_sp, u32 ptys_eth_proto)
+{
+ int i;
+
+ for (i = 0; i < MLXSW_SP1_PORT_LINK_MODE_LEN; i++) {
+ if (ptys_eth_proto & mlxsw_sp1_port_link_mode[i].mask)
+ return mlxsw_sp1_port_link_mode[i].speed;
+ }
+
+ return SPEED_UNKNOWN;
+}
+
static void
mlxsw_sp1_from_ptys_speed_duplex(struct mlxsw_sp *mlxsw_sp, bool carrier_ok,
u32 ptys_eth_proto,
struct ethtool_link_ksettings *cmd)
{
- u32 speed = SPEED_UNKNOWN;
- u8 duplex = DUPLEX_UNKNOWN;
- int i;
+ cmd->base.speed = SPEED_UNKNOWN;
+ cmd->base.duplex = DUPLEX_UNKNOWN;
if (!carrier_ok)
- goto out;
+ return;
- for (i = 0; i < MLXSW_SP1_PORT_LINK_MODE_LEN; i++) {
- if (ptys_eth_proto & mlxsw_sp1_port_link_mode[i].mask) {
- speed = mlxsw_sp1_port_link_mode[i].speed;
- duplex = DUPLEX_FULL;
- break;
- }
- }
-out:
- cmd->base.speed = speed;
- cmd->base.duplex = duplex;
+ cmd->base.speed = mlxsw_sp1_from_ptys_speed(mlxsw_sp, ptys_eth_proto);
+ if (cmd->base.speed != SPEED_UNKNOWN)
+ cmd->base.duplex = DUPLEX_FULL;
}
static u32
@@ -2747,6 +2752,7 @@ static const struct mlxsw_sp_port_type_speed_ops
mlxsw_sp1_port_type_speed_ops = {
.from_ptys_supported_port = mlxsw_sp1_from_ptys_supported_port,
.from_ptys_link = mlxsw_sp1_from_ptys_link,
+ .from_ptys_speed = mlxsw_sp1_from_ptys_speed,
.from_ptys_speed_duplex = mlxsw_sp1_from_ptys_speed_duplex,
.to_ptys_advert_link = mlxsw_sp1_to_ptys_advert_link,
.to_ptys_speed = mlxsw_sp1_to_ptys_speed,
@@ -2997,28 +3003,33 @@ mlxsw_sp2_from_ptys_link(struct mlxsw_sp *mlxsw_sp, u32 ptys_eth_proto,
}
}
+static u32
+mlxsw_sp2_from_ptys_speed(struct mlxsw_sp *mlxsw_sp, u32 ptys_eth_proto)
+{
+ int i;
+
+ for (i = 0; i < MLXSW_SP2_PORT_LINK_MODE_LEN; i++) {
+ if (ptys_eth_proto & mlxsw_sp2_port_link_mode[i].mask)
+ return mlxsw_sp2_port_link_mode[i].speed;
+ }
+
+ return SPEED_UNKNOWN;
+}
+
static void
mlxsw_sp2_from_ptys_speed_duplex(struct mlxsw_sp *mlxsw_sp, bool carrier_ok,
u32 ptys_eth_proto,
struct ethtool_link_ksettings *cmd)
{
- u32 speed = SPEED_UNKNOWN;
- u8 duplex = DUPLEX_UNKNOWN;
- int i;
+ cmd->base.speed = SPEED_UNKNOWN;
+ cmd->base.duplex = DUPLEX_UNKNOWN;
if (!carrier_ok)
- goto out;
+ return;
- for (i = 0; i < MLXSW_SP2_PORT_LINK_MODE_LEN; i++) {
- if (ptys_eth_proto & mlxsw_sp2_port_link_mode[i].mask) {
- speed = mlxsw_sp2_port_link_mode[i].speed;
- duplex = DUPLEX_FULL;
- break;
- }
- }
-out:
- cmd->base.speed = speed;
- cmd->base.duplex = duplex;
+ cmd->base.speed = mlxsw_sp2_from_ptys_speed(mlxsw_sp, ptys_eth_proto);
+ if (cmd->base.speed != SPEED_UNKNOWN)
+ cmd->base.duplex = DUPLEX_FULL;
}
static bool
@@ -3129,6 +3140,7 @@ static const struct mlxsw_sp_port_type_speed_ops
mlxsw_sp2_port_type_speed_ops = {
.from_ptys_supported_port = mlxsw_sp2_from_ptys_supported_port,
.from_ptys_link = mlxsw_sp2_from_ptys_link,
+ .from_ptys_speed = mlxsw_sp2_from_ptys_speed,
.from_ptys_speed_duplex = mlxsw_sp2_from_ptys_speed_duplex,
.to_ptys_advert_link = mlxsw_sp2_to_ptys_advert_link,
.to_ptys_speed = mlxsw_sp2_to_ptys_speed,
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
index 7f8427c1a997..c21cd1a425c7 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
@@ -279,6 +279,7 @@ struct mlxsw_sp_port_type_speed_ops {
struct ethtool_link_ksettings *cmd);
void (*from_ptys_link)(struct mlxsw_sp *mlxsw_sp, u32 ptys_eth_proto,
unsigned long *mode);
+ u32 (*from_ptys_speed)(struct mlxsw_sp *mlxsw_sp, u32 ptys_eth_proto);
void (*from_ptys_speed_duplex)(struct mlxsw_sp *mlxsw_sp,
bool carrier_ok, u32 ptys_eth_proto,
struct ethtool_link_ksettings *cmd);
--
2.20.1
^ permalink raw reply related
* [PATCH net-next 7/8] mlxsw: spectrum: Set up PTP shaper when port status has changed
From: Ido Schimmel @ 2019-07-04 7:07 UTC (permalink / raw)
To: netdev; +Cc: davem, richardcochran, jiri, petrm, shalomt, mlxsw, Ido Schimmel
In-Reply-To: <20190704070740.302-1-idosch@idosch.org>
From: Shalom Toledo <shalomt@mellanox.com>
When getting port up down event (PUDE), change the PTP shaper
configuration based on hardware time stamping on/off and the port's
speed.
Signed-off-by: Shalom Toledo <shalomt@mellanox.com>
Reviewed-by: Petr Machata <petrm@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 8 ++++++++
drivers/net/ethernet/mellanox/mlxsw/spectrum.h | 1 +
.../net/ethernet/mellanox/mlxsw/spectrum_ptp.c | 17 +++++++++++++++++
.../net/ethernet/mellanox/mlxsw/spectrum_ptp.h | 10 ++++++++++
4 files changed, 36 insertions(+)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 7cfca2be09fc..ce285fbeebd3 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -171,6 +171,7 @@ struct mlxsw_sp_ptp_ops {
struct hwtstamp_config *config);
int (*hwtstamp_set)(struct mlxsw_sp_port *mlxsw_sp_port,
struct hwtstamp_config *config);
+ void (*shaper_work)(struct work_struct *work);
int (*get_ts_info)(struct mlxsw_sp *mlxsw_sp,
struct ethtool_ts_info *info);
};
@@ -3716,6 +3717,9 @@ static int mlxsw_sp_port_create(struct mlxsw_sp *mlxsw_sp, u8 local_port,
}
mlxsw_sp_port->default_vlan = mlxsw_sp_port_vlan;
+ INIT_DELAYED_WORK(&mlxsw_sp_port->ptp.shaper_dw,
+ mlxsw_sp->ptp_ops->shaper_work);
+
mlxsw_sp->ports[local_port] = mlxsw_sp_port;
err = register_netdev(dev);
if (err) {
@@ -3770,6 +3774,7 @@ static void mlxsw_sp_port_remove(struct mlxsw_sp *mlxsw_sp, u8 local_port)
struct mlxsw_sp_port *mlxsw_sp_port = mlxsw_sp->ports[local_port];
cancel_delayed_work_sync(&mlxsw_sp_port->periodic_hw_stats.update_dw);
+ cancel_delayed_work_sync(&mlxsw_sp_port->ptp.shaper_dw);
mlxsw_sp_port_ptp_clear(mlxsw_sp_port);
mlxsw_core_port_clear(mlxsw_sp->core, local_port, mlxsw_sp);
unregister_netdev(mlxsw_sp_port->dev); /* This calls ndo_stop */
@@ -4055,6 +4060,7 @@ static void mlxsw_sp_pude_event_func(const struct mlxsw_reg_info *reg,
if (status == MLXSW_PORT_OPER_STATUS_UP) {
netdev_info(mlxsw_sp_port->dev, "link up\n");
netif_carrier_on(mlxsw_sp_port->dev);
+ mlxsw_core_schedule_dw(&mlxsw_sp_port->ptp.shaper_dw, 0);
} else {
netdev_info(mlxsw_sp_port->dev, "link down\n");
netif_carrier_off(mlxsw_sp_port->dev);
@@ -4572,6 +4578,7 @@ static const struct mlxsw_sp_ptp_ops mlxsw_sp1_ptp_ops = {
.transmitted = mlxsw_sp1_ptp_transmitted,
.hwtstamp_get = mlxsw_sp1_ptp_hwtstamp_get,
.hwtstamp_set = mlxsw_sp1_ptp_hwtstamp_set,
+ .shaper_work = mlxsw_sp1_ptp_shaper_work,
.get_ts_info = mlxsw_sp1_ptp_get_ts_info,
};
@@ -4584,6 +4591,7 @@ static const struct mlxsw_sp_ptp_ops mlxsw_sp2_ptp_ops = {
.transmitted = mlxsw_sp2_ptp_transmitted,
.hwtstamp_get = mlxsw_sp2_ptp_hwtstamp_get,
.hwtstamp_set = mlxsw_sp2_ptp_hwtstamp_set,
+ .shaper_work = mlxsw_sp2_ptp_shaper_work,
.get_ts_info = mlxsw_sp2_ptp_get_ts_info,
};
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
index c21cd1a425c7..abbb563db440 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
@@ -267,6 +267,7 @@ struct mlxsw_sp_port {
struct mlxsw_sp_acl_block *ing_acl_block;
struct mlxsw_sp_acl_block *eg_acl_block;
struct {
+ struct delayed_work shaper_dw;
struct hwtstamp_config hwtstamp_config;
u16 ing_types;
u16 egr_types;
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c
index 49aba0ce896b..0f7c4bd22a45 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c
@@ -1000,6 +1000,23 @@ static int mlxsw_sp1_ptp_port_shaper_check(struct mlxsw_sp_port *mlxsw_sp_port)
return mlxsw_sp1_ptp_port_shaper_set(mlxsw_sp_port, ptps);
}
+void mlxsw_sp1_ptp_shaper_work(struct work_struct *work)
+{
+ struct delayed_work *dwork = to_delayed_work(work);
+ struct mlxsw_sp_port *mlxsw_sp_port;
+ int err;
+
+ mlxsw_sp_port = container_of(dwork, struct mlxsw_sp_port,
+ ptp.shaper_dw);
+
+ if (!mlxsw_sp1_ptp_hwtstamp_enabled(mlxsw_sp_port))
+ return;
+
+ err = mlxsw_sp1_ptp_port_shaper_check(mlxsw_sp_port);
+ if (err)
+ netdev_err(mlxsw_sp_port->dev, "Failed to set up PTP shaper\n");
+}
+
int mlxsw_sp1_ptp_hwtstamp_set(struct mlxsw_sp_port *mlxsw_sp_port,
struct hwtstamp_config *config)
{
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.h
index b23abfc0bd76..72e55f6926b9 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.h
@@ -54,6 +54,8 @@ int mlxsw_sp1_ptp_hwtstamp_get(struct mlxsw_sp_port *mlxsw_sp_port,
int mlxsw_sp1_ptp_hwtstamp_set(struct mlxsw_sp_port *mlxsw_sp_port,
struct hwtstamp_config *config);
+void mlxsw_sp1_ptp_shaper_work(struct work_struct *work);
+
int mlxsw_sp1_ptp_get_ts_info(struct mlxsw_sp *mlxsw_sp,
struct ethtool_ts_info *info);
@@ -113,6 +115,10 @@ mlxsw_sp1_ptp_hwtstamp_set(struct mlxsw_sp_port *mlxsw_sp_port,
return -EOPNOTSUPP;
}
+static inline void mlxsw_sp1_ptp_shaper_work(struct work_struct *work)
+{
+}
+
static inline int mlxsw_sp1_ptp_get_ts_info(struct mlxsw_sp *mlxsw_sp,
struct ethtool_ts_info *info)
{
@@ -167,6 +173,10 @@ mlxsw_sp2_ptp_hwtstamp_set(struct mlxsw_sp_port *mlxsw_sp_port,
return -EOPNOTSUPP;
}
+static inline void mlxsw_sp2_ptp_shaper_work(struct work_struct *work)
+{
+}
+
static inline int mlxsw_sp2_ptp_get_ts_info(struct mlxsw_sp *mlxsw_sp,
struct ethtool_ts_info *info)
{
--
2.20.1
^ permalink raw reply related
* [PATCH net-next 8/8] mlxsw: spectrum_ptp: Apply the PTP shaper enable/disable logic
From: Ido Schimmel @ 2019-07-04 7:07 UTC (permalink / raw)
To: netdev; +Cc: davem, richardcochran, jiri, petrm, shalomt, mlxsw, Ido Schimmel
In-Reply-To: <20190704070740.302-1-idosch@idosch.org>
From: Shalom Toledo <shalomt@mellanox.com>
Apply by filling the PTP shaper parameters array.
Signed-off-by: Shalom Toledo <shalomt@mellanox.com>
Reviewed-by: Petr Machata <petrm@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
.../ethernet/mellanox/mlxsw/spectrum_ptp.c | 44 +++++++++++++++++++
1 file changed, 44 insertions(+)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c
index 0f7c4bd22a45..bd9c2bc2d5d6 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c
@@ -767,6 +767,50 @@ struct mlxsw_sp1_ptp_shaper_params {
static const struct mlxsw_sp1_ptp_shaper_params
mlxsw_sp1_ptp_shaper_params[] = {
+ {
+ .ethtool_speed = SPEED_100,
+ .port_speed = MLXSW_REG_QPSC_PORT_SPEED_100M,
+ .shaper_time_exp = 4,
+ .shaper_time_mantissa = 12,
+ .shaper_inc = 9,
+ .shaper_bs = 1,
+ .port_to_shaper_credits = 1,
+ .ing_timestamp_inc = -313,
+ .egr_timestamp_inc = 313,
+ },
+ {
+ .ethtool_speed = SPEED_1000,
+ .port_speed = MLXSW_REG_QPSC_PORT_SPEED_1G,
+ .shaper_time_exp = 0,
+ .shaper_time_mantissa = 12,
+ .shaper_inc = 6,
+ .shaper_bs = 0,
+ .port_to_shaper_credits = 1,
+ .ing_timestamp_inc = -35,
+ .egr_timestamp_inc = 35,
+ },
+ {
+ .ethtool_speed = SPEED_10000,
+ .port_speed = MLXSW_REG_QPSC_PORT_SPEED_10G,
+ .shaper_time_exp = 0,
+ .shaper_time_mantissa = 2,
+ .shaper_inc = 14,
+ .shaper_bs = 1,
+ .port_to_shaper_credits = 1,
+ .ing_timestamp_inc = -11,
+ .egr_timestamp_inc = 11,
+ },
+ {
+ .ethtool_speed = SPEED_25000,
+ .port_speed = MLXSW_REG_QPSC_PORT_SPEED_25G,
+ .shaper_time_exp = 0,
+ .shaper_time_mantissa = 0,
+ .shaper_inc = 11,
+ .shaper_bs = 1,
+ .port_to_shaper_credits = 1,
+ .ing_timestamp_inc = -14,
+ .egr_timestamp_inc = 14,
+ },
};
#define MLXSW_SP1_PTP_SHAPER_PARAMS_LEN ARRAY_SIZE(mlxsw_sp1_ptp_shaper_params)
--
2.20.1
^ permalink raw reply related
* Re: [PATCH v2 3/4] net: dsa: vsc73xx: add support for parallel mode
From: Linus Walleij @ 2019-07-04 7:16 UTC (permalink / raw)
To: Pawel Dembicki
Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, David S. Miller,
Rob Herring, Mark Rutland, netdev,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
linux-kernel@vger.kernel.org
In-Reply-To: <20190703171924.31801-4-paweldembicki@gmail.com>
Hi Pawel,
this looks overall good and the following is just
documentation nit-pick so feel free to add my
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
On Wed, Jul 3, 2019 at 7:21 PM Pawel Dembicki <paweldembicki@gmail.com> wrote:
> This patch add platform part of vsc73xx driver.
> It allows to use chip connected by parallel interface.
>
> Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>
I would elaborate on the fact that we're dealing with memory-mapped
I/O but perhaps even more important to do that in the source code file and
Kconfig so it is close to the users rather than in the changelog.
Write something about the address bus wireing making all
accesses big endian.
> +config NET_DSA_VITESSE_VSC73XX_PLATFORM
> + tristate "Vitesse VSC7385/7388/7395/7398 Platform mode support"
> + depends on HAS_IOMEM
> + select NET_DSA_VITESSE_VSC73XX
> + ---help---
> + This enables support for the Vitesse VSC7385, VSC7388, VSC7395
> + and VSC7398 SparX integrated ethernet switches in Platform managed mode.
I would insert something about memory-mapped I/O mode
over a CPU-attached address bus.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH v2 4/4] net: dsa: vsc73xx: Assert reset if iCPU is enabled
From: Linus Walleij @ 2019-07-04 7:22 UTC (permalink / raw)
To: Pawel Dembicki
Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, David S. Miller,
Rob Herring, Mark Rutland, netdev,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
linux-kernel@vger.kernel.org
In-Reply-To: <20190703171924.31801-5-paweldembicki@gmail.com>
On Wed, Jul 3, 2019 at 7:21 PM Pawel Dembicki <paweldembicki@gmail.com> wrote:
> Driver allow to use devices with disabled iCPU only.
>
> Some devices have pre-initialised iCPU by bootloader.
> That state make switch unmanaged. This patch force reset
> if device is in unmanaged state. In the result chip lost
> internal firmware from RAM and it can be managed.
>
> Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
My devices do not have direct access to the reset line so I
can't assert reset no matter how I try, if it works for you, the
code is certainly better like this.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH v2] let proc net directory inodes reflect to active net namespace
From: Alexey Dobriyan @ 2019-07-04 7:32 UTC (permalink / raw)
To: Hallsmark, Per
Cc: David S. Miller, linux-kernel@vger.kernel.org,
netdev@vger.kernel.org
In-Reply-To: <B7B4BB465792624BAF51F33077E99065DC5D8E5D@ALA-MBD.corp.ad.wrs.com>
On Mon, Jul 01, 2019 at 11:06:34AM +0000, Hallsmark, Per wrote:
> +struct proc_dir_entry *proc_net_mkdir(struct net *net, const char *name,
> + struct proc_dir_entry *parent)
> +{
> + struct proc_dir_entry *pde;
> +
> + pde = proc_mkdir_data(name, 0, parent, net);
> + if (!pde)
> + return NULL;
> + pde->proc_dops = &proc_net_dentry_ops;
OK, this is buggy in a different way:
once proc_mkdir_data() returns, proc entry is live and should be fully
ready, so dentry operations should be glued before that.
I'll send proper patch.
^ permalink raw reply
* Re: [rdma 14/16] RDMA/irdma: Add ABI definitions
From: Leon Romanovsky @ 2019-07-04 7:40 UTC (permalink / raw)
To: Jeff Kirsher
Cc: dledford, jgg, davem, Mustafa Ismail, linux-rdma, netdev, nhorman,
sassmann, poswald, david.m.ertman, Shiraz Saleem
In-Reply-To: <20190704021259.15489-16-jeffrey.t.kirsher@intel.com>
On Wed, Jul 03, 2019 at 07:12:57PM -0700, Jeff Kirsher wrote:
> From: Mustafa Ismail <mustafa.ismail@intel.com>
>
> Add ABI definitions for irdma.
>
> Signed-off-by: Mustafa Ismail <mustafa.ismail@intel.com>
> Signed-off-by: Shiraz Saleem <shiraz.saleem@intel.com>
> ---
> include/uapi/rdma/irdma-abi.h | 130 ++++++++++++++++++++++++++++++++++
> 1 file changed, 130 insertions(+)
> create mode 100644 include/uapi/rdma/irdma-abi.h
>
> diff --git a/include/uapi/rdma/irdma-abi.h b/include/uapi/rdma/irdma-abi.h
> new file mode 100644
> index 000000000000..bdfbda4c829e
> --- /dev/null
> +++ b/include/uapi/rdma/irdma-abi.h
> @@ -0,0 +1,130 @@
> +/* SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause */
> +/* Copyright (c) 2006 - 2019 Intel Corporation. All rights reserved.
> + * Copyright (c) 2005 Topspin Communications. All rights reserved.
> + * Copyright (c) 2005 Cisco Systems. All rights reserved.
> + * Copyright (c) 2005 Open Grid Computing, Inc. All rights reserved.
> + */
> +
> +#ifndef IRDMA_ABI_H
> +#define IRDMA_ABI_H
> +
> +#include <linux/types.h>
> +
> +/* irdma must support legacy GEN_1 i40iw kernel
> + * and user-space whose last ABI ver is 5
> + */
> +#define IRDMA_ABI_VER 6
Can you please elaborate about it more?
There is no irdma code in RDMA yet, so it makes me wonder why new define
shouldn't start from 1.
Thanks
^ permalink raw reply
* Re: [PATCH net-next 1/3] devlink: Introduce PCI PF port flavour and port attribute
From: Jiri Pirko @ 2019-07-04 7:44 UTC (permalink / raw)
To: Parav Pandit
Cc: Andrew Lunn, Jakub Kicinski, Jiri Pirko, netdev@vger.kernel.org,
Saeed Mahameed, vivien.didelot@gmail.com, f.fainelli@gmail.com
In-Reply-To: <AM0PR05MB48665F6CA614A3770D6ABCF4D1FB0@AM0PR05MB4866.eurprd05.prod.outlook.com>
Wed, Jul 03, 2019 at 06:13:17PM CEST, parav@mellanox.com wrote:
>
>
>> -----Original Message-----
>> From: Jiri Pirko <jiri@resnulli.us>
>> Sent: Wednesday, July 3, 2019 8:05 PM
>> To: Andrew Lunn <andrew@lunn.ch>
>> Cc: Parav Pandit <parav@mellanox.com>; Jakub Kicinski
>> <jakub.kicinski@netronome.com>; Jiri Pirko <jiri@mellanox.com>;
>> netdev@vger.kernel.org; Saeed Mahameed <saeedm@mellanox.com>;
>> vivien.didelot@gmail.com; f.fainelli@gmail.com
>> Subject: Re: [PATCH net-next 1/3] devlink: Introduce PCI PF port flavour and
>> port attribute
>>
>> Wed, Jul 03, 2019 at 04:09:58PM CEST, andrew@lunn.ch wrote:
>> >> However, we expose it for DEVLINK_PORT_FLAVOUR_CPU and
>> >> DEVLINK_PORT_FLAVOUR_DSA. Not sure if it makes sense there either.
>> >> Ccing Florian, Andrew and Vivien.
>> >> What do you guys think?
>> >
>> >Hi Jiri
>> >
>> >DSA and CPU ports are physical ports of the switch. And there can be
>> >multiple DSA ports, and maybe sometime real soon now, multiple CPU
>> >ports. So having a number associated with them is useful.
>>
>> Okay. Makes sense.
>>
>Ok. I should probably update the comment section for port_number as its scope is expanded.
>Should I revise the series with updated comment?
Please do. Also put a check to not fillup port_number attribute in case
of pf/vf flavour. Thanks!
>
>> >
>> > Andrew
^ permalink raw reply
* Re: [PATCH v2] rt2x00: no need to check return value of debugfs_create functions
From: Stanislaw Gruszka @ 2019-07-04 7:50 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Helmut Schaa, Kalle Valo, David S. Miller, linux-wireless, netdev
In-Reply-To: <20190703113956.GA26652@kroah.com>
On Wed, Jul 03, 2019 at 01:39:56PM +0200, Greg Kroah-Hartman wrote:
>
> When calling debugfs functions, there is no need to ever check the
> return value. The function can work or not, but the code logic should
> never do something different based on this.
>
> Because we don't need to save the individual debugfs files and
> directories, remove the local storage of them and just remove the entire
> debugfs directory in a single call, making things a lot simpler.
>
> Cc: Stanislaw Gruszka <sgruszka@redhat.com>
> Cc: Helmut Schaa <helmut.schaa@googlemail.com>
> Cc: Kalle Valo <kvalo@codeaurora.org>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: linux-wireless@vger.kernel.org
> Cc: netdev@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Acked-by: Stanislaw Gruszka <sgruszka@redhat.com>
^ permalink raw reply
* Re: [for-next V2 10/10] RDMA/core: Provide RDMA DIM support for ULPs
From: Leon Romanovsky @ 2019-07-04 7:51 UTC (permalink / raw)
To: Sagi Grimberg
Cc: Idan Burstein, Saeed Mahameed, David S. Miller, Doug Ledford,
Jason Gunthorpe, Or Gerlitz, Tal Gilboa, netdev@vger.kernel.org,
linux-rdma@vger.kernel.org, Yamin Friedman, Max Gurtovoy
In-Reply-To: <8d525d64-6da1-48c3-952d-8c6b0d541859@grimberg.me>
On Wed, Jul 03, 2019 at 11:56:04AM -0700, Sagi Grimberg wrote:
>
> > Hi Sagi,
> >
> > I'm not sharing your worries about bad out-of-the-box experience for a
> > number of reasons.
> >
> > First of all, this code is part of upstream kernel and will take time
> > till users actually start to use it as is and not as part of some distro
> > backports or MOFED packages.
>
> True, but I am still saying that this feature is damaging sync IO which
> represents the majority of the users. It might not be an extreme impact
> but it is still a degradation (from a very limited testing I did this
> morning I'm seeing a consistent 5%-10% latency increase for low QD
> workloads which is consistent with what Yamin reported AFAIR).
>
> But having said that, the call is for you guys to make as this is a
> Mellanox device. I absolutely think that this is useful (as I said
> before), I just don't think its necessarily a good idea to opt it by
> default given that only a limited set of users would take full advantage
> of it while the rest would see a negative impact (even if its 10%).
>
> I don't have a hard objection here, just wanted to give you my
> opinion on this because mlx5 is an important driver for rdma
> users.
Your opinion is very valuable for us and we started internal thread to
challenge this "enable by default", it just takes time and I prefer to
enable this code to get test coverage as wide as possible.
>
> > Second, Yamin did extensive testing and worked very close with Or G.
> > and I have very high confident in the results of their team work.
>
> Has anyone tested other RDMA ulps? NFS/RDMA or SRP/iSER?
>
> Would be interesting to understand how other subsystems with different
> characteristics behave with this.
Me too, and I'll revert this default if needed.
Thanks
^ permalink raw reply
* Re: [PATCH][next] wil6210: fix wil_cid_valid with negative cid values
From: merez @ 2019-07-04 7:53 UTC (permalink / raw)
To: Colin King
Cc: Kalle Valo, David S . Miller, linux-wireless, wil6210, netdev,
kernel-janitors, linux-kernel
In-Reply-To: <20190702144026.13013-1-colin.king@canonical.com>
On 2019-07-02 17:40, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> There are several occasions where a negative cid value is passed
> into wil_cid_valid and this is converted into a u8 causing the
> range check of cid >= 0 to always succeed. Fix this by making
> the cid argument an int to handle any -ve error value of cid.
>
> An example of this behaviour is in wil_cfg80211_dump_station,
> where cid is assigned -ENOENT if the call to wil_find_cid_by_idx
> fails, and this -ve value is passed to wil_cid_valid. I believe
> that the conversion of -ENOENT to the u8 value 254 which is
> greater than wil->max_assoc_sta causes wil_find_cid_by_idx to
> currently work fine, but I think is by luck and not the
> intended behaviour.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> drivers/net/wireless/ath/wil6210/wil6210.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ath/wil6210/wil6210.h
> b/drivers/net/wireless/ath/wil6210/wil6210.h
> index 6f456b311a39..25a1adcb38eb 100644
> --- a/drivers/net/wireless/ath/wil6210/wil6210.h
> +++ b/drivers/net/wireless/ath/wil6210/wil6210.h
> @@ -1144,7 +1144,7 @@ static inline void wil_c(struct wil6210_priv
> *wil, u32 reg, u32 val)
> /**
> * wil_cid_valid - check cid is valid
> */
> -static inline bool wil_cid_valid(struct wil6210_priv *wil, u8 cid)
> +static inline bool wil_cid_valid(struct wil6210_priv *wil, int cid)
> {
> return (cid >= 0 && cid < wil->max_assoc_sta);
> }
Reviewed-by: Maya Erez <merez@codeaurora.org>
--
Maya Erez
Qualcomm Israel, Inc. on behalf of Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a
Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH net-next v6 06/15] ethtool: netlink bitset handling
From: Jiri Pirko @ 2019-07-04 8:04 UTC (permalink / raw)
To: Michal Kubecek
Cc: netdev, David Miller, Jakub Kicinski, Andrew Lunn,
Florian Fainelli, John Linville, Stephen Hemminger, Johannes Berg,
linux-kernel
In-Reply-To: <20190703181851.GP20101@unicorn.suse.cz>
Wed, Jul 03, 2019 at 08:18:51PM CEST, mkubecek@suse.cz wrote:
>On Wed, Jul 03, 2019 at 01:49:33PM +0200, Jiri Pirko wrote:
>> Tue, Jul 02, 2019 at 01:50:09PM CEST, mkubecek@suse.cz wrote:
>> >diff --git a/Documentation/networking/ethtool-netlink.txt b/Documentation/networking/ethtool-netlink.txt
>> >index 97c369aa290b..4636682c551f 100644
>> >--- a/Documentation/networking/ethtool-netlink.txt
>> >+++ b/Documentation/networking/ethtool-netlink.txt
>> >@@ -73,6 +73,67 @@ set, the behaviour is the same as (or closer to) the behaviour before it was
>> > introduced.
>> >
>> >
>> >+Bit sets
>> >+--------
>> >+
>> >+For short bitmaps of (reasonably) fixed length, standard NLA_BITFIELD32 type
>> >+is used. For arbitrary length bitmaps, ethtool netlink uses a nested attribute
>> >+with contents of one of two forms: compact (two binary bitmaps representing
>> >+bit values and mask of affected bits) and bit-by-bit (list of bits identified
>> >+by either index or name).
>> >+
>> >+Compact form: nested (bitset) atrribute contents:
>> >+
>> >+ ETHTOOL_A_BITSET_LIST (flag) no mask, only a list
>> >+ ETHTOOL_A_BITSET_SIZE (u32) number of significant bits
>> >+ ETHTOOL_A_BITSET_VALUE (binary) bitmap of bit values
>> >+ ETHTOOL_A_BITSET_MASK (binary) bitmap of valid bits
>> >+
>> >+Value and mask must have length at least ETHTOOL_A_BITSET_SIZE bits rounded up
>> >+to a multiple of 32 bits. They consist of 32-bit words in host byte order,
>>
>> Looks like the blocks are similar to NLA_BITFIELD32. Why don't you user
>> nested array of NLA_BITFIELD32 instead?
>
>That would mean a layout like
>
> 4 bytes of attr header
> 4 bytes of value
> 4 bytes of mask
> 4 bytes of attr header
> 4 bytes of value
> 4 bytes of mask
> ...
>
>i.e. interleaved headers, words of value and words of mask. Having value
>and mask contiguous looks cleaner to me. Also, I can quickly check the
>sizes without iterating through a (potentially long) array.
Yeah, if you are not happy with this, I suggest to introduce
NLA_BITFIELD with arbitrary size. That would be probably cleanest.
>
>> >+words ordered from least significant to most significant (i.e. the same way as
>> >+bitmaps are passed with ioctl interface).
>> >+
>> >+For compact form, ETHTOOL_A_BITSET_SIZE and ETHTOOL_A_BITSET_VALUE are
>> >+mandatory. Similar to BITFIELD32, a compact form bit set requests to set bits
>>
>> Double space^^
>
>Hm, I have to learn how to tell vim not to do that with "gq".
>
>> >+in the mask to 1 (if the bit is set in value) or 0 (if not) and preserve the
>> >+rest. If ETHTOOL_A_BITSET_LIST is present, there is no mask and bitset
>> >+represents a simple list of bits.
>>
>> Okay, that is a bit confusing. Why not to rename to something like:
>> ETHTOOL_A_BITSET_NO_MASK (flag)
>> ?
>
>From the logical point of view, it's used for lists - list of link
>modes, list of netdev features, list of timestamping modes etc.
>
>The point is that in userspace requests, we sometimes want to change
>some values (enable A, disable B), sometimes to define the list of
>values to be set (I want (only) A, C and E to be enabled). In kernel
>replies, sometimes there is a natural value/mask pairing (e.g.
>advertised and supported link modes, enabled and supported WoL modes)
>but often there is just one bitmap.
>
>> >+Kernel bit set length may differ from userspace length if older application is
>> >+used on newer kernel or vice versa. If userspace bitmap is longer, an error is
>> >+issued only if the request actually tries to set values of some bits not
>> >+recognized by kernel.
>> >+
>> >+Bit-by-bit form: nested (bitset) attribute contents:
>> >+
>> >+ ETHTOOL_A_BITSET_LIST (flag) no mask, only a list
>> >+ ETHTOOL_A_BITSET_SIZE (u32) number of significant bits
>> >+ ETHTOOL_A_BITSET_BIT (nested) array of bits
>> >+ ETHTOOL_A_BITSET_BIT+ (nested) one bit
>> >+ ETHTOOL_A_BIT_INDEX (u32) bit index (0 for LSB)
>> >+ ETHTOOL_A_BIT_NAME (string) bit name
>> >+ ETHTOOL_A_BIT_VALUE (flag) present if bit is set
>> >+
>> >+Bit size is optional for bit-by-bit form. ETHTOOL_A_BITSET_BITS nest can only
>> >+contain ETHTOOL_A_BITS_BIT attributes but there can be an arbitrary number of
>> >+them. A bit may be identified by its index or by its name. When used in
>> >+requests, listed bits are set to 0 or 1 according to ETHTOOL_A_BIT_VALUE, the
>> >+rest is preserved. A request fails if index exceeds kernel bit length or if
>> >+name is not recognized.
>> >+
>> >+When ETHTOOL_A_BITSET_LIST flag is present, bitset is interpreted as a simple
>> >+bit list. ETHTOOL_A_BIT_VALUE attributes are not used in such case. Bit list
>> >+represents a bitmap with listed bits set and the rest zero.
>> >+
>> >+In requests, application can use either form. Form used by kernel in reply is
>> >+determined by a flag in flags field of request header. Semantics of value and
>> >+mask depends on the attribute. General idea is that flags control request
>> >+processing, info_mask control which parts of the information are returned in
>> >+"get" request and index identifies a particular subcommand or an object to
>> >+which the request applies.
>>
>> This is quite complex and confusing. Having the same API for 2 APIs is
>> odd. The API should be crystal clear, easy to use.
>>
>> Why can't you have 2 commands, one working with bit arrays only, one
>> working with strings? Something like:
>> X_GET
>> ETHTOOL_A_BITS (nested)
>> ETHTOOL_A_BIT_ARRAY (BITFIELD32)
>> X_NAMES_GET
>> ETHTOOL_A_BIT_NAMES (nested)
>> ETHTOOL_A_BIT_INDEX
>> ETHTOOL_A_BIT_NAME
>>
>> For set, you can also have multiple cmds:
>> X_SET - to set many at once, by bit index
>> ETHTOOL_A_BITS (nested)
>> ETHTOOL_A_BIT_ARRAY (BITFIELD32)
>> X_ONE_SET - to set one, by bit index
>> ETHTOOL_A_BIT_INDEX
>> ETHTOOL_A_BIT_VALUE
>> X_ONE_SET - to set one, by name
>> ETHTOOL_A_BIT_NAME
>> ETHTOOL_A_BIT_VALUE
>
>This looks as if you assume there is nothing except the bitset in the
>message but that is not true. Even with your proposed breaking of
>current groups, you would still have e.g. 4 bitsets in reply to netdev
>features query, 3 in timestamping info GET request and often bitsets
>combined with other data (e.g. WoL modes and optional WoL password).
>If you wanted to further refine the message granularity to the level of
>single parameters, we might be out of message type ids already.
You can still have multiple bitsets(bitfields) in single message and
have separate cmd/cmds to get string-bit mapping. No need to mangle it.
>
>Unless you want to forget about structured data completely and turn
>everything into tunables - but that's rather scary idea.
>
>Michal
^ 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