Hi Julian,
On Wed, Apr 14, 2021 at 9:33 AM Julian Wiedmann wrote:
>
> __netdev_alloc_skb_ip_align() already reserves the NET_IP_ALIGN part.
> So when the NIC stores into the dma-mapped skb->data parts, each
> fragment will automatically have the required alignment - even when
> you only care abou
Hi Julian,
On Wed, Apr 14, 2021 at 8:53 AM Julian Wiedmann wrote:
>
> On a cursory glance, using __netdev_alloc_skb_ip_align() here should
> allow you to get rid of all the RX_HEAD_PADDING gymnastics.
>
> And also avoid the need for setting RX_CFG_B_RX_PAD_2_, as the
> NET_IP_ALIGN part would no
Hi George,
On Fri, Apr 9, 2021 at 10:12 AM George McCollister
wrote:
>
> I'm glad everyone was able to work together to get this fixed properly
> without any figure pointing or mud slinging! Kudos everyone.
Same, this is what the kernel community is supposed to be all about.
And thank you for t
Hi Andrew,
On Thu, Apr 8, 2021 at 9:02 PM Andrew Lunn wrote:
>
> Adding a Suggested-by: would be good. And it might sometime help
> Johnathan Corbet extract some interesting statistics from the commit
> messages if everybody uses the same format.
Thank you for the suggestion. I'll definitely use
From: Sven Van Asbroeck
The ethernet frame length is calculated incorrectly. Depending on
the value of RX_HEAD_PADDING, this may result in ethernet frames
that are too short (cut off at the end), or too long (garbage added
to the end).
Fix by calculating the ethernet frame length correctly. For
Hi Heiner,
On Thu, Apr 8, 2021 at 3:06 PM Heiner Kallweit wrote:
>
> A completely unrelated question:
> How about VLAN packets with a 802.1Q tag? Should VLAN_ETH_HLEN be used?
That's a good question. My use-case does not involve 802.1Q though, so
I'm unable to test.
Thank you so much for your s
Hi George,
On Thu, Apr 8, 2021 at 3:55 PM George McCollister
wrote:
>
> Works for me too.
Sounds good. I'll post a proper patch soon. Would you be able to
review+test, and perhaps offer your Reviewed-by/Tested-by tags when
everything looks ok?
Hi George,
On Thu, Apr 8, 2021 at 2:26 PM Sven Van Asbroeck wrote:
>
> George, I will send a patch for you to try shortly. Except if you're
> already ahead :)
Would this work for you? It does for me.
diff --git a/drivers/net/ethernet/microchip/lan743x_main.c
b/drivers/net/ethe
Hi Heiner,
On Thu, Apr 8, 2021 at 2:22 PM Heiner Kallweit wrote:
>
> Just an idea:
> RX_HEAD_PADDING is an alias for NET_IP_ALIGN that can have two values:
> 0 and 2
> The two systems you use may have different NET_IP_ALIGN values.
> This could explain the behavior. Then what I proposed should wo
Hi Heiner,
On Thu, Apr 8, 2021 at 1:49 PM Heiner Kallweit wrote:
>
> Can't we use frame_length - ETH_FCS_LEN direcctly here?
If the hard-coded "4" refers to ETH_FCS_LEN, then yes, good point. I'd
love to find out first why George and I need different patches to make
the driver work in our use ca
Hi George,
On Thu, Apr 8, 2021 at 1:36 PM George McCollister
wrote:
>
> Can you explain the difference in behavior with what I was observing
> on the LAN7431?
I'm not using DSA in my application, so I cannot test or replicate
what you were observing. It would be great if we could work together
a
From: Sven Van Asbroeck
This reverts commit 3e21a10fdea3c2e4e4d1b72cb9d720256461af40.
The reverted patch completely breaks all network connectivity on the
lan7430. tcpdump indicates missing bytes when receiving ping
packets from an external host:
host$ ping $lan7430_ip
lan7430$ tcpdump -v
IP
Hi Jakub and Bryan,
On Wed, Feb 17, 2021 at 4:43 PM wrote:
>
> Just to let you know, my colleague tested the patches 1 and 2 on x86 PC and
> we are satisfied with the result.
> We confirmed some performance improvements.
> We also confirmed PTP is working.
>
> Thanks for your work on this.
>
> T
Hi Bryan,
On Tue, Feb 16, 2021 at 3:50 PM wrote:
>
> Looks Good, Thanks Sven
> Our testing is in progress, We will let you know our results soon.
>
> Reviewed-by: Bryan Whitehead
>
Thank you Bryan, I really appreciate your help and expertise.
From: Sven Van Asbroeck
Simulate low-memory in lan743x_rx_trim_skb(): fail one allocation
in every 100.
Signed-off-by: Sven Van Asbroeck
---
To: Bryan Whitehead
To: unglinuxdri...@microchip.com
To: "David S. Miller"
To: Jakub Kicinski
Cc: Andrew Lunn
Cc: Alexey Denisov
Cc: Se
From: Sven Van Asbroeck
Simulate low-memory in lan743x_rx_allocate_skb(): fail 10
allocations in a row in every 100.
Signed-off-by: Sven Van Asbroeck
---
To: Bryan Whitehead
To: unglinuxdri...@microchip.com
To: "David S. Miller"
To: Jakub Kicinski
Cc: Andrew Lunn
Cc: Alexey D
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 actual
From: Sven Van Asbroeck
Signed-off-by: Sven Van Asbroeck
---
To: Bryan Whitehead
To: unglinuxdri...@microchip.com
To: "David S. Miller"
To: Jakub Kicinski
Cc: Andrew Lunn
Cc: Alexey Denisov
Cc: Sergej Bauer
Cc: Tim Harvey
Cc: Anders Rønningen
Cc: Hillf Danton
Cc: Christoph H
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 FRAME_L
From: Sven Van Asbroeck
The buffers in the lan743x driver's receive ring are always 9K,
even when the largest packet that can be received (the mtu) is
much smaller. This performs particularly badly on cpu archs
without dma cache snooping (such as ARM): each received packet
results in a 9K
Hi Bryan,
On Fri, Feb 12, 2021 at 3:45 PM wrote:
>
> According to the document I have, FRAME_LENGTH is only valid when LS bit is
> set, and reserved otherwise.
> Therefore, I'm not sure you can rely on it being zero when LS is not set,
> even if your experiments say it is.
> Future chip revisio
Hi Sergej, thank you for testing this !
On Thu, Feb 11, 2021 at 7:18 PM Sergej Bauer wrote:
>
> although whole set of tests might be an overly extensive, but after applying
> patch v2 [1/5]
> tests are:
I am unfamiliar with the test_ber tool. Does this patch improve things?
From: Sven Van Asbroeck
Simulate low-memory in lan743x_rx_trim_skb(): fail one allocation
in every 100.
Signed-off-by: Sven Van Asbroeck
---
To: Bryan Whitehead
To: unglinuxdri...@microchip.com
To: "David S. Miller"
To: Jakub Kicinski
Cc: Andrew Lunn
Cc: Alexey Denisov
Cc: Se
From: Sven Van Asbroeck
Signed-off-by: Sven Van Asbroeck
---
To: Bryan Whitehead
To: unglinuxdri...@microchip.com
To: "David S. Miller"
To: Jakub Kicinski
Cc: Andrew Lunn
Cc: Alexey Denisov
Cc: Sergej Bauer
Cc: Tim Harvey
Cc: Anders Rønningen
Cc: Hillf Danton
Cc: Christoph H
From: Sven Van Asbroeck
Simulate low-memory in lan743x_rx_allocate_skb(): fail 10
allocations in a row in every 100.
Signed-off-by: Sven Van Asbroeck
---
To: Bryan Whitehead
To: unglinuxdri...@microchip.com
To: "David S. Miller"
To: Jakub Kicinski
Cc: Andrew Lunn
Cc: Alexey D
From: Sven Van Asbroeck
Tree: git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git #
e4b62cf7559f
v1 -> v2:
- Andrew Lunn:
+ always keep to Reverse Christmas Tree.
+ "changing the cache operations to operate on the received length" should
go in its
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 actual
From: Sven Van Asbroeck
The buffers in the lan743x driver's receive ring are always 9K,
even when the largest packet that can be received (the mtu) is
much smaller. This performs particularly badly on cpu archs
without dma cache snooping (such as ARM): each received packet
results in a 9K
Hi Christoph,
On Fri, Feb 5, 2021 at 4:31 AM Christoph Hellwig wrote:
>
> This is a pattern we've seen in a few other net driver, so we should
> be ok. It just is rather hairy and needs a good justification, which
> seems to be given here.
Thank you so much for taking the time to look into this
Hi Sergej,
On Fri, Feb 5, 2021 at 10:09 AM Sergej Bauer wrote:
>
> Tests after applying patches [2/6] and [3/6] are:
> $ ifmtu eth7 500
> $ sudo test_ber -l eth7 -c 1000 -n 100 -f500 --no-conf
Thank you! Is there a way for me to run test_ber myself?
Is this a standard, or a bespoke testing t
Hi Sergej,
On Fri, Feb 5, 2021 at 7:44 AM Sergej Bauer wrote:
>
> Hi Sven
> I can confirm great stability improvement after your patch
> "lan743x: boost performance on cpu archs w/o dma cache snooping".
>
> Test machine is Intel Pentium G4560 3.50GHz
> lan743x with rejected virtual phy 'inside'
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.
Thank you Bryan. I will prepare a v2 early next week.
Would Microchip be able to donate some time to test v2? My own tests
are certainly not perfect. Various stress tests across architectures
(intel/arm) would help create confidence in the multi-buffer frame
implementation. Perhaps Microchip has v
On Sun, Jan 31, 2021 at 2:06 AM wrote:
>
> > 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
> mis
On Sat, Jan 30, 2021 at 5:11 PM wrote:
>
> It appears you moved this packet_length assignment from just below the
> following if block, however you left out the le32_to_cpu.See next comment
PS this merge snafu is removed completely by the next patch in the set.
So this will not prevent you from
Hi Bryan, thank you so much for reviewing, I really appreciate it.
On Sat, Jan 30, 2021 at 5:11 PM wrote:
>
> > /* unmap from dma */
> > + packet_length = RX_DESC_DATA0_FRAME_LENGTH_GET_
> > + (descriptor->data0);
On Fri, Jan 29, 2021 at 6:08 PM Willem de Bruijn
wrote:
>
> Okay. I found it a bit hard to parse how much true code change was
> mixed in with just reindenting existing code. If a lot, then no need
> to split of the code refactor.
Thank you. The code is quite hard to review in patch format.
Proba
Hoi Willem, thanks a lot for reviewing this patch, much appreciated !!
On Fri, Jan 29, 2021 at 5:11 PM Willem de Bruijn
wrote:
>
> > +static struct sk_buff *
> > +lan743x_rx_trim_skb(struct sk_buff *skb, int frame_length)
> > +{
> > + if (skb_linearize(skb)) {
>
> Is this needed? That will
Hi Andrew, thank you so much for looking at this patch !
On Fri, Jan 29, 2021 at 3:36 PM Andrew Lunn wrote:
>
> So this patch appears to contain two different changes
> 1) You only allocate a receive buffer as big as the MTU plus overheads
> 2) You change the cache operations to operate on the re
On Fri, Jan 29, 2021 at 5:01 PM Jakub Kicinski wrote:
>
> You may need to rebase to see this:
>
> drivers/net/ethernet/microchip/lan743x_main.c:2123:41: warning: restricted
> __le32 degrades to integer
Good catch. The problem goes away with the next commit in the set.
This is probably because I
From: Sven Van Asbroeck
The buffers in the lan743x driver's receive ring are always 9K,
even when the largest packet that can be received (the mtu) is
much smaller. This performs particularly badly on cpu archs
without dma cache snooping (such as ARM): each received packet
results in a 9K
From: Sven Van Asbroeck
Simulate low-memory in lan743x_rx_trim_skb(): fail one allocation
in every 100.
Signed-off-by: Sven Van Asbroeck
---
Tree: git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git #
46eb3c108fe1
To: Bryan Whitehead
To: unglinuxdri...@microchip.com
To: "
From: Sven Van Asbroeck
Signed-off-by: Sven Van Asbroeck
---
Tree: git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git #
46eb3c108fe1
To: Bryan Whitehead
To: unglinuxdri...@microchip.com
To: "David S. Miller"
To: Jakub Kicinski
Cc: Andrew Lunn
Cc: Alexey Denisov
From: Sven Van Asbroeck
Now that we can use rx ring buffers smaller than the mtu,
we allow users to change the mtu on the fly.
Tested as follows:
Tests with debug logging enabled (add #define DEBUG).
1. Set the chip mtu to 1500, generate lots of network traffic.
Stop all network traffic
From: Sven Van Asbroeck
Simulate low-memory in lan743x_rx_allocate_skb(): fail 10
allocations in a row in every 100.
Signed-off-by: Sven Van Asbroeck
---
Tree: git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git #
46eb3c108fe1
To: Bryan Whitehead
To: unglinuxdri
From: Sven Van Asbroeck
Multi-buffer packets enable us to use rx ring buffers smaller than
the mtu. This will allow us to change the mtu on-the-fly, without
having to stop the network interface in order to re-size the rx
ring buffers.
This is a big change touching a key driver function
From: Sven Van Asbroeck
The first patch of this series boosts the chip's rx performance by up to 3x
on cpus such as ARM. However it introduces a breaking change: the mtu
can no longer be changed while the network interface is up.
To get around this efficiently, the second patch adds d
On Wed, Dec 16, 2020 at 8:01 PM Florian Fainelli wrote:
>
> x86 is a fully cache and device coherent memory architecture and there
> are smarts like DDIO to bring freshly DMA'd data into the L3 cache
> directly. For ARMv7, it depends on the hardware you have, most ARMv7
> SoCs do not have hardware
Hi Andrew,
On Wed, Dec 9, 2020 at 9:10 AM Andrew Lunn wrote:
>
> 9K is not a nice number, since for each allocation it probably has to
> find 4 contiguous pages. See what the performance difference is with
> 2K, 4K and 8K. If there is a big difference, you might want to special
> case when the MT
Hi Jakub,
On Wed, Dec 16, 2020 at 12:03 PM Jakub Kicinski wrote:
>
> Applied, thanks Sven.
>
> I'll leave it out of our stable submission, and expect Sasha's
> autoselection bot to pick it up. This should give us more time
> for testing before the patch makes its way to stable trees.
> Let's see
From: Sven Van Asbroeck
Even if there is more rx data waiting on the chip, the rx napi poll fn
will never run more than once - it will always read a few buffers, then
bail out and re-arm interrupts. Which results in ping-pong between napi
and interrupt.
This defeats the purpose of napi, and is
Hi Jakub,
On Fri, Dec 11, 2020 at 9:38 AM Sven Van Asbroeck wrote:
>
> From: Sven Van Asbroeck
>
> Even if there is more rx data waiting on the chip, the rx napi poll fn
> will never run more than once - it will always read a few buffers, then
> bail out and re-arm interrupts
From: Sven Van Asbroeck
Even if there is more rx data waiting on the chip, the rx napi poll fn
will never run more than once - it will always read a few buffers, then
bail out and re-arm interrupts. Which results in ping-pong between napi
and interrupt.
This defeats the purpose of napi, and is
Hi Heiner,
On Thu, Dec 10, 2020 at 2:32 AM Heiner Kallweit wrote:
>
>
> In addition you could play with sysfs attributes
> /sys/class/net//gro_flush_timeout
> /sys/class/net//napi_defer_hard_irqs
Interesting, I will look into that.
> > @@ -2407,7 +2409,7 @@ static int lan743x_rx_open(struct lan
From: Sven Van Asbroeck
Even if there is more rx data waiting on the chip, the rx napi poll fn
will never run more than once - it will always read a few buffers, then
bail out and re-arm interrupts. Which results in ping-pong between napi
and interrupt.
This defeats the purpose of napi, and is
On Tue, Dec 8, 2020 at 6:36 PM Florian Fainelli wrote:
>
> dma_sync_single_for_{cpu,device} is what you would need in order to make
> a partial cache line invalidation. You would still need to unmap the
> same address+length pair that was used for the initial mapping otherwise
> the DMA-API debugg
On Tue, Dec 8, 2020 at 6:50 PM Eric Dumazet wrote:
>
> Driver could be called with an arbitrary budget (of 64),
> and if its ring buffer has been depleted, return @budget instead of skb
> counts,
> and not ream the interrupt
>
Aha, so the decision to re-arm the interrupts is made by looking
at w
Hi Andrew,
On Tue, Dec 8, 2020 at 5:51 PM Andrew Lunn wrote:
>
> >
> > So I assumed that it's a PCIe dma bandwidth issue, but I could be wrong -
> > I didn't do any PCIe bandwidth measurements.
>
> Sometimes it is actually cache operations which take all the
> time. This needs to invalidate the c
On Tue, Dec 8, 2020 at 2:50 PM Jakub Kicinski wrote:
>
> >
> > +done:
> > /* update RX_TAIL */
> > lan743x_csr_write(adapter, RX_TAIL(rx->channel_number),
> > rx_tail_flags | rx->last_tail);
> > -done:
> > +
>
> I assume this rings the doorbell to let the device
ts/sec0
> >
> > And with both sides set to MTU 9000 bytes:
> > Before:
> > [ ID] Interval Transfer Bandwidth Retr
> > [ 4] 0.00-20.00 sec 1.87 GBytes 803 Mbits/sec 27
> > After:
> > [ ID] Interval Transfer Ban
On Tue, Dec 8, 2020 at 11:52 AM Jakub Kicinski wrote:
>
> What I was talking about in the email yesterday was 0x8794 support
> in ksz8795.c. Is the cpu port configuration going to work there?
> Isn't the CPU port always port 5 (index 4)?
Understood. You expressed concern that .cpu_ports = 0x10 ev
Andrew, Jakub,
On Sat, Dec 5, 2020 at 10:28 AM Sven Van Asbroeck wrote:
>
> From: Sven Van Asbroeck
>
> On the ksz8795, if the devicetree contains a cpu node,
> devicetree parsing fails and the whole driver errors out.
>
> Fix the devicetree parsing code by making it use th
From: Sven Van Asbroeck
To support jumbo frames, each rx ring dma buffer is 9K in size.
But the chip only stores a single frame per dma buffer.
When the chip is working with the default 1500 byte MTU, a 9K
dma buffer goes from chip -> cpu per 1500 byte frame. This means
that to get 1
From: Sven Van Asbroeck
Even if the rx ring is completely full, and there is more rx data
waiting on the chip, the rx napi poll fn will never run more than
once - it will always immediately bail out and re-enable interrupts.
Which results in ping-pong between napi and interrupt.
This defeats
From: Sven Van Asbroeck
Port counts in microchip dsa drivers can be quite confusing:
on the ksz8795, ksz_chip_data->port_cnt excludes the cpu port,
yet on the ksz9477, it includes the cpu port.
Add comments to document this situation explicitly.
Fixes: 912aae27c6af ("net: dsa: m
From: Sven Van Asbroeck
On the ksz8795, if the devicetree contains a cpu node,
devicetree parsing fails and the whole driver errors out.
Fix the devicetree parsing code by making it use the
correct number of ports.
Fixes: 912aae27c6af ("net: dsa: microchip: really look for phy-mode in
Jakub, Andrew,
On Fri, Dec 4, 2020 at 6:24 PM Jakub Kicinski wrote:
>
> All the port counts here are -1 compared to datasheets, so I'm assuming
> the are not supposed to include the host facing port or something?
>
> Can you describe the exact problem you're trying to solve?
>
The ksz8795 driver
From: Sven Van Asbroeck
The ksz8795 has five physical ports, but the driver assumes
it has only four. This prevents the driver from working correctly.
Fix by indicating the correct number of physical ports.
Fixes: e66f840c08a23 ("net: dsa: ksz: Add Microchip KSZ8795 DSA driver")
Hi Jakub, Sergej,
On Tue, Nov 3, 2020 at 8:41 PM Jakub Kicinski wrote:
>
> On Mon, 2 Nov 2020 01:35:55 +0300 Sergej Bauer wrote:
> > This is the 3rd revision of the patch fix for potential null pointer
> > dereference
> > with lan743x card.
>
> Applied, thanks!
I noticed that this went into ne
On Tue, Nov 24, 2020 at 7:17 PM Jakub Kicinski wrote:
>
> Applied both, thank you!
Thank you Jakub !
From: Sven Van Asbroeck
The driver's ISR sends a 'software interrupt' event to the probe()
thread using the following method:
- probe(): write 0 to flag, enable s/w interrupt
- probe(): poll on flag, relax using usleep_range()
- ISR: write 1 to flag
Repla
From: Sven Van Asbroeck
For no apparent reason, this function reads the INT_STS register, and
checks if the software interrupt bit is set. These things have already
been carried out by this function's only caller.
Clean up by removing the redundant code.
Tested-by: Sven Van Asbroeck # la
On Tue, Nov 17, 2020 at 1:47 PM Jakub Kicinski wrote:
>
> On Tue, 17 Nov 2020 03:09:56 +0100 Andrew Lunn wrote:
> > Reviewed-by: Andrew Lunn
> Applied, thanks!
Thank you Andrew and Jakub !
From: Sven Van Asbroeck
The code in this driver which parses the devicetree to determine
the phy/fixed link setup, can be replaced by a single library
function: of_phy_get_and_connect().
Behaviour is identical, except that the library function will
complain when 'phy-connection-type'
On Sat, Nov 14, 2020 at 6:19 PM Jakub Kicinski wrote:
>
> The _irq() cases look a little strange, are you planning a refactor in
> net-next?
I'd like to, but I don't understand skbs/queues well enough (yet).
From: Sven Van Asbroeck
On arm imx6, when opening the chip's netdev, the whole Linux
kernel intermittently hangs/freezes.
This is caused by a bug in the driver code which tests if pcie
interrupts are working correctly, using the software interrupt:
1. open: enable the software interr
From: Sven Van Asbroeck
When running this chip on arm imx6, we intermittently observe
the following kernel warning in the log, especially when the
system is under high load:
[ 50.119484] [ cut here ]
[ 50.124377] WARNING: CPU: 0 PID: 303 at kernel/softirq.c:169
From: Sven Van Asbroeck
When no devicetree is present, the driver will use an
uninitialized variable.
Fix by initializing this variable.
Fixes: 902a66e08cea ("lan743x: correctly handle chips with internal PHY")
Reported-by: kernel test robot
Signed-off-by: Sven Van Asbroeck
A spi core fix has been accepted which makes this patch unnecessary.
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git/commit/?id=766c6b63aa044e84b045803b40b14754d69a2a1d
On Tue, Nov 10, 2020 at 9:20 AM Sven Van Asbroeck wrote:
>
>
> This driver makes sure the underlyin
On Tue, Nov 10, 2020 at 8:55 PM Jakub Kicinski wrote:
>
> Please remember about fixes tags
Will do in the future- I posted this patch before we had this
conversation about the need for Fixes tags in net.
>
> Applied, thanks!
Thank you !
PING Jakub
On Tue, Nov 10, 2020 at 11:30 AM Andy Shevchenko
wrote:
>
> I see that this is a fix for backporing, but maybe you can send a
> patches on top of this to:
> 1) introduce
> #define SPI_MODE_MASK (SPI_CPHA | SPI_CPOL)
>spi->mode &= ~SPI_MODE_MASK;
> > + spi->mode |= SPI
Hi Andy, thank you for the feedback.
On Tue, Nov 10, 2020 at 11:30 AM Andy Shevchenko
wrote:
>
> I see that this is a fix for backporing, but maybe you can send a
> patches on top of this to:
> 1) introduce
> #define SPI_MODE_MASK (SPI_CPHA | SPI_CPOL)
>spi->mode &= ~SPI_MODE_MASK;
>
From: Sven Van Asbroeck
This driver makes sure the underlying SPI bus is set to "mode 0"
by assigning SPI_MODE_0 to spi->mode. Which overwrites all other
SPI mode flags.
In some circumstances, this can break the underlying SPI bus driver.
For example, if SPI_CS_HIGH is set on the
On Mon, Nov 9, 2020 at 5:50 PM Jakub Kicinski wrote:
>
> But please at least repost for net and CC Mark and the SPI list
> for input.
>
> Maybe Mark has a different idea on how client drivers should behave.
>
> Also please obviously CC the author of the patch who introduced
> the breakage.
I
On Mon, Nov 9, 2020 at 5:36 PM Jakub Kicinski wrote:
>
> Yes, most certainly. Especially with 5.10 being LTS.
>
> You can send a minimal fix (perhaps what you got already?), and follow
> up with the helper as suggested by Andrew after ~a week when net is
> merged into net-next.
>
What I already p
On Mon, Nov 9, 2020 at 5:23 PM Jakub Kicinski wrote:
>
> Is it only broken for you in linux-next or just in the current 5.10
> release?
It's broken for me in the current 5.10 release. That means it should
go to net, not net-next, correct?
On Mon, Nov 9, 2020 at 5:04 PM Jakub Kicinski wrote:
>
> Yup
Just a minute. Earlier in the thread, Andrew Lunn is suggesting I
create a new spi helper function, and cross-post to the spi group(s).
That doesn't sound like a minimal fix, should it go to net or net-next?
Thanks,
Sven
On Mon, Nov 9, 2020 at 4:24 PM Jakub Kicinski wrote:
>
> the commit
> which introduced the assignment in the client driver.
That's the commit which adds the initial driver to the tree, back in 2011.
Should I use that for Fixes?
On Mon, Nov 9, 2020 at 4:09 PM Jakub Kicinski wrote:
>
> This is a fix right? You seem to be targeting net-next and there is no
> Fixes tag but it sounds like a bug.
I'm not sure. The original code used to work for me, until the spi bus
driver I'm using to communicate to this chip was changed to
From: Sven Van Asbroeck
In the net core, the struct net_device_ops -> ndo_set_rx_mode()
callback is called with the dev->addr_list_lock spinlock held.
However, this driver's ndo_set_rx_mode callback eventually calls
lan743x_dp_write(), which acquires a mutex. Mutex acquisition
may
On Mon, Nov 9, 2020 at 3:26 PM Andrew Lunn wrote:
>
> Then you should consider adding it, and cross post the SPI list.
>
Good idea. I will give that a try.
On Mon, Nov 9, 2020 at 2:49 PM Andrew Lunn wrote:
>
> Did you check to see if there is a help to set just the mode without
> changing any of the other bits?
Absolutely, but it doesn't exist, AFAIK.
It would be great if client spi drivers would use a helper function like
that. The spi bus driver
From: Sven Van Asbroeck
This driver makes sure the underlying SPI bus is set to "mode 0"
by assigning SPI_MODE_0 to spi->mode. This overwrites all other
SPI mode flags.
In some circumstances, this can break the underlying SPI bus driver.
For example, if SPI_CS_HIGH is set on the
Hi Roelof,
On Sun, Nov 8, 2020 at 2:57 PM Roelof Berg wrote:
>
> Well, it’s not an easy 4 hours attempt between breakfast and lunch,
> unfortunately, but it’s based on inexpensive off-the-shelf parts and doable
> in an experienced team.
>
I would love to test this patch on fixed-link, but unfo
From: Sven Van Asbroeck
Commit 6f197fb63850 ("lan743x: Added fixed link and RGMII support")
assumes that chips with an internal PHY will never have a devicetree
entry. This is incorrect: even for these chips, a devicetree entry
can be useful e.g. to pass the mac address from bootload
From: Sven Van Asbroeck
Commit 6f197fb63850 ("lan743x: Added fixed link and RGMII support")
assumes that chips with an internal PHY will never have a devicetree
entry. This is incorrect: even for these chips, a devicetree entry
can be useful e.g. to pass the mac address from bootload
Hi Roelof, great to meet another user of the lan743x !
On Sun, Nov 8, 2020 at 2:43 AM Roelof Berg wrote:
>
> My last customer has a fixed link setup, contact me if you need one. Maybe
> they let me have it and I can test all future patches on fixed link as well.
Is this a commercially available
On Sat, Nov 7, 2020 at 11:14 PM Andrew Lunn wrote:
>
> This also has nothing to do with the problem you are fixing. It is a
> sensible thing to do, but it should be a separate patch, and target
> net-next, since it is not a fix.
>
Agreed - I will spin a truly minimal v3.
From: Sven Van Asbroeck
Commit 6f197fb63850 ("lan743x: Added fixed link and RGMII support")
assumes that chips with an internal PHY will never have a devicetree
entry. This is incorrect: even for these chips, a devicetree entry
can be useful e.g. to pass the mac address from bootload
Hi Jakub,
On Wed, Nov 4, 2020 at 4:58 PM Jakub Kicinski wrote:
>
> Not a big deal but if you have to change the patch could you make sure
> your email address is spelled the same in the From line and other tags?
Absolutely, thanks for letting me know about those case differences.
1 - 100 of 126 matches
Mail list logo
