* Re: [PATCH net-next v2 5/8] net: phy: meson-gxl: detect LPA corruption
From: Andrew Lunn @ 2017-12-07 16:12 UTC (permalink / raw)
To: Jerome Brunet
Cc: Florian Fainelli, Kevin Hilman, netdev, linux-arm-kernel,
linux-amlogic, linux-kernel
In-Reply-To: <1512661332.7042.5.camel@baylibre.com>
> Would it be Ok if send patches 1 to 5 to net ?
> and 6 to 8 separately on net-next ?
No. The rules for stable is that a patch must really fix something and
be minimal.
Documentation/process/stable-kernel-rules.rst
What might be best is to develop a minimal, but ugly patch for stable.
Get it applied. Around a week later, net will be merged into
net-next. You can then have a 'revert' patch, followed by this series
making it nice and clean.
Andrew
^ permalink raw reply
* Re: [PATCH net-next v2 7/8] net: phy: meson-gxl: add interrupt support
From: Andrew Lunn @ 2017-12-07 16:07 UTC (permalink / raw)
To: Jerome Brunet
Cc: Florian Fainelli, Kevin Hilman, netdev, linux-arm-kernel,
linux-amlogic, linux-kernel
In-Reply-To: <1512662689.7042.20.camel@baylibre.com>
> The phy being internal, I think it is unlikely to ever share its interrupt
> though.
O.K, don't bother.
> Thanks for the lightning fast review by the way !
You are welcome.
Andrew
^ permalink raw reply
* Re: [PATCH net-next v2 7/8] net: phy: meson-gxl: add interrupt support
From: Jerome Brunet @ 2017-12-07 16:04 UTC (permalink / raw)
To: Andrew Lunn
Cc: Florian Fainelli, Kevin Hilman, netdev, linux-arm-kernel,
linux-amlogic, linux-kernel
In-Reply-To: <20171207155421.GJ24750@lunn.ch>
On Thu, 2017-12-07 at 16:54 +0100, Andrew Lunn wrote:
> On Thu, Dec 07, 2017 at 03:27:14PM +0100, Jerome Brunet wrote:
> > Enable interrupt support in meson-gxl PHY driver
>
> Hi Jerome
>
> Is it possible to implement did_interrupt()? That allows for shared
> interrupts. It does however work fine without it, so long as the
> interrupt is not shared.
Hi Andrew,
It is always possible ;). The irq status registers gets reset on read.
In such case, ack_interrupt() and did_interrupt() would be more or less the same
function, except for the returned value, I guess ?
The phy being internal, I think it is unlikely to ever share its interrupt
though. If you prefer I implement this callback, I can certainly re-spin with it
?
Thanks for the lightning fast review by the way !
>
> Thanks
> Andrew
^ permalink raw reply
* Re: [Intel-wired-lan] [next-queue 02/10] ixgbe: add ipsec register access routines
From: Alexander Duyck @ 2017-12-07 16:02 UTC (permalink / raw)
To: Shannon Nelson
Cc: intel-wired-lan, Jeff Kirsher, Steffen Klassert, Sowmini Varadhan,
Netdev
In-Reply-To: <c03223ae-feae-8835-0e74-4f7c11856381@oracle.com>
On Wed, Dec 6, 2017 at 9:43 PM, Shannon Nelson
<shannon.nelson@oracle.com> wrote:
> Thanks, Alex, for your detailed comments, I do appreciate the time and
> thought you put into them.
>
> Responses below...
>
> sln
>
> On 12/5/2017 8:56 AM, Alexander Duyck wrote:
>>
>> On Mon, Dec 4, 2017 at 9:35 PM, Shannon Nelson
>> <shannon.nelson@oracle.com> wrote:
>>>
>>> Add a few routines to make access to the ipsec registers just a little
>>> easier, and throw in the beginnings of an initialization.
>>>
>>> Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
>>> ---
>>> drivers/net/ethernet/intel/ixgbe/Makefile | 1 +
>>> drivers/net/ethernet/intel/ixgbe/ixgbe.h | 6 +
>>> drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c | 157
>>> +++++++++++++++++++++++++
>>> drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.h | 50 ++++++++
>>> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 1 +
>>> 5 files changed, 215 insertions(+)
>>> create mode 100644 drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
>>> create mode 100644 drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.h
>>>
>>> diff --git a/drivers/net/ethernet/intel/ixgbe/Makefile
>>> b/drivers/net/ethernet/intel/ixgbe/Makefile
>>> index 35e6fa6..8319465 100644
>>> --- a/drivers/net/ethernet/intel/ixgbe/Makefile
>>> +++ b/drivers/net/ethernet/intel/ixgbe/Makefile
>>> @@ -42,3 +42,4 @@ ixgbe-$(CONFIG_IXGBE_DCB) += ixgbe_dcb.o
>>> ixgbe_dcb_82598.o \
>>> ixgbe-$(CONFIG_IXGBE_HWMON) += ixgbe_sysfs.o
>>> ixgbe-$(CONFIG_DEBUG_FS) += ixgbe_debugfs.o
>>> ixgbe-$(CONFIG_FCOE:m=y) += ixgbe_fcoe.o
>>> +ixgbe-$(CONFIG_XFRM_OFFLOAD) += ixgbe_ipsec.o
>>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
>>> b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
>>> index dd55787..1e11462 100644
>>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
>>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
>>> @@ -52,6 +52,7 @@
>>> #ifdef CONFIG_IXGBE_DCA
>>> #include <linux/dca.h>
>>> #endif
>>> +#include "ixgbe_ipsec.h"
>>>
>>> #include <net/busy_poll.h>
>>>
>>> @@ -1001,4 +1002,9 @@ void ixgbe_store_key(struct ixgbe_adapter
>>> *adapter);
>>> void ixgbe_store_reta(struct ixgbe_adapter *adapter);
>>> s32 ixgbe_negotiate_fc(struct ixgbe_hw *hw, u32 adv_reg, u32 lp_reg,
>>> u32 adv_sym, u32 adv_asm, u32 lp_sym, u32
>>> lp_asm);
>>> +#ifdef CONFIG_XFRM_OFFLOAD
>>> +void ixgbe_init_ipsec_offload(struct ixgbe_adapter *adapter);
>>> +#else
>>> +static inline void ixgbe_init_ipsec_offload(struct ixgbe_adapter
>>> *adapter) { };
>>> +#endif /* CONFIG_XFRM_OFFLOAD */
>>> #endif /* _IXGBE_H_ */
>>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
>>> b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
>>> new file mode 100644
>>> index 0000000..14dd011
>>> --- /dev/null
>>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
>>> @@ -0,0 +1,157 @@
>>>
>>> +/*******************************************************************************
>>> + *
>>> + * Intel 10 Gigabit PCI Express Linux driver
>>> + * Copyright(c) 2017 Oracle and/or its affiliates. All rights reserved.
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> it
>>> + * under the terms and conditions of the GNU General Public License,
>>> + * version 2, as published by the Free Software Foundation.
>>> + *
>>> + * This program is distributed in the hope it will be useful, but
>>> WITHOUT
>>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>>> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
>>> for
>>> + * more details.
>>> + *
>>> + * You should have received a copy of the GNU General Public License
>>> along with
>>> + * this program. If not, see <http://www.gnu.org/licenses/>.
>>> + *
>>> + * The full GNU General Public License is included in this distribution
>>> in
>>> + * the file called "COPYING".
>>> + *
>>> + * Contact Information:
>>> + * Linux NICS <linux.nics@intel.com>
>>> + * e1000-devel Mailing List <e1000-devel@lists.sourceforge.net>
>>> + * Intel Corporation, 5200 N.E. Elam Young Parkway, Hillsboro, OR
>>> 97124-6497
>>> + *
>>> +
>>> ******************************************************************************/
>>> +
>>> +#include "ixgbe.h"
>>> +
>>> +/**
>>> + * ixgbe_ipsec_set_tx_sa - set the Tx SA registers
>>> + * @hw: hw specific details
>>> + * @idx: register index to write
>>> + * @key: key byte array
>>> + * @salt: salt bytes
>>> + **/
>>> +static void ixgbe_ipsec_set_tx_sa(struct ixgbe_hw *hw, u16 idx,
>>> + u32 key[], u32 salt)
>>> +{
>>> + u32 reg;
>>> + int i;
>>> +
>>> + for (i = 0; i < 4; i++)
>>> + IXGBE_WRITE_REG(hw, IXGBE_IPSTXKEY(i),
>>> cpu_to_be32(key[3-i]));
>>> + IXGBE_WRITE_REG(hw, IXGBE_IPSTXSALT, cpu_to_be32(salt));
>>> + IXGBE_WRITE_FLUSH(hw);
>>> +
>>> + reg = IXGBE_READ_REG(hw, IXGBE_IPSTXIDX);
>>> + reg &= IXGBE_RXTXIDX_IPS_EN;
>>> + reg |= idx << 3 | IXGBE_RXTXIDX_IDX_WRITE;
>>> + IXGBE_WRITE_REG(hw, IXGBE_IPSTXIDX, reg);
>>> + IXGBE_WRITE_FLUSH(hw);
>>> +}
>>> +
>>
>>
>> So there are a few things here to unpack.
>>
>> The first is the carry-forward of the IPS bit. I'm not sure that is
>> the best way to go. Do we really expect to be updating SA values if
>> IPsec offload is not enabled?
>
>
> In order to save on energy, we don't enable the engine until we have the
> first SA successfully stored in the tables, so the enable bit will be off
> for that one.
>
> Also, the datasheet specifically says for the Rx table "Software should not
> make changes in the Rx SA tables while changing the IPSEC_EN bit." I figured
> I'd use the same method on both tables for consistency.
>
>> If so we may just want to carry a bit
>> flag somewhere in the ixgbe_hw struct indicating if Tx IPsec offload
>> is enabled and use that to determine the value for this bit.
>>
>> Also we should probably replace "3" with a value indicating that it is
>> the SA index shift.
>
>
> Sure, that would be good.
>
>>
>> Also technically the WRITE_FLUSH isn't needed if you are doing a PCIe
>> read anyway to get IPSTXIDX.
>
>
> That's from having to be very fastidious about these reads/writes/flushes
> before the engine actually worked for me. I could spend time taking them
> out and testing each change again, but they aren't in a fast path, so I'm
> really not worried about it.
>
>
>>
>>> +/**
>>> + * ixgbe_ipsec_set_rx_item - set an Rx table item
>>> + * @hw: hw specific details
>>> + * @idx: register index to write
>>> + * @tbl: table selector
>>> + *
>>> + * Trigger the device to store into a particular Rx table the
>>> + * data that has already been loaded into the input register
>>> + **/
>>> +static void ixgbe_ipsec_set_rx_item(struct ixgbe_hw *hw, u16 idx, u32
>>> tbl)
>>> +{
>>> + u32 reg;
>>> +
>>> + reg = IXGBE_READ_REG(hw, IXGBE_IPSRXIDX);
>>> + reg &= IXGBE_RXTXIDX_IPS_EN;
>>> + reg |= tbl | idx << 3 | IXGBE_RXTXIDX_IDX_WRITE;
>>> + IXGBE_WRITE_REG(hw, IXGBE_IPSRXIDX, reg);
>>> + IXGBE_WRITE_FLUSH(hw);
>>> +}
>>> +
>>
>>
>> The Rx version of this gets a bit trickier since the datasheet
>> actually indicates there are a few different types of tables that can
>> be indexed via this. Also why is the tbl value not being shifted? It
>> seems like it should be shifted by 1 to avoid overwriting the IPS_EN
>> bit. Really I would like to see the tbl value converted to an enum and
>> shifted by 1 in order to generate the table reference.
>
>
> I would have done this, but we can't use an enum shifted bit because the
> field values are 01, 10, and 11. I used the direct 2, 4, and 6 values
> rather than shifting by one, but I can reset them and shift by 1.
I didn't mean 1 << enum I was referring to enum << 1. Right now you
can be given a table value of 3 if somebody incorrectly used the
function and the side effect is that it overwrites the enable bit.
>>
>> Here the "3" is a table index. It might be nice to call that out with
>> a name instead of using the magic number.
>
>
> Yep
>
>
>>
>>> +/**
>>> + * ixgbe_ipsec_set_rx_sa - set up the register bits to save SA info
>>> + * @hw: hw specific details
>>> + * @idx: register index to write
>>> + * @spi: security parameter index
>>> + * @key: key byte array
>>> + * @salt: salt bytes
>>> + * @mode: rx decrypt control bits
>>> + * @ip_idx: index into IP table for related IP address
>>> + **/
>>> +static void ixgbe_ipsec_set_rx_sa(struct ixgbe_hw *hw, u16 idx, __be32
>>> spi,
>>> + u32 key[], u32 salt, u32 mode, u32
>>> ip_idx)
>>> +{
>>> + int i;
>>> +
>>> + /* store the SPI (in bigendian) and IPidx */
>>> + IXGBE_WRITE_REG(hw, IXGBE_IPSRXSPI, spi);
>>> + IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPIDX, ip_idx);
>>> + IXGBE_WRITE_FLUSH(hw);
>>> +
>>> + ixgbe_ipsec_set_rx_item(hw, idx, IXGBE_RXIDX_TBL_SPI);
>>> +
>>> + /* store the key, salt, and mode */
>>> + for (i = 0; i < 4; i++)
>>> + IXGBE_WRITE_REG(hw, IXGBE_IPSRXKEY(i),
>>> cpu_to_be32(key[3-i]));
>>> + IXGBE_WRITE_REG(hw, IXGBE_IPSRXSALT, cpu_to_be32(salt));
>>> + IXGBE_WRITE_REG(hw, IXGBE_IPSRXMOD, mode);
>>> + IXGBE_WRITE_FLUSH(hw);
>>> +
>>> + ixgbe_ipsec_set_rx_item(hw, idx, IXGBE_RXIDX_TBL_KEY);
>>> +}
>>
>>
>> Is there any reason why you could write the SPI, key, salt, and mode,
>> then flush, and trigger the writes via the IPSRXIDX? Just wondering
>> since it would likely save you a few cycles avoiding PCIe bus stalls.
>
>
> See note above about religiously flushing everything to make a persnickety
> chip work.
I get the flushing. What I am saying is that as far as I can tell the
SPI, salt, and mode don't overlap so you could update all 3, flush,
and then call set_rx_item twice.
>>
>>
>>> +
>>> +/**
>>> + * ixgbe_ipsec_set_rx_ip - set up the register bits to save SA IP addr
>>> info
>>> + * @hw: hw specific details
>>> + * @idx: register index to write
>>> + * @addr: IP address byte array
>>> + **/
>>> +static void ixgbe_ipsec_set_rx_ip(struct ixgbe_hw *hw, u16 idx, u32
>>> addr[])
>>> +{
>>> + int i;
>>> +
>>> + /* store the ip address */
>>> + for (i = 0; i < 4; i++)
>>> + IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPADDR(i), addr[i]);
>>> + IXGBE_WRITE_FLUSH(hw);
>>> +
>>> + ixgbe_ipsec_set_rx_item(hw, idx, IXGBE_RXIDX_TBL_IP);
>>> +}
>>> +
>>
>>
>> This piece is kind of confusing. I would suggest storing the address
>> as a __be32 pointer instead of a u32 array. That way you start with
>> either an IPv6 or an IPv4 address at offset 0 instead of the way the
>> hardware is defined which has you writing it at either 0 or 3
>> depending on if the address is IPv6 or IPv4.
>
>
> Using a __be32 rather than u32 is fine here, it doesn't make much
> difference.
>
> If I understand your suggestion correctly, we would also need an additional
> function parameter to tell us if we were pointing to an ipv6 or ipv4
> address. Since the driver's SW tables are modeling the HW, I think it is
> simpler to leave it in the array.
Actually I am not too concerned about needing a flag, but the __be32
usage addresses another problem. If I am not mistaken in order to
store an IPv6 value you will have to write addr[3] to IPADDR(0) and so
forth since the hardware is storing the IPv6 address as little endian.
So if you store the IPv4 address in addr[0] as a __be32 value and
leave the rest as zero you should get the correct ordering in either
setup when you store either IPv6 or IPv4 values.
>
>>
>>> +/**
>>> + * ixgbe_ipsec_clear_hw_tables - because some tables don't get cleared
>>> on reset
>>> + * @adapter: board private structure
>>> + **/
>>> +void ixgbe_ipsec_clear_hw_tables(struct ixgbe_adapter *adapter)
>>> +{
>>> + struct ixgbe_hw *hw = &adapter->hw;
>>> + u32 buf[4] = {0, 0, 0, 0};
>>> + u16 idx;
>>> +
>>> + /* disable Rx and Tx SA lookup */
>>> + IXGBE_WRITE_REG(hw, IXGBE_IPSRXIDX, 0);
>>> + IXGBE_WRITE_REG(hw, IXGBE_IPSTXIDX, 0);
>>> +
>>> + /* scrub the tables */
>>> + for (idx = 0; idx < IXGBE_IPSEC_MAX_SA_COUNT; idx++)
>>> + ixgbe_ipsec_set_tx_sa(hw, idx, buf, 0);
>>> +
>>> + for (idx = 0; idx < IXGBE_IPSEC_MAX_SA_COUNT; idx++)
>>> + ixgbe_ipsec_set_rx_sa(hw, idx, 0, buf, 0, 0, 0);
>>> +
>>> + for (idx = 0; idx < IXGBE_IPSEC_MAX_RX_IP_COUNT; idx++)
>>> + ixgbe_ipsec_set_rx_ip(hw, idx, buf);
>>> +}
>>> +
>>> +/**
>>> + * ixgbe_init_ipsec_offload - initialize security registers for IPSec
>>> operation
>>> + * @adapter: board private structure
>>> + **/
>>> +void ixgbe_init_ipsec_offload(struct ixgbe_adapter *adapter)
>>> +{
>>> + ixgbe_ipsec_clear_hw_tables(adapter);
>>> +}
>>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.h
>>> b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.h
>>> new file mode 100644
>>> index 0000000..017b13f
>>> --- /dev/null
>>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.h
>>> @@ -0,0 +1,50 @@
>>>
>>> +/*******************************************************************************
>>> +
>>> + Intel 10 Gigabit PCI Express Linux driver
>>> + Copyright(c) 2017 Oracle and/or its affiliates. All rights reserved.
>>> +
>>> + This program is free software; you can redistribute it and/or modify
>>> it
>>> + under the terms and conditions of the GNU General Public License,
>>> + version 2, as published by the Free Software Foundation.
>>> +
>>> + This program is distributed in the hope it will be useful, but WITHOUT
>>> + ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>>> + FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
>>> for
>>> + more details.
>>> +
>>> + You should have received a copy of the GNU General Public License
>>> along with
>>> + this program. If not, see <http://www.gnu.org/licenses/>.
>>> +
>>> + The full GNU General Public License is included in this distribution
>>> in
>>> + the file called "COPYING".
>>> +
>>> + Contact Information:
>>> + Linux NICS <linux.nics@intel.com>
>>> + e1000-devel Mailing List <e1000-devel@lists.sourceforge.net>
>>> + Intel Corporation, 5200 N.E. Elam Young Parkway, Hillsboro, OR
>>> 97124-6497
>>> +
>>>
>>> +*******************************************************************************/
>>> +
>>> +#ifndef _IXGBE_IPSEC_H_
>>> +#define _IXGBE_IPSEC_H_
>>> +
>>> +#define IXGBE_IPSEC_MAX_SA_COUNT 1024
>>> +#define IXGBE_IPSEC_MAX_RX_IP_COUNT 128
>>> +#define IXGBE_IPSEC_BASE_RX_INDEX IXGBE_IPSEC_MAX_SA_COUNT
>>> +#define IXGBE_IPSEC_BASE_TX_INDEX (IXGBE_IPSEC_MAX_SA_COUNT * 2)
>>> +
>>> +#define IXGBE_RXTXIDX_IPS_EN 0x00000001
>>> +#define IXGBE_RXIDX_TBL_MASK 0x00000006
>>> +#define IXGBE_RXIDX_TBL_IP 0x00000002
>>> +#define IXGBE_RXIDX_TBL_SPI 0x00000004
>>> +#define IXGBE_RXIDX_TBL_KEY 0x00000006
>>
>>
>> You might look at converting these table entries into an enum and add
>> a shift value. It will make things much easier to read.
>>
>>> +#define IXGBE_RXTXIDX_IDX_MASK 0x00001ff8
>>> +#define IXGBE_RXTXIDX_IDX_READ 0x40000000
>>> +#define IXGBE_RXTXIDX_IDX_WRITE 0x80000000
>>> +
>>> +#define IXGBE_RXMOD_VALID 0x00000001
>>> +#define IXGBE_RXMOD_PROTO_ESP 0x00000004
>>> +#define IXGBE_RXMOD_DECRYPT 0x00000008
>>> +#define IXGBE_RXMOD_IPV6 0x00000010
>>> +
>>> +#endif /* _IXGBE_IPSEC_H_ */
>>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>> b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>> index 6d5f31e..51fb3cf 100644
>>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>> @@ -10327,6 +10327,7 @@ static int ixgbe_probe(struct pci_dev *pdev,
>>> const struct pci_device_id *ent)
>>> NETIF_F_FCOE_MTU;
>>> }
>>> #endif /* IXGBE_FCOE */
>>> + ixgbe_init_ipsec_offload(adapter);
>>>
>>> if (adapter->flags2 & IXGBE_FLAG2_RSC_CAPABLE)
>>> netdev->hw_features |= NETIF_F_LRO;
>>> --
>>> 2.7.4
>>>
>>> _______________________________________________
>>> Intel-wired-lan mailing list
>>> Intel-wired-lan@osuosl.org
>>> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply
* Re: [PATCH net-next v2 3/8] net: phy: meson-gxl: add read and write helpers for bank registers
From: Andrew Lunn @ 2017-12-07 16:02 UTC (permalink / raw)
To: Jerome Brunet
Cc: Florian Fainelli, Kevin Hilman, netdev, linux-arm-kernel,
linux-amlogic, linux-kernel, Russell King
In-Reply-To: <1512661873.7042.9.camel@baylibre.com>
> Banks actually comes from the datasheet, Yes.
> I don't mind renaming it but I would be making things up. As you wish ?
Keep it as is for the moment.
> Does the usual pages comes with this weird toggle thing to open the access ?
> Would we able to use these generic helpers with our this kind of quirks ?
I don't think the API has been defined yet. But what has been
discussed is adding functions to struct phy_driver. The driver can
then implement whatever is needed to select a given page. There will
then be helpers which take the lock, select the page, do the
read/write, select page 0, and unlock.
Supporting this funny toggle should not be a problem.
Andrew
^ permalink raw reply
* Re: [PATCH v2 net-next] net: stmmac: Add support for DWMAC5 with TSN features
From: Jose Abreu @ 2017-12-07 15:56 UTC (permalink / raw)
To: David Miller, Florian Fainelli
Cc: netdev, linux-kernel, Joao.Pinto, peppe.cavallaro,
alexandre.torgue
In-Reply-To: <20171101.204858.1211893549464034006.davem@davemloft.net>
Hi David, Florian,
On 01-11-2017 11:48, David Miller wrote:
> From: Jose Abreu <Jose.Abreu@synopsys.com>
> Date: Fri, 27 Oct 2017 14:55:44 +0100
>
>> This adds support for IP version 5 of DWMAC. The new introduced
>> features are the Enhancements to Scheduled Traffic (EST) as
>> defined by IEEE802.1Qbv-2015 and Frame Preemption (FPE) as
>> defined by IEEE802.1Qbu.
>>
>> In order to not break previous setups all the necessary
>> configuration is only performed when GMAC5 is detected and
>> only when all the necessary parameters are available in the
>> Device Tree.
>>
>> EST:
>> The IEEE802.1Qbv-2015 defines the schedule for each of the
>> queues which makes the IP aware of traffic arrival time. This
>> information can be used to block the lower priority traffic
>> from transmission in this time window/slot.
>>
>> FPE:
>> The IEEE802.1Qbu defines a mechanism which breaks interfering
>> frames into smaller fragments shuch that we have a more efficient
>> use of network bandwidth. This needs EST enabled.
>>
>> Signed-off-by: Jose Abreu <joabreu@synopsys.com>
>> Cc: David S. Miller <davem@davemloft.net>
>> Cc: Joao Pinto <jpinto@synopsys.com>
>> Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
>> Cc: Alexandre Torgue <alexandre.torgue@st.com>
>> ---
>> Changes since v1:
>> - Removed DT parsing
> This patch is pointless until something is added which turns on the
> boolean enable values.
First submission of this patch had DT bindings included to make
this configurable. I removed them following Florian input, which
I totally agree because DT describes HW and not configuration.
Current version of this patch makes this configurable by platform
data only.
This DWMAC5 is a new controller which will be available in new
NICs shortly so I think this is the best time to start thinking
about including these features. This way we will have support out
of the shelf for these new NICs ... I also must say that these
new features are a big leap forward for Quality-of-service
networking, as they allow us to specify traffic scheduling per
queue, improving not only network performance (by, for example,
reserving specific time slots for traffic that is periodic) as
well as network quality (by providing a constant bandwidth for
scheduled traffic).
David, Florian, may I ask what is in your opinion the best way to
handle the implementation of these new features?
Thanks and Best Regards,
Jose Miguel Abreu
^ permalink raw reply
* Re: [PATCH net-next v2 6/8] net: phy: meson-gxl: leave CONFIG_A6 untouched
From: Jerome Brunet @ 2017-12-07 15:56 UTC (permalink / raw)
To: Andrew Lunn
Cc: Florian Fainelli, Kevin Hilman, linux-kernel, netdev,
linux-amlogic, linux-arm-kernel
In-Reply-To: <20171207154943.GI24750@lunn.ch>
On Thu, 2017-12-07 at 16:49 +0100, Andrew Lunn wrote:
> On Thu, Dec 07, 2017 at 03:27:13PM +0100, Jerome Brunet wrote:
> > The PHY performs just as well when left in its default configuration and
> > it makes senses because this poke gets reset just after init.
>
> The only thing which might speak against this, is some bootloader
> which sets something other than the default, and here we put it back
> to the value it should have. But if you say a reset will put it back
> to the default value anyway, this seems save.
I was worried about this too bu the bank also gets reset on power down and soft
reset, so we won't get bootloader value at this point
>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
>
> Andrew
^ permalink raw reply
* Re: [PATCH net-next v2 8/8] net: phy: meson-gxl: join the authors
From: Andrew Lunn @ 2017-12-07 15:55 UTC (permalink / raw)
To: Jerome Brunet
Cc: Florian Fainelli, Kevin Hilman, netdev, linux-arm-kernel,
linux-amlogic, linux-kernel
In-Reply-To: <20171207142715.32578-9-jbrunet@baylibre.com>
On Thu, Dec 07, 2017 at 03:27:15PM +0100, Jerome Brunet wrote:
> Following previous changes, join the other authors of this driver and
> take the blame with them
:-)
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH net-next v2 7/8] net: phy: meson-gxl: add interrupt support
From: Andrew Lunn @ 2017-12-07 15:54 UTC (permalink / raw)
To: Jerome Brunet
Cc: Florian Fainelli, Kevin Hilman, netdev, linux-arm-kernel,
linux-amlogic, linux-kernel
In-Reply-To: <20171207142715.32578-8-jbrunet@baylibre.com>
On Thu, Dec 07, 2017 at 03:27:14PM +0100, Jerome Brunet wrote:
> Enable interrupt support in meson-gxl PHY driver
Hi Jerome
Is it possible to implement did_interrupt()? That allows for shared
interrupts. It does however work fine without it, so long as the
interrupt is not shared.
Thanks
Andrew
^ permalink raw reply
* Re: [PATCH net-next v2 3/8] net: phy: meson-gxl: add read and write helpers for bank registers
From: Jerome Brunet @ 2017-12-07 15:51 UTC (permalink / raw)
To: Andrew Lunn
Cc: Florian Fainelli, Kevin Hilman, netdev, linux-arm-kernel,
linux-amlogic, linux-kernel
In-Reply-To: <20171207154610.GG24750@lunn.ch>
On Thu, 2017-12-07 at 16:46 +0100, Andrew Lunn wrote:
> On Thu, Dec 07, 2017 at 03:27:10PM +0100, Jerome Brunet wrote:
> > Add read and write helpers to manipulate banked registers on this PHY
> > This helps clarify the settings applied to these registers in the init
> > function and upcoming changes.
> >
> > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> > ---
> > drivers/net/phy/meson-gxl.c | 103 ++++++++++++++++++++++++++++-------------
> > ---
> > 1 file changed, 67 insertions(+), 36 deletions(-)
> >
> > diff --git a/drivers/net/phy/meson-gxl.c b/drivers/net/phy/meson-gxl.c
> > index d82aa8cea401..05054770aefb 100644
> > --- a/drivers/net/phy/meson-gxl.c
> > +++ b/drivers/net/phy/meson-gxl.c
> > @@ -45,11 +45,13 @@
> > #define FR_PLL_DIV0 0x1c
> > #define FR_PLL_DIV1 0x1d
> >
> > -static int meson_gxl_config_init(struct phy_device *phydev)
> > +static int meson_gxl_open_banks(struct phy_device *phydev)
>
> Hi Jerome
>
> Does the word bank come from the datasheet? Most of the phy drives use
> page instead.
>
> Also, we have discovered a race condition which affects drivers using
> pages, which can lead to corruption of registers. At some point, i
> expect we will be adding helpers to access paged registers, which do
> the right thing with respect to locks.
>
> So it would be nice if you used the work page, not bank.
Banks actually comes from the datasheet, Yes.
I don't mind renaming it but I would be making things up. As you wish ?
Does the usual pages comes with this weird toggle thing to open the access ?
Would we able to use these generic helpers with our this kind of quirks ?
>
> Thanks
>
> Andrew
^ permalink raw reply
* [PATCH AUTOSEL for 4.4 080/101] l2tp: cleanup l2tp_tunnel_delete calls
From: alexander.levin @ 2017-12-07 15:50 UTC (permalink / raw)
To: linux-kernel@vger.kernel.org, stable@vger.kernel.org
Cc: Jiri Slaby, Sabrina Dubroca, Guillaume Nault, David S . Miller,
netdev@vger.kernel.org, alexander.levin
In-Reply-To: <20171207155005.5852-1-alexander.levin@verizon.com>
From: Jiri Slaby <jslaby@suse.cz>
[ Upstream commit 4dc12ffeaeac939097a3f55c881d3dc3523dff0c ]
l2tp_tunnel_delete does not return anything since commit 62b982eeb458
("l2tp: fix race condition in l2tp_tunnel_delete"). But call sites of
l2tp_tunnel_delete still do casts to void to avoid unused return value
warnings.
Kill these now useless casts.
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Sabrina Dubroca <sd@queasysnail.net>
Cc: Guillaume Nault <g.nault@alphalink.fr>
Cc: David S. Miller <davem@davemloft.net>
Cc: netdev@vger.kernel.org
Acked-by: Guillaume Nault <g.nault@alphalink.fr>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <alexander.levin@verizon.com>
---
net/l2tp/l2tp_core.c | 2 +-
net/l2tp/l2tp_netlink.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index d48281ca9c72..ec8f6a6485e3 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -1856,7 +1856,7 @@ static __net_exit void l2tp_exit_net(struct net *net)
rcu_read_lock_bh();
list_for_each_entry_rcu(tunnel, &pn->l2tp_tunnel_list, list) {
- (void)l2tp_tunnel_delete(tunnel);
+ l2tp_tunnel_delete(tunnel);
}
rcu_read_unlock_bh();
}
diff --git a/net/l2tp/l2tp_netlink.c b/net/l2tp/l2tp_netlink.c
index 665cc74df5c5..fb3248ff8b48 100644
--- a/net/l2tp/l2tp_netlink.c
+++ b/net/l2tp/l2tp_netlink.c
@@ -285,7 +285,7 @@ static int l2tp_nl_cmd_tunnel_delete(struct sk_buff *skb, struct genl_info *info
l2tp_tunnel_notify(&l2tp_nl_family, info,
tunnel, L2TP_CMD_TUNNEL_DELETE);
- (void) l2tp_tunnel_delete(tunnel);
+ l2tp_tunnel_delete(tunnel);
out:
return ret;
--
2.11.0
^ permalink raw reply related
* Re: [PATCH net-next v2 6/8] net: phy: meson-gxl: leave CONFIG_A6 untouched
From: Andrew Lunn @ 2017-12-07 15:49 UTC (permalink / raw)
To: Jerome Brunet
Cc: Florian Fainelli, Kevin Hilman, linux-kernel, netdev,
linux-amlogic, linux-arm-kernel
In-Reply-To: <20171207142715.32578-7-jbrunet@baylibre.com>
On Thu, Dec 07, 2017 at 03:27:13PM +0100, Jerome Brunet wrote:
> The PHY performs just as well when left in its default configuration and
> it makes senses because this poke gets reset just after init.
The only thing which might speak against this, is some bootloader
which sets something other than the default, and here we put it back
to the value it should have. But if you say a reset will put it back
to the default value anyway, this seems save.
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* [PATCH AUTOSEL for 4.9 129/156] l2tp: cleanup l2tp_tunnel_delete calls
From: alexander.levin @ 2017-12-07 15:49 UTC (permalink / raw)
To: linux-kernel@vger.kernel.org, stable@vger.kernel.org
Cc: Jiri Slaby, Sabrina Dubroca, Guillaume Nault, David S . Miller,
netdev@vger.kernel.org, alexander.levin
In-Reply-To: <20171207154845.4814-1-alexander.levin@verizon.com>
From: Jiri Slaby <jslaby@suse.cz>
[ Upstream commit 4dc12ffeaeac939097a3f55c881d3dc3523dff0c ]
l2tp_tunnel_delete does not return anything since commit 62b982eeb458
("l2tp: fix race condition in l2tp_tunnel_delete"). But call sites of
l2tp_tunnel_delete still do casts to void to avoid unused return value
warnings.
Kill these now useless casts.
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Sabrina Dubroca <sd@queasysnail.net>
Cc: Guillaume Nault <g.nault@alphalink.fr>
Cc: David S. Miller <davem@davemloft.net>
Cc: netdev@vger.kernel.org
Acked-by: Guillaume Nault <g.nault@alphalink.fr>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <alexander.levin@verizon.com>
---
net/l2tp/l2tp_core.c | 2 +-
net/l2tp/l2tp_netlink.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index b06acd0f400d..cfc4dd8997e5 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -1944,7 +1944,7 @@ static __net_exit void l2tp_exit_net(struct net *net)
rcu_read_lock_bh();
list_for_each_entry_rcu(tunnel, &pn->l2tp_tunnel_list, list) {
- (void)l2tp_tunnel_delete(tunnel);
+ l2tp_tunnel_delete(tunnel);
}
rcu_read_unlock_bh();
diff --git a/net/l2tp/l2tp_netlink.c b/net/l2tp/l2tp_netlink.c
index 1ccd310d01a5..ee03bc866d1b 100644
--- a/net/l2tp/l2tp_netlink.c
+++ b/net/l2tp/l2tp_netlink.c
@@ -287,7 +287,7 @@ static int l2tp_nl_cmd_tunnel_delete(struct sk_buff *skb, struct genl_info *info
l2tp_tunnel_notify(&l2tp_nl_family, info,
tunnel, L2TP_CMD_TUNNEL_DELETE);
- (void) l2tp_tunnel_delete(tunnel);
+ l2tp_tunnel_delete(tunnel);
out:
return ret;
--
2.11.0
^ permalink raw reply related
* Re: [PATCH net-next v2 3/8] net: phy: meson-gxl: add read and write helpers for bank registers
From: Neil Armstrong @ 2017-12-07 15:49 UTC (permalink / raw)
To: Andrew Lunn, Jerome Brunet
Cc: Florian Fainelli, Kevin Hilman, linux-kernel, netdev,
linux-amlogic, linux-arm-kernel
In-Reply-To: <20171207154610.GG24750@lunn.ch>
On 07/12/2017 16:46, Andrew Lunn wrote:
> On Thu, Dec 07, 2017 at 03:27:10PM +0100, Jerome Brunet wrote:
>> Add read and write helpers to manipulate banked registers on this PHY
>> This helps clarify the settings applied to these registers in the init
>> function and upcoming changes.
>>
>> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
>> ---
>> drivers/net/phy/meson-gxl.c | 103 ++++++++++++++++++++++++++++----------------
>> 1 file changed, 67 insertions(+), 36 deletions(-)
>>
>> diff --git a/drivers/net/phy/meson-gxl.c b/drivers/net/phy/meson-gxl.c
>> index d82aa8cea401..05054770aefb 100644
>> --- a/drivers/net/phy/meson-gxl.c
>> +++ b/drivers/net/phy/meson-gxl.c
>> @@ -45,11 +45,13 @@
>> #define FR_PLL_DIV0 0x1c
>> #define FR_PLL_DIV1 0x1d
>>
>> -static int meson_gxl_config_init(struct phy_device *phydev)
>> +static int meson_gxl_open_banks(struct phy_device *phydev)
>
> Hi Jerome
>
> Does the word bank come from the datasheet? Most of the phy drives use
> page instead.
Yes, it's explicitly described as banks in the datasheet.
Neil
>
> Also, we have discovered a race condition which affects drivers using
> pages, which can lead to corruption of registers. At some point, i
> expect we will be adding helpers to access paged registers, which do
> the right thing with respect to locks.
>
> So it would be nice if you used the work page, not bank.
>
> Thanks
>
> Andrew
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
^ permalink raw reply
* Re: [PATCH net-next v2 4/8] net: phy: meson-gxl: use genphy_config_init
From: Andrew Lunn @ 2017-12-07 15:46 UTC (permalink / raw)
To: Jerome Brunet
Cc: Florian Fainelli, Kevin Hilman, linux-kernel, netdev,
linux-amlogic, linux-arm-kernel
In-Reply-To: <20171207142715.32578-5-jbrunet@baylibre.com>
On Thu, Dec 07, 2017 at 03:27:11PM +0100, Jerome Brunet wrote:
> Use the generic init function to populate some of the phydev
> structure fields
>
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH net-next v2 3/8] net: phy: meson-gxl: add read and write helpers for bank registers
From: Andrew Lunn @ 2017-12-07 15:46 UTC (permalink / raw)
To: Jerome Brunet
Cc: Florian Fainelli, Kevin Hilman, netdev, linux-arm-kernel,
linux-amlogic, linux-kernel
In-Reply-To: <20171207142715.32578-4-jbrunet@baylibre.com>
On Thu, Dec 07, 2017 at 03:27:10PM +0100, Jerome Brunet wrote:
> Add read and write helpers to manipulate banked registers on this PHY
> This helps clarify the settings applied to these registers in the init
> function and upcoming changes.
>
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> ---
> drivers/net/phy/meson-gxl.c | 103 ++++++++++++++++++++++++++++----------------
> 1 file changed, 67 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/net/phy/meson-gxl.c b/drivers/net/phy/meson-gxl.c
> index d82aa8cea401..05054770aefb 100644
> --- a/drivers/net/phy/meson-gxl.c
> +++ b/drivers/net/phy/meson-gxl.c
> @@ -45,11 +45,13 @@
> #define FR_PLL_DIV0 0x1c
> #define FR_PLL_DIV1 0x1d
>
> -static int meson_gxl_config_init(struct phy_device *phydev)
> +static int meson_gxl_open_banks(struct phy_device *phydev)
Hi Jerome
Does the word bank come from the datasheet? Most of the phy drives use
page instead.
Also, we have discovered a race condition which affects drivers using
pages, which can lead to corruption of registers. At some point, i
expect we will be adding helpers to access paged registers, which do
the right thing with respect to locks.
So it would be nice if you used the work page, not bank.
Thanks
Andrew
^ permalink raw reply
* [PATCH AUTOSEL for 4.14 076/135] l2tp: cleanup l2tp_tunnel_delete calls
From: alexander.levin @ 2017-12-07 15:45 UTC (permalink / raw)
To: linux-kernel@vger.kernel.org, stable@vger.kernel.org
Cc: Jiri Slaby, Sabrina Dubroca, Guillaume Nault, David S . Miller,
netdev@vger.kernel.org, alexander.levin
In-Reply-To: <20171207154513.4154-1-alexander.levin@verizon.com>
From: Jiri Slaby <jslaby@suse.cz>
[ Upstream commit 4dc12ffeaeac939097a3f55c881d3dc3523dff0c ]
l2tp_tunnel_delete does not return anything since commit 62b982eeb458
("l2tp: fix race condition in l2tp_tunnel_delete"). But call sites of
l2tp_tunnel_delete still do casts to void to avoid unused return value
warnings.
Kill these now useless casts.
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Sabrina Dubroca <sd@queasysnail.net>
Cc: Guillaume Nault <g.nault@alphalink.fr>
Cc: David S. Miller <davem@davemloft.net>
Cc: netdev@vger.kernel.org
Acked-by: Guillaume Nault <g.nault@alphalink.fr>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <alexander.levin@verizon.com>
---
net/l2tp/l2tp_core.c | 2 +-
net/l2tp/l2tp_netlink.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index 02d61101b108..af22aa8ae35b 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -1891,7 +1891,7 @@ static __net_exit void l2tp_exit_net(struct net *net)
rcu_read_lock_bh();
list_for_each_entry_rcu(tunnel, &pn->l2tp_tunnel_list, list) {
- (void)l2tp_tunnel_delete(tunnel);
+ l2tp_tunnel_delete(tunnel);
}
rcu_read_unlock_bh();
diff --git a/net/l2tp/l2tp_netlink.c b/net/l2tp/l2tp_netlink.c
index 7135f4645d3a..c28223d8092b 100644
--- a/net/l2tp/l2tp_netlink.c
+++ b/net/l2tp/l2tp_netlink.c
@@ -282,7 +282,7 @@ static int l2tp_nl_cmd_tunnel_delete(struct sk_buff *skb, struct genl_info *info
l2tp_tunnel_notify(&l2tp_nl_family, info,
tunnel, L2TP_CMD_TUNNEL_DELETE);
- (void) l2tp_tunnel_delete(tunnel);
+ l2tp_tunnel_delete(tunnel);
l2tp_tunnel_dec_refcount(tunnel);
--
2.11.0
^ permalink raw reply related
* Re: [PATCH net-next v2 5/8] net: phy: meson-gxl: detect LPA corruption
From: Jerome Brunet @ 2017-12-07 15:42 UTC (permalink / raw)
To: Andrew Lunn
Cc: Florian Fainelli, Kevin Hilman, netdev, linux-arm-kernel,
linux-amlogic, linux-kernel
In-Reply-To: <20171207153403.GE24750@lunn.ch>
On Thu, 2017-12-07 at 16:34 +0100, Andrew Lunn wrote:
> On Thu, Dec 07, 2017 at 03:27:12PM +0100, Jerome Brunet wrote:
> > The purpose of this change is to fix the incorrect detection of the link
> > partner (LP) advertised capabilities which sometimes happens with this PHY
> > (roughly 1 time in a dozen)
>
> Hi Jerome
>
> Since this is a real fix, please send it alone. Base it on net, not
> net-next. David will then get it applied to stable, so it will be
> backported.
I hesitated on this. For this fix to be "not too ugly" I need at least the
helper functions (patch 1 to 3) ... which are not really fixes.
Would it be Ok if send patches 1 to 5 to net ?
and 6 to 8 separately on net-next ?
>
> Thanks
> Andrew
^ permalink raw reply
* Re: [PATCH v3 net-next 0/1] net: dsa: microchip: Add Microchip KSZ8795 DSA driver
From: Vivien Didelot @ 2017-12-07 15:34 UTC (permalink / raw)
To: Andrew Lunn, Tristram.Ha
Cc: Florian Fainelli, Pavel Machek, Ruediger Schmitt, muvarov,
nathan.leigh.conrad, UNGLinuxDriver, netdev
In-Reply-To: <20171206232548.GG28774@lunn.ch>
Andrew Lunn <andrew@lunn.ch> writes:
> On Tue, Dec 05, 2017 at 05:47:30PM -0800, Tristram.Ha@microchip.com wrote:
>> From: Tristram Ha <Tristram.Ha@microchip.com>
>>
>> This patch requires the previous patches for Microchip KSZ9477 DSA driver.
>
> Please don't submit so many patch sets, which all depend on the
> previous one. Wait until they get accepted.
>
> I'm not going to review this or the following patchsets, until the
> previous ones are accepted.
+1
^ permalink raw reply
* Re: [PATCH net-next v2 1/8] net: phy: meson-gxl: check phy_write return value
From: Andrew Lunn @ 2017-12-07 15:34 UTC (permalink / raw)
To: Jerome Brunet
Cc: Florian Fainelli, Kevin Hilman, netdev, linux-arm-kernel,
linux-amlogic, linux-kernel
In-Reply-To: <20171207142715.32578-2-jbrunet@baylibre.com>
On Thu, Dec 07, 2017 at 03:27:08PM +0100, Jerome Brunet wrote:
> Always check phy_write return values. Better to be safe than sorry
>
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH net-next v2 5/8] net: phy: meson-gxl: detect LPA corruption
From: Andrew Lunn @ 2017-12-07 15:34 UTC (permalink / raw)
To: Jerome Brunet
Cc: Florian Fainelli, Kevin Hilman, netdev, linux-arm-kernel,
linux-amlogic, linux-kernel
In-Reply-To: <20171207142715.32578-6-jbrunet@baylibre.com>
On Thu, Dec 07, 2017 at 03:27:12PM +0100, Jerome Brunet wrote:
> The purpose of this change is to fix the incorrect detection of the link
> partner (LP) advertised capabilities which sometimes happens with this PHY
> (roughly 1 time in a dozen)
Hi Jerome
Since this is a real fix, please send it alone. Base it on net, not
net-next. David will then get it applied to stable, so it will be
backported.
Thanks
Andrew
^ permalink raw reply
* Re: [PATCH net 2/2] net: dsa: mv88e6xxx: Unregister MDIO bus on error path
From: Vivien Didelot @ 2017-12-07 15:33 UTC (permalink / raw)
To: Andrew Lunn, David Miller; +Cc: netdev, Andrew Lunn
In-Reply-To: <1512605157-6765-3-git-send-email-andrew@lunn.ch>
Andrew Lunn <andrew@lunn.ch> writes:
> The MDIO busses need to be unregistered before they are freed,
> otherwise BUG() is called. Add a call to the unregister code if the
> registration fails, since we can have multiple busses, of which some
> may correctly register before one fails. This requires moving the code
> around a little.
>
> Fixes: a3c53be55c95 ("net: dsa: mv88e6xxx: Support multiple MDIO busses")
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
^ permalink raw reply
* Re: [PATCH net 1/2] net: dsa: mv88e6xxx: Fix interrupt masking on removal
From: Vivien Didelot @ 2017-12-07 15:33 UTC (permalink / raw)
To: Andrew Lunn, David Miller; +Cc: netdev, Andrew Lunn
In-Reply-To: <1512605157-6765-2-git-send-email-andrew@lunn.ch>
Andrew Lunn <andrew@lunn.ch> writes:
> When removing the interrupt handling code, we should mask the
> generation of interrupts. The code however unmasked all
> interrupts. This can then cause a new interrupt. We then get into a
> deadlock where the interrupt thread is waiting to run, and the code
> continues, trying to remove the interrupt handler, which means waiting
> for the thread to complete. On a UP machine this deadlocks.
>
> Fix so we really mask interrupts in the hardware. The same error is
> made in the error path when install the interrupt handling code.
>
> Fixes: 3460a5770ce9 ("net: dsa: mv88e6xxx: Mask g1 interrupts and free interrupt")
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
^ permalink raw reply
* Re: [PATCH net-next 3/3] net: dsa: mediatek: update MAINTAINERS entry with MediaTek switch driver
From: Andrew Lunn @ 2017-12-07 15:31 UTC (permalink / raw)
To: sean.wang
Cc: davem, f.fainelli, vivien.didelot, netdev, linux-kernel,
linux-mediatek
In-Reply-To: <9462ddd33c2f34839c68b73f76017487d4b5cb21.1512625814.git.sean.wang@mediatek.com>
On Thu, Dec 07, 2017 at 02:06:12PM +0800, sean.wang@mediatek.com wrote:
> From: Sean Wang <sean.wang@mediatek.com>
>
> I work for MediaTek and maintain SoC targeting to home gateway and
> also will keep extending and testing the function from MediaTek
> switch.
>
> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH net-next 2/3] net: dsa: mediatek: combine MediaTek tag with VLAN tag
From: Andrew Lunn @ 2017-12-07 15:30 UTC (permalink / raw)
To: sean.wang
Cc: davem, f.fainelli, vivien.didelot, netdev, linux-kernel,
linux-mediatek
In-Reply-To: <dffc711dc88898f8a5f5387d1fa7d14e0faf9ed5.1512625814.git.sean.wang@mediatek.com>
> @@ -25,20 +28,37 @@ static struct sk_buff *mtk_tag_xmit(struct sk_buff *skb,
> {
> struct dsa_port *dp = dsa_slave_to_port(dev);
> u8 *mtk_tag;
> + bool is_vlan_skb = true;
..
> + /* Mark tag attribute on special tag insertion to notify hardware
> + * whether that's a combined special tag with 802.1Q header.
> + */
> + mtk_tag[0] = is_vlan_skb ? MTK_HDR_XMIT_TAGGED_TPID_8100 :
> + MTK_HDR_XMIT_UNTAGGED;
> mtk_tag[1] = (1 << dp->index) & MTK_HDR_XMIT_DP_BIT_MASK;
> - mtk_tag[2] = 0;
> - mtk_tag[3] = 0;
> +
> + /* Tag control information is kept for 802.1Q */
> + if (!is_vlan_skb) {
> + mtk_tag[2] = 0;
> + mtk_tag[3] = 0;
> + }
>
> return skb;
> }
Hi Sean
So you can mark a packet for egress. What about ingress? How do you
know the VLAN/PORT combination for packets the CPU receives? I would
of expected a similar change to mtk_tag_rcv().
Andrew
^ 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