Hi Sven, see below
> - Bryan Whitehead:
> + multi-buffer patch concept "looks good".
> As a result, I will squash the intermediate "dma buffer only" patch
> which
> demonstrated the speed boost using an inflexible solution
> (w/o multi-buffers).
> + Rename lan743x_rx_pr
Hi Sven,
> Subject: [PATCH net-next v2 1/5] lan743x: boost performance on cpu archs
> w/o dma cache snooping
>
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
>
> From: Sven Van Asbroeck
>
> The buffers in the lan743x driver's receive ring are alw
Hi Sven, see below.
> + if (buffer_info->dma_ptr) {
> + /* unmap from dma */
> + packet_length = RX_DESC_DATA0_FRAME_LENGTH_GET_
> + (le32_to_cpu(descriptor->data0));
> + if (packet_length == 0 ||
> + p
> Will do. Are you planning to hold off your tests until v3? It shouldn't take
> too
> long.
Sure, we will wait for v3
> From: Sven Van Asbroeck
>
> On cpu architectures w/o dma cache snooping, dma_unmap() is a is a very
> expensive operation, because its resulting sync needs to invalidate cpu
> caches.
>
> Increase efficiency/performance by syncing only those sections of the
> lan743x's rx ring buffers that are
> From: Sven Van Asbroeck
>
> Tree: git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git #
> 9ec5eea5b6ac
>
> v2 -> v3:
> - Bryan Whitehead:
> + add Bryan's reviewed-by tag to patch 1/5.
> + Only use FRAME_LENGTH if the LS bit is checked.
> If set use the smaller of F
Sven, see below comments
> @@ -2148,11 +2149,18 @@ static int lan743x_rx_process_packet(struct
> lan743x_rx *rx)
> descriptor = &rx->ring_cpu_ptr[first_index];
>
> /* unmap from dma */
> + packet_length = RX_DESC_DATA0_FRAME_LE
Hi Sven,
Looks good.
see comments below.
> static int lan743x_rx_process_packet(struct lan743x_rx *rx) {
It looks like this function no longer processes a packet, but rather only
processes a single buffer.
So perhaps it should be renamed to lan743x_rx_process_buffer, so it is not
misleading.
Hi Sven, see below
> >
> > If lan743x_rx_init_ring_element fails to allocate an skb, Then
> > lan743x_rx_reuse_ring_element will be called.
> > But that function expects the skb is already allocated and dma mapped.
> > But the dma was unmapped above.
>
> Good catch. I think you're right, the skb
Hi Sven,
We can test on x86 PC. We will just need about a week after you release your
next version.
Thanks,
Bryan
> -Original Message-
> From: Sven Van Asbroeck
> Sent: Wednesday, February 3, 2021 1:53 PM
> To: Bryan Whitehead - C21958
> Cc: UNGLinuxDriver ; David Miller
> ; Jakub Kic
> On Wed, Feb 3, 2021 at 3:14 PM wrote:
> >
> > We can test on x86 PC. We will just need about a week after you release
> your next version.
> >
>
> That's great. If you have any suggestions on how I can improve testing on my
> end, feel free to reach out.
If you are able, in addition to basic r
> In the second and third hunk, you have to check lan743x_rx_allocate_skb()
> for NULL too and act accordingly.
OK David,
I'll work on it.
Bryan
> -Original Message-
> From: David Miller
> Sent: Monday, March 11, 2019 2:37 PM
> To: Bryan Whitehead - C21958
> Cc: netdev@vger.kernel.org; UNGLinuxDriver
>
> Subject: Re: [PATCH v2 net] lan743x: Fix RX Kernel Panic
>
> From: Bryan Whitehead
> Date: Mon, 11 Mar 2019 13:39:39 -0400
>
> Applied and queued up for -stable, thanks.
Thanks David
> Hi Bryan
>
> It is normal to put something in the commit message, even if it is the Subject
> line said in a different way.
>
> Otherwise, this looks O.K.
>
> Andrew
OK, thanks Andrew
> ARRAY_SIZE(lan743x_set0_hw_cnt_addr) ?
>
...snip...
>
> Andrew
Will do, I will resubmit with these changes.
>
> MAX_EEPROM_SIZE ?
>
... snip ...
>
> Andrew
Thanks Andrew, I'll change it.
> > + data = lan743x_csr_read(adapter, PMT_CTL);
>
> Hi Bryan
>
> Why do you do this read? You do not use the result.
>
Good catch, I'll remove it.
> > +
> > + wol->supported = WAKE_BCAST | WAKE_UCAST | WAKE_MCAST |
> > + WAKE_MAGIC | WAKE_PHY | WAKE_ARP;
> > +
> > + wol->wolopt
Thanks Richard,
I'll add it in my next revision.
> -Original Message-
> From: Richard Cochran [mailto:richardcoch...@gmail.com]
> Sent: Friday, July 6, 2018 5:25 PM
> To: Bryan Whitehead - C21958
> Cc: da...@davemloft.net; netdev@vger.kernel.org; UNGLinuxDriver
>
> Subject: Re: [PATCH v1
> > +static int lan743x_ethtool_set_eee(struct net_device *netdev,
> > + struct ethtool_eee *eee)
> > +{
>
> Hi Bryan
>
> You should call phy_ethtool_set_eee() in both cases, so that it gets disabled
> in the PHY as well. It needs to stop advertising it.
>
>
> > +static void lan743x_intr_unregister_isr(struct lan743x_adapter *adapter,
> > + int vector_index)
> > +{
> > + struct lan743x_vector *vector = &adapter->intr.vector_list
> > + [vector_index];
> > +
> > + devm_free_irq(&adap
> On Thu, Feb 22, 2018 at 10:45:34PM +0100, Andrew Lunn wrote:
> > > Also I'm allocating interrupt resources on interface up, and freeing
> > > resources on interface down. So if there is an up, down, up sequence
> > > then the driver will allocate resources twice. In order for devm to
> > > work p
Hi Florian,
Thanks for your review. I have the following questions/comments.
> On 02/21/2018 11:06 AM, Bryan Whitehead wrote:
> > Add main source files for new lan743x driver.
> >
> > Signed-off-by: Bryan Whitehead
> > ---
>
> > +lan743x-objs := lan743x_main.o
>
> Should we assume that you have
> > Ok, but it seems to me that what I have is an example of "specific
> > book keeping private information". Can you clarify the style you prefer?
> >
> > In cases of allocation where I can just compare a pointer to null, I
> > can easily remove the flags. But in other cases I need a record of
> >
> Hi Bryan
>
> It is good to look at other drivers and see how they work. Ideally, your
> driver
> wants to look similar to all other drivers, so making the maintenance easier.
>
> You will find that most drivers have a set of goto statements for error
> handling, which jump to the end of the fu
> > +static int lan743x_phy_reset(struct lan743x_adapter *adapter) {
> > + u32 data;
> > +
> > + data = lan743x_csr_read(adapter, PMT_CTL);
> > + data |= PMT_CTL_ETH_PHY_RST_;
> > + lan743x_csr_write(adapter, PMT_CTL, data);
> > +
> > + return readx_poll_timeout(LAN743X_CSR_READ_OP, PMT_C
> > > > +static int lan743x_phy_reset(struct lan743x_adapter *adapter) {
> > > > + u32 data;
> > > > +
> > > > + data = lan743x_csr_read(adapter, PMT_CTL);
> > > > + data |= PMT_CTL_ETH_PHY_RST_;
> > > > + lan743x_csr_write(adapter, PMT_CTL, data);
> > > > +
> > > > +
> On Mon, Mar 05, 2018 at 02:23:30PM -0500, Bryan Whitehead wrote:
> > Add main source files for new lan743x driver
> >
> > Signed-off-by: Bryan Whitehead
>
> For MDIO & PHY handling etc
>
> Reviewed-by: Andrew Lunn
>
> But i have not look at any packet handling, DMA, NAPI, etc.
>
> Andre
> From: Bryan Whitehead
> Date: Mon, 5 Mar 2018 14:23:29 -0500
>
> > Add new lan743x driver.
> >
> > The lan743x from Microchip Technologies Inc, is a PCIe to Gigabit
> > Ethernet Controller.
>
> Series applied, thank you.
Thanks David
Thanks David and Florian, see below.
> On 7/30/20 4:36 PM, David Miller wrote:
> > From: Bryan Whitehead
> > Date: Mon, 27 Jul 2020 13:18:28 -0400
> >
> >> @@ -929,6 +929,77 @@ static bool vsc8574_is_serdes_init(struct
> >> phy_device *phydev) }
> >>
> >> /* bus->mdio_lock should be locked when
Thanks Florian, I will apply your suggestions.
> -Original Message-
> From: Florian Fainelli
> Sent: Thursday, July 23, 2020 5:19 PM
> To: Bryan Whitehead - C21958 ;
> da...@davemloft.net
> Cc: netdev@vger.kernel.org; UNGLinuxDriver
>
> Subject: Re: [PATCH net-next] mscc: Add LCPLL Reset
Thanks Andrew, I will apply your suggestions.
> -Original Message-
> From: Andrew Lunn
> Sent: Friday, July 24, 2020 9:19 AM
> To: Bryan Whitehead - C21958
> Cc: da...@davemloft.net; netdev@vger.kernel.org; UNGLinuxDriver
>
> Subject: Re: [PATCH net-next] mscc: Add LCPLL Reset to VSC857
Hi Florian, see below.
> > /* bus->mdio_lock should be locked when using this function */
> > +/* Page should already be set to MSCC_PHY_PAGE_EXTENDED_GPIO */
> > +static int vsc8574_wait_for_micro_complete(struct phy_device *phydev)
> > +{
> > + u16 timeout = 500;
> > + u16 reg18g = 0;
>
> -Original Message-
> From: Andrew Lunn
> Sent: Tuesday, November 20, 2018 2:31 PM
> To: Bryan Whitehead - C21958
> Cc: da...@davemloft.net; netdev@vger.kernel.org; UNGLinuxDriver
>
> Subject: Re: [PATCH v1 net] lan743x: fix return value for
> lan743x_tx_napi_poll
>
> On Tue, Nov 20, 2
> Did you look at the output of "perf top" or something along those lines to
> figure out if your lan743x driver is indeed responsible for that by not being
> scheduler friendly? What is likely happening is that you do not reclaim
> "weight" packets and instead keep looping into NAPI context, which
> I notice 2 problems in the driver:
>
> 1. netif_napi_add is used instead of netif_tx_napi_add.
> 2. In all other drivers that use netif_tx_napi_add most do not call
> napi_complete_done.
> They all call napi_complete directly and return 0.
>
> freescale/gianfar.c
> rocker/rocker_main.c
> ti/cps
> On Fri, Jan 18, 2019 at 04:54:53PM -0500, Bryan Whitehead wrote:
> > The LAN743x includes on chip One-Time-Programmable (OTP) memory.
> >
> > This patch extends the ethtool EEPROM read/write interface to access
> > OTP memory space.
> >
> > This is done by adding the private flag OTP_ACCESS, whic
> > > Hi Bryan
> > >
> > > It would be good to explain what is wrong with the current code,
> > > which allows you to select between the OTP and the EEPROM at write
> time.
> >
> > Hi Andrew,
> >
> > The current code does not allow OTP read access.
> > Plus the current code places unreasonable rest
> > +static int lan743x_otp_write(struct lan743x_adapter *adapter, u32 offset,
> > +u32 length, u8 *data)
> > +{
> > + int ret;
> > + int i;
> > +
> > + ret = lan743x_otp_power_up(adapter);
> > + if (ret < 0)
> > + return ret;
> > +
> > + ret = lan743x_ot
> > > This is breaking backwards compatibility. I think you need to
> > > respect the magic value, independent of how adapter->flags.
> >
> > Is backwards compatibility a requirement?
>
> Hi Bryan
>
> You should not change the ABI. And this is an ABI. So yes, backwards
> compatibility should be m
> > - /* set to internal PHY id */
> > - adapter->mdiobus->phy_mask = ~(u32)BIT(1);
> > + if ((adapter->csr.id_rev & ID_REV_ID_MASK_) ==
> ID_REV_ID_LAN7430_)
> > + /* LAN7430 uses internal phy at address 1 */
> > + adapter->mdiobus->phy_mask = ~(u32)BIT(1);
>
> Hi Bryan
> -Original Message-
> From: Roelof Berg
> Sent: Sunday, May 17, 2020 4:45 PM
> To: Andrew Lunn
> Cc: Bryan Whitehead - C21958 ;
> UNGLinuxDriver ; David S. Miller
> ; netdev@vger.kernel.org; linux-
> ker...@vger.kernel.org
> Subject: Re: [PATCH] lan743x: Added fixed link support
>
> EXT
Hi Richard,
Thank you for your detailed feedback. I'm working on it now, but I feel it will
take a little extra time to complete. Therefor I'm planning to remove PTP
support from this patch series, and resubmit it in a new patch later.
I also have a few questions below.
> -Original Message
Hi Andrew,
Thanks for your reviews, and corrections.
I will submit a new patch series soon.
Bryan
> > Can you clarify what is poor and what would be better?
> > For example, should I change "X != 0" to "X ? true : false".
>
> Look at this:
> lan743x_ptp_tx_timestamp_skb(tx->adapter,
>buffer_info->skb,
>
> > +#ifdef CONFIG_PM
> > +static void lan743x_ethtool_get_wol(struct net_device *netdev,
> > + struct ethtool_wolinfo *wol)
> > +{
> > + struct lan743x_adapter *adapter = netdev_priv(netdev);
> > +
> > + wol->supported = 0;
> > + wol->wolopts = 0;
> > + phy_et
> From: David Miller
> Date: Thu, 19 Jul 2018 07:15:52 +0900 (KST)
>
> > Please remove these "#endif FOO, #ifdef FOO" sequences, and instead
> > just have one large continuous "ifdef FOO, endif FOO" section.
>
> BTW, there were other patches that had this problem too, so please go
> through your
> It depends on the hardware designs, but in some designs, the PHY can do
> WOL, without the MAC involved. When a WoL condition happens, it triggers
> an interrupt, or blinks an LED which is also connected to the power supply
> etc. And if the PHY is doing WoL, you can shut down the MAC, save more
> Subject: [PATCH net-next] lan743x: Make symbol lan743x_pm_ops static
>
> Fixes the following sparse warning:
>
> drivers/net/ethernet/microchip/lan743x_main.c:2944:25: warning:
> symbol 'lan743x_pm_ops' was not declared. Should it be static?
>
> Signed-off-by: Wei Yongjun
Acked-by: Bryan Wh
From: Bryan Whitehead
Add LAN743X Driver to Kconfig and Makefile
Removed "depends on SPI" from NET_VENDOR_MICROCHIP group
Because all existing drivers already specify SPI dependency, and
New driver does not have SPI dependency.
Signed-off-by: Bryan Whitehead
---
drivers/net/ethernet/microchip
From: Bryan Whitehead
Add Microchip LAN743X PCIe 3.1 Gigabit Ethernet Controller Driver
Bryan Whitehead (3):
Add LAN743X driver
Add LAN743X to Kconfig and Makefile
Add LAN743X to MAINTAINER list
MAINTAINERS |7 +
drivers/net/ethernet/microchip/Kconfig |
From: Bryan Whitehead
Add LAN743X to MAINTAINER list
Signed-off-by: Bryan Whitehead
---
MAINTAINERS | 7 +++
1 file changed, 7 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 2db0f8c..3216348 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8749,6 +8749,13 @@ F: drivers/net
> -Original Message-
> From: David Miller [mailto:da...@davemloft.net]
> Sent: Friday, August 11, 2017 6:12 PM
> To: Bryan Whitehead - C21958
> Cc: netdev@vger.kernel.org; UNGLinuxDriver
> Subject: Re: [PATCH net-next 1/3] Add LAN743X driver
>
> From:
> Date: Fri, 11 Aug 2017 19:47:57 +
> Could you split it up a bit. Take the PTP support out for the moment, and
> submit it later once the core driver is accepted. The same for any other
> optional bits.
>
> Andrew
Andrew,
thanks for the feedback.
I will work on them and submit again later.
Bryan
>
> The statistics code here is confused.
> You are already counting rx_packets in software in napi_poll Then you get
> values from MAC. One or the other?
> There are two copies of stats, one in netdev and other in your mac structure.
>
> Also what about byte and error counts?
>
> If possible im
> I don't think netdev_priv can return NULL, so lines 6641 to 6646 could just be
> dropped.
>
> julia
>
Julia,
Thanks for the feedback.
I will make changes and submit again later.
Bryan
This is the initial submission of an ethernet driver for
the Microchip LAN9352.
The LAN9352 is a 2-Port 10/100 Managed Ethernet Switch
with 16-Bit Non-PCI CPU Interface.
While the LAN9352 is a Managed Ethernet Switch, this driver
only supports a simple ethernet controller interface.
Signed-off
Thanks David,
I'll submit a revised patch soon.
-Original Message-
From: David Miller [mailto:da...@davemloft.net]
Sent: Wednesday, February 10, 2016 5:34 AM
To: Bryan Whitehead - C21958
Cc: netdev@vger.kernel.org; cor...@lwn.net
Subject: Re: [PATCH net-next] Add LAN9352 Ethernet Driver
Hi Andrew,
Thanks for your comment.
The LAN9352 actually has 2 physical ports, and one virtual port which is tied
internally to the 16-bit Non-PCI CPU Interface. This driver acts as a normal
Ethernet controller on the virtual port, which itself is an input to the
embedded switch. The switch di
This is the initial submission of an ethernet driver for
the Microchip LAN9352.
The LAN9352 is a 2-Port 10/100 Managed Ethernet Switch with
16-Bit Non-PCI CPU Interface. The CPU interface includes a basic
ethernet controller interface whose virtual phy is connected
internally to a 3rd port on the
Hi Andrew,
Sorry I still did not make this clear. And I'm not sure I understand your
question so I'll try to explain again, but please give me feedback if it's
still not clear.
Also you can reference Figure 2-1 for an Internal Block Diagram on page 9 of
http://ww1.microchip.com/downloads/en/Dev
Florian,
I'm not familiar with the DSA subsystem. Can you refer me to documents about it?
Andrew,
I'm also not familiar with switchdev, or which is better in this case. Can you
refer me to documents about it?
At this point my plan is to just get a basic Ethernet controller driver
submitted, and
Lino,
Regarding "a matching smp_rmb() in the irq handler"
There is a smp_wmb() in the irq handler, since in both cases we are forcing a
write operation on software_irq_signal.
I suppose using atomic operations on software_irq_signal would also work, but
this driver was based on
drivers/net/eth
Thanks Florian,
Lots of good comments here. I'm working on fixes and I'll respond when I get
through it all.
Bryan
-Original Message-
From: Florian Fainelli [mailto:f.faine...@gmail.com]
Sent: Thursday, February 11, 2016 9:18 PM
To: Bryan Whitehead - C21958; da...@davemloft.net
Cc: net
Hi Andrew,
You can find my comments below wrapped in ...
-Original Message-
From: Andrew Lunn [mailto:and...@lunn.ch]
Sent: Friday, February 12, 2016 12:11 PM
To: Bryan Whitehead - C21958
Cc: da...@davemloft.net; netdev@vger.kernel.org
Subject: Re: [PATCH net-next,V2] Add LAN9352 Ether
Andrew,
At this point, I am not tasked with implementing switch features, which would
likely take a long time to complete.
If it is too difficult to add switch features later, then they may come as an
entirely new driver at that time.
So this driver should only be thought of as operating a sin
> -Original Message-
> From: David Miller [mailto:da...@davemloft.net]
> Sent: Tuesday, February 16, 2016 3:44 PM
> To: and...@lunn.ch
> Cc: Bryan Whitehead - C21958; f.faine...@gmail.com; netdev@vger.kernel.org
> Subject: Re: [PATCH net-next,V2] Add LAN9352 Ethernet Driver
>
> From: Andre
> From: David Miller [mailto:da...@davemloft.net]
> Sent: Tuesday, February 16, 2016 3:53 PM
> >>
> >> I do not think, with all the infrastructure we have, that we should
> >> accept pure ethernet drivers for such devices any more.
> >>
> >> About year ago I would have responded differently, but th
> From: Andrew Lunn [mailto:and...@lunn.ch]
> Sent: Tuesday, February 16, 2016 3:58 PM
> > I believe I can make the physical phys accessible through ethertool.
> > Would that be reasonable?
>
> How, you have no netdev to attach them to?
>
> There was a nice presentation at netdev last week by Me
> -Original Message-
> From: Andrew Lunn [mailto:and...@lunn.ch]
> Sent: Friday, February 19, 2016 3:14 PM
> > I'm not yet sure how it attaches to the underlying Ethernet driver.
>
> The DSA core does that for you. If you look at the device tree
> binding:
>
> dsa@0 {
>
> From: Andrew Lunn [mailto:and...@lunn.ch]
> Sent: Tuesday, February 16, 2016 5:16 PM
>
> You are already 1/2 way to a DSA driver, since you have a MAC driver. So i
> agree with David, do it right and add a simple DSA driver.
>
Andrew,
I've done a little research on DSA.
I've read Documentation
Thanks Andrew,
Your tips are much appreciated.
Bryan
72 matches
Mail list logo
