Re: Linux mvneta driver unable to read MAC address from HW
Hello Ezra, On Sat, 10 Oct 2020 18:41:24 +0200 Ezra Buehler wrote: > I am running Debian buster (Linux 4.19.146) on a Synology DS214+ NAS > (Marvell ARMADA XP). Unfortunately, I end up with random MAC addresses > for the two Ethernet interfaces after boot. (Also with Debian sid, Linux > 5.4.0.) > > Since commit 8cc3e439ab92 ("net: mvneta: read MAC address from hardware > when available") the mvneta Linux driver reads the MVNETA_MAC_ADDR_* > registers when no MAC address is provided by the DT. In my case, only > zeros are read, causing the driver to fall back to a random address. I > was able to verify that the registers are correctly written by the > bootloader by reading out the registers in the U-Boot prompt. > > As a workaround, I now specify the MAC addresses in the DT. However, I > would prefer not to do that. Also, it would be nice to get to the bottom > of this. > > Could it be, that for some reason, the clock of the MAC is removed > either by U-Boot or Linux during boot? I suspect you have the mvneta driver as a module ? If this is the case, then indeed, the MAC address is lost because Linux turns of all unused clocks at the end of the boot. When the driver is built-in, there is a driver adding a reference to the clock before all unused clocks are disabled. When the driver is compiled as a module, this does not happen. So indeed, the correct solution here is to have U-Boot pass the MAC address in the Device Tree. Best regards, Thomas Petazzoni -- Thomas Petazzoni, CTO, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Re: [EXT] Re: [PATCH net-next 3/5] net: mvpp2: cls: Use RSS contexts to handle RSS tables
On Sat, 9 May 2020 13:48:43 +0100 Russell King - ARM Linux admin wrote: > > Unfortunately, we are no longer actively working on Marvell platform > > support at the moment. We might have a look on a best effort basis, but > > this is potentially a non-trivial issue, so I'm not sure when we will > > have the chance to investigate and fix this. > > That may be the case, but that doesn't excuse the fact that we have a > regression and we need to do something. Absolutely. > Please can you suggest how we resolve this regression prior to > 5.7-final? Since 5.7 is really close, I would suggest to disable the functionality. Best regards, Thomas -- Thomas Petazzoni, CTO, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Re: [EXT] Re: [PATCH net-next 3/5] net: mvpp2: cls: Use RSS contexts to handle RSS tables
Hello, On Sat, 9 May 2020 12:45:18 +0100 Russell King - ARM Linux admin wrote: > Looking at the timeline here, it looks like Matteo raised the issue > very quickly after the patch was sent on the 14th April, and despite > following up on it, despite me following up on it, bootlin have > remained quiet. Unfortunately, we are no longer actively working on Marvell platform support at the moment. We might have a look on a best effort basis, but this is potentially a non-trivial issue, so I'm not sure when we will have the chance to investigate and fix this. Best regards, Thomas -- Thomas Petazzoni, CTO, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Re: network unreliable on ReadyNAS 104 with Debian kernel
Hello Uwe, +Maxime Chevallier and Antoine Ténart, who have also worked on mvneta. On Sat, 2 May 2020 16:14:08 +0200 Uwe Kleine-König wrote: > I own a ReadyNAS 104 (CPU: Armada 370, mvneta driver) and since some > time its network driver isn't reliable any more. I see things like: > > $ rsync -a remotehost:dir /srv/dir > ssh_dispatch_run_fatal: Connection to $remoteaddress port 22: message > authentication code incorrect > rsync: connection unexpectedly closed (11350078 bytes received so far) > [receiver] > rsync error: error in rsync protocol data stream (code 12) at io.c(235) > [receiver=3.1.3] > rsync: connection unexpectedly closed (13675 bytes received so far) > [generator] > rsync error: unexplained error (code 255) at io.c(235) [generator=3.1.3] > > when ever something like this happens, I get > > mvneta d0074000.ethernet eth1: bad rx status 0e8b (overrun error), > size=680 I am also running an Armada 370 ReadyNAS, though with a much older kernel (4.4.x). It is working fine for me, but checking the kernel logs, I in fact also have the same issue: [4141806.620510] mvneta d007.ethernet eth0: bad rx status 0f83 (overrun error), size=1344 [4141821.344100] mvneta d007.ethernet eth0: bad rx status 0f83 (overrun error), size=272 [4141831.098003] mvneta d007.ethernet eth0: bad rx status 0f83 (overrun error), size=896 [4141850.655858] mvneta d007.ethernet eth0: bad rx status 0f83 (overrun error), size=592 [4141850.915259] mvneta d007.ethernet eth0: bad rx status 0d83 (overrun error), size=16 > This happens with Debian's 5.4.0-4-armmp (Version: 5.4.19-1) kernel, but > I also experienced it with the 4.19 series. On slow connections this > isn't a problem so the problem might exist already longer. In fact I > think there are two problems: The first is that the hardware doesn't get > enough buffers in time for the receive path and the other is that in the > error case corrupted packets are given to the upper layers. > > Does this ring a bell for you? I didn't start to debug that yet. I think I do remember seeing reports about this, but I don't remember if it ended up being fixed (and what we're seeing is some other problem), or if it's still the same issue. It's been a long time I looked into mvneta, unfortunately. Best regards, Thomas -- Thomas Petazzoni, CTO, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Re: Handling an Extra Signal at PHY Reset
On Tue, 19 Feb 2019 14:36:29 +0100 Andrew Lunn wrote: > This seems like an odd design. I've normally seen weak pull up/down > resistors, not a switch, so i'm wondering why it is designed like > this. The key point here is that this "CONFIG" pin of the PHY is used during reset to configure the PHY, but then once the reset sequence is finished, this pin is used for PTP. From the datasheet, section 2.28.1 "PTP Control": """ To support the PTP Time Stamping function, the device has four pins that are global to the entire PHY: - PTP clock input pin (The CONFIG pin is used for this purpose.) - PTP Event Request input pin (The LED[1] pin is used for this purpose) - PTP Event Request input pin (The LED[1] pin is used for this purpose) - Interrupt Pin (The LED[2] pin is used for this purpose) """ A bit further down in the datasheet: "After configuration is completed and the external clock source is enabled, the CONFIG pin is used as the external 125 Mhz reference clock input" So that's why our design as a switch: it allows the CONFIG pin to be used for configuration during the reset sequence, and then as the pin for the PTP clock input. Does that clarify why the CONFIG pin is not simply connected to some static pull-up/pull-down ? Best regards, Thomas -- Thomas Petazzoni, CTO, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Re: Handling an Extra Signal at PHY Reset
Hello Paul, On Tue, 19 Feb 2019 10:14:20 +0100 Paul Kocialkowski wrote: > We are dealing with an Ethernet PHY (Marvell 88E1512) that comes with a > CONFIG pin that must be connected to one of the other pins of the PHY > to configure the LSB of the PHY address as well as I/O voltages (see > section 2.18.1 Hardware Configuration of the datasheet). It must be > connected "soon after reset" for the PHY to be correctly configured. > > We have a switch for connecting the CONFIG pin to the other pin (LED0), > which needs to be controlled by Linux. The CONFIG pin seems to be used > for a PTP clock the rest of the time. > > So we are wondering how to properly represent this case, especially on > the device-tree side. > > The trick here is that this step is necessary before the PHY can be > discovered on the MDIO bus (and thus the PHY driver selected) so we > can't rely on the PHY driver to do this. Basically, it looks like we > need to handle this like the reset pin and describe it at the MDIO bus > level. > > Here are some ideas for potential solutions: > - Allowing more than a single GPIO to be passed to the MDIO bus' reset- > gpios via device-tree and toggling all the passed GPIOs at once; > > - Adding a new optional GPIO for the MDIO bus dedicated to controlling > switches for such config switching, perhaps called "config-gpios" > (quite a narrow solution); > > - Adding a broader power sequence description to the MDIO bus (a bit > like it's done with the mmc pwrseq descriptions) which would allow > specifying the toggle order/delays of various GPIOs (would probably be > the most extensive solution); > > - Adding the extra GPIO control to the MAC description and toggling it > through bus->reset (probably the less invasive solution for the core > but not very satisfying from the description perspective, since this is > definitely not MAC-specific). > > What do you think about how we could solve this issue? > Do you see other options that I missed here? I think it's important to mention the sequence that is needed: 1. assert reset 2. wait 10 us 3. switch config signal 4. deassert reset 5. wait 100us 6. de-switch config signal I.e, the config signal needs to be switched properly before deasserting reset, and then switched back to its original state so that the config pin can be used for its normal (non-reset) purpose. So the manipulation of the config signal is intertwined with the assert/de-assert of the reset. So I don't see how your fourth option would work for example. Am I missing something here ? Best regards, Thomas -- Thomas Petazzoni, CTO, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Re: [PATCH] net: phy: mdio_bus: add missing device_del() in mdiobus_register() error handling
Hello Andrew, On Wed, 16 Jan 2019 16:44:39 +0100 Andrew Lunn wrote: > > On Wed, 16 Jan 2019 15:48:29 +0100, Andrew Lunn wrote: > > > > > Reviewed-by: Andrew Lunn > > > > > > However, i wounder if it makes sense to add a label before the > > > existing device_del() at the end of the function, and convert this, > > > and the case above into a goto? That might scale better, avoiding the > > > same issue in the future? > > > > That's another option indeed. > > > > Hmm, now that I looked at it, I think we should use device_unregister() > > instead. device_unregister() does both device_del() and put_device(). > > Hi Thomas > > device_unregister() does seem symmetrical with device_register() which > is what we are trying to undo. Even if DaveM already merged my simple fix, I had a further look at whether we should be using device_unregister(), and in fact we should not, but not really for a good reason: because the mdio API is not very symmetrical. The typical flow is: probe() { bus = mdiobus_alloc(); if (!bus) return -ENOMEM; ret = mdiobus_register(&bus); if (ret) { mdiobus_free(bus); ... } remove() { mdiobus_unregister(); mdiobus_free(); } mdiobus_alloc() only does memory allocation, i.e it has no side effects on the device model data structures. mdiobus_register() does a device_register(). If it fails, it only cleans up with a device_del(), i.e it doesn't do the put_device() that it should do to fully "undo" its effect. mdiobus_unregister() does a device_del(), i.e it also doesn't do the opposite of mdiobus_register(), which should be device_del() + put_device() (device_unregister() is a shortcut for both). mdiobus_free() does the put_device() So: * mdiobus_alloc() / mdiobus_free() are not symmetrical in terms of their interaction with the device model data structures * On error, mdiobus_register() leaves a non-zero reference count to the bus->dev structure, which will be freed up by mdiobus_free() * mdiobus_unregister() leaves a non-zero reference count to the bus->dev structure, which will be freed up by mdiobus_free() So, if we were to use device_unregister() in the error path of mdiobus_register() and in mdiobus_unregister(), it would break how mdiobus_free() works. Best regards, Thomas -- Thomas Petazzoni, CTO, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Re: [PATCH net-next v2 0/6] Add comphy support for Armada 38x
Hello David, On Thu, 07 Feb 2019 18:10:49 -0800 (PST) David Miller wrote: > From: Russell King - ARM Linux admin > Date: Thu, 7 Feb 2019 16:18:25 + > > > This series adds support for the comphy for Armada 38x, which allows > > these SoCs to use 2500BASE-X mode with appropriate SFP modules. > > > > Tested on SolidRun Clearfog after updating for the 5.0 merge window > > changes. > > Series applied, thanks Russell. This series contained: - Device Tree bindings that had not been ACKed by the DT bindings maintainers, one of which should have been merged through the drivers/phy maintainer tree. - A brand new drivers/phy driver that had not been ACKed by the drivers/phy maintainer. - Changes to platform Device Tree that should have been merged through the platform tree. Only patches 4/6 and 5/6 should go through the net-next tree, all the other patches should have gone through other trees. Best regards, Thomas -- Thomas Petazzoni, CTO, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Re: [PATCH] net: phy: mdio_bus: add missing device_del() in mdiobus_register() error handling
Hello, On Wed, 16 Jan 2019 15:48:29 +0100, Andrew Lunn wrote: > Reviewed-by: Andrew Lunn > > However, i wounder if it makes sense to add a label before the > existing device_del() at the end of the function, and convert this, > and the case above into a goto? That might scale better, avoiding the > same issue in the future? That's another option indeed. Hmm, now that I looked at it, I think we should use device_unregister() instead. device_unregister() does both device_del() and put_device(). According to the comment above device_register(), put_device() should always be called: "Always use put_device() to give up the reference initialized in this function instead.". What do you think? Thomas -- Thomas Petazzoni, CTO, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
[PATCH] net: phy: mdio_bus: add missing device_del() in mdiobus_register() error handling
The current code in __mdiobus_register() doesn't properly handle failures returned by the devm_gpiod_get_optional() call: it returns immediately, without unregistering the device that was added by the call to device_register() earlier in the function. This leaves a stale device, which then causes a NULL pointer dereference in the code that handles deferred probing: [1.489982] Unable to handle kernel NULL pointer dereference at virtual address 0074 [1.498110] pgd = (ptrval) [1.500838] [0074] *pgd= [1.504432] Internal error: Oops: 17 [#1] SMP ARM [1.509133] Modules linked in: [1.512192] CPU: 1 PID: 51 Comm: kworker/1:3 Not tainted 4.20.0-00039-g3b73a4cc8b3e-dirty #99 [1.520708] Hardware name: Xilinx Zynq Platform [1.525261] Workqueue: events deferred_probe_work_func [1.530403] PC is at klist_next+0x10/0xfc [1.534403] LR is at device_for_each_child+0x40/0x94 [1.539361] pc : []lr : []psr: 200e0013 [1.545628] sp : ceeefe68 ip : 0001 fp : e000 [1.550863] r10: r9 : c0c66790 r8 : [1.556079] r7 : c0457d44 r6 : r5 : ceeefe8c r4 : cfa2ec78 [1.562604] r3 : 0064 r2 : c0457d44 r1 : ceeefe8c r0 : 0064 [1.569129] Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none [1.576263] Control: 18c5387d Table: 0ed7804a DAC: 0051 [1.582013] Process kworker/1:3 (pid: 51, stack limit = 0x(ptrval)) [1.588280] Stack: (0xceeefe68 to 0xceef) [1.592630] fe60: cfa2ec78 c0c03c08 c0457d44 c0c66790 [1.600814] fe80: c0455d90 ceeefeac 0064 0d7a542e cee9d494 cfa2ec78 [1.608998] fea0: cfa2ec78 c0457d44 c0457d7c cee9d494 c0c03c08 c0455dac [1.617182] fec0: cf98ba44 cf926a00 cee9d494 0d7a542e cf935a10 cf935a10 cf935a10 [1.625366] fee0: c0c4e9b8 c0457d7c c0c4e80c 0001 cf935a10 c0457df4 cf935a10 c0c4e99c [1.633550] ff00: c0c4e99c c045a27c c0c4e9c4 ced63f80 cfde8a80 cfdebc00 c013893c [1.641734] ff20: cfde8a80 cfde8a80 c07bd354 ced63f80 ced63f94 cfde8a80 0008 c0c02d00 [1.649936] ff40: cfde8a98 cfde8a80 e000 c0139a30 e000 c0c6624a c07bd354 [1.658120] ff60: e000 cee9e780 ceebfe00 c000 ced63f80 c0139788 cf8cdea4 [1.666304] ff80: cee9e79c c013e598 0001 ceebfe00 c013e44c [1.674488] ffa0: c01010e8 [1.682671] ffc0: [1.690855] ffe0: 0013 [1.699058] [] (klist_next) from [] (device_for_each_child+0x40/0x94) [1.707241] [] (device_for_each_child) from [] (device_reorder_to_tail+0x38/0x88) [1.716476] [] (device_reorder_to_tail) from [] (device_for_each_child+0x5c/0x94) [1.725692] [] (device_for_each_child) from [] (device_reorder_to_tail+0x38/0x88) [1.734927] [] (device_reorder_to_tail) from [] (device_pm_move_to_tail+0x28/0x40) [1.744235] [] (device_pm_move_to_tail) from [] (deferred_probe_work_func+0x58/0x8c) [1.753746] [] (deferred_probe_work_func) from [] (process_one_work+0x210/0x4fc) [1.762888] [] (process_one_work) from [] (worker_thread+0x2a8/0x5c0) [1.771072] [] (worker_thread) from [] (kthread+0x14c/0x154) [1.778482] [] (kthread) from [] (ret_from_fork+0x14/0x2c) [1.785689] Exception stack(0xceeeffb0 to 0xceeefff8) [1.790739] ffa0: [1.798923] ffc0: [1.807107] ffe0: 0013 [1.813724] Code: e92d47f0 e1a05000 e8900048 e1a3 (e5937010) [1.819844] ---[ end trace 3c2c0c8b65399ec9 ]--- The actual error that we had from devm_gpiod_get_optional() was -EPROBE_DEFER, due to the GPIO being provided by a driver that is probed later than the Ethernet controller driver. To fix this, we simply add the missing device_del() invocation in the error path. Fixes: 69226896ad636 ("mdio_bus: Issue GPIO RESET to PHYs") Signed-off-by: Thomas Petazzoni --- drivers/net/phy/mdio_bus.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c index 2e59a8419b17..66b9cfe692fc 100644 --- a/drivers/net/phy/mdio_bus.c +++ b/drivers/net/phy/mdio_bus.c @@ -390,6 +390,7 @@ int __mdiobus_register(struct mii_bus *bus, struct module *owner) if (IS_ERR(gpiod)) { dev_err(&bus->dev, "mii_bus %s couldn't get reset GPIO\n", bus->id); + device_del(&bus->dev); return PTR_ERR(gpiod); } else if (gpiod) { bus->reset_gpiod = gpiod; -- 2.20.1
Re: [PATCH net] net: mvneta: fix operation for 64K PAGE_SIZE
Hello Marcin, On Mon, 17 Dec 2018 00:25:58 +0100, Marcin Wojtas wrote: > Thanks. Indeed, the patch is valid as a fix for current version of SW > BM. However, because this concept is broken, I will rework it and > submit patch/patches some time early 2019. I know some people are working on XDP support in mvneta, and this work also needs to change parts of the memory allocation strategy in this driver. I'd suggest to get in touch with those folks. Antoine can give you the contact details, I don't have them off-hand. Or perhaps they will see this e-mail :-) Best regards, Thomas -- Thomas Petazzoni, CTO, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Re: [BUG] MVPP2 driver exploding in presence of a tap interface
Hello, On Tue, 30 Oct 2018 14:55:01 +, Marc Zyngier wrote: > > I.e, isn't the firmware fix papering over a bug that should be fixed in > > Linux mvpp2 driver anyway ? > > Absolutely. Leaving this unpatched in the kernel, with a 100% chance of > memory corruption is just mad. > > I'm pretty sure there should be a way to sanely reset the interface > before it starts repainting the memory. I agree here. Do you still have an image of that old firmware version, so that we can try to reproduce, and see if we can come up with a way to reset the BM on boot up that would avoid this issue ? Thanks, Thomas -- Thomas Petazzoni, CTO, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Re: [BUG] MVPP2 driver exploding in presence of a tap interface
Hello Marcin, Thanks for the feedback. On Tue, 30 Oct 2018 13:37:37 +0100, Marcin Wojtas wrote: > You use _really_ archaic firmware, the bug you see is 99% caused by a > bug already fixed long time ago (cleanup all PP2 BM pools correctly > during exit boot services). Please grab the latest release: > https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/wiki/files/flash-image-18.09.4.bin > and let know if you observe any further issues with vanilla kernel. Even if this was a bug in the UEFI firmware, shouldn't the kernel be independent from that, by doing a proper reset/reinit of the HW ? I.e, isn't the firmware fix papering over a bug that should be fixed in Linux mvpp2 driver anyway ? Best regards, Thomas -- Thomas Petazzoni, CTO, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Re: [PATCH net-next v2 02/13] net: phy: sfp: handle non-wired SFP connectors
Hello, On Fri, 4 May 2018 19:23:37 +0200, Antoine Tenart wrote: > On Fri, May 04, 2018 at 10:04:48AM -0700, Florian Fainelli wrote: > > On 05/04/2018 06:56 AM, Antoine Tenart wrote: > > > SFP connectors can be solder on a board without having any of their pins > > > (LOS, i2c...) wired. In such cases the SFP link state cannot be guessed, > > > and the overall link status reporting is left to other layers. > > > > > > In order to achieve this, a new SFP_DEV status is added, named UNKNOWN. > > > This mode is set when it is not possible for the SFP code to get the > > > link status and as a result the link status is reported to be always UP > > > from the SFP point of view. > > > > Why represent the SFP in Device Tree then? Why not just declare this is > > a fixed link which would avoid having to introduce this "unknown" state. > > The other solution would have been to represent this as a fixed-link. > But such a representation would report the link as being up all the > time, which is something we wanted to avoid as the GoP in PPv2 can > report some link status. This is achieved using SFP+phylink+PPv2. > > And representing the SFP cage in the device tree, although it's a > "dummy" one, helps describing the hardware. Just to add to this: the board physically has a SFP cage, and a cable can be connected to it, or not. So it is absolutely not a fixed link (cable can be connected or not) and it really is a SFP cage. Best regards, Thomas -- Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com
Re: [PATCH] net: mvneta: fix enable of all initialized RXQs
Hello, On Fri, 30 Mar 2018 12:05:31 +0200, Gregory CLEMENT wrote: > From: Yelena Krivosheev > > In mvneta_port_up() we enable relevant RX and TX port queues by write > queues bit map to an appropriate register. > > q_map must be ZERO in the beginning of this process. > > Signed-off-by: Yelena Krivosheev > Signed-off-by: Gregory CLEMENT Acked-by: Thomas Petazzoni Thanks, Thomas -- Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com
Re: [PATCH 2/2] net: mvneta: improve suspend/resume
Hello, On Fri, 30 Mar 2018 17:15:47 +0800, Jisheng Zhang wrote: > > > + cpuhp_state_remove_instance_nocalls(online_hpstate, > > > + &pp->node_online); > > > + cpuhp_state_remove_instance_nocalls(CPUHP_NET_MVNETA_DEAD, > > > + &pp->node_dead); > > > > Do we need to remove/add those CPU notifiers when suspending/resuming ? > > Take mvneta_cpu_online() as an example, if we don't remove it during > suspend, when system is resume back, it will touch mac when secondary > cpu is ON, but at this point the mvneta isn't resumed, this is not safe. Hm. I'm still a bit confused by this new CPU hotplug API. I understand the issue you have and indeed unregistering the CPU hotplug callbacks is a way to solve the problem, but I find it weird that we have to do this. Anyway, it's OK to do it, because it's anyway what was done so far. It is just annoying that there is a duplication of the logic between mvneta_suspend() and mvneta_stop() on one side, and duplication between mvneta_resume() and mvnete_start() on the other side. > > > + for (queue = 0; queue < rxq_number; queue++) { > > > + struct mvneta_rx_queue *rxq = &pp->rxqs[queue]; > > > + > > > + mvneta_rxq_drop_pkts(pp, rxq); > > > + } > > > > Wouldn't it make sense to have > > mvneta_rxq_sw_deinit/mvneta_rxq_hw_deinit(), like you did for the > > initialization ? > > For rxq deinit, we'd like to drop rx pkts, this is both HW and SW operation. > So we reuse mvneta_rxq_drop_pkts() here. Hum, OK, indeed. It would have been nicer to have something symmetric, with the hw/sw parts split in a similar way for the init and deinit of both txqs and rxqs, but I agree that dropping the RX packets before going into suspend involves both HW and SW operations. Thanks! Thomas Petazzoni -- Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com
Re: [PATCH 2/2] net: mvneta: improve suspend/resume
gt;rxqs[queue]; > + > + rxq->next_desc_to_proc = 0; > + mvneta_rxq_hw_init(pp, rxq); > } > - rtnl_unlock(); > + > + for (queue = 0; queue < txq_number; queue++) { > + struct mvneta_tx_queue *txq = &pp->txqs[queue]; > + > + txq->next_desc_to_proc = 0; > + mvneta_txq_hw_init(pp, txq); > + } > + > + if (!pp->neta_armada3700) { > + spin_lock(&pp->lock); > + pp->is_stopped = false; > + spin_unlock(&pp->lock); > + cpuhp_state_add_instance_nocalls(online_hpstate, > + &pp->node_online); > + cpuhp_state_add_instance_nocalls(CPUHP_NET_MVNETA_DEAD, > + &pp->node_dead); > + } > + > + mvneta_set_rx_mode(dev); > + mvneta_start_dev(pp); Thanks! Thomas -- Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com
Re: [PATCH 1/2] net: mvneta: split rxq/txq init into SW and HW parts
Hello, On Thu, 29 Mar 2018 18:13:56 +0800, Jisheng Zhang wrote: > This is to prepare the suspend/resume improvement in next patch. The > SW parts can be optimized out during resume. > > Signed-off-by: Jisheng Zhang Thanks, I have two very minor nits below, but otherwise: Acked-by: Thomas Petazzoni > +/* Create a specified RX queue */ > +static int mvneta_rxq_init(struct mvneta_port *pp, > +struct mvneta_rx_queue *rxq) > + > +{ > + int ret; > + > + ret = mvneta_rxq_sw_init(pp, rxq); > + if (ret) Here you're testing if (ret), while in mvneta_txq_init(), in the same situation, you're doing if (ret < 0). I don't have a preference for one or the other, but having them consistent between the two lpaces would be nice. > -/* Create and initialize a tx queue */ > -static int mvneta_txq_init(struct mvneta_port *pp, > -struct mvneta_tx_queue *txq) > +static int mvneta_txq_sw_init(struct mvneta_port *pp, > + struct mvneta_tx_queue *txq) > { > int cpu; > > @@ -2872,7 +2889,6 @@ static int mvneta_txq_init(struct mvneta_port *pp, > txq->tx_stop_threshold = txq->size - MVNETA_MAX_SKB_DESCS; > txq->tx_wake_threshold = txq->tx_stop_threshold / 2; > > - Spurious change. Thanks! Thomas Petazzoni -- Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com
Re: [PATCH net-next 1/5] net: mvpp2: use the same buffer pool for all ports
Hello, On Mon, 5 Mar 2018 11:48:13 +0100, Antoine Tenart wrote: > > > +static void mvpp2_setup_bm_pool(void) > > > +{ > > > + /* Short pool */ > > > + mvpp2_pools[MVPP2_BM_SHORT].buf_num = MVPP2_BM_SHORT_BUF_NUM; > > > + mvpp2_pools[MVPP2_BM_SHORT].pkt_size = MVPP2_BM_SHORT_PKT_SIZE; > > > + > > > + /* Long pool */ > > > + mvpp2_pools[MVPP2_BM_LONG].buf_num = MVPP2_BM_LONG_BUF_NUM; > > > + mvpp2_pools[MVPP2_BM_LONG].pkt_size = MVPP2_BM_LONG_PKT_SIZE; > > > +} > > > > ? > > I wanted to do this, but it's no possible as MVPP2_BM_SHORT_PKT_SIZE and > MVPP2_BM_LONG_PKT_SIZE use a core definition which expands at some point > to __max(...) which has to be called from within a function. Hum, weird: #define MVPP2_BM_LONG_PKT_SIZE MVPP2_RX_MAX_PKT_SIZE(MVPP2_BM_LONG_FRAME_SIZE) #define MVPP2_BM_LONG_FRAME_SIZE2048 #define MVPP2_RX_MAX_PKT_SIZE(total_size) \ ((total_size) - NET_SKB_PAD - MVPP2_SKB_SHINFO_SIZE) #define MVPP2_SKB_SHINFO_SIZE \ SKB_DATA_ALIGN(sizeof(struct skb_shared_info)) #define SKB_DATA_ALIGN(X)ALIGN(X, SMP_CACHE_BYTES) I don't really see a __max(...) call. And if this value really expands depending on other values, then it isn't really a constant, and should be considered as a constant, no? Thomas -- Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering http://bootlin.com
Re: [PATCH net-next 5/5] net: mvpp2: jumbo frames support
Hello, On Sun, 4 Mar 2018 06:56:02 +, Stefan Chulski wrote: > > > + if (port->pool_long->id == MVPP2_BM_JUMBO && port->id != 0) { > > > > Again, all over the place we hardcode the fact that Jumbo frames can only be > > used on port 0. I know port 0 is the only one that can do 10G, but are there > > possibly some use cases where you may want Jumbo frame on another port > > ? > > > > This all really feels very hardcoded to me. > > > > All ports support Jumbo frames. > But only port 0 can do TX HW checksum offload(due to TX FIFO size). > > Packet processor 2.2 has only 19KB TX FIFO size. > So in TX FIFO config code assign for Port 0 - 10KB, Port 1 - 3KB and Port 1 - > 3KB. Yes, but I was also questioning whether hardcoding this configuration was correct. > To perform checksum in HW, HW obviously should work in store and forward > mode. Store all frame in TX FIFO and then check checksum. > If mtu 1500B, everything fine and all port can do this. > > If mtu is 9KB and 9KB frame transmitted, Port 0 still can do HW checksum. But > ports 1 and 2 doesn't has enough FIFO for this. > So we cannot offload this feature and SW should perform checksum. So perhaps the real check should not be "port 0", but whether the MTU is higher or lower than the TX FIFO size assigned to the current port. This would express in much better way the reason why HW checksum can be used or not. > > > + /* 9704 == 9728 - 20 and rounding to 8 */ > > > + dev->max_mtu = MVPP2_BM_JUMBO_PKT_SIZE; > > > > Is this correct for all ports ? Shouldn't the maximum MTU be different > > between port 0 (that supports Jumbo frames) and the other ports ? > > This is correct for all ports. All ports can support Jumbo frames. OK. With your explanation above, I understand better. Best regards, Thomas -- Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering http://bootlin.com
Re: [PATCH net-next 3/5] net: mvpp2: use a data size of 10kB for Tx FIFO on port 0
Hello, On Sun, 4 Mar 2018 06:29:59 +, Stefan Chulski wrote: > > Is there a reason to hardcode 10KB for port 0, and 3KB for the other ports ? > > Would there be use cases where the user may want different configurations > > ? > > Design requirement are 10KB TX FIFO for the 10Gb/sec and 2.5KB for the > 2.5Gb/sec. What is a "design requirement" ? Is it a HW design limitation ? > Since only port 0 support 10Gb/sec and ports 1&2 support up to 2.5Gb/sec. > I don't see any reason to change this configurations. > Also TX FIFO size could be set only during probe. > > > It's just that it feels very "hardcoded" to enforce specifically those > > numbers. > > > > Also, does it make sense to mention the CP110 here ? Is this 19 KB > > limitation > > a limit of the PPv2.2 IP, or of the CP110 ? > > PPv2.2 IP is part of 110 communication processor. Thanks, I know this :-) > Next communication processor will has different Packet processor or next > generation of PPv2.x > Limit is PPv2.2 TX FIFO. So, the limitation has nothing to do with CP110 really, it's just a limitation of PPv2.2, and mentioning CP110 in the comment doesn't make much sense, correct ? Best regards, Thomas -- Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering http://bootlin.com
Re: [PATCH net-next 3/5] net: mvpp2: use a data size of 10kB for Tx FIFO on port 0
Hello, On Fri, 2 Mar 2018 16:40:42 +0100, Antoine Tenart wrote: > -/* Initialize Tx FIFO's */ > +/* Initialize Tx FIFO's > + * The CP110's total tx-fifo size is 19kB. > + * Use large-size 10kB for fast port but 3kB for others. > + */ Is there a reason to hardcode 10KB for port 0, and 3KB for the other ports ? Would there be use cases where the user may want different configurations ? It's just that it feels very "hardcoded" to enforce specifically those numbers. Also, does it make sense to mention the CP110 here ? Is this 19 KB limitation a limit of the PPv2.2 IP, or of the CP110 ? Thomas -- Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering http://bootlin.com
Re: [PATCH net-next 5/5] net: mvpp2: jumbo frames support
Hello, On Fri, 2 Mar 2018 16:40:44 +0100, Antoine Tenart wrote: > /* Attach long pool to rxq */ > @@ -4551,7 +4559,7 @@ mvpp2_bm_pool_use(struct mvpp2_port *port, int pool, > int pkt_size) > struct mvpp2_bm_pool *new_pool = &port->priv->bm_pools[pool]; > int num; > > - if (pool < MVPP2_BM_SHORT || pool > MVPP2_BM_LONG) { > + if (pool < MVPP2_BM_SHORT || pool > MVPP2_BM_JUMBO) { pool could be an unsigned, which would avoid the need for MVPP2_BM_SHORT. And for the upper limit, you have a convenient MVPP2_BM_POOLS_NUM in your mvpp2_bm_pool_log_num enum, so why not use if ? > netdev_err(port->dev, "Invalid pool %d\n", pool); > return NULL; > } > @@ -4596,11 +4604,24 @@ mvpp2_bm_pool_use(struct mvpp2_port *port, int pool, > int pkt_size) > static int mvpp2_swf_bm_pool_init(struct mvpp2_port *port) > { > int rxq; > + enum mvpp2_bm_pool_log_num long_log_pool, short_log_pool; > + > + /* If port pkt_size is higher than 1518B: > + * HW Long pool - SW Jumbo pool, HW Short pool - SW Short pool The comment is wrong. In this case, the HW short pool is the SW long pool. > + * else: HW Long pool - SW Long pool, HW Short pool - SW Short pool > + */ > + if (port->pkt_size > MVPP2_BM_LONG_PKT_SIZE) { > + long_log_pool = MVPP2_BM_JUMBO; > + short_log_pool = MVPP2_BM_LONG; See here. > + } else { > + long_log_pool = MVPP2_BM_LONG; > + short_log_pool = MVPP2_BM_SHORT; > + } > + /* If port MTU is higher than 1518B: > + * HW Long pool - SW Jumbo pool, HW Short pool - SW Short pool And the comment is wrong here as well :) > + * else: HW Long pool - SW Long pool, HW Short pool - SW Short pool > + */ > + if (pkt_size > MVPP2_BM_LONG_PKT_SIZE) > + new_long_pool = MVPP2_BM_JUMBO; > + else > + new_long_pool = MVPP2_BM_LONG; > + > + if (new_long_pool != port->pool_long->id) { > + /* Remove port from old short & long pool */ > + port->pool_long = mvpp2_bm_pool_use(port, port->pool_long->id, > + port->pool_long->pkt_size); > + port->pool_long->port_map &= ~(1 << port->id); BIT(port->id) ? > + port->pool_long = NULL; > + > + port->pool_short = mvpp2_bm_pool_use(port, port->pool_short->id, > + > port->pool_short->pkt_size); > + port->pool_short->port_map &= ~(1 << port->id); Ditto. > + if (port->pool_long->id == MVPP2_BM_JUMBO && port->id != 0) { Again, all over the place we hardcode the fact that Jumbo frames can only be used on port 0. I know port 0 is the only one that can do 10G, but are there possibly some use cases where you may want Jumbo frame on another port ? This all really feels very hardcoded to me. > + dev->features &= ~(NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM); > + dev->hw_features &= ~(NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM); > + } > + > dev->vlan_features |= features; > dev->gso_max_segs = MVPP2_MAX_TSO_SEGS; > > - /* MTU range: 68 - 9676 */ > + /* MTU range: 68 - 9704 */ > dev->min_mtu = ETH_MIN_MTU; > - /* 9676 == 9700 - 20 and rounding to 8 */ > - dev->max_mtu = 9676; How come we already had a max_mtu of 9676 without Jumbo frame support ? > + /* 9704 == 9728 - 20 and rounding to 8 */ > + dev->max_mtu = MVPP2_BM_JUMBO_PKT_SIZE; Is this correct for all ports ? Shouldn't the maximum MTU be different between port 0 (that supports Jumbo frames) and the other ports ? Thanks! Thomas -- Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering http://bootlin.com
Re: [PATCH net-next 1/5] net: mvpp2: use the same buffer pool for all ports
Hello, On Fri, 2 Mar 2018 16:40:40 +0100, Antoine Tenart wrote: > +static struct { > + int pkt_size; > + int buf_num; > +} mvpp2_pools[MVPP2_BM_POOLS_NUM]; Any reason for not doing: } mvpp2_pools[MVPP2_BM_POOLS_NUM] = { [MVPP2_BM_SHORT] = { .pkt_size = MVPP2_BM_SHORT_PKT_SIZE, .buf_num = MVPP2_BM_SHORT_BUF_NUM }, [MVPP2_BM_LONG] = { .pkt_size = MVPP2_BM_LONG_PKT_SIZE, .buf_num = MVPP2_BM_LONG_BUF_NUM, }, }; And get rid of: > +static void mvpp2_setup_bm_pool(void) > +{ > + /* Short pool */ > + mvpp2_pools[MVPP2_BM_SHORT].buf_num = MVPP2_BM_SHORT_BUF_NUM; > + mvpp2_pools[MVPP2_BM_SHORT].pkt_size = MVPP2_BM_SHORT_PKT_SIZE; > + > + /* Long pool */ > + mvpp2_pools[MVPP2_BM_LONG].buf_num = MVPP2_BM_LONG_BUF_NUM; > + mvpp2_pools[MVPP2_BM_LONG].pkt_size = MVPP2_BM_LONG_PKT_SIZE; > +} ? Best regards, Thomas -- Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering http://bootlin.com
Re: [PATCH net-next v4 0/4] net: mvpp2: 1000BaseX and 2500BaseX support
Hello, On Thu, 11 Jan 2018 11:32:03 -0500 (EST), David Miller wrote: > From: David Miller > Date: Thu, 11 Jan 2018 11:23:37 -0500 (EST) > > > From: Antoine Tenart > > Date: Wed, 10 Jan 2018 16:58:04 +0100 > > > >> This series adds 1000BaseX and 2500BaseX support to the Marvell PPv2 > >> driver. In order to use it, the 2.5 SGMII mode is added in the Marvell > >> common PHY driver (cp110-comphy). > > > > Series applied, thank you. > > Actually, this introduced build warnings, I'm reverting. Please fix this > and repost. > > Thank you. > > drivers/net/ethernet/marvell/mvpp2.c: In function > ‘mvpp2_port_mii_gmac_configure_mode’: > drivers/net/ethernet/marvell/mvpp2.c:4687:26: warning: suggest parentheses > around comparison in operand of ‘|’ [-Wparentheses] > if (port->phy_interface == PHY_INTERFACE_MODE_1000BASEX | > ^~~~~~~ This is actually a very good warning: it should be a || and not |. Best regards, Thomas -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
Re: [PATCH 1/2] net: sh_eth: add support for SH7786
Hello, On Tue, 5 Dec 2017 22:49:10 +0300, Sergei Shtylyov wrote: > >>>> This commit adds the sh_eth_cpu_data structure that describes the > >>>> SH7786 variant of the IP. > >>> > >>> The manual seems to be unavailable, so I have to trust you. :-) > >> > >> Yes, sadly. However, if you tell me what to double check, I'd be happy > >> to do so. > > > > I have the manual now, will check against it... > > DaveM, I'm retracting my ACK for the time being. > > Starting to look into the manual, the current patch is wrong. SH7786 SoC > was probably the 1st one to use what we thought was R-Car specific register > layout. Definite NAK on this version. Thanks for the feedback. How do we proceed from there ? I don't have access to a lot of datasheets of the different Renesas SoCs, so it's not easy to figure out which IP variant the SH7786 is using compared to other Renesas SoCs. Just out of curiosity, which specific aspect makes you think the proposed patch is wrong ? Have you noticed a specific register or field that isn't compatible with SH_ETH_REG_FAST_SH4 layout ? Note that my patch makes Ethernet work in practice on SH7784, I have root over NFS working as we speak. This certainly doesn't mean that the patch is entirely correct, but it definitely means that the SH_ETH_REG_FAST_SH4 is close enough to what the SH7786 is using :-) Thanks! Thomas -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
Re: [PATCH v2] net: sh_eth: do not advertise Gigabit capabilities when not available
Hello, On Tue, 5 Dec 2017 22:02:20 +0300, Sergei Shtylyov wrote: > > + /* mask with MAC supported features */ > > + if (mdp->cd->register_type != SH_ETH_REG_GIGABIT) { > > + err = phy_set_max_speed(phydev, SPEED_100); > > + if (err) { > > + netdev_err(ndev, "failed to limit PHY to 100 Mbit/s\n"); > > + goto err_phy_disconnect; > > Er, why do we need a *goto* here at all? Just call phy_disconnect() here > and be done with that... Thanks for the feedback, I've sent a v3 that takes into account this comment. Thanks! Thomas -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
[PATCH v3] net: sh_eth: do not advertise Gigabit capabilities when not available
Not all variants of the sh_eth hardware have Gigabit support. Unfortunately, the current driver doesn't tell the PHY about the limited MAC capabilities. Due to this, if you have a Gigabit capable PHY, the PHY will advertise its Gigabit capability and establish a link at 1Gbit/s, even though the MAC doesn't support it. In order to avoid this, we use the recently introduced phy_set_max_speed() to tell the PHY to not advertise speed higher than 100 MBit/s. Tested on a SH7786 platform, with a Gigabit PHY. Signed-off-by: Thomas Petazzoni --- Changes since v2: - Drop goto construction used in the phy_set_max_speed() error handling, as it is not needed. Suggested by Sergei Shtylyov. Changes since v1: - Use phy_set_max_speed(), as suggested by Sergei Shtylyov . --- drivers/net/ethernet/renesas/sh_eth.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c index db72d13cebb9..75323000c364 100644 --- a/drivers/net/ethernet/renesas/sh_eth.c +++ b/drivers/net/ethernet/renesas/sh_eth.c @@ -1892,6 +1892,16 @@ static int sh_eth_phy_init(struct net_device *ndev) return PTR_ERR(phydev); } + /* mask with MAC supported features */ + if (mdp->cd->register_type != SH_ETH_REG_GIGABIT) { + int err = phy_set_max_speed(phydev, SPEED_100); + if (err) { + netdev_err(ndev, "failed to limit PHY to 100 Mbit/s\n"); + phy_disconnect(phydev); + return err; + } + } + phy_attached_info(phydev); return 0; -- 2.14.3
Re: [PATCH 1/2] net: sh_eth: use correct "struct device" when calling DMA mapping functions
Hello, On Tue, 5 Dec 2017 09:39:35 +0100, Geert Uytterhoeven wrote: > >Using 'ndev->dev.parent' (as in ravb) also should work... not sure which > > is better > > That was going to be my comment, too. I also haven't checked which > generates the smallest code. Using ndev->dev.parent actually generates bigger code (44 bytes larger) : $ size drivers/net/ethernet/renesas/sh_eth.o.* textdata bss dec hex filename 27803 696 0 284996f53 drivers/net/ethernet/renesas/sh_eth.o.new 27759 696 0 284556f27 drivers/net/ethernet/renesas/sh_eth.o.orig .orig is my original proposal, .new is with ndev->dev.parent. I'm using a gcc 6.4.0 compiler. Note also that the driver is already using mdp->pdev->dev all over the place: $ grep -- mdp-\>pdev-\>dev drivers/net/ethernet/renesas/sh_eth.c pm_wakeup_event(&mdp->pdev->dev, 0); pm_runtime_get_sync(&mdp->pdev->dev); pm_runtime_put_sync(&mdp->pdev->dev); device_set_wakeup_enable(&mdp->pdev->dev, mdp->wol_enabled); pm_runtime_get_sync(&mdp->pdev->dev); pm_runtime_put_sync(&mdp->pdev->dev); pm_runtime_put_sync(&mdp->pdev->dev); struct device *dev = &mdp->pdev->dev; Best regards, Thomas Petazzoni -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
[PATCH v2] net: sh_eth: do not advertise Gigabit capabilities when not available
Not all variants of the sh_eth hardware have Gigabit support. Unfortunately, the current driver doesn't tell the PHY about the limited MAC capabilities. Due to this, if you have a Gigabit capable PHY, the PHY will advertise its Gigabit capability and establish a link at 1Gbit/s, even though the MAC doesn't support it. In order to avoid this, we use the recently introduced phy_set_max_speed() to tell the PHY to not advertise speed higher than 100 MBit/s. Tested on a SH7786 platform, with a Gigabit PHY. Signed-off-by: Thomas Petazzoni --- Changes since v1: - Use phy_set_max_speed(), as suggested by Sergei Shtylyov . --- drivers/net/ethernet/renesas/sh_eth.c | 14 ++ 1 file changed, 14 insertions(+) diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c index db72d13cebb9..44ff2835c954 100644 --- a/drivers/net/ethernet/renesas/sh_eth.c +++ b/drivers/net/ethernet/renesas/sh_eth.c @@ -1860,6 +1860,7 @@ static int sh_eth_phy_init(struct net_device *ndev) struct device_node *np = ndev->dev.parent->of_node; struct sh_eth_private *mdp = netdev_priv(ndev); struct phy_device *phydev; + int err; mdp->link = 0; mdp->speed = 0; @@ -1892,9 +1893,22 @@ static int sh_eth_phy_init(struct net_device *ndev) return PTR_ERR(phydev); } + /* mask with MAC supported features */ + if (mdp->cd->register_type != SH_ETH_REG_GIGABIT) { + err = phy_set_max_speed(phydev, SPEED_100); + if (err) { + netdev_err(ndev, "failed to limit PHY to 100 Mbit/s\n"); + goto err_phy_disconnect; + } + } + phy_attached_info(phydev); return 0; + +err_phy_disconnect: + phy_disconnect(phydev); + return err; } /* PHY control start function */ -- 2.13.6
Re: [PATCH 1/2] net: sh_eth: add support for SH7786
Hello, On Mon, 4 Dec 2017 19:56:35 +0300, Sergei Shtylyov wrote: > On 12/04/2017 05:17 PM, Thomas Petazzoni wrote: > > > This commit adds the sh_eth_cpu_data structure that describes the > > SH7786 variant of the IP. > > The manual seems to be unavailable, so I have to trust you. :-) Yes, sadly. However, if you tell me what to double check, I'd be happy to do so. Thanks! Thomas -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
[PATCH 0/2] net: sh_eth: add support for SH7786 and big-endian
Hello, I've recently been working on an SH7786 based platform, which uses the sh_eth network controller. One peculiarity of my setup is that the CPU is configured big-endian (even though little-endian is more traditional in the Linux SuperH world), and the sh_eth driver was not ready for this. The first patch simply adds the sh_eth_cpu_data structure that describes the SH7786 controller. The second patch fixes the driver for big-endian operation. However, I'd like this patch to be carefully reviewed by Sergei Shtylyov who already did some endianness related changes in this driver. Indeed, my change is based on the assumption that the DMA descriptors are in the native endianness of the CPU. Thanks, Thomas Thomas Petazzoni (2): net: sh_eth: add support for SH7786 net: sh_eth: make work on big endian systems drivers/net/ethernet/renesas/sh_eth.c | 89 ++- 1 file changed, 55 insertions(+), 34 deletions(-) -- 2.13.6
[PATCH 1/2] net: sh_eth: add support for SH7786
This commit adds the sh_eth_cpu_data structure that describes the SH7786 variant of the IP. Signed-off-by: Thomas Petazzoni --- drivers/net/ethernet/renesas/sh_eth.c | 25 + 1 file changed, 25 insertions(+) diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c index 0074c5998481..a3c48b2a713c 100644 --- a/drivers/net/ethernet/renesas/sh_eth.c +++ b/drivers/net/ethernet/renesas/sh_eth.c @@ -710,6 +710,30 @@ static struct sh_eth_cpu_data sh7724_data = { .rpadir_value = 0x0002, /* NET_IP_ALIGN assumed to be 2 */ }; +static struct sh_eth_cpu_data sh7786_data = { + .set_duplex = sh_eth_set_duplex, + + .register_type = SH_ETH_REG_FAST_SH4, + + .ecsr_value = ECSR_PSRTO | ECSR_LCHNG | ECSR_ICD, + .ecsipr_value = ECSIPR_PSRTOIP | ECSIPR_LCHNGIP | ECSIPR_ICDIP, + .eesipr_value = EESIPR_RFCOFIP | EESIPR_ADEIP | EESIPR_ECIIP | + EESIPR_FTCIP | EESIPR_TDEIP | EESIPR_TFUFIP | + EESIPR_FRIP | EESIPR_RDEIP | EESIPR_RFOFIP | + EESIPR_RMAFIP | EESIPR_RRFIP | + EESIPR_RTLFIP | EESIPR_RTSFIP | + EESIPR_PREIP | EESIPR_CERFIP, + + .tx_check = EESR_FTC | EESR_CND | EESR_DLC | EESR_CD | EESR_RTO, + .eesr_err_check = EESR_TWB | EESR_TABT | EESR_RABT | EESR_RFE | + EESR_RDE | EESR_RFRMER | EESR_TFE | EESR_TDE, + + .apr= 1, + .mpr= 1, + .tpauser= 1, + .hw_swap= 1, +}; + static void sh_eth_set_rate_sh7757(struct net_device *ndev) { struct sh_eth_private *mdp = netdev_priv(ndev); @@ -3418,6 +3442,7 @@ static const struct platform_device_id sh_eth_id_table[] = { { "sh7757-ether", (kernel_ulong_t)&sh7757_data }, { "sh7757-gether", (kernel_ulong_t)&sh7757_data_giga }, { "sh7763-gether", (kernel_ulong_t)&sh7763_data }, + { "sh7786-ether", (kernel_ulong_t)&sh7786_data }, { } }; MODULE_DEVICE_TABLE(platform, sh_eth_id_table); -- 2.13.6
[PATCH 2/2] net: sh_eth: make work on big endian systems
The sh_eth driver uses cpu_to_le32() and le32_to_cpu() to manipulate the fields of the DMA descriptors, making the assumption that the DMA descriptors are little-endian. However, testing on the Renesas SH7786 running in big-endian mode reveals that the DMA descriptors are also big-endian when running big-endian. Therefore, all the endianness conversion needs to be removed for the sh_eth driver to work. Signed-off-by: Thomas Petazzoni --- Note: I see that Sergei Shtylyov has done some work around endianness on this driver back in 2015. I am not sure if it's used on other non-SuperH platforms in platforms where the CPU might run big-endian but the IP still be little-endian. If this is the case, then we would need a different approach and more discussion. Therefore, it would be useful to wait for an ACK from Sergei before applying this patch. --- drivers/net/ethernet/renesas/sh_eth.c | 64 --- 1 file changed, 30 insertions(+), 34 deletions(-) diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c index a3c48b2a713c..0de8fae143ab 100644 --- a/drivers/net/ethernet/renesas/sh_eth.c +++ b/drivers/net/ethernet/renesas/sh_eth.c @@ -1163,31 +1163,29 @@ static int sh_eth_tx_free(struct net_device *ndev, bool sent_only) for (; mdp->cur_tx - mdp->dirty_tx > 0; mdp->dirty_tx++) { entry = mdp->dirty_tx % mdp->num_tx_ring; txdesc = &mdp->tx_ring[entry]; - sent = !(txdesc->status & cpu_to_le32(TD_TACT)); + sent = !(txdesc->status & TD_TACT); if (sent_only && !sent) break; /* TACT bit must be checked before all the following reads */ dma_rmb(); netif_info(mdp, tx_done, ndev, "tx entry %d status 0x%08x\n", - entry, le32_to_cpu(txdesc->status)); + entry, txdesc->status); /* Free the original skb. */ if (mdp->tx_skbuff[entry]) { - dma_unmap_single(&mdp->pdev->dev, -le32_to_cpu(txdesc->addr), -le32_to_cpu(txdesc->len) >> 16, -DMA_TO_DEVICE); + dma_unmap_single(&mdp->pdev->dev, txdesc->addr, +txdesc->len >> 16, DMA_TO_DEVICE); dev_kfree_skb_irq(mdp->tx_skbuff[entry]); mdp->tx_skbuff[entry] = NULL; free_num++; } - txdesc->status = cpu_to_le32(TD_TFP); + txdesc->status = TD_TFP; if (entry >= mdp->num_tx_ring - 1) - txdesc->status |= cpu_to_le32(TD_TDLE); + txdesc->status |= TD_TDLE; if (sent) { ndev->stats.tx_packets++; - ndev->stats.tx_bytes += le32_to_cpu(txdesc->len) >> 16; + ndev->stats.tx_bytes += txdesc->len >> 16; } } return free_num; @@ -1204,8 +1202,7 @@ static void sh_eth_ring_free(struct net_device *ndev) if (mdp->rx_skbuff[i]) { struct sh_eth_rxdesc *rxdesc = &mdp->rx_ring[i]; - dma_unmap_single(&mdp->pdev->dev, -le32_to_cpu(rxdesc->addr), + dma_unmap_single(&mdp->pdev->dev, rxdesc->addr, ALIGN(mdp->rx_buf_sz, 32), DMA_FROM_DEVICE); } @@ -1280,9 +1277,9 @@ static void sh_eth_ring_format(struct net_device *ndev) /* RX descriptor */ rxdesc = &mdp->rx_ring[i]; - rxdesc->len = cpu_to_le32(buf_len << 16); - rxdesc->addr = cpu_to_le32(dma_addr); - rxdesc->status = cpu_to_le32(RD_RACT | RD_RFP); + rxdesc->len = buf_len << 16; + rxdesc->addr = dma_addr; + rxdesc->status = RD_RACT | RD_RFP; /* Rx descriptor address set */ if (i == 0) { @@ -1297,7 +1294,7 @@ static void sh_eth_ring_format(struct net_device *ndev) /* Mark the last entry as wrapping the ring. */ if (rxdesc) - rxdesc->status |= cpu_to_le32(RD_RDLE); + rxdesc->status |= RD_RDLE; memset(mdp->tx_ring, 0, tx_ringsize); @@ -1305,8 +1302,8 @@ static void sh_eth_ring_format(struct net_device *ndev) for (
[PATCH] net: sh_eth: do not advertise Gigabit capabilities when not available
Not all variants of the sh_eth hardware have Gigabit support. Unfortunately, the current driver doesn't update phydev->supported depending on the MAC capabilities. Due to this, if you have a Gigabit capable PHY, the PHY will advertise its Gigabit capability and establish a link at 1Gbit/s, even though the MAC doesn't support it. In order to avoid this, we mark phydev->supported if we're running a non-Gigabit capable hardware. Tested on a SH7786 platform, with a Gigabit PHY. Signed-off-by: Thomas Petazzoni --- drivers/net/ethernet/renesas/sh_eth.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c index db72d13cebb9..0074c5998481 100644 --- a/drivers/net/ethernet/renesas/sh_eth.c +++ b/drivers/net/ethernet/renesas/sh_eth.c @@ -1892,6 +1892,11 @@ static int sh_eth_phy_init(struct net_device *ndev) return PTR_ERR(phydev); } + /* mask with MAC supported features */ + if (mdp->cd->register_type != SH_ETH_REG_GIGABIT) + phydev->supported &= PHY_BASIC_FEATURES; + phydev->advertising = phydev->supported; + phy_attached_info(phydev); return 0; -- 2.13.6
[PATCH 2/2] net: sh_eth: don't use NULL as "struct device" for the DMA mapping API
Using NULL as argument for the DMA mapping API is bogus, as the DMA mapping API may use information from the "struct device" to perform the DMA mapping operation. Therefore, pass the appropriate "struct device". Signed-off-by: Thomas Petazzoni --- drivers/net/ethernet/renesas/sh_eth.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c index 91e918e654fe..db72d13cebb9 100644 --- a/drivers/net/ethernet/renesas/sh_eth.c +++ b/drivers/net/ethernet/renesas/sh_eth.c @@ -1187,7 +1187,7 @@ static void sh_eth_ring_free(struct net_device *ndev) } } ringsize = sizeof(struct sh_eth_rxdesc) * mdp->num_rx_ring; - dma_free_coherent(NULL, ringsize, mdp->rx_ring, + dma_free_coherent(&mdp->pdev->dev, ringsize, mdp->rx_ring, mdp->rx_desc_dma); mdp->rx_ring = NULL; } @@ -1204,7 +1204,7 @@ static void sh_eth_ring_free(struct net_device *ndev) sh_eth_tx_free(ndev, false); ringsize = sizeof(struct sh_eth_txdesc) * mdp->num_tx_ring; - dma_free_coherent(NULL, ringsize, mdp->tx_ring, + dma_free_coherent(&mdp->pdev->dev, ringsize, mdp->tx_ring, mdp->tx_desc_dma); mdp->tx_ring = NULL; } @@ -1324,8 +1324,8 @@ static int sh_eth_ring_init(struct net_device *ndev) /* Allocate all Rx descriptors. */ rx_ringsize = sizeof(struct sh_eth_rxdesc) * mdp->num_rx_ring; - mdp->rx_ring = dma_alloc_coherent(NULL, rx_ringsize, &mdp->rx_desc_dma, - GFP_KERNEL); + mdp->rx_ring = dma_alloc_coherent(&mdp->pdev->dev, rx_ringsize, + &mdp->rx_desc_dma, GFP_KERNEL); if (!mdp->rx_ring) goto ring_free; @@ -1333,8 +1333,8 @@ static int sh_eth_ring_init(struct net_device *ndev) /* Allocate all Tx descriptors. */ tx_ringsize = sizeof(struct sh_eth_txdesc) * mdp->num_tx_ring; - mdp->tx_ring = dma_alloc_coherent(NULL, tx_ringsize, &mdp->tx_desc_dma, - GFP_KERNEL); + mdp->tx_ring = dma_alloc_coherent(&mdp->pdev->dev, tx_ringsize, + &mdp->tx_desc_dma, GFP_KERNEL); if (!mdp->tx_ring) goto ring_free; return 0; -- 2.13.6
[PATCH 0/2] net: sh_eth: DMA mapping API fixes
Hello, Here are two patches that fix how the sh_eth driver is using the DMA mapping API: a bogus struct device is used in some places, or a NULL struct device is used. Best regards, Thomas Thomas Petazzoni (2): net: sh_eth: use correct "struct device" when calling DMA mapping functions net: sh_eth: don't use NULL as "struct device" for the DMA mapping API drivers/net/ethernet/renesas/sh_eth.c | 31 --- 1 file changed, 16 insertions(+), 15 deletions(-) -- 2.13.6
[PATCH 1/2] net: sh_eth: use correct "struct device" when calling DMA mapping functions
There are two types of "struct device": the one representing the physical device on its physical bus (platform, SPI, PCI, etc.), and the one representing the logical device in its device class (net, etc.). The DMA mapping API expects to receive as argument a "struct device" representing the physical device, as the "struct device" contains information about the bus that the DMA API needs. However, the sh_eth driver mistakenly uses the "struct device" representing the logical device (embedded in "struct net_device") rather than the "struct device" representing the physical device on its bus. This commit fixes that by adjusting all calls to the DMA mapping API. Signed-off-by: Thomas Petazzoni --- drivers/net/ethernet/renesas/sh_eth.c | 19 ++- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c index 7e060aa9fbed..91e918e654fe 100644 --- a/drivers/net/ethernet/renesas/sh_eth.c +++ b/drivers/net/ethernet/renesas/sh_eth.c @@ -1149,7 +1149,8 @@ static int sh_eth_tx_free(struct net_device *ndev, bool sent_only) entry, le32_to_cpu(txdesc->status)); /* Free the original skb. */ if (mdp->tx_skbuff[entry]) { - dma_unmap_single(&ndev->dev, le32_to_cpu(txdesc->addr), + dma_unmap_single(&mdp->pdev->dev, +le32_to_cpu(txdesc->addr), le32_to_cpu(txdesc->len) >> 16, DMA_TO_DEVICE); dev_kfree_skb_irq(mdp->tx_skbuff[entry]); @@ -1179,7 +1180,7 @@ static void sh_eth_ring_free(struct net_device *ndev) if (mdp->rx_skbuff[i]) { struct sh_eth_rxdesc *rxdesc = &mdp->rx_ring[i]; - dma_unmap_single(&ndev->dev, + dma_unmap_single(&mdp->pdev->dev, le32_to_cpu(rxdesc->addr), ALIGN(mdp->rx_buf_sz, 32), DMA_FROM_DEVICE); @@ -1245,9 +1246,9 @@ static void sh_eth_ring_format(struct net_device *ndev) /* The size of the buffer is a multiple of 32 bytes. */ buf_len = ALIGN(mdp->rx_buf_sz, 32); - dma_addr = dma_map_single(&ndev->dev, skb->data, buf_len, + dma_addr = dma_map_single(&mdp->pdev->dev, skb->data, buf_len, DMA_FROM_DEVICE); - if (dma_mapping_error(&ndev->dev, dma_addr)) { + if (dma_mapping_error(&mdp->pdev->dev, dma_addr)) { kfree_skb(skb); break; } @@ -1527,7 +1528,7 @@ static int sh_eth_rx(struct net_device *ndev, u32 intr_status, int *quota) mdp->rx_skbuff[entry] = NULL; if (mdp->cd->rpadir) skb_reserve(skb, NET_IP_ALIGN); - dma_unmap_single(&ndev->dev, dma_addr, + dma_unmap_single(&mdp->pdev->dev, dma_addr, ALIGN(mdp->rx_buf_sz, 32), DMA_FROM_DEVICE); skb_put(skb, pkt_len); @@ -1555,9 +1556,9 @@ static int sh_eth_rx(struct net_device *ndev, u32 intr_status, int *quota) if (skb == NULL) break; /* Better luck next round. */ sh_eth_set_receive_align(skb); - dma_addr = dma_map_single(&ndev->dev, skb->data, + dma_addr = dma_map_single(&mdp->pdev->dev, skb->data, buf_len, DMA_FROM_DEVICE); - if (dma_mapping_error(&ndev->dev, dma_addr)) { + if (dma_mapping_error(&mdp->pdev->dev, dma_addr)) { kfree_skb(skb); break; } @@ -2441,9 +2442,9 @@ static int sh_eth_start_xmit(struct sk_buff *skb, struct net_device *ndev) /* soft swap. */ if (!mdp->cd->hw_swap) sh_eth_soft_swap(PTR_ALIGN(skb->data, 4), skb->len + 2); - dma_addr = dma_map_single(&ndev->dev, skb->data, skb->len, + dma_addr = dma_map_single(&mdp->pdev->dev, skb->data, skb->len, DMA_TO_DEVICE); - if (dma_mapping_error(&ndev->dev, dma_addr)) { + if (dma_mapping_error(&mdp->pdev->dev, dma_addr)) { kfree_skb(skb); return NETDEV_TX_OK; } -- 2.13.6
Re: [BUG] mveta: mvneta_txq_bufs_free NULL pointer dereference
344.384859] x11: x10: > [ 1344.390699] x9 : 08d67000 x8 : 0001000197df > [ 1344.395825] x7 : x6 : > [ 1344.401663] x5 : 0001 x4 : > [ 1344.407234] x3 : 800078b3a000 x2 : 0a43d060 > [ 1344.412715] x1 : 0003 x0 : 0003 > [ 1344.418110] Process swapper/0 (pid: 0, stack limit = 0x08d6) > [ 1344.425026] Call trace: > [ 1344.427277] Exception stack(0x08003bd0 to 0x08003d10) > [ 1344.434372] 3bc0: > 0003 0003 > [ 1344.442550] 3be0: 0a43d060 800078b3a000 > 0001 > [ 1344.450374] 3c00: > 0001000197df 08d67000 > [ 1344.458460] 3c20: > 0001 > [ 1344.466189] 3c40: 0008 > 080ce160 9127b8d0 > [ 1344.474633] 3c60: 002e 800077cb0208 > 00b6 08926110 > [ 1344.482808] 3c80: 0005 800077d32938 > 800077d36a00 0001 > [ 1344.490805] 3ca0: 0003 800077cb0028 > 08d45000 08003d10 > [ 1344.498897] 3cc0: 08685208 08003d10 > 08685198 8145 > [ 1344.506896] 3ce0: 800077cb0208 00b6 > 0001 0005 > [ 1344.515255] 3d00: 08003d10 08685198 > [ 1344.520207] [] mvneta_txq_bufs_free.isra.24+0x68/0x170 > [ 1344.527142] [] mvneta_poll+0x4f0/0xad8 > [ 1344.532528] [] net_rx_action+0x184/0x418 > [ 1344.538461] [] __do_softirq+0x130/0x32c > [ 1344.543594] [] irq_exit+0xc8/0x100 > [ 1344.548812] [] __handle_domain_irq+0x6c/0xc0 > [ 1344.554651] [] gic_handle_irq+0x80/0x184 > [ 1344.560492] Exception stack(0x08d63db0 to 0x08d63ef0) > [ 1344.567233] 3da0: > 08d45000 > [ 1344.575317] 3dc0: 08d63ef0 00784718 > 800073275000 08d63f00 > [ 1344.583403] 3de0: 800073275000 0001 > 08d70fe0 08d63e80 > [ 1344.591403] 3e00: 0a00 > 0001 > [ 1344.599666] 3e20: 0008 > 080ce160 9127b8d0 > [ 1344.607843] 3e40: 002e 08d45000 > 08d69000 08d69000 > [ 1344.615839] 3e60: 08d4f148 08d69bec > > [ 1344.623836] 3e80: 08d70580 7ff963f8 > 00c80018 08d63ef0 > [ 1344.631922] 3ea0: 0808521c 08d63ef0 > 08085220 0145 > [ 1344.640097] 3ec0: 80007bfffb00 08cea028 > > [ 1344.648181] 3ee0: 08d63ef0 08085220 > [ 1344.653037] [] el1_irq+0xb0/0x140 > [ 1344.657891] [] arch_cpu_idle+0x30/0x188 > [ 1344.663911] [] do_idle+0x128/0x1e8 > [ 1344.669041] [] cpu_startup_entry+0x2c/0x30 > [ 1344.674972] [] rest_init+0xb4/0xc0 > [ 1344.679945] [] start_kernel+0x394/0x3a8 > [ 1344.685594] Code: 93407c01 8b011442 f8617879 b479 (b9408321) > [ 1344.691523] ---[ end trace 0e5abdfc76ee83e5 ]--- > [ 1344.696733] Kernel panic - not syncing: Fatal exception in interrupt > [ 1344.703310] SMP: stopping secondary CPUs > [ 1344.707369] Kernel Offset: disabled > [ 1344.710613] CPU features: 0x002008 > [ 1344.714294] Memory Limit: none > [ 1344.717086] ---[ end Kernel panic - not syncing: Fatal exception in > interrupt > > I you want more logs or some other details about my setup i'll be > happy to help :-) > Also with testing a possible fix. > > Thanks, > Sean Nyekjaer -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
Re: Problems with mvneta
Hello, On Tue, 31 Oct 2017 18:09:38 +0100, Simon Guinot wrote: > > On Tue, 31 Oct 2017 15:23:22 +0100, Sven Müller wrote: > > > After quite a long time of trying to reproduce the issue without any > > > success I got 3 network crashes today. And all errors occurred with a > > > kernel including the patch: > > > > > > 2a90f7e1d5d04e4f1060268e0b55a2c702bbd67a > > > > > > At least according to Andreas' and my problems we can exclude the 6ad2 > > > patch as the source of the errors. > > > > Simon, 2a90f7e1d5d04e4f1060268e0b55a2c702bbd67a is your commit, adding > > xmit_more support, and a number of people are reporting stability > > issues with this patch applied. > > I wrote an earlier version of this patch. But I think this commit has > been modified by the submitter Marcin Wojtas because I don't remember > anything about the maximum number of descriptors allowed to be flush. > > > > > Do you think you will have some time to look into this ? > > No I don't have time to look into that. > > But after a quick look, I wonder what is happening if > "txq->pending + frags > MVNETA_TXQ_DEC_SENT_MASK" ? Because IIUC > mvneta_txq_pend_desc_add() is called anyway. And according to the > comment inside the function, it assumes there is less than 255 > descriptors to send... It looks suspect. Thanks for the feedback. Marcin, do you remember this xmit_more patch? Thanks! Thomas -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
Re: Problems with mvneta
Hello, Let's add Simon Guinot in the loop. On Tue, 31 Oct 2017 15:23:22 +0100, Sven Müller wrote: > After quite a long time of trying to reproduce the issue without any success > I got 3 network crashes today. And all errors occurred with a kernel > including the patch: > > 2a90f7e1d5d04e4f1060268e0b55a2c702bbd67a > > At least according to Andreas' and my problems we can exclude the 6ad2 patch > as the source of the errors. Simon, 2a90f7e1d5d04e4f1060268e0b55a2c702bbd67a is your commit, adding xmit_more support, and a number of people are reporting stability issues with this patch applied. Do you think you will have some time to look into this ? Thanks! Thomas -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
Re: Problems with mvneta
Hello, On Fri, 20 Oct 2017 00:25:24 +0200, Sven Müller wrote: > First of all I'm not familiar with kernel programming at all, so > please excuse me, if I don't understand everything at the first > glance. No problem. Your bug report and effort to nail down the issue are much appreciated! > It compiles and runs fine. After a couple of hours and testing no > issues were found. OK, so this really hints at a regression in the mvneta driver itself. Could you try to revert just: 6ad20165d376fa07919a70e4f43dfae564601829 a29b6235560a1ed10c8e1a73bfc616a66b802b90 2a90f7e1d5d04e4f1060268e0b55a2c702bbd67a first all of them, and then each one by one, so that we can pin-point the commit that causes the breakage ? I looked at all the other changes in mvneta between 4.10 and 4.12, and I don't see how any of the other changes can cause a functional difference. So let's focus on those 3 commits for the moment. Assuming you're using Git, to revert a commit just do: "git revert ". Thanks again! Thomas -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
Re: Problems with mvneta
Hello, I'm adding my colleagues Grégory Clement and Antoine Ténart in Cc, as well as Marcin Wojtas, who also worked on mvneta, and the netdev mailing list. I'm keeping your full message below so that others can read the context. On Wed, 18 Oct 2017 22:34:25 +0200, Sven Müller wrote: > I've found your email address in the kernel sources of the mvneta driver. I > didn't find a bug system on free-electrons.com. And on kernel.org searching > for mvneta wasn't really helpful. There is a bug tracker for the Linux kernel at https://bugzilla.kernel.org/. However, I indeed wouldn't be notified of bug reports against mvneta. > Some people including me hacked the Zyxel NSA-326 some time ago. The whole > thread you can find here: > > https://forum.doozan.com/read.php?2,27108 > > Until kernel 4.10.10 everything worked great. I didn't test 4.11. But any > higher kernel version (tested 4.12., 4.13) causes network problems with nfs. > I described it here: > > https://forum.doozan.com/read.php?2,27108,37699#msg-37699 > > Transfering files not with full speed but over a longer period of time, e.g. > playing music files over nfs or reading a lot of smaller files causes the > error: > > Sep 27 17:35:37 nas kernel: rpc-srv/tcp: nfsd: sent only 36488 when sending > 65644 bytes - shutting down socket > > After that message the network is down. I have to reboot the device in order > to get any network connectivity again. And how I wrote: 4.10.10 works > perfectly. 4.12 produced a lot of this errors, 4.13 seems to be a little bit > better. > > Unfortunately I didn't find a way to reproduce this problem directly. It > occurs after 5 minutes up to one hour of transferring files via nfs. > > If you are interested in fixing this bug, I would like to support you with > providing you any information I can find on my system and testing. > > My kernel config, which is working in 4.10.10 and producing the nfs problem > in 4.12 and 4.13: > > https://paste.pound-python.org/show/RCaG9J4yBy79K3NL5F1 > > and the device tree: > > https://paste.pound-python.org/show/UiLpMgUERuCddHOn6Vsp/ There have been a few changes in the mvneta code between 4.10 and 4.12, but not many of them look potentially problematic. f95936cca6a8410ebdaf164bc5d3ade9e1de5bdb net: mvneta: Adjust six checks for null pointers d441b688a1bce8e2e1b43d8090738c306dd09131 net: mvneta: Use kmalloc_array() in mvneta_txq_init() 5d6312ed57a909c86bb9472b2bbc012539392e7d net: mvneta: Improve two size determinations in mvneta_init() 2911063011fc7adcb43c93e9c3e9dc7798f459f5 net: mvneta: Use devm_kmalloc_array() in mvneta_init() 82960fff09bc394e2a33d5369969410699c04861 net: mvneta: fix failed to suspend if WOL is enabled d6956ac87b5ff6841b09c273a70de86200d82019 net: mvneta: set rx mode during resume if interface is running a38d20d791fdcd79ebccda15a8308a6d8ada6e1c net: mvneta: add RGMII_RXID and RGMII_TXID support 9768b45ceb0bc7bdee61837afad331dd6bf7977f net: mvneta: support suspend and resume 4581be42fce5e1d208cbeb8e78df3f1b4673eff7 net: mvneta: make mvneta_eth_tool_ops static 9303ab2b3402b60f6c39abfdbfa4ce00fce8bee4 net: mvneta: fix build errors when linux/phy*.h is removed from net/dsa.h b60a00f9c5f14695991cb77dce7e926623269d88 net: mvneta: implement .set_wol and .get_wol 6ad20165d376fa07919a70e4f43dfae564601829 drivers: net: generalize napi_complete_done() a29b6235560a1ed10c8e1a73bfc616a66b802b90 net: mvneta: add BQL support 2a90f7e1d5d04e4f1060268e0b55a2c702bbd67a net: mvneta: add xmit_more support bc1f44709cf27fb2a5766cadafe7e2ad5e9cb221 net: make ndo_get_stats64 a void function The only ones that really could have an impact are: 6ad20165d376fa07919a70e4f43dfae564601829 drivers: net: generalize napi_complete_done() a29b6235560a1ed10c8e1a73bfc616a66b802b90 net: mvneta: add BQL support 2a90f7e1d5d04e4f1060268e0b55a2c702bbd67a net: mvneta: add xmit_more support Could you try to take mvneta* from Linux 4.10, put that in Linux 4.12, and see if you can still produce the problem? I'd like to first make sure the problem really is inside mvneta, and not in some other place. Thanks! Thomas -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
Re: [PATCH net] net: mvpp2: Fix clock resource by adding an optional bus clock
Hello, On Thu, 28 Sep 2017 17:39:23 +0200, Gregory CLEMENT wrote: > diff --git a/Documentation/devicetree/bindings/net/marvell-pp2.txt > b/Documentation/devicetree/bindings/net/marvell-pp2.txt > index 7e2dad08a12e..49e1be6bb6ba 100644 > --- a/Documentation/devicetree/bindings/net/marvell-pp2.txt > +++ b/Documentation/devicetree/bindings/net/marvell-pp2.txt > @@ -21,8 +21,9 @@ Required properties: > - main controller clock (for both armada-375-pp2 and armada-7k-pp2) > - GOP clock (for both armada-375-pp2 and armada-7k-pp2) > - MG clock (only for armada-7k-pp2) > -- clock-names: names of used clocks, must be "pp_clk", "gop_clk" and > - "mg_clk" (the latter only for armada-7k-pp2). > + - AXI clock (only for armada-7k-pp2) > +- clock-names: names of used clocks, must be "pp_clk", "gop_clk", "mg_clk" > + and "axi"(the 2 latter only for armada-7k-pp2). Should be "axi_clk" not "axi", otherwise your binding documentation doesn't match the driver and example. Also, missing space after "axi". > > The ethernet ports are represented by subnodes. At least one port is > required. > @@ -78,8 +79,9 @@ Example for marvell,armada-7k-pp2: > cpm_ethernet: ethernet@0 { > compatible = "marvell,armada-7k-pp22"; > reg = <0x0 0x10>, <0x129000 0xb000>; > - clocks = <&cpm_syscon0 1 3>, <&cpm_syscon0 1 9>, <&cpm_syscon0 1 5>; > - clock-names = "pp_clk", "gop_clk", "gp_clk"; > + clocks = <&cpm_syscon0 1 3>, <&cpm_syscon0 1 9>, > + <&cpm_syscon0 1 5>, <&cpm_syscon0 1 18>; Please indent the second line with one more tab. > + clock-names = "pp_clk", "gop_clk", "gp_clk", "axi_clk"; > > eth0: eth0 { > interrupts = , > diff --git a/drivers/net/ethernet/marvell/mvpp2.c > b/drivers/net/ethernet/marvell/mvpp2.c > index dd0ee2691c86..33b6791df2bb 100644 > --- a/drivers/net/ethernet/marvell/mvpp2.c > +++ b/drivers/net/ethernet/marvell/mvpp2.c > @@ -792,6 +792,7 @@ struct mvpp2 { > struct clk *pp_clk; > struct clk *gop_clk; > struct clk *mg_clk; > + struct clk *axi_clk; > > /* List of pointers to port structures */ > struct mvpp2_port **port_list; > @@ -7963,6 +7964,16 @@ static int mvpp2_probe(struct platform_device *pdev) > err = clk_prepare_enable(priv->mg_clk); > if (err < 0) > goto err_gop_clk; > + > + priv->axi_clk = devm_clk_get(&pdev->dev, "axi_clk"); > + if (IS_ERR(priv->axi_clk)) { > + err = PTR_ERR(priv->axi_clk); > + priv->axi_clk = NULL; You should handle -EPROBE_DEFER here. Indeed, if we have -EPROBE_DEFER, we shouldn't treat it as "the clock doesn't exist, so let's skip it and continue", but rather as "the clock exists, but isn't ready to use yet, let's try later". Thanks! Thomas -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
[PATCH net-next v2 7/7] dt-bindings: net: marvell-pp2: update interrupt-names with TX interrupts
The PPv2.2 unit has several interrupts used for TX completion notification. This commit updates the Device Tree binding describing this HW block to mention such interrupts. While at it, we update the example to use a recent Device Tree example, that uses interrupts going through the ICU, and not to the GIC directly. Signed-off-by: Thomas Petazzoni --- .../devicetree/bindings/net/marvell-pp2.txt| 28 +++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/Documentation/devicetree/bindings/net/marvell-pp2.txt b/Documentation/devicetree/bindings/net/marvell-pp2.txt index 6b4956b..8918ad3 100644 --- a/Documentation/devicetree/bindings/net/marvell-pp2.txt +++ b/Documentation/devicetree/bindings/net/marvell-pp2.txt @@ -41,6 +41,10 @@ Optional properties (port): - marvell,loopback: port is loopback mode - phy: a phandle to a phy node defining the PHY address (as the reg property, a single integer). +- interrupt-names: if more than a single interrupt for rx is given, must + be the name associated to the interrupts listed. Valid + names are: "tx-cpu0", "tx-cpu1", "tx-cpu2", "tx-cpu3", + "rx-shared". Example for marvell,armada-375-pp2: @@ -80,19 +84,37 @@ cpm_ethernet: ethernet@0 { clock-names = "pp_clk", "gop_clk", "gp_clk"; eth0: eth0 { - interrupts = ; + interrupts = , +, +, +, +; + interrupt-names = "tx-cpu0", "tx-cpu1", "tx-cpu2", + "tx-cpu3", "rx-shared"; port-id = <0>; gop-port-id = <0>; }; eth1: eth1 { - interrupts = ; + interrupts = , +, +, +, +; + interrupt-names = "tx-cpu0", "tx-cpu1", "tx-cpu2", + "tx-cpu3", "rx-shared"; port-id = <1>; gop-port-id = <2>; }; eth2: eth2 { - interrupts = ; + interrupts = , +, +, +, +; + interrupt-names = "tx-cpu0", "tx-cpu1", "tx-cpu2", + "tx-cpu3", "rx-shared"; port-id = <2>; gop-port-id = <3>; }; -- 2.9.4
[PATCH net-next v2 3/7] net: mvpp2: introduce per-port nrxqs/ntxqs variables
Currently, the global variables rxq_number and txq_number hold the number of per-port TXQs and RXQs. Until now, such numbers were constant regardless of the driver configuration. As we are going to introduce different modes for TX and RX queues, these numbers will depend on the configuration (PPv2.1 vs. PPv2.2, exact queue distribution logic). Therefore, as a preparation, we move the number of RXQs and TXQs in the 'struct mvpp2_port' structure, next to the RXQs and TXQs descriptor arrays. For now, they remain initialized to the same default values as rxq_number/txq_number used to be initialized, but this will change in future commits. The only non-mechanical change in this patch is that the check to verify hardware constraints on the number of RXQs and TXQs is moved from mvpp2_probe() to mvpp2_port_probe(), since it's now in mvpp2_port_probe() that we initialize the per-port count of RXQ and TXQ. Signed-off-by: Thomas Petazzoni --- drivers/net/ethernet/marvell/mvpp2.c | 83 ++-- 1 file changed, 41 insertions(+), 42 deletions(-) diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c index 537d2b4..84908aa 100644 --- a/drivers/net/ethernet/marvell/mvpp2.c +++ b/drivers/net/ethernet/marvell/mvpp2.c @@ -768,7 +768,9 @@ struct mvpp2_port { void __iomem *base; struct mvpp2_rx_queue **rxqs; + unsigned int nrxqs; struct mvpp2_tx_queue **txqs; + unsigned int ntxqs; struct net_device *dev; int pkt_size; @@ -1062,13 +1064,6 @@ struct mvpp2_bm_pool { u32 port_map; }; -/* Static declaractions */ - -/* Number of RXQs used by single port */ -static int rxq_number = MVPP2_DEFAULT_RXQ; -/* Number of TXQs used by single port */ -static int txq_number = MVPP2_MAX_TXQ; - #define MVPP2_DRIVER_NAME "mvpp2" #define MVPP2_DRIVER_VERSION "1.0" @@ -4070,7 +4065,7 @@ static int mvpp2_swf_bm_pool_init(struct mvpp2_port *port) port->pool_long->port_map |= (1 << port->id); - for (rxq = 0; rxq < rxq_number; rxq++) + for (rxq = 0; rxq < port->nrxqs; rxq++) mvpp2_rxq_long_pool_set(port, rxq, port->pool_long->id); } @@ -4084,7 +4079,7 @@ static int mvpp2_swf_bm_pool_init(struct mvpp2_port *port) port->pool_short->port_map |= (1 << port->id); - for (rxq = 0; rxq < rxq_number; rxq++) + for (rxq = 0; rxq < port->nrxqs; rxq++) mvpp2_rxq_short_pool_set(port, rxq, port->pool_short->id); } @@ -4376,7 +4371,7 @@ static void mvpp2_defaults_set(struct mvpp2_port *port) MVPP2_RX_LOW_LATENCY_PKT_SIZE(256)); /* Enable Rx cache snoop */ - for (lrxq = 0; lrxq < rxq_number; lrxq++) { + for (lrxq = 0; lrxq < port->nrxqs; lrxq++) { queue = port->rxqs[lrxq]->id; val = mvpp2_read(port->priv, MVPP2_RXQ_CONFIG_REG(queue)); val |= MVPP2_SNOOP_PKT_SIZE_MASK | @@ -4394,7 +4389,7 @@ static void mvpp2_ingress_enable(struct mvpp2_port *port) u32 val; int lrxq, queue; - for (lrxq = 0; lrxq < rxq_number; lrxq++) { + for (lrxq = 0; lrxq < port->nrxqs; lrxq++) { queue = port->rxqs[lrxq]->id; val = mvpp2_read(port->priv, MVPP2_RXQ_CONFIG_REG(queue)); val &= ~MVPP2_RXQ_DISABLE_MASK; @@ -4407,7 +4402,7 @@ static void mvpp2_ingress_disable(struct mvpp2_port *port) u32 val; int lrxq, queue; - for (lrxq = 0; lrxq < rxq_number; lrxq++) { + for (lrxq = 0; lrxq < port->nrxqs; lrxq++) { queue = port->rxqs[lrxq]->id; val = mvpp2_read(port->priv, MVPP2_RXQ_CONFIG_REG(queue)); val |= MVPP2_RXQ_DISABLE_MASK; @@ -4426,7 +4421,7 @@ static void mvpp2_egress_enable(struct mvpp2_port *port) /* Enable all initialized TXs. */ qmap = 0; - for (queue = 0; queue < txq_number; queue++) { + for (queue = 0; queue < port->ntxqs; queue++) { struct mvpp2_tx_queue *txq = port->txqs[queue]; if (txq->descs) @@ -4712,7 +4707,7 @@ static void mvpp2_txq_sent_counter_clear(void *arg) struct mvpp2_port *port = arg; int queue; - for (queue = 0; queue < txq_number; queue++) { + for (queue = 0; queue < port->ntxqs; queue++) { int id = port->txqs[queue]->id; mvpp2_percpu_read(port->priv, smp_processor_id(), @@ -4753,7 +4748,7 @@ static void mvpp2_txp_max_tx_size_set(struct mvpp2_port *port) mvpp2_write(port->priv, MVPP2_TXP_SCHED_TOKEN_SIZE_REG, val); } - for (txq = 0;
[PATCH net-next v2 6/7] net: mvpp2: add support for TX interrupts and RX queue distribution modes
This commit adds the support for two related features: - Support for TX interrupts, with one interrupt for each CPU - Support for different RX queue distribution modes MVPP2_QDIST_SINGLE_MODE where a single interrupt, shared by all CPUs, receives the RX events, and MVPP2_QDIST_MULTI_MODE, where the per-CPU interrupts used for TX events are also used for RX events. Since additional interrupts are needed, an update to the Device Tree binding is needed. However, backward compatibility is preserved with the old Device Tree binding, by gracefully degrading to the original behavior, with only one RX interrupt, and TX completion being handled by an hrtimer. Signed-off-by: Thomas Petazzoni --- drivers/net/ethernet/marvell/mvpp2.c | 275 +++ 1 file changed, 246 insertions(+), 29 deletions(-) diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c index 1bf3272..39bc8fb 100644 --- a/drivers/net/ethernet/marvell/mvpp2.c +++ b/drivers/net/ethernet/marvell/mvpp2.c @@ -120,6 +120,9 @@ #define MVPP2_TXQ_DESC_ADDR_REG0x2084 #define MVPP2_TXQ_DESC_SIZE_REG0x2088 #define MVPP2_TXQ_DESC_SIZE_MASK 0x3ff0 +#define MVPP2_TXQ_THRESH_REG 0x2094 +#defineMVPP2_TXQ_THRESH_OFFSET 16 +#defineMVPP2_TXQ_THRESH_MASK 0x3fff #define MVPP2_AGGR_TXQ_UPDATE_REG 0x2090 #define MVPP2_TXQ_INDEX_REG0x2098 #define MVPP2_TXQ_PREF_BUF_REG 0x209c @@ -183,6 +186,9 @@ #define MVPP22_AXI_CODE_DOMAIN_SYSTEM 3 /* Interrupt Cause and Mask registers */ +#define MVPP2_ISR_TX_THRESHOLD_REG(port) (0x5140 + 4 * (port)) +#define MVPP2_MAX_ISR_TX_THRESHOLD 0xf0 + #define MVPP2_ISR_RX_THRESHOLD_REG(rxq)(0x5200 + 4 * (rxq)) #define MVPP2_MAX_ISR_RX_THRESHOLD 0xf0 #define MVPP21_ISR_RXQ_GROUP_REG(port) (0x5400 + 4 * (port)) @@ -206,6 +212,7 @@ #define MVPP2_ISR_RX_TX_CAUSE_REG(port)(0x5480 + 4 * (port)) #define MVPP2_CAUSE_RXQ_OCCUP_DESC_ALL_MASK0x #define MVPP2_CAUSE_TXQ_OCCUP_DESC_ALL_MASK0xff +#define MVPP2_CAUSE_TXQ_OCCUP_DESC_ALL_OFFSET 16 #define MVPP2_CAUSE_RX_FIFO_OVERRUN_MASK BIT(24) #define MVPP2_CAUSE_FCS_ERR_MASK BIT(25) #define MVPP2_CAUSE_TX_FIFO_UNDERRUN_MASK BIT(26) @@ -372,6 +379,7 @@ /* Coalescing */ #define MVPP2_TXDONE_COAL_PKTS_THRESH 15 #define MVPP2_TXDONE_HRTIMER_PERIOD_NS 100UL +#define MVPP2_TXDONE_COAL_USEC 1000 #define MVPP2_RX_COAL_PKTS 32 #define MVPP2_RX_COAL_USEC 100 @@ -811,6 +819,9 @@ struct mvpp2_port { struct mvpp2_queue_vector qvecs[MVPP2_MAX_QVECS]; unsigned int nqvecs; + bool has_tx_irqs; + + u32 tx_time_coal; }; /* The mvpp2_tx_desc and mvpp2_rx_desc structures describe the @@ -1076,6 +1087,15 @@ struct mvpp2_bm_pool { u32 port_map; }; +/* Queue modes */ +#define MVPP2_QDIST_SINGLE_MODE0 +#define MVPP2_QDIST_MULTI_MODE 1 + +static int queue_mode = MVPP2_QDIST_SINGLE_MODE; + +module_param(queue_mode, int, 0444); +MODULE_PARM_DESC(queue_mode, "Set queue_mode (single=0, multi=1)"); + #define MVPP2_DRIVER_NAME "mvpp2" #define MVPP2_DRIVER_VERSION "1.0" @@ -4187,11 +4207,40 @@ static void mvpp2_interrupts_mask(void *arg) static void mvpp2_interrupts_unmask(void *arg) { struct mvpp2_port *port = arg; + u32 val; + + val = MVPP2_CAUSE_MISC_SUM_MASK | + MVPP2_CAUSE_RXQ_OCCUP_DESC_ALL_MASK; + if (port->has_tx_irqs) + val |= MVPP2_CAUSE_TXQ_OCCUP_DESC_ALL_MASK; mvpp2_percpu_write(port->priv, smp_processor_id(), - MVPP2_ISR_RX_TX_MASK_REG(port->id), - (MVPP2_CAUSE_MISC_SUM_MASK | - MVPP2_CAUSE_RXQ_OCCUP_DESC_ALL_MASK)); + MVPP2_ISR_RX_TX_MASK_REG(port->id), val); +} + +static void +mvpp2_shared_interrupt_mask_unmask(struct mvpp2_port *port, bool mask) +{ + u32 val; + int i; + + if (port->priv->hw_version != MVPP22) + return; + + if (mask) + val = 0; + else + val = MVPP2_CAUSE_RXQ_OCCUP_DESC_ALL_MASK; + + for (i = 0; i < port->nqvecs; i++) { + struct mvpp2_queue_vector *v = port->qvecs + i; + + if (v->type != MVPP2_QUEUE_VECTOR_SHARED) + continue; + + mvpp2_percpu_write(port->priv, v->sw_thread_id, + MVPP2_ISR_RX_TX_MASK_REG(port->id), val); + } } /* Port configuration routines */ @@ -4812,6 +4861,23 @@ static void mvpp2_rx_pkts_coal_set(struct mvpp2_port *port, put_cpu(); } +/* For s
[PATCH net-next v2 4/7] net: mvpp2: move from cpu-centric naming to "software thread" naming
The PPv2.2 IP has a concept of "software thread", with all registers of the PPv2.2 mapped 8 times, for concurrent accesses by 8 "software threads". In addition, interrupts on RX queues are associated to such "software thread". For most cases, we map a "software thread" to the more conventional concept of CPU, but we will soon have one exception: we will have a model where we have one TX interrupt per CPU (each using one software thread), and all RX events mapped to another software thread (associated to another interrupt). In preparation for this change, it makes sense to change the naming from MVPP2_MAX_CPUS to MVPP2_MAX_THREADS, and plan for 8 software threads instead of 4 currently. Signed-off-by: Thomas Petazzoni --- drivers/net/ethernet/marvell/mvpp2.c | 25 + 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c index 84908aa..af38a21 100644 --- a/drivers/net/ethernet/marvell/mvpp2.c +++ b/drivers/net/ethernet/marvell/mvpp2.c @@ -685,7 +685,7 @@ enum mvpp2_prs_l3_cast { #define MVPP21_ADDR_SPACE_SZ 0 #define MVPP22_ADDR_SPACE_SZ SZ_64K -#define MVPP2_MAX_CPUS 4 +#define MVPP2_MAX_THREADS 8 enum mvpp2_bm_type { MVPP2_BM_FREE, @@ -701,11 +701,12 @@ struct mvpp2 { void __iomem *lms_base; void __iomem *iface_base; - /* On PPv2.2, each CPU can access the base register through a -* separate address space, each 64 KB apart from each -* other. + /* On PPv2.2, each "software thread" can access the base +* register through a separate address space, each 64 KB apart +* from each other. Typically, such address spaces will be +* used per CPU. */ - void __iomem *cpu_base[MVPP2_MAX_CPUS]; + void __iomem *swth_base[MVPP2_MAX_THREADS]; /* Common clocks */ struct clk *pp_clk; @@ -1071,12 +1072,12 @@ struct mvpp2_bm_pool { static void mvpp2_write(struct mvpp2 *priv, u32 offset, u32 data) { - writel(data, priv->cpu_base[0] + offset); + writel(data, priv->swth_base[0] + offset); } static u32 mvpp2_read(struct mvpp2 *priv, u32 offset) { - return readl(priv->cpu_base[0] + offset); + return readl(priv->swth_base[0] + offset); } /* These accessors should be used to access: @@ -1118,13 +1119,13 @@ static u32 mvpp2_read(struct mvpp2 *priv, u32 offset) static void mvpp2_percpu_write(struct mvpp2 *priv, int cpu, u32 offset, u32 data) { - writel(data, priv->cpu_base[cpu] + offset); + writel(data, priv->swth_base[cpu] + offset); } static u32 mvpp2_percpu_read(struct mvpp2 *priv, int cpu, u32 offset) { - return readl(priv->cpu_base[cpu] + offset); + return readl(priv->swth_base[cpu] + offset); } static dma_addr_t mvpp2_txdesc_dma_addr_get(struct mvpp2_port *port, @@ -6874,7 +6875,7 @@ static int mvpp2_probe(struct platform_device *pdev) struct mvpp2 *priv; struct resource *res; void __iomem *base; - int port_count, cpu; + int port_count, i; int err; priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); @@ -6901,12 +6902,12 @@ static int mvpp2_probe(struct platform_device *pdev) return PTR_ERR(priv->iface_base); } - for_each_present_cpu(cpu) { + for (i = 0; i < MVPP2_MAX_THREADS; i++) { u32 addr_space_sz; addr_space_sz = (priv->hw_version == MVPP21 ? MVPP21_ADDR_SPACE_SZ : MVPP22_ADDR_SPACE_SZ); - priv->cpu_base[cpu] = base + cpu * addr_space_sz; + priv->swth_base[i] = base + i * addr_space_sz; } if (priv->hw_version == MVPP21) -- 2.9.4
[PATCH net-next v2 5/7] net: mvpp2: introduce queue_vector concept
In preparation to the introduction of TX interrupts and improved RX queue distribution, this commit introduces the concept of "queue vector". A queue vector represents a number of RX and/or TX queues, and an associated NAPI instance and interrupt. This commit currently only creates a single queue_vector, so there are no changes in behavior, but it paves the way for additional queue_vector in the next commits. Signed-off-by: Thomas Petazzoni --- drivers/net/ethernet/marvell/mvpp2.c | 223 ++- 1 file changed, 169 insertions(+), 54 deletions(-) diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c index af38a21..1bf3272 100644 --- a/drivers/net/ethernet/marvell/mvpp2.c +++ b/drivers/net/ethernet/marvell/mvpp2.c @@ -686,6 +686,7 @@ enum mvpp2_prs_l3_cast { #define MVPP22_ADDR_SPACE_SZ SZ_64K #define MVPP2_MAX_THREADS 8 +#define MVPP2_MAX_QVECSMVPP2_MAX_THREADS enum mvpp2_bm_type { MVPP2_BM_FREE, @@ -753,6 +754,18 @@ struct mvpp2_port_pcpu { struct tasklet_struct tx_done_tasklet; }; +struct mvpp2_queue_vector { + int irq; + struct napi_struct napi; + enum { MVPP2_QUEUE_VECTOR_SHARED, MVPP2_QUEUE_VECTOR_PRIVATE } type; + int sw_thread_id; + u16 sw_thread_mask; + int first_rxq; + int nrxqs; + u32 pending_cause_rx; + struct mvpp2_port *port; +}; + struct mvpp2_port { u8 id; @@ -761,8 +774,6 @@ struct mvpp2_port { */ int gop_id; - int irq; - struct mvpp2 *priv; /* Per-port registers' base address */ @@ -776,9 +787,6 @@ struct mvpp2_port { int pkt_size; - u32 pending_cause_rx; - struct napi_struct napi; - /* Per-CPU port control */ struct mvpp2_port_pcpu __percpu *pcpu; @@ -800,6 +808,9 @@ struct mvpp2_port { /* Index of first port's physical RXQ */ u8 first_rxq; + + struct mvpp2_queue_vector qvecs[MVPP2_MAX_QVECS]; + unsigned int nqvecs; }; /* The mvpp2_tx_desc and mvpp2_rx_desc structures describe the @@ -4121,22 +4132,40 @@ static int mvpp2_bm_update_mtu(struct net_device *dev, int mtu) static inline void mvpp2_interrupts_enable(struct mvpp2_port *port) { - int cpu, cpu_mask = 0; + int i, sw_thread_mask = 0; + + for (i = 0; i < port->nqvecs; i++) + sw_thread_mask |= port->qvecs[i].sw_thread_mask; - for_each_present_cpu(cpu) - cpu_mask |= 1 << cpu; mvpp2_write(port->priv, MVPP2_ISR_ENABLE_REG(port->id), - MVPP2_ISR_ENABLE_INTERRUPT(cpu_mask)); + MVPP2_ISR_ENABLE_INTERRUPT(sw_thread_mask)); } static inline void mvpp2_interrupts_disable(struct mvpp2_port *port) { - int cpu, cpu_mask = 0; + int i, sw_thread_mask = 0; + + for (i = 0; i < port->nqvecs; i++) + sw_thread_mask |= port->qvecs[i].sw_thread_mask; + + mvpp2_write(port->priv, MVPP2_ISR_ENABLE_REG(port->id), + MVPP2_ISR_DISABLE_INTERRUPT(sw_thread_mask)); +} + +static inline void mvpp2_qvec_interrupt_enable(struct mvpp2_queue_vector *qvec) +{ + struct mvpp2_port *port = qvec->port; - for_each_present_cpu(cpu) - cpu_mask |= 1 << cpu; mvpp2_write(port->priv, MVPP2_ISR_ENABLE_REG(port->id), - MVPP2_ISR_DISABLE_INTERRUPT(cpu_mask)); + MVPP2_ISR_ENABLE_INTERRUPT(qvec->sw_thread_mask)); +} + +static inline void mvpp2_qvec_interrupt_disable(struct mvpp2_queue_vector *qvec) +{ + struct mvpp2_port *port = qvec->port; + + mvpp2_write(port->priv, MVPP2_ISR_ENABLE_REG(port->id), + MVPP2_ISR_DISABLE_INTERRUPT(qvec->sw_thread_mask)); } /* Mask the current CPU's Rx/Tx interrupts @@ -5287,11 +5316,11 @@ static int mvpp2_setup_txqs(struct mvpp2_port *port) /* The callback for per-port interrupt */ static irqreturn_t mvpp2_isr(int irq, void *dev_id) { - struct mvpp2_port *port = (struct mvpp2_port *)dev_id; + struct mvpp2_queue_vector *qv = dev_id; - mvpp2_interrupts_disable(port); + mvpp2_qvec_interrupt_disable(qv); - napi_schedule(&port->napi); + napi_schedule(&qv->napi); return IRQ_HANDLED; } @@ -5494,8 +5523,8 @@ static u32 mvpp2_skb_tx_csum(struct mvpp2_port *port, struct sk_buff *skb) } /* Main rx processing */ -static int mvpp2_rx(struct mvpp2_port *port, int rx_todo, - struct mvpp2_rx_queue *rxq) +static int mvpp2_rx(struct mvpp2_port *port, struct napi_struct *napi, + int rx_todo, struct mvpp2_rx_queue *rxq) { struct net_device *dev = port->dev; int rx_received; @@ -5573,7 +5602,7 @@ static int mvpp2_rx(struct mvpp2_port *port, int rx_todo,
[PATCH net-next v2 2/7] net: mvpp2: remove RX queue group reset code
The RX queue group allocation is anyway re-done later in mvpp2_port_init(), so resetting it in mvpp2_init() is not very useful, and will be annoying as we are going to rework the RX queue group allocation logic. Signed-off-by: Thomas Petazzoni --- drivers/net/ethernet/marvell/mvpp2.c | 17 - 1 file changed, 17 deletions(-) diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c index 4b36a15..537d2b4 100644 --- a/drivers/net/ethernet/marvell/mvpp2.c +++ b/drivers/net/ethernet/marvell/mvpp2.c @@ -6845,23 +6845,6 @@ static int mvpp2_init(struct platform_device *pdev, struct mvpp2 *priv) /* Rx Fifo Init */ mvpp2_rx_fifo_init(priv); - /* Reset Rx queue group interrupt configuration */ - for (i = 0; i < MVPP2_MAX_PORTS; i++) { - if (priv->hw_version == MVPP21) { - mvpp2_write(priv, MVPP21_ISR_RXQ_GROUP_REG(i), - rxq_number); - continue; - } else { - u32 val; - - val = (i << MVPP22_ISR_RXQ_GROUP_INDEX_GROUP_OFFSET); - mvpp2_write(priv, MVPP22_ISR_RXQ_GROUP_INDEX_REG, val); - - val = (rxq_number << MVPP22_ISR_RXQ_SUB_GROUP_SIZE_OFFSET); - mvpp2_write(priv, MVPP22_ISR_RXQ_SUB_GROUP_CONFIG_REG, val); - } - } - if (priv->hw_version == MVPP21) writel(MVPP2_EXT_GLOBAL_CTRL_DEFAULT, priv->lms_base + MVPP2_MNG_EXTENDED_GLOBAL_CTRL_REG); -- 2.9.4
[PATCH net-next v2 0/7] net: mvpp2: add TX interrupts support
Hello, So far, the mvpp2 driver was using an hrtimer to handle TX completion. This patch series adds support for using TX interrupts (for each CPU) on PPv2.2, the variant of the IP used on Marvell Armada 7K/8K. Dave: this version can be applied right away, it no longer depends on Antoine's patch series. Antoine series had some comments, so he will have to respin later on. Therefore, let's merge this smaller patch series first. Changes since v1: - Rebased on top of net-next, instead of on top of Antoine's series. - Removed the Device Tree patch, as it shouldn't go through the net tree. Thanks! Thomas Thomas Petazzoni (7): net: mvpp2: fix MVPP21_ISR_RXQ_GROUP_REG definition net: mvpp2: remove RX queue group reset code net: mvpp2: introduce per-port nrxqs/ntxqs variables net: mvpp2: move from cpu-centric naming to "software thread" naming net: mvpp2: introduce queue_vector concept net: mvpp2: add support for TX interrupts and RX queue distribution modes dt-bindings: net: marvell-pp2: update interrupt-names with TX interrupts .../devicetree/bindings/net/marvell-pp2.txt| 28 +- drivers/net/ethernet/marvell/mvpp2.c | 611 - 2 files changed, 488 insertions(+), 151 deletions(-) -- 2.9.4
[PATCH net-next v2 1/7] net: mvpp2: fix MVPP21_ISR_RXQ_GROUP_REG definition
The MVPP21_ISR_RXQ_GROUP_REG register is not indexed by rxq, but by port, so we fix the parameter name accordingly. There are no functional changes. Signed-off-by: Thomas Petazzoni --- drivers/net/ethernet/marvell/mvpp2.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c index 48d21c1..4b36a15 100644 --- a/drivers/net/ethernet/marvell/mvpp2.c +++ b/drivers/net/ethernet/marvell/mvpp2.c @@ -185,7 +185,7 @@ /* Interrupt Cause and Mask registers */ #define MVPP2_ISR_RX_THRESHOLD_REG(rxq)(0x5200 + 4 * (rxq)) #define MVPP2_MAX_ISR_RX_THRESHOLD 0xf0 -#define MVPP21_ISR_RXQ_GROUP_REG(rxq) (0x5400 + 4 * (rxq)) +#define MVPP21_ISR_RXQ_GROUP_REG(port) (0x5400 + 4 * (port)) #define MVPP22_ISR_RXQ_GROUP_INDEX_REG 0x5400 #define MVPP22_ISR_RXQ_GROUP_INDEX_SUBGROUP_MASK 0xf -- 2.9.4
Re: [PATCH] net: ethernet: stmmac: properly set PS bit in MII configurations during reset
Hello Giuseppe, On Wed, 28 Jun 2017 16:40:51 +0200, Giuseppe CAVALLARO wrote: > I do not want to change a critical reset function shared among > different platforms where > this problem has never met but you are right that we have to find a > way to proceed in order > to finalize your work. Let me elaborate your initial patch and I try > to give you a proposal asap. > In my mind, we should have a dedicated spear_dma_reset for your case > that should be used on > SPEAr platform driver (or by using st,spear600-gmac compatibility). > Also your patch did not consider the RMII and (R)GMII cases. Have you had the chance to cook a different proposal? Alternatively, do you have some specific hints to give me to make a new proposal that would be acceptable for you ? Thanks a lot, Thomas Petazzoni -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
Re: [PATCH 0/8] net: mvpp2: add TX interrupts support
Hello, On Wed, 26 Jul 2017 13:35:33 -0700 (PDT), David Miller wrote: > > - This series depends on the previous series sent by Antoine Ténart > >"net: mvpp2: MAC/GoP configuration and optional PHYs". Functionally > >speaking there is no real dependency, but we touch in a few areas > >the same piece of code, so I based my patch series on top of > >Antoine's. > > > > - Please do not apply the last patch of this series "arm64: dts: > >marvell: add TX interrupts for PPv2.2", it will be taken by the ARM > >mvebu maintainers. > > Please don't do things this way. > > Patiently wait for Antione's series to make it into my tree, then > submit your's. I'll resubmit when Antoine's series is merged. The idea of submitting my patches was to allow others to review/test them, not necessarily to have them merged right away. Perhaps I should have marked them RFC to make it clear that I don't expect you to merge them right away. > Also, if we're continually going to elide the DTS file patches, just It's the dutty of the sub-architecture maintainers to merge the DTS changes. Merging them through your tree doesn't work really well, as the changes might very likely conflict with other DTS changes merged by the sub-architecture maintainer. But it makes sense to have such DTS changes in the same patch series, since they are really related. It's very common to have patch series that contain patches that will be merged by different maintainers. > don't bother adding them to the series. That way you don't have to > give me special instructions, and I don't have the possibility of > making a mistake and applying it accidently. ... but OK, I won't resend them, so you don't get potentially confused by such patches. Thanks! Thomas Petazzoni -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
[PATCH 3/8] net: mvpp2: introduce per-port nrxqs/ntxqs variables
Currently, the global variables rxq_number and txq_number hold the number of per-port TXQs and RXQs. Until now, such numbers were constant regardless of the driver configuration. As we are going to introduce different modes for TX and RX queues, these numbers will depend on the configuration (PPv2.1 vs. PPv2.2, exact queue distribution logic). Therefore, as a preparation, we move the number of RXQs and TXQs in the 'struct mvpp2_port' structure, next to the RXQs and TXQs descriptor arrays. For now, they remain initialized to the same default values as rxq_number/txq_number used to be initialized, but this will change in future commits. The only non-mechanical change in this patch is that the check to verify hardware constraints on the number of RXQs and TXQs is moved from mvpp2_probe() to mvpp2_port_probe(), since it's now in mvpp2_port_probe() that we initialize the per-port count of RXQ and TXQ. Signed-off-by: Thomas Petazzoni --- drivers/net/ethernet/marvell/mvpp2.c | 83 ++-- 1 file changed, 41 insertions(+), 42 deletions(-) diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c index eb8853f..8479269 100644 --- a/drivers/net/ethernet/marvell/mvpp2.c +++ b/drivers/net/ethernet/marvell/mvpp2.c @@ -837,7 +837,9 @@ struct mvpp2_port { void __iomem *base; struct mvpp2_rx_queue **rxqs; + unsigned int nrxqs; struct mvpp2_tx_queue **txqs; + unsigned int ntxqs; struct net_device *dev; int pkt_size; @@ -1131,13 +1133,6 @@ struct mvpp2_bm_pool { u32 port_map; }; -/* Static declaractions */ - -/* Number of RXQs used by single port */ -static int rxq_number = MVPP2_DEFAULT_RXQ; -/* Number of TXQs used by single port */ -static int txq_number = MVPP2_MAX_TXQ; - #define MVPP2_DRIVER_NAME "mvpp2" #define MVPP2_DRIVER_VERSION "1.0" @@ -4139,7 +4134,7 @@ static int mvpp2_swf_bm_pool_init(struct mvpp2_port *port) port->pool_long->port_map |= (1 << port->id); - for (rxq = 0; rxq < rxq_number; rxq++) + for (rxq = 0; rxq < port->nrxqs; rxq++) mvpp2_rxq_long_pool_set(port, rxq, port->pool_long->id); } @@ -4153,7 +4148,7 @@ static int mvpp2_swf_bm_pool_init(struct mvpp2_port *port) port->pool_short->port_map |= (1 << port->id); - for (rxq = 0; rxq < rxq_number; rxq++) + for (rxq = 0; rxq < port->nrxqs; rxq++) mvpp2_rxq_short_pool_set(port, rxq, port->pool_short->id); } @@ -4678,7 +4673,7 @@ static void mvpp2_defaults_set(struct mvpp2_port *port) MVPP2_RX_LOW_LATENCY_PKT_SIZE(256)); /* Enable Rx cache snoop */ - for (lrxq = 0; lrxq < rxq_number; lrxq++) { + for (lrxq = 0; lrxq < port->nrxqs; lrxq++) { queue = port->rxqs[lrxq]->id; val = mvpp2_read(port->priv, MVPP2_RXQ_CONFIG_REG(queue)); val |= MVPP2_SNOOP_PKT_SIZE_MASK | @@ -4696,7 +4691,7 @@ static void mvpp2_ingress_enable(struct mvpp2_port *port) u32 val; int lrxq, queue; - for (lrxq = 0; lrxq < rxq_number; lrxq++) { + for (lrxq = 0; lrxq < port->nrxqs; lrxq++) { queue = port->rxqs[lrxq]->id; val = mvpp2_read(port->priv, MVPP2_RXQ_CONFIG_REG(queue)); val &= ~MVPP2_RXQ_DISABLE_MASK; @@ -4709,7 +4704,7 @@ static void mvpp2_ingress_disable(struct mvpp2_port *port) u32 val; int lrxq, queue; - for (lrxq = 0; lrxq < rxq_number; lrxq++) { + for (lrxq = 0; lrxq < port->nrxqs; lrxq++) { queue = port->rxqs[lrxq]->id; val = mvpp2_read(port->priv, MVPP2_RXQ_CONFIG_REG(queue)); val |= MVPP2_RXQ_DISABLE_MASK; @@ -4728,7 +4723,7 @@ static void mvpp2_egress_enable(struct mvpp2_port *port) /* Enable all initialized TXs. */ qmap = 0; - for (queue = 0; queue < txq_number; queue++) { + for (queue = 0; queue < port->ntxqs; queue++) { struct mvpp2_tx_queue *txq = port->txqs[queue]; if (txq->descs) @@ -5014,7 +5009,7 @@ static void mvpp2_txq_sent_counter_clear(void *arg) struct mvpp2_port *port = arg; int queue; - for (queue = 0; queue < txq_number; queue++) { + for (queue = 0; queue < port->ntxqs; queue++) { int id = port->txqs[queue]->id; mvpp2_percpu_read(port->priv, smp_processor_id(), @@ -5055,7 +5050,7 @@ static void mvpp2_txp_max_tx_size_set(struct mvpp2_port *port) mvpp2_write(port->priv, MVPP2_TXP_SCHED_TOKEN_SIZE_REG, val); } - for (txq = 0;
[PATCH 6/8] net: mvpp2: add support for TX interrupts and RX queue distribution modes
This commit adds the support for two related features: - Support for TX interrupts, with one interrupt for each CPU - Support for different RX queue distribution modes MVPP2_QDIST_SINGLE_MODE where a single interrupt, shared by all CPUs, receives the RX events, and MVPP2_QDIST_MULTI_MODE, where the per-CPU interrupts used for TX events are also used for RX events. Since additional interrupts are needed, an update to the Device Tree binding is needed. However, backward compatibility is preserved with the old Device Tree binding, by gracefully degrading to the original behavior, with only one RX interrupt, and TX completion being handled by an hrtimer. Signed-off-by: Thomas Petazzoni --- drivers/net/ethernet/marvell/mvpp2.c | 275 +++ 1 file changed, 246 insertions(+), 29 deletions(-) diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c index a8d448b..cabdbd3 100644 --- a/drivers/net/ethernet/marvell/mvpp2.c +++ b/drivers/net/ethernet/marvell/mvpp2.c @@ -122,6 +122,9 @@ #define MVPP2_TXQ_DESC_ADDR_REG0x2084 #define MVPP2_TXQ_DESC_SIZE_REG0x2088 #define MVPP2_TXQ_DESC_SIZE_MASK 0x3ff0 +#define MVPP2_TXQ_THRESH_REG 0x2094 +#defineMVPP2_TXQ_THRESH_OFFSET 16 +#defineMVPP2_TXQ_THRESH_MASK 0x3fff #define MVPP2_AGGR_TXQ_UPDATE_REG 0x2090 #define MVPP2_TXQ_INDEX_REG0x2098 #define MVPP2_TXQ_PREF_BUF_REG 0x209c @@ -185,6 +188,9 @@ #define MVPP22_AXI_CODE_DOMAIN_SYSTEM 3 /* Interrupt Cause and Mask registers */ +#define MVPP2_ISR_TX_THRESHOLD_REG(port) (0x5140 + 4 * (port)) +#define MVPP2_MAX_ISR_TX_THRESHOLD 0xf0 + #define MVPP2_ISR_RX_THRESHOLD_REG(rxq)(0x5200 + 4 * (rxq)) #define MVPP2_MAX_ISR_RX_THRESHOLD 0xf0 #define MVPP21_ISR_RXQ_GROUP_REG(port) (0x5400 + 4 * (port)) @@ -208,6 +214,7 @@ #define MVPP2_ISR_RX_TX_CAUSE_REG(port)(0x5480 + 4 * (port)) #define MVPP2_CAUSE_RXQ_OCCUP_DESC_ALL_MASK0x #define MVPP2_CAUSE_TXQ_OCCUP_DESC_ALL_MASK0xff +#define MVPP2_CAUSE_TXQ_OCCUP_DESC_ALL_OFFSET 16 #define MVPP2_CAUSE_RX_FIFO_OVERRUN_MASK BIT(24) #define MVPP2_CAUSE_FCS_ERR_MASK BIT(25) #define MVPP2_CAUSE_TX_FIFO_UNDERRUN_MASK BIT(26) @@ -435,6 +442,7 @@ /* Coalescing */ #define MVPP2_TXDONE_COAL_PKTS_THRESH 15 #define MVPP2_TXDONE_HRTIMER_PERIOD_NS 100UL +#define MVPP2_TXDONE_COAL_USEC 1000 #define MVPP2_RX_COAL_PKTS 32 #define MVPP2_RX_COAL_USEC 100 @@ -881,6 +889,9 @@ struct mvpp2_port { struct mvpp2_queue_vector qvecs[MVPP2_MAX_QVECS]; unsigned int nqvecs; + bool has_tx_irqs; + + u32 tx_time_coal; }; /* The mvpp2_tx_desc and mvpp2_rx_desc structures describe the @@ -1146,6 +1157,15 @@ struct mvpp2_bm_pool { u32 port_map; }; +/* Queue modes */ +#define MVPP2_QDIST_SINGLE_MODE0 +#define MVPP2_QDIST_MULTI_MODE 1 + +static int queue_mode = MVPP2_QDIST_SINGLE_MODE; + +module_param(queue_mode, int, 0444); +MODULE_PARM_DESC(queue_mode, "Set queue_mode (single=0, multi=1)"); + #define MVPP2_DRIVER_NAME "mvpp2" #define MVPP2_DRIVER_VERSION "1.0" @@ -4257,11 +4277,40 @@ static void mvpp2_interrupts_mask(void *arg) static void mvpp2_interrupts_unmask(void *arg) { struct mvpp2_port *port = arg; + u32 val; + + val = MVPP2_CAUSE_MISC_SUM_MASK | + MVPP2_CAUSE_RXQ_OCCUP_DESC_ALL_MASK; + if (port->has_tx_irqs) + val |= MVPP2_CAUSE_TXQ_OCCUP_DESC_ALL_MASK; mvpp2_percpu_write(port->priv, smp_processor_id(), - MVPP2_ISR_RX_TX_MASK_REG(port->id), - (MVPP2_CAUSE_MISC_SUM_MASK | - MVPP2_CAUSE_RXQ_OCCUP_DESC_ALL_MASK)); + MVPP2_ISR_RX_TX_MASK_REG(port->id), val); +} + +static void +mvpp2_shared_interrupt_mask_unmask(struct mvpp2_port *port, bool mask) +{ + u32 val; + int i; + + if (port->priv->hw_version != MVPP22) + return; + + if (mask) + val = 0; + else + val = MVPP2_CAUSE_RXQ_OCCUP_DESC_ALL_MASK; + + for (i = 0; i < port->nqvecs; i++) { + struct mvpp2_queue_vector *v = port->qvecs + i; + + if (v->type != MVPP2_QUEUE_VECTOR_SHARED) + continue; + + mvpp2_percpu_write(port->priv, v->sw_thread_id, + MVPP2_ISR_RX_TX_MASK_REG(port->id), val); + } } /* Port configuration routines */ @@ -5115,6 +5164,23 @@ static void mvpp2_rx_pkts_coal_set(struct mvpp2_port *port, put_cpu(); } +/* For s
[PATCH 1/8] net: mvpp2: fix MVPP21_ISR_RXQ_GROUP_REG definition
The MVPP21_ISR_RXQ_GROUP_REG register is not indexed by rxq, but by port, so we fix the parameter name accordingly. There are no functional changes. Signed-off-by: Thomas Petazzoni --- drivers/net/ethernet/marvell/mvpp2.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c index 33a7eb8..f5c84f4 100644 --- a/drivers/net/ethernet/marvell/mvpp2.c +++ b/drivers/net/ethernet/marvell/mvpp2.c @@ -187,7 +187,7 @@ /* Interrupt Cause and Mask registers */ #define MVPP2_ISR_RX_THRESHOLD_REG(rxq)(0x5200 + 4 * (rxq)) #define MVPP2_MAX_ISR_RX_THRESHOLD 0xf0 -#define MVPP21_ISR_RXQ_GROUP_REG(rxq) (0x5400 + 4 * (rxq)) +#define MVPP21_ISR_RXQ_GROUP_REG(port) (0x5400 + 4 * (port)) #define MVPP22_ISR_RXQ_GROUP_INDEX_REG 0x5400 #define MVPP22_ISR_RXQ_GROUP_INDEX_SUBGROUP_MASK 0xf -- 2.9.4
[PATCH 8/8] arm64: dts: marvell: add TX interrupts for PPv2.2
This commit updates the Marvell Armada 7K/8K Device Tree to describe the TX interrupts of the Ethernet controllers, in both the master and slave CP110s. Signed-off-by: Thomas Petazzoni --- Dave: please do *not* apply this patch. It will go through the ARM mvebu maintainers. Thanks! .../arm64/boot/dts/marvell/armada-cp110-master.dtsi | 21 ++--- arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi | 21 ++--- 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi b/arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi index 9278ba6..d192cea 100644 --- a/arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi +++ b/arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi @@ -71,8 +71,13 @@ cpm_eth0: eth0 { interrupts = , +, +, +, +, ; - interrupt-names = "rx-shared", "link"; + interrupt-names = "tx-cpu0", "tx-cpu1", "tx-cpu2", + "tx-cpu3", "rx-shared", "link"; port-id = <0>; gop-port-id = <0>; status = "disabled"; @@ -80,8 +85,13 @@ cpm_eth1: eth1 { interrupts = , +, +, +, +, ; - interrupt-names = "rx-shared", "link"; + interrupt-names = "tx-cpu0", "tx-cpu1", "tx-cpu2", + "tx-cpu3", "rx-shared", "link"; port-id = <1>; gop-port-id = <2>; status = "disabled"; @@ -89,8 +99,13 @@ cpm_eth2: eth2 { interrupts = , +, +, +, +, ; - interrupt-names = "rx-shared", "link"; + interrupt-names = "tx-cpu0", "tx-cpu1", "tx-cpu2", + "tx-cpu3", "rx-shared", "link"; port-id = <2>; gop-port-id = <3>; status = "disabled"; diff --git a/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi b/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi index 3515817..8bde91b 100644 --- a/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi +++ b/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi @@ -78,8 +78,13 @@ cps_eth0: eth0 { interrupts = , +, +, +, +, ; - interrupt-names = "rx-shared", "link"; + interrupt-names = "tx-cpu0", "tx-cpu1", "tx-cpu2", + "tx-cpu3", "rx-shared", "link"; port-id = <0>; gop-port-id = <0>; status = "disabled"; @@ -87,8 +92,13 @@ cps_eth1: eth1 { interrupts = , +
[PATCH 4/8] net: mvpp2: move from cpu-centric naming to "software thread" naming
The PPv2.2 IP has a concept of "software thread", with all registers of the PPv2.2 mapped 8 times, for concurrent accesses by 8 "software threads". In addition, interrupts on RX queues are associated to such "software thread". For most cases, we map a "software thread" to the more conventional concept of CPU, but we will soon have one exception: we will have a model where we have one TX interrupt per CPU (each using one software thread), and all RX events mapped to another software thread (associated to another interrupt). In preparation for this change, it makes sense to change the naming from MVPP2_MAX_CPUS to MVPP2_MAX_THREADS, and plan for 8 software threads instead of 4 currently. Signed-off-by: Thomas Petazzoni --- drivers/net/ethernet/marvell/mvpp2.c | 25 + 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c index 8479269..7d37869 100644 --- a/drivers/net/ethernet/marvell/mvpp2.c +++ b/drivers/net/ethernet/marvell/mvpp2.c @@ -748,7 +748,7 @@ enum mvpp2_prs_l3_cast { #define MVPP21_ADDR_SPACE_SZ 0 #define MVPP22_ADDR_SPACE_SZ SZ_64K -#define MVPP2_MAX_CPUS 4 +#define MVPP2_MAX_THREADS 8 enum mvpp2_bm_type { MVPP2_BM_FREE, @@ -764,11 +764,12 @@ struct mvpp2 { void __iomem *lms_base; void __iomem *iface_base; - /* On PPv2.2, each CPU can access the base register through a -* separate address space, each 64 KB apart from each -* other. + /* On PPv2.2, each "software thread" can access the base +* register through a separate address space, each 64 KB apart +* from each other. Typically, such address spaces will be +* used per CPU. */ - void __iomem *cpu_base[MVPP2_MAX_CPUS]; + void __iomem *swth_base[MVPP2_MAX_THREADS]; /* On PPv2.2, some port control registers are located into the system * controller space. These registers are accessible through a regmap. @@ -1140,12 +1141,12 @@ struct mvpp2_bm_pool { static void mvpp2_write(struct mvpp2 *priv, u32 offset, u32 data) { - writel(data, priv->cpu_base[0] + offset); + writel(data, priv->swth_base[0] + offset); } static u32 mvpp2_read(struct mvpp2 *priv, u32 offset) { - return readl(priv->cpu_base[0] + offset); + return readl(priv->swth_base[0] + offset); } /* These accessors should be used to access: @@ -1187,13 +1188,13 @@ static u32 mvpp2_read(struct mvpp2 *priv, u32 offset) static void mvpp2_percpu_write(struct mvpp2 *priv, int cpu, u32 offset, u32 data) { - writel(data, priv->cpu_base[cpu] + offset); + writel(data, priv->swth_base[cpu] + offset); } static u32 mvpp2_percpu_read(struct mvpp2 *priv, int cpu, u32 offset) { - return readl(priv->cpu_base[cpu] + offset); + return readl(priv->swth_base[cpu] + offset); } static dma_addr_t mvpp2_txdesc_dma_addr_get(struct mvpp2_port *port, @@ -7282,7 +7283,7 @@ static int mvpp2_probe(struct platform_device *pdev) struct mvpp2 *priv; struct resource *res; void __iomem *base; - int port_count, cpu; + int port_count, i; int err; priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); @@ -7320,12 +7321,12 @@ static int mvpp2_probe(struct platform_device *pdev) priv->sysctrl_base = NULL; } - for_each_present_cpu(cpu) { + for (i = 0; i < MVPP2_MAX_THREADS; i++) { u32 addr_space_sz; addr_space_sz = (priv->hw_version == MVPP21 ? MVPP21_ADDR_SPACE_SZ : MVPP22_ADDR_SPACE_SZ); - priv->cpu_base[cpu] = base + cpu * addr_space_sz; + priv->swth_base[i] = base + i * addr_space_sz; } if (priv->hw_version == MVPP21) -- 2.9.4
[PATCH 5/8] net: mvpp2: introduce queue_vector concept
In preparation to the introduction of TX interrupts and improved RX queue distribution, this commit introduces the concept of "queue vector". A queue vector represents a number of RX and/or TX queues, and an associated NAPI instance and interrupt. This commit currently only creates a single queue_vector, so there are no changes in behavior, but it paves the way for additional queue_vector in the next commits. Signed-off-by: Thomas Petazzoni --- drivers/net/ethernet/marvell/mvpp2.c | 250 +-- 1 file changed, 178 insertions(+), 72 deletions(-) diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c index 7d37869..a8d448b 100644 --- a/drivers/net/ethernet/marvell/mvpp2.c +++ b/drivers/net/ethernet/marvell/mvpp2.c @@ -749,6 +749,7 @@ enum mvpp2_prs_l3_cast { #define MVPP22_ADDR_SPACE_SZ SZ_64K #define MVPP2_MAX_THREADS 8 +#define MVPP2_MAX_QVECSMVPP2_MAX_THREADS enum mvpp2_bm_type { MVPP2_BM_FREE, @@ -821,6 +822,18 @@ struct mvpp2_port_pcpu { struct tasklet_struct tx_done_tasklet; }; +struct mvpp2_queue_vector { + int irq; + struct napi_struct napi; + enum { MVPP2_QUEUE_VECTOR_SHARED, MVPP2_QUEUE_VECTOR_PRIVATE } type; + int sw_thread_id; + u16 sw_thread_mask; + int first_rxq; + int nrxqs; + u32 pending_cause_rx; + struct mvpp2_port *port; +}; + struct mvpp2_port { u8 id; @@ -829,7 +842,6 @@ struct mvpp2_port { */ int gop_id; - int irq; int link_irq; struct mvpp2 *priv; @@ -845,9 +857,6 @@ struct mvpp2_port { int pkt_size; - u32 pending_cause_rx; - struct napi_struct napi; - /* Per-CPU port control */ struct mvpp2_port_pcpu __percpu *pcpu; @@ -869,6 +878,9 @@ struct mvpp2_port { /* Index of first port's physical RXQ */ u8 first_rxq; + + struct mvpp2_queue_vector qvecs[MVPP2_MAX_QVECS]; + unsigned int nqvecs; }; /* The mvpp2_tx_desc and mvpp2_rx_desc structures describe the @@ -4190,22 +4202,40 @@ static int mvpp2_bm_update_mtu(struct net_device *dev, int mtu) static inline void mvpp2_interrupts_enable(struct mvpp2_port *port) { - int cpu, cpu_mask = 0; + int i, sw_thread_mask = 0; + + for (i = 0; i < port->nqvecs; i++) + sw_thread_mask |= port->qvecs[i].sw_thread_mask; - for_each_present_cpu(cpu) - cpu_mask |= 1 << cpu; mvpp2_write(port->priv, MVPP2_ISR_ENABLE_REG(port->id), - MVPP2_ISR_ENABLE_INTERRUPT(cpu_mask)); + MVPP2_ISR_ENABLE_INTERRUPT(sw_thread_mask)); } static inline void mvpp2_interrupts_disable(struct mvpp2_port *port) { - int cpu, cpu_mask = 0; + int i, sw_thread_mask = 0; + + for (i = 0; i < port->nqvecs; i++) + sw_thread_mask |= port->qvecs[i].sw_thread_mask; + + mvpp2_write(port->priv, MVPP2_ISR_ENABLE_REG(port->id), + MVPP2_ISR_DISABLE_INTERRUPT(sw_thread_mask)); +} + +static inline void mvpp2_qvec_interrupt_enable(struct mvpp2_queue_vector *qvec) +{ + struct mvpp2_port *port = qvec->port; + + mvpp2_write(port->priv, MVPP2_ISR_ENABLE_REG(port->id), + MVPP2_ISR_ENABLE_INTERRUPT(qvec->sw_thread_mask)); +} + +static inline void mvpp2_qvec_interrupt_disable(struct mvpp2_queue_vector *qvec) +{ + struct mvpp2_port *port = qvec->port; - for_each_present_cpu(cpu) - cpu_mask |= 1 << cpu; mvpp2_write(port->priv, MVPP2_ISR_ENABLE_REG(port->id), - MVPP2_ISR_DISABLE_INTERRUPT(cpu_mask)); + MVPP2_ISR_DISABLE_INTERRUPT(qvec->sw_thread_mask)); } /* Mask the current CPU's Rx/Tx interrupts @@ -5589,11 +5619,11 @@ static int mvpp2_setup_txqs(struct mvpp2_port *port) /* The callback for per-port interrupt */ static irqreturn_t mvpp2_isr(int irq, void *dev_id) { - struct mvpp2_port *port = (struct mvpp2_port *)dev_id; + struct mvpp2_queue_vector *qv = dev_id; - mvpp2_interrupts_disable(port); + mvpp2_qvec_interrupt_disable(qv); - napi_schedule(&port->napi); + napi_schedule(&qv->napi); return IRQ_HANDLED; } @@ -5850,8 +5880,8 @@ static u32 mvpp2_skb_tx_csum(struct mvpp2_port *port, struct sk_buff *skb) } /* Main rx processing */ -static int mvpp2_rx(struct mvpp2_port *port, int rx_todo, - struct mvpp2_rx_queue *rxq) +static int mvpp2_rx(struct mvpp2_port *port, struct napi_struct *napi, + int rx_todo, struct mvpp2_rx_queue *rxq) { struct net_device *dev = port->dev; int rx_received; @@ -5929,7 +5959,7 @@ static int mvpp2_rx(struct mvpp2_port *port, int rx_todo, skb->protocol = eth_type_trans(skb,
[PATCH 2/8] net: mvpp2: remove RX queue group reset code
The RX queue group allocation is anyway re-done later in mvpp2_port_init(), so resetting it in mvpp2_init() is not very useful, and will be annoying as we are going to rework the RX queue group allocation logic. Signed-off-by: Thomas Petazzoni --- drivers/net/ethernet/marvell/mvpp2.c | 17 - 1 file changed, 17 deletions(-) diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c index f5c84f4..eb8853f 100644 --- a/drivers/net/ethernet/marvell/mvpp2.c +++ b/drivers/net/ethernet/marvell/mvpp2.c @@ -7253,23 +7253,6 @@ static int mvpp2_init(struct platform_device *pdev, struct mvpp2 *priv) /* Rx Fifo Init */ mvpp2_rx_fifo_init(priv); - /* Reset Rx queue group interrupt configuration */ - for (i = 0; i < MVPP2_MAX_PORTS; i++) { - if (priv->hw_version == MVPP21) { - mvpp2_write(priv, MVPP21_ISR_RXQ_GROUP_REG(i), - rxq_number); - continue; - } else { - u32 val; - - val = (i << MVPP22_ISR_RXQ_GROUP_INDEX_GROUP_OFFSET); - mvpp2_write(priv, MVPP22_ISR_RXQ_GROUP_INDEX_REG, val); - - val = (rxq_number << MVPP22_ISR_RXQ_SUB_GROUP_SIZE_OFFSET); - mvpp2_write(priv, MVPP22_ISR_RXQ_SUB_GROUP_CONFIG_REG, val); - } - } - if (priv->hw_version == MVPP21) writel(MVPP2_EXT_GLOBAL_CTRL_DEFAULT, priv->lms_base + MVPP2_MNG_EXTENDED_GLOBAL_CTRL_REG); -- 2.9.4
[PATCH 0/8] net: mvpp2: add TX interrupts support
Hello, So far, the mvpp2 driver was using an hrtimer to handle TX completion. This patch series adds support for using TX interrupts (for each CPU) on PPv2.2, the variant of the IP used on Marvell Armada 7K/8K. This series has been tested on Marvell Armada 7K (PPv2.2) and Armada 375 (PPv2.1). Dave: - This series depends on the previous series sent by Antoine Ténart "net: mvpp2: MAC/GoP configuration and optional PHYs". Functionally speaking there is no real dependency, but we touch in a few areas the same piece of code, so I based my patch series on top of Antoine's. - Please do not apply the last patch of this series "arm64: dts: marvell: add TX interrupts for PPv2.2", it will be taken by the ARM mvebu maintainers. Thanks! Thomas Thomas Petazzoni (8): net: mvpp2: fix MVPP21_ISR_RXQ_GROUP_REG definition net: mvpp2: remove RX queue group reset code net: mvpp2: introduce per-port nrxqs/ntxqs variables net: mvpp2: move from cpu-centric naming to "software thread" naming net: mvpp2: introduce queue_vector concept net: mvpp2: add support for TX interrupts and RX queue distribution modes dt-bindings: net: marvell-pp2: update interrupt-names with TX interrupts arm64: dts: marvell: add TX interrupts for PPv2.2 .../devicetree/bindings/net/marvell-pp2.txt| 33 +- .../boot/dts/marvell/armada-cp110-master.dtsi | 21 +- .../arm64/boot/dts/marvell/armada-cp110-slave.dtsi | 21 +- drivers/net/ethernet/marvell/mvpp2.c | 638 +++-- 4 files changed, 534 insertions(+), 179 deletions(-) -- 2.9.4
[PATCH 7/8] dt-bindings: net: marvell-pp2: update interrupt-names with TX interrupts
The PPv2.2 unit has several interrupts used for TX completion notification. This commit updates the Device Tree binding describing this HW block to mention such interrupts. While at it, we update the example to use a recent Device Tree example, that uses interrupts going through the ICU, and not to the GIC directly. Signed-off-by: Thomas Petazzoni --- .../devicetree/bindings/net/marvell-pp2.txt| 33 +- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/Documentation/devicetree/bindings/net/marvell-pp2.txt b/Documentation/devicetree/bindings/net/marvell-pp2.txt index 553aadc..80c4d14 100644 --- a/Documentation/devicetree/bindings/net/marvell-pp2.txt +++ b/Documentation/devicetree/bindings/net/marvell-pp2.txt @@ -44,7 +44,8 @@ Optional properties (port): - marvell,system-controller: a phandle to the system controller. - interrupt-names: if more than a single interrupt for rx is given, must be the name associated to the interrupts listed. Valid - names are: "rx-shared", "link". + names are: "tx-cpu0", "tx-cpu1", "tx-cpu2", "tx-cpu3", + "rx-shared", "link". Example for marvell,armada-375-pp2: @@ -84,22 +85,40 @@ cpm_ethernet: ethernet@0 { clock-names = "pp_clk", "gop_clk", "gp_clk"; eth0: eth0 { - interrupts = ; - interrupt-names = "rx-shared"; + interrupts = , +, +, +, +, +; + interrupt-names = "tx-cpu0", "tx-cpu1", "tx-cpu2", + "tx-cpu3", "rx-shared", "link"; port-id = <0>; gop-port-id = <0>; }; eth1: eth1 { - interrupts = ; - interrupt-names = "rx-shared"; + interrupts = , +, +, +, +, +; + interrupt-names = "tx-cpu0", "tx-cpu1", "tx-cpu2", + "tx-cpu3", "rx-shared", "link"; port-id = <1>; gop-port-id = <2>; }; eth2: eth2 { - interrupts = ; - interrupt-names = "rx-shared"; + interrupts = , +, +, +, +, +; + interrupt-names = "tx-cpu0", "tx-cpu1", "tx-cpu2", + "tx-cpu3", "rx-shared", "link"; port-id = <2>; gop-port-id = <3>; }; -- 2.9.4
Re: [PATCH net-next 10/18] net: mvpp2: use the GoP interrupt for link status changes
Hello, On Mon, 24 Jul 2017 15:48:40 +0200, Antoine Tenart wrote: > + > + port->link_irq = of_irq_get_byname(port_node, "link"); > + if (port->link_irq == -EPROBE_DEFER) { > + err = -EPROBE_DEFER; > + goto err_free_irq; > + } > + if (port->link_irq <= 0) > + /* the link irq is optional */ > + port->link_irq = 0; You need to add the irq_dispose_mapping() call corresponding to this of_irq_get_by_name() in the error path and in the remove path. Best regards, Thomas -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
Re: [PATCH] net: ethernet: stmmac: properly set PS bit in MII configurations during reset
Hello Giuseppe, On Mon, 15 May 2017 16:27:34 +0200, Thomas Petazzoni wrote: > On Wed, 10 May 2017 09:18:17 +0200, Thomas Petazzoni wrote: > > > On Wed, 10 May 2017 09:03:12 +0200, Giuseppe CAVALLARO wrote: > > > > > > Please, read again my patch and the description of the problem that I > > > > have sent. But basically, any solution that does not allow to set the > > > > PS bit between asserting the DMA reset bit and polling for it to clear > > > > will not work for MII PHYs. > > > > > > yes your point was clear to me, I was just wondering if we could find an > > > easier way > > > to solve it w/o changing the API, adding the set_ps and propagating the > > > "interface" > > > inside the DMA reset. > > > > > > Maybe this could be fixed in the glue-logic in some way. Let me know > > > what do you think. > > > > Well, it's more up to you to tell me how you would like this be solved. > > We figured out what the problem was, but I don't know well enough the > > architecture of the driver to decide how the solution to this problem > > should be designed. I made an initial simple proposal to show what is > > needed, but I'm definitely open to suggestions. > > Do you have any suggestion on how to move forward with this? Another kind ping on this topic. I really would like to have the SPEAr600 network support work out of the box in mainline, which currently isn't the case with an MII PHY. I posted a patch that fixes the problem, see https://patchwork.ozlabs.org/patch/755926/, but the feedback I got so far does not give any direction on how to rework the patch to make it acceptable. Would it be possible to get some more feedback? Thanks a lot, Thomas -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
[PATCH net-next 3/3] net: mvpp2: remove mvpp2_pool_refill()
When all a function does is calling another function with the exact same arguments, in the exact same order, you know it's time to remove said function. Which is exactly what this commit does. Signed-off-by: Thomas Petazzoni --- drivers/net/ethernet/marvell/mvpp2.c | 14 +++--- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c index aad763c..48d21c1 100644 --- a/drivers/net/ethernet/marvell/mvpp2.c +++ b/drivers/net/ethernet/marvell/mvpp2.c @@ -3953,14 +3953,6 @@ static inline void mvpp2_bm_pool_put(struct mvpp2_port *port, int pool, put_cpu(); } -/* Refill BM pool */ -static void mvpp2_pool_refill(struct mvpp2_port *port, int pool, - dma_addr_t dma_addr, - phys_addr_t phys_addr) -{ - mvpp2_bm_pool_put(port, pool, dma_addr, phys_addr); -} - /* Allocate buffers for the pool */ static int mvpp2_bm_bufs_add(struct mvpp2_port *port, struct mvpp2_bm_pool *bm_pool, int buf_num) @@ -5015,7 +5007,7 @@ static void mvpp2_rxq_drop_pkts(struct mvpp2_port *port, pool = (status & MVPP2_RXD_BM_POOL_ID_MASK) >> MVPP2_RXD_BM_POOL_ID_OFFS; - mvpp2_pool_refill(port, pool, + mvpp2_bm_pool_put(port, pool, mvpp2_rxdesc_dma_addr_get(port, rx_desc), mvpp2_rxdesc_cookie_get(port, rx_desc)); } @@ -5469,7 +5461,7 @@ static int mvpp2_rx_refill(struct mvpp2_port *port, if (!buf) return -ENOMEM; - mvpp2_pool_refill(port, pool, dma_addr, phys_addr); + mvpp2_bm_pool_put(port, pool, dma_addr, phys_addr); return 0; } @@ -5553,7 +5545,7 @@ static int mvpp2_rx(struct mvpp2_port *port, int rx_todo, dev->stats.rx_errors++; mvpp2_rx_error(port, rx_desc); /* Return the buffer to the pool */ - mvpp2_pool_refill(port, pool, dma_addr, phys_addr); + mvpp2_bm_pool_put(port, pool, dma_addr, phys_addr); continue; } -- 2.9.4
[PATCH net-next 0/3] net: mvpp2: misc improvements
David, Here are a few patches making various small improvements/refactoring in the mvpp2 driver. They are based on today's net-next. Thanks! Thomas Thomas Petazzoni (3): net: mvpp2: add comments about smp_processor_id() usage net: mvpp2: remove unused mvpp2_bm_cookie_pool_set() function net: mvpp2: remove mvpp2_pool_refill() drivers/net/ethernet/marvell/mvpp2.c | 58 1 file changed, 32 insertions(+), 26 deletions(-) -- 2.9.4
[PATCH net-next 1/3] net: mvpp2: add comments about smp_processor_id() usage
A previous commit modified a number of smp_processor_id() used in migration-enabled contexts into get_cpu/put_cpu sections. However, a few smp_processor_id() calls remain in the driver, and this commit adds comments explaining why they can be kept. Signed-off-by: Thomas Petazzoni --- drivers/net/ethernet/marvell/mvpp2.c | 33 + 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c index ca4b55c..4b3bf37 100644 --- a/drivers/net/ethernet/marvell/mvpp2.c +++ b/drivers/net/ethernet/marvell/mvpp2.c @@ -4162,7 +4162,10 @@ static inline void mvpp2_interrupts_disable(struct mvpp2_port *port) MVPP2_ISR_DISABLE_INTERRUPT(cpu_mask)); } -/* Mask the current CPU's Rx/Tx interrupts */ +/* Mask the current CPU's Rx/Tx interrupts + * Called by on_each_cpu(), guaranteed to run with migration disabled, + * using smp_processor_id() is OK. + */ static void mvpp2_interrupts_mask(void *arg) { struct mvpp2_port *port = arg; @@ -4171,7 +4174,10 @@ static void mvpp2_interrupts_mask(void *arg) MVPP2_ISR_RX_TX_MASK_REG(port->id), 0); } -/* Unmask the current CPU's Rx/Tx interrupts */ +/* Unmask the current CPU's Rx/Tx interrupts. + * Called by on_each_cpu(), guaranteed to run with migration disabled, + * using smp_processor_id() is OK. + */ static void mvpp2_interrupts_unmask(void *arg) { struct mvpp2_port *port = arg; @@ -4554,7 +4560,11 @@ mvpp2_txq_next_desc_get(struct mvpp2_tx_queue *txq) return txq->descs + tx_desc; } -/* Update HW with number of aggregated Tx descriptors to be sent */ +/* Update HW with number of aggregated Tx descriptors to be sent + * + * Called only from mvpp2_tx(), so migration is disabled, using + * smp_processor_id() is OK. + */ static void mvpp2_aggr_txq_pend_desc_add(struct mvpp2_port *port, int pending) { /* aggregated access - relevant TXQ number is written in TX desc */ @@ -4565,6 +4575,9 @@ static void mvpp2_aggr_txq_pend_desc_add(struct mvpp2_port *port, int pending) /* Check if there are enough free descriptors in aggregated txq. * If not, update the number of occupied descriptors and repeat the check. + * + * Called only from mvpp2_tx(), so migration is disabled, using + * smp_processor_id() is OK. */ static int mvpp2_aggr_desc_num_check(struct mvpp2 *priv, struct mvpp2_tx_queue *aggr_txq, int num) @@ -4583,7 +4596,12 @@ static int mvpp2_aggr_desc_num_check(struct mvpp2 *priv, return 0; } -/* Reserved Tx descriptors allocation request */ +/* Reserved Tx descriptors allocation request + * + * Called only from mvpp2_txq_reserved_desc_num_proc(), itself called + * only by mvpp2_tx(), so migration is disabled, using + * smp_processor_id() is OK. + */ static int mvpp2_txq_alloc_reserved_desc(struct mvpp2 *priv, struct mvpp2_tx_queue *txq, int num) { @@ -4687,6 +4705,10 @@ static u32 mvpp2_txq_desc_csum(int l3_offs, int l3_proto, /* Get number of sent descriptors and decrement counter. * The number of sent descriptors is returned. * Per-CPU access + * + * Called only from mvpp2_txq_done(), called from mvpp2_tx() + * (migration disabled) and from the TX completion tasklet (migration + * disabled) so using smp_processor_id() is OK. */ static inline int mvpp2_txq_sent_desc_proc(struct mvpp2_port *port, struct mvpp2_tx_queue *txq) @@ -4701,6 +4723,9 @@ static inline int mvpp2_txq_sent_desc_proc(struct mvpp2_port *port, MVPP2_TRANSMITTED_COUNT_OFFSET; } +/* Called through on_each_cpu(), so runs on all CPUs, with migration + * disabled, therefore using smp_processor_id() is OK. + */ static void mvpp2_txq_sent_counter_clear(void *arg) { struct mvpp2_port *port = arg; -- 2.9.4
[PATCH net-next 2/3] net: mvpp2: remove unused mvpp2_bm_cookie_pool_set() function
This function is not used in the driver, remove it. Signed-off-by: Thomas Petazzoni --- drivers/net/ethernet/marvell/mvpp2.c | 11 --- 1 file changed, 11 deletions(-) diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c index 4b3bf37..aad763c 100644 --- a/drivers/net/ethernet/marvell/mvpp2.c +++ b/drivers/net/ethernet/marvell/mvpp2.c @@ -3917,17 +3917,6 @@ static void *mvpp2_buf_alloc(struct mvpp2_port *port, return data; } -/* Set pool number in a BM cookie */ -static inline u32 mvpp2_bm_cookie_pool_set(u32 cookie, int pool) -{ - u32 bm; - - bm = cookie & ~(0xFF << MVPP2_BM_COOKIE_POOL_OFFS); - bm |= ((pool & 0xFF) << MVPP2_BM_COOKIE_POOL_OFFS); - - return bm; -} - /* Release buffer to BM */ static inline void mvpp2_bm_pool_put(struct mvpp2_port *port, int pool, dma_addr_t buf_dma_addr, -- 2.9.4
[PATCH v2 0/2] net: mvpp2: driver fixes
Hello, As requested, here is a series of patches containing only bug fixes for the mvpp2 driver. It is based on the latest "net" branch. Changes since v1: - Fixed a build breakage that occurred when only PATCH 1 was only, and not later patches in the series. Was reported by the kbuild report on the first submission. - Added Tested-by from Marc Zyngier on PATCH 2. Thanks! Thomas Thomas Petazzoni (2): net: mvpp2: remove mvpp2_bm_cookie_{build,pool_get} net: mvpp2: use {get,put}_cpu() instead of smp_processor_id() drivers/net/ethernet/marvell/mvpp2.c | 74 1 file changed, 33 insertions(+), 41 deletions(-) -- 2.9.4
[PATCH v2 2/2] net: mvpp2: use {get,put}_cpu() instead of smp_processor_id()
smp_processor_id() should not be used in migration-enabled contexts. We originally thought it was OK in the specific situation of this driver, but it was wrong, and calling smp_processor_id() in a migration-enabled context prints a big fat warning when CONFIG_DEBUG_PREEMPT=y. Therefore, this commit replaces the smp_processor_id() in migration-enabled contexts by the appropriate get_cpu/put_cpu sections. Reported-by: Marc Zyngier Fixes: a786841df72e ("net: mvpp2: handle register mapping and access for PPv2.2") Signed-off-by: Thomas Petazzoni Tested-by: Marc Zyngier --- drivers/net/ethernet/marvell/mvpp2.c | 27 +++ 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c index 5841e53..33c9016 100644 --- a/drivers/net/ethernet/marvell/mvpp2.c +++ b/drivers/net/ethernet/marvell/mvpp2.c @@ -3719,7 +3719,7 @@ static void mvpp2_bm_bufs_get_addrs(struct device *dev, struct mvpp2 *priv, dma_addr_t *dma_addr, phys_addr_t *phys_addr) { - int cpu = smp_processor_id(); + int cpu = get_cpu(); *dma_addr = mvpp2_percpu_read(priv, cpu, MVPP2_BM_PHY_ALLOC_REG(bm_pool->id)); @@ -3740,6 +3740,8 @@ static void mvpp2_bm_bufs_get_addrs(struct device *dev, struct mvpp2 *priv, if (sizeof(phys_addr_t) == 8) *phys_addr |= (u64)phys_addr_highbits << 32; } + + put_cpu(); } /* Free all buffers from the pool */ @@ -3925,7 +3927,7 @@ static inline void mvpp2_bm_pool_put(struct mvpp2_port *port, int pool, dma_addr_t buf_dma_addr, phys_addr_t buf_phys_addr) { - int cpu = smp_processor_id(); + int cpu = get_cpu(); if (port->priv->hw_version == MVPP22) { u32 val = 0; @@ -3952,6 +3954,8 @@ static inline void mvpp2_bm_pool_put(struct mvpp2_port *port, int pool, MVPP2_BM_VIRT_RLS_REG, buf_phys_addr); mvpp2_percpu_write(port->priv, cpu, MVPP2_BM_PHY_RLS_REG(pool), buf_dma_addr); + + put_cpu(); } /* Refill BM pool */ @@ -4732,7 +4736,7 @@ static void mvpp2_txp_max_tx_size_set(struct mvpp2_port *port) static void mvpp2_rx_pkts_coal_set(struct mvpp2_port *port, struct mvpp2_rx_queue *rxq) { - int cpu = smp_processor_id(); + int cpu = get_cpu(); if (rxq->pkts_coal > MVPP2_OCCUPIED_THRESH_MASK) rxq->pkts_coal = MVPP2_OCCUPIED_THRESH_MASK; @@ -4740,6 +4744,8 @@ static void mvpp2_rx_pkts_coal_set(struct mvpp2_port *port, mvpp2_percpu_write(port->priv, cpu, MVPP2_RXQ_NUM_REG, rxq->id); mvpp2_percpu_write(port->priv, cpu, MVPP2_RXQ_THRESH_REG, rxq->pkts_coal); + + put_cpu(); } static u32 mvpp2_usec_to_cycles(u32 usec, unsigned long clk_hz) @@ -4920,7 +4926,7 @@ static int mvpp2_rxq_init(struct mvpp2_port *port, mvpp2_write(port->priv, MVPP2_RXQ_STATUS_REG(rxq->id), 0); /* Set Rx descriptors queue starting address - indirect access */ - cpu = smp_processor_id(); + cpu = get_cpu(); mvpp2_percpu_write(port->priv, cpu, MVPP2_RXQ_NUM_REG, rxq->id); if (port->priv->hw_version == MVPP21) rxq_dma = rxq->descs_dma; @@ -4929,6 +4935,7 @@ static int mvpp2_rxq_init(struct mvpp2_port *port, mvpp2_percpu_write(port->priv, cpu, MVPP2_RXQ_DESC_ADDR_REG, rxq_dma); mvpp2_percpu_write(port->priv, cpu, MVPP2_RXQ_DESC_SIZE_REG, rxq->size); mvpp2_percpu_write(port->priv, cpu, MVPP2_RXQ_INDEX_REG, 0); + put_cpu(); /* Set Offset */ mvpp2_rxq_offset_set(port, rxq->id, NET_SKB_PAD); @@ -4991,10 +4998,11 @@ static void mvpp2_rxq_deinit(struct mvpp2_port *port, * free descriptor number */ mvpp2_write(port->priv, MVPP2_RXQ_STATUS_REG(rxq->id), 0); - cpu = smp_processor_id(); + cpu = get_cpu(); mvpp2_percpu_write(port->priv, cpu, MVPP2_RXQ_NUM_REG, rxq->id); mvpp2_percpu_write(port->priv, cpu, MVPP2_RXQ_DESC_ADDR_REG, 0); mvpp2_percpu_write(port->priv, cpu, MVPP2_RXQ_DESC_SIZE_REG, 0); + put_cpu(); } /* Create and initialize a Tx queue */ @@ -5017,7 +5025,7 @@ static int mvpp2_txq_init(struct mvpp2_port *port, txq->last_desc = txq->size - 1; /* Set Tx descriptors queue starting address - indirect access */ - cpu = smp_processor_id(); + cpu = get_cpu(); mvpp2_percpu_write(port->priv, cpu, MVPP2_TXQ_NUM_REG, txq->id); mvpp2_percpu_write(port->priv, cpu, MVPP2_TXQ_DESC_ADDR_REG, txq->descs_
[PATCH v2 1/2] net: mvpp2: remove mvpp2_bm_cookie_{build,pool_get}
This commit removes the useless remove mvpp2_bm_cookie_{build,pool_get} functions. All what mvpp2_bm_cookie_build() was doing is compute a 32-bit value by concatenating the pool number and the CPU number... only to get the pool number re-extracted by mvpp2_bm_cookie_pool_get() later on. Instead, just get the pool number directly from RX descriptor status, and pass it to mvpp2_pool_refill() and mvpp2_rx_refill(). This has the added benefit of dropping a smp_processor_id() call in a migration-enabled context, which is wrong, and is the original motivation for making this change. Fixes: 3f518509dedc9 ("ethernet: Add new driver for Marvell Armada 375 network unit") Signed-off-by: Thomas Petazzoni --- drivers/net/ethernet/marvell/mvpp2.c | 47 +++- 1 file changed, 14 insertions(+), 33 deletions(-) diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c index 70bca2a..5841e53 100644 --- a/drivers/net/ethernet/marvell/mvpp2.c +++ b/drivers/net/ethernet/marvell/mvpp2.c @@ -3920,12 +3920,6 @@ static inline u32 mvpp2_bm_cookie_pool_set(u32 cookie, int pool) return bm; } -/* Get pool number from a BM cookie */ -static inline int mvpp2_bm_cookie_pool_get(unsigned long cookie) -{ - return (cookie >> MVPP2_BM_COOKIE_POOL_OFFS) & 0xFF; -} - /* Release buffer to BM */ static inline void mvpp2_bm_pool_put(struct mvpp2_port *port, int pool, dma_addr_t buf_dma_addr, @@ -3961,12 +3955,10 @@ static inline void mvpp2_bm_pool_put(struct mvpp2_port *port, int pool, } /* Refill BM pool */ -static void mvpp2_pool_refill(struct mvpp2_port *port, u32 bm, +static void mvpp2_pool_refill(struct mvpp2_port *port, int pool, dma_addr_t dma_addr, phys_addr_t phys_addr) { - int pool = mvpp2_bm_cookie_pool_get(bm); - mvpp2_bm_pool_put(port, pool, dma_addr, phys_addr); } @@ -4513,21 +4505,6 @@ static void mvpp2_rxq_offset_set(struct mvpp2_port *port, mvpp2_write(port->priv, MVPP2_RXQ_CONFIG_REG(prxq), val); } -/* Obtain BM cookie information from descriptor */ -static u32 mvpp2_bm_cookie_build(struct mvpp2_port *port, -struct mvpp2_rx_desc *rx_desc) -{ - int cpu = smp_processor_id(); - int pool; - - pool = (mvpp2_rxdesc_status_get(port, rx_desc) & - MVPP2_RXD_BM_POOL_ID_MASK) >> - MVPP2_RXD_BM_POOL_ID_OFFS; - - return ((pool & 0xFF) << MVPP2_BM_COOKIE_POOL_OFFS) | - ((cpu & 0xFF) << MVPP2_BM_COOKIE_CPU_OFFS); -} - /* Tx descriptors helper methods */ /* Get pointer to next Tx descriptor to be processed (send) by HW */ @@ -4978,9 +4955,13 @@ static void mvpp2_rxq_drop_pkts(struct mvpp2_port *port, for (i = 0; i < rx_received; i++) { struct mvpp2_rx_desc *rx_desc = mvpp2_rxq_next_desc_get(rxq); - u32 bm = mvpp2_bm_cookie_build(port, rx_desc); + u32 status = mvpp2_rxdesc_status_get(port, rx_desc); + int pool; + + pool = (status & MVPP2_RXD_BM_POOL_ID_MASK) >> + MVPP2_RXD_BM_POOL_ID_OFFS; - mvpp2_pool_refill(port, bm, + mvpp2_pool_refill(port, pool, mvpp2_rxdesc_dma_addr_get(port, rx_desc), mvpp2_rxdesc_cookie_get(port, rx_desc)); } @@ -5418,7 +5399,7 @@ static void mvpp2_rx_csum(struct mvpp2_port *port, u32 status, /* Reuse skb if possible, or allocate a new skb and add it to BM pool */ static int mvpp2_rx_refill(struct mvpp2_port *port, - struct mvpp2_bm_pool *bm_pool, u32 bm) + struct mvpp2_bm_pool *bm_pool, int pool) { dma_addr_t dma_addr; phys_addr_t phys_addr; @@ -5430,7 +5411,7 @@ static int mvpp2_rx_refill(struct mvpp2_port *port, if (!buf) return -ENOMEM; - mvpp2_pool_refill(port, bm, dma_addr, phys_addr); + mvpp2_pool_refill(port, pool, dma_addr, phys_addr); return 0; } @@ -5488,7 +5469,7 @@ static int mvpp2_rx(struct mvpp2_port *port, int rx_todo, unsigned int frag_size; dma_addr_t dma_addr; phys_addr_t phys_addr; - u32 bm, rx_status; + u32 rx_status; int pool, rx_bytes, err; void *data; @@ -5500,8 +5481,8 @@ static int mvpp2_rx(struct mvpp2_port *port, int rx_todo, phys_addr = mvpp2_rxdesc_cookie_get(port, rx_desc); data = (void *)phys_to_virt(phys_addr); - bm = mvpp2_bm_cookie_build(port, rx_desc); - pool = mvpp2_bm_cookie_pool_get(bm); + pool = (rx_status & MVPP2_RXD_BM_POOL_ID_MASK) >> + MVP
[PATCH 5/5] net: mvpp2: remove mvpp2_pool_refill()
When all a function does is calling another function with the exact same arguments, in the exact same order, you know it's time to remove said function. Which is exactly what this commit does. Signed-off-by: Thomas Petazzoni --- drivers/net/ethernet/marvell/mvpp2.c | 14 +++--- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c index 4ca1639..747e3a4 100644 --- a/drivers/net/ethernet/marvell/mvpp2.c +++ b/drivers/net/ethernet/marvell/mvpp2.c @@ -3943,14 +3943,6 @@ static inline void mvpp2_bm_pool_put(struct mvpp2_port *port, int pool, put_cpu(); } -/* Refill BM pool */ -static void mvpp2_pool_refill(struct mvpp2_port *port, int pool, - dma_addr_t dma_addr, - phys_addr_t phys_addr) -{ - mvpp2_bm_pool_put(port, pool, dma_addr, phys_addr); -} - /* Allocate buffers for the pool */ static int mvpp2_bm_bufs_add(struct mvpp2_port *port, struct mvpp2_bm_pool *bm_pool, int buf_num) @@ -4980,7 +4972,7 @@ static void mvpp2_rxq_drop_pkts(struct mvpp2_port *port, pool = (status & MVPP2_RXD_BM_POOL_ID_MASK) >> MVPP2_RXD_BM_POOL_ID_OFFS; - mvpp2_pool_refill(port, pool, + mvpp2_bm_pool_put(port, pool, mvpp2_rxdesc_dma_addr_get(port, rx_desc), mvpp2_rxdesc_cookie_get(port, rx_desc)); } @@ -5434,7 +5426,7 @@ static int mvpp2_rx_refill(struct mvpp2_port *port, if (!buf) return -ENOMEM; - mvpp2_pool_refill(port, pool, dma_addr, phys_addr); + mvpp2_bm_pool_put(port, pool, dma_addr, phys_addr); return 0; } @@ -5518,7 +5510,7 @@ static int mvpp2_rx(struct mvpp2_port *port, int rx_todo, dev->stats.rx_errors++; mvpp2_rx_error(port, rx_desc); /* Return the buffer to the pool */ - mvpp2_pool_refill(port, pool, dma_addr, phys_addr); + mvpp2_bm_pool_put(port, pool, dma_addr, phys_addr); continue; } -- 2.7.4
[PATCH 3/5] net: mvpp2: add comments about smp_processor_id() usage
A previous commit modified a number of smp_processor_id() used in migration-enabled contexts into get_cpu/put_cpu sections. However, a few smp_processor_id() calls remain in the driver, and this commit adds comments explaining why they can be kept. Signed-off-by: Thomas Petazzoni --- drivers/net/ethernet/marvell/mvpp2.c | 33 + 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c index fb6e9dc..037f6bd 100644 --- a/drivers/net/ethernet/marvell/mvpp2.c +++ b/drivers/net/ethernet/marvell/mvpp2.c @@ -4152,7 +4152,10 @@ static inline void mvpp2_interrupts_disable(struct mvpp2_port *port) MVPP2_ISR_DISABLE_INTERRUPT(cpu_mask)); } -/* Mask the current CPU's Rx/Tx interrupts */ +/* Mask the current CPU's Rx/Tx interrupts + * Called by on_each_cpu(), guaranteed to run with migration disabled, + * using smp_processor_id() is OK. + */ static void mvpp2_interrupts_mask(void *arg) { struct mvpp2_port *port = arg; @@ -4161,7 +4164,10 @@ static void mvpp2_interrupts_mask(void *arg) MVPP2_ISR_RX_TX_MASK_REG(port->id), 0); } -/* Unmask the current CPU's Rx/Tx interrupts */ +/* Unmask the current CPU's Rx/Tx interrupts. + * Called by on_each_cpu(), guaranteed to run with migration disabled, + * using smp_processor_id() is OK. + */ static void mvpp2_interrupts_unmask(void *arg) { struct mvpp2_port *port = arg; @@ -4519,7 +4525,11 @@ mvpp2_txq_next_desc_get(struct mvpp2_tx_queue *txq) return txq->descs + tx_desc; } -/* Update HW with number of aggregated Tx descriptors to be sent */ +/* Update HW with number of aggregated Tx descriptors to be sent + * + * Called only from mvpp2_tx(), so migration is disabled, using + * smp_processor_id() is OK. + */ static void mvpp2_aggr_txq_pend_desc_add(struct mvpp2_port *port, int pending) { /* aggregated access - relevant TXQ number is written in TX desc */ @@ -4530,6 +4540,9 @@ static void mvpp2_aggr_txq_pend_desc_add(struct mvpp2_port *port, int pending) /* Check if there are enough free descriptors in aggregated txq. * If not, update the number of occupied descriptors and repeat the check. + * + * Called only from mvpp2_tx(), so migration is disabled, using + * smp_processor_id() is OK. */ static int mvpp2_aggr_desc_num_check(struct mvpp2 *priv, struct mvpp2_tx_queue *aggr_txq, int num) @@ -4548,7 +4561,12 @@ static int mvpp2_aggr_desc_num_check(struct mvpp2 *priv, return 0; } -/* Reserved Tx descriptors allocation request */ +/* Reserved Tx descriptors allocation request + * + * Called only from mvpp2_txq_reserved_desc_num_proc(), itself called + * only by mvpp2_tx(), so migration is disabled, using + * smp_processor_id() is OK. + */ static int mvpp2_txq_alloc_reserved_desc(struct mvpp2 *priv, struct mvpp2_tx_queue *txq, int num) { @@ -4652,6 +4670,10 @@ static u32 mvpp2_txq_desc_csum(int l3_offs, int l3_proto, /* Get number of sent descriptors and decrement counter. * The number of sent descriptors is returned. * Per-CPU access + * + * Called only from mvpp2_txq_done(), called from mvpp2_tx() + * (migration disabled) and from the TX completion tasklet (migration + * disabled) so using smp_processor_id() is OK. */ static inline int mvpp2_txq_sent_desc_proc(struct mvpp2_port *port, struct mvpp2_tx_queue *txq) @@ -4666,6 +4688,9 @@ static inline int mvpp2_txq_sent_desc_proc(struct mvpp2_port *port, MVPP2_TRANSMITTED_COUNT_OFFSET; } +/* Called through on_each_cpu(), so runs on all CPUs, with migration + * disabled, therefore using smp_processor_id() is OK. + */ static void mvpp2_txq_sent_counter_clear(void *arg) { struct mvpp2_port *port = arg; -- 2.7.4
[PATCH 4/5] net: mvpp2: remove unused mvpp2_bm_cookie_pool_set() function
This function is not used in the driver, remove it. Signed-off-by: Thomas Petazzoni --- drivers/net/ethernet/marvell/mvpp2.c | 11 --- 1 file changed, 11 deletions(-) diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c index 037f6bd..4ca1639 100644 --- a/drivers/net/ethernet/marvell/mvpp2.c +++ b/drivers/net/ethernet/marvell/mvpp2.c @@ -3907,17 +3907,6 @@ static void *mvpp2_buf_alloc(struct mvpp2_port *port, return data; } -/* Set pool number in a BM cookie */ -static inline u32 mvpp2_bm_cookie_pool_set(u32 cookie, int pool) -{ - u32 bm; - - bm = cookie & ~(0xFF << MVPP2_BM_COOKIE_POOL_OFFS); - bm |= ((pool & 0xFF) << MVPP2_BM_COOKIE_POOL_OFFS); - - return bm; -} - /* Release buffer to BM */ static inline void mvpp2_bm_pool_put(struct mvpp2_port *port, int pool, dma_addr_t buf_dma_addr, -- 2.7.4
[PATCH 0/5] net: mvpp2: fixes and cleanups
Hello, Here is a small set of fixes/improvements for the mvpp2 driver. The first two patches are fixes: they fix bogus usage of smp_processor_id() in a migration-enabled context. Indeed currently the driver outputs some fat warnings in CONFIG_DEBUG_PREEMPT-enabled kernels. Therefore, some fixes should be pushed to stable. The last three patches are cleanups and not needed for stable, but they stack on top of the fixes. Thanks, Thomas Thomas Petazzoni (5): net: mvpp2: remove mvpp2_bm_cookie_{build,pool_get} net: mvpp2: use {get,put}_cpu() instead of smp_processor_id() net: mvpp2: add comments about smp_processor_id() usage net: mvpp2: remove unused mvpp2_bm_cookie_pool_set() function net: mvpp2: remove mvpp2_pool_refill() drivers/net/ethernet/marvell/mvpp2.c | 126 +-- 1 file changed, 60 insertions(+), 66 deletions(-) -- 2.7.4
[PATCH 2/5] net: mvpp2: use {get,put}_cpu() instead of smp_processor_id()
smp_processor_id() should not be used in migration-enabled contexts. We originally thought it was OK in the specific situation of this driver, but it was wrong, and calling smp_processor_id() in a migration-enabled context prints a big fat warning when CONFIG_DEBUG_PREEMPT=y. Therefore, this commit replaces the smp_processor_id() in migration-enabled contexts by the appropriate get_cpu/put_cpu sections. Reported-by: Marc Zyngier Fixes: a786841df72e ("net: mvpp2: handle register mapping and access for PPv2.2") Signed-off-by: Thomas Petazzoni --- drivers/net/ethernet/marvell/mvpp2.c | 27 +++ 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c index cff45e3..fb6e9dc 100644 --- a/drivers/net/ethernet/marvell/mvpp2.c +++ b/drivers/net/ethernet/marvell/mvpp2.c @@ -3715,7 +3715,7 @@ static void mvpp2_bm_bufs_get_addrs(struct device *dev, struct mvpp2 *priv, dma_addr_t *dma_addr, phys_addr_t *phys_addr) { - int cpu = smp_processor_id(); + int cpu = get_cpu(); *dma_addr = mvpp2_percpu_read(priv, cpu, MVPP2_BM_PHY_ALLOC_REG(bm_pool->id)); @@ -3736,6 +3736,8 @@ static void mvpp2_bm_bufs_get_addrs(struct device *dev, struct mvpp2 *priv, if (sizeof(phys_addr_t) == 8) *phys_addr |= (u64)phys_addr_highbits << 32; } + + put_cpu(); } /* Free all buffers from the pool */ @@ -3921,7 +3923,7 @@ static inline void mvpp2_bm_pool_put(struct mvpp2_port *port, int pool, dma_addr_t buf_dma_addr, phys_addr_t buf_phys_addr) { - int cpu = smp_processor_id(); + int cpu = get_cpu(); if (port->priv->hw_version == MVPP22) { u32 val = 0; @@ -3948,6 +3950,8 @@ static inline void mvpp2_bm_pool_put(struct mvpp2_port *port, int pool, MVPP2_BM_VIRT_RLS_REG, buf_phys_addr); mvpp2_percpu_write(port->priv, cpu, MVPP2_BM_PHY_RLS_REG(pool), buf_dma_addr); + + put_cpu(); } /* Refill BM pool */ @@ -4730,7 +4734,7 @@ static void mvpp2_txp_max_tx_size_set(struct mvpp2_port *port) static void mvpp2_rx_pkts_coal_set(struct mvpp2_port *port, struct mvpp2_rx_queue *rxq) { - int cpu = smp_processor_id(); + int cpu = get_cpu(); if (rxq->pkts_coal > MVPP2_OCCUPIED_THRESH_MASK) rxq->pkts_coal = MVPP2_OCCUPIED_THRESH_MASK; @@ -4738,6 +4742,8 @@ static void mvpp2_rx_pkts_coal_set(struct mvpp2_port *port, mvpp2_percpu_write(port->priv, cpu, MVPP2_RXQ_NUM_REG, rxq->id); mvpp2_percpu_write(port->priv, cpu, MVPP2_RXQ_THRESH_REG, rxq->pkts_coal); + + put_cpu(); } static u32 mvpp2_usec_to_cycles(u32 usec, unsigned long clk_hz) @@ -4918,7 +4924,7 @@ static int mvpp2_rxq_init(struct mvpp2_port *port, mvpp2_write(port->priv, MVPP2_RXQ_STATUS_REG(rxq->id), 0); /* Set Rx descriptors queue starting address - indirect access */ - cpu = smp_processor_id(); + cpu = get_cpu(); mvpp2_percpu_write(port->priv, cpu, MVPP2_RXQ_NUM_REG, rxq->id); if (port->priv->hw_version == MVPP21) rxq_dma = rxq->descs_dma; @@ -4927,6 +4933,7 @@ static int mvpp2_rxq_init(struct mvpp2_port *port, mvpp2_percpu_write(port->priv, cpu, MVPP2_RXQ_DESC_ADDR_REG, rxq_dma); mvpp2_percpu_write(port->priv, cpu, MVPP2_RXQ_DESC_SIZE_REG, rxq->size); mvpp2_percpu_write(port->priv, cpu, MVPP2_RXQ_INDEX_REG, 0); + put_cpu(); /* Set Offset */ mvpp2_rxq_offset_set(port, rxq->id, NET_SKB_PAD); @@ -4989,10 +4996,11 @@ static void mvpp2_rxq_deinit(struct mvpp2_port *port, * free descriptor number */ mvpp2_write(port->priv, MVPP2_RXQ_STATUS_REG(rxq->id), 0); - cpu = smp_processor_id(); + cpu = get_cpu(); mvpp2_percpu_write(port->priv, cpu, MVPP2_RXQ_NUM_REG, rxq->id); mvpp2_percpu_write(port->priv, cpu, MVPP2_RXQ_DESC_ADDR_REG, 0); mvpp2_percpu_write(port->priv, cpu, MVPP2_RXQ_DESC_SIZE_REG, 0); + put_cpu(); } /* Create and initialize a Tx queue */ @@ -5015,7 +5023,7 @@ static int mvpp2_txq_init(struct mvpp2_port *port, txq->last_desc = txq->size - 1; /* Set Tx descriptors queue starting address - indirect access */ - cpu = smp_processor_id(); + cpu = get_cpu(); mvpp2_percpu_write(port->priv, cpu, MVPP2_TXQ_NUM_REG, txq->id); mvpp2_percpu_write(port->priv, cpu, MVPP2_TXQ_DESC_ADDR_REG, txq->descs_dma); @@ -5040,6 +5048,7
[PATCH 1/5] net: mvpp2: remove mvpp2_bm_cookie_{build,pool_get}
This commit removes the useless remove mvpp2_bm_cookie_{build,pool_get} functions. All what mvpp2_bm_cookie_build() was doing is compute a 32-bit value by concatenating the pool number and the CPU number... only to get the pool number re-extracted by mvpp2_bm_cookie_pool_get() later on. Instead, just get the pool number directly from RX descriptor status, and pass it to mvpp2_pool_refill() and mvpp2_rx_refill(). This has the added benefit of dropping a smp_processor_id() call in a migration-enabled context, which is wrong, and is the original motivation for making this change. Fixes: 3f518509dedc9 ("ethernet: Add new driver for Marvell Armada 375 network unit") Signed-off-by: Thomas Petazzoni --- drivers/net/ethernet/marvell/mvpp2.c | 51 ++-- 1 file changed, 14 insertions(+), 37 deletions(-) diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c index 9b875d7..cff45e3 100644 --- a/drivers/net/ethernet/marvell/mvpp2.c +++ b/drivers/net/ethernet/marvell/mvpp2.c @@ -666,10 +666,6 @@ enum mvpp2_prs_l3_cast { #define MVPP2_BM_SWF_LONG_POOL(port) ((port > 2) ? 2 : port) #define MVPP2_BM_SWF_SHORT_POOL3 -/* BM cookie (32 bits) definition */ -#define MVPP2_BM_COOKIE_POOL_OFFS 8 -#define MVPP2_BM_COOKIE_CPU_OFFS 24 - /* BM short pool packet size * These value assure that for SWF the total number * of bytes allocated for each buffer will be 512 @@ -3920,12 +3916,6 @@ static inline u32 mvpp2_bm_cookie_pool_set(u32 cookie, int pool) return bm; } -/* Get pool number from a BM cookie */ -static inline int mvpp2_bm_cookie_pool_get(unsigned long cookie) -{ - return (cookie >> MVPP2_BM_COOKIE_POOL_OFFS) & 0xFF; -} - /* Release buffer to BM */ static inline void mvpp2_bm_pool_put(struct mvpp2_port *port, int pool, dma_addr_t buf_dma_addr, @@ -3961,12 +3951,10 @@ static inline void mvpp2_bm_pool_put(struct mvpp2_port *port, int pool, } /* Refill BM pool */ -static void mvpp2_pool_refill(struct mvpp2_port *port, u32 bm, +static void mvpp2_pool_refill(struct mvpp2_port *port, int pool, dma_addr_t dma_addr, phys_addr_t phys_addr) { - int pool = mvpp2_bm_cookie_pool_get(bm); - mvpp2_bm_pool_put(port, pool, dma_addr, phys_addr); } @@ -4515,21 +4503,6 @@ static void mvpp2_rxq_offset_set(struct mvpp2_port *port, mvpp2_write(port->priv, MVPP2_RXQ_CONFIG_REG(prxq), val); } -/* Obtain BM cookie information from descriptor */ -static u32 mvpp2_bm_cookie_build(struct mvpp2_port *port, -struct mvpp2_rx_desc *rx_desc) -{ - int cpu = smp_processor_id(); - int pool; - - pool = (mvpp2_rxdesc_status_get(port, rx_desc) & - MVPP2_RXD_BM_POOL_ID_MASK) >> - MVPP2_RXD_BM_POOL_ID_OFFS; - - return ((pool & 0xFF) << MVPP2_BM_COOKIE_POOL_OFFS) | - ((cpu & 0xFF) << MVPP2_BM_COOKIE_CPU_OFFS); -} - /* Tx descriptors helper methods */ /* Get pointer to next Tx descriptor to be processed (send) by HW */ @@ -4980,9 +4953,13 @@ static void mvpp2_rxq_drop_pkts(struct mvpp2_port *port, for (i = 0; i < rx_received; i++) { struct mvpp2_rx_desc *rx_desc = mvpp2_rxq_next_desc_get(rxq); - u32 bm = mvpp2_bm_cookie_build(port, rx_desc); + u32 status = mvpp2_rxdesc_status_get(port, rx_desc); + int pool; + + pool = (status & MVPP2_RXD_BM_POOL_ID_MASK) >> + MVPP2_RXD_BM_POOL_ID_OFFS; - mvpp2_pool_refill(port, bm, + mvpp2_pool_refill(port, pool, mvpp2_rxdesc_dma_addr_get(port, rx_desc), mvpp2_rxdesc_cookie_get(port, rx_desc)); } @@ -5420,7 +5397,7 @@ static void mvpp2_rx_csum(struct mvpp2_port *port, u32 status, /* Reuse skb if possible, or allocate a new skb and add it to BM pool */ static int mvpp2_rx_refill(struct mvpp2_port *port, - struct mvpp2_bm_pool *bm_pool, u32 bm) + struct mvpp2_bm_pool *bm_pool, int pool) { dma_addr_t dma_addr; phys_addr_t phys_addr; @@ -5432,7 +5409,7 @@ static int mvpp2_rx_refill(struct mvpp2_port *port, if (!buf) return -ENOMEM; - mvpp2_pool_refill(port, bm, dma_addr, phys_addr); + mvpp2_pool_refill(port, pool, dma_addr, phys_addr); return 0; } @@ -5490,7 +5467,7 @@ static int mvpp2_rx(struct mvpp2_port *port, int rx_todo, unsigned int frag_size; dma_addr_t dma_addr; phys_addr_t phys_addr; - u32 bm, rx_status; + u32 rx_status; int pool, rx_bytes, err; void *data; @@ -5502,
Re: [PATCH] net: mvpp2: do not bypass the mvpp22_port_mii_set function
Hello, On Tue, 6 Jun 2017 15:36:15 +0200, Antoine Tenart wrote: > The mvpp22_port_mii_set() function was added by 2697582144dd, but the > function directly returns without doing anything. This return was used > when debugging and wasn't removed before sending the patch. Fix this. > > Signed-off-by: Antoine Tenart Please add: Fixes: 2697582144dd ("net: mvpp2: handle misc PPv2.1/PPv2.2 differences") with this: Acked-by: Thomas Petazzoni I am wondering if we shouldn't Cc: stable as well. I don't think we have seen issues on our side because U-Boot does the necessary initialization, but people using other bootloaders might have issues. Best regards, Thomas -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
Re: [PATCH] net: ethernet: stmmac: properly set PS bit in MII configurations during reset
Hello Giuseppe, On Wed, 10 May 2017 09:18:17 +0200, Thomas Petazzoni wrote: > On Wed, 10 May 2017 09:03:12 +0200, Giuseppe CAVALLARO wrote: > > > > Please, read again my patch and the description of the problem that I > > > have sent. But basically, any solution that does not allow to set the > > > PS bit between asserting the DMA reset bit and polling for it to clear > > > will not work for MII PHYs. > > > > yes your point was clear to me, I was just wondering if we could find an > > easier way > > to solve it w/o changing the API, adding the set_ps and propagating the > > "interface" > > inside the DMA reset. > > > > Maybe this could be fixed in the glue-logic in some way. Let me know > > what do you think. > > Well, it's more up to you to tell me how you would like this be solved. > We figured out what the problem was, but I don't know well enough the > architecture of the driver to decide how the solution to this problem > should be designed. I made an initial simple proposal to show what is > needed, but I'm definitely open to suggestions. Do you have any suggestion on how to move forward with this? Thanks a lot, Thomas -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
Re: [PATCH] net: ethernet: stmmac: properly set PS bit in MII configurations during reset
Hello, On Wed, 10 May 2017 09:03:12 +0200, Giuseppe CAVALLARO wrote: > > Please, read again my patch and the description of the problem that I > > have sent. But basically, any solution that does not allow to set the > > PS bit between asserting the DMA reset bit and polling for it to clear > > will not work for MII PHYs. > > yes your point was clear to me, I was just wondering if we could find an > easier way > to solve it w/o changing the API, adding the set_ps and propagating the > "interface" > inside the DMA reset. > > Maybe this could be fixed in the glue-logic in some way. Let me know > what do you think. Well, it's more up to you to tell me how you would like this be solved. We figured out what the problem was, but I don't know well enough the architecture of the driver to decide how the solution to this problem should be designed. I made an initial simple proposal to show what is needed, but I'm definitely open to suggestions. Best regards, Thomas -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
Re: [PATCH] net: ethernet: stmmac: properly set PS bit in MII configurations during reset
Hello, On Mon, 8 May 2017 16:28:21 +0200, Giuseppe CAVALLARO wrote: > > I just see that GMAC_CONTROL and MAC_CTRL_REG are the same, so why not > > create a custom adjust_link for each dwmac type ? > > This will permit to call it instead of set_ps() and remove lots of if > > (has_gmac) and co in stmmac_adjust_link() > > Basicly replace all between "ctrl = readl()... and writel(ctrl)" by a sot > > of priv->hw->mac->adjust_link() > > > > It will also help a lot for my dwmac-sun8i inclusion (since I add some if > > has_sun8i:)) > > Corentin, I think this is a good idea and maybe necessary now that the > driver is supporting a lot of chips. > In the past it was sufficient to have a adjust link function and a > stmmac_hw_fix_mac_speed > to invoke dedicated hook shared between MAC10/100 and GMAC inside STM > platforms. > > Thomas, I wonder if you could take a look at the > priv->plat->fix_mac_speed. This can be used > for setting internal registers too. Once again, this is not called at the right time to fix the issue I'm seeing with a MII PHY. I need to adjust the PS bit between asserting the reset and polling for the reset bit to clear. ->fix_mac_speed() is called in the adjust_link() call-back, which is called way too late. Please, read again my patch and the description of the problem that I have sent. But basically, any solution that does not allow to set the PS bit between asserting the DMA reset bit and polling for it to clear will not work for MII PHYs. Best regards, Thomas Petazzoni -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
Re: [PATCH] net: ethernet: stmmac: properly set PS bit in MII configurations during reset
Hello Giuseppe, On Wed, 3 May 2017 10:13:56 +0200, Giuseppe CAVALLARO wrote: > this was initially set by using the hw->link.port; both the core_init > and adjust callback > should invoke the hook and tuning the PS bit according to the speed and > mode. But this doesn't work: core_init and adjust callbacks are called too late / not at the appropriate time. Here, I need the PS to be set after asserting the DMA reset bit but *before* polling for the DMA reset bit to clear. I.e, I need: int dwmac_dma_reset(void __iomem *ioaddr, struct mac_device_info *hw, phy_interface_t interface) { u32 value = readl(ioaddr + DMA_BUS_MODE); int limit; /* DMA SW reset */ value |= DMA_BUS_MODE_SFT_RESET; writel(value, ioaddr + DMA_BUS_MODE); /* ===> PS must be set here when the PHY interface is MII */ limit = 10; while (limit--) { if (!(readl(ioaddr + DMA_BUS_MODE) & DMA_BUS_MODE_SFT_RESET)) break; mdelay(10); } if (limit < 0) return -EBUSY; return 0; } Setting PS *before* asserting the DMA reset bit doesn't work, because asserting the DMA reset bit clears all bits in all registers. Setting PS *after* waiting for the DMA reset bit to clear doesn't work, because this bit never clears if the PS configuration is not correct with the regard to the PHY being used (which was my original problem). Am I missing something here? Best regards, Thomas Petazzoni -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
Re: stmmac still supporting spear600 ?
Hello Giuseppe, On Mon, 3 Apr 2017 08:16:50 +0200, Giuseppe CAVALLARO wrote: > I tested the SMSC on other platform (+ stmmac), not on SPEAr. > > ok for reset, keep the radar on clock. Hmm, can you attach a piece of > log file to see the failure? We finally identified the issue: in a MII configuration, the PS bit need to be set for the DMA reset procedure to work, but setting the DMA reset bit clears the PS bit. So you have to set the PS bit after asserting the DMA reset, and before polling for the DMA reset bit to clear. I have sent a fix that works for us (tested GMII and MII platforms), but not sure if the implementation is the most appropriate. Let me know if you have better suggestions. See: http://marc.info/?l=linux-netdev&m=149328635210461&w=2 Thanks! Thomas -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
[PATCH] net: ethernet: stmmac: properly set PS bit in MII configurations during reset
On the SPEAr600 SoC, which has the dwmac1000 variant of the IP block, the DMA reset never succeeds when a MII PHY is used (no problem with a GMII PHY). The dwmac_dma_reset() function sets the DMA_BUS_MODE_SFT_RESET bit in the DMA_BUS_MODE register, and then polls until this bit clears. When a MII PHY is used, with the current driver, this bit never clears and the driver therefore doesn't work. The reason is that the PS bit of the GMAC_CONTROL register should be correctly configured for the DMA reset to work. When the PS bit is 0, it tells the MAC we have a GMII PHY, when the PS bit is 1, it tells the MAC we have a MII PHY. Doing a DMA reset clears all registers, so the PS bit is cleared as well. This makes the DMA reset work fine with a GMII PHY. However, with MII PHY, the PS bit should be set. We have identified this issue thanks to two SPEAr600 platform: - One equipped with a GMII PHY, with which the existing driver was working fine. - One equipped with a MII PHY, where the current driver fails because the DMA reset times out. This patch fixes the problem for the MII PHY configuration, and has been tested with a GMII PHY configuration as well. In terms of implement, since the ->reset() hook is implemented in the DMA related code, we do not want to touch directly from this function the MAC registers. Therefore, a ->set_ps() hook has been added to stmmac_ops, which gets called between the moment the reset is asserted and the polling loop waiting for the reset bit to clear. In order for this ->set_ps() hook to decide what to do, we pass it the "struct mac_device_info" so it can access the MAC registers, and the PHY interface type so it knows if we're using a MII PHY or not. The ->set_ps() hook is only implemented for the dwmac1000 case. Signed-off-by: Thomas Petazzoni Cc: --- Do not hesitate to suggest ideas for alternative implementations, I'm not sure if the current proposal is the one that fits best with the current design of the driver. --- drivers/net/ethernet/stmicro/stmmac/common.h | 12 +--- drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c | 16 drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.h | 3 ++- drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c | 7 ++- drivers/net/ethernet/stmicro/stmmac/dwmac_dma.h | 3 ++- drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c | 6 +- drivers/net/ethernet/stmicro/stmmac/stmmac_main.c| 3 ++- 7 files changed, 42 insertions(+), 8 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h index 04d9245..d576f95 100644 --- a/drivers/net/ethernet/stmicro/stmmac/common.h +++ b/drivers/net/ethernet/stmicro/stmmac/common.h @@ -407,10 +407,13 @@ struct stmmac_desc_ops { extern const struct stmmac_desc_ops enh_desc_ops; extern const struct stmmac_desc_ops ndesc_ops; +struct mac_device_info; + /* Specific DMA helpers */ struct stmmac_dma_ops { /* DMA core initialization */ - int (*reset)(void __iomem *ioaddr); + int (*reset)(void __iomem *ioaddr, struct mac_device_info *hw, +phy_interface_t interface); void (*init)(void __iomem *ioaddr, struct stmmac_dma_cfg *dma_cfg, u32 dma_tx, u32 dma_rx, int atds); /* Configure the AXI Bus Mode Register */ @@ -445,12 +448,15 @@ struct stmmac_dma_ops { void (*enable_tso)(void __iomem *ioaddr, bool en, u32 chan); }; -struct mac_device_info; - /* Helpers to program the MAC core */ struct stmmac_ops { /* MAC core initialization */ void (*core_init)(struct mac_device_info *hw, int mtu); + /* Set port select. Called between asserting DMA reset and +* waiting for the reset bit to clear. +*/ + void (*set_ps)(struct mac_device_info *hw, + phy_interface_t interface); /* Enable and verify that the IPC module is supported */ int (*rx_ipc)(struct mac_device_info *hw); /* Enable RX Queues */ diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c index 19b9b308..dfcbb5b 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c @@ -75,6 +75,21 @@ static void dwmac1000_core_init(struct mac_device_info *hw, int mtu) #endif } +static void dwmac1000_set_ps(struct mac_device_info *hw, +phy_interface_t interface) +{ + void __iomem *ioaddr = hw->pcsr; + u32 value = readl(ioaddr + GMAC_CONTROL); + + /* When a MII PHY is used, we must set the PS bit for the DMA +* reset to succeed. +*/ + if (interface == PHY_INTERFACE_MODE_MII) + value |= GMAC_CONTROL_PS; + + writel(value, ioaddr + GMAC_CONTROL); +} + static int dwmac1000_rx_ipc_enable(struct mac_device_info *hw) {
Re: stmmac still supporting spear600 ?
Viresh, Shiraz, As the SPEAr600 platform maintainers, have you tested Ethernet in recent times, especially with a MII PHY ? I'm still struggling to get it working. Quick summary: - The same kernel works fine on another SPEAr600 platform that has a GMII PHY - The SPEAr600 platform with a MII PHY has Ethernet working fine under U-Boot (TFTP works), so the HW is known to be working. - I've compared the values of the MAC registers between U-Boot and Linux, and traced all the PHY registers read/write over the MDIO bus, and compared the traces between U-Boot and Linux. Found a few differences, but solving them didn't change the problem. I'm suspecting the problem is not directly in the MAC/PHY configuration. Would you have some hints like clock configuration, or other system-level configuration that could affect this? Especially the TXCLK from the PHY is not coming in through the same pin when in GMII and MII mode it seems. Does this needs some special configuration? Any hint/idea would definitely be welcome. Thanks a lot! Thomas On Wed, 12 Apr 2017 15:05:58 +0200, Thomas Petazzoni wrote: > Hello, > > Thanks again for your answer, sorry for the delay, I was away from the > spear600 board for a while. > > On Mon, 3 Apr 2017 08:16:50 +0200, Giuseppe CAVALLARO wrote: > > > I tested the SMSC on other platform (+ stmmac), not on SPEAr. > > OK. But I believe there might be a SPEAr specific issue here, which > might explain why you don't reproduce the problem. > > > ok for reset, keep the radar on clock. Hmm, can you attach a piece of > > log file to see the failure? > > During the boot, nothing bad: > > libphy: Fixed MDIO Bus: probed > stmmaceth e080.ethernet: no reset control found > stmmac - user ID: 0x10, Synopsys ID: 0x32 > Ring mode enabled > DMA HW capability register supported Normal descriptors > libphy: stmmac: probed > eth0: PHY ID 0007c0c4 at 31 IRQ POLL (stmmac-0:1f) active > > Then, when upping the interface: > > # ifconfig eth0 up > eth0: device MAC address 00:30:d3:21:22:60 > stmmaceth e080.ethernet: Failed to reset the dma > stmmac_hw_setup: DMA engine initialization failed > stmmac_open: Hw setup failed > SIOCSIFFLAGS: Device or resource busy > > As I said earlier, the "Failed to reset the dma" is because > dwmac_dma_reset() returns -EBUSY because the DMA reset bit never > clears. Again, we see the same behavior in U-Boot (DMA reset bit never > clears), but Ethernet does work in U-Boot. > > Best regards, > > Thomas -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
Re: stmmac still supporting spear600 ?
Hello, Thanks again for your answer, sorry for the delay, I was away from the spear600 board for a while. On Mon, 3 Apr 2017 08:16:50 +0200, Giuseppe CAVALLARO wrote: > I tested the SMSC on other platform (+ stmmac), not on SPEAr. OK. But I believe there might be a SPEAr specific issue here, which might explain why you don't reproduce the problem. > ok for reset, keep the radar on clock. Hmm, can you attach a piece of > log file to see the failure? During the boot, nothing bad: libphy: Fixed MDIO Bus: probed stmmaceth e080.ethernet: no reset control found stmmac - user ID: 0x10, Synopsys ID: 0x32 Ring mode enabled DMA HW capability register supported Normal descriptors libphy: stmmac: probed eth0: PHY ID 0007c0c4 at 31 IRQ POLL (stmmac-0:1f) active Then, when upping the interface: # ifconfig eth0 up eth0: device MAC address 00:30:d3:21:22:60 stmmaceth e080.ethernet: Failed to reset the dma stmmac_hw_setup: DMA engine initialization failed stmmac_open: Hw setup failed SIOCSIFFLAGS: Device or resource busy As I said earlier, the "Failed to reset the dma" is because dwmac_dma_reset() returns -EBUSY because the DMA reset bit never clears. Again, we see the same behavior in U-Boot (DMA reset bit never clears), but Ethernet does work in U-Boot. Best regards, Thomas -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
Re: [PATCH] net: mvpp2: sleeping function called from invalid context
Hello, Thanks for your patch! However, it is badly line wrapped, you should consider sending your patch with "git send-email". On Sat, 8 Apr 2017 15:07:14 +0200, Gerald Guillaume wrote: > --- linux-3.18/drivers/net/ethernet/marvell/mvpp2.c 2017-04-03 > 10:29:31.863264347 +0200 > +++ linux-3.x/drivers/net/ethernet/marvell/mvpp2.c 2017-04-03 > 10:45:23.008339453 +0200 > @@ -2997,7 +2997,7 @@ > struct mvpp2_prs_entry *pe; > int tid; > > - pe = kzalloc(sizeof(*pe), GFP_KERNEL); > + pe = kzalloc(sizeof(*pe), GFP_ATOMIC); > if (!pe) > return NULL; > mvpp2_prs_tcam_lu_set(pe, MVPP2_PRS_LU_MAC); > @@ -3059,7 +3059,7 @@ > if (tid < 0) > return tid; > > - pe = kzalloc(sizeof(*pe), GFP_KERNEL); > + pe = kzalloc(sizeof(*pe), GFP_ATOMIC); > if (!pe) > return -1; > mvpp2_prs_tcam_lu_set(pe, MVPP2_PRS_LU_MAC); I am wondering if doing a GFP_ATOMIC allocation for this is the right solution. Should it be pre-allocated instead? I would have to look at how other drivers typically do this. Perhaps allocating on the stack is reasonable? After all, sizeof(struct mvpp2_prs_entry) is only: 4 + 6 * 4 + 4 * 4 = 44 bytes. It's allocated at the beginning of the function, and freed at the end, so it really calls for a local variable. Anyway, I think it's worth investigating a different solution than blindly converting to a GFP_ATOMIC allocation. Best regards, Thomas -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
Re: stmmac still supporting spear600 ?
Hello, On Thu, 23 Mar 2017 11:33:23 +0100, Giuseppe CAVALLARO wrote: > > Further research has revealed that everything is working fine on a > > platform with a Gigabit PHY connected via GMII. > > > > However, on a different platform (which I'm using) with a 10/100 PHY > > connected via MII, DMA_RESET never clears, and networking doesn't work. > > The SMSC PHY LAN8700 is also supposed to be providing the clock through > > its TX_CLK pin. I double checked, and both the MAC and PHY are in MII > > mode, but still no luck so far. > > > > Of course, if you have any suggestion or hint, I'm all ears :) > > I can just you to keep the focus on clock configuration. I tested the > SMSC PHY LAN8700 > w/o any issues on several platform. In MII both rx/tx_clk are provided > by PHY and if you > have an external oscillator this should be safe enough, indeed. > Another check you can do is about the reset time! Maybe you need to > change something > when reset the SMSC transceiver, try to increase the delay (if you use > GPIO to reset it). On which platform did you test with the LAN8700 PHY ? Was it on a SPEAr600 based platform ? If you tested on SPEAr600, what is the GMAC clock configuration (i.e the value of the GMAC_CFG_CTR and GMAC_CFG_SYNT registers) ? Regarding the PHY reset time, our PHY reset pin is not connected to a GPIO, but to the system reset logic, so Linux cannot reset the PHY with a GPIO. Thanks! Thomas -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
Re: stmmac still supporting spear600 ?
Hello, On Thu, 9 Mar 2017 15:56:31 +0100, Giuseppe CAVALLARO wrote: > On 3/9/2017 10:32 AM, Thomas Petazzoni wrote: > > > OK, I'll have a look. However, I'm still confused by this DMA_RESET bit > > that never clears, contrary to what the datasheet says. Are there some > > erratas? > > I suggest you to take a look at the tx/rx clocks from PHY. > You have to provide these otherwise you cannot reset the engine. Thanks for the hint. Further research has revealed that everything is working fine on a platform with a Gigabit PHY connected via GMII. However, on a different platform (which I'm using) with a 10/100 PHY connected via MII, DMA_RESET never clears, and networking doesn't work. The SMSC PHY LAN8700 is also supposed to be providing the clock through its TX_CLK pin. I double checked, and both the MAC and PHY are in MII mode, but still no luck so far. Of course, if you have any suggestion or hint, I'm all ears :) Thanks, Thomas -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
Re: stmmac still supporting spear600 ?
Hello Giuseppe, Thanks for your answer. On Thu, 9 Mar 2017 10:09:02 +0100, Giuseppe CAVALLARO wrote: > We do not test stmmac on this spear board since many years > and I guess you have to provide parameters from the platform. Ok, that's what I was afraid of :-/ > In fact, stmmac is recently tested with DT. I'm also using the DT, so this is definitely not the problem. > IIRC, the mac on spear600 could not have the HW cap register Correct. > so it is mandatory to provide all the right parameters and > configurations for this HW. OK, I'll have a look. However, I'm still confused by this DMA_RESET bit that never clears, contrary to what the datasheet says. Are there some erratas? Best regards, Thomas -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
stmmac still supporting spear600 ?
Hello, I'm porting Linux to an old spear600 based platform, and therefore trying to use the stmmac driver on this SoC. Unfortunately, it doesn't work quite well and I'm wondering if the stmmac driver still properly supports the old version of the IP that is used in this SoC. I'm testing with v4.11-rc1. First, the logs: Linux version 4.11.0-rc1 (thomas@skate) (gcc version 5.4.0 20160609 (Ubuntu/Linaro 5.4.0-6ubuntu1~16.04.4) ) #18 Thu Mar 9 09:18:01 CET 2017 [...] libphy: Fixed MDIO Bus: probed stmmaceth e080.ethernet: no reset control found stmmac - user ID: 0x10, Synopsys ID: 0x32 stmmaceth e080.ethernet: Ring mode enabled stmmaceth e080.ethernet: DMA HW capability register supported stmmaceth e080.ethernet: Normal descriptors libphy: stmmac: probed stmmaceth e080.ethernet (unnamed net_device) (uninitialized): PHY ID 0007c0c4 at 31 IRQ POLL (stmmac-0:1f) active [...] # ifconfig eth0 192.168.1.89 stmmaceth e080.ethernet eth0: device MAC address 00:30:d3:21:22:60 Generic PHY stmmac-0:1f: attached PHY driver [Generic PHY] (mii_bus:phy_addr=stmmac-0:1f, irq=-1) stmmaceth e080.ethernet: Failed to reset the dma stmmaceth e080.ethernet eth0: stmmac_hw_setup: DMA engine initialization failed stmmaceth e080.ethernet eth0: stmmac_open: Hw setup failed ifconfig: SIOCSIFFLAGS: Device or resource busy So the reason why it fails to reset the DMA is because dwmac_dma_reset() sets bit DMA_BUS_MODE_SFT_RESET in register DMA_BUS_MODE and waits for this bit to clear, but that never happens. The reason why I'm not sure if this IP is still supported is because dwmac1000_get_hw_feature() reads the DMA_HW_FEATURE register (offset 0x1058), and my spear600 datasheet doesn't mention this register at all. It is worth mentioning that: - Ethernet is working fine under U-Boot (it's an old 2010.03 U-Boot version, with a good number of patches), so I'm sure the HW is working. - Setting bit 1 in the DMA_BUS_MODE register from U-Boot also leaves this bit set forever, it apparently never clears (unless my tests were wrong, which is very possible). In terms of Device Tree, I'm simply using spear600.dtsi, and enabling the gmac node, nothing else. Has the stmmac driver been recently used/tested on spear600 ? Thanks, Thomas Petazzoni -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
Re: [PATCHv3 net-next 21/22] net: mvpp2: set dma mask and coherent dma mask on PPv2.2
Hello, On Tue, 7 Mar 2017 17:24:21 +, David Laight wrote: > Are the coherent mappings just used for ring structures? > If it might be reasonable to allocate them as a single entity, > thus guaranteeing they all reside in a single 4G region. Do we have the guarantee that a DMA coherent allocation will not span a 4G boundary? Thanks, Thomas -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
[PATCHv3 net-next 22/22] net: mvpp2: finally add the PPv2.2 compatible string
Now that the mvpp2 driver has been modified to accommodate the support for PPv2.2, we can finally advertise this support by adding the appropriate compatible string. At the same time, we update the Kconfig description of the MVPP2 driver. Signed-off-by: Thomas Petazzoni --- drivers/net/ethernet/marvell/Kconfig | 4 ++-- drivers/net/ethernet/marvell/mvpp2.c | 4 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/marvell/Kconfig b/drivers/net/ethernet/marvell/Kconfig index d2555e8b..da6fb82 100644 --- a/drivers/net/ethernet/marvell/Kconfig +++ b/drivers/net/ethernet/marvell/Kconfig @@ -82,13 +82,13 @@ config MVNETA_BM that all dependencies are met. config MVPP2 - tristate "Marvell Armada 375 network interface support" + tristate "Marvell Armada 375/7K/8K network interface support" depends on ARCH_MVEBU || COMPILE_TEST depends on HAS_DMA select MVMDIO ---help--- This driver supports the network interface units in the - Marvell ARMADA 375 SoC. + Marvell ARMADA 375, 7K and 8K SoCs. config PXA168_ETH tristate "Marvell pxa168 ethernet support" diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c index 92c47f3..af5bfa1 100644 --- a/drivers/net/ethernet/marvell/mvpp2.c +++ b/drivers/net/ethernet/marvell/mvpp2.c @@ -7037,6 +7037,10 @@ static const struct of_device_id mvpp2_match[] = { .compatible = "marvell,armada-375-pp2", .data = (void *)MVPP21, }, + { + .compatible = "marvell,armada-7k-pp22", + .data = (void *)MVPP22, + }, { } }; MODULE_DEVICE_TABLE(of, mvpp2_match); -- 2.7.4
[PATCHv3 net-next 17/22] net: mvpp2: add AXI bridge initialization for PPv2.2
The PPv2.2 unit is connected to an AXI bus on Armada 7K/8K, so this commit adds the necessary initialization of the AXI bridge. Signed-off-by: Thomas Petazzoni --- drivers/net/ethernet/marvell/mvpp2.c | 85 1 file changed, 85 insertions(+) diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c index bd7dc4b6..0e10303 100644 --- a/drivers/net/ethernet/marvell/mvpp2.c +++ b/drivers/net/ethernet/marvell/mvpp2.c @@ -154,6 +154,34 @@ #define MVPP2_WIN_REMAP(w) (0x4040 + ((w) << 2)) #define MVPP2_BASE_ADDR_ENABLE 0x4060 +/* AXI Bridge Registers */ +#define MVPP22_AXI_BM_WR_ATTR_REG 0x4100 +#define MVPP22_AXI_BM_RD_ATTR_REG 0x4104 +#define MVPP22_AXI_AGGRQ_DESCR_RD_ATTR_REG 0x4110 +#define MVPP22_AXI_TXQ_DESCR_WR_ATTR_REG 0x4114 +#define MVPP22_AXI_TXQ_DESCR_RD_ATTR_REG 0x4118 +#define MVPP22_AXI_RXQ_DESCR_WR_ATTR_REG 0x411c +#define MVPP22_AXI_RX_DATA_WR_ATTR_REG 0x4120 +#define MVPP22_AXI_TX_DATA_RD_ATTR_REG 0x4130 +#define MVPP22_AXI_RD_NORMAL_CODE_REG 0x4150 +#define MVPP22_AXI_RD_SNOOP_CODE_REG 0x4154 +#define MVPP22_AXI_WR_NORMAL_CODE_REG 0x4160 +#define MVPP22_AXI_WR_SNOOP_CODE_REG 0x4164 + +/* Values for AXI Bridge registers */ +#define MVPP22_AXI_ATTR_CACHE_OFFS 0 +#define MVPP22_AXI_ATTR_DOMAIN_OFFS12 + +#define MVPP22_AXI_CODE_CACHE_OFFS 0 +#define MVPP22_AXI_CODE_DOMAIN_OFFS4 + +#define MVPP22_AXI_CODE_CACHE_NON_CACHE0x3 +#define MVPP22_AXI_CODE_CACHE_WR_CACHE 0x7 +#define MVPP22_AXI_CODE_CACHE_RD_CACHE 0xb + +#define MVPP22_AXI_CODE_DOMAIN_OUTER_DOM 2 +#define MVPP22_AXI_CODE_DOMAIN_SYSTEM 3 + /* Interrupt Cause and Mask registers */ #define MVPP2_ISR_RX_THRESHOLD_REG(rxq)(0x5200 + 4 * (rxq)) #define MVPP2_MAX_ISR_RX_THRESHOLD 0xf0 @@ -6664,6 +6692,60 @@ static void mvpp2_rx_fifo_init(struct mvpp2 *priv) mvpp2_write(priv, MVPP2_RX_FIFO_INIT_REG, 0x1); } +static void mvpp2_axi_init(struct mvpp2 *priv) +{ + u32 val, rdval, wrval; + + mvpp2_write(priv, MVPP22_BM_ADDR_HIGH_RLS_REG, 0x0); + + /* AXI Bridge Configuration */ + + rdval = MVPP22_AXI_CODE_CACHE_RD_CACHE + << MVPP22_AXI_ATTR_CACHE_OFFS; + rdval |= MVPP22_AXI_CODE_DOMAIN_OUTER_DOM + << MVPP22_AXI_ATTR_DOMAIN_OFFS; + + wrval = MVPP22_AXI_CODE_CACHE_WR_CACHE + << MVPP22_AXI_ATTR_CACHE_OFFS; + wrval |= MVPP22_AXI_CODE_DOMAIN_OUTER_DOM + << MVPP22_AXI_ATTR_DOMAIN_OFFS; + + /* BM */ + mvpp2_write(priv, MVPP22_AXI_BM_WR_ATTR_REG, wrval); + mvpp2_write(priv, MVPP22_AXI_BM_RD_ATTR_REG, rdval); + + /* Descriptors */ + mvpp2_write(priv, MVPP22_AXI_AGGRQ_DESCR_RD_ATTR_REG, rdval); + mvpp2_write(priv, MVPP22_AXI_TXQ_DESCR_WR_ATTR_REG, wrval); + mvpp2_write(priv, MVPP22_AXI_TXQ_DESCR_RD_ATTR_REG, rdval); + mvpp2_write(priv, MVPP22_AXI_RXQ_DESCR_WR_ATTR_REG, wrval); + + /* Buffer Data */ + mvpp2_write(priv, MVPP22_AXI_TX_DATA_RD_ATTR_REG, rdval); + mvpp2_write(priv, MVPP22_AXI_RX_DATA_WR_ATTR_REG, wrval); + + val = MVPP22_AXI_CODE_CACHE_NON_CACHE + << MVPP22_AXI_CODE_CACHE_OFFS; + val |= MVPP22_AXI_CODE_DOMAIN_SYSTEM + << MVPP22_AXI_CODE_DOMAIN_OFFS; + mvpp2_write(priv, MVPP22_AXI_RD_NORMAL_CODE_REG, val); + mvpp2_write(priv, MVPP22_AXI_WR_NORMAL_CODE_REG, val); + + val = MVPP22_AXI_CODE_CACHE_RD_CACHE + << MVPP22_AXI_CODE_CACHE_OFFS; + val |= MVPP22_AXI_CODE_DOMAIN_OUTER_DOM + << MVPP22_AXI_CODE_DOMAIN_OFFS; + + mvpp2_write(priv, MVPP22_AXI_RD_SNOOP_CODE_REG, val); + + val = MVPP22_AXI_CODE_CACHE_WR_CACHE + << MVPP22_AXI_CODE_CACHE_OFFS; + val |= MVPP22_AXI_CODE_DOMAIN_OUTER_DOM + << MVPP22_AXI_CODE_DOMAIN_OFFS; + + mvpp2_write(priv, MVPP22_AXI_WR_SNOOP_CODE_REG, val); +} + /* Initialize network controller common part HW */ static int mvpp2_init(struct platform_device *pdev, struct mvpp2 *priv) { @@ -6683,6 +6765,9 @@ static int mvpp2_init(struct platform_device *pdev, struct mvpp2 *priv) if (dram_target_info) mvpp2_conf_mbus_windows(dram_target_info, priv); + if (priv->hw_version == MVPP22) + mvpp2_axi_init(priv); + /* Disable HW PHY polling */ if (priv->hw_version == MVPP21) { val = readl(priv->lms_base + MVPP2_PHY_AN_CFG0_REG); -- 2.7.4
[PATCHv3 net-next 15/22] net: mvpp2: handle register mapping and access for PPv2.2
This commit adjusts the mvpp2 driver register mapping and access logic to support PPv2.2, to handle a number of differences. Due to how the registers are laid out in memory, the Device Tree binding for the "reg" property is different: - On PPv2.1, we had a first area for the packet processor registers (common to all ports), and then one area per port. - On PPv2.2, we have a first area for the packet processor registers (common to all ports), and a second area for numerous other registers, including a large number of per-port registers In addition, on PPv2.2, the area for the common registers is split into so-called "address spaces" of 64 KB each. They allow to access per-CPU registers, where each CPU has its own copy of some registers. A few other registers, which have a single copy, also need to be accessed from those per-CPU windows if they are related to a per-CPU register. For example: - Writing to MVPP2_TXQ_NUM_REG selects a TX queue. This register is a per-CPU register, it must be accessed from the current CPU register window. - Then a write to MVPP2_TXQ_PENDING_REG, MVPP2_TXQ_DESC_ADDR_REG (and a few others) will affect the TX queue that was selected by the write to MVPP2_TXQ_NUM_REG. It must be accessed from the same CPU window as the write to the TXQ_NUM_REG. Therefore, the ->base member of 'struct mvpp2' is replaced with a ->cpu_base[] array, each entry pointing to a mapping of the per-CPU area. Since PPv2.1 doesn't have this concept of per-CPU windows, all entries in ->cpu_base[] point to the same io-remapped area. The existing mvpp2_read() and mvpp2_write() accessors use cpu_base[0], they are used for registers for which the CPU window doesn't matter. mvpp2_percpu_read() and mvpp2_percpu_write() are new accessors added to access the registers for which the CPU window does matter, which is why they take a "cpu" as argument. The driver is then changed to use mvpp2_percpu_read() and mvpp2_percpu_write() where it matters. Signed-off-by: Thomas Petazzoni --- drivers/net/ethernet/marvell/mvpp2.c | 257 +-- 1 file changed, 188 insertions(+), 69 deletions(-) diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c index 2eec380..2b4b4f0 100644 --- a/drivers/net/ethernet/marvell/mvpp2.c +++ b/drivers/net/ethernet/marvell/mvpp2.c @@ -295,6 +295,8 @@ #define MVPP2_GMAC_TX_FIFO_MIN_TH_MASK(v) (((v) << 6) & \ MVPP2_GMAC_TX_FIFO_MIN_TH_ALL_MASK) +#define MVPP22_GMAC_BASE(port) (0x7000 + (port) * 0x1000 + 0xe00) + #define MVPP2_CAUSE_TXQ_SENT_DESC_ALL_MASK 0xff /* Descriptor ring Macros */ @@ -622,6 +624,11 @@ enum mvpp2_prs_l3_cast { */ #define MVPP2_BM_SHORT_PKT_SIZEMVPP2_RX_MAX_PKT_SIZE(512) +#define MVPP21_ADDR_SPACE_SZ 0 +#define MVPP22_ADDR_SPACE_SZ SZ_64K + +#define MVPP2_MAX_CPUS 4 + enum mvpp2_bm_type { MVPP2_BM_FREE, MVPP2_BM_SWF_LONG, @@ -633,8 +640,14 @@ enum mvpp2_bm_type { /* Shared Packet Processor resources */ struct mvpp2 { /* Shared registers' base addresses */ - void __iomem *base; void __iomem *lms_base; + void __iomem *iface_base; + + /* On PPv2.2, each CPU can access the base register through a +* separate address space, each 64 KB apart from each +* other. +*/ + void __iomem *cpu_base[MVPP2_MAX_CPUS]; /* Common clocks */ struct clk *pp_clk; @@ -680,6 +693,11 @@ struct mvpp2_port_pcpu { struct mvpp2_port { u8 id; + /* Index of the port from the "group of ports" complex point +* of view +*/ + int gop_id; + int irq; struct mvpp2 *priv; @@ -996,12 +1014,60 @@ static int txq_number = MVPP2_MAX_TXQ; static void mvpp2_write(struct mvpp2 *priv, u32 offset, u32 data) { - writel(data, priv->base + offset); + writel(data, priv->cpu_base[0] + offset); } static u32 mvpp2_read(struct mvpp2 *priv, u32 offset) { - return readl(priv->base + offset); + return readl(priv->cpu_base[0] + offset); +} + +/* These accessors should be used to access: + * + * - per-CPU registers, where each CPU has its own copy of the + * register. + * + * MVPP2_BM_VIRT_ALLOC_REG + * MVPP2_BM_ADDR_HIGH_ALLOC + * MVPP22_BM_ADDR_HIGH_RLS_REG + * MVPP2_BM_VIRT_RLS_REG + * MVPP2_ISR_RX_TX_CAUSE_REG + * MVPP2_ISR_RX_TX_MASK_REG + * MVPP2_TXQ_NUM_REG + * MVPP2_AGGR_TXQ_UPDATE_REG + * MVPP2_TXQ_RSVD_REQ_REG + * MVPP2_TXQ_RSVD_RSLT_REG + * MVPP2_TXQ_SENT_REG + * MVPP2_RXQ_NUM_REG + * + * - global registers that must be accessed through a specific CPU + * window, because they are related to an access to a per-CPU + * register + * + * MVPP2_BM_PHY_ALLOC_REG(related to MVPP2_BM_VIRT_ALLOC_RE
[PATCHv3 net-next 04/22] net: mvpp2: remove unused register definition MVPP2_TXQ_THRESH_REG
This register is no longer used since commit edc660fa09e2 ("net: mvpp2: replace TX coalescing interrupts with hrtimer"). Signed-off-by: Thomas Petazzoni --- drivers/net/ethernet/marvell/mvpp2.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c index fb8b5e9..60cc020 100644 --- a/drivers/net/ethernet/marvell/mvpp2.c +++ b/drivers/net/ethernet/marvell/mvpp2.c @@ -117,9 +117,6 @@ #define MVPP2_TXQ_DESC_SIZE_REG0x2088 #define MVPP2_TXQ_DESC_SIZE_MASK 0x3ff0 #define MVPP2_AGGR_TXQ_UPDATE_REG 0x2090 -#define MVPP2_TXQ_THRESH_REG 0x2094 -#define MVPP2_TRANSMITTED_THRESH_OFFSET16 -#define MVPP2_TRANSMITTED_THRESH_MASK 0x3fff #define MVPP2_TXQ_INDEX_REG0x2098 #define MVPP2_TXQ_PREF_BUF_REG 0x209c #define MVPP2_PREF_BUF_PTR(desc) ((desc) & 0xfff) -- 2.7.4
[PATCHv3 net-next 00/22] net: mvpp2: add initial support for PPv2.2
The goal of this patch series is to add basic support for PPv2.2 in the existing mvpp2 driver. mvpp2 currently supported the PPv2.1 version of the IP, used in the 32 bits Marvell Armada 375 SoC. PPv2.2 is an evolution of this IP block, used in the 64 bits Marvell Armada 7K/8K SoCs. In order to ease the review, the introduction of PPv2.2 support has been made into multiple small commits, with the final commit adding the compatible string that makes the PPv2.2 support actually usable. The series remain fully bisectable. People interested in testing the code will find the full series (plus a few Device Tree patches) at: https://github.com/MISL-EBU-System-SW/mainline-public/tree/4.11/mvpp2.2-support-v3 I'd like to thank Stefan Chulski and Marcin Wojtas, who helped me a lot in the development of this patch series, by reviewing the patches, and giving lots of useful hints to debug the driver on PPv2.2. Thanks as well to Russell King for reviewing previous iterations of this series, and providing suggestions and fixes. Changes between v2 and v3: - Rebased on v4.11-rc1. - Add patch "net: mvpp2: fix DMA address calculation in mvpp2_txq_inc_put()", to properly take into account the "packet offset" field of the TX descriptors. Without this, we were getting DMA_API_DEBUG warnings that we are unmapping DMA mappings with a non-mapped DMA address. - In patch "net: mvpp2: add and use accessors for TX/RX descriptors", add a function named mvpp2_txdesc_offset_get(), which is needed for the DMA address calculation fix. - In patch "net: mvpp2: add and use accessors for TX/RX descriptors", fix the calculation of tx_desc physical address and packet offset in mvpp2_tx_frag_process(). The offset was assigned into the buffer physical address, and the physical address to the packet offset, which meant the fragment process was completely broken. - In patch "net: mvpp2: adjust the allocation/free of BM pools for PPv2.2" fix how MVPP22_BM_ADDR_HIGH_VIRT_RLS_MASK is used. This mask is already shifted. So the value should be shifted before being masked and not the opposite. - Add a new patch "net: mvpp2: set dma mask and coherent dma mask on PPv2.2", to set the DMA mask and DMA coherent mask. By setting the DMA mask to 40 bits we avoid using bounce buffers when network packets are above the 4 GB limit. The coherent mask remains set to 32 bits, because the BM pools must all have the same high 32 bits in their addresses. - Use "dma" instead of "phys" where appropriate, as suggested by Russell King. - Use the "cookie" field of the RX descriptor to store the physical address instead of the virtual address, and then use phys_to_virt() to get the virtual address. This allows to work around the limit that the "cookie" field only has 40 bits, which is not sufficient to store a virtual address on 64 bits platforms. This was suggested by Russell King. As part of this change, also got rid of all the compile time conditionals on CONFIG_ARCH_DMA_ADDR_T_64BIT, to get better compile-time coverage. - In patch "net: mvpp2: handle misc PPv2.1/PPv2.2 differences": * Instead of calling mvpp21_port_power_up(port) only on PPv2.1, remove this function, and call its relevant parts directly from ->probe(). Only mvpp2_port_fc_adv_enable() is PPv2.1 specific. Reported by Russell King. * Add a mvpp22_port_mii_set() function that properly initializes SGMII support on PPv2.2. Code provided by Russell King. - In patch "net: mvpp2: handle register mapping and access for PPv2.2": * Adjust the code to match the change of the DT binding in terms of mapping the second register area on PPv2.2. * Rework the register accessors to remove the get_cpu()/put_cpu(), and instead use separate accessors for global registers vs. per-CPU registers. - Add a few new patches removing dead/unused/useless code: net: mvpp2: remove support for buffer header net: mvpp2: remove unused register definition MVPP2_TXQ_THRESH_REG net: mvpp2: remove mvpp2_txq_pend_desc_num_get() function - Fix a number of checkpatch warnings. Changes between v1 and v2: - Made a separate series from the set of patches doing preparation changes/fixes to the mvpp2 driver. - Rebased on top of v4.10-rc1. - Update Kconfig text of the mvpp2 driver to mention the support for Armada 7K and 8K (PPv2.2). Thomas Petazzoni (22): dt-bindings: net: update Marvell PPv2 binding for PPv2.2 support net: mvpp2: use "dma" instead of "phys" where appropriate net: mvpp2: remove support for buffer header net: mvpp2: remove unused register definition MVPP2_TXQ_THRESH_REG net: mvpp2: remove mvpp2_txq_pend_desc_num_get() function net: mvpp2: store physical address of buffer in rx_desc-&
[PATCHv3 net-next 18/22] net: mvpp2: rework RXQ interrupt group initialization for PPv2.2
This commit adjusts how the MVPP2_ISR_RXQ_GROUP_REG register is configured, since it changed between PPv2.1 and PPv2.2. Signed-off-by: Thomas Petazzoni --- drivers/net/ethernet/marvell/mvpp2.c | 46 1 file changed, 42 insertions(+), 4 deletions(-) diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c index 0e10303..21f47d2 100644 --- a/drivers/net/ethernet/marvell/mvpp2.c +++ b/drivers/net/ethernet/marvell/mvpp2.c @@ -185,7 +185,21 @@ /* Interrupt Cause and Mask registers */ #define MVPP2_ISR_RX_THRESHOLD_REG(rxq)(0x5200 + 4 * (rxq)) #define MVPP2_MAX_ISR_RX_THRESHOLD 0xf0 -#define MVPP2_ISR_RXQ_GROUP_REG(rxq) (0x5400 + 4 * (rxq)) +#define MVPP21_ISR_RXQ_GROUP_REG(rxq) (0x5400 + 4 * (rxq)) + +#define MVPP22_ISR_RXQ_GROUP_INDEX_REG 0x5400 +#define MVPP22_ISR_RXQ_GROUP_INDEX_SUBGROUP_MASK 0xf +#define MVPP22_ISR_RXQ_GROUP_INDEX_GROUP_MASK 0x380 +#define MVPP22_ISR_RXQ_GROUP_INDEX_GROUP_OFFSET 7 + +#define MVPP22_ISR_RXQ_GROUP_INDEX_SUBGROUP_MASK 0xf +#define MVPP22_ISR_RXQ_GROUP_INDEX_GROUP_MASK 0x380 + +#define MVPP22_ISR_RXQ_SUB_GROUP_CONFIG_REG 0x5404 +#define MVPP22_ISR_RXQ_SUB_GROUP_STARTQ_MASK0x1f +#define MVPP22_ISR_RXQ_SUB_GROUP_SIZE_MASK 0xf00 +#define MVPP22_ISR_RXQ_SUB_GROUP_SIZE_OFFSET8 + #define MVPP2_ISR_ENABLE_REG(port) (0x5420 + 4 * (port)) #define MVPP2_ISR_ENABLE_INTERRUPT(mask) ((mask) & 0x) #define MVPP2_ISR_DISABLE_INTERRUPT(mask) (((mask) << 16) & 0x) @@ -6406,7 +6420,18 @@ static int mvpp2_port_init(struct mvpp2_port *port) } /* Configure Rx queue group interrupt for this port */ - mvpp2_write(priv, MVPP2_ISR_RXQ_GROUP_REG(port->id), rxq_number); + if (priv->hw_version == MVPP21) { + mvpp2_write(priv, MVPP21_ISR_RXQ_GROUP_REG(port->id), + rxq_number); + } else { + u32 val; + + val = (port->id << MVPP22_ISR_RXQ_GROUP_INDEX_GROUP_OFFSET); + mvpp2_write(priv, MVPP22_ISR_RXQ_GROUP_INDEX_REG, val); + + val = (rxq_number << MVPP22_ISR_RXQ_SUB_GROUP_SIZE_OFFSET); + mvpp2_write(priv, MVPP22_ISR_RXQ_SUB_GROUP_CONFIG_REG, val); + } /* Create Rx descriptor rings */ for (queue = 0; queue < rxq_number; queue++) { @@ -6799,8 +6824,21 @@ static int mvpp2_init(struct platform_device *pdev, struct mvpp2 *priv) mvpp2_rx_fifo_init(priv); /* Reset Rx queue group interrupt configuration */ - for (i = 0; i < MVPP2_MAX_PORTS; i++) - mvpp2_write(priv, MVPP2_ISR_RXQ_GROUP_REG(i), rxq_number); + for (i = 0; i < MVPP2_MAX_PORTS; i++) { + if (priv->hw_version == MVPP21) { + mvpp2_write(priv, MVPP21_ISR_RXQ_GROUP_REG(i), + rxq_number); + continue; + } else { + u32 val; + + val = (i << MVPP22_ISR_RXQ_GROUP_INDEX_GROUP_OFFSET); + mvpp2_write(priv, MVPP22_ISR_RXQ_GROUP_INDEX_REG, val); + + val = (rxq_number << MVPP22_ISR_RXQ_SUB_GROUP_SIZE_OFFSET); + mvpp2_write(priv, MVPP22_ISR_RXQ_SUB_GROUP_CONFIG_REG, val); + } + } if (priv->hw_version == MVPP21) writel(MVPP2_EXT_GLOBAL_CTRL_DEFAULT, -- 2.7.4
[PATCHv3 net-next 19/22] net: mvpp2: adapt rxq distribution to PPv2.2
In PPv2.1, we have a maximum of 8 RXQs per port, with a default of 4 RXQs per port, and we were assigning RXQs 0->3 to the first port, 4->7 to the second port, 8->11 to the third port, etc. In PPv2.2, we have a maximum of 32 RXQs per port, and we must allocate RXQs from the range of 32 RXQs available for each port. So port 0 must use RXQs in the range 0->31, port 1 in the range 32->63, etc. This commit adapts the mvpp2 to this difference between PPv2.1 and PPv2.2: - The constant definition MVPP2_MAX_RXQ is replaced by a new field 'max_port_rxqs' in 'struct mvpp2', which stores the maximum number of RXQs per port. This field is initialized during ->probe() depending on the IP version. - MVPP2_RXQ_TOTAL_NUM is removed, and instead we calculate the total number of RXQs by multiplying the number of ports by the maximum of RXQs per port. This was anyway used in only one place. - In mvpp2_port_probe(), the calculation of port->first_rxq is adjusted to cope with the different allocation strategy between PPv2.1 and PPv2.2. Due to this change, the 'next_first_rxq' argument of this function is no longer needed and is removed. Signed-off-by: Thomas Petazzoni --- drivers/net/ethernet/marvell/mvpp2.c | 35 +++ 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c index 21f47d2..1bb1aa5 100644 --- a/drivers/net/ethernet/marvell/mvpp2.c +++ b/drivers/net/ethernet/marvell/mvpp2.c @@ -402,15 +402,9 @@ /* Maximum number of TXQs used by single port */ #define MVPP2_MAX_TXQ 8 -/* Maximum number of RXQs used by single port */ -#define MVPP2_MAX_RXQ 8 - /* Dfault number of RXQs in use */ #define MVPP2_DEFAULT_RXQ 4 -/* Total number of RXQs available to all ports */ -#define MVPP2_RXQ_TOTAL_NUM(MVPP2_MAX_PORTS * MVPP2_MAX_RXQ) - /* Max number of Rx descriptors */ #define MVPP2_MAX_RXD 128 @@ -730,6 +724,9 @@ struct mvpp2 { /* HW version */ enum { MVPP21, MVPP22 } hw_version; + + /* Maximum number of RXQs per port */ + unsigned int max_port_rxqs; }; struct mvpp2_pcpu_stats { @@ -6352,7 +6349,8 @@ static int mvpp2_port_init(struct mvpp2_port *port) struct mvpp2_txq_pcpu *txq_pcpu; int queue, cpu, err; - if (port->first_rxq + rxq_number > MVPP2_RXQ_TOTAL_NUM) + if (port->first_rxq + rxq_number > + MVPP2_MAX_PORTS * priv->max_port_rxqs) return -EINVAL; /* Disable port */ @@ -6473,8 +6471,7 @@ static int mvpp2_port_init(struct mvpp2_port *port) /* Ports initialization */ static int mvpp2_port_probe(struct platform_device *pdev, struct device_node *port_node, - struct mvpp2 *priv, - int *next_first_rxq) + struct mvpp2 *priv) { struct device_node *phy_node; struct mvpp2_port *port; @@ -6532,7 +6529,11 @@ static int mvpp2_port_probe(struct platform_device *pdev, port->priv = priv; port->id = id; - port->first_rxq = *next_first_rxq; + if (priv->hw_version == MVPP21) + port->first_rxq = port->id * rxq_number; + else + port->first_rxq = port->id * priv->max_port_rxqs; + port->phy_node = phy_node; port->phy_interface = phy_mode; @@ -6632,8 +6633,6 @@ static int mvpp2_port_probe(struct platform_device *pdev, } netdev_info(dev, "Using %s mac address %pM\n", mac_from, dev->dev_addr); - /* Increment the first Rx queue number to be used by the next port */ - *next_first_rxq += rxq_number; priv->port_list[id] = port; return 0; @@ -6779,7 +6778,7 @@ static int mvpp2_init(struct platform_device *pdev, struct mvpp2 *priv) u32 val; /* Checks for hardware constraints */ - if (rxq_number % 4 || (rxq_number > MVPP2_MAX_RXQ) || + if (rxq_number % 4 || (rxq_number > priv->max_port_rxqs) || (txq_number > MVPP2_MAX_TXQ)) { dev_err(&pdev->dev, "invalid queue size parameter\n"); return -EINVAL; @@ -6870,7 +6869,7 @@ static int mvpp2_probe(struct platform_device *pdev) struct mvpp2 *priv; struct resource *res; void __iomem *base; - int port_count, first_rxq, cpu; + int port_count, cpu; int err; priv = devm_kzalloc(&pdev->dev, sizeof(struct mvpp2), GFP_KERNEL); @@ -6905,6 +6904,11 @@ static int mvpp2_probe(struct platform_device *pdev) priv->cpu_base[cpu] = base + cpu * addr_space_sz; } + if (priv->hw_version == MVPP21) + priv->max_po